Misc #15136: Fix -Wparentheses warnings

• Status: Open
• Assignee: -

[jaruga (Jun Aruga)]

Currently the -Wno-parentheses was set.
I assumed if we could fix the warning, we could remove the -Wno-parentheses.

I fixed the warnings, because the warning is used as a default on Fedora Project build environment.
I sent pull-request. https://github.com/ruby/ruby/pull/1958

I would show you the explanation of -Wparentheses.

$ man gcc (or gcc --help --verbose)
...
       -Wparentheses
           Warn if parentheses are omitted in certain contexts, such as when there is an
           assignment in a context where a truth value is expected, or when operators are
           nested whose precedence people often get confused about.

           Also warn if a comparison like "x<=y<=z" appears; this is equivalent to "(x<=y ?
           1 : 0) <= z", which is a different interpretation from that of ordinary
           mathematical notation.

           Also warn for dangerous uses of the GNU extension to "?:" with omitted middle
           operand. When the condition in the "?": operator is a boolean expression, the
           omitted value is always 1.  Often programmers expect it to be a value computed
           inside the conditional expression instead.

           For C++ this also warns for some cases of unnecessary parentheses in
           declarations, which can indicate an attempt at a function call instead of a
           declaration:

                   {
                     // Declares a local variable called mymutex.
                     std::unique_lock<std::mutex> (mymutex);
                     // User meant std::unique_lock<std::mutex> lock (mymutex);
                   }

           This warning is enabled by -Wall.
...

[jaruga (Jun Aruga)]

Before the modification, there were 18 warnings for -Wparentheses.

$ make >& make.log
$ grep -r Wparentheses make.log  | wc -l
18

[duerst (Martin Dürst)]

jaruga (Jun Aruga) wrote:

Before the modification, there were 18 warnings for -Wparentheses.

My crude guess is that all of them would be for cases such as "assignment in a context where a truth value is expected" (first paragraph of man gcc). All the other cases will have problems on other compilers anyway.

You say "I fixed the warnings". So why is this issue still open.

[jaruga (Jun Aruga)]

• File make.log make.log added

You say "I fixed the warnings". So why is this issue still open.

Sorry it's mistake. Correctly I fixed the warnings on my git repository, and sent pull-request. And not merged in the main repository's trunk.

The patch is here.
https://github.com/ruby/ruby/pull/1958
or
https://github.com/ruby/ruby/pull/1958.patch

On the condition for the current trunk branch + below modification.

diff --git a/configure.ac b/configure.ac
index 14fe5279c7..6182a5d792 100644
--- a/configure.ac
+++ b/configure.ac
@@ -458,7 +458,7 @@ AS_IF([test "$GCC:${warnflags+set}:no" = yes::no], [
     AS_IF([test $icc_version -gt 0], [
        particular_werror_flags=no
     ])
-    for wflag in -Wno-unused-parameter -Wno-parentheses -Wno-long-long \
+    for wflag in -Wno-unused-parameter -Wno-long-long \
                  -diag-disable=175,188,2259 \
                 -Wno-missing-field-initializers \
                 -Wno-tautological-compare \

$ autoconf

$ ./configure

$ make >& make.log

$ grep -r Wparentheses make.log  | wc -l
18

I would attach the make.log now.

There 2 cases in the warnings.

Yes, as you said, 1st case is about assignment.

mjit_worker.c:1097:9: warning: suggest parentheses around assignment used as truth value [-Wparentheses]
     if (success = compile_c_to_o(c_file, o_file)) {
         ^~~~~~~

2nd case is

signal.c:889:24: warning: suggest parentheses around ‘&&’ within ‘||’ [-Wparentheses]
  sp_page <= fault_page && fault_page <= bp_page) {
  ~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~

[nobu (Nobuyoshi Nakada)]

Those 2 cases are actually different.
IIRC, matz has rejected the 1st case, and preferred explicit comparison with 0.

[jaruga (Jun Aruga)]

IIRC, matz has rejected the 1st case, and preferred explicit comparison with 0.

@nobu (Nobuyoshi Nakada) you mean basically like this?

if (success = compile_c_to_o(c_file, o_file)) {

to

if ((success = compile_c_to_o(c_file, o_file)) != 0) {

(In case of compile_c_to_o function, the return value: 0 is success case, otherwise error case.)

I will update and rebase my pull-request for the way.

[jaruga (Jun Aruga)]

I will update and rebase my pull-request for the way.

I updated and rebased my pull-request now!
You can pick up and merge a part of the pull-request, if you like it.