On Tue, Nov 04, 2025 at 12:57:53PM +0300, Jason Merrill wrote:
> On 10/30/25 3:00 PM, Nathaniel Shead wrote:
> > One unfortunate side effect of this is that even with -pedantic-errors,
> > unless the user specifies '-Wtemplate-names-tu-local' when building the
> > module interface there will be no diagnostic at all from instantiating a
> > template that exposes global TU-local entities, either when building the
> > module or its importer.
> >
> > Would it be reasonable to attempt to change behaviour when
> > '-pedantic-errors' or '-Werror=expose-global-module-tu-local' is
> > specified and build TU_LOCAL_ENTITY nodes for such declarations in this
> > case? (Though I'm not sure how to detect the latter.) Or maybe even go
> > the other way around and always do this for template bodies unless
> > '-fpermissive' (or somesuch) is specified; thoughts?
> >
> > Otherwise bootstrapped and regtested on x86_64-pc-linux-gnu, OK for
> > trunk?
> >
> > -- >8 --
> >
> > A frequent issue with migrating to C++20 modules has been dealing with
> > third-party libraries with internal functions or data. This causes GCC
> > to currently refuse to build the module if any references to these
> > internal-linkage declarations escape into the module CMI.
> >
> > This can seem needlessly hostile, however, especially since we have the
> > capabilities to support this (to a degree) from header units, albeit
> > with the inherent ODR issues associated with their use. In aid of this,
> > this patch demotes the error to a pedwarn in various scenarios, by
> > treating some declarations as not being TU-local even if they otherwise
> > would have been.
> >
> > Effort has been made to not alter semantics of valid programs, and to
> > continue to diagnose cases that the standard says we must. In
> > particular, any code in the module purview is still a hard error, due to
> > the inherent issues with exposing TU-local entities, and the lack of any
> > migration requirements. One unfortunate side effect however is that
> > instantiations of templates referencing gmf TU-local entities will have
> > no diagnostics on the importing side at all.
> >
> > Because this patch is just to assist migration, we only deal with the
> > simplest (yet most common) cases: namespace scope functions and
> > variables. Types are hard to handle neatly as we risk getting thousands
> > of unhelpful warnings as we continue to walk the type body and find new
> > TU-local entities to complain about. Templates are also tricky because
> > it's hard to tell if an instantiation that occurred in the module
> > purview only refers to global module entities or if it's inadvertantly
> > exposing a purview entity as well. Neither of these are likely to occur
> > frequently in third-party code; if need be, this can be relaxed later as
> > well.
> >
> > Similarly, even in the GMF a constexpr variable with a TU-local value
> > will not be usable in constant expressions in the importer, and since we
> > cannot easily warn about this from the importer we continue to make this
> > an error in the module interface.
> >
> > @@ -15334,6 +15377,128 @@ template_has_explicit_inst (tree tmpl)
> > return false;
> > }
> > +/* Complain about DEP that exposes a TU-local entity.
> > +
> > + If STRICT, DEP only referenced entities from the GMF. Returns TRUE
> > + if we explained anything. */
> > +
> > +bool
> > +depset::hash::diagnose_bad_internal_ref (depset *dep, bool strict)
> > +{
> > + tree decl = dep->get_entity ();
> > +
> > + /* Don't need to walk if we're not going to be emitting
> > + any diagnostics anyway. */
> > + if (strict && !warning_enabled_at (DECL_SOURCE_LOCATION (decl),
> > + OPT_Wexpose_global_module_tu_local))
> > + return false;
> > +
> > + for (depset *rdep : dep->deps)
> > + if (!rdep->is_binding () && rdep->is_tu_local (strict)
> > + && !is_exposure_of_member_type (dep, rdep))
> > + {
> > + // FIXME:QOI Better location information? We're
> > + // losing, so it doesn't matter about efficiency
>
> Period at end of comment.
>
Fixed on my branch.
> > + tree exposed = rdep->get_entity ();
> > + auto_diagnostic_group d;
> > + if (strict)
> > + {
> > + /* Allow suppressing the warning from the point of declaration
> > + of the otherwise-exposed decl, for cases we know that
> > + exposures will never be 'bad'. */
> > + if (warning_enabled_at (DECL_SOURCE_LOCATION (exposed),
> > + OPT_Wexpose_global_module_tu_local)
> > + && pedwarn (DECL_SOURCE_LOCATION (decl),
> > + OPT_Wexpose_global_module_tu_local,
> > + "%qD exposes TU-local entity %qD", decl, exposed))
> > + {
> > + bool informed = is_tu_local_entity (exposed, /*explain=*/true);
> > + gcc_checking_assert (informed);
> > + return true;
> > + }
> > + }
> > + else
> > + {
> > + error_at (DECL_SOURCE_LOCATION (decl),
> > + "%qD exposes TU-local entity %qD", decl, exposed);
> > + bool informed = is_tu_local_entity (exposed, /*explain=*/true);
> > + gcc_checking_assert (informed);
> > + if (dep->is_tu_local (/*strict=*/true))
> > + inform (DECL_SOURCE_LOCATION (decl),
> > + "%qD is also TU-local but has been exposed elsewhere",
> > + decl);
>
> Is it worthwhile to warn in this case? We should certainly complain about
> the original exposure, but after that it's natural for internal things to
> refer to other internal things, this seems like it would be undesirable
> diagnostic cascade.
>
This is giving an explanation when an internal entity is an exposure,
but the error would have normally been silenced exactly because we
don't complain if a TU-local entity references another TU-local entity.
For instance, in:
module;
static inline int x = []{ return 1; }();
export module M;
using ::x;
the error here is complaining about the TU-local lambda type, exposed by
the 'using ::x' (TU-local because it inherited the linkage of 'x').
This is necessary because the exposure of 'x' is now only a warning (and
that warning could even have been disabled), and this additional
explanatory note attempts to describe that. I felt the note was useful
to explain why we're complaining about this despite [basic.link] p17
normally only applying to non-TU-local entities.
> > + return true;
> > + }
> > + }
> > +
> > + return false;
> > +}
> > +
> > @@ -15358,30 +15523,21 @@ depset::hash::finalize_dependencies ()
> > if (CHECKING_P)
> > for (depset *entity : dep->deps)
> > gcc_checking_assert (!entity->is_import ());
> > + continue;
> > }
> > - else if (dep->is_exposure () && !dep->is_tu_local ())
> > - {
> > - ok = false;
> > - bool explained = false;
> > - tree decl = dep->get_entity ();
> > - for (depset *rdep : dep->deps)
> > - if (!rdep->is_binding ()
> > - && rdep->is_tu_local ()
> > - && !is_exposure_of_member_type (dep, rdep))
> > - {
> > - // FIXME:QOI Better location information? We're
> > - // losing, so it doesn't matter about efficiency
>
> Period at end of comment.
>
> Jason
This was the original comment from the implementation that is getting
moved into the new 'diagnose_bad_internal_ref' function.
Thanks,
Nathaniel
