Improve generated CPP code to trace function calls

[agharish17]

I have worked on C++ programming for quite sometime and I understand the basics. I am very much new to compiler area and I found it very easy to get started with BNFC to generate a C++ Lexer, parser (additionally a Skeleton class for Visitor design pattern) and a makefile. However, I had some trouble to visualize the visitor pattern in the applied context as there was quite less documentation about it.

So, I prepared an UML diagram (not too accurate though, but fairly conveys the idea, the blue boxes are abstract classes). PFA. Can I provide this as a small contribution to the BNFC after it goes through a review ?

Also, I made the skeleton pattern work by adding minor code to the generated Test.C, Skeleton.H and Skeleton.C files. With this, it becomes more easier to trace the order in which different methods get called and becomes easy to proceed further with type-checking etc..

Since I am unable to upload SRC files, below is the summary: I have also attached the generated output in my following comment refer here

Skeleton.H: provide destructor and add a method to accept a parse-tree

public:
  void showSkeleton(Visitable *v);

Skeleton.C: just add print() statements to get a call trace!

void Skeleton::showSkeleton(Visitable *v)
{
  printf("%s()\n", __FUNCTION__);
  v->accept(this);
}

Test.C : instantiate skeleton and invoke showSkeleton() method by providing a parse-tree.

printf("Trying Skeleton\n");
      Skeleton* skeletonPtr = new Skeleton();
      skeletonPtr->showSkeleton(parse_tree);

Note: Since all the 3 files are generated automatically by running $bnfc -m --cpp <language-file>.cf

I was wondering if my above implementation can also be considered and included while generating the files ?

Is this acceptable ? If so, please mention the next steps (as I haven't contributed to any open-source projects till now).

[agharish17]

The attached file is the new output for a typical "Hello World" program.

[andreasabel]

Hello @agharish17, the current code for the skeleton methods looks like this:

void Skeleton::visitEAdd(EAdd *e_add)
{
  /* Code For EAdd Goes Here */

  if (e_add->exp_1) e_add->exp_1->accept(this);
  if (e_add->exp_2) e_add->exp_2->accept(this);
}

I think it would be ok to add an option to BNFC that inserts something like

printf("%s()\n", __FUNCTION__);

at the beginning of each such visit method, if that would help to understand the control-flow. (It is really just the standard visitor pattern, though: https://en.wikipedia.org/wiki/Visitor_pattern) This option could also instrument the generated test parser to pass the skeleton visitor to the parsed AST.

I am not sure about this suggestion:

void showSkeleton(Visitable *v);

Overall, my impression is somehow that your suggested changes would be good for the beginner, but could be annoying to a more senior user that has a good grip on the visitor pattern and then has to delete the extra print statements that only output the function name. Maybe instead of just printing the function name, also printing the arguments could be more useful in general.

Have you already implemented this?

I am unable to upload SRC files,

If yes, you can open a PR so I can form a more clear opinion. I can't make any promises at this stage.

[agharish17]

Hello @andreasabel Thank you so much for the answer. Yes, I have already implemented them on-top of the generated C++ files (i.e. modified the generated C++ files). And, I am not sure if I can put them in a PR as they are generated out of bnfc :| At the moment, I don't know where to implement them inside the bnfc repo. Please advise.

[andreasabel]

So the skeleton files are created in the backends, e.g. for C++: https://github.com/BNFC/bnfc/blob/742b9fb0ae78594936473a71738e0a90b53ff381/source/src/BNFC/Backend/CPP/STL/CFtoCVisitSkelSTL.hs

You will probably have to learn a bit about the BNFC code base to carry out the change.
Grepping for the usage of a function/constructor/field is often the way to learn its purpose.

In general, please understand that I am not available for a supervision-intensive process here, as I think the proposed feature isn't in the core functionality of BNFC.

[agharish17]

Thank you for the hints. So, I feel that it will take sometime to understand and contribute. So, I would pick this task during summer when I will have enough time :)

I hope that is fine (as its also not really a core functionality of BNFC). Maybe this ticket / task can be put as on-hold or something until I take it up again.