Author: Dmitri Gribenko Date: 2023-01-21T01:28:03+01:00 New Revision: c8b31da1ef0a3f2a0ba5c39bb4281b1438e511fb
URL: https://github.com/llvm/llvm-project/commit/c8b31da1ef0a3f2a0ba5c39bb4281b1438e511fb DIFF: https://github.com/llvm/llvm-project/commit/c8b31da1ef0a3f2a0ba5c39bb4281b1438e511fb.diff LOG: [clang][dataflow] Allow analyzing multiple functions in unit tests In unit tests for concrete dataflow analyses we typically use the testonly `checkDataflow()` helper to analyse a free function called "target". This pattern allows our tests to be uniform and focused on specific statement- or expression-level C++ features. As we expand our feature coverage, we want to analyze functions whose names we don't fully control, like constructors, destructors, operators etc. In such tests it is often convenient to analyze all functions defined in the input code, to avoid having to carefully craft an AST matcher that finds the exact function we're interested in. That can be easily done by providing `checkDataflow()` with a catch-all matcher like `functionDecl()`. It is also often convenient to define multiple special member functions in a single unit test, for example, multiple constructors, and share the rest of the class definition code between constructors. As a result, it makes sense to analyze multiple functions in one unit test. This change allows `checkDataflow()` to correctly handle AST matchers that match more than one function. Previously, it would only ever analyze the first matched function, and silently ignore the rest. Now it runs dataflow analysis in a loop, and calls `VerifyResults` for each function that was found in the input and analyzed. Reviewed By: ymandel, sgatev Differential Revision: https://reviews.llvm.org/D140859 Added: Modified: clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp clang/unittests/Analysis/FlowSensitive/TestingSupport.h clang/unittests/Analysis/FlowSensitive/TestingSupportTest.cpp Removed: ################################################################################ diff --git a/clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp b/clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp index 8541ac336b6e..0d441b71e69c 100644 --- a/clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp +++ b/clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp @@ -6,6 +6,7 @@ #include "clang/ASTMatchers/ASTMatchers.h" #include "clang/Basic/LLVM.h" #include "clang/Basic/LangOptions.h" +#include "clang/Basic/SourceLocation.h" #include "clang/Basic/SourceManager.h" #include "clang/Basic/TokenKinds.h" #include "clang/Lex/Lexer.h" @@ -45,18 +46,23 @@ isAnnotationDirectlyAfterStatement(const Stmt *Stmt, unsigned AnnotationBegin, return true; } -llvm::DenseMap<unsigned, std::string> -test::buildLineToAnnotationMapping(SourceManager &SM, - llvm::Annotations AnnotatedCode) { +llvm::DenseMap<unsigned, std::string> test::buildLineToAnnotationMapping( + const SourceManager &SM, const LangOptions &LangOpts, + SourceRange BoundingRange, llvm::Annotations AnnotatedCode) { + CharSourceRange CharBoundingRange = + Lexer::getAsCharRange(BoundingRange, SM, LangOpts); + llvm::DenseMap<unsigned, std::string> LineNumberToContent; auto Code = AnnotatedCode.code(); auto Annotations = AnnotatedCode.ranges(); for (auto &AnnotationRange : Annotations) { - auto LineNumber = - SM.getPresumedLineNumber(SM.getLocForStartOfFile(SM.getMainFileID()) - .getLocWithOffset(AnnotationRange.Begin)); - auto Content = Code.slice(AnnotationRange.Begin, AnnotationRange.End).str(); - LineNumberToContent[LineNumber] = Content; + SourceLocation Loc = SM.getLocForStartOfFile(SM.getMainFileID()) + .getLocWithOffset(AnnotationRange.Begin); + if (SM.isPointWithin(Loc, CharBoundingRange.getBegin(), + CharBoundingRange.getEnd())) { + LineNumberToContent[SM.getPresumedLineNumber(Loc)] = + Code.slice(AnnotationRange.Begin, AnnotationRange.End).str(); + } } return LineNumberToContent; } @@ -83,11 +89,18 @@ test::buildStatementToAnnotationMapping(const FunctionDecl *Func, Stmts[Offset] = S; } - unsigned I = 0; - auto Annotations = AnnotatedCode.ranges(); + unsigned FunctionBeginOffset = + SourceManager.getFileOffset(Func->getBeginLoc()); + unsigned FunctionEndOffset = SourceManager.getFileOffset(Func->getEndLoc()); + + std::vector<llvm::Annotations::Range> Annotations = AnnotatedCode.ranges(); + llvm::erase_if(Annotations, [=](llvm::Annotations::Range R) { + return R.Begin < FunctionBeginOffset || R.End >= FunctionEndOffset; + }); std::reverse(Annotations.begin(), Annotations.end()); auto Code = AnnotatedCode.code(); + unsigned I = 0; for (auto OffsetAndStmt = Stmts.rbegin(); OffsetAndStmt != Stmts.rend(); OffsetAndStmt++) { unsigned Offset = OffsetAndStmt->first; diff --git a/clang/unittests/Analysis/FlowSensitive/TestingSupport.h b/clang/unittests/Analysis/FlowSensitive/TestingSupport.h index c3243d5e222f..b40bcbf76ad0 100644 --- a/clang/unittests/Analysis/FlowSensitive/TestingSupport.h +++ b/clang/unittests/Analysis/FlowSensitive/TestingSupport.h @@ -144,7 +144,7 @@ template <typename AnalysisT> struct AnalysisInputs { /// Required. Input code that is analyzed. llvm::StringRef Code; - /// Required. The body of the function which matches this matcher is analyzed. + /// Required. All functions that match this matcher are analyzed. ast_matchers::internal::Matcher<FunctionDecl> TargetFuncMatcher; /// Required. The analysis to be run is constructed with this function that /// takes as argument the AST generated from the code being analyzed and the @@ -176,15 +176,16 @@ llvm::Expected<llvm::DenseMap<const Stmt *, std::string>> buildStatementToAnnotationMapping(const FunctionDecl *Func, llvm::Annotations AnnotatedCode); -/// Returns line numbers and content of the annotations in `AnnotatedCode`. -llvm::DenseMap<unsigned, std::string> -buildLineToAnnotationMapping(SourceManager &SM, - llvm::Annotations AnnotatedCode); +/// Returns line numbers and content of the annotations in `AnnotatedCode` +/// within the token range `BoundingRange`. +llvm::DenseMap<unsigned, std::string> buildLineToAnnotationMapping( + const SourceManager &SM, const LangOptions &LangOpts, + SourceRange BoundingRange, llvm::Annotations AnnotatedCode); -/// Runs dataflow specified from `AI.MakeAnalysis` and `AI.PostVisitCFG` on the -/// body of the function that matches `AI.TargetFuncMatcher` in `AI.Code`. -/// Given the analysis outputs, `VerifyResults` checks that the results from the -/// analysis are correct. +/// Runs dataflow specified from `AI.MakeAnalysis` and `AI.PostVisitCFG` on all +/// functions that match `AI.TargetFuncMatcher` in `AI.Code`. Given the +/// analysis outputs, `VerifyResults` checks that the results from the analysis +/// are correct. /// /// Requirements: /// @@ -212,36 +213,13 @@ checkDataflow(AnalysisInputs<AnalysisT> AI, "they were printed to the test log"); } - // Get AST node of target function. - const FunctionDecl *Target = ast_matchers::selectFirst<FunctionDecl>( - "target", ast_matchers::match( - ast_matchers::functionDecl(ast_matchers::isDefinition(), - AI.TargetFuncMatcher) - .bind("target"), - Context)); - if (Target == nullptr) - return llvm::make_error<llvm::StringError>( - llvm::errc::invalid_argument, "Could not find target function."); - - // Build control flow graph from body of target function. - auto MaybeCFCtx = - ControlFlowContext::build(Target, *Target->getBody(), Context); - if (!MaybeCFCtx) - return MaybeCFCtx.takeError(); - auto &CFCtx = *MaybeCFCtx; - - // Initialize states for running dataflow analysis. - DataflowAnalysisContext DACtx(std::make_unique<WatchedLiteralsSolver>(), - {/*Opts=*/AI.BuiltinOptions}); - Environment InitEnv(DACtx, *Target); - auto Analysis = AI.MakeAnalysis(Context, InitEnv); std::function<void(const CFGElement &, const TypeErasedDataflowAnalysisState &)> - PostVisitCFGClosure = nullptr; + TypeErasedPostVisitCFG = nullptr; if (AI.PostVisitCFG) { - PostVisitCFGClosure = [&AI, &Context]( - const CFGElement &Element, - const TypeErasedDataflowAnalysisState &State) { + TypeErasedPostVisitCFG = [&AI, &Context]( + const CFGElement &Element, + const TypeErasedDataflowAnalysisState &State) { AI.PostVisitCFG(Context, Element, TransferStateForDiagnostics<typename AnalysisT::Lattice>( llvm::any_cast<const typename AnalysisT::Lattice &>( @@ -250,32 +228,57 @@ checkDataflow(AnalysisInputs<AnalysisT> AI, }; } - // Additional test setup. - AnalysisOutputs AO{AnnotatedCode, Context, Target, CFCtx, - Analysis, InitEnv, {}}; - if (AI.SetupTest) { - if (auto Error = AI.SetupTest(AO)) - return Error; + for (const ast_matchers::BoundNodes &BN : + ast_matchers::match(ast_matchers::functionDecl( + ast_matchers::hasBody(ast_matchers::stmt()), + AI.TargetFuncMatcher) + .bind("target"), + Context)) { + // Get the AST node of the target function. + const FunctionDecl *Target = BN.getNodeAs<FunctionDecl>("target"); + if (Target == nullptr) + return llvm::make_error<llvm::StringError>( + llvm::errc::invalid_argument, "Could not find the target function."); + + // Build the control flow graph for the target function. + auto MaybeCFCtx = + ControlFlowContext::build(Target, *Target->getBody(), Context); + if (!MaybeCFCtx) return MaybeCFCtx.takeError(); + auto &CFCtx = *MaybeCFCtx; + + // Initialize states for running dataflow analysis. + DataflowAnalysisContext DACtx(std::make_unique<WatchedLiteralsSolver>(), + {/*Opts=*/AI.BuiltinOptions}); + Environment InitEnv(DACtx, *Target); + auto Analysis = AI.MakeAnalysis(Context, InitEnv); + + AnalysisOutputs AO{AnnotatedCode, Context, Target, CFCtx, + Analysis, InitEnv, {}}; + + // Additional test setup. + if (AI.SetupTest) { + if (auto Error = AI.SetupTest(AO)) return Error; + } + + // If successful, the dataflow analysis returns a mapping from block IDs to + // the post-analysis states for the CFG blocks that have been evaluated. + llvm::Expected<std::vector<llvm::Optional<TypeErasedDataflowAnalysisState>>> + MaybeBlockStates = runTypeErasedDataflowAnalysis( + CFCtx, Analysis, InitEnv, TypeErasedPostVisitCFG); + if (!MaybeBlockStates) return MaybeBlockStates.takeError(); + AO.BlockStates = *MaybeBlockStates; + + // Verify dataflow analysis outputs. + VerifyResults(AO); } - // If successful, the dataflow analysis returns a mapping from block IDs to - // the post-analysis states for the CFG blocks that have been evaluated. - llvm::Expected<std::vector<std::optional<TypeErasedDataflowAnalysisState>>> - MaybeBlockStates = runTypeErasedDataflowAnalysis(CFCtx, Analysis, InitEnv, - PostVisitCFGClosure); - if (!MaybeBlockStates) - return MaybeBlockStates.takeError(); - AO.BlockStates = *MaybeBlockStates; - - // Verify dataflow analysis outputs. - VerifyResults(AO); return llvm::Error::success(); } -/// Runs dataflow specified from `AI.MakeAnalysis` and `AI.PostVisitCFG` on the -/// body of the function that matches `AI.TargetFuncMatcher` in `AI.Code`. Given -/// the annotation line numbers and analysis outputs, `VerifyResults` checks -/// that the results from the analysis are correct. +/// Runs dataflow specified from `AI.MakeAnalysis` and `AI.PostVisitCFG` on all +/// functions that match `AI.TargetFuncMatcher` in `AI.Code`. Given the +/// annotation line numbers and analysis outputs, `VerifyResults` checks that +/// the results from the analysis are correct. /// /// Requirements: /// @@ -292,16 +295,17 @@ checkDataflow(AnalysisInputs<AnalysisT> AI, VerifyResults) { return checkDataflow<AnalysisT>( std::move(AI), [&VerifyResults](const AnalysisOutputs &AO) { - auto AnnotationLinesAndContent = - buildLineToAnnotationMapping(AO.ASTCtx.getSourceManager(), AO.Code); + auto AnnotationLinesAndContent = buildLineToAnnotationMapping( + AO.ASTCtx.getSourceManager(), AO.ASTCtx.getLangOpts(), + AO.Target->getSourceRange(), AO.Code); VerifyResults(AnnotationLinesAndContent, AO); }); } -/// Runs dataflow specified from `AI.MakeAnalysis` and `AI.PostVisitCFG` on the -/// body of the function that matches `AI.TargetFuncMatcher` in `AI.Code`. Given -/// the state computed at each annotated statement and analysis outputs, -/// `VerifyResults` checks that the results from the analysis are correct. +/// Runs dataflow specified from `AI.MakeAnalysis` and `AI.PostVisitCFG` on all +/// functions that match `AI.TargetFuncMatcher` in `AI.Code`. Given the state +/// computed at each annotated statement and analysis outputs, `VerifyResults` +/// checks that the results from the analysis are correct. /// /// Requirements: /// @@ -371,6 +375,10 @@ checkDataflow(AnalysisInputs<AnalysisT> AI, .withPostVisitCFG(std::move(PostVisitCFG)), [&VerifyResults, &AnnotationStates](const AnalysisOutputs &AO) { VerifyResults(AnnotationStates, AO); + + // `checkDataflow()` can analyze more than one function. Reset the + // variables to prepare for analyzing the next function. + AnnotationStates.clear(); }); } diff --git a/clang/unittests/Analysis/FlowSensitive/TestingSupportTest.cpp b/clang/unittests/Analysis/FlowSensitive/TestingSupportTest.cpp index c00e376d30a7..937ab039f72f 100644 --- a/clang/unittests/Analysis/FlowSensitive/TestingSupportTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/TestingSupportTest.cpp @@ -15,6 +15,7 @@ using namespace dataflow; namespace { using ::clang::ast_matchers::functionDecl; +using ::clang::ast_matchers::hasAnyName; using ::clang::ast_matchers::hasName; using ::clang::ast_matchers::isDefinition; using ::clang::dataflow::test::AnalysisInputs; @@ -74,21 +75,21 @@ TEST(BuildStatementToAnnotationMappingTest, ReturnStmt) { } void checkDataflow( - llvm::StringRef Code, llvm::StringRef Target, + llvm::StringRef Code, + ast_matchers::internal::Matcher<FunctionDecl> TargetFuncMatcher, std::function< void(const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &, const AnalysisOutputs &)> Expectations) { ASSERT_THAT_ERROR(checkDataflow<NoopAnalysis>( AnalysisInputs<NoopAnalysis>( - Code, hasName(Target), + Code, std::move(TargetFuncMatcher), [](ASTContext &Context, Environment &) { return NoopAnalysis( Context, /*ApplyBuiltinTransfer=*/false); }) .withASTBuildArgs({"-fsyntax-only", "-std=c++17"}), - /*VerifyResults=*/ - std::move(Expectations)), + /*VerifyResults=*/std::move(Expectations)), llvm::Succeeded()); } @@ -100,7 +101,8 @@ TEST(ProgramPointAnnotations, NoAnnotations) { EXPECT_CALL(Expectations, Call(IsEmpty(), _)).Times(1); - checkDataflow("void target() {}", "target", Expectations.AsStdFunction()); + checkDataflow("void target() {}", hasName("target"), + Expectations.AsStdFunction()); } TEST(ProgramPointAnnotations, NoAnnotationsDifferentTarget) { @@ -111,10 +113,11 @@ TEST(ProgramPointAnnotations, NoAnnotationsDifferentTarget) { EXPECT_CALL(Expectations, Call(IsEmpty(), _)).Times(1); - checkDataflow("void fun() {}", "fun", Expectations.AsStdFunction()); + checkDataflow("void target() {}", hasName("target"), + Expectations.AsStdFunction()); } -TEST(ProgramPointAnnotations, WithCodepoint) { +TEST(ProgramPointAnnotations, WithProgramPoint) { ::testing::MockFunction<void( const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &, const AnalysisOutputs &)> @@ -126,13 +129,13 @@ TEST(ProgramPointAnnotations, WithCodepoint) { .Times(1); checkDataflow(R"cc(void target() { - int n; - // [[program-point]] - })cc", - "target", Expectations.AsStdFunction()); + int n; + // [[program-point]] + })cc", + hasName("target"), Expectations.AsStdFunction()); } -TEST(ProgramPointAnnotations, MultipleCodepoints) { +TEST(ProgramPointAnnotations, MultipleProgramPoints) { ::testing::MockFunction<void( const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &, const AnalysisOutputs &)> @@ -145,15 +148,59 @@ TEST(ProgramPointAnnotations, MultipleCodepoints) { .Times(1); checkDataflow(R"cc(void target(bool b) { - if (b) { - int n; - // [[program-point-1]] - } else { - int m; - // [[program-point-2]] - } - })cc", - "target", Expectations.AsStdFunction()); + if (b) { + int n; + // [[program-point-1]] + } else { + int m; + // [[program-point-2]] + } + })cc", + hasName("target"), Expectations.AsStdFunction()); +} + +TEST(ProgramPointAnnotations, MultipleFunctionsMultipleProgramPoints) { + ::testing::MockFunction<void( + const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &, + const AnalysisOutputs &)> + Expectations; + + EXPECT_CALL(Expectations, Call(UnorderedElementsAre( + IsStringMapEntry("program-point-1a", _), + IsStringMapEntry("program-point-1b", _)), + _)) + .Times(1); + + EXPECT_CALL(Expectations, Call(UnorderedElementsAre( + IsStringMapEntry("program-point-2a", _), + IsStringMapEntry("program-point-2b", _)), + _)) + .Times(1); + + checkDataflow( + R"cc( + void target1(bool b) { + if (b) { + int n; + // [[program-point-1a]] + } else { + int m; + // [[program-point-1b]] + } + } + + void target2(bool b) { + if (b) { + int n; + // [[program-point-2a]] + } else { + int m; + // [[program-point-2b]] + } + } + )cc", + functionDecl(hasAnyName("target1", "target2")), + Expectations.AsStdFunction()); } } // namespace _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
