Re: gnucash unstable: Multiple changes pushed

[John Ralls]

> On Mar 10, 2018, at 1:42 AM, Geert Janssens  
> wrote:
> 
> Op zaterdag 10 maart 2018 01:28:16 CET schreef John Ralls:
>> diff --git a/CMakeLists.txt b/CMakeLists.txt
>> index 86607a1..dbf92a5 100644
>> --- a/CMakeLists.txt
>> +++ b/CMakeLists.txt
>> @@ -511,7 +511,7 @@ SET (Boost_FIND_QUIETLY ON)
>> IF (NOT DEFINED ${BOOST_ROOT})
>>   SET(BOOST_ROOT $ENV{BOOST_ROOT})
>> ENDIF()
>> -FIND_PACKAGE (Boost 1.54.0 REQUIRED COMPONENTS date_time regex locale
>> filesystem) +FIND_PACKAGE (Boost 1.54.0 REQUIRED COMPONENTS date_time regex
>> locale filesystem system)
>> 
> Would that also mean we need to add an extra boost dll in the windows 
> installer (along the lines of the boost-filestsystem issue Bob recently 
> discovered) ?

Possibly, I haven’t tested it on Windows yet. Thanks for reminding me.

Regards,
John Ralls

___
gnucash-devel mailing list
gnucash-devel@gnucash.org
https://lists.gnucash.org/mailman/listinfo/gnucash-devel

Re: gnucash unstable: Multiple changes pushed

[Geert Janssens]

Op zaterdag 10 maart 2018 01:28:16 CET schreef John Ralls:
> diff --git a/CMakeLists.txt b/CMakeLists.txt
> index 86607a1..dbf92a5 100644
> --- a/CMakeLists.txt
> +++ b/CMakeLists.txt
> @@ -511,7 +511,7 @@ SET (Boost_FIND_QUIETLY ON)
>  IF (NOT DEFINED ${BOOST_ROOT})
>SET(BOOST_ROOT $ENV{BOOST_ROOT})
>  ENDIF()
> -FIND_PACKAGE (Boost 1.54.0 REQUIRED COMPONENTS date_time regex locale
> filesystem) +FIND_PACKAGE (Boost 1.54.0 REQUIRED COMPONENTS date_time regex
> locale filesystem system)
> 
Would that also mean we need to add an extra boost dll in the windows 
installer (along the lines of the boost-filestsystem issue Bob recently 
discovered) ?

Geert


___
gnucash-devel mailing list
gnucash-devel@gnucash.org
https://lists.gnucash.org/mailman/listinfo/gnucash-devel

Re: gnucash unstable: Multiple changes pushed

[Geert Janssens]

John Ralls  schreef op 7 januari 2018 :

>  * Add GDK_PIXBUF_MODULE_FILE, allows running the Mac bundle without a
>  launcher script.

Yay ! That is cool 
___
gnucash-devel mailing list
gnucash-devel@gnucash.org
https://lists.gnucash.org/mailman/listinfo/gnucash-devel

Re: gnucash unstable: Multiple changes pushed

[John Ralls]

> On Dec 29, 2017, at 8:55 AM, Geert Janssens  
> wrote:
> 
> Op vrijdag 29 december 2017 17:22:29 CET schreef John Ralls:
>>> On Dec 29, 2017, at 7:41 AM, Geert Janssens 
>>> wrote:> 
>>> Op woensdag 27 december 2017 00:37:19 CET schreef John Ralls:
 commit 91727525b9cd3c12f4fa25268131b0161c6fc79e
 Author: John Ralls 
 Date:   Tue Dec 26 13:01:50 2017 -0800
 
   Enforce -Werror on C++ files and fix resulting errors.
>>> 
>>> 
>>> 
 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.

Yes, and static in a class declaration means something else again.

Careless use of shared state will cause thread safety issues, yes. But static 
variables aren’t the main source of the shared state problem: Promiscuous 
passing of pointers to heap/free store variables is. Consider the possibility 
that two threads get the pointer to a particular split from QofContainer and 
proceed to edit it. There’s no mutex in QofInstance’s begin_edit/commit_edit, 
so the results are almost guaranteed to be interesting.

Regards,
John Ralls

___
gnucash-devel mailing list
gnucash-devel@gnucash.org
https://lists.gnucash.org/mailman/listinfo/gnucash-devel

Re: gnucash unstable: Multiple changes pushed

[Aaron Laws]

On Fri, Dec 29, 2017 at 12:06 PM, Aaron Laws  wrote:

> 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.
>
>
> There two "static"s to keep in mind: static storage duration, and members
> not bound to instances. Static functions are functions that can't be used
> outside the current code file:
>
> static void func () { printf ("can't be used outside " __FILE__); }
> (or something like that.)
>
> I should have also said that this first type of static can be applied to
variables, too!

static int my_int; // can only be used in this translation unit: __FILE__.

I hope I answered your question and didn't add confusion!
___
gnucash-devel mailing list
gnucash-devel@gnucash.org
https://lists.gnucash.org/mailman/listinfo/gnucash-devel

Re: gnucash unstable: Multiple changes pushed

[Aaron Laws]

>
> 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.


There two "static"s to keep in mind: static storage duration, and members
not bound to instances. Static functions are functions that can't be used
outside the current code file:

static void func () { printf ("can't be used outside " __FILE__); }
(or something like that.)

But very different is:

struct A { int a;  static int s_a; };
int A::s_a = 0;
int main () {
   printf ("%d is accessible.\n", A::s_a);
   //printf ("%d\n", A::a); this is illegal
   return 0;
}

Here, A::s_a can be accessed without ever creating an instance of "A".
___
gnucash-devel mailing list
gnucash-devel@gnucash.org
https://lists.gnucash.org/mailman/listinfo/gnucash-devel

Re: 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 
> > wrote:> 
> > Op woensdag 27 december 2017 00:37:19 CET schreef John Ralls:
> >> commit 91727525b9cd3c12f4fa25268131b0161c6fc79e
> >> Author: John Ralls 
> >> Date:   Tue Dec 26 13:01:50 2017 -0800
> >> 
> >>Enforce -Werror on C++ files and fix resulting errors.
> > 
> > 
> > 
> >> 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
___
gnucash-devel mailing list
gnucash-devel@gnucash.org
https://lists.gnucash.org/mailman/listinfo/gnucash-devel

Re: gnucash unstable: Multiple changes pushed

[Aaron Laws]

On Fri, Dec 29, 2017 at 11:22 AM, John Ralls  wrote:

>
>
> 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.


I agree. I use '-Wall -Wextra -Wpedantic -Wconversion -Werrro' in my
projects (because -Wall is really -Wsome), but adding them to old code can
result in quite a mess.
___
gnucash-devel mailing list
gnucash-devel@gnucash.org
https://lists.gnucash.org/mailman/listinfo/gnucash-devel

Re: gnucash unstable: Multiple changes pushed

[John Ralls]

> On Dec 29, 2017, at 7:41 AM, Geert Janssens  
> wrote:
> 
> Op woensdag 27 december 2017 00:37:19 CET schreef John Ralls:
>> commit 91727525b9cd3c12f4fa25268131b0161c6fc79e
>> Author: John Ralls 
>> Date:   Tue Dec 26 13:01:50 2017 -0800
>> 
>>Enforce -Werror on C++ files and fix resulting errors.
>> 
> 
> 
> 
>> 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.

I’m in favor of having missing-prototypes/missing-declarations and declaring 
compilation-unit-local functions static to make their scope explicit.

Regards,
John Ralls



___
gnucash-devel mailing list
gnucash-devel@gnucash.org
https://lists.gnucash.org/mailman/listinfo/gnucash-devel

Re: gnucash unstable: Multiple changes pushed

[Geert Janssens]

Op woensdag 27 december 2017 00:37:19 CET schreef John Ralls:
> commit 91727525b9cd3c12f4fa25268131b0161c6fc79e
> Author: John Ralls 
> Date:   Tue Dec 26 13:01:50 2017 -0800
> 
> Enforce -Werror on C++ files and fix resulting errors.
> 



> 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 ?

Geert
___
gnucash-devel mailing list
gnucash-devel@gnucash.org
https://lists.gnucash.org/mailman/listinfo/gnucash-devel