https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104702

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org,
                   |                            |msebor at gcc dot gnu.org

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
I think the problem in this case are primarily the warning-control.cc changes.
In GCC 11, build_vec_init did at the end:
  /* Now make the result have the correct type.  */
  if (TREE_CODE (atype) == ARRAY_TYPE)
    {
      atype = build_pointer_type (atype);
      stmt_expr = build1 (NOP_EXPR, atype, stmt_expr);
      stmt_expr = cp_build_fold_indirect_ref (stmt_expr);
      TREE_NO_WARNING (stmt_expr) = 1;
    }
GCC 12 does instead:
  if (TREE_CODE (atype) == ARRAY_TYPE)
    {
      atype = build_pointer_type (atype);
      stmt_expr = build1 (NOP_EXPR, atype, stmt_expr);
      stmt_expr = cp_build_fold_indirect_ref (stmt_expr);
      suppress_warning (stmt_expr /* What warning? */);
    }
Clearly, at least one of the intended warnings that should be suppressed is
OPT_Wunused_value.
But as EXPR_LOCATION (stmt_expr) == UNKNOWN_LOCATION here, all suppress_warning
does is set TREE_NO_WARNING bit on it (i.e. like GCC 11 used).
Later on, add_stmt is called on it and does:
              if (!EXPR_HAS_LOCATION (t))
                SET_EXPR_LOCATION (t, input_location);
There was also even before build_vec_init a call to
suppress_warning with some unrelated expression that had the
284032 {file = 0x3f67310 "pr104702.C", line = 11, column = 31, data = 0x0, sysp
= false}
location (with OPT_Wparentheses) which is the input_location now newly set by
add_stmt on the INDIRECT_REF.
Later on, convert_to_void calls
                        && !warning_suppressed_p (expr, OPT_Wunused_value)
on the INDIRECT_REF.  TREE_NO_WARNING bit is set, but at this point it has a
location, so it looks up the location in hash map, finds that OPT_Wparantheses
is the warning to be suppressed there and returns that OPT_Wunused_value isn't
suppressed there, even when we wanted to suppress all warnings on the tree.

So, I think the options are either to analyze everything where suppress_warning
might be called on trees that don't have EXPR_LOCATION set yet but might get it
changed later on, or perhaps change the warning-control.cc implementation for
the cases where it is called on trees with reserved location (grab another bit
next to TREE_NO_WARNING and set it too if we want to suppress some warnings on
reserved location tree, which would then be sticky and would imply suppress all
warnings on this tree from now on).
Or change warning-control.cc not to base anything on location_t's but on the
trees or gimple * pointers instead.  We'd of course need to copy records in the
hash map if we copy a tree or gimple stmt including the
TREE_NO_WARNING/gimple_no_warning bit (and would need to be deletable so that
when some trees/gimple stmts are garbage collected we remove entries from those
hash tables).  In fact, I thought that was the plan for warning control.
Basing it on location_t has beyond the above mentioned problem also a problem
if ever EXPR_LOCATION changes later on (I admit except for UNKNOWN_LOCATION ->
some other location changes that is quite rare).

Also, unless I'm misreading the warning-control.cc code, it seems it is based
on whatever location_t a tree or especially gimple * has, but we encode in
location_t not just the location, but also the BLOCK starting with
gimple-low.cc, and I don't see any stripping of that BLOCK stuff, i.e. no uses
of
LOCATION_LOCUS anywhere.  So again I don't see how it can work well,
suppress_warning before gimple-low.cc sets gimple_no_warning bit and fills in
details for one location, then we change the location_t because we add BLOCK to
it, then some warning pass tests suppress_warning_p and either will not find
the new location_t in there and thus will imply all warnings are suppressed, or
worse suppress_location before gimple-low.cc was for one warning, there is
another suppress_location after it on the same stmt (that will add a record on
the new location_t) and finally suppress_warning_p test on the first warning
(will return false).

Reply via email to