Minor improvements

include/revng/Pipeline/Context.h

@@ -81,7 +81,7 @@ public:
   llvm::Expected<T *> getExternalContext(llvm::StringRef Name) const {
     auto Iter = Contexts.find(Name);
     if (Iter == Contexts.end()) {
-      auto *Message = "pipeline loader context did not contained object ";
+      auto *Message = "pipeline loader context did not contain the object ";
       return revng::createError(Message + Name);
     }
 

include/revng/Pipeline/CopyPipe.h

@@ -29,10 +29,6 @@ public:
     // Commit
     EC.commitAllFor(T);
   }
-
-  llvm::Error checkPrecondition(const pipeline::Context &Ctx) const {
-    return llvm::Error::success();
-  }
 };
 
 } // namespace pipeline

include/revng/Pipeline/Errors.h

@@ -4,6 +4,7 @@
 // This file is distributed under the MIT License. See LICENSE.md for details.
 //
 
+#include "llvm/Support/Debug.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/raw_ostream.h"
 
@@ -62,13 +63,7 @@ private:
 
 public:
   AnnotatedError(llvm::Error Error, const llvm::Twine &ExtraData) :
-    ExtraData(ExtraData.str()) {
-    revng_assert(Error);
-    llvm::raw_string_ostream S(Inner);
-    S << Error;
-    S.flush();
-    llvm::consumeError(std::move(Error));
-  }
+    Inner(consumeToString(std::move(Error))), ExtraData(ExtraData.str()) {}
 
 public:
   std::error_code convertToErrorCode() const override;

include/revng/Pipes/FileContainer.h

@@ -135,12 +135,11 @@ public:
     if (empty())
       return llvm::Error::success();
 
-    if (auto MaybeBuffer = llvm::MemoryBuffer::getFile(File->path());
-        !MaybeBuffer)
+    if (auto MaybeBuffer = llvm::MemoryBuffer::getFile(File->path()))
+      OS << (*MaybeBuffer)->getBuffer();
+    else
       return llvm::createStringError(MaybeBuffer.getError(),
                                      "could not read file");
-    else
-      OS << (*MaybeBuffer)->getBuffer();
 
     return llvm::Error::success();
   }

include/revng/Pipes/PipelineManager.h

@@ -145,7 +145,7 @@ public:
   invalidateFromDiff(const llvm::StringRef Name,
                      const pipeline::GlobalTupleTreeDiff &Diff);
 
-  /// returns the cached list of targets that are known to be available to be
+  /// \returns the cached list of targets that are known to be available to be
   /// produced in a container
   const pipeline::TargetsList *
   getTargetsAvailableFor(const Container &TheContainer) const {

include/revng/Pipes/ToolCLOptions.h

@@ -51,7 +51,7 @@ public:
   llvm::Error overrideModel(revng::FilePath ModelOverride,
                             PipelineManager &Manager) {
     const auto &Name = ModelGlobalName;
-    auto *Model(cantFail(Manager.context().getGlobal<ModelGlobal>(Name)));
+    auto *Model = cantFail(Manager.context().getGlobal<ModelGlobal>(Name));
     return Model->load(ModelOverride);
   }
 

include/revng/Pipes/TupleTreeContainer.h

@@ -115,10 +115,9 @@ public:
   }
 
   llvm::Error serialize(llvm::raw_ostream &OS) const override {
-    if (not Content)
-      return llvm::Error::success();
+    if (Content)
+      Content.value().serialize(OS);
 
-    Content.value().serialize(OS);
     return llvm::Error::success();
   }
 

lib/ABI/ModelHelpers.cpp

@@ -110,7 +110,7 @@ model::UpcastableType fromLLVMString(llvm::Value *V,
     std::string Error = "Could not deserialize the model type from LLVM "
                         "constant string \""
                         + BaseTypeString.str()
-                        + "\2: " + consumeToString(ParsedType) + ".";
+                        + "\": " + consumeToString(ParsedType) + ".";
     revng_abort(Error.c_str());
   }
 

lib/ImportFromC/HeaderToModel.cpp

@@ -1262,7 +1262,6 @@ void HeaderToModelDiagnosticConsumer::HandleDiagnostic(Level DiagLevel,
   llvm::raw_svector_ostream DiagMessageStream(OutStr);
 
   std::string Text;
-  std::string ErrorLocation;
   llvm::raw_string_ostream OS(Text);
   auto *DiagOpts = &Info.getDiags()->getDiagnosticOptions();
 

lib/LLMRename/LLMRenameAnalysis.cpp

@@ -38,16 +38,14 @@ struct LLMRename {
       ProgramRunner::Result Result = ::Runner.run("revng",
                                                   { "llm-rename" },
                                                   Options);
-      if (Result.ExitCode != 0) {
-        if (Result.ExitCode == 2) {
-          // Exit code 2 is treated specially to propagate stderr as the error
-          // message
-          return revng::createError(Result.Stderr);
-        } else {
-          return revng::createError("Failed to run llm-rename, process "
-                                    "returned with exit code: "
-                                    + std::to_string(Result.ExitCode));
-        }
+      if (Result.ExitCode == 2) {
+        // Exit code 2 is treated specially to propagate stderr as the error
+        // message
+        return revng::createError(Result.Stderr);
+      } else if (Result.ExitCode != 0) {
+        return revng::createError("Failed to run llm-rename, process "
+                                  "returned with exit code: "
+                                  + std::to_string(Result.ExitCode));
       }
 
       auto MaybeChanges = fromString<TupleTreeDiff<ModelT>>(Result.Stdout);

lib/Lift/LiftPipe.cpp

@@ -142,8 +142,8 @@ llvm::Error Lift::checkPrecondition(const pipeline::Context &Context) const {
 
   if (Model.DefaultABI() == model::ABI::Invalid
       and Model.DefaultPrototype().isEmpty()) {
-    return revng::createError("Cannot lift binary without either a DefaultABI "
-                              "or a DefaultPrototype.");
+    return revng::createError("Cannot lift binary with neither `DefaultABI` "
+                              "nor `DefaultPrototype`.");
   }
 
   return llvm::Error::success();

lib/Model/Importer/Binary/ELFImporter.cpp

@@ -484,11 +484,11 @@ void ELFImporter<T, HasAddend>::findMissingTypes(object::ELFFile<T> &TheELF,
         .EnableRemoteDebugInfo = Opts.EnableRemoteDebugInfo,
         .AdditionalDebugInfoPaths = Opts.AdditionalDebugInfoPaths
       };
-      if (auto E = importELF(DepModel, *TheBinary, AdjustedOptions)) {
+      if (auto Error = importELF(DepModel, *TheBinary, AdjustedOptions)) {
         revng_log(ELFImporterLog,
                   "Can't import model for " << DependencyLibrary << " due to "
-                                            << E);
-        llvm::consumeError(std::move(E));
+                                            << Error);
+        llvm::consumeError(std::move(Error));
         ModelsOfLibraries.erase(DependencyLibrary);
         continue;
       }

lib/Model/Importer/DebugInfo/DwarfImporter.cpp

@@ -1200,25 +1200,23 @@ void DwarfImporter::import(StringRef FileName, const ImporterOptions &Options) {
   // improvement.
   auto HasDebugInfo = [](ObjectFile *Object) {
     for (const SectionRef &Section : Object->sections()) {
-      StringRef SectionName;
       if (Expected<StringRef> NameOrErr = Section.getName()) {
-        SectionName = *NameOrErr;
+        // TODO: When adding support for Split dwarf, there will be
+        // .debug_info.dwo section, so we need to handle it.
+        if (*NameOrErr == ".debug_info")
+          return true;
+
       } else {
         llvm::consumeError(NameOrErr.takeError());
         continue;
       }
-
-      // TODO: When adding support for Split dwarf, there will be
-      // .debug_info.dwo section, so we need to handle it.
-      if (SectionName == ".debug_info")
-        return true;
     }
     return false;
   };
 
   auto PerformImport = [this, &T, &Options](StringRef FilePath,
                                             StringRef TheDebugFile) {
-    auto ExpectedBinary = object::createBinary(FilePath);
+    llvm::Expected ExpectedBinary = object::createBinary(FilePath);
     if (!ExpectedBinary) {
       revng_log(DILogger, "Can't create binary for " << FilePath);
       llvm::consumeError(ExpectedBinary.takeError());

lib/Pipeline/Global.cpp

@@ -46,6 +46,5 @@ Error Global::load(const revng::FilePath &Path) {
   }
 
   llvm::StringRef String = MaybeBuffer.get()->buffer().getBuffer();
-  llvm::Error DeserializeError = fromString(String);
-  return DeserializeError;
+  return fromString(String);
 }

lib/Pipeline/LLVMContainer.cpp

@@ -65,14 +65,14 @@ std::unique_ptr<ContainerBase>
 LLVMContainer::cloneFiltered(const TargetsList &Targets) const {
   using InspectorT = LLVMKind;
   auto ToClone = InspectorT::functions(Targets, *this->self());
-  auto ToClonedNotOwned = InspectorT::untrackedFunctions(*this->self());
+  auto ToCloneUntracked = InspectorT::untrackedFunctions(*this->self());
 
-  const auto Filter = [&ToClone, &ToClonedNotOwned](const auto &GlobalSym) {
+  const auto Filter = [&ToClone, &ToCloneUntracked](const auto &GlobalSym) {
     if (not llvm::isa<llvm::Function>(GlobalSym))
       return true;
 
     const auto &F = llvm::cast<llvm::Function>(GlobalSym);
-    return ToClone.contains(F) or ToClonedNotOwned.contains(F);
+    return ToClone.contains(F) or ToCloneUntracked.contains(F);
   };
 
   llvm::ValueToValueMapTy Map;

lib/Pipeline/Step.cpp

@@ -459,10 +459,10 @@ Step::loadInvalidationMetadataImpl(const revng::DirectoryPath &Path,
     for (NamedPathTargetBimapVector &Entry : *Parsed) {
       Global *Global = llvm::cantFail(TheContext->getGlobals()
                                         .get(Entry.GlobalName));
-      auto Parsed(Entry.Map.deserialize(*TheContext,
-                                        *Global,
-                                        Pipe.Pipe->getName(),
-                                        Container.first()));
+      auto Parsed = Entry.Map.deserialize(*TheContext,
+                                          *Global,
+                                          Pipe.Pipe->getName(),
+                                          Container.first());
       if (not Parsed)
         return Parsed.takeError();
       Pipe.InvalidationMetadata.getPathCache(Global->getName())

lib/Pipes/PipelineManager.cpp

@@ -245,7 +245,7 @@ static llvm::Expected<FilePath> migrateModel(const FilePath &ModelFile) {
 
   if (auto MigrationError = migrateModelFile(ModelFile); !!MigrationError) {
     // Migration failed, that's okay though, as the original model has not been
-    // written (see migrateModelFile). Just return the error.
+    // overridden (see migrateModelFile). Just return the error.
     return MigrationError;
   }
 
@@ -278,10 +278,11 @@ PipelineManager::setUpPipeline(llvm::ArrayRef<std::string> TextPipelines) {
         return revng::joinErrors(ModelFileExists.takeError(),
                                  std::move(FirstLoadError));
 
-      if (not ModelFileExists.get())
+      if (not ModelFileExists.get()) {
         // Model file does not exist, so nothing to migrate. Stop here and
         // return the first load error.
         return FirstLoadError;
+      }
 
       llvm::Expected<FilePath> BackupFilePath = migrateModel(ModelFile);
 
@@ -291,13 +292,13 @@ PipelineManager::setUpPipeline(llvm::ArrayRef<std::string> TextPipelines) {
 
       if (auto SecondLoadError = Runner
                                    ->loadContextDirectory(ExecutionDirectory)) {
-        // Doesn't load even after migration, restore and report the errors.
         if (auto Error = BackupFilePath.get().copyTo(ModelFile)) {
           return revng::joinErrors(std::move(Error),
                                    std::move(SecondLoadError),
                                    std::move(FirstLoadError));
         }
 
+        // Doesn't load even after migration, restore and report the errors?
         return llvm::joinErrors(std::move(SecondLoadError),
                                 std::move(FirstLoadError));
       }

lib/Pipes/Pipes.cpp

@@ -57,16 +57,15 @@ public:
   void registerContainersAndPipes(Loader &Loader) override {
     using namespace llvm;
     auto &Context = Loader.getContext();
-    auto MaybeLLVMContext = Context.getExternalContext<LLVMContext>("LLVMContex"
-                                                                    "t");
+    auto MaybeContext = Context.getExternalContext<LLVMContext>("LLVMContext");
 
-    if (auto Error = MaybeLLVMContext.takeError()) {
+    if (auto Error = MaybeContext.takeError()) {
       consumeError(std::move(Error));
       return;
     }
 
     auto &PipeContext = Loader.getContext();
-    auto &LLVMContext = **MaybeLLVMContext;
+    auto &LLVMContext = **MaybeContext;
     auto Factory = ContainerFactory::fromGlobal<LLVMContainer>(&PipeContext,
                                                                &LLVMContext);
 

lib/RestructureCFG/RestructureCFG.cpp

@@ -1493,7 +1493,6 @@ bool restructureCFG(Function &F, ASTTree &AST) {
 
     float Increase = float(FinalWeight) / float(InitialWeight);
 
-    ExitOnError ExitOnError;
     std::error_code EC;
 
     const char *FunctionName = F.getName().data();

lib/RestructureCFG/ScopeGraphLoggerPass.cpp

@@ -28,7 +28,6 @@ public:
 
 public:
   bool run() {
-    ExitOnError ExitOnError;
     std::error_code EC;
 
     llvm::ToolOutputFile OutputFile(OutputPath, EC, llvm::sys::fs::OF_Text);

tools/pipeline/Main.cpp

@@ -194,9 +194,8 @@ int main(int argc, char *argv[]) {
   else if (ProduceAllPossibleTargetsSingle)
     AbortOnError(Manager.produceAllPossibleSingleTargets());
 
-  if (InvalidateAll) {
+  if (InvalidateAll)
     AbortOnError(Manager.invalidateAllPossibleTargets());
-  }
 
   AbortOnError(Manager.store(StoresOverrides));
   AbortOnError(Manager.store());