Merge pull request #59339 from eeckstein/improve-diagnose-inf-recursion

DiagnoseInfiniteRecursion: improve detection in (potentially) overridden class methods

Author: eeckstein

lib/SILOptimizer/Mandatory/DiagnoseInfiniteRecursion.cpp

@@ -71,31 +71,50 @@ static bool isRecursiveCall(FullApplySite applySite) {
 
   if (auto *CMI = dyn_cast<ClassMethodInst>(callee)) {
 
+    SILModule &module = parentFunc->getModule();
+    CanType classType = CMI->getOperand()->getType().getASTType();
+    if (auto mt = dyn_cast<MetatypeType>(classType)) {
+      classType = mt.getInstanceType();
+    }
+    ClassDecl *classDecl = classType.getClassOrBoundGenericClass();
+
     // FIXME: If we're not inside the module context of the method,
     // we may have to deserialize vtables.  If the serialized tables
     // are damaged, the pass will crash.
     //
     // Though, this has the added bonus of not looking into vtables
     // outside the current module.  Because we're not doing IPA, let
     // alone cross-module IPA, this is all well and good.
-    SILModule &module = parentFunc->getModule();
-    CanType classType = CMI->getOperand()->getType().getASTType();
-    ClassDecl *classDecl = classType.getClassOrBoundGenericClass();
     if (classDecl && classDecl->getModuleContext() != module.getSwiftModule())
       return false;
 
-    if (!calleesAreStaticallyKnowable(module, CMI->getMember()))
+    SILFunction *method = getTargetClassMethod(module, classDecl, CMI);
+    if (method != parentFunc)
       return false;
 
-    // The "statically knowable" check just means that we have all the
-    // callee candidates available for analysis. We still need to check
-    // if the current function has a known override point.
-    auto *methodDecl = CMI->getMember().getAbstractFunctionDecl();
-    if (methodDecl->isOverridden())
-      return false;
+    SILDeclRef member = CMI->getMember();
+    if (calleesAreStaticallyKnowable(module, member) &&
+        // The "statically knowable" check just means that we have all the
+        // callee candidates available for analysis. We still need to check
+        // if the current function has a known override point.
+        !member.getAbstractFunctionDecl()->isOverridden()) {
+      return true;
+    }
 
-    SILFunction *method = getTargetClassMethod(module, classDecl, CMI);
-    return method == parentFunc;
+    // Even if the method is (or could be) overridden, it's a recursive call if
+    // it's called on the self argument:
+    // ```
+    // class X {
+    //   // Even if foo() is overridden in a derived class, it'll end up in an
+    //   // infinite recursion if initially called on an instance of `X`.
+    //   func foo() { foo() }
+    // }
+    // ```
+    if (parentFunc->hasSelfParam() &&
+        CMI->getOperand() == SILValue(parentFunc->getSelfArgument())) {
+      return true;
+    }
+    return false;
   }
 
   if (auto *WMI = dyn_cast<WitnessMethodInst>(callee)) {

test/SILOptimizer/infinite_recursion.swift

@@ -242,11 +242,11 @@ class S {
     return a() // expected-warning {{function call causes an infinite recursion}}
   }
 
-  func b() { // No warning - has a known override.
+  func b() {
     var i = 0
     repeat {
       i += 1
-      b()
+      b() // expected-warning {{function call causes an infinite recursion}}
     } while (i > 5)
   }
 
@@ -273,6 +273,12 @@ class T: S {
   }
 }
 
+public class U {
+  required convenience init(x: Int) {
+    self.init(x: x) // expected-warning {{function call causes an infinite recursion}}
+  }
+}
+
 func == (l: S?, r: S?) -> Bool {
   if l == nil && r == nil { return true } // expected-warning {{function call causes an infinite recursion}}
   guard let l = l, let r = r else { return false }