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

improve DiscriminantKind handling #74276

Merged
merged 1 commit into from
Jul 16, 2020
Merged

Conversation

lcnr
Copy link
Contributor

@lcnr lcnr commented Jul 12, 2020

Adds a lang item discriminant_type for the associated type DiscriminantKind::Discriminant.

Changes the discriminant of generators from i32 to u32, which should not be observable to fix an
oversight where MIR was using u32 and codegen and typeck used i32.

@rust-highfive
Copy link
Collaborator

r? @matthewjasper

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 12, 2020
Copy link
Member

@nagisa nagisa left a comment

Choose a reason for hiding this comment

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

You mentioned that discriminant for Generators became u32. Can you elaborate "how" (in e.g. the commit message), given that the changes otherwise only involve removing the previously present i32… default.

Otheriwse r=me on implementation...

@bors
Copy link
Contributor

bors commented Jul 15, 2020

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

@lcnr lcnr force-pushed the discriminant-kind-what branch 2 times, most recently from 3647808 to c51e366 Compare July 15, 2020 08:19
This now reuses `fn discriminant_ty` in project, removing
some code duplication. Doing so made me realize that
we previously had a disagreement about the discriminant
type of generators, with MIR using `u32` and codegen and
trait selection using `i32`.

We now always use `u32`.
@lcnr
Copy link
Contributor Author

lcnr commented Jul 15, 2020

Renamed the lang item to discriminant_type and added a note to the commit message explaining the change to the discriminant type of generators.

@nagisa
Copy link
Member

nagisa commented Jul 15, 2020

@bors r+

@bors
Copy link
Contributor

bors commented Jul 15, 2020

📌 Commit fcf52c1 has been approved by nagisa

@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 Jul 15, 2020
@nagisa nagisa assigned nagisa and unassigned matthewjasper Jul 15, 2020
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 16, 2020
…arth

Rollup of 14 pull requests

Successful merges:

 - rust-lang#72973 (RISC-V GNU/Linux as host platform)
 - rust-lang#73918 (Clean up E0715 explanation)
 - rust-lang#73959 (Clean up E0716 explanation)
 - rust-lang#74119 (Remove `Compiler::compile()`.)
 - rust-lang#74196 (Add option to collapse automatically implementors)
 - rust-lang#74218 (Add margin after doc search results)
 - rust-lang#74276 (improve DiscriminantKind handling)
 - rust-lang#74291 (Added docs for `From<c_int>` for `ExitStatus`)
 - rust-lang#74294 (Update cross-compilation README)
 - rust-lang#74337 (Handle case of incomplete local ty more gracefully)
 - rust-lang#74344 (Remove string comparison and use diagnostic item instead)
 - rust-lang#74347 (Initialize default providers only once)
 - rust-lang#74353 (Edit docs for rustc_middle::dep_graph::dep_node)
 - rust-lang#74374 (Add a 1.45 release note on lto vs. embed-bitcode)

Failed merges:

 - rust-lang#74251 (Teach bootstrap about target files vs target triples)

r? @ghost
@bors bors merged commit 47f6a9d into rust-lang:master Jul 16, 2020
@lcnr lcnr deleted the discriminant-kind-what branch July 16, 2020 06:54
@cuviper cuviper added this to the 1.47.0 milestone May 2, 2024
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants