-
-
Notifications
You must be signed in to change notification settings - Fork 14.3k
Regression test for type params on eii #150915
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
Regression test for type params on eii #150915
Conversation
|
rustbot has assigned @petrochenkov. Use |
This comment has been minimized.
This comment has been minimized.
|
r? me r=me when ci is green |
6f628c9 to
6996af3
Compare
|
Some changes occurred in compiler/rustc_attr_parsing cc @jdonszelmann, @JonathanBrouwer Some changes occurred in compiler/rustc_hir/src/attrs cc @jdonszelmann, @JonathanBrouwer Some changes occurred in compiler/rustc_passes/src/check_attr.rs |
This comment has been minimized.
This comment has been minimized.
|
@Kivooeo ended up rebasing on a little renaming PR so they don't conflict down the line, assigned you too, and improved the error messages a bit so there's no duplicate. Could you take a look again? Despite having already approved the original PR |
not sure why this PR contains changes from #150972 |
|
Cause I rebased on it, to use the new names. This one can merge after that one |
|
(i.e. it's a stacked PR, if only github could render that.... :/) |
|
|
|
I think not, an EII definition always generates a foreign function, and foreign functions are already disallowed to take generics. This would give a duplicate error which is exactly what I noticed and why I added another commit on top to fix. |
|
wait, dya mind if I add another test for that to make sure? |
|
okay, I realized you were totally right: we need the other check too make sure the implementation of the EII has no generics. The duplicate error is caused by the fact that an EII with default implementation also contains an implementation, so we error on both the definition and implementation. I'll fix |
not really, it seems like this check was originally added to fix that issue #149983, so removing it was kinda questionable because maybe it was fixed by some another pr that I'm unaware of and this check already unnecessary but glad that you found out that this check is still needed ^^ |
|
wasn't fixed by anything else, but the remove was wrong. Now I just changed it slightly so there's no duplicate error with that last commit, so I think this is the best outcome :) |
|
just to make sure, first two commits will be removed from this pr before merge after #150972 lands right? |
|
indeed, same hashes too :) |
|
yeah, i just don't quite sure what "stacked PR" is, so trying to figure out what are we going to do with this next btw, implementation wise i have no questions, so feel free to r=me once it unblocked after other pr lands |
|
☔ The latest upstream changes (presumably #150981) made this pull request unmergeable. Please resolve the merge conflicts. |
b3477f9 to
b454f76
Compare
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
|
@bors r=kivooeo rollup |
Regression test for type params on eii Somewhere along the line I fixed this one with my other PRs. This just adds the regression tests Closes rust-lang#149983
Regression test for type params on eii Somewhere along the line I fixed this one with my other PRs. This just adds the regression tests Closes rust-lang#149983
…uwer Rollup of 13 pull requests Successful merges: - #150586 (rustdoc: Fix intra-doc link bugs involving type aliases and associated items) - #150677 (Improve std::path::Path::join documentation) - #150723 (std: move `errno` and related functions into `sys::io`) - #150737 (diagnostics: make implicit Sized bounds explicit in E0277) - #150771 (Remove legacy homu `try` and `auto` branch mentions) - #150915 (Regression test for type params on eii) - #150962 (Remove `FeedConstTy` and provide ty when lowering const arg) - #151017 (Port the rustc dump attributes to the attribute parser) - #151019 (Make `Type::of` support unsized types) - #151034 (std: Change UEFI env vars to volatile storage) - #151052 (ui: add regression test for macro resolution ICE (issue #150711)) - #151053 (Reduce flakyness for `tests/rustdoc-gui/notable-trait.goml`) - #151055 (Emit error instead of delayed bug when meeting mismatch type for const array) r? @ghost
Regression test for type params on eii Somewhere along the line I fixed this one with my other PRs. This just adds the regression tests Closes rust-lang#149983
Rollup of 13 pull requests Successful merges: - #150587 (triagebot: add A-rustdoc-js autolabel) - #150677 (Improve std::path::Path::join documentation) - #150737 (diagnostics: make implicit Sized bounds explicit in E0277) - #150771 (Remove legacy homu `try` and `auto` branch mentions) - #150840 (Make `--print=check-cfg` output compatible `--check-cfg` arguments) - #150915 (Regression test for type params on eii) - #151017 (Port the rustc dump attributes to the attribute parser) - #151019 (Make `Type::of` support unsized types) - #151031 (Support arrays in type reflection) - #151043 (armv7-unknown-linux-uclibceabihf.md: Fix bootstrap.toml syntax) - #151052 (ui: add regression test for macro resolution ICE (issue #150711)) - #151053 (Reduce flakyness for `tests/rustdoc-gui/notable-trait.goml`) - #151055 (Emit error instead of delayed bug when meeting mismatch type for const array) r? @ghost
Somewhere along the line I fixed this one with my other PRs. This just adds the regression tests
Closes #149983