Skip to content
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

Java: Improve concatenation in loops #16859

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

yoff
Copy link
Contributor

@yoff yoff commented Jun 27, 2024

This is the results so far of a series of mob pairing sessions to improve the query finding concatenation in loops.

One thing I wanted to illustrate is the inline tests, but currently I had to include the query code in the test to make it work. Thus, this PR is not quite ready to merge.

- create test
- check for statements being executed often
- add test for recursive function
- update test expectations
- Use a deep callgraph traversal to identify loops. This should be checked for performance issues and possibly be restriceted/optimised.
- Add inline expectation test. Currently dupicating the query code. It should be refactored so that the code can be reused.
@github-actions github-actions bot added the Java label Jun 27, 2024
Comment on lines +11 to +24
/**
* An assignment of the form
*
* ```
* v = ... + ... v ...
* ```
* or
*
* ```
* v += ...
* ```
* where `v` is a simple variable (and not, for example,
* an array element).
*/

Check warning

Code scanning / CodeQL

Predicate QLDoc style. Warning

The QLDoc for a predicate without a result should start with 'Holds'.
exists(ControlFlowNode n | a.getControlFlowNode() = n | getADeepSuccessor(n) = n)
}

/** Gets a sucessor of `n`, also following function calls. */

Check warning

Code scanning / CodeQL

Misspelling Warning

This comment contains the common misspelling 'sucessor', which should instead be 'successor'.
exists(ControlFlowNode n | a.getControlFlowNode() = n | getADeepSuccessor(n) = n)
}

/** Gets a sucessor of `n`, also following function calls. */

Check warning

Code scanning / CodeQL

Misspelling Warning

This comment contains the common misspelling 'sucessor', which should instead be 'successor'.
}

predicate queryResult(Assignment a) {
exists(Variable v |

Check warning

Code scanning / CodeQL

Omittable 'exists' variable Warning

This exists variable can be omitted by using a don't-care expression
in this argument
.
/**
* Holds if `e` is executed often in loop `loop`.
*/
predicate executedOften(Assignment a) {

Check warning

Code scanning / CodeQL

Missing QLDoc for parameter Warning

The QLDoc has no documentation for a, but the QLDoc mentions loop
/**
* Holds if `e` is executed often in loop `loop`.
*/
predicate executedOften(Assignment a) {

Check warning

Code scanning / CodeQL

Missing QLDoc for parameter Warning

The QLDoc has no documentation for a, but the QLDoc mentions loop

module TheQuery {
import semmle.code.java.Type
import semmle.code.java.Expr

Check warning

Code scanning / CodeQL

Redundant import Warning

Redundant import, the module is already imported inside
semmle.code.java.Statement
.
import semmle.code.java.Type
import semmle.code.java.Expr
import semmle.code.java.Statement
import semmle.code.java.JDK

Check warning

Code scanning / CodeQL

Redundant import Warning

Redundant import, the module is already imported inside
semmle.code.java.Type
.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant