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

Ommiting "C" in extern "C" fn in derive input causes spans to get lost. #64561

Closed
rodrimati1992 opened this issue Sep 17, 2019 · 7 comments · Fixed by #66271
Closed

Ommiting "C" in extern "C" fn in derive input causes spans to get lost. #64561

rodrimati1992 opened this issue Sep 17, 2019 · 7 comments · Fixed by #66271
Assignees
Labels
A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) A-parser Area: The parsing of Rust source code to an AST. C-bug Category: This is a bug. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@rodrimati1992
Copy link
Contributor

For some reason ommiting the "C" in extern "C" fn causes the spans to get lost in error messages.

I've created this repository to demonstrate it,simply git clone it and try to build the using_proc_macro crate.

The proc_macro_crate crate:

extern crate proc_macro;

use proc_macro::TokenStream;

use syn::{
    visit::{self, Visit},
    Lifetime, Type, TypeBareFn, TypeReference,
};


#[proc_macro_derive(ProcMacro)]
pub fn stable_abi(input: TokenStream) -> TokenStream {
    syn::parse::<syn::DeriveInput>(input)
        .and_then(|di|{
            let mut res=Ok(());
            Visitor { err:&mut res }.visit_derive_input(&di);
            res.map(|_| quote::quote!() )
        })
        .unwrap_or_else(|e| e.to_compile_error() )
        .into()
}

pub(crate) struct Visitor<'a> {
    err:&'a mut Result<(),syn::Error>,
}

/////////////

impl<'a> Visit<'_> for Visitor<'a> {
    fn visit_type_reference(&mut self, ref_: &TypeReference) {
        *self.err=Err(syn::Error::new_spanned(ref_,format!("hello from proc macro")));
    }
    fn visit_lifetime(&mut self, lt: &Lifetime) {   
        *self.err=Err(syn::Error::new_spanned(lt,format!("hello from proc macro")));
    }
}

The using_proc_macro crate:

#[derive(proc_macro_crate::ProcMacro)]
struct Hello(
    &'a (),
    extern fn(),
);


#[derive(proc_macro_crate::ProcMacro)]
struct World(
    &'a (),
    extern "C" fn(),
);

The error message

error: hello from proc macro

error: hello from proc macro
  --> src/lib.rs:10:5
   |
10 |     &'a (),
   |     ^^^^^^

The first error is for Hello and the second is for World.

@jonas-schievink jonas-schievink added A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) A-parser Area: The parsing of Rust source code to an AST. C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Sep 17, 2019
@dtolnay
Copy link
Member

dtolnay commented Sep 17, 2019

Thanks! This is one example of a general problem of the compiler losing span information that is being tracked in #43081.

@dtolnay dtolnay closed this as completed Sep 17, 2019
@petrochenkov
Copy link
Contributor

This one is very easily fixable though, by changing Abi to Option<Abi> in AST.
(I'm going to reopen and mark as easy/mentor.)

@petrochenkov petrochenkov reopened this Sep 17, 2019
@petrochenkov petrochenkov added E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. labels Sep 17, 2019
@petrochenkov
Copy link
Contributor

ast.rs before:

pub struct BareFnTy {
    pub abi: Abi,
    // ...
}
pub struct ForeignMod {
    pub abi: Abi,
    // ...
}
pub struct FnHeader {
    pub abi: Abi,
    ...
}

ast.rs after:

pub enum Extern {
    None,
    Implicit,
    Explicit(Abi),
}

pub struct BareFnTy {
    pub ext: Extern,
    // ...
}
pub struct ForeignMod {
    pub abi: Option<Abi>,
    // ...
}
pub struct FnHeader {
    pub ext: Extern,
    ...
}

@jaredforth
Copy link

If no one else is taking this issue, I am up to work on it

@jaredforth
Copy link

@petrochenkov

I added the pub enum Extern to ast.rs without any issues, but when I added pub ext: Extern to pub struct BareFnTy the compiler complains because the struct signature has changed.

The errors are below:

error[E0308]: mismatched types
   --> src/libsyntax/feature_gate/check.rs:437:32
    |
437 |                 self.check_abi(bare_fn_ty.abi, ty.span);
    |                                ^^^^^^^^^^^^^^ expected enum `rustc_target::spec::abi::Abi`, found enum `std::option::Option`
    |
    = note: expected type `rustc_target::spec::abi::Abi`
               found type `std::option::Option<rustc_target::spec::abi::Abi>`

error[E0308]: mismatched types
   --> src/libsyntax/parse/parser/ty.rs:303:13
    |
303 |             abi,
    |             ^^^
    |             |
    |             expected enum `std::option::Option`, found enum `rustc_target::spec::abi::Abi`
    |             help: try using a variant of the expected type: `Some(abi)`
    |
    = note: expected type `std::option::Option<rustc_target::spec::abi::Abi>`
               found type `rustc_target::spec::abi::Abi`

error[E0308]: mismatched types
    --> src/libsyntax/print/pprust.rs:1006:34
     |
1006 |                 self.print_ty_fn(f.abi,
     |                                  ^^^^^ expected enum `rustc_target::spec::abi::Abi`, found enum `std::option::Option`
     |
     = note: expected type `rustc_target::spec::abi::Abi`
                found type `std::option::Option<rustc_target::spec::abi::Abi>`

Is the solution to change code in check.rs, ty.rs, and pprust.rs or is there an alternate solution that I am just missing?

@petrochenkov
Copy link
Contributor

@jaredforth
Yes, ABI uses outside of ast.rs need to be adjusted to use the new Option<Abi> and Extern.

@petrochenkov
Copy link
Contributor

Fixed in #66271.

Centril added a commit to Centril/rust that referenced this issue Nov 17, 2019
syntax: Keep string literals in ABIs and `asm!` more precisely

As a result we don't lose spans when `extern` functions or blocks are passed to proc macros, and also escape all string literals consistently.
Continuation of rust-lang#60679, which did a similar thing with all literals besides those in ABIs and `asm!`.

TODO: Add tests.

Fixes rust-lang#60493
Fixes rust-lang#64561
r? @Centril
@bors bors closed this as completed in b83d50d Nov 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) A-parser Area: The parsing of Rust source code to an AST. C-bug Category: This is a bug. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants