bnfc icon indicating copy to clipboard operation
bnfc copied to clipboard

Use smart-pointer and generate C++ code for bison/flex

Open hangingman opened this issue 2 years ago • 10 comments

ref https://github.com/BNFC/bnfc/issues/293

PR description: I modified large amount of source code related with issue #293 . I would like to take a review. If there are any problems in this PR, please point them out. And If it's OK, tell me what unit tests should be added.

PR Summary

  • bnfc

    • Changes on Makefile and file extension
    • Switching C++ AST/Test code generator by flag
    • use using instead of typedef
    • Introduce std::shared_ptr
    • Modified Bison/Flex code genrator
  • bnfc-system-tests

    • Make enable filtering of bnfc-system-tests

PR contents

  • Makefile and file extension changes:

    • If bnfc not received --ansi flag, Makefile would use -std=c++14 compile option otherwise use --ansi flag.
    • If bnfc not received --ansi flag, bnfc will generate .cc, .hh, .yy, .ll files for bison/flex. In the nutshell, it represents C++ things.
  • C++ AST/Test code generator switching

    • If bnfc not received --ansi flag, bnfc would generate codes using smart-pointer (std::shared_ptr), otherwise bnfc generate codes same as before.
  • use using instead of typedef

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

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

using Hex = std::string;
using Label = std::string;
  • Introduce std::shared_ptr, like following:
    • So far, just my personal opinion, bnfc generated C++ code requires users to recursively destroy created instances after parsed AST. Because, AST classes hold new-ed class instances and there is no code to release those instances. (... but it was not so required because after parsed something and exited the program, memory will be released by OS. So I wrote it as a my personal opinion.)
    • After introduced std::unique_ptr, it's not necessary to release memory manually. It's convenient. (std::unique_ptr is not copy-able. This is not good for storing parsed AST and returning it. On the other hand, std::shared_ptr is copy-able , furthermore it will be automatically released after their reference count will be 0. So I introduced shared_ptr. We can find several implmentations using std::shared_ptr as bison's semantic type in the world GitHub. So, this is common implementation.)
/********************   Abstract Syntax Classes    ********************/

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

};

...

class Prog : public Program
{
public:
    std::shared_ptr<ListStatement> liststatement_;

    Prog(std::shared_ptr<ListStatement> p1)
    : Program(), liststatement_{p1}
    {};


    virtual void accept(Visitor *v) override;
    std::shared_ptr<Program>  clone() const override;
};
  • I modified Impl class as simple.
void Prog::accept(Visitor *v)
{
    v->visitProg(this);
}

std::shared_ptr<Program> Prog::clone() const
{
    return std::make_shared<Prog>(*this);
}
  • Bison/Flex code genrator modified:
    • To generate C++ code by bison, I selected lalr1.cc as skeleton.
    • Use variant instead of union. It requires bison >= 3.2.
    • bnfc will generate "Driver" class to handle parser/lexer classes. It will hold parsed AST as their field. I referred Flex and Bison in C++ to implement it.

bison's yy file will be like following; You can find bison's yy file using variant, and shared_ptr.

/* Generate header file for lexer. */
%defines "bison.hh"
%define api.namespace {Xxxx}
/* Specify the namespace for the C++ parser class. */

/* Reentrant parser */
/* lalr1.cc always pure parser. needless to define %define api.pure full */

%define api.parser.class {XxxxParser}
%code top {
#include <memory>
}
%code requires{
#include "absyn.hh"

    namespace Xxxx {
        class XxxxScanner;
        class XxxxDriver;
    }
}
%parse-param { XxxxScanner  &scanner  }
%parse-param { XxxxDriver  &driver  }

%locations
/* variant based implementation of semantic values for C++ */
%require "3.2"
%define api.value.type variant
/* 'yacc.c' does not support variant, so use skeleton 'lalr1.cc' */
%skeleton "lalr1.cc"

%code{
/* Begin C++ preamble code */
#include <algorithm> /* for std::reverse */
#include <iostream>
#include <cstdlib>
#include <fstream>

/* include for all driver functions */
#include "driver.hh"

#undef yylex
#define yylex scanner.lex
}


%token              _ERROR_

...

%type <std::shared_ptr<Program>> Program
%type <std::shared_ptr<ListStatement>> ListStatement

%start Program

%%

Program : ListStatement { $1->reverse();$$ = std::make_shared<Prog>($1); driver.program_ = $$; }
;
ListStatement : Statement { $$ = std::make_shared<ListStatement>(); $$->cons($1); driver.liststatement_ = $$; }
  | Statement ListStatement { $2->cons($1); $$ = $2; driver.liststatement_ = $$; }
  • I modified bnfc to generate entrypoint has std::istream &stream as their parameters. Because it can handle file contents and stdin data. Entrypoints will be generated in "Driver" class.
namespace Xxxx{

class  XxxxDriver{
public:

...
    std::shared_ptr<Program> pProgram(std::istream &stream);
  • Use smart pointer in test C++ source
    • Finally, we can see smart-pointer edition of Test.cc ( Compiler Front-End Test )
int main(int argc, char ** argv)
{
    int quiet = 0;
    char *filename = NULL;

    if (argc > 1) {
        if (strcmp(argv[1], "-s") == 0) {
            quiet = 1;
            if (argc > 2) {
                filename = argv[2];
            }
        } else {
            filename = argv[1];
        }
    }

    /* The default entry point is used. For other options see Parser.H */
    std::shared_ptr<Program> parse_tree = nullptr;
    try {

        auto driver = std::make_unique<Xxxx::XxxxDriver>();
        if (filename) {
            std::ifstream input(filename);
            if ( ! input.good() ) {
                usage();
                exit(1);
            }
            parse_tree = driver->pProgram(input);
        } else {
            parse_tree = driver->pProgram(std::cin);
        }

    } catch( parse_error &e) {
        std::cerr << "Parse error on line " << e.getLine() << "\n";
    }

    if (parse_tree)
    {
        printf("\nParse Successful!\n");
        if (!quiet) {
            printf("\n[Abstract Syntax]\n");
            auto s = std::make_unique<ShowAbsyn>(ShowAbsyn());
            printf("%s\n\n", s->show(parse_tree.get()));
            printf("[Linearized Tree]\n");
            auto p = std::make_unique<PrintAbsyn>(PrintAbsyn());
            printf("%s\n\n", p->print(parse_tree.get()));
      }

      return 0;
    }
    return 1;
}
  • Make enable filtering of bnfc-system-tests ( unit tests filtering by using HTF )
    • By using HTF PATTREN , -n PATTERNoption, make enable filtering of bnfc-system-tests
    • HTF documents: https://hackage.haskell.org/package/HTF-0.15.0.0/docs/Test-Framework-Tutorial.html#g:9

hangingman avatar Jan 29 '22 14:01 hangingman

Thanks for the update, @hangingman ! I will have a look soon, but I decided to make 2.9.4 a conservative release, and only then merge new features.

andreasabel avatar Feb 08 '22 17:02 andreasabel

Thanks @hangingman for the PR!
Please make sure the tests still work. You can run the testsuite from the /testing directory:

/testing$ make

I pushed a commit that puts the C++ tests first.

At the moment, they error if flag -l is supplied (which adds position information).

Also, the plain --ansi version fails to compile:

Calc.y:40:33: error: typedef redefinition with different types ('struct yy_buffer_state *' vs
      'struct calc__buffer_state *')
typedef struct yy_buffer_state *YY_BUFFER_STATE;

(This is for bnfc -m --cpp --ansi /examples/Calc.cf && make.)

andreasabel avatar Mar 03 '22 10:03 andreasabel

@andreasabel Thank you !

At the moment, they error if flag -l is supplied (which adds position information).

Got it. I'll look into it.

Also, the plain --ansi version fails to compile:

OK, I will fix it.

hangingman avatar Mar 03 '22 11:03 hangingman

Looks like Haskell-CI is currently broken because of

  • #416.

I'll fix a push to master and then this PR can be rebased onto master.

andreasabel avatar Mar 13 '22 11:03 andreasabel

@andreasabel Oh, I got it.

hangingman avatar Mar 13 '22 12:03 hangingman

I pushed the fix for #416 to master just now.

andreasabel avatar Mar 13 '22 12:03 andreasabel

Sorry broke the stack build by accident, but now both builds should work. (Fixed in #417.)

andreasabel avatar Mar 13 '22 15:03 andreasabel

@andreasabel CI is passed. But, I realized test cases under the testing directory is failing. I'm checking it.

working on this PR https://github.com/hangingman/bnfc/pull/5

hangingman avatar Mar 22 '22 03:03 hangingman

@andreasabel Hi, I make it passed all C/C++ system tests. https://github.com/hangingman/bnfc/pull/5#issue-1178753984 I think bnfc code in this branch can generate decent C++ code. Please take a look it.

hangingman avatar May 03 '22 10:05 hangingman

@andreasabel @VBeatrice Hi, I have already tried this code in my project and it works very well. Could you please tell me when you will be able to merge this pull request? Thank you for your time and attention.

tokomine avatar Aug 31 '23 07:08 tokomine