Produits Entr'ouvert » Lasso

I'm ok to remove it, and create a --pedantic option instead.

Adding --pedantic to enable -Werror sounds like a good approach to me, it separates two concepts (debugging and strictness) that got co-mingled. At some point the warnings should be looked at, I assume since -Werror has been a part of --debugging for a while it implies there was a recent point in time where there were no warnings so it shouldn't be too hard.

On an almost unrelated note, I've got a number patches prepared to support using Python3 during the build. It appears as if although the generated Python binding had been ported to Py3 other build scripts and the automake files were not ready to handle Py3. It was while I was working on getting the Py3 build working that I tried to turn on debugging and discovered it failed. My quick and dirty interim solution was to just temporarily remove -Werror from the CFLAGS and I was able to move on.

Hi Benjamin:

I think there are a couple of issues with the patch or perhaps I'm not in sync with what you were trying to achieve, so here's my questions:

Why is there any connection between debugging and the pedantic option? The two seem independent to me. In one case I want to produce a build I can run under the debugger and in the other case I want to see if the compiler can help me diagnose coding mistakes. I don't see any correlation between the two.

There was already an enable pedantic option and now it looks like you're processing that arg twice.

if test "z$enable_pedantic" = "zyes" ; then
enable_pedantic=yes

Isn't the assignment of 'yes' to enable_pedantic a redundant no-op here? The assignment is inside a conditional block that has confirmed that enable_pedantic is already 'yes'.

+ if test "z$enable_debugging" "!=" "zyes" ; then
+ LASSO_DEFINES="$LASSO_DEFINES -DLASSO_DEBUG"
+ CFLAGS="$CFLAGS -O0 -g -Wall -Wextra"
+ fi

This seems to enforce the requirement that if you enable pedantic you must compile with the option to produce a debuggable build. Is this something gcc requires? If not why are we enforcing this?

I'm not even sure -Wall and -Wextra should be turned on by saying I want a build that I can run under gdb, but I'm OK with it, I can see the value in it but on the other hand I prefer options that do not have unrelated side-effects.

John Dennis a écrit :

Hi Benjamin:

I think there are a couple of issues with the patch or perhaps I'm not in sync with what you were trying to achieve, so here's my questions:

Why is there any connection between debugging and the pedantic option? The two seem independent to me. In one case I want to produce a build I can run under the debugger and in the other case I want to see if the compiler can help me diagnose coding mistakes. I don't see any correlation between the two.

There was already an enable pedantic option and now it looks like you're processing that arg twice.

if test "z$enable_pedantic" = "zyes" ; then
enable_pedantic=yes

Isn't the assignment of 'yes' to enable_pedantic a redundant no-op here? The assignment is inside a conditional block that has confirmed that enable_pedantic is already 'yes'.

+ if test "z$enable_debugging" "!=" "zyes" ; then
+ LASSO_DEFINES="$LASSO_DEFINES -DLASSO_DEBUG"
+ CFLAGS="$CFLAGS -O0 -g -Wall -Wextra"
+ fi

This seems to enforce the requirement that if you enable pedantic you must compile with the option to produce a debuggable build. Is this something gcc requires? If not why are we enforcing this?

I'm not even sure -Wall and -Wextra should be turned on by saying I want a build that I can run under gdb, but I'm OK with it, I can see the value in it but on the other hand I prefer options that do not have unrelated side-effects.

Finally I choosed to simply remove -Werror from the debugging CFLAGS, so all is well.