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

passes: prohibit invalid attrs on generic params #79073

Merged
merged 1 commit into from
Dec 19, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
23 changes: 23 additions & 0 deletions compiler/rustc_hir/src/target.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,13 @@ use crate::{Item, ItemKind, TraitItem, TraitItemKind};

use std::fmt::{self, Display};

#[derive(Copy, Clone, PartialEq, Debug)]
pub enum GenericParamKind {
Type,
Lifetime,
Const,
}

#[derive(Copy, Clone, PartialEq, Debug)]
pub enum MethodKind {
Trait { body: bool },
Expand Down Expand Up @@ -43,6 +50,7 @@ pub enum Target {
ForeignFn,
ForeignStatic,
ForeignTy,
GenericParam(GenericParamKind),
}

impl Display for Target {
Expand Down Expand Up @@ -77,6 +85,11 @@ impl Display for Target {
Target::ForeignFn => "foreign function",
Target::ForeignStatic => "foreign static item",
Target::ForeignTy => "foreign type",
Target::GenericParam(kind) => match kind {
GenericParamKind::Type => "type parameter",
GenericParamKind::Lifetime => "lifetime parameter",
GenericParamKind::Const => "const parameter",
},
}
)
}
Expand Down Expand Up @@ -124,4 +137,14 @@ impl Target {
hir::ForeignItemKind::Type => Target::ForeignTy,
}
}

pub fn from_generic_param(generic_param: &hir::GenericParam<'_>) -> Target {
match generic_param.kind {
hir::GenericParamKind::Type { .. } => Target::GenericParam(GenericParamKind::Type),
hir::GenericParamKind::Lifetime { .. } => {
Target::GenericParam(GenericParamKind::Lifetime)
}
hir::GenericParamKind::Const { .. } => Target::GenericParam(GenericParamKind::Const),
}
}
}
12 changes: 12 additions & 0 deletions compiler/rustc_passes/src/check_attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -814,6 +814,18 @@ impl Visitor<'tcx> for CheckAttrVisitor<'tcx> {
intravisit::walk_item(self, item)
}

fn visit_generic_param(&mut self, generic_param: &'tcx hir::GenericParam<'tcx>) {
let target = Target::from_generic_param(generic_param);
self.check_attributes(
generic_param.hir_id,
generic_param.attrs,
&generic_param.span,
target,
None,
);
intravisit::walk_generic_param(self, generic_param)
}

fn visit_trait_item(&mut self, trait_item: &'tcx TraitItem<'tcx>) {
let target = Target::from_trait_item(trait_item);
self.check_attributes(trait_item.hir_id, &trait_item.attrs, &trait_item.span, target, None);
Expand Down
30 changes: 30 additions & 0 deletions src/test/ui/issues/issue-78957.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
#![deny(unused_attributes)]
#![feature(min_const_generics)]

use std::marker::PhantomData;

pub struct Foo<#[inline] const N: usize>;
//~^ ERROR attribute should be applied to function or closure
pub struct Bar<#[cold] const N: usize>;
//~^ ERROR attribute should be applied to a function
//~| WARN this was previously accepted
pub struct Baz<#[repr(C)] const N: usize>;
//~^ ERROR attribute should be applied to a struct, enum, or union
//
pub struct Foo2<#[inline] 'a>(PhantomData<&'a ()>);
//~^ ERROR attribute should be applied to function or closure
pub struct Bar2<#[cold] 'a>(PhantomData<&'a ()>);
//~^ ERROR attribute should be applied to a function
//~| WARN this was previously accepted
pub struct Baz2<#[repr(C)] 'a>(PhantomData<&'a ()>);
//~^ ERROR attribute should be applied to a struct, enum, or union
//
pub struct Foo3<#[inline] T>(PhantomData<T>);
//~^ ERROR attribute should be applied to function or closure
pub struct Bar3<#[cold] T>(PhantomData<T>);
//~^ ERROR attribute should be applied to a function
//~| WARN this was previously accepted
pub struct Baz3<#[repr(C)] T>(PhantomData<T>);
//~^ ERROR attribute should be applied to a struct, enum, or union

fn main() {}
davidtwco marked this conversation as resolved.
Show resolved Hide resolved
69 changes: 69 additions & 0 deletions src/test/ui/issues/issue-78957.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
error[E0518]: attribute should be applied to function or closure
--> $DIR/issue-78957.rs:6:16
|
LL | pub struct Foo<#[inline] const N: usize>;
| ^^^^^^^^^ - not a function or closure

error: attribute should be applied to a function
--> $DIR/issue-78957.rs:8:16
|
LL | pub struct Bar<#[cold] const N: usize>;
| ^^^^^^^ - not a function
|
note: the lint level is defined here
--> $DIR/issue-78957.rs:1:9
|
LL | #![deny(unused_attributes)]
| ^^^^^^^^^^^^^^^^^
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
Copy link
Contributor

Choose a reason for hiding this comment

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

So it seems like we always just emit a future compat lint if cold is applied incorrectly 🤔

It would be nice to increase that lint to deny by default. Don't know how much this would break though

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll leave this for a follow-up.


error[E0517]: attribute should be applied to a struct, enum, or union
--> $DIR/issue-78957.rs:11:23
|
LL | pub struct Baz<#[repr(C)] const N: usize>;
| ^ - not a struct, enum, or union

error[E0518]: attribute should be applied to function or closure
--> $DIR/issue-78957.rs:14:17
|
LL | pub struct Foo2<#[inline] 'a>(PhantomData<&'a ()>);
| ^^^^^^^^^ -- not a function or closure

error: attribute should be applied to a function
--> $DIR/issue-78957.rs:16:17
|
LL | pub struct Bar2<#[cold] 'a>(PhantomData<&'a ()>);
| ^^^^^^^ -- not a function
|
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!

error[E0517]: attribute should be applied to a struct, enum, or union
--> $DIR/issue-78957.rs:19:24
|
LL | pub struct Baz2<#[repr(C)] 'a>(PhantomData<&'a ()>);
| ^ -- not a struct, enum, or union

error[E0518]: attribute should be applied to function or closure
--> $DIR/issue-78957.rs:22:17
|
LL | pub struct Foo3<#[inline] T>(PhantomData<T>);
| ^^^^^^^^^ - not a function or closure

error: attribute should be applied to a function
--> $DIR/issue-78957.rs:24:17
|
LL | pub struct Bar3<#[cold] T>(PhantomData<T>);
| ^^^^^^^ - not a function
|
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!

error[E0517]: attribute should be applied to a struct, enum, or union
--> $DIR/issue-78957.rs:27:24
|
LL | pub struct Baz3<#[repr(C)] T>(PhantomData<T>);
| ^ - not a struct, enum, or union

error: aborting due to 9 previous errors

Some errors have detailed explanations: E0517, E0518.
For more information about an error, try `rustc --explain E0517`.
4 changes: 3 additions & 1 deletion src/test/ui/proc-macro/ambiguous-builtin-attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@ fn test() {}
#[bench] // OK, shadowed
fn bench() {}

fn non_macro_expanded_location<#[repr(C)] T>() { //~ ERROR `repr` is ambiguous
fn non_macro_expanded_location<#[repr(C)] T>() {
//~^ ERROR `repr` is ambiguous
//~| ERROR attribute should be applied to a struct, enum, or union
match 0u8 {
#[repr(C)] //~ ERROR `repr` is ambiguous
_ => {}
Expand Down
14 changes: 10 additions & 4 deletions src/test/ui/proc-macro/ambiguous-builtin-attrs.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error[E0425]: cannot find value `NonExistent` in this scope
--> $DIR/ambiguous-builtin-attrs.rs:30:5
--> $DIR/ambiguous-builtin-attrs.rs:32:5
|
LL | NonExistent;
| ^^^^^^^^^^^ not found in this scope
Expand Down Expand Up @@ -47,7 +47,7 @@ LL | use builtin_attrs::*;
= help: use `crate::repr` to refer to this attribute macro unambiguously

error[E0659]: `repr` is ambiguous (built-in attribute vs any other name)
--> $DIR/ambiguous-builtin-attrs.rs:22:11
--> $DIR/ambiguous-builtin-attrs.rs:24:11
|
LL | #[repr(C)]
| ^^^^ ambiguous name
Expand All @@ -74,7 +74,13 @@ LL | use builtin_attrs::*;
| ^^^^^^^^^^^^^^^^
= help: use `crate::feature` to refer to this attribute macro unambiguously

error: aborting due to 6 previous errors
error[E0517]: attribute should be applied to a struct, enum, or union
--> $DIR/ambiguous-builtin-attrs.rs:20:39
|
LL | fn non_macro_expanded_location<#[repr(C)] T>() {
| ^ - not a struct, enum, or union

error: aborting due to 7 previous errors

Some errors have detailed explanations: E0425, E0659.
Some errors have detailed explanations: E0425, E0517, E0659.
For more information about an error, try `rustc --explain E0425`.