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

Rustup #11316

Merged
merged 34 commits into from
Aug 11, 2023
Merged

Rustup #11316

merged 34 commits into from
Aug 11, 2023

Conversation

flip1995
Copy link
Member

r? @ghost

cc @max-niederman With the latest sync, I'm getting a lot of FP in the redundant_locals lint you recently added. Any ideas where this could come from?

changelog: none

Urgau and others added 30 commits July 13, 2023 23:01
This is necessary for closure captures in 2021 edition, as they capture individual fields, not the full mentioned variables. So it may try to capture a field of an opaque (because the hidden type is known to be something with a field).
…, r=oli-obk

Remove `constness` from `ParamEnv`

This should be replaced by keyword generics/effects. cc #110395

r? `@oli-obk`
…mpiler-errors

make `noop_method_call` warn by default

r? `@compiler-errors`
Rename and allow `cast_ref_to_mut` lint

This PR is a small subset of rust-lang/rust#112431, that is the renaming of the lint (`cast_ref_to_mut` -> `invalid_reference_casting`).

BUT also temporarily change the default level of the lint from deny-by-default to allow-by-default until rust-lang/rust#112431 is merged.

r? `@Nilstrieb`
Mark `map_or` as `#[must_use]`

I don't know what else to say.

r? libs
…cro, r=estebank

Avoid wrong code suggesting for attribute macro

Fixes #107113
r? `@estebank`
It lints against features that are inteded to be internal to the
compiler and standard library. Implements MCP rust-lang#596.

We allow `internal_features` in the standard library and compiler as those
use many features and this _is_ the standard library from the "internal to the compiler and
standard library" after all.

Marking some features as internal wasn't exactly the most scientific approach, I just marked some
mostly obvious features. While there is a categorization in the macro,
it's not very well upheld (should probably be fixed in another PR).

We always pass `-Ainternal_features` in the testsuite
About 400 UI tests and several other tests use internal features.
Instead of throwing the attribute on each one, just always allow them.
There's nothing wrong with testing internal features^^
…r=cjgillot

Expand, rename and improve `incorrect_fn_null_checks` lint

This PR,

 - firstly, expand the lint by now linting on references
 - secondly, it renames the lint `incorrect_fn_null_checks` -> `useless_ptr_null_checks`
 - and thirdly it improves the lint by catching `ptr::from_mut`, `ptr::from_ref`, as well as `<*mut _>::cast` and `<*const _>::cast_mut`

Fixes rust-lang/rust#113601
cc ```@est31```
Add `internal_features` lint

Implements rust-lang/compiler-team#596

Also requires some more test blessing for codegen tests etc

`@jyn514` had the idea of just `allow`ing the lint by default in the test suite. I'm not sure whether this is a good idea, but it's definitely one worth considering. Additional input encouraged.
…cjgillot

Perform OpaqueCast field projection on HIR, too.

fixes #105819

This is necessary for closure captures in 2021 edition, as they capture individual fields, not the full mentioned variables. So it may try to capture a field of an opaque (because the hidden type is known to be something with a field).

See rust-lang/rust#99806 for when and why we added OpaqueCast to MIR.
Indexing is similar to method calls in having an arbitrary
left-hand-side and then something on the right, which is the main part
of the expression. Method calls already have a span for that right part,
but indexing does not. This means that long method chains that use
indexing have really bad spans, especially when the indexing panics and
that span in coverted into a panic location.

This does the same thing as method calls for the AST and HIR, storing an
extra span which is then put into the `fn_span` field in THIR.
Lots of tiny incremental simplifications of `EmitterWriter` internals

ignore the first commit, it's rust-lang/rust#114088 squashed and rebased, but it's needed to use to use `derive_setters`, as they need a newer `syn` version.

Then this PR starts out with removing many arguments that are almost always defaulted to `None` or `false` and replace them with builder methods that can set these fields in the few cases that want to set them.

After that it's one commit after the other that removes or merges things until everything becomes some very simple trait objects
Improve spans for indexing expressions

fixes #114388

Indexing is similar to method calls in having an arbitrary left-hand-side and then something on the right, which is the main part of the expression. Method calls already have a span for that right part, but indexing does not. This means that long method chains that use indexing have really bad spans, especially when the indexing panics and that span in coverted into a panic location.

This does the same thing as method calls for the AST and HIR, storing an extra span which is then put into the `fn_span` field in THIR.

r? compiler-errors
Rollup of 9 pull requests

Successful merges:

 - #113945 (Fix wrong span for trait selection failure error reporting)
 - #114351 ([rustc_span][perf] Remove unnecessary string joins and allocs.)
 - #114418 (bump parking_lot to 0.12)
 - #114434 (Improve spans for indexing expressions)
 - #114450 (Fix ICE failed to get layout for ReferencesError)
 - #114461 (Fix unwrap on None)
 - #114462 (interpret: add mplace_to_ref helper method)
 - #114472 (Reword `confusable_idents` lint)
 - #114477 (Account for `Rc` and `Arc` when suggesting to clone)

r? `@ghost`
`@rustbot` modify labels: rollup
Add documentation to has_deref

Documentation of `has_deref` needed some polish to be more clear about where it should be used and what's it's purpose.

cc rust-lang/rust#114401

r? `@RalfJung`
…c, r=oli-obk

Store the laziness of type aliases in their `DefKind`

Previously, we would treat paths referring to type aliases as *lazy* type aliases if the current crate had lazy type aliases enabled independently of whether the crate which the alias was defined in had the feature enabled or not.

With this PR, the laziness of a type alias depends on the crate it is defined in. This generally makes more sense to me especially if / once lazy type aliases become the default in a new edition and we need to think about *edition interoperability*:

Consider the hypothetical case where the dependency crate has an older edition (and thus eager type aliases), it exports a type alias with bounds & a where-clause (which are void but technically valid), the dependent crate has the latest edition (and thus lazy type aliases) and it uses that type alias. Arguably, the bounds should *not* be checked since at any time, the dependency crate should be allowed to change the bounds at will with a *non*-major version bump & without negatively affecting downstream crates.

As for the reverse case (dependency: lazy type aliases, dependent: eager type aliases), I guess it rules out anything from slight confusion to mild annoyance from upstream crate authors that would be caused by the compiler ignoring the bounds of their type aliases in downstream crates with older editions.

---

This fixes #114468 since before, my assumption that the type alias associated with a given weak projection was lazy (and therefore had its variances computed) did not necessarily hold in cross-crate scenarios (which [I kinda had a hunch about](rust-lang/rust#114253 (comment))) as outlined above. Now it does hold.

`@rustbot` label F-lazy_type_alias
r? `@oli-obk`
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Aug 10, 2023
@max-niederman
Copy link
Contributor

r? @ghost

cc @max-niederman With the latest sync, I'm getting a lot of FP in the redundant_locals lint you recently added. Any ideas where this could come from?

changelog: none

Excuse my ignorance. What is FP in this context?

@matthiaskrgr
Copy link
Member

FP / FN is usually False Positive and False Negative in clippy context

@Centri3
Copy link
Member

Centri3 commented Aug 10, 2023

I think it's #11290 in particular. Maybe there are other issues, @flip1995?

Basically, for shadowing a local in a new scope, we need to ensure that it's never explicitly dropped or mutated within that block to lint it. We can still lint other cases fine.

I will work on this soon, or you can if you want.

@flip1995
Copy link
Member Author

@Centri3 A lot of FPs in our own UI tests: https://github.com/rust-lang/rust-clippy/actions/runs/5825476361/job/15810918074#step:7:941

I would have posted the logs yesterday, but GitHub had issues where actions wouldn't start... Why does this always happen when I want to do the sync? 😮‍💨

@flip1995
Copy link
Member Author

I won't get to this over the weekend 😐

@Centri3
Copy link
Member

Centri3 commented Aug 11, 2023

Looks like async functions are expanded to have a redundant redefinition now: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=3cfab52003203ea576c2ebdf97ba6843 (open it in HIR)

Maybe there's a from_expansion-like function for compiler generated-expansions, regardless of whether it's external or not? (Ok, is_desugaring/desugaring_kind. @max-niederman, should be a one-line addition, once you get the chance.)

@flip1995
Copy link
Member Author

Thanks for investigating. I managed to fix it with your suggested one-liner ❤️

@flip1995 flip1995 marked this pull request as ready for review August 11, 2023 08:54
@rustbot
Copy link
Collaborator

rustbot commented Aug 11, 2023

There are merge commits (commits with multiple parents) in your changes. We have a no merge policy so these commits will need to be removed for this pull request to be merged.

You can start a rebase with the following commands:

$ # rebase
$ git rebase -i master
$ # delete any merge commits in the editor that appears
$ git push --force-with-lease

The following commits are merge commits:

@flip1995
Copy link
Member Author

@bors r+ p=10

@bors
Copy link
Collaborator

bors commented Aug 11, 2023

📌 Commit 3927677 has been approved by flip1995

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Aug 11, 2023

⌛ Testing commit 3927677 with merge 8703661...

@bors
Copy link
Collaborator

bors commented Aug 11, 2023

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: flip1995
Pushing 8703661 to master...

@bors bors merged commit 8703661 into rust-lang:master Aug 11, 2023
5 checks passed
@flip1995 flip1995 deleted the rustup branch August 11, 2023 09:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet