Skip to content

[Clang] Functions called in discarded statements should not be instantiated #140576

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -745,6 +745,7 @@ Bug Fixes to C++ Support
- Fix an incorrect deduction when calling an explicit object member function template through an overload set address.
- Fixed bug in constant evaluation that would allow using the value of a
reference in its own initializer in C++23 mode (#GH131330).
- Clang could incorrectly instantiate functions in discarded contexts (#GH140449)

Bug Fixes to AST Handling
^^^^^^^^^^^^^^^^^^^^^^^^^
Expand Down
16 changes: 16 additions & 0 deletions clang/include/clang/Sema/EnterExpressionEvaluationContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,22 @@ class EnterExpressionEvaluationContext {
}
};

/// RAII object that enters a new function expression evaluation context.
class EnterExpressionEvaluationContextForFunction {
Sema &Actions;

public:
EnterExpressionEvaluationContextForFunction(
Sema &Actions, Sema::ExpressionEvaluationContext NewContext,
FunctionDecl *FD = nullptr)
: Actions(Actions) {
Actions.PushExpressionEvaluationContextForFunction(NewContext, FD);
}
~EnterExpressionEvaluationContextForFunction() {
Actions.PopExpressionEvaluationContext();
}
};

} // namespace clang

#endif
29 changes: 11 additions & 18 deletions clang/include/clang/Sema/Sema.h
Original file line number Diff line number Diff line change
Expand Up @@ -6825,8 +6825,9 @@ class Sema final : public SemaBase {

bool isDiscardedStatementContext() const {
return Context == ExpressionEvaluationContext::DiscardedStatement ||
(Context ==
ExpressionEvaluationContext::ImmediateFunctionContext &&
((Context ==
ExpressionEvaluationContext::ImmediateFunctionContext ||
isPotentiallyEvaluated()) &&
InDiscardedStatement);
}
};
Expand Down Expand Up @@ -6915,6 +6916,10 @@ class Sema final : public SemaBase {
ExpressionEvaluationContext NewContext, Decl *LambdaContextDecl = nullptr,
ExpressionEvaluationContextRecord::ExpressionKind Type =
ExpressionEvaluationContextRecord::EK_Other);

void PushExpressionEvaluationContextForFunction(
ExpressionEvaluationContext NewContext, FunctionDecl *FD);

enum ReuseLambdaContextDecl_t { ReuseLambdaContextDecl };
void PushExpressionEvaluationContext(
ExpressionEvaluationContext NewContext, ReuseLambdaContextDecl_t,
Expand Down Expand Up @@ -13371,23 +13376,11 @@ class Sema final : public SemaBase {
: S(S), SavedContext(S, DC) {
auto *FD = dyn_cast<FunctionDecl>(DC);
S.PushFunctionScope();
S.PushExpressionEvaluationContext(
(FD && FD->isImmediateFunction())
? ExpressionEvaluationContext::ImmediateFunctionContext
: ExpressionEvaluationContext::PotentiallyEvaluated);
if (FD) {
auto &Current = S.currentEvaluationContext();
const auto &Parent = S.parentEvaluationContext();

S.PushExpressionEvaluationContextForFunction(
ExpressionEvaluationContext::PotentiallyEvaluated, FD);
if (FD)
FD->setWillHaveBody(true);
Current.InImmediateFunctionContext =
FD->isImmediateFunction() ||
(isLambdaMethod(FD) && (Parent.isConstantEvaluated() ||
Parent.isImmediateFunctionContext()));

Current.InImmediateEscalatingFunctionContext =
S.getLangOpts().CPlusPlus20 && FD->isImmediateEscalating();
} else
else
assert(isa<ObjCMethodDecl>(DC));
}

Expand Down
6 changes: 4 additions & 2 deletions clang/lib/Sema/SemaCoroutine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -697,8 +697,10 @@ static void checkReturnStmtInCoroutine(Sema &S, FunctionScopeInfo *FSI) {
bool Sema::ActOnCoroutineBodyStart(Scope *SC, SourceLocation KWLoc,
StringRef Keyword) {
// Ignore previous expr evaluation contexts.
EnterExpressionEvaluationContext PotentiallyEvaluated(
*this, Sema::ExpressionEvaluationContext::PotentiallyEvaluated);
EnterExpressionEvaluationContextForFunction PotentiallyEvaluated(
*this, Sema::ExpressionEvaluationContext::PotentiallyEvaluated,
dyn_cast_or_null<FunctionDecl>(CurContext));

if (!checkCoroutineContext(*this, KWLoc, Keyword))
return false;
auto *ScopeInfo = getCurFunction();
Expand Down
20 changes: 2 additions & 18 deletions clang/lib/Sema/SemaDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15870,24 +15870,8 @@ Decl *Sema::ActOnStartOfFunctionDef(Scope *FnBodyScope, Decl *D,
// Do not push if it is a lambda because one is already pushed when building
// the lambda in ActOnStartOfLambdaDefinition().
if (!isLambdaCallOperator(FD))
// [expr.const]/p14.1
// An expression or conversion is in an immediate function context if it is
// potentially evaluated and either: its innermost enclosing non-block scope
// is a function parameter scope of an immediate function.
PushExpressionEvaluationContext(
FD->isConsteval() ? ExpressionEvaluationContext::ImmediateFunctionContext
: ExprEvalContexts.back().Context);

// Each ExpressionEvaluationContextRecord also keeps track of whether the
// context is nested in an immediate function context, so smaller contexts
// that appear inside immediate functions (like variable initializers) are
// considered to be inside an immediate function context even though by
// themselves they are not immediate function contexts. But when a new
// function is entered, we need to reset this tracking, since the entered
// function might be not an immediate function.
ExprEvalContexts.back().InImmediateFunctionContext = FD->isConsteval();
ExprEvalContexts.back().InImmediateEscalatingFunctionContext =
getLangOpts().CPlusPlus20 && FD->isImmediateEscalating();
PushExpressionEvaluationContextForFunction(ExprEvalContexts.back().Context,
FD);

// Check for defining attributes before the check for redefinition.
if (const auto *Attr = FD->getAttr<AliasAttr>()) {
Expand Down
85 changes: 58 additions & 27 deletions clang/lib/Sema/SemaExpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17748,6 +17748,47 @@ Sema::PushExpressionEvaluationContext(
PushExpressionEvaluationContext(NewContext, ClosureContextDecl, ExprContext);
}

void Sema::PushExpressionEvaluationContextForFunction(
ExpressionEvaluationContext NewContext, FunctionDecl *FD) {
// [expr.const]/p14.1
// An expression or conversion is in an immediate function context if it is
// potentially evaluated and either: its innermost enclosing non-block scope
// is a function parameter scope of an immediate function.
PushExpressionEvaluationContext(
FD && FD->isConsteval()
? ExpressionEvaluationContext::ImmediateFunctionContext
: NewContext);
const Sema::ExpressionEvaluationContextRecord &Parent =
parentEvaluationContext();
Sema::ExpressionEvaluationContextRecord &Current = currentEvaluationContext();

Current.InDiscardedStatement = false;

if (FD) {

// Each ExpressionEvaluationContextRecord also keeps track of whether the
// context is nested in an immediate function context, so smaller contexts
// that appear inside immediate functions (like variable initializers) are
// considered to be inside an immediate function context even though by
// themselves they are not immediate function contexts. But when a new
// function is entered, we need to reset this tracking, since the entered
// function might be not an immediate function.

Current.InImmediateEscalatingFunctionContext =
getLangOpts().CPlusPlus20 && FD->isImmediateEscalating();

if (isLambdaMethod(FD)) {
Current.InDiscardedStatement = Parent.isDiscardedStatementContext();
Current.InImmediateFunctionContext =
FD->isConsteval() ||
(isLambdaMethod(FD) && (Parent.isConstantEvaluated() ||
Parent.isImmediateFunctionContext()));
} else {
Current.InImmediateFunctionContext = FD->isConsteval();
}
}
}

namespace {

const DeclRefExpr *CheckPossibleDeref(Sema &S, const Expr *PossibleDeref) {
Expand Down Expand Up @@ -18360,35 +18401,23 @@ enum class OdrUseContext {
/// Are we within a context in which references to resolved functions or to
/// variables result in odr-use?
static OdrUseContext isOdrUseContext(Sema &SemaRef) {
OdrUseContext Result;

switch (SemaRef.ExprEvalContexts.back().Context) {
case Sema::ExpressionEvaluationContext::Unevaluated:
case Sema::ExpressionEvaluationContext::UnevaluatedList:
case Sema::ExpressionEvaluationContext::UnevaluatedAbstract:
return OdrUseContext::None;

case Sema::ExpressionEvaluationContext::ConstantEvaluated:
case Sema::ExpressionEvaluationContext::ImmediateFunctionContext:
case Sema::ExpressionEvaluationContext::PotentiallyEvaluated:
Result = OdrUseContext::Used;
break;
const Sema::ExpressionEvaluationContextRecord &Context =
SemaRef.currentEvaluationContext();

case Sema::ExpressionEvaluationContext::DiscardedStatement:
Result = OdrUseContext::FormallyOdrUsed;
break;

case Sema::ExpressionEvaluationContext::PotentiallyEvaluatedIfUsed:
// A default argument formally results in odr-use, but doesn't actually
// result in a use in any real sense until it itself is used.
Result = OdrUseContext::FormallyOdrUsed;
break;
}
if (Context.isUnevaluated())
return OdrUseContext::None;

if (SemaRef.CurContext->isDependentContext())
return OdrUseContext::Dependent;

return Result;
if (Context.isDiscardedStatementContext())
return OdrUseContext::FormallyOdrUsed;

else if (Context.Context ==
Sema::ExpressionEvaluationContext::PotentiallyEvaluatedIfUsed)
return OdrUseContext::FormallyOdrUsed;

return OdrUseContext::Used;
}

static bool isImplicitlyDefinableConstexprFunction(FunctionDecl *Func) {
Expand Down Expand Up @@ -20355,9 +20384,11 @@ MarkExprReferenced(Sema &SemaRef, SourceLocation Loc, Decl *D, Expr *E,
}

void Sema::MarkDeclRefReferenced(DeclRefExpr *E, const Expr *Base) {
// TODO: update this with DR# once a defect report is filed.
// C++11 defect. The address of a pure member should not be an ODR use, even
// if it's a qualified reference.
// [basic.def.odr] (CWG 1614)
// A function is named by an expression or conversion [...]
// unless it is a pure virtual function and either the expression is not an
// id-expression naming the function with an explicitly qualified name or
// the expression forms a pointer to member
bool OdrUse = true;
if (const CXXMethodDecl *Method = dyn_cast<CXXMethodDecl>(E->getDecl()))
if (Method->isVirtual() &&
Expand Down
10 changes: 2 additions & 8 deletions clang/lib/Sema/SemaLambda.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1574,14 +1574,8 @@ void Sema::ActOnStartOfLambdaDefinition(LambdaIntroducer &Intro,

// Enter a new evaluation context to insulate the lambda from any
// cleanups from the enclosing full-expression.
PushExpressionEvaluationContext(
LSI->CallOperator->isConsteval()
? ExpressionEvaluationContext::ImmediateFunctionContext
: ExpressionEvaluationContext::PotentiallyEvaluated);
ExprEvalContexts.back().InImmediateFunctionContext =
LSI->CallOperator->isConsteval();
ExprEvalContexts.back().InImmediateEscalatingFunctionContext =
getLangOpts().CPlusPlus20 && LSI->CallOperator->isImmediateEscalating();
PushExpressionEvaluationContextForFunction(
ExpressionEvaluationContext::PotentiallyEvaluated, LSI->CallOperator);
}

void Sema::ActOnLambdaError(SourceLocation StartLoc, Scope *CurScope,
Expand Down
33 changes: 33 additions & 0 deletions clang/test/CXX/stmt.stmt/stmt.select/stmt.if/p2.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -177,4 +177,37 @@ void f() {
}
} // namespace deduced_return_type_in_discareded_statement

namespace GH140449 {

template <typename T>
int f() {
T *ptr;
return 0;
}

template <typename T>
constexpr int g() {
T *ptr; // expected-error{{'ptr' declared as a pointer to a reference of type 'int &'}}
return 0;
}

template <typename T>
auto h() {
T *ptr; // expected-error{{'ptr' declared as a pointer to a reference of type 'int &'}}
return 0;
}

void test() {
if constexpr (false) {
int x = f<int &>();
constexpr int y = g<int &>();
// expected-error@-1 {{constexpr variable 'y' must be initialized by a constant expression}} \
// expected-note@-1{{in instantiation of function template specialization}}
int z = h<int &>();
// expected-note@-1{{in instantiation of function template specialization}}

}
}
}

#endif
Loading