Skip to content

Commit 84bef5d

Browse files
hvitvedhvitved
authored
Merge pull request #21420 from hvitved/rust/type-inference-qualified-trait-arg-path
Rust: More conservative resolution of `<Foo as Bar<...>>` paths
2 parents a3e9aed + ff41917 commit 84bef5d

File tree

6 files changed

+117
-33
lines changed

6 files changed

+117
-33
lines changed

‎rust/ql/lib/codeql/rust/internal/typeinference/AssociatedType.qll‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,14 @@ AssocType getTraitAssocType(Trait trait) { result.getTrait() = trait.getSupertra
2424

2525
/** Holds if `path` is of the form `<type as trait>::name` */
2626
pragma[nomagic]
27-
predicate pathTypeAsTraitAssoc(Path path, TypeRepr typeRepr, Path traitPath, string name) {
27+
predicate pathTypeAsTraitAssoc(
28+
Path path, TypeRepr typeRepr, PathTypeRepr traitRepr, Trait trait, string name
29+
) {
2830
exists(PathSegment segment |
2931
segment = path.getQualifier().getSegment() and
3032
typeRepr = segment.getTypeRepr() and
31-
traitPath = segment.getTraitTypeRepr().getPath() and
33+
traitRepr = segment.getTraitTypeRepr() and
34+
trait = resolvePath(traitRepr.getPath()) and
3235
name = path.getText()
3336
)
3437
}
@@ -43,10 +46,10 @@ predicate tpAssociatedType(TypeParam tp, AssocType assoc, Path path) {
4346
resolvePath(path.getQualifier()) = tp and
4447
resolvePath(path) = assoc
4548
or
46-
exists(PathTypeRepr typeRepr, Path traitPath, string name |
47-
pathTypeAsTraitAssoc(path, typeRepr, traitPath, name) and
49+
exists(PathTypeRepr typeRepr, TraitItemNode trait, string name |
50+
pathTypeAsTraitAssoc(path, typeRepr, _, trait, name) and
4851
tp = resolvePath(typeRepr.getPath()) and
49-
assoc = resolvePath(traitPath).(TraitItemNode).getAssocItem(name)
52+
assoc = trait.getAssocItem(name)
5053
)
5154
}
5255

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

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -701,17 +701,16 @@ class PreTypeMention = PreTypeMention::TypeMention;
701701
* concrete type given by `tm`.
702702
*/
703703
private predicate pathConcreteTypeAssocType(
704-
Path path, PreTypeMention tm, Trait trait, TypeAlias alias
704+
Path path, PreTypeMention tm, TraitItemNode trait, PreTypeMention tmTrait, TypeAlias alias
705705
) {
706706
exists(Path qualifier |
707707
qualifier = path.getQualifier() and
708708
not resolvePath(tm.(PathTypeRepr).getPath()) instanceof TypeParam
709709
|
710710
// path of the form `<Type as Trait>::AssocType`
711711
// ^^^ tm ^^^^^^^^^ name
712-
exists(string name, Path traitPath |
713-
pathTypeAsTraitAssoc(path, tm, traitPath, name) and
714-
trait = resolvePath(traitPath) and
712+
exists(string name |
713+
pathTypeAsTraitAssoc(path, tm, tmTrait, trait, name) and
715714
getTraitAssocType(trait, name) = alias
716715
)
717716
or
@@ -721,14 +720,15 @@ private predicate pathConcreteTypeAssocType(
721720
alias = resolvePath(path) and
722721
qualifier = impl.getASelfPath() and
723722
tm = impl.(Impl).getSelfTy() and
724-
trait.(TraitItemNode).getAnAssocItem() = alias
723+
trait.getAnAssocItem() = alias and
724+
tmTrait = impl.getTraitPath()
725725
)
726726
)
727727
}
728728

729729
private module PathSatisfiesConstraintInput implements SatisfiesConstraintInputSig<PreTypeMention> {
730730
predicate relevantConstraint(PreTypeMention tm, Type constraint) {
731-
pathConcreteTypeAssocType(_, tm, constraint.(TraitType).getTrait(), _)
731+
pathConcreteTypeAssocType(_, tm, constraint.(TraitType).getTrait(), _, _)
732732
}
733733
}
734734

@@ -740,10 +740,22 @@ private module PathSatisfiesConstraint =
740740
* on a concrete type.
741741
*/
742742
private Type getPathConcreteAssocTypeAt(Path path, TypePath typePath) {
743-
exists(PreTypeMention tm, TraitItemNode t, TypeAlias alias, TypePath path0 |
744-
pathConcreteTypeAssocType(path, tm, t, alias) and
745-
PathSatisfiesConstraint::satisfiesConstraintType(tm, TTrait(t), path0, result) and
746-
path0.isCons(TAssociatedTypeTypeParameter(t, alias), typePath)
743+
exists(
744+
PreTypeMention tm, ImplItemNode impl, TraitItemNode trait, TraitType t, PreTypeMention tmTrait,
745+
TypeAlias alias, TypePath path0
746+
|
747+
pathConcreteTypeAssocType(path, tm, trait, tmTrait, alias) and
748+
t = TTrait(trait) and
749+
PathSatisfiesConstraint::satisfiesConstraintTypeThrough(tm, impl, t, path0, result) and
750+
path0.isCons(TAssociatedTypeTypeParameter(trait, alias), typePath)
751+
|
752+
tmTrait.getTypeAt(TypePath::nil()) != t
753+
or
754+
not exists(TypePath path1, Type t1 |
755+
t1 = impl.getTraitPath().(PreTypeMention).getTypeAt(path1) and
756+
not t1 instanceof TypeParameter and
757+
t1 != tmTrait.getTypeAt(path1)
758+
)
747759
)
748760
}
749761

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

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

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -236,8 +236,8 @@ mod concrete_type_as_generic_access_associated_type {
236236

237237
pub fn test() {
238238
let s = S;
239-
let _a = s.convert(true); // $ target=S::convert type=_a:i32 SPURIOUS: bool
240-
let _b = s.convert(42); // $ target=S::convert type=_b:bool SPURIOUS: i32
239+
let _a = s.convert(true); // $ target=S::convert $ MISSING: type=_a:i32
240+
let _b = s.convert(42); // $ target=S::convert $ MISSING: type=_b:bool
241241
}
242242
}
243243

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

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,3 +32,45 @@ mod regression1 {
3232
opt_e.unwrap() // $ target=unwrap
3333
}
3434
}
35+
36+
mod regression2 {
37+
use std::ops::Sub;
38+
39+
#[derive(Copy, Clone)]
40+
struct S1;
41+
#[derive(Copy, Clone)]
42+
struct S2;
43+
44+
impl Sub for S1 {
45+
type Output = Self;
46+
47+
// S1SubS1
48+
fn sub(self, _rhs: Self) -> Self::Output {
49+
S1
50+
}
51+
}
52+
53+
impl Sub<S2> for S1 {
54+
type Output = S2;
55+
56+
// S1SubS2
57+
fn sub(self, _rhs: S2) -> Self::Output {
58+
S2
59+
}
60+
}
61+
62+
impl Sub<&S2> for S1 {
63+
type Output = <S1 as Sub<S2>>::Output;
64+
65+
// S1SubRefS2
66+
fn sub(self, other: &S2) -> <S1 as Sub<S2>>::Output {
67+
Sub::sub(self, *other) // $ target=S1SubS2 target=deref
68+
}
69+
}
70+
71+
fn foo() {
72+
let s1 = S1;
73+
let s2 = S2;
74+
let x = s1 - &s2; // $ target=S1SubRefS2 type=x:S2
75+
}
76+
}

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

Lines changed: 43 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -105,8 +105,6 @@ inferCertainType
105105
| associated_types.rs:229:23:229:27 | SelfParam | | {EXTERNAL LOCATION} | & |
106106
| associated_types.rs:229:23:229:27 | SelfParam | TRef | associated_types.rs:10:1:11:9 | S |
107107
| associated_types.rs:229:30:229:30 | t | | associated_types.rs:229:20:229:20 | T |
108-
| associated_types.rs:232:9:234:9 | { ... } | | {EXTERNAL LOCATION} | bool |
109-
| associated_types.rs:232:9:234:9 | { ... } | | {EXTERNAL LOCATION} | i32 |
110108
| associated_types.rs:233:24:233:24 | t | | associated_types.rs:229:20:229:20 | T |
111109
| associated_types.rs:237:19:241:5 | { ... } | | {EXTERNAL LOCATION} | () |
112110
| associated_types.rs:239:28:239:31 | true | | {EXTERNAL LOCATION} | bool |
@@ -4947,6 +4945,21 @@ inferCertainType
49474945
| regressions.rs:27:37:27:41 | vec_e | | {EXTERNAL LOCATION} | Vec |
49484946
| regressions.rs:27:37:27:41 | vec_e | A | {EXTERNAL LOCATION} | Global |
49494947
| regressions.rs:28:9:30:9 | { ... } | | {EXTERNAL LOCATION} | () |
4948+
| regressions.rs:48:16:48:19 | SelfParam | | regressions.rs:39:5:40:14 | S1 |
4949+
| regressions.rs:48:22:48:25 | _rhs | | regressions.rs:39:5:40:14 | S1 |
4950+
| regressions.rs:48:50:50:9 | { ... } | | regressions.rs:39:5:40:14 | S1 |
4951+
| regressions.rs:57:16:57:19 | SelfParam | | regressions.rs:39:5:40:14 | S1 |
4952+
| regressions.rs:57:22:57:25 | _rhs | | regressions.rs:41:5:42:14 | S2 |
4953+
| regressions.rs:57:48:59:9 | { ... } | | regressions.rs:41:5:42:14 | S2 |
4954+
| regressions.rs:66:16:66:19 | SelfParam | | regressions.rs:39:5:40:14 | S1 |
4955+
| regressions.rs:66:22:66:26 | other | | {EXTERNAL LOCATION} | & |
4956+
| regressions.rs:66:22:66:26 | other | TRef | regressions.rs:41:5:42:14 | S2 |
4957+
| regressions.rs:66:61:68:9 | { ... } | | regressions.rs:41:5:42:14 | S2 |
4958+
| regressions.rs:67:22:67:25 | self | | regressions.rs:39:5:40:14 | S1 |
4959+
| regressions.rs:67:29:67:33 | other | | {EXTERNAL LOCATION} | & |
4960+
| regressions.rs:67:29:67:33 | other | TRef | regressions.rs:41:5:42:14 | S2 |
4961+
| regressions.rs:71:14:75:5 | { ... } | | {EXTERNAL LOCATION} | () |
4962+
| regressions.rs:74:22:74:24 | &s2 | | {EXTERNAL LOCATION} | & |
49504963
inferType
49514964
| associated_types.rs:5:15:5:18 | SelfParam | | associated_types.rs:1:1:2:21 | Wrapper |
49524965
| associated_types.rs:5:15:5:18 | SelfParam | A | associated_types.rs:4:6:4:6 | A |
@@ -5114,25 +5127,13 @@ inferType
51145127
| associated_types.rs:229:23:229:27 | SelfParam | | {EXTERNAL LOCATION} | & |
51155128
| associated_types.rs:229:23:229:27 | SelfParam | TRef | associated_types.rs:10:1:11:9 | S |
51165129
| associated_types.rs:229:30:229:30 | t | | associated_types.rs:229:20:229:20 | T |
5117-
| associated_types.rs:232:9:234:9 | { ... } | | {EXTERNAL LOCATION} | bool |
5118-
| associated_types.rs:232:9:234:9 | { ... } | | {EXTERNAL LOCATION} | i32 |
5119-
| associated_types.rs:233:13:233:25 | ...::through(...) | | {EXTERNAL LOCATION} | bool |
5120-
| associated_types.rs:233:13:233:25 | ...::through(...) | | {EXTERNAL LOCATION} | i32 |
51215130
| associated_types.rs:233:24:233:24 | t | | associated_types.rs:229:20:229:20 | T |
51225131
| associated_types.rs:237:19:241:5 | { ... } | | {EXTERNAL LOCATION} | () |
51235132
| associated_types.rs:238:13:238:13 | s | | associated_types.rs:10:1:11:9 | S |
51245133
| associated_types.rs:238:17:238:17 | S | | associated_types.rs:10:1:11:9 | S |
5125-
| associated_types.rs:239:13:239:14 | _a | | {EXTERNAL LOCATION} | bool |
5126-
| associated_types.rs:239:13:239:14 | _a | | {EXTERNAL LOCATION} | i32 |
51275134
| associated_types.rs:239:18:239:18 | s | | associated_types.rs:10:1:11:9 | S |
5128-
| associated_types.rs:239:18:239:32 | s.convert(...) | | {EXTERNAL LOCATION} | bool |
5129-
| associated_types.rs:239:18:239:32 | s.convert(...) | | {EXTERNAL LOCATION} | i32 |
51305135
| associated_types.rs:239:28:239:31 | true | | {EXTERNAL LOCATION} | bool |
5131-
| associated_types.rs:240:13:240:14 | _b | | {EXTERNAL LOCATION} | bool |
5132-
| associated_types.rs:240:13:240:14 | _b | | {EXTERNAL LOCATION} | i32 |
51335136
| associated_types.rs:240:18:240:18 | s | | associated_types.rs:10:1:11:9 | S |
5134-
| associated_types.rs:240:18:240:30 | s.convert(...) | | {EXTERNAL LOCATION} | bool |
5135-
| associated_types.rs:240:18:240:30 | s.convert(...) | | {EXTERNAL LOCATION} | i32 |
51365137
| associated_types.rs:240:28:240:29 | 42 | | {EXTERNAL LOCATION} | i32 |
51375138
| associated_types.rs:248:30:248:34 | thing | | associated_types.rs:248:19:248:27 | T |
51385139
| associated_types.rs:248:65:250:5 | { ... } | | associated_types.rs:248:19:248:27 | T::Output[GetSet] |
@@ -14789,4 +14790,32 @@ inferType
1478914790
| regressions.rs:32:9:32:13 | opt_e | | {EXTERNAL LOCATION} | Option |
1479014791
| regressions.rs:32:9:32:13 | opt_e | T | regressions.rs:5:5:7:5 | E |
1479114792
| regressions.rs:32:9:32:22 | opt_e.unwrap() | | regressions.rs:5:5:7:5 | E |
14793+
| regressions.rs:48:16:48:19 | SelfParam | | regressions.rs:39:5:40:14 | S1 |
14794+
| regressions.rs:48:22:48:25 | _rhs | | regressions.rs:39:5:40:14 | S1 |
14795+
| regressions.rs:48:50:50:9 | { ... } | | regressions.rs:39:5:40:14 | S1 |
14796+
| regressions.rs:49:13:49:14 | S1 | | regressions.rs:39:5:40:14 | S1 |
14797+
| regressions.rs:57:16:57:19 | SelfParam | | regressions.rs:39:5:40:14 | S1 |
14798+
| regressions.rs:57:22:57:25 | _rhs | | regressions.rs:41:5:42:14 | S2 |
14799+
| regressions.rs:57:48:59:9 | { ... } | | regressions.rs:41:5:42:14 | S2 |
14800+
| regressions.rs:58:13:58:14 | S2 | | regressions.rs:41:5:42:14 | S2 |
14801+
| regressions.rs:66:16:66:19 | SelfParam | | regressions.rs:39:5:40:14 | S1 |
14802+
| regressions.rs:66:22:66:26 | other | | {EXTERNAL LOCATION} | & |
14803+
| regressions.rs:66:22:66:26 | other | TRef | regressions.rs:41:5:42:14 | S2 |
14804+
| regressions.rs:66:61:68:9 | { ... } | | regressions.rs:41:5:42:14 | S2 |
14805+
| regressions.rs:67:13:67:34 | ...::sub(...) | | regressions.rs:41:5:42:14 | S2 |
14806+
| regressions.rs:67:22:67:25 | self | | regressions.rs:39:5:40:14 | S1 |
14807+
| regressions.rs:67:28:67:33 | * ... | | regressions.rs:41:5:42:14 | S2 |
14808+
| regressions.rs:67:29:67:33 | other | | {EXTERNAL LOCATION} | & |
14809+
| regressions.rs:67:29:67:33 | other | TRef | regressions.rs:41:5:42:14 | S2 |
14810+
| regressions.rs:71:14:75:5 | { ... } | | {EXTERNAL LOCATION} | () |
14811+
| regressions.rs:72:13:72:14 | s1 | | regressions.rs:39:5:40:14 | S1 |
14812+
| regressions.rs:72:18:72:19 | S1 | | regressions.rs:39:5:40:14 | S1 |
14813+
| regressions.rs:73:13:73:14 | s2 | | regressions.rs:41:5:42:14 | S2 |
14814+
| regressions.rs:73:18:73:19 | S2 | | regressions.rs:41:5:42:14 | S2 |
14815+
| regressions.rs:74:13:74:13 | x | | regressions.rs:41:5:42:14 | S2 |
14816+
| regressions.rs:74:17:74:18 | s1 | | regressions.rs:39:5:40:14 | S1 |
14817+
| regressions.rs:74:17:74:24 | ... - ... | | regressions.rs:41:5:42:14 | S2 |
14818+
| regressions.rs:74:22:74:24 | &s2 | | {EXTERNAL LOCATION} | & |
14819+
| regressions.rs:74:22:74:24 | &s2 | TRef | regressions.rs:41:5:42:14 | S2 |
14820+
| regressions.rs:74:23:74:24 | s2 | | regressions.rs:41:5:42:14 | S2 |
1479214821
testFailures

0 commit comments

Comments
 (0)