Skip to content

Commit 9bba764

Browse files
paldepindCopilot
paldepind
and
Copilot
committed
Unified: Remove indexed access to expanded class members
When constructing parameters of synthesized constructors always assume that the `then` branch of conditional compilation blocks. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: a9225dea-8342-448b-88ec-ba9587d93816
1 parent 03ab718 commit 9bba764

3 files changed

Lines changed: 37 additions & 47 deletions

File tree

‎unified/ql/lib/codeql/unified/internal/AstPluginSwift.qll‎

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,26 @@ module;
44
private import unified
55
private import AstPlugin
66

7+
private Stmt assumeThenCompilationBranches(Stmt stmt) {
8+
// For now we simply assume that the `then` branch is always taken for
9+
// conditional compilation blocks. This means that a parameter index results
10+
// in a unique parameter type.
11+
if stmt.(IfExpr).isConditionalCompilation()
12+
then result = assumeThenCompilationBranches(stmt.(IfExpr).getThen().(Block).getAStmt())
13+
else result = stmt
14+
}
15+
16+
/**
17+
* Holds if a constructor declaration `cd` of a struct has a parameter for the
18+
* field `decl` with name `name` and default value `hasDefault`.
19+
*/
720
private predicate structHasParameterForField(
8-
ConstructorDeclaration cd, VariableDeclaration decl, int i, string name, boolean hasDefault
21+
ConstructorDeclaration cd, VariableDeclaration decl, string name, boolean hasDefault
922
) {
1023
exists(ClassLikeDeclaration struct |
1124
struct.hasModifier("struct") and
1225
cd = struct.getAMember() and
13-
decl = struct.getMember(i) and
26+
decl = assumeThenCompilationBranches(struct.getADirectMember()) and
1427
not decl.hasModifier("static") and
1528
name = decl.getPattern().(Identifier).getValue()
1629
|
@@ -57,15 +70,25 @@ private class AstPluginSwift extends AstPlugin {
5770
override predicate defaultConstructorParameter(
5871
ConstructorDeclaration cd, int i, string name, boolean hasDefault
5972
) {
60-
name = rank[i](int j, string s | structHasParameterForField(cd, _, j, s, _) | s order by j) and
61-
structHasParameterForField(cd, _, _, name, hasDefault)
73+
name =
74+
rank[i](VariableDeclaration decl, string s |
75+
structHasParameterForField(cd, decl, s, _)
76+
|
77+
// Due to conditional compilation blocks, we cannot simply order
78+
// parameters by their parent index. Instead we order by their location.
79+
s
80+
order by
81+
decl.getLocation().getStartLine(), decl.getLocation().getStartColumn(),
82+
decl.getParentIndex()
83+
) and
84+
structHasParameterForField(cd, _, name, hasDefault)
6285
}
6386

64-
bindingset[cd]
87+
// bindingset[cd]
6588
override predicate defaultConstructorParameterType(ConstructorDeclaration cd, int i, AstNode type) {
6689
exists(VariableDeclaration decl, string name |
6790
this.defaultConstructorParameter(cd, i, name, _) and
68-
structHasParameterForField(cd, decl, _, name, _) and
91+
structHasParameterForField(cd, decl, name, _) and
6992
type = decl.getType()
7093
)
7194
}

‎unified/ql/lib/codeql/unified/internal/FacadeAst.qll‎

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -242,21 +242,6 @@ module Unified {
242242
/** Gets a member, looking through conditional-compilation branches. */
243243
Member getAMember() { result = compilationExpandedStmt(this.getADirectMember()) }
244244

245-
/**
246-
* Gets the `i`th member in source order across all conditional-compilation alternatives,
247-
* indexed from zero.
248-
*/
249-
Member getMember(int i) {
250-
result =
251-
rank[i + 1](Member m |
252-
m = this.getAMember()
253-
|
254-
m
255-
order by
256-
m.getLocation().getStartLine(), m.getLocation().getStartColumn(), m.getParentIndex()
257-
)
258-
}
259-
260245
/** Gets the name of this declaration. */
261246
string getName() { result = this.getNameNode().getValue() }
262247

‎unified/ql/test/library-tests/constructors/test.expected‎

Lines changed: 8 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -38,20 +38,13 @@ defaultConstructorParameter
3838
| constructors.swift:39:1:39:0 | S1.init | 4 | constructors.swift:39:1:39:0 | f7 [init default constructor] | f7 | true |
3939
| constructors.swift:39:1:39:0 | S1.init | 5 | constructors.swift:39:1:39:0 | f8 [init default constructor] | f8 | true |
4040
| constructors.swift:74:1:74:0 | ConditionalFields.init | 1 | constructors.swift:74:1:74:0 | first [init default constructor] | first | false |
41-
| constructors.swift:74:1:74:0 | ConditionalFields.init | 2 | constructors.swift:74:1:74:0 | second [init default constructor] | second | false |
42-
| constructors.swift:74:1:74:0 | ConditionalFields.init | 3 | constructors.swift:74:1:74:0 | third [init default constructor] | third | false |
41+
| constructors.swift:74:1:74:0 | ConditionalFields.init | 2 | constructors.swift:74:1:74:0 | third [init default constructor] | third | false |
4342
| constructors.swift:85:1:85:0 | UnequalConditionalFields.init | 1 | constructors.swift:85:1:85:0 | alpha [init default constructor] | alpha | false |
44-
| constructors.swift:85:1:85:0 | UnequalConditionalFields.init | 2 | constructors.swift:85:1:85:0 | alpha [init default constructor] | alpha | false |
45-
| constructors.swift:85:1:85:0 | UnequalConditionalFields.init | 3 | constructors.swift:85:1:85:0 | beta [init default constructor] | beta | false |
46-
| constructors.swift:85:1:85:0 | UnequalConditionalFields.init | 4 | constructors.swift:85:1:85:0 | gamma [init default constructor] | gamma | false |
47-
| constructors.swift:85:1:85:0 | UnequalConditionalFields.init | 5 | constructors.swift:85:1:85:0 | omega [init default constructor] | omega | false |
43+
| constructors.swift:85:1:85:0 | UnequalConditionalFields.init | 2 | constructors.swift:85:1:85:0 | omega [init default constructor] | omega | false |
4844
| constructors.swift:96:1:96:0 | NestedConditionalFields.init | 1 | constructors.swift:96:1:96:0 | before [init default constructor] | before | false |
4945
| constructors.swift:96:1:96:0 | NestedConditionalFields.init | 2 | constructors.swift:96:1:96:0 | selected [init default constructor] | selected | false |
50-
| constructors.swift:96:1:96:0 | NestedConditionalFields.init | 3 | constructors.swift:96:1:96:0 | selected [init default constructor] | selected | false |
51-
| constructors.swift:96:1:96:0 | NestedConditionalFields.init | 4 | constructors.swift:96:1:96:0 | afterNested [init default constructor] | afterNested | false |
52-
| constructors.swift:96:1:96:0 | NestedConditionalFields.init | 5 | constructors.swift:96:1:96:0 | ignoredElseIf [init default constructor] | ignoredElseIf | false |
53-
| constructors.swift:96:1:96:0 | NestedConditionalFields.init | 6 | constructors.swift:96:1:96:0 | ignoredElse [init default constructor] | ignoredElse | false |
54-
| constructors.swift:96:1:96:0 | NestedConditionalFields.init | 7 | constructors.swift:96:1:96:0 | after [init default constructor] | after | false |
46+
| constructors.swift:96:1:96:0 | NestedConditionalFields.init | 3 | constructors.swift:96:1:96:0 | afterNested [init default constructor] | afterNested | false |
47+
| constructors.swift:96:1:96:0 | NestedConditionalFields.init | 4 | constructors.swift:96:1:96:0 | after [init default constructor] | after | false |
5548
| constructors.swift:113:1:113:0 | OptionalConditionalFields.init | 1 | constructors.swift:113:1:113:0 | first [init default constructor] | first | false |
5649
| constructors.swift:113:1:113:0 | OptionalConditionalFields.init | 2 | constructors.swift:113:1:113:0 | last [init default constructor] | last | false |
5750
defaultConstructorParameterType
@@ -60,23 +53,12 @@ defaultConstructorParameterType
6053
| constructors.swift:39:1:39:0 | S1.init | 4 | constructors.swift:39:1:39:0 | f7 [init default constructor] | f7 | constructors.swift:53:13:53:18 | Double |
6154
| constructors.swift:39:1:39:0 | S1.init | 5 | constructors.swift:39:1:39:0 | f8 [init default constructor] | f8 | constructors.swift:61:13:61:16 | GenericTypeExpr |
6255
| constructors.swift:74:1:74:0 | ConditionalFields.init | 1 | constructors.swift:74:1:74:0 | first [init default constructor] | first | constructors.swift:76:20:76:22 | Int |
63-
| constructors.swift:74:1:74:0 | ConditionalFields.init | 2 | constructors.swift:74:1:74:0 | second [init default constructor] | second | constructors.swift:78:21:78:26 | String |
64-
| constructors.swift:74:1:74:0 | ConditionalFields.init | 3 | constructors.swift:74:1:74:0 | third [init default constructor] | third | constructors.swift:80:16:80:19 | Bool |
56+
| constructors.swift:74:1:74:0 | ConditionalFields.init | 2 | constructors.swift:74:1:74:0 | third [init default constructor] | third | constructors.swift:80:16:80:19 | Bool |
6557
| constructors.swift:85:1:85:0 | UnequalConditionalFields.init | 1 | constructors.swift:85:1:85:0 | alpha [init default constructor] | alpha | constructors.swift:87:20:87:22 | Int |
66-
| constructors.swift:85:1:85:0 | UnequalConditionalFields.init | 1 | constructors.swift:85:1:85:0 | alpha [init default constructor] | alpha | constructors.swift:89:20:89:25 | String |
67-
| constructors.swift:85:1:85:0 | UnequalConditionalFields.init | 2 | constructors.swift:85:1:85:0 | alpha [init default constructor] | alpha | constructors.swift:87:20:87:22 | Int |
68-
| constructors.swift:85:1:85:0 | UnequalConditionalFields.init | 2 | constructors.swift:85:1:85:0 | alpha [init default constructor] | alpha | constructors.swift:89:20:89:25 | String |
69-
| constructors.swift:85:1:85:0 | UnequalConditionalFields.init | 3 | constructors.swift:85:1:85:0 | beta [init default constructor] | beta | constructors.swift:90:19:90:24 | String |
70-
| constructors.swift:85:1:85:0 | UnequalConditionalFields.init | 4 | constructors.swift:85:1:85:0 | gamma [init default constructor] | gamma | constructors.swift:91:20:91:25 | String |
71-
| constructors.swift:85:1:85:0 | UnequalConditionalFields.init | 5 | constructors.swift:85:1:85:0 | omega [init default constructor] | omega | constructors.swift:93:16:93:19 | Bool |
58+
| constructors.swift:85:1:85:0 | UnequalConditionalFields.init | 2 | constructors.swift:85:1:85:0 | omega [init default constructor] | omega | constructors.swift:93:16:93:19 | Bool |
7259
| constructors.swift:96:1:96:0 | NestedConditionalFields.init | 1 | constructors.swift:96:1:96:0 | before [init default constructor] | before | constructors.swift:97:17:97:19 | Int |
7360
| constructors.swift:96:1:96:0 | NestedConditionalFields.init | 2 | constructors.swift:96:1:96:0 | selected [init default constructor] | selected | constructors.swift:100:27:100:32 | String |
74-
| constructors.swift:96:1:96:0 | NestedConditionalFields.init | 2 | constructors.swift:96:1:96:0 | selected [init default constructor] | selected | constructors.swift:102:27:102:32 | Double |
75-
| constructors.swift:96:1:96:0 | NestedConditionalFields.init | 3 | constructors.swift:96:1:96:0 | selected [init default constructor] | selected | constructors.swift:100:27:100:32 | String |
76-
| constructors.swift:96:1:96:0 | NestedConditionalFields.init | 3 | constructors.swift:96:1:96:0 | selected [init default constructor] | selected | constructors.swift:102:27:102:32 | Double |
77-
| constructors.swift:96:1:96:0 | NestedConditionalFields.init | 4 | constructors.swift:96:1:96:0 | afterNested [init default constructor] | afterNested | constructors.swift:104:26:104:29 | Bool |
78-
| constructors.swift:96:1:96:0 | NestedConditionalFields.init | 5 | constructors.swift:96:1:96:0 | ignoredElseIf [init default constructor] | ignoredElseIf | constructors.swift:106:28:106:32 | Float |
79-
| constructors.swift:96:1:96:0 | NestedConditionalFields.init | 6 | constructors.swift:96:1:96:0 | ignoredElse [init default constructor] | ignoredElse | constructors.swift:108:26:108:31 | Double |
80-
| constructors.swift:96:1:96:0 | NestedConditionalFields.init | 7 | constructors.swift:96:1:96:0 | after [init default constructor] | after | constructors.swift:110:16:110:19 | Bool |
61+
| constructors.swift:96:1:96:0 | NestedConditionalFields.init | 3 | constructors.swift:96:1:96:0 | afterNested [init default constructor] | afterNested | constructors.swift:104:26:104:29 | Bool |
62+
| constructors.swift:96:1:96:0 | NestedConditionalFields.init | 4 | constructors.swift:96:1:96:0 | after [init default constructor] | after | constructors.swift:110:16:110:19 | Bool |
8163
| constructors.swift:113:1:113:0 | OptionalConditionalFields.init | 1 | constructors.swift:113:1:113:0 | first [init default constructor] | first | constructors.swift:115:20:115:22 | Int |
8264
| constructors.swift:113:1:113:0 | OptionalConditionalFields.init | 2 | constructors.swift:113:1:113:0 | last [init default constructor] | last | constructors.swift:117:15:117:18 | Bool |

0 commit comments

Comments
 (0)