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

Tracking Issue for #[collapse_debuginfo] #100758

Closed
1 of 4 tasks
davidtwco opened this issue Aug 19, 2022 · 6 comments · Fixed by #120845
Closed
1 of 4 tasks

Tracking Issue for #[collapse_debuginfo] #100758

davidtwco opened this issue Aug 19, 2022 · 6 comments · Fixed by #120845
Labels
A-debuginfo Area: Debugging information in compiled programs (DWARF, PDB, etc.) C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC F-collapse_debuginfo `#![feature(collapse_debuginfo)]` T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@davidtwco
Copy link
Member

This is a tracking issue for the #[collapse_debuginfo] attribute (rust-lang/compiler-team#386).
The feature gate for the issue is #![feature(collapse_debuginfo)].

About tracking issues

Tracking issues are used to record the overall progress of implementation.
They are also used as hubs connecting to other relevant issues, e.g., bugs or open design questions.
A tracking issue is however not meant for large scale discussion, questions, or bug reports about a feature.
Instead, open a dedicated issue for the specific matter and add the relevant feature gate label.

Steps

Unresolved Questions

Implementation history

@davidtwco davidtwco added A-debuginfo Area: Debugging information in compiled programs (DWARF, PDB, etc.) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC F-collapse_debuginfo `#![feature(collapse_debuginfo)]` labels Aug 19, 2022
@petrochenkov
Copy link
Contributor

Some conclusions from discussing #118903 with @azhogin:

  • The final accumulated value of collapse_debuginfo: bool is determined at a specific call site, so we can use other properties than just #[collapse_debuginfo] on the macro definition. For example, whether the macro comes from the current crate or some other crate. (Value of -Z debug-macros for the current crate is already used like this, in particular).
  • We could make an option or crate-level attribute -C collapse-debuginfo/#![collapse_debuginfo] setting the global default for unmarked macros, as a replacement for -Zdebug-macros.
  • Both the option and the attributes could be configurable to make them cover more cases, for example #[collapse_debuginfo(always|never|local|external|something_else)] and -Ccollapse_debuginfo=always|never|local|external|something_else.
  • #[collapse_debuginfo(external)] looks like a reasonable default for argument-less #[collapse_debuginfo], because you almost always want to go inside locally defined macros? (Standard library macros are not locally defined in almost all scenarios.)
  • The global option should be able to override local annotations if it is "less collapsed", because it would be good to be able to step inside anything regardless of definition-site attributes if really necessary.
  • Built-in macros can be made collapse_debuginfo(true) by default in the compiler to avoid library annotation noise, because for majority of them the compiler-generated code has no real location in source code besides call site. There are some exceptions though, like include!, which have real non-call-site locations in generated code.
  • The use of feature(collapse_debuginfo) to change behavior instead of just gating the attributes is not idiomatic, features are not supposed to be used like this. The global default should be set in some other way.

bors added a commit to rust-lang-ci/rust that referenced this issue Dec 25, 2023
…uginfo.rs, r=petrochenkov

Improved support of collapse_debuginfo attribute for macroses.

Added walk_chain_collapsed function to consider collapse_debuginfo attribute in parent macroses in call chain.
Fixed collapse_debuginfo attribute processing for cranelift (there was if/else branches error swap).
Enabled attribute by default for builtin and core/std macroses.

cc rust-lang#100758
bors added a commit to rust-lang-ci/rust that referenced this issue Dec 27, 2023
…uginfo.rs, r=petrochenkov

Improved support of collapse_debuginfo attribute for macros.

Added walk_chain_collapsed function to consider collapse_debuginfo attribute in parent macros in call chain.
Fixed collapse_debuginfo attribute processing for cranelift (there was if/else branches error swap).

cc rust-lang#100758
bors added a commit to rust-lang-ci/rust that referenced this issue Dec 28, 2023
…uginfo.rs, r=petrochenkov

Improved support of collapse_debuginfo attribute for macros.

Added walk_chain_collapsed function to consider collapse_debuginfo attribute in parent macros in call chain.
Fixed collapse_debuginfo attribute processing for cranelift (there was if/else branches error swap).

cc rust-lang#100758
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jan 8, 2024
…ebuginfo.rs, r=petrochenkov

Improved support of collapse_debuginfo attribute for macros.

Added walk_chain_collapsed function to consider collapse_debuginfo attribute in parent macros in call chain.
Fixed collapse_debuginfo attribute processing for cranelift (there was if/else branches error swap).

cc rust-lang#100758
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jan 8, 2024
…ebuginfo.rs, r=petrochenkov

Improved support of collapse_debuginfo attribute for macros.

Added walk_chain_collapsed function to consider collapse_debuginfo attribute in parent macros in call chain.
Fixed collapse_debuginfo attribute processing for cranelift (there was if/else branches error swap).

cc rust-lang#100758
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jan 8, 2024
…ebuginfo.rs, r=petrochenkov

Improved support of collapse_debuginfo attribute for macros.

Added walk_chain_collapsed function to consider collapse_debuginfo attribute in parent macros in call chain.
Fixed collapse_debuginfo attribute processing for cranelift (there was if/else branches error swap).

cc rust-lang#100758
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jan 8, 2024
…ebuginfo.rs, r=petrochenkov

Improved support of collapse_debuginfo attribute for macros.

Added walk_chain_collapsed function to consider collapse_debuginfo attribute in parent macros in call chain.
Fixed collapse_debuginfo attribute processing for cranelift (there was if/else branches error swap).

cc rust-lang#100758
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Jan 9, 2024
Rollup merge of rust-lang#118903 - azhogin:azhogin/skip_second_stmt_debuginfo.rs, r=petrochenkov

Improved support of collapse_debuginfo attribute for macros.

Added walk_chain_collapsed function to consider collapse_debuginfo attribute in parent macros in call chain.
Fixed collapse_debuginfo attribute processing for cranelift (there was if/else branches error swap).

cc rust-lang#100758
compiler-errors added a commit to compiler-errors/rust that referenced this issue Jan 17, 2024
…_improved_attr, r=petrochenkov

Improved collapse_debuginfo attribute, added command-line flag

Improved attribute collapse_debuginfo with variants: `#[collapse_debuginfo=(no|external|yes)]`.
Added command-line flag for default behaviour.
Work-in-progress: will add more tests.

cc rust-lang#100758
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jan 18, 2024
…_improved_attr, r=petrochenkov

Improved collapse_debuginfo attribute, added command-line flag

Improved attribute collapse_debuginfo with variants: `#[collapse_debuginfo=(no|external|yes)]`.
Added command-line flag for default behaviour.
Work-in-progress: will add more tests.

cc rust-lang#100758
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Jan 19, 2024
Rollup merge of rust-lang#119828 - azhogin:azhogin/collapse_debuginfo_improved_attr, r=petrochenkov

Improved collapse_debuginfo attribute, added command-line flag

Improved attribute collapse_debuginfo with variants: `#[collapse_debuginfo=(no|external|yes)]`.
Added command-line flag for default behaviour.
Work-in-progress: will add more tests.

cc rust-lang#100758
github-actions bot pushed a commit to rust-lang/miri that referenced this issue Jan 21, 2024
…_attr, r=petrochenkov

Improved collapse_debuginfo attribute, added command-line flag

Improved attribute collapse_debuginfo with variants: `#[collapse_debuginfo=(no|external|yes)]`.
Added command-line flag for default behaviour.
Work-in-progress: will add more tests.

cc rust-lang/rust#100758
bjorn3 pushed a commit to rust-lang/rustc_codegen_cranelift that referenced this issue Jan 21, 2024
…rs, r=petrochenkov

Improved support of collapse_debuginfo attribute for macros.

Added walk_chain_collapsed function to consider collapse_debuginfo attribute in parent macros in call chain.
Fixed collapse_debuginfo attribute processing for cranelift (there was if/else branches error swap).

cc rust-lang/rust#100758
@petrochenkov
Copy link
Contributor

I think we need to stabilize the -Z debug-macros option as soon as possible (or rather enable it by default and remove).
It's not even directly tied to stabilization of #[collapse_debuginfo], which can be used in the standard library even if it's unstable.

In any case, I'll try to stabilize #[collapse_debuginfo] sooner too, in the next comments I'll try to describe some possible debugging scenarios for it.

@petrochenkov
Copy link
Contributor

First of all, none of these features (-Z debug-macros, collapse_debuginfo) have anything to do with debugging code written in the macro input.

dsl! {
  some();
  macro();
  taking();
  multiline();
  input();
  and();
  processing();
  it();
  in();
  some();
  way();
}

If the macro preserves the input spans in its generated code, then we'll just go through input lines in the order in which they exist in the generated code.
This is an important property and debugging of macros with multi-line input should indeed behave that way, so everything is fine here.

The -Z debug-macros/collapse_debuginfo features are for code with spans located at a macro definition, like this.

macro generate_code() {
  some();
  generated();
  code();
}

generate_code!();

@petrochenkov
Copy link
Contributor

petrochenkov commented Feb 9, 2024

Debugging scenarios for code with macros.

Definitions

  • Crate of focus - crate in which we are currently debugging some code, the code may include macro invocations.
  • Editable crate - crate that we can edit to add some #[collapse_debuginfo] attributes.
  • Rebuildable crate - crate that we can rebuild with some -Ccollapse-macro-debuginfo options.
    Let's assume that we have source code, then pretty much everything is rebuildable, even standard library, but maybe with some effort.
    If we don't have source code, then we have larger problems with debugging.

The end binary

  • We may debug an executable crate that contains most of its code, then the executable is the crate of focus, it's editable and rebuildable.
  • We may debug an executable crate that is a thin wrapper around a different library crate, or a number of crates, then the library is the crate of focus, it's editable and rebuildable.
  • Something may go wrong in one of third-party dependencies, including standard library, then that library is the crate of focus, it's not editable but rebuildable.

In all scenarios the focus crate may depend on other crates, including standard library.

How can options be passed if necessary

  • -Ccollapse-macro-debuginfo can be passed using cargo rustc.
    I guess that's not a very common case.
  • -Ccollapse-macro-debuginfo can be passed when compiling all crates in the dependency tree using RUSTFLAGS
    • it is typically not passed to the standard library even in this case, but it can potentially be with -Zbuild-std
  • -Ccollapse-macro-debuginfo can be passed to specific dependency packages using this feature

General assumptions

Very few people are going to care about debugging and add #[collapse_debuginfo] annotations to their code, standard library may be one of the exceptions.

As a result we should probably default on the side of revealing more information, i.e. not collapsing, so that manual uncollapsing with editing or rebuilding with -Ccollapse-macro-debuginfo is not common.
It should also always be possible to reveal more information using RUSTFLAGS or cargo rustc.

If something is uncollapsed undesirably, there's a universal way to deal with it - set a breakpoint after the macro call and continue, which seems like not a big issue.

Macros defined in the crate of focus

We probably want to uncollapse such macros by default, to side on revealing more.

The crate of focus is typically editable, so we can usually temporarily add #[collapse_debuginfo] attributes if the default is unsuitable.

Macro defined outside of the crate of focus

One special case of this is the standard library.
According to the survey in #39153 (comment) most macros in it should be collapsed.

Default behavior for other crates defining macros is probably the main question of this stabilization.
I suggest following the example of the standard library in this case, and default to collapsing external macros from other crates as well.

The crate of focus can always be rebuilt with -Ccollapse-macro-debuginfo=false if it's necessary to uncollapse external macros, or maybe add #[collapse_debuginfo(false)] if the corresponding crate is editable, or just use "continue to breakpoint".

Note that macro-only crates are always external (that includes proc macro crates).

When we start debugging non-macro code in a different crate, macros defined in that crate become local.
E.g. println! becomes local and gets uncollapsed if we are debugging code inside libstd, which seems fine.

Procedural macros

Proc macros may actually have locations pointing to their definition (generated by quote! in case of proc macros).
#[collapse_debuginfo] attribute is currently not supported for proc macros, and considered false, this is fine, the standard quote! is unstable and rarely used anyway.
It may be supported in the future.

Other heuristics

We can use other heuristics for (un)collapsing macros by default, besides extern-ness, but I cannot think of any good ones.

  • Number of lines in the input or output - unlikely.
    • Macros with multiple lines in the input are more likely to be DSL-like, macros with a single input line are more likely to be function-like, but that doesn't say whether we should collapse or not.
  • Change extern-ness to some other definition of "own code" - e.g. all crates in the current workspace, we have no precedent for this in rustc so far.

Macros used to avoid boilerplate and code duplication seem to be the primary candidates for uncollapsing, such macros are more likely to be local and tailored for specific tasks, but otherwise I don't see good and simple heuristics for detecting them.

@petrochenkov
Copy link
Contributor

Basically, I suggest choosing #[collapse_debuginfo(external)] as the default and stabilizing everything, based on the comment above.

@c410-f3r
Copy link
Contributor

c410-f3r commented Feb 9, 2024

From what I read (unless I am mistaken), the stabilization of #[collapse_debuginfo] is not related to the unresolved question.

I am saying this because #95152 and the theoretical applicability of #[track_caller] in macro definitions is still something I would like to tackle in a future with lots of free time.

bors added a commit to rust-lang-ci/rust that referenced this issue Feb 12, 2024
debuginfo: Stabilize `-Z debug-macros`, `-Z collapse-macro-debuginfo` and `#[collapse_debuginfo]`

`-Z debug-macros` is "stabilized" by enabling it by default and removing.

`-Z collapse-macro-debuginfo` is stabilized as `-C collapse-macro-debuginfo`.
It now supports all typical boolean values (`parse_opt_bool`) in addition to just yes/no.

Default value of `collapse_debuginfo` was changed from `false` to `external` (i.e. collapsed if external, not collapsed if local) - rust-lang#100758 (comment) describes some debugging scenarios that motivate this default as reasonable.
`#[collapse_debuginfo]` attribute without a value is no longer supported to avoid guessing the default.

Closes rust-lang#100758
Closes rust-lang#41743
Closes rust-lang#39153

TODO: Stabilization report
bors added a commit to rust-lang-ci/rust that referenced this issue Apr 25, 2024
debuginfo: Stabilize `-Z debug-macros`, `-Z collapse-macro-debuginfo` and `#[collapse_debuginfo]`

`-Z debug-macros` is "stabilized" by enabling it by default and removing.

`-Z collapse-macro-debuginfo` is stabilized as `-C collapse-macro-debuginfo`.
It now supports all typical boolean values (`parse_opt_bool`) in addition to just yes/no.

Default value of `collapse_debuginfo` was changed from `false` to `external` (i.e. collapsed if external, not collapsed if local) - rust-lang#100758 (comment) describes some debugging scenarios that motivate this default as reasonable.
`#[collapse_debuginfo]` attribute without a value is no longer supported to avoid guessing the default.

Stabilization report: rust-lang#120845 (comment)

Closes rust-lang#100758
Closes rust-lang#41743
Closes rust-lang#39153
@bors bors closed this as completed in 6acb9e7 Apr 26, 2024
github-actions bot pushed a commit to rust-lang/miri that referenced this issue Apr 26, 2024
debuginfo: Stabilize `-Z debug-macros`, `-Z collapse-macro-debuginfo` and `#[collapse_debuginfo]`

`-Z debug-macros` is "stabilized" by enabling it by default and removing.

`-Z collapse-macro-debuginfo` is stabilized as `-C collapse-macro-debuginfo`.
It now supports all typical boolean values (`parse_opt_bool`) in addition to just yes/no.

Default value of `collapse_debuginfo` was changed from `false` to `external` (i.e. collapsed if external, not collapsed if local) - rust-lang/rust#100758 (comment) describes some debugging scenarios that motivate this default as reasonable.
`#[collapse_debuginfo]` attribute without a value is no longer supported to avoid guessing the default.

Stabilization report: rust-lang/rust#120845 (comment)

Closes rust-lang/rust#100758
Closes rust-lang/rust#41743
Closes rust-lang/rust#39153
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.) C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC F-collapse_debuginfo `#![feature(collapse_debuginfo)]` T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
3 participants