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

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)

3821–3828

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

3823–3830

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);
 }
 
+/// Returns the underlaying type of a fixed enum of the \a SCS's @c FromType
+/// if the \a SCS uses an integral promotion. Upon failure an empty type is
+/// returned.
+static QualType
+getFixedEnumUnderlayingType(const StandardConversionSequence &SCS) {
+
+  if (SCS.Second != ICK_Integral_Promotion)
+    return QualType();
+
+  QualType FromType = SCS.getFromType();
+  if (!FromType->isEnumeralType())
+    return QualType();
+
+  EnumDecl *Enum = FromType->getAs<EnumType>()->getDecl();
+  if (!Enum->isFixed())
+    return QualType();
+
+  return Enum->getIntegerType();
+}
+
 /// 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)
 {
   // Standard conversion sequence S1 is a better conversion sequence
   // than standard conversion sequence S2 if (C++ 13.3.3.2p3):
 
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.
+  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 &&
+             SCS2.getToType(1) == UnderlayingType2)
+      return 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);

clang/test/CXX/drs/dr16xx.cpp

template<typename E> class initializer_list {
     size_t size;
 
   public:
     initializer_list();
   };
 } // std
 #endif
 
+namespace dr1601 { // dr1601
+#if __cplusplus >= 201103L
+enum E : char { e };
+void f(char);
+void f(int);
+void g() {
+  f(e);
+}
+#endif
+} // 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 dr685 { // dr685: yes
   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*);

clang/www/cxx_dr_status.html

 <!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01//EN"
           "http://www.w3.org/TR/html4/strict.dtd">
 <!-- This file is auto-generated by make_cxx_dr_status. Do not modify. -->
 <html>
 <head>
   <META http-equiv="Content-Type" content="text/html; charset=ISO-8859-1">
   <title>Clang - C++ Defect Report Status</title>
   <link type="text/css" rel="stylesheet" href="menu.css">
   <link type="text/css" rel="stylesheet" href="content.css">
   <style type="text/css">
     .none { background-color: #FFCCCC }
<tr id="1600">
     <td>CD4</td>
     <td>Erroneous reference initialization in example</td>
     <td class="none" align="center">Unknown</td>
   </tr>
   <tr id="1601">
     <td><a href="http://wg21.link/cwg1601">1601</a></td>
     <td>C++14</td>
     <td>Promotion of enumeration with fixed underlying type</td>
-    <td class="none" align="center">Unknown</td>
+    <td class="svn" align="center">SVN (C++11 onwards)</td>
   </tr>
   <tr class="open" id="1602">
     <td><a href="http://wg21.link/cwg1602">1602</a></td>
     <td>open</td>
     <td>Linkage of specialization vs linkage of template arguments</td>
     <td align="center">Not resolved</td>
   </tr>
   <tr id="1603">