Skip to content

Commit 6c09325

Browse files
committed
C/C++ Overlay: Preserve entities that have at least one location in an unchanged file
Previously, an entity would be discarded if it had any location in a changed file. This caused issues for entities with multiple declaration entries, such as extern variables declared in one file and defined in another. For example, given: // a.c (changed) // b.c (unchanged) extern int x; int x; The variable `x` should be preserved because it has a location in the unchanged file b.c, even though it also has a location in the changed file a.c.
1 parent 39136f3 commit 6c09325

File tree

1 file changed

+40
-16
lines changed

1 file changed

+40
-16
lines changed

cpp/ql/lib/semmle/code/cpp/internal/Overlay.qll

Lines changed: 40 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -16,25 +16,49 @@ private string getLocationFilePath(@location_default loc) {
1616
exists(@file file | locations_default(loc, file, _, _, _, _) | files(file, result))
1717
}
1818

19-
/**
20-
* Gets the file path for an element in the base variant.
21-
*/
2219
overlay[local]
23-
private string getElementPathInBase(@element e) {
24-
not isOverlay() and
25-
exists(@location_default loc |
26-
// Direct location (declarations)
27-
var_decls(e, _, _, _, loc)
28-
or
29-
// Indirect location (entities)
30-
exists(@var_decl vd | var_decls(vd, e, _, _, _) | var_decls(vd, _, _, _, loc))
31-
|
32-
result = getLocationFilePath(loc)
33-
)
20+
private class DiscardableEntityBase extends @element {
21+
/** Gets the path to the file in which this element occurs. */
22+
abstract string getFilePath();
23+
24+
/** Holds if this element exists in the base variant. */
25+
predicate existsInBase() { not isOverlay() }
26+
27+
/** Gets a textual representation of this discardable element. */
28+
string toString() { none() }
3429
}
3530

3631
/**
37-
* Discard any element from the base that is in a changed file.
32+
* Discard an entity from the base if all its locations are in changed files.
33+
* Entities with at least one location in an unchanged file are kept.
3834
*/
3935
overlay[discard_entity]
40-
private predicate discardElement(@element e) { overlayChangedFiles(getElementPathInBase(e)) }
36+
private predicate discardEntity(@element e) {
37+
e =
38+
any(DiscardableEntityBase de |
39+
de.existsInBase() and
40+
overlayChangedFiles(de.getFilePath()) and
41+
// Only discard if ALL file paths are in changed files
42+
forall(string path | path = de.getFilePath() | overlayChangedFiles(path))
43+
)
44+
}
45+
46+
/** A discardable variable declaration entry. */
47+
overlay[local]
48+
private class DiscardableVarDecl extends DiscardableEntityBase instanceof @var_decl {
49+
override string getFilePath() {
50+
exists(@location_default loc | var_decls(this, _, _, _, loc) |
51+
result = getLocationFilePath(loc)
52+
)
53+
}
54+
}
55+
56+
/** A discardable variable. */
57+
overlay[local]
58+
private class DiscardableVariable extends DiscardableEntityBase instanceof @variable {
59+
override string getFilePath() {
60+
exists(@var_decl vd, @location_default loc | var_decls(vd, this, _, _, loc) |
61+
result = getLocationFilePath(loc)
62+
)
63+
}
64+
}

0 commit comments

Comments
 (0)