[Bug c++/105233] New: Incorrect "alignment not an integer constant" error in alignas with template parameter dependent argument

[Bug c++/105233] New: Incorrect "alignment not an integer constant" error in alignas with template parameter dependent argument

[Jonathan.Strobl at gmx dot de]

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

            Bug ID: 105233
           Summary: Incorrect "alignment not an integer constant" error in
                    alignas with template parameter dependent argument
           Product: gcc
           Version: 11.2.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: Jonathan.Strobl at gmx dot de
  Target Milestone: ---

Dear GCC maintainers,

g++ rejects the following code, which I believe to be valid:

g++ -std=c++20 -x c++ - <<EOF
#include <bit>
template<typename T>
struct Data {
    T m_a;
    T m_b;
    T m_c;
};
template<typename T>
struct alignas(std::bit_ceil(sizeof(Data<T>))) AlignedData {
    Data<T> m_data;
};
AlignedData<int> g_test;
EOF
<stdin>: In instantiation of ‘struct AlignedData<int>’:
<stdin>:12:18:   required from here
<stdin>:9:48: error: requested alignment is not an integer constant

This bug is present since GCC 10; I have tested all versions up to 12.0 (trunk)
using Godbolt: https://godbolt.org/z/3eaGGrvWM

One strange thing that I noticed: This only occurs when a value that depends on
a template parameter passes through a standard library function. If the
argument passed to std::bit_ceil doesn't depend on a template parameter, the
code compiles fine. Similarly, when std::bit_ceil is replaced with a hand-made
version of the same function, it also compiles.

std::bit_ceil seems to somehow strip the value passed to it of its
"constant-ness", but only if that value depends on a template parameter.

Thank you for your support,
Jonathan

[Bug c++/105233] Incorrect "alignment not an integer constant" error in alignas with template parameter dependent argument

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Slightly reduced:

template <typename T>
struct __int_traits {
  static constexpr int __digits = sizeof (T) * 8;
  static constexpr T __max = ~(T) 0;
};
namespace std __attribute__ ((__visibility__ ("default")))
{
  template<typename _Tp>
    constexpr int
    __countl_zero(_Tp __x) noexcept
    {
      constexpr auto _Nd = __int_traits<_Tp>::__digits;
      if (__x == 0)
        return _Nd;
      constexpr auto _Nd_ull = __int_traits<unsigned long long>::__digits;
      constexpr auto _Nd_ul = __int_traits<unsigned long>::__digits;
      constexpr auto _Nd_u = __int_traits<unsigned>::__digits;
      if constexpr (_Nd <= _Nd_u)
        {
          constexpr int __diff = _Nd_u - _Nd;
          return __builtin_clz(__x) - __diff;
        }
      else if constexpr (_Nd <= _Nd_ul)
        {
          constexpr int __diff = _Nd_ul - _Nd;
          return __builtin_clzl(__x) - __diff;
        }
      else if constexpr (_Nd <= _Nd_ull)
        {
          constexpr int __diff = _Nd_ull - _Nd;
          return __builtin_clzll(__x) - __diff;
        }
      else
        {
          static_assert(_Nd <= (2 * _Nd_ull),
                        "Maximum supported integer size is 128-bit");
          unsigned long long __high = __x >> _Nd_ull;
          if (__high != 0)
            {
              constexpr int __diff = (2 * _Nd_ull) - _Nd;
              return __builtin_clzll(__high) - __diff;
            }
          constexpr auto __max_ull = __int_traits<unsigned long long>::__max;
          unsigned long long __low = __x & __max_ull;
          return (_Nd - _Nd_ull) + __builtin_clzll(__low);
        }
    }
  template<typename _Tp>
    constexpr _Tp
    __bit_ceil(_Tp __x) noexcept
    {
      constexpr auto _Nd = __int_traits<_Tp>::__digits;
      if (__x == 0 || __x == 1)
        return 1;
      auto __shift_exponent = _Nd - std::__countl_zero((_Tp)(__x - 1u));
      if (!__builtin_is_constant_evaluated())
        {
          ;
        }
      return (_Tp)1u << __shift_exponent;
    }
}
template<typename T>
struct Data {
    T m_a;
    T m_b;
    T m_c;
};
template<typename T>
struct alignas(std::__bit_ceil(sizeof(Data<T>))) AlignedData {
    Data<T> m_data;
};
AlignedData<int> g_test;

Seems it is the if (!__builtin_is_constant_evaluated()) { ; }
part.

[Bug c++/105233] Incorrect "alignment not an integer constant" error in alignas with template parameter dependent argument

[jakub at gcc dot gnu.org]

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

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
      // If the shift exponent equals _Nd then the correct result is not
      // representable as a value of _Tp, and so the result is undefined.
      // Want that undefined behaviour to be detected in constant expressions,
      // by UBSan, and by debug assertions.
      if (!std::__is_constant_evaluated())
        {
          __glibcxx_assert( __shift_exponent != __int_traits<_Tp>::__digits );
        }
is what is in the header.

[Bug c++/105233] Incorrect "alignment not an integer constant" error in alignas with template parameter dependent argument

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2022-04-12
             Status|UNCONFIRMED                 |NEW

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Slightly more reduced:

template <typename _Tp>
constexpr _Tp
foo (_Tp __x) noexcept
{
  if (!__builtin_is_constant_evaluated ())
    {
      ;
    }
  return 4 * __alignof (int);
}

template <typename T>
struct A { T a, b, c; };

template <typename T>
struct alignas (foo (sizeof (A<T>))) B { A<T> d; };

B<int> e;

And we reject this since __builtin_is_constant_evaluated has been in
r9-2311-ge408261123697a82b5965c700fa2465999f0fd62 .

[Bug c++/105233] Incorrect "alignment not an integer constant" error in alignas with template parameter dependent argument

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jason at gcc dot gnu.org,
                   |                            |ppalka at gcc dot gnu.org

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Even
template <typename T>
constexpr T
foo (T x) noexcept
{
  bool a = __builtin_is_constant_evaluated ();
  return 4 * __alignof (int);
}

template <typename T>
struct A { T a, b, c; };

template <typename T>
struct alignas (foo (sizeof (A<T>))) B { A<T> d; };

B<int> e;

I see that the foo (sizeof (A<T>) expression is constexpr evaluated once
without manifestly_const_eval and with allow_non_constant set, so because of
the __builtin_is_constant_evaluated () fails in that case, as it isn't a
manifestly constant evaluation but manifestly constant evaluation could appear
later.

It isn't evaluated with it later though.

alignas argument is a constant expression (or type id), so at some point it
should be evaluated with manifestly_const_eval set.

Under the hood, we transform alignas into the gnu::aligned attribute, so the
question is, shall we treat as manifestly constant expression just alignas
argument, or gnu::aligned attribute's argument as well, or some other
attribute's arguments too?  And where exactly we'd evaluate those arguments?

Seems the C++ FE has cp_check_const_attributes for this, but there it just
calls:
          if (EXPR_P (expr))
            TREE_VALUE (arg) = fold_non_dependent_expr (expr);
which is non-manifestly constant evaluated and doesn't guarantee it will be a
constant expression (we then just error if it is not later on).

[Bug c++/105233] Incorrect "alignment not an integer constant" error in alignas with template parameter dependent argument

[jakub at gcc dot gnu.org]

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

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 52791
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52791&action=edit
gcc12-pr105233.patch

Untested patch that makes alignas and aligned attribute argument manifestly
constant evaluated expression.

[Bug c++/105233] Incorrect "alignment not an integer constant" error in alignas with template parameter dependent argument

[ppalka at gcc dot gnu.org]

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

--- Comment #6 from Patrick Palka <ppalka at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #5)
> Created attachment 52791 [details]
> gcc12-pr105233.patch
> 
> Untested patch that makes alignas and aligned attribute argument manifestly
> constant evaluated expression.

Makes sense to me FWIW

[Bug c++/105233] Incorrect "alignment not an integer constant" error in alignas with template parameter dependent argument

[cvs-commit at gcc dot gnu.org]

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

--- Comment #7 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:13c32c1984f5857ccce2aeb00ce34343e5a26954

commit r12-8140-g13c32c1984f5857ccce2aeb00ce34343e5a26954
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Wed Apr 13 15:43:34 2022 +0200

    c++: Treat alignas align_expr and aligned attribute's operand as manifestly
constant evaluation [PR105233]

    The following testcase fails, because we only constant evaluate the
    alignas argument as non-manifestly constant-evaluated and as
    __builtin_is_constant_evaluated appears, we make it non-constant
    (the reason is that we often try to evaluate some expression without
    manifestly_const_eval perhaps even multiple times before actually
    evaluating it with manifestly_const_eval (e.g. when folding for warnings
    and in many other places), and we don't want
__builtin_is_constant_evaluated
    to evaluate to false in those cases, because we could get a different
    result from when we actually evaluate it with manifestly_const_eval
    set).
    Now, for alignas the standard seems to be clear, it says the
    argument is constant-expression, which means we should
    manifestly-constant-eval it.
    Attributes are a fuzzy area, they are extensions and various attributes
    take e.g. identifiers, or string literals etc. as arguments.

    Either we can just treat alignas as manifestly-const-eval, for that
    we'd need some way how to differentiate between alignas and gnu::aligned
    or aligned attribute.

    Another possibility is what the patch below implements, treat
    both alignas and gnu::aligned and aligned attribute's argument as
    manifestly-const-eval and not do that for other attributes.

    Another is to go through all attributes and figure out for which
    such treatment is useful (e.g. those that expect INTEGER_CST as argument),
    and either add a new column in the attribute table or have another table
    in the C++ FE to find out which attribute needs that.

    Another is do that for all the attribute arguments that are EXPR_P
    and see what breaks (bet that it could be quite risky this late in
    GCC 12 cycle and especially for backporting).

    2022-04-13  Jakub Jelinek  <jakub@redhat.com>

            PR c++/105233
            * decl2.cc (cp_check_const_attributes): For aligned attribute
            pass manifestly_const_eval=true to fold_non_dependent_expr.

            * g++.dg/cpp2a/is-constant-evaluated13.C: New test.

[Bug c++/105233] Incorrect "alignment not an integer constant" error in alignas with template parameter dependent argument

[jakub at gcc dot gnu.org]

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

--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed on the trunk, I'd wait some time before considering to backport this, so
not going to be in 11.3.

[Bug c++/105233] Incorrect "alignment not an integer constant" error in alignas with template parameter dependent argument

[Jonathan.Strobl at gmx dot de]

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

--- Comment #9 from Jonathan Strobl <Jonathan.Strobl at gmx dot de> ---
Thanks a lot for fixing this so quickly!

Here's my two cents about whether to special-case attribute aligned or whether
to make a general solution: aligned isn't the only attribute that's affected.
It'd also make sense for things like vector_size to evaluate their arguments
with manifestly_const_eval; adding another column in the attribute table
therefore seems like a logical thing to do to me.

However, special-casing aligned for now is of course also a perfectly valid fix
until there's a more general solution.

Here's a small, somewhat realistic test case for vector_size that currently
fails because of the same problem:

constexpr unsigned bit_ceil(unsigned val) {
    bool a = __builtin_is_constant_evaluated();
    unsigned result = 1;
    while (result < val) result <<= 2;
    return result;
}

template<typename T, unsigned num>
struct VecImpl {
    typedef __attribute__((vector_size(bit_ceil(sizeof(T) * num)))) T type;
};

using Vec3f = VecImpl<float, 3>::type;

[Bug c++/105233] Incorrect "alignment not an integer constant" error in alignas with template parameter dependent argument

[jakub at gcc dot gnu.org]

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

--- Comment #10 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Sure, but while for alignas it is the standard that makes it clear, for the
attributes it is up to us to decide what to do.
Jason said he'd like for GCC 13 us to try making all attribute arguments
manifestly const eval and see how far we get with that.
As for your #c9 testcase, there is an easy workaround, just add a constexpr var
(or static data member etc.) with the value, that will make it clearly
manifestly constant evaluated and will work even with older compilers.