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

[Mordante]

Summary

The overload resolution for enums with a fixed underlying type has changed in the C++14 standard. This patch implements the new rule.

Note: I don't have access to the CWG 1601 paper, but based on https://en.cppreference.com/w/cpp/language/overload_resolution I applied the fix retroactively to C++11.

[Mordante]

clang/www/cxx_dr_status.html

3 ↗

(On Diff #213185)

I'll properly update this file as explained in D65696.

[Mordante]

Add the proper markers in the unit test to update cxx_dr_status.html. As discussed on IRC; the up to date cwg_index.html is not public, so I only updated the unit test and removed the changes to cxx_dr_status.html.

[rsmith]

clang/lib/Sema/SemaOverload.cpp

3758

Underlaying -> Underlying (here and below)

3829–3836

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".

clang/test/CXX/drs/dr16xx.cpp

26

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.)

[Mordante]

clang/test/CXX/drs/dr16xx.cpp

26

I will also update dr685.

[Mordante]

Implemented the changes requested by @rsmith.

[rsmith]

clang/lib/Sema/SemaOverload.cpp

3776

== on QualType compares the precise expression of the type, rather than checking whether types are semantically the same. Use S.Context.hasSameType instead to make this more robust (though I think the difference is not observable because the ToType in the case of a fixed enum promotion always happens to come exactly from calling getIntegerType()).

3831–3838

This logic can be simplified:

if (FEP1 != FixedEnumPromotion::None && FEP2 != FixedEnumPromotion::None && FEP1 != FEP2)
  return FEP1 == FixedEnumPromotion::ToUnderlyingType ? ImplicitConversionSequence::Better
                                                      : ImplicitConversionSequence::Worse;

[Mordante]

Addresses the review remarks:

• Use S.Context.hasSameType for the comparision
• Simplify an if statement

[rsmith]

Thanks!

[Mordante]

Thanks for the review. Can you commit the patch since I don't have commit access?

clang/lib/Sema/SemaOverload.cpp

if (SCS1.BindsImplicitObjectArgumentWithoutRefQualifier ||
     return false;
 
   return (!SCS1.IsLvalueReference && SCS1.BindsToRvalue &&
           SCS2.IsLvalueReference) ||
          (SCS1.IsLvalueReference && SCS1.BindsToFunctionLvalue &&
           !SCS2.IsLvalueReference && SCS2.BindsToFunctionLvalue);
 }
 
+enum class FixedEnumPromotion {
+  None,
+  ToUnderlyingType,
+  ToPromotedUnderlyingType
+};
+
+/// Returns kind of fixed enum promotion the \a SCS uses.
+static FixedEnumPromotion
+getFixedEnumPromtion(Sema &S, const StandardConversionSequence &SCS) {
+
+  if (SCS.Second != ICK_Integral_Promotion)
+    return FixedEnumPromotion::None;
+
+  QualType FromType = SCS.getFromType();
+  if (!FromType->isEnumeralType())
+    return FixedEnumPromotion::None;
+
+  EnumDecl *Enum = FromType->getAs<EnumType>()->getDecl();
+  if (!Enum->isFixed())
+    return FixedEnumPromotion::None;
+
+  QualType UnderlyingType = Enum->getIntegerType();
+  if (S.Context.hasSameType(SCS.getToType(1), UnderlyingType))
+    return FixedEnumPromotion::ToUnderlyingType;
+
+  return FixedEnumPromotion::ToPromotedUnderlyingType;
+}
+
 /// CompareStandardConversionSequences - Compare two standard
 /// conversion sequences to determine whether one is better than the
 /// other or if they are indistinguishable (C++ 13.3.3.2p3).
 static ImplicitConversionSequence::CompareKind
 CompareStandardConversionSequences(Sema &S, SourceLocation Loc,
                                    const StandardConversionSequence& SCS1,
                                    const StandardConversionSequence& SCS2)
 {
CompareStandardConversionSequences(Sema &S, SourceLocation Loc,
   //   A conversion that is not a conversion of a pointer, or
   //   pointer to member, to bool is better than another conversion
   //   that is such a conversion.
   if (SCS1.isPointerConversionToBool() != SCS2.isPointerConversionToBool())
     return SCS2.isPointerConversionToBool()
              ? ImplicitConversionSequence::Better
              : ImplicitConversionSequence::Worse;
 
+  // C++14 [over.ics.rank]p4b2:
+  // This is retroactively applied to C++11 by CWG 1601.
+  //
+  //   A conversion that promotes an enumeration whose underlying type is fixed
+  //   to its underlying type is better than one that promotes to the promoted
+  //   underlying type, if the two are different.
+  FixedEnumPromotion FEP1 = getFixedEnumPromtion(S, SCS1);
+  FixedEnumPromotion FEP2 = getFixedEnumPromtion(S, SCS2);
+  if (FEP1 != FixedEnumPromotion::None && FEP2 != FixedEnumPromotion::None &&
+      FEP1 != FEP2)
+    return FEP1 == FixedEnumPromotion::ToUnderlyingType
+               ? ImplicitConversionSequence::Better
+               : ImplicitConversionSequence::Worse;
+
   // C++ [over.ics.rank]p4b2:
   //
   //   If class B is derived directly or indirectly from class A,
   //   conversion of B* to A* is better than conversion of B* to
   //   void*, and conversion of A* to void* is better than conversion
   //   of B* to void*.
   bool SCS1ConvertsToVoid
     = SCS1.isPointerConversionToVoidPointer(S.Context);
   bool SCS2ConvertsToVoid
     = SCS2.isPointerConversionToVoidPointer(S.Context);

clang/test/CXX/drs/dr16xx.cpp

template<typename E> class initializer_list {
     size_t size;
 
   public:
     initializer_list();
   };
 } // std
 #endif
 
+namespace dr1601 { // dr1601: 10
+enum E : char { e };
+#if __cplusplus < 201103L
+    // expected-error@-2 {{enumeration types with a fixed underlying type are a C++11 extension}}
+#endif
+void f(char);
+void f(int);
+void g() {
+  f(e);
+}
+} // namespace dr1601
+
 namespace dr1611 { // dr1611: dup 1658
   struct A { A(int); };
   struct B : virtual A { virtual void f() = 0; };
   struct C : B { C() : A(0) {} void f(); };
   C c;
 }
 
 namespace dr1684 { // dr1684: 3.6

clang/test/CXX/drs/dr6xx.cpp

 namespace dr684 { // dr684: sup 1454
   void f() {
     int a; // expected-note {{here}}
     constexpr int *p = &a; // expected-error {{constant expression}} expected-note {{pointer to 'a'}}
   }
 }
 #endif
 
-#if __cplusplus >= 201103L
 namespace dr685 { // dr685: yes
   enum E : long { e };
+#if __cplusplus < 201103L
+    // expected-error@-2 {{enumeration types with a fixed underlying type are a C++11 extension}}
+#endif
   void f(int);
   int f(long);
   int a = f(e);
 
   enum G : short { g };
+#if __cplusplus < 201103L
+    // expected-error@-2 {{enumeration types with a fixed underlying type are a C++11 extension}}
+#endif
   int h(short);
   void h(long);
   int b = h(g);
 
   int i(int);
   void i(long);
   int c = i(g);
 
   int j(unsigned int); // expected-note {{candidate}}
   void j(long); // expected-note {{candidate}}
   int d = j(g); // expected-error {{ambiguous}}
 
-  int k(short); // expected-note {{candidate}}
-  void k(int); // expected-note {{candidate}}
-  int x = k(g); // expected-error {{ambiguous}}
+  // Valid per dr1601
+  int k(short);
+  void k(int);
+  int x = k(g);
 }
-#endif
 
 namespace dr686 { // dr686: yes
   void f() {
     (void)dynamic_cast<struct A*>(0); // expected-error {{incomplete}} expected-note {{forward}}
     (void)dynamic_cast<struct A{}*>(0); // expected-error {{cannot be defined in a type specifier}}
     (void)typeid(struct B*);
     (void)typeid(struct B{}*); // expected-error {{cannot be defined in a type specifier}}
     (void)static_cast<struct C*>(0);