paulsemel marked 12 inline comments as done.
paulsemel added inline comments.
================
Comment at: lib/CodeGen/CGBuiltin.cpp:1231
+ Types[getContext().VoidPtrTy] = "%p";
+ Types[getContext().FloatTy] = "%f";
+ Types[getContext().DoubleTy] = "%f";
----------------
aaron.ballman wrote:
> paulsemel wrote:
> > aaron.ballman wrote:
> > > It's unfortunate that you cannot distinguish between `float` and
> > > `double`. Do you need to use the format specifiers exactly?
> > >
> > > What about other type information, like qualifiers or array extents? How
> > > should this handle types that do not exist in this mapping, like
> > > structure or enum types?
> > So, I've think about it. What I am going to do is that if I do not know
> > the type of the field, I am just going to print it as a pointer. I know
> > that it is not the best solution, but I think it's a okay-ish solution
> > until I implement the other types.
> > For the qualifiers, at it is printed the same way with and without those, I
> > can just add the entries in the DenseMap.
> > So, I've think about it. What I am going to do is that if I do not know the
> > type of the field, I am just going to print it as a pointer. I know that it
> > is not the best solution, but I think it's a okay-ish solution until I
> > implement the other types.
>
> Eek. That seems unfortunate. I'm thinking about very common use cases, like:
> ```
> struct S {
> int i, j;
> float x, y;
> };
>
> struct T {
> struct S s;
> int k;
> };
> ```
> Printing out `s` as a pointer seems... not particularly useful.
Yes, I see that this is true for other types that I am not handling for the
moment.. What do you think is the best behavior for those cases ?
Just not print anything and go to the next entry ?
================
Comment at: lib/CodeGen/CGBuiltin.cpp:1218
+ Types[getContext().getConstType(type)] = format; \
+ Types[getContext().getVolatileType(type)] = format; \
+ Types[getContext().getConstType(getContext().getVolatileType(type))] =
format;
----------------
aaron.ballman wrote:
> This seems insufficient, as there are other qualifiers (restrict, ObjC GC
> qualifiers, etc). I think a better way to do this is to call
> `QualType::getUnqualifiedType()` on the type accessing the map.
Yes, I think you're totally right !
================
Comment at: lib/CodeGen/CGBuiltin.cpp:1252
+ Types[getContext().getPointerType(getContext().CharTy)] = "%s";
+ GENERATE_TYPE_QUALIFIERS_NO_RESTRICT(getContext().CharTy, "%s")
+ }
----------------
aaron.ballman wrote:
> What about other types that have format specifiers (ptrdiff_t, size_t,
> intptr_t, char16_t, etc)?
So, for typedef, why not apply the `QualType::getCanonicalType` to our field
type, so that we try to get rid of those intermediate typedefs ?
================
Comment at: lib/Sema/SemaChecking.cpp:1121
+ this->Diag(Arg0->getLocStart(),
diag::err_dump_struct_invalid_argument_type)
+ << Arg0->getType() << "structure pointer type";
+ return ExprError();
----------------
aaron.ballman wrote:
> The string literals should be part of a `%select` in the diagnostic itself
> rather than printed this way.
So, I am now using an other diagnostic definition, so I am constrained to keep
using string literals I think..
================
Comment at: lib/Sema/SemaChecking.cpp:1135
+ this->Diag(Arg1->getLocStart(),
diag::err_dump_struct_invalid_argument_type)
+ << Arg1->getType() << "printf like function pointer type";
+ return ExprError();
----------------
aaron.ballman wrote:
> What is a "printf like function pointer type"?
I have changed this to the actual prototype of a "printf like" function so that
it's way clearer of what I am expecting !
Repository:
rC Clang
https://reviews.llvm.org/D44093
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
