Skip to content

[NFC] Add Compilation::Result #35034

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 5 commits into from
Dec 10, 2020
Merged

[NFC] Add Compilation::Result #35034

merged 5 commits into from
Dec 10, 2020

Conversation

CodaFi
Copy link
Contributor

@CodaFi CodaFi commented Dec 10, 2020

Add a data structure to encapsulate the return code, the "had abnormal exit" bit, and the module dependency graph. The last of these is needed for a future patch to turn on cross-module incremental builds.

In order to extract the module dependency graph from the compilation the driver just ran, define a separate semantic type to hold a result code and the graph itself.
This required dropping some superfluous 'const' qualification in the module dependency graph.
@CodaFi
Copy link
Contributor Author

CodaFi commented Dec 10, 2020

@swift-ci smoke test

Copy link
Contributor

@davidungar davidungar left a comment

Choose a reason for hiding this comment

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

LGTM!

@CodaFi
Copy link
Contributor Author

CodaFi commented Dec 10, 2020

@CodaFi CodaFi merged commit 07e3f30 into swiftlang:main Dec 10, 2020
@CodaFi CodaFi deleted the turnout-results branch December 10, 2020 05:58
CodaFi pushed a commit that referenced this pull request Dec 12, 2020
VS2017 seems to have problems with ternaries where the two branches are
given as std::move() (see https://stackoverflow.com/questions/53374182)
and the compiler was given a warning C4172 "returning address of local
or temporary".

Switching the ternary operator into a `if/else` works correctly for
VS2017 and should work for other compilers too, without changing the
meaning (as far as I see).

This was causing the Windows VS2017 CI to fail compiling the stdlib.
When bisecting, the problem was reduced to the changes in #35034. In a
debugger I managed to see the problem being a heap corruption. With all
the moves I was suspicious that something was being used while
destroyed, but the code was correct. While recompiling the compiler
warning gave me the clue.
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