Port #[must_use] to new attribute parsing infrastructure
#142780
Conversation
|
|
rustbot
added
A-attributes
|
Some changes occurred in compiler/rustc_attr_parsing Some changes occurred in compiler/rustc_passes/src/check_attr.rs Some changes occurred in src/tools/clippy cc @rust-lang/clippy Some changes occurred in compiler/rustc_attr_data_structures |
| @@ -326,8 +328,14 @@ pub fn has_drop<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool { | |||
| // Returns whether the type has #[must_use] attribute | |||
| pub fn is_must_use_ty<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool { | |||
| match ty.kind() { | |||
| ty::Adt(adt, _) => cx.tcx.has_attr(adt.did(), sym::must_use), | |||
| ty::Foreign(did) => cx.tcx.has_attr(*did, sym::must_use), | |||
| ty::Adt(adt, _) => find_attr!( | |||
There was a problem hiding this comment.
All the find_attr!s with cx.tcx.get_all_attrs are a little ugly, is there a nicer way to do this?
If not we could consider making a new macro for that
There was a problem hiding this comment.
if only there were associated macros... at least this is easy to find now to fix later but I'm open for suggestions for better approaches.
There was a problem hiding this comment.
Could still get away with a find_attr_ctx!(cx.tcx, adr.did(), ...), but not sure how big of an improvement that is.
This is also fine for now, definitely no need to block the PR on this
| @@ -686,7 +688,8 @@ impl<'tcx> CheckAttrVisitor<'tcx> { | |||
| Attribute::Parsed( | |||
| AttributeKind::Deprecation { .. } | |||
| | AttributeKind::Repr { .. } | |||
| | AttributeKind::Align { .. }, | |||
| | AttributeKind::Align { .. } | |||
| | AttributeKind::MustUse { .. }, | |||
There was a problem hiding this comment.
I've added MustUse here but not removed it from the ALLOW_LIST above, because you also seem to not have done this for the other attributes. Is this correct?
JonathanBrouwer
force-pushed
the
must_use_new_attr
branch
from
74be5fa to
49cf486
Compare
|
Rebased to solve merge conflicts |
This comment has been minimized.
This comment has been minimized.
JonathanBrouwer
force-pushed
the
must_use_new_attr
branch
from
49cf486 to
7eb1d5c
Compare
|
These commits modify Please ensure that if you've changed the output:
cc @aDotInTheVoid, @obi1kenobi rustdoc-json-types is a public (although nightly-only) API. If possible, consider changing |
JonathanBrouwer
force-pushed
the
must_use_new_attr
branch
from
7eb1d5c to
b17cbe2
Compare
Signed-off-by: Jonathan Brouwer <[email protected]>
JonathanBrouwer
force-pushed
the
must_use_new_attr
branch
from
b17cbe2 to
f549f43
Compare
| ArgParser::NoArgs => None, | ||
| ArgParser::NameValue(name_value) => name_value.value_as_str(), | ||
| ArgParser::List(_) => { | ||
| return None; |
There was a problem hiding this comment.
This is a case in which the attribute is malformed, but it seems like the error lint is already emitted (see the new malformed-muse_use testcase, there apparently wasn't a test for this yet)
Is it correct that I don't need to emit an error here myself? I see other attributes do emit lints manually, which confuses me
Ports
must_useto the new attribute parsing infrastructure for #131229 (comment)r? @jdonszelmann