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

[Micah Weston via Phabricator via cfe-commits]

red1bluelost marked 3 inline comments as done.
red1bluelost added inline comments.


================
Comment at: clang/lib/Sema/SemaChecking.cpp:13555-13557
+    DiagnoseImpCast(S, E, T, CC, DiagID);
+    if (!isa<EnumType>(Target) || !isa<EnumType>(Source))
+      return;
----------------
aaron.ballman wrote:
> I don't think this change is correct -- we *want* to silence the conversion warnings in this case. GCC behaves that way as well: https://godbolt.org/z/oona5Mr5T
Updated the code to silence the sign conversion and only warn the enum-conversion.


================
Comment at: clang/test/Sema/enum-enum-conversion.c:10-14
+#ifdef ENUM_CONV
+  return E; // expected-warning {{implicit conversion from enumeration type 'enum PE1' to different enumeration type 'enum PE2'}}
+#else
+  return E; // expected-warning {{implicit conversion from enumeration type 'enum PE1' to different enumeration type 'enum PE2'}}
+#endif
----------------
aaron.ballman wrote:
> Testing tip: instead of using the preprocessor and duplicating so much, you can use a feature of `-verify` where you give it a prefix. e.g.,
> ```
> // RUN: ... -verify=stuff ...
> // RUN: ... -verify=other ...
> 
> void func(void) {
>   thing();  // stuff-warning {{"this is a warning about stuff}} \
>                other-error {{"this is an error, it's different than stuff}}
> }
> ```
> Using this sort of style makes the tests much easier to read.
Thank! That is helpful to know!
I ended up not needing it here since the silencing of sign-conversion means only one warning will occur.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123009