Skip to content

Commit d2f75f2

Browse files
mizvekovmizvekov
authored
[clang] SFINAE context refactor (#164703)
This teases the SFINAE handling bits out of the CodeSynthesisContext, and moves that functionality into SFINAETrap and a new class. There is also a small performance benefit here: <img width="1460" height="20" alt="image" src="https://github.com/user-attachments/assets/aeb446e3-04c3-418e-83de-c80904c83574" />
1 parent f1bf0b0 commit d2f75f2

16 files changed

+264
-388
lines changed

‎clang/include/clang/Sema/Sema.h‎

Lines changed: 60 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -11309,9 +11309,6 @@ class Sema final : public SemaBase {
1130911309
InventedParameterInfos.end());
1131011310
}
1131111311

11312-
/// The number of SFINAE diagnostics that have been trapped.
11313-
unsigned NumSFINAEErrors;
11314-
1131511312
ArrayRef<sema::FunctionScopeInfo *> getFunctionScopes() const {
1131611313
return llvm::ArrayRef(FunctionScopes.begin() + FunctionScopesStart,
1131711314
FunctionScopes.end());
@@ -12385,49 +12382,65 @@ class Sema final : public SemaBase {
1238512382
///@{
1238612383

1238712384
public:
12388-
/// When true, access checking violations are treated as SFINAE
12389-
/// failures rather than hard errors.
12390-
bool AccessCheckingSFINAE;
12385+
class SFINAETrap;
12386+
12387+
struct SFINAEContextBase {
12388+
SFINAEContextBase(Sema &S, SFINAETrap *Cur)
12389+
: S(S), Prev(std::exchange(S.CurrentSFINAEContext, Cur)) {}
12390+
12391+
protected:
12392+
Sema &S;
12393+
~SFINAEContextBase() { S.CurrentSFINAEContext = Prev; }
12394+
12395+
private:
12396+
SFINAETrap *Prev;
12397+
};
12398+
12399+
struct NonSFINAEContext : SFINAEContextBase {
12400+
NonSFINAEContext(Sema &S) : SFINAEContextBase(S, nullptr) {}
12401+
};
1239112402

1239212403
/// RAII class used to determine whether SFINAE has
1239312404
/// trapped any errors that occur during template argument
1239412405
/// deduction.
12395-
class SFINAETrap {
12396-
Sema &SemaRef;
12397-
unsigned PrevSFINAEErrors;
12398-
bool PrevInNonInstantiationSFINAEContext;
12399-
bool PrevAccessCheckingSFINAE;
12400-
bool PrevLastDiagnosticIgnored;
12406+
class SFINAETrap : SFINAEContextBase {
12407+
bool HasErrorOcurred = false;
12408+
bool WithAccessChecking = false;
12409+
bool PrevLastDiagnosticIgnored =
12410+
S.getDiagnostics().isLastDiagnosticIgnored();
12411+
sema::TemplateDeductionInfo *DeductionInfo = nullptr;
12412+
12413+
SFINAETrap(Sema &S, sema::TemplateDeductionInfo *Info,
12414+
bool WithAccessChecking)
12415+
: SFINAEContextBase(S, this), WithAccessChecking(WithAccessChecking),
12416+
DeductionInfo(Info) {}
1240112417

1240212418
public:
12403-
/// \param ForValidityCheck If true, discard all diagnostics (from the
12419+
/// \param WithAccessChecking If true, discard all diagnostics (from the
1240412420
/// immediate context) instead of adding them to the currently active
12405-
/// \ref TemplateDeductionInfo (as returned by \ref isSFINAEContext).
12406-
explicit SFINAETrap(Sema &SemaRef, bool ForValidityCheck = false)
12407-
: SemaRef(SemaRef), PrevSFINAEErrors(SemaRef.NumSFINAEErrors),
12408-
PrevInNonInstantiationSFINAEContext(
12409-
SemaRef.InNonInstantiationSFINAEContext),
12410-
PrevAccessCheckingSFINAE(SemaRef.AccessCheckingSFINAE),
12411-
PrevLastDiagnosticIgnored(
12412-
SemaRef.getDiagnostics().isLastDiagnosticIgnored()) {
12413-
if (ForValidityCheck || !SemaRef.isSFINAEContext())
12414-
SemaRef.InNonInstantiationSFINAEContext = true;
12415-
SemaRef.AccessCheckingSFINAE = ForValidityCheck;
12416-
}
12421+
/// \ref TemplateDeductionInfo.
12422+
explicit SFINAETrap(Sema &S, bool WithAccessChecking = false)
12423+
: SFINAETrap(S, /*Info=*/nullptr, WithAccessChecking) {}
12424+
12425+
SFINAETrap(Sema &S, sema::TemplateDeductionInfo &Info)
12426+
: SFINAETrap(S, &Info, /*WithAccessChecking=*/false) {}
1241712427

1241812428
~SFINAETrap() {
12419-
SemaRef.NumSFINAEErrors = PrevSFINAEErrors;
12420-
SemaRef.InNonInstantiationSFINAEContext =
12421-
PrevInNonInstantiationSFINAEContext;
12422-
SemaRef.AccessCheckingSFINAE = PrevAccessCheckingSFINAE;
12423-
SemaRef.getDiagnostics().setLastDiagnosticIgnored(
12424-
PrevLastDiagnosticIgnored);
12429+
S.getDiagnostics().setLastDiagnosticIgnored(PrevLastDiagnosticIgnored);
1242512430
}
1242612431

12427-
/// Determine whether any SFINAE errors have been trapped.
12428-
bool hasErrorOccurred() const {
12429-
return SemaRef.NumSFINAEErrors > PrevSFINAEErrors;
12432+
SFINAETrap(const SFINAETrap &) = delete;
12433+
SFINAETrap &operator=(const SFINAETrap &) = delete;
12434+
12435+
sema::TemplateDeductionInfo *getDeductionInfo() const {
12436+
return DeductionInfo;
1243012437
}
12438+
12439+
/// Determine whether any SFINAE errors have been trapped.
12440+
bool hasErrorOccurred() const { return HasErrorOcurred; }
12441+
void setErrorOccurred() { HasErrorOcurred = true; }
12442+
12443+
bool withAccessChecking() const { return WithAccessChecking; }
1243112444
};
1243212445

1243312446
/// RAII class used to indicate that we are performing provisional
@@ -13148,9 +13161,6 @@ class Sema final : public SemaBase {
1314813161
PartialOrderingTTP,
1314913162
} Kind;
1315013163

13151-
/// Was the enclosing context a non-instantiation SFINAE context?
13152-
bool SavedInNonInstantiationSFINAEContext;
13153-
1315413164
/// Whether we're substituting into constraints.
1315513165
bool InConstraintSubstitution;
1315613166

@@ -13195,22 +13205,15 @@ class Sema final : public SemaBase {
1319513205
return {TemplateArgs, NumTemplateArgs};
1319613206
}
1319713207

13198-
/// The template deduction info object associated with the
13199-
/// substitution or checking of explicit or deduced template arguments.
13200-
sema::TemplateDeductionInfo *DeductionInfo;
13201-
1320213208
/// The source range that covers the construct that cause
1320313209
/// the instantiation, e.g., the template-id that causes a class
1320413210
/// template instantiation.
1320513211
SourceRange InstantiationRange;
1320613212

1320713213
CodeSynthesisContext()
13208-
: Kind(TemplateInstantiation),
13209-
SavedInNonInstantiationSFINAEContext(false),
13210-
InConstraintSubstitution(false),
13214+
: Kind(TemplateInstantiation), InConstraintSubstitution(false),
1321113215
InParameterMappingSubstitution(false), Entity(nullptr),
13212-
Template(nullptr), TemplateArgs(nullptr), NumTemplateArgs(0),
13213-
DeductionInfo(nullptr) {}
13216+
Template(nullptr), TemplateArgs(nullptr), NumTemplateArgs(0) {}
1321413217

1321513218
/// Determines whether this template is an actual instantiation
1321613219
/// that should be counted toward the maximum instantiation depth.
@@ -13262,15 +13265,13 @@ class Sema final : public SemaBase {
1326213265
FunctionTemplateDecl *FunctionTemplate,
1326313266
ArrayRef<TemplateArgument> TemplateArgs,
1326413267
CodeSynthesisContext::SynthesisKind Kind,
13265-
sema::TemplateDeductionInfo &DeductionInfo,
1326613268
SourceRange InstantiationRange = SourceRange());
1326713269

1326813270
/// Note that we are instantiating as part of template
1326913271
/// argument deduction for a class template declaration.
1327013272
InstantiatingTemplate(Sema &SemaRef, SourceLocation PointOfInstantiation,
1327113273
TemplateDecl *Template,
1327213274
ArrayRef<TemplateArgument> TemplateArgs,
13273-
sema::TemplateDeductionInfo &DeductionInfo,
1327413275
SourceRange InstantiationRange = SourceRange());
1327513276

1327613277
/// Note that we are instantiating as part of template
@@ -13279,7 +13280,6 @@ class Sema final : public SemaBase {
1327913280
InstantiatingTemplate(Sema &SemaRef, SourceLocation PointOfInstantiation,
1328013281
ClassTemplatePartialSpecializationDecl *PartialSpec,
1328113282
ArrayRef<TemplateArgument> TemplateArgs,
13282-
sema::TemplateDeductionInfo &DeductionInfo,
1328313283
SourceRange InstantiationRange = SourceRange());
1328413284

1328513285
/// Note that we are instantiating as part of template
@@ -13288,7 +13288,6 @@ class Sema final : public SemaBase {
1328813288
InstantiatingTemplate(Sema &SemaRef, SourceLocation PointOfInstantiation,
1328913289
VarTemplatePartialSpecializationDecl *PartialSpec,
1329013290
ArrayRef<TemplateArgument> TemplateArgs,
13291-
sema::TemplateDeductionInfo &DeductionInfo,
1329213291
SourceRange InstantiationRange = SourceRange());
1329313292

1329413293
/// Note that we are instantiating a default argument for a function
@@ -13334,7 +13333,6 @@ class Sema final : public SemaBase {
1333413333
/// concept.
1333513334
InstantiatingTemplate(Sema &SemaRef, SourceLocation PointOfInstantiation,
1333613335
ConstraintSubstitution, NamedDecl *Template,
13337-
sema::TemplateDeductionInfo &DeductionInfo,
1333813336
SourceRange InstantiationRange);
1333913337

1334013338
struct ConstraintNormalization {};
@@ -13354,7 +13352,6 @@ class Sema final : public SemaBase {
1335413352
/// a requirement of a requires expression.
1335513353
InstantiatingTemplate(Sema &SemaRef, SourceLocation PointOfInstantiation,
1335613354
concepts::Requirement *Req,
13357-
sema::TemplateDeductionInfo &DeductionInfo,
1335813355
SourceRange InstantiationRange = SourceRange());
1335913356

1336013357
/// \brief Note that we are checking the satisfaction of the constraint
@@ -13366,7 +13363,6 @@ class Sema final : public SemaBase {
1336613363
/// \brief Note that we are checking a requires clause.
1336713364
InstantiatingTemplate(Sema &SemaRef, SourceLocation PointOfInstantiation,
1336813365
const RequiresExpr *E,
13369-
sema::TemplateDeductionInfo &DeductionInfo,
1337013366
SourceRange InstantiationRange);
1337113367

1337213368
struct BuildingDeductionGuidesTag {};
@@ -13399,8 +13395,7 @@ class Sema final : public SemaBase {
1339913395
SourceLocation PointOfInstantiation,
1340013396
SourceRange InstantiationRange, Decl *Entity,
1340113397
NamedDecl *Template = nullptr,
13402-
ArrayRef<TemplateArgument> TemplateArgs = {},
13403-
sema::TemplateDeductionInfo *DeductionInfo = nullptr);
13398+
ArrayRef<TemplateArgument> TemplateArgs = {});
1340413399

1340513400
InstantiatingTemplate(const InstantiatingTemplate &) = delete;
1340613401

@@ -13541,12 +13536,7 @@ class Sema final : public SemaBase {
1354113536
/// recent visible declaration of that namespace.
1354213537
llvm::DenseMap<NamedDecl *, NamedDecl *> VisibleNamespaceCache;
1354313538

13544-
/// Whether we are in a SFINAE context that is not associated with
13545-
/// template instantiation.
13546-
///
13547-
/// This is used when setting up a SFINAE trap (\c see SFINAETrap) outside
13548-
/// of a template instantiation or template argument deduction.
13549-
bool InNonInstantiationSFINAEContext;
13539+
SFINAETrap *CurrentSFINAEContext = nullptr;
1355013540

1355113541
/// The number of \p CodeSynthesisContexts that are not template
1355213542
/// instantiations and, therefore, should not be counted as part of the
@@ -13617,15 +13607,13 @@ class Sema final : public SemaBase {
1361713607
PrintInstantiationStack(getDefaultDiagFunc());
1361813608
}
1361913609

13620-
/// Determines whether we are currently in a context where
13621-
/// template argument substitution failures are not considered
13622-
/// errors.
13623-
///
13624-
/// \returns An empty \c Optional if we're not in a SFINAE context.
13625-
/// Otherwise, contains a pointer that, if non-NULL, contains the nearest
13626-
/// template-deduction context object, which can be used to capture
13627-
/// diagnostics that will be suppressed.
13628-
std::optional<sema::TemplateDeductionInfo *> isSFINAEContext() const;
13610+
/// Returns a pointer to the current SFINAE context, if any.
13611+
[[nodiscard]] SFINAETrap *getSFINAEContext() const {
13612+
return CurrentSFINAEContext;
13613+
}
13614+
[[nodiscard]] bool isSFINAEContext() const {
13615+
return CurrentSFINAEContext != nullptr;
13616+
}
1362913617

1363013618
/// Perform substitution on the type T with a given set of template
1363113619
/// arguments.
@@ -14637,7 +14625,8 @@ class Sema final : public SemaBase {
1463714625
ArrayRef<UnexpandedParameterPack> Unexpanded,
1463814626
const MultiLevelTemplateArgumentList &TemplateArgs,
1463914627
bool FailOnPackProducingTemplates, bool &ShouldExpand,
14640-
bool &RetainExpansion, UnsignedOrNone &NumExpansions);
14628+
bool &RetainExpansion, UnsignedOrNone &NumExpansions,
14629+
bool Diagnose = true);
1464114630

1464214631
/// Determine the number of arguments in the given pack expansion
1464314632
/// type.

‎clang/lib/Sema/Sema.cpp‎

Lines changed: 22 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -321,9 +321,8 @@ Sema::Sema(Preprocessor &pp, ASTContext &ctxt, ASTConsumer &consumer,
321321
static_cast<unsigned>(ComparisonCategoryType::Last) + 1),
322322
StdSourceLocationImplDecl(nullptr), CXXTypeInfoDecl(nullptr),
323323
GlobalNewDeleteDeclared(false), DisableTypoCorrection(false),
324-
TyposCorrected(0), IsBuildingRecoveryCallExpr(false), NumSFINAEErrors(0),
325-
AccessCheckingSFINAE(false), CurrentInstantiationScope(nullptr),
326-
InNonInstantiationSFINAEContext(false), NonInstantiationEntries(0),
324+
TyposCorrected(0), IsBuildingRecoveryCallExpr(false),
325+
CurrentInstantiationScope(nullptr), NonInstantiationEntries(0),
327326
ArgPackSubstIndex(std::nullopt), SatisfactionCache(Context) {
328327
assert(pp.TUKind == TUKind);
329328
TUScope = nullptr;
@@ -670,7 +669,9 @@ void Sema::addExternalSource(IntrusiveRefCntPtr<ExternalSemaSource> E) {
670669

671670
void Sema::PrintStats() const {
672671
llvm::errs() << "\n*** Semantic Analysis Stats:\n";
673-
llvm::errs() << NumSFINAEErrors << " SFINAE diagnostics trapped.\n";
672+
if (SFINAETrap *Trap = getSFINAEContext())
673+
llvm::errs() << int(Trap->hasErrorOccurred())
674+
<< " SFINAE diagnostics trapped.\n";
674675

675676
BumpAlloc.PrintStats();
676677
AnalysisWarnings.PrintStats();
@@ -1681,7 +1682,8 @@ void Sema::EmitDiagnostic(unsigned DiagID, const DiagnosticBuilder &DB) {
16811682
// issue I am not seeing yet), then there should at least be a clarifying
16821683
// comment somewhere.
16831684
Diagnostic DiagInfo(&Diags, DB);
1684-
if (std::optional<TemplateDeductionInfo *> Info = isSFINAEContext()) {
1685+
if (SFINAETrap *Trap = getSFINAEContext()) {
1686+
sema::TemplateDeductionInfo *Info = Trap->getDeductionInfo();
16851687
switch (DiagnosticIDs::getDiagnosticSFINAEResponse(DiagInfo.getID())) {
16861688
case DiagnosticIDs::SFINAE_Report:
16871689
// We'll report the diagnostic below.
@@ -1690,37 +1692,37 @@ void Sema::EmitDiagnostic(unsigned DiagID, const DiagnosticBuilder &DB) {
16901692
case DiagnosticIDs::SFINAE_SubstitutionFailure:
16911693
// Count this failure so that we know that template argument deduction
16921694
// has failed.
1693-
++NumSFINAEErrors;
1695+
Trap->setErrorOccurred();
16941696

16951697
// Make a copy of this suppressed diagnostic and store it with the
16961698
// template-deduction information.
1697-
if (*Info && !(*Info)->hasSFINAEDiagnostic()) {
1698-
(*Info)->addSFINAEDiagnostic(DiagInfo.getLocation(),
1699-
PartialDiagnostic(DiagInfo, Context.getDiagAllocator()));
1700-
}
1699+
if (Info && !Info->hasSFINAEDiagnostic())
1700+
Info->addSFINAEDiagnostic(
1701+
DiagInfo.getLocation(),
1702+
PartialDiagnostic(DiagInfo, Context.getDiagAllocator()));
17011703

17021704
Diags.setLastDiagnosticIgnored(true);
17031705
return;
17041706

17051707
case DiagnosticIDs::SFINAE_AccessControl: {
17061708
// Per C++ Core Issue 1170, access control is part of SFINAE.
1707-
// Additionally, the AccessCheckingSFINAE flag can be used to temporarily
1709+
// Additionally, the WithAccessChecking flag can be used to temporarily
17081710
// make access control a part of SFINAE for the purposes of checking
17091711
// type traits.
1710-
if (!AccessCheckingSFINAE && !getLangOpts().CPlusPlus11)
1712+
if (!Trap->withAccessChecking() && !getLangOpts().CPlusPlus11)
17111713
break;
17121714

17131715
SourceLocation Loc = DiagInfo.getLocation();
17141716

17151717
// Suppress this diagnostic.
1716-
++NumSFINAEErrors;
1718+
Trap->setErrorOccurred();
17171719

17181720
// Make a copy of this suppressed diagnostic and store it with the
17191721
// template-deduction information.
1720-
if (*Info && !(*Info)->hasSFINAEDiagnostic()) {
1721-
(*Info)->addSFINAEDiagnostic(DiagInfo.getLocation(),
1722-
PartialDiagnostic(DiagInfo, Context.getDiagAllocator()));
1723-
}
1722+
if (Info && !Info->hasSFINAEDiagnostic())
1723+
Info->addSFINAEDiagnostic(
1724+
DiagInfo.getLocation(),
1725+
PartialDiagnostic(DiagInfo, Context.getDiagAllocator()));
17241726

17251727
Diags.setLastDiagnosticIgnored(true);
17261728

@@ -1740,13 +1742,13 @@ void Sema::EmitDiagnostic(unsigned DiagID, const DiagnosticBuilder &DB) {
17401742
return;
17411743
// Make a copy of this suppressed diagnostic and store it with the
17421744
// template-deduction information;
1743-
if (*Info) {
1744-
(*Info)->addSuppressedDiagnostic(
1745+
if (Info) {
1746+
Info->addSuppressedDiagnostic(
17451747
DiagInfo.getLocation(),
17461748
PartialDiagnostic(DiagInfo, Context.getDiagAllocator()));
17471749
if (!Diags.getDiagnosticIDs()->isNote(DiagID))
17481750
PrintContextStack([Info](SourceLocation Loc, PartialDiagnostic PD) {
1749-
(*Info)->addSuppressedDiagnostic(Loc, std::move(PD));
1751+
Info->addSuppressedDiagnostic(Loc, std::move(PD));
17501752
});
17511753
}
17521754

‎clang/lib/Sema/SemaAMDGPU.cpp‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -558,6 +558,8 @@ AMDGPUMaxNumWorkGroupsAttr *SemaAMDGPU::CreateAMDGPUMaxNumWorkGroupsAttr(
558558
const AttributeCommonInfo &CI, Expr *XExpr, Expr *YExpr, Expr *ZExpr) {
559559
ASTContext &Context = getASTContext();
560560
AMDGPUMaxNumWorkGroupsAttr TmpAttr(Context, CI, XExpr, YExpr, ZExpr);
561+
assert(!SemaRef.isSFINAEContext() &&
562+
"Can't produce SFINAE diagnostic pointing to temporary attribute");
561563

562564
if (checkAMDGPUMaxNumWorkGroupsArguments(SemaRef, XExpr, YExpr, ZExpr,
563565
TmpAttr))

0 commit comments

Comments
 (0)