BNFC / bnfc

BNF Converter
http://bnfc.digitalgrammars.com/
582 stars 161 forks source link

[WIP] Issues/293 use new c++ feature and some fix #389

Closed hangingman closed 2 years ago

hangingman commented 2 years ago

ref #293

PR description: I modified some C++ code related with issue #293 . ~This is not so big change.~

PR contents

Sample code:

use using instead of typedef

/********************   TypeDef Section    ********************/

using Integer = int;
using Char = char;
using Double = double;
using String = std::string;
using Ident = std::string;

clone() returns instance wrapped std::unique_ptr

/********************   Abstract Syntax Classes    ********************/

class Program : public Visitable
{
public:
  virtual std::unique_ptr<Program> clone() const = 0;
};

public: // for "std::move" Prog(Prog&& rhs); Prog& operator=(Prog&& rhs); // for normal copy Prog(const Prog& rhs); Prog& operator=(const Prog& rhs); Prog(const ListStatement& p1); ~Prog(); virtual void accept(Visitor *v); std::unique_ptr clone() const override; };


- [WIP] Bison/Flex code genrator switching
  - Still work in progress, but code might require [Bison3.2](https://lwn.net/Articles/770138/)
  - I think I will modify process switching some code in bison

- Use smart pointer in test C++ source
  - This enables test.C to use unique_ptr, however this is just a example. I think this kind of smart-pointer can apply for other C++ code generated by bnfc also. To use smart pointer, I added `#include <memory>`.
  - will generate following code
```cpp
  if (parse_tree)
  {
    printf("\nParse Successful!\n");
    if (!quiet) {
      printf("\n[Abstract Syntax]\n");
      std::unique_ptr<ShowAbsyn> s(new ShowAbsyn());
      printf("%s\n\n", s->show(parse_tree));
      printf("[Linearized Tree]\n");
      std::unique_ptr<PrintAbsyn> p(new PrintAbsyn());
      printf("%s\n\n", p->print(parse_tree));
    }
    delete(parse_tree);
    return 0;
  }
hangingman commented 2 years ago

@andreasabel Hello, I have created PR for using std::unique_ptr on test.C as a first step.

andreasabel commented 2 years ago

@hangingman Thanks for the PR.

I am not sure what the impact would be to switch the C++ backend to C++11 and the unique_ptr style. Thus, I would like to preserve the old behavior under a flag. Do you think --ansi would be a good flag for the C++ backend to preserve the old output?

hangingman commented 2 years ago

@andreasabel

Thus, I would like to preserve the old behavior under a flag.

OK. However, how to switch new behavior and old behavior ?

Do you think --ansi would be a good flag for the C++ backend to preserve the old output?

Yes. According to the link of stackoverflow, -ansi flag represents -std=c++98 (ISO 1998 C++ standard) in C++ mode.

I could say it's obvious there is big difference between before c++11 and after c++11. So, if we need to preserve the old output, we can select -ansi (-std=c++98) or -std=c++03 (before c++11). I think -ansi is sufficient. Because it's working so far, and there are few people want to use c++03.

andreasabel commented 2 years ago

@hangingman:

However, how to switch new behavior and old behavior ?

I added a new BNFC option --ansi for the C++ backends in https://github.com/BNFC/bnfc/commit/d06ef0ab4dbac07c8151a14903716ae62cc8097e. You can case on ansi opts (cases Ansi and BeyondAnsi) in the places where you add the new behavior.

hangingman commented 2 years ago

@andreasabel OKay, I got it. I'll take a look it

andreasabel commented 2 years ago

Thanks for the updated PR, @hangingman ! I took a quick look an noticed that you have made a clone, CFtoSTLAbsBeyondAnsi, of CFtoSTLAbs. I inherited BNFC at a point where it had many clones and have worked hard since then to reduce the number of clones. Clones just make the whole project unmaintainable, as each change has to performed again-and-again, clone-for-clone. You may understand that I do not want to accept new clones in this repository, even though it still contains some clones (I have not eliminated all). So, instead of making a clone, the code in CFtoSTLAbs should be parametrized so that it does slightly different things, depending on whether flag --ansi was given or not.

hangingman commented 2 years ago

@andreasabel

So, instead of making a clone, the code in CFtoSTLAbs should be parametrized so that it does slightly different things, depending on whether flag --ansi was given or not.

I understand. But, I think it will be very different "ansi C++ code" and "beyond ansi C++ code". So I need to modify functions in CFtoSTLAbs.hs different code output, which is concretely prAbs, prCon, prList...

If we introduced flags to switch "ansi" and "not ansi" for each functions, it would be not easy to understand thing. So, I decided purposely divide CFtoSTLAbs modules as CFtoSTLAbsAnsi and CFtoSTLAbsBeyondAnsi. It's easy to understand and maybe maintainable (?). Haskell has module system it's equivalent to C++ namespace I think.

How do you think ? If you think it should be parametrized, please tell me where we should switch "ansi" and "not ansi" concretely.

andreasabel commented 2 years ago

@hangingman Thanks for the reply. I will have a closer look at the overlap and then answer you.

andreasabel commented 2 years ago

The two files have alone 127 identical lines:

$ comm -12 CFtoSTLAbsAnsi.hs CFtoSTLAbsBeyondAnsi.hs | wc
     127     545    3506

There is ~160 lines each that have deviated, but when I looked at the differences in an ediff session it seemed to me that these lines still share common parts and structure.

@hangingman

If you think it should be parametrized, please tell me where we should switch "ansi" and "not ansi" concretely.

A model can be https://github.com/BNFC/bnfc/blob/1c12bb1d790b06ae5c1814bf363e21268f3744d7/source/src/BNFC/Backend/C/CFtoBisonC.hs which I fused from three clones in PR #351. I added a parameter ParserMode to many functions to get different behavior for the C, C++, and C++/NoSTL backends. Function generateAction was so different that I kept two versions, ...C and ...STL. The other functions were similar enough so that one parametrized version would do.

What do you think? Can you give it a try?

hangingman commented 2 years ago

@andreasabel OK, I guess I know what you are trying to say. https://github.com/BNFC/bnfc/pull/389#issuecomment-948575244 I'll take a look CFtoBisonC.hs and try to refactor CFtoCPPAbs.hs.

andreasabel commented 2 years ago

@hangingman CI is passing. Let me know when the PR is ready again for review and testing.

hangingman commented 2 years ago

@andreasabel Thank you! Now I have added code related with AST, bison/flex part. It will take some time to fix rest of them (pritty-print and some). I will notify after that, please be patient and wait for a little while 👍

andreasabel commented 2 years ago

@hangingman : Happy new year 2022! How is the progress?

hangingman commented 2 years ago

@andreasabel Unfortunately my progress is not good. I don't have much time. I need to modify some code , it was unexpected. GitHub action is working on my forked repository https://github.com/hangingman/bnfc/pull/2

So I will resend pull-request after fixed code. Thanks!

hangingman commented 2 years ago

I close this PR to organize.