aaron.ballman added inline comments.

================
Comment at: lib/CodeGen/CGBuiltin.cpp:1231
+      Types[getContext().VoidPtrTy] = "%p";
+      Types[getContext().FloatTy] = "%f";
+      Types[getContext().DoubleTy] = "%f";
----------------
paulsemel wrote:
> 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 ?
I would probably try to recurse into the contained member with another layer of 
`{}` around its fields; if you restructured the code such that it is a 
function, this recursion probably wouldn't be too bad to implement.


================
Comment at: lib/CodeGen/CGBuiltin.cpp:1252
+      Types[getContext().getPointerType(getContext().CharTy)] = "%s";
+      GENERATE_TYPE_QUALIFIERS_NO_RESTRICT(getContext().CharTy, "%s")
+    }
----------------
paulsemel wrote:
> 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 ?
It should be possible to do for type aliases, because you know the canonical 
type information. However, that won't work for builtin types that aren't a 
typedef like `char16_t`.


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

Reply via email to