[llvm-branch-commits] [clang] [SSAF][UnsafeBufferUsage] Remove UnsafeBufferUsageExtractor.h (PR #191931)

[Balázs Benics via llvm-branch-commits]

================
@@ -125,39 +130,36 @@ void UnsafeBufferUsageTUSummaryExtractor::HandleTranslationUnit(
 
       if (DC->isFileContext() || DC->isNamespace())
         Contributors.push_back(D);
-      return false;
+      return true;
     }
   } ContributorFinder;
 
-  ContributorFinder.VisitTranslationUnitDecl(Ctx.getTranslationUnitDecl());
-
-  llvm::Error Errors = llvm::ErrorSuccess();
-  auto addError = [&Errors](llvm::Error Err) {
-    Errors = llvm::joinErrors(std::move(Errors), std::move(Err));
-  };
-
+  ContributorFinder.TraverseAST(Ctx);
   for (auto *CD : ContributorFinder.Contributors) {
-    llvm::Error Error = llvm::ErrorSuccess();
-    auto EntitySummary = extractEntitySummary(CD, Ctx, Error);
+    auto EntitySummary = extractEntitySummary(CD, Ctx);
 
-    if (Error)
-      addError(std::move(Error));
-    if (EntitySummary->empty())
+    if (!EntitySummary)
+      llvm::reportFatalInternalError(EntitySummary.takeError());
+    assert(*EntitySummary);
----------------
steakhal wrote:

I think these 3 lines could be replaced by `llvm::cantFail`. The ergonomic thing about it that it collapses the `Expected<T>` into just `T` and aborts if not satisfied.

There would be one difference however, that with non-debug builds it wouldn't print the error from Expected. And if `LLVM_UNREACHABLE_OPTIMIZE` was set, it would optimize away the failure branch, thus exhibit UB in that case on failure.
So it really depends what we want here.

BTW what you have here also totally makes sense. What I'm saying it was kindof weird to see that we are not using `assert` for expressing something similar, while on the very next line we do use an assert. And this was what led me to this question.

https://github.com/llvm/llvm-project/pull/191931