llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Devajith (devajithvs) <details> <summary>Changes</summary> `ActOnStartTopLevelStmtDecl` immediately adds the new `TopLevelStmtDecl` to the current `DeclContext`. If parsing fails, the decl remains attached to the TU even though it has no valid `Stmt`, leaving the AST in an invalid state. --- Full diff: https://github.com/llvm/llvm-project/pull/153945.diff 2 Files Affected: - (modified) clang/lib/Parse/ParseDecl.cpp (+4-1) - (modified) clang/unittests/Interpreter/InterpreterTest.cpp (+48) ``````````diff diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp index fd53cca5a13ff..59c5f4cacc36e 100644 --- a/clang/lib/Parse/ParseDecl.cpp +++ b/clang/lib/Parse/ParseDecl.cpp @@ -5696,8 +5696,11 @@ Parser::DeclGroupPtrTy Parser::ParseTopLevelStmtDecl() { TopLevelStmtDecl *TLSD = Actions.ActOnStartTopLevelStmtDecl(getCurScope()); StmtResult R = ParseStatementOrDeclaration(Stmts, SubStmtCtx); Actions.ActOnFinishTopLevelStmtDecl(TLSD, R.get()); - if (!R.isUsable()) + if (!R.isUsable()) { + if (DeclContext *DC = TLSD->getDeclContext()) + DC->removeDecl(TLSD); // unlink from TU R = Actions.ActOnNullStmt(Tok.getLocation()); + } if (Tok.is(tok::annot_repl_input_end) && Tok.getAnnotationValue() != nullptr) { diff --git a/clang/unittests/Interpreter/InterpreterTest.cpp b/clang/unittests/Interpreter/InterpreterTest.cpp index 8639fb668f1fe..c9eb65f346041 100644 --- a/clang/unittests/Interpreter/InterpreterTest.cpp +++ b/clang/unittests/Interpreter/InterpreterTest.cpp @@ -21,6 +21,8 @@ #include "clang/Interpreter/Value.h" #include "clang/Sema/Lookup.h" #include "clang/Sema/Sema.h" +#include "llvm/Support/FileSystem.h" +#include "llvm/Support/Path.h" #include "llvm/TargetParser/Host.h" @@ -91,6 +93,52 @@ TEST_F(InterpreterTest, IncrementalInputTopLevelDecls) { EXPECT_EQ("var2", DeclToString(*R2DeclRange.begin())); } +TEST_F(InterpreterTest, BadIncludeDoesNotCorruptTU) { + // Create a temporary header that triggers an error at top level. + llvm::SmallString<256> HeaderPath; + ASSERT_FALSE(llvm::sys::fs::createTemporaryFile("interp-bad-include", "h", + HeaderPath)); + { + std::error_code EC; + llvm::raw_fd_ostream OS(HeaderPath, EC, llvm::sys::fs::OF_Text); + ASSERT_FALSE(EC); + OS << R"(error_here; // expected-error {{use of undeclared identifier}})"; + } + + llvm::SmallString<256> HeaderDir = llvm::sys::path::parent_path(HeaderPath); + llvm::SmallString<256> HeaderFile = llvm::sys::path::filename(HeaderPath); + + // Set up the interpreter with the include path. + using Args = std::vector<const char *>; + Args ExtraArgs = {"-I", HeaderDir.c_str(), + "-Xclang", "-diagnostic-log-file", + "-Xclang", "-"}; + + // Create the diagnostic engine with unowned consumer. + std::string DiagnosticOutput; + llvm::raw_string_ostream DiagnosticsOS(DiagnosticOutput); + auto DiagPrinter = std::make_unique<TextDiagnosticPrinter>( + DiagnosticsOS, new DiagnosticOptions()); + + auto Interp = createInterpreter(ExtraArgs, DiagPrinter.get()); + // This parse should fail because of an undeclared identifier, but should + // leave TU intact. + auto Err = + Interp->Parse("#include \"" + HeaderFile.str().str() + "\"").takeError(); + using ::testing::HasSubstr; + EXPECT_THAT(DiagnosticOutput, + HasSubstr("use of undeclared identifier 'error_here'")); + EXPECT_EQ("Parsing failed.", llvm::toString(std::move(Err))); + + // Inspect the TU, there must not be any TopLevelStmtDecl left. + TranslationUnitDecl *TU = + Interp->getCompilerInstance()->getASTContext().getTranslationUnitDecl(); + EXPECT_EQ(0U, DeclsSize(TU)); + + // Cleanup temp file. + llvm::sys::fs::remove(HeaderPath); +} + TEST_F(InterpreterTest, Errors) { Args ExtraArgs = {"-Xclang", "-diagnostic-log-file", "-Xclang", "-"}; `````````` </details> https://github.com/llvm/llvm-project/pull/153945 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
