Hi Victor, Thanks for your answer. Please, keep all this public, keep the CC.
I don’t have time right time to answer in details, I’ll do that during the weekend. I also want to wrap 3.1 with what we have now. Thanks! > Le 19 août 2018 à 22:11, Victor Khomenko <[email protected]> a > écrit : > > Hi Akim, > >>> I did look at it - this example scared me off c++ parsers :-( >> >> Really??? Then I failed. Can you be more specific? > > Ok, I'll try to make some suggestions for the manual and bison. Please note > that much of what I say might be due to lack of understanding - I'm just a > regular bison user, and don't understand the skeletons, internals, etc. The > context is that I have 8 bison C-style pure parsers in my program, most of > them are very simple, but a couple are fairly complicated, including a glr > one. > > 1. The C++ section in the bison manual starts with the description of > generated files: position.hh, location.hh, stack.hh, file.hh and file.cc. > That's 5 generated files, so at this point many prospective users would just > run away screaming. I'd say the first three files are completely unnecessary > and can be dumped into file.hh (or even .cc) - note that the contents could > be in some bison::detail namespace, so that the types are exactly the same in > all parsers and the linker can merge the functions if they are declared as > inline but were not actually inlined. The work-around for multiple parsers > suggested in Sect. 10.1.3.3 (making one parser generate these includes in > master/ and the other parsers use them) is ugly and introduces unwanted > include and build dependencies (e.g. there could be two independent libraries > within a project, each using a parser; this technique would make them > dependent both for includes and the built order). > > I'd say file.hh can also be suppressed in simple cases, e.g. if one replaces > the generated class by a function (see below) - it's then possible to declare > the parser function at the point of use. > > 2. int yylex (semantic type* yylval, location type* yylloc, type1 arg1, ...) > [Method on parser] > In C++ one would usually pass the parameters by references rather than > pointers - also to avoid handling null-pointers in yylex. > > 4. calc++ is a rather complicated example, for warming up it would be better > to use something simple. Then calc++ would be good to illustrate various > intricacies like mutual #include dependencies and a flex scanner. > > 3. the destructor of calcxx_driver should not be virtual; in fact, the > automatically generated destructor should be ok, so no need to declare it at > all. The constructor is not necessary either - since C++11 one can give the > default initial values to data members: > std::map<std::string, int> variables={ {"one", 1}, {"two", 2} }; > trace_scanning=false; > trace_parsing=false; > Maybe some other functions within the class, like error(), can be defined at > the point of declaration. > > 4. int calcxx_driver::parse (const std::string& f); > This is confusing, as the parser also has the function with the same name, > but no parameters. Maybe call it parse_file? > > 5. scan_begin() and scan_end() could be removed to simplify the interface, > and inlined into parse(...). > > 6. implementing calcxx_driver::scan_begin() and calcxx_driver::scan_end() in > the lexer file is really bad - I guess one can easily implement them in the > calcxx_driver.cpp file - this does not cause any problems with #includes. > > 7. it would be better to use C++ streams rather than C-style fopen/fclose > (note that error() does use streams). > >> Note that you don’t have anything to do to >> reset the parser, all the problem is actually the scanner. > > That's good - I suggest to say this explicitly in the documentation. > > >> The parser object holds no state, everything is local to yyparse. > > That's a surprise to me! (and needs to be said in the manual) Well, this is > actually very nice, but also means that the parser class is used as a > namespace, so could/should be converted to one (or just dissolved, with its > content ejected into the surrounding yy namespace, thus shortening those long > multiply-qualified names), and parse() becomes just a function. The > user-declared parameters can then be passed to yyparse() directly, i.e. we > get something similar to the old style pure parsers. > > >> I’ll try to look at this section with a fresh eye for the following release, >> and fix >> these issues. Help will be deeply appreciated :) > > I hope some of the above could help, please ignore the silly bits. > > Cheers, > Victor. > > > > > > > > > >
