gribozavr2 added inline comments.
================
Comment at: clang/unittests/Analysis/FlowSensitive/TestingSupport.h:389
/// verify the results.
-template <typename VerifyResultsT>
-llvm::Error
-runDataflowReturnError(llvm::StringRef Code, VerifyResultsT VerifyResults,
- DataflowAnalysisOptions Options,
- LangStandard::Kind Std = LangStandard::lang_cxx17,
- llvm::StringRef TargetFun = "target") {
- using ast_matchers::hasName;
- llvm::SmallVector<std::string, 3> ASTBuildArgs = {
- // -fnodelayed-template-parsing is the default everywhere but on Windows.
- // Set it explicitly so that tests behave the same on Windows as on other
- // platforms.
- "-fsyntax-only", "-fno-delayed-template-parsing",
- "-std=" +
- std::string(LangStandard::getLangStandardForKind(Std).getName())};
- AnalysisInputs<NoopAnalysis> AI(
- Code, hasName(TargetFun),
- [UseBuiltinModel = Options.BuiltinOpts.has_value()](ASTContext &C,
- Environment &Env) {
- return NoopAnalysis(
- C,
- DataflowAnalysisOptions{
- UseBuiltinModel ? Env.getDataflowAnalysisContext().getOptions()
- : std::optional<BuiltinOptions>()});
- });
- AI.ASTBuildArgs = ASTBuildArgs;
- if (Options.BuiltinOpts)
- AI.BuiltinOptions = *Options.BuiltinOpts;
- return checkDataflow<NoopAnalysis>(
- std::move(AI),
- /*VerifyResults=*/
- [&VerifyResults](
- const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
- const AnalysisOutputs &AO) { VerifyResults(Results, AO.ASTCtx); });
-}
+llvm::Error runDataflowReturnError(
+ llvm::StringRef Code,
----------------
mboehme wrote:
> gribozavr2 wrote:
> > mboehme wrote:
> > > gribozavr2 wrote:
> > > > I don't quite understand your comment from the previous patch:
> > > >
> > > > > This is really just moved from TransferTest.cpp -- and it's more
> > > > > closely related to the `runDataflow()` functions there and in the
> > > > > newly added RecordOps.cpp. (I can't call it `runDataflow()` because
> > > > > then it would differ only in return type from one of the functions in
> > > > > the overload set.)
> > > >
> > > > I don't see another `checkDataflow()` function with the same signature.
> > > > Furthermore, `checkDataflow()` overloads above already return an
> > > > `llvm::Error`.
> > > What I meant is: This function seemed to me to be more closely related to
> > > the `runDataflow()` functions in TransferTest.cpp than to the
> > > `checkDataflow()` functions above -- because of the parameter types (the
> > > `checkDataflow()` functions take `AnalysisInputs`, while this this
> > > function doesn't), and also because this function, like the
> > > `runDataflow()` functions, is not a template, while the `checkDataflow()`
> > > functions are.
> > >
> > > Should I ignore these superficial similarities?
> > Ah I think I understand. I'm just trying to make sense of the many
> > overloads we have here.
> >
> > Okay so since this function hardcodes `NoopAnalysis` (unlike
> > `checkDataflow()`) do you think that should be reflected in the name
> > somehow?
> > Ah I think I understand. I'm just trying to make sense of the many
> > overloads we have here.
>
> Yup, it's confusing...
>
> > Okay so since this function hardcodes `NoopAnalysis` (unlike
> > `checkDataflow()`) do you think that should be reflected in the name
> > somehow?
>
> Good point. Maybe this is a path to a good name. How about something like
> `checkDataflowWithNoopAnalysis()`? Still a mouthful, but I think it reflects
> better what this actually does, and it also explains why it's not a template.
SGTM!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D154339/new/
https://reviews.llvm.org/D154339
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
