[clang][sema] Generate builtin operator overloads for (volatile) _Ato…

…mic types

We observed a failed assert in overloaded compound-assignment operator resolution:

```
Assertion failed: (Result.isInvalid() && "C++ binary operator overloading is missing candidates!"), function CreateOverloadedBinOp, file SemaOverload.cpp, line 13944.
...
frame rust-lang#4: clang` clang::Sema::CreateOverloadedBinOp(..., Opc=BO_OrAssign, ..., PerformADL=true, AllowRewrittenCandidates=false, ...) at SemaOverload.cpp:13943
frame rust-lang#5: clang` BuildOverloadedBinOp(..., Opc=BO_OrAssign, ...) at SemaExpr.cpp:15228
frame rust-lang#6: clang` clang::Sema::BuildBinOp(..., Opc=BO_OrAssign, ...) at SemaExpr.cpp:15330
frame rust-lang#7: clang` clang::Sema::ActOnBinOp(..., Kind=pipeequal, ...) at SemaExpr.cpp:15187
frame rust-lang#8: clang` clang::Parser::ParseRHSOfBinaryExpression(..., MinPrec=Assignment) at ParseExpr.cpp:629
frame rust-lang#9: clang` clang::Parser::ParseAssignmentExpression(..., isTypeCast=NotTypeCast) at ParseExpr.cpp:176
frame rust-lang#10: clang` clang::Parser::ParseExpression(... isTypeCast=NotTypeCast) at ParseExpr.cpp:124
frame rust-lang#11: clang` clang::Parser::ParseExprStatement(...) at ParseStmt.cpp:464
```

A simple reproducer is:

```
_Atomic unsigned an_atomic_uint;

enum { an_enum_value = 1 };

void enum1() { an_atomic_uint += an_enum_value; }
```

This patch fixes the issue by generating builtin operator overloads for (volatile) _Atomic types.

Reviewed By: aaron.ballman

Differential Revision: https://reviews.llvm.org/D125349

clang/include/clang/AST/Type.h

@@ -265,16 +265,31 @@ class Qualifiers {
   bool hasOnlyConst() const { return Mask == Const; }
   void removeConst() { Mask &= ~Const; }
   void addConst() { Mask |= Const; }
+  Qualifiers withConst() const {
+    Qualifiers Qs = *this;
+    Qs.addConst();
+    return Qs;
+  }
 
   bool hasVolatile() const { return Mask & Volatile; }
   bool hasOnlyVolatile() const { return Mask == Volatile; }
   void removeVolatile() { Mask &= ~Volatile; }
   void addVolatile() { Mask |= Volatile; }
+  Qualifiers withVolatile() const {
+    Qualifiers Qs = *this;
+    Qs.addVolatile();
+    return Qs;
+  }
 
   bool hasRestrict() const { return Mask & Restrict; }
   bool hasOnlyRestrict() const { return Mask == Restrict; }
   void removeRestrict() { Mask &= ~Restrict; }
   void addRestrict() { Mask |= Restrict; }
+  Qualifiers withRestrict() const {
+    Qualifiers Qs = *this;
+    Qs.addRestrict();
+    return Qs;
+  }
 
   bool hasCVRQualifiers() const { return getCVRQualifiers(); }
   unsigned getCVRQualifiers() const { return Mask & CVRMask; }
@@ -609,6 +624,47 @@ class Qualifiers {
   static const uint32_t AddressSpaceShift = 9;
 };
 
+class QualifiersAndAtomic {
+  Qualifiers Quals;
+  bool HasAtomic;
+
+public:
+  QualifiersAndAtomic() : HasAtomic(false) {}
+  QualifiersAndAtomic(Qualifiers Quals, bool HasAtomic)
+      : Quals(Quals), HasAtomic(HasAtomic) {}
+
+  operator Qualifiers() const { return Quals; }
+
+  bool hasVolatile() const { return Quals.hasVolatile(); }
+  bool hasConst() const { return Quals.hasConst(); }
+  bool hasRestrict() const { return Quals.hasRestrict(); }
+  bool hasAtomic() const { return HasAtomic; }
+
+  void addVolatile() { Quals.addVolatile(); }
+  void addConst() { Quals.addConst(); }
+  void addRestrict() { Quals.addRestrict(); }
+  void addAtomic() { HasAtomic = true; }
+
+  void removeVolatile() { Quals.removeVolatile(); }
+  void removeConst() { Quals.removeConst(); }
+  void removeRestrict() { Quals.removeRestrict(); }
+  void removeAtomic() { HasAtomic = false; }
+
+  QualifiersAndAtomic withVolatile() {
+    return {Quals.withVolatile(), HasAtomic};
+  }
+  QualifiersAndAtomic withConst() { return {Quals.withConst(), HasAtomic}; }
+  QualifiersAndAtomic withRestrict() {
+    return {Quals.withRestrict(), HasAtomic};
+  }
+  QualifiersAndAtomic withAtomic() { return {Quals, true}; }
+
+  QualifiersAndAtomic &operator+=(Qualifiers RHS) {
+    Quals += RHS;
+    return *this;
+  }
+};
+
 /// A std::pair-like structure for storing a qualified type split
 /// into its local qualifiers and its locally-unqualified type.
 struct SplitQualType {

clang/lib/Sema/SemaOverload.cpp

@@ -8200,6 +8200,49 @@ static  Qualifiers CollectVRQualifiers(ASTContext &Context, Expr* ArgExpr) {
     return VRQuals;
 }
 
+// Note: We're currently only handling qualifiers that are meaningful for the
+// LHS of compound assignment overloading.
+static void forAllQualifierCombinationsImpl(
+    QualifiersAndAtomic Available, QualifiersAndAtomic Applied,
+    llvm::function_ref<void(QualifiersAndAtomic)> Callback) {
+  // _Atomic
+  if (Available.hasAtomic()) {
+    Available.removeAtomic();
+    forAllQualifierCombinationsImpl(Available, Applied.withAtomic(), Callback);
+    forAllQualifierCombinationsImpl(Available, Applied, Callback);
+    return;
+  }
+
+  // volatile
+  if (Available.hasVolatile()) {
+    Available.removeVolatile();
+    assert(!Applied.hasVolatile());
+    forAllQualifierCombinationsImpl(Available, Applied.withVolatile(),
+                                    Callback);
+    forAllQualifierCombinationsImpl(Available, Applied, Callback);
+    return;
+  }
+
+  Callback(Applied);
+}
+
+static void forAllQualifierCombinations(
+    QualifiersAndAtomic Quals,
+    llvm::function_ref<void(QualifiersAndAtomic)> Callback) {
+  return forAllQualifierCombinationsImpl(Quals, QualifiersAndAtomic(),
+                                         Callback);
+}
+
+static QualType makeQualifiedLValueReferenceType(QualType Base,
+                                                 QualifiersAndAtomic Quals,
+                                                 Sema &S) {
+  if (Quals.hasAtomic())
+    Base = S.Context.getAtomicType(Base);
+  if (Quals.hasVolatile())
+    Base = S.Context.getVolatileType(Base);
+  return S.Context.getLValueReferenceType(Base);
+}
+
 namespace {
 
 /// Helper class to manage the addition of builtin operator overload
@@ -8210,7 +8253,7 @@ class BuiltinOperatorOverloadBuilder {
   // Common instance state available to all overload candidate addition methods.
   Sema &S;
   ArrayRef<Expr *> Args;
-  Qualifiers VisibleTypeConversionsQuals;
+  QualifiersAndAtomic VisibleTypeConversionsQuals;
   bool HasArithmeticOrEnumeralCandidateType;
   SmallVectorImpl<BuiltinCandidateTypeSet> &CandidateTypes;
   OverloadCandidateSet &CandidateSet;
@@ -8334,7 +8377,7 @@ class BuiltinOperatorOverloadBuilder {
 public:
   BuiltinOperatorOverloadBuilder(
     Sema &S, ArrayRef<Expr *> Args,
-    Qualifiers VisibleTypeConversionsQuals,
+    QualifiersAndAtomic VisibleTypeConversionsQuals,
     bool HasArithmeticOrEnumeralCandidateType,
     SmallVectorImpl<BuiltinCandidateTypeSet> &CandidateTypes,
     OverloadCandidateSet &CandidateSet)
@@ -8955,18 +8998,14 @@ class BuiltinOperatorOverloadBuilder {
         ParamTypes[1] = ArithmeticTypes[Right];
         auto LeftBaseTy = AdjustAddressSpaceForBuiltinOperandType(
             S, ArithmeticTypes[Left], Args[0]);
-        // Add this built-in operator as a candidate (VQ is empty).
-        ParamTypes[0] = S.Context.getLValueReferenceType(LeftBaseTy);
-        S.AddBuiltinCandidate(ParamTypes, Args, CandidateSet,
-                              /*IsAssignmentOperator=*/isEqualOp);
 
-        // Add this built-in operator as a candidate (VQ is 'volatile').
-        if (VisibleTypeConversionsQuals.hasVolatile()) {
-          ParamTypes[0] = S.Context.getVolatileType(LeftBaseTy);
-          ParamTypes[0] = S.Context.getLValueReferenceType(ParamTypes[0]);
-          S.AddBuiltinCandidate(ParamTypes, Args, CandidateSet,
-                                /*IsAssignmentOperator=*/isEqualOp);
-        }
+        forAllQualifierCombinations(
+            VisibleTypeConversionsQuals, [&](QualifiersAndAtomic Quals) {
+              ParamTypes[0] =
+                  makeQualifiedLValueReferenceType(LeftBaseTy, Quals, S);
+              S.AddBuiltinCandidate(ParamTypes, Args, CandidateSet,
+                                    /*IsAssignmentOperator=*/isEqualOp);
+            });
       }
     }
 
@@ -9013,16 +9052,13 @@ class BuiltinOperatorOverloadBuilder {
         ParamTypes[1] = ArithmeticTypes[Right];
         auto LeftBaseTy = AdjustAddressSpaceForBuiltinOperandType(
             S, ArithmeticTypes[Left], Args[0]);
-        // Add this built-in operator as a candidate (VQ is empty).
-        ParamTypes[0] = S.Context.getLValueReferenceType(LeftBaseTy);
-        S.AddBuiltinCandidate(ParamTypes, Args, CandidateSet);
-        if (VisibleTypeConversionsQuals.hasVolatile()) {
-          // Add this built-in operator as a candidate (VQ is 'volatile').
-          ParamTypes[0] = LeftBaseTy;
-          ParamTypes[0] = S.Context.getVolatileType(ParamTypes[0]);
-          ParamTypes[0] = S.Context.getLValueReferenceType(ParamTypes[0]);
-          S.AddBuiltinCandidate(ParamTypes, Args, CandidateSet);
-        }
+
+        forAllQualifierCombinations(
+            VisibleTypeConversionsQuals, [&](QualifiersAndAtomic Quals) {
+              ParamTypes[0] =
+                  makeQualifiedLValueReferenceType(LeftBaseTy, Quals, S);
+              S.AddBuiltinCandidate(ParamTypes, Args, CandidateSet);
+            });
       }
     }
   }
@@ -9186,10 +9222,13 @@ void Sema::AddBuiltinOperatorCandidates(OverloadedOperatorKind Op,
   // if the operator we're looking at has built-in operator candidates
   // that make use of these types. Also record whether we encounter non-record
   // candidate types or either arithmetic or enumeral candidate types.
-  Qualifiers VisibleTypeConversionsQuals;
+  QualifiersAndAtomic VisibleTypeConversionsQuals;
   VisibleTypeConversionsQuals.addConst();
-  for (unsigned ArgIdx = 0, N = Args.size(); ArgIdx != N; ++ArgIdx)
+  for (unsigned ArgIdx = 0, N = Args.size(); ArgIdx != N; ++ArgIdx) {
     VisibleTypeConversionsQuals += CollectVRQualifiers(Context, Args[ArgIdx]);
+    if (Args[ArgIdx]->getType()->isAtomicType())
+      VisibleTypeConversionsQuals.addAtomic();
+  }
 
   bool HasNonRecordCandidateType = false;
   bool HasArithmeticOrEnumeralCandidateType = false;

clang/test/CodeGenCXX/atomic-builtin-compound-assignment-overload.cpp

@@ -0,0 +1,55 @@
+// RUN: %clang_cc1 -std=gnu++11 -emit-llvm -triple=x86_64-linux-gnu -o - %s | FileCheck %s
+
+_Atomic unsigned an_atomic_uint;
+
+enum { an_enum_value = 1 };
+
+// CHECK-LABEL: define {{.*}}void @_Z5enum1v()
+void enum1() {
+  an_atomic_uint += an_enum_value;
+  // CHECK: atomicrmw add ptr
+}
+
+// CHECK-LABEL: define {{.*}}void @_Z5enum2v()
+void enum2() {
+  an_atomic_uint |= an_enum_value;
+  // CHECK: atomicrmw or ptr
+}
+
+// CHECK-LABEL: define {{.*}}void @_Z5enum3RU7_Atomicj({{.*}})
+void enum3(_Atomic unsigned &an_atomic_uint_param) {
+  an_atomic_uint_param += an_enum_value;
+  // CHECK: atomicrmw add ptr
+}
+
+// CHECK-LABEL: define {{.*}}void @_Z5enum4RU7_Atomicj({{.*}})
+void enum4(_Atomic unsigned &an_atomic_uint_param) {
+  an_atomic_uint_param |= an_enum_value;
+  // CHECK: atomicrmw or ptr
+}
+
+volatile _Atomic unsigned an_volatile_atomic_uint;
+
+// CHECK-LABEL: define {{.*}}void @_Z5enum5v()
+void enum5() {
+  an_volatile_atomic_uint += an_enum_value;
+  // CHECK: atomicrmw add ptr
+}
+
+// CHECK-LABEL: define {{.*}}void @_Z5enum6v()
+void enum6() {
+  an_volatile_atomic_uint |= an_enum_value;
+  // CHECK: atomicrmw or ptr
+}
+
+// CHECK-LABEL: define {{.*}}void @_Z5enum7RVU7_Atomicj({{.*}})
+void enum7(volatile _Atomic unsigned &an_volatile_atomic_uint_param) {
+  an_volatile_atomic_uint_param += an_enum_value;
+  // CHECK: atomicrmw add ptr
+}
+
+// CHECK-LABEL: define {{.*}}void @_Z5enum8RVU7_Atomicj({{.*}})
+void enum8(volatile _Atomic unsigned &an_volatile_atomic_uint_param) {
+  an_volatile_atomic_uint_param |= an_enum_value;
+  // CHECK: atomicrmw or ptr
+}

clang/test/SemaCXX/atomic-builtin-compound-assignment-overload.cpp

@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -std=gnu++98 -fsyntax-only -verify %s
+// expected-no-diagnostics
+
+_Atomic unsigned an_atomic_uint;
+
+enum { an_enum_value = 1 };
+
+void enum1() { an_atomic_uint += an_enum_value; }
+
+void enum2() { an_atomic_uint |= an_enum_value; }
+
+volatile _Atomic unsigned an_volatile_atomic_uint;
+
+void enum3() { an_volatile_atomic_uint += an_enum_value; }
+
+void enum4() { an_volatile_atomic_uint |= an_enum_value; }