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

Implement the DynamicMemberLookupProtocol proposal. #13361

Closed
wants to merge 1 commit into from

Conversation

@lattner
Copy link
Collaborator

commented Dec 10, 2017

This is the second cut at implementing the DynamicMemberLookupProtocol proposal. Changes from PR13076 include:

  1. Incorporating @slavapestov's awesome code review feedback.
  2. Retroactive conformance to DynamicMemberLookupProtocol now passes.

This also passes all tests for me locally, but the other patch apparently failed some test on linux. the logs are gone, so I can't investigate it. If this patch fails again then I'll investigate more promptly.

@lattner

This comment has been minimized.

Copy link
Collaborator Author

commented Dec 10, 2017

@swift-ci please test

@@ -6501,7 +6545,8 @@ Expr *ExprRewriter::coerceToType(Expr *expr, Type toType,
}

// Unresolved types come up in diagnostics for lvalue and inout types.
if (fromType->hasUnresolvedType() || toType->hasUnresolvedType())
if (fromType->hasUnresolvedType() || toType->hasUnresolvedType() /* ||
fromType->is<ErrorType>()*/)

This comment has been minimized.

Copy link
@slavapestov

slavapestov Dec 11, 2017

Member

Did you intend to leave this commented out?

This comment has been minimized.

Copy link
@lattner

lattner Dec 11, 2017

Author Collaborator

No, will remove. Thanks for the catch.

@rudkx

This comment has been minimized.

Copy link
Member

commented Jan 23, 2018

It looks like there is a trivial conflict from a change of mine (#13811) that adds buildSubscriptHelper taking a SelectedOverload & rather than SelectedOverload *.

There's also a more extensive conflict due to #13565.

Once you have an opportunity to fix these up I'd like to take a closer look.

Note I also have a PR in the works for removing IUOs that refactors a bit of the dynamic member and dynamic subscript code, which might cause more conflicts here (hopefully nothing too serious).

@lattner

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 23, 2018

Thanks @rudkx! After the review has completed I'll rebase and clean up the patch and send it for final review. I appreciate you taking a look.

@lattner

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 12, 2018

Thank you for the feedback everyone. I've put the updated patch over in PR 14546

@lattner lattner closed this Feb 12, 2018

@lattner lattner deleted the lattner:dynamicMemberV2 branch Feb 12, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.