[clang][Frontend] Add missing error handling

Some early errors during the ASTUnit creation were not transferred to the `FailedParseDiagnostic` so when the code in `LoadFromCommandLine` swaps its content with the content of `StoredDiagnostics` they cannot be retrieved by the user in any way.

Reviewed By: andrewrk, dblaikie

Differential Revision: https://reviews.llvm.org/D78658

diff --git a/clang/lib/Frontend/ASTUnit.cpp b/clang/lib/Frontend/ASTUnit.cpp
index 7fca190..0728fee 100644 (file)
--- a/clang/lib/Frontend/ASTUnit.cpp
+++ b/clang/lib/Frontend/ASTUnit.cpp
@@ -69,6 +69,7 @@
 #include "llvm/ADT/None.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringMap.h"
@@ -1118,6 +1119,19 @@ bool ASTUnit::Parse(std::shared_ptr<PCHContainerOperations> PCHContainerOps,
   std::unique_ptr<CompilerInstance> Clang(
       new CompilerInstance(std::move(PCHContainerOps)));
 
+  // Clean up on error, disengage it if the function returns successfully.
+  auto CleanOnError = llvm::make_scope_exit([&]() {
+    // Remove the overridden buffer we used for the preamble.
+    SavedMainFileBuffer = nullptr;
+
+    // Keep the ownership of the data in the ASTUnit because the client may
+    // want to see the diagnostics.
+    transferASTDataFromCompilerInstance(*Clang);
+    FailedParseDiagnostics.swap(StoredDiagnostics);
+    StoredDiagnostics.clear();
+    NumStoredDiagnosticsFromDriver = 0;
+  });
+
   // Ensure that Clang has a FileManager with the right VFS, which may have
   // changed above in AddImplicitPreamble.  If VFS is nullptr, rely on
   // createFileManager to create one.
@@ -1200,7 +1214,7 @@ bool ASTUnit::Parse(std::shared_ptr<PCHContainerOperations> PCHContainerOps,
     ActCleanup(Act.get());
 
   if (!Act->BeginSourceFile(*Clang.get(), Clang->getFrontendOpts().Inputs[0]))
-    goto error;
+    return true;
 
   if (SavedMainFileBuffer)
     TranslateStoredDiagnostics(getFileManager(), getSourceManager(),
@@ -1210,7 +1224,7 @@ bool ASTUnit::Parse(std::shared_ptr<PCHContainerOperations> PCHContainerOps,
 
   if (llvm::Error Err = Act->Execute()) {
     consumeError(std::move(Err)); // FIXME this drops errors on the floor.
-    goto error;
+    return true;
   }
 
   transferASTDataFromCompilerInstance(*Clang);
@@ -1219,19 +1233,9 @@ bool ASTUnit::Parse(std::shared_ptr<PCHContainerOperations> PCHContainerOps,
 
   FailedParseDiagnostics.clear();
 
-  return false;
+  CleanOnError.release();
 
-error:
-  // Remove the overridden buffer we used for the preamble.
-  SavedMainFileBuffer = nullptr;
-
-  // Keep the ownership of the data in the ASTUnit because the client may
-  // want to see the diagnostics.
-  transferASTDataFromCompilerInstance(*Clang);
-  FailedParseDiagnostics.swap(StoredDiagnostics);
-  StoredDiagnostics.clear();
-  NumStoredDiagnosticsFromDriver = 0;
-  return true;
+  return false;
 }
 
 static std::pair<unsigned, unsigned>

diff --git a/clang/unittests/Frontend/ASTUnitTest.cpp b/clang/unittests/Frontend/ASTUnitTest.cpp
index c5b84e7..eb3fdb4 100644 (file)
--- a/clang/unittests/Frontend/ASTUnitTest.cpp
+++ b/clang/unittests/Frontend/ASTUnitTest.cpp
@@ -150,4 +150,28 @@ TEST_F(ASTUnitTest, ModuleTextualHeader) {
       &File->getFileEntry()));
 }
 
+TEST_F(ASTUnitTest, LoadFromCommandLineEarlyError) {
+  EXPECT_FALSE(
+      llvm::sys::fs::createTemporaryFile("ast-unit", "c", FD, InputFileName));
+  input_file = std::make_unique<ToolOutputFile>(InputFileName, FD);
+  input_file->os() << "";
+
+  const char *Args[] = {"clang", "-target", "foobar", InputFileName.c_str()};
+
+  auto Diags = CompilerInstance::createDiagnostics(new DiagnosticOptions());
+  auto PCHContainerOps = std::make_shared<PCHContainerOperations>();
+  std::unique_ptr<clang::ASTUnit> ErrUnit;
+
+  ASTUnit *AST = ASTUnit::LoadFromCommandLine(
+      &Args[0], &Args[4], PCHContainerOps, Diags, "", false,
+      CaptureDiagsKind::All, None, true, 0, TU_Complete, false, false, false,
+      SkipFunctionBodiesScope::None, false, true, false, false, None, &ErrUnit,
+      nullptr);
+
+  ASSERT_EQ(AST, nullptr);
+  ASSERT_NE(ErrUnit, nullptr);
+  ASSERT_TRUE(Diags->hasErrorOccurred());
+  ASSERT_NE(ErrUnit->stored_diag_size(), 0U);
+}
+
 } // anonymous namespace