Change __auto_type behavior with qualifiers to match GCC behavior

[aaron.ballman]

Summary

Currently, Clang handles some qualifiers correctly for __auto_type, but it does not handle the restrict or _Atomic qualifiers in the same way that GCC does. This patch handles those qualifiers so that they attach to the deduced type the same as const and volatile already do.

This fixes https://github.com/llvm/llvm-project/issues/53652

[erichkeane]

Just a couple of questions/concerns! Otherwise it LGTM. Since I have a suggestion for a code change, I'll hold off on approval until you do it or give a reasonable reason why not.

clang/lib/AST/ASTContext.cpp

10290

So do we care if BOTH sides are this auto type? Further question: can there be more than 1 of these 'GNUAutoType's in the type system such that 10211 wouldn't fire?

Also, slight preference for AutoType having 'isGNUAutoTYpe' on it instead of this dance everywhere.

Speaking of which: that also makes me concerned about all the places in our code that assume !isDeclTypeAuto means C++ auto.... But that is perhaps for a future bug reporter to discover.

[aaron.ballman]

Updated based on review feedback.

clang/lib/AST/ASTContext.cpp

10290

So do we care if BOTH sides are this auto type?

This only fires when the type classes don't match, see 10272.

Further question: can there be more than 1 of these 'GNUAutoType's in the type system such that 10211 wouldn't fire?

Yes, but we wouldn't hit this block of code in that case because the type classes would match.

Also, slight preference for AutoType having 'isGNUAutoTYpe' on it instead of this dance everywhere.

I was on the fence about that, and this gets me off the fence.

Speaking of which: that also makes me concerned about all the places in our code that assume !isDeclTypeAuto means C++ auto.... But that is perhaps for a future bug reporter to discover.

+1

[erichkeane]

clang/lib/Sema/SemaType.cpp

1929

The RHS here should NEVER be true. I think the entirety of the change in this branch other than the 'if' condition needs to be reverted.

[xbolva00]

clang/lib/AST/ASTContext.cpp

10294

Same?

[erichkeane]

clang/lib/AST/ASTContext.cpp

10294

Oooh, good catch! Looks like it should be RHS->getAs

[aaron.ballman]

Updated based on review feedback.

[aaron.ballman]

clang/lib/AST/ASTContext.cpp

10294

Great catch -- I've fixed and added additional test coverage for it.

[aaron.ballman]

FWIW, I plan to land this sometime tomorrow (Mar 23) unless there are additional review comments before then.

[aaron.ballman]

Thanks for the reviews, I've committed in e4a42c5b64d044ae28d9483b0ebd12038d5b5917.

clang/docs/ReleaseNotes.rst

 Bug Fixes
 ------------------
 - ``CXXNewExpr::getArraySize()`` previously returned a ``llvm::Optional``
   wrapping a ``nullptr`` when the ``CXXNewExpr`` did not have an array
   size expression. This was fixed and ``::getArraySize()`` will now always
   either return ``None`` or a ``llvm::Optional`` wrapping a valid ``Expr*``.
   This fixes `Issue 53742 <https://github.com/llvm/llvm-project/issues/53742>`_.
 
+- Now allow the `restrict` and `_Atomic` qualifiers to be used in conjunction
+  with `__auto_type` to match the behavior in GCC. This fixes
+  `Issue 53652 <https://github.com/llvm/llvm-project/issues/53652>`_.
+
+
 Improvements to Clang's diagnostics
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 - ``-Wliteral-range`` will warn on floating-point equality comparisons with
   constants that are not representable in a casted value. For example,
   ``(float) f == 0.1`` is always false.
 
 Non-comprehensive list of changes in this release
 -------------------------------------------------

clang/include/clang/AST/Type.h

   bool isConstrained() const {
     return TypeConstraintConcept != nullptr;
   }
 
   bool isDecltypeAuto() const {
     return getKeyword() == AutoTypeKeyword::DecltypeAuto;
   }
 
+  bool isGNUAutoType() const {
+    return getKeyword() == AutoTypeKeyword::GNUAutoType;
+  }
+
   AutoTypeKeyword getKeyword() const {
     return (AutoTypeKeyword)AutoTypeBits.Keyword;
   }
 
   void Profile(llvm::FoldingSetNodeID &ID, const ASTContext &Context) {
     Profile(ID, Context, getDeducedType(), getKeyword(), isDependentType(),
             getTypeConstraintConcept(), getTypeConstraintArguments());
   }

clang/lib/AST/ASTContext.cpp

     }
     // allow block pointer type to match an 'id' type.
     if (OfBlockPointer && !BlockReturnType) {
        if (LHS->isObjCIdType() && RHS->isBlockPointerType())
          return LHS;
       if (RHS->isObjCIdType() && LHS->isBlockPointerType())
         return RHS;
     }
-
+    // Allow __auto_type to match anything; it merges to the type with more
+    // information.
+    if (const auto *AT = LHS->getAs<AutoType>()) {
+      if (AT->isGNUAutoType())
+        return RHS;
+    }
+    if (const auto *AT = RHS->getAs<AutoType>()) {
+      if (AT->isGNUAutoType())
+        return LHS;
+    }
     return {};
   }
 
   // The canonical type classes match.
   switch (LHSClass) {
 #define TYPE(Class, Base)
 #define ABSTRACT_TYPE(Class, Base)
 #define NON_CANONICAL_UNLESS_DEPENDENT_TYPE(Class, Base) case Type::Class:

clang/lib/Sema/SemaExpr.cpp

   RHSType = Context.getCanonicalType(RHSType).getUnqualifiedType();
 
   // Common case: no conversion required.
   if (LHSType == RHSType) {
     Kind = CK_NoOp;
     return Compatible;
   }
 
+  // If the LHS has an __auto_type, there are no additional type constraints
+  // to be worried about.
+  if (const auto *AT = dyn_cast<AutoType>(LHSType)) {
+    if (AT->isGNUAutoType()) {
+      Kind = CK_NoOp;
+      return Compatible;
+    }
+  }
+
   // If we have an atomic type, try a non-atomic assignment, then just add an
   // atomic qualification step.
   if (const AtomicType *AtomicTy = dyn_cast<AtomicType>(LHSType)) {
     Sema::AssignConvertType result =
       CheckAssignmentConstraints(AtomicTy->getValueType(), RHS, Kind);
     if (result != Compatible)
       return result;
     if (Kind != CK_NoOp && ConvertRHS)

clang/lib/Sema/SemaType.cpp

 
 static std::string getPrintableNameForEntity(DeclarationName Entity) {
   if (Entity)
     return Entity.getAsString();
 
   return "type name";
 }
 
+static bool isDependentOrGNUAutoType(QualType T) {
+  if (T->isDependentType())
+    return true;
+
+  const auto *AT = dyn_cast<AutoType>(T);
+  return AT && AT->isGNUAutoType();
+}
+
 QualType Sema::BuildQualifiedType(QualType T, SourceLocation Loc,
                                   Qualifiers Qs, const DeclSpec *DS) {
   if (T.isNull())
     return QualType();
 
   // Ignore any attempt to form a cv-qualified reference.
   if (T->isReferenceType()) {
     Qs.removeConst();

         EltTy = T->getPointeeType();
 
       // If we have a pointer or reference, the pointee must have an object
       // incomplete type.
       if (!EltTy->isIncompleteOrObjectType()) {
         DiagID = diag::err_typecheck_invalid_restrict_invalid_pointee;
         ProblemTy = EltTy;
       }
-    } else if (!T->isDependentType()) {
+    } else if (!isDependentOrGNUAutoType(T)) {
+      // For an __auto_type variable, we may not have seen the initializer yet
+      // and so have no idea whether the underlying type is a pointer type or
+      // not.
       DiagID = diag::err_typecheck_invalid_restrict_not_pointer;
       ProblemTy = T;
     }
 
     if (DiagID) {
       Diag(DS ? DS->getRestrictSpecLoc() : Loc, DiagID) << ProblemTy;
       Qs.removeRestrict();
     }
   }
 

       return Context.getUnaryTransformType(BaseType, Underlying,
                                         UnaryTransformType::EnumUnderlyingType);
     }
   }
   llvm_unreachable("unknown unary transform type");
 }
 
 QualType Sema::BuildAtomicType(QualType T, SourceLocation Loc) {
-  if (!T->isDependentType()) {
+  if (!isDependentOrGNUAutoType(T)) {
     // FIXME: It isn't entirely clear whether incomplete atomic types
     // are allowed or not; for simplicity, ban them for the moment.
     if (RequireCompleteType(Loc, T, diag::err_atomic_specifier_bad_type, 0))
       return QualType();
 
     int DisallowedKind = -1;
     if (T->isArrayType())
       DisallowedKind = 1;

clang/test/Sema/auto-type.c

int i() {
   struct bitfield { int field:2; };
   __auto_type j = (struct bitfield){1}.field; // expected-error {{cannot pass bit-field as __auto_type initializer in C}}
 
 }
 
 int k(l)
 __auto_type l; // expected-error {{'__auto_type' not allowed in K&R-style function parameter}}
 {}
+
+void Issue53652(void) {
+  // Ensure that qualifiers all work the same way as GCC.
+  const __auto_type cat = a;
+  const __auto_type pcat = &a;
+  volatile __auto_type vat = a;
+  volatile __auto_type pvat = &a;
+  restrict __auto_type rat = &a;
+  _Atomic __auto_type aat1 = a;
+  _Atomic __auto_type paat = &a;
+
+  // GCC does not accept this either, for the same reason.
+  _Atomic(__auto_type) aat2 = a; // expected-error {{'__auto_type' not allowed here}} \
+                                 // expected-warning {{type specifier missing, defaults to 'int'}}
+
+  // Ensure the types are what we expect them to be, regardless of order we
+  // pass the types.
+  _Static_assert(__builtin_types_compatible_p(__typeof(cat), const int), "");
+  _Static_assert(__builtin_types_compatible_p(const int, __typeof(cat)), "");
+  _Static_assert(__builtin_types_compatible_p(__typeof(pcat), int *const), "");
+  _Static_assert(__builtin_types_compatible_p(int *const, __typeof(pcat)), "");
+  _Static_assert(__builtin_types_compatible_p(__typeof(vat), volatile int), "");
+  _Static_assert(__builtin_types_compatible_p(volatile int, __typeof(vat)), "");
+  _Static_assert(__builtin_types_compatible_p(__typeof(pvat), int *volatile), "");
+  _Static_assert(__builtin_types_compatible_p(int *volatile, __typeof(pvat)), "");
+  _Static_assert(__builtin_types_compatible_p(__typeof(rat), int *restrict), "");
+  _Static_assert(__builtin_types_compatible_p(int *restrict, __typeof(rat)), "");
+  _Static_assert(__builtin_types_compatible_p(__typeof(aat1), _Atomic int), "");
+  _Static_assert(__builtin_types_compatible_p(_Atomic int, __typeof(aat1)), "");
+  _Static_assert(__builtin_types_compatible_p(__typeof(paat), _Atomic(int *)), "");
+  _Static_assert(__builtin_types_compatible_p(_Atomic(int *), __typeof(paat)), "");
+
+  // Ensure the types also work in generic selection expressions. Remember, the
+  // type of the expression argument to _Generic is treated as-if it undergoes
+  // lvalue to rvalue conversion, which drops qualifiers. We're making sure the
+  // use of __auto_type doesn't impact that.
+  (void)_Generic(cat, int : 0);
+  (void)_Generic(pcat, int * : 0);
+  (void)_Generic(vat, int : 0);
+  (void)_Generic(pvat, int * : 0);
+  (void)_Generic(rat, int * : 0);
+  (void)_Generic(aat1, int : 0);
+  (void)_Generic(paat, int * : 0);
+
+  // Ensure that trying to merge two different __auto_type types does not
+  // decide that they are both the same type when they're actually different,
+  // and that we reject when the types are the same.
+  __auto_type i = 12;
+  __auto_type f = 1.2f;
+  (void)_Generic(a, __typeof__(i) : 0, __typeof__(f) : 1);
+  (void)_Generic(a,
+                 __typeof__(i) : 0,   // expected-note {{compatible type 'typeof (i)' (aka 'int') specified here}}
+                 __typeof__(a) : 1);  // expected-error {{type 'typeof (a)' (aka 'int') in generic association compatible with previously specified type 'typeof (i)' (aka 'int')}}
+}