Dmitry.Kozhevnikov created this revision.
Dmitry.Kozhevnikov added reviewers: ilya-biryukov, sammccall, milianw.
Herald added subscribers: cfe-commits, ioeric.
Related to https://reviews.llvm.org/D50455
There is some code here and there that assume that no sane output is required
if a fatal error has occurred. When using clangd/libclang, it's no longer true:
diagnostics might still be requested, and AST might still be required for other
IDE/tooling features, so it has to be as complete as possible.
Here I try to separate the following use cases:
1. Some clients check `hasFatalErrorOccurred()` because they are known to work
unstable in presence of compile errors and want to mitigate it - they'll work
as before
2. However, we don't want to take shortcuts in PP and Sema and still want to
process include directives and instantiate templates
Note: I've found out that initially the flag in `DiagnosticsEngine` (which is
now called `SuppressAfterFatalError`) had different meaning and was just
demoting all fatal errors to non-fatal (https://reviews.llvm.org/rL262318).
This would also fix this issue, however, it was partly reverted in
https://reviews.llvm.org/rL301992 to the current state. Maybe we should go with
the old approach instead (I assume the issue was that this flag was not
serialized/restored, but probably should?)
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50462
Files:
include/clang/Basic/Diagnostic.h
lib/Lex/PPDirectives.cpp
lib/Sema/SemaTemplateInstantiate.cpp
unittests/Frontend/PCHPreambleTest.cpp
Index: unittests/Frontend/PCHPreambleTest.cpp
===================================================================
--- unittests/Frontend/PCHPreambleTest.cpp
+++ unittests/Frontend/PCHPreambleTest.cpp
@@ -77,7 +77,8 @@
RemappedFiles[Filename] = Contents;
}
- std::unique_ptr<ASTUnit> ParseAST(const std::string &EntryFile) {
+ std::unique_ptr<ASTUnit> ParseAST(const std::string &EntryFile,
+ bool SuppressAfterFatalError = true) {
PCHContainerOpts = std::make_shared<PCHContainerOperations>();
std::shared_ptr<CompilerInvocation> CI(new CompilerInvocation);
CI->getFrontendOpts().Inputs.push_back(
@@ -88,11 +89,15 @@
CI->getPreprocessorOpts().RemappedFileBuffers = GetRemappedFiles();
+ CI->getLangOpts()->CPlusPlus = true;
+ CI->getLangOpts()->CPlusPlus11 = true;
+
PreprocessorOptions &PPOpts = CI->getPreprocessorOpts();
PPOpts.RemappedFilesKeepOriginalName = true;
IntrusiveRefCntPtr<DiagnosticsEngine>
Diags(CompilerInstance::createDiagnostics(new DiagnosticOptions, new DiagnosticConsumer));
+ Diags->setSuppressAfterFatalError(SuppressAfterFatalError);
FileManager *FileMgr = new FileManager(FSOpts, VFS);
@@ -197,4 +202,33 @@
ASSERT_LE(HeaderReadCount, GetFileReadCount(Header));
}
+TEST_F(PCHPreambleTest, MissingHeader) {
+ std::string Header1 = "//./header1.h";
+ AddFile(Header1,
+ "template <class T, class U = int> class C;\n"
+ "template <class T, class U> class C{};\n");
+
+ std::string Header2 = "//./header2.h";
+ AddFile(Header2, "using Alias = C<int>;\n");
+
+ std::string Main = "//./main.cpp";
+ AddFile(Main,
+ "#include \"nonexistent1\"\n"
+ "#include \"//./header1.h\"\n"
+ "#include \"nonexistent2\"\n"
+ "#include \"//./header2.h\"\n"
+ "Alias Var;");
+
+ std::unique_ptr<ASTUnit> AST(ParseAST(Main, /*SuppressAfterFatalError=*/false));
+ ASSERT_TRUE(AST.get());
+
+ // only "file not found" errors should be emitted,
+ // "Alias" should be visible for lookup.
+ auto ExpectedErrorsCount = 2u;
+
+ ASSERT_EQ(AST->getDiagnostics().getClient()->getNumErrors(), ExpectedErrorsCount);
+ ASSERT_TRUE(ReparseAST(AST));
+ ASSERT_EQ(AST->getDiagnostics().getClient()->getNumErrors(), ExpectedErrorsCount);
+}
+
} // anonymous namespace
Index: lib/Sema/SemaTemplateInstantiate.cpp
===================================================================
--- lib/Sema/SemaTemplateInstantiate.cpp
+++ lib/Sema/SemaTemplateInstantiate.cpp
@@ -218,7 +218,7 @@
// Don't allow further instantiation if a fatal error and an uncompilable
// error have occurred. Any diagnostics we might have raised will not be
// visible, and we do not need to construct a correct AST.
- if (SemaRef.Diags.hasFatalErrorOccurred() &&
+ if (SemaRef.Diags.areDiagnosticsSuppressedAfterFatalError() &&
SemaRef.Diags.hasUncompilableErrorOccurred()) {
Invalid = true;
return;
Index: lib/Lex/PPDirectives.cpp
===================================================================
--- lib/Lex/PPDirectives.cpp
+++ lib/Lex/PPDirectives.cpp
@@ -1899,7 +1899,7 @@
// Any diagnostics after the fatal error will not be visible. As the
// compilation failed already and errors in subsequently included files won't
// be visible, avoid preprocessing those files.
- if (ShouldEnter && Diags->hasFatalErrorOccurred())
+ if (ShouldEnter && Diags->areDiagnosticsSuppressedAfterFatalError())
ShouldEnter = false;
// Determine whether we should try to import the module for this #include, if
Index: include/clang/Basic/Diagnostic.h
===================================================================
--- include/clang/Basic/Diagnostic.h
+++ include/clang/Basic/Diagnostic.h
@@ -750,8 +750,21 @@
bool hasUncompilableErrorOccurred() const {
return UncompilableErrorOccurred;
}
+
+ /// Determine whether a fatal error (either a proper one or
+ /// promoted from another diagnostic) has occurred.
+ ///
+ /// Mainly to be used by clients which are known to have
+ /// unwanted behavior on severely ill-formed code.
bool hasFatalErrorOccurred() const { return FatalErrorOccurred; }
+ /// Determine whether no more diagnostics are expected,
+ /// because a fatal error was issued, so clients can cut
+ /// off extra processing that might be wasteful in this case.
+ bool areDiagnosticsSuppressedAfterFatalError() const {
+ return FatalErrorOccurred && SuppressAfterFatalError;
+ }
+
/// Determine whether any kind of unrecoverable error has occurred.
bool hasUnrecoverableErrorOccurred() const {
return FatalErrorOccurred || UnrecoverableErrorOccurred;
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
