Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Out-of-line destructor syntax ambiguity #4999

Closed
jonmeow opened this issue Feb 21, 2025 · 3 comments
Closed

Out-of-line destructor syntax ambiguity #4999

jonmeow opened this issue Feb 21, 2025 · 3 comments
Labels
leads question A question for the leads team

Comments

@jonmeow
Copy link
Contributor

jonmeow commented Feb 21, 2025

Summary of issue:

The accepted destructor syntax includes out-of-line definitions such as:

destructor MyClass [addr self: Self*] { ... }

The implicit parameter here could be interpreted as either an implicit parameter for MyClass or an implicit parameter for the destructor. How should ambiguities like this be resolved?

Details:

The full example is:

class MyClass {
  destructor [addr self: Self*];
}
destructor MyClass [addr self: Self*] { ... }

For comparison, note a generic might look like:

class GenericClass[T:! type](N:! T) { ... }
destructor GenericClass[T:! type](N:! T) [addr self: Self*] { ... }

Destructors were adopted in #1154, and 2021-08-03 notes are referenced there but don't seem to get into detail on the out-of-line syntax.

I can offer are a few options for resolving the ambiguity:

  1. Use arbitrary lookahead to see what is after the [].

For the toolchain, in tokenization we will have found the closing ] and we can parse differently based on whether the { follows the ]. Note this may have effects on error recovery, and while it would be efficient in the toolchain, may be more difficult to implement in third-party parsing systems, such as tree-sitter.

This option would also mean that generic types must always have explicit parameters if they have implicit parameters, which may affect other open questions. Because the { would be disambiguating, it would also mean a typo of adding () (as in destructor MyClass [addr self: Self*]()) would lead to diagnostic as a generic type, not incorrectly adding () after a destructor.

Note, I'm not pursuing this approach right now because I believe we don't want arbitrary lookahead to be required.

  1. Require a . before the destructor's implicit parameters, similar to qualified names.

The . indicates a separation of name components. In the context of a destructor introducer, we can treat the final name component in a special way.

For example:

destructor MyClass.[addr self: Self*] { ... }
destructor GenericClass[T:! type](N:! T).[addr self: Self*] { ... }
  1. Switch to a more fn-like declaration, using destructor as the name.

This has benefits of aligning more with fn parsing, and destructor still cleanly disambiguates versus a regular function. In this approach, the destructor keyword takes the place of the function name and can make the explicit parameters disallowed.

Note, I actually thought we'd discussed this approach in the past, but I can't find it from #1154.

For example:

class MyClass {
  fn destructor[addr self: Self*];
}
fn MyClass.destructor[addr self: Self*] { ... }

Any other information that you want to share?

I have a half-working destructor parse, and it was specifically looking at making the qualified name optional that made me notice the challenge here.

@jonmeow jonmeow added the leads question A question for the leads team label Feb 21, 2025
@geoffromer
Copy link
Contributor

Side note: if we go with option 3, I think the keyword should be a verb or verb phrase (e.g. destroy). If we're going to put the keyword in the function-name position, it should follow function naming conventions.

@CJ-Johnson
Copy link
Contributor

If we're open to reevaluating the destructor syntax, I'd also like to point out an inconsistency in the current design. Functions with no explicit params (parens) are always variadic auto using positional params ($0). The exception to that is the destructor. For consistency, could we add empty explicit params list to the destructor?

class C {
  fn destroy[addr self: Self*]() {...}
}

@jonmeow
Copy link
Contributor Author

jonmeow commented Feb 25, 2025

Discussion seemed to settle towards fn destroy; see 2025-02-25 Toolchain minutes, and #5017 for the proposal to update the design.

@jonmeow jonmeow closed this as completed Feb 25, 2025
github-merge-queue bot pushed a commit that referenced this issue Mar 5, 2025
Fix destructor syntax ambiguity by switching to `fn destroy` mirroring
standard function syntax. This is a purely syntactic change, maintaining
destructor semantics.

This comes from leads question #4999

---------

Co-authored-by: Chandler Carruth <chandlerc@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
leads question A question for the leads team
Projects
None yet
Development

No branches or pull requests

3 participants