[NFC] `goto fail` has failed us in the past...

This patch replaces reliance on `goto failure` pattern with
`llvm::scope_exit`.

Reviewed By: bkramer

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

1 files changed, 35 insertions, 30 deletions

diff --git a/clang/lib/Frontend/FrontendAction.cpp b/clang/lib/Frontend/FrontendAction.cpp
index c996c9c..0bb62a9 100644
--- a/clang/lib/Frontend/FrontendAction.cpp
+++ b/clang/lib/Frontend/FrontendAction.cpp
@@ -27,6 +27,7 @@
 #include "clang/Serialization/ASTDeserializationListener.h"
 #include "clang/Serialization/ASTReader.h"
 #include "clang/Serialization/GlobalModuleIndex.h"
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/Support/BuryPointer.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/FileSystem.h"
@@ -558,8 +559,20 @@ bool FrontendAction::BeginSourceFile(CompilerInstance &CI,
   bool HasBegunSourceFile = false;
   bool ReplayASTFile = Input.getKind().getFormat() == InputKind::Precompiled &&
                        usesPreprocessorOnly();
+
+  // If we fail, reset state since the client will not end up calling the
+  // matching EndSourceFile(). All paths that return true should release this.
+  auto FailureCleanup = llvm::make_scope_exit([&]() {
+    if (HasBegunSourceFile)
+      CI.getDiagnosticClient().EndSourceFile();
+    CI.clearOutputFiles(/*EraseFiles=*/true);
+    CI.getLangOpts().setCompilingModule(LangOptions::CMK_None);
+    setCurrentInput(FrontendInputFile());
+    setCompilerInstance(nullptr);
+  });
+
   if (!BeginInvocation(CI))
-    goto failure;
+    return false;
 
   // If we're replaying the build of an AST file, import it and set up
   // the initial state from its build.
@@ -580,7 +593,7 @@ bool FrontendAction::BeginSourceFile(CompilerInstance &CI,
         ASTUnit::LoadPreprocessorOnly, ASTDiags, CI.getFileSystemOpts(),
         CI.getCodeGenOpts().DebugTypeExtRefs);
     if (!AST)
-      goto failure;
+      return false;
 
     // Options relating to how we treat the input (but not what we do with it)
     // are inherited from the AST unit.
@@ -649,7 +662,7 @@ bool FrontendAction::BeginSourceFile(CompilerInstance &CI,
         CI.getCodeGenOpts().DebugTypeExtRefs);
 
     if (!AST)
-      goto failure;
+      return false;
 
     // Inform the diagnostic client we are processing a source file.
     CI.getDiagnosticClient().BeginSourceFile(CI.getLangOpts(), nullptr);
@@ -669,20 +682,21 @@ bool FrontendAction::BeginSourceFile(CompilerInstance &CI,
 
     // Initialize the action.
     if (!BeginSourceFileAction(CI))
-      goto failure;
+      return false;
 
     // Create the AST consumer.
     CI.setASTConsumer(CreateWrappedASTConsumer(CI, InputFile));
     if (!CI.hasASTConsumer())
-      goto failure;
+      return false;
 
+    FailureCleanup.release();
     return true;
   }
 
   // Set up the file and source managers, if needed.
   if (!CI.hasFileManager()) {
     if (!CI.createFileManager()) {
-      goto failure;
+      return false;
     }
   }
   if (!CI.hasSourceManager())
@@ -710,12 +724,13 @@ bool FrontendAction::BeginSourceFile(CompilerInstance &CI,
 
     // Initialize the action.
     if (!BeginSourceFileAction(CI))
-      goto failure;
+      return false;
 
     // Initialize the main file entry.
     if (!CI.InitializeSourceManager(CurrentInput))
-      goto failure;
+      return false;
 
+    FailureCleanup.release();
     return true;
   }
 
@@ -748,7 +763,7 @@ bool FrontendAction::BeginSourceFile(CompilerInstance &CI,
 
       if (!Found) {
         CI.getDiagnostics().Report(diag::err_fe_no_pch_in_dir) << PCHInclude;
-        goto failure;
+        return false;
       }
     }
   }
@@ -765,7 +780,7 @@ bool FrontendAction::BeginSourceFile(CompilerInstance &CI,
 
   // Initialize the main file entry.
   if (!CI.InitializeSourceManager(Input))
-    goto failure;
+    return false;
 
   // For module map files, we first parse the module map and synthesize a
   // "<module-includes>" buffer before more conventional processing.
@@ -777,11 +792,11 @@ bool FrontendAction::BeginSourceFile(CompilerInstance &CI,
     if (loadModuleMapForModuleBuild(CI, Input.isSystem(),
                                     Input.isPreprocessed(),
                                     PresumedModuleMapFile, OffsetToContents))
-      goto failure;
+      return false;
 
     auto *CurrentModule = prepareToBuildModule(CI, Input.getFile());
     if (!CurrentModule)
-      goto failure;
+      return false;
 
     CurrentModule->PresumedModuleMapFile = PresumedModuleMapFile;
 
@@ -792,7 +807,7 @@ bool FrontendAction::BeginSourceFile(CompilerInstance &CI,
       // Otherwise, convert the module description to a suitable input buffer.
       auto Buffer = getInputBufferForModule(CI, CurrentModule);
       if (!Buffer)
-        goto failure;
+        return false;
 
       // Reinitialize the main file entry to refer to the new input.
       auto Kind = CurrentModule->IsSystem ? SrcMgr::C_System : SrcMgr::C_User;
@@ -805,7 +820,7 @@ bool FrontendAction::BeginSourceFile(CompilerInstance &CI,
 
   // Initialize the action.
   if (!BeginSourceFileAction(CI))
-    goto failure;
+    return false;
 
   // If we were asked to load any module map files, do so now.
   for (const auto &Filename : CI.getFrontendOpts().ModuleMapFiles) {
@@ -839,7 +854,7 @@ bool FrontendAction::BeginSourceFile(CompilerInstance &CI,
     std::unique_ptr<ASTConsumer> Consumer =
         CreateWrappedASTConsumer(CI, PresumedInputFile);
     if (!Consumer)
-      goto failure;
+      return false;
 
     // FIXME: should not overwrite ASTMutationListener when parsing model files?
     if (!isModelParsingAction())
@@ -850,7 +865,7 @@ bool FrontendAction::BeginSourceFile(CompilerInstance &CI,
       IntrusiveRefCntPtr<ExternalSemaSource> source, FinalReader;
       source = createChainedIncludesSource(CI, FinalReader);
       if (!source)
-        goto failure;
+        return false;
       CI.setASTReader(static_cast<ASTReader *>(FinalReader.get()));
       CI.getASTContext().setExternalSource(source);
     } else if (CI.getLangOpts().Modules ||
@@ -879,7 +894,7 @@ bool FrontendAction::BeginSourceFile(CompilerInstance &CI,
             CI.getPreprocessorOpts().AllowPCHWithCompilerErrors,
             DeserialListener, DeleteDeserialListener);
         if (!CI.getASTContext().getExternalSource())
-          goto failure;
+          return false;
       }
       // If modules are enabled, create the AST reader before creating
       // any builtins, so that all declarations know that they might be
@@ -894,7 +909,7 @@ bool FrontendAction::BeginSourceFile(CompilerInstance &CI,
 
     CI.setASTConsumer(std::move(Consumer));
     if (!CI.hasASTConsumer())
-      goto failure;
+      return false;
   }
 
   // Initialize built-in info as long as we aren't using an external AST
@@ -915,7 +930,7 @@ bool FrontendAction::BeginSourceFile(CompilerInstance &CI,
   // If we were asked to load any module files, do so now.
   for (const auto &ModuleFile : CI.getFrontendOpts().ModuleFiles)
     if (!CI.loadModuleFile(ModuleFile))
-      goto failure;
+      return false;
 
   // If there is a layout overrides file, attach an external AST source that
   // provides the layouts from that file.
@@ -927,18 +942,8 @@ bool FrontendAction::BeginSourceFile(CompilerInstance &CI,
     CI.getASTContext().setExternalSource(Override);
   }
 
+  FailureCleanup.release();
   return true;
-
-  // If we failed, reset state since the client will not end up calling the
-  // matching EndSourceFile().
-failure:
-  if (HasBegunSourceFile)
-    CI.getDiagnosticClient().EndSourceFile();
-  CI.clearOutputFiles(/*EraseFiles=*/true);
-  CI.getLangOpts().setCompilingModule(LangOptions::CMK_None);
-  setCurrentInput(FrontendInputFile());
-  setCompilerInstance(nullptr);
-  return false;
 }
 
 llvm::Error FrontendAction::Execute() {