On Fri, Oct 03, 2025 at 05:29:13PM +0100, Jason Merrill wrote:
> On 9/26/25 1:14 PM, Nathaniel Shead wrote:
> > Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?
> >
> > -- >8 --
> >
> > While investigating another issue I noticed that the condition in
> > check_module_override seems incorrect: the wording in [basic.link] p11
> > has no exceptions for internal-linkage entities.
>
> Agreed, it only mentions reachability. But perhaps there should be such an
> exception? Do other compilers error on this case? OK if they do.
>
Thanks. Clang seems happy with this case but MSVC complains:
// a.ixx
export module A;
namespace ns {}
// b.cpp
import A;
static int ns;
$ cl /c /std:c++latest /nologo a.ixx
a.ixx
$ cl /c /std:c++latest /nologo b.cpp
b.cpp
b.cpp(2): error C2365: 'ns': redefinition; previous definition was 'namespace'
...\a.ixx(2): note: see declaration of 'ns'
This change is also needed for the followup to this patch, since in
pushtag the decl is not TREE_PUBLIC until after pushdecl is called.
Though I might be able to find a way around that.
I also don't think this would be particularly useful to allow, as the
definition of 'ns' in b.cpp can be allowed by wrapping it into an
anonymous namespace instead of just marking 'static'. But maybe there's
something I'm missing.
Unless you have any other issues I'll push this patch and its follow-on
sometime next week then.
> > gcc/cp/ChangeLog:
> >
> > * name-lookup.cc (check_module_override): Remove check for
> > TREE_PUBLIC when checking mergeable entities.
> >
> > gcc/testsuite/ChangeLog:
> >
> > * g++.dg/modules/namespace-1_c.C: Adjust to expect errors.
> > * g++.dg/modules/namespace-2_b.C: Likewise.
> > * g++.dg/modules/namespace-3_a.C: Removed.
> > * g++.dg/modules/namespace-3_b.C: Removed.
> >
> > Signed-off-by: Nathaniel Shead <[email protected]>
> > ---
> > gcc/cp/name-lookup.cc | 2 +-
> > gcc/testsuite/g++.dg/modules/namespace-1_c.C | 14 ++++---------
> > gcc/testsuite/g++.dg/modules/namespace-2_b.C | 13 +-----------
> > gcc/testsuite/g++.dg/modules/namespace-3_a.C | 21 --------------------
> > gcc/testsuite/g++.dg/modules/namespace-3_b.C | 12 -----------
> > 5 files changed, 6 insertions(+), 56 deletions(-)
> > delete mode 100644 gcc/testsuite/g++.dg/modules/namespace-3_a.C
> > delete mode 100644 gcc/testsuite/g++.dg/modules/namespace-3_b.C
> >
> > diff --git a/gcc/cp/name-lookup.cc b/gcc/cp/name-lookup.cc
> > index 32af7a6aed9..86482bc110e 100644
> > --- a/gcc/cp/name-lookup.cc
> > +++ b/gcc/cp/name-lookup.cc
> > @@ -3856,7 +3856,7 @@ check_module_override (tree decl, tree mvec, bool
> > hiding,
> > }
> > }
> > - if (TREE_PUBLIC (scope) && TREE_PUBLIC (STRIP_TEMPLATE (decl))
> > + if (TREE_PUBLIC (scope)
> > /* Namespaces are dealt with specially in
> > make_namespace_finish. */
> > && !(TREE_CODE (decl) == NAMESPACE_DECL && !DECL_NAMESPACE_ALIAS
> > (decl)))
> > diff --git a/gcc/testsuite/g++.dg/modules/namespace-1_c.C
> > b/gcc/testsuite/g++.dg/modules/namespace-1_c.C
> > index 748ef5d79a4..e4f81b6bea6 100644
> > --- a/gcc/testsuite/g++.dg/modules/namespace-1_c.C
> > +++ b/gcc/testsuite/g++.dg/modules/namespace-1_c.C
> > @@ -1,13 +1,7 @@
> > // { dg-additional-options "-fmodules-ts" }
> > -// The indirect import of frob, with namespaces impl and ompl doesn't
> > -// affect us.
> > -static int impl;
> > -import Frink;
> > -static int ompl;
> > +static int impl; // IF but no diagnostic required: impl@Frob not
> > reachable from here
> > +
> > +import Frink;
> > -void corge (int x)
> > -{
> > - impl = x;
> > - ompl = frab (x);
> > -}
> > +static int ompl; // { dg-error "different kind" }
> > diff --git a/gcc/testsuite/g++.dg/modules/namespace-2_b.C
> > b/gcc/testsuite/g++.dg/modules/namespace-2_b.C
> > index 6ab5113c23a..d71c630c75e 100644
> > --- a/gcc/testsuite/g++.dg/modules/namespace-2_b.C
> > +++ b/gcc/testsuite/g++.dg/modules/namespace-2_b.C
> > @@ -2,16 +2,5 @@
> > import foo;
> > -static int also_not_exported; // ok
> > -
> > -void X ()
> > -{
> > - implicit_export::bob ();
> > -}
> > -
> > +static int also_not_exported; // { dg-error "different kind" }
> > static int implicit_export; // { dg-error "different kind" }
> > -
> > -void Y ()
> > -{
> > - also_not_exported = 1;
> > -}
> > diff --git a/gcc/testsuite/g++.dg/modules/namespace-3_a.C
> > b/gcc/testsuite/g++.dg/modules/namespace-3_a.C
> > deleted file mode 100644
> > index 8e9508d7ff8..00000000000
> > --- a/gcc/testsuite/g++.dg/modules/namespace-3_a.C
> > +++ /dev/null
> > @@ -1,21 +0,0 @@
> > -// Check namespace needed only by internal reference is not made visible
> > -// { dg-additional-options "-fmodules-ts" }
> > -
> > -export module frob;
> > -// { dg-module-cmi frob }
> > -
> > -namespace silent
> > -{
> > - namespace inner
> > - {
> > - static int X ()
> > - {
> > - return 1;
> > - }
> > - }
> > -}
> > -
> > -export int f (int y)
> > -{
> > - return y + silent::inner::X ();
> > -}
> > diff --git a/gcc/testsuite/g++.dg/modules/namespace-3_b.C
> > b/gcc/testsuite/g++.dg/modules/namespace-3_b.C
> > deleted file mode 100644
> > index f779ffe8c8f..00000000000
> > --- a/gcc/testsuite/g++.dg/modules/namespace-3_b.C
> > +++ /dev/null
> > @@ -1,12 +0,0 @@
> > -// { dg-additional-options "-fmodules-ts" }
> > -
> > -import frob;
> > -
> > -int x = silent; // { dg-error "not declared" }
> > -
> > -static int silent;
> > -
> > -int user ()
> > -{
> > - return f (silent);
> > -}
>
