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.
>
> > -- >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);
> > +}
>
>
