Rust: Refine implHasAmbiguousSiblingAt

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

@@ -22,17 +22,8 @@
  * how to resolve type mentions (`PreTypeMention` vs. `TypeMention`).
  */
 private module MkSiblingImpls<resolveTypeMentionAtSig/2 resolveTypeMentionAt> {
-  pragma[nomagic]
-  private Type resolveNonTypeParameterTypeAt(AstNode tm, TypePath path) {
-    result = resolveTypeMentionAt(tm, path) and
-    not result instanceof TypeParameter
-  }
-
-  bindingset[t1, t2]
-  private predicate typeMentionEqual(AstNode t1, AstNode t2) {
-    forex(TypePath path, Type type | resolveNonTypeParameterTypeAt(t1, path) = type |
-      resolveNonTypeParameterTypeAt(t2, path) = type
-    )
+  private class Tm extends AstNode {
+    Type getTypeAt(TypePath path) { result = resolveTypeMentionAt(this, path) }
   }
 
   pragma[nomagic]
@@ -50,6 +41,18 @@
     trait = impl.resolveTraitTy()
   }
 
+  private module FooIsInstantiationOfInput implements IsInstantiationOfInputSig<Tm, Tm> {
+    predicate potentialInstantiationOf(Tm cond, TypeAbstraction abs, Tm constraint) {
+      exists(TraitItemNode trait, Type rootType |
+        implSiblingCandidate(_, trait, rootType, cond) and
+        implSiblingCandidate(abs, trait, rootType, constraint) and
+        cond != constraint
+      )
+    }
+  }
+
+  private module FooIsInstantiationOf = IsInstantiationOf<Tm, Tm, FooIsInstantiationOfInput>;
+
   /**
    * Holds if `impl1` and `impl2` are sibling implementations of `trait`. We
    * consider implementations to be siblings if they implement the same trait for
@@ -59,23 +62,19 @@
    */
   pragma[inline]
   predicate implSiblings(TraitItemNode trait, Impl impl1, Impl impl2) {
-    impl1 != impl2 and
-    (
-      exists(Type rootType, AstNode selfTy1, AstNode selfTy2 |
-        implSiblingCandidate(impl1, trait, rootType, selfTy1) and
-        implSiblingCandidate(impl2, trait, rootType, selfTy2) and
-        // In principle the second conjunct below should be superfluous, but we still
-        // have ill-formed type mentions for types that we don't understand. For
-        // those checking both directions restricts further. Note also that we check
-        // syntactic equality, whereas equality up to renaming would be more
-        // correct.
-        typeMentionEqual(selfTy1, selfTy2) and
-        typeMentionEqual(selfTy2, selfTy1)
-      )
-      or
-      blanketImplSiblingCandidate(impl1, trait) and
-      blanketImplSiblingCandidate(impl2, trait)
+    // impl1.fromSource() and
+    // impl1 instanceof Builtins::BuiltinImpl and
+    exists(Type rootType, AstNode selfTy1, AstNode selfTy2 |
+      implSiblingCandidate(impl1, trait, rootType, selfTy1) and
+      implSiblingCandidate(impl2, trait, rootType, selfTy2)
+    |
+      FooIsInstantiationOf::isInstantiationOf(selfTy1, impl2, selfTy2) or
+      FooIsInstantiationOf::isInstantiationOf(selfTy2, impl1, selfTy1)
     )
+    or
+    blanketImplSiblingCandidate(impl1, trait) and
+    blanketImplSiblingCandidate(impl2, trait) and
+    impl1 != impl2
   }
 
   /**
@@ -86,24 +85,27 @@
   predicate implHasSibling(ImplItemNode impl, Trait trait) { implSiblings(trait, impl, _) }
 
   pragma[nomagic]
-  predicate implHasAmbiguousSiblingAt(ImplItemNode impl, Trait trait, TypePath path) {
-    exists(ImplItemNode impl2, Type t1, Type t2 |
+  predicate implHasAmbiguousSiblingAt(
+    ImplItemNode impl, ImplItemNode impl2, Trait trait, TypePath path
+  ) {
+    // impl instanceof Builtins::BuiltinImpl and
+    exists(Type t1, Type t2 |
       implSiblings(trait, impl, impl2) and
       t1 = resolveTypeMentionAt(impl.getTraitPath(), path) and
       t2 = resolveTypeMentionAt(impl2.getTraitPath(), path) and
       t1 != t2
     |
       not t1 instanceof TypeParameter or
       not t2 instanceof TypeParameter
     )
     or
     // todo: handle blanket/non-blanket siblings in `implSiblings`
     trait =
       any(IndexTrait it |
         implSiblingCandidate(impl, it, _, _) and
         impl instanceof Builtins::BuiltinImpl and
         path = TypePath::singleton(TAssociatedTypeTypeParameter(trait, it.getOutputType()))
       )
   }
 }
 
@@ -113,7 +115,7 @@
 
 private module PreSiblingImpls = MkSiblingImpls<resolvePreTypeMention/2>;
 
-predicate preImplHasAmbiguousSiblingAt = PreSiblingImpls::implHasAmbiguousSiblingAt/3;
+predicate preImplHasAmbiguousSiblingAt = PreSiblingImpls::implHasAmbiguousSiblingAt/4;
 
 private Type resolveTypeMention(AstNode tm, TypePath path) {
   result = tm.(TypeMention).getTypeAt(path)
@@ -152,19 +154,14 @@
    * ```rust
    * trait MyTrait<T> {
    *     fn method(&self, value: Foo<T>) -> Self;
-   * //                   ^^^^^^^^^^^^^ `pos` = 0
+   * //                   ^^^^^^^^^^^^^ `pos` = 1
    * //                              ^ `path` = "T"
    * }
    * impl MyAdd<i64> for i64 {
    *     fn method(&self, value: Foo<i64>) -> Self { ... }
    * //                              ^^^ `type` = i64
    * }
    * ```
-   *
-   * Note that we only check the root type symbol at the position. If the type
-   * at that position is a type constructor (for instance `Vec<..>`) then
-   * inspecting the entire type tree could be necessary to disambiguate the
-   * method. In that case we will still resolve several methods.
    */
 
   exists(TraitItemNode trait |
@@ -262,6 +259,7 @@
 predicate functionResolutionDependsOnArgument(
   ImplItemNode impl, Function f, TypeParameter traitTp, FunctionPosition pos
 ) {
+  // impl.fromSource() and
   exists(string functionName |
     functionResolutionDependsOnArgumentCand(impl, f, functionName, traitTp, pos, _)
   |

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

@@ -37,6 +37,11 @@ private module Input1 implements InputSig1<Location> {
 
   class Type = T::Type;
 
+  predicate isPseudoType(Type t) {
+    t instanceof UnknownType or
+    t instanceof NeverType
+  }
+
   class TypeParameter = T::TypeParameter;
 
   class TypeAbstraction = TA::TypeAbstraction;
@@ -230,9 +235,10 @@ private module PreInput2 implements InputSig2<PreTypeMention> {
   }
 
   predicate typeAbstractionHasAmbiguousConstraintAt(
-    TypeAbstraction abs, Type constraint, TypePath path
+    TypeAbstraction abs, Type constraint, TypeAbstraction other, TypePath path
   ) {
-    FunctionOverloading::preImplHasAmbiguousSiblingAt(abs, constraint.(TraitType).getTrait(), path)
+    FunctionOverloading::preImplHasAmbiguousSiblingAt(abs, other, constraint.(TraitType).getTrait(),
+      path)
   }
 
   predicate typeParameterIsFunctionallyDetermined =
@@ -256,9 +262,10 @@ private module Input2 implements InputSig2<TypeMention> {
   }
 
   predicate typeAbstractionHasAmbiguousConstraintAt(
-    TypeAbstraction abs, Type constraint, TypePath path
+    TypeAbstraction abs, Type constraint, TypeAbstraction other, TypePath path
   ) {
-    FunctionOverloading::implHasAmbiguousSiblingAt(abs, constraint.(TraitType).getTrait(), path)
+    FunctionOverloading::implHasAmbiguousSiblingAt(abs, other, constraint.(TraitType).getTrait(),
+      path)
   }
 
   predicate typeParameterIsFunctionallyDetermined =
@@ -1925,6 +1932,17 @@ private module AssocFunctionResolution {
       )
     }
 
+    // private AssocFunctionDeclaration testresolveCallTarget(
+    //   ImplOrTraitItemNode i, FunctionPosition selfPos, DerefChain derefChain, BorrowKind borrow,
+    //   FunctionPosition pos, TypePath path, Type t
+    // ) {
+    //   this = Debug::getRelevantLocatable() and
+    //   exists(AssocFunctionCallCand afcc |
+    //     afcc = MkAssocFunctionCallCand(this, selfPos, derefChain, borrow) and
+    //     result = afcc.resolveCallTarget(i) and
+    //     t = result.getParameterType(any(ImplOrTraitItemNodeOption o | o.asSome() = i), pos, path)
+    //   )
+    // }
     /**
      * Holds if the argument `arg` of this call has been implicitly dereferenced
      * and borrowed according to `derefChain` and `borrow`, in order to be able to
@@ -3942,7 +3960,7 @@ private module Debug {
     exists(string filepath, int startline, int startcolumn, int endline, int endcolumn |
       result.getLocation().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn) and
       filepath.matches("%/main.rs") and
-      startline = 103
+      startline = 441
     )
   }
 

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

@@ -15818,18 +15818,12 @@ inferType
 | regressions.rs:150:24:153:5 | { ... } |  | regressions.rs:136:5:136:22 | S2 |
 | regressions.rs:150:24:153:5 | { ... } | T2 | regressions.rs:135:5:135:14 | S1 |
 | regressions.rs:151:13:151:13 | x |  | regressions.rs:136:5:136:22 | S2 |
-| regressions.rs:151:13:151:13 | x | T2 | {EXTERNAL LOCATION} | & |
 | regressions.rs:151:13:151:13 | x | T2 | regressions.rs:135:5:135:14 | S1 |
-| regressions.rs:151:13:151:13 | x | T2.TRef | regressions.rs:135:5:135:14 | S1 |
 | regressions.rs:151:17:151:18 | S1 |  | regressions.rs:135:5:135:14 | S1 |
 | regressions.rs:151:17:151:25 | S1.into() |  | regressions.rs:136:5:136:22 | S2 |
-| regressions.rs:151:17:151:25 | S1.into() | T2 | {EXTERNAL LOCATION} | & |
 | regressions.rs:151:17:151:25 | S1.into() | T2 | regressions.rs:135:5:135:14 | S1 |
-| regressions.rs:151:17:151:25 | S1.into() | T2.TRef | regressions.rs:135:5:135:14 | S1 |
 | regressions.rs:152:9:152:9 | x |  | regressions.rs:136:5:136:22 | S2 |
-| regressions.rs:152:9:152:9 | x | T2 | {EXTERNAL LOCATION} | & |
 | regressions.rs:152:9:152:9 | x | T2 | regressions.rs:135:5:135:14 | S1 |
-| regressions.rs:152:9:152:9 | x | T2.TRef | regressions.rs:135:5:135:14 | S1 |
 | regressions.rs:164:16:164:19 | SelfParam |  | regressions.rs:158:5:158:19 | S |
 | regressions.rs:164:16:164:19 | SelfParam | T | regressions.rs:160:10:160:10 | T |
 | regressions.rs:164:22:164:25 | _rhs |  | regressions.rs:158:5:158:19 | S |
@@ -15861,3 +15855,4 @@ inferType
 | regressions.rs:179:24:179:27 | S(...) | T | {EXTERNAL LOCATION} | i32 |
 | regressions.rs:179:26:179:26 | 1 |  | {EXTERNAL LOCATION} | i32 |
 testFailures
+| regressions.rs:152:11:152:127 | //... | Fixed spurious result: type=x:T2.TRef.S1 |