Re: [PATCH] Related: OS#3663 Fix failed compilation on macOS Mojave with clang in CMakeLists.txt
Michael Auß michael.auss@gmail.com writes:
I did encounter problems with syntax errors when compiling with clang, see OS#3663 for details. (Relating to rtl_tcp)
I did not add something to the README.
I can certainly believe you had errors. But, changing the compiler requirement is significant -- particularly on platforms that don't have last week's compiler -- and if it isn't the plan to require C++14, then it seems like a workaround to enable that. My point really is that there should be a decision about the language, that should be in the README, and the build system should set --std for that chosen language, and then whatever needs to be fixed should be fixed.
Am 20.10.2018 um 01:45 schrieb Greg Troxel gdt@lexort.com: My point really is that there should be a decision about the language, that should be in the README, and the build system should set --std for that chosen language, and then whatever needs to be fixed should be fixed.
I only added the flag for compilers based on the existing pattern *.clang and my new pattern matching "c++“. So the Build system is setting the language only for clang at this point.
I tested the output for c++98 which resulted in the bugs original errors:
[ 61%] Building CXX object lib/CMakeFiles/gnuradio-osmosdr.dir/rtl_tcp/rtl_tcp_source_c.cc.o /Users/aussmich/Projekte/gr-osmosdr/lib/rtl_tcp/rtl_tcp_source_c.cc:301:9: error: expected expression cmd = { 0x09, htonl(direct_samp) }; ^ /Users/aussmich/Projekte/gr-osmosdr/lib/rtl_tcp/rtl_tcp_source_c.cc:307:9: error: expected expression cmd = { 0x0a, htonl(offset_tune) }; ^ /Users/aussmich/Projekte/gr-osmosdr/lib/rtl_tcp/rtl_tcp_source_c.cc:311:9: error: expected expression cmd = { 0x0e, htonl(bias_tee) }; ^ /Users/aussmich/Projekte/gr-osmosdr/lib/rtl_tcp/rtl_tcp_source_c.cc:570:9: error: expected expression cmd = { 0x08, htonl(automatic) }; ^ In file included from /Users/aussmich/Projekte/gr-osmosdr/lib/rtl_tcp/rtl_tcp_source_c.cc:30: In file included from /Users/aussmich/Projekte/gr-osmosdr/lib/rtl_tcp/rtl_tcp_source_c.h:23: In file included from /usr/local/Cellar/gnuradio/3.7.13.4/include/gnuradio/sync_block.h:27: In file included from /usr/local/Cellar/gnuradio/3.7.13.4/include/gnuradio/block.h:29: /usr/local/Cellar/gnuradio/3.7.13.4/include/gnuradio/logger.h:766:16: warning: private field 'd_logger' is not used [-Wunused-private-field] logger_ptr d_logger; ^ 1 warning and 4 errors generated. make[2]: *** [lib/CMakeFiles/gnuradio-osmosdr.dir/rtl_tcp/rtl_tcp_source_c.cc.o] Error 1 make[1]: *** [lib/CMakeFiles/gnuradio-osmosdr.dir/all] Error 2 make: *** [all] Error 2
I then tried c++11 which compiles just fine, so the minimum language required for this file is c++11 and not c++14 as stated in my patch.
PS:
C++11 defined list initialization: https://en.cppreference.com/w/cpp/language/list_initialization
Am 20.10.2018 um 10:36 schrieb Michael Auß michael.auss@gmail.com:
Am 20.10.2018 um 01:45 schrieb Greg Troxel gdt@lexort.com: My point really is that there should be a decision about the language, that should be in the README, and the build system should set --std for that chosen language, and then whatever needs to be fixed should be fixed.
I only added the flag for compilers based on the existing pattern *.clang and my new pattern matching "c++“. So the Build system is setting the language only for clang at this point.
I tested the output for c++98 which resulted in the bugs original errors:
[ 61%] Building CXX object lib/CMakeFiles/gnuradio-osmosdr.dir/rtl_tcp/rtl_tcp_source_c.cc.o /Users/aussmich/Projekte/gr-osmosdr/lib/rtl_tcp/rtl_tcp_source_c.cc:301:9: error: expected expression cmd = { 0x09, htonl(direct_samp) }; ^ /Users/aussmich/Projekte/gr-osmosdr/lib/rtl_tcp/rtl_tcp_source_c.cc:307:9: error: expected expression cmd = { 0x0a, htonl(offset_tune) }; ^ /Users/aussmich/Projekte/gr-osmosdr/lib/rtl_tcp/rtl_tcp_source_c.cc:311:9: error: expected expression cmd = { 0x0e, htonl(bias_tee) }; ^ /Users/aussmich/Projekte/gr-osmosdr/lib/rtl_tcp/rtl_tcp_source_c.cc:570:9: error: expected expression cmd = { 0x08, htonl(automatic) }; ^ In file included from /Users/aussmich/Projekte/gr-osmosdr/lib/rtl_tcp/rtl_tcp_source_c.cc:30: In file included from /Users/aussmich/Projekte/gr-osmosdr/lib/rtl_tcp/rtl_tcp_source_c.h:23: In file included from /usr/local/Cellar/gnuradio/3.7.13.4/include/gnuradio/sync_block.h:27: In file included from /usr/local/Cellar/gnuradio/3.7.13.4/include/gnuradio/block.h:29: /usr/local/Cellar/gnuradio/3.7.13.4/include/gnuradio/logger.h:766:16: warning: private field 'd_logger' is not used [-Wunused-private-field] logger_ptr d_logger; ^ 1 warning and 4 errors generated. make[2]: *** [lib/CMakeFiles/gnuradio-osmosdr.dir/rtl_tcp/rtl_tcp_source_c.cc.o] Error 1 make[1]: *** [lib/CMakeFiles/gnuradio-osmosdr.dir/all] Error 2 make: *** [all] Error 2
I then tried c++11 which compiles just fine, so the minimum language required for this file is c++11 and not c++14 as stated in my patch.
Michael Auß michael.auss@gmail.com writes:
I only added the flag for compilers based on the existing pattern *.clang and my new pattern matching "c++“. So the Build system is setting the language only for clang at this point.
So this is not really about your change, but it seems that the modern C++ world is complicated enough and various compilers have various defaults (which itself seems buggy), so my point is that we should be setting --std always (to a value that matches what the README says is needed), rather than special casing which is building up a record of defaults that don't work.
I tested the output for c++98 which resulted in the bugs original errors:
And there is c++03. But if it really needs C++11, that's how it is. The last release does not, but it seems master and the last release are very far apart.
-
Greg Troxel -
Michael Auß