Skip to content

[clang-repl] Names declared in if conditions and for-init statements are local to the inner context (C++ 3.3.2p4) #84150

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
16 changes: 10 additions & 6 deletions clang/include/clang/AST/Decl.h
Original file line number Diff line number Diff line change
Expand Up @@ -4419,15 +4419,15 @@ class FileScopeAsmDecl : public Decl {
///
/// \note This is used in libInterpreter, clang -cc1 -fincremental-extensions
/// and in tools such as clang-repl.
class TopLevelStmtDecl : public Decl {
class TopLevelStmtDecl : public Decl, public DeclContext {
friend class ASTDeclReader;
friend class ASTDeclWriter;

Stmt *Statement = nullptr;
bool IsSemiMissing = false;

TopLevelStmtDecl(DeclContext *DC, SourceLocation L, Stmt *S)
: Decl(TopLevelStmt, DC, L), Statement(S) {}
: Decl(TopLevelStmt, DC, L), DeclContext(TopLevelStmt), Statement(S) {}

virtual void anchor();

Expand All @@ -4438,15 +4438,19 @@ class TopLevelStmtDecl : public Decl {
SourceRange getSourceRange() const override LLVM_READONLY;
Stmt *getStmt() { return Statement; }
const Stmt *getStmt() const { return Statement; }
void setStmt(Stmt *S) {
assert(IsSemiMissing && "Operation supported for printing values only!");
Statement = S;
}
void setStmt(Stmt *S);
bool isSemiMissing() const { return IsSemiMissing; }
void setSemiMissing(bool Missing = true) { IsSemiMissing = Missing; }

static bool classof(const Decl *D) { return classofKind(D->getKind()); }
static bool classofKind(Kind K) { return K == TopLevelStmt; }

static DeclContext *castToDeclContext(const TopLevelStmtDecl *D) {
return static_cast<DeclContext *>(const_cast<TopLevelStmtDecl *>(D));
}
static TopLevelStmtDecl *castFromDeclContext(const DeclContext *DC) {
return static_cast<TopLevelStmtDecl *>(const_cast<DeclContext *>(DC));
}
};

/// Represents a block literal declaration, which is like an
Expand Down
1 change: 1 addition & 0 deletions clang/include/clang/AST/DeclBase.h
Original file line number Diff line number Diff line change
Expand Up @@ -2120,6 +2120,7 @@ class DeclContext {
case Decl::Block:
case Decl::Captured:
case Decl::ObjCMethod:
case Decl::TopLevelStmt:
return true;
default:
return getDeclKind() >= Decl::firstFunction &&
Expand Down
2 changes: 1 addition & 1 deletion clang/include/clang/Basic/DeclNodes.td
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ def LinkageSpec : DeclNode<Decl>, DeclContext;
def Export : DeclNode<Decl>, DeclContext;
def ObjCPropertyImpl : DeclNode<Decl>;
def FileScopeAsm : DeclNode<Decl>;
def TopLevelStmt : DeclNode<Decl>;
def TopLevelStmt : DeclNode<Decl>, DeclContext;
def AccessSpec : DeclNode<Decl>;
def Friend : DeclNode<Decl>;
def FriendTemplate : DeclNode<Decl>;
Expand Down
3 changes: 2 additions & 1 deletion clang/include/clang/Sema/Sema.h
Original file line number Diff line number Diff line change
Expand Up @@ -3263,7 +3263,8 @@ class Sema final {
Decl *ActOnFileScopeAsmDecl(Expr *expr, SourceLocation AsmLoc,
SourceLocation RParenLoc);

Decl *ActOnTopLevelStmtDecl(Stmt *Statement);
TopLevelStmtDecl *ActOnStartTopLevelStmtDecl(Scope *S);
void ActOnFinishTopLevelStmtDecl(TopLevelStmtDecl *D, Stmt *Statement);

void ActOnPopScope(SourceLocation Loc, Scope *S);

Expand Down
11 changes: 8 additions & 3 deletions clang/lib/AST/Decl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5552,14 +5552,13 @@ FileScopeAsmDecl *FileScopeAsmDecl::CreateDeserialized(ASTContext &C,
void TopLevelStmtDecl::anchor() {}

TopLevelStmtDecl *TopLevelStmtDecl::Create(ASTContext &C, Stmt *Statement) {
assert(Statement);
assert(C.getLangOpts().IncrementalExtensions &&
"Must be used only in incremental mode");

SourceLocation BeginLoc = Statement->getBeginLoc();
SourceLocation Loc = Statement ? Statement->getBeginLoc() : SourceLocation();
DeclContext *DC = C.getTranslationUnitDecl();

return new (C, DC) TopLevelStmtDecl(DC, BeginLoc, Statement);
return new (C, DC) TopLevelStmtDecl(DC, Loc, Statement);
}

TopLevelStmtDecl *TopLevelStmtDecl::CreateDeserialized(ASTContext &C,
Expand All @@ -5572,6 +5571,12 @@ SourceRange TopLevelStmtDecl::getSourceRange() const {
return SourceRange(getLocation(), Statement->getEndLoc());
}

void TopLevelStmtDecl::setStmt(Stmt *S) {
assert(S);
Statement = S;
setLocation(Statement->getBeginLoc());
}

void EmptyDecl::anchor() {}

EmptyDecl *EmptyDecl::Create(ASTContext &C, DeclContext *DC, SourceLocation L) {
Expand Down
1 change: 1 addition & 0 deletions clang/lib/AST/DeclBase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1352,6 +1352,7 @@ DeclContext *DeclContext::getPrimaryContext() {
case Decl::ExternCContext:
case Decl::LinkageSpec:
case Decl::Export:
case Decl::TopLevelStmt:
case Decl::Block:
case Decl::Captured:
case Decl::OMPDeclareReduction:
Expand Down
24 changes: 16 additions & 8 deletions clang/lib/Parse/ParseDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5678,24 +5678,32 @@ Parser::DeclGroupPtrTy Parser::ParseTopLevelStmtDecl() {
// Parse a top-level-stmt.
Parser::StmtVector Stmts;
ParsedStmtContext SubStmtCtx = ParsedStmtContext();
Actions.PushFunctionScope();
ParseScope FnScope(this, Scope::FnScope | Scope::DeclScope |
Copy link
Contributor

Choose a reason for hiding this comment

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

I was wondering if we could survive without this scope here or at least if we could drop the Scope::CompoundStmtScope part of it.

Copy link
Member Author

Choose a reason for hiding this comment

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

I tested without it the ParseScope and got a segfault, so I assume we can not drop it (without looking into the details). I kept both, FnScope and CompoundStmtScope, because in ActOnStartTopLevelStmtDecl() we push them on the Sema side as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

It's a bit awkward to push scoping state in two places. I am wondering if moving the FnScope down in ActOnStartTopLevelStmtDecl and ActOnFinishTopLevelStmtDecl would be a good idea. It would make the implementation look cleaner but if an error occurs the state of the scopes will be inconsistent (the R.isUsable branch). Maybe it is fine as it is...

Scope::CompoundStmtScope);
TopLevelStmtDecl *TLSD = Actions.ActOnStartTopLevelStmtDecl(getCurScope());
StmtResult R = ParseStatementOrDeclaration(Stmts, SubStmtCtx);
Actions.PopFunctionScopeInfo();
if (!R.isUsable())
return nullptr;

SmallVector<Decl *, 2> DeclsInGroup;
DeclsInGroup.push_back(Actions.ActOnTopLevelStmtDecl(R.get()));
Actions.ActOnFinishTopLevelStmtDecl(TLSD, R.get());

if (Tok.is(tok::annot_repl_input_end) &&
Tok.getAnnotationValue() != nullptr) {
ConsumeAnnotationToken();
cast<TopLevelStmtDecl>(DeclsInGroup.back())->setSemiMissing();
TLSD->setSemiMissing();
}

// Currently happens for things like -fms-extensions and use `__if_exists`.
for (Stmt *S : Stmts)
DeclsInGroup.push_back(Actions.ActOnTopLevelStmtDecl(S));
SmallVector<Decl *, 2> DeclsInGroup;
DeclsInGroup.push_back(TLSD);

// Currently happens for things like -fms-extensions and use `__if_exists`.
for (Stmt *S : Stmts) {
// Here we should be safe as `__if_exists` and friends are not introducing
// new variables which need to live outside file scope.
TopLevelStmtDecl *D = Actions.ActOnStartTopLevelStmtDecl(getCurScope());
Actions.ActOnFinishTopLevelStmtDecl(D, S);
DeclsInGroup.push_back(D);
}

return Actions.BuildDeclaratorGroup(DeclsInGroup);
}
Expand Down
16 changes: 13 additions & 3 deletions clang/lib/Sema/SemaDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20519,12 +20519,22 @@ Decl *Sema::ActOnFileScopeAsmDecl(Expr *expr,
return New;
}

Decl *Sema::ActOnTopLevelStmtDecl(Stmt *Statement) {
auto *New = TopLevelStmtDecl::Create(Context, Statement);
Context.getTranslationUnitDecl()->addDecl(New);
TopLevelStmtDecl *Sema::ActOnStartTopLevelStmtDecl(Scope *S) {
auto *New = TopLevelStmtDecl::Create(Context, /*Statement=*/nullptr);
CurContext->addDecl(New);
PushDeclContext(S, New);
PushFunctionScope();
PushCompoundScope(false);
return New;
}

void Sema::ActOnFinishTopLevelStmtDecl(TopLevelStmtDecl *D, Stmt *Statement) {
D->setStmt(Statement);
PopCompoundScope();
PopFunctionScopeInfo();
PopDeclContext();
}

void Sema::ActOnPragmaRedefineExtname(IdentifierInfo* Name,
IdentifierInfo* AliasName,
SourceLocation PragmaLoc,
Expand Down
14 changes: 13 additions & 1 deletion clang/test/Interpreter/execute-stmts.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
//CODEGEN-CHECK-COUNT-2: define internal void @__stmts__
//CODEGEN-CHECK-NOT: define internal void @__stmts__


extern "C" int printf(const char*,...);

template <typename T> T call() { printf("called\n"); return T(); }
Expand Down Expand Up @@ -41,3 +40,16 @@ for (; i > 4; --i) { printf("i = %d\n", i); };

int j = i; printf("j = %d\n", j);
// CHECK-NEXT: j = 4

if (int i = j) printf("i = %d\n", i);
// CHECK-NEXT: i = 4

for (int i = j; i > 3; --i) printf("i = %d\n", i);
// CHECK-NEXT: i = 4

for(int i=0; i<2; i+=1) {};

for(int i=0; i<2; i+=1) ;

int *aa=nullptr;
if (auto *b=aa) *b += 1;