Silence GCC 7 warning by using an enum class.
ClosedPublic

Authored by fhahn on Dec 8 2017, 3:10 AM.

Details

Summary

This silences the following GCC7 warning:

lib/Target/Hexagon/HexagonISelDAGToDAGHVX.cpp:142:30: warning:
enumeral and non-enumeral type in conditional expression [-Wextra]
     return F != Colors.end() ? F->second : None;
                ~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~

Diff Detail

Repository
rL LLVM
fhahn created this revision.Dec 8 2017, 3:10 AM
fhahn added a subscriber: RKSimon.

@RKSimon I saw that you committed a couple of changes fixing some GCC warnings in the Hexagon backend. Thank you very much for that! This patch here fixes the same warning as rL320254, in a different way.

fhahn added a comment.Wed, Jan 10, 5:25 AM

ping. Please let me know if you think this should be included, otherwise I am happy to drop this.

davide accepted this revision.Wed, Jan 10, 7:25 AM
davide added a subscriber: davide.

Go ahead if you still see this warning.

This revision is now accepted and ready to land.Wed, Jan 10, 7:25 AM
RKSimon accepted this revision.Wed, Jan 10, 7:58 AM

LGTM

lib/Target/Hexagon/HexagonISelDAGToDAGHVX.cpp
95 ↗(On Diff #126110)

(style) Worth using a CK_ prefix?

enum class ColorKind { CK_None, CK_Red, CK_Black };

Thanks for having a look!

lib/Target/Hexagon/HexagonISelDAGToDAGHVX.cpp
95 ↗(On Diff #126110)

enum classes are strongly scoped, so any access needs to be prefixed with ColorKind::. With that, having a CK_ prefix seems slightly redundant to me. What do you think?

RKSimon added inline comments.Thu, Jan 11, 12:35 PM
fhahn added inline comments.Thu, Jan 11, 12:42 PM
lib/Target/Hexagon/HexagonISelDAGToDAGHVX.cpp
95 ↗(On Diff #126110)

Hm, it says Unless the enumerators are defined in their own small namespace or .... My understanding is that enum class define the enumerators in their own namespace (although the coding standard could explicitly spell out the enum class case). Also, I had a look at a couple of files with enum classes and all enumerators were without prefix.

RKSimon added inline comments.Thu, Jan 11, 12:44 PM
lib/Target/Hexagon/HexagonISelDAGToDAGHVX.cpp
95 ↗(On Diff #126110)

Fair enough.

This revision was automatically updated to reflect the committed changes.