Remove drop order twist of && and || and make them associative #103293

est31 · 2022-10-20T08:52:48Z

Previously a short circuiting binop chain (chain of && or ||s) would drop the temporaries created by the first element after all the other elements, and otherwise follow evaluation order. So f(1).g() && f(2).g() && f(3).g() && f(4).g() would drop the temporaries in the order 2,3,4,1. This made && and || non-associative regarding drop order. In other words, adding ()'s to the expression would change drop order: f(1).g() && (f(2).g() && f(3).g()) && f(4).g() for example would drop in the order 3,2,4,1.

As, except for the bool result, there is no data returned by the sub-expressions of the short circuiting binops, we can safely discard of any temporaries created by the sub-expr. Previously, code was already putting the rhs's into terminating scopes, but missed it for the lhs's.

This commit addresses this "twist". We now also put the lhs into a terminating scope. The drop order of the above expressions becomes 1,2,3,4.

There might be code relying on the current order, and therefore I'd recommend doing a crater run to gauge the impact. I'd argue that such code is already quite wonky as it is one foo() && addition away from breaking. For the impact, I don't expect any build failures, as the compiler gets strictly more tolerant: shortening the lifetime of temporaries only expands the list of programs the compiler accepts as valid. There might be runtime failures caused by this change however. Edit: both build and runtime failures are possible, e.g. see the example provided by dtolnay below. Edit2: the crater run has finished and results are that there is only one build failure which is easy to fix with a +/- 1 line diff.

I've included a testcase that now compiles thanks to this patch.

The breakage is also limited to drop order relative to conditionals in the && chain: that is, in code like this:

let hello = foo().hi() && bar().world();
println!("hi");

we already drop the temporaries of foo().hi() before we reach "hi".

I'd ideally have this PR merged before let chains are stabilized. If this PR is taking too long, I'd love to have a more restricted version of this change limited to &&'s in let chains: the &&'s of such chains are quite special anyways as they accept let bindings, in there the && is therefore more a part of the "if let chain" construct than a construct of its own.

Fixes #103107

Status: waiting on this accepted FCP finishing.

rust-highfive · 2022-10-20T08:52:52Z

r? @wesleywiser

(rust-highfive has picked a reviewer for you, use r? to override)

dtolnay · 2022-10-20T11:08:13Z

For the impact, I don't expect any build failures, as the compiler gets strictly more tolerant: shortening the lifetime of temporaries only expands the list of programs the compiler accepts as valid.

This is not quite right, for example this PR would break the following program.

struct Thing;

impl Drop for Thing {
    fn drop(&mut self) {}
}

struct Ref<'a>(&'a Thing);

impl<'a> Drop for Ref<'a> {
    fn drop(&mut self) {}
}

fn main() {
    let tmp;
    let _ = match Thing {
        ref thing => {
            tmp = thing;
            true
        }
    } && match Ref(tmp) {
        _ => false,
    };
}

It seems weird but it's not that much weirder than things like https://github.com/dtolnay/syn/blob/1.0.102/src/data.rs#L289-L293, so it's hard for me to gauge how widespread this might be.

est31 · 2022-10-20T11:21:29Z

Good point, I confirm this fails with the PR applied. Text edited.

joshtriplett · 2022-10-20T11:58:48Z

I think this is worth a crater run, and worth attempting even if we need to fix a couple of crates; this is a substantial improvement to understandability.

With this change, will that also make let chains able to use a consistent order?

est31 · 2022-10-20T12:25:21Z

@joshtriplett indeed my investigation of how let chains work has motivated this change. let chains are affected by this twist, yes: if there is a bool condition at the very start of a let chain, it will be dropped last (but if there is any number of let conditions before, this twist doesn't activate, so it's not just "first bool condition comes last"). After this PR, the drop order rule for let chains will be simple: first bool conditions are dropped in reading order, then comes the body, then let conditions in reverse order (as the body might rely on them, as well as the let conditions on each other, so reverse order is the only one you can really do here).

The other behaviour that from my POV should be improved is the drop order of let temporaries regarding the else block. You can either choose consistency with if let and match here, or on the other side, with if and let else (edit: see #103248 as the issue). Personally I'd prefer the latter two as it frees up resources earlier, but feel free to disagree (it might be surprising to people that adding/removing a bool conditional changes what the still-existing let bindings are doing regarding the else drop order).

The entire topic of what a good drop order would be would benefit from lang team input. I've made a longer description of behaviour and how I would like it to change here. I've edited the comment to reflect recent changes so it should be up to date.

est31 · 2022-10-20T18:47:04Z

@bors try

bors · 2022-10-20T18:47:13Z

⌛ Trying commit 0e70267b50790094287a0d6711fab2849b9fe948 with merge 51ad93974f526eaf4ede64932870490e6c96e4fd...

bors · 2022-10-20T20:46:06Z

☀️ Try build successful - checks-actions
Build commit: 51ad93974f526eaf4ede64932870490e6c96e4fd (51ad93974f526eaf4ede64932870490e6c96e4fd)

Dylan-DPC · 2022-10-21T07:27:45Z

@craterbot check

craterbot · 2022-10-21T07:27:49Z

👌 Experiment pr-103293 created and queued.
🤖 Automatically detected try build 51ad93974f526eaf4ede64932870490e6c96e4fd
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

craterbot · 2022-10-21T07:28:06Z

🚧 Experiment pr-103293 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

craterbot · 2022-10-22T06:01:00Z

🎉 Experiment pr-103293 is completed!
📊 76 regressed and 9 fixed (246085 total)
📰 Open the full report.

⚠️ If you notice any spurious failure please add them to the blacklist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

est31 · 2022-10-23T20:43:46Z

So it seems that there is one build failure due to the drop order changes. It occurs in a match guard in spaad_internal-0.4.0.

Relevant log excerpt:

[INFO] [stdout] error[E0716]: temporary value dropped while borrowed
[INFO] [stdout]    --> src/entangle/transform.rs:459:51
[INFO] [stdout]     |
[INFO] [stdout] 459 |         ReturnType::Type(_, ty) if !(is_name(ty, &name.to_string()) || is_name(ty, "Self"))
[INFO] [stdout]     |                                                   ^^^^^^^^^^^^^^^^-    ------- borrow later used here
[INFO] [stdout]     |                                                   |               |
[INFO] [stdout]     |                                                   |               temporary value is freed at the end of this statement
[INFO] [stdout]     |                                                   creates a temporary which is freed while still in use
[INFO] [stdout]     |
[INFO] [stdout] help: consider using a `let` binding to create a longer lived value
[INFO] [stdout]     |
[INFO] [stdout] 457 ~     let binding = name.to_string();
[INFO] [stdout] 458 ~     if matches!(
[INFO] [stdout] 459 |         &sig.output,
[INFO] [stdout] 460 ~         ReturnType::Type(_, ty) if !(is_name(ty, &binding) || is_name(ty, "Self"))
[INFO] [stdout]     |

This is the relevant line on github.

I don't really understand how this change can cause the build failure, as name is a const reference. All the other involved variables are references too. I don't get why the borrow should later be used. It seems like a bug in rustc to me, that I've missed something when writing my PR. It might be interesting to create a minimum reproducing example.

For the other failures, most of them are either in proc macros or build.rs failures. I need to investigate more closely, but a lot of crate failures are due to libc's build.rs failing:

[INFO] [stderr] error: failed to run custom build command for `libc v0.2.93`
[INFO] [stderr] 
[INFO] [stderr] Caused by:
[INFO] [stderr]   process didn't exit successfully: `/opt/rustwide/target/debug/build/libc-249f7f643d38f60c/build-script-build` (exit status: 101)
[INFO] [stderr]   --- stdout
[INFO] [stderr]   cargo:rerun-if-changed=build.rs
[INFO] [stderr] 
[INFO] [stderr]   --- stderr
[INFO] [stderr]   thread 'main' panicked at 'Failed to get rustc version', /opt/rustwide/cargo-home/registry/src/github.com-1ecc6299db9ec823/libc-0.2.93/build.rs:9:63

The code in libc doesn't use any && or || except for one place where it uses ||.

I'm not sure what is going on here. I think though that many more crates depend on libc than the 44 failed crates detected by crater, so it can't be a general build failure of libc.

est31 · 2022-10-24T21:03:36Z

So a minimum reproducing example is

fn main() {
    fn is_empty(s: &str) -> bool {
        s.is_empty()
    }
    // Doesn't fail if provided with this
    //let check_val: fn(&str) -> bool = |v| is_empty(v);
    let check_val = |v| is_empty(v);
    let n = "Hi";
    if check_val(&n.to_string()) || check_val("Hello") {}
}

error[E0716]: temporary value dropped while borrowed
  --> file.rs:10:19
   |
LL |     if check_val(&n.to_string()) || check_val("Hello") {}
   |                   ^^^^^^^^^^^^^-    --------- borrow later used here
   |                   |            |
   |                   |            temporary value is freed at the end of this statement
   |                   creates a temporary which is freed while still in use
   |
   = note: consider using a `let` binding to create a longer lived value

And yes, on current Rust it fails if you add a false || to this chain, or move the two checks out like:

check_val(&n.to_string());
check_val("Hello");

There seems to be something going on in the auto-generation of check_val's signature that makes it fail: if you provide the signature for check_val yourself then it won't issue the error. It will issue the error if you specify it as fn(_) -> bool but it won't issue it if you specify it as fn(&_) -> bool. Something really interesting is going on here, maybe even a bug, but it's not a bug of my PR.

The other errors I still need to investigate.

est31 · 2022-11-29T05:06:48Z

r? @RalfJung

RalfJung · 2022-11-29T07:07:58Z

This is not code I am familiar with, I cannot review this. r? compiler

nagisa · 2022-12-03T14:27:30Z

@nikomatsakis said:

rfcbot reviewed

The crater run doesn't give evidence that we aren't changing runtime semantics, but it seems equally likely to me that this change fixes latent bugs as causes them.

For some reason your checkbox is unchecked, and seems like the rfcbot never actually checked it. A bug in rfcbot?

nagisa

This PR is changing the behaviour of mixed && and || chains as well. Please adjust a test to exercise those scenarios as well.

Especially keep in mind the fact that operator precedence for && and || is different, so a && b || c && d has a different evaluation order compared to e.g. a && (b || c) && d. I believe with this change the difference cannot be observed either way, though.

nagisa · 2022-12-03T14:32:54Z

src/test/ui/drop/issue-103107.rs

@@ -0,0 +1,37 @@
+// run-pass


For this test to make sense as a run-pass test, I think it would need to assert something at the runtime. However it doesn't and we already have a test that does that (src/test/ui/drop/drop_order.rs), so I think this should be changed to a check-pass test.

nagisa · 2022-12-03T14:47:05Z

src/test/ui/drop/drop_order.rs

+        let _ = self.option_loud_drop(7).is_none() // 1
+            || self.option_loud_drop(8).is_none() // 2
+            || self.option_loud_drop(9).is_none(); // 3
+        self.print(10); // 4


What is the purpose of this print? Is it testing that the operands to the expression above are dropped before the next statement? Could you add a comment to that effect?

Is it testing that the operands to the expression above are dropped before the next statement?

Yes. I can add a comment that explains this, but note that printing in the body is done with tests for other constructs in the file too.

nagisa · 2022-12-03T14:57:16Z

compiler/rustc_hir_analysis/src/check/region.rs

+                // If the LHS is not another binop itself of the same kind as ours,
+                // we also mark it as terminating, so that in && or || chains,
+                // the temporaries are dropped in order instead of the very first
+                // being dropped last. For the Let exception, see below.


Style nit: it is easier to understand the compound comment if it is split and placed directly above the relevant branches.

This applies to the pre-existing comment at the top of this body as well.

nagisa · 2022-12-03T15:14:15Z

compiler/rustc_hir_analysis/src/check/region.rs

                ref r,
            ) => {
                // For shortcircuiting operators, mark the RHS as a terminating
                // scope since it only executes conditionally.

+                // If the LHS is not another binop itself of the same kind as ours,


Perhaps

Suggested change

// If the LHS is not another binop itself of the same kind as ours,

// If the LHS is not another binop itself of the same kind as the current expression,

FWIW there are a couple of implied contexts that aren’t necessarily immediately obvious. For example, why is it that only the LHS matters, and not RHS (because LAnd/LOr binary ops are left-associative.) The double negative makes this somewhat difficult to understand as well.

Something like the following might be easier to grok, perhaps?

We want to make sure that operands in expressions like a && b && c and a || b || c are dropped in order. These operators are left-associative, so in order to achieve the desired behaviour we don’t want to terminate the LHS expression, if LHS is the same kind of binary operation as the current expression.

At least that’s what I understand this code to be doing.

Previously a short circuiting && chain would drop the first element after all the other elements, and otherwise follow evaluation order, so code like: f(1).g() && f(2).g() && f(3).g() && f(4).g() would drop the temporaries in the order 2,3,4,1. This made && and || non-associative regarding drop order, so adding ()'s to the expression would change drop order: f(1).g() && (f(2).g() && f(3).g()) && f(4).g() for example would drop in the order 3,2,4,1. As, except for the bool result, there is no data returned by the sub-expressions of the short circuiting binops, we can safely discard of any temporaries created by the sub-expr. Previously, code was already putting the rhs's into terminating scopes, but missed it for the lhs's. This commit addresses this "twist". In the expression, we now also put the lhs into a terminating scope. The drop order for the above expressions is 1,2,3,4 now.

This avoids creation of a terminating scope in chains that contain both && and ||, because also there we know that a terminating scope is not neccessary: all the chain members are already in such terminating scopes. Also add a mixed && / || test.

nagisa · 2022-12-04T12:06:55Z

@bors r+

bors · 2022-12-04T12:06:57Z

📌 Commit a59a2d3 has been approved by nagisa

It is now in the queue for this repository.

bors · 2022-12-04T14:03:16Z

⌛ Testing commit a59a2d3 with merge 19c250a...

bors · 2022-12-04T16:32:15Z

☀️ Test successful - checks-actions
Approved by: nagisa
Pushing 19c250a to master...

rust-timer · 2022-12-04T17:50:34Z

Finished benchmarking commit (19c250a): 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)

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

Cycles

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

Remove drop order twist of && and || and make them associative Previously a short circuiting binop chain (chain of && or ||s) would drop the temporaries created by the first element after all the other elements, and otherwise follow evaluation order. So `f(1).g() && f(2).g() && f(3).g() && f(4).g()` would drop the temporaries in the order `2,3,4,1`. This made `&&` and `||` non-associative regarding drop order. In other words, adding ()'s to the expression would change drop order: `f(1).g() && (f(2).g() && f(3).g()) && f(4).g()` for example would drop in the order `3,2,4,1`. As, except for the bool result, there is no data returned by the sub-expressions of the short circuiting binops, we can safely discard of any temporaries created by the sub-expr. Previously, code was already putting the rhs's into terminating scopes, but missed it for the lhs's. This commit addresses this "twist". We now also put the lhs into a terminating scope. The drop order of the above expressions becomes `1,2,3,4`. There might be code relying on the current order, and therefore I'd recommend doing a crater run to gauge the impact. I'd argue that such code is already quite wonky as it is one `foo() &&` addition away from breaking. ~~For the impact, I don't expect any *build* failures, as the compiler gets strictly more tolerant: shortening the lifetime of temporaries only expands the list of programs the compiler accepts as valid. There might be *runtime* failures caused by this change however.~~ Edit: both build and runtime failures are possible, e.g. see the example provided by dtolnay [below](rust-lang/rust#103293 (comment)). Edit2: the crater run has finished and [results](rust-lang/rust#103293 (comment)) are that there is only one build failure which is easy to fix with a +/- 1 line diff. I've included a testcase that now compiles thanks to this patch. The breakage is also limited to drop order relative to conditionals in the && chain: that is, in code like this: ```Rust let hello = foo().hi() && bar().world(); println!("hi"); ``` we already drop the temporaries of `foo().hi()` before we reach "hi". I'd ideally have this PR merged before let chains are stabilized. If this PR is taking too long, I'd love to have a more restricted version of this change limited to `&&`'s in let chains: the `&&`'s of such chains are quite special anyways as they accept `let` bindings, in there the `&&` is therefore more a part of the "if let chain" construct than a construct of its own. Fixes #103107 Status: waiting on [this accepted FCP](rust-lang/rust#103293 (comment)) finishing.

Pkgsrc changes: * Adjust patches and cargo checksums to new versions, but also one strange "mips" conditional. Upstream changes: Version 1.67.0 (2023-01-26) ========================== Language -------- - [Make `Sized` predicates coinductive, allowing cycles.] (rust-lang/rust#100386) - [`#[must_use]` annotations on `async fn` also affect the `Future::Output`.] (rust-lang/rust#100633) - [Elaborate supertrait obligations when deducing closure signatures.] (rust-lang/rust#101834) - [Invalid literals are no longer an error under `cfg(FALSE)`.] (rust-lang/rust#102944) - [Unreserve braced enum variants in value namespace.] (rust-lang/rust#103578) Compiler -------- - [Enable varargs support for calling conventions other than `C` or `cdecl`.] (rust-lang/rust#97971) - [Add new MIR constant propagation based on dataflow analysis.] (rust-lang/rust#101168) - [Optimize field ordering by grouping m\*2^n-sized fields with equivalently aligned ones.] (rust-lang/rust#102750) - [Stabilize native library modifier `verbatim`.] (rust-lang/rust#104360) Added and removed targets: - [Add a tier 3 target for PowerPC on AIX] (rust-lang/rust#102293), `powerpc64-ibm-aix`. - [Add a tier 3 target for the Sony PlayStation 1] (rust-lang/rust#102689), `mipsel-sony-psx`. - [Add tier 3 `no_std` targets for the QNX Neutrino RTOS] (rust-lang/rust#102701), `aarch64-unknown-nto-qnx710` and `x86_64-pc-nto-qnx710`. - [Remove tier 3 `linuxkernel` targets] (rust-lang/rust#104015) (not used by the actual kernel). Refer to Rust's [platform support page][platform-support-doc] for more information on Rust's tiered platform support. Libraries --------- - [Merge `crossbeam-channel` into `std::sync::mpsc`.] (rust-lang/rust#93563) - [Fix inconsistent rounding of 0.5 when formatted to 0 decimal places.] (rust-lang/rust#102935) - [Derive `Eq` and `Hash` for `ControlFlow`.] (rust-lang/rust#103084) - [Don't build `compiler_builtins` with `-C panic=abort`.] (rust-lang/rust#103786) Stabilized APIs --------------- - [`{integer}::checked_ilog`] (https://doc.rust-lang.org/stable/std/primitive.i32.html#method.checked_ilog) - [`{integer}::checked_ilog2`] (https://doc.rust-lang.org/stable/std/primitive.i32.html#method.checked_ilog2) - [`{integer}::checked_ilog10`] (https://doc.rust-lang.org/stable/std/primitive.i32.html#method.checked_ilog10) - [`{integer}::ilog`] (https://doc.rust-lang.org/stable/std/primitive.i32.html#method.ilog) - [`{integer}::ilog2`] (https://doc.rust-lang.org/stable/std/primitive.i32.html#method.ilog2) - [`{integer}::ilog10`] (https://doc.rust-lang.org/stable/std/primitive.i32.html#method.ilog10) - [`NonZeroU*::ilog2`] (https://doc.rust-lang.org/stable/std/num/struct.NonZeroU32.html#method.ilog2) - [`NonZeroU*::ilog10`] (https://doc.rust-lang.org/stable/std/num/struct.NonZeroU32.html#method.ilog10) - [`NonZero*::BITS`] (https://doc.rust-lang.org/stable/std/num/struct.NonZeroU32.html#associatedconstant.BITS) These APIs are now stable in const contexts: - [`char::from_u32`] (https://doc.rust-lang.org/stable/std/primitive.char.html#method.from_u32) - [`char::from_digit`] (https://doc.rust-lang.org/stable/std/primitive.char.html#method.from_digit) - [`char::to_digit`] (https://doc.rust-lang.org/stable/std/primitive.char.html#method.to_digit) - [`core::char::from_u32`] (https://doc.rust-lang.org/stable/core/char/fn.from_u32.html) - [`core::char::from_digit`] (https://doc.rust-lang.org/stable/core/char/fn.from_digit.html) Compatibility Notes ------------------- - [The layout of `repr(Rust)` types now groups m\*2^n-sized fields with equivalently aligned ones.] (rust-lang/rust#102750) This is intended to be an optimization, but it is also known to increase type sizes in a few cases for the placement of enum tags. As a reminder, the layout of `repr(Rust)` types is an implementation detail, subject to change. - [0.5 now rounds to 0 when formatted to 0 decimal places.] (rust-lang/rust#102935) This makes it consistent with the rest of floating point formatting that rounds ties toward even digits. - [Chains of `&&` and `||` will now drop temporaries from their sub-expressions in evaluation order, left-to-right.] (rust-lang/rust#103293) Previously, it was "twisted" such that the _first_ expression dropped its temporaries _last_, after all of the other expressions dropped in order. - [Underscore suffixes on string literals are now a hard error.] (rust-lang/rust#103914) This has been a future-compatibility warning since 1.20.0. - [Stop passing `-export-dynamic` to `wasm-ld`.] (rust-lang/rust#105405) - [`main` is now mangled as `__main_void` on `wasm32-wasi`.] (rust-lang/rust#105468) - [Cargo now emits an error if there are multiple registries in the configuration with the same index URL.] (rust-lang/cargo#10592) Internal Changes ---------------- These changes do not affect any public interfaces of Rust, but they represent significant improvements to the performance or internals of rustc and related tools. - [Rewrite LLVM's archive writer in Rust.] (rust-lang/rust#97485)

Pkgsrc changes: * Adjust patches (add & remove) and cargo checksums to new versions. * It's conceivable that the workaround for LLVM based NetBSD works even less in this version (ref. PKGSRC_HAVE_LIBCPP not having a corresponding patch anymore). Upstream changes: Version 1.68.2 (2023-03-28) =========================== - [Update the GitHub RSA host key bundled within Cargo] (rust-lang/cargo#11883). The key was [rotated by GitHub] (https://github.blog/2023-03-23-we-updated-our-rsa-ssh-host-key/) on 2023-03-24 after the old one leaked. - [Mark the old GitHub RSA host key as revoked] (rust-lang/cargo#11889). This will prevent Cargo from accepting the leaked key even when trusted by the system. - [Add support for `@revoked` and a better error message for `@cert-authority` in Cargo's SSH host key verification] (rust-lang/cargo#11635) Version 1.68.1 (2023-03-23) =========================== - [Fix miscompilation in produced Windows MSVC artifacts] (rust-lang/rust#109094) This was introduced by enabling ThinLTO for the distributed rustc which led to miscompilations in the resulting binary. Currently this is believed to be limited to the -Zdylib-lto flag used for rustc compilation, rather than a general bug in ThinLTO, so only rustc artifacts should be affected. - [Fix --enable-local-rust builds] (rust-lang/rust#109111) - [Treat `$prefix-clang` as `clang` in linker detection code] (rust-lang/rust#109156) - [Fix panic in compiler code] (rust-lang/rust#108162) Version 1.68.0 (2023-03-09) =========================== Language -------- - [Stabilize default_alloc_error_handler] (rust-lang/rust#102318) This allows usage of `alloc` on stable without requiring the definition of a handler for allocation failure. Defining custom handlers is still unstable. - [Stabilize `efiapi` calling convention.] (rust-lang/rust#105795) - [Remove implicit promotion for types with drop glue] (rust-lang/rust#105085) Compiler -------- - [Change `bindings_with_variant_name` to deny-by-default] (rust-lang/rust#104154) - [Allow .. to be parsed as let initializer] (rust-lang/rust#105701) - [Add `armv7-sony-vita-newlibeabihf` as a tier 3 target] (rust-lang/rust#105712) - [Always check alignment during compile-time const evaluation] (rust-lang/rust#104616) - [Disable "split dwarf inlining" by default.] (rust-lang/rust#106709) - [Add vendor to Fuchsia's target triple] (rust-lang/rust#106429) - [Enable sanitizers for s390x-linux] (rust-lang/rust#107127) Libraries --------- - [Loosen the bound on the Debug implementation of Weak.] (rust-lang/rust#90291) - [Make `std::task::Context` !Send and !Sync] (rust-lang/rust#95985) - [PhantomData layout guarantees] (rust-lang/rust#104081) - [Don't derive Debug for `OnceWith` & `RepeatWith`] (rust-lang/rust#104163) - [Implement DerefMut for PathBuf] (rust-lang/rust#105018) - [Add O(1) `Vec -> VecDeque` conversion guarantee] (rust-lang/rust#105128) - [Leak amplification for peek_mut() to ensure BinaryHeap's invariant is always met] (rust-lang/rust#105851) Stabilized APIs --------------- - [`{core,std}::pin::pin!`] (https://doc.rust-lang.org/stable/std/pin/macro.pin.html) - [`impl From<bool> for {f32,f64}`] (https://doc.rust-lang.org/stable/std/primitive.f32.html#impl-From%3Cbool%3E-for-f32) - [`std::path::MAIN_SEPARATOR_STR`] (https://doc.rust-lang.org/stable/std/path/constant.MAIN_SEPARATOR_STR.html) - [`impl DerefMut for PathBuf`] (https://doc.rust-lang.org/stable/std/path/struct.PathBuf.html#impl-DerefMut-for-PathBuf) These APIs are now stable in const contexts: - [`VecDeque::new`] (https://doc.rust-lang.org/stable/std/collections/struct.VecDeque.html#method.new) Cargo ----- - [Stabilize sparse registry support for crates.io] (rust-lang/cargo#11224) - [`cargo build --verbose` tells you more about why it recompiles.] (rust-lang/cargo#11407) - [Show progress of crates.io index update even `net.git-fetch-with-cli` option enabled] (rust-lang/cargo#11579) Misc ---- Compatibility Notes ------------------- - [Add `SEMICOLON_IN_EXPRESSIONS_FROM_MACROS` to future-incompat report] (rust-lang/rust#103418) - [Only specify `--target` by default for `-Zgcc-ld=lld` on wasm] (rust-lang/rust#101792) - [Bump `IMPLIED_BOUNDS_ENTAILMENT` to Deny + ReportNow] (rust-lang/rust#106465) - [`std::task::Context` no longer implements Send and Sync] (rust-lang/rust#95985) nternal Changes ---------------- These changes do not affect any public interfaces of Rust, but they represent significant improvements to the performance or internals of rustc and related tools. - [Encode spans relative to the enclosing item] (rust-lang/rust#84762) - [Don't normalize in AstConv] (rust-lang/rust#101947) - [Find the right lower bound region in the scenario of partial order relations] (rust-lang/rust#104765) - [Fix impl block in const expr] (rust-lang/rust#104889) - [Check ADT fields for copy implementations considering regions] (rust-lang/rust#105102) - [rustdoc: simplify JS search routine by not messing with lev distance] (rust-lang/rust#105796) - [Enable ThinLTO for rustc on `x86_64-pc-windows-msvc`] (rust-lang/rust#103591) - [Enable ThinLTO for rustc on `x86_64-apple-darwin`] (rust-lang/rust#103647) Version 1.67.0 (2023-01-26) ========================== Language -------- - [Make `Sized` predicates coinductive, allowing cycles.] (rust-lang/rust#100386) - [`#[must_use]` annotations on `async fn` also affect the `Future::Output`.] (rust-lang/rust#100633) - [Elaborate supertrait obligations when deducing closure signatures.] (rust-lang/rust#101834) - [Invalid literals are no longer an error under `cfg(FALSE)`.] (rust-lang/rust#102944) - [Unreserve braced enum variants in value namespace.] (rust-lang/rust#103578) Compiler -------- - [Enable varargs support for calling conventions other than `C` or `cdecl`.] (rust-lang/rust#97971) - [Add new MIR constant propagation based on dataflow analysis.] (rust-lang/rust#101168) - [Optimize field ordering by grouping m\*2^n-sized fields with equivalently aligned ones.] (rust-lang/rust#102750) - [Stabilize native library modifier `verbatim`.] (rust-lang/rust#104360) Added and removed targets: - [Add a tier 3 target for PowerPC on AIX] (rust-lang/rust#102293), `powerpc64-ibm-aix`. - [Add a tier 3 target for the Sony PlayStation 1] (rust-lang/rust#102689), `mipsel-sony-psx`. - [Add tier 3 `no_std` targets for the QNX Neutrino RTOS] (rust-lang/rust#102701), `aarch64-unknown-nto-qnx710` and `x86_64-pc-nto-qnx710`. - [Remove tier 3 `linuxkernel` targets] (rust-lang/rust#104015) (not used by the actual kernel). Refer to Rust's [platform support page][platform-support-doc] for more information on Rust's tiered platform support. Libraries --------- - [Merge `crossbeam-channel` into `std::sync::mpsc`.] (rust-lang/rust#93563) - [Fix inconsistent rounding of 0.5 when formatted to 0 decimal places.] (rust-lang/rust#102935) - [Derive `Eq` and `Hash` for `ControlFlow`.] (rust-lang/rust#103084) - [Don't build `compiler_builtins` with `-C panic=abort`.] (rust-lang/rust#103786) Stabilized APIs --------------- - [`{integer}::checked_ilog`] (https://doc.rust-lang.org/stable/std/primitive.i32.html#method.checked_ilog) - [`{integer}::checked_ilog2`] (https://doc.rust-lang.org/stable/std/primitive.i32.html#method.checked_ilog2) - [`{integer}::checked_ilog10`] (https://doc.rust-lang.org/stable/std/primitive.i32.html#method.checked_ilog10) - [`{integer}::ilog`] (https://doc.rust-lang.org/stable/std/primitive.i32.html#method.ilog) - [`{integer}::ilog2`] (https://doc.rust-lang.org/stable/std/primitive.i32.html#method.ilog2) - [`{integer}::ilog10`] (https://doc.rust-lang.org/stable/std/primitive.i32.html#method.ilog10) - [`NonZeroU*::ilog2`] (https://doc.rust-lang.org/stable/std/num/struct.NonZeroU32.html#method.ilog2) - [`NonZeroU*::ilog10`] (https://doc.rust-lang.org/stable/std/num/struct.NonZeroU32.html#method.ilog10) - [`NonZero*::BITS`] (https://doc.rust-lang.org/stable/std/num/struct.NonZeroU32.html#associatedconstant.BITS) These APIs are now stable in const contexts: - [`char::from_u32`] (https://doc.rust-lang.org/stable/std/primitive.char.html#method.from_u32) - [`char::from_digit`] (https://doc.rust-lang.org/stable/std/primitive.char.html#method.from_digit) - [`char::to_digit`] (https://doc.rust-lang.org/stable/std/primitive.char.html#method.to_digit) - [`core::char::from_u32`] (https://doc.rust-lang.org/stable/core/char/fn.from_u32.html) - [`core::char::from_digit`] (https://doc.rust-lang.org/stable/core/char/fn.from_digit.html) Compatibility Notes ------------------- - [The layout of `repr(Rust)` types now groups m\*2^n-sized fields with equivalently aligned ones.] (rust-lang/rust#102750) This is intended to be an optimization, but it is also known to increase type sizes in a few cases for the placement of enum tags. As a reminder, the layout of `repr(Rust)` types is an implementation detail, subject to change. - [0.5 now rounds to 0 when formatted to 0 decimal places.] (rust-lang/rust#102935) This makes it consistent with the rest of floating point formatting that rounds ties toward even digits. - [Chains of `&&` and `||` will now drop temporaries from their sub-expressions in evaluation order, left-to-right.] (rust-lang/rust#103293) Previously, it was "twisted" such that the _first_ expression dropped its temporaries _last_, after all of the other expressions dropped in order. - [Underscore suffixes on string literals are now a hard error.] (rust-lang/rust#103914) This has been a future-compatibility warning since 1.20.0. - [Stop passing `-export-dynamic` to `wasm-ld`.] (rust-lang/rust#105405) - [`main` is now mangled as `__main_void` on `wasm32-wasi`.] (rust-lang/rust#105468) - [Cargo now emits an error if there are multiple registries in the configuration with the same index URL.] (rust-lang/cargo#10592) Internal Changes ---------------- These changes do not affect any public interfaces of Rust, but they represent significant improvements to the performance or internals of rustc and related tools. - [Rewrite LLVM's archive writer in Rust.] (rust-lang/rust#97485)

rust-highfive assigned wesleywiser Oct 20, 2022

rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 20, 2022

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 20, 2022

est31 force-pushed the untwist_and_drop_order branch from a602b97 to 6172afd Compare October 20, 2022 08:53

This comment has been minimized.

Sign in to view

est31 force-pushed the untwist_and_drop_order branch 4 times, most recently from 2ea77b0 to 0e70267 Compare October 20, 2022 09:52

est31 mentioned this pull request Oct 20, 2022

let_chains desugaring is wrong #100513

Closed

craterbot added S-waiting-on-crater Status: Waiting on a crater run to be completed. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 21, 2022

craterbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-crater Status: Waiting on a crater run to be completed. labels Oct 22, 2022

rustbot assigned RalfJung and unassigned wesleywiser Nov 29, 2022

rustbot assigned nagisa and unassigned RalfJung Nov 29, 2022

nagisa reviewed Dec 3, 2022

View reviewed changes

est31 force-pushed the untwist_and_drop_order branch from 0e70267 to 604af02 Compare December 4, 2022 00:10

est31 added 2 commits December 4, 2022 04:09

Improve comments

a2076dc

est31 force-pushed the untwist_and_drop_order branch from 604af02 to a59a2d3 Compare December 4, 2022 03:10

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 Dec 4, 2022

bors added the merged-by-bors This PR was explicitly merged by bors. label Dec 4, 2022

bors merged commit 19c250a into rust-lang:master Dec 4, 2022

rustbot added this to the 1.67.0 milestone Dec 4, 2022

zachs18 mentioned this pull request Dec 19, 2022

"a temporary value which is freed while still in use" in nightly but not on stable #105891

Open

apiraino removed the to-announce Announce this issue on triage meeting label Jan 5, 2023

est31 mentioned this pull request Jan 30, 2023

add another exception rust-lang/crater#674

Merged

est31 mentioned this pull request Jun 18, 2023

Rescope temp lifetime in let-chain into IfElse #107251

Open

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove drop order twist of && and || and make them associative #103293

Remove drop order twist of && and || and make them associative #103293

est31 commented Oct 20, 2022 •

edited

rust-highfive commented Oct 20, 2022

This comment has been minimized.

dtolnay commented Oct 20, 2022

est31 commented Oct 20, 2022

joshtriplett commented Oct 20, 2022

est31 commented Oct 20, 2022 •

edited

est31 commented Oct 20, 2022

bors commented Oct 20, 2022

bors commented Oct 20, 2022

Dylan-DPC commented Oct 21, 2022

craterbot commented Oct 21, 2022

craterbot commented Oct 21, 2022

craterbot commented Oct 22, 2022

est31 commented Oct 23, 2022

est31 commented Oct 24, 2022

est31 commented Nov 29, 2022

RalfJung commented Nov 29, 2022 via email

nagisa commented Dec 3, 2022

nagisa left a comment •

edited

nagisa Dec 3, 2022

nagisa Dec 3, 2022

est31 Dec 3, 2022

nagisa Dec 3, 2022 •

edited

nagisa Dec 3, 2022 •

edited

nagisa Dec 3, 2022 •

edited

nagisa commented Dec 4, 2022

bors commented Dec 4, 2022

bors commented Dec 4, 2022

bors commented Dec 4, 2022

rust-timer commented Dec 4, 2022

	// If the LHS is not another binop itself of the same kind as ours,
	// If the LHS is not another binop itself of the same kind as the current expression,

Remove drop order twist of && and || and make them associative #103293

Remove drop order twist of && and || and make them associative #103293

Conversation

est31 commented Oct 20, 2022 • edited

rust-highfive commented Oct 20, 2022

This comment has been minimized.

dtolnay commented Oct 20, 2022

est31 commented Oct 20, 2022

joshtriplett commented Oct 20, 2022

est31 commented Oct 20, 2022 • edited

est31 commented Oct 20, 2022

bors commented Oct 20, 2022

bors commented Oct 20, 2022

Dylan-DPC commented Oct 21, 2022

craterbot commented Oct 21, 2022

craterbot commented Oct 21, 2022

craterbot commented Oct 22, 2022

est31 commented Oct 23, 2022

est31 commented Oct 24, 2022

est31 commented Nov 29, 2022

RalfJung commented Nov 29, 2022 via email

nagisa commented Dec 3, 2022

nagisa left a comment • edited

Choose a reason for hiding this comment

nagisa Dec 3, 2022

Choose a reason for hiding this comment

nagisa Dec 3, 2022

Choose a reason for hiding this comment

est31 Dec 3, 2022

Choose a reason for hiding this comment

nagisa Dec 3, 2022 • edited

Choose a reason for hiding this comment

nagisa Dec 3, 2022 • edited

Choose a reason for hiding this comment

nagisa Dec 3, 2022 • edited

Choose a reason for hiding this comment

nagisa commented Dec 4, 2022

bors commented Dec 4, 2022

bors commented Dec 4, 2022

bors commented Dec 4, 2022

rust-timer commented Dec 4, 2022

Overall result: no relevant changes - no action needed

Instruction count

Max RSS (memory usage)

Cycles

est31 commented Oct 20, 2022 •

edited

est31 commented Oct 20, 2022 •

edited

nagisa left a comment •

edited

nagisa Dec 3, 2022 •

edited

nagisa Dec 3, 2022 •

edited

nagisa Dec 3, 2022 •

edited