C++: Fix IR edge case where there are no function calls taking an argument

[jketema]

No description provided.

[Copilot]

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 hasPositionalArgIndex in IRCppLanguage.qll to 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 {

[Copilot]

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.

Suggested change

	exists(Cpp::DeleteExpr d) and
	argIndex = 0
	exists(Cpp::DeleteExpr d | exists(d.getChild(argIndex)))

[MathiasVP]

Changes LGTM if DCA is happy. I'll let someone from the C team formally approve it 🙂

[tausbn]

I am by no means an expert in this stuff, but the tests and changes look good to me. My only question is whether there are other parts that (like delete) are represented as function calls, and that should therefore also be added to hasPositionalArgIndex.

[jketema]

My only question is whether there are other parts that (like delete) are represented as function calls, and that should therefore also be added to hasPositionalArgIndex.

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?

[MathiasVP]

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:

1. I searched for Opcode::Call in our repo (which is how we generate CallInstructions. This gave me 1 hit: The one in the TranslatedCall abstract class.
2. I then searched for any class that (transitively) extend TranslatedCall. This gives all the obvious subclasses in TranslatedCall (which works directly on Calls), and the TranslatedDeleteOrDeleteArrayExpr case that we're fixing in this PR.

So since 2. gave me no other results I'm pretty sure the answer is no 🤞