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

Introduce callables. #23517

Closed
wants to merge 5 commits into from
Closed

Introduce callables. #23517

wants to merge 5 commits into from

Conversation

dan-zheng
Copy link
Contributor

Introduce callables: values that can be "called" like functions.
Add a new call declaration kind to represent callable methods.

"Introducing callables" pitch. Proposal coming soon.

Introduce callables: values that can be "called" like functions.
Add a new `call` declaration kind to represent callable methods.
include/swift/AST/Decl.h Show resolved Hide resolved
lib/Parse/ParseDecl.cpp Show resolved Hide resolved
lib/Sema/CSSimplify.cpp Outdated Show resolved Hide resolved
@@ -2829,6 +2829,8 @@ NodePointer Demangler::demangleFunctionEntity() {
Args = TypeAndMaybePrivateName; Kind = Node::Kind::Allocator; break;
case 'c':
Args = TypeAndMaybePrivateName; Kind = Node::Kind::Constructor; break;
case 'F':
Copy link
Contributor Author

Choose a reason for hiding this comment

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

TODO: Add mangling/demangling tests.
Currently, there are no SIL tests for call declarations.
I informally checked that demangling works via -emit-silgen.
The old remangler might not work - haven't look into how to invoke/test it yet.

lib/Serialization/Serialization.cpp Show resolved Hide resolved
@@ -1,8 +1,8 @@
<s118><s163><s92><s13><NULL/><NULL/><t6>// REQUIRES: syntax_parser_lib
<s119><s164><s93><s13><NULL/><NULL/><t6>// REQUIRES: syntax_parser_lib
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I’m not sure why changing this test to pass was necessary. I wonder if changing this test is breaking.

test/decl/call/protocol.swift Outdated Show resolved Hide resolved
@@ -97,6 +97,7 @@ enum class SyntaxStructureKind : uint8_t {
EnumElement,
TypeAlias,
Subscript,
Call,
Copy link
Contributor

Choose a reason for hiding this comment

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

fyi will need a simple change to ModelASTWalker::walkToDeclPre() to actually emit this, and I think a possibly less-simple change to SyntaxModelContext::SyntaxModelContext() to classify tok::kw_call as identifer/keyword depending on context (this affects syntax highlighting).

- Use uniform naming: either "`call` member" or "`call` declaration".
- Fix `call` member resolution during type-checking.
- Enable more `func` attributes on `call` declarations.
- Add tests.
@dan-zheng dan-zheng marked this pull request as ready for review March 27, 2019 05:23
@dan-zheng
Copy link
Contributor Author

The main callable functionality is done and tested: ready for review.
Code completion and editor features require some more work.

@DougGregor
Copy link
Member

@swift-ci build toolchain

@tkremenek
Copy link
Member

@swift-ci please build toolchain

clang requires minor changes to support `call` declarations in Swift.

Note: the clang remote should be reverted back to "apple/swift-clang".
- Parse "call" as identifier in patterns and enum case names. Add tests.
- Disallow `@warn_unqualified_access` on `call` declarations.
- Update failing attribute tests.
@dan-zheng
Copy link
Contributor Author

@DougGregor @tkremenek

Sorry, building failed because this PR depends on some minor changes to clang to support call declarations: apple/swift-clang#284

I temporarily updated the clang version in utils/update_checkout/update-checkout-config.json to branch callable of dan-zheng/swift-clang so that building will work. If there's a better way, please let me know.

I've verified that all swift tests pass locally. Retriggering toolchain build.

@dan-zheng
Copy link
Contributor Author

@swift-ci Please build toolchain

@dan-zheng
Copy link
Contributor Author

Investigating errors.

@dan-zheng
Copy link
Contributor Author

@swift-ci Please build toolchain

@dan-zheng
Copy link
Contributor Author

@swift-ci Please build toolchain macOS

@swift-ci
Copy link
Contributor

Linux Toolchain (Ubuntu 16.04)
Download Toolchain
Git Sha - 8dff4cd

Install command
tar zxf swift-PR-23517-193-ubuntu16.04.tar.gz
More info

@dan-zheng
Copy link
Contributor Author

@swift-ci Please build toolchain macOS

@swift-ci
Copy link
Contributor

macOS Toolchain
Download Toolchain
Git Sha - 8dff4cd

Install command
tar -zxf swift-PR-23517-251-osx.tar.gz --directory ~/

@dan-zheng
Copy link
Contributor Author

@swift-ci Please benchmark

@swift-ci
Copy link
Contributor

swift-ci commented Apr 1, 2019

!!! Couldn't read commit file !!!

@DougGregor
Copy link
Member

@swift-ci please test source compatibility

1 similar comment
@DougGregor
Copy link
Member

@swift-ci please test source compatibility

@DougGregor
Copy link
Member

FWIW, SwiftLint (from the source compatibility suite) is failing with this error:

/Users/buildnode/jenkins/workspace-private/swift-PR-source-compat-suite-debug/project_cache/SwiftLint/Source/SwiftLintFramework/Rules/Style/MultilineFunctionChainsRule.swift:178:52: error: consecutive statements on a line must be separated by ';'
        return subcalls.flatMap { call -> [NSRange] in
                                                   ^
                                                   ;

@dan-zheng
Copy link
Contributor Author

Closing, as SE-0253 was returned for revision.
Redesign using func call as the name for call-syntax delegate methods: #24299

@dan-zheng dan-zheng closed this Apr 26, 2019
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.

5 participants