Skip to content

Conversation

@ileitch
Copy link
Contributor

@ileitch ileitch commented Dec 18, 2025

Constructors called via Self.NestedType() weren't being indexed because the AST uses DotSyntaxCallExpr (not ConstructorRefCallExpr), causing the implicit DeclRefExpr to the constructor to be skipped.

Fix by:

  1. Not skipping implicit constructor DeclRefExpr when CtorRefs is empty (i.e., when not already handled by ConstructorRefCallExpr)
  2. Unwrapping AutoClosureExpr/CallExpr in isBeingCalled() to correctly detect the call context and add the Call role

Constructors called via Self.NestedType() weren't being indexed because
the AST uses DotSyntaxCallExpr (not ConstructorRefCallExpr), causing the
implicit DeclRefExpr to the constructor to be skipped.

Fix by:
1. Not skipping implicit constructor DeclRefExpr when CtorRefs is empty
   (i.e., when not already handled by ConstructorRefCallExpr)
2. Unwrapping AutoClosureExpr/CallExpr in isBeingCalled() to correctly
   detect the call context and add the Call role
Expr *Fn = AE->getFn();
// Unwrap AutoClosureExpr and nested CallExpr to get the actual function
// expression. This handles cases like Self.NestedType() where the
// constructor call is wrapped in an autoclosure containing another call.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a bug that these calls are wrapped in an autoclosure. We can land this as a workaround, but do you want to investigate and fix the underlying issue?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps the problem is somewhere in shouldBuildCurryThunk() in CSApply.cpp. It has this weird bit of code in it which suggests something is not being modeled properly with constructors:

        // FIXME: Representational gunk because "T(...)" is really
        // "T.init(...)" -- pretend it has two argument lists like
        // a real '.' call.
        if (isa<ConstructorDecl>(member) &&
            isa<CallExpr>(prev) &&
            isa<TypeExpr>(cast<CallExpr>(prev)->getFn())) {
          assert(maxArgCount == 2);
          return 2;
        }

Or maybe the problem is even further upstream. Really, Self.NestedType should be identical to Container.NestedType, because the special meaning of Self inside a class type should not have any effect for a nested type reference.

Nice discovery, by the way, this is really bizarre.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I looked into this and there are two issues:

  • We don't fold Self.NestedType in the same way as Container.NestedType
  • We form an autoclosure erroneously in other cases

I have a fix for the first problem coming up, which eliminates the autoclosure. I'll see if I can fix the second as well.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is the fix for the underlying problem: #86146

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants