Skip to content

Conversation

@DougGregor
Copy link
Member

@DougGregor DougGregor commented Dec 21, 2022

Local discriminators for named entities are currently being set by the
parser, so entities not created by the parser (e.g., that come from
synthesized code) don't get local discriminators. Moreover, there is
no checking to ensure that every named local entity gets a local
discriminator, so some entities would incorrectly get a local
discriminator of 0.

Assign local discriminators as part of setting closure discriminators,
in response to a request asking for the local discriminator, so the
parser does not need to track this information, and all local
declarations---including synthesized ones---get local discriminators.
And add checking to make sure that every entity that needs a local
discriminator gets assigned one.

There are a few interesting cases in here:

  • There was a potential mangling collision with local property
    wrappers because their generated variables weren't getting local
    discriminators
  • "Local rename" when dealing with captures like [x] was dependent on
    the new delcaration of x not getting a local discriminator. There
    are funny cases involving nesting where it would do the wrong thing.

With the parser no longer assigning local named discriminators, eliminate
the notion of "local contexts" as parser state entirely.

Local discriminators for named entities are currently being set by the
parser, so entities not created by the parser (e.g., that come from
synthesized code) don't get local discriminators. Moreover, there is
no checking to ensure that every named local entity gets a local
discriminator, so some entities would incorrectly get a local
discriminator of 0.

Assign local discriminators as part of setting closure discriminators,
in response to a request asking for the local discriminator, so the
parser does not need to track this information, and all local
declarations---including synthesized ones---get local discriminators.
And add checking to make sure that every entity that needs a local
discriminator gets assigned one.

There are a few interesting cases in here:
* There was a potential mangling collision with local property
wrappers because their generated variables weren't getting local
discriminators
* $interpolation variables introduced for string interpolation weren't
getting local discriminators, they were just wrong.
* "Local rename" when dealing with captures like `[x]` was dependent on
the new delcaration of `x` *not* getting a local discriminator. There
are funny cases involving nesting where it would do the wrong thing.
The parser no longer sets local discriminators, and this function is
currently only responsible for adding local type declarations to the
source file. Rename it and remove most of the former callers so it
does just that.
The "local context" was only used to prevent parsing of closures in a
non-local context, and also string interpolations because they are
similar-ish to closures. However, this isn't something a parser should
decide, so remove this special-case semantic check from the parser and
eliminate the notion of "local context" entirely.
@DougGregor DougGregor force-pushed the local-named-discriminators branch from 373c831 to b399b92 Compare December 21, 2022 23:01
@DougGregor
Copy link
Member Author

@swift-ci please smoke test

@DougGregor
Copy link
Member Author

@swift-ci please test source compatibility

@DougGregor
Copy link
Member Author

swiftlang/llvm-project#5861

@swift-ci please smoke test

@DougGregor
Copy link
Member Author

swiftlang/llvm-project#5861

@swift-ci please test source compatibility

@DougGregor
Copy link
Member Author

@swift-ci please smoke test macOS

@DougGregor
Copy link
Member Author

@swift-ci please smoke test

@DougGregor
Copy link
Member Author

@swift-ci please smoke test

@DougGregor DougGregor merged commit c1a85c1 into swiftlang:main Jan 3, 2023
@DougGregor DougGregor deleted the local-named-discriminators branch January 3, 2023 05:19
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.

1 participant