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 #120290

Closed
wants to merge 21 commits into from
Closed

Rollup of 7 pull requests #120290

wants to merge 21 commits into from

Commits on May 12, 2023

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

Commits on Jan 10, 2024

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

Commits on Jan 18, 2024

  1. llvm: simplify data layout check

    Don't skip the inconsistent data layout check for custom LLVMs.
    
    With rust-lang#118708, all targets will have a simple test that would trigger this
    check if LLVM's data layouts do change - so data layouts would be
    corrected during the LLVM upgrade. Therefore, with builtin targets, this
    check won't trigger with our LLVM because each target will have been
    confirmed to work. With non-builtin targets, this check is probably
    useful to have because you can change the data layout in your target and
    if its wrong then that could lead to bugs.
    
    When using a custom LLVM, the same justification makes sense for
    non-builtin targets as with our LLVM, the user can update their target to
    match their LLVM and that's probably a good thing to do. However, with
    a custom LLVM, the user cannot change the builtin target data layouts if
    they don't match - though given that the compiler's data layout is used
    for layout computation and a bunch of other things - you could get some
    bugs because of the mismatch and probably want to know about that.
    
    `CFG_LLVM_ROOT` was also always set during local development with
    `download-ci-llvm` so this bug would never trigger locally.
    
    Signed-off-by: David Wood <[email protected]>
    davidtwco committed Jan 18, 2024
    Configuration menu
    Copy the full SHA
    46652dd View commit details
    Browse the repository at this point in the history

Commits on Jan 19, 2024

  1. Split assembly tests for ELF and MachO

    On ELF, the text section is opened with ".text", on MachO with
    ".section __TEXT,__text".
    
    Previously, on ELF this test was actually matching a GNU note
    section, which is no longer emitted on Solaris starting with
    LLVM 18.
    
    Fixes rust-lang#120105.
    nikic committed Jan 19, 2024
    Configuration menu
    Copy the full SHA
    c7a77b2 View commit details
    Browse the repository at this point in the history

Commits on Jan 20, 2024

  1. Configuration menu
    Copy the full SHA
    c852e3d View commit details
    Browse the repository at this point in the history
  2. Add NonZero symbol.

    reitermarkus committed Jan 20, 2024
    Configuration menu
    Copy the full SHA
    3b9022a View commit details
    Browse the repository at this point in the history
  3. Fix NonZero suggestions.

    reitermarkus committed Jan 20, 2024
    Configuration menu
    Copy the full SHA
    4700207 View commit details
    Browse the repository at this point in the history
  4. Update tests.

    reitermarkus committed Jan 20, 2024
    Configuration menu
    Copy the full SHA
    bd1b265 View commit details
    Browse the repository at this point in the history

Commits on Jan 22, 2024

  1. Configuration menu
    Copy the full SHA
    41dcba8 View commit details
    Browse the repository at this point in the history
  2. Configuration menu
    Copy the full SHA
    21e5bea View commit details
    Browse the repository at this point in the history

Commits on Jan 23, 2024

  1. Configuration menu
    Copy the full SHA
    823e8b0 View commit details
    Browse the repository at this point in the history
  2. Remove uses of no-system-llvm

    It looks like none of these are actually needed.
    nikic committed Jan 23, 2024
    Configuration menu
    Copy the full SHA
    31f5f03 View commit details
    Browse the repository at this point in the history
  3. Remove support for no-system-llvm

    Also add tests for min-system-llvm-version.
    nikic committed Jan 23, 2024
    Configuration menu
    Copy the full SHA
    f4f589a View commit details
    Browse the repository at this point in the history
  4. Remove extra # from url

    est31 committed Jan 23, 2024
    Configuration menu
    Copy the full SHA
    9676e18 View commit details
    Browse the repository at this point in the history

Commits on Jan 24, 2024

  1. Rollup merge of rust-lang#119460 - Zalathar:improper-region, r=wesley…

    …wiser
    
    coverage: Never emit improperly-ordered coverage regions
    
    If we emit a coverage region that is improperly ordered (end < start), `llvm-cov` will fail with `coveragemap_error::malformed`, which is inconvenient for users and also very hard to debug.
    
    Ideally we would fix the root causes of these situations, but they tend to occur in very obscure edge-case scenarios (often involving nested macros), and we don't always have a good MCVE to work from. So it makes sense to also have a catch-all check that will prevent improperly-ordered regions from ever being emitted.
    
    ---
    
    This is mainly aimed at resolving rust-lang#119453. We don't have a specific way to reproduce it, which is why I haven't been able to add a test case in this PR. But based on the information provided in that issue, this change seems likely to avoid the error in `llvm-cov`.
    
    ```@rustbot``` label +A-code-coverage
    fmease authored Jan 24, 2024
    Configuration menu
    Copy the full SHA
    00736d2 View commit details
    Browse the repository at this point in the history
  2. Rollup merge of rust-lang#120062 - davidtwco:llvm-data-layout-check, …

    …r=wesleywiser
    
    llvm: change data layout bug to an error and make it trigger more
    
    Fixes rust-lang#33446.
    
    Don't skip the inconsistent data layout check for custom LLVMs or non-built-in targets.
    
    With rust-lang#118708, all targets will have a simple test that would trigger this error if LLVM's data layouts do change - so data layouts would be corrected during the LLVM upgrade. Therefore, with builtin targets, this error won't happen with our LLVM because each target will have been confirmed to work. With non-builtin targets, this error is probably useful to have because you can change the data layout in your target and if it is wrong then that could lead to bugs.
    
    When using a custom LLVM, the same justification makes sense for non-builtin targets as with our LLVM, the user can update their target to match their LLVM and that's probably a good thing to do. However, with a custom LLVM, the user cannot change the builtin target data layouts if they don't match - though given that the compiler's data layout is used for layout computation and a bunch of other things - you could get some bugs because of the mismatch and probably want to know about that. I'm not sure if this is something that people do and is okay, but I doubt it?
    
    `CFG_LLVM_ROOT` was also always set during local development with `download-ci-llvm` so this bug would never trigger locally.
    
    In rust-lang#33446, two points are raised:
    
    - In the issue itself, changing this from a `bug!` to a proper error is what is suggested, by using `isCompatibleDataLayout` from LLVM, but that function still just does the same thing that we do and check for equality, so I've avoided the additional code necessary to do that FFI call.
    - ```@Mark-Simulacrum``` suggests a different check is necessary to maintain backwards compatibility with old LLVM versions. I don't know how often this comes up, but we can do that with some simple string manipulation + LLVM version checks as happens already for LLVM 17 just above this diff.
    fmease authored Jan 24, 2024
    Configuration menu
    Copy the full SHA
    6d95bdf View commit details
    Browse the repository at this point in the history
  3. Rollup merge of rust-lang#120124 - nikic:fix-assembly-test, r=davidtwco

    Split assembly tests for ELF and MachO
    
    On ELF, the text section is opened with ".text", on MachO with ".section __TEXT,__text".
    
    Previously, on ELF this test was actually matching a GNU note section, which is no longer emitted on Solaris starting with LLVM 18.
    
    Fixes rust-lang#120105.
    
    r? ```@davidtwco```
    fmease authored Jan 24, 2024
    Configuration menu
    Copy the full SHA
    7f80c2a View commit details
    Browse the repository at this point in the history
  4. Rollup merge of rust-lang#120165 - reitermarkus:nonzero-switch-alias-…

    …direction, r=dtolnay
    
    Switch `NonZero` alias direction.
    
    Step 4 mentioned in rust-lang#100428 (review).
    
    Depends on rust-lang#120160.
    
    r? `@dtolnay`
    fmease authored Jan 24, 2024
    Configuration menu
    Copy the full SHA
    f164fc5 View commit details
    Browse the repository at this point in the history
  5. Rollup merge of rust-lang#120185 - Zalathar:auto-derived, r=wesleywiser

    coverage: Don't instrument `#[automatically_derived]` functions
    
    This PR makes the coverage instrumentor detect and skip functions that have [`#[automatically_derived]`](https://doc.rust-lang.org/reference/attributes/derive.html#the-automatically_derived-attribute) on their enclosing impl block.
    
    Most notably, this means that methods generated by built-in derives (e.g. `Clone`, `Debug`, `PartialEq`) are now ignored by coverage instrumentation, and won't appear as executed or not-executed in coverage reports.
    
    This is a noticeable change in user-visible behaviour, but overall I think it's a net improvement. For example, we've had a few user requests for this sort of change (e.g. rust-lang#105055, rust-lang#84605 (comment)), and I believe it's the behaviour that most users will expect/prefer by default.
    
    It's possible to imagine situations where users would want to instrument these derived implementations, but I think it's OK to treat that as an opportunity to consider adding more fine-grained option flags to control the details of coverage instrumentation, while leaving this new behaviour as the default.
    
    (Also note that while `-Cinstrument-coverage` is a stable feature, the exact details of coverage instrumentation are allowed to change. So we *can* make this change; the main question is whether we *should*.)
    
    Fixes rust-lang#105055.
    fmease authored Jan 24, 2024
    Configuration menu
    Copy the full SHA
    1443be0 View commit details
    Browse the repository at this point in the history
  6. Rollup merge of rust-lang#120265 - nikic:no-no-system-llvm, r=nagisa

    Remove no-system-llvm
    
    We currently have a bunch of codegen tests that use no-system-llvm -- however, all of those tests also pass with system LLVM 16.
    
    I've opted to remove `no-system-llvm` entirely, as there's basically no valid use case for it anymore:
    
     * The only thing this option could have legitimately been used for (testing the target feature support that requires an LLVM patch) doesn't use it, and the need for this will go away with LLVM 18 anyway.
     * In cases where the test depends on optimizations/fixes from newer LLVM versions, `min-llvm-version` should be used instead.
     * In case it depends on optimization/fixes from newer LLVM versions that have been backported into our fork, `min-system-llvm-version` (with the major version larger than the one in our fork) should be used instead.
    
    r? ```@cuviper```
    fmease authored Jan 24, 2024
    Configuration menu
    Copy the full SHA
    0fcd5a1 View commit details
    Browse the repository at this point in the history
  7. Rollup merge of rust-lang#120285 - est31:remove_extra_pound, r=fmease

    Remove extra # from url in suggestion
    
    The suggestion added in rust-lang#119805 contains an unnecessary # hash sign.
    fmease authored Jan 24, 2024
    Configuration menu
    Copy the full SHA
    1c10029 View commit details
    Browse the repository at this point in the history