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

Fix directives for lint-non-snake-case-crate #130860

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix directives for lint-non-snake-case-crate #130860

wants to merge 1 commit into from
+4 −4

Conversation

tmandry
Copy link
Member

@tmandry tmandry commented Sep 25, 2024 •
edited
Loading

This test fails on targets without unwinding or with --target-rustcflags=-Cpanic=abort because the proc macro was compiled as the target, not the host. Some targets were explicitly disabled to pass CI, but these directives are more general.

  • needs-dynamic-linking is self explanatory
  • force-host for proc macros
  • no-prefer-dynamic is apparently also used for proc macros

Note that needs-unwind can also be useful for situations other than proc macros where unwinding is necessary.

r? @jieyouxu

@tmandry
Fix directives for lint-non-snake-case-crate
c5925ef 
This test fails on targets without unwinding because the proc macro was
compiled as the target, not the host. Some targets were explicitly
disabled to pass CI, but these directives are more general.

Fixes Fuchsia tests.
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Sep 25, 2024
@compiler-errors
Copy link
Member

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Sep 26, 2024

📌 Commit c5925ef has been approved by compiler-errors

It is now in the queue for this repository.

@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 Sep 26, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Sep 26, 2024
@matthiaskrgr
Rollup merge of rust-lang#130860 - tmandry:fix-directives, r=compiler…
2a3de63 
…-errors

Fix directives for lint-non-snake-case-crate

This test fails on targets without unwinding or with `--target-rustcflags=-Cpanic=abort` because the proc macro was compiled as the target, not the host. Some targets were explicitly disabled to pass CI, but these directives are more general.

* `needs-dynamic-linking` is self explanatory
* `force-host` for proc macros
* `no-prefer-dynamic` is apparently also used for proc macros

Note that `needs-unwind` can also be useful for situations other than proc macros where unwinding is necessary.

r? `@jieyouxu`
@matthiaskrgr matthiaskrgr mentioned this pull request Sep 26, 2024
Closed
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 26, 2024
@bors
Auto merge of rust-lang#130871 - matthiaskrgr:rollup-2cfpxt7, r=matth…
dd8dd7f 
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#130820 (Fix diagnostics for coroutines with () as input.)
 - rust-lang#130833 (Fix the misleading diagnostic for `let_underscore_drop` on type without `Drop` implementation)
 - rust-lang#130845 (Utf8Chunks: add link to Utf8Chunk)
 - rust-lang#130860 (Fix directives for lint-non-snake-case-crate)
 - rust-lang#130861 (Use `mem::offset_of!` for `sockaddr_un.sun_path`)
 - rust-lang#130865 (Use `&raw` in the standard library)
 - rust-lang#130868 (Update FIXME comment in s390x_unknown_linux_*.rs)

r? `@ghost`
`@rustbot` modify labels: rollup
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Sep 26, 2024
@matthiaskrgr
Rollup merge of rust-lang#130860 - tmandry:fix-directives, r=compiler…
ba631b1 
…-errors

Fix directives for lint-non-snake-case-crate

This test fails on targets without unwinding or with `--target-rustcflags=-Cpanic=abort` because the proc macro was compiled as the target, not the host. Some targets were explicitly disabled to pass CI, but these directives are more general.

* `needs-dynamic-linking` is self explanatory
* `force-host` for proc macros
* `no-prefer-dynamic` is apparently also used for proc macros

Note that `needs-unwind` can also be useful for situations other than proc macros where unwinding is necessary.

r? ``@jieyouxu``
@matthiaskrgr matthiaskrgr mentioned this pull request Sep 26, 2024
Closed
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 26, 2024
@bors
Auto merge of rust-lang#130876 - matthiaskrgr:rollup-ntrxdhe, r=matth…
d80c3fe 
…iaskrgr

Rollup of 5 pull requests

Successful merges:

 - rust-lang#130820 (Fix diagnostics for coroutines with () as input.)
 - rust-lang#130833 (Fix the misleading diagnostic for `let_underscore_drop` on type without `Drop` implementation)
 - rust-lang#130845 (Utf8Chunks: add link to Utf8Chunk)
 - rust-lang#130860 (Fix directives for lint-non-snake-case-crate)
 - rust-lang#130868 (Update FIXME comment in s390x_unknown_linux_*.rs)

r? `@ghost`
`@rustbot` modify labels: rollup
@matthiaskrgr
Copy link
Member

@bors r-
#130876 (comment)

@bors bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Sep 26, 2024
@jieyouxu
Copy link
Member

Apparently needs-dynamic-linking is not equivalent to checking if dylib or cdylib crate types are supported.

  • In compiletest, needs-dynamic-linking performs a check based on target cfg's dynamic_linking field + --print=cfg --target $TARGET.
  • However, target cfg has an additional field only_cdylib which, if dynamic_linking is true, indicates that only cdylib crate type is supported and not dylib.

    rust/compiler/rustc_target/src/spec/mod.rs

    Lines 2148 to 2153 in f2becdf

    /// Whether dynamic linking is available on this target. Defaults to false.
    pub dynamic_linking: bool,
    /// Whether dynamic linking can export TLS globals. Defaults to true.
    pub dll_tls_export: bool,
    /// If dynamic linking is available, whether only cdylibs are supported.
    pub only_cdylib: bool,
    • This is the case for wasm base, dynamic linking is supported but not dylib crate type, only cdylib is supported.

      rust/compiler/rustc_target/src/spec/base/wasm.rs

      Lines 58 to 62 in f2becdf

      // we allow dynamic linking, but only cdylibs. Basically we allow a
      // final library artifact that exports some symbols (a wasm module) but
      // we don't allow intermediate `dylib` crate types
      dynamic_linking: true,
      only_cdylib: true,

In the short-run (for this PR), I think it's okay to keep the ignore-wasm, but please leave me a FIXME that states something like we should add a proper //@ target-supports-crate-type: rlib directive or something.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@compiler-errors compiler-errors compiler-errors approved these changes

Assignees

@jieyouxu jieyouxu

Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@tmandry @compiler-errors @bors @matthiaskrgr @jieyouxu @rustbot