Correct the diagnostic behavior for unreachable _Generic associations in C++

[aaron.ballman]

Summary

New diagnostics were added for unreachable generic selection expression associations in ca75ac5f04f269def97e6844c2f5c9596b29c84c, but it did not account for a difference in behavior between C and C++ regarding lvalue to rvalue conversions. So we would issue diagnostics about a selection being unreachable and then reach it. This corrects the diagnostic behavior in that case.

[erichkeane]

Took me a bit to get the prose, but I think I got it now. PERHAPS there is use to 'finish the thought' on the outcome there? Either way, LGTM.

clang/lib/Sema/SemaExpr.cpp

1699

This seems like it is a sentence short (or perhaps I'm a few synapses short?).

The fallout from these rules is that "Non-class-types" never have qualifiers, thus cannot match a qualified type, but a class-type can, because it keeps its qualifiers?

[tahonermann]

LGTM!

[aaron.ballman]

clang/lib/Sema/SemaExpr.cpp

1699

The fallout from these rules is that "Non-class-types" never have qualifiers, thus cannot match a qualified type, but a class-type can, because it keeps its qualifiers?

Correct for C++ -- the basic idea is that in C, all qualifiers are stripped, and in C++, only non-class type types have their qualifiers stripped. I'll update the comment when landing.

clang/lib/Sema/SemaExpr.cpp

if (Types[i]) {
           D = diag::err_assoc_type_variably_modified;
         else {
           // Because the controlling expression undergoes lvalue conversion,
           // array conversion, and function conversion, an association which is
           // of array type, function type, or is qualified can never be
           // reached. We will warn about this so users are less surprised by
           // the unreachable association. However, we don't have to handle
           // function types; that's not an object type, so it's handled above.
+          //
+          // The logic is somewhat different for C++ because C++ has different
+          // lvalue to rvalue conversion rules than C. [conv.lvalue]p1 says,
+          // If T is a non-class type, the type of the prvalue is the cv-
+          // unqualified version of T. Otherwise, the type of the prvalue is T.
+          // The result of these rules is that all qualified types in an
+          // association in C are unreachable, and in C++, only qualified non-
+          // class types are unreachable.
           unsigned Reason = 0;
           QualType QT = Types[i]->getType();
           if (QT->isArrayType())
             Reason = 1;
-          else if (QT.hasQualifiers())
+          else if (QT.hasQualifiers() &&
+                   (!LangOpts.CPlusPlus || !QT->isRecordType()))
             Reason = 2;
 
           if (Reason)
             Diag(Types[i]->getTypeLoc().getBeginLoc(),
                  diag::warn_unreachable_association)
                 << QT << (Reason - 1);
         }
 

clang/test/Sema/generic-selection.c

void GH50227(void) {
   // result dependent, and testing whether that controlling expression has side
   // effects would cause a crash.
   _Generic( // ext-warning {{'_Generic' is a C11 extension}}
     n(
       _Generic(n++, int : 0) // expected-error {{cannot increment value of type 'int ()'}} ext-warning {{'_Generic' is a C11 extension}}
     ), int : 0);
 }
 
-void unreachable_associations(const int i) {
+struct Test {
+  int i;
+};
+
+void unreachable_associations(const int i, const struct Test t) {
   _Static_assert( // ext-warning {{'_Static_assert' is a C11 extension}}
     _Generic(i, // ext-warning {{'_Generic' is a C11 extension}}
       const int : 1,    // expected-warning {{due to lvalue conversion of the controlling expression, association of type 'const int' will never be selected because it is qualified}}
       volatile int : 2, // expected-warning {{due to lvalue conversion of the controlling expression, association of type 'volatile int' will never be selected because it is qualified}}
       int[12] : 3,      // expected-warning {{due to lvalue conversion of the controlling expression, association of type 'int[12]' will never be selected because it is of array type}}
       int : 4,
       default : 5
     ) == 4, "we had better pick int!");
+  _Static_assert( // ext-warning {{'_Static_assert' is a C11 extension}}
+    _Generic(t, // ext-warning {{'_Generic' is a C11 extension}}
+      struct Test : 1,
+      const struct Test : 2, // expected-warning {{due to lvalue conversion of the controlling expression, association of type 'const struct Test' will never be selected because it is qualified}}
+      default : 3
+    ) == 1, "we had better pick struct Test, not const struct Test!"); // C-specific result
 }

clang/test/SemaCXX/generic-selection.cpp

template <class... Args> struct TypeMask {
   enum {
    result = Or<_Generic(Args(), int: 1, long: 2, short: 4, float: 8)...>::result
   };
 };
 
 static_assert(TypeMask<int, long, short>::result == 7, "fail");
 static_assert(TypeMask<float, short>::result == 12, "fail");
 static_assert(TypeMask<int, float, float>::result == 9, "fail");
+
+
+struct Test {
+  int i;
+};
+
+void unreachable_associations(const int i, const Test t) {
+  // FIXME: it's not clear to me whether we intended to deviate from the C
+  // semantics in terms of how qualifiers are handled, so this documents the
+  // existing behavior but perhaps not the desired behavior.
+  static_assert(
+    _Generic(i,
+      const int : 1,    // expected-warning {{due to lvalue conversion of the controlling expression, association of type 'const int' will never be selected because it is qualified}}
+      volatile int : 2, // expected-warning {{due to lvalue conversion of the controlling expression, association of type 'volatile int' will never be selected because it is qualified}}
+      int[12] : 3,      // expected-warning {{due to lvalue conversion of the controlling expression, association of type 'int[12]' will never be selected because it is of array type}}
+      int : 4,
+      default : 5
+    ) == 4, "we had better pick int, not const int!");
+  static_assert(
+    _Generic(t,
+      Test : 1,
+      const Test : 2,  // Ok in C++, warned in C
+      default : 3
+    ) == 2, "we had better pick const Test, not Test!"); // C++-specific result
+}