nik updated this revision to Diff 175210.
nik added a comment.
> Maybe produce a **fatal** error in the preprocessor? That seems to be the
> simplest option: the preprocessor is aware it's building the preamble and
> there's definitely some logic to produce fatal errors in other cases (include
> not found).
> A fatal error currently aborts other stages of the compilation pipeline and
> producing one would make the run of the compiler that tries to produce the
> preamble fail, giving the empty preamble as a result.
Done. I'm using diag::err_pp_error_opening_file as introducing an new
artificial diagnostic error that the user will never see feels wrong.
Note that a preamble is generated for fatal errors like an unresolved #include
and I think that's fine. As such, I need a way to propagate the error up to
PrecompilePreambleConsumer to avoid writing the preamble. I've done that with
an extra flag in Preprocessor. An alternative might be to put it into
PreambleConditionalStackStore (as state? and rename that class to something
more general?) - what do you think?
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D53866/new/
https://reviews.llvm.org/D53866
Files:
include/clang/Lex/Preprocessor.h
include/clang/Serialization/ASTWriter.h
lib/Frontend/PrecompiledPreamble.cpp
lib/Lex/PPDirectives.cpp
test/Index/preamble-cyclic-include.cpp
Index: test/Index/preamble-cyclic-include.cpp
===================================================================
--- /dev/null
+++ test/Index/preamble-cyclic-include.cpp
@@ -0,0 +1,9 @@
+// RUN: env CINDEXTEST_EDITING=1 c-index-test -test-annotate-tokens=%s:4:1:8:1
%s 2>&1 | FileCheck %s
+// CHECK-NOT: error: unterminated conditional directive
+// CHECK-NOT: Skipping: [4:1 - 8:7]
+#ifndef A_H
+#define A_H
+# include "preamble-cyclic-include.cpp"
+int bar();
+#endif
+
Index: lib/Lex/PPDirectives.cpp
===================================================================
--- lib/Lex/PPDirectives.cpp
+++ lib/Lex/PPDirectives.cpp
@@ -1933,6 +1933,20 @@
return;
}
+ // Check whether it makes sense to continue preamble generation. We can't
+ // generate a consistent preamble with regard to the conditional stack if the
+ // main file is included again as due to the preamble bounds some directives
+ // (e.g. #endif of a header guard) will never be seen. Since this will lead
to
+ // confusing errors, abort the preamble generation.
+ if (File && PreambleConditionalStack.isRecording() &&
+ SourceMgr.translateFile(File) == SourceMgr.getMainFileID()) {
+ PreambleGenerationFailed = true;
+ // Generate a fatal error to skip further processing.
+ Diag(FilenameTok.getLocation(), diag::err_pp_error_opening_file) << ""
+ << "";
+ return;
+ }
+
// Should we enter the source file? Set to false if either the source file is
// known to have no effect beyond its effect on module visibility -- that is,
// if it's got an include guard that is already defined or is a modular
header
Index: lib/Frontend/PrecompiledPreamble.cpp
===================================================================
--- lib/Frontend/PrecompiledPreamble.cpp
+++ lib/Frontend/PrecompiledPreamble.cpp
@@ -170,6 +170,9 @@
}
void HandleTranslationUnit(ASTContext &Ctx) override {
+ if (getPreprocessor().preambleGenerationFailed())
+ return;
+
PCHGenerator::HandleTranslationUnit(Ctx);
if (!hasEmittedPCH())
return;
Index: include/clang/Serialization/ASTWriter.h
===================================================================
--- include/clang/Serialization/ASTWriter.h
+++ include/clang/Serialization/ASTWriter.h
@@ -979,6 +979,7 @@
ASTWriter &getWriter() { return Writer; }
const ASTWriter &getWriter() const { return Writer; }
SmallVectorImpl<char> &getPCH() const { return Buffer->Data; }
+ const Preprocessor &getPreprocessor() const { return PP; }
public:
PCHGenerator(const Preprocessor &PP, StringRef OutputFile, StringRef
isysroot,
Index: include/clang/Lex/Preprocessor.h
===================================================================
--- include/clang/Lex/Preprocessor.h
+++ include/clang/Lex/Preprocessor.h
@@ -388,6 +388,7 @@
SmallVector<PPConditionalInfo, 4> ConditionalStack;
State ConditionalStackState = Off;
} PreambleConditionalStack;
+ bool PreambleGenerationFailed = false;
/// The current top of the stack that we're lexing from if
/// not expanding a macro and we are lexing directly from source code.
@@ -2159,6 +2160,10 @@
Module *M,
SourceLocation MLoc);
+ bool preambleGenerationFailed() const {
+ return PreambleGenerationFailed;
+ }
+
bool isRecordingPreamble() const {
return PreambleConditionalStack.isRecording();
}
Index: test/Index/preamble-cyclic-include.cpp
===================================================================
--- /dev/null
+++ test/Index/preamble-cyclic-include.cpp
@@ -0,0 +1,9 @@
+// RUN: env CINDEXTEST_EDITING=1 c-index-test -test-annotate-tokens=%s:4:1:8:1 %s 2>&1 | FileCheck %s
+// CHECK-NOT: error: unterminated conditional directive
+// CHECK-NOT: Skipping: [4:1 - 8:7]
+#ifndef A_H
+#define A_H
+# include "preamble-cyclic-include.cpp"
+int bar();
+#endif
+
Index: lib/Lex/PPDirectives.cpp
===================================================================
--- lib/Lex/PPDirectives.cpp
+++ lib/Lex/PPDirectives.cpp
@@ -1933,6 +1933,20 @@
return;
}
+ // Check whether it makes sense to continue preamble generation. We can't
+ // generate a consistent preamble with regard to the conditional stack if the
+ // main file is included again as due to the preamble bounds some directives
+ // (e.g. #endif of a header guard) will never be seen. Since this will lead to
+ // confusing errors, abort the preamble generation.
+ if (File && PreambleConditionalStack.isRecording() &&
+ SourceMgr.translateFile(File) == SourceMgr.getMainFileID()) {
+ PreambleGenerationFailed = true;
+ // Generate a fatal error to skip further processing.
+ Diag(FilenameTok.getLocation(), diag::err_pp_error_opening_file) << ""
+ << "";
+ return;
+ }
+
// Should we enter the source file? Set to false if either the source file is
// known to have no effect beyond its effect on module visibility -- that is,
// if it's got an include guard that is already defined or is a modular header
Index: lib/Frontend/PrecompiledPreamble.cpp
===================================================================
--- lib/Frontend/PrecompiledPreamble.cpp
+++ lib/Frontend/PrecompiledPreamble.cpp
@@ -170,6 +170,9 @@
}
void HandleTranslationUnit(ASTContext &Ctx) override {
+ if (getPreprocessor().preambleGenerationFailed())
+ return;
+
PCHGenerator::HandleTranslationUnit(Ctx);
if (!hasEmittedPCH())
return;
Index: include/clang/Serialization/ASTWriter.h
===================================================================
--- include/clang/Serialization/ASTWriter.h
+++ include/clang/Serialization/ASTWriter.h
@@ -979,6 +979,7 @@
ASTWriter &getWriter() { return Writer; }
const ASTWriter &getWriter() const { return Writer; }
SmallVectorImpl<char> &getPCH() const { return Buffer->Data; }
+ const Preprocessor &getPreprocessor() const { return PP; }
public:
PCHGenerator(const Preprocessor &PP, StringRef OutputFile, StringRef isysroot,
Index: include/clang/Lex/Preprocessor.h
===================================================================
--- include/clang/Lex/Preprocessor.h
+++ include/clang/Lex/Preprocessor.h
@@ -388,6 +388,7 @@
SmallVector<PPConditionalInfo, 4> ConditionalStack;
State ConditionalStackState = Off;
} PreambleConditionalStack;
+ bool PreambleGenerationFailed = false;
/// The current top of the stack that we're lexing from if
/// not expanding a macro and we are lexing directly from source code.
@@ -2159,6 +2160,10 @@
Module *M,
SourceLocation MLoc);
+ bool preambleGenerationFailed() const {
+ return PreambleGenerationFailed;
+ }
+
bool isRecordingPreamble() const {
return PreambleConditionalStack.isRecording();
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
