[Bug c++/88115] Incorrect result from alignof in templates, if also using __alignof__.

[Bug c++/88115] Incorrect result from alignof in templates, if also using __alignof__.

[redi at gcc dot gnu.org]

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

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           See Also|                            |https://gcc.gnu.org/bugzill
                   |                            |a/show_bug.cgi?id=97273
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2020-10-05

[Bug c++/88115] Incorrect result from alignof in templates, if also using __alignof__.

[cvs-commit at gcc dot gnu.org]

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

--- Comment #1 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Patrick Palka <ppalka@gcc.gnu.org>:

https://gcc.gnu.org/g:592fe221735bdaa375b1834dd49ce125d0b600d8

commit r11-3704-g592fe221735bdaa375b1834dd49ce125d0b600d8
Author: Patrick Palka <ppalka@redhat.com>
Date:   Wed Oct 7 10:49:00 2020 -0400

    c++: Distinguish alignof and __alignof__ in cp_tree_equal [PR97273]

    cp_tree_equal currently considers alignof the same as __alignof__, but
    these operators are semantically different ever since r8-7957.  In the
    testcase below, this causes the second static_assert to fail on targets
    where alignof(double) != __alignof__(double) because the specialization
    table (which uses cp_tree_equal as its equality predicate) conflates the
    two dependent specializations integral_constant<__alignof__(T)> and
    integral_constant<alignof(T)>.

    This patch makes cp_tree_equal distinguish between these two operators
    by inspecting the ALIGNOF_EXPR_STD_P flag.

    gcc/cp/ChangeLog:

            PR c++/88115
            PR libstdc++/97273
            * tree.c (cp_tree_equal) <case ALIGNOF_EXPR>: Return false if
            ALIGNOF_EXPR_STD_P differ.

    gcc/testsuite/ChangeLog:

            PR c++/88115
            PR libstdc++/97273
            * g++.dg/template/alignof3.C: New test.

[Bug c++/88115] Incorrect result from alignof in templates, if also using __alignof__.

[cvs-commit at gcc dot gnu.org]

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

--- Comment #2 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-10 branch has been updated by Patrick Palka
<ppalka@gcc.gnu.org>:

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

commit r10-8872-gaeb69dda51e93379fce10fb03ec57650fbf73f31
Author: Patrick Palka <ppalka@redhat.com>
Date:   Thu Oct 8 19:31:57 2020 -0400

    c++: Distinguish alignof and __alignof__ in cp_tree_equal [PR97273]

    cp_tree_equal currently considers alignof the same as __alignof__, but
    these operators are semantically different ever since r8-7957.  In the
    testcase below, this causes the second static_assert to fail on targets
    where alignof(double) != __alignof__(double) because the specialization
    table (which uses cp_tree_equal as its equality predicate) conflates the
    two dependent specializations integral_constant<__alignof__(T)> and
    integral_constant<alignof(T)>.

    This patch makes cp_tree_equal distinguish between these two operators
    by inspecting the ALIGNOF_EXPR_STD_P flag.

    gcc/cp/ChangeLog:

            PR c++/88115
            PR libstdc++/97273
            * tree.c (cp_tree_equal) <case ALIGNOF_EXPR>: Return false if
            ALIGNOF_EXPR_STD_P differ.

    gcc/testsuite/ChangeLog:

            PR c++/88115
            PR libstdc++/97273
            * g++.dg/template/alignof3.C: New test.

    (cherry picked from commit 592fe221735bdaa375b1834dd49ce125d0b600d8)

[Bug c++/88115] Incorrect result from alignof in templates, if also using __alignof__.

[cvs-commit at gcc dot gnu.org]

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

--- Comment #3 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Patrick Palka <ppalka@gcc.gnu.org>:

https://gcc.gnu.org/g:61827d5d9a5a09a8c05d5e41f95b03ebc6c43f61

commit r11-4925-g61827d5d9a5a09a8c05d5e41f95b03ebc6c43f61
Author: Patrick Palka <ppalka@redhat.com>
Date:   Wed Nov 11 14:43:39 2020 -0500

    c++: Correct the handling of alignof(expr) [PR88115]

    We're currently neglecting to set the ALIGNOF_EXPR_STD_P flag on an
    ALIGNOF_EXPR when its operand is an expression.  This leads to us
    handling alignof(expr) as if it were written __alignof__(expr), and
    returning the preferred alignment instead of the ABI alignment.  In the
    testcase below, this causes the first and third static_assert to fail on
    x86.

    gcc/cp/ChangeLog:

            PR c++/88115
            * cp-tree.h (cxx_sizeof_or_alignof_expr): Add bool parameter.
            * decl.c (fold_sizeof_expr): Pass false to
            cxx_sizeof_or_alignof_expr.
            * parser.c (cp_parser_unary_expression): Pass std_alignof to
            cxx_sizeof_or_alignof_expr.
            * pt.c (tsubst_copy): Pass false to cxx_sizeof_or_alignof_expr.
            (tsubst_copy_and_build): Pass std_alignof to
            cxx_sizeof_or_alignof_expr.
            * typeck.c (cxx_alignof_expr): Add std_alignof bool parameter
            and pass it to cxx_sizeof_or_alignof_type.  Set ALIGNOF_EXPR_STD_P
            appropriately.
            (cxx_sizeof_or_alignof_expr): Add std_alignof bool parameter
            and pass it to cxx_alignof_expr.  Assert op is either
            SIZEOF_EXPR or ALIGNOF_EXPR.

    libcc1/ChangeLog:

            PR c++/88115
            * libcp1plugin.cc (plugin_build_unary_expr): Pass true to
            cxx_sizeof_or_alignof_expr.

    gcc/testsuite/ChangeLog:

            PR c++/88115
            * g++.dg/cpp0x/alignof6.C: New test.

[Bug c++/88115] Incorrect result from alignof in templates, if also using __alignof__.

[cvs-commit at gcc dot gnu.org]

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

--- Comment #4 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Patrick Palka <ppalka@gcc.gnu.org>:

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

commit r11-4926-gb1c9b3c3408c1ec8043f9b9e1a148f84bb7f3b25
Author: Patrick Palka <ppalka@redhat.com>
Date:   Wed Nov 11 15:11:23 2020 -0500

    c++: Change the mangling of __alignof__ [PR88115]

    This patch changes the mangling of __alignof__ to v111__alignof__,
    making its mangling distinct from that of alignof(type) and
    alignof(expr).

    How we mangle ALIGNOF_EXPR now depends on its ALIGNOF_EXPR_STD_P flag,
    which after the previous patch gets consistently set for alignof(type)
    as well as alignof(expr).

    gcc/c-family/ChangeLog:

            PR c++/88115
            * c-opts.c (c_common_post_options): Update latest_abi_version.

    gcc/ChangeLog:

            PR c++/88115
            * common.opt (-fabi-version): Document =15.
            * doc/invoke.texi (C++ Dialect Options): Likewise.

    gcc/cp/ChangeLog:

            PR c++/88115
            * mangle.c (write_expression): Mangle __alignof_ differently
            from alignof when the ABI version is at least 15.

    libiberty/ChangeLog:

            PR c++/88115
            * cp-demangle.c (d_print_comp_inner)
            <case DEMANGLE_COMPONENT_EXTENDED_OPERATOR>: Don't print the
            "operator " prefix for __alignof__.
            <case DEMANGLE_COMPONENT_UNARY>: Always print parens around the
            operand of __alignof__.
            * testsuite/demangle-expected: Test demangling for __alignof__.

    gcc/testsuite/ChangeLog:

            PR c++/88115
            * g++.dg/abi/macro0.C: Adjust.
            * g++.dg/cpp0x/alignof7.C: New test.
            * g++.dg/cpp0x/alignof8.C: New test.

[Bug c++/88115] Incorrect result from alignof in templates, if also using __alignof__.

[cvs-commit at gcc dot gnu.org]

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

--- Comment #5 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-9 branch has been updated by Patrick Palka
<ppalka@gcc.gnu.org>:

https://gcc.gnu.org/g:9df05884b3a30d32744a070d3fc5780b7323231a

commit r9-9043-g9df05884b3a30d32744a070d3fc5780b7323231a
Author: Patrick Palka <ppalka@redhat.com>
Date:   Wed Oct 7 10:49:00 2020 -0400

    c++: Distinguish alignof and __alignof__ in cp_tree_equal [PR97273]

    cp_tree_equal currently considers alignof the same as __alignof__, but
    these operators are semantically different ever since r8-7957.  In the
    testcase below, this causes the second static_assert to fail on targets
    where alignof(double) != __alignof__(double) because the specialization
    table (which uses cp_tree_equal as its equality predicate) conflates the
    two dependent specializations integral_constant<__alignof__(T)> and
    integral_constant<alignof(T)>.

    This patch makes cp_tree_equal distinguish between these two operators
    by inspecting the ALIGNOF_EXPR_STD_P flag.

    gcc/cp/ChangeLog:

            PR c++/88115
            PR libstdc++/97273
            * tree.c (cp_tree_equal) <case ALIGNOF_EXPR>: Return false if
            ALIGNOF_EXPR_STD_P differ.

    gcc/testsuite/ChangeLog:

            PR c++/88115
            PR libstdc++/97273
            * g++.dg/template/alignof3.C: New test.

    (cherry picked from commit 592fe221735bdaa375b1834dd49ce125d0b600d8)

[Bug c++/88115] Incorrect result from alignof in templates, if also using __alignof__.

[foom at fuhm dot net]

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

--- Comment #6 from James Y Knight <foom at fuhm dot net> ---
>     c++: Change the mangling of __alignof__ [PR88115]

The new mangling chosen for __alignof__(type) seems problematic, and I think
this commit ought to be reverted until a new mangling scheme has been chosen.

At the very least, both __alignof__(expression) and __alignof__(type) can't
both be encoded as "v111__alignof__", with no indication of whether the
argument is a type or an expression, because that's ambiguous.

Possibly it'd be okay to use v112__alignof__z and v112__alignof__t, similar to
how "az" and "at" are used for standard alignof expression/type. That solves
the ambiguity. But -- this violates the generic grammar, which doesn't allow
for a vendor operator to take type arguments at all.
  <operator-name> ::= v <digit> <source-name>
  <expression> ::= <unary operator-name> <expression>

Now...possibly it's okay to violate that and pass a type, regardless. But, then
the demangler needs to know what every vendor extension operator's semantics
are in order to demangle -- which seems against the intent of the standardized
"vendor operator" syntax. So, that's not great.

This seems like a problem which may be worth solving properly and generally in
the itanium ABI -- maybe now's the time to actually do this? (Fixing this would
allow __typeof__, __builtin_convertvector, and __builtin_offsetof to be
mangleable too...) See also PR13740 and PR11078, for earlier (MUCH earlier!)
discussion of possible mangling syntax extensions for this sort of thing.
Unfortunately, it never went anywhere...

ISTM that the most promising proposal in those earlier threads was to add a "Y"
to both type and expression expansions.

To support vendor-specific builtins which returns a type, and can take types or
expressions as arguments (e.g. typeof), add:
  <type> ::= Y <sourcename> <template-arg>+ E

To support vendor-specific builtins which return a value, and can take types or
expressions as arguments (e.g. __builtin_offsetof, or __alignof__):
  <expression> ::= Y <sourcename> <template-arg>+ E

[Bug c++/88115] Incorrect result from alignof in templates, if also using __alignof__.

[foom at fuhm dot net]

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

--- Comment #7 from James Y Knight <foom at fuhm dot net> ---
I've created an ABI proposal against itanium abi, here:
https://github.com/itanium-cxx-abi/cxx-abi/issues/112

I realized since writing my last comment here that
  <type> ::= u <source-name> [<template-args>] # vendor extended type
has been added a few months ago. Therefore, nothing needs to be added to
support extended expressions as <type>.

And, therefore, since 'u' is also free in expression, I've proposed to simply
add the same 'u' syntax to "expression", rather than using "Y", as per previous
comment.

That is:
  <expression> ::= u <source-name> [<template-args>] # vendor extended
expression

[Bug c++/88115] Incorrect result from alignof in templates, if also using __alignof__.

[redi at gcc dot gnu.org]

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

--- Comment #8 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Thanks for starting the discussion there.

[Bug c++/88115] Incorrect result from alignof in templates, if also using __alignof__.

[foom at fuhm dot net]

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

--- Comment #9 from James Y Knight <foom at fuhm dot net> ---
Proposed patch posted for the itanium-cxx-abi:
 https://github.com/itanium-cxx-abi/cxx-abi/pull/115/files
using the syntax:
 u <source-name> <template-arg>* E

And to Clang, to use that syntax:
 https://reviews.llvm.org/D93922

I hope both GCC and Clang can do the same thing here.

[Bug c++/88115] Incorrect result from alignof in templates, if also using __alignof__.

[foom at fuhm dot net]

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

--- Comment #10 from James Y Knight <foom at fuhm dot net> ---
Seeing as GCC is now in Stage 4 before the next release, I'd like to ping this
bug.

Should the priority be upgraded? I think fixing this so that GCC doesn't use
'v111__alignof__' should be considered a release-blocker: either the mangling
change should be reverted to the previous behavior, or fixed.

FWIW, the change to clang has been committed, and is planned to be in the
upcoming Clang 12 release
(https://reviews.llvm.org/rG9c7aeaebb3ac1b94200b59b111742cb6b8f090c2)

[Bug c++/88115] Incorrect result from alignof in templates, if also using __alignof__.

[ppalka at gcc dot gnu.org]

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

Patrick Palka <ppalka at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |ppalka at gcc dot gnu.org
                 CC|                            |ppalka at gcc dot gnu.org

--- Comment #11 from Patrick Palka <ppalka at gcc dot gnu.org> ---
(In reply to James Y Knight from comment #10)
> Seeing as GCC is now in Stage 4 before the next release, I'd like to ping
> this bug.
> 
> Should the priority be upgraded? I think fixing this so that GCC doesn't use
> 'v111__alignof__' should be considered a release-blocker: either the
> mangling change should be reverted to the previous behavior, or fixed.

Yes, thanks for the ping.  I'll work on a patch for GCC 11 to follow suit, so
that Clang and GCC do the same thing.

> 
> FWIW, the change to clang has been committed, and is planned to be in the
> upcoming Clang 12 release
> (https://reviews.llvm.org/rG9c7aeaebb3ac1b94200b59b111742cb6b8f090c2)

[Bug c++/88115] Incorrect result from alignof in templates, if also using __alignof__.

[cvs-commit at gcc dot gnu.org]

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

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

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

commit r11-7921-ga3bf6ce7f2e17f2c977c13df23eb718e7b433dcd
Author: Patrick Palka <ppalka@redhat.com>
Date:   Tue Mar 30 22:57:11 2021 -0400

    c++: Adjust mangling of __alignof__ [PR88115]

    r11-4926 made __alignof__ get mangled differently from alignof,
    encoding __alignof__ as a vendor extended operator.  But this
    mangling is problematic for the reasons mentioned in
    https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88115#c6.

    This patch changes our mangling of __alignof__ to instead use the
    new "vendor extended expression" syntax that's proposed in
    https://github.com/itanium-cxx-abi/cxx-abi/issues/112.  Clang does
    the same thing already, so after this patch Clang and GCC agree
    about the mangling of __alignof__(type) and __alignof__(expr).

    gcc/cp/ChangeLog:

            PR c++/88115
            * mangle.c (write_expression): Adjust the mangling of
            __alignof__.

    include/ChangeLog:

            PR c++/88115
            * demangle.h (enum demangle_component_type): Add
            DEMANGLE_COMPONENT_VENDOR_EXPR.

    libiberty/ChangeLog:

            PR c++/88115
            * cp-demangle.c (d_dump, d_make_comp, d_expression_1)
            (d_count_templates_scopes): Handle DEMANGLE_COMPONENT_VENDOR_EXPR.
            (d_print_comp_inner): Likewise.
            <case DEMANGLE_COMPONENT_EXTENDED_OPERATOR>: Revert r11-4926
            change.
            <case DEMANGLE_COMPONENT_UNARY>: Likewise.
            * testsuite/demangle-expected: Adjust __alignof__ tests.

    gcc/testsuite/ChangeLog:

            PR c++/88115
            * g++.dg/cpp0x/alignof7.C: Adjust expected mangling.

[Bug c++/88115] Incorrect result from alignof in templates, if also using __alignof__.

[cvs-commit at gcc dot gnu.org]

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

--- Comment #13 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-8 branch has been updated by Patrick Palka
<ppalka@gcc.gnu.org>:

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

commit r8-10855-ga34c19926b87f9cadd5ea84f5c5b93ae76b14558
Author: Patrick Palka <ppalka@redhat.com>
Date:   Wed Oct 7 10:49:00 2020 -0400

    c++: Distinguish alignof and __alignof__ in cp_tree_equal [PR97273]

    cp_tree_equal currently considers alignof the same as __alignof__, but
    these operators are semantically different ever since r8-7957.  In the
    testcase below, this causes the second static_assert to fail on targets
    where alignof(double) != __alignof__(double) because the specialization
    table (which uses cp_tree_equal as its equality predicate) conflates the
    two dependent specializations integral_constant<__alignof__(T)> and
    integral_constant<alignof(T)>.

    This patch makes cp_tree_equal distinguish between these two operators
    by inspecting the ALIGNOF_EXPR_STD_P flag.

    gcc/cp/ChangeLog:

            PR c++/88115
            PR libstdc++/97273
            * tree.c (cp_tree_equal) <case ALIGNOF_EXPR>: Return false if
            ALIGNOF_EXPR_STD_P differ.

    gcc/testsuite/ChangeLog:

            PR c++/88115
            PR libstdc++/97273
            * g++.dg/template/alignof3.C: New test.

    (cherry picked from commit 592fe221735bdaa375b1834dd49ce125d0b600d8)

[Bug c++/88115] Incorrect result from alignof in templates, if also using __alignof__.

[ppalka at gcc dot gnu.org]

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

Patrick Palka <ppalka at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|NEW                         |RESOLVED
   Target Milestone|---                         |8.5

--- Comment #14 from Patrick Palka <ppalka at gcc dot gnu.org> ---
Fixed.