[PATCH] D65695: Implements CWG 1601 in [over.ics.rank/4.2]

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.


================
Comment at: clang/lib/Sema/SemaOverload.cpp:3758
+static QualType
+getFixedEnumUnderlayingType(const StandardConversionSequence &SCS) {
+
----------------
Underlaying -> Underlying (here and below)


================
Comment at: clang/lib/Sema/SemaOverload.cpp:3821-3828
+  QualType UnderlayingType1 = getFixedEnumUnderlayingType(SCS1);
+  QualType UnderlayingType2 = getFixedEnumUnderlayingType(SCS2);
+  if (!UnderlayingType1.isNull() && !UnderlayingType2.isNull()) {
+    if (SCS1.getToType(1) == UnderlayingType1 &&
+        SCS2.getToType(1) != UnderlayingType2)
+      return ImplicitConversionSequence::Better;
+    else if (SCS1.getToType(1) != UnderlayingType1 &&
----------------
I think this would be cleaner if you moved the comparison of the target type against the underlying type into `getFixedEnumUnderlyingType`, and changed it to return an enumeration of "not a fixed enum promotion", or "fixed enum promotion to underlying type", or "fixed enum promotion to promoted underlying type".


================
Comment at: clang/test/CXX/drs/dr16xx.cpp:26
 
+namespace dr1601 { // dr1601: 10 c++11
+#if __cplusplus >= 201103L
----------------
No need for the "c++11" marker here. (We accept fixed underlying types in C++98 as an extension, and your change applies there too.)


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

https://reviews.llvm.org/D65695