llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
Author: yashg16422-design
<details>
<summary>Changes</summary>
# [clang-repl] Fix Lexical `DeclContext` Mismatch Inside Namespaces (#<!--
-->206670)
## Summary
This pull request fixes the compiler assertion reported in **Issue #<!--
-->206670** that causes **`clang-repl`** to abort when parsing an invalid
declaration inside a nested namespace.
Prior to this change, entering malformed code such as:
```cpp
namespace v1 {
int x = }
```
would trigger an internal compiler assertion and terminate `clang-repl` with a
`SIGABRT`.
With this patch applied, the assertion is eliminated. The interpreter remains
alive, and the malformed input is instead handled through Clang's normal
diagnostic pipeline.
---
# Root Cause Analysis
The failure was caused by a disagreement between two independent parts of the
compiler regarding the lexical ownership of a `TopLevelStmtDecl`.
## 1. Declaration Creation
`clang/lib/AST/Decl.cpp`
`TopLevelStmtDecl::Create()` always initialized the declaration using the
Translation Unit as its lexical declaration context:
```cpp
C.getTranslationUnitDecl()
```
This assumption was valid only for declarations created at global scope.
---
## 2. Declaration Insertion
`clang/lib/Sema/SemaDecl.cpp`
To support executing statements inside nested scopes (introduced for Issue
#<!-- -->73632), `ActOnStartTopLevelStmtDecl()` inserts the declaration into
the currently active parsing context:
```cpp
CurContext->addDecl(New);
```
When parsing code like:
```cpp
namespace v1 {
int x = }
```
the current semantic context is the namespace (`CurContext`), while the
declaration itself still believes its lexical context is the Translation Unit.
Eventually this reaches:
```cpp
DeclContext::addHiddenDecl(...)
```
which verifies that the declaration belongs to the context into which it is
being inserted.
That invariant fails:
```cpp
assert(D->getLexicalDeclContext() == this &&
"Decl inserted into wrong lexical context");
```
leading to an immediate compiler abort.
---
# Why the Parse Error Was Not the Actual Bug
The malformed declaration itself is **not** the underlying problem.
The syntax error simply forces `ParseTopLevelStmtDecl()` to execute while
parsing is still inside the namespace body.
That exposes an already-existing mismatch between
- the declaration's stored lexical context, and
- the semantic context into which it is inserted.
In other words, the invalid declaration only acts as a trigger for a latent
structural bug.
---
# Implementation
The fix removes the hardcoded assumption that every `TopLevelStmtDecl` belongs
to the Translation Unit.
Instead, the active declaration context is propagated throughout the creation
pipeline.
## 1. `clang/include/clang/AST/Decl.h`
Updated the factory method signature:
```cpp
TopLevelStmtDecl::Create(ASTContext &C,
DeclContext *DC,
SourceLocation BeginLoc,
SourceLocation EndLoc);
```
---
## 2. `clang/lib/AST/Decl.cpp`
Replaced:
```cpp
C.getTranslationUnitDecl()
```
with the supplied declaration context:
```cpp
DC
```
allowing the declaration to inherit the correct lexical owner.
---
## 3. `clang/lib/Sema/SemaDecl.cpp`
Updated the caller to forward the active semantic context:
```cpp
TopLevelStmtDecl::Create(
Context,
CurContext,
BeginLoc,
EndLoc);
```
As a result, the declaration is now created with the same lexical context into
which it is later inserted, satisfying the invariant enforced by
`DeclContext::addHiddenDecl()`.
---
# Additional Issue Uncovered During Validation
While validating this fix, a second and unrelated issue surfaced within Clang's
parser recovery logic.
Once the assertion is removed, malformed statements inside **any**
brace-enclosed scope no longer crash the interpreter. Instead, they expose a
broader parser recovery problem that results in an infinite loop.
For example:
```cpp
{
int x = }
```
or
```cpp
namespace v1 {
int x = }
```
both enter an endless recovery cycle.
---
# Why the Infinite Loop Happens
After a syntax error, Clang attempts to recover by skipping tokens until it
reaches a synchronization point (typically a semicolon).
During this recovery, the parser may consume past the closing brace (`}`) of
the current scope.
Once that happens:
- the parser loses track of the active compound statement,
- it incorrectly believes the block is still open,
- and repeatedly attempts to recover from the same location without advancing
the token stream.
The parser therefore loops indefinitely while consuming 100% CPU.
---
## Observed Call Stack
A live debugging session with `gdb` repeatedly produced the same execution
stack:
```text
(gdb) bt
#<!-- -->0 clang::Sema::DiscardCleanupsInEvaluationContext()
#<!-- -->1 clang::Sema::ActOnExprStmtError()
#<!-- -->2 clang::Parser::ParseExprStatement()
#<!-- -->3 clang::Parser::ParseCompoundStatementBody()
#<!-- -->4 clang::Parser::ParseStatementOrDeclaration()
```
The stack remains unchanged across iterations, confirming that parsing is
trapped inside the same recovery path.
---
# Why a Regression Test Is Not Included
Normally this change would be accompanied by a regression test in:
```
clang/test/Interpreter/fail.cpp
```
using a command similar to:
```text
// RUN: not clang-repl "namespace v1 { int x = }"
```
However, the command-line evaluation path uses the same incremental parser as
the interactive REPL.
Because the parser currently enters the infinite recovery loop described above,
the command never terminates.
As a result:
- `clang-repl` never returns an exit code,
- `llvm-lit` waits indefinitely,
- and the entire CI test suite hangs.
Adding such a regression test at this point would therefore stall automated
builds.
---
# Scope of This Pull Request
This pull request intentionally focuses on the structural `DeclContext`
mismatch responsible for the assertion failure.
The parser recovery infinite loop is a separate issue affecting Clang's general
error-recovery mechanism and extends well beyond namespace handling.
Because resolving it will likely require changes to the parser's
synchronization and recovery logic, it is better addressed independently in a
dedicated issue and follow-up patch.
---
# Final Result
Running the reproducer after applying this patch:
```bash
yash@<!-- -->yash-VirtualBox:~/llvm-project$ ./build/bin/clang-repl "namespace
v1 { int x = }"
In file included from <<< inputs >>>:1:
input_line_1:1:24: error: expected expression
1 | namespace v1 { int x = }
| ^
input_line_1:1:25: error: expected ';' after top level declarator
1 | namespace v1 { int x = }
| ^
| ;
<<< inputs >>>:1:1: error: expected expression
<<< inputs >>>:1:1: error: expected expression
<<< inputs >>>:1:1: error: expected expression
<<< inputs >>>:1:1: error: expected expression
<<< inputs >>>:1:1: error: expected expression
<<< inputs >>>:1:1: error: expected expression
<<< inputs >>>:1:1: error: expected expression
<<< inputs >>>:1:1: error: expected expression
fatal error: too many errors emitted, stopping now [-ferror-limit=]
```
The interpreter no longer terminates with an internal compiler assertion.
Instead, it remains alive and reports diagnostics through Clang's normal
error-reporting mechanism, exposing the independent parser recovery issue
described above.
---
Full diff: https://github.com/llvm/llvm-project/pull/206807.diff
3 Files Affected:
- (modified) clang/include/clang/AST/Decl.h (+1-1)
- (modified) clang/lib/AST/Decl.cpp (+3-7)
- (modified) clang/lib/Sema/SemaDecl.cpp (+1-1)
``````````diff
diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h
index 2ea16d0ba6b03..aae82ad8826cd 100644
--- a/clang/include/clang/AST/Decl.h
+++ b/clang/include/clang/AST/Decl.h
@@ -4676,7 +4676,7 @@ class TopLevelStmtDecl : public Decl, public DeclContext {
virtual void anchor();
public:
- static TopLevelStmtDecl *Create(ASTContext &C, Stmt *Statement);
+ static TopLevelStmtDecl *Create(ASTContext &C, DeclContext *DC, Stmt
*Statement);
static TopLevelStmtDecl *CreateDeserialized(ASTContext &C, GlobalDeclID ID);
SourceRange getSourceRange() const override LLVM_READONLY;
diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index b23bf73ae803c..640e8c114b84b 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -5860,17 +5860,13 @@ std::string FileScopeAsmDecl::getAsmString() const {
}
void TopLevelStmtDecl::anchor() {}
-
-TopLevelStmtDecl *TopLevelStmtDecl::Create(ASTContext &C, Stmt *Statement) {
+TopLevelStmtDecl *TopLevelStmtDecl::Create(ASTContext &C, DeclContext *DC,
Stmt *Statement) {
assert(C.getLangOpts().IncrementalExtensions &&
"Must be used only in incremental mode");
- SourceLocation Loc = Statement ? Statement->getBeginLoc() : SourceLocation();
- DeclContext *DC = C.getTranslationUnitDecl();
-
- return new (C, DC) TopLevelStmtDecl(DC, Loc, Statement);
+ SourceLocation BeginLoc = Statement ? Statement->getBeginLoc() :
SourceLocation();
+ return new (C, DC) TopLevelStmtDecl(DC, BeginLoc, Statement);
}
-
TopLevelStmtDecl *TopLevelStmtDecl::CreateDeserialized(ASTContext &C,
GlobalDeclID ID) {
return new (C, ID)
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index d45c3eb35094f..fec055965b8ef 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -21149,7 +21149,7 @@ Decl *Sema::ActOnFileScopeAsmDecl(Expr *expr,
SourceLocation StartLoc,
}
TopLevelStmtDecl *Sema::ActOnStartTopLevelStmtDecl(Scope *S) {
- auto *New = TopLevelStmtDecl::Create(Context, /*Statement=*/nullptr);
+ auto *New = TopLevelStmtDecl::Create(Context, CurContext,
/*Statement=*/nullptr);
CurContext->addDecl(New);
PushDeclContext(S, New);
PushFunctionScope();
``````````
</details>
https://github.com/llvm/llvm-project/pull/206807
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
