-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Store argument labels + locations in directly in call and call-like expressions #3759
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
Yet another step on the way to SE-0111, capture the argument labels (and their locations) directly in CallExpr, rather than depending on them being part of the tuple argument.
Factor out the trailing storage of call arguments, since we'll need it for a few different kinds of expression nodes. Use it for both CallExpr (which already had this storage, albeit with a specialized implementation) and now SubscriptExpr.
@swift-ci please smoke test and merge |
A race in ErrorProtocol bridging? Unrelated but disturbing... |
} | ||
|
||
size_t numTrailingObjects( | ||
typename TrailingObjects::template OverloadToken<SourceLoc>) const { |
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.
Hi @DougGregor, sorry to bring this up from the past.
I'm trying (and not succeeding) to build swiftAST using MSVC. I've managed to get most things to work, fixing errors and MSVC bugs (my god, there are a lot of them).
I have one final hurdle I've been unable to cross. I get an error saying the following (on this line):
Error C2751 'llvm::TrailingObjects<BaseTy,TrailingTys...>::operator OverloadToken': the name of a function parameter cannot be qualified (compiling source file lib\AST\ASTNode.cpp
Since you added the code, I just wanted to inquire as to what you were trying to do here. Perhaps MSVC is being buggy, or maybe Clang was letting this pass when it shouldn't have. Maybe this code is invalid according to the C++ Specification.
If you have any suggestions, I'd really appreciate it!
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.
It's correct C++, but you might need to break it up for MSVC to handle, e.g., by putting it into a separate typedef:
typedef typename TrailingObjects::template OverloadToken<SourceLoc> SourceLocOverloadToken;
or dropping the "template" keyword just on MSVC using preprocessor macros.
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.
Thanks, Doug!
What's in this pull request?
Swift's current expression representation places call argument labels (and their locations) within TupleExprs buried in the arguments, which is inconvenient for the removal of argument labels from function type (SE-0111). Start storing the argument labels (and their locations) on CallExpr itself, as well as on other call-like expressions (subscripts, unresolved members, etc.), separately from the actual arguments (which will stay the same--ParenExpr or CallExpr).
There is some shimming going on here, so we can extract the argument labels from the ParenExpr or TupleExpr argument. Over time, the shimming will go away.
This whole thing should be NFC, except for a few places where we now properly wrap up single arguments in ParenExpr or are more consistent about 'implicit' bits.
Resolved bug number: (SR-)
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.