Error if -no-crt is used without -no-thread-locals
#4704
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
Now using any thread-local variables with -no-crt enabled will cause a compiler error, unless -no-thread-local is given. Also fixed a minor typo in a comment.
ae960c4 to
8bc19e1
Compare
8bc19e1 to
3f20b63
Compare
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 PR enforces the use of
-no-thread-localswhen compiling with-no-crt. The reasoning is as follows: There are a few places in thebase:collection that initialize things with thread locals, as well as a bunch of places in thecore:collection. When-no-crtis used by itself, you'll just crash, because there's no libc to initialize the TLS and any access to TLS would lead to a crash. This makes it seem like an unintended behavior (#4614), but it is in fact very intended, because at the current stage we can't build our own TLS model, and we can't match any dynamic loaders' TLS model.This PR resolves #4614 by making it an explicit error.
There's also a small typo fix in a comment that caught my eye. Oh, and I added
-default-to-panic-allocatorto-help, because just noticed it wasn't there.