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 8 pull requests #92925

Closed
wants to merge 34 commits into from

Commits on Dec 21, 2021

  1. Configuration menu
    Copy the full SHA
    cca0aa9 View commit details
    Browse the repository at this point in the history

Commits on Dec 22, 2021

  1. Configuration menu
    Copy the full SHA
    e467840 View commit details
    Browse the repository at this point in the history
  2. Configuration menu
    Copy the full SHA
    50ac0a3 View commit details
    Browse the repository at this point in the history

Commits on Jan 7, 2022

  1. Implement panic::update_hook

    Badel2 committed Jan 7, 2022
    Configuration menu
    Copy the full SHA
    8bdf5c3 View commit details
    Browse the repository at this point in the history
  2. Change panic::update_hook to simplify usage

    And to remove possibility of panics while changing the panic handler,
    because that resulted in a double panic.
    Badel2 committed Jan 7, 2022
    Configuration menu
    Copy the full SHA
    8ef3ce8 View commit details
    Browse the repository at this point in the history
  3. Configuration menu
    Copy the full SHA
    0c58586 View commit details
    Browse the repository at this point in the history

Commits on Jan 9, 2022

  1. feat: pass_by_value lint attribute

    Useful for thin wrapper attributes that are best passed as value instead
    of reference.
    mdibaiee committed Jan 9, 2022
    Configuration menu
    Copy the full SHA
    4c3e330 View commit details
    Browse the repository at this point in the history

Commits on Jan 10, 2022

  1. Configuration menu
    Copy the full SHA
    ad57295 View commit details
    Browse the repository at this point in the history
  2. Configuration menu
    Copy the full SHA
    91ed689 View commit details
    Browse the repository at this point in the history
  3. Configuration menu
    Copy the full SHA
    71e3314 View commit details
    Browse the repository at this point in the history

Commits on Jan 11, 2022

  1. Remove hack that is no longer necessary

    This hack was added in 6ab1f05.
    I don't know what change allowed removing the hack, but that commit
    added a test (which I presume covered the hack's behavior), and all
    tests are passing with this change. So, I think it should be good.
    camelid committed Jan 11, 2022
    Configuration menu
    Copy the full SHA
    49553bb View commit details
    Browse the repository at this point in the history
  2. Configuration menu
    Copy the full SHA
    e18b23b View commit details
    Browse the repository at this point in the history
  3. Enable ignored part of test

    Inherent associated types *are* supported, just unstable.
    camelid committed Jan 11, 2022
    Configuration menu
    Copy the full SHA
    ca20d64 View commit details
    Browse the repository at this point in the history
  4. Add test for disambiguator mismatch with crate

    This currently calls `std` a "crate" in one part of the message and a
    "module" in another part. The next commits fix this so it says "crate"
    in both places.
    camelid committed Jan 11, 2022
    Configuration menu
    Copy the full SHA
    977a7ca View commit details
    Browse the repository at this point in the history
  5. Use Res instead of Disambiguator for resolved in report_mismatch

    This allows simplifying a lot of code. It also fixes a subtle bug,
    exemplified by the test output changes.
    camelid committed Jan 11, 2022
    Configuration menu
    Copy the full SHA
    9acd813 View commit details
    Browse the repository at this point in the history
  6. Remove unnecessary conditional for suggesting disambiguator

    Now that `res` is used directly, it seems the conditional is
    unnecessary.
    camelid committed Jan 11, 2022
    Configuration menu
    Copy the full SHA
    591ec49 View commit details
    Browse the repository at this point in the history
  7. Update comment and make code clearer

    I'm still not sure why this hack works so seemingly well.
    camelid committed Jan 11, 2022
    Configuration menu
    Copy the full SHA
    a5f09f7 View commit details
    Browse the repository at this point in the history
  8. Extract functions for two closures

    These closures were quite complex and part of a quite complex function.
    The fact that they are closures makes mistakes likely when refactoring.
    For example, earlier, I meant to use `resolved`, an argument of the
    closure, but I instead typed `res`, which captured a local variable and
    caused a subtle bug that led to a confusing test failure.
    
    Extracting them as functions makes the code easier to understand and
    refactor.
    camelid committed Jan 11, 2022
    Configuration menu
    Copy the full SHA
    895fa9c View commit details
    Browse the repository at this point in the history
  9. Configuration menu
    Copy the full SHA
    28d2353 View commit details
    Browse the repository at this point in the history
  10. rustc_pass_by_value: allow types with no parameters on self

    includes minor refactorings
    mdibaiee committed Jan 11, 2022
    Configuration menu
    Copy the full SHA
    a6762e9 View commit details
    Browse the repository at this point in the history
  11. Configuration menu
    Copy the full SHA
    959bf2b View commit details
    Browse the repository at this point in the history
  12. Configuration menu
    Copy the full SHA
    2728af7 View commit details
    Browse the repository at this point in the history

Commits on Jan 12, 2022

  1. Configuration menu
    Copy the full SHA
    c84f2b2 View commit details
    Browse the repository at this point in the history

Commits on Jan 13, 2022

  1. Configuration menu
    Copy the full SHA
    51d7665 View commit details
    Browse the repository at this point in the history
  2. Configuration menu
    Copy the full SHA
    9ff8ae0 View commit details
    Browse the repository at this point in the history
  3. Configuration menu
    Copy the full SHA
    ae20500 View commit details
    Browse the repository at this point in the history

Commits on Jan 15, 2022

  1. Rollup merge of rust-lang#92183 - tmandry:issue-74256, r=estebank

    Point at correct argument when async fn output type lifetime disagrees with signature
    
    Fixes most of rust-lang#74256.
    
    ## Problems fixed
    
    This PR fixes a couple of related problems in the error reporting code.
    
    ### Highlighting the wrong argument
    
    First, the error reporting code was looking at the desugared return type of an `async fn` to decide which parameter to highlight. For example, a function like
    
    ```rust
    async fn async_fn(self: &Struct, f: &u32) -> &u32
    { f }
    ```
    
    desugars to
    
    ```rust
    async fn async_fn<'a, 'b>(self: &'a Struct, f: &'b u32)
    -> impl Future<Output = &'a u32> + 'a + 'b
    { f }
    ```
    
    Since `f: &'b u32` is returned but the output type is `&'a u32`, the error would occur when checking that `'a: 'b`.
    
    The reporting code would look to see if the "offending" lifetime `'b` was included in the return type, and because the code was looking at the desugared future type, it was included. So it defaulted to reporting that the source of the other lifetime `'a` (the `self` type) was the problem, when it was really the type of `f`. (Note that if it had chosen instead to look at `'a` first, it too would have been included in the output type, and it would have arbitrarily reported the error (correctly this time) on the type of `f`.)
    
    Looking at the actual future type isn't useful for this reason; it captures all input lifetimes. Using the written return type for `async fn` solves this problem and results in less confusing error messages for the user.
    
    This isn't a perfect fix, unfortunately; writing the "manually desugared" form of the above function still results in the wrong parameter being highlighted. Looking at the output type of every `impl Future` return type doesn't feel like a very principled approach, though it might work. The problem would remain for function signatures that look like the desugared one above but use different traits. There may be deeper changes required to pinpoint which part of each type is conflicting.
    
    ### Lying about await point capture causing lifetime conflicts
    
    The second issue fixed by this PR is the unnecessary complexity in `try_report_anon_anon_conflict`. It turns out that the root cause I suggested in rust-lang#76547 (comment) wasn't really the root cause. Adding special handling to report that a variable was captured over an await point only made the error messages less correct and pointed to a problem other than the one that actually occurred.
    
    Given the above discussion, it's easy to see why: `async fn`s capture all input lifetimes in their return type, so holding an argument across an await point should never cause a lifetime conflict! Removing the special handling simplified the code and improved the error messages (though they still aren't very good!)
    
    ## Future work
    
    * Fix error reporting on the "desugared" form of this code
    * Get the `suggest_adding_lifetime_params` suggestion firing on these examples
      * cc rust-lang#42703, I think
    
    r? `@estebank`
    matthiaskrgr authored Jan 15, 2022
    Configuration menu
    Copy the full SHA
    8a93aef View commit details
    Browse the repository at this point in the history
  2. Rollup merge of rust-lang#92598 - Badel2:panic-update-hook, r=yaahc

    Implement `panic::update_hook`
    
    Add a new function `panic::update_hook` to allow creating panic hooks that forward the call to the previously set panic hook, without race conditions. It works by taking a closure that transforms the old panic hook into a new one, while ensuring that during the execution of the closure no other thread can modify the panic hook. This is a small function so I hope it can be discussed here without a formal RFC, however if you prefer I can write one.
    
    Consider the following example:
    
    ```rust
    let prev = panic::take_hook();
    panic::set_hook(Box::new(move |info| {
        println!("panic handler A");
        prev(info);
    }));
    ```
    
    This is a common pattern in libraries that need to do something in case of panic: log panic to a file, record code coverage, send panic message to a monitoring service, print custom message with link to github to open a new issue, etc. However it is impossible to avoid race conditions with the current API, because two threads can execute in this order:
    
    * Thread A calls `panic::take_hook()`
    * Thread B calls `panic::take_hook()`
    * Thread A calls `panic::set_hook()`
    * Thread B calls `panic::set_hook()`
    
    And the result is that the original panic hook has been lost, as well as the panic hook set by thread A. The resulting panic hook will be the one set by thread B, which forwards to the default panic hook. This is not considered a big issue because the panic handler setup is usually run during initialization code, probably before spawning any other threads.
    
    Using the new `panic::update_hook` function, this race condition is impossible, and the result will be either `A, B, original` or `B, A, original`.
    
    ```rust
    panic::update_hook(|prev| {
        Box::new(move |info| {
            println!("panic handler A");
            prev(info);
        })
    });
    ```
    
    I found one real world use case here: https://github.com/dtolnay/proc-macro2/blob/988cf403e741aadfd5340bbf67e35e1062a526aa/src/detection.rs#L32 the workaround is to detect the race condition and panic in that case.
    
    The pattern of `take_hook` + `set_hook` is very common, you can see some examples in this pull request, so I think it's natural to have a function that combines them both. Also using `update_hook` instead of `take_hook` + `set_hook` reduces the number of calls to `HOOK_LOCK.write()` from 2 to 1, but I don't expect this to make any difference in performance.
    
    ### Unresolved questions:
    
    * `panic::update_hook` takes a closure, if that closure panics the error message is "panicked while processing panic" which is not nice. This is a consequence of holding the `HOOK_LOCK` while executing the closure. Could be avoided using `catch_unwind`?
    
    * Reimplement `panic::set_hook` as `panic::update_hook(|_prev| hook)`?
    matthiaskrgr authored Jan 15, 2022
    Configuration menu
    Copy the full SHA
    3129fd7 View commit details
    Browse the repository at this point in the history
  3. Rollup merge of rust-lang#92635 - camelid:yet-more-cleanup, r=Manishe…

    …arth
    
    rustdoc: Yet more intra-doc links cleanup
    
    r? `@Manishearth`
    matthiaskrgr authored Jan 15, 2022
    Configuration menu
    Copy the full SHA
    29adca6 View commit details
    Browse the repository at this point in the history
  4. Rollup merge of rust-lang#92646 - mdibaiee:76935/pass-by-value, r=lcnr

    feat: rustc_pass_by_value lint attribute
    
    Useful for thin wrapper attributes that are best passed as value instead
    of reference.
    
    Fixes rust-lang#76935
    matthiaskrgr authored Jan 15, 2022
    Configuration menu
    Copy the full SHA
    3373a0e View commit details
    Browse the repository at this point in the history
  5. Rollup merge of rust-lang#92710 - jackh726:issue-92280, r=nikomatsakis

    Include Projections when elaborating TypeOutlives
    
    Fixes rust-lang#92280
    
    In `Elaborator`, we elaborate that `Foo<<Bar as Baz>::Assoc>: 'a` -> `<Bar as Baz>::Assoc: 'a`. This is the same rule that would be applied to any other `Param`. If there are escaping vars, we continue to do nothing.
    
    r? `@nikomatsakis`
    matthiaskrgr authored Jan 15, 2022
    Configuration menu
    Copy the full SHA
    f7e7dbb View commit details
    Browse the repository at this point in the history
  6. Rollup merge of rust-lang#92792 - mdibaiee:92662/fix-intra-doc-generi…

    …cs, r=camelid
    
    rustdoc: fix intra-link for generic trait impls
    
    fixes rust-lang#92662
    
    r? `@camelid`
    matthiaskrgr authored Jan 15, 2022
    Configuration menu
    Copy the full SHA
    e0c04b6 View commit details
    Browse the repository at this point in the history
  7. Rollup merge of rust-lang#92799 - rust-lang:followup-from-92533, r=Aa…

    …ron1011
    
    Remove some unnecessary uses of `FieldDef::ident`
    
    Followup from rust-lang#92533.
    
    cc `@Aaron1011` `@petrochenkov`
    matthiaskrgr authored Jan 15, 2022
    Configuration menu
    Copy the full SHA
    5f916fd View commit details
    Browse the repository at this point in the history
  8. Rollup merge of rust-lang#92819 - euclio:atty, r=CraftSpider

    rustdoc: remove hand-rolled isatty
    
    This PR replaces bindings to the platform-specific isatty APIs with the `isatty` crate, as done elsewhere in the repository.
    matthiaskrgr authored Jan 15, 2022
    Configuration menu
    Copy the full SHA
    19c88ad View commit details
    Browse the repository at this point in the history