[Bug c++/104594] New: narrowing conversion of -1 to unsigned char at compile time not detected

[Bug c++/104594] New: narrowing conversion of -1 to unsigned char at compile time not detected

[raffael at casagrande dot ch]

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

            Bug ID: 104594
           Summary: narrowing conversion of -1 to unsigned char at compile
                    time not detected
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: raffael at casagrande dot ch
  Target Milestone: ---

The current gcc trunk compiles the following piece of code:
------------------------------------------------------------
template <unsigned char DIM_FROM>
concept Geometry = (DIM_FROM == -1);

template <class INIT>
requires Geometry<INIT::n>
auto GaussNewton(const INIT& init) -> void {}

template<int N>
struct X {
  static constexpr int n = N;
};

int main() { GaussNewton(X<-1>{}); }
--------------------------------------

I think this should NOT compile since it entails a narrowing conversion of -1
to an unsigned char type at compile time. Clang as well as MSVC fail to compile
the code.
(In many other cases, gcc also fails to compile if such a narrowing conversion
happens at compile time.)

[Bug c++/104594] narrowing of -1 to unsigned char not detected with requires concepts

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2022-02-18
            Summary|narrowing conversion of -1  |narrowing of -1 to unsigned
                   |to unsigned char at compile |char not detected with
                   |time not detected           |requires concepts
     Ever confirmed|0                           |1

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Confirmed.
GCC correctly detects:
template <unsigned char DIM_FROM>
constexpr bool Geometry = (DIM_FROM == -1);
template <class INIT>
constexpr bool tt1 = Geometry<INIT::n>;
template<int N>
struct X {
  static constexpr int n = N;
};
bool t = tt1<X<-1>>;

[Bug c++/104594] narrowing of -1 to unsigned char not detected with requires concepts

[ppalka at gcc dot gnu.org]

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

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
             Status|NEW                         |ASSIGNED
                 CC|                            |ppalka at gcc dot gnu.org

[Bug c++/104594] narrowing of -1 to unsigned char not detected with requires concepts

[ppalka at gcc dot gnu.org]

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

--- Comment #2 from Patrick Palka <ppalka at gcc dot gnu.org> ---
Here's a testcase which illustrates that the fix must happen during
normalization, not during satisfaction, since after normalization we don't know
which concepts we looked through to yield the given atom:

template<class T, T X>
concept C = (X != 0);

template<class T, T X>
concept D = C<int, X>;

template<auto X>
concept E = D<unsigned, X>;

static_assert(E<-1>); // should fail due to implicit conversion from '-1' to
unsigned

[Bug c++/104594] narrowing of -1 to unsigned char not detected with requires concepts

[ppalka at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           See Also|                            |https://gcc.gnu.org/bugzill
                   |                            |a/show_bug.cgi?id=67898
                 CC|                            |jason at gcc dot gnu.org

--- Comment #3 from Patrick Palka <ppalka at gcc dot gnu.org> ---
I wonder if this is ultimately a manifestation of PR67898.  After substituting
into an NTTP, if the type of the NTTP or the type of its argument is still
dependent then we can't yet check the implicit conversion from the argument
type to the parameter type.  And we don't encode this implicit conversion
within the substituted argument (as e.g. an IMPLICIT_CONV_EXPR) because we'll
check the conversion again when we call coerce_template_parms the next time
around anyway.

However, as PR67898 illustrates, this causes problems if a subsequent template
parameter refers to this NTTP, e.g:

  template<class T, T V, class = decltype(V)> struct A;
  template<class U, int W> using B = A<U, W>;

Here A<U, W> should resolve to A<U, W, U> but it instead resolves to A<U, W,
int> because we don't encode the implicit conversion (from int to U) within the
substituted argument for V, and hence substitution into the default argument
yields decltype(W), which is just int, rather than yielding decltype(U(W)).

Seems we can run into the same issue during normalization as illustrated in
this current PR.  The normal form for E in commment #2 is just (X != 0) (with
mapping X -> X) but we really should be encoding the implicit conversions into
the mapping (yielding something like X -> unsigned(int(X))).

So I wonder if the best way to tackle this PR is to tackle PR67898 more
generally?

[Bug c++/104594] narrowing of -1 to unsigned char not detected with requires concepts

[ppalka at gcc dot gnu.org]

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

--- Comment #4 from Patrick Palka <ppalka at gcc dot gnu.org> ---
N.B. coerce_template_parms sometimes does encode the implicit conversion for a
dependent NTTP argument, but only for sake of specialization matching (in light
of 'auto' template parms IIUC), the IMPLICIT_CONV_EXPR built is a no-op
otherwise.

[Bug c++/104594] narrowing of -1 to unsigned char not detected with requires concepts

[ppalka at gcc dot gnu.org]

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

--- Comment #5 from Patrick Palka <ppalka at gcc dot gnu.org> ---
(In reply to Patrick Palka from comment #4)
> N.B. coerce_template_parms sometimes does encode the implicit conversion for
> a dependent NTTP argument, but only for sake of specialization matching (in
> light of 'auto' template parms IIUC), the IMPLICIT_CONV_EXPR built is a
> no-op otherwise.

This IMPLICIT_CONV_EXPRs I'm referring to are built by 
maybe_convert_nontype_argument.

[Bug c++/104594] narrowing of -1 to unsigned char not detected with requires concepts

[jason at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           See Also|                            |https://gcc.gnu.org/bugzill
                   |                            |a/show_bug.cgi?id=99795

--- Comment #6 from Jason Merrill <jason at gcc dot gnu.org> ---
This looks like the same issue as PR99795, to which I attached a partial patch.
 Feel free to use whatever of that looks useful to you.

[Bug c++/104594] narrowing of -1 to unsigned char not detected with requires concepts

[cvs-commit at gcc dot gnu.org]

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

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

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

commit r14-8291-gf1e5bf0d83ee4da81b6317c6d7f1278fe7eaa5a0
Author: Jason Merrill <jason@redhat.com>
Date:   Wed Jan 17 17:29:33 2024 -0500

    c++: alias template argument conversion [PR112632]

    We've had a problem with lost conversions to template parameter types for a
    while now; looking at this PR, it occurred to me that the problem is really
    with alias (and concept) templates, since we do substitution of dependent
    arguments into them in a way that we don't for other templates.  And fixing
    that specific problem is a lot simpler than adding IMPLICIT_CONV_EXPR
around
    all dependent template arguments the way I gave up on for 111357.

    The other part of the fix was changing tsubst_expr to actually call
    convert_nontype_argument instead of assuming it will eventually happen.

    I waffled about stripping the forced conversion when !force_conv
    vs. skipping them in iterative_hash_template_arg and
    template_args_equal (like we already do for some other conversions) and
    decided to go with the former, but that isn't a strong preference if it
    turns out to be somehow problematic.

            PR c++/112632
            PR c++/112594
            PR c++/111357
            PR c++/104594
            PR c++/67898

    gcc/cp/ChangeLog:

            * cp-tree.h (IMPLICIT_CONV_EXPR_FORCED): New.
            * pt.cc (expand_integer_pack): Remove 111357 workaround.
            (maybe_convert_nontype_argument): Add force parm.
            (convert_template_argument): Handle alias template args
            specially.
            (tsubst_expr): Don't ignore IMPLICIT_CONV_EXPR_NONTYPE_ARG.
            * error.cc (dump_expr) [CASE_CONVERT]: Handle null optype.

    gcc/testsuite/ChangeLog:

            * g++.dg/cpp0x/alias-decl-nontype1.C: New test.
            * g++.dg/cpp2a/concepts-narrowing1.C: New test.
            * g++.dg/cpp2a/nontype-class63.C: New test.
            * g++.dg/cpp2a/nontype-class63a.C: New test.

[Bug c++/104594] narrowing of -1 to unsigned char not detected with requires concepts

[jason at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED
   Target Milestone|---                         |14.0
           Assignee|ppalka at gcc dot gnu.org          |jason at gcc dot gnu.org

--- Comment #8 from Jason Merrill <jason at gcc dot gnu.org> ---
Fixed.