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!() without parameters #57847

Open
wants to merge 2 commits into
base: master
from

Conversation

@clarfon
Copy link
Contributor

clarfon commented Jan 22, 2019

Fixes #57845.

@rust-highfive

This comment has been minimized.

Copy link
Collaborator

rust-highfive commented Jan 22, 2019

r? @Kimundi

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

@zackmdavis
Copy link
Member

zackmdavis left a comment

I argue that the new behavior should be exercised in the expected-behavior test file.

/// [stderr]: https://en.wikipedia.org/wiki/Standard_streams#Standard_error_(stderr)
#[macro_export]
#[stable(feature = "dbg_macro", since = "1.32.0")]
macro_rules! dbg {
() => {
eprintln!("[{}:{}]", file!(), line!());

This comment has been minimized.

@Centril

Centril Jan 24, 2019

Contributor

I would just let this call dbg!(()) and make it a shorthand.

This comment has been minimized.

@memoryruins

memoryruins Jan 24, 2019

Contributor

@Centril the commit with dbg!() prints

[src/main.rs:2]

If it called dbg!(()), it would print

[src/main.rs:2] () = ()

Is that preferred?

This comment has been minimized.

@Centril

Centril Jan 24, 2019

Contributor

Well... it's more noisy, but also more uniform (the return type of dbg!() is () in either case). YMMV, but I think dbg!(()) this is simpler and cleaner and moreover factors better if we ever change the output format and etc. I think there are benefits and drawbacks to each decision.

This comment has been minimized.

@clarfon

clarfon Jan 24, 2019

Author Contributor

I feel like the noise would ruin the benefits of this. Separating out lines without the expression on them in the debug output with stuff like this might help make debugging visually easier. I think.

This comment has been minimized.

@Centril

Centril Jan 24, 2019

Contributor

Hmm; I mainly thought of the benefits as typing convenience, not changes in output...
Anyways, I don't mind this the way it is now -- we can always change it if we want since the format is unstable. (but please add a test for let _: () = dbg!(); to make sure it compiles)

This comment has been minimized.

@SimonSapin

SimonSapin Feb 12, 2019

Contributor

I would prefer not to conflate “zero argument” with “one argument which has the unit type”, and printing () = () seems noisy at best.

@clarfon

This comment has been minimized.

Copy link
Contributor Author

clarfon commented Jan 31, 2019

Added test for this, so, it should be ready to merge. It seems that an FCP is required so we probably want to start that now.

@rust-highfive

This comment has been minimized.

Copy link
Collaborator

rust-highfive commented Jan 31, 2019

The job x86_64-gnu-llvm-6.0 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
travis_time:end:008ec2db:start=1548913547281438917,finish=1548913620486458746,duration=73205019829
$ git checkout -qf FETCH_HEAD
travis_fold:end:git.checkout

Encrypted environment variables have been removed for security reasons.
See https://docs.travis-ci.com/user/pull-requests/#pull-requests-and-security-restrictions
$ export SCCACHE_BUCKET=rust-lang-ci-sccache2
$ export SCCACHE_REGION=us-west-1
Setting environment variables from .travis.yml
$ export IMAGE=x86_64-gnu-llvm-6.0
---
[01:03:46] .................................................................................................... 4200/5355
[01:03:51] .................................................................................................... 4300/5355
[01:03:54] .................................................................................................... 4400/5355
[01:03:57] .................................................................................................... 4500/5355
[01:04:02] ......i........................................................................F.................... 4600/5355
[01:04:10] .................................................................................................... 4800/5355
[01:04:14] .................................................................................................... 4900/5355
[01:04:17] .................................................................................................... 5000/5355
[01:04:21] .................................................................................................... 5100/5355
---
[01:04:28] ------------------------------------------
[01:04:28] stderr:
[01:04:28] ------------------------------------------
[01:04:28] thread 'main' panicked at 'assertion failed: `(left == right)`
[01:04:28]   left: `[":21] Unit = Unit", ":22] a = Unit", ":28] Point{x: 42, y: 24,} = Point {", "    x: 42,", "    y: 24", "}", ":29] b = Point {", "    x: 42,", "    y: 24", "}", ":37]", ":41] &a = NoCopy(", "    1337", ")", ":41] dbg!(& a) = NoCopy(", "    1337", ")", ":46] f(&42) = 42", "before", ":51] { foo += 1; eprintln!(\"before\"); 7331 } = 7331"]`,
[01:04:28]  right: `[":21] Unit = Unit", ":22] a = Unit", ":28] Point{x: 42, y: 24,} = Point {", "    x: 42,", "    y: 24", "}", ":29] b = Point {", "    x: 42,", "    y: 24", "}", ":37]", ":40] &a = NoCopy(", "    1337", ")", ":40] dbg!(& a) = NoCopy(", "    1337", ")", ":45] f(&42) = 42", "before", ":50] { foo += 1; eprintln!(\"before\"); 7331 } = 7331"]`', /checkout/src/test/ui/rfc-2361-dbg-macro/dbg-macro-expected-behavior.rs:60:5
[01:04:28] 
[01:04:28] ------------------------------------------
[01:04:28] 
[01:04:28] thread '[ui] ui/rfc-2361-dbg-macro/dbg-macro-expected-behavior.rs' panicked at 'explicit panic', src/tools/compiletest/src/runtest.rs:3291:9
---
[01:04:28] 
[01:04:28] thread 'main' panicked at 'Some tests failed', src/tools/compiletest/src/main.rs:495:22
[01:04:28] 
[01:04:28] 
[01:04:28] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/compiletest" "--compile-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib" "--run-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/x86_64-unknown-linux-gnu/lib" "--rustc-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "--src-base" "/checkout/src/test/ui" "--build-base" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui" "--stage-id" "stage2-x86_64-unknown-linux-gnu" "--mode" "ui" "--target" "x86_64-unknown-linux-gnu" "--host" "x86_64-unknown-linux-gnu" "--llvm-filecheck" "/usr/lib/llvm-6.0/bin/FileCheck" "--host-rustcflags" "-Crpath -O -Zunstable-options  -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--target-rustcflags" "-Crpath -O -Zunstable-options  -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--docck-python" "/usr/bin/python2.7" "--lldb-python" "/usr/bin/python2.7" "--gdb" "/usr/bin/gdb" "--quiet" "--llvm-version" "6.0.0\n" "--system-llvm" "--cc" "" "--cxx" "" "--cflags" "" "--llvm-components" "" "--llvm-cxxflags" "" "--adb-path" "adb" "--adb-test-dir" "/data/tmp/work" "--android-cross-path" "" "--color" "always"
[01:04:28] 
[01:04:28] 
[01:04:28] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test
[01:04:28] Build completed unsuccessfully in 0:04:04
[01:04:28] Build completed unsuccessfully in 0:04:04
[01:04:28] make: *** [check] Error 1
[01:04:28] Makefile:48: recipe for target 'check' failed
The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:0d289130
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
Thu Jan 31 06:51:37 UTC 2019
---
travis_time:end:007dc580:start=1548917498628774313,finish=1548917498633131201,duration=4356888
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:18a81c5b
$ ln -s . checkout && for CORE in obj/cores/core.*; do EXE=$(echo $CORE | sed 's|obj/cores/core\.[0-9]*\.!che

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@clarfon clarfon force-pushed the clarfon:dbg_no_params branch from ee0a893 to ea9e2c4 Feb 3, 2019

@Dylan-DPC

This comment has been minimized.

Copy link
Member

Dylan-DPC commented Feb 11, 2019

r? @zackmdavis

ping from triage @clarcharr you have failing tests to resolve

@clarfon

This comment has been minimized.

Copy link
Contributor Author

clarfon commented Feb 11, 2019

@Dylan-DPC ...I resolved the tests. This is ready for FCP.

@Centril

This comment has been minimized.

Copy link
Contributor

Centril commented Feb 11, 2019

Test was added

@SimonSapin

This comment has been minimized.

Copy link
Contributor

SimonSapin commented Feb 12, 2019

This seems like an obvious improvement to me, but it adds a stable API:

@rfcbot fcp merge

@rfcbot

This comment has been minimized.

Copy link

rfcbot commented Feb 12, 2019

Team member @SimonSapin 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 none object), 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

This comment has been minimized.

Copy link

rfcbot commented Feb 16, 2019

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment