On 05/24/2017 04:28 PM, Martin Sebor wrote:
Attached is an updated patch with the requested changes. I've
also renamed the option -Wclass-memaccess to avoid suggesting
that the warning focuses solely on non-trivial types, and
updated its wording and comments throughout to refer to value
initialization rather than default initialization. Retesting
exposed another problem recently introduced into dumplfile.c
so this revision of the patch also contains a fix for that.
Emacs wasn't done saving the patch so here's the latest one with
the dumpfile.c fix.
+/* Return true if class TYPE meets a relaxed definition of standard-layout.
+ In particular, the requirement that it "has all non-static data members
+ nd bit-fields in the class and its base classes first declared in the
+ same class" is not considered. */
+
+static bool
+almost_std_layout_p (tree type, tree *field)
It looks like this will only return false when trivial_type_p will also
return false, so its only real function is setting *field. Can we
remove it and instead make first_non_public_field recursive?
+ case BUILT_IN_MEMSET:
+ if (!integer_zerop (args[1]))
+ {
+ /* Diagnose setting non-copy-assignable, non-trivial or non-
+ standard layout objects (in that order, since the latter
+ two are subsets of one another), to (potentially) non-zero
+ bytes. Since the value of the bytes being written is unknown,
+ suggest using assignment instead (if one exists). */
+ if (!trivassign)
+ warnfmt = G_("%qD writing to an object of type %#qT without "
+ "trivial copy assignment");
+ else if (!trivial)
+ warnfmt = G_("%qD writing to an object of non-trivial "
+ "type %#qT; use assignment instead");
Let's put the !trivial check first.
+ else if (!stdlayout)
+ warnfmt = G_("%qD writing to an object of non-standard-layout "
+ "type %#qT; use assignment instead");
+ else if (fld)
+ {
+ const char *access = TREE_PRIVATE (fld) ? "private" : "protected";
+ warned = warning_at (loc, OPT_Wclass_memaccess,
+ "%qD writing to an object of type %#qT with "
+ "%qs member %qD; use assignment instead",
+ fndecl, desttype, access, fld);
+ }
+ else if (!zero_init_p (desttype))
+ warnfmt = G_("%qD writing to an object of type %#qT containing "
+ "a pointer to data member; use assignment instead");
+
+ break;
+ }
+ /* Fall through. */
+
+ case BUILT_IN_BZERO:
+ /* Similarly to the above, diagnose clearing non-trivial or non-
+ standard layout objects, or objects of types with no assignmenmt. */
+ if (!trivassign)
+ warnfmt = (trivcopy
+ ? G_("%qD clearing an object of type %#qT without trivial "
+ "copy assignment; use copy constructor instead")
+ : G_("%qD clearing an object of type %#qT without trivial "
+ "copy assignment"));
+ else if (!trivial)
+ warnfmt = G_("%qD clearing an object of non-trivial type "
+ "%#qT; use assignment or value-initialization instead");
Again, let's put the !trivial check first; then we don't need to check
trivcopy in the !trivassign branch.
+ else if (!stdlayout)
+ warnfmt = G_("%qD clearing an object of non-standard-layout type "
+ "%#qT; use assignment or value-initialization instead");
+ else if (!zero_init_p (desttype))
+ warnfmt = G_("%qD clearing an object of type %#qT containing "
+ "a pointer-to-member; use assignment or value-"
+ "initialization instead");
+ break;
+
+ case BUILT_IN_BCOPY:
+ case BUILT_IN_MEMCPY:
+ case BUILT_IN_MEMMOVE:
+ case BUILT_IN_MEMPCPY:
+ /* Determine the type of the source object. */
+ srctype = (TREE_CODE (args[1]) == NOP_EXPR
+ ? TREE_OPERAND (args[1], 0) : args[1]);
+
+ srctype = TREE_TYPE (TREE_TYPE (srctype));
+
+ if (!trivassign)
+ warnfmt = (trivcopy
+ ? G_("%qD writing to an object of type %#qT without trivial "
+ "copy assignment; use copy constructor instead")
+ : G_("%qD writing to an object of type %#qT without trivial
"
+ "copy assignment"));
+ else if (!trivially_copyable_p (desttype))
+ warnfmt = G_("%qD writing to an object of non-trivially-copyable type "
+ "%#qT; use assignment or copy-initialization instead");
And here put the trivially-copyable test first, and drop checking
trivcopy in the !trivassign branch.
+ else if (!trivial
+ && !VOID_TYPE_P (srctype)
+ && !char_type_p (TYPE_MAIN_VARIANT (srctype))
+ && !same_type_ignoring_top_level_qualifiers_p (desttype,
+ srctype))
How can this ever be true? If !trivial, we would have already
complained about the type being non-trivially-copyable.
+ {
+ /* Warn when copying into a non-trivial object from an object
+ of a different type other than void or char. */
+ warned = warning_at (loc, OPT_Wclass_memaccess,
+ "%qD copying an object of non-trivial type "
+ "%#qT from an array of %#qT",
+ fndecl, desttype, srctype);
+ }
+ else if (fld
+ && !VOID_TYPE_P (srctype)
+ && !char_type_p (TYPE_MAIN_VARIANT (srctype))
+ && !same_type_ignoring_top_level_qualifiers_p (desttype,
+ srctype))
+ {
+ const char *access = TREE_PRIVATE (fld) ? "private" : "protected";
+ warned = warning_at (loc, OPT_Wclass_memaccess,
+ "%qD copying an object of type %#qT with "
+ "%qs member %qD from an arrayt of %#qT; use "
"array"
+ "assignment or copy-initialization instead",
+ fndecl, desttype, access, fld, srctype);
+ }
+ else if (!zero_init_p (desttype)
+ && !VOID_TYPE_P (srctype)
+ && !char_type_p (TYPE_MAIN_VARIANT (srctype))
+ && !same_type_ignoring_top_level_qualifiers_p (desttype,
+ srctype))
+ warnfmt = G_("%qD writing to an object of type %#qT containing "
+ "a pointer-to-member; use assignment or copy-"
+ "initialization instead");
This check seems unnecessary; copying from e.g. an array of
pointer-to-members would be fine. I think we only want to check
zero_init_p for the bzero case above.
+ else if (!trivial && TREE_CODE (args[2]) == INTEGER_CST)
As above, I don't see how we can ever hit this branch.
Jason
