[llvm-bugs] [Bug 39457] New: Wcomma misleading suggestions

[via llvm-bugs]

https://bugs.llvm.org/show_bug.cgi?id=39457

            Bug ID: 39457
           Summary: Wcomma misleading suggestions
           Product: clang
           Version: trunk
          Hardware: PC
                OS: Linux
            Status: NEW
          Severity: enhancement
          Priority: P
         Component: C++
          Assignee: unassignedclangbugs at nondot.org
          Reporter: federico.kircheis at gmail.com
                CC: dgregor at apple.com, llvm-bugs at lists.llvm.org,
                    richard-llvm at metafoo.co.uk

I have some "issues" with the "-Wcomma" warning.
I'm able to reproduce the issue with clang 5.0, 6.0, 7.0 and trunk (8.0), with
"-Wcomma" as compile parameter.

Consider following program (the return type of `foo` is not really relevant,
could be `void`).

----
int foo(){return 0;}

int main(){
        return foo(), 5;
}
-----

clang warns about a possible misuse of the comma operator, and AFAIK a common
technique is adding a `void()` expression between the operands:

----
int foo(){return 0;}

int main(){
        return foo(), void(), 5;
}
----

This is where the trouble begins for me, clang still complains, and suggests to
change the code in the following way.

----
int foo(){return 0;}

int main(){
        return static_cast<void>(foo()), static_cast<void>(void()), 5;
}
----

Yes, `static_cast<void>(void())` is actually needed for silencing the
warning(!)
(Also `(void)(void())` silences the warning of course...)


I think there are two or three issues here, but I'm not able to split them
since they partially depend on each other.

a) clang does not recognize the pattern "x, void(), y" for saying the compiler
that you really wanted to use the comma operator. 
Unless "static_cast<void>(x), y" has some benefits, it would be nice if both
pattern where recognized.
I'm nor sure if the cast has any advantage, it is surely more verbose, which
might be an advantage (probably easier to find), but also makes code less
readable (and people might use an old c-cast just to make it shorter/more
readable).
Since (personal opinion), `void()` is not used that often, it is easy to grep
too.
To me it also seems strange to add a cast, whereas (at least to me), adding
`void(), ` seems more appropriate, but maybe that's just because I'm more used
to the second syntax.
To conclude, IMHO, `return foo(), void(), 5;` should not generate a warning
when compiling with "-Wcomma".

b) The real issue for me is that `void()` doe snot seem to be recognized as a
void expression, and to silence the second warning I need to cast it to void.
I do not know if this "issue" appears somewhere else, so far this is the only
place where clang made such a suggestion.

Ideally clang should newer suggest to replace `void()` (explicitly written as
is, no templates, macros or something strange) with
`static_cast<void>(void())`.


c) If we replace `foo()` with `static_cast<void>(foo())`, there is no reason
the leave the second `void()` in the source code: Both the static cast and the
`void()` are used to avoid issues with user-declared comma operators, so both
of those forms are redundant.
It means given `static_cast<void>(foo()), static_cast<void>(void()), 5;` or
`static_cast<void>(foo()), void(), 5;` clang should/could suggest to reduce the
code to `static_cast<void>(foo()), 5;`

And if clang does not like `foo(), void(), 5;` because the cast has some of
advantage that I do not know of (see point a), then it should also suggest to
modify that form too `static_cast<void>(foo()), 5;`, instead of
`static_cast<void>(foo()), static_cast<void>(void()), 5;`

-- 
You are receiving this mail because:
You are on the CC list for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-bugs/attachments/20181026/5b97c920/attachment.html>