Re: gnucash unstable: Multiple changes pushed
> 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
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
John Rallsschreef 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
> 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
On Fri, Dec 29, 2017 at 12:06 PM, Aaron Lawswrote: > 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
> > 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
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
On Fri, Dec 29, 2017 at 11:22 AM, John Rallswrote: > > > 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
> 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
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