Skip to content

Commit ac6715f

Browse files
paldepindpaldepind
committed
Rust: Avoid mixing up type parameters and associated types in path resolution
1 parent 71a5e41 commit ac6715f

File tree

5 files changed

+79
-69
lines changed

5 files changed

+79
-69
lines changed

‎rust/ql/lib/codeql/rust/internal/PathResolution.qll

Lines changed: 66 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -112,13 +112,18 @@ abstract class ItemNode extends Locatable {
112112
result = this.(SourceFileItemNode).getSuper()
113113
}
114114

115+
pragma[nomagic]
116+
private ItemNode getAChildSuccessor(string name) {
117+
this = result.getImmediateParent() and
118+
name = result.getName()
119+
}
120+
115121
cached
116122
ItemNode getASuccessorRec(string name) {
117123
Stages::PathResolutionStage::ref() and
118124
sourceFileEdge(this, name, result)
119125
or
120-
this = result.getImmediateParent() and
121-
name = result.getName()
126+
result = this.getAChildSuccessor(name)
122127
or
123128
fileImportEdge(this, name, result)
124129
or
@@ -224,6 +229,38 @@ abstract class ItemNode extends Locatable {
224229
result.(CrateItemNode).isPotentialDollarCrateTarget()
225230
}
226231

232+
/**
233+
* Holds if the successor `item` with the name `name` is not available locally
234+
* for unqualified paths.
235+
*
236+
* This has the effect that a path of the form `name` inside `this` will not
237+
* resolve to `item`.
238+
*/
239+
pragma[nomagic]
240+
predicate excludedLocally(string name, ItemNode item) {
241+
// Associated items in an impl or trait block are not directly available
242+
// inside the block, they require a qualified path with a `Self` prefix.
243+
item = this.getAChildSuccessor(name) and
244+
this instanceof ImplOrTraitItemNode and
245+
item instanceof AssocItemNode
246+
}
247+
248+
/**
249+
* Holds if the successor `item` with the name `name` is not available
250+
* externally for qualified paths that resolve to this item.
251+
*
252+
* This has the effect that a path of the form `Qualifier::name`, where
253+
* `Qualifier` resolves to this item, will not resolve to `item`.
254+
*/
255+
pragma[nomagic]
256+
predicate excludedExternally(string name, ItemNode item) {
257+
// Type parameters for an `impl` or trait block are not available outside of
258+
// the block.
259+
item = this.getAChildSuccessor(name) and
260+
this instanceof ImplOrTraitItemNode and
261+
item instanceof TypeParamItemNode
262+
}
263+
227264
pragma[nomagic]
228265
private predicate hasSourceFunction(string name) {
229266
this.getASuccessorFull(name).(Function).fromSource()
@@ -1145,7 +1182,9 @@ pragma[nomagic]
11451182
private predicate declares(ItemNode item, Namespace ns, string name) {
11461183
exists(ItemNode child | child.getImmediateParent() = item |
11471184
child.getName() = name and
1148-
child.getNamespace() = ns
1185+
child.getNamespace() = ns and
1186+
// If `item` is excluded locally then it does not declare `name`.
1187+
not item.excludedLocally(name, child)
11491188
or
11501189
useTreeDeclares(child.(Use).getUseTree(), name) and
11511190
exists(ns) // `use foo::bar` can refer to both a value and a type
@@ -1193,38 +1232,27 @@ private ItemNode getOuterScope(ItemNode i) {
11931232
result = i.getImmediateParent()
11941233
}
11951234

1196-
pragma[nomagic]
1197-
private ItemNode getAdjustedEnclosing(ItemNode encl0, Namespace ns) {
1198-
// functions in `impl` blocks need to use explicit `Self::` to access other
1199-
// functions in the `impl` block
1200-
if encl0 instanceof ImplOrTraitItemNode and ns.isValue()
1201-
then result = encl0.getImmediateParent()
1202-
else result = encl0
1203-
}
1204-
12051235
/**
12061236
* Holds if the unqualified path `p` references an item named `name`, and `name`
12071237
* may be looked up in the `ns` namespace inside enclosing item `encl`.
12081238
*/
12091239
pragma[nomagic]
12101240
private predicate unqualifiedPathLookup(ItemNode encl, string name, Namespace ns, RelevantPath p) {
1211-
exists(ItemNode encl0 | encl = getAdjustedEnclosing(encl0, ns) |
1212-
// lookup in the immediately enclosing item
1213-
p.isUnqualified(name) and
1214-
encl0.getADescendant() = p and
1215-
exists(ns) and
1216-
not name = ["crate", "$crate", "super", "self"]
1217-
or
1218-
// lookup in an outer scope, but only if the item is not declared in inner scope
1219-
exists(ItemNode mid |
1220-
unqualifiedPathLookup(mid, name, ns, p) and
1221-
not declares(mid, ns, name) and
1222-
not (
1223-
name = "Self" and
1224-
mid = any(ImplOrTraitItemNode i).getAnItemInSelfScope()
1225-
) and
1226-
encl0 = getOuterScope(mid)
1227-
)
1241+
// lookup in the immediately enclosing item
1242+
p.isUnqualified(name) and
1243+
encl.getADescendant() = p and
1244+
exists(ns) and
1245+
not name = ["crate", "$crate", "super", "self"]
1246+
or
1247+
// lookup in an outer scope, but only if the item is not declared in inner scope
1248+
exists(ItemNode mid |
1249+
unqualifiedPathLookup(mid, name, ns, p) and
1250+
not declares(mid, ns, name) and
1251+
not (
1252+
name = "Self" and
1253+
mid = any(ImplOrTraitItemNode i).getAnItemInSelfScope()
1254+
) and
1255+
encl = getOuterScope(mid)
12281256
)
12291257
}
12301258

@@ -1245,10 +1273,10 @@ private predicate sourceFileHasCratePathTc(ItemNode i1, ItemNode i2) =
12451273

12461274
/**
12471275
* Holds if the unqualified path `p` references a keyword item named `name`, and
1248-
* `name` may be looked up in the `ns` namespace inside enclosing item `encl`.
1276+
* `name` may be looked up inside enclosing item `encl`.
12491277
*/
12501278
pragma[nomagic]
1251-
private predicate keywordLookup(ItemNode encl, string name, Namespace ns, RelevantPath p) {
1279+
private predicate keywordLookup(ItemNode encl, string name, RelevantPath p) {
12521280
// For `($)crate`, jump directly to the root module
12531281
exists(ItemNode i | p.isCratePath(name, i) |
12541282
encl instanceof SourceFile and
@@ -1259,18 +1287,17 @@ private predicate keywordLookup(ItemNode encl, string name, Namespace ns, Releva
12591287
or
12601288
name = ["super", "self"] and
12611289
p.isUnqualified(name) and
1262-
exists(ItemNode encl0 |
1263-
encl0.getADescendant() = p and
1264-
encl = getAdjustedEnclosing(encl0, ns)
1265-
)
1290+
encl.getADescendant() = p
12661291
}
12671292

12681293
pragma[nomagic]
12691294
private ItemNode unqualifiedPathLookup(RelevantPath p, Namespace ns) {
1270-
exists(ItemNode encl, string name | result = getASuccessorFull(encl, name, ns) |
1295+
exists(ItemNode encl, string name |
1296+
result = getASuccessorFull(encl, name, ns) and not encl.excludedLocally(name, result)
1297+
|
12711298
unqualifiedPathLookup(encl, name, ns, p)
12721299
or
1273-
keywordLookup(encl, name, ns, p)
1300+
keywordLookup(encl, name, p) and exists(ns)
12741301
)
12751302
}
12761303

@@ -1291,7 +1318,8 @@ private ItemNode resolvePath0(RelevantPath path, Namespace ns) {
12911318
or
12921319
exists(ItemNode q, string name |
12931320
q = resolvePathQualifier(path, name) and
1294-
result = getASuccessorFull(q, name, ns)
1321+
result = getASuccessorFull(q, name, ns) and
1322+
not q.excludedExternally(name, result)
12951323
)
12961324
or
12971325
result = resolveUseTreeListItem(_, _, path) and

‎rust/ql/test/library-tests/path-resolution/main.rs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -661,29 +661,29 @@ mod associated_types {
661661
Error, // IError
662662
> Reduce // $ item=IReduce
663663
for MyImpl<
664-
Input, // $ item=IInput SPURIOUS: item=IInputAssociated
665-
Error, // $ item=IError SPURIOUS: item=IErrorAssociated
664+
Input, // $ item=IInput
665+
Error, // $ item=IError
666666
> // $ item=MyImpl
667667
{
668668
type Input = Result<
669-
Input, // $ item=IInput SPURIOUS: item=IInputAssociated
670-
Self::Error, // $ item=IErrorAssociated SPURIOUS: item=IError
669+
Input, // $ item=IInput
670+
Self::Error, // $ item=IErrorAssociated
671671
> // $ item=Result
672672
; // IInputAssociated
673673
type Error = Option<
674-
Error // $ item=IError SPURIOUS: item=IErrorAssociated
674+
Error // $ item=IError
675675
> // $ item=Option
676676
; // IErrorAssociated
677677
type Output =
678-
Input // $ item=IInput SPURIOUS: item=IInputAssociated
678+
Input // $ item=IInput
679679
; // IOutputAssociated
680680

681681
fn feed(
682682
&mut self,
683-
item: Self::Input // $ item=IInputAssociated SPURIOUS: item=IInput
683+
item: Self::Input // $ item=IInputAssociated
684684
) -> Result<
685685
Self::Output, // $ item=IOutputAssociated
686-
Self::Error // $ item=IErrorAssociated SPURIOUS: item=IError
686+
Self::Error // $ item=IErrorAssociated
687687
> { // $ item=Result
688688
item
689689
}

‎rust/ql/test/library-tests/path-resolution/path-resolution.expected

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -297,28 +297,20 @@ resolvePath
297297
| main.rs:662:11:662:16 | Reduce | main.rs:643:5:651:5 | trait Reduce |
298298
| main.rs:663:13:666:9 | MyImpl::<...> | main.rs:653:5:656:5 | struct MyImpl |
299299
| main.rs:664:13:664:17 | Input | main.rs:660:13:660:17 | Input |
300-
| main.rs:664:13:664:17 | Input | main.rs:668:9:672:9 | type Input |
301300
| main.rs:665:13:665:17 | Error | main.rs:661:13:661:17 | Error |
302-
| main.rs:665:13:665:17 | Error | main.rs:672:11:676:9 | type Error |
303301
| main.rs:668:22:671:9 | Result::<...> | {EXTERNAL LOCATION} | enum Result |
304302
| main.rs:669:13:669:17 | Input | main.rs:660:13:660:17 | Input |
305-
| main.rs:669:13:669:17 | Input | main.rs:668:9:672:9 | type Input |
306303
| main.rs:670:13:670:16 | Self | main.rs:658:5:690:5 | impl Reduce for MyImpl::<...> { ... } |
307-
| main.rs:670:13:670:23 | ...::Error | main.rs:661:13:661:17 | Error |
308304
| main.rs:670:13:670:23 | ...::Error | main.rs:672:11:676:9 | type Error |
309305
| main.rs:673:22:675:9 | Option::<...> | {EXTERNAL LOCATION} | enum Option |
310306
| main.rs:674:11:674:15 | Error | main.rs:661:13:661:17 | Error |
311-
| main.rs:674:11:674:15 | Error | main.rs:672:11:676:9 | type Error |
312307
| main.rs:678:13:678:17 | Input | main.rs:660:13:660:17 | Input |
313-
| main.rs:678:13:678:17 | Input | main.rs:668:9:672:9 | type Input |
314308
| main.rs:683:19:683:22 | Self | main.rs:658:5:690:5 | impl Reduce for MyImpl::<...> { ... } |
315-
| main.rs:683:19:683:29 | ...::Input | main.rs:660:13:660:17 | Input |
316309
| main.rs:683:19:683:29 | ...::Input | main.rs:668:9:672:9 | type Input |
317310
| main.rs:684:14:687:9 | Result::<...> | {EXTERNAL LOCATION} | enum Result |
318311
| main.rs:685:13:685:16 | Self | main.rs:658:5:690:5 | impl Reduce for MyImpl::<...> { ... } |
319312
| main.rs:685:13:685:24 | ...::Output | main.rs:676:11:679:9 | type Output |
320313
| main.rs:686:13:686:16 | Self | main.rs:658:5:690:5 | impl Reduce for MyImpl::<...> { ... } |
321-
| main.rs:686:13:686:23 | ...::Error | main.rs:661:13:661:17 | Error |
322314
| main.rs:686:13:686:23 | ...::Error | main.rs:672:11:676:9 | type Error |
323315
| main.rs:693:5:693:7 | std | {EXTERNAL LOCATION} | Crate([email protected]) |
324316
| main.rs:693:11:693:14 | self | {EXTERNAL LOCATION} | Crate([email protected]) |

‎rust/ql/test/library-tests/type-inference/main.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2162,7 +2162,7 @@ mod loops {
21622162

21632163
for i in [1, 2, 3] {} // $ type=i:i32
21642164
for i in [1, 2, 3].map(|x| x + 1) {} // $ target=map MISSING: type=i:i32
2165-
for i in [1, 2, 3].into_iter() {} // $ target=into_iter MISSING: type=i:i32
2165+
for i in [1, 2, 3].into_iter() {} // $ target=into_iter type=i:i32
21662166

21672167
let vals1 = [1u8, 2, 3]; // $ type=vals1:[T;...].u8
21682168
for u in vals1 {} // $ type=u:u8

‎rust/ql/test/library-tests/type-inference/type-inference.expected

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3526,8 +3526,12 @@ inferType
35263526
| main.rs:2164:22:2164:22 | 2 | | {EXTERNAL LOCATION} | i32 |
35273527
| main.rs:2164:25:2164:25 | 3 | | {EXTERNAL LOCATION} | i32 |
35283528
| main.rs:2164:40:2164:40 | 1 | | {EXTERNAL LOCATION} | i32 |
3529+
| main.rs:2165:13:2165:13 | i | | {EXTERNAL LOCATION} | Item |
3530+
| main.rs:2165:13:2165:13 | i | | {EXTERNAL LOCATION} | i32 |
35293531
| main.rs:2165:18:2165:26 | [...] | | file://:0:0:0:0 | [] |
35303532
| main.rs:2165:18:2165:26 | [...] | [T;...] | {EXTERNAL LOCATION} | i32 |
3533+
| main.rs:2165:18:2165:38 | ... .into_iter() | | {EXTERNAL LOCATION} | IntoIter |
3534+
| main.rs:2165:18:2165:38 | ... .into_iter() | T | {EXTERNAL LOCATION} | i32 |
35313535
| main.rs:2165:19:2165:19 | 1 | | {EXTERNAL LOCATION} | i32 |
35323536
| main.rs:2165:22:2165:22 | 2 | | {EXTERNAL LOCATION} | i32 |
35333537
| main.rs:2165:25:2165:25 | 3 | | {EXTERNAL LOCATION} | i32 |
@@ -3739,11 +3743,8 @@ inferType
37393743
| main.rs:2226:39:2226:39 | 2 | | {EXTERNAL LOCATION} | u16 |
37403744
| main.rs:2226:42:2226:42 | 3 | | {EXTERNAL LOCATION} | i32 |
37413745
| main.rs:2226:42:2226:42 | 3 | | {EXTERNAL LOCATION} | u16 |
3742-
| main.rs:2227:13:2227:13 | u | | {EXTERNAL LOCATION} | Vec |
37433746
| main.rs:2227:13:2227:13 | u | | {EXTERNAL LOCATION} | u16 |
37443747
| main.rs:2227:13:2227:13 | u | | file://:0:0:0:0 | & |
3745-
| main.rs:2227:13:2227:13 | u | A | {EXTERNAL LOCATION} | Global |
3746-
| main.rs:2227:13:2227:13 | u | T | {EXTERNAL LOCATION} | u16 |
37473748
| main.rs:2227:18:2227:23 | vals4a | | {EXTERNAL LOCATION} | Vec |
37483749
| main.rs:2227:18:2227:23 | vals4a | A | {EXTERNAL LOCATION} | Global |
37493750
| main.rs:2227:18:2227:23 | vals4a | T | {EXTERNAL LOCATION} | u16 |
@@ -3773,13 +3774,9 @@ inferType
37733774
| main.rs:2232:38:2232:38 | 2 | | {EXTERNAL LOCATION} | u32 |
37743775
| main.rs:2232:41:2232:41 | 3 | | {EXTERNAL LOCATION} | i32 |
37753776
| main.rs:2232:41:2232:41 | 3 | | {EXTERNAL LOCATION} | u32 |
3776-
| main.rs:2233:13:2233:13 | u | | {EXTERNAL LOCATION} | Vec |
37773777
| main.rs:2233:13:2233:13 | u | | {EXTERNAL LOCATION} | i32 |
37783778
| main.rs:2233:13:2233:13 | u | | {EXTERNAL LOCATION} | u32 |
37793779
| main.rs:2233:13:2233:13 | u | | file://:0:0:0:0 | & |
3780-
| main.rs:2233:13:2233:13 | u | A | {EXTERNAL LOCATION} | Global |
3781-
| main.rs:2233:13:2233:13 | u | T | {EXTERNAL LOCATION} | i32 |
3782-
| main.rs:2233:13:2233:13 | u | T | {EXTERNAL LOCATION} | u32 |
37833780
| main.rs:2233:18:2233:22 | vals5 | | {EXTERNAL LOCATION} | Vec |
37843781
| main.rs:2233:18:2233:22 | vals5 | A | {EXTERNAL LOCATION} | Global |
37853782
| main.rs:2233:18:2233:22 | vals5 | T | {EXTERNAL LOCATION} | i32 |
@@ -3801,12 +3798,8 @@ inferType
38013798
| main.rs:2235:39:2235:39 | 2 | | {EXTERNAL LOCATION} | u64 |
38023799
| main.rs:2235:42:2235:42 | 3 | | {EXTERNAL LOCATION} | i32 |
38033800
| main.rs:2235:42:2235:42 | 3 | | {EXTERNAL LOCATION} | u64 |
3804-
| main.rs:2236:13:2236:13 | u | | {EXTERNAL LOCATION} | Vec |
38053801
| main.rs:2236:13:2236:13 | u | | file://:0:0:0:0 | & |
38063802
| main.rs:2236:13:2236:13 | u | &T | {EXTERNAL LOCATION} | u64 |
3807-
| main.rs:2236:13:2236:13 | u | A | {EXTERNAL LOCATION} | Global |
3808-
| main.rs:2236:13:2236:13 | u | T | file://:0:0:0:0 | & |
3809-
| main.rs:2236:13:2236:13 | u | T.&T | {EXTERNAL LOCATION} | u64 |
38103803
| main.rs:2236:18:2236:22 | vals6 | | {EXTERNAL LOCATION} | Vec |
38113804
| main.rs:2236:18:2236:22 | vals6 | A | {EXTERNAL LOCATION} | Global |
38123805
| main.rs:2236:18:2236:22 | vals6 | T | file://:0:0:0:0 | & |
@@ -3821,11 +3814,8 @@ inferType
38213814
| main.rs:2239:9:2239:13 | vals7 | A | {EXTERNAL LOCATION} | Global |
38223815
| main.rs:2239:9:2239:13 | vals7 | T | {EXTERNAL LOCATION} | u8 |
38233816
| main.rs:2239:20:2239:22 | 1u8 | | {EXTERNAL LOCATION} | u8 |
3824-
| main.rs:2240:13:2240:13 | u | | {EXTERNAL LOCATION} | Vec |
38253817
| main.rs:2240:13:2240:13 | u | | {EXTERNAL LOCATION} | u8 |
38263818
| main.rs:2240:13:2240:13 | u | | file://:0:0:0:0 | & |
3827-
| main.rs:2240:13:2240:13 | u | A | {EXTERNAL LOCATION} | Global |
3828-
| main.rs:2240:13:2240:13 | u | T | {EXTERNAL LOCATION} | u8 |
38293819
| main.rs:2240:18:2240:22 | vals7 | | {EXTERNAL LOCATION} | Vec |
38303820
| main.rs:2240:18:2240:22 | vals7 | A | {EXTERNAL LOCATION} | Global |
38313821
| main.rs:2240:18:2240:22 | vals7 | T | {EXTERNAL LOCATION} | u8 |

0 commit comments

Comments
 (0)