gnucash unstable: Multiple changes pushed

[Geert Janssens]

Op vrijdag 29 december 2017 17:22:29 CET schreef John Ralls:
> > On Dec 29, 2017, at 7:41 AM, Geert Janssens <geert.gnucash at kobaltwit.be>
> > wrote:> 
> > Op woensdag 27 december 2017 00:37:19 CET schreef John Ralls:
> >> commit 91727525b9cd3c12f4fa25268131b0161c6fc79e
> >> Author: John Ralls <jralls at ceridwen.us>
> >> Date:   Tue Dec 26 13:01:50 2017 -0800
> >> 
> >>    Enforce -Werror on C++ files and fix resulting errors.
> > 
> > <snip>
> > 
> >> diff --git a/gnucash/import-export/csv-imp/assistant-csv-trans-import.cpp
> >> b/gnucash/import-export/csv-imp/assistant-csv-trans-import.cpp index
> >> 9402f38..1bfa08e 100644
> >> --- a/gnucash/import-export/csv-imp/assistant-csv-trans-import.cpp
> >> +++ b/gnucash/import-export/csv-imp/assistant-csv-trans-import.cpp
> >> @@ -355,12 +355,12 @@ static void csv_tximp_preview_currency_fmt_sel_cb
> >> (GtkComboBox* format_selector, info->preview_update_currency_format();
> >> }
> >> 
> >> -void csv_tximp_preview_col_type_changed_cb (GtkComboBox* cbox,
> >> CsvImpTransAssist* info)
> >> +static void csv_tximp_preview_col_type_changed_cb
> >> (GtkComboBox* cbox, CsvImpTransAssist* info) {
> >> 
> >>     info->preview_update_col_type (cbox);
> >> 
> >> }
> > 
> > Apparently your new compile options require function declarations or to
> > declare local functions static. What was wrong with the previous behavior
> > that you choose to enforce this ?
> 
> CMakeLists.txt didn’t set any warning or error flags on C++ files, not even
> -Werror, so I just copied over the C flags and then adjusted them to deal
> with g++ wanting -Wmissing-declarations instead of -Wmissing-prototypes.
> 
> I just now looked at configure.ac and the flags there are rather different,
> with neither missing-declarations nor missing-prototypes on either C or
> C++.
> 
> configure.ac:
> AM_CFLAGS=-Wall -Werror -Wno-unused -Wno-deprecated-declarations
> -Wmissing-prototypes -Wmissing-prototypes
> -Werror-implicit-function-declaration
> 
> AM_CXXFLAGS=-Wall -Werror -Wno-unused
> 
> CMakeLists.txt:
> CMAKE_C_FLAGS=-Werror -Wdeclaration-after-statement -Wno-pointer-sign -Wall
> -Wunused -Wmissing-prototypes -Wmissing-declarations  -Wno-unused
> -Wno-deprecated-declarations -std=gnu11 CMAKE_CXX_FLAGS=-std=gnu++11
> -Werror -Wall -Wmissing-declarations -Wno-unused
> 
> I think that we should remove -Wno-unused from CXXFLAGS in general and only
> use it where needed on external sources. GnuCash C++ code is all new and it
> should all compile clean. For that code the more warnings the better: It
> gets the compiler to help us write better code.
> 
Ok, I fully agree.

> I’m in favor of having missing-prototypes/missing-declarations and declaring
> compilation-unit-local functions static to make their scope explicit.
> 
Ok, that's a good motivation. Just to be clear for me, having a function 
static is different from having a static variable, right ? As I understand it 
the latter is bad for thread safety, the former only ensures its local scope.

Geert