[clang] 744ec74 - [NFC] `goto fail` has failed us in the past...
Chris Bieneman via cfe-commits
cfe-commits at lists.llvm.org
Tue Sep 21 07:18:46 PDT 2021
Author: Chris Bieneman
Date: 2021-09-21T09:18:37-05:00
New Revision: 744ec74b305a5039dc74e2d043e1c136e06beac1
URL: https://github.com/llvm/llvm-project/commit/744ec74b305a5039dc74e2d043e1c136e06beac1
DIFF: https://github.com/llvm/llvm-project/commit/744ec74b305a5039dc74e2d043e1c136e06beac1.diff
LOG: [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
Added:
Modified:
clang/lib/Frontend/FrontendAction.cpp
Removed:
################################################################################
diff --git a/clang/lib/Frontend/FrontendAction.cpp b/clang/lib/Frontend/FrontendAction.cpp
index c996c9c486bc4..0bb62a96e02cc 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() {
More information about the cfe-commits
mailing list