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

dbg!: also print column #114910

Closed
matthiaskrgr opened this issue Aug 16, 2023 · 3 comments · Fixed by #114962
Closed

dbg!: also print column #114910

matthiaskrgr opened this issue Aug 16, 2023 · 3 comments · Fixed by #114962
Assignees
Labels
C-feature-request Category: A feature request, i.e: not implemented / a PR. Libs-Small Libs issues that are considered "small" or self-contained T-libs Relevant to the library team, which will review and decide on the PR/issue.

Comments

@matthiaskrgr
Copy link
Member

matthiaskrgr commented Aug 16, 2023

I just found myself writing

 .map(|line| if double_ice {  dbg!(&line);   (line, ICEKind::DoubleIce) } else {  dbg!(&line);  (line, ICEKind::Ice(interestingness)) });

only to discover that the dbg macro in fact only prints the source line and not the column of the macro start or something like that 😅

[src/main.rs:1987] &line = "'  left: Align(2 bytes)' ' right: Align(4 bytes)'    assertion `left == right` failed: alignment mismatch between ABI and layout in TyAndLayout {"
[src/main.rs:1987] &line = "'  left: Align(2 bytes)' ' right: Align(4 bytes)'    assertion `left == right` failed: alignment mismatch between ABI and layout in TyAndLayout {"
[src/main.rs:1987] &line = "'  left: Align(2 bytes)' ' right: Align(4 bytes)'    assertion `left == right` failed: alignment mismatch between ABI and layout in TyAndLayout {"
[src/main.rs:1987] &line = "'  left: Align(2 bytes)' ' right: Align(4 bytes)'    assertion `left == right` failed: alignment mismatch between ABI and layout in TyAndLayout {"
[src/main.rs:1987] &line = "'  left: Align(2 bytes)' ' right: Align(4 bytes)'    assertion `left == right` failed: alignment mismatch between ABI and layout in TyAndLayout {"

Is it possible to add the column as well?

@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Aug 16, 2023
@matthiaskrgr matthiaskrgr added the Libs-Small Libs issues that are considered "small" or self-contained label Aug 16, 2023
@darklyspaced
Copy link
Contributor

@rustbot claim

@Urgau
Copy link
Member

Urgau commented Aug 17, 2023

This section from the RFC is quite relevant, Excluding the column number:

Most likely, only one dbg!(expr) call will occur per line. The remaining cases will likely occur when dealing with binary operators such as with: dbg!(x) + dbg!(y) + dbg!(z), or with several arguments to a function / method call. However, since the macro prints out stringify!(expr), the user can clearly see which expression on the line that generated the value.
The only exception to this is if the same expression is used multiple times and crucically has side effects altering the value between calls. This scenario is probably uncommon. Furthermore, even in this case, one can visually distinguish between the calls since one is first and the second comes next.

Another reason to exclude column!() is that we want to keep the macro simple, and thus,
we only want to keep the essential parts that help debugging most.

However, the column!() isn't very visually disturbing since it uses horizontal screen real-estate but not vertical real-estate, which may still be a good reason to keep it. Nonetheless, this argument is not sufficient to keep column!(), wherefore this RFC will not include it.

@darklyspaced
Copy link
Contributor

darklyspaced commented Aug 18, 2023

while I agree with the fact that often there is only one debug statement a line, i think that there is one major benefit that the rfc overlooked: users can directly jump to the pertinent debug statement through one unified method regardless of what <expr> are used in the statement itself and if multiple share a single line. searching visually is always slower than a hard, concrete jump.

additionally, the rfc failed to acknowledge the case above as being an ambiguous situation when using solely line numbers. all this combined with the fact that

column!() isn't very visually disturbing since it uses horizontal screen real-estate but not vertical real-estate

means that it seems like a pretty sensible addition to the macro.

@Noratrieb Noratrieb added T-libs Relevant to the library team, which will review and decide on the PR/issue. C-feature-request Category: A feature request, i.e: not implemented / a PR. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Aug 22, 2023
bors added a commit to rust-lang-ci/rust that referenced this issue Dec 16, 2023
adds a column number to `dbg!()`

this would be very nice to have for a few reasons:
1. the rfc, when deciding not to add column numbers to macro, failed to acknowledge any potential ambiguous cases -- such as the one provided in rust-lang#114910 -- which do exist
2. would be able to consistently and easily jump directly to the `dbg!()` regardless of the sutation
3. takes up, at a maximum, 3 characters of _horizontal_ screen space

fixes rust-lang#114910
@bors bors closed this as completed in 43dcc9b Dec 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-feature-request Category: A feature request, i.e: not implemented / a PR. Libs-Small Libs issues that are considered "small" or self-contained T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants