Skip to content

Commit 48b10fe

Browse files
committed
Split MacArgs in two.
`MacArgs` is an enum with three variants: `Empty`, `Delimited`, and `Eq`. It's used in two ways: - For representing attribute macro arguments (e.g. in `AttrItem`), where all three variants are used. - For representing function-like macros (e.g. in `MacCall` and `MacroDef`), where only the `Delimited` variant is used. In other words, `MacArgs` is used in two quite different places due to them having partial overlap. I find this makes the code hard to read. It also leads to various unreachable code paths, and allows invalid values (such as accidentally using `MacArgs::Empty` in a `MacCall`). This commit splits `MacArgs` in two: - `DelimArgs` is a new struct just for the "delimited arguments" case. It is now used in `MacCall` and `MacroDef`. - `AttrArgs` is a renaming of the old `MacArgs` enum for the attribute macro case. Its `Delimited` variant now contains a `DelimArgs`. Various other related things are renamed as well. These changes make the code clearer, avoids several unreachable paths, and disallows the invalid values.
1 parent 3597ed5 commit 48b10fe

File tree

2 files changed

+13
-9
lines changed

2 files changed

+13
-9
lines changed

clippy_lints/src/crate_in_macro_def.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ impl EarlyLintPass for CrateInMacroDef {
5555
if_chain! {
5656
if item.attrs.iter().any(is_macro_export);
5757
if let ItemKind::MacroDef(macro_def) = &item.kind;
58-
let tts = macro_def.body.inner_tokens();
58+
let tts = macro_def.body.tokens.clone();
5959
if let Some(span) = contains_unhygienic_crate_reference(&tts);
6060
then {
6161
span_lint_and_sugg(

clippy_utils/src/ast_utils.rs

+12-8
Original file line numberDiff line numberDiff line change
@@ -388,7 +388,7 @@ pub fn eq_item_kind(l: &ItemKind, r: &ItemKind) -> bool {
388388
&& over(li, ri, |l, r| eq_item(l, r, eq_assoc_item_kind))
389389
},
390390
(MacCall(l), MacCall(r)) => eq_mac_call(l, r),
391-
(MacroDef(l), MacroDef(r)) => l.macro_rules == r.macro_rules && eq_mac_args(&l.body, &r.body),
391+
(MacroDef(l), MacroDef(r)) => l.macro_rules == r.macro_rules && eq_delim_args(&l.body, &r.body),
392392
_ => false,
393393
}
394394
}
@@ -709,26 +709,30 @@ pub fn eq_assoc_constraint(l: &AssocConstraint, r: &AssocConstraint) -> bool {
709709
}
710710

711711
pub fn eq_mac_call(l: &MacCall, r: &MacCall) -> bool {
712-
eq_path(&l.path, &r.path) && eq_mac_args(&l.args, &r.args)
712+
eq_path(&l.path, &r.path) && eq_delim_args(&l.args, &r.args)
713713
}
714714

715715
pub fn eq_attr(l: &Attribute, r: &Attribute) -> bool {
716716
use AttrKind::*;
717717
l.style == r.style
718718
&& match (&l.kind, &r.kind) {
719719
(DocComment(l1, l2), DocComment(r1, r2)) => l1 == r1 && l2 == r2,
720-
(Normal(l), Normal(r)) => eq_path(&l.item.path, &r.item.path) && eq_mac_args(&l.item.args, &r.item.args),
720+
(Normal(l), Normal(r)) => eq_path(&l.item.path, &r.item.path) && eq_attr_args(&l.item.args, &r.item.args),
721721
_ => false,
722722
}
723723
}
724724

725-
pub fn eq_mac_args(l: &MacArgs, r: &MacArgs) -> bool {
726-
use MacArgs::*;
725+
pub fn eq_attr_args(l: &AttrArgs, r: &AttrArgs) -> bool {
726+
use AttrArgs::*;
727727
match (l, r) {
728728
(Empty, Empty) => true,
729-
(Delimited(_, ld, lts), Delimited(_, rd, rts)) => ld == rd && lts.eq_unspanned(rts),
730-
(Eq(_, MacArgsEq::Ast(le)), Eq(_, MacArgsEq::Ast(re))) => eq_expr(le, re),
731-
(Eq(_, MacArgsEq::Hir(ll)), Eq(_, MacArgsEq::Hir(rl))) => ll.kind == rl.kind,
729+
(Delimited(la), Delimited(ra)) => eq_delim_args(la, ra),
730+
(Eq(_, AttrArgsEq::Ast(le)), Eq(_, AttrArgsEq::Ast(re))) => eq_expr(le, re),
731+
(Eq(_, AttrArgsEq::Hir(ll)), Eq(_, AttrArgsEq::Hir(rl))) => ll.kind == rl.kind,
732732
_ => false,
733733
}
734734
}
735+
736+
pub fn eq_delim_args(l: &DelimArgs, r: &DelimArgs) -> bool {
737+
l.delim == r.delim && l.tokens.eq_unspanned(&r.tokens)
738+
}

0 commit comments

Comments
 (0)