[Bug c++/114867] New: [modules] name lookup issues when a function overload set is exported from GMF

[Bug c++/114867] New: [modules] name lookup issues when a function overload set is exported from GMF

[m.cencora at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114867

            Bug ID: 114867
           Summary: [modules] name lookup issues when a function overload
                    set is exported from GMF
           Product: gcc
           Version: 14.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: m.cencora at gmail dot com
  Target Milestone: ---

$ cat future.cpp 
module;

namespace std {
template<class _Tp>
void swap(_Tp& a, _Tp& b);

namespace __exception_ptr {
class exception_ptr
{};
void swap(exception_ptr&, exception_ptr&);
}
using __exception_ptr::swap;

template < typename _Tp> struct shared_ptr {
  void swap(shared_ptr& other) { std::swap(_M_ptr, other._M_ptr); }
  _Tp *_M_ptr;
};

struct future {
  shared_ptr< void > _M_state;
  void operator=(future& other) { _M_state.swap(other._M_state); }
};
} // std

export module std;
namespace std {
using std::swap;
}

$ g++ -std=c++2b -fmodules-ts -Wno-global-module future.cpp

<source>: In instantiation of 'void
std::shared_ptr<_Tp>::swap(std::shared_ptr<_Tp>&) [with _Tp = void]':
<source>:23:48:   required from here
   23 |   void operator=(future& other) { _M_state.swap(other._M_state); }
      |                                   ~~~~~~~~~~~~~^~~~~~~~~~~~~~~~
<source>:17:44: error: invalid initialization of reference of type
'std::__exception_ptr::exception_ptr&' from expression of type 'void*'
   17 |   void swap(shared_ptr& other) { std::swap(_M_ptr, other._M_ptr); }
      |                                            ^~~~~~
<source>:10:11: note: in passing argument 1 of 'void
std::__exception_ptr::swap(exception_ptr&, exception_ptr&)'
   10 | void swap(exception_ptr&, exception_ptr&);
      |           ^~~~~~~~~~~~~~
<source>:27:8: warning: not writing module 'std' due to errors
   27 | export module std;
      |        ^~~~~~

This is reduced code from an issue found when creating std
module(https://gcc.gnu.org/PR114600#c10)
module;

#include <future>

export module std;

export namespace std
{
using std::swap;
}



The problem seems to be that using __exception_ptr::swap; hides any other
previous declaration of swap function.
The workarounds is to move decl of template swap after the using
__exception_ptr::swap;

Also this compiles fine if we change shared_ptr::_M_ptr to not become a
template dependent type.

[Bug c++/114867] [modules] name lookup issues when a function overload set is exported from GMF

[m.cencora at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114867

--- Comment #1 from m.cencora at gmail dot com ---
The unreduced code is actually a regression from gcc-12.

@Jonathan Wakely:
Could you maybe workaround it in libstdc++ by declaring the std::swap overload
for exception_ptr in additional inline namespace in std, like this:

namespace std {

namespace __exception_ptr {
    class exception_ptr;

    void swap(exception_ptr& a, exception_ptr& b);
}

using __exception_ptr::exception_ptr;

inline namespace __exception_ptr_swap {
    using __exception_ptr::swap;
}

}

[Bug c++/114867] [modules] name lookup issues when a function overload set is exported from GMF

[nshead at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114867

Nathaniel Shead <nshead at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
                 CC|                            |nshead at gcc dot gnu.org
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2024-05-25

--- Comment #2 from Nathaniel Shead <nshead at gcc dot gnu.org> ---
Confirmed.  The issue is that this part of 'do_nonmember_using_decl' for the
'using std::swap' declaration in the module purview removes the template
candidate and reinserts it to "reveal" it (possibly as newly exported):

  if (new_fn == old_fn)
    {
      /* The function already exists in the current
         namespace.  We will still want to insert it if
         it is revealing a not-revealed thing.  */
      found = true;
      if (!revealing_p)
        ;
      else if (old.using_p ())
        {
          if (exporting)
            /* Update in place.  'tis ok.  */
            OVL_EXPORT_P (old.get_using ()) = true;
          ;
        }
      else if (DECL_MODULE_EXPORT_P (new_fn))
        ;
      else
        {
          value = old.remove_node (value);
          found = false;
        }
      break;
    }

  // ...

  if (!found && insert_p)
    /* Unlike the decl-pushing case we don't drop anticipated
       builtins here.  They don't cause a problem, and we'd
       like to match them with a future declaration.  */
    value = ovl_insert (new_fn, value, 1 + exporting);

However, within the 'shared_ptr::swap' template it's already holding a pointer
to the OVERLOAD, and the 'remove_node' call drops it from this existing
overload set but the 'ovl_insert' doesn't add it back to that set, rather
returning a new one.

I suppose a fix is possibly to ensure that we only do this update in-place but
I  don't know if that'll work with OVL_HIDDEN declarations, since they need to
always be kept at the front of the list, and reordering might still cause the
same issues.

[Bug c++/114867] [modules] name lookup issues when a function overload set is exported from GMF

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114867

--- Comment #3 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Nathaniel Shead <nshead@gcc.gnu.org>:

https://gcc.gnu.org/g:85f15ea65a97686ad39af0c14b7dd9a9372e3a19

commit r15-964-g85f15ea65a97686ad39af0c14b7dd9a9372e3a19
Author: Nathaniel Shead <nathanieloshead@gmail.com>
Date:   Sat Jun 1 01:14:44 2024 +1000

    c++/modules: Fix revealing with using-decls [PR114867]

    This patch fixes a couple issues with the current handling of revealing
    declarations with using-decls.

    Firstly, doing 'remove_node' when handling function overload sets is not
    safe, because it not only mutates the OVERLOAD we're walking over but
    potentially any other references to this OVERLOAD that are cached from
    phase-1 template lookup.  This causes the attached using-17 testcase to
    fail because the overload set in 'X::test()' no longer contains the
    'ns::f(T)' template once instantiated at the end of the file.

    This patch works around this by simply not removing the old declaration.
    This does make the overload list potentially longer than it otherwise
    would have been, but only when re-exporting the same set of functions in
    a using-decl.  Additionally, because 'ovl_insert' always prepends these
    newly inserted overloads, repeated exported using-decls won't continue
    to add declarations, as the first exported using-decl will be found
    before the original (unexported) declaration.

    Another, related, issue is that using-decls of GMF entities currently
    doesn't mark them as reachable unless they are also exported, and thus
    they may not be available in e.g. module implementation units.  We solve
    this with a new flag on OVERLOADs set when they are declared within the
    module purview.  This starts to run into the more general issue of
    handling using-decls of non-functions (see e.g. PR114863) but by just
    marking such GMF entities as purview we can work around this for now.

    This also allows us to get rid of the special-casing of exported
    using-decls in 'add_binding_entity', which was incorrect anyway: a
    non-exported using-decl still needs to be emitted anyway if it lives in
    the module purview, even if referring to a non-purview item.

            PR c++/114867

    gcc/cp/ChangeLog:

            * cp-tree.h (OVL_PURVIEW_P): New.
            (ovl_iterator::purview_p): New.
            * module.cc (depset::hash::add_binding_entity): Only ignore
            entities not within module purview. Set OVL_PURVIEW_P on new
            OVERLOADs for emitted declarations.
            (module_state::read_cluster): Imported using-decls are always
            in purview, mark as OVL_PURVIEW_P.
            * name-lookup.h (enum WMB_Flags): New WMB_Purview flag.
            * name-lookup.cc (walk_module_binding): Set WMB_Purview as
            needed.
            (do_nonmember_using_decl): Don't remove from existing OVERLOADs.
            Also reveal non-exported decls. Also reveal 'extern "C"' decls.
            Add workaround to reveal non-function decls.
            * tree.cc (ovl_insert): Adjust to also set OVL_PURVIEW_P when
            needed.

    gcc/testsuite/ChangeLog:

            * g++.dg/modules/using-17_a.C: New test.
            * g++.dg/modules/using-17_b.C: New test.
            * g++.dg/modules/using-18_a.C: New test.
            * g++.dg/modules/using-18_b.C: New test.

    Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>

[Bug c++/114867] [modules] name lookup issues when a function overload set is exported from GMF

[nshead at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114867

Nathaniel Shead <nshead at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |15.0
         Resolution|---                         |FIXED
           Assignee|unassigned at gcc dot gnu.org      |nshead at gcc dot gnu.org
             Status|NEW                         |RESOLVED

--- Comment #4 from Nathaniel Shead <nshead at gcc dot gnu.org> ---
Fixed for GCC 15.