Skip to content

[clang][dataflow] Fix getResultObjectLocation() on CXXDefaultArgExpr. #85072

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 18, 2024

Conversation

martinboehme
Copy link
Contributor

@martinboehme martinboehme commented Mar 13, 2024

This patch includes a test that causes an assertion failure without the other
changes in this patch.

@martinboehme martinboehme requested a review from ymand March 13, 2024 13:19
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:dataflow Clang Dataflow Analysis framework - https://clang.llvm.org/docs/DataFlowAnalysisIntro.html labels Mar 13, 2024
@llvmbot
Copy link
Member

llvmbot commented Mar 13, 2024

@llvm/pr-subscribers-clang-analysis

@llvm/pr-subscribers-clang

Author: None (martinboehme)

Changes

I'm working on an upcoming change that will improve the semantics of
Environment::GetResultObjectLocation() (see the FIXME in the method comment),
and this patch improves the test coverage for this change.


Full diff: https://github.com/llvm/llvm-project/pull/85072.diff

1 Files Affected:

  • (modified) clang/unittests/Analysis/FlowSensitive/TransferTest.cpp (+30)
diff --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
index a8c282f140b4cd..86c7f32f0104be 100644
--- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -2924,6 +2924,36 @@ TEST(TransferTest, ResultObjectLocation) {
       });
 }
 
+TEST(TransferTest, ResultObjectLocationForDefaultArgExpr) {
+  std::string Code = R"(
+    struct S {};
+    void funcWithDefaultArg(S s = S());
+    void target() {
+      funcWithDefaultArg();
+      // [[p]]
+    }
+  )";
+
+  using ast_matchers::cxxDefaultArgExpr;
+  using ast_matchers::match;
+  using ast_matchers::selectFirst;
+  runDataflow(
+      Code,
+      [](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
+         ASTContext &ASTCtx) {
+        const Environment &Env = getEnvironmentAtAnnotation(Results, "p");
+
+        auto *DefaultArg = selectFirst<CXXDefaultArgExpr>(
+            "default_arg",
+            match(cxxDefaultArgExpr().bind("default_arg"), ASTCtx));
+        ASSERT_NE(DefaultArg, nullptr);
+
+        // The values for default arguments aren't modeled; we merely verify
+        // that we can get a result object location for a default arg.
+        Env.getResultObjectLocation(*DefaultArg);
+      });
+}
+
 TEST(TransferTest, ResultObjectLocationForDefaultInitExpr) {
   std::string Code = R"(
     struct S {};

@martinboehme
Copy link
Contributor Author

The newly added test is failing, but apparently only on Windows. Will need to take a closer look at why this is, and will update this PR when I have something new.

@martinboehme
Copy link
Contributor Author

The new test now does actually also fail for me locally. Not sure why I didn't notice this before. Will add a fix.

@martinboehme martinboehme force-pushed the piper_export_cl_615390226 branch from a9193ba to 7c61dc4 Compare March 14, 2024 08:46
@martinboehme martinboehme changed the title [clang][dataflow] Add a test for result object location on CXXDefaultArgExpr. [clang][dataflow] Fix getResultObjectLocation() on CXXDefaultArgExpr. Mar 14, 2024
@martinboehme
Copy link
Contributor Author

New commit pushed with fix. I have changed the title and description of the PR accordingly.

@martinboehme martinboehme requested a review from Xazax-hun March 14, 2024 08:47
…pr`.

This patch includes a test that causes an assertion failure without the other
changes in this patch.
@martinboehme martinboehme force-pushed the piper_export_cl_615390226 branch from 7c61dc4 to dfa4da9 Compare March 18, 2024 08:29
@martinboehme martinboehme merged commit 27d5049 into llvm:main Mar 18, 2024
chencha3 pushed a commit to chencha3/llvm-project that referenced this pull request Mar 23, 2024
…pr`. (llvm#85072)

This patch includes a test that causes an assertion failure without the
other
changes in this patch.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:analysis clang:dataflow Clang Dataflow Analysis framework - https://clang.llvm.org/docs/DataFlowAnalysisIntro.html clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants