Skip to content

[ClangImporter] Handle diagnostics about cast types in macros #13972

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

Merged
merged 2 commits into from
Jan 19, 2018

Conversation

jrose-apple
Copy link
Contributor

The importer handles these by first trying to look up the type by name using Clang's Sema, but that lookup can cause diagnostics to be emitted (usually availability diagnostics). We could try to figure out how to propagate that to the macro when we import it, but for now just drop the macro instead if there are any diagnostics emitted when looking up the type.

This will be a small source compatibility break if anyone was using a macro defined in terms of a type that's deprecated or that has partial availability; the macro will now silently not be imported instead of producing an unsilenceable warning.

rdar://problem/36528212

The importer handles these by first trying to look up the type by name
using Clang's Sema, but that lookup can cause diagnostics to be
emitted (usually availability diagnostics). We could try to figure out
how to propagate that to the macro when we import it, but for now just
drop the macro instead if there are any diagnostics emitted when
looking up the type.

This will be a small source compatibility break if anyone was using a
macro defined in terms of a type that's deprecated or that has partial
availability; the macro will now silently not be imported instead of
producing an unsilenceable warning.

rdar://problem/36528212
@jrose-apple
Copy link
Contributor Author

jrose-apple commented Jan 16, 2018

Since this is a well-defined, isolated source compatibility break that's independent of already-discussed language features, the core team has to review it, right? @DougGregor, @rjmccall, how do you want to handle that?

@jrose-apple
Copy link
Contributor Author

@swift-ci Please test

@jrose-apple
Copy link
Contributor Author

@swift-ci Please test source compatibility

@slavapestov
Copy link
Contributor

I don't think we want a full review process, but it would be nice if these "mini tweaks" were at least documented somewhere. Perhaps CHANGELOG.md?

Hey, it's the first Swift 5 changelog note.
@jrose-apple
Copy link
Contributor Author

(Both the regular test and the source compat suite passed already.)

@swift-ci Please smoke test

@jrose-apple jrose-apple merged commit 59ceb22 into swiftlang:master Jan 19, 2018
@jrose-apple jrose-apple deleted the cast-out-the-macros branch January 19, 2018 22:57
@jrose-apple
Copy link
Contributor Author

jrose-apple commented Jan 19, 2018

Merged ahead of any such discussion to clear out some warnings on internal Apple bots. We can revisit if the core team decides this is unacceptable.

@rjmccall
Copy link
Contributor

I think that's reasonable.

jrose-apple added a commit to jrose-apple/swift that referenced this pull request Jan 23, 2018
[ClangImporter] Handle diagnostics about cast types in macros

(cherry picked from commit 59ceb22)
jrose-apple added a commit that referenced this pull request Jan 23, 2018
[ClangImporter] Handle diagnostics about cast types in macros

(cherry picked from commit 59ceb22)
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.

3 participants