Tyker updated this revision to Diff 382702.
Tyker added a comment.
In D74130#3085313 <https://reviews.llvm.org/D74130#3085313>, @aaron.ballman
wrote:
> FWIW, I am not seeing double errors on that code. Here's the output I get
> with this patch applied locally:
>
> F:\source\llvm-project>cat "C:\Users\aballman\OneDrive - Intel
> Corporation\Desktop\test.cpp"
> consteval int f1() { return 0; }
> consteval auto g() { return f1; }
>
> constexpr auto e = g();
> constexpr auto e1 = f1;
>
> F:\source\llvm-project>llvm\out\build\x64-Debug\bin\clang.exe -fsyntax-only
> -std=c++2b "C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.cpp"
> C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.cpp:4:20:
> error: call to consteval function 'g' is not a
> constant expression
> constexpr auto e = g();
> ^
> C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.cpp:4:20: note:
> pointer to a consteval declaration is not a
> constant expression
> C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.cpp:1:15: note:
> declared here
> consteval int f1() { return 0; }
> ^
> C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.cpp:4:16:
> error: constexpr variable 'e' must be initialized
> by a constant expression
> constexpr auto e = g();
> ^ ~~~
> C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.cpp:4:16: note:
> pointer to a consteval declaration is not a
> constant expression
> C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.cpp:1:15: note:
> declared here
> consteval int f1() { return 0; }
> ^
> C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.cpp:5:21:
> error: cannot take address of consteval function
> 'f1' outside of an immediate invocation
> constexpr auto e1 = f1;
> ^
> C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.cpp:1:15: note:
> declared here
> consteval int f1() { return 0; }
> ^
> C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.cpp:5:16:
> error: constexpr variable 'e1' must be initialized
> by a constant expression
> constexpr auto e1 = f1;
> ^ ~~
> C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.cpp:5:16: note:
> pointer to a consteval declaration is not a
> constant expression
> C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.cpp:1:15: note:
> declared here
> consteval int f1() { return 0; }
> ^
> 4 errors generated.
>
> These look like valid errors to me, so am I misunderstanding something? Is
> the concern that we're emitting `error: constexpr variable '<whatever>' must
> be initialized by a constant expression` after we already issued a diagnostic
> on the same line?
Yes, i think that
`error: constexpr variable '<whatever>' must be initialized by a constant
expression` + `call to consteval function '<whatever>' is not a constant
expression` or
`error: constexpr variable '<whatever>' must be initialized by a constant
expression` + `cannot take address of consteval function <whatever>' outside of
an immediate invocation`
is emitting 2 errors for the same root cause. the correct solution in my mind
would be to swap too constant context when processing the initializer of a
constexpr (or constinit) variable. this would disable consteval checking.
> From my testing, there's more work to be done, but I think it can be done in
> a follow-up. Consider:
>
> template <typename Ty>
> struct S {
> consteval S() = default;
> Ty Val;
> };
>
> struct foo {
> foo();
> };
>
> S<int> s1; // Correctly rejected
> S<foo> s2; // Incorrectly accepted
>
> With this patch applied, `s1` is correctly diagnosed, but `s2` is not. The
> reason it's not is because `Sema::CheckForImmediateInvocation()` tests
> `!Decl->isConsteval()` to early return, and for the instantiation of
> `S<foo>`, the constructor is marked as "unspecified" rather than `consteval`.
> The reason for that is because we set
> `DefaultedDefaultConstructorIsConstexpr` to false at
> https://github.com/llvm/llvm-project/blob/main/clang/lib/AST/DeclCXX.cpp#L1309
> when instantiating the class. I think we may have some confusion where we
> mean "*IsConstexpr" to mean "is a constexpr function" or "is usable as a
> constexpr function", because my reading of
> https://eel.is/c++draft/dcl.constexpr#7 is that the instantiated template
> constructor IS a constexpr function but it is not usable in a constant
> expression. However, that might be orthogonal to the fixes here and should be
> done separately.
having a function that is explicitly specified consteval not returning true for
isConsteval seems wrong.
IsConstexpr means the function has a either a consteval or constexpr(maybe
implicit) specifier.
so i guess that "is a constexpr function" would be isConstexprSpecified
and "is usable as a constexpr function" would be isConstexpr
BTW I found an other case were we accept invalid code:
struct A { consteval A(); } a;
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D74130/new/
https://reviews.llvm.org/D74130
Files:
clang/include/clang/Parse/Parser.h
clang/include/clang/Sema/ScopeInfo.h
clang/include/clang/Sema/Sema.h
clang/lib/Parse/ParseDecl.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaExpr.cpp
clang/test/SemaCXX/cxx2a-consteval.cpp
Index: clang/test/SemaCXX/cxx2a-consteval.cpp
===================================================================
--- clang/test/SemaCXX/cxx2a-consteval.cpp
+++ clang/test/SemaCXX/cxx2a-consteval.cpp
@@ -258,6 +258,26 @@
return f(0);
};
+consteval int f1() {
+// expected-note@-1+ {{declared here}}
+ return 0;
+}
+consteval auto g() { return f1; }
+consteval int h(int (*p)() = g()) { return p(); }
+int h1(int (*p)() = g()) { return p(); }
+// expected-error@-1 {{is not a constant expression}}
+// expected-note@-2 {{pointer to a consteval}}
+
+// FIXME: These 2 cases generate 2 errors instead of just 1.
+// constexpr auto e = g();
+// constexpr auto e1 = f1;
+
+auto l = [](int (*p)() = g()) { return p(); };
+// expected-error@-1 {{is not a constant expression}}
+// expected-note@-2 {{pointer to a consteval}}
+
+auto l2 = [](int (*p)() = g()) consteval { return p(); };
+
}
namespace std {
@@ -643,3 +663,22 @@
// Show that we reject when not in an immediate context.
int w2 = (a.*&test::f)(); // expected-error {{cannot take address of consteval function 'f' outside of an immediate invocation}}
}
+
+namespace top_level {
+struct S {
+ consteval S() {}
+ int a;
+// expected-note@-1 {{subobject declared here}}
+};
+
+S s; // expected-error {{is not a constant expression}}
+// expected-note@-1 {{is not initialized}}
+
+struct S1 {
+ consteval S1() {}
+ int a = 0;
+};
+
+S1 s1;
+
+}
Index: clang/lib/Sema/SemaExpr.cpp
===================================================================
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -16667,7 +16667,10 @@
ConstantExpr::getStorageKind(Decl->getReturnType().getTypePtr(),
getASTContext()),
/*IsImmediateInvocation*/ true);
- ExprEvalContexts.back().ImmediateInvocationCandidates.emplace_back(Res, 0);
+ if (LambdaScopeInfo *LSI = getCurLambda())
+ LSI->ImmediateInvocationCandidates.emplace_back(Res, 0);
+ else
+ ExprEvalContexts.back().ImmediateInvocationCandidates.emplace_back(Res, 0);
return Res;
}
@@ -16700,16 +16703,19 @@
}
static void RemoveNestedImmediateInvocation(
- Sema &SemaRef, Sema::ExpressionEvaluationContextRecord &Rec,
+ Sema &SemaRef,
+ llvm::SmallVectorImpl<Sema::ImmediateInvocationCandidate>
+ &ImmediateInvocationCandidates,
+ llvm::SmallPtrSetImpl<DeclRefExpr *> &ReferenceToConsteval,
SmallVector<Sema::ImmediateInvocationCandidate, 4>::reverse_iterator It) {
struct ComplexRemove : TreeTransform<ComplexRemove> {
using Base = TreeTransform<ComplexRemove>;
llvm::SmallPtrSetImpl<DeclRefExpr *> &DRSet;
- SmallVector<Sema::ImmediateInvocationCandidate, 4> &IISet;
+ SmallVectorImpl<Sema::ImmediateInvocationCandidate> &IISet;
SmallVector<Sema::ImmediateInvocationCandidate, 4>::reverse_iterator
CurrentII;
ComplexRemove(Sema &SemaRef, llvm::SmallPtrSetImpl<DeclRefExpr *> &DR,
- SmallVector<Sema::ImmediateInvocationCandidate, 4> &II,
+ SmallVectorImpl<Sema::ImmediateInvocationCandidate> &II,
SmallVector<Sema::ImmediateInvocationCandidate,
4>::reverse_iterator Current)
: Base(SemaRef), DRSet(DR), IISet(II), CurrentII(Current) {}
@@ -16759,8 +16765,8 @@
return Res;
}
bool AllowSkippingFirstCXXConstructExpr = true;
- } Transformer(SemaRef, Rec.ReferenceToConsteval,
- Rec.ImmediateInvocationCandidates, It);
+ } Transformer(SemaRef, ReferenceToConsteval, ImmediateInvocationCandidates,
+ It);
/// CXXConstructExpr with a single argument are getting skipped by
/// TreeTransform in some situtation because they could be implicit. This
@@ -16777,33 +16783,35 @@
It->getPointer()->setSubExpr(Res.get());
}
-static void
-HandleImmediateInvocations(Sema &SemaRef,
- Sema::ExpressionEvaluationContextRecord &Rec) {
- if ((Rec.ImmediateInvocationCandidates.size() == 0 &&
- Rec.ReferenceToConsteval.size() == 0) ||
- SemaRef.RebuildingImmediateInvocation)
+void Sema::HandleImmediateInvocations(
+ llvm::SmallVectorImpl<ImmediateInvocationCandidate>
+ &ImmediateInvocationCandidates,
+ llvm::SmallPtrSetImpl<DeclRefExpr *> &ReferenceToConsteval) {
+ if ((ImmediateInvocationCandidates.size() == 0 &&
+ ReferenceToConsteval.size() == 0) ||
+ RebuildingImmediateInvocation || isUnevaluatedContext())
return;
/// When we have more then 1 ImmediateInvocationCandidates we need to check
/// for nested ImmediateInvocationCandidates. when we have only 1 we only
/// need to remove ReferenceToConsteval in the immediate invocation.
- if (Rec.ImmediateInvocationCandidates.size() > 1) {
+ if (ImmediateInvocationCandidates.size() > 1) {
/// Prevent sema calls during the tree transform from adding pointers that
/// are already in the sets.
- llvm::SaveAndRestore<bool> DisableIITracking(
- SemaRef.RebuildingImmediateInvocation, true);
+ llvm::SaveAndRestore<bool> DisableIITracking(RebuildingImmediateInvocation,
+ true);
/// Prevent diagnostic during tree transfrom as they are duplicates
- Sema::TentativeAnalysisScope DisableDiag(SemaRef);
+ Sema::TentativeAnalysisScope DisableDiag(*this);
- for (auto It = Rec.ImmediateInvocationCandidates.rbegin();
- It != Rec.ImmediateInvocationCandidates.rend(); It++)
+ for (auto It = ImmediateInvocationCandidates.rbegin();
+ It != ImmediateInvocationCandidates.rend(); It++)
if (!It->getInt())
- RemoveNestedImmediateInvocation(SemaRef, Rec, It);
- } else if (Rec.ImmediateInvocationCandidates.size() == 1 &&
- Rec.ReferenceToConsteval.size()) {
+ RemoveNestedImmediateInvocation(*this, ImmediateInvocationCandidates,
+ ReferenceToConsteval, It);
+ } else if (ImmediateInvocationCandidates.size() == 1 &&
+ ReferenceToConsteval.size()) {
struct SimpleRemove : RecursiveASTVisitor<SimpleRemove> {
llvm::SmallPtrSetImpl<DeclRefExpr *> &DRSet;
SimpleRemove(llvm::SmallPtrSetImpl<DeclRefExpr *> &S) : DRSet(S) {}
@@ -16811,18 +16819,17 @@
DRSet.erase(E);
return DRSet.size();
}
- } Visitor(Rec.ReferenceToConsteval);
+ } Visitor(ReferenceToConsteval);
Visitor.TraverseStmt(
- Rec.ImmediateInvocationCandidates.front().getPointer()->getSubExpr());
+ ImmediateInvocationCandidates.front().getPointer()->getSubExpr());
}
- for (auto CE : Rec.ImmediateInvocationCandidates)
- if (!CE.getInt())
- EvaluateAndDiagnoseImmediateInvocation(SemaRef, CE);
- for (auto DR : Rec.ReferenceToConsteval) {
+ for (auto CE : ImmediateInvocationCandidates)
+ if (!CE.getInt() && !CE.getPointer()->hasAPValueResult())
+ EvaluateAndDiagnoseImmediateInvocation(*this, CE);
+ for (auto DR : ReferenceToConsteval) {
auto *FD = cast<FunctionDecl>(DR->getDecl());
- SemaRef.Diag(DR->getBeginLoc(), diag::err_invalid_consteval_take_address)
- << FD;
- SemaRef.Diag(FD->getLocation(), diag::note_declared_at);
+ Diag(DR->getBeginLoc(), diag::err_invalid_consteval_take_address) << FD;
+ Diag(FD->getLocation(), diag::note_declared_at);
}
}
@@ -16861,7 +16868,8 @@
}
WarnOnPendingNoDerefs(Rec);
- HandleImmediateInvocations(*this, Rec);
+ HandleImmediateInvocations(Rec.ImmediateInvocationCandidates,
+ Rec.ReferenceToConsteval);
// Warn on any volatile-qualified simple-assignments that are not discarded-
// value expressions nor unevaluated operands (those cases get removed from
@@ -18767,8 +18775,12 @@
if (auto *FD = dyn_cast<FunctionDecl>(E->getDecl()))
if (!isUnevaluatedContext() && !isConstantEvaluated() &&
- FD->isConsteval() && !RebuildingImmediateInvocation)
- ExprEvalContexts.back().ReferenceToConsteval.insert(E);
+ FD->isConsteval() && !RebuildingImmediateInvocation) {
+ if (LambdaScopeInfo *LSI = getCurLambda())
+ LSI->ReferenceToConsteval.insert(E);
+ else
+ ExprEvalContexts.back().ReferenceToConsteval.insert(E);
+ }
MarkExprReferenced(*this, E->getLocation(), E->getDecl(), E, OdrUse,
RefsMinusAssignments);
}
Index: clang/lib/Sema/SemaDecl.cpp
===================================================================
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -14505,6 +14505,16 @@
// meant to pop the context added in ActOnStartOfFunctionDef().
ExitFunctionBodyRAII ExitRAII(*this, isLambdaCallOperator(FD));
+ if (isLambdaCallOperator(FD)) {
+ LambdaScopeInfo* LSI = getCurLambda();
+ /// When processing the default argument of lambdas we dont know wether the
+ /// lambda is consteval or not because the consteval token arrives later.
+ /// so here we do not try to diagnose consteval lambdas.
+ if (!FD->isConsteval())
+ HandleImmediateInvocations(LSI->ImmediateInvocationCandidates,
+ LSI->ReferenceToConsteval);
+ }
+
if (FD) {
FD->setBody(Body);
FD->setWillHaveBody(false);
Index: clang/lib/Parse/ParseDecl.cpp
===================================================================
--- clang/lib/Parse/ParseDecl.cpp
+++ clang/lib/Parse/ParseDecl.cpp
@@ -2452,13 +2452,13 @@
assert(!Exprs.empty() && Exprs.size()-1 == CommaLocs.size() &&
"Unexpected number of commas!");
- InitScope.pop();
-
ExprResult Initializer = Actions.ActOnParenListExpr(T.getOpenLocation(),
T.getCloseLocation(),
Exprs);
Actions.AddInitializerToDecl(ThisDecl, Initializer.get(),
/*DirectInit=*/true);
+
+ InitScope.pop();
}
break;
}
@@ -2471,16 +2471,19 @@
PreferredType.enterVariableInit(Tok.getLocation(), ThisDecl);
ExprResult Init(ParseBraceInitializer());
- InitScope.pop();
-
if (Init.isInvalid()) {
Actions.ActOnInitializerError(ThisDecl);
} else
Actions.AddInitializerToDecl(ThisDecl, Init.get(), /*DirectInit=*/true);
+
+ InitScope.pop();
break;
}
case InitKind::Uninitialized: {
+ InitializerScopeRAII InitScope(*this, D, ThisDecl);
+
Actions.ActOnUninitializedDecl(ThisDecl);
+ InitScope.pop();
break;
}
}
@@ -6614,7 +6617,9 @@
} else {
if (Tok.isNot(tok::r_paren))
ParseParameterDeclarationClause(D.getContext(), FirstArgAttrs, ParamInfo,
- EllipsisLoc);
+ EllipsisLoc,
+ D.getDeclSpec().getConstexprSpecifier() ==
+ ConstexprSpecKind::Consteval);
else if (RequiresArg)
Diag(Tok, diag::err_argument_required_after_attribute);
@@ -6865,10 +6870,9 @@
/// [C++11] attribute-specifier-seq parameter-declaration
///
void Parser::ParseParameterDeclarationClause(
- DeclaratorContext DeclaratorCtx,
- ParsedAttributes &FirstArgAttrs,
- SmallVectorImpl<DeclaratorChunk::ParamInfo> &ParamInfo,
- SourceLocation &EllipsisLoc) {
+ DeclaratorContext DeclaratorCtx, ParsedAttributes &FirstArgAttrs,
+ SmallVectorImpl<DeclaratorChunk::ParamInfo> &ParamInfo,
+ SourceLocation &EllipsisLoc, bool InConstantContext) {
// Avoid exceeding the maximum function scope depth.
// See https://bugs.llvm.org/show_bug.cgi?id=19607
@@ -7019,7 +7023,10 @@
// used.
EnterExpressionEvaluationContext Eval(
Actions,
- Sema::ExpressionEvaluationContext::PotentiallyEvaluatedIfUsed,
+ InConstantContext
+ ? Sema::ExpressionEvaluationContext::ConstantEvaluated
+ : Sema::ExpressionEvaluationContext::
+ PotentiallyEvaluatedIfUsed,
Param);
ExprResult DefArgResult;
Index: clang/include/clang/Sema/Sema.h
===================================================================
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -6109,6 +6109,11 @@
/// invocation.
ExprResult CheckForImmediateInvocation(ExprResult E, FunctionDecl *Decl);
+ void HandleImmediateInvocations(
+ llvm::SmallVectorImpl<ImmediateInvocationCandidate>
+ &ImmediateInvocationCandidates,
+ llvm::SmallPtrSetImpl<DeclRefExpr *> &ReferenceToConsteval);
+
bool CompleteConstructorCall(CXXConstructorDecl *Constructor,
QualType DeclInitType, MultiExprArg ArgsPtr,
SourceLocation Loc,
Index: clang/include/clang/Sema/ScopeInfo.h
===================================================================
--- clang/include/clang/Sema/ScopeInfo.h
+++ clang/include/clang/Sema/ScopeInfo.h
@@ -895,6 +895,14 @@
/// A map of explicit capture indices to their introducer source ranges.
llvm::DenseMap<unsigned, SourceRange> ExplicitCaptureRanges;
+ /// Set of candidates for starting an immediate invocation.
+ llvm::SmallVector<llvm::PointerIntPair<ConstantExpr *, 1>, 2>
+ ImmediateInvocationCandidates;
+
+ /// Set of DeclRefExprs referencing a consteval function when used in a
+ /// context not already known to be immediately invoked.
+ llvm::SmallPtrSet<DeclRefExpr *, 2> ReferenceToConsteval;
+
/// Contains all of the variables defined in this lambda that shadow variables
/// that were defined in parent contexts. Used to avoid warnings when the
/// shadowed variables are uncaptured by this lambda.
Index: clang/include/clang/Parse/Parser.h
===================================================================
--- clang/include/clang/Parse/Parser.h
+++ clang/include/clang/Parse/Parser.h
@@ -3024,10 +3024,9 @@
Declarator &D,
SmallVectorImpl<DeclaratorChunk::ParamInfo> &ParamInfo);
void ParseParameterDeclarationClause(
- DeclaratorContext DeclaratorContext,
- ParsedAttributes &attrs,
- SmallVectorImpl<DeclaratorChunk::ParamInfo> &ParamInfo,
- SourceLocation &EllipsisLoc);
+ DeclaratorContext DeclaratorContext, ParsedAttributes &attrs,
+ SmallVectorImpl<DeclaratorChunk::ParamInfo> &ParamInfo,
+ SourceLocation &EllipsisLoc, bool InConstantContext = false);
void ParseBracketDeclarator(Declarator &D);
void ParseMisplacedBracketDeclarator(Declarator &D);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
