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

Can't allow elided_lifetimes_in_paths inside a module that denies rust_2018_idioms, other lints work #71957

Closed
carols10cents opened this issue May 6, 2020 · 6 comments · Fixed by #90446
Labels
A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. C-bug Category: This is a bug. L-elided_lifetimes_in_paths Lint: elided_lifetimes_in_paths T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@carols10cents
Copy link
Member

carols10cents commented May 6, 2020

What happened

This code fails to compile:

#![deny(rust_2018_idioms)]

mod blah {
    #![allow(rust_2018_idioms)]

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

    struct Bar<T>(T);

    fn foo(b: Bar<Thing>) {}
}

with the error:

error: hidden lifetime parameters in types are deprecated
  --> src/lib.rs:10:19
   |
10 |     fn foo(b: Bar<Thing>) {}
   |                   ^^^^^- help: indicate the anonymous lifetime: `<'_>`
   |
note: the lint level is defined here
  --> src/lib.rs:1:9
   |
1  | #![deny(rust_2018_idioms)]
   |         ^^^^^^^^^^^^^^^^
   = note: `#[deny(elided_lifetimes_in_paths)]` implied by `#[deny(rust_2018_idioms)]`

and I expected the inner allow to override the outer deny.

Variations

What's weird is that this code, with unused_variables instead of rust_2018_idioms, compiles (with other warnings) as I expect:

#![deny(unused_variables)]

mod blah {
    #![allow(unused_variables)]

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

    struct Bar<T>(T);

    fn foo(b: Bar<Thing>) {}
}

(and removing the allow causes it to not compile as I would expect)

AND this code compiles (with other warnings) as I expect:

#![deny(rust_2018_idioms)]

mod blah {
    #![allow(rust_2018_idioms)]
    extern crate rand;
    use rand::Rng;
}

so it seems to be something to do with elided_lifetimes_in_paths in particular...?

I also tried this, which I would expect to compile but does not:

#![deny(rust_2018_idioms)]

mod blah {
    #![allow(elided_lifetimes_in_paths)]

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

    struct Bar<T>(T);

    fn foo(b: Bar<Thing>) {}
}

This I would also expect to compile but does not:

#![deny(elided_lifetimes_in_paths)]

mod blah {
    #![allow(elided_lifetimes_in_paths)]

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

    struct Bar<T>(T);

    fn foo(b: Bar<Thing>) {}
}

I started thinking maybe it had something to do with allow-by-default lints, but this compiles as I would expect:

#![deny(missing_docs)]
//! hi

/// Hi
pub mod blah {
    #![allow(missing_docs)]

    pub struct Foo {
        pub field: i32,
    }
}

Sooo I don't have any real guesses without compiler internals knowledge 🤷‍♀️

Related

It sounds related to this issue: #70819
but I decided to file a new issue because that one appears to be about the same "scope level" and I think I'm doing different scope levels? Please close if this is a dupe!

Versions

I'm using stable 1.43; I also confirmed that 1.42 and 1.41 have the same behavior so this is not a recent regression.

I also confirmed that beta 1.44.0-beta.2 and nightly 2020-05-05 f8d394e have the same behavior.

Why this matters

I'm trying to include! code generated by a build script from another crate that generates 2015 edition Rust, from within a crate where I'd like to enforce 2018 idioms.

@carols10cents carols10cents added the C-bug Category: This is a bug. label May 6, 2020
@jonas-schievink jonas-schievink added A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels May 6, 2020
@ehuss
Copy link
Contributor

ehuss commented May 8, 2020

I think the issue is that allow/warn/deny/forbid for elided_lifetimes_in_paths can only be specified at the crate root (all other locations are ignored). This line looks suspicious, but I don't know the compiler internals too well, either.

@carols10cents
Copy link
Member Author

Oh that's... unexpected and undocumented...

@zackmdavis
Copy link
Member

This line looks suspicious

I agree; that line does look suspicious! (I've self-assigned this because while I've been out of the compiler-development game for a while, and probably still won't be available for a some weeks yet, I wrote the modern form of this lint and should hope to help fix any bugs in it as part of its promotion to a 2021 lint.)

@zackmdavis
Copy link
Member

It turns out that thanks to #73300, there's at least a "ignored unless specified at crate level" lint on the attribute, but that's still not ideal. I don't think there's a good reason for elided-lifetimes-in-paths to need to be at the crate level; I think it just turned out that way because of my incompetence in #52069.

Guided by intuition rather than understanding, I tried this diff:

diff --git a/compiler/rustc_ast_lowering/src/path.rs b/compiler/rustc_ast_lowering/src/path.rs
index 6afed35..d43a034 100644
--- a/compiler/rustc_ast_lowering/src/path.rs
+++ b/compiler/rustc_ast_lowering/src/path.rs
@@ -322,7 +322,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
                     AnonymousLifetimeMode::PassThrough | AnonymousLifetimeMode::ReportError => {
                         self.resolver.lint_buffer().buffer_lint_with_diagnostic(
                             ELIDED_LIFETIMES_IN_PATHS,
-                            CRATE_NODE_ID,
+                            segment.id,
                             path_span,
                             "hidden lifetime parameters in types are deprecated",
                             BuiltinLintDiagnostics::ElidedLifetimesInPaths(

But that doesn't work; we can't build the standard library:

error: internal compiler error: failed to process buffered lint here
   --> library/alloc/src/collections/btree/map.rs:286:10
    |
286 |   #[derive(Debug)]
    |            ^^^^^ in this macro invocation
    | 
   ::: /home/zmd/Code/rust/library/core/src/fmt/mod.rs:592:5
    |
592 | /     pub macro Debug($item:item) {
593 | |         /* compiler built-in */
594 | |     }
    | |_____- in this expansion of `#[derive(Debug)]`
    |
    = note: delayed at /home/zmd/Code/rust/compiler/rustc_lint/src/early.rs:391:22

Maybe there's some way to not buffer the lint when we're inside a macro? (I remember there being something like this, but I've been out of the compiler game for a year and a half and forgot where to find everything.)

@zackmdavis
Copy link
Member

I ... don't think I know what I'm doing here. 😿

This diff (trying to ignore macro code, inspired by the diff in my previous comment failing inside a Derive macro)

diff --git a/compiler/rustc_ast_lowering/src/path.rs b/compiler/rustc_ast_lowering/src/path.rs
index 6afed35..82825c6 100644
--- a/compiler/rustc_ast_lowering/src/path.rs
+++ b/compiler/rustc_ast_lowering/src/path.rs
@@ -320,9 +320,10 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
                         err.emit();
                     }
                     AnonymousLifetimeMode::PassThrough | AnonymousLifetimeMode::ReportError => {
+                        if path_span.ctxt().outer_expn_data().is_root() {
                             self.resolver.lint_buffer().buffer_lint_with_diagnostic(
                                 ELIDED_LIFETIMES_IN_PATHS,
-                            CRATE_NODE_ID,
+                                segment.id,
                                 path_span,
                                 "hidden lifetime parameters in types are deprecated",
                                 BuiltinLintDiagnostics::ElidedLifetimesInPaths(
@@ -337,6 +338,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
                     }
                 }
             }
+        }
 
         let res = self.expect_full_res(segment.id);

hits a similar failure:

error: internal compiler error: failed to process buffered lint here
   --> /home/zmd/.cargo/registry/src/github.com-1ecc6299db9ec823/rustc-demangle-0.1.18/src/lib.rs:191:27
    |
191 |     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
    |                           ^^^^^^^^^^^^^^
    |
    = note: delayed at /home/zmd/Code/rust/compiler/rustc_lint/src/early.rs:391:22

@danielhenrymantilla
Copy link
Contributor

Note that the opposite may also be desirable: some macros may want to spot the elided lifetimes present in a given type, and without type-level information, it is impossible to guess how many lifetime parameters does a given Path involve. Because of that, having the macro emit a #[deny(elided_lifetimes_in_paths)] would be a great way to require that the caller feed the necessary '_ lifetime parameters (which can then be used by the macro since they are syntactically visible)

But given the fact the crate is only allowed to be used at the top-level of the crate, this is not possible yet.

  • A possible workaround for macro authors

    (The only workaround I have found is to emit dummy type Foo = caller::provided::TypePath; which errors when lifetime parameters are required). It just happens that the macro may not always have an ergonomic scope to write these hacks (e.g., imagine the macro is called within an impl block, on an associated function: we don't want to emit tangential type or const annotations polluting that impl))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. C-bug Category: This is a bug. L-elided_lifetimes_in_paths Lint: elided_lifetimes_in_paths T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants