rednaxelafx commented on a change in pull request #28463:
URL: https://github.com/apache/spark/pull/28463#discussion_r421973199



##########
File path: core/src/main/scala/org/apache/spark/util/ClosureCleaner.scala
##########
@@ -538,43 +604,56 @@ private[spark] object IndylambdaScalaClosures extends 
Logging {
 
         override def visitMethodInsn(
             op: Int, owner: String, name: String, desc: String, itf: Boolean): 
Unit = {
-          if (owner == implClassInternalName) {
-            val ownerExternalName = owner.replace('/', '.')
+          val ownerExternalName = owner.replace('/', '.')
+          if (owner == currentClassInternalName) {
             logTrace(s"    found intra class call to 
$ownerExternalName.$name$desc")
-            val calleeMethodId = MethodIdentifier(implClass, name, desc)
-            if (!visited.contains(calleeMethodId)) {
-              stack.push(calleeMethodId)
+            // could be invoking a helper method or a field accessor method, 
just follow it.
+            pushIfNotVisited(MethodIdentifier(currentClass, name, desc))
+          } else if (isInnerClassCtorCapturingOuter(
+              op, owner, name, desc, currentClassInternalName)) {
+            // Discover inner classes.
+            // This this the InnerClassFinder equivalent for inner classes, 
which still use the
+            // `$outer` chain. So this is NOT controlled by the 
`findTransitively` flag.
+            logTrace(s"    found inner class $ownerExternalName")
+            val innerClassInfo = getOrUpdateClassInfo(owner)
+            val innerClass = innerClassInfo._1
+            val innerClassNode = innerClassInfo._2
+            trackedClasses += innerClass
+            // We need to visit all methods on the inner class so that we 
don't missing anything.
+            for (m <- innerClassNode.methods.asScala) {
+              pushIfNotVisited(MethodIdentifier(innerClass, m.name, m.desc))
             }
+          } else if (findTransitively &&
+              trackedClasses.find(_.getName == ownerExternalName).isDefined) {

Review comment:
       Thanks! It would indeed. I had other uses for the trackedClasses but 
they were gone later and I had forgotten to put this set back...




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

Reply via email to