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

Support swift 5.9+ syntax. #51

Merged
merged 11 commits into from
Apr 15, 2024
Merged

Support swift 5.9+ syntax. #51

merged 11 commits into from
Apr 15, 2024

Conversation

furby-tm
Copy link
Contributor

@furby-tm furby-tm commented Apr 13, 2024

This revision adds support for Swift Syntax 5.9 and above.

Important

It also points to the latest revision of @stackotter's fork of swift-argument-parser, which otherwise conflicts with the name of the GenerateManual plugin if not brought to that version. As well as updating the swift-log dependency to the latest v1.5.4.

@furby-tm
Copy link
Contributor Author

furby-tm commented Apr 13, 2024

Note, there are some misc. minor deprecation warnings with some of the existing SwiftSyntax usage but wanted to keep this revision low-touch, and specific to breaking changes with the upgrade from 5.85.10.

Copy link
Owner

@stackotter stackotter left a comment

Choose a reason for hiding this comment

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

If you've tested schema-gen and it still produces the same output as it did before your changes, then I'm happy to merge with that one minor formatting change (for the guard statement that I commented on).

* Using the trimmed description here, in an attempt to keep behavior
  consistent with the deprecated, and removed withoutTrivia() function,
  so that we don't run into incorrect comparisons (ex. "static" != "static ").
@furby-tm
Copy link
Contributor Author

@stackotter When I run:

./generate_schema.sh

I receive the following output:

Enum 'PlistValue' requires an explicit schema

And the entirety of Bundler.schema.json gets deleted, lol. I guess something got messed up...

@stackotter
Copy link
Owner

@stackotter When I run:

./generate_schema.sh

I receive the following output:

Enum 'PlistValue' requires an explicit schema

And the entirety of Bundler.schema.json gets deleted, lol. I guess something got messed up...

lol whoops, yeah it works for me (on the commit before your changes), must've messed up something in the explicit schema parsing stuff (cause it's true that PlistValue requires an explicit schema, but it's also true that one is provided in its doc comment).

@furby-tm
Copy link
Contributor Author

furby-tm commented Apr 15, 2024

@stackotter actually the deprecations were so stupidly minimal I've included them as well to void stupid warnings, they were simple renamings.

~ also ew, I am unsure why my rebase committed your commits under my name 😕

this is why we make branches, and do not work out of main 😅 I should have put this in a feature branch 🤦🏻‍♀️.

@furby-tm
Copy link
Contributor Author

furby-tm commented Apr 15, 2024

Important

Do not merge until ./generate_schema.sh runs successfully without file changes.

@furby-tm furby-tm changed the title Support swift 5.9+ syntax. [DO NOT MERGE] Support swift 5.9+ syntax. Apr 15, 2024
@furby-tm furby-tm changed the title [DO NOT MERGE] Support swift 5.9+ syntax. Support swift 5.9+ syntax. Apr 15, 2024
@furby-tm
Copy link
Contributor Author

furby-tm commented Apr 15, 2024

All fixed, due to my dumb guard in .parse(from:), which caused an early return due to the lack of any leadingTrivia, which I later realized was only for the purposes of documentation parsing. Feel free to merge @stackotter 👍🏻

@furby-tm furby-tm requested a review from stackotter April 15, 2024 07:30
@furby-tm
Copy link
Contributor Author

furby-tm commented Apr 15, 2024

Also, since parsing the documentation alone was half the logic in the parse(from:) function, it was making the core parsing logic unclear, I opted to move documentation parsing into it's own function: parseDocumentation(from:), and the guard was removed from parse(from:) because we still want the property parsed even if there is no doc comment.

@stackotter
Copy link
Owner

Remove needless stack allocation for docs.

The ultimate premature optimisation 😅 (don't think that affects performance at all, swiftc rearranges things and the stack is constant sized within a function, see this godbolt experiment).

Lgtm 👍

@stackotter stackotter merged commit 66d68b3 into stackotter:main Apr 15, 2024
2 checks passed
@furby-tm
Copy link
Contributor Author

Remove needless stack allocation for docs.

The ultimate premature optimisation 😅 (don't think that affects performance at all, swiftc rearranges things and the stack is constant sized within a function, see this godbolt experiment).

Lgtm 👍

@stackotter
C++ has given me some terrible PTSD when it comes to recognizing certain design patterns, that I can never unsee 😅 To be honest, I wasn't aware swiftc was that smart, lol!

@stackotter
Copy link
Owner

To be honest, I wasn't aware swiftc was that smart, lol!

Hahah yeah it's pretty smart, today I found out some pretty cool tricks it uses to optimise converting enums to raw values (and vice versa)

@furby-tm
Copy link
Contributor Author

I found out some pretty cool tricks it [swiftc] uses to optimise converting enums to raw values (and vice versa)

@stackotter Too much greatness 😭 How so?

@stackotter
Copy link
Owner

enum A: UInt8 {
    case one = 2
    case two = 3
    case three = 5
    case four = 7
    case five = 11
    case six = 13
    case seven = 17
}

Under the hood A.one is represented as the byte 0, and A.two is represented as the byte 1 (not sure what the implementation reason is, could be for ABI stability) and so on.

The function that computes A.rawValue is just just 3 instructions (to map the numbers 0..<7 to the seven corresponding raw values). It stores all the raw values in one big UInt64 and uses bit shifting to extract each of the values.

output.A.rawValue.getter : Swift.UInt8:
        lea     ecx, [8*rdi]
        movabs  rax, 4799415617651458
        shr     rax, cl
        ret

And for an enum such as B (below) it figures out the pattern and computes it in just two instructions,

enum B: UInt8 {
  case one = 1
  case two = 4
  case three = 7
  case four = 10
}
output.A.rawValue.getter : Swift.UInt8:
        lea     eax, [rdi + 2*rdi]
        inc     al
        ret

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