This is an archive of the discontinued LLVM Phabricator instance.

[Sema] Extend -Wenum-compare to handle mixed enum comparisons in switch statements
ClosedPublic

Authored by rnkovacs on Aug 7 2017, 9:26 AM.

Details

Summary

-Wenum-compare warns if two values with different enumeration types are compared in expressions with binary operators. This patch extends this diagnostic so that comparisons of mixed enumeration types are recognized in switch statements as well.

Example:

enum MixedA { A1, A2, A3 };
enum MixedB { B1, B2, B3 };

void MixedEnums(MixedA a) {
  switch (a) {
    case A1: break; // OK, same enum types.
    case B1: break; // Warn, different enum types.
  }
}

Diff Detail

Repository
rL LLVM

Event Timeline

rnkovacs created this revision.Aug 7 2017, 9:26 AM
aaron.ballman edited edge metadata.Aug 8 2017, 8:07 AM

Can you generate the updated patch with more context (https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface)?

lib/Sema/SemaStmt.cpp
746 ↗(On Diff #110019)

Cond and Case can be declared as const pointers.

758 ↗(On Diff #110019)

You can lower this in to the diagnostic.

rnkovacs updated this revision to Diff 110219.Aug 8 2017, 9:42 AM
rnkovacs marked 2 inline comments as done.

Uploaded the full diff and addressed comments. Added const qualifiers to GetTypeBeforeIntegralPromotion() function.

aaron.ballman accepted this revision.Aug 8 2017, 11:03 AM

Aside from a minor naming nit, LGTM!

lib/Sema/SemaStmt.cpp
605–608 ↗(On Diff #110219)

Since you're touching the code -- can you change the names to E (or some other, more descriptive name), Cleanups and Impcast to meet the coding standards?

This revision is now accepted and ready to land.Aug 8 2017, 11:03 AM
This revision was automatically updated to reflect the committed changes.