Skip to content

Commit 551e7ad

Browse files
hvitvedhvitved
committed
Rust: Refine Self resolution inside impl blocks
1 parent 17c3ee1 commit 551e7ad

File tree

11 files changed

+81
-68
lines changed

11 files changed

+81
-68
lines changed

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

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -264,6 +264,9 @@ abstract class ItemNode extends Locatable {
264264
pragma[nomagic]
265265
ItemNode getImmediateParent() { this = result.getADescendant() }
266266

267+
/** Gets a child item of this item, if any. */
268+
ItemNode getAChild() { this = result.getImmediateParent() }
269+
267270
/** Gets the immediately enclosing module (or source file) of this item. */
268271
pragma[nomagic]
269272
ModuleLikeNode getImmediateParentModule() {
@@ -339,10 +342,13 @@ abstract class ItemNode extends Locatable {
339342
typeImplEdge(this, _, name, kind, result, useOpt)
340343
or
341344
// trait items with default implementations made available in an implementation
342-
exists(ImplItemNodeImpl impl, ItemNode trait |
345+
exists(ImplItemNodeImpl impl, TraitItemNode trait |
343346
this = impl and
344347
trait = impl.resolveTraitTyCand() and
345348
result = trait.getASuccessor(name, kind, useOpt) and
349+
// do not inherit default implementations from super traits; those are inherited by
350+
// their `impl` blocks
351+
result = trait.getAssocItem(name) and
346352
result.(AssocItemNode).hasImplementation() and
347353
kind.isExternalOrBoth() and
348354
not impl.hasAssocItem(name)
@@ -402,8 +408,14 @@ abstract class ItemNode extends Locatable {
402408
this instanceof SourceFile and
403409
builtin(name, result)
404410
or
405-
name = "Self" and
406-
this = result.(ImplOrTraitItemNode).getAnItemInSelfScope()
411+
exists(ImplOrTraitItemNode i |
412+
name = "Self" and
413+
this = i.getAnItemInSelfScope()
414+
|
415+
result = i.(Trait)
416+
or
417+
result = i.(ImplItemNodeImpl).resolveSelfTyCand()
418+
)
407419
or
408420
name = "crate" and
409421
this = result.(CrateItemNode).getASourceFile()
@@ -734,7 +746,7 @@ abstract class ImplOrTraitItemNode extends ItemNode {
734746
Path getASelfPath() {
735747
Stages::PathResolutionStage::ref() and
736748
isUnqualifiedSelfPath(result) and
737-
this = unqualifiedPathLookup(result, _, _)
749+
result = this.getAnItemInSelfScope().getADescendant()
738750
}
739751

740752
/** Gets an associated item belonging to this trait or `impl` block. */
@@ -960,7 +972,7 @@ private class ImplItemNodeImpl extends ImplItemNode {
960972
result = this.resolveSelfTyBuiltin()
961973
}
962974

963-
TraitItemNode resolveTraitTyCand() { result = resolvePathCand(this.getTraitPath()) }
975+
TraitItemNodeImpl resolveTraitTyCand() { result = resolvePathCand(this.getTraitPath()) }
964976
}
965977

966978
private class StructItemNode extends TypeItemNode, ParameterizableItemNode instanceof Struct {
@@ -1813,15 +1825,7 @@ private module DollarCrateResolution {
18131825

18141826
pragma[nomagic]
18151827
private ItemNode resolvePathCand0(PathExt path, Namespace ns) {
1816-
exists(ItemNode res |
1817-
res = unqualifiedPathLookup(path, ns, _) and
1818-
if
1819-
not any(PathExt parent).getQualifier() = path and
1820-
isUnqualifiedSelfPath(path) and
1821-
res instanceof ImplItemNode
1822-
then result = res.(ImplItemNodeImpl).resolveSelfTyCand()
1823-
else result = res
1824-
)
1828+
result = unqualifiedPathLookup(path, ns, _)
18251829
or
18261830
DollarCrateResolution::resolveDollarCrate(path, result) and
18271831
ns = result.getNamespace()

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -382,6 +382,8 @@ TypeParamTypeParameter getPtrTypeParameter() {
382382
/** A type parameter. */
383383
abstract class TypeParameter extends Type {
384384
override TypeParameter getPositionalTypeParameter(int i) { none() }
385+
386+
abstract ItemNode getDeclaringItem();
385387
}
386388

387389
private class RawTypeParameter = @type_param or @trait or @type_alias or @impl_trait_type_repr;
@@ -400,6 +402,8 @@ class TypeParamTypeParameter extends TypeParameter, TTypeParamTypeParameter {
400402

401403
TypeParam getTypeParam() { result = typeParam }
402404

405+
override ItemNode getDeclaringItem() { result.getTypeParam(_) = typeParam }
406+
403407
override string toString() { result = typeParam.toString() }
404408

405409
override Location getLocation() { result = typeParam.getLocation() }
@@ -433,6 +437,8 @@ class AssociatedTypeTypeParameter extends TypeParameter, TAssociatedTypeTypePara
433437
/** Gets the trait that contains this associated type declaration. */
434438
TraitItemNode getTrait() { result.getAnAssocItem() = typeAlias }
435439

440+
override ItemNode getDeclaringItem() { result = this.getTrait() }
441+
436442
override string toString() { result = typeAlias.getName().getText() }
437443

438444
override Location getLocation() { result = typeAlias.getLocation() }
@@ -465,6 +471,8 @@ class DynTraitTypeParameter extends TypeParameter, TDynTraitTypeParameter {
465471
result = [this.getTypeParam().toString(), this.getTypeAlias().getName().toString()]
466472
}
467473

474+
override ItemNode getDeclaringItem() { none() }
475+
468476
override string toString() { result = "dyn(" + this.toStringInner() + ")" }
469477

470478
override Location getLocation() { result = n.getLocation() }
@@ -480,6 +488,8 @@ class ImplTraitTypeParameter extends TypeParameter, TImplTraitTypeParameter {
480488

481489
ImplTraitTypeRepr getImplTraitTypeRepr() { result = implTrait }
482490

491+
override ItemNode getDeclaringItem() { none() }
492+
483493
override string toString() { result = "impl(" + typeParam.toString() + ")" }
484494

485495
override Location getLocation() { result = typeParam.getLocation() }
@@ -499,6 +509,8 @@ class SelfTypeParameter extends TypeParameter, TSelfTypeParameter {
499509

500510
Trait getTrait() { result = trait }
501511

512+
override ItemNode getDeclaringItem() { result = trait }
513+
502514
override string toString() { result = "Self [" + trait.toString() + "]" }
503515

504516
override Location getLocation() { result = trait.getLocation() }
@@ -526,6 +538,8 @@ class ImplTraitTypeTypeParameter extends ImplTraitType, TypeParameter {
526538

527539
ImplTraitTypeTypeParameter() { impl = function.getAParam().getTypeRepr() }
528540

541+
override ItemNode getDeclaringItem() { none() }
542+
529543
override Function getFunction() { result = function }
530544

531545
override TypeParameter getPositionalTypeParameter(int i) { none() }

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,14 @@ import TypeInference::Consistency
1111

1212
query predicate illFormedTypeMention(TypeMention tm) {
1313
Consistency::illFormedTypeMention(tm) and
14-
not tm instanceof PathTypeReprMention and // avoid overlap with `PathTypeMention`
14+
// avoid overlap with `PathTypeMention`
15+
not tm instanceof PathTypeReprMention and
16+
// known limitation for type mentions that would mention an escaping type parameter
17+
not tm =
18+
any(PathTypeMention ptm |
19+
exists(ptm.resolvePathTypeAt(TypePath::nil())) and
20+
not exists(ptm.resolveType())
21+
) and
1522
// Only include inconsistencies in the source, as we otherwise get
1623
// inconsistencies from library code in every project.
1724
tm.fromSource()

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

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,19 @@ class SliceTypeReprMention extends TypeMention instanceof SliceTypeRepr {
7777
}
7878
}
7979

80-
abstract class PathTypeMention extends TypeMention, Path { }
80+
abstract class PathTypeMention extends TypeMention, Path {
81+
abstract Type resolvePathTypeAt(TypePath typePath);
82+
83+
final override Type resolveTypeAt(TypePath typePath) {
84+
result = this.resolvePathTypeAt(typePath) and
85+
(
86+
not result instanceof TypeParameter
87+
or
88+
// Prevent type parameters from escaping their scope
89+
this = result.(TypeParameter).getDeclaringItem().getAChild*().getADescendant()
90+
)
91+
}
92+
}
8193

8294
class AliasPathTypeMention extends PathTypeMention {
8395
TypeAlias resolved;
@@ -94,7 +106,7 @@ class AliasPathTypeMention extends PathTypeMention {
94106
* Holds if this path resolved to a type alias with a rhs. that has the
95107
* resulting type at `typePath`.
96108
*/
97-
override Type resolveTypeAt(TypePath typePath) {
109+
override Type resolvePathTypeAt(TypePath typePath) {
98110
result = rhs.resolveTypeAt(typePath) and
99111
not result = pathGetTypeParameter(resolved, _)
100112
or
@@ -275,7 +287,7 @@ class NonAliasPathTypeMention extends PathTypeMention {
275287
result = TAssociatedTypeTypeParameter(resolved)
276288
}
277289

278-
override Type resolveTypeAt(TypePath typePath) {
290+
override Type resolvePathTypeAt(TypePath typePath) {
279291
typePath.isEmpty() and
280292
result = this.resolveRootType()
281293
or
@@ -307,7 +319,9 @@ class ImplSelfMention extends PathTypeMention {
307319

308320
ImplSelfMention() { this = impl.getASelfPath() }
309321

310-
override Type resolveTypeAt(TypePath typePath) { result = resolveImplSelfTypeAt(impl, typePath) }
322+
override Type resolvePathTypeAt(TypePath typePath) {
323+
result = resolveImplSelfTypeAt(impl, typePath)
324+
}
311325
}
312326

313327
class PathTypeReprMention extends TypeMention, PathTypeRepr {

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,4 +4,3 @@ multipleCallTargets
44
| main.rs:369:9:371:16 | ...::f(...) |
55
| main.rs:450:9:454:16 | ...::f(...) |
66
| main.rs:455:9:459:16 | ...::f(...) |
7-
| main.rs:460:9:460:16 | ...::g(...) |

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -437,9 +437,9 @@ mod m16 {
437437
> // $ item=I89
438438
for S { // $ item=I90
439439
fn f(&self) -> S { // $ item=I90
440-
Self::g(&self); // $ MISSING: item=I92 $ SPURIOUS: item=I85
440+
Self::g(&self); // $ item=I92
441441
println!("m16::<S as Trait2<S>>::f"); // $ item=println
442-
Self::c // $ MISSING: item=I95
442+
Self::c // $ item=I95
443443
} // I93
444444
}
445445

@@ -457,7 +457,7 @@ mod m16 {
457457
S // $ item=I90
458458
> // $ item=I89
459459
>::f(&x); // $ MISSING: item=I93
460-
S::g(&x); // $ item=I92 $ SPURIOUS: item=I85
460+
S::g(&x); // $ item=I92
461461
x.g(); // $ item=I92
462462
S::h(&x); // $ item=I96
463463
x.h(); // $ item=I96
@@ -485,7 +485,7 @@ mod m16 {
485485

486486
impl Trait4 for S2 { // $ item=Trait4 item=S2
487487
fn g(&self) {
488-
Self::f(&self); // $ MISSING: item=S2asTrait3::f
488+
Self::f(&self); // $ item=S2asTrait3::f
489489
S2::f(&self); // $ item=S2asTrait3::f
490490
}
491491
}

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

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ resolvePath
133133
| main.rs:169:22:169:29 | MyStruct | main.rs:162:5:162:22 | struct MyStruct |
134134
| main.rs:171:13:171:19 | println | {EXTERNAL LOCATION} | MacroRules |
135135
| main.rs:172:13:172:13 | f | main.rs:164:5:166:5 | fn f |
136-
| main.rs:173:13:173:16 | Self | main.rs:168:5:179:5 | impl MyTrait for MyStruct { ... } |
136+
| main.rs:173:13:173:16 | Self | main.rs:162:5:162:22 | struct MyStruct |
137137
| main.rs:173:13:173:19 | ...::g | main.rs:176:9:178:9 | fn g |
138138
| main.rs:177:13:177:19 | println | {EXTERNAL LOCATION} | MacroRules |
139139
| main.rs:182:10:182:17 | MyStruct | main.rs:162:5:162:22 | struct MyStruct |
@@ -197,7 +197,7 @@ resolvePath
197197
| main.rs:341:10:341:15 | Trait1 | main.rs:307:5:311:5 | trait Trait1 |
198198
| main.rs:342:11:342:11 | S | main.rs:338:5:338:13 | struct S |
199199
| main.rs:344:13:344:19 | println | {EXTERNAL LOCATION} | MacroRules |
200-
| main.rs:345:13:345:16 | Self | main.rs:340:5:352:5 | impl Trait1 for S { ... } |
200+
| main.rs:345:13:345:16 | Self | main.rs:338:5:338:13 | struct S |
201201
| main.rs:345:13:345:19 | ...::g | main.rs:349:9:351:9 | fn g |
202202
| main.rs:350:13:350:19 | println | {EXTERNAL LOCATION} | MacroRules |
203203
| main.rs:355:10:355:15 | Trait2 | main.rs:313:5:321:5 | trait Trait2 |
@@ -230,22 +230,23 @@ resolvePath
230230
| main.rs:418:11:418:11 | S | main.rs:412:5:412:13 | struct S |
231231
| main.rs:419:24:419:24 | S | main.rs:412:5:412:13 | struct S |
232232
| main.rs:420:13:420:19 | println | {EXTERNAL LOCATION} | MacroRules |
233-
| main.rs:421:13:421:16 | Self | main.rs:414:5:432:5 | impl Trait1::<...> for S { ... } |
233+
| main.rs:421:13:421:16 | Self | main.rs:412:5:412:13 | struct S |
234234
| main.rs:421:13:421:19 | ...::g | main.rs:425:9:428:9 | fn g |
235235
| main.rs:425:24:425:24 | S | main.rs:412:5:412:13 | struct S |
236236
| main.rs:426:13:426:19 | println | {EXTERNAL LOCATION} | MacroRules |
237-
| main.rs:427:13:427:16 | Self | main.rs:414:5:432:5 | impl Trait1::<...> for S { ... } |
237+
| main.rs:427:13:427:16 | Self | main.rs:412:5:412:13 | struct S |
238238
| main.rs:427:13:427:19 | ...::c | main.rs:430:9:431:9 | Const |
239239
| main.rs:430:18:430:18 | S | main.rs:412:5:412:13 | struct S |
240240
| main.rs:430:22:430:22 | S | main.rs:412:5:412:13 | struct S |
241241
| main.rs:435:10:437:5 | Trait2::<...> | main.rs:397:5:410:5 | trait Trait2 |
242242
| main.rs:436:7:436:7 | S | main.rs:412:5:412:13 | struct S |
243243
| main.rs:438:11:438:11 | S | main.rs:412:5:412:13 | struct S |
244244
| main.rs:439:24:439:24 | S | main.rs:412:5:412:13 | struct S |
245-
| main.rs:440:13:440:16 | Self | main.rs:434:5:444:5 | impl Trait2::<...> for S { ... } |
246-
| main.rs:440:13:440:19 | ...::g | main.rs:384:9:386:9 | fn g |
245+
| main.rs:440:13:440:16 | Self | main.rs:412:5:412:13 | struct S |
246+
| main.rs:440:13:440:19 | ...::g | main.rs:425:9:428:9 | fn g |
247247
| main.rs:441:13:441:19 | println | {EXTERNAL LOCATION} | MacroRules |
248-
| main.rs:442:13:442:16 | Self | main.rs:434:5:444:5 | impl Trait2::<...> for S { ... } |
248+
| main.rs:442:13:442:16 | Self | main.rs:412:5:412:13 | struct S |
249+
| main.rs:442:13:442:19 | ...::c | main.rs:430:9:431:9 | Const |
249250
| main.rs:448:9:448:15 | println | {EXTERNAL LOCATION} | MacroRules |
250251
| main.rs:449:17:449:17 | S | main.rs:412:5:412:13 | struct S |
251252
| main.rs:450:10:450:10 | S | main.rs:412:5:412:13 | struct S |
@@ -255,7 +256,6 @@ resolvePath
255256
| main.rs:456:14:458:11 | Trait2::<...> | main.rs:397:5:410:5 | trait Trait2 |
256257
| main.rs:457:13:457:13 | S | main.rs:412:5:412:13 | struct S |
257258
| main.rs:460:9:460:9 | S | main.rs:412:5:412:13 | struct S |
258-
| main.rs:460:9:460:12 | ...::g | main.rs:384:9:386:9 | fn g |
259259
| main.rs:460:9:460:12 | ...::g | main.rs:425:9:428:9 | fn g |
260260
| main.rs:462:9:462:9 | S | main.rs:412:5:412:13 | struct S |
261261
| main.rs:462:9:462:12 | ...::h | main.rs:388:9:391:9 | fn h |
@@ -268,7 +268,8 @@ resolvePath
268268
| main.rs:482:21:482:22 | S2 | main.rs:480:5:480:14 | struct S2 |
269269
| main.rs:486:10:486:15 | Trait4 | main.rs:476:5:478:5 | trait Trait4 |
270270
| main.rs:486:21:486:22 | S2 | main.rs:480:5:480:14 | struct S2 |
271-
| main.rs:488:13:488:16 | Self | main.rs:486:5:491:5 | impl Trait4 for S2 { ... } |
271+
| main.rs:488:13:488:16 | Self | main.rs:480:5:480:14 | struct S2 |
272+
| main.rs:488:13:488:19 | ...::f | main.rs:482:26:483:23 | fn f |
272273
| main.rs:489:13:489:14 | S2 | main.rs:480:5:480:14 | struct S2 |
273274
| main.rs:489:13:489:17 | ...::f | main.rs:482:26:483:23 | fn f |
274275
| main.rs:506:14:506:16 | Foo | main.rs:496:9:498:9 | trait Foo |
@@ -385,17 +386,17 @@ resolvePath
385386
| main.rs:763:13:763:17 | Error | main.rs:759:13:759:17 | Error |
386387
| main.rs:766:22:769:9 | Result::<...> | {EXTERNAL LOCATION} | enum Result |
387388
| main.rs:767:13:767:17 | Input | main.rs:758:13:758:17 | Input |
388-
| main.rs:768:13:768:16 | Self | main.rs:756:5:788:5 | impl Reduce for MyImpl::<...> { ... } |
389+
| main.rs:768:13:768:16 | Self | main.rs:751:5:754:5 | struct MyImpl |
389390
| main.rs:768:13:768:23 | ...::Error | main.rs:770:11:774:9 | type Error |
390391
| main.rs:771:22:773:9 | Option::<...> | {EXTERNAL LOCATION} | enum Option |
391392
| main.rs:772:11:772:15 | Error | main.rs:759:13:759:17 | Error |
392393
| main.rs:776:13:776:17 | Input | main.rs:758:13:758:17 | Input |
393-
| main.rs:781:19:781:22 | Self | main.rs:756:5:788:5 | impl Reduce for MyImpl::<...> { ... } |
394+
| main.rs:781:19:781:22 | Self | main.rs:751:5:754:5 | struct MyImpl |
394395
| main.rs:781:19:781:29 | ...::Input | main.rs:766:9:770:9 | type Input |
395396
| main.rs:782:14:785:9 | Result::<...> | {EXTERNAL LOCATION} | enum Result |
396-
| main.rs:783:13:783:16 | Self | main.rs:756:5:788:5 | impl Reduce for MyImpl::<...> { ... } |
397+
| main.rs:783:13:783:16 | Self | main.rs:751:5:754:5 | struct MyImpl |
397398
| main.rs:783:13:783:24 | ...::Output | main.rs:774:11:777:9 | type Output |
398-
| main.rs:784:13:784:16 | Self | main.rs:756:5:788:5 | impl Reduce for MyImpl::<...> { ... } |
399+
| main.rs:784:13:784:16 | Self | main.rs:751:5:754:5 | struct MyImpl |
399400
| main.rs:784:13:784:23 | ...::Error | main.rs:770:11:774:9 | type Error |
400401
| main.rs:791:5:791:7 | std | {EXTERNAL LOCATION} | Crate(std@0.0.0) |
401402
| main.rs:791:11:791:14 | self | {EXTERNAL LOCATION} | Crate(std@0.0.0) |

‎rust/ql/test/library-tests/sensitivedata/CONSISTENCY/PathResolutionConsistency.expected‎

Lines changed: 0 additions & 2 deletions
This file was deleted.

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,13 @@ multipleCallTargets
55
| dereference.rs:184:17:184:30 | ... .foo() |
66
| dereference.rs:186:17:186:25 | S.bar(...) |
77
| dereference.rs:187:17:187:29 | S.bar(...) |
8-
| main.rs:590:9:590:18 | ...::m(...) |
98
| main.rs:2634:13:2634:31 | ...::from(...) |
109
| main.rs:2635:13:2635:31 | ...::from(...) |
1110
| main.rs:2636:13:2636:31 | ...::from(...) |
1211
| main.rs:2642:13:2642:31 | ...::from(...) |
1312
| main.rs:2643:13:2643:31 | ...::from(...) |
1413
| main.rs:2644:13:2644:31 | ...::from(...) |
14+
multiplePathResolutions
15+
| main.rs:2463:41:2463:52 | ...::Output |
16+
| main.rs:2472:38:2472:49 | ...::Output |
17+
| main.rs:2484:42:2484:53 | ...::Output |

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -587,7 +587,7 @@ mod impl_overlap {
587587
println!("{:?}", S3::m(&w, x)); // $ target=S3<T>::m
588588

589589
S4.m(); // $ target=<S4_as_MyTrait1>::m
590-
S4::m(&S4); // $ target=<S4_as_MyTrait1>::m $ SPURIOUS: target=MyTrait1::m
590+
S4::m(&S4); // $ target=<S4_as_MyTrait1>::m
591591
S5(0i32).m(); // $ target=<S5<i32>_as_MyTrait1>::m
592592
S5::m(&S5(0i32)); // $ target=<S5<i32>_as_MyTrait1>::m
593593
S5(true).m(); // $ target=MyTrait1::m

0 commit comments

Comments
 (0)