This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] warn on mem calls modifying objects of non-trivial types (PR 80560)


On 06/04/2017 10:01 PM, Jason Merrill wrote:
On 06/02/2017 05:28 PM, Martin Sebor wrote:
On 05/31/2017 05:34 PM, Jason Merrill wrote:
On 05/27/2017 06:44 PM, Martin Sebor wrote:
+  /* True if the class is trivial and has a trivial non-deleted copy
+     assignment, copy ctor, and default ctor, respectively.  The last
+     one isn't used to issue warnings but only to decide what suitable
+     alternatives to offer as replacements for the raw memory
operation.  */
+  bool trivial = trivial_type_p (desttype);

This comment seems out of date; there's only one variable here now.

+  /* True if the class is has a non-deleted trivial assignment.  Set

s/is//

+  /* True if the class has a (possibly deleted) trivial copy ctor.  */
+  bool trivcopy = trivially_copyable_p (desttype);

"True if the class is trivially copyable."

+      if (delassign)
+        warnfmt = G_("%qD writing to an object of type %#qT with "
+             "deleted copy assignment");
+      else if (!trivassign)
+        warnfmt = G_("%qD writing to an object of type %#qT with "
+             "no trivial copy assignment");
+      else if (!trivial)
+        warnfmt = G_("%qD writing to an object of non-trivial "
+             "type %#qT; use assignment instead");

I'd still like the !trivial test to come first in all the memset cases,
!trivcopy in the copy cases.

The tests are in the order they're in to provide as much useful
detail in the diagnostics as necessary to understand the problem
make the suggestion meaningful.  To what end you want to change
it?

Mostly I was thinking that whether a class is trivial(ly copyable) is more to the point, but I guess what you have now is fine.

+static bool
+has_trivial_special_function (tree ctype, special_function_kind sfk,
+                  bool *deleted_p)

This seems redundant with type_has_trivial_fn.  If that function is
giving the wrong answer for a class where all of the SFK are deleted,
let's fix that, under check_bases_and_members, rather than introduce a
new function.  I don't want to call synthesized_method_walk every time
we want to check whether a function is trivial.

A deleted special function can be trivial.

I believe that in the language, the triviality of a deleted function cannot be determined. But I believe you're right about the behavior of type_has_trivial_fn, which is why I mentioned changing it.

Maybe I should use a different approach and instead of trying
to see if a function is deleted use trivially_xible to see if
it's usable.  That will mean changing the diagnostics from
"with a deleted special function" to "without trivial special
function" but it will avoid calling synthesized_method_walk
while still avoiding giving bogus suggestions.

Actually, this would check for one possible argument type and not others, so I think it's better to keep looking at the declarations. You can do that by just looking them up (lookup_fnfields_slot) and iterating over them, you don't need to call synthesized_method_walk.

Jason


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]