[clang] 56a8729 - Remove improper uses of DiagnosticErrorTrap and hasErrorOccurred.
Richard Smith via cfe-commits
cfe-commits at lists.llvm.org
Mon Jun 8 14:20:09 PDT 2020
Author: Richard Smith
Date: 2020-06-08T14:19:57-07:00
New Revision: 56a872947acca7635ca969f39f50062f2f779af0
URL: https://github.com/llvm/llvm-project/commit/56a872947acca7635ca969f39f50062f2f779af0
DIFF: https://github.com/llvm/llvm-project/commit/56a872947acca7635ca969f39f50062f2f779af0.diff
LOG: Remove improper uses of DiagnosticErrorTrap and hasErrorOccurred.
DiagnosticErrorTrap is usually inappropriate because it indicates
whether an error message was rendered in a given region (and is
therefore affected by -ferror-limit and by suppression of errors if we
see an invalid declaration).
hasErrorOccurred() is usually inappropriate because it indicates
whethere an "error:" message was displayed, regardless of whether the
message was a warning promoted to an error, and therefore depends on
things like -Werror that are usually irrelevant.
Where applicable, CodeSynthesisContexts are used to attach notes to
the first diagnostic produced in a region of code, isnstead of using an
error trap and then attaching a note to whichever diagnostic happened to
be produced last (or suppressing the note if the final diagnostic is a
disabled warning!).
This is mostly NFC.
Added:
Modified:
clang/include/clang/Basic/Diagnostic.h
clang/include/clang/Sema/Scope.h
clang/include/clang/Sema/ScopeInfo.h
clang/include/clang/Sema/Sema.h
clang/lib/Frontend/FrontendActions.cpp
clang/lib/Parse/ParseExprCXX.cpp
clang/lib/Sema/Sema.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/lib/Sema/SemaExprCXX.cpp
clang/lib/Sema/SemaTemplateInstantiate.cpp
clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
clang/test/Parser/cxx2a-concepts-requires-expr.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/Basic/Diagnostic.h b/clang/include/clang/Basic/Diagnostic.h
index 9cc94c38e60d..304207779c0f 100644
--- a/clang/include/clang/Basic/Diagnostic.h
+++ b/clang/include/clang/Basic/Diagnostic.h
@@ -1010,6 +1010,11 @@ class DiagnosticsEngine : public RefCountedBase<DiagnosticsEngine> {
/// RAII class that determines when any errors have occurred
/// between the time the instance was created and the time it was
/// queried.
+///
+/// Note that you almost certainly do not want to use this. It's usually
+/// meaningless to ask whether a particular scope triggered an error message,
+/// because error messages outside that scope can mark things invalid (or cause
+/// us to reach an error limit), which can suppress errors within that scope.
class DiagnosticErrorTrap {
DiagnosticsEngine &Diag;
unsigned NumErrors;
diff --git a/clang/include/clang/Sema/Scope.h b/clang/include/clang/Sema/Scope.h
index 169ca175eed2..9b8f1415a1e3 100644
--- a/clang/include/clang/Sema/Scope.h
+++ b/clang/include/clang/Sema/Scope.h
@@ -325,8 +325,10 @@ class Scope {
DeclContext *getEntity() const { return Entity; }
void setEntity(DeclContext *E) { Entity = E; }
- bool hasErrorOccurred() const { return ErrorTrap.hasErrorOccurred(); }
-
+ /// Determine whether any unrecoverable errors have occurred within this
+ /// scope. Note that this may return false even if the scope contains invalid
+ /// declarations or statements, if the errors for those invalid constructs
+ /// were suppressed because some prior invalid construct was referenced.
bool hasUnrecoverableErrorOccurred() const {
return ErrorTrap.hasUnrecoverableErrorOccurred();
}
diff --git a/clang/include/clang/Sema/ScopeInfo.h b/clang/include/clang/Sema/ScopeInfo.h
index 3c4847a2932c..f0f9cb9e40ae 100644
--- a/clang/include/clang/Sema/ScopeInfo.h
+++ b/clang/include/clang/Sema/ScopeInfo.h
@@ -174,9 +174,11 @@ class FunctionScopeInfo {
/// First SEH '__try' statement in the current function.
SourceLocation FirstSEHTryLoc;
+private:
/// Used to determine if errors occurred in this function or block.
DiagnosticErrorTrap ErrorTrap;
+public:
/// A SwitchStmt, along with a flag indicating if its list of case statements
/// is incomplete (because we dropped an invalid one while parsing).
using SwitchInfo = llvm::PointerIntPair<SwitchStmt*, 1, bool>;
@@ -375,6 +377,17 @@ class FunctionScopeInfo {
virtual ~FunctionScopeInfo();
+ /// Determine whether an unrecoverable error has occurred within this
+ /// function. Note that this may return false even if the function body is
+ /// invalid, because the errors may be suppressed if they're caused by prior
+ /// invalid declarations.
+ ///
+ /// FIXME: Migrate the caller of this to use containsErrors() instead once
+ /// it's ready.
+ bool hasUnrecoverableErrorOccurred() const {
+ return ErrorTrap.hasUnrecoverableErrorOccurred();
+ }
+
/// Record that a weak object was accessed.
///
/// Part of the implementation of -Wrepeated-use-of-weak.
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 8f914ec451f6..8df78a5a7aca 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -8293,6 +8293,12 @@ class Sema final {
/// We are rewriting a comparison operator in terms of an operator<=>.
RewritingOperatorAsSpaceship,
+ /// We are initializing a structured binding.
+ InitializingStructuredBinding,
+
+ /// We are marking a class as __dllexport.
+ MarkingClassDllexported,
+
/// Added for Template instantiation observation.
/// Memoization means we are _not_ instantiating a template because
/// it is already instantiated (but we entered a context where we
diff --git a/clang/lib/Frontend/FrontendActions.cpp b/clang/lib/Frontend/FrontendActions.cpp
index 42615d27e92c..3a86a8d05c49 100644
--- a/clang/lib/Frontend/FrontendActions.cpp
+++ b/clang/lib/Frontend/FrontendActions.cpp
@@ -434,6 +434,10 @@ class DefaultTemplateInstCallback : public TemplateInstantiationCallback {
return "RequirementInstantiation";
case CodeSynthesisContext::NestedRequirementConstraintsCheck:
return "NestedRequirementConstraintsCheck";
+ case CodeSynthesisContext::InitializingStructuredBinding:
+ return "InitializingStructuredBinding";
+ case CodeSynthesisContext::MarkingClassDllexported:
+ return "MarkingClassDllexported";
}
return "";
}
diff --git a/clang/lib/Parse/ParseExprCXX.cpp b/clang/lib/Parse/ParseExprCXX.cpp
index bf7ada348878..d7947777977a 100644
--- a/clang/lib/Parse/ParseExprCXX.cpp
+++ b/clang/lib/Parse/ParseExprCXX.cpp
@@ -3369,7 +3369,6 @@ ExprResult Parser::ParseRequiresExpression() {
ParsedAttributes FirstArgAttrs(getAttrFactory());
SourceLocation EllipsisLoc;
llvm::SmallVector<DeclaratorChunk::ParamInfo, 2> LocalParameters;
- DiagnosticErrorTrap Trap(Diags);
ParseParameterDeclarationClause(DeclaratorContext::RequiresExprContext,
FirstArgAttrs, LocalParameters,
EllipsisLoc);
@@ -3377,8 +3376,6 @@ ExprResult Parser::ParseRequiresExpression() {
Diag(EllipsisLoc, diag::err_requires_expr_parameter_list_ellipsis);
for (auto &ParamInfo : LocalParameters)
LocalParameterDecls.push_back(cast<ParmVarDecl>(ParamInfo.Param));
- if (Trap.hasErrorOccurred())
- SkipUntil(tok::r_paren, StopBeforeMatch);
}
Parens.consumeClose();
}
diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp
index ffe2e4d4d56a..49fbde85cdfb 100644
--- a/clang/lib/Sema/Sema.cpp
+++ b/clang/lib/Sema/Sema.cpp
@@ -1952,7 +1952,7 @@ void Sema::PopCompoundScope() {
/// Determine whether any errors occurred within this function/method/
/// block.
bool Sema::hasAnyUnrecoverableErrorsInThisFunction() const {
- return getCurFunction()->ErrorTrap.hasUnrecoverableErrorOccurred();
+ return getCurFunction()->hasUnrecoverableErrorOccurred();
}
void Sema::setFunctionHasBranchIntoScope() {
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 64e93963727e..401aea355c41 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -14385,7 +14385,7 @@ Decl *Sema::ActOnFinishFunctionBody(Decl *dcl, Stmt *Body,
// If any errors have occurred, clear out any temporaries that may have
// been leftover. This ensures that these temporaries won't be picked up for
// deletion in some later function.
- if (getDiagnostics().hasErrorOccurred() ||
+ if (getDiagnostics().hasUncompilableErrorOccurred() ||
getDiagnostics().getSuppressAllDiagnostics()) {
DiscardCleanupsInEvaluationContext();
}
@@ -14441,7 +14441,7 @@ Decl *Sema::ActOnFinishFunctionBody(Decl *dcl, Stmt *Body,
// If any errors have occurred, clear out any temporaries that may have
// been leftover. This ensures that these temporaries won't be picked up for
// deletion in some later function.
- if (getDiagnostics().hasErrorOccurred()) {
+ if (getDiagnostics().hasUncompilableErrorOccurred()) {
DiscardCleanupsInEvaluationContext();
}
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index ed12b6cacdbe..09d381a5a1ba 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -1101,16 +1101,17 @@ static QualType getTupleLikeElementType(Sema &S, SourceLocation Loc,
}
namespace {
-struct BindingDiagnosticTrap {
+struct InitializingBinding {
Sema &S;
- DiagnosticErrorTrap Trap;
- BindingDecl *BD;
-
- BindingDiagnosticTrap(Sema &S, BindingDecl *BD)
- : S(S), Trap(S.Diags), BD(BD) {}
- ~BindingDiagnosticTrap() {
- if (Trap.hasErrorOccurred())
- S.Diag(BD->getLocation(), diag::note_in_binding_decl_init) << BD;
+ InitializingBinding(Sema &S, BindingDecl *BD) : S(S) {
+ Sema::CodeSynthesisContext Ctx;
+ Ctx.Kind = Sema::CodeSynthesisContext::InitializingStructuredBinding;
+ Ctx.PointOfInstantiation = BD->getLocation();
+ Ctx.Entity = BD;
+ S.pushCodeSynthesisContext(Ctx);
+ }
+ ~InitializingBinding() {
+ S.popCodeSynthesisContext();
}
};
}
@@ -1159,7 +1160,7 @@ static bool checkTupleLikeDecomposition(Sema &S,
unsigned I = 0;
for (auto *B : Bindings) {
- BindingDiagnosticTrap Trap(S, B);
+ InitializingBinding InitContext(S, B);
SourceLocation Loc = B->getLocation();
ExprResult E = S.BuildDeclRefExpr(Src, DecompType, VK_LValue, Loc);
@@ -5797,6 +5798,23 @@ static void ReferenceDllExportedMembers(Sema &S, CXXRecordDecl *Class) {
// declaration.
return;
+ // Add a context note to explain how we got to any diagnostics produced below.
+ struct MarkingClassDllexported {
+ Sema &S;
+ MarkingClassDllexported(Sema &S, CXXRecordDecl *Class,
+ SourceLocation AttrLoc)
+ : S(S) {
+ Sema::CodeSynthesisContext Ctx;
+ Ctx.Kind = Sema::CodeSynthesisContext::MarkingClassDllexported;
+ Ctx.PointOfInstantiation = AttrLoc;
+ Ctx.Entity = Class;
+ S.pushCodeSynthesisContext(Ctx);
+ }
+ ~MarkingClassDllexported() {
+ S.popCodeSynthesisContext();
+ }
+ } MarkingDllexportedContext(S, Class, ClassAttr->getLocation());
+
if (S.Context.getTargetInfo().getTriple().isWindowsGNUEnvironment())
S.MarkVTableUsed(Class->getLocation(), Class, true);
@@ -5832,13 +5850,7 @@ static void ReferenceDllExportedMembers(Sema &S, CXXRecordDecl *Class) {
// defaulted methods, and the copy and move assignment operators. The
// latter are exported even if they are trivial, because the address of
// an operator can be taken and should compare equal across libraries.
- DiagnosticErrorTrap Trap(S.Diags);
S.MarkFunctionReferenced(Class->getLocation(), MD);
- if (Trap.hasErrorOccurred()) {
- S.Diag(ClassAttr->getLocation(), diag::note_due_to_dllexported_class)
- << Class << !S.getLangOpts().CPlusPlus11;
- break;
- }
// There is no later point when we will see the definition of this
// function, so pass it to the consumer now.
diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index c22af86d57aa..b3d04e40b5e9 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -7595,13 +7595,13 @@ ExprResult Sema::BuildCXXMemberCallExpr(Expr *E, NamedDecl *FoundDecl,
// a
diff erence in ARC, but outside of ARC the resulting block literal
// follows the normal lifetime rules for block literals instead of being
// autoreleased.
- DiagnosticErrorTrap Trap(Diags);
PushExpressionEvaluationContext(
ExpressionEvaluationContext::PotentiallyEvaluated);
ExprResult BlockExp = BuildBlockForLambdaConversion(
Exp.get()->getExprLoc(), Exp.get()->getExprLoc(), Method, Exp.get());
PopExpressionEvaluationContext();
+ // FIXME: This note should be produced by a CodeSynthesisContext.
if (BlockExp.isInvalid())
Diag(Exp.get()->getExprLoc(), diag::note_lambda_to_block_conv);
return BlockExp;
diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp
index 0cb50b3ea368..a27b87e3c801 100644
--- a/clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -215,6 +215,8 @@ bool Sema::CodeSynthesisContext::isInstantiationRecord() const {
case ParameterMappingSubstitution:
case ConstraintNormalization:
case RewritingOperatorAsSpaceship:
+ case InitializingStructuredBinding:
+ case MarkingClassDllexported:
return false;
// This function should never be called when Kind's value is Memoization.
@@ -760,6 +762,18 @@ void Sema::PrintInstantiationStack() {
diag::note_rewriting_operator_as_spaceship);
break;
+ case CodeSynthesisContext::InitializingStructuredBinding:
+ Diags.Report(Active->PointOfInstantiation,
+ diag::note_in_binding_decl_init)
+ << cast<BindingDecl>(Active->Entity);
+ break;
+
+ case CodeSynthesisContext::MarkingClassDllexported:
+ Diags.Report(Active->PointOfInstantiation,
+ diag::note_due_to_dllexported_class)
+ << cast<CXXRecordDecl>(Active->Entity) << !getLangOpts().CPlusPlus11;
+ break;
+
case CodeSynthesisContext::Memoization:
break;
@@ -861,6 +875,8 @@ Optional<TemplateDeductionInfo *> Sema::isSFINAEContext() const {
case CodeSynthesisContext::DeclaringImplicitEqualityComparison:
case CodeSynthesisContext::DefiningSynthesizedFunction:
case CodeSynthesisContext::RewritingOperatorAsSpaceship:
+ case CodeSynthesisContext::InitializingStructuredBinding:
+ case CodeSynthesisContext::MarkingClassDllexported:
// This happens in a context unrelated to template instantiation, so
// there is no SFINAE.
return None;
diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 519d9128037d..b4aa234a3934 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -5879,10 +5879,11 @@ NamedDecl *Sema::FindInstantiatedDecl(SourceLocation Loc, NamedDecl *D,
if (!Result) {
if (isa<UsingShadowDecl>(D)) {
// UsingShadowDecls can instantiate to nothing because of using hiding.
- } else if (Diags.hasErrorOccurred()) {
- // We've already complained about something, so most likely this
- // declaration failed to instantiate. There's no point in complaining
- // further, since this is normal in invalid code.
+ } else if (Diags.hasUncompilableErrorOccurred()) {
+ // We've already complained about some ill-formed code, so most likely
+ // this declaration failed to instantiate. There's no point in
+ // complaining further, since this is normal in invalid code.
+ // FIXME: Use more fine-grained 'invalid' tracking for this.
} else if (IsBeingInstantiated) {
// The class in which this member exists is currently being
// instantiated, and we haven't gotten around to instantiating this
diff --git a/clang/test/Parser/cxx2a-concepts-requires-expr.cpp b/clang/test/Parser/cxx2a-concepts-requires-expr.cpp
index fa42b0633850..d1a31b4f93ef 100644
--- a/clang/test/Parser/cxx2a-concepts-requires-expr.cpp
+++ b/clang/test/Parser/cxx2a-concepts-requires-expr.cpp
@@ -43,6 +43,7 @@ bool r14 = requires (int (*f)(int)) { requires true; };
bool r15 = requires (10) { requires true; };
// expected-error at -1 {{expected parameter declarator}}
+// expected-error at -2 {{expected ')'}} expected-note at -2 {{to match}}
bool r16 = requires (auto x) { requires true; };
// expected-error at -1 {{'auto' not allowed in requires expression parameter}}
More information about the cfe-commits
mailing list