gccrs icon indicating copy to clipboard operation
gccrs copied to clipboard

Suffixed literals with unknown suffix not accepted

Open P-E-P opened this issue 2 years ago • 7 comments

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

  • c7728c8020334fecb47789163ba94326aab6208c

P-E-P avatar Oct 08 '23 15:10 P-E-P

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

mvvsmk avatar Nov 13 '23 10:11 mvvsmk

@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

CohenArthur avatar Nov 15 '23 14:11 CohenArthur

@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.

P-E-P avatar Nov 15 '23 14:11 P-E-P

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

mvvsmk avatar Dec 18 '23 10:12 mvvsmk

@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?

mvvsmk avatar Jan 15 '24 12:01 mvvsmk

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 ?
  1. 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.

P-E-P avatar Jan 15 '24 13:01 P-E-P

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.

mvvsmk avatar Jan 15 '24 15:01 mvvsmk