On 1/28/21 1:59 PM, Martin Sebor wrote:
On 1/28/21 1:31 AM, Richard Biener wrote:
On Thu, Jan 28, 2021 at 12:08 AM Martin Sebor via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
Attached is another attempt to fix the problem caused by allowing
front-end trees representing nontrivial VLA bound expressions to
stay in attribute access attached to functions. Since removing
these trees seems to be everyone's preference this patch does that
by extending the free_lang_data pass to look for and zero out these
trees.
Because free_lang_data only frees anything when LTO is enabled and
we want these trees cleared regardless to keep them from getting
clobbered during gimplification, this change also modifies the pass
to do the clearing even when the pass is otherwise inactive.
if (TREE_CODE (bound) == NOP_EXPR)
+ bound = TREE_OPERAND (bound, 0);
+
+ if (TREE_CODE (bound) == CONVERT_EXPR)
+ {
+ tree op0 = TREE_OPERAND (bound, 0);
+ tree bndtyp = TREE_TYPE (bound);
+ tree op0typ = TREE_TYPE (op0);
+ if (TYPE_PRECISION (bndtyp) == TYPE_PRECISION (op0typ))
+ bound = op0;
+ }
+
+ if (TREE_CODE (bound) == NON_LVALUE_EXPR)
+ bound = TREE_OPERAND (bound, 0);
all of the above can be just
STRIP_NOPS (bound);
which also handles nesting of the above in any order.
No, it can't be just STRIP_NOPS.
The goal is to strip the meaningless (to the user) cast to sizetype
from the array type. For example:
void f (int n, int[n]);
void f (int n, int[n + 1]);
I want the type in the warning to reflect the source:
warning: argument 2 of type ‘int[n + 1]’ declared with mismatched
bound ‘n + 1’ [-Wvla-parameter]
and not:
warning: ... ‘int[(sizetype)(n + 1)]’ ...
+ if (TREE_CODE (bound) == PLUS_EXPR
+ && integer_all_onesp (TREE_OPERAND (bound, 1)))
+ {
+ bound = TREE_OPERAND (bound, 0);
+ if (TREE_CODE (bound) == NOP_EXPR)
+ bound = TREE_OPERAND (bound, 0);
+ }
so it either does or does not strip a -1 but has no indication on
whether it did that? That looks fragile and broken.
Indication to what? The caller? The function is only used to recover
a meaningful VLA bound for warnings and its callers aren't interested
in whether the -1 was stripped or not.
Anyway, the split out of this function seems unrelated to the
original problem and should be submitted separately.
It was a remnant of the previous patch where it was used to format
the string representation of the VLA bounds and called from three
sites. I've removed the function from this revision (and restored
the one site in the pretty printer that open-codes the same thing).
+ for (vblist = TREE_VALUE (vblist); vblist; vblist = TREE_CHAIN
(vblist))
+ {
+ tree *pvbnd = &TREE_VALUE (vblist);
+ if (!*pvbnd || DECL_P (*pvbnd))
+ continue;
so this doesn't let constant bounds prevail but only symbolical ones?
Not
that I care but I'd have expected || CONSTANT_CLASS_P (*pvbnd)
There must be some confusion here. There are no constant VLA bounds.
The essential purpose of this patch is to remove expressions from
the attributes, such as PLUS_EXPR, that denote nontrivial VLA bounds.
The test above retains decls that might refer to function parameters
or global variables so that they can be mentioned in middle end
warnings.
Attached is yet another revision of this fix that moves the call
to attr_access:free_lang_data() to c_parse_final_cleanups() as
Jakub suggested.
With no further comments I have committed the final patch in
g:0718336a528.
Martin
