2

Fix nonstandard_macro_braces FP and docs of disallowed_types by DevinR528 · Pull...

 2 years ago
source link: https://github.com/rust-lang/rust-clippy/pull/7478
Go to the source link to view the article. You can view the picture content, updated content and better typesetting reading experience. If the link is broken, please click the button below to view the snapshot at that time.

Copy link

Collaborator

llogiq left a comment

This looks good in general, I only have one small suggestion for readability.

@@ -93,13 +93,21 @@ impl EarlyLintPass for MacroBraces {

}

fn is_offending_macro<'a>(cx: &EarlyContext<'_>, span: Span, mac_braces: &'a MacroBraces) -> Option<MacroInfo<'a>> {

let unnested_or_local = || {

let nested = in_macro(span.ctxt().outer_expn_data().call_site);

let in_local_macro = nested

llogiq 25 days ago

Collaborator

Having nested && here and returning !nested || .. might befuddle at cursory reading. I'd suggest using a guard if instead.

DevinR528 24 days ago

Author

Contributor

Is this what you mean?

let unnested_or_local = || {
    let nested = in_macro(span.ctxt().outer_expn_data().call_site);
    let in_local_macro = matches!(
        span.macro_backtrace().last().and_then(|e| e.macro_def_id),
        Some(defid) if nested && defid.is_local() // adding `nested` to the if guard?
    );
    
    !nested || in_local_macro
};

llogiq 17 days ago

edited

Collaborator

I was more thinking of

let unnested_or_local = || {
    let nested = in_macro(span.ctxt().outer_expn_data().call_site);
    !nested || span.macro_backtrace().last().map_or(false, |e| 
        e.macro_def_id.map_or(false, |defid| defid.is_local()))
    )
};

DevinR528 16 days ago

Author

Contributor

Oh right duh facepalm I was asking the same question two times in a row (!nested then nested && ..)


About Joyk


Aggregate valuable and interesting links.
Joyk means Joy of geeK