[Bug c++/104855] -Wclass-memaccess is too broad with valid code

2022-03-14 Thread msebor at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104855

Martin Sebor  changed:

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution|--- |INVALID
 CC||msebor at gcc dot gnu.org

--- Comment #5 from Martin Sebor  ---
The documented purpose of the warning is to detect not just code that's
undefined (such as overwriting const or reference members) but also code that
violates encapsulation (and with it potentially also class invariants).  Such
code may be valid in the strict language sense but it's almost certainly not
valid under the design of the class, as in the test case in comment #0.

In the rare instances when such code is intentional and safe or where it cannot
be changed, an explicit cast along with a suitable comment certainly seems like
a preferable solution to avoid the warning than would be disabling it for the
majority of uses where it is not intentional.

Since any object pointer can be converted to void*, either implicitly (by
passing it to a memcpy argument) or by a static_cast, using reinterpret_cast is
not necessary to suppress the warning (nor would it be appropriate).

If -Wclass-memaccess had the ability to analyze the class invariants splitting
up -Wclass-memaccess into multiple levels as Jonathan suggests in comment #3
might perhaps be doable and useful, with level 1 triggering for the subset of
code that's strictly undefined, and higher levels for the rest.  But because
the warning runs in the C++ front end where the actual implementation of the
class is not readily analyzable, the separation would result in a bias heavily
skewed toward the latter (i.e., lots of false negatives at level 1).

The guidance for deciding whether or not a subset of warnings should be in
-Wall is in the GCC manual:

  [-Wall] enables all the warnings about constructions that some users consider
questionable, and that are easy to avoid (or modify to prevent the warning),
even in conjunction with macros. 

As Richard notes in comment #1, this instance of -Wclass-memacess (and in my
experience the overwhelming majority of others) with the easy suppression fit
this description.

[Bug c++/104855] -Wclass-memaccess is too broad with valid code

2022-03-10 Thread redi at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104855

--- Comment #4 from Jonathan Wakely  ---
And the former category *should* be in -Wall, but IMHO the latter shouldn't be.

[Bug c++/104855] -Wclass-memaccess is too broad with valid code

2022-03-10 Thread redi at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104855

Jonathan Wakely  changed:

   What|Removed |Added

   Last reconfirmed||2022-03-10
 Status|UNCONFIRMED |NEW
 Ever confirmed|0   |1

--- Comment #3 from Jonathan Wakely  ---
Confirmed. I think we should separate "this is undefined because your type
isn't trivially copyable" from "this seems like poor style".

[Bug c++/104855] -Wclass-memaccess is too broad with valid code

2022-03-10 Thread soap at gentoo dot org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104855

--- Comment #2 from David Seifert  ---
(In reply to Richard Biener from comment #1)
> I think the code is definitely "bad style", it seems A lacks a CTOR from
> 'unsigned' and the code tries to workaround this issue.
> 
> Not sure whether these kind of "style" diagnostics should be in -Wall

Even if I add a single argument ctor, the warning remains.

[Bug c++/104855] -Wclass-memaccess is too broad with valid code

2022-03-10 Thread rguenth at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104855

Richard Biener  changed:

   What|Removed |Added

Version|unknown |12.0
   Keywords||diagnostic

--- Comment #1 from Richard Biener  ---
I think the code is definitely "bad style", it seems A lacks a CTOR from
'unsigned' and the code tries to workaround this issue.

Not sure whether these kind of "style" diagnostics should be in -Wall