Skip to content

Commit f22d60f

Browse files
committed
Swift: clean up VarDecl, NamedPattern and SwitchStmt interactions
* `variables` under `CaseStmt` are now AST children, which solves orphan `VarDecl`s in that case * reordered `CaseStmt` AST children to be `labels > variables > body` (was `body > labels`) * made `NamedPattern::getVarDecl` an extracted property instead of `getName` * The above led to duplicate DB entities because of a quirk in the Swift compiler code. This is solved by tweaking the extraction of `variables` under `CaseStmt` to not use `getCaseBodyVariables`.
1 parent 140ff72 commit f22d60f

36 files changed

+602
-327
lines changed

swift/extractor/translators/PatternTranslator.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ namespace codeql {
44

55
codeql::NamedPattern PatternTranslator::translateNamedPattern(const swift::NamedPattern& pattern) {
66
auto entry = dispatcher.createEntry(pattern);
7-
entry.name = pattern.getNameStr().str();
7+
entry.var_decl = dispatcher.fetchLabel(pattern.getDecl());
88
return entry;
99
}
1010

swift/extractor/translators/StmtTranslator.cpp

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ codeql::ForEachStmt StmtTranslator::translateForEachStmt(const swift::ForEachStm
7373
auto entry = dispatcher.createEntry(stmt);
7474
fillLabeledStmt(stmt, entry);
7575
entry.body = dispatcher.fetchLabel(stmt.getBody());
76-
// entry.sequence = dispatcher.fetchLabel(stmt.getTypeCheckedSequence());
76+
// entry.sequence = dispatcher.fetchLabel(stmt.getTypeCheckedSequence());
7777
entry.pattern = dispatcher.fetchLabel(stmt.getPattern());
7878
entry.iteratorVar = dispatcher.fetchLabel(stmt.getIteratorVar());
7979
entry.where = dispatcher.fetchOptionalLabel(stmt.getWhere());
@@ -133,12 +133,17 @@ codeql::DoCatchStmt StmtTranslator::translateDoCatchStmt(const swift::DoCatchStm
133133

134134
codeql::CaseStmt StmtTranslator::translateCaseStmt(const swift::CaseStmt& stmt) {
135135
auto entry = dispatcher.createEntry(stmt);
136+
auto labels = stmt.getCaseLabelItems();
136137
entry.body = dispatcher.fetchLabel(stmt.getBody());
137-
entry.labels = dispatcher.fetchRepeatedLabels(stmt.getCaseLabelItems());
138-
if (stmt.hasCaseBodyVariables()) {
139-
for (auto var : stmt.getCaseBodyVariables()) {
140-
entry.variables.push_back(dispatcher.fetchLabel(var));
141-
}
138+
entry.labels = dispatcher.fetchRepeatedLabels(labels);
139+
// we don't use stmt.getCaseBodyVariables() because it's actually filled with copies of the
140+
// variable declarations, which would lead to duplicate `DeclVar` entities.
141+
// Instead we follow the same logic that's used to fill getCaseBodyVariables, looking at the first
142+
// pattern and collecting variables from it. See
143+
// https://github.com/apple/swift/blob/71fff6649b3ce57cc22954f141cf8b567be6de88/lib/Parse/ParseStmt.cpp#L2210
144+
if (!labels.empty() && labels.front().getPattern()) {
145+
labels.front().getPattern()->forEachVariable(
146+
[&](const swift::VarDecl* var) { entry.variables.push_back(dispatcher.fetchLabel(var)); });
142147
}
143148
return entry;
144149
}

swift/ql/.generated.list

Lines changed: 12 additions & 7 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

swift/ql/.gitattributes

Lines changed: 8 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

swift/ql/lib/codeql/swift/elements/pattern/NamedPattern.qll

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,19 +9,16 @@ private import codeql.swift.elements.decl.VarDecl
99
*/
1010
class NamedPattern extends Generated::NamedPattern {
1111
/**
12-
* Holds if this named pattern has a corresponding `VarDecl`.
13-
* This will be the case as long as the variable is subsequently used.
12+
* Holds if this named pattern has a corresponding `VarDecl`, which is currently always true.
13+
*
14+
* DEPRECATED: unless there was a compilation error, this will always hold.
1415
*/
15-
predicate hasVarDecl() { exists(this.getVarDecl()) }
16+
deprecated predicate hasVarDecl() { exists(this.getVarDecl()) }
1617

1718
/**
18-
* Gets the `VarDecl` bound by this named pattern, if any.
19-
* This will be the case as long as the variable is subsequently used.
19+
* Gets the name of the variable bound by this pattern.
2020
*/
21-
VarDecl getVarDecl() {
22-
this.getImmediateEnclosingPattern*() = result.getImmediateParentPattern() and
23-
pragma[only_bind_out](result.getName()) = pragma[only_bind_out](this.getName())
24-
}
21+
string getName() { result = this.getVarDecl().getName() }
2522

2623
override string toString() { result = this.getName() }
2724
}

swift/ql/lib/codeql/swift/generated/ParentChild.qll

Lines changed: 10 additions & 6 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

swift/ql/lib/codeql/swift/generated/Raw.qll

Lines changed: 7 additions & 7 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

swift/ql/lib/codeql/swift/generated/pattern/NamedPattern.qll

Lines changed: 7 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

swift/ql/lib/codeql/swift/generated/stmt/CaseStmt.qll

Lines changed: 8 additions & 8 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

swift/ql/lib/swift.dbscheme

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)