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

adds a column number to dbg!() #114962

Merged
merged 2 commits into from
Dec 18, 2023
Merged

adds a column number to dbg!() #114962

merged 2 commits into from
Dec 18, 2023

Conversation

darklyspaced
Copy link
Contributor

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 dbg!: also print column #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 #114910

@rustbot
Copy link
Collaborator

rustbot commented Aug 18, 2023

r? @cuviper

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Aug 18, 2023
@rust-log-analyzer

This comment has been minimized.

@cuviper
Copy link
Member

cuviper commented Aug 18, 2023

I think this needs API consideration, especially since it changes behavior that was explicitly chosen in an RFC.

@rustbot label -T-libs +T-libs-api
r? libs-api

@rustbot rustbot added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. and removed T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Aug 18, 2023
@rustbot rustbot assigned m-ou-se and unassigned cuviper Aug 18, 2023
@est31
Copy link
Member

est31 commented Aug 19, 2023

Having done a bunch of column number changes over the years, like #42938, #46762, #51980 or #79002, I am glad to see that my appreciation for column numbers is shared. 👍 from me! See those threads for discussion and arguments why column numbers are useful, especially the second and last linked PR.

@darklyspaced
Copy link
Contributor Author

@m-ou-se have you gotten a chance to look over the API changes yet?

@m-ou-se
Copy link
Member

m-ou-se commented Nov 28, 2023

Seems fine to me!

Since #106824, the impact of this is very minimal: the file name, line number and column number will all be 'inlined' into the stringl literal. dbg!() will expand to eprintln!("[{}:{}:{}]", file!(), line!(), column!()) which gets optimized into just eprintln!("[src/main.rs:2:4]"): as a single string literal. So the difference is just a few bytes more in the string literals, and nothing else.

@rfcbot merge

@rfcbot
Copy link

rfcbot commented Nov 28, 2023

Team member @m-ou-se has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Nov 28, 2023
@m-ou-se m-ou-se added S-waiting-on-fcp Status: PR is in FCP and is awaiting for FCP to complete. relnotes Marks issues that should be documented in the release notes of the next release. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 28, 2023
@rfcbot rfcbot added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels Nov 28, 2023
@rfcbot
Copy link

rfcbot commented Nov 28, 2023

🔔 This is now entering its final comment period, as per the review above. 🔔

@joshtriplett
Copy link
Member

👍 for column numbers anywhere we emit line numbers.

This could also affect someone's test cases, but the exact textual output of dbg! and similar have no stability guarantees, so relying on the output of dbg! in a test case is not a good idea.

@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this PR / Issue. to-announce Announce this issue on triage meeting and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels Dec 8, 2023
@rfcbot
Copy link

rfcbot commented Dec 8, 2023

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

@bors
Copy link
Contributor

bors commented Dec 11, 2023

☔ The latest upstream changes (presumably #114571) made this pull request unmergeable. Please resolve the merge conflicts.

@bors bors added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Dec 11, 2023
@rustbot rustbot added the has-merge-commits PR has merge commits, merge with caution. label Dec 12, 2023
@est31
Copy link
Member

est31 commented Dec 16, 2023

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Dec 16, 2023

📌 Commit b05f211 has been approved by est31

It is now in the queue for this repository.

@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Dec 16, 2023
@matthiaskrgr
Copy link
Member

@bors rollup=never
I suspect this will have at least some impact on binary size

@bors
Copy link
Contributor

bors commented Dec 16, 2023

⌛ Testing commit b05f211 with merge 21fd484...

bors added a commit to rust-lang-ci/rust that referenced this pull request 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
@rust-log-analyzer
Copy link
Collaborator

The job aarch64-gnu failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
 Documenting rustc_ast_passes v0.0.0 (/checkout/compiler/rustc_ast_passes)
[RUSTC-TIMING] rustc_ast_passes test:false 1.339
    Checking rustc_expand v0.0.0 (/checkout/compiler/rustc_expand)
 Documenting rustc_expand v0.0.0 (/checkout/compiler/rustc_expand)
##[error]The runner has received a shutdown signal. This can happen when the runner service is stopped, or a manually started runner is canceled.
##[group]Clock drift check
  local time: Sun Dec 17 00:02:05 UTC 2023
  local time: Sun Dec 17 00:02:05 UTC 2023
Session terminated, killing shell... ...killed.
##[error]The operation was canceled.
Cleaning up orphan processes

@bors
Copy link
Contributor

bors commented Dec 17, 2023

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Dec 17, 2023
@est31
Copy link
Member

est31 commented Dec 17, 2023

Seems like an unrelated issue (maybe the CI environment?).

@matthiaskrgr
Copy link
Member

@bors retry

@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 Dec 17, 2023
@bors
Copy link
Contributor

bors commented Dec 17, 2023

⌛ Testing commit b05f211 with merge 43dcc9b...

@bors
Copy link
Contributor

bors commented Dec 18, 2023

☀️ Test successful - checks-actions
Approved by: est31
Pushing 43dcc9b to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Dec 18, 2023
@bors bors merged commit 43dcc9b into rust-lang:master Dec 18, 2023
12 checks passed
@rustbot rustbot added this to the 1.76.0 milestone Dec 18, 2023
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (43dcc9b): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
0.5% [0.5%, 0.5%] 1
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-0.6% [-0.9%, -0.4%] 3
Improvements ✅
(secondary)
-1.8% [-2.5%, -1.2%] 2
All ❌✅ (primary) -0.3% [-0.9%, 0.5%] 4

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
0.8% [0.7%, 0.9%] 3
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-0.4% [-0.4%, -0.4%] 1
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 0.5% [-0.4%, 0.9%] 4

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 670.812s -> 670.368s (-0.07%)
Artifact size: 312.51 MiB -> 312.44 MiB (-0.02%)

wcampbell0x2a added a commit to wcampbell0x2a/dbg_hex that referenced this pull request Jan 20, 2024
wcampbell0x2a added a commit to wcampbell0x2a/dbg_hex that referenced this pull request Jan 20, 2024
wcampbell0x2a added a commit to wcampbell0x2a/dbg_hex that referenced this pull request Jan 20, 2024
@dtolnay dtolnay removed the has-merge-commits PR has merge commits, merge with caution. label Jan 21, 2024
wip-sync pushed a commit to NetBSD/pkgsrc-wip that referenced this pull request Feb 18, 2024
Pkgsrc changes:
 * Adapt checksums and patches.

Upstream chnages:

Version 1.76.0 (2024-02-08)
==========================

Language
--------
- [Document Rust ABI compatibility between various types]
  (rust-lang/rust#115476)
- [Also: guarantee that char and u32 are ABI-compatible]
  (rust-lang/rust#118032)
- [Warn against ambiguous wide pointer comparisons]
  (rust-lang/rust#117758)

Compiler
--------
- [Lint pinned `#[must_use]` pointers (in particular, `Box<T>`
  where `T` is `#[must_use]`) in `unused_must_use`.]
  (rust-lang/rust#118054)
- [Soundness fix: fix computing the offset of an unsized field in
  a packed struct]
  (rust-lang/rust#118540)
- [Soundness fix: fix dynamic size/align computation logic for
  packed types with dyn Trait tail]
  (rust-lang/rust#118538)
- [Add `$message_type` field to distinguish json diagnostic outputs]
  (rust-lang/rust#115691)
- [Enable Rust to use the EHCont security feature of Windows]
  (rust-lang/rust#118013)
- [Add tier 3 {x86_64,i686}-win7-windows-msvc targets]
  (rust-lang/rust#118150)
- [Add tier 3 aarch64-apple-watchos target]
  (rust-lang/rust#119074)
- [Add tier 3 arm64e-apple-ios & arm64e-apple-darwin targets]
  (rust-lang/rust#115526)

Refer to Rust's [platform support page][platform-support-doc]
for more information on Rust's tiered platform support.

Libraries
---------
- [Add a column number to `dbg!()`]
  (rust-lang/rust#114962)
- [Add `std::hash::{DefaultHasher, RandomState}` exports]
  (rust-lang/rust#115694)
- [Fix rounding issue with exponents in fmt]
  (rust-lang/rust#116301)
- [Add T: ?Sized to `RwLockReadGuard` and `RwLockWriteGuard`'s Debug impls.]
  (rust-lang/rust#117138)
- [Windows: Allow `File::create` to work on hidden files]
  (rust-lang/rust#116438)

Stabilized APIs
---------------
- [`Arc::unwrap_or_clone`]
  (https://doc.rust-lang.org/stable/std/sync/struct.Arc.html#method.unwrap_or_clone)
- [`Rc::unwrap_or_clone`]
  (https://doc.rust-lang.org/stable/std/rc/struct.Rc.html#method.unwrap_or_clone)
- [`Result::inspect`]
  (https://doc.rust-lang.org/stable/std/result/enum.Result.html#method.inspect)
- [`Result::inspect_err`]
  (https://doc.rust-lang.org/stable/std/result/enum.Result.html#method.inspect_err)
- [`Option::inspect`]
  (https://doc.rust-lang.org/stable/std/option/enum.Option.html#method.inspect)
- [`type_name_of_val`]
  (https://doc.rust-lang.org/stable/std/any/fn.type_name_of_val.html)
- [`std::hash::{DefaultHasher, RandomState}`]
  (https://doc.rust-lang.org/stable/std/hash/index.html#structs)
  These were previously available only through `std::collections::hash_map`.
- [`ptr::{from_ref, from_mut}`]
  (https://doc.rust-lang.org/stable/std/ptr/fn.from_ref.html)
- [`ptr::addr_eq`](https://doc.rust-lang.org/stable/std/ptr/fn.addr_eq.html)

Cargo
-----

See [Cargo release notes]
(https://github.com/rust-lang/cargo/blob/master/CHANGELOG.md#cargo-176-2024-02-08).

Rustdoc
-------
- [Don't merge cfg and doc(cfg) attributes for re-exports]
  (rust-lang/rust#113091)
- [rustdoc: allow resizing the sidebar / hiding the top bar]
  (rust-lang/rust#115660)
- [rustdoc-search: add support for traits and associated types]
  (rust-lang/rust#116085)
- [rustdoc: Add highlighting for comments in items declaration]
  (rust-lang/rust#117869)

Compatibility Notes
-------------------
- [Add allow-by-default lint for unit bindings]
  (rust-lang/rust#112380)
  This is expected to be upgraded to a warning by default in a future Rust
  release. Some macros emit bindings with type `()` with user-provided spans,
  which means that this lint will warn for user code.
- [Remove x86_64-sun-solaris target.]
  (rust-lang/rust#118091)
- [Remove asmjs-unknown-emscripten target]
  (rust-lang/rust#117338)
- [Report errors in jobserver inherited through environment variables]
  (rust-lang/rust#113730)
  This [may warn](rust-lang/rust#120515)
  on benign problems too.
- [Update the minimum external LLVM to 16.]
  (rust-lang/rust#117947)
- [Improve `print_tts`](rust-lang/rust#114571)
  This change can break some naive manual parsing of token trees
  in proc macro code which expect a particular structure after
  `.to_string()`, rather than just arbitrary Rust code.
- [Make `IMPLIED_BOUNDS_ENTAILMENT` into a hard error from a lint]
  (rust-lang/rust#117984)
- [Vec's allocation behavior was changed when collecting some iterators]
  (rust-lang/rust#110353)
  Allocation behavior is currently not specified, nevertheless
  changes can be surprising.
  See [`impl FromIterator for Vec`]
  (https://doc.rust-lang.org/nightly/std/vec/struct.Vec.html#impl-FromIterator%3CT%3E-for-Vec%3CT%3E)
  for more details.
- [Properly reject `default` on free const items]
  (rust-lang/rust#117818)
raxyte added a commit to raxyte/linux that referenced this pull request Apr 14, 2024
In Rust 1.76.0, the `dbg!()` macro was updated to also format the column
number. The reason cited was usage of a few characters worth of
horizontal space while allowing direct jumps to the source location. [1]

Link: rust-lang/rust#114962 [1]
Link: Rust-for-Linux#1065

Signed-off-by: Raghav Narang <[email protected]>
ojeda pushed a commit to ojeda/linux that referenced this pull request May 5, 2024
In Rust 1.76.0, the `dbg!()` macro was updated to also format the column
number. The reason cited was usage of a few characters worth of
horizontal space while allowing direct jumps to the source location. [1]

Link: rust-lang/rust#114962 [1]
Link: Rust-for-Linux#1065
Signed-off-by: Raghav Narang <[email protected]>
Reviewed-by: Trevor Gross <[email protected]>
Reviewed-by: Gary Guo <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
[ Fixed commit author name and removed spurious newline in message. - Miguel ]
Signed-off-by: Miguel Ojeda <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. merged-by-bors This PR was explicitly merged by bors. relnotes Marks issues that should be documented in the release notes of the next release. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. S-waiting-on-fcp Status: PR is in FCP and is awaiting for FCP to complete. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

dbg!: also print column