Gardening for @dynamicMemberLookup.
#20498
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
cc @lattner: could you please review? |
dan-zheng
commented
Nov 11, 2018
dan-zheng
changed the title
@dynamicMemberLookup.@dynamicMemberLookup.
dan-zheng
commented
Nov 11, 2018
| @@ -3053,17 +3053,17 @@ getArgumentLabels(ConstraintSystem &cs, ConstraintLocatorBuilder locator) { | |||
| /// conformances, but this is fine because it doesn't get invoked in the normal | |||
| /// name lookup path (only when lookup is about to fail). | |||
| static bool hasDynamicMemberLookupAttribute(CanType ty, | |||
There was a problem hiding this comment.
Deduping this function with getDynamicCallableMethods (which also defines the same "type traversal" logic) would be nice. However, the two functions use different cache types (llvm::DenseMap<CanType, bool> vs llvm::DenseMap<CanType, DynamicCallableMethods>), which makes generalization difficult. 🙁
Any generalization ideas appreciated.
- Unify code style of `@dynamicMemberLookup` and `@dynamicCallable` implementations. - Use consistent variable names, doc comments, etc. - Move `@dynamicMemberLookup` test to `test/attr` directory.
dan-zheng
force-pushed
the
dynamic-member-lookup-cleanup
branch
from
b98388b to
6583acd
Compare
|
@swift-ci Please smoke test |
1 similar comment
|
@swift-ci Please smoke test |
|
This all LGTM, but please fix the attr/attr_dynamic_member_lookup.swift failure. |
lattner
approved these changes
Nov 12, 2018
Address comment by @xwu.
dan-zheng
force-pushed
the
dynamic-member-lookup-cleanup
branch
from
18a8ea9 to
9f46a53
Compare
|
Sorry, tests failed because I updated the diagnostic message without updating tests. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
@dynamicMemberLookupand@dynamicCallableimplementations.@dynamicMemberLookuptest totest/attrdirectory.New consistent naming:
invalid_dynamic_callable_type,invalid_dynamic_member_lookup_typeDynamicCallableCache,DynamicMemberLookupCacheisValidDynamicCallableMethod,isValidDynamicMemberLookupSubscriptThis PR is almost NFC. The only user-facing change is a new error message for
invalid_dynamic_member_lookup_type.