Skip to content

Unqualified lookup fixes, part 2 #19891

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 6 commits into from
Oct 16, 2018
Merged

Unqualified lookup fixes, part 2 #19891

merged 6 commits into from
Oct 16, 2018

Conversation

slavapestov
Copy link
Contributor

@slavapestov slavapestov commented Oct 15, 2018 •
edited
Loading

Fixes rdar://problem/22587551, rdar://problem/44777661.

@slavapestov slavapestov force-pushed the unqualified-lookup-fixes-part-2 branch from fab39e4 to 6437a23 Compare October 16, 2018 01:11
@slavapestov
Copy link
Contributor Author

@nkcsgexi Please take a look at the swift-api-digester changes.

nkcsgexi
nkcsgexi reviewed Oct 16, 2018
View reviewed changes
tools/swift-api-digester/ModuleAnalyzerNodes.cpp
if (Left->hasDefaultArgument() != Right->hasDefaultArgument())
return false;
if (Left->getParamValueOwnership() != Right->getParamValueOwnership())
return false;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we extract these two conditions along with "if (Left->hasAttributeChange(*Right)) return false;" to a static helper function so this type alias logic can be shared with other types? Otherwise LGTM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks you!

@slavapestov slavapestov force-pushed the unqualified-lookup-fixes-part-2 branch from 6437a23 to fa40c89 Compare October 16, 2018 01:37
@slavapestov
Copy link
Contributor Author

@swift-ci Please test

@slavapestov
Copy link
Contributor Author

@swift-ci Please test source compatibility

@slavapestov
Copy link
Contributor Author

@swift-ci Please test

@slavapestov
Copy link
Contributor Author

@swift-ci Please test source compatibility

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - 6437a2328866384e275a108beacb3d23b27c45c2

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - 6437a2328866384e275a108beacb3d23b27c45c2

@slavapestov
swift-api-digester: Don't drop value ownership when parameter has typ…
ba066b8 
…ealias type
@slavapestov
swift-api-digester: Subscripts don't have storage
5e7a289
@slavapestov
swift-api-digester: Don't desugar type aliases all the way
91fc404
@slavapestov
swift-api-digester: Minor typos
7c6a6bd
@slavapestov
AST: Don't allow declarations to shadow generic parameters
4ef1904 
Associated type inference can synthesize type aliases with the same name
as a generic parameter. This is all fine since the underlying type of
the alias is the generic parameter type, however it might have been
synthesized in a constrained extension, resulting in bogus diagnostics
that depend on the order in which declarations are type checked, which
can vary between WMO and non-WMO, different batch mode settings, etc.

Instead, let's just check the generic parameter list first.

Fixes <rdar://problem/22587551>, <rdar://problem/44777661>.
@slavapestov
Sema: Use TypeResolution::forInterface() to resolve type alias underl…
8f97511 
…ying types
@slavapestov slavapestov force-pushed the unqualified-lookup-fixes-part-2 branch from fa40c89 to 8f97511 Compare October 16, 2018 02:24
@slavapestov
Copy link
Contributor Author

@swift-ci Please test

@slavapestov
Copy link
Contributor Author

@swift-ci Please test source compatibility

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - fa40c89f6c31e4fcf108d46c8c568342ccd520ef

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - fa40c89f6c31e4fcf108d46c8c568342ccd520ef

@slavapestov
Copy link
Contributor Author 

22:07:36 ld: warning: Could not find auto-linked library 'swiftMetal'
22:07:36 Undefined symbols for architecture x86_64:
22:07:36   "__swift_FORCE_LOAD_$_swiftMetal", referenced from:
22:07:36       __swift_FORCE_LOAD_$_swiftMetal_$_Vision in Vision.o
22:07:36      (maybe you meant: __swift_FORCE_LOAD_$_swiftMetal_$_Vision)
22:07:36 ld: symbol(s) not found for architecture x86_64

@shahmishal This problem is back... any ideas?

@slavapestov
Copy link
Contributor Author

@swift-ci Please test source compatibility

2 similar comments
@slavapestov
Copy link
Contributor Author

@swift-ci Please test source compatibility

@slavapestov
Copy link
Contributor Author

@swift-ci Please test source compatibility

@slavapestov slavapestov merged commit 8954e5f into swiftlang:master Oct 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nkcsgexi nkcsgexi nkcsgexi left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@slavapestov @swift-ci @nkcsgexi