Skip to content

Conversation

@ahoppen
Copy link
Member

@ahoppen ahoppen commented Mar 21, 2023

gyb is gone from this repository for good and all remaining references should be removed. I’m also removing the implementation status of SwiftParser because it’s no longer up to date and doesn’t seem to provide any value anymore.

@ahoppen ahoppen requested review from DougGregor and bnbarham March 21, 2023 00:23
@ahoppen
Copy link
Member Author

ahoppen commented Mar 21, 2023

@swift-ci Please smoke test

@ahoppen
Copy link
Member Author

ahoppen commented Mar 21, 2023

@swift-ci Please test

Comment on lines +14 to +15
To re-generate the files after changing `CodeGeneration` run the `generate-swiftsyntax`
target of `CodeGeneration` and pass `path/to/swift-syntax/Sources` as the argument.
Copy link
Contributor

@bnbarham bnbarham Mar 21, 2023

Choose a reason for hiding this comment

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

generate-source-code still exists. If you'd prefer generate-swiftsyntax, it'd be nice to spell it out completely:

swift run --package-path CodeGeneration generate-swiftsyntax Sources

(not sure if we handle the relative path there)

Copy link
Member Author

Choose a reason for hiding this comment

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

I prefer to tell people to just run generate-swiftsyntax now. I’d like to reduce the number of uses for build-script.py. Maybe one day we can even get rid of that last remaining Python script.

I added the command though, that’s a good suggestion.

Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to keep generate-source-code?


On the command line, this would be
```bash
swift run --package-path CodeGeneration generate-swift Sources
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
swift run --package-path CodeGeneration generate-swift Sources
swift run --package-path CodeGeneration generate-swiftsyntax Sources

Some source code inside SwiftSyntax is generated using [SwiftSyntaxBuilder](../Sources/SwiftSyntaxBuilder), a Swift library whose purpose is to generate Swift code using Swift itself. This kind of code generation is performed by the Swift package defined in this directory.

This directory is a standalone package that uses a pinned version of SwiftSyntaxBuilder. It is thus NOT using SwiftSyntaxBuilder of the parent directory. This guarantees that when `generate-swiftsyntaxbuilder` is run, it can't break its own build.
This directory is a standalone package that uses a pinned version of SwiftSyntaxBuilder. It is thus NOT using SwiftSyntaxBuilder of the parent directory. This guarantees that when `generate-swiftsyntaxbuilder` is run, it can't break its own build.
Copy link
Contributor

@bnbarham bnbarham Mar 21, 2023

Choose a reason for hiding this comment

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

Looks like this was changed to generate-swiftsyntax. That also needs to be updated in all the generateCopyrightHeader expressions.

Personally I'd prefer generate-swift-syntax :P

EDIT: Also, it's probably worth putting how to generate here as well. I would personally miss the ChangingSwiftSyntax.md one.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch. Done 👍🏽

As to renaming generate-swiftsyntax to generate-swift-syntax that would be another PR but I agree.

@ahoppen ahoppen force-pushed the ahoppen/no-gyb branch 2 times, most recently from 848df95 to 36a9ee0 Compare March 21, 2023 01:46
@ahoppen
Copy link
Member Author

ahoppen commented Mar 21, 2023

@swift-ci Please test

@ahoppen
Copy link
Member Author

ahoppen commented Mar 21, 2023

@swift-ci Please test

@kimdv
Copy link
Contributor

kimdv commented Mar 21, 2023

@swift-ci please test windows

@ahoppen
Copy link
Member Author

ahoppen commented Mar 22, 2023

@swift-ci Please test Windows

Copy link
Contributor

@bnbarham bnbarham left a comment

Choose a reason for hiding this comment

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

LGTM. The extra documentation could be a separate PR if you want given this is just cleaning up gyb references.

```
$ ./build-script.py verify-source-code --toolchain /path/to/toolchain/usr
```
Or if you open the `CodeGeneration` package in Xcode, you can add the argument using
Copy link
Contributor

Choose a reason for hiding this comment

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

Same change here as in the other file

```python
TypeAttribute('_myAttribute'), # Becomes @_myAttribute in the compiler
```
```swift
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we have documentation for what these fields mean, especially traits + parserFunction?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think we do not. We should add them as doc comments.

@ahoppen
Copy link
Member Author

ahoppen commented Mar 22, 2023

@swift-ci Please test

@ahoppen
Copy link
Member Author

ahoppen commented Mar 23, 2023

@swift-ci Please test Windows

@ahoppen ahoppen merged commit c23e23d into swiftlang:main Mar 23, 2023
@ahoppen ahoppen deleted the ahoppen/no-gyb branch March 23, 2023 17:34
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