Re: [lmi] [lmi-commits] master 2ca1e113 6/9: Comment out superfluous semicolons

[Greg Chicares]

On 7/30/22 13:15, Vadim Zeitlin wrote:
> On Fri, 29 Jul 2022 20:42:33 -0400 (EDT) Greg Chicares 
> <gchicares@sbcglobal.net> wrote:
[...]
> GC>     Comment out superfluous semicolons
> GC>     
> GC>     "//;" can be removed if wx changes someday.

I had supposed that wx wouldn't change:
 - there may be quite a few wx-dependent applications with which
   such a change would be incompatible; and
 - most programmers think the issue is important.

> GC> --- a/wx_test_about_version.cpp
> GC> +++ b/wx_test_about_version.cpp
> GC> @@ -246,7 +246,7 @@ LMI_WX_TEST_CASE(about_dialog_version)
> GC>              wxTEST_DIALOG
> GC>                  (wxYield()
> GC>                  ,expect_license_dialog()
> GC> -                );
> GC> +                )//;
> GC>  
> GC>              return wxID_OK;
> GC>              }
> 
>  I'd indeed like to change this because it's definitely an oversight, all
> statement-like macros are supposed to require a semicolon after them and
> the example in the documentation shows using it too.

That's good IMO.

>  Unfortunately doing it in 3.2 would be, strictly speaking,
> backwards-incompatible and so can't be done there. But if I do it only in
> the latest wx master, it would make lmi not compile with it any more, yet
> we wouldn't be able to remove "//" as we'd still want lmi to compile with
> 3.2 too.

Then my assumption was incorrect, but that's okay.

In 'pchfile_wx.hpp' we already have a commented-out
//#   pragma GCC diagnostic ignored "-Wextra-semi"
and if we activate that, for clang and not just gcc,
then we could just restore all the semicolons
  %s,//;,;,
and everything would build with old or new wx versions.
At the appropriate time, then, we could revert the
"-Wextra-semi" pragma, which is just a one-line change.

If that works, then it seems to be the clear winner,
and everything I wrote below should be ignored.

>  So the only way out of this I see is to define some LMI_TEST_WX_DIALOG()
> macro that would wrap wxTEST_DIALOG() but would always require a semicolon.
> Should I do it like this or do you think it's too ugly and not worth it?

I almost did it that way, but macros are a specialty and I barely
have enough knowledge to maintain my own, so I was afraid of going
down a token-pasting (e.g.) rabbit hole. However, I wouldn't object
if you want to propose a patch that works.

Alternatively, I might have enough knowledge to attempt something like
the throwaway patch below, which just creates a non-function-like macro
that's either ';' or ''. This should work regardless of the complexity
of the preceding function-like macro.

Either way, there's no one lmi header that's included in every file that
uses wxTEST_DIALOG (except lmi's 'config.hpp'), but it looks like all
such files include either "wx_test_document.hpp" or "wx_test_case.hpp",
and it wouldn't be too bad to define LMI_END_OF_STATEMENT_1 in one of
those headers and LMI_END_OF_STATEMENT_2 in the other, I guess; or maybe
we should require every wx-dependent file to #include 'wx_utility.hpp'
or 'wx_checks.hpp'.

--8<----8<----8<----8<----8<----8<----8<----8<----8<--
diff --git a/wx_test_calculation_summary.cpp b/wx_test_calculation_summary.cpp
index 37113e61d..7cb67dfe9 100644
--- a/wx_test_calculation_summary.cpp
+++ b/wx_test_calculation_summary.cpp
@@ -97,7 +97,7 @@ class expect_preferences_dialog_base
         wxUIActionSimulator ui;
         ui.Char('f', wxMOD_CONTROL);    // "File|Preferences"
 
-        wxTEST_DIALOG(wxYield(), *this)//;
+        wxTEST_DIALOG(wxYield(), *this) LMI_EOS
         }
 
     int OnInvoked(MvcController* dialog) const override
diff --git a/wx_test_document.hpp b/wx_test_document.hpp
index 73b229d93..fd50312a0 100644
--- a/wx_test_document.hpp
+++ b/wx_test_document.hpp
@@ -32,6 +32,12 @@
 
 #include <exception>                    // uncaught_exceptions()
 
+#if !wxCHECK_VERSION(3,2,1) // wx prior to version 3.2.1 .
+#   define LMI_EOS
+#else  // wx version 3.2.1 or greater.
+#   define LMI_EOS ;
+#endif // wx version 3.2.1 or greater.
+
 /// Helper function for finding and focusing a control with the specified name
 /// inside MvcController (actually it could be any top level window containing
 /// a book control).
--8<----8<----8<----8<----8<----8<----8<----8<----8<--