mboehme 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,
----------------
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.
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
