[clang] SFINAE context refactor #164703
Merged
[clang] SFINAE context refactor #164703
+264
−388
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mizvekov
requested review from
cor3ntin,
erichkeane,
ilya-biryukov and
zyn0217
llvmbot
added
clang
|
@llvm/pr-subscribers-clang-modules @llvm/pr-subscribers-clang Author: Matheus Izvekov (mizvekov) ChangesThis 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: Patch is 86.59 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/164703.diff 26 Files Affected:
diff --git a/clang-tools-extra/clang-include-fixer/IncludeFixer.cpp b/clang-tools-extra/clang-include-fixer/IncludeFixer.cpp
index e825547ba0134..2eb736e06c249 100644
--- a/clang-tools-extra/clang-include-fixer/IncludeFixer.cpp
+++ b/clang-tools-extra/clang-include-fixer/IncludeFixer.cpp
@@ -149,7 +149,7 @@ static bool addDiagnosticsForContext(TypoCorrection &Correction,
bool IncludeFixerSemaSource::MaybeDiagnoseMissingCompleteType(
clang::SourceLocation Loc, clang::QualType T) {
// Ignore spurious callbacks from SFINAE contexts.
- if (CI->getSema().isSFINAEContext())
+ if (CI->getSema().getSFINAEContext())
return false;
clang::ASTContext &context = CI->getASTContext();
@@ -186,7 +186,7 @@ clang::TypoCorrection IncludeFixerSemaSource::CorrectTypo(
CorrectionCandidateCallback &CCC, DeclContext *MemberContext,
bool EnteringContext, const ObjCObjectPointerType *OPT) {
// Ignore spurious callbacks from SFINAE contexts.
- if (CI->getSema().isSFINAEContext())
+ if (CI->getSema().getSFINAEContext())
return clang::TypoCorrection();
// We currently ignore the unidentified symbol which is not from the
diff --git a/clang-tools-extra/clangd/IncludeFixer.cpp b/clang-tools-extra/clangd/IncludeFixer.cpp
index 3f3d7fbefd58e..33ce8f75e587d 100644
--- a/clang-tools-extra/clangd/IncludeFixer.cpp
+++ b/clang-tools-extra/clangd/IncludeFixer.cpp
@@ -506,7 +506,7 @@ class IncludeFixer::UnresolvedNameRecorder : public ExternalSemaSource {
const ObjCObjectPointerType *OPT) override {
dlog("CorrectTypo: {0}", Typo.getAsString());
assert(SemaPtr && "Sema must have been set.");
- if (SemaPtr->isSFINAEContext())
+ if (SemaPtr->getSFINAEContext())
return TypoCorrection();
if (!isInsideMainFile(Typo.getLoc(), SemaPtr->SourceMgr))
return clang::TypoCorrection();
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index cb21335ede075..6c4515a2c1c9a 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -11318,9 +11318,6 @@ class Sema final : public SemaBase {
InventedParameterInfos.end());
}
- /// The number of SFINAE diagnostics that have been trapped.
- unsigned NumSFINAEErrors;
-
ArrayRef<sema::FunctionScopeInfo *> getFunctionScopes() const {
return llvm::ArrayRef(FunctionScopes.begin() + FunctionScopesStart,
FunctionScopes.end());
@@ -12398,45 +12395,65 @@ class Sema final : public SemaBase {
/// failures rather than hard errors.
bool AccessCheckingSFINAE;
+ class SFINAETrap;
+
+ struct SFINAEContextBase {
+ SFINAEContextBase(Sema &S, SFINAETrap *Cur)
+ : S(S), Prev(std::exchange(S.CurrentSFINAEContext, Cur)) {}
+
+ protected:
+ Sema &S;
+ ~SFINAEContextBase() { S.CurrentSFINAEContext = Prev; }
+
+ private:
+ SFINAETrap *Prev;
+ };
+
+ struct NonSFINAEContext : SFINAEContextBase {
+ NonSFINAEContext(Sema &S) : SFINAEContextBase(S, nullptr) {}
+ };
+
/// RAII class used to determine whether SFINAE has
/// trapped any errors that occur during template argument
/// deduction.
- class SFINAETrap {
- Sema &SemaRef;
- unsigned PrevSFINAEErrors;
- bool PrevInNonInstantiationSFINAEContext;
- bool PrevAccessCheckingSFINAE;
- bool PrevLastDiagnosticIgnored;
+ class SFINAETrap : SFINAEContextBase {
+ bool HasErrorOcurred = false;
+ bool PrevAccessCheckingSFINAE = S.AccessCheckingSFINAE;
+ bool PrevLastDiagnosticIgnored =
+ S.getDiagnostics().isLastDiagnosticIgnored();
+ sema::TemplateDeductionInfo *DeductionInfo = nullptr;
+
+ SFINAETrap(Sema &S, sema::TemplateDeductionInfo *Info,
+ bool ForValidityCheck)
+ : SFINAEContextBase(S, this), DeductionInfo(Info) {
+ S.AccessCheckingSFINAE = ForValidityCheck;
+ }
public:
/// \param ForValidityCheck If true, discard all diagnostics (from the
/// immediate context) instead of adding them to the currently active
/// \ref TemplateDeductionInfo (as returned by \ref isSFINAEContext).
- explicit SFINAETrap(Sema &SemaRef, bool ForValidityCheck = false)
- : SemaRef(SemaRef), PrevSFINAEErrors(SemaRef.NumSFINAEErrors),
- PrevInNonInstantiationSFINAEContext(
- SemaRef.InNonInstantiationSFINAEContext),
- PrevAccessCheckingSFINAE(SemaRef.AccessCheckingSFINAE),
- PrevLastDiagnosticIgnored(
- SemaRef.getDiagnostics().isLastDiagnosticIgnored()) {
- if (ForValidityCheck || !SemaRef.isSFINAEContext())
- SemaRef.InNonInstantiationSFINAEContext = true;
- SemaRef.AccessCheckingSFINAE = ForValidityCheck;
- }
+ explicit SFINAETrap(Sema &S, bool ForValidityCheck = false)
+ : SFINAETrap(S, /*Info=*/nullptr, ForValidityCheck) {}
+
+ SFINAETrap(Sema &S, sema::TemplateDeductionInfo &Info)
+ : SFINAETrap(S, &Info, /*ForValidityCheck=*/false) {}
~SFINAETrap() {
- SemaRef.NumSFINAEErrors = PrevSFINAEErrors;
- SemaRef.InNonInstantiationSFINAEContext =
- PrevInNonInstantiationSFINAEContext;
- SemaRef.AccessCheckingSFINAE = PrevAccessCheckingSFINAE;
- SemaRef.getDiagnostics().setLastDiagnosticIgnored(
- PrevLastDiagnosticIgnored);
+ S.AccessCheckingSFINAE = PrevAccessCheckingSFINAE;
+ S.getDiagnostics().setLastDiagnosticIgnored(PrevLastDiagnosticIgnored);
}
- /// Determine whether any SFINAE errors have been trapped.
- bool hasErrorOccurred() const {
- return SemaRef.NumSFINAEErrors > PrevSFINAEErrors;
+ SFINAETrap(const SFINAETrap &) = delete;
+ SFINAETrap &operator=(const SFINAETrap &) = delete;
+
+ sema::TemplateDeductionInfo *getDeductionInfo() const {
+ return DeductionInfo;
}
+
+ /// Determine whether any SFINAE errors have been trapped.
+ bool hasErrorOccurred() const { return HasErrorOcurred; }
+ void setErrorOccurred() { HasErrorOcurred = true; }
};
/// RAII class used to indicate that we are performing provisional
@@ -13157,9 +13174,6 @@ class Sema final : public SemaBase {
PartialOrderingTTP,
} Kind;
- /// Was the enclosing context a non-instantiation SFINAE context?
- bool SavedInNonInstantiationSFINAEContext;
-
/// Whether we're substituting into constraints.
bool InConstraintSubstitution;
@@ -13204,22 +13218,15 @@ class Sema final : public SemaBase {
return {TemplateArgs, NumTemplateArgs};
}
- /// The template deduction info object associated with the
- /// substitution or checking of explicit or deduced template arguments.
- sema::TemplateDeductionInfo *DeductionInfo;
-
/// The source range that covers the construct that cause
/// the instantiation, e.g., the template-id that causes a class
/// template instantiation.
SourceRange InstantiationRange;
CodeSynthesisContext()
- : Kind(TemplateInstantiation),
- SavedInNonInstantiationSFINAEContext(false),
- InConstraintSubstitution(false),
+ : Kind(TemplateInstantiation), InConstraintSubstitution(false),
InParameterMappingSubstitution(false), Entity(nullptr),
- Template(nullptr), TemplateArgs(nullptr), NumTemplateArgs(0),
- DeductionInfo(nullptr) {}
+ Template(nullptr), TemplateArgs(nullptr), NumTemplateArgs(0) {}
/// Determines whether this template is an actual instantiation
/// that should be counted toward the maximum instantiation depth.
@@ -13271,7 +13278,6 @@ class Sema final : public SemaBase {
FunctionTemplateDecl *FunctionTemplate,
ArrayRef<TemplateArgument> TemplateArgs,
CodeSynthesisContext::SynthesisKind Kind,
- sema::TemplateDeductionInfo &DeductionInfo,
SourceRange InstantiationRange = SourceRange());
/// Note that we are instantiating as part of template
@@ -13279,7 +13285,6 @@ class Sema final : public SemaBase {
InstantiatingTemplate(Sema &SemaRef, SourceLocation PointOfInstantiation,
TemplateDecl *Template,
ArrayRef<TemplateArgument> TemplateArgs,
- sema::TemplateDeductionInfo &DeductionInfo,
SourceRange InstantiationRange = SourceRange());
/// Note that we are instantiating as part of template
@@ -13288,7 +13293,6 @@ class Sema final : public SemaBase {
InstantiatingTemplate(Sema &SemaRef, SourceLocation PointOfInstantiation,
ClassTemplatePartialSpecializationDecl *PartialSpec,
ArrayRef<TemplateArgument> TemplateArgs,
- sema::TemplateDeductionInfo &DeductionInfo,
SourceRange InstantiationRange = SourceRange());
/// Note that we are instantiating as part of template
@@ -13297,7 +13301,6 @@ class Sema final : public SemaBase {
InstantiatingTemplate(Sema &SemaRef, SourceLocation PointOfInstantiation,
VarTemplatePartialSpecializationDecl *PartialSpec,
ArrayRef<TemplateArgument> TemplateArgs,
- sema::TemplateDeductionInfo &DeductionInfo,
SourceRange InstantiationRange = SourceRange());
/// Note that we are instantiating a default argument for a function
@@ -13343,7 +13346,6 @@ class Sema final : public SemaBase {
/// concept.
InstantiatingTemplate(Sema &SemaRef, SourceLocation PointOfInstantiation,
ConstraintSubstitution, NamedDecl *Template,
- sema::TemplateDeductionInfo &DeductionInfo,
SourceRange InstantiationRange);
struct ConstraintNormalization {};
@@ -13363,7 +13365,6 @@ class Sema final : public SemaBase {
/// a requirement of a requires expression.
InstantiatingTemplate(Sema &SemaRef, SourceLocation PointOfInstantiation,
concepts::Requirement *Req,
- sema::TemplateDeductionInfo &DeductionInfo,
SourceRange InstantiationRange = SourceRange());
/// \brief Note that we are checking the satisfaction of the constraint
@@ -13375,7 +13376,6 @@ class Sema final : public SemaBase {
/// \brief Note that we are checking a requires clause.
InstantiatingTemplate(Sema &SemaRef, SourceLocation PointOfInstantiation,
const RequiresExpr *E,
- sema::TemplateDeductionInfo &DeductionInfo,
SourceRange InstantiationRange);
struct BuildingDeductionGuidesTag {};
@@ -13408,8 +13408,7 @@ class Sema final : public SemaBase {
SourceLocation PointOfInstantiation,
SourceRange InstantiationRange, Decl *Entity,
NamedDecl *Template = nullptr,
- ArrayRef<TemplateArgument> TemplateArgs = {},
- sema::TemplateDeductionInfo *DeductionInfo = nullptr);
+ ArrayRef<TemplateArgument> TemplateArgs = {});
InstantiatingTemplate(const InstantiatingTemplate &) = delete;
@@ -13550,12 +13549,7 @@ class Sema final : public SemaBase {
/// recent visible declaration of that namespace.
llvm::DenseMap<NamedDecl *, NamedDecl *> VisibleNamespaceCache;
- /// Whether we are in a SFINAE context that is not associated with
- /// template instantiation.
- ///
- /// This is used when setting up a SFINAE trap (\c see SFINAETrap) outside
- /// of a template instantiation or template argument deduction.
- bool InNonInstantiationSFINAEContext;
+ SFINAETrap *CurrentSFINAEContext = nullptr;
/// The number of \p CodeSynthesisContexts that are not template
/// instantiations and, therefore, should not be counted as part of the
@@ -13626,15 +13620,10 @@ class Sema final : public SemaBase {
PrintInstantiationStack(getDefaultDiagFunc());
}
- /// Determines whether we are currently in a context where
- /// template argument substitution failures are not considered
- /// errors.
- ///
- /// \returns An empty \c Optional if we're not in a SFINAE context.
- /// Otherwise, contains a pointer that, if non-NULL, contains the nearest
- /// template-deduction context object, which can be used to capture
- /// diagnostics that will be suppressed.
- std::optional<sema::TemplateDeductionInfo *> isSFINAEContext() const;
+ /// Returns a pointer to the current SFINAE context, if any.
+ [[nodiscard]] SFINAETrap *getSFINAEContext() const {
+ return CurrentSFINAEContext;
+ }
/// Perform substitution on the type T with a given set of template
/// arguments.
@@ -14646,7 +14635,8 @@ class Sema final : public SemaBase {
ArrayRef<UnexpandedParameterPack> Unexpanded,
const MultiLevelTemplateArgumentList &TemplateArgs,
bool FailOnPackProducingTemplates, bool &ShouldExpand,
- bool &RetainExpansion, UnsignedOrNone &NumExpansions);
+ bool &RetainExpansion, UnsignedOrNone &NumExpansions,
+ bool Diagnose = true);
/// Determine the number of arguments in the given pack expansion
/// type.
diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp
index 39fa25f66f3b7..26a4260b825b1 100644
--- a/clang/lib/Sema/Sema.cpp
+++ b/clang/lib/Sema/Sema.cpp
@@ -322,10 +322,10 @@ Sema::Sema(Preprocessor &pp, ASTContext &ctxt, ASTConsumer &consumer,
static_cast<unsigned>(ComparisonCategoryType::Last) + 1),
StdSourceLocationImplDecl(nullptr), CXXTypeInfoDecl(nullptr),
GlobalNewDeleteDeclared(false), DisableTypoCorrection(false),
- TyposCorrected(0), IsBuildingRecoveryCallExpr(false), NumSFINAEErrors(0),
+ TyposCorrected(0), IsBuildingRecoveryCallExpr(false),
AccessCheckingSFINAE(false), CurrentInstantiationScope(nullptr),
- InNonInstantiationSFINAEContext(false), NonInstantiationEntries(0),
- ArgPackSubstIndex(std::nullopt), SatisfactionCache(Context) {
+ NonInstantiationEntries(0), ArgPackSubstIndex(std::nullopt),
+ SatisfactionCache(Context) {
assert(pp.TUKind == TUKind);
TUScope = nullptr;
@@ -671,7 +671,9 @@ void Sema::addExternalSource(IntrusiveRefCntPtr<ExternalSemaSource> E) {
void Sema::PrintStats() const {
llvm::errs() << "\n*** Semantic Analysis Stats:\n";
- llvm::errs() << NumSFINAEErrors << " SFINAE diagnostics trapped.\n";
+ if (SFINAETrap *Trap = getSFINAEContext())
+ llvm::errs() << int(Trap->hasErrorOccurred())
+ << " SFINAE diagnostics trapped.\n";
BumpAlloc.PrintStats();
AnalysisWarnings.PrintStats();
@@ -1678,7 +1680,8 @@ void Sema::EmitDiagnostic(unsigned DiagID, const DiagnosticBuilder &DB) {
// issue I am not seeing yet), then there should at least be a clarifying
// comment somewhere.
Diagnostic DiagInfo(&Diags, DB);
- if (std::optional<TemplateDeductionInfo *> Info = isSFINAEContext()) {
+ if (SFINAETrap *Trap = getSFINAEContext()) {
+ sema::TemplateDeductionInfo *Info = Trap->getDeductionInfo();
switch (DiagnosticIDs::getDiagnosticSFINAEResponse(DiagInfo.getID())) {
case DiagnosticIDs::SFINAE_Report:
// We'll report the diagnostic below.
@@ -1687,14 +1690,14 @@ void Sema::EmitDiagnostic(unsigned DiagID, const DiagnosticBuilder &DB) {
case DiagnosticIDs::SFINAE_SubstitutionFailure:
// Count this failure so that we know that template argument deduction
// has failed.
- ++NumSFINAEErrors;
+ Trap->setErrorOccurred();
// Make a copy of this suppressed diagnostic and store it with the
// template-deduction information.
- if (*Info && !(*Info)->hasSFINAEDiagnostic()) {
- (*Info)->addSFINAEDiagnostic(DiagInfo.getLocation(),
- PartialDiagnostic(DiagInfo, Context.getDiagAllocator()));
- }
+ if (Info && !Info->hasSFINAEDiagnostic())
+ Info->addSFINAEDiagnostic(
+ DiagInfo.getLocation(),
+ PartialDiagnostic(DiagInfo, Context.getDiagAllocator()));
Diags.setLastDiagnosticIgnored(true);
return;
@@ -1710,14 +1713,14 @@ void Sema::EmitDiagnostic(unsigned DiagID, const DiagnosticBuilder &DB) {
SourceLocation Loc = DiagInfo.getLocation();
// Suppress this diagnostic.
- ++NumSFINAEErrors;
+ Trap->setErrorOccurred();
// Make a copy of this suppressed diagnostic and store it with the
// template-deduction information.
- if (*Info && !(*Info)->hasSFINAEDiagnostic()) {
- (*Info)->addSFINAEDiagnostic(DiagInfo.getLocation(),
- PartialDiagnostic(DiagInfo, Context.getDiagAllocator()));
- }
+ if (Info && !Info->hasSFINAEDiagnostic())
+ Info->addSFINAEDiagnostic(
+ DiagInfo.getLocation(),
+ PartialDiagnostic(DiagInfo, Context.getDiagAllocator()));
Diags.setLastDiagnosticIgnored(true);
@@ -1737,13 +1740,13 @@ void Sema::EmitDiagnostic(unsigned DiagID, const DiagnosticBuilder &DB) {
return;
// Make a copy of this suppressed diagnostic and store it with the
// template-deduction information;
- if (*Info) {
- (*Info)->addSuppressedDiagnostic(
+ if (Info) {
+ Info->addSuppressedDiagnostic(
DiagInfo.getLocation(),
PartialDiagnostic(DiagInfo, Context.getDiagAllocator()));
if (!Diags.getDiagnosticIDs()->isNote(DiagID))
PrintContextStack([Info](SourceLocation Loc, PartialDiagnostic PD) {
- (*Info)->addSuppressedDiagnostic(Loc, std::move(PD));
+ Info->addSuppressedDiagnostic(Loc, std::move(PD));
});
}
@@ -2827,7 +2830,7 @@ bool Sema::tryToRecoverWithCall(ExprResult &E, const PartialDiagnostic &PD,
// If this is a SFINAE context, don't try anything that might trigger ADL
// prematurely.
- if (!isSFINAEContext()) {
+ if (!getSFINAEContext()) {
QualType ZeroArgCallTy;
if (tryExprAsCall(*E.get(), ZeroArgCallTy, Overloads) &&
!ZeroArgCallTy.isNull() &&
diff --git a/clang/lib/Sema/SemaAMDGPU.cpp b/clang/lib/Sema/SemaAMDGPU.cpp
index e32f4376a5ebf..71238c0720dc9 100644
--- a/clang/lib/Sema/SemaAMDGPU.cpp
+++ b/clang/lib/Sema/SemaAMDGPU.cpp
@@ -517,6 +517,8 @@ AMDGPUMaxNumWorkGroupsAttr *SemaAMDGPU::CreateAMDGPUMaxNumWorkGroupsAttr(
const AttributeCommonInfo &CI, Expr *XExpr, Expr *YExpr, Expr *ZExpr) {
ASTContext &Context = getASTContext();
AMDGPUMaxNumWorkGroupsAttr TmpAttr(Context, CI, XExpr, YExpr, ZExpr);
+ assert(!SemaRef.getSFINAEContext() &&
+ "Can't produce SFINAE diagnostic pointing to temporary attribute");
if (checkAMDGPUMaxNumWorkGroupsArguments(SemaRef, XExpr, YExpr, ZExpr,
TmpAttr))
diff --git a/clang/lib/Sema/SemaCXXScopeSpec.cpp b/clang/lib/Sema/SemaCXXScopeSpec.cpp
index c52fc5bf815af..7fe21af85433a 100644
--- a/clang/lib/Sema/SemaCXXScopeSpec.cpp
+++ b/clang/lib/Sema/SemaCXXScopeSpec.cpp
@@ -100,8 +100,8 @@ DeclContext *Sema::computeDeclContext(const CXXScopeSpec &SS,
// A declaration of the partial specialization must be visible.
// We can always recover here, because this only happens when we're
// entering the context, and that can't happen in a SFINAE context.
- assert(!isSFINAEContext() && "partial specialization scope "
- "specifier in SFINAE context?");
+ assert(!getSFINAEContext() && "partial specialization scope "
+ "specifier in SFINAE context?");
if (PartialSpec->hasDefinition() &&
!hasReachableDefinition(PartialSpec))
diagnoseMissingImport(SS.getLastQualifierNameLoc(), PartialSpec,
@@ -227,7 +227,7 @@ bool Sema::RequireCompleteEnumDecl(EnumDecl *EnumD, SourceLocation L,
/*OnlyNeedComplete*/ false)) {
// If the user is going to see an error here, recover by making the
// definition visible.
- bool TreatAsComplete = !isSFINAEContext();
+ bool TreatAsComplete = !getSFINAEContext();
diagnoseMissingImport(L, SuggestedDef, MissingImportKind::Definition,
/*Recover*/ TreatAsComplete);
re...
[truncated]
|
|
@llvm/pr-subscribers-backend-amdgpu Author: Matheus Izvekov (mizvekov) ChangesThis 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: Patch is 86.59 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/164703.diff 26 Files Affected:
diff --git a/clang-tools-extra/clang-include-fixer/IncludeFixer.cpp b/clang-tools-extra/clang-include-fixer/IncludeFixer.cpp
index e825547ba0134..2eb736e06c249 100644
--- a/clang-tools-extra/clang-include-fixer/IncludeFixer.cpp
+++ b/clang-tools-extra/clang-include-fixer/IncludeFixer.cpp
@@ -149,7 +149,7 @@ static bool addDiagnosticsForContext(TypoCorrection &Correction,
bool IncludeFixerSemaSource::MaybeDiagnoseMissingCompleteType(
clang::SourceLocation Loc, clang::QualType T) {
// Ignore spurious callbacks from SFINAE contexts.
- if (CI->getSema().isSFINAEContext())
+ if (CI->getSema().getSFINAEContext())
return false;
clang::ASTContext &context = CI->getASTContext();
@@ -186,7 +186,7 @@ clang::TypoCorrection IncludeFixerSemaSource::CorrectTypo(
CorrectionCandidateCallback &CCC, DeclContext *MemberContext,
bool EnteringContext, const ObjCObjectPointerType *OPT) {
// Ignore spurious callbacks from SFINAE contexts.
- if (CI->getSema().isSFINAEContext())
+ if (CI->getSema().getSFINAEContext())
return clang::TypoCorrection();
// We currently ignore the unidentified symbol which is not from the
diff --git a/clang-tools-extra/clangd/IncludeFixer.cpp b/clang-tools-extra/clangd/IncludeFixer.cpp
index 3f3d7fbefd58e..33ce8f75e587d 100644
--- a/clang-tools-extra/clangd/IncludeFixer.cpp
+++ b/clang-tools-extra/clangd/IncludeFixer.cpp
@@ -506,7 +506,7 @@ class IncludeFixer::UnresolvedNameRecorder : public ExternalSemaSource {
const ObjCObjectPointerType *OPT) override {
dlog("CorrectTypo: {0}", Typo.getAsString());
assert(SemaPtr && "Sema must have been set.");
- if (SemaPtr->isSFINAEContext())
+ if (SemaPtr->getSFINAEContext())
return TypoCorrection();
if (!isInsideMainFile(Typo.getLoc(), SemaPtr->SourceMgr))
return clang::TypoCorrection();
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index cb21335ede075..6c4515a2c1c9a 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -11318,9 +11318,6 @@ class Sema final : public SemaBase {
InventedParameterInfos.end());
}
- /// The number of SFINAE diagnostics that have been trapped.
- unsigned NumSFINAEErrors;
-
ArrayRef<sema::FunctionScopeInfo *> getFunctionScopes() const {
return llvm::ArrayRef(FunctionScopes.begin() + FunctionScopesStart,
FunctionScopes.end());
@@ -12398,45 +12395,65 @@ class Sema final : public SemaBase {
/// failures rather than hard errors.
bool AccessCheckingSFINAE;
+ class SFINAETrap;
+
+ struct SFINAEContextBase {
+ SFINAEContextBase(Sema &S, SFINAETrap *Cur)
+ : S(S), Prev(std::exchange(S.CurrentSFINAEContext, Cur)) {}
+
+ protected:
+ Sema &S;
+ ~SFINAEContextBase() { S.CurrentSFINAEContext = Prev; }
+
+ private:
+ SFINAETrap *Prev;
+ };
+
+ struct NonSFINAEContext : SFINAEContextBase {
+ NonSFINAEContext(Sema &S) : SFINAEContextBase(S, nullptr) {}
+ };
+
/// RAII class used to determine whether SFINAE has
/// trapped any errors that occur during template argument
/// deduction.
- class SFINAETrap {
- Sema &SemaRef;
- unsigned PrevSFINAEErrors;
- bool PrevInNonInstantiationSFINAEContext;
- bool PrevAccessCheckingSFINAE;
- bool PrevLastDiagnosticIgnored;
+ class SFINAETrap : SFINAEContextBase {
+ bool HasErrorOcurred = false;
+ bool PrevAccessCheckingSFINAE = S.AccessCheckingSFINAE;
+ bool PrevLastDiagnosticIgnored =
+ S.getDiagnostics().isLastDiagnosticIgnored();
+ sema::TemplateDeductionInfo *DeductionInfo = nullptr;
+
+ SFINAETrap(Sema &S, sema::TemplateDeductionInfo *Info,
+ bool ForValidityCheck)
+ : SFINAEContextBase(S, this), DeductionInfo(Info) {
+ S.AccessCheckingSFINAE = ForValidityCheck;
+ }
public:
/// \param ForValidityCheck If true, discard all diagnostics (from the
/// immediate context) instead of adding them to the currently active
/// \ref TemplateDeductionInfo (as returned by \ref isSFINAEContext).
- explicit SFINAETrap(Sema &SemaRef, bool ForValidityCheck = false)
- : SemaRef(SemaRef), PrevSFINAEErrors(SemaRef.NumSFINAEErrors),
- PrevInNonInstantiationSFINAEContext(
- SemaRef.InNonInstantiationSFINAEContext),
- PrevAccessCheckingSFINAE(SemaRef.AccessCheckingSFINAE),
- PrevLastDiagnosticIgnored(
- SemaRef.getDiagnostics().isLastDiagnosticIgnored()) {
- if (ForValidityCheck || !SemaRef.isSFINAEContext())
- SemaRef.InNonInstantiationSFINAEContext = true;
- SemaRef.AccessCheckingSFINAE = ForValidityCheck;
- }
+ explicit SFINAETrap(Sema &S, bool ForValidityCheck = false)
+ : SFINAETrap(S, /*Info=*/nullptr, ForValidityCheck) {}
+
+ SFINAETrap(Sema &S, sema::TemplateDeductionInfo &Info)
+ : SFINAETrap(S, &Info, /*ForValidityCheck=*/false) {}
~SFINAETrap() {
- SemaRef.NumSFINAEErrors = PrevSFINAEErrors;
- SemaRef.InNonInstantiationSFINAEContext =
- PrevInNonInstantiationSFINAEContext;
- SemaRef.AccessCheckingSFINAE = PrevAccessCheckingSFINAE;
- SemaRef.getDiagnostics().setLastDiagnosticIgnored(
- PrevLastDiagnosticIgnored);
+ S.AccessCheckingSFINAE = PrevAccessCheckingSFINAE;
+ S.getDiagnostics().setLastDiagnosticIgnored(PrevLastDiagnosticIgnored);
}
- /// Determine whether any SFINAE errors have been trapped.
- bool hasErrorOccurred() const {
- return SemaRef.NumSFINAEErrors > PrevSFINAEErrors;
+ SFINAETrap(const SFINAETrap &) = delete;
+ SFINAETrap &operator=(const SFINAETrap &) = delete;
+
+ sema::TemplateDeductionInfo *getDeductionInfo() const {
+ return DeductionInfo;
}
+
+ /// Determine whether any SFINAE errors have been trapped.
+ bool hasErrorOccurred() const { return HasErrorOcurred; }
+ void setErrorOccurred() { HasErrorOcurred = true; }
};
/// RAII class used to indicate that we are performing provisional
@@ -13157,9 +13174,6 @@ class Sema final : public SemaBase {
PartialOrderingTTP,
} Kind;
- /// Was the enclosing context a non-instantiation SFINAE context?
- bool SavedInNonInstantiationSFINAEContext;
-
/// Whether we're substituting into constraints.
bool InConstraintSubstitution;
@@ -13204,22 +13218,15 @@ class Sema final : public SemaBase {
return {TemplateArgs, NumTemplateArgs};
}
- /// The template deduction info object associated with the
- /// substitution or checking of explicit or deduced template arguments.
- sema::TemplateDeductionInfo *DeductionInfo;
-
/// The source range that covers the construct that cause
/// the instantiation, e.g., the template-id that causes a class
/// template instantiation.
SourceRange InstantiationRange;
CodeSynthesisContext()
- : Kind(TemplateInstantiation),
- SavedInNonInstantiationSFINAEContext(false),
- InConstraintSubstitution(false),
+ : Kind(TemplateInstantiation), InConstraintSubstitution(false),
InParameterMappingSubstitution(false), Entity(nullptr),
- Template(nullptr), TemplateArgs(nullptr), NumTemplateArgs(0),
- DeductionInfo(nullptr) {}
+ Template(nullptr), TemplateArgs(nullptr), NumTemplateArgs(0) {}
/// Determines whether this template is an actual instantiation
/// that should be counted toward the maximum instantiation depth.
@@ -13271,7 +13278,6 @@ class Sema final : public SemaBase {
FunctionTemplateDecl *FunctionTemplate,
ArrayRef<TemplateArgument> TemplateArgs,
CodeSynthesisContext::SynthesisKind Kind,
- sema::TemplateDeductionInfo &DeductionInfo,
SourceRange InstantiationRange = SourceRange());
/// Note that we are instantiating as part of template
@@ -13279,7 +13285,6 @@ class Sema final : public SemaBase {
InstantiatingTemplate(Sema &SemaRef, SourceLocation PointOfInstantiation,
TemplateDecl *Template,
ArrayRef<TemplateArgument> TemplateArgs,
- sema::TemplateDeductionInfo &DeductionInfo,
SourceRange InstantiationRange = SourceRange());
/// Note that we are instantiating as part of template
@@ -13288,7 +13293,6 @@ class Sema final : public SemaBase {
InstantiatingTemplate(Sema &SemaRef, SourceLocation PointOfInstantiation,
ClassTemplatePartialSpecializationDecl *PartialSpec,
ArrayRef<TemplateArgument> TemplateArgs,
- sema::TemplateDeductionInfo &DeductionInfo,
SourceRange InstantiationRange = SourceRange());
/// Note that we are instantiating as part of template
@@ -13297,7 +13301,6 @@ class Sema final : public SemaBase {
InstantiatingTemplate(Sema &SemaRef, SourceLocation PointOfInstantiation,
VarTemplatePartialSpecializationDecl *PartialSpec,
ArrayRef<TemplateArgument> TemplateArgs,
- sema::TemplateDeductionInfo &DeductionInfo,
SourceRange InstantiationRange = SourceRange());
/// Note that we are instantiating a default argument for a function
@@ -13343,7 +13346,6 @@ class Sema final : public SemaBase {
/// concept.
InstantiatingTemplate(Sema &SemaRef, SourceLocation PointOfInstantiation,
ConstraintSubstitution, NamedDecl *Template,
- sema::TemplateDeductionInfo &DeductionInfo,
SourceRange InstantiationRange);
struct ConstraintNormalization {};
@@ -13363,7 +13365,6 @@ class Sema final : public SemaBase {
/// a requirement of a requires expression.
InstantiatingTemplate(Sema &SemaRef, SourceLocation PointOfInstantiation,
concepts::Requirement *Req,
- sema::TemplateDeductionInfo &DeductionInfo,
SourceRange InstantiationRange = SourceRange());
/// \brief Note that we are checking the satisfaction of the constraint
@@ -13375,7 +13376,6 @@ class Sema final : public SemaBase {
/// \brief Note that we are checking a requires clause.
InstantiatingTemplate(Sema &SemaRef, SourceLocation PointOfInstantiation,
const RequiresExpr *E,
- sema::TemplateDeductionInfo &DeductionInfo,
SourceRange InstantiationRange);
struct BuildingDeductionGuidesTag {};
@@ -13408,8 +13408,7 @@ class Sema final : public SemaBase {
SourceLocation PointOfInstantiation,
SourceRange InstantiationRange, Decl *Entity,
NamedDecl *Template = nullptr,
- ArrayRef<TemplateArgument> TemplateArgs = {},
- sema::TemplateDeductionInfo *DeductionInfo = nullptr);
+ ArrayRef<TemplateArgument> TemplateArgs = {});
InstantiatingTemplate(const InstantiatingTemplate &) = delete;
@@ -13550,12 +13549,7 @@ class Sema final : public SemaBase {
/// recent visible declaration of that namespace.
llvm::DenseMap<NamedDecl *, NamedDecl *> VisibleNamespaceCache;
- /// Whether we are in a SFINAE context that is not associated with
- /// template instantiation.
- ///
- /// This is used when setting up a SFINAE trap (\c see SFINAETrap) outside
- /// of a template instantiation or template argument deduction.
- bool InNonInstantiationSFINAEContext;
+ SFINAETrap *CurrentSFINAEContext = nullptr;
/// The number of \p CodeSynthesisContexts that are not template
/// instantiations and, therefore, should not be counted as part of the
@@ -13626,15 +13620,10 @@ class Sema final : public SemaBase {
PrintInstantiationStack(getDefaultDiagFunc());
}
- /// Determines whether we are currently in a context where
- /// template argument substitution failures are not considered
- /// errors.
- ///
- /// \returns An empty \c Optional if we're not in a SFINAE context.
- /// Otherwise, contains a pointer that, if non-NULL, contains the nearest
- /// template-deduction context object, which can be used to capture
- /// diagnostics that will be suppressed.
- std::optional<sema::TemplateDeductionInfo *> isSFINAEContext() const;
+ /// Returns a pointer to the current SFINAE context, if any.
+ [[nodiscard]] SFINAETrap *getSFINAEContext() const {
+ return CurrentSFINAEContext;
+ }
/// Perform substitution on the type T with a given set of template
/// arguments.
@@ -14646,7 +14635,8 @@ class Sema final : public SemaBase {
ArrayRef<UnexpandedParameterPack> Unexpanded,
const MultiLevelTemplateArgumentList &TemplateArgs,
bool FailOnPackProducingTemplates, bool &ShouldExpand,
- bool &RetainExpansion, UnsignedOrNone &NumExpansions);
+ bool &RetainExpansion, UnsignedOrNone &NumExpansions,
+ bool Diagnose = true);
/// Determine the number of arguments in the given pack expansion
/// type.
diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp
index 39fa25f66f3b7..26a4260b825b1 100644
--- a/clang/lib/Sema/Sema.cpp
+++ b/clang/lib/Sema/Sema.cpp
@@ -322,10 +322,10 @@ Sema::Sema(Preprocessor &pp, ASTContext &ctxt, ASTConsumer &consumer,
static_cast<unsigned>(ComparisonCategoryType::Last) + 1),
StdSourceLocationImplDecl(nullptr), CXXTypeInfoDecl(nullptr),
GlobalNewDeleteDeclared(false), DisableTypoCorrection(false),
- TyposCorrected(0), IsBuildingRecoveryCallExpr(false), NumSFINAEErrors(0),
+ TyposCorrected(0), IsBuildingRecoveryCallExpr(false),
AccessCheckingSFINAE(false), CurrentInstantiationScope(nullptr),
- InNonInstantiationSFINAEContext(false), NonInstantiationEntries(0),
- ArgPackSubstIndex(std::nullopt), SatisfactionCache(Context) {
+ NonInstantiationEntries(0), ArgPackSubstIndex(std::nullopt),
+ SatisfactionCache(Context) {
assert(pp.TUKind == TUKind);
TUScope = nullptr;
@@ -671,7 +671,9 @@ void Sema::addExternalSource(IntrusiveRefCntPtr<ExternalSemaSource> E) {
void Sema::PrintStats() const {
llvm::errs() << "\n*** Semantic Analysis Stats:\n";
- llvm::errs() << NumSFINAEErrors << " SFINAE diagnostics trapped.\n";
+ if (SFINAETrap *Trap = getSFINAEContext())
+ llvm::errs() << int(Trap->hasErrorOccurred())
+ << " SFINAE diagnostics trapped.\n";
BumpAlloc.PrintStats();
AnalysisWarnings.PrintStats();
@@ -1678,7 +1680,8 @@ void Sema::EmitDiagnostic(unsigned DiagID, const DiagnosticBuilder &DB) {
// issue I am not seeing yet), then there should at least be a clarifying
// comment somewhere.
Diagnostic DiagInfo(&Diags, DB);
- if (std::optional<TemplateDeductionInfo *> Info = isSFINAEContext()) {
+ if (SFINAETrap *Trap = getSFINAEContext()) {
+ sema::TemplateDeductionInfo *Info = Trap->getDeductionInfo();
switch (DiagnosticIDs::getDiagnosticSFINAEResponse(DiagInfo.getID())) {
case DiagnosticIDs::SFINAE_Report:
// We'll report the diagnostic below.
@@ -1687,14 +1690,14 @@ void Sema::EmitDiagnostic(unsigned DiagID, const DiagnosticBuilder &DB) {
case DiagnosticIDs::SFINAE_SubstitutionFailure:
// Count this failure so that we know that template argument deduction
// has failed.
- ++NumSFINAEErrors;
+ Trap->setErrorOccurred();
// Make a copy of this suppressed diagnostic and store it with the
// template-deduction information.
- if (*Info && !(*Info)->hasSFINAEDiagnostic()) {
- (*Info)->addSFINAEDiagnostic(DiagInfo.getLocation(),
- PartialDiagnostic(DiagInfo, Context.getDiagAllocator()));
- }
+ if (Info && !Info->hasSFINAEDiagnostic())
+ Info->addSFINAEDiagnostic(
+ DiagInfo.getLocation(),
+ PartialDiagnostic(DiagInfo, Context.getDiagAllocator()));
Diags.setLastDiagnosticIgnored(true);
return;
@@ -1710,14 +1713,14 @@ void Sema::EmitDiagnostic(unsigned DiagID, const DiagnosticBuilder &DB) {
SourceLocation Loc = DiagInfo.getLocation();
// Suppress this diagnostic.
- ++NumSFINAEErrors;
+ Trap->setErrorOccurred();
// Make a copy of this suppressed diagnostic and store it with the
// template-deduction information.
- if (*Info && !(*Info)->hasSFINAEDiagnostic()) {
- (*Info)->addSFINAEDiagnostic(DiagInfo.getLocation(),
- PartialDiagnostic(DiagInfo, Context.getDiagAllocator()));
- }
+ if (Info && !Info->hasSFINAEDiagnostic())
+ Info->addSFINAEDiagnostic(
+ DiagInfo.getLocation(),
+ PartialDiagnostic(DiagInfo, Context.getDiagAllocator()));
Diags.setLastDiagnosticIgnored(true);
@@ -1737,13 +1740,13 @@ void Sema::EmitDiagnostic(unsigned DiagID, const DiagnosticBuilder &DB) {
return;
// Make a copy of this suppressed diagnostic and store it with the
// template-deduction information;
- if (*Info) {
- (*Info)->addSuppressedDiagnostic(
+ if (Info) {
+ Info->addSuppressedDiagnostic(
DiagInfo.getLocation(),
PartialDiagnostic(DiagInfo, Context.getDiagAllocator()));
if (!Diags.getDiagnosticIDs()->isNote(DiagID))
PrintContextStack([Info](SourceLocation Loc, PartialDiagnostic PD) {
- (*Info)->addSuppressedDiagnostic(Loc, std::move(PD));
+ Info->addSuppressedDiagnostic(Loc, std::move(PD));
});
}
@@ -2827,7 +2830,7 @@ bool Sema::tryToRecoverWithCall(ExprResult &E, const PartialDiagnostic &PD,
// If this is a SFINAE context, don't try anything that might trigger ADL
// prematurely.
- if (!isSFINAEContext()) {
+ if (!getSFINAEContext()) {
QualType ZeroArgCallTy;
if (tryExprAsCall(*E.get(), ZeroArgCallTy, Overloads) &&
!ZeroArgCallTy.isNull() &&
diff --git a/clang/lib/Sema/SemaAMDGPU.cpp b/clang/lib/Sema/SemaAMDGPU.cpp
index e32f4376a5ebf..71238c0720dc9 100644
--- a/clang/lib/Sema/SemaAMDGPU.cpp
+++ b/clang/lib/Sema/SemaAMDGPU.cpp
@@ -517,6 +517,8 @@ AMDGPUMaxNumWorkGroupsAttr *SemaAMDGPU::CreateAMDGPUMaxNumWorkGroupsAttr(
const AttributeCommonInfo &CI, Expr *XExpr, Expr *YExpr, Expr *ZExpr) {
ASTContext &Context = getASTContext();
AMDGPUMaxNumWorkGroupsAttr TmpAttr(Context, CI, XExpr, YExpr, ZExpr);
+ assert(!SemaRef.getSFINAEContext() &&
+ "Can't produce SFINAE diagnostic pointing to temporary attribute");
if (checkAMDGPUMaxNumWorkGroupsArguments(SemaRef, XExpr, YExpr, ZExpr,
TmpAttr))
diff --git a/clang/lib/Sema/SemaCXXScopeSpec.cpp b/clang/lib/Sema/SemaCXXScopeSpec.cpp
index c52fc5bf815af..7fe21af85433a 100644
--- a/clang/lib/Sema/SemaCXXScopeSpec.cpp
+++ b/clang/lib/Sema/SemaCXXScopeSpec.cpp
@@ -100,8 +100,8 @@ DeclContext *Sema::computeDeclContext(const CXXScopeSpec &SS,
// A declaration of the partial specialization must be visible.
// We can always recover here, because this only happens when we're
// entering the context, and that can't happen in a SFINAE context.
- assert(!isSFINAEContext() && "partial specialization scope "
- "specifier in SFINAE context?");
+ assert(!getSFINAEContext() && "partial specialization scope "
+ "specifier in SFINAE context?");
if (PartialSpec->hasDefinition() &&
!hasReachableDefinition(PartialSpec))
diagnoseMissingImport(SS.getLastQualifierNameLoc(), PartialSpec,
@@ -227,7 +227,7 @@ bool Sema::RequireCompleteEnumDecl(EnumDecl *EnumD, SourceLocation L,
/*OnlyNeedComplete*/ false)) {
// If the user is going to see an error here, recover by making the
// definition visible.
- bool TreatAsComplete = !isSFINAEContext();
+ bool TreatAsComplete = !getSFINAEContext();
diagnoseMissingImport(L, SuggestedDef, MissingImportKind::Definition,
/*Recover*/ TreatAsComplete);
re...
[truncated]
|
cor3ntin
reviewed
Oct 22, 2025
mizvekov
force-pushed
the
users/mizvekov/sfinae-context-refactor
branch
from
1dbada8 to
5b3fba3
Compare
zyn0217
approved these changes
Oct 23, 2025
Comment on lines
+12404
to
+12397
| private: | ||
| SFINAETrap *Prev; | ||
| }; |
There was a problem hiding this comment.
It looks weird to have a sub-class's pointer in a base class. Though this is not a blocker
There was a problem hiding this comment.
It does allow to share the CurrentSFINAEContext setup between all derived classes.
If you have a better idea I am all ears.
There was a problem hiding this comment.
Seems that doing so makes clients enough simple, though I don't think I have better ideas yet :(
There was a problem hiding this comment.
The alternative would be SFINAEContext _(S, SFINAEContext::NoSFINAE) - that would also make sense to me.
I don't have strong feelings
mizvekov
force-pushed
the
users/mizvekov/sfinae-context-refactor
branch
from
5b3fba3 to
e709b8e
Compare
cor3ntin
reviewed
Oct 23, 2025
Comment on lines
+12404
to
+12397
| private: | ||
| SFINAETrap *Prev; | ||
| }; |
There was a problem hiding this comment.
The alternative would be SFINAEContext _(S, SFINAEContext::NoSFINAE) - that would also make sense to me.
I don't have strong feelings
| SFINAETrap *Prev; | ||
| }; | ||
|
|
||
| struct NonSFINAEContext : SFINAEContextBase { |
There was a problem hiding this comment.
Suggested change
| struct NonSFINAEContext : SFINAEContextBase { | |
| struct NonSFINAEContextRAII : SFINAEContextBase { |
Maybe that would be clearer?
There was a problem hiding this comment.
I think having both the 'Context' and 'RAII' qualifiers would be a bit too much, feels redundant to me.
There was a problem hiding this comment.
You wanted to rename ContextRAII 😄?
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:
mizvekov
force-pushed
the
users/mizvekov/sfinae-context-refactor
branch
from
e709b8e to
526a4c9
Compare
|
If there are no blocking objections, I plan to merge this tomorrow. We can always do any renames later, this is used in few places so that's an easy change. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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:
