[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
This commit is contained in:
Jan Svoboda
2022-06-08 13:40:14 +02:00
parent f1255186c7
commit b02d970b43
4 changed files with 191 additions and 25 deletions

View File

@@ -265,16 +265,31 @@ public:
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 @@ private:
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 {

View File

@@ -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 @@ public:
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 @@ public:
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;

View File

@@ -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
}

View File

@@ -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; }