[AST/Sema] Associate `@autoclosure` flag with parameter declaration #20152

xedin · 2018-10-30T07:12:05Z

These changes aim to:

Move @autoclosure flag from function type to parameter declaration
- Serialize @autoclosure as part of the parameter declaration
- Convert it from being a type attribute to be declaration attribute
- Remove it from AnyFunctionType::ExtInfo
Cleanup logic in constraint solver associated with @autoclosure handling
and clarify overloading behavior
Add more tests

Resolves: rdar://problem/30906031

xedin · 2018-11-02T19:20:44Z

The way I wanted to change parser didn't work out so I think this is really for review, because I can add some more tests to test/Compatibility separately too

xedin · 2018-11-02T21:46:10Z

@swift-ci please clean test

xedin · 2018-11-02T21:57:05Z

@swift-ci please test source compatibility

xedin · 2018-11-05T06:28:58Z

@swift-ci please test

xedin · 2018-11-05T08:15:45Z

@swift-ci please test Linux platform

xedin · 2018-11-05T18:15:35Z

@swift-ci please test macOS platform

rudkx

LGTM! It's a pretty big change and could probably use another pair of eyes, though!

lib/AST/ASTMangler.cpp

xedin · 2018-11-05T23:07:34Z

@swift-ci please test

xedin · 2018-11-06T02:46:47Z

@swift-ci please test

swift-ci · 2018-11-06T04:06:01Z

Build failed
Swift Test Linux Platform
Git Sha - fa306b1c16f0c5d077c13b494e10f444c7bee94d

xedin · 2018-11-07T02:53:58Z

If there are no other problems, I'll soon make changes to XCTest calls which pass @autoclosure functions types from one call to another, and that would be the end of this PR :)

xedin · 2018-11-07T19:50:29Z

/cc @moiseev XCTest overlay was doing a lot of @autoclosure <-> @autoclosure conversions which I fixed by calling expression* and message explicitly. Could you please take a look?

jrose-apple · 2018-11-07T20:45:31Z

cc @stmontgomery too (about XCTest)

stmontgomery · 2018-11-07T20:55:34Z

stdlib/public/SDK/XCTest/XCTest.swift

@@ -146,17 +146,17 @@ public func XCTAssertNil(_ expression: @autoclosure () throws -> Any?, _ message
    if !passed {
      // TODO: @auto_string expression

-      _XCTRegisterFailure(true, _XCTFailureDescription(assertionType, 0, expressionValueStr as NSString), message, file, line)
+      _XCTRegisterFailure(true, _XCTFailureDescription(assertionType, 0, expressionValueStr as NSString), message(), file, line)


Would it make sense to remove @autoclosure from the message: parameter of the _XCTRegisterFailure helper function, now? Are there any remaining use cases for that to be @autoclosure?

/cc @briancroom

Considering that _XCTRegisterFailure unconditionally evaluates the message parameter, I doubt there's any need for it to actually be an autoclosure.

Do you guys want me to make that change or would you handle it?

FWIW: XCTest as well as the majority of other overlays is built in Swift 4 mode, and so this change is not strictly required. Thanks @xedin for fixing it anyways. LGTM, removing an autoclosure completely also works, and can probably be done separately.

swift-ci · 2018-11-10T00:14:24Z

Build failed
Swift Test OS X Platform
Git Sha - 4c8ca4913e7978091239d0790ebc6d22e7f22043

It should become a declaration attribute instead, but it can't be done at the moment because `@autoclosure` is a type attribute.

…rmat

`typeCheckParameterDefault` is used to type-check default value expression associated with a given parameter. It makes it possible to look-through `@autoclosure` function to use its result as contextual type, and later build implicit autoclosure expression if needed.

Make sure that presence of `@autoclosure` attribute handled in one place - `matchCallArguments`, which makes it possible to remove the rest of (now redundant) autoclosure related logic scattered throughout solver.

There is one place which handles `@autoclosure` attribute associated with parameters - `matchCallArguments`.

…arameter flags There is only one place where we need to do that - `typeCheckArgumentChildIndependently`

Mangled implicit closures shouldn't have `@autoclosure` flag associated with them, because their type is just a regular function type.

…eter flag

@autoclosure

…ity mode Swift versions < 5 allowed argument passed to @autoclosure parameter to be @autoclosure function type, we need to make sure that such behavior is preserved.

@autoclosure

… < 5 Since it was allowed to pass function types to @autoclosure parameters in Swift versions < 5, it has to be handled as a regular function coversion by `coerceToType`.

…ltArgContext()`

xedin · 2018-11-10T20:00:23Z

@swift-ci please test

xedin · 2018-11-10T20:00:40Z

@swift-ci please test source compatibility

swift-ci · 2018-11-10T20:00:56Z

Build failed
Swift Test OS X Platform
Git Sha - c106e54a464289920fac761af0885b1d357a5cac

swift-ci · 2018-11-10T20:02:16Z

Build failed
Swift Test Linux Platform
Git Sha - c106e54a464289920fac761af0885b1d357a5cac

xedin · 2018-11-10T22:53:07Z

Source compat failure with Tagged is unrelated, merging.

xedin · 2018-11-11T07:06:26Z

Looks like with all these rebases I've lost commit with added tests Slava requested me to add, going to add them in follow-up commit...

xedin requested a review from DougGregor October 30, 2018 07:12

xedin force-pushed the add-autoclosure-flag-to-param-decl branch 3 times, most recently from 6034e41 to 642caa5 Compare November 1, 2018 23:43

xedin changed the title ~~[WIP][AST/Sema] Associate @autoclosure flag with parameter declaration~~ [AST/Sema] Associate @autoclosure flag with parameter declaration Nov 2, 2018

This comment has been minimized.

Sign in to view

xedin force-pushed the add-autoclosure-flag-to-param-decl branch from b2c2666 to d6fc3cf Compare November 5, 2018 06:28

swiftlang deleted a comment from swift-ci Nov 5, 2018

xedin requested a review from rudkx November 5, 2018 18:20

rudkx approved these changes Nov 5, 2018

View reviewed changes

lib/AST/ASTMangler.cpp Outdated Show resolved Hide resolved

xedin force-pushed the add-autoclosure-flag-to-param-decl branch from e4c2288 to fa306b1 Compare November 5, 2018 23:07

swiftlang deleted a comment from swift-ci Nov 6, 2018

xedin force-pushed the add-autoclosure-flag-to-param-decl branch from a31137d to 9a4b6ba Compare November 7, 2018 02:52

stmontgomery reviewed Nov 7, 2018

View reviewed changes

xedin added 17 commits November 10, 2018 11:58

[AST] Register @autoclosure as parameter decl flag

4c1885b

It should become a declaration attribute instead, but it can't be done at the moment because `@autoclosure` is a type attribute.

[Serialization] Add @autoclosure to parameter decl serialization fo…

0857611

…rmat

[AST] Propagate @autoclosure flag to parameter decl and type flags

53417f4

[ConstraintSystem] Refactor arg/param matching to handle autoclosures

f462521

Make sure that presence of `@autoclosure` attribute handled in one place - `matchCallArguments`, which makes it possible to remove the rest of (now redundant) autoclosure related logic scattered throughout solver.

[ConstraintSystem] Remove now redundant @autoclosure checking

6224d12

There is one place which handles `@autoclosure` attribute associated with parameters - `matchCallArguments`.

[CodeCompletion] Add isAutoClosure flag to parameter builder

b3f8625

[CSDiag] Switch expression diagnostics to check @autoclosure from p…

6b62ac8

…arameter flags There is only one place where we need to do that - `typeCheckArgumentChildIndependently`

Associate @autoclosure only with parameter types

a1df238

Mangled implicit closures shouldn't have `@autoclosure` flag associated with them, because their type is just a regular function type.

[CSApply] Refactor ExprRewriter to handle @autoclosure from param…

d113b4a

…eter flag

[AST] Remove @autoclosure flag from function type ExtInfo

bce1ba5

[AST] NFC: Remove a FIXME related to ParamDecl::IsAutoClosure field

ed84f18

[ConstraintSystem] Test @autoclosure behavior in Swift < 5 compatibil…

6c8513f

…ity mode Swift versions < 5 allowed argument passed to @autoclosure parameter to be @autoclosure function type, we need to make sure that such behavior is preserved.

[CSApply] Handle @autoclosure arg/param conversions in Swift versions…

e50537d

… < 5 Since it was allowed to pass function types to @autoclosure parameters in Swift versions < 5, it has to be handled as a regular function coversion by `coerceToType`.

[ASTManger] Refactor parameter mangling code a bit based on feedback

3ffda7e

[TypeChecker] NFC: Delete rudimentary `AutoClosureListener::isInDefau…

c532583

…ltArgContext()`

[Overlay] Fix XCTest overlay to avoid passing autoclosures through

7bd2688

xedin force-pushed the add-autoclosure-flag-to-param-decl branch from c106e54 to 7bd2688 Compare November 10, 2018 20:00

xedin merged commit 2ad754d into swiftlang:master Nov 10, 2018

slavapestov mentioned this pull request Nov 16, 2018

Force corelibs-xctest to build with -swift-version 4 swiftlang/swift-corelibs-xctest#238

Merged

xedin mentioned this pull request Nov 11, 2018

[SR-4188] withoutActuallyEscaping doesn't accept an @autoclosure argument #46771

Closed

norio-nomura mentioned this pull request Dec 6, 2018

[SR-9420] Swift 5 produces error on passing closure generated by @autoclosure parameter to @autoclosure parameter #51884

Open

xedin mentioned this pull request Dec 12, 2018

[SR-9494] Disable @autoclosure forwarding and provide a fix-it to add () #51949

Closed

[AST/Sema] Associate @autoclosure flag with parameter declaration #20152

[AST/Sema] Associate @autoclosure flag with parameter declaration #20152

Uh oh!

Conversation

xedin commented Oct 30, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

xedin commented Nov 2, 2018

Uh oh!

xedin commented Nov 2, 2018

Uh oh!

This comment has been minimized.

This comment has been minimized.

xedin commented Nov 2, 2018

Uh oh!

xedin commented Nov 5, 2018

Uh oh!

xedin commented Nov 5, 2018

Uh oh!

xedin commented Nov 5, 2018

Uh oh!

rudkx left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

xedin commented Nov 5, 2018

Uh oh!

xedin commented Nov 6, 2018

Uh oh!

swift-ci commented Nov 6, 2018

Uh oh!

xedin commented Nov 7, 2018

Uh oh!

xedin commented Nov 7, 2018

Uh oh!

jrose-apple commented Nov 7, 2018

Uh oh!

stmontgomery Nov 7, 2018

Choose a reason for hiding this comment

Uh oh!

briancroom Nov 7, 2018

Choose a reason for hiding this comment

Uh oh!

xedin Nov 7, 2018

Choose a reason for hiding this comment

Uh oh!

moiseev Nov 7, 2018

Choose a reason for hiding this comment

Uh oh!

swift-ci commented Nov 10, 2018

Uh oh!

xedin commented Nov 10, 2018

Uh oh!

xedin commented Nov 10, 2018

Uh oh!

swift-ci commented Nov 10, 2018

Uh oh!

swift-ci commented Nov 10, 2018

Uh oh!

xedin commented Nov 10, 2018

Uh oh!

xedin commented Nov 11, 2018

Uh oh!

Uh oh!

[AST/Sema] Associate `@autoclosure` flag with parameter declaration #20152

[AST/Sema] Associate `@autoclosure` flag with parameter declaration #20152

xedin commented Oct 30, 2018 •

edited

Loading