Skip to content

Commit 654d410

Browse files
committed
Rust: Address PR feedback
1 parent f4ff815 commit 654d410

File tree

5 files changed

+147
-131
lines changed

5 files changed

+147
-131
lines changed

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

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -335,7 +335,16 @@ class SelfTypeParameter extends TypeParameter, TSelfTypeParameter {
335335
override Location getLocation() { result = trait.getLocation() }
336336
}
337337

338-
/** A type abstraction. */
338+
/**
339+
* A type abstraction. I.e., a place in the program where type variables are
340+
* introduced.
341+
*
342+
* Example:
343+
* ```rust
344+
* impl<A, B> Foo<A, B> { }
345+
* // ^^^^^^ a type abstraction
346+
* ```
347+
*/
339348
abstract class TypeAbstraction extends AstNode {
340349
abstract TypeParameter getATypeParameter();
341350
}

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

Lines changed: 26 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -19,16 +19,6 @@ private module Input1 implements InputSig1<Location> {
1919

2020
class TypeParameter = T::TypeParameter;
2121

22-
/**
23-
* A type abstraction. I.e., a place in the program where type variables are
24-
* introduced.
25-
*
26-
* Example:
27-
* ```rust
28-
* impl<A, B> Foo<A, B> { }
29-
* // ^^^^^^ a type abstraction
30-
* ```
31-
*/
3222
class TypeAbstraction = T::TypeAbstraction;
3323

3424
private newtype TTypeArgumentPosition =
@@ -156,7 +146,7 @@ private module Input2 implements InputSig2 {
156146
exists(TypeParam param |
157147
abs = param.getTypeBoundList().getABound() and
158148
condition = param and
159-
constraint = param.getTypeBoundList().getABound().getTypeRepr()
149+
constraint = abs.(TypeBound).getTypeRepr()
160150
)
161151
or
162152
// the implicit `Self` type parameter satisfies the trait
@@ -968,22 +958,6 @@ private module Cached {
968958
)
969959
}
970960

971-
pragma[nomagic]
972-
private Type receiverRootType(Expr e) {
973-
any(MethodCallExpr mce).getReceiver() = e and
974-
result = inferType(e)
975-
}
976-
977-
pragma[nomagic]
978-
private Type inferReceiverType(Expr e, TypePath path) {
979-
exists(Type root | root = receiverRootType(e) |
980-
// for reference types, lookup members in the type being referenced
981-
if root = TRefType()
982-
then result = inferType(e, TypePath::cons(TRefTypeParameter(), path))
983-
else result = inferType(e, path)
984-
)
985-
}
986-
987961
private class ReceiverExpr extends Expr {
988962
MethodCallExpr mce;
989963

@@ -993,13 +967,22 @@ private module Cached {
993967

994968
int getNumberOfArgs() { result = mce.getArgList().getNumberOfArgs() }
995969

996-
Type resolveTypeAt(TypePath path) { result = inferReceiverType(this, path) }
970+
pragma[nomagic]
971+
Type getTypeAt(TypePath path) {
972+
exists(TypePath path0 | result = inferType(this, path0) |
973+
path0 = TypePath::cons(TRefTypeParameter(), path)
974+
or
975+
not path0.isCons(TRefTypeParameter(), _) and
976+
not (path0.isEmpty() and result = TRefType()) and
977+
path = path0
978+
)
979+
}
997980
}
998981

999982
/** Holds if a method for `type` with the name `name` and the arity `arity` exists in `impl`. */
1000983
pragma[nomagic]
1001984
private predicate methodCandidate(Type type, string name, int arity, Impl impl) {
1002-
type = impl.(ImplTypeAbstraction).getSelfTy().(TypeReprMention).resolveType() and
985+
type = impl.getSelfTy().(TypeReprMention).resolveType() and
1003986
exists(Function f |
1004987
f = impl.(ImplItemNode).getASuccessor(name) and
1005988
f.getParamList().hasSelfParam() and
@@ -1009,17 +992,16 @@ private module Cached {
1009992

1010993
private module IsInstantiationOfInput implements IsInstantiationOfInputSig<ReceiverExpr> {
1011994
pragma[nomagic]
1012-
predicate potentialInstantiationOf(ReceiverExpr receiver, TypeAbstraction impl, TypeMention sub) {
1013-
methodCandidate(receiver.resolveTypeAt(TypePath::nil()), receiver.getField(),
995+
predicate potentialInstantiationOf(
996+
ReceiverExpr receiver, TypeAbstraction impl, TypeMention constraint
997+
) {
998+
methodCandidate(receiver.getTypeAt(TypePath::nil()), receiver.getField(),
1014999
receiver.getNumberOfArgs(), impl) and
1015-
sub = impl.(ImplTypeAbstraction).getSelfTy()
1000+
constraint = impl.(ImplTypeAbstraction).getSelfTy()
10161001
}
10171002

1018-
predicate relevantTypeMention(TypeMention sub) {
1019-
exists(TypeAbstraction impl |
1020-
methodCandidate(_, _, _, impl) and
1021-
sub = impl.(ImplTypeAbstraction).getSelfTy()
1022-
)
1003+
predicate relevantTypeMention(TypeMention constraint) {
1004+
exists(Impl impl | methodCandidate(_, _, _, impl) and constraint = impl.getSelfTy())
10231005
}
10241006
}
10251007

@@ -1044,8 +1026,7 @@ private module Cached {
10441026
*/
10451027
private Function getMethodFromImpl(ReceiverExpr receiver) {
10461028
exists(Impl impl |
1047-
IsInstantiationOf<ReceiverExpr, IsInstantiationOfInput>::isInstantiationOf(receiver, impl,
1048-
impl.(ImplTypeAbstraction).getSelfTy().(TypeReprMention)) and
1029+
IsInstantiationOf<ReceiverExpr, IsInstantiationOfInput>::isInstantiationOf(receiver, impl, _) and
10491030
result = getMethodSuccessor(impl, receiver.getField())
10501031
)
10511032
}
@@ -1059,19 +1040,17 @@ private module Cached {
10591040
or
10601041
// The type of `receiver` is a type parameter and the method comes from a
10611042
// trait bound on the type parameter.
1062-
result = getTypeParameterMethod(receiver.resolveTypeAt(TypePath::nil()), receiver.getField())
1043+
result = getTypeParameterMethod(receiver.getTypeAt(TypePath::nil()), receiver.getField())
10631044
)
10641045
}
10651046

10661047
pragma[inline]
10671048
private Type inferRootTypeDeref(AstNode n) {
1068-
exists(Type t |
1069-
t = inferType(n) and
1070-
// for reference types, lookup members in the type being referenced
1071-
if t = TRefType()
1072-
then result = inferType(n, TypePath::singleton(TRefTypeParameter()))
1073-
else result = t
1074-
)
1049+
result = inferType(n) and
1050+
result != TRefType()
1051+
or
1052+
// for reference types, lookup members in the type being referenced
1053+
result = inferType(n, TypePath::singleton(TRefTypeParameter()))
10751054
}
10761055

10771056
pragma[nomagic]

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

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,4 @@
11
testFailures
2-
| main.rs:793:25:793:75 | //... | Missing result: type=a:A.S1 |
3-
| main.rs:793:25:793:75 | //... | Missing result: type=a:MyThing |
42
inferType
53
| loop/main.rs:7:12:7:15 | SelfParam | | loop/main.rs:6:1:8:1 | Self [trait T1] |
64
| loop/main.rs:11:12:11:15 | SelfParam | | loop/main.rs:10:1:14:1 | Self [trait T2] |
@@ -798,10 +796,13 @@ inferType
798796
| main.rs:788:9:788:9 | x | | main.rs:787:26:787:41 | T2 |
799797
| main.rs:788:9:788:14 | x.m1() | | main.rs:787:22:787:23 | T1 |
800798
| main.rs:791:56:791:56 | x | | main.rs:791:39:791:53 | T |
801-
| main.rs:793:13:793:13 | a | | main.rs:741:20:741:22 | Tr2 |
799+
| main.rs:793:13:793:13 | a | | main.rs:721:5:724:5 | MyThing |
800+
| main.rs:793:13:793:13 | a | A | main.rs:731:5:732:14 | S1 |
802801
| main.rs:793:17:793:17 | x | | main.rs:791:39:791:53 | T |
803-
| main.rs:793:17:793:22 | x.m1() | | main.rs:741:20:741:22 | Tr2 |
804-
| main.rs:794:26:794:26 | a | | main.rs:741:20:741:22 | Tr2 |
802+
| main.rs:793:17:793:22 | x.m1() | | main.rs:721:5:724:5 | MyThing |
803+
| main.rs:793:17:793:22 | x.m1() | A | main.rs:731:5:732:14 | S1 |
804+
| main.rs:794:26:794:26 | a | | main.rs:721:5:724:5 | MyThing |
805+
| main.rs:794:26:794:26 | a | A | main.rs:731:5:732:14 | S1 |
805806
| main.rs:798:13:798:13 | x | | main.rs:721:5:724:5 | MyThing |
806807
| main.rs:798:13:798:13 | x | A | main.rs:731:5:732:14 | S1 |
807808
| main.rs:798:17:798:33 | MyThing {...} | | main.rs:721:5:724:5 | MyThing |
@@ -856,9 +857,7 @@ inferType
856857
| main.rs:816:17:816:33 | MyThing {...} | A | main.rs:731:5:732:14 | S1 |
857858
| main.rs:816:30:816:31 | S1 | | main.rs:731:5:732:14 | S1 |
858859
| main.rs:817:13:817:13 | s | | main.rs:731:5:732:14 | S1 |
859-
| main.rs:817:13:817:13 | s | | main.rs:741:20:741:22 | Tr2 |
860860
| main.rs:817:17:817:32 | call_trait_m1(...) | | main.rs:731:5:732:14 | S1 |
861-
| main.rs:817:17:817:32 | call_trait_m1(...) | | main.rs:741:20:741:22 | Tr2 |
862861
| main.rs:817:31:817:31 | x | | main.rs:721:5:724:5 | MyThing |
863862
| main.rs:817:31:817:31 | x | A | main.rs:731:5:732:14 | S1 |
864863
| main.rs:819:13:819:13 | x | | main.rs:726:5:729:5 | MyThing2 |
@@ -867,10 +866,8 @@ inferType
867866
| main.rs:819:17:819:34 | MyThing2 {...} | A | main.rs:733:5:734:14 | S2 |
868867
| main.rs:819:31:819:32 | S2 | | main.rs:733:5:734:14 | S2 |
869868
| main.rs:820:13:820:13 | s | | main.rs:721:5:724:5 | MyThing |
870-
| main.rs:820:13:820:13 | s | | main.rs:741:20:741:22 | Tr2 |
871869
| main.rs:820:13:820:13 | s | A | main.rs:733:5:734:14 | S2 |
872870
| main.rs:820:17:820:32 | call_trait_m1(...) | | main.rs:721:5:724:5 | MyThing |
873-
| main.rs:820:17:820:32 | call_trait_m1(...) | | main.rs:741:20:741:22 | Tr2 |
874871
| main.rs:820:17:820:32 | call_trait_m1(...) | A | main.rs:733:5:734:14 | S2 |
875872
| main.rs:820:31:820:31 | x | | main.rs:726:5:729:5 | MyThing2 |
876873
| main.rs:820:31:820:31 | x | A | main.rs:733:5:734:14 | S2 |

rust/ql/test/query-tests/security/CWE-089/CONSISTENCY/PathResolutionConsistency.expected

Whitespace-only changes.

0 commit comments

Comments
 (0)