[PATCH] D45615: [builtins] __builtin_dump_struct : added more types format

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Apr 13 05:42:39 PDT 2018


aaron.ballman requested changes to this revision.
aaron.ballman added a comment.
This revision now requires changes to proceed.

In https://reviews.llvm.org/D45615#1066975, @paulsemel wrote:

> In https://reviews.llvm.org/D45615#1066973, @lebedev.ri wrote:
>
> > Tests?
>
>
> I can do this. But currently, I am not testing the formats in the tests..


Now might be a good time to test that, because this patch almost added a bug by missing the length modifiers. Also, all patches should come with some tests to demonstrate the behavioral differences from the current trunk.



================
Comment at: lib/CodeGen/CGBuiltin.cpp:954-955
+    Types[Context.ShortTy] = "%d";
+    Types[Context.UnsignedCharTy] = "%u";
+    Types[Context.UnsignedShortTy] = "%u";
     Types[Context.IntTy] = "%d";
----------------
Can you keep the signed/unsigned ordering?

Also, I think these should have the length modifiers added (`hh` for char).

Finally: `ShortTy` and `UnsignedShortTy` were already handled (see lines 962-963 in this patch), so I don't think those need to be added.


Repository:
  rC Clang

https://reviews.llvm.org/D45615





More information about the cfe-commits mailing list