Remove Bogus Macro Assert in LUT Building #42436
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
This assertion assumes that only visibility directives for builtin-in macros find their way here. However, the presence of textual headers in modules means any macro directive can wind up here since clang will report that it is currently not building a module. It's been half a decade since this code did anything useful with macro directives in textual headers, so just leave things be. rdar://91768988
|
@swift-ci smoke test |
vsapsai
reviewed
Apr 20, 2022
There was a problem hiding this comment.
I don't know the code enough to make any claims. And maybe non-VisibilityMacroDirective macros are supposed to be handled somewhere else but that's not happening. But if that's not the case, the assertion that a macro directive must be VisibilityMacroDirective seems too strong.
| @@ -1969,12 +1969,6 @@ void importer::addMacrosToLookupTable(SwiftLookupTable &table, | |||
|
|
|||
| // If we're in a module, we really need moduleMacro to be valid. | |||
There was a problem hiding this comment.
Is this comment still correct? I don't have enough context, so not 100% sure.
DougGregor
approved these changes
Apr 20, 2022
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.
This assertion assumes that only visibility directives for builtin
macros find their way here. However, the presence of textual headers
in modules means any macro directive can wind up here since clang will
report that it is currently not building a module.
It's been half a decade since this code did anything useful with
macro directives in textual headers, so just leave things be.
rdar://91768988