[Bug c++/97358] New: [8/9/10/11 Regression] ICE while building firefox since r8-2720

[Bug c++/97358] New: [8/9/10/11 Regression] ICE while building firefox since r8-2720

[jakub at gcc dot gnu.org]

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

            Bug ID: 97358
           Summary: [8/9/10/11 Regression] ICE while building firefox
                    since r8-2720
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: jakub at gcc dot gnu.org
  Target Milestone: ---

The following testcase reduced from firefox ICEs with -std=c++17 starting with
r8-2720-gf44a8dd56f5bfbd0596c39693e268ef880c06221 (before it has been
rejected).
clang++ accepts it, though unsure if the reduction didn't make it invalid (or
whether the original 13MB+ :( testcase is valid).

struct A;
struct B { A foo (); };
enum C { E };
struct A { template <typename... T> void bar (T... x) { auto v {[x...] { x;
}...}; } };
template <typename, C> using F = B;
struct G { F<int, E> baz (); };
enum H { J };
struct D { using K = G; };
template <H> struct I : D { void qux (); };
template <H h> void I<h>::qux () { K a; a.baz ().foo ().bar (); }
template class I<J>;

[Bug c++/97358] [8/9/10/11 Regression] ICE while building firefox since r8-2720

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |8.5
   Last reconfirmed|                            |2020-10-09
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |NEW
                 CC|                            |jason at gcc dot gnu.org,
                   |                            |mpolacek at gcc dot gnu.org,
                   |                            |ppalka at gcc dot gnu.org

[Bug c++/97358] [8/9/10/11 Regression] ICE while building firefox since r8-2720

[jakub at gcc dot gnu.org]

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

--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Slightly modified testcase where bar is surely instantiated and clang++ still
accepts it.  It actually ICEs in all of -std=c++{11,14,17,20}.
struct A;
struct B { A foo (); };
enum C { E };
struct A { template <typename... T> void bar (T... x) { int v {[x...] { x;
}...}; } };
template <typename, C> using F = B;
struct G { F<int, E> baz (); };
enum H { J };
struct D { using K = G; };
template <H> struct I : D { void qux (); };
template <H h> void I<h>::qux () { K a; a.baz ().foo ().bar (); }
void
test ()
{
  I<J> i;
  i.qux ();
}

[Bug c++/97358] [8/9/10/11 Regression] ICE while building firefox since r8-2720

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |ice-on-valid-code
           Priority|P3                          |P2

[Bug c++/97358] [8/9/10/11 Regression] ICE while building firefox since r8-2720

[jakub at gcc dot gnu.org]

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

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
template <typename... T> void foo (T... x)
{
}

template <typename... T> void bar (T... x)
{
  foo ([x...] { static_cast<void> (x); }...);
}

void
test ()
{
  bar ();
  bar (1);
  bar (2.0, 3LL, 4);
}

ICEs too.

[Bug c++/97358] [8/9/10/11 Regression] ICE while building firefox since r8-2720

[jakub at gcc dot gnu.org]

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

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
The original Firefox problematic template is:
  template <typename... SpecialValueMappers>
  ErrorResult ExtractErrorResult(SpecialValueMappers... aSpecialValueMappers) {
    return mVariant.match(
        [](ErrorResult& aException) { return std::move(aException); },
        [aSpecialValueMappers...](const SpecialConstant<S>& aSpecialValue) {
          return ErrorResult{aSpecialValueMappers(aSpecialValue)};
        }...);
  }
The ICE is because a local specialization for the capture proxy has not been
registered.

[Bug c++/97358] [8/9/10/11 Regression] ICE while building firefox since r8-2720

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |sumbera at volny dot cz

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
*** Bug 97423 has been marked as a duplicate of this bug. ***

[Bug c++/97358] [8/9/10/11 Regression] ICE while building firefox since r8-2720

[sumbera at volny dot cz]

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

--- Comment #5 from Petr Sumbera <sumbera at volny dot cz> ---
Is there any workaround for this issue?

[Bug c++/97358] [8/9/10/11 Regression] ICE while building firefox since r8-2720

[jakub at gcc dot gnu.org]

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

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to Petr Sumbera from comment #5)
> Is there any workaround for this issue?

I guess replacing that
[aSpecialValueMappers...](const SpecialConstant<S>& aSpecialValue) {
with
[=](const SpecialConstant<S>& aSpecialValue) {

[Bug c++/97358] [8/9/10/11 Regression] ICE while building firefox since r8-2720

[ppalka at gcc dot gnu.org]

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

--- Comment #7 from Patrick Palka <ppalka at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #6)
> (In reply to Petr Sumbera from comment #5)
> > Is there any workaround for this issue?
> 
> I guess replacing that
> [aSpecialValueMappers...](const SpecialConstant<S>& aSpecialValue) {
> with
> [=](const SpecialConstant<S>& aSpecialValue) {

And just removing the ellipsis from the lambda capture in

  [aSpecialValueMappers...](const SpecialConstant<S>& aSpecialValue) {

should be a valid workaround too, I think.


In comment #2, the lambda capture can basically be written in three ways:

1. foo([=] { return x; }, ...);
2. foo([x...] { return x; }, ...);
3. foo([...y{x}] { return y; }, ...);

GCC accepts 1 and ICEs on 2 & 3.
Clang accepts 1 & 2 and rejects 3 with "unexpanded parameter pack 'y'".

[Bug c++/97358] [8/9/10/11 Regression] ICE while building firefox since r8-2720

[ppalka at gcc dot gnu.org]

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

--- Comment #8 from Patrick Palka <ppalka at gcc dot gnu.org> ---
Also, I wonder what the final value of copy_counter should be in the below
testcase (assuming it's valid):

static int copy_counter;

struct S {
  S() = default;
  S(const S&) { ++copy_counter; }
};

template <typename... Ts>
void foo(Ts...) { }

template <typename... Ts>
void bar(Ts... x) {
  foo([x...] { return x; }...);
}

int main() {
  bar(S{}, S{}, S{});
}

According to Clang it's 9, FWIW.

[Bug c++/97358] [8/9/10/11 Regression] ICE while building firefox since r8-2720

[jakub at gcc dot gnu.org]

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

--- Comment #9 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
If what clang++ does with the copy ctors is right, then what firefox does is
inefficient and
[aSpecialValueMappers](const SpecialConstant<S>& aSpecialValue) {
          return ErrorResult{aSpecialValueMappers(aSpecialValue)};
        }...
would be more efficient.
That doesn't mean we shouldn't fix this PR.

[Bug c++/97358] [8/9/10/11 Regression] ICE while building firefox since r8-2720

[jason at gcc dot gnu.org]

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

--- Comment #10 from Jason Merrill <jason at gcc dot gnu.org> ---
This doesn't look valid to me.  In

  [x...] { x; }...

we capture the entire pack, but then try to use only a single element.  This
should be rejected because the use of x in the lambda body is not expanded, as
with the clang error for option 3 in comment #7.  Probably what they mean is

  [x] { x; }...

or, as you say,

  [=] { x; }...

[Bug c++/97358] [8/9/10/11 Regression] ICE while building firefox since r8-2720

[jason at gcc dot gnu.org]

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

Jason Merrill <jason at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |jason at gcc dot gnu.org
             Status|NEW                         |ASSIGNED

[Bug c++/97358] [8/9/10/11 Regression] ICE while building firefox since r8-2720

[jakub at gcc dot gnu.org]

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

--- Comment #11 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to Jason Merrill from comment #10)
> This doesn't look valid to me.  In
> 
>   [x...] { x; }...

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97358#c3 is what they actually
have (not sure if it changes anything on the validness) and I have full
preprocessed source, so can supply further details that would be needed.

[Bug c++/97358] [8/9/10/11 Regression] ICE while building firefox since r8-2720

[cvs-commit at gcc dot gnu.org]

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

--- Comment #12 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jason Merrill <jason@gcc.gnu.org>:

https://gcc.gnu.org/g:270c5a982ccb4ef83bd9ad37d39cf47461acb55a

commit r11-3893-g270c5a982ccb4ef83bd9ad37d39cf47461acb55a
Author: Jason Merrill <jason@redhat.com>
Date:   Wed Oct 14 14:55:04 2020 -0400

    c++: Diagnose bogus variadic lambda. [PR97358]

    If the lambda has a capture pack, it cannot be used unexpanded within the
    body of the lambda.  If you want to expand the pack across multiple
lambdas,
    don't capture the whole pack.

    gcc/cp/ChangeLog:

            PR c++/97358
            * pt.c (check_for_bare_parameter_packs): Diagnose use of
            capture pack.

    gcc/testsuite/ChangeLog:

            PR c++/97358
            * g++.dg/cpp0x/lambda/lambda-variadic11.C: New test.

[Bug c++/97358] [8/9/10 Regression] ICE while building firefox since r8-2720

[jason at gcc dot gnu.org]

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

Jason Merrill <jason at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[8/9/10/11 Regression] ICE  |[8/9/10 Regression] ICE
                   |while building firefox      |while building firefox
                   |since r8-2720               |since r8-2720

--- Comment #13 from Jason Merrill <jason at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #11)
> (In reply to Jason Merrill from comment #10)
> > This doesn't look valid to me.  In
> > 
> >   [x...] { x; }...
> 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97358#c3 is what they actually
> have (not sure if it changes anything on the validness) and I have full
> preprocessed source, so can supply further details that would be needed.

That doesn't change anything; they should remove the ... from the capture to
make the testcase valid.  Please check that the patch I just checked in
replaces the ICE with an error, thanks.

[Bug c++/97358] [8/9/10 Regression] ICE while building firefox since r8-2720

[jakub at gcc dot gnu.org]

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

--- Comment #14 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to Jason Merrill from comment #13)
> (In reply to Jakub Jelinek from comment #11)
> > (In reply to Jason Merrill from comment #10)
> > > This doesn't look valid to me.  In
> > > 
> > >   [x...] { x; }...
> > 
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97358#c3 is what they actually
> > have (not sure if it changes anything on the validness) and I have full
> > preprocessed source, so can supply further details that would be needed.
> 
> That doesn't change anything; they should remove the ... from the capture to
> make the testcase valid.  Please check that the patch I just checked in
> replaces the ICE with an error, thanks.

Ok, confirmed (with a backport to 10 because that in a cross to powerpc64le is
where I was testing it before), the unmodified original *.ii now errors out:
cchFugMG.ii: In lambda function:
cchFugMG.ii:73385:65: error: parameter packs not expanded with ‘...’:
73385 |           return ErrorResult{aSpecialValueMappers(aSpecialValue)};
      |                                                                 ^
cchFugMG.ii:73385:65: note:         ‘aSpecialValueMappers’
and compiles fine with the
   ErrorResult ExtractErrorResult(SpecialValueMappers... aSpecialValueMappers)
{
     return mVariant.match(
         [](ErrorResult& aException) { return std::move(aException); },
-        [aSpecialValueMappers...](const SpecialConstant<S>& aSpecialValue) {
+        [aSpecialValueMappers](const SpecialConstant<S>& aSpecialValue) {
           return ErrorResult{aSpecialValueMappers(aSpecialValue)};
         }...);
   }
change (in that case both with unmodified as well as modified gcc).

[Bug c++/97358] [8/9/10 Regression] ICE while building firefox since r8-2720

[dan at danny dot cz]

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

--- Comment #15 from Dan Horák <dan at danny dot cz> ---
Petr, are you going to open a Firefox bug to fix their code or shall I do it?

[Bug c++/97358] [8/9/10 Regression] ICE while building firefox since r8-2720

[jakub at gcc dot gnu.org]

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

--- Comment #16 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Perhaps also mention it in https://bugzilla.mozilla.org/show_bug.cgi?id=1665347

[Bug c++/97358] [8/9/10 Regression] ICE while building firefox since r8-2720

[sumbera at volny dot cz]

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

--- Comment #17 from Petr Sumbera <sumbera at volny dot cz> ---
(In reply to Dan Horák from comment #15)
> Petr, are you going to open a Firefox bug to fix their code or shall I do it?

Please file it. I haven't had time to read whole thread and I don't know what
shall be the right fix. For now I used just:

--- a/dom/indexedDB/IDBResult.h Thu Oct 15 08:38:16 2020 +0200
+++ b/dom/indexedDB/IDBResult.h Thu Oct 15 08:38:38 2020 +0200
@@ -101,7 +101,7 @@

   template <typename... SpecialValueMappers>
   ErrorResult ExtractErrorResult(SpecialValueMappers... aSpecialValueMappers)
{
-#if (defined(__clang__) || (defined(__GNUC__) && __GNUC__ >= 8)) && \
+#if (defined(__clang__) || (defined(__GNUC__) && __GNUC__ >= 11)) && \
     !defined(XGILL_PLUGIN)
     return mVariant.match(
         [](ErrorResult& aException) { return std::move(aException); },

But if you are busy I can do my best to report it.

[Bug c++/97358] [8/9/10 Regression] ICE while building firefox since r8-2720

[dan at danny dot cz]

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

--- Comment #18 from Dan Horák <dan at danny dot cz> ---
So there is https://bugzilla.mozilla.org/show_bug.cgi?id=1671345 now, after I
commented in the bug Jakub mentioned. I like such cooperation :-)

[Bug c++/97358] [8/9/10 Regression] ICE while building firefox since r8-2720

[richard-gccbugzilla at metafoo dot co.uk]

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

Richard Smith <richard-gccbugzilla at metafoo dot co.uk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |richard-gccbugzilla@metafoo
                   |                            |.co.uk

--- Comment #19 from Richard Smith <richard-gccbugzilla at metafoo dot co.uk> ---
(In reply to Jason Merrill from comment #10)
> This doesn't look valid to me.  In
> 
>   [x...] { x; }...
> 
> we capture the entire pack, but then try to use only a single element.  This
> should be rejected because the use of x in the lambda body is not expanded

The reference to 'x' is expanded within the scope of the parameter pack, though
(which is the entire function template, because 'x' here refers to the
parameter of the enclosing function template). So I think that case 2 is valid,
and expands to

foo(
  [x...] { return x.[0]; },
  [x...] { return x.[1]; },
  ...
  [x...] { return x.[N]; },
);

(That is, each lambda captures the entire pack and then uses only part of it.)
More generally, I think cases such as

foo([x...] { return f(x..., x); } ...);

... are valid, resulting in expansions such as

foo(
  [x...] { return f(x.[0], x.[1], ..., x.[N], x.[0]; },
  [x...] { return f(x.[0], x.[1], ..., x.[N], x.[1]; },
  ...
  [x...] { return f(x.[0], x.[1], ..., x.[N], x.[N]; },
);

I think Case 3 is not valid because the reference to the pack y is not expanded
*within the scope of that pack*, though it's not clear that the standard
actually clearly says that anywhere.

[Bug c++/97358] [9/10 Regression] ICE while building firefox since r8-2720

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|8.5                         |9.4

--- Comment #20 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
GCC 8 branch is being closed.

[Bug c++/97358] [9/10 Regression] ICE while building firefox since r8-2720

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|9.4                         |9.5

--- Comment #21 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCC 9.4 is being released, retargeting bugs to GCC 9.5.

[Bug c++/97358] [10 Regression] ICE while building firefox since r8-2720

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|9.5                         |10.4

--- Comment #22 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCC 9 branch is being closed

[Bug c++/97358] [10 Regression] ICE while building firefox since r8-2720

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|10.4                        |10.5

--- Comment #23 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
GCC 10.4 is being released, retargeting bugs to GCC 10.5.

[Bug c++/97358] [10 Regression] ICE while building firefox since r8-2720

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to fail|                            |10.5.0
   Target Milestone|10.5                        |11.0
      Known to work|                            |11.1.0
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #24 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed for GCC 11.