Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Clang] Fix P2564 handling of variable initializers #89565

Merged
merged 11 commits into from
May 9, 2024
4 changes: 4 additions & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -699,6 +699,10 @@ Bug Fixes to C++ Support
performed incorrectly when checking constraints. Fixes (#GH90349).
- Clang now allows constrained member functions to be explicitly specialized for an implicit instantiation
of a class template.
- Fix a C++23 bug in implementation of P2564R3 which evaluates immediate invocations in place
within initializers for variables that are usable in constant expressions or are constant
initialized, rather than evaluating them as a part of the larger manifestly constant evaluated
expression.

Bug Fixes to AST Handling
^^^^^^^^^^^^^^^^^^^^^^^^^
Expand Down
4 changes: 3 additions & 1 deletion clang/include/clang/Sema/Sema.h
Original file line number Diff line number Diff line change
Expand Up @@ -10202,7 +10202,9 @@ class Sema final : public SemaBase {
S.ExprEvalContexts.back().InImmediateFunctionContext =
FD->isImmediateFunction() ||
S.ExprEvalContexts[S.ExprEvalContexts.size() - 2]
.isConstantEvaluated();
.isConstantEvaluated() ||
S.ExprEvalContexts[S.ExprEvalContexts.size() - 2]
.isImmediateFunctionContext();
S.ExprEvalContexts.back().InImmediateEscalatingFunctionContext =
S.getLangOpts().CPlusPlus20 && FD->isImmediateEscalating();
} else
Expand Down
21 changes: 10 additions & 11 deletions clang/lib/Parse/ParseDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2587,25 +2587,30 @@ Decl *Parser::ParseDeclarationAfterDeclaratorAndAttributes(
Parser &P;
Declarator &D;
Decl *ThisDecl;
bool Entered;

InitializerScopeRAII(Parser &P, Declarator &D, Decl *ThisDecl)
: P(P), D(D), ThisDecl(ThisDecl) {
: P(P), D(D), ThisDecl(ThisDecl), Entered(false) {
if (ThisDecl && P.getLangOpts().CPlusPlus) {
Scope *S = nullptr;
if (D.getCXXScopeSpec().isSet()) {
P.EnterScope(0);
S = P.getCurScope();
}
P.Actions.ActOnCXXEnterDeclInitializer(S, ThisDecl);
if (ThisDecl && !ThisDecl->isInvalidDecl()) {
P.Actions.ActOnCXXEnterDeclInitializer(S, ThisDecl);
Entered = true;
}
}
}
~InitializerScopeRAII() { pop(); }
void pop() {
~InitializerScopeRAII() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a nice improvement

if (ThisDecl && P.getLangOpts().CPlusPlus) {
Scope *S = nullptr;
if (D.getCXXScopeSpec().isSet())
S = P.getCurScope();
P.Actions.ActOnCXXExitDeclInitializer(S, ThisDecl);

if (Entered)
P.Actions.ActOnCXXExitDeclInitializer(S, ThisDecl);
if (S)
P.ExitScope();
}
Expand Down Expand Up @@ -2736,8 +2741,6 @@ Decl *Parser::ParseDeclarationAfterDeclaratorAndAttributes(
FRI->RangeExpr = Init;
}

InitScope.pop();

if (Init.isInvalid()) {
SmallVector<tok::TokenKind, 2> StopTokens;
StopTokens.push_back(tok::comma);
Expand Down Expand Up @@ -2785,8 +2788,6 @@ Decl *Parser::ParseDeclarationAfterDeclaratorAndAttributes(

bool SawError = ParseExpressionList(Exprs, ExpressionStarts);

InitScope.pop();

if (SawError) {
if (ThisVarDecl && PP.isCodeCompletionReached() && !CalledSignatureHelp) {
Actions.ProduceConstructorSignatureHelp(
Expand Down Expand Up @@ -2818,8 +2819,6 @@ Decl *Parser::ParseDeclarationAfterDeclaratorAndAttributes(
PreferredType.enterVariableInit(Tok.getLocation(), ThisDecl);
ExprResult Init(ParseBraceInitializer());

InitScope.pop();

if (Init.isInvalid()) {
Actions.ActOnInitializerError(ThisDecl);
} else
Expand Down
9 changes: 4 additions & 5 deletions clang/lib/Sema/SemaChecking.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16574,11 +16574,10 @@ static void CheckImplicitConversion(Sema &S, Expr *E, QualType T,
std::string PrettySourceValue = toString(Value, 10);
std::string PrettyTargetValue = PrettyPrintInRange(Value, TargetRange);

S.DiagRuntimeBehavior(
E->getExprLoc(), E,
S.PDiag(diag::warn_impcast_integer_precision_constant)
<< PrettySourceValue << PrettyTargetValue << E->getType() << T
<< E->getSourceRange() << SourceRange(CC));
katzdm marked this conversation as resolved.
Show resolved Hide resolved
S.Diag(E->getExprLoc(),
S.PDiag(diag::warn_impcast_integer_precision_constant)
<< PrettySourceValue << PrettyTargetValue << E->getType()
<< T << E->getSourceRange() << SourceRange(CC));
return;
}
}
Expand Down
53 changes: 29 additions & 24 deletions clang/lib/Sema/SemaDeclCXX.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18553,15 +18553,6 @@ void Sema::ActOnPureSpecifier(Decl *D, SourceLocation ZeroLoc) {
Diag(D->getLocation(), diag::err_illegal_initializer);
}

/// Determine whether the given declaration is a global variable or
/// static data member.
static bool isNonlocalVariable(const Decl *D) {
if (const VarDecl *Var = dyn_cast_or_null<VarDecl>(D))
return Var->hasGlobalStorage();

return false;
}

/// Invoked when we are about to parse an initializer for the declaration
/// 'Dcl'.
///
Expand All @@ -18570,9 +18561,7 @@ static bool isNonlocalVariable(const Decl *D) {
/// class X. If the declaration had a scope specifier, a scope will have
/// been created and passed in for this purpose. Otherwise, S will be null.
void Sema::ActOnCXXEnterDeclInitializer(Scope *S, Decl *D) {
// If there is no declaration, there was an error parsing it.
if (!D || D->isInvalidDecl())
return;
cor3ntin marked this conversation as resolved.
Show resolved Hide resolved
assert(D && !D->isInvalidDecl());

// We will always have a nested name specifier here, but this declaration
// might not be out of line if the specifier names the current namespace:
Expand All @@ -18581,25 +18570,41 @@ void Sema::ActOnCXXEnterDeclInitializer(Scope *S, Decl *D) {
if (S && D->isOutOfLine())
EnterDeclaratorContext(S, D->getDeclContext());

// If we are parsing the initializer for a static data member, push a
// new expression evaluation context that is associated with this static
// data member.
if (isNonlocalVariable(D))
PushExpressionEvaluationContext(
ExpressionEvaluationContext::PotentiallyEvaluated, D);
PushExpressionEvaluationContext(
ExpressionEvaluationContext::PotentiallyEvaluated, D);
}

/// Invoked after we are finished parsing an initializer for the declaration D.
void Sema::ActOnCXXExitDeclInitializer(Scope *S, Decl *D) {
// If there is no declaration, there was an error parsing it.
if (!D || D->isInvalidDecl())
return;
katzdm marked this conversation as resolved.
Show resolved Hide resolved

if (isNonlocalVariable(D))
PopExpressionEvaluationContext();
assert(D);

if (S && D->isOutOfLine())
ExitDeclaratorContext(S);

if (getLangOpts().CPlusPlus23) {
// An expression or conversion is 'manifestly constant-evaluated' if it is:
// [...]
// - the initializer of a variable that is usable in constant expressions or
// has constant initialization.
if (auto *VD = dyn_cast<VarDecl>(D);
VD && (VD->isUsableInConstantExpressions(Context) ||
VD->hasConstantInitialization())) {
// An expression or conversion is in an 'immediate function context' if it
// is potentially evaluated and either:
// [...]
// - it is a subexpression of a manifestly constant-evaluated expression
// or conversion.
ExprEvalContexts.back().InImmediateFunctionContext = true;
}
}

// Unless the initializer is in an immediate function context (as determined
// above), this will evaluate all contained immediate function calls as
// constant expressions. If the initializer IS an immediate function context,
// the initializer has been determined to be a constant expression, and all
// such evaluations will be elided (i.e., as if we "knew the whole time" that
// it was a constant expression).
PopExpressionEvaluationContext();
}

/// ActOnCXXConditionDeclarationExpr - Parsed a condition declaration of a
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/Sema/SemaExpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18039,7 +18039,7 @@ HandleImmediateInvocations(Sema &SemaRef,
Sema::ExpressionEvaluationContextRecord &Rec) {
if ((Rec.ImmediateInvocationCandidates.size() == 0 &&
Rec.ReferenceToConsteval.size() == 0) ||
SemaRef.RebuildingImmediateInvocation)
Rec.isImmediateFunctionContext() || SemaRef.RebuildingImmediateInvocation)
return;

/// When we have more than 1 ImmediateInvocationCandidates or previously
Expand Down
16 changes: 12 additions & 4 deletions clang/test/SemaCXX/cxx2a-consteval.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1068,6 +1068,14 @@ void test() {
constexpr int (*f2)(void) = lstatic; // expected-error {{constexpr variable 'f2' must be initialized by a constant expression}} \
// expected-note {{pointer to a consteval declaration is not a constant expression}}

int (*f3)(void) = []() consteval { return 3; }; // expected-error {{cannot take address of consteval call operator of '(lambda at}} \
// expected-note {{declared here}}
}

consteval void consteval_test() {
constexpr auto l1 = []() consteval { return 3; };

int (*f1)(void) = l1; // ok
}
}

Expand Down Expand Up @@ -1098,11 +1106,11 @@ int bad = 10; // expected-note 6{{declared here}}
tester glob1(make_name("glob1"));
tester glob2(make_name("glob2"));
constexpr tester cglob(make_name("cglob"));
tester paddedglob(make_name(pad(bad))); // expected-error {{call to consteval function 'GH58207::make_name' is not a constant expression}} \
tester paddedglob(make_name(pad(bad))); // expected-error {{call to consteval function 'GH58207::tester::tester' is not a constant expression}} \
// expected-note {{read of non-const variable 'bad' is not allowed in a constant expression}}

constexpr tester glob3 = { make_name("glob3") };
constexpr tester glob4 = { make_name(pad(bad)) }; // expected-error {{call to consteval function 'GH58207::make_name' is not a constant expression}} \
constexpr tester glob4 = { make_name(pad(bad)) }; // expected-error {{call to consteval function 'GH58207::tester::tester' is not a constant expression}} \
// expected-error {{constexpr variable 'glob4' must be initialized by a constant expression}} \
// expected-note 2{{read of non-const variable 'bad' is not allowed in a constant expression}}

Expand All @@ -1114,12 +1122,12 @@ auto V1 = make_name(pad(bad)); // expected-error {{call to consteval function 'G
void foo() {
static tester loc1(make_name("loc1"));
static constexpr tester loc2(make_name("loc2"));
static tester paddedloc(make_name(pad(bad))); // expected-error {{call to consteval function 'GH58207::make_name' is not a constant expression}} \
static tester paddedloc(make_name(pad(bad))); // expected-error {{call to consteval function 'GH58207::tester::tester' is not a constant expression}} \
// expected-note {{read of non-const variable 'bad' is not allowed in a constant expression}}
}

void bar() {
static tester paddedloc(make_name(pad(bad))); // expected-error {{call to consteval function 'GH58207::make_name' is not a constant expression}} \
static tester paddedloc(make_name(pad(bad))); // expected-error {{call to consteval function 'GH58207::tester::tester' is not a constant expression}} \
// expected-note {{read of non-const variable 'bad' is not allowed in a constant expression}}
}
}
Expand Down
26 changes: 26 additions & 0 deletions clang/test/SemaCXX/cxx2b-consteval-propagate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -394,3 +394,29 @@ static_assert(none_of(
));

}

#if __cplusplus >= 202302L
namespace lvalue_to_rvalue_init_from_heap {

struct S {
int *value;
constexpr S(int v) : value(new int {v}) {} // expected-note 2 {{heap allocation performed here}}
constexpr ~S() { delete value; }
};
consteval S fn() { return S(5); }
int fn2() { return 2; } // expected-note {{declared here}}

constexpr int a = *fn().value;
constinit int b = *fn().value;
const int c = *fn().value;
int d = *fn().value;

constexpr int e = *fn().value + fn2(); // expected-error {{must be initialized by a constant expression}} \
// expected-error {{call to consteval function 'lvalue_to_rvalue_init_from_heap::fn' is not a constant expression}} \
// expected-note {{non-constexpr function 'fn2'}} \
// expected-note {{pointer to heap-allocated object}}

int f = *fn().value + fn2(); // expected-error {{call to consteval function 'lvalue_to_rvalue_init_from_heap::fn' is not a constant expression}} \
// expected-note {{pointer to heap-allocated object}}
}
#endif
1 change: 1 addition & 0 deletions clang/test/SemaCXX/enum-scoped.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ enum class E4 {
e1 = -2147483648, // ok
e2 = 2147483647, // ok
e3 = 2147483648 // expected-error{{enumerator value evaluates to 2147483648, which cannot be narrowed to type 'int'}}
// expected-warning@-1{{changes value}}
};

enum class E5 {
Expand Down
Loading