Bug 97399 - [10 Regression] g++ 9.3 cannot compile SFINAE code with separated declaration and definition, g++ 7.3 compiles
Summary: [10 Regression] g++ 9.3 cannot compile SFINAE code with separated declaration...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 9.3.0
: P2 normal
Target Milestone: 11.0
Assignee: Patrick Palka
URL:
Keywords: ice-on-valid-code, rejects-valid
Depends on:
Blocks: 101680
  Show dependency treegraph
 
Reported: 2020-10-13 09:58 UTC by Renlin Li
Modified: 2023-07-07 09:08 UTC (History)
4 users (show)

See Also:
Host:
Target:
Build:
Known to work: 11.1.0, 7.5.0
Known to fail: 10.2.1, 10.5.0
Last reconfirmed: 2020-10-13 00:00:00


Attachments
test case 1 (321 bytes, text/x-csrc)
2020-10-13 09:58 UTC, Renlin Li
Details
test case 2 (317 bytes, text/x-csrc)
2020-10-13 09:58 UTC, Renlin Li
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Renlin Li 2020-10-13 09:58:14 UTC
Created attachment 49362 [details]
test case 1

For gcc_1.c++
gcc 7.3 compiles for this code
clang 7 compiles for this code
gcc 9.3 fails to compile with following message

Not sure if this is gcc's issue or clang.

```
<source>:29:16: error: no declaration matches 'constexpr enable_if_t<((tmp*)this)->is_integral<E>(), bool> tmp::func(E, E) const'

   29 | constexpr auto tmp::func(E f_lhs, E f_rhs)

      |                ^~~

<source>:18:27: note: candidate is: 'template<class E> static constexpr enable_if_t<((tmp*)this)->is_integral<E>(), bool> tmp::func(E, E)'

   18 |     static constexpr auto func(E f_lhs, E f_rhs)

      |                           ^~~~

<source>:12:8: note: 'struct tmp' defined here

   12 | struct tmp
```

Meanwhile for gcc_2.c++
gcc compiles without any issue.
clang gives the following error message
```
<source>:27:28: error: template parameter redefines default argument

template <class E, class = enable_if_t<tmp::is_integral<E>(), bool>>

                           ^

<source>:17:32: note: previous default template argument defined here

    template <class E, class = enable_if_t<tmp::is_integral<E>(), bool>>
```
It seems this is not an new issue, and might be duplicated.
Comment 1 Renlin Li 2020-10-13 09:58:39 UTC
Created attachment 49363 [details]
test case 2
Comment 2 Richard Biener 2020-10-13 11:02:09 UTC
GCC 10 ICEs:

> g++-10 t.C -S -std=c++11
t.C: In substitution of 'template<class E> static constexpr enable_if_t<((tmp*)this)->is_integral<E>(), bool> tmp::func(E, E) [with E = int]':
t.C:40:30:   required from here
t.C:19:43: internal compiler error: in tsubst_copy, at cp/pt.c:16383
   19 |         -> enable_if_t<tmp::is_integral<E>(), bool>;
      |                        ~~~~~~~~~~~~~~~~~~~^~
Please submit a full bug report,
with preprocessed source if appropriate.
See <https://bugs.opensuse.org/> for instructions.
Comment 3 Martin Liška 2020-10-13 14:10:07 UTC
Reduced test-case:

$ cat pr97399.ii
template <int __v> struct integral_constant {
  static constexpr int value = __v;
};
template <bool, class> using enable_if_t = int;
struct tmp {
  template <class> static constexpr auto is_integral() -> bool;
  template <class E>
  static auto func(E, E) -> enable_if_t<tmp::is_integral<E>(), bool>;
};
template <class> constexpr auto tmp::is_integral() -> bool {
  return integral_constant<false>::value;
}
int main() { tmp::func(1, 0); }

Started with r9-5972-g10839133ce6c196c.
Comment 4 Patrick Palka 2021-01-20 20:00:46 UTC
The alias template in the reduced testcase is a red herring, the ICE can be reproduced without it:

template <bool> struct enable_if_t {};
struct tmp {
  template <class> static constexpr bool is_integral();
  template <class E>
  static auto func(E, E) -> enable_if_t<tmp::is_integral<E>()>;
};
template <class> constexpr bool tmp::is_integral() { return true; }
int main() { tmp::func(1, 0); }


Looks like the problematic hunk from r9-5972 is

--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -2096,7 +2096,8 @@ finish_qualified_id_expr (tree qualifying_class,
     {
       /* See if any of the functions are non-static members.  */
       /* If so, the expression may be relative to 'this'.  */
-      if (!shared_member_p (expr)
+      if ((type_dependent_expression_p (expr)
+          || !shared_member_p (expr))
          && current_class_ptr
          && DERIVED_FROM_P (qualifying_class,
                             current_nonlambda_class_type ()))
Comment 5 GCC Commits 2021-01-23 05:25:06 UTC
The master branch has been updated by Patrick Palka <ppalka@gcc.gnu.org>:

https://gcc.gnu.org/g:a8cef3cba6945730c69e15dcdad726e74b50fe58

commit r11-6878-ga8cef3cba6945730c69e15dcdad726e74b50fe58
Author: Patrick Palka <ppalka@redhat.com>
Date:   Sat Jan 23 00:24:17 2021 -0500

    c++: 'this' injection and static member functions [PR97399]
    
    In the testcase pr97399.C below, finish_qualified_id_expr at parse time
    adds an implicit 'this->' to the expression tmp::integral<T> (because
    it's type-dependent, and also current_class_ptr is set at this point)
    within the trailing return type.  Later when substituting into this
    trailing return type we crash because we can't resolve the 'this', since
    tsubst_function_type does inject_this_parm only for non-static member
    functions, which tmp::func is not.
    
    This patch fixes this issue by removing the type-dependence check
    in finish_qualified_id_expr added by r9-5972, and instead relaxes
    shared_member_p to handle dependent USING_DECLs:
    
    > I think I was wrong in my assertion around Alex's patch that
    > shared_member_p should abort on a dependent USING_DECL; it now seems
    > appropriate for it to return false if we don't know, we just need to
    > adjust the comment to say that.
    
    And when parsing a friend function declaration, we shouldn't be setting
    current_class_ptr at all, so this patch additionally suppresses
    inject_this_parm in this case.
    
    Finally, the self-contained change to cp_parser_init_declarator is so
    that we properly communicate static-ness to cp_parser_direct_declarator
    when parsing a member function template.  This lets us reject the
    explicit use of 'this' in the testcase this2.C below.
    
    gcc/cp/ChangeLog:
    
            PR c++/97399
            * cp-tree.h (shared_member_p): Adjust declaration.
            * parser.c (cp_parser_init_declarator): If the storage class
            specifier is sc_static, pass true for static_p to
            cp_parser_declarator.
            (cp_parser_direct_declarator): Don't do inject_this_parm when
            the declarator is a friend.
            * search.c (shared_member_p): Change return type to bool and
            adjust function body accordingly.  Return false for a dependent
            USING_DECL instead of aborting.
            * semantics.c (finish_qualified_id_expr): Rely on shared_member_p
            even when type-dependent.
    
    gcc/testsuite/ChangeLog:
    
            PR c++/88548
            PR c++/97399
            * g++.dg/cpp0x/this2.C: New test.
            * g++.dg/template/pr97399.C: New test.
Comment 6 Patrick Palka 2021-01-23 05:25:44 UTC
Fixed for GCC 11 so far.
Comment 7 GCC Commits 2021-04-10 20:33:23 UTC
The master branch has been updated by Jason Merrill <jason@gcc.gnu.org>:

https://gcc.gnu.org/g:82198676c80764ca7cf05f891afaee83b9179dd1

commit r11-8118-g82198676c80764ca7cf05f891afaee83b9179dd1
Author: Jason Merrill <jason@redhat.com>
Date:   Sat Apr 10 10:55:58 2021 -0400

    c++: ICE with invalid use of 'this' with static memfn [PR98800]
    
    Here instantiation of the fake 'this' parameter we used when parsing the
    trailing return type of func() was failing because there is no actual 'this'
    parameter in the instantiation.  For PR97399 I told Patrick to do the 'this'
    injection even for statics, but now I think I was wrong; the out-of-class
    definition case I was concerned about does not break with this patch.  And
    we don't set current_class_ptr in the body of a static member function.
    
    And the OMP code should continue to parse 'this' and complain about it
    rather than give a syntax error.
    
    gcc/cp/ChangeLog:
    
            PR c++/98800
            PR c++/97399
            * parser.c (cp_parser_direct_declarator): Don't
            inject_this_parameter if static_p.
            (cp_parser_omp_var_list_no_open): Parse 'this' even if
            current_class_ptr isn't set for a better diagnostic.
    
    gcc/testsuite/ChangeLog:
    
            PR c++/98800
            * g++.dg/gomp/this-1.C: Adjust diagnostic.
            * g++.dg/cpp0x/constexpr-this1.C: New test.
Comment 8 Richard Biener 2021-06-01 08:18:43 UTC
GCC 9.4 is being released, retargeting bugs to GCC 9.5.
Comment 9 Richard Biener 2022-05-27 09:43:40 UTC
GCC 9 branch is being closed
Comment 10 Jakub Jelinek 2022-06-28 10:42:11 UTC
GCC 10.4 is being released, retargeting bugs to GCC 10.5.
Comment 11 Richard Biener 2023-07-07 09:08:17 UTC
Fixed for GCC 11.