Revert "Rollup merge of #62696 - chocol4te:fix_#62194, r=estebank"
This reverts commit df21a6f040 (#62696), reversing
changes made to cc16d04869.
That PR makes error messages worse than before, and we couldn't come up with a way of actually making them better, so revert it for now. Any idea for making this error message better is welcome!
Fixes#63145.
r? @estebank
fix UB in a test
We used to compare two mutable references that were supposed to point to the same thing. That's no good.
Compare them as raw pointers instead.
Make use of possibly uninitialized data [E0381] a hard error
This is one of the behaviors we no longer allow in NLL. Since it can
lead to undefined behavior, I think it's definitely worth making it a
hard error without waiting to turn off migration mode (#58781).
Closes#60450.
My ulterior motive here is making it impossible to leave variables
partially initialized across a yield (see #60889, discussion at #63035), so
tests are included for that.
cc #54987
---
I'm not sure if bypassing the buffer is a good way of doing this. We could also make a `force_errors_buffer` or similar that gets recombined with all the errors as they are emitted. But this is simpler and seems fine to me.
r? @Centril
cc @cramertj @nikomatsakis @pnkfelix @RalfJung
Explaining the reason why validation is performed in to_str of path.rs
I thought it's good to explain the reason for the validation during the conversion between Path/PathBuffer into str, which explains the reason for returning an Option at this point (good for beginners who are reading through the docs).
Remove special code-path for handing unknown tokens
In `StringReader`, we have a buffer of fatal errors, which is used only in a single case: when we see something which is not a reasonable token at all, like `🦀`. I think a more straightforward thing to do here is to produce an explicit error token in this case, and let the next layer (the parser), deal with it.
However currently this leads to duplicated error messages. What should we do with this? Naively, I would think that emitting (just emitting, not raising) `FatalError` should stop other errors, but looks like this is not the case? We can also probably tweak parser on the case-by-case basis, to avoid emitting "expected" errors if the current token is an `Err`. I personally also fine with cascading errors in this case: it's quite unlikely that you actually type a fully invalid token.
@petrochenkov, which approach should we take to fight cascading errors?
Implement DoubleEndedIterator for iter::{StepBy, Peekable, Take}
Now that `DoubleEndedIterator::nth_back` has landed, `StepBy` and `Take` can have an efficient `DoubleEndedIterator` implementation. I don't know if there was any particular reason for `Peekable` not having a `DoubleEndedIterator` implementation, but it's quite trivial and I don't see any drawbacks to having it.
I'm not very happy about the implementation of `Peekable::try_rfold`, but I didn't see another way to only take the value out of `self.peeked` in case `self.iter.try_rfold` didn't exit early.
I only added `Peekable::rfold` (in addition to `try_rfold`) because its `Iterator` implementation has both `fold` and `try_fold` (and for similar reasons I added `Take::try_rfold` but not `Take::rfold`). Do we have any guidelines on whether we want both? If we do want both, maybe we should investigate which iterator adaptors override `try_fold` but not `fold` and add the missing implementations. At the moment I think that it's better to always have iterator adaptors implement both, because some iterators have a simpler `fold` implementation than their `try_fold` implementation.
The tests that I added may not be sufficient because they're all just existing tests where `next`/`nth`/`fold`/`try_fold` are replaced by their DEI counterparts, but I do think all paths are covered. Is there anything in particular that I should probably also test?
This is one of the behaviors we no longer allow in NLL. Since it can
lead to undefined behavior, I think it's definitely worth making it a
hard error without waiting to turn off migration mode (#58781).
Closes#60450.
My ulterior motive here is making it impossible to leave variables
partially initialized across a yield (see discussion at #63035), so
tests are included for that.
`unwrap_usize` should at least try to evaluate the underlying constant
r? @eddyb
fixes#59016
I know that I'm still using `ParamEnv` wrongly, but that's a preexisting issue not amplified by this PR.
CTFE: simplify ConstValue by not checking for alignment
I hope the test suite actually covers the problematic cases here?
r? @oli-obk
Fixes https://github.com/rust-lang/rust/issues/61952
Move special treatment of `derive(Copy, PartialEq, Eq)` from expansion infrastructure to elsewhere
As described in https://github.com/rust-lang/rust/pull/62086#issuecomment-515195477.
Reminder:
- `derive(PartialEq, Eq)` makes the type it applied to a "structural match" type, so constants of this type can be used in patterns (and const generics in the future).
- `derive(Copy)` notifies other derives that the type it applied to implements `Copy`, so `derive(Clone)` can generate optimized code and other derives can generate code working with `packed` types and types with `rustc_layout_scalar_valid_range` attributes.
First, the special behavior is now enabled after properly resolving the derives, rather than after textually comparing them with `"Copy"`, `"PartialEq"` and `"Eq"` in `fn add_derived_markers`.
The markers are no longer kept as attributes in AST since derives cannot modify items and previously did it through hacks in the expansion infra.
Instead, the markers are now kept in a "global context" available from all the necessary places, namely - resolver.
For `derive(PartialEq, Eq)` the markers are created by the derive macros themselves and then consumed during HIR lowering to add the `#[structural_match]` attribute in HIR.
This is still a hack, but now it's a hack local to two specific macros rather than affecting the whole expansion infra.
Ideally we should find the way to put `#[structural_match]` on the impls rather than on the original item, and then consume it in `rustc_mir`, then no hacks in expansion and lowering will be required.
(I'll make an issue about this for someone else to solve, after this PR lands.)
The marker for `derive(Copy)` cannot be emitted by the `Copy` macro itself because we need to know it *before* the `Copy` macro is expanded for expanding other macros.
So we have to do it in resolve and block expansion of any derives in a `derive(...)` container until we know for sure whether this container has `Copy` in it or not.
Nasty stuff.
r? @eddyb or @matthewjasper
More Miri error tweaks
* Add `err_` version of the `_format!` macros
* Add `UbExperimental` variant so that Miri can mark some UB as experimental (e.g. Stacked Borrows)
r? @oli-obk
Rename `ItemKind::Ty` to `ItemKind::TyAlias`
The current name is not entirely clear without context and `TyAlias` is consistent with `ItemKind::TraitAlias`.
