[PATCH] D123009: [Sema] Enum conversion warning when one signed and other unsigned.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM aside from some minor corrections. Btw, do you need someone to land this on your behalf? If so, what name and email address would you like used for patch attribution?



================
Comment at: clang/test/Sema/enum-sign-conversion.c:11-14
+// Signed enums
+enum SE1 { N1 = -1 };
+// Unsigned unums
+enum UE1 { P1 };
----------------
Then use `E1` in place of `UE1` -- it was the name `UE1` plus the comment that kept tripping me up.


================
Comment at: clang/test/Sema/enum-sign-conversion.c:6-8
+enum X { A,
+         B,
+         C };
----------------
red1bluelost wrote:
> aaron.ballman wrote:
> > Unrelated formatting change?
> Changed back.
Almost, there's still whitespace changes. :-)


================
Comment at: clang/test/Sema/enum-sign-conversion.c:37-43
+unsigned f5(enum UE1 E) {
+  return E; // signed-warning {{implicit conversion changes signedness: 'enum UE1' to 'unsigned int'}}
+}
+
+enum UE1 f6(unsigned E) {
+  return E; // signed-warning {{implicit conversion changes signedness: 'unsigned int' to 'enum UE1'}}
+}
----------------
red1bluelost wrote:
> aaron.ballman wrote:
> > Shouldn't these not warn as well -- the underlying enum type is unsigned?
> This is the case with 'win32' for the ABI. It has enum be signed as default. I changed the name of the verify to win32 to that it is easier to see that is the case.
Ah, thanks, based on that, I have some other naming suggestions.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123009/new/

https://reviews.llvm.org/D123009