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.

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

Clean up uses of the unstable dwarf_version option #135739

Merged
merged 2 commits into from
Jan 30, 2025

Conversation

wesleywiser
Copy link
Member

  • Consolidate calculation of the effective value.
  • Check the target DebuginfoKind instead of using is_like_msvc.
  • Add the tracking issue to the unstable book page for this feature.

cc #103057

@rustbot
Copy link
Collaborator

rustbot commented Jan 19, 2025

r? @chenyukang

rustbot has assigned @chenyukang.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@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 Jan 19, 2025
@wesleywiser wesleywiser force-pushed the dwarf_version_handling branch from 89c7fcc to a9fae0d Compare January 19, 2025 20:59
@wesleywiser wesleywiser changed the title Clean up uses of the unstable dwarf_verison option Clean up uses of the unstable dwarf_version option Jan 19, 2025
@lqd
Copy link
Member

lqd commented Jan 19, 2025

Sweet, r? lqd and r=me with a tiny doc comment and green CI. Thanks!

@rustbot rustbot assigned lqd and unassigned chenyukang Jan 19, 2025
@wesleywiser wesleywiser force-pushed the dwarf_version_handling branch from a9fae0d to 26c10d0 Compare January 19, 2025 22:02
@mati865
Copy link
Contributor

mati865 commented Jan 20, 2025

I think this might break windows-gnu targets. Their spec for some reason states pdb debuginfo but they use dwarf. Previously this was handled by is_like_msvc.

debuginfo_kind: DebuginfoKind::Pdb,

@wesleywiser
Copy link
Member Author

Thanks for pointing that out! That seems super weird. I'll look into that tomorrow.

@lqd
Copy link
Member

lqd commented Jan 20, 2025

Weird.

We should document that in both cg_llvm and the target specs themselves. And/or maybe the centralization should be in rustc_target/TargetOptions (but made available via the Session if you still want to, without that logic), since it's both target-specific and has subtle rules (that can contradict the spec fields).

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jan 22, 2025
…uginfokind, r=lqd

Update windows-gnu targets to set `DebuginfoKind::DWARF`

These targets have always used DWARF debuginfo and not CodeView/PDB debuginfo like the MSVC Windows targets. However, their target definitions claim to use `DebuginfoKind::PDB` probably to ensure that we do not try to allow the use of split-DWARF debuginfo.

This does not appear to be necessary since the targets set their supported split debug info to `Off`. I've looked at all of the uses of these properties and this patch does not appear to cause any functional changes in compiler behavior. I also added UI tests to attempt to validate there is no change in the behavior of these options on stable compilers.

cc `@mati865` since you mentioned this in rust-lang#135739
cc `@davidtwco` for split-dwarf
jhpratt added a commit to jhpratt/rust that referenced this pull request Jan 23, 2025
…uginfokind, r=lqd

Update windows-gnu targets to set `DebuginfoKind::DWARF`

These targets have always used DWARF debuginfo and not CodeView/PDB debuginfo like the MSVC Windows targets. However, their target definitions claim to use `DebuginfoKind::PDB` probably to ensure that we do not try to allow the use of split-DWARF debuginfo.

This does not appear to be necessary since the targets set their supported split debug info to `Off`. I've looked at all of the uses of these properties and this patch does not appear to cause any functional changes in compiler behavior. I also added UI tests to attempt to validate there is no change in the behavior of these options on stable compilers.

cc ``@mati865`` since you mentioned this in rust-lang#135739
cc ``@davidtwco`` for split-dwarf
jieyouxu added a commit to jieyouxu/rust that referenced this pull request Jan 23, 2025
…uginfokind, r=lqd

Update windows-gnu targets to set `DebuginfoKind::DWARF`

These targets have always used DWARF debuginfo and not CodeView/PDB debuginfo like the MSVC Windows targets. However, their target definitions claim to use `DebuginfoKind::PDB` probably to ensure that we do not try to allow the use of split-DWARF debuginfo.

This does not appear to be necessary since the targets set their supported split debug info to `Off`. I've looked at all of the uses of these properties and this patch does not appear to cause any functional changes in compiler behavior. I also added UI tests to attempt to validate there is no change in the behavior of these options on stable compilers.

cc ```@mati865``` since you mentioned this in rust-lang#135739
cc ```@davidtwco``` for split-dwarf
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jan 23, 2025
…uginfokind, r=lqd

Update windows-gnu targets to set `DebuginfoKind::DWARF`

These targets have always used DWARF debuginfo and not CodeView/PDB debuginfo like the MSVC Windows targets. However, their target definitions claim to use `DebuginfoKind::PDB` probably to ensure that we do not try to allow the use of split-DWARF debuginfo.

This does not appear to be necessary since the targets set their supported split debug info to `Off`. I've looked at all of the uses of these properties and this patch does not appear to cause any functional changes in compiler behavior. I also added UI tests to attempt to validate there is no change in the behavior of these options on stable compilers.

cc ````@mati865```` since you mentioned this in rust-lang#135739
cc ````@davidtwco```` for split-dwarf
@wesleywiser
Copy link
Member Author

Now that #135790 has landed, I think this is good to go.

@lqd
Copy link
Member

lqd commented Jan 28, 2025

r=me with the link fixed

- Consolidate calculation of the effective value.
- Check the target `DebuginfoKind` instead of using `is_like_msvc`.
@wesleywiser wesleywiser force-pushed the dwarf_version_handling branch from 26c10d0 to 4d5a63f Compare January 30, 2025 03:45
@lqd
Copy link
Member

lqd commented Jan 30, 2025

Thanks! @bors r+

@bors
Copy link
Contributor

bors commented Jan 30, 2025

📌 Commit 4d5a63f has been approved by lqd

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 Jan 30, 2025
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jan 30, 2025
…, r=lqd

Clean up uses of the unstable `dwarf_version` option

- Consolidate calculation of the effective value.
- Check the target `DebuginfoKind` instead of using `is_like_msvc`.
- Add the tracking issue to the unstable book page for this feature.

cc rust-lang#103057
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 30, 2025
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#133636 ([rustdoc] Add sans-serif font setting)
 - rust-lang#135434 (Match Ergonomics 2024: update edition 2024 behavior of feature gates)
 - rust-lang#135739 (Clean up uses of the unstable `dwarf_version` option)
 - rust-lang#135882 (simplify `similar_tokens` from `Option<Vec<_>>` to `&[_]`)
 - rust-lang#136179 (Allow transmuting generic pattern types to and from their base)
 - rust-lang#136199 (Fix a couple Emscripten tests)
 - rust-lang#136238 (ci: refactor how directories are removed in free-disk-space disk)
 - rust-lang#136251 (use impl Into<String> instead of explicit type args with bounds)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 30, 2025
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#133636 ([rustdoc] Add sans-serif font setting)
 - rust-lang#135434 (Match Ergonomics 2024: update edition 2024 behavior of feature gates)
 - rust-lang#135739 (Clean up uses of the unstable `dwarf_version` option)
 - rust-lang#135882 (simplify `similar_tokens` from `Option<Vec<_>>` to `&[_]`)
 - rust-lang#136179 (Allow transmuting generic pattern types to and from their base)
 - rust-lang#136199 (Fix a couple Emscripten tests)
 - rust-lang#136251 (use impl Into<String> instead of explicit type args with bounds)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit aedc0a3 into rust-lang:master Jan 30, 2025
6 checks passed
@rustbot rustbot added this to the 1.86.0 milestone Jan 30, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jan 30, 2025
Rollup merge of rust-lang#135739 - wesleywiser:dwarf_version_handling, r=lqd

Clean up uses of the unstable `dwarf_version` option

- Consolidate calculation of the effective value.
- Check the target `DebuginfoKind` instead of using `is_like_msvc`.
- Add the tracking issue to the unstable book page for this feature.

cc rust-lang#103057
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

Successfully merging this pull request may close these issues.

6 participants