BNFC / bnfc

BNF Converter
http://bnfc.digitalgrammars.com/
586 stars 165 forks source link

Fix invalid conversion from string constant to "char *". #195

Closed Teemperor closed 6 years ago

Teemperor commented 7 years ago

Compiling BNFC generated code with clang triggers the warning ISO C++ forbids converting a string constant to ‘char*’ (see -Wwrite-strings) when calling the render function with a string const like this render("return");. As render() is called quite often in the pretty-printer, this results in bloated build-logs.

This PR fixes this by adding the missing qualifier.

Disclaimer: I haven't tested this PR myself because I wasn't able to get the testsuite running on Arch.

gdetrez commented 7 years ago

Thanks for that. I'm trying to add a regression test to make sure that we don't re-introduce this particular problem. It seems that there is a similar problem that remains in other modules. My test is simply running bnfc on the example grammar and then building the test program with clang. For example with the C example grammar:

$ bnfc --cpp -m examples/C/C.cf
$ make CC=clang++  CCFLAGS='-Wwrite-strings -Werror'

With this C example (and a few others) I still get the following kind of errors:

clang++ -Wwrite-strings -Werror -c Lexer.C 
C.l:134:18: error: conversion from string literal to 'char *' is deprecated [-Werror,-Wc++11-compat-deprecated-writable-strings]
YY_BUFFER_APPEND("\n"); BEGIN STRING;

C.l:135:18: error: conversion from string literal to 'char *' is deprecated [-Werror,-Wc++11-compat-deprecated-writable-strings]
YY_BUFFER_APPEND("\""); BEGIN STRING ;
...

I'm not really a C++ expert, do you think you could take a look? Otherwise I will of course accept your PR but I don't have an easy way to add a regression test for it.

gdetrez commented 7 years ago

I run the existing regression tests on your changes and it seems that it broke the C++ no STL backend with gcc, here's a example output again with the C example grammar, using -w to suppress the (many) warnings:

$ bnfc --cpp-nostl -m -examples/C/C.cf
$ make CCFLAGS=-w                                                                                                                                                                 make: Entering directory '/home/gregoire/src/bnfc/source/tmp'
g++ -w -c Absyn.C
flex -oLexer.C C.l
g++ -w -c Lexer.C 
bison C.y -o Parser.C
C.y: warning: 1 shift/reduce conflict [-Wconflicts-sr]
g++ -w -c Parser.C
g++ -w -c Printer.C
Printer.C:61:6: error: prototype for ‘void PrintAbsyn::render(const char*)’ does not match any in class ‘PrintAbsyn’
 void PrintAbsyn::render(const char *s)
      ^~~~~~~~~~
In file included from Printer.C:4:0:
Printer.H:23:8: error: candidates are: void PrintAbsyn::render(String)
   void render(String s);
        ^~~~~~
Printer.C:9:6: error:                 void PrintAbsyn::render(Char)
 void PrintAbsyn::render(Char c)
      ^~~~~~~~~~
Makefile:42: recipe for target 'Printer.o' failed
make: *** [Printer.o] Error 1
make: Leaving directory '/home/gregoire/src/bnfc/source/tmp'
Teemperor commented 7 years ago

Fixed the compilation error with -nostl now. I'll open separate PRs for the other warnings.