aaron.ballman added inline comments.
================
Comment at: lib/Sema/SemaChecking.cpp:11264
+
+ const EnumType *LHSEnumType = LHSStrippedType->getAs<EnumType>();
+ if (!LHSEnumType)
----------------
`const auto *` (and below as well) since the type is spelled out in the
initialization.
================
Comment at: lib/Sema/SemaChecking.cpp:11272-11274
+ if (!LHSEnumType->getDecl()->getIdentifier() &&
+ !LHSEnumType->getDecl()->getTypedefNameForAnonDecl())
+ return;
----------------
Would it make sense to use `!LHSEnumType->getDecl()->hasNameForLinkage()` here?
It seems like that's the situation we care about.
================
Comment at: lib/Sema/SemaChecking.cpp:11785
CheckConditionalOperand(S, E->getFalseExpr(), T, CC, Suspicious);
+ checkConditionalWithEnumTypes(S, E->getBeginLoc(), E->getTrueExpr(),
+ E->getFalseExpr());
----------------
Might make sense to name the new function `CheckConditionalWithEnumTypes()` to
match the local style better.
================
Comment at: test/Sema/warn-conditional-emum-types-mismatch.c:15
+ #else
+ // expected-no-diagnostics'
+ #endif
----------------
Spurious `'`
================
Comment at: test/Sema/warn-conditional-emum-types-mismatch.c:19
+
+int get_flag_anon_enum(int cond) {
+ return cond ? A : C;
----------------
xbolva00 wrote:
> Gcc warns here, but Clang does not warn when A != C..
>
> So not sure here..
My gut reaction is that I think Clang should warn here as well because the code
pattern is confusing, but I'd also say that if there's a lot of false positives
where the code is sensible, it may make sense to suppress the diagnostic. One
situation I was thinking of where you could run into something like this is:
```
enum {
STATUS_SUCCESS,
STATUS_FAILURE,
...
MAX_BASE_STATUS_CODE
};
enum ExtendedStatusCodes {
STATUS_SOMETHING_INTERESTING = MAX_BASE_STATUS_CODE + 1000,
...
};
int whatever(void) {
return some_condition() ? STATUS_SOMETHING_INTERESTING : STATUS_SUCCESS;
}
```
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D67919/new/
https://reviews.llvm.org/D67919
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
