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

Rollup of 7 pull requests #129261

Merged
merged 17 commits into from
Aug 19, 2024
Merged

Rollup of 7 pull requests #129261

merged 17 commits into from
Aug 19, 2024

Conversation

tgross35
Copy link
Contributor

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

surechen and others added 17 commits July 23, 2024 10:24
For following:

```rust
struct A;
impl A {
    fn test4(&self) {
        let mut _file = File::create("foo.txt")?;
        //~^ ERROR the `?` operator can only be used in a method
    }
```
Suggest:

```rust
impl A {
    fn test4(&self) -> Result<(), Box<dyn std::error::Error>> {
        let mut _file = File::create("foo.txt")?;
        //~^ ERROR the `?` operator can only be used in a method

    Ok(())
    }
}
```

For rust-lang#125997
After deduplication the block conceptually belongs to multiple locations
in the source. Although these blocks are unreachable, in rust-lang#123341 we did
come across a real side effect, an unreachable block that survives into
the compiled code can cause a debugger to set a breakpoint on the wrong
instruction. Erasing the source information ensures that a debugger will
never be misled into thinking that the unreachable block is worth setting
a breakpoint on, especially after rust-lang#128627.

Technically we don't need to erase the source information if all the
deduplicated blocks have identical source information, but tracking
that seems like more effort than it's worth.
The documentation incorrectly stated that std::env::var could return
an error for variable names containing '=' or the NUL byte. Copy the
correct documentation from var_os.

var_os was fixed in Commit 8a7a665, Pull Request rust-lang#109894, which
closed Issue rust-lang#109893.

This documentation was incorrectly added in commit f2c0f29, which
replaced a panic in var_os by returning None, but documented the
change as "May error if ...".

Reference the specific error values and link to them.
The current cross-compilation toolchain for the LoongArch64 target
consists of GCC 13.2.0, Binutils 2.40, and Glibc 2.36. However, Binutils
2.40 has known issues that in broken binaries without any error reports:

- rust-lang#121289
- cross-rs/cross#1538

This patch upgrades the cross-compilation toolchain for the LoongArch64 target
to resolve these issues.

- GCC: 13.2.0 -> 14.2.0
- Binutils: 2.40 -> 2.42

The new binaries remain compatible with the existing GCC 13.2.0/Glibc 2.36
distribution, and no issues have been identified.
Fixes rust-lang#126831.

Without this patch, type normalization is not always idempotent, which
leads to all sorts of bugs in places that assume that normalizing a
normalized type does nothing.
Stabilize `raw_ref_op` (RFC 2582)

This stabilizes the syntax `&raw const $expr` and `&raw mut $expr`. It has existed unstably for ~4 years now, and has been exposed on stable via the `addr_of` and `addr_of_mut` macros since Rust 1.51 (released more than 3 years ago). I think it has become clear that these operations are here to stay. So it is about time we give them proper primitive syntax. This has two advantages over the macro:

- Being macros, `addr_of`/`addr_of_mut` could in theory do arbitrary magic with the expression on which they work. The only "magic" they actually do is using the argument as a place expression rather than as a value expression. Place expressions are already a subtle topic and poorly understood by many programmers; having this hidden behind a macro using unstable language features makes this even worse. Conversely, people do have an idea of what happens below `&`/`&mut`, so we can make the subtle topic a lot more approachable by connecting to existing intuition.
- The name `addr_of` is quite unfortunate from today's perspective, given that we have accepted provenance as a reality, which means that a pointer is *not* just an address. Strict provenance has a method, `addr`, which extracts the address of a pointer; using the term `addr` in two different ways is quite unfortunate. That's why this PR soft-deprecates `addr_of` -- we will wait a long time before actually showing any warning here, but we should start telling people that the "addr" part of this name is somewhat misleading, and `&raw` avoids that potential confusion.

In summary, this syntax improves developers' ability to conceptualize the operational semantics of Rust, while making a fundamental operation frequently used in unsafe code feel properly built in.

Possible questions to consider, based on the RFC and [this](rust-lang#64490 (comment)) great summary by `@CAD97:`

- Some questions are entirely about the semantics. The semantics are the same as with the macros so I don't think this should have any impact on this syntax PR. Still, for completeness' sake:
  - Should `&raw const *mut_ref` give a read-only pointer?
    - Tracked at: rust-lang/unsafe-code-guidelines#257
    - I think ideally the answer is "no". Stacked Borrows says that pointer is read-only, but Tree Borrows says it is mutable.
  - What exactly does `&raw const (*ptr).field` require? Answered in [the reference](https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html): the arithmetic to compute the field offset follows the rules of `ptr::offset`, making it UB if it goes out-of-bounds. Making this a safe operation (using `wrapping_offset` rules) is considered too much of a loss for alias analysis.
- Choose a different syntax? I don't want to re-litigate the RFC. The only credible alternative that has been proposed is `&raw $place` instead of `&raw const $place`, which (IIUC) could be achieved by making `raw` a contextual keyword in a new edition. The type is named `*const T`, so the explicit `const` is consistent in that regard. `&raw expr` lacks the explicit indication of immutability. However, `&raw const expr` is quite a but longer than `addr_of!(expr)`.
- Shouldn't we have a completely new, better raw pointer type instead? Yes we all want to see that happen -- but I don't think we should block stabilization on that, given that such a nicer type is not on the horizon currently and given the issues with `addr_of!` mentioned above. (If we keep the `&raw $place` syntax free for this, we could use it in the future for that new type.)
- What about the lint the RFC talked about? It hasn't been implemented yet.  Given that the problematic code is UB with or without this stabilization, I don't think the lack of the lint should block stabilization.
  - I created an issue to track adding it: rust-lang#127724
- Other points from the "future possibilites of the RFC
  - "Syntactic sugar" extension: this has not been implemented. I'd argue this is too confusing, we should stick to what the RFC suggested and if we want to do anything about such expressions, add the lint.
  - Encouraging / requiring `&raw` in situations where references are often/definitely incorrect: this has been / is being implemented. On packed fields this already is a hard error, and for `static mut` a lint suggesting raw pointers is being rolled out.
  - Lowering of casts: this has been implemented. (It's also an invisible implementation detail.)
  - `offsetof` woes: we now have native `offset_of` so this is not relevant any more.

To be done before landing:

- [x] Suppress `unused_parens` lint around `&raw {const|mut}` expressions
  - See bottom of rust-lang#127679 (comment) for rationale
  - Implementation: rust-lang#128782
- [ ] Update the Reference.
  - rust-lang/reference#1567

Fixes rust-lang#64490

cc `@rust-lang/lang` `@rust-lang/opsem`

try-job: x86_64-msvc
try-job: test-various
try-job: dist-various-1
try-job: armhf-gnu
try-job: aarch64-apple
Suggest adding Result return type for associated method in E0277.

Recommit rust-lang#126515 because I messed up during rebase,

Suggest adding Result return type for associated method in E0277.

For following:

```rust
struct A;
impl A {
    fn test4(&self) {
        let mut _file = File::create("foo.txt")?;
        //~^ ERROR the `?` operator can only be used in a method
    }
```

Suggest:

```rust
impl A {
    fn test4(&self) -> Result<(), Box<dyn std::error::Error>> {
        let mut _file = File::create("foo.txt")?;
        //~^ ERROR the `?` operator can only be used in a method

    Ok(())
    }
}
```

For rust-lang#125997

r? `@cjgillot`
…, r=nnethercote

When deduplicating unreachable blocks, erase the source information.

After deduplication the block conceptually belongs to multiple locations in the source. Although these blocks are unreachable, in rust-lang#123341 we did come across a real side effect, an unreachable block that survives into the compiled code can cause a debugger to set a breakpoint on the wrong instruction. Erasing the source information ensures that a debugger will never be misled into thinking that the unreachable block is worth setting a breakpoint on, especially after rust-lang#128627.

Technically we don't need to erase the source information if all the deduplicated blocks have identical source information, but tracking that seems like more effort than it's worth.

I'll let njn redirect this one too. r? `@nnethercote`
…rkingjubilee

doc: std::env::var: Returns None for names with '=' or NUL byte

The documentation incorrectly stated that std::env::var could return an error for variable names containing '=' or the NUL byte. Copy the correct documentation from var_os.

var_os was fixed in Commit 8a7a665, Pull Request rust-lang#109894, which closed Issue rust-lang#109893.

This documentation was incorrectly added in commit f2c0f29, which replaced a panic in var_os by returning None, but documented the change as "May error if ...".

Reference the specific error values and link to them.
…4, r=Mark-Simulacrum

Update `crosstool-ng` for loongarch64

The current cross-compilation toolchain for the LoongArch64 target consists of GCC 13.2.0, Binutils 2.40, and Glibc 2.36. However, Binutils 2.40 has known issues that in broken binaries without any error reports:

- rust-lang#121289
- cross-rs/cross#1538

This patch upgrades the cross-compilation toolchain for the LoongArch64 target to resolve these issues.

- GCC: 13.2.0 -> 14.2.0
- Binutils: 2.40 -> 2.42

The new binaries remain compatible with the existing GCC 13.2.0/Glibc 2.36 distribution, and no issues have been identified.

try-job: dist-loongarch64-linux
…acrum

Include a copy of `compiler-rt` source in the `download-ci-llvm` tarball

This will make it possible to experiment with allowing `download-ci-llvm` builds to build `library/profiler_builtins`, without needing to check out the `src/llvm-project` submodule.

By itself, this PR just adds the files to the tarball, but doesn't actually do anything with them. The idea is that once this is merged, it will then be much easier to proceed with work on the necessary bootstrap changes (using the real downloaded tarball), without having to rig up weird hacks to simulate downloading a modified tarball.

---

Adding these files to the compressed tarballs appears to increase its size by a negligible amount (<1 MB out of 400/800+ MB).

The uncompressed size is about 14 MB (out of 2+ GB for the whole tarball).

(The excluded test files would have been another 35 MB.)
Fix order of normalization and recursion in const folding.

Fixes rust-lang#126831.

Without this patch, type normalization is not always idempotent, which leads to all sorts of bugs in places that assume that normalizing a normalized type does nothing.

Tracking issue: rust-lang#95174

r? BoxyUwU
@rustbot rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc PG-exploit-mitigations Project group: Exploit mitigations S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. rollup A PR which is a rollup labels Aug 19, 2024
@tgross35
Copy link
Contributor Author

@bors r+ rollup=never p=7

@bors
Copy link
Contributor

bors commented Aug 19, 2024

📌 Commit 8a513f1 has been approved by tgross35

It is now in the queue for this repository.

@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 Aug 19, 2024
@bors
Copy link
Contributor

bors commented Aug 19, 2024

⌛ Testing commit 8a513f1 with merge e3f909b...

@tgross35
Copy link
Contributor Author

The loongarch jobs have been running for a long time, but it seems this is to be expected based on the try job at #129048. Hopefully just caching.

@bors
Copy link
Contributor

bors commented Aug 19, 2024

☀️ Test successful - checks-actions
Approved by: tgross35
Pushing e3f909b to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Aug 19, 2024
@bors bors merged commit e3f909b into rust-lang:master Aug 19, 2024
7 checks passed
@rustbot rustbot added this to the 1.82.0 milestone Aug 19, 2024
@rust-timer
Copy link
Collaborator

📌 Perf builds for each rolled up PR:

PR# Message Perf Build Sha
#127679 Stabilize raw_ref_op (RFC 2582) adde2c652f6af751d20e4109b7d48dc4b821de3f (link)
#128084 Suggest adding Result return type for associated method in … 89e27026584fcb7cbd4428cf4d960cd56c49cef7 (link)
#128628 When deduplicating unreachable blocks, erase the source inf… 2eeaff4412df1be4fad6649bc78ddbb774b593de (link)
#128902 doc: std::env::var: Returns None for names with '=' or NUL … dca5b66dec823640f48970056bd9f10cc4c45f39 (link)
#129048 Update crosstool-ng for loongarch64 f9f3f0680d679510cb02cbd33e93978719bf3f95 (link)
#129116 Include a copy of compiler-rt source in the `download-ci-… 27de218a6c1bc81aef7a9e45eca2616489f087e7 (link)
#129208 Fix order of normalization and recursion in const folding. 47147a65b622914e261e84fb18d431417857b2b6 (link)

previous master: 804be74e3c

In the case of a perf regression, run the following command for each PR you suspect might be the cause: @rust-timer build $SHA

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (e3f909b): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results (secondary 3.3%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
3.3% [3.3%, 3.3%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Cycles

Results (primary 0.6%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
0.6% [0.6%, 0.6%] 1
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 0.6% [0.6%, 0.6%] 1

Binary size

Results (primary 0.0%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
0.0% [0.0%, 0.0%] 18
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 0.0% [0.0%, 0.0%] 18

Bootstrap: 751.973s -> 750.46s (-0.20%)
Artifact size: 339.15 MiB -> 339.15 MiB (0.00%)

@tgross35 tgross35 deleted the rollup-xjbvqx7 branch November 2, 2024 21:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc merged-by-bors This PR was explicitly merged by bors. PG-exploit-mitigations Project group: Exploit mitigations rollup A PR which is a rollup S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.