On Thu, Mar 06, 2025 at 12:45:33PM -0500, Jason Merrill wrote:
> On 3/5/25 7:54 AM, Nathaniel Shead wrote:
> > On Wed, Feb 26, 2025 at 10:29:59AM -0500, Jason Merrill wrote:
> > > On 2/21/25 6:05 AM, Nathaniel Shead wrote:
> > > > After seeing PR c++/118964 I'm coming back around to this [1] patch
> > > > series, since it appears that this can cause errors on otherwise valid
> > > > code by instantiations coming into module purview that reference
> > > > TU-local entities.
> > > >
> > > > [1]: https://gcc.gnu.org/pipermail/gcc-patches/2024-May/650324.html
> > > >
> > > > We'd previously discussed that the best way to solve this in general
> > > > would be to perform all deferred instantiations at the end of the GMF to
> > > > ensure that they would not leak into the module purview. I still
> > > > tentatively agree that this would be the "nicer" way to go (though I've
> > > > since come across https://eel.is/c++draft/temp.point#7 and wonder if
> > > > this may not be strictly conforming according to the current wording?).
> > >
> > > Hmm, interesting point.
> > >
> > > > I've not yet managed to implement this however, and even if I had, at
> > > > this stage it would definitely not be appropriate for GCC15; would the
> > > > approach described below be appropriate for GCC15 as a stop-gap to
> > > > reduce these issues?
> > >
> > > As I suggested earlier in this discussion, it seems to me that the
> > > purviewness of an implicit instantiation shouldn't matter, they should all
> > > be treated as discardable.
> >
> > So looking further I believe that currently they are not, in that
> > implicit instantiations themselves are only ever emitted if they are
> > reached from elsewhere (regardless of their purviewness), so the issues
> > are mostly just with deferred explicit instantiations etc. However...
> >
> > > Could we instead set DECL_MODULE_PURVIEW_P as appropriate when we see an
> > > explicit instantiation and use that + DECL_EXPLICIT_INSTANTIATION to
> > > recompute module_kind?
>
> As in the attached, which works for me. Do you see a problem with this
> approach?
>
Ah I see, I think I misunderstood; yes, this makes sense to me and
should work fine. I'm OK with this patch; here's another couple of
testcases I tried as well that also passed with your change:
-- >8 --
// { dg-additional-options "-fmodules-ts -Wno-global-module -fdump-lang-module"
}
// { dg-module-cmi M }
module;
// Deferred instantiation of GM virtual functions or friend functions
// should not place newly discovered declarations in the module purview.
template <typename T>
void go() {
extern T fn_decl();
}
template <typename T>
struct S {
friend void x() {}
virtual void f() {
go<char>();
}
};
S<int> s;
export module M;
// The whole GMF should be discarded here
// { dg-final { scan-lang-dump "Wrote 0 clusters" module } }
-- >8 --
// { dg-additional-options "-fmodules-ts -fdump-lang-module" }
// { dg-module-cmi empty }
module;
#include "xtreme-header.h"
export module empty;
// The whole GMF should be discarded here
// { dg-final { scan-lang-dump "Wrote 0 clusters" module } }
-- >8 --
> > > Or go with this patch but only look at the saved module_kind if
> > > DECL_EXPLICIT_INSTANTIATION, but that seems a bit of a waste of space.
> > > Might
> > > be safer at this point, though.
> >
> > Part of the issue is handling things that are "implicit instantiations"
> > that we don't track were as such; the obvious case here is friend decls,
> > which get instantiated with the current purviewness but might only have
> > been instantiated due to a deferred implicit instantiation, and for
> > which we remove any evidence that they were created due to an implicit
> > instantiation.
> >
> > So maybe looking at the problem from that side would also work, but I'm
> > not sure exactly how best to do that just yet.
> >
> > > > Another approach would be to maybe lower the error to a permerror, so
> > > > that users that 'know better' could compile such modules anyway (at risk
> > > > of various link errors and ODR issues), though I would also need to
> > > > adjust the streaming logic to handle this better. Thoughts?
> > >
> > > This also sounds desirable. With a warning flag so it can be disabled for
> > > certain deliberate cases like the gthr stuff.
> >
> > OK, I'll try and took a look if I get a chance (unfortunately life has
> > interrupted me again for the time being...). I'm not sure that this
> > would work for the gthr case though (since the error occurs at the point
> > of use not at the point of declaration); for that we would probably need
> > some kind of attribute I guess?
>
> We frequently use warning_enabled_at to suppress a diagnostic based on
> whether the diagnostic is enabled at the point of definition of some entity.
>
> Jason
Ah, thanks for the pointer; I didn't realise that existed. I'm not sure
I'll have anything done in the GCC 15 timeframe though unfortunately,
but I'll see how I go.
Nathaniel
