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

Argument names in foreign functions, function types and trait methods are not resolved #33995

Open
petrochenkov opened this issue May 31, 2016 · 3 comments
Labels
A-resolve Area: Path resolution C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@petrochenkov
Copy link
Contributor

petrochenkov commented May 31, 2016

Arguments in foreign functions, function types and traits are pattern bindings like all other function arguments and supposedly they should be resolved in the same way, however they are not resolved at all.
It doesn't make much difference because these bindings can't be used in function bodies, however it makes some difference, for example foreign functions can reuse constant names for their arguments, while other functions can't (whether it's good or bad is a separate question).

Accidentally, some function arguments being unresolved also affect lints like non_snake_case, such lints don't report warnings for them. This may be reasonable for foreign functions though, because such functions may follow foreign naming conventions. If this lint is "fixed" it may affects crates like winapi (cc @retep998). This is not so reasonable for trait methods however.

Examples:

#![allow(dead_code, unused_variables)]

const SOD: u8 = 0;
const SOB: u8 = 0;
const MOD: u8 = 0;
const MDC: u8 = 0;
const DRI: u8 = 0;

// warning: variable `SOD` should have a snake case name
// error: let variables cannot be named the same as const variables
fn f(SOD: u8) {}

trait Tr {
    // warning: variable `SOB` should have a snake case name
    // error: let variables cannot be named the same as const variables
    fn g(SOB: u8) {}

    // No warnings, no errors
    fn h(MOD: u8);
}

// No warnings, no errors
type A = fn(MDC: u8);

extern "C" {
    // No warnings, no errors
    fn k(DRI: u8);
}

fn main() {
}
@retep998
Copy link
Member

I'd appreciate if the lints did fire for foreign functions (winapi sets those lints to allow anyway so it doesn't affect it). However being unable to use identifiers for function parameters when there is a constant with the same name is rather dumb in my opinion, and I'm opposed to extending that to cover foreign functions.

@petrochenkov
Copy link
Contributor Author

petrochenkov commented Jun 2, 2016

This also means arguments with same names are allowed:

#![allow(dead_code, unused_variables)]

// error: identifier `a` is bound more than once in this parameter list
fn f(a: u8, a: f64) {}

trait Tr {
    // error: identifier `a` is bound more than once in this parameter list
    fn g(a: u8, a: f64) {}

    // No errors
    fn h(a: u8, a: f64);
}

// No errors
type A = fn(a: u8, a: f64);

extern "C" {
    // No errors
    fn k(a: u8, a: f64);
}

fn main() {
}

@retep998
Copy link
Member

retep998 commented Jun 2, 2016

I'm also okay with that being fixed.

@petrochenkov petrochenkov added the A-resolve Area: Path resolution label Feb 19, 2017
@Mark-Simulacrum Mark-Simulacrum added the C-bug Category: This is a bug. label Jul 25, 2017
@Nilstrieb Nilstrieb added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Apr 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-resolve Area: Path resolution C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants