On Tue, Jul 23, 2024 at 05:14:30PM -0400, Patrick Palka wrote:
> On Tue, 23 Jul 2024, Jason Merrill wrote:
>
> > On 7/7/24 12:39 AM, Nathaniel Shead wrote:
> > > Bootstrapped on x86_64-pc-linux-gnu, successfully regtested modules.exp;
> > > OK for trunk if full regtest passes?
> >
> > Patrick, I assume this change won't mess with your streaming optimizations?
>
> Should be fine, those optimizations are currently confined to
> tree_node_bools where we stream many consecutive bools (and so we want
> to avoid conditionally streaming a bit, so that the bit buffer position
> of each streamed bit is statically known). I don't think the technique
> can really apply to core_vals since it streams mostly trees.
>
> >
> > OK with Patrick's approval or on Friday, whichever comes first.
> >
Thanks, pushed.
> > > -- >8 --
> > >
> > > Currently we don't stream the contents of 'nowarn_map'; this means that
> > > warning suppressions don't get applied in importers, which is
> > > particularly relevant for templates (as in the linked testcase).
> > >
> > > Rather than streaming the whole contents of 'nowarn_map', this patch
> > > instead just streams the exported suppressions for each tree node
> > > individually, to not build up additional locations and suppressions for
> > > tree nodes that do not need to be streamed.
> > >
> > > PR c++/115757
> > >
> > > gcc/cp/ChangeLog:
> > >
> > > * module.cc (trees_out::core_vals): Write warning specs for
> > > DECLs and EXPRs.
> > > (trees_in::core_vals): Read warning specs.
> > >
> > > gcc/ChangeLog:
> > >
> > > * tree.h (put_warning_spec_at): Declare new function.
> > > (has_warning_spec): Likewise.
> > > (get_warning_spec): Likewise.
> > > (put_warning_spec): Likewise.
> > > * diagnostic-spec.h (nowarn_spec_t::from_bits): New function.
> > > * diagnostic-spec.cc (put_warning_spec_at): New function.
> > > * warning-control.cc (has_warning_spec): New function.
> > > (get_warning_spec): New function.
> > > (put_warning_spec): New function.
> > >
> > > gcc/testsuite/ChangeLog:
> > >
> > > * g++.dg/modules/warn-spec-1_a.C: New test.
> > > * g++.dg/modules/warn-spec-1_b.C: New test.
> > >
> > > Signed-off-by: Nathaniel Shead <nathanielosh...@gmail.com>
> > > ---
> > > gcc/cp/module.cc | 12 +++++++++
> > > gcc/diagnostic-spec.cc | 21 ++++++++++++++++
> > > gcc/diagnostic-spec.h | 7 ++++++
> > > gcc/testsuite/g++.dg/modules/warn-spec-1_a.C | 10 ++++++++
> > > gcc/testsuite/g++.dg/modules/warn-spec-1_b.C | 8 ++++++
> > > gcc/tree.h | 9 +++++++
> > > gcc/warning-control.cc | 26 ++++++++++++++++++++
> > > 7 files changed, 93 insertions(+)
> > > create mode 100644 gcc/testsuite/g++.dg/modules/warn-spec-1_a.C
> > > create mode 100644 gcc/testsuite/g++.dg/modules/warn-spec-1_b.C
> > >
> > > diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> > > index dc5d046f04d..0f9a689dbec 100644
> > > --- a/gcc/cp/module.cc
> > > +++ b/gcc/cp/module.cc
> > > @@ -6000,6 +6000,10 @@ trees_out::core_vals (tree t)
> > > if (state)
> > > state->write_location (*this, t->decl_minimal.locus);
> > > +
> > > + if (streaming_p ())
> > > + if (has_warning_spec (t))
> > > + u (get_warning_spec (t));
> > > }
> > > if (CODE_CONTAINS_STRUCT (code, TS_TYPE_COMMON))
> > > @@ -6113,6 +6117,10 @@ trees_out::core_vals (tree t)
> > > if (state)
> > > state->write_location (*this, t->exp.locus);
> > > + if (streaming_p ())
> > > + if (has_warning_spec (t))
> > > + u (get_warning_spec (t));
> > > +
> > > /* Walk in forward order, as (for instance) REQUIRES_EXPR has a
> > > bunch of unscoped parms on its first operand. It's safer to
> > > create those in order. */
> > > @@ -6576,6 +6584,8 @@ trees_in::core_vals (tree t)
> > > /* Don't zap the locus just yet, we don't record it correctly
> > > and thus lose all location information. */
> > > t->decl_minimal.locus = state->read_location (*this);
> > > + if (has_warning_spec (t))
> > > + put_warning_spec (t, u ());
> > > }
> > > if (CODE_CONTAINS_STRUCT (code, TS_TYPE_COMMON))
> > > @@ -6654,6 +6664,8 @@ trees_in::core_vals (tree t)
> > > if (CODE_CONTAINS_STRUCT (code, TS_EXP))
> > > {
> > > t->exp.locus = state->read_location (*this);
> > > + if (has_warning_spec (t))
> > > + put_warning_spec (t, u ());
> > > bool vl = TREE_CODE_CLASS (code) == tcc_vl_exp;
> > > for (unsigned limit = (vl ? VL_EXP_OPERAND_LENGTH (t)
> > > diff --git a/gcc/diagnostic-spec.cc b/gcc/diagnostic-spec.cc
> > > index 996ad6b273a..addaf089f03 100644
> > > --- a/gcc/diagnostic-spec.cc
> > > +++ b/gcc/diagnostic-spec.cc
> > > @@ -179,6 +179,27 @@ suppress_warning_at (location_t loc, opt_code opt /*
> > > =
> > > all_warnings */,
> > > return true;
> > > }
> > > +/* Change the warning disposition for LOC to match OPTSPEC. */
> > > +
> > > +void
> > > +put_warning_spec_at (location_t loc, unsigned bits)
> > > +{
> > > + gcc_checking_assert (!RESERVED_LOCATION_P (loc));
> > > +
> > > + nowarn_spec_t optspec = nowarn_spec_t::from_bits (bits);
> > > + if (!optspec)
> > > + {
> > > + if (nowarn_map)
> > > + nowarn_map->remove (loc);
> > > + }
> > > + else
> > > + {
> > > + if (!nowarn_map)
> > > + nowarn_map = nowarn_map_t::create_ggc (32);
> > > + nowarn_map->put (loc, optspec);
> > > + }
> > > +}
> > > +
> > > /* Copy the no-warning disposition from one location to another. */
> > > void
> > > diff --git a/gcc/diagnostic-spec.h b/gcc/diagnostic-spec.h
> > > index 22d4c067158..0b155a5cde3 100644
> > > --- a/gcc/diagnostic-spec.h
> > > +++ b/gcc/diagnostic-spec.h
> > > @@ -56,6 +56,13 @@ public:
> > > nowarn_spec_t (opt_code);
> > > + static nowarn_spec_t from_bits (unsigned bits)
> > > + {
> > > + nowarn_spec_t spec;
> > > + spec.m_bits = bits;
> > > + return spec;
> > > + }
> > > +
> > > /* Return the raw bitset. */
> > > operator unsigned() const
> > > {
> > > diff --git a/gcc/testsuite/g++.dg/modules/warn-spec-1_a.C
> > > b/gcc/testsuite/g++.dg/modules/warn-spec-1_a.C
> > > new file mode 100644
> > > index 00000000000..22be880216d
> > > --- /dev/null
> > > +++ b/gcc/testsuite/g++.dg/modules/warn-spec-1_a.C
> > > @@ -0,0 +1,10 @@
> > > +// PR c++/115757
> > > +// { dg-additional-options "-fmodules-ts -Wunused" }
> > > +// { dg-module-cmi test }
> > > +
> > > +export module test;
> > > +
> > > +export template <typename T>
> > > +void foo(T n [[maybe_unused]]) {
> > > + int x [[maybe_unused]];
> > > +}
> > > diff --git a/gcc/testsuite/g++.dg/modules/warn-spec-1_b.C
> > > b/gcc/testsuite/g++.dg/modules/warn-spec-1_b.C
> > > new file mode 100644
> > > index 00000000000..aa5f14c8572
> > > --- /dev/null
> > > +++ b/gcc/testsuite/g++.dg/modules/warn-spec-1_b.C
> > > @@ -0,0 +1,8 @@
> > > +// PR c++/115757
> > > +// { dg-additional-options "-fmodules-ts -Wunused" }
> > > +
> > > +import test;
> > > +
> > > +int main() {
> > > + foo(0);
> > > +}
> > > diff --git a/gcc/tree.h b/gcc/tree.h
> > > index 28e8e71b036..5dcbb2fb5dd 100644
> > > --- a/gcc/tree.h
> > > +++ b/gcc/tree.h
> > > @@ -6929,6 +6929,8 @@ extern bool warning_suppressed_at (location_t,
> > > opt_code = all_warnings);
> > > at a location to disabled by default. */
> > > extern bool suppress_warning_at (location_t, opt_code = all_warnings,
> > > bool = true);
> > > +/* Overwrite warning disposition bitmap for a location with given spec.
> > > */
> > > +extern void put_warning_spec_at (location_t loc, unsigned);
> > > /* Copy warning disposition from one location to another. */
> > > extern void copy_warning (location_t, location_t);
> > > @@ -6942,6 +6944,13 @@ extern void suppress_warning (tree, opt_code =
> > > all_warnings, bool = true)
> > > /* Copy warning disposition from one expression to another. */
> > > extern void copy_warning (tree, const_tree);
> > > +/* Whether the tree might have a warning spec. */
> > > +extern bool has_warning_spec (const_tree);
> > > +/* Retrieve warning spec bitmap for tree streaming. */
> > > +extern unsigned get_warning_spec (const_tree);
> > > +/* Overwrite warning spec bitmap for a tree with given spec. */
> > > +extern void put_warning_spec (tree, unsigned);
> > > +
> > > /* Return the zero-based number corresponding to the argument being
> > > deallocated if FNDECL is a deallocation function or an out-of-bounds
> > > value if it isn't. */
> > > diff --git a/gcc/warning-control.cc b/gcc/warning-control.cc
> > > index 08a0bb0e7d5..11ca5db69c3 100644
> > > --- a/gcc/warning-control.cc
> > > +++ b/gcc/warning-control.cc
> > > @@ -256,3 +256,29 @@ copy_warning (gimple *to, const gimple *from)
> > > return;
> > > copy_warning<gimple *, const gimple *>(to, from);
> > > }
> > > +
> > > +/* Whether the tree might have a warning spec. */
> > > +
> > > +bool has_warning_spec (const_tree t)
> > > +{
> > > + const location_t loc = get_location (t);
> > > + return !RESERVED_LOCATION_P (loc) && !get_no_warning_bit (t);
> > > +}
> > > +
> > > +/* Retrieve warning dispostion bitmap for tree streaming. */
> > > +
> > > +unsigned
> > > +get_warning_spec (const_tree t)
> > > +{
> > > + const nowarn_spec_t *spec = get_nowarn_spec (t);
> > > + return spec ? *spec : 0;
> > > +}
> > > +
> > > +/* Write warning disposition bitmap for streamed-in tree. */
> > > +
> > > +void
> > > +put_warning_spec (tree t, unsigned bits)
> > > +{
> > > + const location_t loc = get_location (t);
> > > + put_warning_spec_at (loc, bits);
> > > +}
> >
> >
>
