Shopify / rbs_parser

Ruby RBS parsing and translation to Sorbet RBI
MIT License
27 stars 4 forks source link

parser: rely on Bison to type symbols #18

Closed akimd closed 4 years ago

akimd commented 4 years ago

Hi!

I believe these changes will make the grammar easier to maintain. If this commit accepted, I can provide more changes to simplify it further. Unfortunately this big change cannot be broken into pieces: type-checking in Bison is all or nothing, I can't be incremental.

Currently "typing" is done by hand in the actions:

namespace
  : %empty
  { $$.string = new std::string(""); }
  | kCOLON2 tNAMESPACE
  { $$.string = new std::string(*$1.string + *$2.string); }

This can be simplified using %type.

namespace
  : %empty
  { $$ = new std::string(""); }
  | kCOLON2 tNAMESPACE
  { $$ = new std::string(*$1 + *$2); }

Besides, it will help migrating to Bison variants to use genuine objects, instead of pointers to object:

namespace
  : %empty
  { $$ = ""; }
  | kCOLON2 tNAMESPACE
  { $$ = $1 + $2; }
akimd commented 4 years ago

Hi! I signed it. It does seem to have changed anything here, yet.

Morriar commented 4 years ago

I reran the CLA check, everything's ok 👍

Morriar commented 4 years ago

Thanks you!