C#: Loop unrolling for foreach statements#1549
Conversation
812f77d to
aa069fe
Compare
5759ffd to
ab09056
Compare
1047c0a to
706090d
Compare
706090d to
6752557
Compare
calumgrant
left a comment
There was a problem hiding this comment.
I really love this idea. It will definitely help make some queries more precise and reduce false positives.
| abstract predicate isSingleton(); | ||
|
|
||
| /** | ||
| * Holds if this value describes a referential property. For example, emptiness |
There was a problem hiding this comment.
I'm struggling with the term "referential property". I understand the concept but the name makes no sense to me.
There was a problem hiding this comment.
Could it be a "transient property" or "readonly property" instead?
There was a problem hiding this comment.
What I mean is that it is a property of the thing that an expression references, not the value of the expression itself. Is "reference property" better?
There was a problem hiding this comment.
Isn't that just "property"? How about abstract property? I don't really know.
| def1.getEnclosingCallable() != def2.getEnclosingCallable() | ||
| ) | ||
| } | ||
| SimpleLocalScopeVariable() { not getAnAssigningCallable(this) != getAnAssigningCallable(this) } |
There was a problem hiding this comment.
Don't change this here, but words like "Simple" don't convey enough information. Perhaps "UncapturedLocalScopeVariable" etc.
Certainly, that should just make things simpler (less splitting). I guess I got so focused on the "loop unrolling" terminology, that I forgot that all we need to unroll is the loop condition. |
|
Updated dist-compare report: https://git.semmle.com/gist/tom/e8904f2fc636c0ddeebe4d777203e8f4#file-report2-md. |
calumgrant
left a comment
There was a problem hiding this comment.
LGTM. Optionally, use implies if you think it's clearer.

This PR adds loop unrolling for
foreachstatements. For example, the CFG foris now

The QL code for loop unrolling is made general enough that we can later add loop unrolling for other types of loop constructs (for example
forloops).The dist-compare report is here (internal link), and I recommend commit-by-commit review.