On Tue, Sep 23, 2025 at 12:46:31PM +0100, Jason Merrill wrote:
> On 9/22/25 10:26 AM, Nathaniel Shead wrote:
> > Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk/15?
> > 
> > -- >8 --
> > 
> > The ICE in the linked PR is caused because when current_lang_name is
> > lang_name_c, set_decl_linkage calls decl_linkage on the retrofitted
> > declaration.  This is problematic because at this point we haven't
> > finished streaming the declaration, and so we crash when attempting to
> > access these missing bits (such as the type).
> > 
> > The only declarations we can reach here will be things like types that
> > don't get a language linkage anyway, so it seems reasonable to just
> > hardcode a C++ language linkage here to work around the issue.
> > 
> > An alternative fix would be to override current_lang_name in
> > lazy_load_binding instead, but this potentially is potentially confusing
> > if we ever deliberately support `extern "C" import "header_unit.hpp";`
> > to override the language linkage of imported declarations,
> 
> I can't imagine wanting that; an import should not be affected by the
> context of the import.
> 
> But the patch is OK.
> 

Thanks.  Out of interest I dug up the reasoning behind this wording, it
comes from NB ballot US033 https://github.com/cplusplus/nbballot/issues/32
which indicates it's needed to better handle include translation.
(Though the comment implies that GCC already implemented this?)

That said, I don't intend to work on this.

> > so I went
> > with this approach instead.
> 
> > While testing this I found that we don't currently complain about
> > mismatching language linkages for variables, and module_may_redeclare
> > doesn't cope well with implementation units, so this patch also fixes
> > those issues.
> > 
> >     PR c++/122019
> > 
> > gcc/cp/ChangeLog:
> > 
> >     * module.cc (trees_in::install_entity): Don't be affected by
> >     global language linkage state.
> >     (trees_in::is_matching_decl): Check mismatching language linkage
> >     for variables too.
> >     (module_may_redeclare): Report the correct module name for
> >     partitions and implementation units.
> > 
> > gcc/testsuite/ChangeLog:
> > 
> >     * g++.dg/modules/lang-4_a.C: New test.
> >     * g++.dg/modules/lang-4_b.C: New test.
> > 
> > Signed-off-by: Nathaniel Shead <[email protected]>
> > ---
> >   gcc/cp/module.cc                        | 35 +++++++++++++++----------
> >   gcc/testsuite/g++.dg/modules/lang-4_a.C | 22 ++++++++++++++++
> >   gcc/testsuite/g++.dg/modules/lang-4_b.C | 26 ++++++++++++++++++
> >   3 files changed, 69 insertions(+), 14 deletions(-)
> >   create mode 100644 gcc/testsuite/g++.dg/modules/lang-4_a.C
> >   create mode 100644 gcc/testsuite/g++.dg/modules/lang-4_b.C
> > 
> > diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> > index d5c3a83c728..7b69e806328 100644
> > --- a/gcc/cp/module.cc
> > +++ b/gcc/cp/module.cc
> > @@ -8144,7 +8144,14 @@ trees_in::install_entity (tree decl)
> >     if (!DECL_LANG_SPECIFIC (not_tmpl)
> >         || !DECL_MODULE_ENTITY_P (not_tmpl))
> >       {
> > -      retrofit_lang_decl (not_tmpl);
> > +      /* We don't want to use retrofit_lang_decl directly so that we aren't
> > +    affected by the language state when we load in.  */
> > +      if (!DECL_LANG_SPECIFIC (not_tmpl))
> > +   {
> > +     maybe_add_lang_decl_raw (not_tmpl, false);
> > +     gcc_checking_assert (!VAR_OR_FUNCTION_DECL_P (not_tmpl));
> > +     SET_DECL_LANGUAGE (not_tmpl, lang_cplusplus);
> > +   }
> >         DECL_MODULE_ENTITY_P (not_tmpl) = true;
> >         /* Insert into the entity hash (it cannot already be there).  */
> > @@ -12320,7 +12327,15 @@ trees_in::is_matching_decl (tree existing, tree 
> > decl, bool is_typedef)
> >     // FIXME: do more precise errors at point of mismatch
> >     const char *mismatch_msg = nullptr;
> > -  if (TREE_CODE (d_inner) == FUNCTION_DECL)
> > +
> > +  if (VAR_OR_FUNCTION_DECL_P (d_inner)
> > +      && DECL_EXTERN_C_P (d_inner) != DECL_EXTERN_C_P (e_inner))
> > +    {
> > +      mismatch_msg = G_("conflicting language linkage for imported "
> > +                   "declaration %#qD");
> > +      goto mismatch;
> > +    }
> > +  else if (TREE_CODE (d_inner) == FUNCTION_DECL)
> >       {
> >         tree e_ret = fndecl_declared_return_type (existing);
> >         tree d_ret = fndecl_declared_return_type (decl);
> > @@ -12337,13 +12352,6 @@ trees_in::is_matching_decl (tree existing, tree 
> > decl, bool is_typedef)
> >         tree e_type = TREE_TYPE (e_inner);
> >         tree d_type = TREE_TYPE (d_inner);
> > -      if (DECL_EXTERN_C_P (d_inner) != DECL_EXTERN_C_P (e_inner))
> > -   {
> > -     mismatch_msg = G_("conflicting language linkage for imported "
> > -                       "declaration %#qD");
> > -     goto mismatch;
> > -   }
> > -
> >         for (tree e_args = TYPE_ARG_TYPES (e_type),
> >          d_args = TYPE_ARG_TYPES (d_type);
> >        e_args != d_args && (e_args || d_args);
> > @@ -21151,7 +21159,7 @@ module_may_redeclare (tree olddecl, tree newdecl)
> >       // FIXME: Should we be checking this in more places on the scope 
> > chain?
> >       return true;
> > -  module_state *old_mod = this_module ();
> > +  module_state *old_mod = get_primary (this_module ());
> >     module_state *new_mod = old_mod;
> >     tree old_origin = get_originating_module_decl (decl);
> > @@ -21161,7 +21169,7 @@ module_may_redeclare (tree olddecl, tree newdecl)
> >     if (DECL_LANG_SPECIFIC (old_inner) && DECL_MODULE_IMPORT_P (old_inner))
> >       {
> >         unsigned index = import_entity_index (old_origin);
> > -      old_mod = import_entity_module (index);
> > +      old_mod = get_primary (import_entity_module (index));
> >       }
> >     bool newdecl_attached_p = module_attach_p ();
> > @@ -21174,7 +21182,7 @@ module_may_redeclare (tree olddecl, tree newdecl)
> >         if (DECL_LANG_SPECIFIC (new_inner) && DECL_MODULE_IMPORT_P 
> > (new_inner))
> >     {
> >       unsigned index = import_entity_index (new_origin);
> > -     new_mod = import_entity_module (index);
> > +     new_mod = get_primary (import_entity_module (index));
> >     }
> >       }
> > @@ -21185,8 +21193,7 @@ module_may_redeclare (tree olddecl, tree newdecl)
> >     /* Both are GM entities, OK.  */
> >     return true;
> > -      if (new_mod == old_mod
> > -     || (new_mod && get_primary (new_mod) == get_primary (old_mod)))
> > +      if (new_mod == old_mod)
> >     /* Both attached to same named module, OK.  */
> >     return true;
> >       }
> > diff --git a/gcc/testsuite/g++.dg/modules/lang-4_a.C 
> > b/gcc/testsuite/g++.dg/modules/lang-4_a.C
> > new file mode 100644
> > index 00000000000..cef2aae00ed
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/modules/lang-4_a.C
> > @@ -0,0 +1,22 @@
> > +// PR c++/122019
> > +// { dg-additional-options "-fmodules -Wno-global-module" }
> > +
> > +module;
> > +
> > +typedef int pthread_once_t;
> > +
> > +export module M;
> > +
> > +namespace ns {
> > +  using ::pthread_once_t;
> > +}
> > +
> > +// note: non-function types don't have language linkage
> > +extern "C++" enum E { c };
> > +extern "C" typedef int T;
> > +
> > +extern "C" int foo;
> > +extern "C++" int bar;
> > +
> > +extern "C" int qux;
> > +extern "C++" int zap;
> > diff --git a/gcc/testsuite/g++.dg/modules/lang-4_b.C 
> > b/gcc/testsuite/g++.dg/modules/lang-4_b.C
> > new file mode 100644
> > index 00000000000..2085ed15211
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/modules/lang-4_b.C
> > @@ -0,0 +1,26 @@
> > +// PR c++/122019
> > +// { dg-additional-options "-fmodules -Wno-global-module" }
> > +// Language linkage for types, variables, and lazy-loading in extern 
> > contexts.
> > +
> > +module;
> > +
> > +extern "C" enum E { c };
> > +extern "C++" typedef int T;
> > +
> > +extern "C++" int foo;  // { dg-message "existing" }
> > +extern "C" int bar;  // { dg-message "existing" }
> > +
> > +module M;
> > +
> > +extern "C" ns::pthread_once_t x;
> > +
> > +E e;
> > +T t;
> > +
> > +extern "C" { int use1 = foo; }  // { dg-message "during load" }
> > +extern "C" { int use2 = bar; }  // { dg-message "during load" }
> > +// { dg-error "conflicting language linkage for imported declaration 'int 
> > foo'" "" { target *-*-* } 0 }
> > +// { dg-error "conflicting language linkage for imported declaration 'int 
> > bar'" "" { target *-*-* } 0 }
> > +
> > +extern "C++" int qux;  // { dg-error "conflicting declaration" }
> > +extern "C" int zap;  // { dg-error "conflicting declaration" }
> 

Reply via email to