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

Update Clippy, RLS, and rustfmt #72671

Merged
merged 97 commits into from
May 29, 2020
Merged

Update Clippy, RLS, and rustfmt #72671

merged 97 commits into from
May 29, 2020

Conversation

flip1995
Copy link
Member

@flip1995 flip1995 commented May 27, 2020

r? @Dylan-DPC

This makes Clippy test-pass again: 3089c3b

Otherwise this includes bugfixes and a few new lints.

Fixes #72231
Fixes #72232

flip1995 and others added 30 commits May 3, 2020 16:11
Co-authored-by: Phil Hansch <dev@phansch.net>
TupleStruct matches are checked for exhaustiveness
Stabilize fn-like proc macros in expression, pattern and statement positions

I.e. all the positions in which stable `macro_rules` macros are supported.

Depends on rust-lang#68716 ("Stabilize `Span::mixed_site`").

cc rust-lang#54727
cc rust-lang#54727 (comment)

Stabilization report: rust-lang#68717 (comment).
Implement new asm! syntax from RFC 2850

This PR implements the new `asm!` syntax proposed in rust-lang/rfcs#2850.

# Design

A large part of this PR revolves around taking an `asm!` macro invocation and plumbing it through all of the compiler layers down to LLVM codegen. Throughout the various stages, an `InlineAsm` generally consists of 3 components:

- The template string, which is stored as an array of `InlineAsmTemplatePiece`. Each piece represents either a literal or a placeholder for an operand (just like format strings).
```rust
pub enum InlineAsmTemplatePiece {
    String(String),
    Placeholder { operand_idx: usize, modifier: Option<char>, span: Span },
}
```

- The list of operands to the `asm!` (`in`, `[late]out`, `in[late]out`, `sym`, `const`). These are represented differently at each stage of lowering, but follow a common pattern:
  - `in`, `out` and `inout` all have an associated register class (`reg`) or explicit register (`"eax"`).
  - `inout` has 2 forms: one with a single expression that is both read from and written to, and one with two separate expressions for the input and output parts.
  - `out` and `inout` have a `late` flag (`lateout` / `inlateout`) to indicate that the register allocator is allowed to reuse an input register for this output.
  - `out` and the split variant of `inout` allow `_` to be specified for an output, which means that the output is discarded. This is used to allocate scratch registers for assembly code.
  - `sym` is a bit special since it only accepts a path expression, which must point to a `static` or a `fn`.

- The options set at the end of the `asm!` macro. The only one that is particularly of interest to rustc is `NORETURN` which makes `asm!` return `!` instead of `()`.
```rust
bitflags::bitflags! {
    pub struct InlineAsmOptions: u8 {
        const PURE = 1 << 0;
        const NOMEM = 1 << 1;
        const READONLY = 1 << 2;
        const PRESERVES_FLAGS = 1 << 3;
        const NORETURN = 1 << 4;
        const NOSTACK = 1 << 5;
    }
}
```

## AST

`InlineAsm` is represented as an expression in the AST:

```rust
pub struct InlineAsm {
    pub template: Vec<InlineAsmTemplatePiece>,
    pub operands: Vec<(InlineAsmOperand, Span)>,
    pub options: InlineAsmOptions,
}

pub enum InlineAsmRegOrRegClass {
    Reg(Symbol),
    RegClass(Symbol),
}

pub enum InlineAsmOperand {
    In {
        reg: InlineAsmRegOrRegClass,
        expr: P<Expr>,
    },
    Out {
        reg: InlineAsmRegOrRegClass,
        late: bool,
        expr: Option<P<Expr>>,
    },
    InOut {
        reg: InlineAsmRegOrRegClass,
        late: bool,
        expr: P<Expr>,
    },
    SplitInOut {
        reg: InlineAsmRegOrRegClass,
        late: bool,
        in_expr: P<Expr>,
        out_expr: Option<P<Expr>>,
    },
    Const {
        expr: P<Expr>,
    },
    Sym {
        expr: P<Expr>,
    },
}
```

The `asm!` macro is implemented in librustc_builtin_macros and outputs an `InlineAsm` AST node. The template string is parsed using libfmt_macros, positional and named operands are resolved to explicit operand indicies. Since target information is not available to macro invocations, validation of the registers and register classes is deferred to AST lowering.

## HIR

`InlineAsm` is represented as an expression in the HIR:

```rust
pub struct InlineAsm<'hir> {
    pub template: &'hir [InlineAsmTemplatePiece],
    pub operands: &'hir [InlineAsmOperand<'hir>],
    pub options: InlineAsmOptions,
}

pub enum InlineAsmRegOrRegClass {
    Reg(InlineAsmReg),
    RegClass(InlineAsmRegClass),
}

pub enum InlineAsmOperand<'hir> {
    In {
        reg: InlineAsmRegOrRegClass,
        expr: Expr<'hir>,
    },
    Out {
        reg: InlineAsmRegOrRegClass,
        late: bool,
        expr: Option<Expr<'hir>>,
    },
    InOut {
        reg: InlineAsmRegOrRegClass,
        late: bool,
        expr: Expr<'hir>,
    },
    SplitInOut {
        reg: InlineAsmRegOrRegClass,
        late: bool,
        in_expr: Expr<'hir>,
        out_expr: Option<Expr<'hir>>,
    },
    Const {
        expr: Expr<'hir>,
    },
    Sym {
        expr: Expr<'hir>,
    },
}
```

AST lowering is where `InlineAsmRegOrRegClass` is converted from `Symbol`s to an actual register or register class. If any modifiers are specified for a template string placeholder, these are validated against the set allowed for that operand type. Finally, explicit registers for inputs and outputs are checked for conflicts (same register used for different operands).

## Type checking

Each register class has a whitelist of types that it may be used with. After the types of all operands have been determined, the `intrinsicck` pass will check that these types are in the whitelist. It also checks that split `inout` operands have compatible types and that `const` operands are integers or floats. Suggestions are emitted where needed if a template modifier should be used for an operand based on the type that was passed into it.

## HAIR

`InlineAsm` is represented as an expression in the HAIR:

```rust
crate enum ExprKind<'tcx> {
    // [..]
    InlineAsm {
        template: &'tcx [InlineAsmTemplatePiece],
        operands: Vec<InlineAsmOperand<'tcx>>,
        options: InlineAsmOptions,
    },
}
crate enum InlineAsmOperand<'tcx> {
    In {
        reg: InlineAsmRegOrRegClass,
        expr: ExprRef<'tcx>,
    },
    Out {
        reg: InlineAsmRegOrRegClass,
        late: bool,
        expr: Option<ExprRef<'tcx>>,
    },
    InOut {
        reg: InlineAsmRegOrRegClass,
        late: bool,
        expr: ExprRef<'tcx>,
    },
    SplitInOut {
        reg: InlineAsmRegOrRegClass,
        late: bool,
        in_expr: ExprRef<'tcx>,
        out_expr: Option<ExprRef<'tcx>>,
    },
    Const {
        expr: ExprRef<'tcx>,
    },
    SymFn {
        expr: ExprRef<'tcx>,
    },
    SymStatic {
        expr: ExprRef<'tcx>,
    },
}
```

The only significant change compared to HIR is that `Sym` has been lowered to either a `SymFn` whose `expr` is a `Literal` ZST of the `fn`, or a `SymStatic` whose `expr` is a `StaticRef`.

## MIR

`InlineAsm` is represented as a `Terminator` in the MIR:

```rust
pub enum TerminatorKind<'tcx> {
    // [..]

    /// Block ends with an inline assembly block. This is a terminator since
    /// inline assembly is allowed to diverge.
    InlineAsm {
        /// The template for the inline assembly, with placeholders.
        template: &'tcx [InlineAsmTemplatePiece],

        /// The operands for the inline assembly, as `Operand`s or `Place`s.
        operands: Vec<InlineAsmOperand<'tcx>>,

        /// Miscellaneous options for the inline assembly.
        options: InlineAsmOptions,

        /// Destination block after the inline assembly returns, unless it is
        /// diverging (InlineAsmOptions::NORETURN).
        destination: Option<BasicBlock>,
    },
}

pub enum InlineAsmOperand<'tcx> {
    In {
        reg: InlineAsmRegOrRegClass,
        value: Operand<'tcx>,
    },
    Out {
        reg: InlineAsmRegOrRegClass,
        late: bool,
        place: Option<Place<'tcx>>,
    },
    InOut {
        reg: InlineAsmRegOrRegClass,
        late: bool,
        in_value: Operand<'tcx>,
        out_place: Option<Place<'tcx>>,
    },
    Const {
        value: Operand<'tcx>,
    },
    SymFn {
        value: Box<Constant<'tcx>>,
    },
    SymStatic {
        value: Box<Constant<'tcx>>,
    },
}
```

As part of HAIR lowering, `InOut` and `SplitInOut` operands are lowered to a split form with a separate `in_value` and `out_place`.

Semantically, the `InlineAsm` terminator is similar to the `Call` terminator except that it has multiple output places where a `Call` only has a single return place output.

The constant promotion pass is used to ensure that `const` operands are actually constants (using the same logic as `#[rustc_args_required_const]`).

## Codegen

Operands are lowered one more time before being passed to LLVM codegen:

```rust
pub enum InlineAsmOperandRef<'tcx, B: BackendTypes + ?Sized> {
    In {
        reg: InlineAsmRegOrRegClass,
        value: OperandRef<'tcx, B::Value>,
    },
    Out {
        reg: InlineAsmRegOrRegClass,
        late: bool,
        place: Option<PlaceRef<'tcx, B::Value>>,
    },
    InOut {
        reg: InlineAsmRegOrRegClass,
        late: bool,
        in_value: OperandRef<'tcx, B::Value>,
        out_place: Option<PlaceRef<'tcx, B::Value>>,
    },
    Const {
        string: String,
    },
    SymFn {
        instance: Instance<'tcx>,
    },
    SymStatic {
        def_id: DefId,
    },
}
```

The operands are lowered to LLVM operands and constraint codes as follow:
- `out` and the output part of `inout` operands are added first, as required by LLVM. Late output operands have a `=` prefix added to their constraint code, non-late output operands have a `=&` prefix added to their constraint code.
- `in` operands are added normally.
- `inout` operands are tied to the matching output operand.
- `sym` operands are passed as function pointers or pointers, using the `"s"` constraint.
- `const` operands are formatted to a string and directly inserted in the template string.

The template string is converted to LLVM form:
- `$` characters are escaped as `$$`.
- `const` operands are converted to strings and inserted directly.
- Placeholders are formatted as `${X:M}` where `X` is the operand index and `M` is the modifier character. Modifiers are converted from the Rust form to the LLVM form.

The various options are converted to clobber constraints or LLVM attributes, refer to the [RFC](https://github.com/Amanieu/rfcs/blob/inline-asm/text/0000-inline-asm.md#mapping-to-llvm-ir) for more details.

Note that LLVM is sometimes rather picky about what types it accepts for certain constraint codes so we sometimes need to insert conversions to/from a supported type. See the target-specific ISelLowering.cpp files in LLVM for details.

# Adding support for new architectures

Adding inline assembly support to an architecture is mostly a matter of defining the registers and register classes for that architecture. All the definitions for register classes are located in `src/librustc_target/asm/`.

Additionally you will need to implement lowering of these register classes to LLVM constraint codes in `src/librustc_codegen_llvm/asm.rs`.
Rustup

@oli-obk Do you know, how we can enforce (ui-)tests pass in rust-lang/rust for Clippy? I can open a PR for this, if you tell me what would be necessary for this.

changelog: none
…variants, r=flip1995

New lint: `match_wildcard_for_single_variants`

changelog: Added a new lint match_wildcard_for_single_variants to warn on enum matches where a wildcard is used to match a single variant

Closes rust-lang#5556
…flip1995

Downgrade `match_wild_err_arm` to pedantic and update help messages

Hi,
This fixes rust-lang#3688 and downgrades `match_wild_err_arm` to pedantic.
There are a lot of different reasons in that issue, for me the biggest are:
1. Rust's errors aren't like Java's exceptions because they're type safe and in most cases the type of error can't change by itself.
2. Sometimes matching can be more ergonomic, and before the `track_caller` feature got introduced it was actually easier to track the panic location with explicit `panic!` than with `expect`.

Currently clippy is failing to build because of a breaking change in rust-lang#69171 I tried fixing it but it is too complex for my little knowledge of clippy and rustc so I'll leave that to people who know what they're doing :)

Another thing, if rustc is breaking clippy a lot then maybe it's better to use something like `miri` does, where it's hard codes the latest tested rustc commit and they keep bumping it, that way when you develop locally it should work even if there was a breaking change (https://github.com/rust-lang/miri/blob/master/rustup-toolchain#L23-L29)

changelog: Downgrade `match_wild_err_arm` to pedantic
Add to the list of words clippy::doc_markdown ignores

"TypeScript" is the only one of these I actually ran into organically; I can remove the others if they're too much.

changelog: Add to the list of words `clippy::doc_markdown` ignores
@bors
Copy link
Contributor

bors commented May 28, 2020

📌 Commit 0e857c2 has been approved by Mark-Simulacrum

@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 May 28, 2020
@bors
Copy link
Contributor

bors commented May 28, 2020

⌛ Testing commit 0e857c2 with merge 759d34b7bfb06c62fbeb967c304206cb265ad481...

@bors
Copy link
Contributor

bors commented May 28, 2020

💔 Test failed - checks-azure

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels May 28, 2020
@matthiaskrgr
Copy link
Member

https://dev.azure.com/rust-lang/rust/_build/results?buildId=30459&view=logs&j=10c53b70-9ae7-511a-b369-2febdb4c7298&t=b1d84b67-447f-5eeb-2a71-5fe302881ad5&l=11434

   Finished release [optimized] target(s) in 7m 58s
duplicate artifacts found when compiling a tool, this typically means that something was recompiled because a transitive dependency has different features activated than in a previous build:

the following dependencies are duplicated although they have the same features enabled:
the following dependencies have different features:
  winapi 0.3.8 (registry+https://github.com/rust-lang/crates.io-index)
    `rustfmt` additionally enabled features {"fibersapi"} at "D:\\a\\1\\s\\build\\x86_64-pc-windows-msvc\\stage1-tools\\x86_64-pc-windows-msvc\\release\\deps\\libwinapi-8368770fccd67b10.rlib"
    `cargo` additionally enabled features {} at "D:\\a\\1\\s\\build\\x86_64-pc-windows-msvc\\stage1-tools\\x86_64-pc-windows-msvc\\release\\deps\\libwinapi-2441560a7cfe905d.rlib"

to fix this you will probably want to edit the local src/tools/rustc-workspace-hack/Cargo.toml crate, as that will update the dependency graph to ensure that these crates all share the same feature set
thread 'main' panicked at 'tools should not compile multiple copies of the same crate', src\bootstrap\tool.rs:195:13
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
failed to run: D:\a\1\s\build\bootstrap\debug\bootstrap dist
Build completed unsuccessfully in 2:22:26

Co-authored-by: Eric Huss <ehuss@users.noreply.github.com>
@flip1995
Copy link
Member Author

flip1995 commented May 29, 2020

I added the patch @ehuss suggested in #72423 (comment)

Maybe we should do a try run before re-r+ing this, to not spam the r+ queue?

@JohnTitor
Copy link
Member

Maybe we should do a try run before re-r+ing this, to not spam the r+ queue?

IIUC current try builds don't contain windows-related ones. So, if we want to make sure things, we should tweak try config temporarily.

@Xanewok
Copy link
Member

Xanewok commented May 29, 2020

@bors r+

@bors
Copy link
Contributor

bors commented May 29, 2020

📌 Commit 3f3e0ee has been approved by Xanewok

@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 May 29, 2020
@Xanewok
Copy link
Member

Xanewok commented May 29, 2020

I think the unified features should be the last hurdle - I'll verify locally on a Windows box too, just in case.

@JohnTitor JohnTitor changed the title Update Clippy Update Clippy, RLS, and rustfmt May 29, 2020
@bors
Copy link
Contributor

bors commented May 29, 2020

⌛ Testing commit 3f3e0ee with merge 96dd469...

@bors
Copy link
Contributor

bors commented May 29, 2020

☀️ Test successful - checks-azure
Approved by: Xanewok
Pushing 96dd469 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label May 29, 2020
@bors bors merged commit 96dd469 into rust-lang:master May 29, 2020
@rust-highfive
Copy link
Collaborator

📣 Toolstate changed by #72671!

Tested on commit 96dd469.
Direct link to PR: #72671

🎉 rls on windows: build-fail → test-pass (cc @Xanewok).
🎉 rls on linux: build-fail → test-pass (cc @Xanewok).
🎉 rustfmt on windows: build-fail → test-pass (cc @topecongiro).
🎉 rustfmt on linux: build-fail → test-pass (cc @topecongiro).

rust-highfive added a commit to rust-lang-nursery/rust-toolstate that referenced this pull request May 29, 2020
Tested on commit rust-lang/rust@96dd469.
Direct link to PR: <rust-lang/rust#72671>

🎉 rls on windows: build-fail → test-pass (cc @Xanewok).
🎉 rls on linux: build-fail → test-pass (cc @Xanewok).
🎉 rustfmt on windows: build-fail → test-pass (cc @topecongiro).
🎉 rustfmt on linux: build-fail → test-pass (cc @topecongiro).
@flip1995 flip1995 deleted the clippyup branch May 31, 2020 11:21
tesuji pushed a commit to tesuji/rustc that referenced this pull request Jun 4, 2020
Update Clippy, RLS, and rustfmt

r? @Dylan-DPC

This makes Clippy test-pass again: 3089c3b

Otherwise this includes bugfixes and a few new lints.

Fixes rust-lang#72231
Fixes rust-lang#72232
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rustfmt no longer builds after rust-lang/rust#69659 rls no longer builds after rust-lang/rust#69659