Skip to content

Commit 0d42b28

Browse files
hvitvedhvitved
committed
Rust: Refine Self resolution inside impl blocks
1 parent 7da2d52 commit 0d42b28

File tree

10 files changed

+81
-66
lines changed

10 files changed

+81
-66
lines changed

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

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

227+
/** Gets a child item of this item, if any. */
228+
ItemNode getAChild() { this = result.getImmediateParent() }
229+
227230
/** Gets the immediately enclosing module (or source file) of this item. */
228231
pragma[nomagic]
229232
ModuleLikeNode getImmediateParentModule() {
@@ -300,10 +303,13 @@ abstract class ItemNode extends Locatable {
300303
typeImplEdge(this, _, name, kind, result, useOpt)
301304
or
302305
// trait items with default implementations made available in an implementation
303-
exists(ImplItemNodeImpl impl, ItemNode trait |
306+
exists(ImplItemNodeImpl impl, TraitItemNode trait |
304307
this = impl and
305308
trait = impl.resolveTraitTyCand() and
306309
result = trait.getASuccessor(name, kind, useOpt) and
310+
// do not inerit default implementations from super traits; those are inherited by
311+
// their `impl` blocks
312+
result = trait.getAssocItem(name) and
307313
result.(AssocItemNode).hasImplementation() and
308314
kind.isExternalOrBoth() and
309315
not impl.hasAssocItem(name)
@@ -363,8 +369,14 @@ abstract class ItemNode extends Locatable {
363369
this instanceof SourceFile and
364370
builtin(name, result)
365371
or
366-
name = "Self" and
367-
this = result.(ImplOrTraitItemNode).getAnItemInSelfScope()
372+
exists(ImplOrTraitItemNode i |
373+
name = "Self" and
374+
this = i.getAnItemInSelfScope()
375+
|
376+
result = i.(Trait)
377+
or
378+
result = i.(ImplItemNodeImpl).resolveSelfTyCand()
379+
)
368380
or
369381
name = "crate" and
370382
this = result.(CrateItemNode).getASourceFile()
@@ -695,7 +707,7 @@ abstract class ImplOrTraitItemNode extends ItemNode {
695707
Path getASelfPath() {
696708
Stages::PathResolutionStage::ref() and
697709
isUnqualifiedSelfPath(result) and
698-
this = unqualifiedPathLookup(result, _, _)
710+
result = this.getAnItemInSelfScope().getADescendant()
699711
}
700712

701713
/** Gets an associated item belonging to this trait or `impl` block. */
@@ -921,7 +933,7 @@ private class ImplItemNodeImpl extends ImplItemNode {
921933
result = this.resolveSelfTyBuiltin()
922934
}
923935

924-
TraitItemNode resolveTraitTyCand() { result = resolvePathCand(this.getTraitPath()) }
936+
TraitItemNodeImpl resolveTraitTyCand() { result = resolvePathCand(this.getTraitPath()) }
925937
}
926938

927939
private class StructItemNode extends TypeItemNode, ParameterizableItemNode instanceof Struct {
@@ -1773,15 +1785,7 @@ private module DollarCrateResolution {
17731785

17741786
pragma[nomagic]
17751787
private ItemNode resolvePathCand0(RelevantPath path, Namespace ns) {
1776-
exists(ItemNode res |
1777-
res = unqualifiedPathLookup(path, ns, _) and
1778-
if
1779-
not any(RelevantPath parent).getQualifier() = path and
1780-
isUnqualifiedSelfPath(path) and
1781-
res instanceof ImplItemNode
1782-
then result = res.(ImplItemNodeImpl).resolveSelfTyCand()
1783-
else result = res
1784-
)
1788+
result = unqualifiedPathLookup(path, ns, _)
17851789
or
17861790
DollarCrateResolution::resolveDollarCrate(path, result) and
17871791
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
@@ -383,6 +383,8 @@ TypeParamTypeParameter getPtrTypeParameter() {
383383
/** A type parameter. */
384384
abstract class TypeParameter extends Type {
385385
override TypeParameter getPositionalTypeParameter(int i) { none() }
386+
387+
abstract ItemNode getDeclaringItem();
386388
}
387389

388390
private class RawTypeParameter = @type_param or @trait or @type_alias or @impl_trait_type_repr;
@@ -401,6 +403,8 @@ class TypeParamTypeParameter extends TypeParameter, TTypeParamTypeParameter {
401403

402404
TypeParam getTypeParam() { result = typeParam }
403405

406+
override ItemNode getDeclaringItem() { result.getTypeParam(_) = typeParam }
407+
404408
override string toString() {
405409
this = any(SliceType st).getATypeParameter() and
406410
result = "[T]"
@@ -459,6 +463,8 @@ class AssociatedTypeTypeParameter extends TypeParameter, TAssociatedTypeTypePara
459463
/** Gets the trait that contains this associated type declaration. */
460464
TraitItemNode getTrait() { result.getAnAssocItem() = typeAlias }
461465

466+
override ItemNode getDeclaringItem() { result = this.getTrait() }
467+
462468
override string toString() { result = typeAlias.getName().getText() }
463469

464470
override Location getLocation() { result = typeAlias.getLocation() }
@@ -491,6 +497,8 @@ class DynTraitTypeParameter extends TypeParameter, TDynTraitTypeParameter {
491497
result = [this.getTypeParam().toString(), this.getTypeAlias().getName().toString()]
492498
}
493499

500+
override ItemNode getDeclaringItem() { none() }
501+
494502
override string toString() { result = "dyn(" + this.toStringInner() + ")" }
495503

496504
override Location getLocation() { result = n.getLocation() }
@@ -506,6 +514,8 @@ class ImplTraitTypeParameter extends TypeParameter, TImplTraitTypeParameter {
506514

507515
ImplTraitTypeRepr getImplTraitTypeRepr() { result = implTrait }
508516

517+
override ItemNode getDeclaringItem() { none() }
518+
509519
override string toString() { result = "impl(" + typeParam.toString() + ")" }
510520

511521
override Location getLocation() { result = typeParam.getLocation() }
@@ -525,6 +535,8 @@ class SelfTypeParameter extends TypeParameter, TSelfTypeParameter {
525535

526536
Trait getTrait() { result = trait }
527537

538+
override ItemNode getDeclaringItem() { result = trait }
539+
528540
override string toString() { result = "Self [" + trait.toString() + "]" }
529541

530542
override Location getLocation() { result = trait.getLocation() }
@@ -552,6 +564,8 @@ class ImplTraitTypeTypeParameter extends ImplTraitType, TypeParameter {
552564

553565
ImplTraitTypeTypeParameter() { impl = function.getAParam().getTypeRepr() }
554566

567+
override ItemNode getDeclaringItem() { none() }
568+
555569
override Function getFunction() { result = function }
556570

557571
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
@@ -132,7 +132,7 @@ resolvePath
132132
| main.rs:169:22:169:29 | MyStruct | main.rs:162:5:162:22 | struct MyStruct |
133133
| main.rs:171:13:171:19 | println | {EXTERNAL LOCATION} | MacroRules |
134134
| main.rs:172:13:172:13 | f | main.rs:164:5:166:5 | fn f |
135-
| main.rs:173:13:173:16 | Self | main.rs:168:5:179:5 | impl MyTrait for MyStruct { ... } |
135+
| main.rs:173:13:173:16 | Self | main.rs:162:5:162:22 | struct MyStruct |
136136
| main.rs:173:13:173:19 | ...::g | main.rs:176:9:178:9 | fn g |
137137
| main.rs:177:13:177:19 | println | {EXTERNAL LOCATION} | MacroRules |
138138
| main.rs:182:10:182:17 | MyStruct | main.rs:162:5:162:22 | struct MyStruct |
@@ -196,7 +196,7 @@ resolvePath
196196
| main.rs:341:10:341:15 | Trait1 | main.rs:307:5:311:5 | trait Trait1 |
197197
| main.rs:342:11:342:11 | S | main.rs:338:5:338:13 | struct S |
198198
| main.rs:344:13:344:19 | println | {EXTERNAL LOCATION} | MacroRules |
199-
| main.rs:345:13:345:16 | Self | main.rs:340:5:352:5 | impl Trait1 for S { ... } |
199+
| main.rs:345:13:345:16 | Self | main.rs:338:5:338:13 | struct S |
200200
| main.rs:345:13:345:19 | ...::g | main.rs:349:9:351:9 | fn g |
201201
| main.rs:350:13:350:19 | println | {EXTERNAL LOCATION} | MacroRules |
202202
| main.rs:355:10:355:15 | Trait2 | main.rs:313:5:321:5 | trait Trait2 |
@@ -229,22 +229,23 @@ resolvePath
229229
| main.rs:418:11:418:11 | S | main.rs:412:5:412:13 | struct S |
230230
| main.rs:419:24:419:24 | S | main.rs:412:5:412:13 | struct S |
231231
| main.rs:420:13:420:19 | println | {EXTERNAL LOCATION} | MacroRules |
232-
| main.rs:421:13:421:16 | Self | main.rs:414:5:432:5 | impl Trait1::<...> for S { ... } |
232+
| main.rs:421:13:421:16 | Self | main.rs:412:5:412:13 | struct S |
233233
| main.rs:421:13:421:19 | ...::g | main.rs:425:9:428:9 | fn g |
234234
| main.rs:425:24:425:24 | S | main.rs:412:5:412:13 | struct S |
235235
| main.rs:426:13:426:19 | println | {EXTERNAL LOCATION} | MacroRules |
236-
| main.rs:427:13:427:16 | Self | main.rs:414:5:432:5 | impl Trait1::<...> for S { ... } |
236+
| main.rs:427:13:427:16 | Self | main.rs:412:5:412:13 | struct S |
237237
| main.rs:427:13:427:19 | ...::c | main.rs:430:9:431:9 | Const |
238238
| main.rs:430:18:430:18 | S | main.rs:412:5:412:13 | struct S |
239239
| main.rs:430:22:430:22 | S | main.rs:412:5:412:13 | struct S |
240240
| main.rs:435:10:437:5 | Trait2::<...> | main.rs:397:5:410:5 | trait Trait2 |
241241
| main.rs:436:7:436:7 | S | main.rs:412:5:412:13 | struct S |
242242
| main.rs:438:11:438:11 | S | main.rs:412:5:412:13 | struct S |
243243
| main.rs:439:24:439:24 | S | main.rs:412:5:412:13 | struct S |
244-
| main.rs:440:13:440:16 | Self | main.rs:434:5:444:5 | impl Trait2::<...> for S { ... } |
245-
| main.rs:440:13:440:19 | ...::g | main.rs:384:9:386:9 | fn g |
244+
| main.rs:440:13:440:16 | Self | main.rs:412:5:412:13 | struct S |
245+
| main.rs:440:13:440:19 | ...::g | main.rs:425:9:428:9 | fn g |
246246
| main.rs:441:13:441:19 | println | {EXTERNAL LOCATION} | MacroRules |
247-
| main.rs:442:13:442:16 | Self | main.rs:434:5:444:5 | impl Trait2::<...> for S { ... } |
247+
| main.rs:442:13:442:16 | Self | main.rs:412:5:412:13 | struct S |
248+
| main.rs:442:13:442:19 | ...::c | main.rs:430:9:431:9 | Const |
248249
| main.rs:448:9:448:15 | println | {EXTERNAL LOCATION} | MacroRules |
249250
| main.rs:449:17:449:17 | S | main.rs:412:5:412:13 | struct S |
250251
| main.rs:450:10:450:10 | S | main.rs:412:5:412:13 | struct S |
@@ -254,7 +255,6 @@ resolvePath
254255
| main.rs:456:14:458:11 | Trait2::<...> | main.rs:397:5:410:5 | trait Trait2 |
255256
| main.rs:457:13:457:13 | S | main.rs:412:5:412:13 | struct S |
256257
| main.rs:460:9:460:9 | S | main.rs:412:5:412:13 | struct S |
257-
| main.rs:460:9:460:12 | ...::g | main.rs:384:9:386:9 | fn g |
258258
| main.rs:460:9:460:12 | ...::g | main.rs:425:9:428:9 | fn g |
259259
| main.rs:462:9:462:9 | S | main.rs:412:5:412:13 | struct S |
260260
| main.rs:462:9:462:12 | ...::h | main.rs:388:9:391:9 | fn h |
@@ -267,7 +267,8 @@ resolvePath
267267
| main.rs:482:21:482:22 | S2 | main.rs:480:5:480:14 | struct S2 |
268268
| main.rs:486:10:486:15 | Trait4 | main.rs:476:5:478:5 | trait Trait4 |
269269
| main.rs:486:21:486:22 | S2 | main.rs:480:5:480:14 | struct S2 |
270-
| main.rs:488:13:488:16 | Self | main.rs:486:5:491:5 | impl Trait4 for S2 { ... } |
270+
| main.rs:488:13:488:16 | Self | main.rs:480:5:480:14 | struct S2 |
271+
| main.rs:488:13:488:19 | ...::f | main.rs:482:26:483:23 | fn f |
271272
| main.rs:489:13:489:14 | S2 | main.rs:480:5:480:14 | struct S2 |
272273
| main.rs:489:13:489:17 | ...::f | main.rs:482:26:483:23 | fn f |
273274
| main.rs:506:14:506:16 | Foo | main.rs:496:9:498:9 | trait Foo |
@@ -384,17 +385,17 @@ resolvePath
384385
| main.rs:763:13:763:17 | Error | main.rs:759:13:759:17 | Error |
385386
| main.rs:766:22:769:9 | Result::<...> | {EXTERNAL LOCATION} | enum Result |
386387
| main.rs:767:13:767:17 | Input | main.rs:758:13:758:17 | Input |
387-
| main.rs:768:13:768:16 | Self | main.rs:756:5:788:5 | impl Reduce for MyImpl::<...> { ... } |
388+
| main.rs:768:13:768:16 | Self | main.rs:751:5:754:5 | struct MyImpl |
388389
| main.rs:768:13:768:23 | ...::Error | main.rs:770:11:774:9 | type Error |
389390
| main.rs:771:22:773:9 | Option::<...> | {EXTERNAL LOCATION} | enum Option |
390391
| main.rs:772:11:772:15 | Error | main.rs:759:13:759:17 | Error |
391392
| main.rs:776:13:776:17 | Input | main.rs:758:13:758:17 | Input |
392-
| main.rs:781:19:781:22 | Self | main.rs:756:5:788:5 | impl Reduce for MyImpl::<...> { ... } |
393+
| main.rs:781:19:781:22 | Self | main.rs:751:5:754:5 | struct MyImpl |
393394
| main.rs:781:19:781:29 | ...::Input | main.rs:766:9:770:9 | type Input |
394395
| main.rs:782:14:785:9 | Result::<...> | {EXTERNAL LOCATION} | enum Result |
395-
| main.rs:783:13:783:16 | Self | main.rs:756:5:788:5 | impl Reduce for MyImpl::<...> { ... } |
396+
| main.rs:783:13:783:16 | Self | main.rs:751:5:754:5 | struct MyImpl |
396397
| main.rs:783:13:783:24 | ...::Output | main.rs:774:11:777:9 | type Output |
397-
| main.rs:784:13:784:16 | Self | main.rs:756:5:788:5 | impl Reduce for MyImpl::<...> { ... } |
398+
| main.rs:784:13:784:16 | Self | main.rs:751:5:754:5 | struct MyImpl |
398399
| main.rs:784:13:784:23 | ...::Error | main.rs:770:11:774:9 | type Error |
399400
| main.rs:791:5:791:7 | std | {EXTERNAL LOCATION} | Crate(std@0.0.0) |
400401
| main.rs:791:11:791:14 | self | {EXTERNAL LOCATION} | Crate(std@0.0.0) |

‎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)