Implement context validation on parse #2336
Conversation
1b1305f to
18a2b16
Compare
|
Rebased on master. |
include/rbs/parser.h
Outdated
| bool no_void_allowed_here; /* If true, `void` type is not allowed, but it's allowed in one depth.*/ | ||
| bool no_self; /* If true, `self` type is not allowed.*/ | ||
| bool 8000 no_classish; /* If true, `class` or `instance` types are not allowed.*/ | ||
| } rbs_type_validation_t; |
There was a problem hiding this comment.
We might have a better name for this struct. It was a validation but now it's more tightly coupled with the parser. Something like rbs_type_parsing_option_t would be good (while it may look like too generic name...)
| * */ | ||
| typedef struct { | ||
| bool no_void; /* If true, `void` type is not allowed.*/ | ||
| bool no_void_allowed_here; /* If true, `void` type is not allowed, but it's allowed in one depth.*/ |
There was a problem hiding this comment.
no_void_allowed_here looks a bit too cryptic to me. Wondering if we can have a better name...
But, I'm wondering if we can merge the two void options. (I know it was from the original validation code, so I mean the original implementation was bad...)
- Parsing function return types allows direct
voidtypes, - Parsing generics arguments allows direct
voidtypes, - Anything else prohibits
voidtypes (right?)
I'm assuming we can implement this with only one flag.
There was a problem hiding this comment.
I agree with you, but merging the two void options would likely result in an implementation that is no simpler—or potentially more complex—than the current one. Here are the two ideas I’ve come up with:
- Option 1: Make
no_voida type that can represent three states instead of just abool. - Option 2: Add a member to the parser to retain context during parsing.
Neither of these seems like a particularly good idea.
|
Thank you for reviewing. |
8f5dda9 to
4f7e063
Compare
|
I only renamed |
Contextual limitations are now performed at parse time.
This change was originally planned for version 3.4 but remained unaddressed.
I propose introducing it with the release of version 4.0.
Changes
Parser.parse_signature.RBS::UnitTest, they have not been removed in this PR.--[no-]exit-error-on-syntax-erroroption.RBS::ParsingErrorinstead ofRBS::WillSyntaxError.RBS::WillSyntaxError.rbs validatecommand have been stopped.CONST: selfin therbs prototype rb.Not changes
RBS::ParsingErroris raised for compatibility with Steep. It may be possible to use a different error name.Parser.parse_typeandParser.parse_method_typedo not currently perform checks. It might be worth adding them.has_self_type?,has_classish_type?, andwith_nonreturn_void?are used inRBS::UnitTest, they have not been removed in this PR.