Rust-GCC / gccrs

GCC Front-End for Rust
https://rust-gcc.github.io/
GNU General Public License v2.0
2.38k stars 153 forks source link

Suffixed literals with unknown suffix not accepted #2662

Open P-E-P opened 1 year ago

P-E-P commented 1 year ago

GCCRS lexer refuses literals with an unknown suffixes.

I tried this code:

-frust-compile-until=ast

extern crate test_pm;
use test_pm::my_macro;

my_macro! {
    0q
    1.0q
}

I expected to see this happen: Parse correctly.

Instead, this happened:

./test.rs:5:5: error: unknown number suffix ‘q’
    5 |     0q
      |     ^
./test.rs:6:5: error: unknown number suffix ‘q’
    6 |     1.0q
      |     ^

Meta

mvvsmk commented 11 months ago

hey @P-E-P , I am not sure how I should go about it, because the rustc also catches this as a lexical error. :3

CohenArthur commented 11 months ago

@mvvsmk with the following code:

fn foo() {
    let a = 15q;
}

and running rustc with -Z parse-only, we don't get any error. so the parsing is correct, but rustc has an extra AST validation step that checks that the suffix is correct.

I'm not sure if this is overkill and if we should just have a parsing error. This is probably useful for custom suffixes and future suffix developments in Rust, but we won't have changes like these in gccrs - we'll only reflect what rustc is doing

P-E-P commented 11 months ago

@mvvsmk with the following code:

fn foo() {
    let a = 15q;
}

and running rustc with -Z parse-only, we don't get any error. so the parsing is correct, but rustc has an extra AST validation step that checks that the suffix is correct.

I'm not sure if this is overkill and if we should just have a parsing error. This is probably useful for custom suffixes and future suffix developments in Rust, but we won't have changes like these in gccrs - we'll only reflect what rustc is doing

Not only custom suffixes, procedural macros are impacted by this behavior.

mvvsmk commented 10 months ago

@CohenArthur I'll start working on this could you assign this to me.

mvvsmk commented 9 months ago

@P-E-P could you hellp me out a bit https://github.com/Rust-GCC/gccrs/blob/3fcd86e404cac6763e40ca032aff942a3da09666/gcc/rust/lex/rust-lex.cc#L1221-L1227 just removing the rust_error_at would remove the error , is there any specific info you would like me to return in the case of an unkown suffix?

P-E-P commented 9 months ago

just removing the rust_error_at would remove the error , is there any specific info you would like me to return in the case of an unkown suffix?

Tbh it's been a while since I've created this issue. IIRC this was because proc macros content should be relaxed and the suffixes should be rejected later (when ? during ast validation ?).

You'll have to rework the token structure in order to store custom suffixes. From here I see two ways of doing it:

  1. Revamp the whole token system to accommodate custom suffixes as well as standard suffixes in an unified way
    • A lot of work
    • Expect a lot of things to break during the refactor
    • Is this really a better way ?
  2. Extend the current token implementation with a new pointer field for custom suffixes. A new coretype to differentiate those from unsuffixed literals (CORETYPE_CUSTOM_SUFFIX ?)
    • Easier to do
    • add a member to an enum
    • add a getter to retrieve the suffix from the associated text, we probably want to avoid duplicating the suffix as it is already stored and tokens should be kept slim
      • a pointer/reference_wrapper to the suffix position would be ok
      • but processing the suffix position everytime might be good too, the getter probably won't be called that much.
    • change the parser to build those types
    • You should be careful, some components might silently fail after that

I would probably go with solution 2 for now, it'll be easier and such a refactor could be done later anyway.

Do not hesitate to ask more questions about this, it's been a while and I may have forgotten some details.

mvvsmk commented 9 months ago

Thanks for the detailed response. I'll try and implement solution 2 and will ping you if I run into something odd. Most likely the thing that seems difficult is to catch the things that might fail silently, but I'll get to that when I am done with a rough solution.