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

Get rid of the absolute path printer in type mismatch diagnostics #116813

Closed
wants to merge 1 commit into from

Conversation

Noratrieb
Copy link
Member

The logic was added in #42826.

The test for it (run-make/type-mismatch-same-crate-name) passes when just using the macros to control pretty printing. Note that the test does fail unless both of the macros are used, indicating that we're doing something right.

@rustbot
Copy link
Collaborator

rustbot commented Oct 16, 2023

r? @cjgillot

(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-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 16, 2023
@rust-log-analyzer

This comment has been minimized.

Copy link
Member

@compiler-errors compiler-errors left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@rust-log-analyzer

This comment has been minimized.

The logic was added in rust-lang#42826.

The tests for it (`run-make/type-mismatch-same-crate-name` and
`ui/type/type/type-mismatch-same-crate-name.rs) pass when being replaced
by this, which makes sense, as `no_trimmed!(no_visible!())` should be
equivalent to the custom absolute printer.
@rust-log-analyzer
Copy link
Collaborator

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

Click to see the possible cause of the failure (guessed by this bot)
GITHUB_ENV=/home/runner/work/_temp/_runner_file_commands/set_env_aaa2ab4d-c30b-44e5-94c6-87d037348e32
GITHUB_EVENT_NAME=pull_request
GITHUB_EVENT_PATH=/home/runner/work/_temp/_github_workflow/event.json
GITHUB_GRAPHQL_URL=https://api.github.com/graphql
GITHUB_HEAD_REF=funny-pretty-printers
GITHUB_JOB=pr
GITHUB_PATH=/home/runner/work/_temp/_runner_file_commands/add_path_aaa2ab4d-c30b-44e5-94c6-87d037348e32
GITHUB_REF=refs/pull/116813/merge
GITHUB_REF_NAME=116813/merge
GITHUB_REF_PROTECTED=false
---
failures:

---- [incremental] tests/incremental/circular-dependencies.rs stdout ----

error in revision `cfail2`: /checkout/tests/incremental/circular-dependencies.rs:23: expected note not found: the crate `circular_dependencies` is compiled multiple times, possibly with different configurations

error in revision `cfail2`: /checkout/tests/incremental/circular-dependencies.rs:31: expected note not found: the crate `circular_dependencies` is compiled multiple times, possibly with different configurations

error in revision `cfail2`: 0 unexpected errors found, 2 expected errors not found
status: exit status: 1
command: RUSTC_ICE="0" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/tests/incremental/circular-dependencies.rs" "-Zthreads=1" "-Zsimulate-remapped-rust-src-base=/rustc/FAKE_PREFIX" "-Ztranslate-remapped-path-to-local-path=no" "-Z" "ignore-directory-in-diagnostics-source-blocks=/cargo" "--sysroot" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2" "--target=x86_64-unknown-linux-gnu" "--cfg" "cfail2" "-C" "incremental=/checkout/obj/build/x86_64-unknown-linux-gnu/test/incremental/circular-dependencies/circular-dependencies.inc" "-Z" "incremental-verify-ich" "-O" "--error-format" "json" "--json" "future-incompat" "-Zui-testing" "-Zdeduplicate-diagnostics=no" "-C" "prefer-dynamic" "--out-dir" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/incremental/circular-dependencies" "-A" "internal_features" "-Crpath" "-Cdebuginfo=0" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/incremental/circular-dependencies/auxiliary" "--edition=2021" "--test" "--extern" "aux=/checkout/obj/build/x86_64-unknown-linux-gnu/test/incremental/circular-dependencies/auxiliary/libcircular_dependencies_aux.rmeta" "-L" "dependency=/checkout/obj/build/x86_64-unknown-linux-gnu/test/incremental/circular-dependencies"
    Error {
        line_num: 23,
        kind: Some(
            Note,
            Note,
        ),
        msg: "the crate `circular_dependencies` is compiled multiple times, possibly with different configurations",
    Error {
        line_num: 31,
        kind: Some(
            Note,
            Note,
        ),
        msg: "the crate `circular_dependencies` is compiled multiple times, possibly with different configurations",
]

thread '[incremental] tests/incremental/circular-dependencies.rs' panicked at src/tools/compiletest/src/runtest.rs:1826:13:
explicit panic

|| abs_path(did1)? == abs_path(did2)?)
};
if same_path().unwrap_or(false) {
// We check for both equality of their definition paths (when the crates have the same name) anbd equality
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// We check for both equality of their definition paths (when the crates have the same name) anbd equality
// We check for both equality of their definition paths (when the crates have the same name) and equality

@Noratrieb
Copy link
Member Author

The test failure happens because def_path_str refuses to print the crate root of the local crate, so if one of the types is from the local crate, it doesn't match with the one from the other crate that does get its root printed as the crate name.

https://github.com/Nilstrieb/rust/blob/a00c09e9d80b763fb29206b47b04e1d99c3ace96/compiler/rustc_middle/src/ty/print/pretty.rs#L1964

This makes me think that we do need the special "absolute path" printer here...

@bors
Copy link
Contributor

bors commented Oct 18, 2023

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

@Noratrieb Noratrieb closed this Oct 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants