Calling main

[llogiq]

Apart from special setups (which we could detect following attributes like #![no_std]), recursing into main() seems like an unintuitive antipattern we should be able to detect.

[llogiq]

The admin of the German Rust Forum told me he wanted to try this one.

[phansch]

@thomanski It's been a while and I just saw the commit in your fork, would you like to continue with it and open a PR?

[thomanski]

Nope, thanks, I think @oli-obk <https://github.com/oli-obk> was last looking into that. I wish I'd have more time for Rust but still just don't.

…

On 14 April 2018 at 13:04, Philipp Hansch ***@***.***> wrote: @thomanski <https://github.com/thomanski> It's been a while and I just saw the commit in your fork, would you like to continue with it and open a PR? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#333 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AMwtIPdpINAgWenDTT1R_JfGGq6e6NlKks5todfXgaJpZM4F-O9Y> .

[Urriel]

Hello, is this issue still relevant?
I would like to work on this one.

[Urriel]

I would like to add this lint. Where should I add it? (I mean in which file ?).

[flip1995]

Oh sorry, I missed your previous comment. Yes, this is still relevant.

I think this lint would best fit in the methods module. Just add a new file for this lint there.

Taking a closer look at this module, it does not fit in there. So just create a new file for this lint.

[Urriel]

OK, no worries.

[Urriel]

I have looked into how to check functions and macros. But in order to disable this lint, I need a state for whether the no_std macro is found or not.
I see that the trait applies to the lint pass variable. Is their a way to add a boolean variable to that structure in order to store that state?

[flip1995]

#![no_std] is usually added at the crate root, since it doesn't really make sense to have only a few modules without the standard library (I'm not sure if it's even possible to add this attribute module wise 🤔 ). This means you have to check the crate root for the no_std attribute. You can do this with the check_crate method. The check_crate method is called before any other check_* method of the Late/EarlyLintPasses, so you can save this state inside the lint struct.

struct MainCall {
    has_no_std: bool
}

impl_lint_pass!(..)

impl EarlyLintPass for MainCall {
    fn check_crate(.., krate: &Crate) {
        self.has_no_std = krate.attrs.iter().any(/*check if no_std*/);
    }

   fn check_expr(..) { ... }
}

[Urriel]

I am looking for a way to test that "main" recursion. I cannot figure a way to have a no_std and an std version of a test working...

#![no_std]

fn main() {
    main();
}

in the code above there is no exit from the recursion...
I tried to add an "if" statement using a mutable static variable. But it throws more warning and uses unsafe code.

[flip1995]

I cannot figure a way to have a no_std and an std version of a test working...

You have to create two testfiles for this. These can be identical (except for the no_std line of course)

"if" statement using a mutable static variable

use core::sync::atomic::{AtomicUsize, Ordering};

static N: AtomicUsize = AtomicUsize::new(0);

fn main() {
    let x = N.load(Ordering::Relaxed);
    N.store(x + 1, Ordering::Relaxed);
        
    println!("{}", x);
        
    if x < 3 {
        main();
    }
}

Internal mutability to the rescue. Clippy tests never get executed, but only compiled (=> just allow unconditional_recursion). So the test code you posted is also totally fine.

[Urriel]

oh, alright. Thank you !

[Urriel]

After many tries for the no_std part, I have a cc link issue:

error: linking with `cc` failed: exit code: 1
   |
   = note: "cc" "-Wl,--as-needed" "-Wl,-z,noexecstack" "-m64" "-L" "/home/vincent_dalmaso/.rustup/toolchains/master/lib/rustlib/x86_64-unknown-linux-gnu/lib" "/home/vincent_dalmaso/projects/playground/rust-clippy/target/debug/test_build_base/crate_level_checks/no_std_main_recursion.no_std_main_recursion.7rcbfp3g-cgu.0.rcgu.o" "/home/vincent_dalmaso/projects/playground/rust-clippy/target/debug/test_build_base/crate_level_checks/no_std_main_recursion.no_std_main_recursion.7rcbfp3g-cgu.1.rcgu.o" "/home/vincent_dalmaso/projects/playground/rust-clippy/target/debug/test_build_base/crate_level_checks/no_std_main_recursion.no_std_main_recursion.7rcbfp3g-cgu.2.rcgu.o" "-o" "/home/vincent_dalmaso/projects/playground/rust-clippy/target/debug/test_build_base/crate_level_checks/no_std_main_recursion.stage-id" "-Wl,--gc-sections" "-pie" "-Wl,-zrelro" "-Wl,-znow" "-nodefaultlibs" "-L" "/home/vincent_dalmaso/projects/playground/rust-clippy/target/debug/test_build_base" "-L" "target/debug" "-L" "target/debug/deps" "-L" "/home/vincent_dalmaso/projects/playground/rust-clippy/target/debug/test_build_base/crate_level_checks/no_std_main_recursion.stage-id.aux" "-L" "/home/vincent_dalmaso/.rustup/toolchains/master/lib/rustlib/x86_64-unknown-linux-gnu/lib" "-Wl,-Bstatic" "/home/vincent_dalmaso/.rustup/toolchains/master/lib/rustlib/x86_64-unknown-linux-gnu/lib/librustc_std_workspace_core-401af6db50fd190a.rlib" "/home/vincent_dalmaso/.rustup/toolchains/master/lib/rustlib/x86_64-unknown-linux-gnu/lib/libcore-f8a897fd6423a31c.rlib" "/home/vincent_dalmaso/.rustup/toolchains/master/lib/rustlib/x86_64-unknown-linux-gnu/lib/libcompiler_builtins-528852add6ef9c91.rlib" "-Wl,-Bdynamic"
   = note: /usr/lib/gcc/x86_64-linux-gnu/7/../../../x86_64-linux-gnu/Scrt1.o: In function `_start':
           (.text+0x12): undefined reference to `__libc_csu_fini'
           (.text+0x19): undefined reference to `__libc_csu_init'
           (.text+0x26): undefined reference to `__libc_start_main'
           /home/vincent_dalmaso/projects/playground/rust-clippy/target/debug/test_build_base/crate_level_checks/no_std_main_recursion.no_std_main_recursion.7rcbfp3g-cgu.1.rcgu.o: In function `core::sync::atomic::atomic_store':
           no_std_main_recursion.7rcbfp3g-cgu.1:(.text._ZN4core4sync6atomic12atomic_store17hbb36836c0316fcaeE+0x53): undefined reference to `_Unwind_Resume'
           collect2: error: ld returned 1 exit status

where my test look like this:

#![feature(lang_items, start, libc)]
#![no_std]

use core::panic::PanicInfo;
use core::sync::atomic::{AtomicUsize, Ordering};

static N: AtomicUsize = AtomicUsize::new(0);

#[allow(unconditional_recursion)]
#[start]
fn main(argc: isize, argv: *const *const u8) -> isize {
    let x = N.load(Ordering::Relaxed);
    N.store(x + 1, Ordering::Relaxed);

    if x < 3 {
        main(argc, argv);
    }

    0
}

#[allow(clippy::empty_loop)]
#[panic_handler]
fn panic(_info: &PanicInfo) -> ! {
    loop {}
}

#[lang = "eh_personality"]
extern fn eh_personality() {}

[flip1995]

See rust-lang/rust#17346, especially the last 2 comments. Please try this and report back here. (Your code get's build successfully on the playground)

[Urriel]

#![feature(lang_items, link_args, start, libc)]
#![link_args="-nostartfiles"]

Those lines did the trick.
Thanks

[Urriel]

I have started to get crate symbols checked:

impl EarlyLintPass for MainRecursion {
    fn check_crate(&mut self, _cx: &EarlyContext<'_>, krate: &Crate) {
        self.has_no_std = false;
        for attr in krate.attrs.iter() {

            if attr.path == sym!(no_std) {
                self.has_no_std = true;
            }
        }
    }
}

This code seems to works.
On the other hand, I have trouble to check for a call to the main function. Using the sym!(main) does not seems to work as intended.

    fn check_expr(&mut self, _: &EarlyContext, expr: &Expr) {
        // if !self.has_no_std {
            match expr.node {
                ExprKind::Call(ref method, _) => {
                    dbg!(method);
                },
                _ => {}
            };
        // }
    }

I get [clippy_lints/src/main_recursion.rs:41] method = expr(16: main), so I guess, I am on the right path.

[flip1995]

The first function can be done more efficient with an iterator:

self.has_no_std_attr = krate.attrs.iter().any(|attr| attr.path == sym::no_std);

This should work:

if_chain! {
    if let ExprKind::Call(func) = &expr.node;
    if let ExprKind::Path(_, path) = &func.node;
    if path == sym::main;
    then {
        // report your lint here
    }
}

https://doc.rust-lang.org/nightly/nightly-rustc/syntax/symbol/sym/index.html

[Urriel]

self.has_no_std_attr = krate.attrs.iter().any(|attr| attr.path == sym::no_std);

So I don't need to use the sym! macro ?

if_chain! {
    if let ExprKind::Call(func) = &expr.node;
    if let ExprKind::Path(_, path) = &func.node;
    if path == sym::main;
    then {
        // report your lint here
    }
}

I learned something new today.

[flip1995]

I kind of lost track about the symbol back and forth in rustc/Clippy. But IIUC since no_std and main is already in syntax::symbol::sym you can use it directly. If it doesn't work, just use the macro. 👍

This is pretty much the output of the #[clippy::author] attribute. A really useful tool for writing lints :) You can try it out, by running Clippy on this Code in the Playground