Feature/ai for cpp#51
Conversation
git-subtree-dir: agentic/cpp git-subtree-split: e1e5aeb54ff729b22245d09d6d9cdc4e14b0a60d
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 89a2a14804
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if node.get("kind") != "DeclRefExpr": | ||
| continue | ||
| referenced = node.get("referencedDecl") | ||
| if isinstance(referenced, dict) and isinstance(referenced.get("name"), str): | ||
| return referenced["name"] |
There was a problem hiding this comment.
call_expr_callee_name() returns the first DeclRefExpr it sees, which is often the receiver variable for CXXMemberCallExpr (e.g. a in a.run()), not the called function. That builds an incorrect call graph (caller -> a instead of caller -> run), so reverse propagation can miss benchmark functions that are only transitively impacted through member-function calls. This leads to false negatives in affected_benchmarks and can skip relevant perf checks.
Useful? React with 👍 / 👎.
| if callee == "register_op": | ||
| literal_values = string_literals_in_node(subnode) |
There was a problem hiding this comment.
Dynamic benchmark names are only captured when the callee equals register_op, but this codebase registers them via RegisterOp/benchmark::RegisterBenchmark (e.g. in src/benchmarks/rmm_benchmark_base.h). As written, those benchmark names are never added to benchmark_names/affected_names, so changed RmM benchmark code can be reported as having no affected benchmark targets (filtered out later when names are empty), producing false negatives.
Useful? React with 👍 / 👎.
| for match in re.finditer(r"register_op\(\s*\"([^\"]+)\"", text): | ||
| names.add(match.group(1)) |
There was a problem hiding this comment.
The source fallback extractor only recognizes BENCHMARK(...) and lowercase register_op(...), so it misses dynamic registrations done through RegisterOp/benchmark::RegisterBenchmark. In fallback paths (AST errors or infra fallback), this can leave affected_benchmarks empty for dynamically-registered suites, degrading scope selection and causing conservative or incorrect benchmark targeting.
Useful? React with 👍 / 👎.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #51 +/- ##
=======================================
Coverage 89.58% 89.58%
=======================================
Files 15 15
Lines 4416 4416
Branches 814 814
=======================================
Hits 3956 3956
Misses 280 280
Partials 180 180
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
1 participant
No description provided.