Skip to content

Commit b02d970

Browse files
committed
[clang][sema] Generate builtin operator overloads for (volatile) _Atomic 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 #4: clang` clang::Sema::CreateOverloadedBinOp(..., Opc=BO_OrAssign, ..., PerformADL=true, AllowRewrittenCandidates=false, ...) at SemaOverload.cpp:13943 frame #5: clang` BuildOverloadedBinOp(..., Opc=BO_OrAssign, ...) at SemaExpr.cpp:15228 frame #6: clang` clang::Sema::BuildBinOp(..., Opc=BO_OrAssign, ...) at SemaExpr.cpp:15330 frame #7: clang` clang::Sema::ActOnBinOp(..., Kind=pipeequal, ...) at SemaExpr.cpp:15187 frame #8: clang` clang::Parser::ParseRHSOfBinaryExpression(..., MinPrec=Assignment) at ParseExpr.cpp:629 frame #9: clang` clang::Parser::ParseAssignmentExpression(..., isTypeCast=NotTypeCast) at ParseExpr.cpp:176 frame #10: clang` clang::Parser::ParseExpression(... isTypeCast=NotTypeCast) at ParseExpr.cpp:124 frame #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
1 parent f125518 commit b02d970

File tree

4 files changed

+191
-25
lines changed

4 files changed

+191
-25
lines changed

clang/include/clang/AST/Type.h

+56
Original file line numberDiff line numberDiff line change
@@ -265,16 +265,31 @@ class Qualifiers {
265265
bool hasOnlyConst() const { return Mask == Const; }
266266
void removeConst() { Mask &= ~Const; }
267267
void addConst() { Mask |= Const; }
268+
Qualifiers withConst() const {
269+
Qualifiers Qs = *this;
270+
Qs.addConst();
271+
return Qs;
272+
}
268273

269274
bool hasVolatile() const { return Mask & Volatile; }
270275
bool hasOnlyVolatile() const { return Mask == Volatile; }
271276
void removeVolatile() { Mask &= ~Volatile; }
272277
void addVolatile() { Mask |= Volatile; }
278+
Qualifiers withVolatile() const {
279+
Qualifiers Qs = *this;
280+
Qs.addVolatile();
281+
return Qs;
282+
}
273283

274284
bool hasRestrict() const { return Mask & Restrict; }
275285
bool hasOnlyRestrict() const { return Mask == Restrict; }
276286
void removeRestrict() { Mask &= ~Restrict; }
277287
void addRestrict() { Mask |= Restrict; }
288+
Qualifiers withRestrict() const {
289+
Qualifiers Qs = *this;
290+
Qs.addRestrict();
291+
return Qs;
292+
}
278293

279294
bool hasCVRQualifiers() const { return getCVRQualifiers(); }
280295
unsigned getCVRQualifiers() const { return Mask & CVRMask; }
@@ -609,6 +624,47 @@ class Qualifiers {
609624
static const uint32_t AddressSpaceShift = 9;
610625
};
611626

627+
class QualifiersAndAtomic {
628+
Qualifiers Quals;
629+
bool HasAtomic;
630+
631+
public:
632+
QualifiersAndAtomic() : HasAtomic(false) {}
633+
QualifiersAndAtomic(Qualifiers Quals, bool HasAtomic)
634+
: Quals(Quals), HasAtomic(HasAtomic) {}
635+
636+
operator Qualifiers() const { return Quals; }
637+
638+
bool hasVolatile() const { return Quals.hasVolatile(); }
639+
bool hasConst() const { return Quals.hasConst(); }
640+
bool hasRestrict() const { return Quals.hasRestrict(); }
641+
bool hasAtomic() const { return HasAtomic; }
642+
643+
void addVolatile() { Quals.addVolatile(); }
644+
void addConst() { Quals.addConst(); }
645+
void addRestrict() { Quals.addRestrict(); }
646+
void addAtomic() { HasAtomic = true; }
647+
648+
void removeVolatile() { Quals.removeVolatile(); }
649+
void removeConst() { Quals.removeConst(); }
650+
void removeRestrict() { Quals.removeRestrict(); }
651+
void removeAtomic() { HasAtomic = false; }
652+
653+
QualifiersAndAtomic withVolatile() {
654+
return {Quals.withVolatile(), HasAtomic};
655+
}
656+
QualifiersAndAtomic withConst() { return {Quals.withConst(), HasAtomic}; }
657+
QualifiersAndAtomic withRestrict() {
658+
return {Quals.withRestrict(), HasAtomic};
659+
}
660+
QualifiersAndAtomic withAtomic() { return {Quals, true}; }
661+
662+
QualifiersAndAtomic &operator+=(Qualifiers RHS) {
663+
Quals += RHS;
664+
return *this;
665+
}
666+
};
667+
612668
/// A std::pair-like structure for storing a qualified type split
613669
/// into its local qualifiers and its locally-unqualified type.
614670
struct SplitQualType {

clang/lib/Sema/SemaOverload.cpp

+64-25
Original file line numberDiff line numberDiff line change
@@ -8200,6 +8200,49 @@ static Qualifiers CollectVRQualifiers(ASTContext &Context, Expr* ArgExpr) {
82008200
return VRQuals;
82018201
}
82028202

8203+
// Note: We're currently only handling qualifiers that are meaningful for the
8204+
// LHS of compound assignment overloading.
8205+
static void forAllQualifierCombinationsImpl(
8206+
QualifiersAndAtomic Available, QualifiersAndAtomic Applied,
8207+
llvm::function_ref<void(QualifiersAndAtomic)> Callback) {
8208+
// _Atomic
8209+
if (Available.hasAtomic()) {
8210+
Available.removeAtomic();
8211+
forAllQualifierCombinationsImpl(Available, Applied.withAtomic(), Callback);
8212+
forAllQualifierCombinationsImpl(Available, Applied, Callback);
8213+
return;
8214+
}
8215+
8216+
// volatile
8217+
if (Available.hasVolatile()) {
8218+
Available.removeVolatile();
8219+
assert(!Applied.hasVolatile());
8220+
forAllQualifierCombinationsImpl(Available, Applied.withVolatile(),
8221+
Callback);
8222+
forAllQualifierCombinationsImpl(Available, Applied, Callback);
8223+
return;
8224+
}
8225+
8226+
Callback(Applied);
8227+
}
8228+
8229+
static void forAllQualifierCombinations(
8230+
QualifiersAndAtomic Quals,
8231+
llvm::function_ref<void(QualifiersAndAtomic)> Callback) {
8232+
return forAllQualifierCombinationsImpl(Quals, QualifiersAndAtomic(),
8233+
Callback);
8234+
}
8235+
8236+
static QualType makeQualifiedLValueReferenceType(QualType Base,
8237+
QualifiersAndAtomic Quals,
8238+
Sema &S) {
8239+
if (Quals.hasAtomic())
8240+
Base = S.Context.getAtomicType(Base);
8241+
if (Quals.hasVolatile())
8242+
Base = S.Context.getVolatileType(Base);
8243+
return S.Context.getLValueReferenceType(Base);
8244+
}
8245+
82038246
namespace {
82048247

82058248
/// Helper class to manage the addition of builtin operator overload
@@ -8210,7 +8253,7 @@ class BuiltinOperatorOverloadBuilder {
82108253
// Common instance state available to all overload candidate addition methods.
82118254
Sema &S;
82128255
ArrayRef<Expr *> Args;
8213-
Qualifiers VisibleTypeConversionsQuals;
8256+
QualifiersAndAtomic VisibleTypeConversionsQuals;
82148257
bool HasArithmeticOrEnumeralCandidateType;
82158258
SmallVectorImpl<BuiltinCandidateTypeSet> &CandidateTypes;
82168259
OverloadCandidateSet &CandidateSet;
@@ -8334,7 +8377,7 @@ class BuiltinOperatorOverloadBuilder {
83348377
public:
83358378
BuiltinOperatorOverloadBuilder(
83368379
Sema &S, ArrayRef<Expr *> Args,
8337-
Qualifiers VisibleTypeConversionsQuals,
8380+
QualifiersAndAtomic VisibleTypeConversionsQuals,
83388381
bool HasArithmeticOrEnumeralCandidateType,
83398382
SmallVectorImpl<BuiltinCandidateTypeSet> &CandidateTypes,
83408383
OverloadCandidateSet &CandidateSet)
@@ -8955,18 +8998,14 @@ class BuiltinOperatorOverloadBuilder {
89558998
ParamTypes[1] = ArithmeticTypes[Right];
89568999
auto LeftBaseTy = AdjustAddressSpaceForBuiltinOperandType(
89579000
S, ArithmeticTypes[Left], Args[0]);
8958-
// Add this built-in operator as a candidate (VQ is empty).
8959-
ParamTypes[0] = S.Context.getLValueReferenceType(LeftBaseTy);
8960-
S.AddBuiltinCandidate(ParamTypes, Args, CandidateSet,
8961-
/*IsAssignmentOperator=*/isEqualOp);
89629001

8963-
// Add this built-in operator as a candidate (VQ is 'volatile').
8964-
if (VisibleTypeConversionsQuals.hasVolatile()) {
8965-
ParamTypes[0] = S.Context.getVolatileType(LeftBaseTy);
8966-
ParamTypes[0] = S.Context.getLValueReferenceType(ParamTypes[0]);
8967-
S.AddBuiltinCandidate(ParamTypes, Args, CandidateSet,
8968-
/*IsAssignmentOperator=*/isEqualOp);
8969-
}
9002+
forAllQualifierCombinations(
9003+
VisibleTypeConversionsQuals, [&](QualifiersAndAtomic Quals) {
9004+
ParamTypes[0] =
9005+
makeQualifiedLValueReferenceType(LeftBaseTy, Quals, S);
9006+
S.AddBuiltinCandidate(ParamTypes, Args, CandidateSet,
9007+
/*IsAssignmentOperator=*/isEqualOp);
9008+
});
89709009
}
89719010
}
89729011

@@ -9013,16 +9052,13 @@ class BuiltinOperatorOverloadBuilder {
90139052
ParamTypes[1] = ArithmeticTypes[Right];
90149053
auto LeftBaseTy = AdjustAddressSpaceForBuiltinOperandType(
90159054
S, ArithmeticTypes[Left], Args[0]);
9016-
// Add this built-in operator as a candidate (VQ is empty).
9017-
ParamTypes[0] = S.Context.getLValueReferenceType(LeftBaseTy);
9018-
S.AddBuiltinCandidate(ParamTypes, Args, CandidateSet);
9019-
if (VisibleTypeConversionsQuals.hasVolatile()) {
9020-
// Add this built-in operator as a candidate (VQ is 'volatile').
9021-
ParamTypes[0] = LeftBaseTy;
9022-
ParamTypes[0] = S.Context.getVolatileType(ParamTypes[0]);
9023-
ParamTypes[0] = S.Context.getLValueReferenceType(ParamTypes[0]);
9024-
S.AddBuiltinCandidate(ParamTypes, Args, CandidateSet);
9025-
}
9055+
9056+
forAllQualifierCombinations(
9057+
VisibleTypeConversionsQuals, [&](QualifiersAndAtomic Quals) {
9058+
ParamTypes[0] =
9059+
makeQualifiedLValueReferenceType(LeftBaseTy, Quals, S);
9060+
S.AddBuiltinCandidate(ParamTypes, Args, CandidateSet);
9061+
});
90269062
}
90279063
}
90289064
}
@@ -9186,10 +9222,13 @@ void Sema::AddBuiltinOperatorCandidates(OverloadedOperatorKind Op,
91869222
// if the operator we're looking at has built-in operator candidates
91879223
// that make use of these types. Also record whether we encounter non-record
91889224
// candidate types or either arithmetic or enumeral candidate types.
9189-
Qualifiers VisibleTypeConversionsQuals;
9225+
QualifiersAndAtomic VisibleTypeConversionsQuals;
91909226
VisibleTypeConversionsQuals.addConst();
9191-
for (unsigned ArgIdx = 0, N = Args.size(); ArgIdx != N; ++ArgIdx)
9227+
for (unsigned ArgIdx = 0, N = Args.size(); ArgIdx != N; ++ArgIdx) {
91929228
VisibleTypeConversionsQuals += CollectVRQualifiers(Context, Args[ArgIdx]);
9229+
if (Args[ArgIdx]->getType()->isAtomicType())
9230+
VisibleTypeConversionsQuals.addAtomic();
9231+
}
91939232

91949233
bool HasNonRecordCandidateType = false;
91959234
bool HasArithmeticOrEnumeralCandidateType = false;
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
// RUN: %clang_cc1 -std=gnu++11 -emit-llvm -triple=x86_64-linux-gnu -o - %s | FileCheck %s
2+
3+
_Atomic unsigned an_atomic_uint;
4+
5+
enum { an_enum_value = 1 };
6+
7+
// CHECK-LABEL: define {{.*}}void @_Z5enum1v()
8+
void enum1() {
9+
an_atomic_uint += an_enum_value;
10+
// CHECK: atomicrmw add ptr
11+
}
12+
13+
// CHECK-LABEL: define {{.*}}void @_Z5enum2v()
14+
void enum2() {
15+
an_atomic_uint |= an_enum_value;
16+
// CHECK: atomicrmw or ptr
17+
}
18+
19+
// CHECK-LABEL: define {{.*}}void @_Z5enum3RU7_Atomicj({{.*}})
20+
void enum3(_Atomic unsigned &an_atomic_uint_param) {
21+
an_atomic_uint_param += an_enum_value;
22+
// CHECK: atomicrmw add ptr
23+
}
24+
25+
// CHECK-LABEL: define {{.*}}void @_Z5enum4RU7_Atomicj({{.*}})
26+
void enum4(_Atomic unsigned &an_atomic_uint_param) {
27+
an_atomic_uint_param |= an_enum_value;
28+
// CHECK: atomicrmw or ptr
29+
}
30+
31+
volatile _Atomic unsigned an_volatile_atomic_uint;
32+
33+
// CHECK-LABEL: define {{.*}}void @_Z5enum5v()
34+
void enum5() {
35+
an_volatile_atomic_uint += an_enum_value;
36+
// CHECK: atomicrmw add ptr
37+
}
38+
39+
// CHECK-LABEL: define {{.*}}void @_Z5enum6v()
40+
void enum6() {
41+
an_volatile_atomic_uint |= an_enum_value;
42+
// CHECK: atomicrmw or ptr
43+
}
44+
45+
// CHECK-LABEL: define {{.*}}void @_Z5enum7RVU7_Atomicj({{.*}})
46+
void enum7(volatile _Atomic unsigned &an_volatile_atomic_uint_param) {
47+
an_volatile_atomic_uint_param += an_enum_value;
48+
// CHECK: atomicrmw add ptr
49+
}
50+
51+
// CHECK-LABEL: define {{.*}}void @_Z5enum8RVU7_Atomicj({{.*}})
52+
void enum8(volatile _Atomic unsigned &an_volatile_atomic_uint_param) {
53+
an_volatile_atomic_uint_param |= an_enum_value;
54+
// CHECK: atomicrmw or ptr
55+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
// RUN: %clang_cc1 -std=gnu++98 -fsyntax-only -verify %s
2+
// expected-no-diagnostics
3+
4+
_Atomic unsigned an_atomic_uint;
5+
6+
enum { an_enum_value = 1 };
7+
8+
void enum1() { an_atomic_uint += an_enum_value; }
9+
10+
void enum2() { an_atomic_uint |= an_enum_value; }
11+
12+
volatile _Atomic unsigned an_volatile_atomic_uint;
13+
14+
void enum3() { an_volatile_atomic_uint += an_enum_value; }
15+
16+
void enum4() { an_volatile_atomic_uint |= an_enum_value; }

0 commit comments

Comments
 (0)