Re: [PATCH RFC] c++: lambda mangling alias issues [PR107897]

2023-03-30 Thread Jan Hubicka via Gcc-patches
> 
> How about moving it to symtab_node and using dyn_cast for the cgraph bits,
> like this:

> From 1d869ceb04573727e59be6518903133c8654069a Mon Sep 17 00:00:00 2001
> From: Jason Merrill 
> Date: Mon, 6 Mar 2023 15:33:45 -0500
> Subject: [PATCH] c++: lambda mangling alias issues [PR107897]
> To: gcc-patches@gcc.gnu.org
> 
> In 107897, by the time we are looking at the mangling clash, the
> alias has already been removed from the symbol table by analyze_functions,
> so we can't look at n->cpp_implicit_alias.  So just assume that it's an
> alias if it's internal.
> 
> In 108887 the problem is that removing the mangling alias from the symbol
> table confuses analyze_functions, because it ended up as first_analyzed
> somehow, so it becomes a dangling pointer.  Fixed by clearing various flags
> to neutralize the alias.
> 
>   PR c++/107897
>   PR c++/108887
> 
> gcc/ChangeLog:
> 
>   * cgraph.h: Move reset() from cgraph_node to symtab_node.
>   * cgraphunit.cc (symtab_node::reset): Adjust.
> 
> gcc/cp/ChangeLog:
> 
>   * decl2.cc (record_mangling): Use symtab_node::reset.
> 
> gcc/testsuite/ChangeLog:
> 
>   * g++.dg/cpp2a/concepts-lambda3.C: Use -flto if supported.
>   * g++.dg/cpp0x/lambda/lambda-mangle7.C: New test.

OK and I apologize for the delay (my travels were more busy than I
hoped)
Honza


Re: [PATCH RFC] c++: lambda mangling alias issues [PR107897]

2023-03-29 Thread Martin Jambor
Hello,

On Wed, Mar 08 2023, Jason Merrill via Gcc-patches wrote:
> On 3/8/23 11:15, Jason Merrill wrote:
>> On 3/8/23 10:53, Jan Hubicka wrote:
[...]
>>> We have n->reset () for that which is used in similar situation when
>>> frontends overwrites extern inline function by its different offline
>>> implementation.
>> 
>> The problem there is that reset() is a member of cgraph_node, not 
>> symtab_node, and I need something that works for variables as well.
>> 
>>> reset doesn't call remove_from_same_comdat_group probably because it was
>>> never used on anything in comdat group.  So I think it would make sense
>>> to call n->reset() here and add remove_from_same_comdat_group into that.
>
> How about moving it to symtab_node and using dyn_cast for the cgraph 
> bits, like this:
> From 1d869ceb04573727e59be6518903133c8654069a Mon Sep 17 00:00:00 2001
> From: Jason Merrill 
> Date: Mon, 6 Mar 2023 15:33:45 -0500
> Subject: [PATCH] c++: lambda mangling alias issues [PR107897]
> To: gcc-patches@gcc.gnu.org
>
> In 107897, by the time we are looking at the mangling clash, the
> alias has already been removed from the symbol table by analyze_functions,
> so we can't look at n->cpp_implicit_alias.  So just assume that it's an
> alias if it's internal.
>
> In 108887 the problem is that removing the mangling alias from the symbol
> table confuses analyze_functions, because it ended up as first_analyzed
> somehow, so it becomes a dangling pointer.  Fixed by clearing various flags
> to neutralize the alias.
>
>   PR c++/107897
>   PR c++/108887
>
> gcc/ChangeLog:
>
>   * cgraph.h: Move reset() from cgraph_node to symtab_node.
>   * cgraphunit.cc (symtab_node::reset): Adjust.
>
> gcc/cp/ChangeLog:
>
>   * decl2.cc (record_mangling): Use symtab_node::reset.

The patch is OK.

Thanks,

Martin


PING^2 Re: [PATCH RFC] c++: lambda mangling alias issues [PR107897]

2023-03-28 Thread Jason Merrill via Gcc-patches

On 3/14/23 11:16, Jason Merrill wrote:

On 3/8/23 11:54, Jason Merrill wrote:

On 3/8/23 11:15, Jason Merrill wrote:

On 3/8/23 10:53, Jan Hubicka wrote:
Tested x86_64-pc-linux-gnu.  Does this look good, or do we want to 
factor the

flag clearing into a symtab_node counterpart to cgraph_node::reset?

-- 8< --

In 107897, by the time we are looking at the mangling clash, the
alias has already been removed from the symbol table by 
analyze_functions,
so we can't look at n->cpp_implicit_alias.  So just assume that 
it's an

alias if it's internal.

In 108887 the problem is that removing the mangling alias from the 
symbol
table confuses analyze_functions, because it ended up as 
first_analyzed
somehow, so it becomes a dangling pointer.  Fixed by clearing 
various flags

to neutralize the alias.

PR c++/107897
PR c++/108887

gcc/cp/ChangeLog:

* decl2.cc (record_mangling): Improve symbol table handling.

gcc/testsuite/ChangeLog:

* g++.dg/cpp2a/concepts-lambda3.C: Use -flto if supported.
* g++.dg/cpp0x/lambda/lambda-mangle7.C: New test.
---
  gcc/cp/decl2.cc   | 25 +--
  .../g++.dg/cpp0x/lambda/lambda-mangle7.C  | 70 
+++

  gcc/testsuite/g++.dg/cpp2a/concepts-lambda3.C |  1 +
  3 files changed, 91 insertions(+), 5 deletions(-)
  create mode 100644 
gcc/testsuite/g++.dg/cpp0x/lambda/lambda-mangle7.C


diff --git a/gcc/cp/decl2.cc b/gcc/cp/decl2.cc
index 387e24542cd..e6e58b08de4 100644
--- a/gcc/cp/decl2.cc
+++ b/gcc/cp/decl2.cc
@@ -4742,15 +4742,30 @@ record_mangling (tree decl, bool need_warning)
  = mangled_decls->find_slot_with_hash (id, 
IDENTIFIER_HASH_VALUE (id),

    INSERT);
-  /* If this is already an alias, remove the alias, because the real
+  /* If this is already an alias, cancel the alias, because the real
   decl takes precedence.  */
    if (*slot && DECL_ARTIFICIAL (*slot) && DECL_IGNORED_P (*slot))
-    if (symtab_node *n = symtab_node::get (*slot))
-  if (n->cpp_implicit_alias)
+    {
+  if (symtab_node *n = symtab_node::get (*slot))
  {
-  n->remove ();
-  *slot = NULL_TREE;
+  if (n->cpp_implicit_alias)
+    {
+  /* Actually removing the node isn't safe if other code 
is already

+ holding a pointer to it, so just neutralize it.  */
+  n->remove_from_same_comdat_group ();
+  n->analyzed = false;
+  n->definition = false;
+  n->alias = false;
+  n->cpp_implicit_alias = false;

We have n->reset () for that which is used in similar situation when
frontends overwrites extern inline function by its different offline
implementation.


The problem there is that reset() is a member of cgraph_node, not 
symtab_node, and I need something that works for variables as well.


reset doesn't call remove_from_same_comdat_group probably because it 
was

never used on anything in comdat group.  So I think it would make sense
to call n->reset() here and add remove_from_same_comdat_group into 
that.


How about moving it to symtab_node and using dyn_cast for the cgraph 
bits, like this:


Ping? (Patch in attachment in earlier message)





Ping Re: [PATCH RFC] c++: lambda mangling alias issues [PR107897]

2023-03-14 Thread Jason Merrill via Gcc-patches

On 3/8/23 11:54, Jason Merrill wrote:

On 3/8/23 11:15, Jason Merrill wrote:

On 3/8/23 10:53, Jan Hubicka wrote:
Tested x86_64-pc-linux-gnu.  Does this look good, or do we want to 
factor the

flag clearing into a symtab_node counterpart to cgraph_node::reset?

-- 8< --

In 107897, by the time we are looking at the mangling clash, the
alias has already been removed from the symbol table by 
analyze_functions,

so we can't look at n->cpp_implicit_alias.  So just assume that it's an
alias if it's internal.

In 108887 the problem is that removing the mangling alias from the 
symbol

table confuses analyze_functions, because it ended up as first_analyzed
somehow, so it becomes a dangling pointer.  Fixed by clearing 
various flags

to neutralize the alias.

PR c++/107897
PR c++/108887

gcc/cp/ChangeLog:

* decl2.cc (record_mangling): Improve symbol table handling.

gcc/testsuite/ChangeLog:

* g++.dg/cpp2a/concepts-lambda3.C: Use -flto if supported.
* g++.dg/cpp0x/lambda/lambda-mangle7.C: New test.
---
  gcc/cp/decl2.cc   | 25 +--
  .../g++.dg/cpp0x/lambda/lambda-mangle7.C  | 70 
+++

  gcc/testsuite/g++.dg/cpp2a/concepts-lambda3.C |  1 +
  3 files changed, 91 insertions(+), 5 deletions(-)
  create mode 100644 gcc/testsuite/g++.dg/cpp0x/lambda/lambda-mangle7.C

diff --git a/gcc/cp/decl2.cc b/gcc/cp/decl2.cc
index 387e24542cd..e6e58b08de4 100644
--- a/gcc/cp/decl2.cc
+++ b/gcc/cp/decl2.cc
@@ -4742,15 +4742,30 @@ record_mangling (tree decl, bool need_warning)
  = mangled_decls->find_slot_with_hash (id, 
IDENTIFIER_HASH_VALUE (id),

    INSERT);
-  /* If this is already an alias, remove the alias, because the real
+  /* If this is already an alias, cancel the alias, because the real
   decl takes precedence.  */
    if (*slot && DECL_ARTIFICIAL (*slot) && DECL_IGNORED_P (*slot))
-    if (symtab_node *n = symtab_node::get (*slot))
-  if (n->cpp_implicit_alias)
+    {
+  if (symtab_node *n = symtab_node::get (*slot))
  {
-  n->remove ();
-  *slot = NULL_TREE;
+  if (n->cpp_implicit_alias)
+    {
+  /* Actually removing the node isn't safe if other code is 
already

+ holding a pointer to it, so just neutralize it.  */
+  n->remove_from_same_comdat_group ();
+  n->analyzed = false;
+  n->definition = false;
+  n->alias = false;
+  n->cpp_implicit_alias = false;

We have n->reset () for that which is used in similar situation when
frontends overwrites extern inline function by its different offline
implementation.


The problem there is that reset() is a member of cgraph_node, not 
symtab_node, and I need something that works for variables as well.



reset doesn't call remove_from_same_comdat_group probably because it was
never used on anything in comdat group.  So I think it would make sense
to call n->reset() here and add remove_from_same_comdat_group into that.


How about moving it to symtab_node and using dyn_cast for the cgraph 
bits, like this:


Ping? (Patch in attachment in earlier message)



Re: [PATCH RFC] c++: lambda mangling alias issues [PR107897]

2023-03-08 Thread Jason Merrill via Gcc-patches

On 3/8/23 11:15, Jason Merrill wrote:

On 3/8/23 10:53, Jan Hubicka wrote:
Tested x86_64-pc-linux-gnu.  Does this look good, or do we want to 
factor the

flag clearing into a symtab_node counterpart to cgraph_node::reset?

-- 8< --

In 107897, by the time we are looking at the mangling clash, the
alias has already been removed from the symbol table by 
analyze_functions,

so we can't look at n->cpp_implicit_alias.  So just assume that it's an
alias if it's internal.

In 108887 the problem is that removing the mangling alias from the 
symbol

table confuses analyze_functions, because it ended up as first_analyzed
somehow, so it becomes a dangling pointer.  Fixed by clearing various 
flags

to neutralize the alias.

PR c++/107897
PR c++/108887

gcc/cp/ChangeLog:

* decl2.cc (record_mangling): Improve symbol table handling.

gcc/testsuite/ChangeLog:

* g++.dg/cpp2a/concepts-lambda3.C: Use -flto if supported.
* g++.dg/cpp0x/lambda/lambda-mangle7.C: New test.
---
  gcc/cp/decl2.cc   | 25 +--
  .../g++.dg/cpp0x/lambda/lambda-mangle7.C  | 70 +++
  gcc/testsuite/g++.dg/cpp2a/concepts-lambda3.C |  1 +
  3 files changed, 91 insertions(+), 5 deletions(-)
  create mode 100644 gcc/testsuite/g++.dg/cpp0x/lambda/lambda-mangle7.C

diff --git a/gcc/cp/decl2.cc b/gcc/cp/decl2.cc
index 387e24542cd..e6e58b08de4 100644
--- a/gcc/cp/decl2.cc
+++ b/gcc/cp/decl2.cc
@@ -4742,15 +4742,30 @@ record_mangling (tree decl, bool need_warning)
  = mangled_decls->find_slot_with_hash (id, IDENTIFIER_HASH_VALUE 
(id),

    INSERT);
-  /* If this is already an alias, remove the alias, because the real
+  /* If this is already an alias, cancel the alias, because the real
   decl takes precedence.  */
    if (*slot && DECL_ARTIFICIAL (*slot) && DECL_IGNORED_P (*slot))
-    if (symtab_node *n = symtab_node::get (*slot))
-  if (n->cpp_implicit_alias)
+    {
+  if (symtab_node *n = symtab_node::get (*slot))
  {
-  n->remove ();
-  *slot = NULL_TREE;
+  if (n->cpp_implicit_alias)
+    {
+  /* Actually removing the node isn't safe if other code is 
already

+ holding a pointer to it, so just neutralize it.  */
+  n->remove_from_same_comdat_group ();
+  n->analyzed = false;
+  n->definition = false;
+  n->alias = false;
+  n->cpp_implicit_alias = false;

We have n->reset () for that which is used in similar situation when
frontends overwrites extern inline function by its different offline
implementation.


The problem there is that reset() is a member of cgraph_node, not 
symtab_node, and I need something that works for variables as well.



reset doesn't call remove_from_same_comdat_group probably because it was
never used on anything in comdat group.  So I think it would make sense
to call n->reset() here and add remove_from_same_comdat_group into that.


How about moving it to symtab_node and using dyn_cast for the cgraph 
bits, like this:
From 1d869ceb04573727e59be6518903133c8654069a Mon Sep 17 00:00:00 2001
From: Jason Merrill 
Date: Mon, 6 Mar 2023 15:33:45 -0500
Subject: [PATCH] c++: lambda mangling alias issues [PR107897]
To: gcc-patches@gcc.gnu.org

In 107897, by the time we are looking at the mangling clash, the
alias has already been removed from the symbol table by analyze_functions,
so we can't look at n->cpp_implicit_alias.  So just assume that it's an
alias if it's internal.

In 108887 the problem is that removing the mangling alias from the symbol
table confuses analyze_functions, because it ended up as first_analyzed
somehow, so it becomes a dangling pointer.  Fixed by clearing various flags
to neutralize the alias.

	PR c++/107897
	PR c++/108887

gcc/ChangeLog:

	* cgraph.h: Move reset() from cgraph_node to symtab_node.
	* cgraphunit.cc (symtab_node::reset): Adjust.

gcc/cp/ChangeLog:

	* decl2.cc (record_mangling): Use symtab_node::reset.

gcc/testsuite/ChangeLog:

	* g++.dg/cpp2a/concepts-lambda3.C: Use -flto if supported.
	* g++.dg/cpp0x/lambda/lambda-mangle7.C: New test.
---
 gcc/cgraph.h  | 16 ++---
 gcc/cgraphunit.cc | 27 ---
 gcc/cp/decl2.cc   | 19 +++--
 .../g++.dg/cpp0x/lambda/lambda-mangle7.C  | 70 +++
 gcc/testsuite/g++.dg/cpp2a/concepts-lambda3.C |  1 +
 5 files changed, 109 insertions(+), 24 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/lambda/lambda-mangle7.C

diff --git a/gcc/cgraph.h b/gcc/cgraph.h
index b5fc739f1b0..fb938470be9 100644
--- a/gcc/cgraph.h
+++ b/gcc/cgraph.h
@@ -149,6 +149,14 @@ public:
  cgraph/varpool node creation routines.  */
   void register_symbol (void);
 
+  /* As an GCC extension we allow redefinition of the function.  The
+ semantics when both copies of bodies differ is not well defined.
+ We replace the old body with new body so in unit at a time mode
+ we always use 

Re: [PATCH RFC] c++: lambda mangling alias issues [PR107897]

2023-03-08 Thread Jason Merrill via Gcc-patches

On 3/8/23 10:53, Jan Hubicka wrote:

Tested x86_64-pc-linux-gnu.  Does this look good, or do we want to factor the
flag clearing into a symtab_node counterpart to cgraph_node::reset?

-- 8< --

In 107897, by the time we are looking at the mangling clash, the
alias has already been removed from the symbol table by analyze_functions,
so we can't look at n->cpp_implicit_alias.  So just assume that it's an
alias if it's internal.

In 108887 the problem is that removing the mangling alias from the symbol
table confuses analyze_functions, because it ended up as first_analyzed
somehow, so it becomes a dangling pointer.  Fixed by clearing various flags
to neutralize the alias.

PR c++/107897
PR c++/108887

gcc/cp/ChangeLog:

* decl2.cc (record_mangling): Improve symbol table handling.

gcc/testsuite/ChangeLog:

* g++.dg/cpp2a/concepts-lambda3.C: Use -flto if supported.
* g++.dg/cpp0x/lambda/lambda-mangle7.C: New test.
---
  gcc/cp/decl2.cc   | 25 +--
  .../g++.dg/cpp0x/lambda/lambda-mangle7.C  | 70 +++
  gcc/testsuite/g++.dg/cpp2a/concepts-lambda3.C |  1 +
  3 files changed, 91 insertions(+), 5 deletions(-)
  create mode 100644 gcc/testsuite/g++.dg/cpp0x/lambda/lambda-mangle7.C

diff --git a/gcc/cp/decl2.cc b/gcc/cp/decl2.cc
index 387e24542cd..e6e58b08de4 100644
--- a/gcc/cp/decl2.cc
+++ b/gcc/cp/decl2.cc
@@ -4742,15 +4742,30 @@ record_mangling (tree decl, bool need_warning)
  = mangled_decls->find_slot_with_hash (id, IDENTIFIER_HASH_VALUE (id),
  INSERT);
  
-  /* If this is already an alias, remove the alias, because the real

+  /* If this is already an alias, cancel the alias, because the real
   decl takes precedence.  */
if (*slot && DECL_ARTIFICIAL (*slot) && DECL_IGNORED_P (*slot))
-if (symtab_node *n = symtab_node::get (*slot))
-  if (n->cpp_implicit_alias)
+{
+  if (symtab_node *n = symtab_node::get (*slot))
{
- n->remove ();
- *slot = NULL_TREE;
+ if (n->cpp_implicit_alias)
+   {
+ /* Actually removing the node isn't safe if other code is already
+holding a pointer to it, so just neutralize it.  */
+ n->remove_from_same_comdat_group ();
+ n->analyzed = false;
+ n->definition = false;
+ n->alias = false;
+ n->cpp_implicit_alias = false;

We have n->reset () for that which is used in similar situation when
frontends overwrites extern inline function by its different offline
implementation.


The problem there is that reset() is a member of cgraph_node, not 
symtab_node, and I need something that works for variables as well.



reset doesn't call remove_from_same_comdat_group probably because it was
never used on anything in comdat group.  So I think it would make sense
to call n->reset() here and add remove_from_same_comdat_group into that.

OK with that change.
Honza

+   }
}
+  else
+   /* analyze_functions might have already removed the alias from the
+  symbol table if it's internal.  */
+   gcc_checking_assert (!TREE_PUBLIC (*slot));
+
+  *slot = NULL_TREE;
+}
  
if (!*slot)

  *slot = decl;
diff --git a/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-mangle7.C 
b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-mangle7.C
new file mode 100644
index 000..c7946a2be08
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-mangle7.C
@@ -0,0 +1,70 @@
+// PR c++/108887
+// { dg-do compile { target c++11 } }
+
+template  struct integral_constant {
+  static constexpr int value = __v;
+};
+using false_type = integral_constant;
+template  struct __result_of_impl;
+template 
+struct __result_of_impl {
+  typedef decltype(0) type;
+};
+template 
+struct __invoke_result
+: __result_of_impl {};
+template 
+void __invoke_impl(_Fn __f, _Args... __args) {
+  __f(__args...);
+}
+template 
+void __invoke_r(_Callable __fn, _Args... __args) {
+  using __result = __invoke_result<_Args...>;
+  using __type = typename __result::type;
+  __invoke_impl<__type>(__fn, __args...);
+}
+struct QString {
+  QString(const char *);
+};
+template  class function;
+template  struct _Base_manager {
+  static _Functor _M_get_pointer(int) { __builtin_abort (); }
+};
+template  class _Function_handler;
+template 
+struct _Function_handler<_Res(_ArgTypes...), _Functor> {
+  using _Base = _Base_manager<_Functor>;
+  static _Res _M_invoke(const int &__functor, _ArgTypes &&...__args) {
+auto __trans_tmp_1 = _Base::_M_get_pointer(__functor);
+__invoke_r<_Res>(__trans_tmp_1, __args...);
+  }
+};
+template 
+struct function<_Res(_ArgTypes...)> {
+  template 
+  using _Handler = _Function_handler<_Res(_ArgTypes...), _Functor>;
+  template  function(_Functor) {
+using _My_handler = _Handler<_Functor>;
+_M_invoker = _My_handler::_M_invoke;
+  }
+  using _Invoker_type = _Res (*)(const int &, 

Re: [PATCH RFC] c++: lambda mangling alias issues [PR107897]

2023-03-08 Thread Jan Hubicka via Gcc-patches
> Tested x86_64-pc-linux-gnu.  Does this look good, or do we want to factor the
> flag clearing into a symtab_node counterpart to cgraph_node::reset?
> 
> -- 8< --
> 
> In 107897, by the time we are looking at the mangling clash, the
> alias has already been removed from the symbol table by analyze_functions,
> so we can't look at n->cpp_implicit_alias.  So just assume that it's an
> alias if it's internal.
> 
> In 108887 the problem is that removing the mangling alias from the symbol
> table confuses analyze_functions, because it ended up as first_analyzed
> somehow, so it becomes a dangling pointer.  Fixed by clearing various flags
> to neutralize the alias.
> 
>   PR c++/107897
>   PR c++/108887
> 
> gcc/cp/ChangeLog:
> 
>   * decl2.cc (record_mangling): Improve symbol table handling.
> 
> gcc/testsuite/ChangeLog:
> 
>   * g++.dg/cpp2a/concepts-lambda3.C: Use -flto if supported.
>   * g++.dg/cpp0x/lambda/lambda-mangle7.C: New test.
> ---
>  gcc/cp/decl2.cc   | 25 +--
>  .../g++.dg/cpp0x/lambda/lambda-mangle7.C  | 70 +++
>  gcc/testsuite/g++.dg/cpp2a/concepts-lambda3.C |  1 +
>  3 files changed, 91 insertions(+), 5 deletions(-)
>  create mode 100644 gcc/testsuite/g++.dg/cpp0x/lambda/lambda-mangle7.C
> 
> diff --git a/gcc/cp/decl2.cc b/gcc/cp/decl2.cc
> index 387e24542cd..e6e58b08de4 100644
> --- a/gcc/cp/decl2.cc
> +++ b/gcc/cp/decl2.cc
> @@ -4742,15 +4742,30 @@ record_mangling (tree decl, bool need_warning)
>  = mangled_decls->find_slot_with_hash (id, IDENTIFIER_HASH_VALUE (id),
> INSERT);
>  
> -  /* If this is already an alias, remove the alias, because the real
> +  /* If this is already an alias, cancel the alias, because the real
>   decl takes precedence.  */
>if (*slot && DECL_ARTIFICIAL (*slot) && DECL_IGNORED_P (*slot))
> -if (symtab_node *n = symtab_node::get (*slot))
> -  if (n->cpp_implicit_alias)
> +{
> +  if (symtab_node *n = symtab_node::get (*slot))
>   {
> -   n->remove ();
> -   *slot = NULL_TREE;
> +   if (n->cpp_implicit_alias)
> + {
> +   /* Actually removing the node isn't safe if other code is already
> +  holding a pointer to it, so just neutralize it.  */
> +   n->remove_from_same_comdat_group ();
> +   n->analyzed = false;
> +   n->definition = false;
> +   n->alias = false;
> +   n->cpp_implicit_alias = false;
We have n->reset () for that which is used in similar situation when
frontends overwrites extern inline function by its different offline
implementation.

reset doesn't call remove_from_same_comdat_group probably because it was
never used on anything in comdat group.  So I think it would make sense
to call n->reset() here and add remove_from_same_comdat_group into that.

OK with that change.
Honza
> + }
>   }
> +  else
> + /* analyze_functions might have already removed the alias from the
> +symbol table if it's internal.  */
> + gcc_checking_assert (!TREE_PUBLIC (*slot));
> +
> +  *slot = NULL_TREE;
> +}
>  
>if (!*slot)
>  *slot = decl;
> diff --git a/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-mangle7.C 
> b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-mangle7.C
> new file mode 100644
> index 000..c7946a2be08
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-mangle7.C
> @@ -0,0 +1,70 @@
> +// PR c++/108887
> +// { dg-do compile { target c++11 } }
> +
> +template  struct integral_constant {
> +  static constexpr int value = __v;
> +};
> +using false_type = integral_constant;
> +template  struct __result_of_impl;
> +template 
> +struct __result_of_impl {
> +  typedef decltype(0) type;
> +};
> +template 
> +struct __invoke_result
> +: __result_of_impl 
> {};
> +template 
> +void __invoke_impl(_Fn __f, _Args... __args) {
> +  __f(__args...);
> +}
> +template 
> +void __invoke_r(_Callable __fn, _Args... __args) {
> +  using __result = __invoke_result<_Args...>;
> +  using __type = typename __result::type;
> +  __invoke_impl<__type>(__fn, __args...);
> +}
> +struct QString {
> +  QString(const char *);
> +};
> +template  class function;
> +template  struct _Base_manager {
> +  static _Functor _M_get_pointer(int) { __builtin_abort (); }
> +};
> +template  class _Function_handler;
> +template 
> +struct _Function_handler<_Res(_ArgTypes...), _Functor> {
> +  using _Base = _Base_manager<_Functor>;
> +  static _Res _M_invoke(const int &__functor, _ArgTypes &&...__args) {
> +auto __trans_tmp_1 = _Base::_M_get_pointer(__functor);
> +__invoke_r<_Res>(__trans_tmp_1, __args...);
> +  }
> +};
> +template 
> +struct function<_Res(_ArgTypes...)> {
> +  template 
> +  using _Handler = _Function_handler<_Res(_ArgTypes...), _Functor>;
> +  template  function(_Functor) {
> +using _My_handler = _Handler<_Functor>;
> +_M_invoker = _My_handler::_M_invoke;
> +  }
> +  using _Invoker_type = 

[PATCH RFC] c++: lambda mangling alias issues [PR107897]

2023-03-07 Thread Jason Merrill via Gcc-patches
Tested x86_64-pc-linux-gnu.  Does this look good, or do we want to factor the
flag clearing into a symtab_node counterpart to cgraph_node::reset?

-- 8< --

In 107897, by the time we are looking at the mangling clash, the
alias has already been removed from the symbol table by analyze_functions,
so we can't look at n->cpp_implicit_alias.  So just assume that it's an
alias if it's internal.

In 108887 the problem is that removing the mangling alias from the symbol
table confuses analyze_functions, because it ended up as first_analyzed
somehow, so it becomes a dangling pointer.  Fixed by clearing various flags
to neutralize the alias.

PR c++/107897
PR c++/108887

gcc/cp/ChangeLog:

* decl2.cc (record_mangling): Improve symbol table handling.

gcc/testsuite/ChangeLog:

* g++.dg/cpp2a/concepts-lambda3.C: Use -flto if supported.
* g++.dg/cpp0x/lambda/lambda-mangle7.C: New test.
---
 gcc/cp/decl2.cc   | 25 +--
 .../g++.dg/cpp0x/lambda/lambda-mangle7.C  | 70 +++
 gcc/testsuite/g++.dg/cpp2a/concepts-lambda3.C |  1 +
 3 files changed, 91 insertions(+), 5 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/lambda/lambda-mangle7.C

diff --git a/gcc/cp/decl2.cc b/gcc/cp/decl2.cc
index 387e24542cd..e6e58b08de4 100644
--- a/gcc/cp/decl2.cc
+++ b/gcc/cp/decl2.cc
@@ -4742,15 +4742,30 @@ record_mangling (tree decl, bool need_warning)
 = mangled_decls->find_slot_with_hash (id, IDENTIFIER_HASH_VALUE (id),
  INSERT);
 
-  /* If this is already an alias, remove the alias, because the real
+  /* If this is already an alias, cancel the alias, because the real
  decl takes precedence.  */
   if (*slot && DECL_ARTIFICIAL (*slot) && DECL_IGNORED_P (*slot))
-if (symtab_node *n = symtab_node::get (*slot))
-  if (n->cpp_implicit_alias)
+{
+  if (symtab_node *n = symtab_node::get (*slot))
{
- n->remove ();
- *slot = NULL_TREE;
+ if (n->cpp_implicit_alias)
+   {
+ /* Actually removing the node isn't safe if other code is already
+holding a pointer to it, so just neutralize it.  */
+ n->remove_from_same_comdat_group ();
+ n->analyzed = false;
+ n->definition = false;
+ n->alias = false;
+ n->cpp_implicit_alias = false;
+   }
}
+  else
+   /* analyze_functions might have already removed the alias from the
+  symbol table if it's internal.  */
+   gcc_checking_assert (!TREE_PUBLIC (*slot));
+
+  *slot = NULL_TREE;
+}
 
   if (!*slot)
 *slot = decl;
diff --git a/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-mangle7.C 
b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-mangle7.C
new file mode 100644
index 000..c7946a2be08
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-mangle7.C
@@ -0,0 +1,70 @@
+// PR c++/108887
+// { dg-do compile { target c++11 } }
+
+template  struct integral_constant {
+  static constexpr int value = __v;
+};
+using false_type = integral_constant;
+template  struct __result_of_impl;
+template 
+struct __result_of_impl {
+  typedef decltype(0) type;
+};
+template 
+struct __invoke_result
+: __result_of_impl {};
+template 
+void __invoke_impl(_Fn __f, _Args... __args) {
+  __f(__args...);
+}
+template 
+void __invoke_r(_Callable __fn, _Args... __args) {
+  using __result = __invoke_result<_Args...>;
+  using __type = typename __result::type;
+  __invoke_impl<__type>(__fn, __args...);
+}
+struct QString {
+  QString(const char *);
+};
+template  class function;
+template  struct _Base_manager {
+  static _Functor _M_get_pointer(int) { __builtin_abort (); }
+};
+template  class _Function_handler;
+template 
+struct _Function_handler<_Res(_ArgTypes...), _Functor> {
+  using _Base = _Base_manager<_Functor>;
+  static _Res _M_invoke(const int &__functor, _ArgTypes &&...__args) {
+auto __trans_tmp_1 = _Base::_M_get_pointer(__functor);
+__invoke_r<_Res>(__trans_tmp_1, __args...);
+  }
+};
+template 
+struct function<_Res(_ArgTypes...)> {
+  template 
+  using _Handler = _Function_handler<_Res(_ArgTypes...), _Functor>;
+  template  function(_Functor) {
+using _My_handler = _Handler<_Functor>;
+_M_invoker = _My_handler::_M_invoke;
+  }
+  using _Invoker_type = _Res (*)(const int &, _ArgTypes &&...);
+  _Invoker_type _M_invoker;
+};
+struct QRegularExpression {
+  QRegularExpression(QString);
+};
+struct AbstractAccount {
+  void get(function,
+   function);
+};
+struct AbstractTimelineModel {
+  AbstractAccount m_account;
+};
+struct LinkPaginationTimelineModel : AbstractTimelineModel {
+  void fillTimeline();
+};
+void LinkPaginationTimelineModel::fillTimeline() {
+  [] {};
+  m_account.get([](AbstractAccount *) { static QRegularExpression re(""); },
+[](AbstractAccount *) {});
+}
diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-lambda3.C