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

Non-deterministic diagonstics output #106417

Closed
weiznich opened this issue Jan 3, 2023 · 3 comments · Fixed by #106468
Closed

Non-deterministic diagonstics output #106417

weiznich opened this issue Jan 3, 2023 · 3 comments · Fixed by #106468
Assignees
Labels
A-diagnostics Area: Messages for errors, warnings, and lints E-needs-bisection Call for participation: This issue needs bisection: https://github.com/rust-lang/cargo-bisect-rustc E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@weiznich
Copy link
Contributor

weiznich commented Jan 3, 2023

Given the following code: diesel-rs/diesel@49b39fc

Diesel has a larger amount of compile test to check whether some constructs are rejected at compile time and to check for specific error messages. These tests are implemented using trybuild. We pin specific compiler versions as the output obviously might change with newer compiler versions. From time to time an update of the pinned version is required. While doing such an update we noticed that newer versions of rustc do emit non-deterministic error messages. These sometimes include more or less information.

To reproduce this behaviour perform the following steps:

git clone https://github.com/diesel-rs/diesel
cd diesel 
git checkout 49b39fc9f92018b41b48bba2b9401dad953899fe
cd diesel_compile_tests
# edit the rust-toolchain file to a "recent" nightly compiler
TRYBUILD=overwrite cargo test #adopt the output for the chosen compiler version
cargo test # execute the tests again to observe the tests failing again

We do not observe this behaviour with nightly-2022-08-12. I've observed this behaviour at least since nightly-2022-09-15 (that's the earliest nightly I've checked, so there might be earlier versions as well)

Exemplary changes between two runs (obviously that changes as well):

diff --git a/diesel_compile_tests/tests/fail/find_requires_correct_type.stderr b/diesel_compile_tests/tests/fail/find_requires_correct_type.stderr
index 91374aa0e3..9a6bd16fe6 100644
--- a/diesel_compile_tests/tests/fail/find_requires_correct_type.stderr
+++ b/diesel_compile_tests/tests/fail/find_requires_correct_type.stderr
@@ -60,6 +60,8 @@ error[E0277]: the trait bound `{integer}: diesel::Expression` is not satisfied
              (T0, T1, T2, T3, T4, T5, T6, T7)
            and $N others
    = note: required for `diesel::expression::operators::Eq<string_primary_key::columns::id, {integer}>` to implement `diesel::Expression`
+   = note: 1 redundant requirement hidden
+   = note: required for `diesel::expression::grouped::Grouped<diesel::expression::operators::Eq<string_primary_key::columns::id, {integer}>>` to implement `diesel::Expression`
 note: required for `string_primary_key::columns::id` to implement `EqAll<{integer}>`
   --> tests/fail/find_requires_correct_type.rs:13:9
    |
@@ -91,6 +93,8 @@ error[E0277]: the trait bound `{integer}: ValidGrouping<()>` is not satisfied
              <(T0, T1, T2, T3, T4, T5, T6, T7) as ValidGrouping<__GroupByClause>>
            and $N others
    = note: required for `diesel::expression::operators::Eq<string_primary_key::columns::id, {integer}>` to implement `ValidGrouping<()>`
+   = note: 1 redundant requirement hidden
+   = note: required for `diesel::expression::grouped::Grouped<diesel::expression::operators::Eq<string_primary_key::columns::id, {integer}>>` to implement `ValidGrouping<()>`
    = note: required for `diesel::expression::grouped::Grouped<diesel::expression::operators::Eq<string_primary_key::columns::id, {integer}>>` to implement `NonAggregate`
    = note: required for `SelectStatement<FromClause<string_primary_key::table>>` to implement `FilterDsl<diesel::expression::grouped::Grouped<diesel::expression::operators::Eq<string_primary_key::columns::id, {integer}>>>`
    = note: 1 redundant requirement hidden
diff --git a/diesel_compile_tests/tests/fail/join_with_explicit_on_requires_valid_boolean_expression.stderr b/diesel_compile_tests/tests/fail/join_with_explicit_on_requires_valid_boolean_expression.stderr
index 8522dc94c6..b116c48165 100644
--- a/diesel_compile_tests/tests/fail/join_with_explicit_on_requires_valid_boolean_expression.stderr
+++ b/diesel_compile_tests/tests/fail/join_with_explicit_on_requires_valid_boolean_expression.stderr
@@ -26,8 +26,6 @@ error[E0277]: the trait bound `diesel::sql_types::Integer: BoolOrNullableBool` i
              Bool
              Nullable<Bool>
    = note: required for `JoinOn<query_source::joins::Join<users::table, posts::table, Inner>, users::columns::id>` to implement `QuerySource`
-   = note: required for `SelectStatement<FromClause<JoinOn<query_source::joins::Join<users::table, posts::table, Inner>, users::columns::id>>>` to implement `Query`
-   = note: required for `SelectStatement<FromClause<JoinOn<query_source::joins::Join<users::table, posts::table, Inner>, users::columns::id>>>` to implement `AsQuery`
    = note: required for `SelectStatement<FromClause<users::table>>` to implement `InternalJoinDsl<posts::table, Inner, users::columns::id>`
    = note: 1 redundant requirement hidden
    = note: required for `users::table` to implement `InternalJoinDsl<posts::table, Inner, users::columns::id>`
@weiznich weiznich added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jan 3, 2023
@jyn514 jyn514 added E-needs-bisection Call for participation: This issue needs bisection: https://github.com/rust-lang/cargo-bisect-rustc E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example labels Jan 4, 2023
@jyn514
Copy link
Member

jyn514 commented Jan 4, 2023

If you have time, it would be helpful to find exactly which commit regressed this (using https://github.com/rust-lang/cargo-bisect-rustc).

@weiznich
Copy link
Contributor Author

weiznich commented Jan 4, 2023

searched nightlies: from nightly-2022-08-12 to nightly-2022-09-15
regressed nightly: nightly-2022-08-23
searched commit range: c0941df...015a824
regressed commit: 0b71ffc

bisected with cargo-bisect-rustc v0.6.5

Host triple: x86_64-unknown-linux-gnu
Reproduce with:

cargo bisect-rustc --start=2022-08-12 --end=2022-09-15 --script=./test.sh 

test.sh:

#!/bin/sh
set -x 

TRYBUILD=overwrite cargo test

cargo test

cc @compiler-errors as author of the relevant PR

@compiler-errors
Copy link
Member

I'll take a look.

@rustbot claim

JohnTitor pushed a commit to JohnTitor/rust that referenced this issue Jan 9, 2023
…=lcnr

Use FxIndexSet when updating obligation causes in `adjust_fulfillment_errors_for_expr_obligation`

I have no idea how to test this reliably, but I've **manually** verified it fixes the instability in rust-lang#106417 that isn't due to dtolnay/trybuild#212.

Fixes rust-lang#106417
JohnTitor pushed a commit to JohnTitor/rust that referenced this issue Jan 9, 2023
…=lcnr

Use FxIndexSet when updating obligation causes in `adjust_fulfillment_errors_for_expr_obligation`

I have no idea how to test this reliably, but I've **manually** verified it fixes the instability in rust-lang#106417 that isn't due to dtolnay/trybuild#212.

Fixes rust-lang#106417
@bors bors closed this as completed in 357128a Jan 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints E-needs-bisection Call for participation: This issue needs bisection: https://github.com/rust-lang/cargo-bisect-rustc E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
3 participants