Skip to content

Conversation

@davidungar
Copy link
Contributor

…pplementary output filemap.

If the supplementary output file map does not get transmitted correctly from the driver into the frontend, one symptom can be that not all primary inputs are represented when the frontend parses the map. This PR adds a check for that condition to the compiler.

@davidungar davidungar requested a review from jrose-apple March 22, 2018 19:59
@davidungar
Copy link
Contributor Author

@swift-ci please smoke test

Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick: no "the". (You can see a writeup I did about diagnostic phrasing in docs/Diagnostics.md.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick: paragraph breaks mean something in Doxygen (they separate the summary from the full description), but only if you do two of them. If you don't want that, please don't wrap before the end of the line.

I also don't think it's worth mentioning that the diagnostics will stop compilation. That's not something ArgsToFrontendOutputsConverter needs to think about.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The whole comment is gone, now.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd rather have this be part of the existing loop. It's one thing to do lookups that might fail, but it's another thing to repeat existing lookups just to keep two bits of code separate.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure! Folded in.

Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason for all these newlines?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

@davidungar
Copy link
Contributor Author

@swift-ci please smoke test

@jrose-apple
Copy link
Contributor

Ah, you forgot to push.

@davidungar davidungar force-pushed the PR-18-13-every_primary branch from 87da786 to bebb009 Compare March 22, 2018 21:20
@davidungar
Copy link
Contributor Author

@swift-ci please smoke test

@davidungar
Copy link
Contributor Author

Oops! Sorry. Now, I've pushed.

@davidungar davidungar force-pushed the PR-18-13-every_primary branch from bebb009 to 3376ca7 Compare March 22, 2018 21:58
@davidungar
Copy link
Contributor Author

@swift-ci please smoke test

@davidungar davidungar merged commit 29681ef into swiftlang:master Mar 22, 2018
@davidungar davidungar deleted the PR-18-13-every_primary branch May 16, 2018 16:59
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.

2 participants