Operator << incorrectly truncates values. about slang HOT 10 CLOSED

I couldn't manage to work on this issue for the iteration 2.
Rescheduling to iteration 3.

from slang.

We still need to decide on whether or not we want to C integer promotion behavior.
My preference is we don't allow it, and just print an error here.

See a related discussion here: https://jeffhurchalla.com/2019/01/16/c-c-surprises-and-undefined-behavior-due-to-unsigned-integer-promotion/

from slang.

Thank you for the link. It was an interesting read.
In C/C++, any operations with unsigned short will be implicitly promoted to int, and it can caused unexpected behaviors, because unsigned-ness gets turned into signed-ness. To avoid the problem, the code needs to be written to explicitly promoted to unsigned int.

from slang.

Now I am not sure what the problem is on this issue.
If the given expression, "uint8_t << 15", uses "operator<<(uint8_t, uint8_t)", it may not be conformant to the C/C++ standard, but it wouldn't cause a problem.

Do we still want to promote it to use "operator<<(int,int)" with an error?
Is this issue mainly for targeting C and CUDA?

What is a repro case for this issue when "operator<<(uint8_t,uint8_t)" is used?

from slang.

I think the right thing to do here without introducing breaking changes is to issue a warning when we see a call to << with anything less than int and an implicitly casted int literal.

from slang.

The function signature of operator<< is following,

__generic<L: __BuiltinIntegerType, R: __BuiltinIntegerType>
__intrinsic_op(251)
L operator<<(L left, R right);

When I use it with (uint8_t, int), it returns uint8_t as the initial issue description says.
But the second argument is not demoted from int to uint8_t.
This may not be conformant to the C standard, but this looks to be a reasonable solution to avoid the problematic type promotion from unsigned small types to a signed int.

As an experiment, when I changed to use a same type, the type is promoted from uint8_t to int.

__generic<L: __BuiltinIntegerType>
__intrinsic_op(251)
L operator<<(L left, L right);

I think our discussions were based on an assumption that the bitwise-shift takes only one generic-type.
Because it is not the case, I am not sure how to proceed about the bitwise-shift operators.

However, there are other operators that take only one generic-type.
And I confirmed that uint8_t is promoted to int when the second argument is int.
These operators will need to print a warning as discussed.

operator+
operator-
operator*
operator/
operator%

operator<
operator>
operator<=
operator>=

operator|
operator&
operator^

I will work on those operators while waiting to decide what to do about the bitwise-shift.

from slang.

For the bitwise shift, if the right hand side value is too big for the size if the left hand side type, I can print a warning.

That might be what the original problem was about.

from slang.

I found a comment from core.meta.slang related to this issue,

        // TODO: We should handle a `SHIFT` flag on the op
        // by changing `rightType` to `int` in order to
        // account for the fact that the shift amount should
        // always have a fixed type independent of the LHS.
        //
        // (It is unclear why this change hadn't been made
        // already, so it is possible that such a change
        // breaks overload resolution or other parts of
        // the compiler)

This comment was left by the following git commit,

18be2d81
Tim Foley <[email protected]>
3/6/2020 11:37:36 AM

from slang.

After investigating the issue for a while, it appears that the bitwise-shift operators are intentionally taking two generic types rather than one.
For that, I think I should close this issue as WNF.
I need a feedback from @csyonghe .

For the other operators such as operator+, operator-, and so on, I will need to create a new bug to print a warning when the given types are using different sign-ness.

from slang.

I am sharing an update.
We decide to continue working on this issue.
When the second argument to operator<< is too big for the first argument type, a warning will be printed.
I will focus on this task for this issue, and we will handle the other operators in another issue.

from slang.