Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Update no_default_libraries handling for emscripten target #98355

Merged
merged 2 commits into from
Jun 24, 2022
Merged

Update no_default_libraries handling for emscripten target #98355

merged 2 commits into from
Jun 24, 2022

Conversation

hoodmane
Copy link
Contributor

@sbc100 says:

-sDEFAULT_LIBRARY_FUNCS_TO_INCLUDE=[] is almost certainly wrong/out-of-date. This setting defaults to the empty list anyway these days so its redundant. Also we now support -nodefaultlibs so you can use that, as with other toolchains.

#98303 (comment)

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jun 21, 2022
@rust-highfive
Copy link
Collaborator

r? @nagisa

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 21, 2022
@hoodmane
Copy link
Contributor Author

r? @petrochenkov

@rust-highfive rust-highfive assigned petrochenkov and unassigned nagisa Jun 21, 2022
@hoodmane hoodmane changed the title Update Emscripten's no_default_libraries handling Update no_default_libraries handling for emscripten target Jun 21, 2022
@petrochenkov
Copy link
Contributor

Given that no_default_libraries is currently true for emscripten targets it means that rustc will be in charge of passing or not passing -lc and similar.
Are you sure that's what you want, given that rustc is not aware of e.g. side modules?

@petrochenkov petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 22, 2022
@hoodmane
Copy link
Contributor Author

I think this fixes the no_default_libraries function. Maybe it is a separate issue whether to set no_default_libraries: true or no_default_libraries: false? I agree it would probably be best to set no_default_libraries: false, I can do that in this PR too if you like.

@hoodmane
Copy link
Contributor Author

@rustbot label -S-waiting-on-author +S-waiting-on-review

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 23, 2022
@petrochenkov
Copy link
Contributor

Ok, this PR keeps the status quo behavior now.
@bors r+ rollup

@bors
Copy link
Contributor

bors commented Jun 23, 2022

📌 Commit b96ae9b has been approved by petrochenkov

@bors
Copy link
Contributor

bors commented Jun 23, 2022

🌲 The tree is currently closed for pull requests below priority 1000. This pull request will be tested once the tree is reopened.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 23, 2022
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jun 23, 2022
@matthiaskrgr
Rollup merge of rust-lang#98355 - hoodmane:emscripten-no-default, r=p…
0252df9 
…etrochenkov

Update no_default_libraries handling for emscripten target

`@sbc100` says:

> `-sDEFAULT_LIBRARY_FUNCS_TO_INCLUDE=[]` is almost certainly wrong/out-of-date.   This setting defaults to the empty list anyway these days so its redundant.  Also we now support `-nodefaultlibs` so you can use that, as with other toolchains.

rust-lang#98303 (comment)
@matthiaskrgr matthiaskrgr mentioned this pull request Jun 23, 2022
Closed
compiler-errors added a commit to compiler-errors/rust that referenced this pull request Jun 23, 2022
@compiler-errors
Rollup merge of rust-lang#98355 - hoodmane:emscripten-no-default, r=p…
060aeed 
…etrochenkov

Update no_default_libraries handling for emscripten target

``@sbc100`` says:

> `-sDEFAULT_LIBRARY_FUNCS_TO_INCLUDE=[]` is almost certainly wrong/out-of-date.   This setting defaults to the empty list anyway these days so its redundant.  Also we now support `-nodefaultlibs` so you can use that, as with other toolchains.

rust-lang#98303 (comment)
@compiler-errors compiler-errors mentioned this pull request Jun 23, 2022
Closed
@compiler-errors compiler-errors mentioned this pull request Jun 23, 2022
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 24, 2022
@bors
Auto merge of rust-lang#98438 - compiler-errors:rollup-fudubjn, r=com…
e605a88 
…piler-errors

Rollup of 16 pull requests

Successful merges:

 - rust-lang#96173 (Fix documentation for  `with_capacity` and `reserve` families of methods)
 - rust-lang#98184 (Give name if anonymous region appears in impl signature)
 - rust-lang#98259 (Greatly improve error reporting for futures and generators in `note_obligation_cause_code`)
 - rust-lang#98269 (Provide a `PathSegment.res` in more cases)
 - rust-lang#98283 (Point at private fields in struct literal)
 - rust-lang#98305 (prohibit_generics: don't alloc error string if no error emitted)
 - rust-lang#98310 (rustdoc: optimize loading of source sidebar)
 - rust-lang#98353 (Migrate two diagnostics from the `rustc_builtin_macros` crate)
 - rust-lang#98355 (Update no_default_libraries handling for emscripten target)
 - rust-lang#98364 (clarify Arc::clone overflow check comment)
 - rust-lang#98365 (Address review comments from rust-lang#98259)
 - rust-lang#98388 (implement `iter_projections` function on `PlaceRef`)
 - rust-lang#98390 (Fixes handling of keywords in rustdoc json output)
 - rust-lang#98409 (triagebot.toml: Allow applying nominated labels)
 - rust-lang#98410 (Update books)
 - rust-lang#98422 (Update browser-ui-test version to 0.9.6)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit cc45ad5 into rust-lang:master Jun 24, 2022
@rustbot rustbot added this to the 1.63.0 milestone Jun 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sbc100 sbc100 sbc100 approved these changes

Assignees

@petrochenkov petrochenkov

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.63.0
Development

Successfully merging this pull request may close these issues.
7 participants
@hoodmane @rust-highfive @petrochenkov @bors @sbc100 @nagisa @rustbot