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

Add a new -Z force-full-debuginfo flag #109311

Closed
wants to merge 1 commit into from

Conversation

jyn514
Copy link
Member

@jyn514 jyn514 commented Mar 18, 2023

Apparently firefox depends on the current behavior that adds too much debuginfo, but that doesn't mean we need to force it on for rustc itself. Add a new unstable flag for turning it off.

cc #104968, #64405

r? @cuviper

@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 Mar 18, 2023
@jyn514 jyn514 added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 18, 2023
@rust-log-analyzer

This comment has been minimized.

Apparently firefox depends on the current behavior that adds too much
debuginfo, but that doesn't mean we need to force it on for rustc
itself. Add a new unstable flag for turning it off.
@jyn514 jyn514 force-pushed the force-force-debuginfo branch from 51b891c to eb34b82 Compare March 18, 2023 17:19
@jyn514 jyn514 added A-debuginfo Area: Debugging information in compiled programs (DWARF, PDB, etc.) A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. labels Mar 18, 2023
@@ -1425,6 +1425,8 @@ options! {
flatten_format_args: bool = (false, parse_bool, [TRACKED],
"flatten nested format_args!() and literals into a simplified format_args!() call \
(default: no)"),
force_full_debuginfo: bool = (true, parse_bool, [TRACKED],
"force all codegen-units to have full debuginfo even for -C debuginfo=1. see #64405 (default: yes)"),
Copy link
Member

@bjorn3 bjorn3 Mar 19, 2023

Choose a reason for hiding this comment

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

This is not entirely correct. It merely causes more debuginfo to be added, not everything. For example function arguments aren't added to the debuginfo. Just basic things like the linkage name and demangled function name.

@jyn514
Copy link
Member Author

jyn514 commented Mar 20, 2023

@michaelwoerister mentioned on Zulip that #83947 already has an approved FCP exposing this as a stable option - happy to rebase that PR if you'd prefer.

@cuviper
Copy link
Member

cuviper commented Mar 23, 2023

Reviving #83947 seems like a better long-term fix to me.

@jyn514 jyn514 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 Mar 23, 2023
@jyn514
Copy link
Member Author

jyn514 commented Mar 31, 2023

#109808

@jyn514 jyn514 closed this Mar 31, 2023
@jyn514 jyn514 deleted the force-force-debuginfo branch March 31, 2023 09:42
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 4, 2023
…oerister

Extend -Cdebuginfo with new options and named aliases

This is a rebase of rust-lang#83947, along with my best guess at what the new options mean. I tried to follow the LLVM source code to get a better idea but ran into quite a lot of trouble (https://rust-lang.zulipchat.com/#narrow/stream/187780-t-compiler.2Fwg-llvm/topic/go-to-definition.20in.20src.2Fllvm-project.3F). The description for the original PR follows below.

Note that the changes in this PR have already been through FCP: rust-lang#83947 (comment)

Closes rust-lang#109311. Helps with rust-lang#104968.
r? `@michaelwoerister` cc `@cuviper`

---

The -Cdebuginfo=1 option was never line tables only and can't be due to backwards compatibility issues. This was clarified and an option for emitting line tables only was added. Additionally an option for emitting line info directives only was added, which is needed for some targets, i.e. nvptx. The debug info options should now behave similarly to clang's debug info options.

Fix rust-lang#60020
Fix rust-lang#64405
saethlin pushed a commit to saethlin/miri that referenced this pull request Apr 10, 2023
Extend -Cdebuginfo with new options and named aliases

This is a rebase of rust-lang/rust#83947, along with my best guess at what the new options mean. I tried to follow the LLVM source code to get a better idea but ran into quite a lot of trouble (https://rust-lang.zulipchat.com/#narrow/stream/187780-t-compiler.2Fwg-llvm/topic/go-to-definition.20in.20src.2Fllvm-project.3F). The description for the original PR follows below.

Note that the changes in this PR have already been through FCP: rust-lang/rust#83947 (comment)

Closes rust-lang/rust#109311. Helps with rust-lang/rust#104968.
r? `@michaelwoerister` cc `@cuviper`

---

The -Cdebuginfo=1 option was never line tables only and can't be due to backwards compatibility issues. This was clarified and an option for emitting line tables only was added. Additionally an option for emitting line info directives only was added, which is needed for some targets, i.e. nvptx. The debug info options should now behave similarly to clang's debug info options.

Fix rust-lang/rust#60020
Fix rust-lang/rust#64405
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-debuginfo Area: Debugging information in compiled programs (DWARF, PDB, etc.) A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. 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
Development

Successfully merging this pull request may close these issues.

5 participants