Skip to content

Rustup #12867

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

Merged
merged 24 commits into from
May 30, 2024
Merged

Rustup #12867

merged 24 commits into from
May 30, 2024

Conversation

flip1995
Copy link
Member

r? @ghost

changelog: none

compiler-errors and others added 24 commits May 16, 2024 14:24
Rename Unsafe to Safety

Alternative to #124455, which is to just have one Safety enum to use everywhere, this opens the posibility of adding `ast::Safety::Safe` that's useful for unsafe extern blocks.

This leaves us today with:

```rust
enum ast::Safety {
    Unsafe(Span),
    Default,
    // Safe (going to be added for unsafe extern blocks)
}

enum hir::Safety {
    Unsafe,
    Safe,
}
```

We would convert from `ast::Safety::Default` into the right Safety level according the context.
…rrors

chore: Remove repeated words (extension of #124924)

When I saw #124924 I thought "Hey, I'm sure that there are far more than just two typos of this nature in the codebase". So here's some more typo-fixing.

Some found with regex, some found with a spellchecker. Every single one manually reviewed by me (along with hundreds of false negatives by the tools)
Rollup of 4 pull requests

Successful merges:

 - #124948 (chore: Remove repeated words (extension of #124924))
 - #124992 (Add example to IsTerminal::is_terminal)
 - #125279 (make `Debug` impl for `Term` simpler)
 - #125286 (Miri subtree update)

r? `@ghost`
`@rustbot` modify labels: rollup
And explain when it should be used.
* instead simply set the primary message inside the lint decorator functions
* it used to be this way before [#]101986 which introduced `msg` to prevent
  good path delayed bugs (which no longer exist) from firing under certain
  circumstances when lints were suppressed / silenced
* this is no longer necessary for various reasons I presume
* it shaves off complexity and makes further changes easier to implement
…iler-errors

Remove `DefId` from `EarlyParamRegion`

Currently we represent usages of `Region` parameters via the `ReEarlyParam` or `ReLateParam` variants. The `ReEarlyParam` is effectively equivalent to `TyKind::Param` and `ConstKind::Param` (i.e. it stores a `Symbol` and a `u32` index) however it also stores a `DefId` for the definition of the lifetime parameter.

This was used in roughly two places:
- Borrowck diagnostics instead of threading the appropriate `body_id` down to relevant locations. Interestingly there were already some places that had to pass down a `DefId` manually.
- Some opaque type checking logic was using the `DefId` field to track captured lifetimes

I've split this PR up into a commit for generate rote changes to diagnostics code to pass around a `DefId` manually everywhere, and another commit for the opaque type related changes which likely require more careful review as they might change the semantics of lints/errors.

Instead of manually passing the `DefId` around everywhere I previously tried to bundle it in with `TypeErrCtxt` but ran into issues with some call sites of `infcx.err_ctxt` being unable to provide a `DefId`, particularly places involved with trait solving and normalization. It might be worth investigating adding some new wrapper type to pass this around everywhere but I think this might be acceptable for now.

This pr also has the effect of reducing the size of `EarlyParamRegion` from 16 bytes -> 8 bytes. I wouldn't expect this to have any direct performance improvement however, other variants of `RegionKind` over `8` bytes are all because they contain a `BoundRegionKind` which is, as far as I know, mostly there for diagnostics. If we're ever able to remove this it would shrink the `RegionKind` type from `24` bytes to `12` (and with clever bit packing we might be able to get it to `8` bytes). I am curious what the performance impact would be of removing interning of `Region`'s if we ever manage to shrink `RegionKind` that much.

Sidenote: by removing the `DefId` the `Debug` output for `Region` has gotten significantly nicer. As an example see this opaque type debug print before vs after this PR:
`Opaque(DefId(0:13 ~ impl_trait_captures[aeb9]::foo::{opaque#0}), [DefId(0:9 ~ impl_trait_captures[aeb9]::foo::'a)_'a/#0, T, DefId(0:9 ~ impl_trait_captures[aeb9]::foo::'a)_'a/#0])`
`Opaque(DefId(0:13 ~ impl_trait_captures[aeb9]::foo::{opaque#0}), ['a/#0, T, 'a/#0])`

r? `@compiler-errors` (I would like someone who understands the opaque type setup to atleast review the type system commit, but the rest is likely reviewable by anyone)
[perf] Delay the construction of early lint diag structs

Attacks some of the perf regressions from rust-lang/rust#124417 (comment).

See individual commits for details. The first three commits are not strictly necessary.
However, the 2nd one (06bc4fc, *Remove `LintDiagnostic::msg`*) makes the main change way nicer to implement.
It's also pretty sweet on its own if I may say so myself.
don't inhibit random field reordering on repr(packed(1))

`inhibit_struct_field_reordering_opt` being false means we exclude this type from random field shuffling. However, `packed(1)` types can still be shuffled! The logic was added in rust-lang/rust#48528 since it's pointless to reorder fields in packed(1) types (there's no padding that could be saved) -- but that shouldn't inhibit `-Zrandomize-layout` (which did not exist at the time).

We could add an optimization elsewhere to not bother sorting the fields for `repr(packed)` types, but I don't think that's worth the effort.

This *does* change the behavior in that we may now reorder fields of `packed(1)` structs (e.g. if there are niches, we'll try to move them to the start/end, according to `NicheBias`).  We were always allowed to do that but so far we didn't. Quoting the [reference](https://doc.rust-lang.org/reference/type-layout.html):

> On their own, align and packed do not provide guarantees about the order of fields in the layout of a struct or the layout of an enum variant, although they may be combined with representations (such as C) which do provide such guarantees.
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label May 30, 2024
@flip1995
Copy link
Member Author

@bors r+ p=1

@bors
Copy link
Contributor

bors commented May 30, 2024

📌 Commit 280ed2b has been approved by flip1995

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented May 30, 2024

⌛ Testing commit 280ed2b with merge 28743b9...

bors added a commit that referenced this pull request May 30, 2024
Rustup

r? `@ghost`

changelog: none
@flip1995
Copy link
Member Author

@bors retry

@bors
Copy link
Contributor

bors commented May 30, 2024

⌛ Testing commit 280ed2b with merge c9139bd...

@bors
Copy link
Contributor

bors commented May 30, 2024

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

@bors bors merged commit c9139bd into rust-lang:master May 30, 2024
@flip1995 flip1995 deleted the rustup branch May 30, 2024 08:37
@bors bors mentioned this pull request May 30, 2024
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.