lmi
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [lmi] Removing comment induces compiler error


From: Greg Chicares
Subject: Re: [lmi] Removing comment induces compiler error
Date: Mon, 6 Mar 2017 02:47:52 +0000
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Icedove/45.6.0

On 2017-03-06 00:45, Vadim Zeitlin wrote:
> On Mon, 6 Mar 2017 00:24:06 +0000 Greg Chicares <address@hidden> wrote:
> 
> GC> I apply the following patch, and everything compiles. But if I
> GC> delete any one of the three changed lines, compilation fails.
> GC> How can deleting a commented-out line cause that? AFAICS it's
> GC> impossible: the preprocessor removes the commented-out lines
> GC> unconditionally, so the compiler can never see them.
> 
>  I admit I was surprised by this too, but the good thing with the computers
> is that you can always debug them and it didn't take long to find out what
> was going on here: the compiler indeed can never see the commented out
> line, but other tools can, notably grep, which is used by workhorse.make to
> build the list of wx_dependent_objects which are compiled without
> -Wcast-qual which is used for the other ones. So, the explanation of the
> riddle is that the code in these files uses wxWidgets indirectly and so
> must not be compiled with -Wcast-qual.

Wow. Thanks.

>  There are several possible solutions, but I think the most robust one
> could be to disable this warning with a compiler pragma in pchfile_wx.hpp
> and get rid of wx_dependent_objects in the makefiles entirely. This would
> keep things at the code, rather than the makefile, level which is IMO
> preferable and would rely on a convention we already use ("all files using
> wx must include pchfile_wx.hpp") instead of using heuristics.
> 
>  If you agree with this solution, please let me know if you'd like me to
> make (and test) the simple implementing it.

No, thanks. This method does have its advantages, but:

  "If possible, #pragmas are best avoided." -- TC++PL4, 12.6.3

In this case, correctness would depend on a pragma. Do we ever do that
elsewhere? No. We have only three different pragma instances: one is
perhaps unavoidable, but it's a workaround for a compiler defect; the
second is a boost workaround because it's inconvenient to fix boost;
and the third, AFAICT, is just a mistake. We should have fewer pragmas,
not more.

'cpp_main.cpp' and 'main_common.cpp': One pragma, guarded for MinGW only,
to make it conform to the standard.

'rate_table.cpp': One pragma, guarded for clang only, to work around a
boost defect.

'wx_new_test.cpp': One pragma, guarded for MinGW only, which seems to
avoid a diagnostic concerning shared-library attributes, when we
#include "wx.cpp" in order to...avoid shared-library attributes? No,
we should revert commit d2b707bd5af4b399c5986490e6b830e23e45bc63
and apply something like this, to remove the cause instead of
suppressing the symptom--and then it even becomes comprehensible:

---------8<--------8<--------8<--------8<--------8<--------8<--------8<-------
diff --git a/wx_new.hpp b/wx_new.hpp
index c0ee3af..d8289c3 100644
--- a/wx_new.hpp
+++ b/wx_new.hpp
@@ -32,6 +32,8 @@
 #if defined HAVE_CONFIG_H
 // For msw, rely on the 'auto-import' kludge favored by autotools.
 #   define LMI_WX_NEW_SO
+#elif defined UNIT_TESTING_WX_NEW
+#   define LMI_WX_NEW_SO
 #elif defined LMI_MSW
 #   if defined LMI_WX_NEW_BUILD_SO
 #       define LMI_WX_NEW_SO __declspec(dllexport)
diff --git a/wx_new_test.cpp b/wx_new_test.cpp
index d2921b1..2469b11 100644
--- a/wx_new_test.cpp
+++ b/wx_new_test.cpp
@@ -23,24 +23,15 @@
 // suite ensures that it'll be compiled with stronger warning options
 // than wx would permit.
 
+#define UNIT_TESTING_WX_NEW
+
 #include "pchfile.hpp"
 
 // The '.cpp' file is deliberately included here instead of the header
 // because it was probably already compiled for inclusion in a dll,
 // resulting in an object that wouldn't necessarily work here.
-//
-// Explicitly include "wx_new.hpp" first for LMI_GCC_VERSION from
-// "config.hpp".
-
-#include "wx_new.hpp"
-#if defined __GNUC__ && 40600 <= LMI_GCC_VERSION
-#   pragma GCC diagnostic push
-#   pragma GCC diagnostic ignored "-Wattributes"
-#endif // defined __GNUC__ && 40600 <= LMI_GCC_VERSION
+
 #include "wx_new.cpp"
-#if defined __GNUC__ && 40600 <= LMI_GCC_VERSION
-#   pragma GCC diagnostic pop
-#endif // defined __GNUC__ && 40600 <= LMI_GCC_VERSION
 
 #include "test_tools.hpp"
 
--------->8-------->8-------->8-------->8-------->8-------->8-------->8-------




reply via email to

[Prev in Thread] Current Thread [Next in Thread]