Start emitting labels even if their pointed to file is not available locally #104449

oli-obk · 2022-11-15T15:55:52Z

rustbot · 2022-11-15T15:56:00Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @estebank (or someone else) soon.

Please see the contribution instructions for more information.

rustbot · 2022-11-15T15:56:03Z

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

RalfJung

Can't check the emitter stuff, but the CTFE part looks good!

compiler/rustc_const_eval/src/interpret/eval_context.rs

rustbot · 2022-11-15T16:08:29Z

The Miri subtree was changed

cc @rust-lang/miri

src/tools/miri/src/diagnostics.rs

RalfJung · 2022-11-15T18:32:12Z

Nice redundancy reduction on the Miri backtraces. :)

bors · 2022-11-16T20:22:33Z

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

estebank · 2022-11-23T18:47:15Z

src/test/ui/infinite/infinite-recursion-const-fn.stderr

+   |     inside `b`
+   |     inside `b`
+   |     inside `b`
+   |     inside `b`
+   |     inside `b`


It seems like we could afford to add some deduplication here (probably best on its own PR), maybe replacing this with a single span label message like "multiple times inside b"

estebank · 2022-11-23T18:48:37Z

src/test/ui/consts/recursive.stderr

-   |     inside `f::<i32>` at $DIR/recursive.rs:4:5
-   |     [... 126 additional calls inside `f::<i32>` at $DIR/recursive.rs:4:5 ...]
+   |     inside `f::<i32>`
+   |     [... 126 additional calls inside `f::<i32>` ...]


Oh! We already do some deduplication!

estebank · 2022-11-23T18:51:57Z

src/test/ui/consts/missing_span_in_backtrace.stderr

+error[E0080]: evaluation of constant value failed
+/rustc/xyz/library/core/src/ptr/mod.rs:925:14: inside `swap_nonoverlapping::<MaybeUninit<u8>>`
+/rustc/xyz/library/core/src/ptr/mod.rs:944:9: inside `ptr::swap_nonoverlapping_simple_untyped::<MaybeUninit<u8>>`
+--> /rustc/xyz/library/core/src/ptr/mod.rs:1135:9
+note: unable to copy parts of a pointer from memory at alloc10
+note: inside `std::ptr::read::<MaybeUninit<MaybeUninit<u8>>>`
+/rustc/xyz/library/core/src/mem/mod.rs:773:17: inside `mem::swap_simple::<MaybeUninit<MaybeUninit<u8>>>`
+   |
+  ::: $DIR/missing_span_in_backtrace.rs:16:9


I think the expected output for these (which doesn't need to be followed, but I'd prefer to) would be

Suggested change

error[E0080]: evaluation of constant value failed

/rustc/xyz/library/core/src/ptr/mod.rs:925:14: inside `swap_nonoverlapping::<MaybeUninit<u8>>`

/rustc/xyz/library/core/src/ptr/mod.rs:944:9: inside `ptr::swap_nonoverlapping_simple_untyped::<MaybeUninit<u8>>`

--> /rustc/xyz/library/core/src/ptr/mod.rs:1135:9

note: unable to copy parts of a pointer from memory at alloc10

note: inside `std::ptr::read::<MaybeUninit<MaybeUninit<u8>>>`

/rustc/xyz/library/core/src/mem/mod.rs:773:17: inside `mem::swap_simple::<MaybeUninit<MaybeUninit<u8>>>`

|

::: $DIR/missing_span_in_backtrace.rs:16:9

error[E0080]: evaluation of constant value failed

--> /rustc/xyz/library/core/src/ptr/mod.rs:1135:9

|

= /rustc/xyz/library/core/src/ptr/mod.rs:925:14: inside `swap_nonoverlapping::<MaybeUninit<u8>>`

= /rustc/xyz/library/core/src/ptr/mod.rs:944:9: inside `ptr::swap_nonoverlapping_simple_untyped::<MaybeUninit<u8>>`

= note: unable to copy parts of a pointer from memory at alloc10

= note: inside `std::ptr::read::<MaybeUninit<MaybeUninit<u8>>>`

= /rustc/xyz/library/core/src/mem/mod.rs:773:17: inside `mem::swap_simple::<MaybeUninit<MaybeUninit<u8>>>`

|

::: $DIR/missing_span_in_backtrace.rs:16:9

Aligning with the available path (so accounting for the biggest number shown in the gutter) and separating the message from the notes given no available span.

Also, what about the following formatting?

Suggested change

error[E0080]: evaluation of constant value failed

/rustc/xyz/library/core/src/ptr/mod.rs:925:14: inside `swap_nonoverlapping::<MaybeUninit<u8>>`

/rustc/xyz/library/core/src/ptr/mod.rs:944:9: inside `ptr::swap_nonoverlapping_simple_untyped::<MaybeUninit<u8>>`

--> /rustc/xyz/library/core/src/ptr/mod.rs:1135:9

note: unable to copy parts of a pointer from memory at alloc10

note: inside `std::ptr::read::<MaybeUninit<MaybeUninit<u8>>>`

/rustc/xyz/library/core/src/mem/mod.rs:773:17: inside `mem::swap_simple::<MaybeUninit<MaybeUninit<u8>>>`

|

::: $DIR/missing_span_in_backtrace.rs:16:9

error[E0080]: evaluation of constant value failed

--> /rustc/xyz/library/core/src/ptr/mod.rs:1135:9

|

note: inside `swap_nonoverlapping::<MaybeUninit<u8>>`

--> /rustc/xyz/library/core/src/ptr/mod.rs:925:14

|

note: inside `ptr::swap_nonoverlapping_simple_untyped::<MaybeUninit<u8>>`

--> /rustc/xyz/library/core/src/ptr/mod.rs:944:9

|

= note: unable to copy parts of a pointer from memory at alloc10

= note: inside `std::ptr::read::<MaybeUninit<MaybeUninit<u8>>>`

= /rustc/xyz/library/core/src/mem/mod.rs:773:17: inside `mem::swap_simple::<MaybeUninit<MaybeUninit<u8>>>`

|

::: $DIR/missing_span_in_backtrace.rs:16:9

I also noticed that the new pseudo-notes come before the path for the main diagnostic, that should be changed.

One last thing: if there are span labels for the primary span, they should be displayed first and as a = note: without a path. I think that with all of these we stay inside the established grammar of the printed diagnostics. (But I don't think we have a test for that case.)

At the emitter stage we do not have enough information to figure this out. Labels are reordered to fit the file structure. I will check if converting them to notes at const eval error time will still yield reasonable diagnostics

estebank · 2022-11-23T18:54:51Z

compiler/rustc_errors/src/emitter.rs

@@ -1410,6 +1406,42 @@ impl EmitterWriter {
        for annotated_file in annotated_files {
            // we can't annotate anything if the source is unavailable.
            if !sm.ensure_source_file_source_present(annotated_file.file.clone()) {
+                if !self.short_message {
+                    // We'll just print an unannotated message.


Here is where we'd have to duplicate some of the printing logic for accurate gutter alignment. :-/

Ideally we'd dedup the logic, but it is fine as long as we have test coverage (which we do) and leave a comment in both places reminding us to modify logic in the other.

estebank

I'm +1 on the changes as they are, but I have a bunch of nitpicks on the resulting output. The one I would love to have addressed before merging is ensuring the --> main diagnostic path line comes before all the recovered span labels.

oli-obk · 2022-12-01T13:37:10Z

@bors r=estebank,RalfJung

bors · 2022-12-01T13:37:12Z

📌 Commit c6f80a80e5620f40a9c973c987a61fc2f6cf678d has been approved by estebank,RalfJung

It is now in the queue for this repository.

src/tools/miri/src/diagnostics.rs

bors · 2022-12-08T21:41:43Z

💔 Test failed - checks-actions

ehuss · 2022-12-08T21:45:14Z

@bors retry

run-make/coverage-reports

RalfJung · 2022-12-08T21:45:31Z

Is it only this PR that keeps failing with coverage reports? That would be a bit suspicious.
@bors retry

RalfJung · 2022-12-08T21:46:19Z

I can't even find an issue tracking that clear_expected_if_blessed is noisy?

EDIT: opened #105475

ehuss · 2022-12-08T21:50:15Z

@RalfJung It's all PRs. I started a thread on #t-infra. I'm tempted to close the tree, but it is not a 100% failure.

bors · 2022-12-09T06:24:31Z

⌛ Testing commit 717fdb5 with merge badd6a5...

bors · 2022-12-09T09:19:19Z

☀️ Test successful - checks-actions
Approved by: estebank,RalfJung
Pushing badd6a5 to master...

rust-log-analyzer · 2022-12-09T10:27:25Z

The job x86_64-msvc-1 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)


test result: FAILED. 46 passed; 1 failed; 29 ignored; 0 measured; 0 filtered out; finished in 13.26s

Build completed unsuccessfully in 1:05:42
make: *** [Makefile:73: ci-subset-1] Error 1

rust-timer · 2022-12-09T10:38:28Z

Finished benchmarking commit (badd6a5): 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
Regressions ❌ (secondary)	7.1%	[7.1%, 7.1%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-1.8%	[-1.9%, -1.8%]	2
All ❌✅ (primary)	-	-	0

Cycles

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

…tebank Make some diagnostics not depend on the source of what they reference being available r? `@estebank` follow up to rust-lang#104449

…bank,RalfJung Start emitting labels even if their pointed to file is not available locally r? `@estebank` cc `@RalfJung` fixes rust-lang#97699

rustbot assigned estebank Nov 15, 2022

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 Nov 15, 2022

RalfJung approved these changes Nov 15, 2022

View reviewed changes

compiler/rustc_const_eval/src/interpret/eval_context.rs Show resolved Hide resolved

oli-obk force-pushed the unhide_unknown_spans branch from 652e268 to 8c99c63 Compare November 15, 2022 16:08

saethlin reviewed Nov 15, 2022

View reviewed changes

src/tools/miri/src/diagnostics.rs Outdated Show resolved Hide resolved

oli-obk force-pushed the unhide_unknown_spans branch 3 times, most recently from 872ac59 to ad41e6c Compare November 15, 2022 17:04

oli-obk force-pushed the unhide_unknown_spans branch from ad41e6c to 1aa0310 Compare November 23, 2022 17:18

estebank reviewed Nov 23, 2022

View reviewed changes

estebank approved these changes Nov 23, 2022

View reviewed changes

This comment has been minimized.

Sign in to view

oli-obk force-pushed the unhide_unknown_spans branch from a9b2beb to c70be5d Compare November 29, 2022 15:41

This comment has been minimized.

Sign in to view

estebank approved these changes Nov 29, 2022

View reviewed changes

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 1, 2022

RalfJung reviewed Dec 1, 2022

View reviewed changes

src/tools/miri/src/diagnostics.rs Outdated Show resolved Hide resolved

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 8, 2022

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 8, 2022

RalfJung mentioned this pull request Dec 8, 2022

coverage-reports keeps failing on Windows (error mentions clear_expected_if_blessed) #105475

Closed

bors added the merged-by-bors This PR was explicitly merged by bors. label Dec 9, 2022

bors merged commit badd6a5 into rust-lang:master Dec 9, 2022

rustbot added this to the 1.67.0 milestone Dec 9, 2022

bors mentioned this pull request Dec 9, 2022

always check alignment during CTFE #104616

Merged

oli-obk deleted the unhide_unknown_spans branch December 9, 2022 14:30

This comment was marked as resolved.

Sign in to view

oli-obk mentioned this pull request Dec 12, 2022

Make some diagnostics not depend on the source of what they reference being available #105500

Merged

oli-obk mentioned this pull request Dec 16, 2022

Improve diagnostics of evaluation of constant value failed errors #85155

Closed

ehuss mentioned this pull request Mar 20, 2023

CI random failure on x86_64-apple-2 InvalidInput in create_dir_all while running test/rustdoc/primitive/no_std.rs #109397

Closed

jruderman mentioned this pull request Nov 5, 2023

Empty "tuple variant defined here" note #105241

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Start emitting labels even if their pointed to file is not available locally #104449

Start emitting labels even if their pointed to file is not available locally #104449

oli-obk commented Nov 15, 2022 •

edited

Loading

rustbot commented Nov 15, 2022

rustbot commented Nov 15, 2022

RalfJung left a comment

rustbot commented Nov 15, 2022

RalfJung commented Nov 15, 2022

bors commented Nov 16, 2022

estebank Nov 23, 2022 •

edited

Loading

estebank Nov 23, 2022

estebank Nov 23, 2022 •

edited

Loading

estebank Nov 23, 2022

estebank Nov 23, 2022 •

edited

Loading

oli-obk Nov 29, 2022

estebank Nov 23, 2022

estebank left a comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

oli-obk commented Dec 1, 2022

bors commented Dec 1, 2022

bors commented Dec 8, 2022

ehuss commented Dec 8, 2022

RalfJung commented Dec 8, 2022

RalfJung commented Dec 8, 2022 •

edited

Loading

ehuss commented Dec 8, 2022

bors commented Dec 9, 2022

bors commented Dec 9, 2022

rust-log-analyzer commented Dec 9, 2022

rust-timer commented Dec 9, 2022

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

Start emitting labels even if their pointed to file is not available locally #104449

Start emitting labels even if their pointed to file is not available locally #104449

Conversation

oli-obk commented Nov 15, 2022 • edited Loading

rustbot commented Nov 15, 2022

rustbot commented Nov 15, 2022

RalfJung left a comment

Choose a reason for hiding this comment

rustbot commented Nov 15, 2022

RalfJung commented Nov 15, 2022

bors commented Nov 16, 2022

estebank Nov 23, 2022 • edited Loading

Choose a reason for hiding this comment

estebank Nov 23, 2022

Choose a reason for hiding this comment

estebank Nov 23, 2022 • edited Loading

Choose a reason for hiding this comment

estebank Nov 23, 2022

Choose a reason for hiding this comment

estebank Nov 23, 2022 • edited Loading

Choose a reason for hiding this comment

oli-obk Nov 29, 2022

Choose a reason for hiding this comment

estebank Nov 23, 2022

Choose a reason for hiding this comment

estebank left a comment

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

oli-obk commented Dec 1, 2022

bors commented Dec 1, 2022

bors commented Dec 8, 2022

ehuss commented Dec 8, 2022

RalfJung commented Dec 8, 2022

RalfJung commented Dec 8, 2022 • edited Loading

ehuss commented Dec 8, 2022

bors commented Dec 9, 2022

bors commented Dec 9, 2022

rust-log-analyzer commented Dec 9, 2022

rust-timer commented Dec 9, 2022

Overall result: no relevant changes - no action needed

Instruction count

Max RSS (memory usage)

Cycles

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

oli-obk commented Nov 15, 2022 •

edited

Loading

estebank Nov 23, 2022 •

edited

Loading

estebank Nov 23, 2022 •

edited

Loading

estebank Nov 23, 2022 •

edited

Loading

RalfJung commented Dec 8, 2022 •

edited

Loading