C++: Fix IR edge case where there are no function calls taking an argument #19493
Conversation
There was a problem hiding this comment.
Pull Request Overview
Adds handling for an IR edge case where no function calls take arguments but a delete expression is present; includes tests and updates the printing logic to accommodate delete expressions.
- Introduces a new test (
test.cpp) covering a function with only a delete expression and a call without args. - Adds configurations and expected outputs for both IR and AST printing.
- Extends
hasPositionalArgIndexinIRCppLanguage.qllto treat delete expressions as taking a single argument.
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| cpp/ql/test/library-tests/ir/no-function-calls/test.cpp | New C++ test for function with only delete and no-arg calls |
| cpp/ql/test/library-tests/ir/no-function-calls/aliased_ir.ql | IR graph import and custom PrintConfig for aliased IR output |
| cpp/ql/test/library-tests/ir/no-function-calls/aliased_ir.expected | Expected IR output for the new test |
| cpp/ql/test/library-tests/ir/no-function-calls/PrintConfig.qll | Predicates for filtering declarations based on source location |
| cpp/ql/test/library-tests/ir/no-function-calls/PrintAST.ql | AST graph import and custom PrintConfig for AST output |
| cpp/ql/test/library-tests/ir/no-function-calls/PrintAST.expected | Expected AST output for the new test |
| cpp/ql/lib/semmle/code/cpp/ir/internal/IRCppLanguage.qll | Extended hasPositionalArgIndex to include delete expressions |
Comments suppressed due to low confidence (1)
cpp/ql/test/library-tests/ir/no-function-calls/PrintAST.ql:9
- [nitpick] This class name duplicates the IR printing configuration. Rename to
PrintAstConfig(or similar) to avoid confusion between IR and AST configurations.
private class PrintConfig extends PrintAstConfiguration {
| exists(Cpp::DeleteExpr d) and | ||
| argIndex = 0 |
There was a problem hiding this comment.
The clause makes hasPositionalArgIndex(0) true for any delete expression in the module rather than when the delete’s argument is at the target index. Consider scoping this to the specific delete node, e.g., exists(Cpp::DeleteExpr d | exists(d.getChild(argIndex))) to ensure correctness.
| exists(Cpp::DeleteExpr d) and | |
| argIndex = 0 | |
| exists(Cpp::DeleteExpr d | exists(d.getChild(argIndex))) |
Copilot uses AI. Check for mistakes.
That's a good question. I don't think there are any other cases where we're synthesizing a function call with at least one argument, while there's no actual call in the database. What do you think @MathiasVP? |
I'm not aware of any such cases, no. I'm also fairly sure the answer is 'no' based on the following rather ad-hoc investigation:
So since 2. gave me no other results I'm pretty sure the answer is no 🤞 |
No description provided.