-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Follow-up for fixing IDE/SourceKit tests after changes for [SE-0111] #4182
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
Conversation
rdar://27642873
Also fixup 'test/SourceKit/CodeExpand/code-expand.swift' and use the syntax for adding parameter names with an underscore for first name.
- Make sure VarDecls have an associated TypeLoc, like ParamDecls do, then use it for printing the VarDecl's type. This is done by moving ParamDecl's TypeLoc up to the VarDecl. This is useful for being able to display the parameter names of function types embedded in VarDecls. - Use the result TypeLoc of functions for printing. This enables printing parameter names of function types embedded in return types. - Make sure to annotate attributes while they are printed.
@swift-ci Please test |
@swift-ci smoke test OS X platform |
@benlangmuir please review. |
@swift-ci smoke test OS X platform |
@swift-ci Please test |
@swift-ci smoke test OS X platform |
@swift-ci Please test |
@@ -425,6 +425,7 @@ struct PrintOptions { | |||
result.AbstractAccessors = false; | |||
result.PrintForSIL = true; | |||
result.PrintInSILBody = true; | |||
result.PreferTypeRepr = 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.
Why are you changing printSIL
, Decl::print
and swift-ide-test's Type reconstruction to use PreferTypeRepr = 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.
The SIL tests and that one ide test were depending on the fact that VarDecls were always printed using the Type (they were checking the output as printed when using the type).
I preferred to make the dependency on type printing explicit instead of updating all the tests, which for SIL printing I think using type printing makes more sense.
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.
Theoretically this would change behaviour for non-VarDecl type printing though right? For swift-ide-test I think that's fine, but I'd rather @slavapestov or @jckarter commented on any change for SIL printing.
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.
SIL only really cares about the canonical types of things, so ignoring the type repr makes sense to me.
LGTM |
Linux test had a build timeout (30 mins timeout for some reason, probably a mistake), merging. |
What's in this pull request?
Fixes for IDE/SourceKit tests addressing changes related to [SE-0111].
Resolved bug number: (SR-)
rdar://27642873
Before merging this pull request to apple/swift repository:
Triggering Swift CI
The swift-ci is triggered by writing a comment on this PR addressed to the GitHub user @swift-ci. Different tests will run depending on the specific comment that you use. The currently available comments are:
Smoke Testing
A smoke test on macOS does the following:
device standard libraries are not built.
version of these tests are not run.
A smoke test on Linux does the following:
tests are not run.
Validation Testing
Lint Testing
Note: Only members of the Apple organization can trigger swift-ci.