Merge pull request #21043 from hvitved/rust/type-inference-trait-bounds-overlap

Rust: Fix candidate receiver type calculation for trait bounds

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

@@ -1572,20 +1572,18 @@ private module MethodResolution {
     }
 
     /**
-     * Same as `getACandidateReceiverTypeAt`, but with traits substituted in for types
-     * with trait bounds.
+     * Same as `getACandidateReceiverTypeAt`, but excludes pseudo types `!` and `unknown`.
      */
     pragma[nomagic]
-    Type getACandidateReceiverTypeAtSubstituteLookupTraits(
-      string derefChain, boolean borrow, TypePath path
-    ) {
-      result = substituteLookupTraits(this.getACandidateReceiverTypeAt(derefChain, borrow, path))
+    Type getANonPseudoCandidateReceiverTypeAt(string derefChain, boolean borrow, TypePath path) {
+      result = this.getACandidateReceiverTypeAt(derefChain, borrow, path) and
+      result != TNeverType() and
+      result != TUnknownType()
     }
 
     pragma[nomagic]
     private Type getComplexStrippedType(string derefChain, boolean borrow, TypePath strippedTypePath) {
-      result =
-        this.getACandidateReceiverTypeAtSubstituteLookupTraits(derefChain, borrow, strippedTypePath) and
+      result = this.getANonPseudoCandidateReceiverTypeAt(derefChain, borrow, strippedTypePath) and
       isComplexRootStripped(strippedTypePath, result)
     }
 
@@ -1624,12 +1622,11 @@ private module MethodResolution {
       )
     }
 
-    /**
-     * Holds if the candidate receiver type represented by `derefChain` does not
-     * have a matching method target.
-     */
+    // forex using recursion
     pragma[nomagic]
-    predicate hasNoCompatibleTargetNoBorrow(string derefChain) {
+    private predicate hasNoCompatibleTargetNoBorrowToIndex(
+      string derefChain, TypePath strippedTypePath, Type strippedType, int n
+    ) {
       (
         this.supportsAutoDerefAndBorrow()
         or
@@ -1637,10 +1634,46 @@ private module MethodResolution {
         // `ReceiverSatisfiesBlanketLikeConstraintInput::hasBlanketCandidate`
         derefChain = ""
       ) and
-      exists(TypePath strippedTypePath, Type strippedType |
-        not derefChain.matches("%.ref") and // no need to try a borrow if the last thing we did was a deref
-        strippedType = this.getComplexStrippedType(derefChain, false, strippedTypePath) and
-        this.hasNoCompatibleTargetCheck(derefChain, false, strippedTypePath, strippedType)
+      strippedType = this.getComplexStrippedType(derefChain, false, strippedTypePath) and
+      n = -1
+      or
+      this.hasNoCompatibleTargetNoBorrowToIndex(derefChain, strippedTypePath, strippedType, n - 1) and
+      exists(Type t | t = getNthLookupType(strippedType, n) |
+        this.hasNoCompatibleTargetCheck(derefChain, false, strippedTypePath, t)
+      )
+    }
+
+    /**
+     * Holds if the candidate receiver type represented by `derefChain` does not
+     * have a matching method target.
+     */
+    pragma[nomagic]
+    predicate hasNoCompatibleTargetNoBorrow(string derefChain) {
+      exists(Type strippedType |
+        this.hasNoCompatibleTargetNoBorrowToIndex(derefChain, _, strippedType,
+          getLastLookupTypeIndex(strippedType))
+      )
+    }
+
+    // forex using recursion
+    pragma[nomagic]
+    private predicate hasNoCompatibleNonBlanketTargetNoBorrowToIndex(
+      string derefChain, TypePath strippedTypePath, Type strippedType, int n
+    ) {
+      (
+        this.supportsAutoDerefAndBorrow()
+        or
+        // needed for the `hasNoCompatibleTarget` check in
+        // `ReceiverSatisfiesBlanketLikeConstraintInput::hasBlanketCandidate`
+        derefChain = ""
+      ) and
+      strippedType = this.getComplexStrippedType(derefChain, false, strippedTypePath) and
+      n = -1
+      or
+      this.hasNoCompatibleNonBlanketTargetNoBorrowToIndex(derefChain, strippedTypePath,
+        strippedType, n - 1) and
+      exists(Type t | t = getNthLookupType(strippedType, n) |
+        this.hasNoCompatibleNonBlanketTargetCheck(derefChain, false, strippedTypePath, t)
       )
     }
 
@@ -1650,17 +1683,24 @@ private module MethodResolution {
      */
     pragma[nomagic]
     predicate hasNoCompatibleNonBlanketTargetNoBorrow(string derefChain) {
-      (
-        this.supportsAutoDerefAndBorrow()
-        or
-        // needed for the `hasNoCompatibleTarget` check in
-        // `ReceiverSatisfiesBlanketLikeConstraintInput::hasBlanketCandidate`
-        derefChain = ""
-      ) and
-      exists(TypePath strippedTypePath, Type strippedType |
-        not derefChain.matches("%.ref") and // no need to try a borrow if the last thing we did was a deref
-        strippedType = this.getComplexStrippedType(derefChain, false, strippedTypePath) and
-        this.hasNoCompatibleNonBlanketTargetCheck(derefChain, false, strippedTypePath, strippedType)
+      exists(Type strippedType |
+        this.hasNoCompatibleNonBlanketTargetNoBorrowToIndex(derefChain, _, strippedType,
+          getLastLookupTypeIndex(strippedType))
+      )
+    }
+
+    // forex using recursion
+    pragma[nomagic]
+    private predicate hasNoCompatibleTargetBorrowToIndex(
+      string derefChain, TypePath strippedTypePath, Type strippedType, int n
+    ) {
+      this.hasNoCompatibleTargetNoBorrow(derefChain) and
+      strippedType = this.getComplexStrippedType(derefChain, true, strippedTypePath) and
+      n = -1
+      or
+      this.hasNoCompatibleTargetBorrowToIndex(derefChain, strippedTypePath, strippedType, n - 1) and
+      exists(Type t | t = getNthLookupType(strippedType, n) |
+        this.hasNoCompatibleNonBlanketLikeTargetCheck(derefChain, true, strippedTypePath, t)
       )
     }
 
@@ -1670,11 +1710,25 @@ private module MethodResolution {
      */
     pragma[nomagic]
     predicate hasNoCompatibleTargetBorrow(string derefChain) {
-      exists(TypePath strippedTypePath, Type strippedType |
-        this.hasNoCompatibleTargetNoBorrow(derefChain) and
-        strippedType = this.getComplexStrippedType(derefChain, true, strippedTypePath) and
-        this.hasNoCompatibleNonBlanketLikeTargetCheck(derefChain, true, strippedTypePath,
-          strippedType)
+      exists(Type strippedType |
+        this.hasNoCompatibleTargetBorrowToIndex(derefChain, _, strippedType,
+          getLastLookupTypeIndex(strippedType))
+      )
+    }
+
+    // forex using recursion
+    pragma[nomagic]
+    private predicate hasNoCompatibleNonBlanketTargetBorrowToIndex(
+      string derefChain, TypePath strippedTypePath, Type strippedType, int n
+    ) {
+      this.hasNoCompatibleTargetNoBorrow(derefChain) and
+      strippedType = this.getComplexStrippedType(derefChain, true, strippedTypePath) and
+      n = -1
+      or
+      this.hasNoCompatibleNonBlanketTargetBorrowToIndex(derefChain, strippedTypePath, strippedType,
+        n - 1) and
+      exists(Type t | t = getNthLookupType(strippedType, n) |
+        this.hasNoCompatibleNonBlanketTargetCheck(derefChain, true, strippedTypePath, t)
       )
     }
 
@@ -1684,10 +1738,9 @@ private module MethodResolution {
      */
     pragma[nomagic]
     predicate hasNoCompatibleNonBlanketTargetBorrow(string derefChain) {
-      exists(TypePath strippedTypePath, Type strippedType |
-        this.hasNoCompatibleTargetNoBorrow(derefChain) and
-        strippedType = this.getComplexStrippedType(derefChain, true, strippedTypePath) and
-        this.hasNoCompatibleNonBlanketTargetCheck(derefChain, true, strippedTypePath, strippedType)
+      exists(Type strippedType |
+        this.hasNoCompatibleNonBlanketTargetBorrowToIndex(derefChain, _, strippedType,
+          getLastLookupTypeIndex(strippedType))
       )
     }
 
@@ -1905,9 +1958,8 @@ private module MethodResolution {
     MethodCall getMethodCall() { result = mc_ }
 
     Type getTypeAt(TypePath path) {
-      result = mc_.getACandidateReceiverTypeAtSubstituteLookupTraits(derefChain, borrow, path) and
-      not result = TNeverType() and
-      not result = TUnknownType()
+      result =
+        substituteLookupTraits(mc_.getANonPseudoCandidateReceiverTypeAt(derefChain, borrow, path))
     }
 
     pragma[nomagic]

rust/ql/lib/codeql/rust/internal/typeinference/FunctionType.qll

@@ -194,6 +194,7 @@ class AssocFunctionType extends MkAssocFunctionType {
   Location getLocation() { result = this.getTypeMention().getLocation() }
 }
 
+pragma[nomagic]
 private Trait getALookupTrait(Type t) {
   result = t.(TypeParamTypeParameter).getTypeParam().(TypeParamItemNode).resolveABound()
   or
@@ -208,7 +209,7 @@ private Trait getALookupTrait(Type t) {
  * Gets the type obtained by substituting in relevant traits in which to do function
  * lookup, or `t` itself when no such trait exist.
  */
-bindingset[t]
+pragma[nomagic]
 Type substituteLookupTraits(Type t) {
   not exists(getALookupTrait(t)) and
   result = t
@@ -216,6 +217,30 @@ Type substituteLookupTraits(Type t) {
   result = TTrait(getALookupTrait(t))
 }
 
+/**
+ * Gets the `n`th `substituteLookupTraits` type for `t`, per some arbitrary order.
+ */
+pragma[nomagic]
+Type getNthLookupType(Type t, int n) {
+  not exists(getALookupTrait(t)) and
+  result = t and
+  n = 0
+  or
+  result =
+    TTrait(rank[n + 1](Trait trait, int i |
+        trait = getALookupTrait(t) and
+        i = idOfTypeParameterAstNode(trait)
+      |
+        trait order by i
+      ))
+}
+
+/**
+ * Gets the index of the last `substituteLookupTraits` type for `t`.
+ */
+pragma[nomagic]
+int getLastLookupTypeIndex(Type t) { result = max(int n | exists(getNthLookupType(t, n))) }
+
 /**
  * A wrapper around `IsInstantiationOf` which ensures to substitute in lookup
  * traits when checking whether argument types are instantiations of function

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

@@ -13,23 +13,23 @@ multipleResolvedTargets
 | dyn_type.rs:90:10:90:13 | * ... |
 | invalid/main.rs:69:13:69:17 | * ... |
 | invalid/main.rs:76:13:76:17 | * ... |
-| main.rs:1077:14:1077:18 | * ... |
-| main.rs:1159:26:1159:30 | * ... |
-| main.rs:1503:14:1503:21 | * ... |
-| main.rs:1503:16:1503:20 | * ... |
-| main.rs:1508:14:1508:18 | * ... |
-| main.rs:1539:27:1539:29 | * ... |
-| main.rs:1653:17:1653:24 | * ... |
-| main.rs:1653:18:1653:24 | * ... |
-| main.rs:1791:17:1791:21 | * ... |
-| main.rs:1806:28:1806:32 | * ... |
-| main.rs:2439:13:2439:18 | * ... |
-| main.rs:2633:13:2633:31 | ...::from(...) |
-| main.rs:2634:13:2634:31 | ...::from(...) |
-| main.rs:2635:13:2635:31 | ...::from(...) |
-| main.rs:2641:13:2641:31 | ...::from(...) |
-| main.rs:2642:13:2642:31 | ...::from(...) |
-| main.rs:2643:13:2643:31 | ...::from(...) |
-| main.rs:3072:13:3072:17 | x.f() |
+| main.rs:1092:14:1092:18 | * ... |
+| main.rs:1174:26:1174:30 | * ... |
+| main.rs:1518:14:1518:21 | * ... |
+| main.rs:1518:16:1518:20 | * ... |
+| main.rs:1523:14:1523:18 | * ... |
+| main.rs:1554:27:1554:29 | * ... |
+| main.rs:1668:17:1668:24 | * ... |
+| main.rs:1668:18:1668:24 | * ... |
+| main.rs:1806:17:1806:21 | * ... |
+| main.rs:1821:28:1821:32 | * ... |
+| main.rs:2454:13:2454:18 | * ... |
+| main.rs:2648:13:2648:31 | ...::from(...) |
+| main.rs:2649:13:2649:31 | ...::from(...) |
+| main.rs:2650:13:2650:31 | ...::from(...) |
+| main.rs:2656:13:2656:31 | ...::from(...) |
+| main.rs:2657:13:2657:31 | ...::from(...) |
+| main.rs:2658:13:2658:31 | ...::from(...) |
+| main.rs:3087:13:3087:17 | x.f() |
 | pattern_matching.rs:273:13:273:27 | * ... |
 | pattern_matching.rs:273:14:273:27 | * ... |

rust/ql/test/library-tests/type-inference/blanket_impl.rs

@@ -236,7 +236,7 @@ mod blanket_like_impl {
     impl MyTrait2 for &&S1 {
         // MyTrait2RefRefS1::m2
         fn m2(self) {
-            self.m1() // $ MISSING: target=S1::m1
+            self.m1() // $ target=S1::m1
         }
     }
 

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

@@ -827,6 +827,21 @@ mod function_trait_bounds {
         }
     }
 
+    trait MyTrait2 {
+        // MyTrait2::m2
+        fn m2(self);
+    }
+
+    trait MyTrait3 {
+        // MyTrait3::m2
+        fn m2(&self);
+    }
+
+    fn bound_overlap<T: MyTrait2 + MyTrait3>(x: T, y: &T) {
+        x.m2(); // $ target=MyTrait2::m2
+        y.m2(); // $ target=MyTrait3::m2
+    }
+
     pub fn f() {
         let x = MyThing { a: S1 };
         let y = MyThing { a: S2 };