Skip to content

[SwiftLexicalLookup][GSoC] Add switch, generic parameter and function scopes. #2782

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

Merged

Conversation

MAJKFL
Copy link
Contributor

@MAJKFL MAJKFL commented Aug 7, 2024

This PR introduces switch case, generic parameter/primary associated types and function scopes.

  • WithGenericParametersOrAssociatedTypesScopeSyntax and GenericParameterOrAssociatedTypeScopeSyntax are used to properly handle generic parameters/primary associated types by providing an alternative route for the lookup method before propagating it to parent scope.
  • switch now introduces names in it's case bodies
  • Function parameters are now passed into to the body
  • Includes some changes suggested in [SwiftLexicalLookup][GSoC] Add proper guard scope and implicit name lookup #2748

@ahoppen
Copy link
Member

ahoppen commented Aug 7, 2024

Would it be possible to do the changes that address my review comments from #2748 in a separate PR again? It just makes it easier to review.

@MAJKFL
Copy link
Contributor Author

MAJKFL commented Aug 8, 2024

Would it be possible to do the changes that address my review comments from #2748 in a separate PR again?

Ok, I think this is a great idea. I've opened #2789.

Copy link
Member

@DougGregor DougGregor left a comment

Choose a reason for hiding this comment

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

I have a few comments, but in general this looks great. There are a few constructs that probably still need to be covered here, but they can be in a follow-up PR:

  • subscripts
  • type aliases
  • where clauses and inheritance clauses of types/extensions to make sure they're seeing the right things


import SwiftSyntax

@_spi(Experimental) public protocol WithGenericParametersScopeSyntax: ScopeSyntax {
Copy link
Member

Choose a reason for hiding this comment

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

This looks like something we should define via CodeGeneration so it's declarative. (Doesn't have to be now)

let 7️⃣x: 3️⃣T1 = v
let y: 4️⃣T2 = v

class B<5️⃣T1> {
Copy link
Member

Choose a reason for hiding this comment

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

I am curious what the compiler (and this implementation) would do for the second T2 in class B<T1, T2: T2>.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

At least when it comes to this implementation, the result would be empty.

Copy link
Member

Choose a reason for hiding this comment

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

So, it seems that there's no ordering amongst the various generic parameters. Given this:

class A<X> {
  class B<T2: T1, T1> { }
}  

unqualified lookup is finding T1 and the type checker is rejecting it:

1 | class A<X> {
2 |   class B<T1, T2: T1> { }
  |                   `- error: type 'T2' constrained to non-protocol, non-class type 'T1'
3 | }  
4 | 

Copy link
Member

Choose a reason for hiding this comment

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

FWIW, here's a case where it's also semantically well-formed to refer to generic parameters out-of-order:

struct X<C: Collection<T>, T> { }

@DougGregor
Copy link
Member

@swift-ci please test

@DougGregor
Copy link
Member

@swift-ci please test Windows

Copy link
Member

@ahoppen ahoppen left a comment

Choose a reason for hiding this comment

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

Nice. I’m really curious how close we are to being feature-equivalent with the C++ name lookup and how many more edge cases we might hit.

@MAJKFL MAJKFL requested a review from ahoppen August 28, 2024 13:45
@DougGregor
Copy link
Member

@swift-ci please test

func testGenericParameterOrdering() {
assertLexicalNameLookup(
source: """
class Foo<1️⃣A: 2️⃣A, B: 3️⃣A, 4️⃣C: 5️⃣D, D: 6️⃣C> {}
Copy link
Member

Choose a reason for hiding this comment

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

Based on my comment above, I think the results from the lookup at "2" should find "1", and the results from the lookup at "5" should find the generic parameter D.

@DougGregor
Copy link
Member

@swift-ci please test Windows

@DougGregor
Copy link
Member

@swift-ci please test

@DougGregor DougGregor merged commit 984ec6d into swiftlang:main Sep 11, 2024
3 checks passed
@MAJKFL MAJKFL deleted the generic-parameter-list-function-scope branch October 16, 2024 11:24
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.

3 participants