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

Better error for non const PartialEq call generated by match #112232

Merged
merged 2 commits into from
Jun 20, 2023

Conversation

fee1-dead
Copy link
Member

Resolves #90237

@rustbot
Copy link
Collaborator

rustbot commented Jun 3, 2023

r? @b-naber

(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 Jun 3, 2023
@rustbot
Copy link
Collaborator

rustbot commented Jun 3, 2023

This PR changes Stable MIR

cc @oli-obk, @celinval

This PR changes MIR

cc @oli-obk, @RalfJung, @JakobDegen, @davidtwco, @celinval, @vakaras

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

@rust-log-analyzer

This comment has been minimized.

@fee1-dead fee1-dead force-pushed the match-eq-const-msg branch from f565198 to fd60cbc Compare June 3, 2023 04:45
@rustbot
Copy link
Collaborator

rustbot commented Jun 3, 2023

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

|
= note: `str` cannot be compared in compile-time, and therefore cannot be used in `match`es
= note: calls in constant functions are limited to constant functions, tuple structs and tuple variants
= help: add `#![feature(const_trait_impl)]` to the crate attributes to enable
Copy link
Contributor

@deltragon deltragon Jun 3, 2023

Choose a reason for hiding this comment

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

Does this help note appear to users on stable too? It seems confusing either way, as adding it doesn't fix the error (just makes it slightly more detailed).

Copy link
Member Author

@fee1-dead fee1-dead Jun 3, 2023

Choose a reason for hiding this comment

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

No it doesn't. Stable users don't get the suggestion to add unstable features.

Copy link
Contributor

@b-naber b-naber left a comment

Choose a reason for hiding this comment

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

LGTM. Had some thoughts/nits.

@@ -271,6 +271,18 @@ pub struct RawBytesNote {
pub bytes: String,
}

// FIXME(fee1-dead) do not use stringly typed `ConstContext`
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you include this as a reminder to be fixed in this PR or do you want to change this later?

Copy link
Member Author

Choose a reason for hiding this comment

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

I want to change this later, not in this PR

/// Represents how a `TerminatorKind::Call` was constructed, used for diagnostics
#[derive(Clone, Copy, TyEncodable, TyDecodable, Debug, PartialEq, Hash, HashStable)]
#[derive(TypeFoldable, TypeVisitable)]
pub enum CallDesugaring {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this use a different name? Isn't desugaring exclusively used to refer to lowering of source-level constructs to internal constructs? So maybe ThirCallSource or something would be more appropriate?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure if ThirCallSource would be better as that means Option<ThirCallSource>::None means that there was no call source from THIR. Option<CallDesugaring>::None suggests that it was a normal call. For CallDesugaring::Misc they still didn't come from normal calls so it would still mean using calls internally to represent code that does not explicitly try to call functions

Copy link
Contributor

Choose a reason for hiding this comment

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

Why can't we do this? 😅

enum ThirCallSource {
  HirCall,
  OverloadedOperator,
  MatchCmp,
  Misc,
}

@@ -100,7 +100,7 @@ pub struct FnCallNonConst<'tcx> {
pub callee: DefId,
pub substs: SubstsRef<'tcx>,
pub span: Span,
pub from_hir_call: bool,
pub desugar: Option<CallDesugaring>,
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add a similar comment to the one in TerminatorKind?

/// is false then this is the desugaring.
OverloadedOperator,
/// This was from comparison generated by a match.
MatchCmp,
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you link to #90237 here?

@bors
Copy link
Contributor

bors commented Jun 15, 2023

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

@fee1-dead fee1-dead force-pushed the match-eq-const-msg branch from fd60cbc to f345403 Compare June 15, 2023 17:07
@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Contributor

bors commented Jun 17, 2023

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

@fee1-dead fee1-dead force-pushed the match-eq-const-msg branch from 61db4ed to 3de318b Compare June 18, 2023 05:08
@fee1-dead fee1-dead force-pushed the match-eq-const-msg branch from 3de318b to 89c24af Compare June 18, 2023 05:26
@b-naber
Copy link
Contributor

b-naber commented Jun 18, 2023

Thanks for changing that.

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Jun 18, 2023

📌 Commit 89c24af has been approved by b-naber

It is now in the queue for this repository.

@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 Jun 18, 2023
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jun 18, 2023
…sg, r=b-naber

Better error for non const `PartialEq` call generated by `match`

Resolves rust-lang#90237
@compiler-errors
Copy link
Member

@bors r- failed in rollup: #112771 (comment)

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jun 18, 2023
@b-naber
Copy link
Contributor

b-naber commented Jun 19, 2023

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Jun 19, 2023

📌 Commit 446db51 has been approved by b-naber

It is now in the queue for this repository.

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 19, 2023
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 20, 2023
…mpiler-errors

Rollup of 8 pull requests

Successful merges:

 - rust-lang#112232 (Better error for non const `PartialEq` call generated by `match`)
 - rust-lang#112499 (Fix python linting errors)
 - rust-lang#112596 (Suggest correct signature on missing fn returning RPITIT/AFIT)
 - rust-lang#112606 (Alter `Display` for `Ipv6Addr` for IPv4-compatible addresses)
 - rust-lang#112781 (Don't consider TAIT normalizable to hidden ty if it would result in impossible item bounds)
 - rust-lang#112787 (Add gha problem matcher)
 - rust-lang#112799 (Clean up "doc(hidden)" check)
 - rust-lang#112803 (Format the examples directory of cg_clif)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 31d1fbf into rust-lang:master Jun 20, 2023
@rustbot rustbot added this to the 1.72.0 milestone Jun 20, 2023
flip1995 pushed a commit to flip1995/rust that referenced this pull request Jun 30, 2023
…sg, r=b-naber

Better error for non const `PartialEq` call generated by `match`

Resolves rust-lang#90237
bjorn3 pushed a commit to bjorn3/rust that referenced this pull request Jul 22, 2023
…sg, r=b-naber

Better error for non const `PartialEq` call generated by `match`

Resolves rust-lang#90237
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. 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.

Confusing error message when matching on &str in const fn
7 participants