Skip to content

Commit 173db82

Browse files
hvitvedhvitved
committed
Rust: Only infer types from trait bounds when their implementation is unambiguous
1 parent ae988a5 commit 173db82

File tree

8 files changed

+271
-143
lines changed

8 files changed

+271
-143
lines changed

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

Lines changed: 91 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -13,68 +13,105 @@ private import TypeMention
1313
private import TypeInference
1414
private import FunctionType
1515

16-
pragma[nomagic]
17-
private Type resolveNonTypeParameterTypeAt(TypeMention tm, TypePath path) {
18-
result = tm.getTypeAt(path) and
19-
not result instanceof TypeParameter
20-
}
16+
private signature Type resolveTypeMentionAtSig(AstNode tm, TypePath path);
2117

22-
bindingset[t1, t2]
23-
private predicate typeMentionEqual(TypeMention t1, TypeMention t2) {
24-
forex(TypePath path, Type type | resolveNonTypeParameterTypeAt(t1, path) = type |
25-
resolveNonTypeParameterTypeAt(t2, path) = type
26-
)
27-
}
18+
/**
19+
* Provides logic for identifying sibling implementations, parameterized over
20+
* how to resolve type mentions (`PreTypeMention` vs. `TypeMention`).
21+
*/
22+
private module MkSiblingImpls<resolveTypeMentionAtSig/2 resolveTypeMentionAt> {
23+
pragma[nomagic]
24+
private Type resolveNonTypeParameterTypeAt(AstNode tm, TypePath path) {
25+
result = resolveTypeMentionAt(tm, path) and
26+
not result instanceof TypeParameter
27+
}
2828

29-
pragma[nomagic]
30-
private predicate implSiblingCandidate(
31-
Impl impl, TraitItemNode trait, Type rootType, TypeMention selfTy
32-
) {
33-
trait = impl.(ImplItemNode).resolveTraitTy() and
34-
selfTy = impl.getSelfTy() and
35-
rootType = selfTy.getType()
29+
bindingset[t1, t2]
30+
private predicate typeMentionEqual(AstNode t1, AstNode t2) {
31+
forex(TypePath path, Type type | resolveNonTypeParameterTypeAt(t1, path) = type |
32+
resolveNonTypeParameterTypeAt(t2, path) = type
33+
)
34+
}
35+
36+
pragma[nomagic]
37+
private predicate implSiblingCandidate(
38+
Impl impl, TraitItemNode trait, Type rootType, AstNode selfTy
39+
) {
40+
trait = impl.(ImplItemNode).resolveTraitTy() and
41+
selfTy = impl.getSelfTy() and
42+
rootType = resolveTypeMentionAt(selfTy, TypePath::nil())
43+
}
44+
45+
pragma[nomagic]
46+
private predicate blanketImplSiblingCandidate(ImplItemNode impl, Trait trait) {
47+
impl.isBlanketImplementation() and
48+
trait = impl.resolveTraitTy()
49+
}
50+
51+
/**
52+
* Holds if `impl1` and `impl2` are a sibling implementations of `trait`. We
53+
* consider implementations to be siblings if they implement the same trait for
54+
* the same type. In that case `Self` is the same type in both implementations,
55+
* and method calls to the implementations cannot be resolved unambiguously
56+
* based only on the receiver type.
57+
*/
58+
pragma[inline]
59+
predicate implSiblings(TraitItemNode trait, Impl impl1, Impl impl2) {
60+
impl1 != impl2 and
61+
(
62+
exists(Type rootType, AstNode selfTy1, AstNode selfTy2 |
63+
implSiblingCandidate(impl1, trait, rootType, selfTy1) and
64+
implSiblingCandidate(impl2, trait, rootType, selfTy2) and
65+
// In principle the second conjunct below should be superflous, but we still
66+
// have ill-formed type mentions for types that we don't understand. For
67+
// those checking both directions restricts further. Note also that we check
68+
// syntactic equality, whereas equality up to renaming would be more
69+
// correct.
70+
typeMentionEqual(selfTy1, selfTy2) and
71+
typeMentionEqual(selfTy2, selfTy1)
72+
)
73+
or
74+
blanketImplSiblingCandidate(impl1, trait) and
75+
blanketImplSiblingCandidate(impl2, trait)
76+
)
77+
}
78+
79+
/**
80+
* Holds if `impl` is an implementation of `trait` and if another implementation
81+
* exists for the same type.
82+
*/
83+
pragma[nomagic]
84+
predicate implHasSibling(ImplItemNode impl, Trait trait) { implSiblings(trait, impl, _) }
85+
86+
pragma[nomagic]
87+
predicate implHasAmbigousSiblingAt(ImplItemNode impl, Trait trait, TypePath path) {
88+
exists(ImplItemNode impl2, Type t1, Type t2 |
89+
implSiblings(trait, impl, impl2) and
90+
t1 = resolveTypeMentionAt(impl.getTraitPath(), path) and
91+
t2 = resolveTypeMentionAt(impl2.getTraitPath(), path) and
92+
t1 != t2
93+
|
94+
not t1 instanceof TypeParameter or
95+
not t2 instanceof TypeParameter
96+
)
97+
}
3698
}
3799

38-
pragma[nomagic]
39-
private predicate blanketImplSiblingCandidate(ImplItemNode impl, Trait trait) {
40-
impl.isBlanketImplementation() and
41-
trait = impl.resolveTraitTy()
100+
private Type resolvePreTypeMention(AstNode tm, TypePath path) {
101+
result = tm.(PreTypeMention).getTypeAt(path)
42102
}
43103

44-
/**
45-
* Holds if `impl1` and `impl2` are a sibling implementations of `trait`. We
46-
* consider implementations to be siblings if they implement the same trait for
47-
* the same type. In that case `Self` is the same type in both implementations,
48-
* and method calls to the implementations cannot be resolved unambiguously
49-
* based only on the receiver type.
50-
*/
51-
pragma[inline]
52-
private predicate implSiblings(TraitItemNode trait, Impl impl1, Impl impl2) {
53-
impl1 != impl2 and
54-
(
55-
exists(Type rootType, TypeMention selfTy1, TypeMention selfTy2 |
56-
implSiblingCandidate(impl1, trait, rootType, selfTy1) and
57-
implSiblingCandidate(impl2, trait, rootType, selfTy2) and
58-
// In principle the second conjunct below should be superflous, but we still
59-
// have ill-formed type mentions for types that we don't understand. For
60-
// those checking both directions restricts further. Note also that we check
61-
// syntactic equality, whereas equality up to renaming would be more
62-
// correct.
63-
typeMentionEqual(selfTy1, selfTy2) and
64-
typeMentionEqual(selfTy2, selfTy1)
65-
)
66-
or
67-
blanketImplSiblingCandidate(impl1, trait) and
68-
blanketImplSiblingCandidate(impl2, trait)
69-
)
104+
private module PreSiblingImpls = MkSiblingImpls<resolvePreTypeMention/2>;
105+
106+
predicate preImplHasAmbigousSiblingAt = PreSiblingImpls::implHasAmbigousSiblingAt/3;
107+
108+
private Type resolveTypeMention(AstNode tm, TypePath path) {
109+
result = tm.(TypeMention).getTypeAt(path)
70110
}
71111

72-
/**
73-
* Holds if `impl` is an implementation of `trait` and if another implementation
74-
* exists for the same type.
75-
*/
76-
pragma[nomagic]
77-
private predicate implHasSibling(ImplItemNode impl, Trait trait) { implSiblings(trait, impl, _) }
112+
private module SiblingImpls = MkSiblingImpls<resolveTypeMention/2>;
113+
114+
import SiblingImpls
78115

79116
/**
80117
* Holds if `f` is a function declared inside `trait`, and the type of `f` at

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

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -439,11 +439,11 @@ class TypeParamTypeParameter extends TypeParameter, TTypeParamTypeParameter {
439439
*/
440440
class AssociatedTypeTypeParameter extends TypeParameter, TAssociatedTypeTypeParameter {
441441
private Trait trait;
442-
private TypeAlias typeAlias;
442+
private AssocType typeAlias;
443443

444444
AssociatedTypeTypeParameter() { this = TAssociatedTypeTypeParameter(trait, typeAlias) }
445445

446-
TypeAlias getTypeAlias() { result = typeAlias }
446+
AssocType getTypeAlias() { result = typeAlias }
447447

448448
/** Gets the trait that contains this associated type declaration. */
449449
TraitItemNode getTrait() { result = trait }
@@ -457,7 +457,13 @@ class AssociatedTypeTypeParameter extends TypeParameter, TAssociatedTypeTypePara
457457
override ItemNode getDeclaringItem() { result = trait }
458458

459459
override string toString() {
460-
result = typeAlias.getName().getText() + "[" + trait.getName().toString() + "]"
460+
exists(string fromString, TraitItemNode trait2 |
461+
result = typeAlias.getName().getText() + "[" + trait.getName() + fromString + "]" and
462+
trait2 = typeAlias.getTrait() and
463+
if trait = trait2
464+
then fromString = ""
465+
else fromString = " (inherited from " + trait2.getName() + ")"
466+
)
461467
}
462468

463469
override Location getLocation() { result = typeAlias.getLocation() }

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

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,12 @@ private module Input implements InputSig1<Location>, InputSig2<PreTypeMention> {
4141

4242
class TypeAbstraction = TA::TypeAbstraction;
4343

44+
predicate typeAbstractionHasAmbigousConstraintAt(
45+
TypeAbstraction abs, Type constraint, TypePath path
46+
) {
47+
FunctionOverloading::implHasAmbigousSiblingAt(abs, constraint.(TraitType).getTrait(), path)
48+
}
49+
4450
class TypeArgumentPosition extends TTypeArgumentPosition {
4551
int asMethodTypeArgumentPosition() { this = TMethodTypeArgumentPosition(result) }
4652

@@ -127,17 +133,15 @@ private module Input implements InputSig1<Location>, InputSig2<PreTypeMention> {
127133

128134
PreTypeMention getABaseTypeMention(Type t) { none() }
129135

130-
Type getATypeParameterConstraint(TypeParameter tp, TypePath path) {
131-
exists(TypeMention tm | result = tm.getTypeAt(path) |
132-
tm = tp.(TypeParamTypeParameter).getTypeParam().getATypeBound().getTypeRepr() or
133-
tm = tp.(SelfTypeParameter).getTrait() or
134-
tm =
135-
tp.(ImplTraitTypeTypeParameter)
136-
.getImplTraitTypeRepr()
137-
.getTypeBoundList()
138-
.getABound()
139-
.getTypeRepr()
140-
)
136+
PreTypeMention getATypeParameterConstraint(TypeParameter tp) {
137+
result = tp.(TypeParamTypeParameter).getTypeParam().getATypeBound().getTypeRepr() or
138+
result = tp.(SelfTypeParameter).getTrait() or
139+
result =
140+
tp.(ImplTraitTypeTypeParameter)
141+
.getImplTraitTypeRepr()
142+
.getTypeBoundList()
143+
.getABound()
144+
.getTypeRepr()
141145
}
142146

143147
/**
@@ -1170,7 +1174,7 @@ private module ContextTyping {
11701174
or
11711175
exists(TypeParameter mid |
11721176
assocFunctionMentionsTypeParameterAtNonRetPos(i, f, mid) and
1173-
tp = getATypeParameterConstraint(mid, _)
1177+
tp = getATypeParameterConstraint(mid).getTypeAt(_)
11741178
)
11751179
}
11761180

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

Lines changed: 46 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ private import Type
77
private import TypeAbstraction
88
private import TypeInference
99
private import AssociatedType
10+
private import FunctionOverloading
1011

1112
bindingset[trait, name]
1213
pragma[inline_late]
@@ -205,6 +206,13 @@ private module MkTypeMention<getAdditionalPathTypeAtSig/2 getAdditionalPathTypeA
205206
)
206207
or
207208
exists(TypeParamItemNode tp | this = tp.getABoundPath() and result = tp)
209+
or
210+
// `<result as this>::...`
211+
exists(PathTypeRepr typeRepr, PathTypeRepr traitRepr |
212+
pathTypeAsTraitAssoc(_, typeRepr, traitRepr, _, _) and
213+
this = traitRepr.getPath() and
214+
result = typeRepr.getPath()
215+
)
208216
}
209217

210218
pragma[nomagic]
@@ -696,75 +704,80 @@ private module PreTypeMention = MkTypeMention<preGetAdditionalPathTypeAt/2>;
696704

697705
class PreTypeMention = PreTypeMention::TypeMention;
698706

707+
private class TraitOrTmTrait extends AstNode {
708+
Type getTypeAt(TypePath path) {
709+
pathTypeAsTraitAssoc(_, _, this, _, _) and
710+
result = this.(PreTypeMention).getTypeAt(path)
711+
or
712+
result = TTrait(this) and
713+
path.isEmpty()
714+
}
715+
}
716+
699717
/**
700718
* Holds if `path` accesses an associated type `alias` from `trait` on a
701719
* concrete type given by `tm`.
702720
*
703-
* `implOrTmTrait` is either the mention that resolves to `trait` when `path`
704-
* is of the form `<Type as Trait>::AssocType`, or the enclosing `impl` block
705-
* when `path` is of the form `Self::AssocType`.
721+
* `traitOrTmTrait` is either the mention that resolves to `trait` when `path`
722+
* is of the form `<Type as Trait>::AssocType`, or the trait being implemented
723+
* when `path` is of the form `Self::AssocType` within an `impl` block.
706724
*/
707725
private predicate pathConcreteTypeAssocType(
708-
Path path, PreTypeMention tm, TraitItemNode trait, AstNode implOrTmTrait, TypeAlias alias
726+
Path path, PreTypeMention tm, TraitItemNode trait, TraitOrTmTrait traitOrTmTrait, TypeAlias alias
709727
) {
710728
exists(Path qualifier |
711729
qualifier = path.getQualifier() and
712730
not resolvePath(tm.(PathTypeRepr).getPath()) instanceof TypeParam
713731
|
714732
// path of the form `<Type as Trait>::AssocType`
715733
// ^^^ tm ^^^^^^^^^ name
734+
// ^^^^^ traitOrTmTrait
716735
exists(string name |
717-
pathTypeAsTraitAssoc(path, tm, implOrTmTrait, trait, name) and
736+
pathTypeAsTraitAssoc(path, tm, traitOrTmTrait, trait, name) and
718737
getTraitAssocType(trait, name) = alias
719738
)
720739
or
721740
// path of the form `Self::AssocType` within an `impl` block
722741
// tm ^^^^ ^^^^^^^^^ name
723-
implOrTmTrait =
724-
any(ImplItemNode impl |
725-
alias = resolvePath(path) and
726-
qualifier = impl.getASelfPath() and
727-
tm = impl.(Impl).getSelfTy() and
728-
trait.getAnAssocItem() = alias
729-
)
742+
exists(ImplItemNode impl |
743+
alias = resolvePath(path) and
744+
qualifier = impl.getASelfPath() and
745+
tm = impl.(Impl).getSelfTy() and
746+
trait.getAnAssocItem() = alias and
747+
traitOrTmTrait = trait
748+
)
730749
)
731750
}
732751

733-
private module PathSatisfiesConstraintInput implements SatisfiesTypeInputSig<PreTypeMention> {
734-
predicate relevantConstraint(PreTypeMention tm, Type constraint) {
735-
pathConcreteTypeAssocType(_, tm, constraint.(TraitType).getTrait(), _, _)
752+
private module PathSatisfiesConstraintInput implements
753+
SatisfiesConstraintInputSig<PreTypeMention, TraitOrTmTrait>
754+
{
755+
predicate relevantConstraint(PreTypeMention tm, TraitOrTmTrait constraint) {
756+
pathConcreteTypeAssocType(_, tm, _, constraint, _)
757+
}
758+
759+
predicate typeAbstractionHasAmbigousConstraintAtOverride(
760+
TypeAbstraction abs, Type constraint, TypePath path
761+
) {
762+
preImplHasAmbigousSiblingAt(abs, constraint.(TraitType).getTrait(), path)
736763
}
737764
}
738765

739766
private module PathSatisfiesConstraint =
740-
SatisfiesType<PreTypeMention, PathSatisfiesConstraintInput>;
767+
SatisfiesConstraint<PreTypeMention, TraitOrTmTrait, PathSatisfiesConstraintInput>;
741768

742769
/**
743770
* Gets the type of `path` at `typePath` when `path` accesses an associated type
744771
* on a concrete type.
745772
*/
746773
private Type getPathConcreteAssocTypeAt(Path path, TypePath typePath) {
747774
exists(
748-
PreTypeMention tm, ImplItemNode impl, TraitItemNode trait, TraitType t, AstNode implOrTmTrait,
775+
PreTypeMention tm, ImplItemNode impl, TraitItemNode trait, TraitOrTmTrait traitOrTmTrait,
749776
TypeAlias alias, TypePath path0
750777
|
751-
pathConcreteTypeAssocType(path, tm, trait, implOrTmTrait, alias) and
752-
t = TTrait(trait) and
753-
PathSatisfiesConstraint::satisfiesConstraintTypeThrough(tm, impl, t, path0, result) and
778+
pathConcreteTypeAssocType(path, tm, trait, traitOrTmTrait, alias) and
779+
PathSatisfiesConstraint::satisfiesConstraintTypeThrough(tm, impl, traitOrTmTrait, path0, result) and
754780
path0.isCons(TAssociatedTypeTypeParameter(trait, alias), typePath)
755-
|
756-
implOrTmTrait instanceof Impl
757-
or
758-
// When `path` is of the form `<Type as Trait>::AssocType` we need to check
759-
// that `impl` is not more specific than the mentioned trait
760-
implOrTmTrait =
761-
any(PreTypeMention tmTrait |
762-
not exists(TypePath path1, Type t1 |
763-
t1 = impl.getTraitPath().(PreTypeMention).getTypeAt(path1) and
764-
not t1 instanceof TypeParameter and
765-
t1 != tmTrait.getTypeAt(path1)
766-
)
767-
)
768781
)
769782
}
770783

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -509,15 +509,15 @@ mod trait_bound_impl_overlap {
509509

510510
fn test() {
511511
let x = S(0);
512-
let y = call_f(x); // $ target=call_f type=y:i32 $ SPURIOUS: type=y:i64
512+
let y = call_f(x); // $ target=call_f type=y:i32
513513
let z: i32 = y;
514514

515515
let x = S(0);
516516
let y = call_f::<i32, _>(x); // $ target=call_f type=y:i32
517517

518518
let x = S(0);
519-
let y = call_f2(S(0i32), x); // $ target=call_f2 type=y:i32 $ SPURIOUS: type=y:i64
519+
let y = call_f2(S(0i32), x); // $ target=call_f2 $ MISSING: type=y:i32
520520
let x = S(0);
521-
let y = call_f2(S(0i64), x); // $ target=call_f2 type=y:i64 $ SPURIOUS: type=y:i32
521+
let y = call_f2(S(0i64), x); // $ target=call_f2 $ MISSING: type=y:i64
522522
}
523523
}

0 commit comments

Comments
 (0)