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.

Sign up for GitHub

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

Rustup to rustc 1.41.0-nightly (35ef33a8 2019-11-21) #4825

Merged
merged 9 commits into from
Nov 23, 2019
Merged

Rustup to rustc 1.41.0-nightly (35ef33a8 2019-11-21) #4825

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
aa5a95f
Rustup to rustc 1.41.0-nightly (a0d40f8bd 2019-11-18)
Manishearth Nov 18, 2019
d183bda
Rustup to rustc 1.41.0-nightly (d1da8023d 2019-11-19)
Manishearth Nov 19, 2019
3c308b8
Remove never_type feature
flip1995 Nov 22, 2019
e4636f3
Rustup to rust-lang/rust#66515
flip1995 Nov 22, 2019
cadc35a
Rustup to rust-lang/rust#66389
flip1995 Nov 22, 2019
e3a74ed
Set mir_opt_level=0
flip1995 Nov 22, 2019
7bae5bd
Add comment for mir_opt_level=0
flip1995 Nov 23, 2019
fb18354
Temporarily disable serde integration test
flip1995 Nov 23, 2019
553db87
Disable chalk integration test. Output too large
flip1995 Nov 23, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@ rustc_tools_util = { version = "0.2.0", path = "rustc_tools_util"}

[dev-dependencies]
cargo_metadata = "0.9.0"
compiletest_rs = { version = "0.3.24", features = ["tmp"] }
compiletest_rs = { version = "0.4.0", features = ["tmp"] }
tester = "0.7"
lazy_static = "1.0"
clippy-mini-macro-test = { version = "0.2", path = "mini-macro" }
serde = { version = "1.0", features = ["derive"] }
Expand Down
92 changes: 35 additions & 57 deletions clippy_lints/src/write.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use rustc_errors::Applicability;
use rustc_lexer::unescape::{self, EscapeError};
use rustc_parse::parser;
use syntax::ast::*;
use syntax::symbol::Symbol;
use syntax::token;
use syntax::tokenstream::TokenStream;
use syntax_pos::{BytePos, Span};
Expand Down Expand Up @@ -190,7 +191,7 @@ impl EarlyLintPass for Write {
if mac.path == sym!(println) {
span_lint(cx, PRINT_STDOUT, mac.span, "use of `println!`");
if let (Some(fmt_str), _) = check_tts(cx, &mac.tts, false) {
if fmt_str.contents.is_empty() {
if fmt_str.symbol == Symbol::intern("") {
span_lint_and_sugg(
cx,
PRINTLN_EMPTY_STRING,
Expand All @@ -205,7 +206,7 @@ impl EarlyLintPass for Write {
} else if mac.path == sym!(print) {
span_lint(cx, PRINT_STDOUT, mac.span, "use of `print!`");
if let (Some(fmt_str), _) = check_tts(cx, &mac.tts, false) {
if check_newlines(&fmt_str.contents, fmt_str.style) {
if check_newlines(&fmt_str) {
span_lint_and_then(
cx,
PRINT_WITH_NEWLINE,
Expand All @@ -216,7 +217,7 @@ impl EarlyLintPass for Write {
"use `println!` instead",
vec![
(mac.path.span, String::from("println")),
(fmt_str.newline_span(), String::new()),
(newline_span(&fmt_str), String::new()),
],
Applicability::MachineApplicable,
);
Expand All @@ -226,7 +227,7 @@ impl EarlyLintPass for Write {
}
} else if mac.path == sym!(write) {
if let (Some(fmt_str), _) = check_tts(cx, &mac.tts, true) {
if check_newlines(&fmt_str.contents, fmt_str.style) {
if check_newlines(&fmt_str) {
span_lint_and_then(
cx,
WRITE_WITH_NEWLINE,
Expand All @@ -237,7 +238,7 @@ impl EarlyLintPass for Write {
"use `writeln!()` instead",
vec![
(mac.path.span, String::from("writeln")),
(fmt_str.newline_span(), String::new()),
(newline_span(&fmt_str), String::new()),
],
Applicability::MachineApplicable,
);
Expand All @@ -247,7 +248,7 @@ impl EarlyLintPass for Write {
}
} else if mac.path == sym!(writeln) {
if let (Some(fmt_str), expr) = check_tts(cx, &mac.tts, true) {
if fmt_str.contents.is_empty() {
if fmt_str.symbol == Symbol::intern("") {
let mut applicability = Applicability::MachineApplicable;
let suggestion = expr.map_or_else(
move || {
Expand All @@ -272,37 +273,27 @@ impl EarlyLintPass for Write {
}
}

/// The arguments of a `print[ln]!` or `write[ln]!` invocation.
struct FmtStr {
/// The contents of the format string (inside the quotes).
contents: String,
style: StrStyle,
/// The span of the format string, including quotes, the raw marker, and any raw hashes.
span: Span,
}

impl FmtStr {
/// Given a format string that ends in a newline and its span, calculates the span of the
/// newline.
fn newline_span(&self) -> Span {
let sp = self.span;
/// Given a format string that ends in a newline and its span, calculates the span of the
/// newline.
fn newline_span(fmtstr: &StrLit) -> Span {
let sp = fmtstr.span;
let contents = &fmtstr.symbol.as_str();

let newline_sp_hi = sp.hi()
- match self.style {
StrStyle::Cooked => BytePos(1),
StrStyle::Raw(hashes) => BytePos((1 + hashes).into()),
};

let newline_sp_len = if self.contents.ends_with('\n') {
BytePos(1)
} else if self.contents.ends_with(r"\n") {
BytePos(2)
} else {
panic!("expected format string to contain a newline");
let newline_sp_hi = sp.hi()
- match fmtstr.style {
StrStyle::Cooked => BytePos(1),
StrStyle::Raw(hashes) => BytePos((1 + hashes).into()),
};

sp.with_lo(newline_sp_hi - newline_sp_len).with_hi(newline_sp_hi)
}
let newline_sp_len = if contents.ends_with('\n') {
BytePos(1)
} else if contents.ends_with(r"\n") {
BytePos(2)
} else {
panic!("expected format string to contain a newline");
};

sp.with_lo(newline_sp_hi - newline_sp_len).with_hi(newline_sp_hi)
}

/// Checks the arguments of `print[ln]!` and `write[ln]!` calls. It will return a tuple of two
Expand All @@ -325,7 +316,7 @@ impl FmtStr {
/// (Some("string to write: {}"), Some(buf))
/// ```
#[allow(clippy::too_many_lines)]
fn check_tts<'a>(cx: &EarlyContext<'a>, tts: &TokenStream, is_write: bool) -> (Option<FmtStr>, Option<Expr>) {
fn check_tts<'a>(cx: &EarlyContext<'a>, tts: &TokenStream, is_write: bool) -> (Option<StrLit>, Option<Expr>) {
use fmt_macros::*;
let tts = tts.clone();

Expand All @@ -342,12 +333,11 @@ fn check_tts<'a>(cx: &EarlyContext<'a>, tts: &TokenStream, is_write: bool) -> (O
}
}

let (fmtstr, fmtstyle) = match parser.parse_str().map_err(|mut err| err.cancel()) {
Ok((fmtstr, fmtstyle)) => (fmtstr.to_string(), fmtstyle),
let fmtstr = match parser.parse_str_lit() {
Ok(fmtstr) => fmtstr,
Err(_) => return (None, expr),
};
let fmtspan = parser.prev_span;
let tmp = fmtstr.clone();
let tmp = fmtstr.symbol.as_str();
let mut args = vec![];
let mut fmt_parser = Parser::new(&tmp, None, Vec::new(), false);
while let Some(piece) = fmt_parser.next() {
Expand Down Expand Up @@ -377,26 +367,12 @@ fn check_tts<'a>(cx: &EarlyContext<'a>, tts: &TokenStream, is_write: bool) -> (O
ty_span: None,
};
if !parser.eat(&token::Comma) {
return (
Some(FmtStr {
contents: fmtstr,
style: fmtstyle,
span: fmtspan,
}),
expr,
);
return (Some(fmtstr), expr);
}
let token_expr = if let Ok(expr) = parser.parse_expr().map_err(|mut err| err.cancel()) {
expr
} else {
return (
Some(FmtStr {
contents: fmtstr,
style: fmtstyle,
span: fmtspan,
}),
None,
);
return (Some(fmtstr), None);
};
match &token_expr.kind {
ExprKind::Lit(_) => {
Expand Down Expand Up @@ -448,11 +424,13 @@ fn check_tts<'a>(cx: &EarlyContext<'a>, tts: &TokenStream, is_write: bool) -> (O
/// Checks if the format string contains a single newline that terminates it.
///
/// Literal and escaped newlines are both checked (only literal for raw strings).
fn check_newlines(contents: &str, style: StrStyle) -> bool {
fn check_newlines(fmtstr: &StrLit) -> bool {
let mut has_internal_newline = false;
let mut last_was_cr = false;
let mut should_lint = false;

let contents = &fmtstr.symbol.as_str();

let mut cb = |r: Range<usize>, c: Result<char, EscapeError>| {
let c = c.unwrap();

Expand All @@ -466,7 +444,7 @@ fn check_newlines(contents: &str, style: StrStyle) -> bool {
}
};

match style {
match fmtstr.style {
StrStyle::Cooked => unescape::unescape_str(contents, &mut cb),
StrStyle::Raw(_) => unescape::unescape_raw_str(contents, &mut cb),
}
Expand Down
2 changes: 1 addition & 1 deletion tests/compile-test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#![feature(test)]

use compiletest_rs as compiletest;
extern crate test;
extern crate tester as test;

use std::env::{set_var, var};
use std::ffi::OsStr;
Expand Down
3 changes: 0 additions & 3 deletions tests/ui/missing_const_for_fn/cant_be_const.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,8 @@ fn random_caller() -> u32 {

static Y: u32 = 0;

// We should not suggest to make this function `const` because const functions are not allowed to
// refer to a static variable
fn get_y() -> u32 {
Copy link
Member

@flip1995 flip1995 Nov 19, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn't be linted still. Adding const to this function gives the error described in the comment. But for some reason rustc_mir::transform::qualify_min_const_fn::is_min_const_fn returns Ok(()) for this function:

rust-clippy/clippy_lints/src/missing_const_for_fn.rs

Lines 108 to 110 in b4f1769

let mir = cx.tcx.optimized_mir(def_id);
if let Err((span, err)) = is_min_const_fn(cx.tcx, def_id, &mir) {

cc @oli-obk Does optimized_mir replace the static with a const value and is_min_const_fn then thinks this could be a const fn?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did something change in the handling of Statements in MIR recently? The error in redundant_clone is similar. This code

rust-clippy/clippy_lints/src/redundant_clone.rs

Lines 374 to 383 in b4f1769

fn visit_local(&mut self, local: &mir::Local, ctx: PlaceContext, _: mir::Location) {
match ctx {
PlaceContext::MutatingUse(MutatingUseContext::Drop) | PlaceContext::NonUse(_) => return,
_ => {},
}
if *local == self.local {
self.used_other_than_drop = true;
}
}

stopped detecting, that in (a.clone(), a) both as are the same.

Copy link
Member

@flip1995 flip1995 Nov 19, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I mean with

The error [...] is similar

is, that in both cases the code that should check for them is never actually reached by the traversal of the Statement (or the Terminator?)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh... we run is_min_const_fn on the optimized mir while rustc runs it on mir_const. This will keep causing problems. One thing clippy could do is to make the optimized_mir query not do any optimizations. This is possible since rust-lang/rust#66297

This comment was marked as outdated.

Sign in to view

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So how do I override the optimized_mir query? When I just copy and paste its source code [1] from rustc and remove the run_optimization_passes call¹, a lot of tests began to ICE. I added this:

        config.override_queries = Some(|_, providers, _| {
            providers.optimized_mir = |tcx, def_id| {
                // (Mir-)Borrowck uses `mir_validated`, so we have to force it to
                // execute before we can steal.
                tcx.ensure().mir_borrowck(def_id);

                let (body, _) = tcx.mir_validated(def_id);
                let body = body.steal();
                tcx.arena.alloc(body)
            }
        });

to

rust-clippy/src/driver.rs

Lines 63 to 64 in b4f1769

impl rustc_driver::Callbacks for ClippyCallbacks {
fn config(&mut self, config: &mut interface::Config) {

¹ I also removed this:

    if tcx.is_constructor(def_id) {
        // There's no reason to run all of the MIR passes on constructors when
        // we can just output the MIR we want directly. This also saves const
        // qualification and borrow checking the trouble of special casing
        // constructors.
        return shim::build_adt_ctor(tcx, def_id);
    }

because the shim module is private.

[1] https://github.com/rust-lang/rust/blob/9d6ff1553b7debbe5c99c102ce0978b6130592f8/src/librustc_mir/transform/mod.rs#L316-L333

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm. So we'd need to make more things public. But that's probably not the failure reason. One thing you can try is set the mir opt level to 0 instead of trying to replace the query. If that doesn't work, reintroduce the mir optimizations but remove all the ones that break clippy (e.g. const_prop)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thing you can try is set the mir opt level to 0

How do I set the opt level? And how can I enable/disable mir passes?

Y
//~^ ERROR E0013
}

// Don't lint entrypoint functions
Expand Down
1 change: 1 addition & 0 deletions tests/ui/redundant_clone.fixed
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// run-rustfix
// rustfix-only-machine-applicable

use std::ffi::OsString;
use std::path::Path;

Expand Down
1 change: 1 addition & 0 deletions tests/ui/redundant_clone.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// run-rustfix
// rustfix-only-machine-applicable

use std::ffi::OsString;
use std::path::Path;

Expand Down
Loading