[Question] What should `CallDecl` inherit from?

[dan-zheng]

Hello AST experts,

I'm implementing callables, as proposed in this pitch. The pitch introduces a new call declaration for declaring callable methods: call declarations are function-like, similar to the existing subscript and init declarations.

We're aware that adding a new declaration kind is controversial. But unless it's a non-starter due to insurmountable issues, we'd like to explore that design.

---

Implementation question: should the new CallDecl inherit from FuncDecl or AbstractFunctionDecl?

Looking at other function-like declaration kinds: AccessorDecl inherits from FuncDecl, but ConstructorDecl does not. (SubscriptDecl is not really comparable: it inherits from AbstractStorageDecl and contains AccessorDecls.)

The pro of conforming CallDecl to FuncDecl is code-sharing: there's no need to reimplement parts of SILGen/IRGen for CallDecl.

However, FuncDecl stores extra info unused by CallDecl, which is not efficient.

[dan-zheng]

cc'ing some pitch thread participants:
@Slava_Pestov @jrose @Joe_Groff

[Slava_Pestov]

Which parts of FuncDecl are unused? I think inheriting from FuncDecl is your best bet because a CallDecl is most similar to a function, it just has a funny name.

[John_McCall]

Agreed, and you might consider just using FuncDecl unless there's actually something else to the storage.

[dan-zheng]

CallDecl cannot be static (storing StaticLoc and StaticSpellingKind are unnecessary).

Though yes, this is a negligible difference. Would reusing FuncDecl and adding a IsCallDecl bit be preferable? Let me know and I can pivot to that.

---

EDIT: To clarify, please reply if you feel that reusing FuncDecl (and adding a IsCallDecl bit) is the way to go. (cc @John_McCall, since you brought up this idea)

For now, I'll continue with the current implementation, with a new CallDecl class inheriting from FuncDecl.

[jrose (Jordan Rose)]

I like it being a separate decl kind better if it's really going to be a new decl kind at the language level. That also allows us to control what attributes can appear on one more easily.

[Slava_Pestov]

@dan-zheng

CallDecl cannot be static (storing StaticLoc and StaticSpellingKind are unnecessary).

This sounds like an aesthetic decision and not an inherent limitation. You could conceivably allow static call on metatypes in the future.

@dan-zheng

Though yes, this is a negligible difference. Would reusing FuncDecl and adding a IsCallDecl bit be preferable? Let me know and I can pivot to that.

This actually sounds isomorphic to making CallDecl a subclass of FuncDecl. Is there a real difference between isa<CallDecl>(D) vs D->isCallDecl()? I would personally prefer the subclass approach; since AccessorDecl also inherits from FuncDecl, and an AccessorDecl with IsCallDecl set to true is not a valid state. Adding the new subclass defines away the invalid state.

[dan-zheng]

@Slava_Pestov

This actually sounds isomorphic to making CallDecl a subclass of FuncDecl. Is there a real difference between isa<CallDecl>(D) vs D->isCallDecl() ?

Aha, the isomorphism and invalid state possibility are good points.

It seems there's consensus on the subclass approach.

[Slava_Pestov]

If I may, I have another implementation-level suggestion. So far we have three places in our code where given a nominal type, we wish to visit all of its superclasses, conforming protocols, and any protocols they refine, and so on:

• CSSimplify.cpp, getDynamicCallableMethods()
• CSSimplify.cpp, hasDynamicMemberLookupAttribute()
• NameLookup.cpp, lookupQualified()

(There might be others actually; these are the three I'm aware of).

I'm assuming you'll be adding a fourth, to find your CallDecls for a given base type. It would be nice to extract this into a common utility; perhaps a method on DeclContext that takes an llvm::function_ref() tp call at each step would suffice.

[dan-zheng]

@Slava_Pestov

I'm assuming you'll be adding a fourth, to find your CallDecls for a given base type. It would be nice to extract this into a common utility; perhaps a method on DeclContext that takes an llvm::function_ref() tp call at each step would suffice.

Thanks for the suggestion, I'll keep this in mind!

I've started by duping the code for getCallDecls, but I'm interested in unifying the code. May do it in a follow-up PR if it's too involved.

[dan-zheng]

I ran into a problem with CallDecl subclassing FuncDecl and would appreciate advice.

call declarations are unnamed (much like subscript and init).
It seems sensible to add new special cases DeclBaseName::Kind::Call and DeclBaseName::createCall(), for call declarations.

However, if that is done, constructing CallDecl triggers an assertion in FuncDecl's constructor and FuncDecl::getName():

  FuncDecl(DeclKind Kind,
           SourceLoc StaticLoc, StaticSpellingKind StaticSpelling,
           SourceLoc FuncLoc,
           DeclName Name, SourceLoc NameLoc,
           bool Throws, SourceLoc ThrowsLoc,
           bool HasImplicitSelfDecl,
           GenericParamList *GenericParams, DeclContext *Parent)
    : AbstractFunctionDecl(Kind, Parent,
                           Name, NameLoc,
                           Throws, ThrowsLoc,
                           HasImplicitSelfDecl, GenericParams),
      StaticLoc(StaticLoc), FuncLoc(FuncLoc) {
    assert(!Name.getBaseName().isSpecial());
    // ^ Assertion triggered.
    // `DeclBaseName::createCall` is special.
  }

  Identifier getName() const { return getFullName().getBaseIdentifier(); }
  // ^ Crashes; `getBaseIdentifier` asserts that base name is not special.

What's the best solution here?

One idea is to use DeclBaseName(Identifier("call")) instead of a special case - but then this makes call declarations conflict with func `call` , which we need to support.

One solution is to make CallDecl inherit from AbstractFunctionDecl (as does ConstructorDecl) instead of FuncDecl - code sharing with FuncDecl is lost, but that seems kind of okay.

Advice would be appreciated!

[jrose (Jordan Rose)]

You could just add || Kind == DeclKind::Call to that assertion, but I'd add a tally to "things that push away from sharing with FuncDecl".

[dan-zheng]

@jrose

You could just add || Kind == DeclKind::Call to that assertion, but I'd add a tally to "things that push away from sharing with FuncDecl".

Yes. The bigger problem lies in DeclBaseName::getBaseIdentifier, called in FuncDecl::getName:

  /// Return the identifier backing the name. Assumes that the name is not
  /// special.
  Identifier getIdentifier() const {
    assert(!isSpecial() && "Cannot retrieve identifier from special names");
    return Ident;
    // `Ident` is not set for special names.
    // Not sure how to construct `Identifier("call")` without access to
    // an `ASTContext`.
  }

Not sure what's the best fix, this blocks CallDecl inheriting from FuncDecl. I'm using a workaround for now to continue the implementation.

Let me know if you have ideas for a workaround (so that CallDecl can still inherit from FuncDecl), or if you feel I should go ahead and change CallDecl to inherit from AbstractFunctionDecl.

[jrose (Jordan Rose)]

That's another tally mark. You could return an empty Identifier from FuncDecl::getName, like an accessor, but at some point you're going to keep getting in trouble with the rest of the compiler that expects FuncDecls to have names. (Mangling?)

[dan-zheng]

@jrose

That's another tally mark. You could return an empty Identifier from FuncDecl::getName , like an accessor, but at some point you're going to keep getting in trouble with the rest of the compiler that expects FuncDecls to have names. (Mangling?)

Those are good points. Shall I go ahead and change CallDecl to inherit from AbstractFunctionDecl?

I added new mangling/demangling rules for CallDecl - not sure what are the exact requirements for that, but the demangler seems to work okay:

struct Hi {
  call(x: Int) -> Int {
    return x
  }
}

let hi = Hi()
print(hi(x: 1))

sil [ossa] @main : $@convention(c) (Int32, UnsafeMutablePointer<Optional<UnsafeMutablePointer<Int8>>>) -> Int32 {
bb0(%0 : $Int32, %1 : $UnsafeMutablePointer<Optional<UnsafeMutablePointer<Int8>>>):
  ...
  // function_ref Hi.call(x:) << It gets printed.
 %19 = function_ref @$s4call2HiV1xS2i_tcfF : $@convention(method) (Int, Hi) -> Int // user: %20
  ...
} // end sil function 'main'

[dan-zheng]

On the topic of mangling, here are some other invasive-ish changes I found were necessary for adding a new decl kind:

• clang/include/clang/Index/IndexSymbol.h
  • Need to add a SwiftCall case to enum class SymbolSubKind.
• Mangling and demangling support.
• Code completion support.
• swift/utils/gyb_syntax_support/NodeSerializationCodes.py
  • Need to add syntax nodes (and serialization codes).
  • I wonder if changing existing syntax node serialization codes is a breaking change? It may be nice to put the CallDecl serialization code right after the other decls (instead of at the very end), but that requires incrementing the codes for a lot of other node kinds.
• swift/utils/gyb_sourcekit_support/UIDs.py
  • Need to add DeclCall and RefCall UID kinds.
• No TBDGen changes necessary? Is TBDGenVisitor::visitAbstractFunctionDecl sufficient for handling CallDecl?

Are any of these breaking changes? In any case, I've made enough progress to share a WIP branch soon for initial review.

[dan-zheng]

@dan-zheng

Yes. The bigger problem lies in DeclBaseName::getBaseIdentifier , called in FuncDecl::getName :

I'm using this solution for the CallDecl name problem: just use ctx.getName("call") instead of a new special DeclBaseName kind.

This causes func `call`(...) and call(...) to conflict, but I think this may actually be desirable behavior. If a func `call` declaration and call declaration have the exact same signature, the expression foo.call(...) would indeed be ambiguous.

This does mean that name alone is not sufficient to distinguish declaration kinds anymore: the name call could from either a FuncDecl or CallDecl. I'm not sure if there's ever a need to distinguish declarations by name alone, so I'll roll with this for now.

This unblocks CallDecl inheriting from FuncDecl.

[dan-zheng]

Callable WIP branch available at Introduce callables. by dan-zheng · Pull Request #23517 · apple/swift · GitHub.

Most things are done:

• Parsing is nearly done.
  • Added parsing source compatibility tests: call is a keyword only in declaration-parsing contexts so func call(...) and call(...) apply expression are still valid.
  • There's an unideal parsing diagnostic to fix.
• Sema needs a bit more work - namely on "looking up all call declarations" in a robust way, as @Slava_Pestov mentioned above. The ideal solution is to unify code with lookupQualified.

If you're interested in doing an early review, that would be much appreciated. The PR isn't ready for full review until sema is fixed.