-
Notifications
You must be signed in to change notification settings - Fork 0
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
[GSoC] Swift PR #1
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a few nits, rest looks good to me.
Expr *Arg; | ||
|
||
public: | ||
PackageAttr(SourceLoc AtLoc, SourceRange Range, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this has a body, it should be moved into the .cpp
file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AFAIK simple functions and the main initializer stay in Attr.h
?
@@ -1860,5 +1860,9 @@ ERROR(expected_closure_literal,none, | |||
ERROR(expected_multiple_closures_block_rbrace,none, | |||
"expected '}' at the end of a trailing closures block", ()) | |||
|
|||
// Package declaration errors | |||
ERROR(package_declaration_not_allowed,none, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should probably provide a little more detail about why it is not allowed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤔Since this is an experimental feature, we need some flag to allow such syntax, and this error will be thrown if these flags are not specified. I just didn’t come up with a better name?
include/swift/Basic/LangOptions.h
Outdated
@@ -443,6 +443,12 @@ namespace swift { | |||
// FrontendOptions. | |||
bool AllowModuleWithCompilerErrors = false; | |||
|
|||
// Allow @package attribute in the file. | |||
bool AllowPackageDeclaration = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think a little more information about when either of these flags should be set would be helpful. Also this should be plural (i.e. Declaration*s*
).
Replace this paragraph with a description of your changes and rationale. Provide links to external references/discussions if appropriate.
Resolves SR-NNNN.