Skip to content

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

Merged
andreasabel merged 2 commits intoBNFC:masterfrom
Teemperor:master
Jan 1, 2018
Merged

Fix invalid conversion from string constant to "char *".#195
andreasabel merged 2 commits intoBNFC:masterfrom
Teemperor:master

Conversation

@Teemperor
Copy link
Copy Markdown
Contributor

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.

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 literal like this `render("return");`.
This commit fixes this by adding the missing qualifier.
@gdetrez
Copy link
Copy Markdown
Contributor

gdetrez commented Nov 18, 2016

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
Copy link
Copy Markdown
Contributor

gdetrez commented Nov 18, 2016

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
Copy link
Copy Markdown
Contributor Author

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

@gdetrez gdetrez self-assigned this Nov 25, 2016
@andreasabel andreasabel added this to the 2.8.2 milestone Jan 1, 2018
@andreasabel andreasabel assigned andreasabel and unassigned gdetrez Jan 1, 2018
@andreasabel andreasabel merged commit e8107a0 into BNFC:master Jan 1, 2018
andreasabel added a commit that referenced this pull request Jan 1, 2018
Issue introduced by PR #195, commit e8107a0
@andreasabel andreasabel added the C++ label Jan 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants