Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

C# compiler casting bug (erroneous CS0675) #42816

Closed
vsfeedback opened this issue Mar 26, 2020 · 11 comments · Fixed by #58178
Closed

C# compiler casting bug (erroneous CS0675) #42816

vsfeedback opened this issue Mar 26, 2020 · 11 comments · Fixed by #58178
Labels
Area-External Resolution-External The behavior lies outside the functionality covered by this repository

Comments

@vsfeedback
Copy link

This issue has been moved from a ticket on Developer Community.


Is this really the expected and documented compiler behaviour, that casting an integral to uint is in reality producing something different, that to really get an uint you need to cast that to uint? Issue title was selected from the expectation not even the C# language specification can be that messed up, and this is therefore a compiler bug.

class C
{
  static uint fn(sbyte a, sbyte b)
  {
    uint ret;
    ret = (uint)(a << 8) | (uint)b; // CS0675
    ret = (uint)(uint)(a << 8) | (uint)b; // CS0675
    ret = (uint)(a << 8) | (uint) (uint)b;
    ret = (uint)(uint)(a << 8) | (uint) (uint)b;
    return ret;
  }
}

that is, the only way to silence the compiler warning is to twice cast the second operand of the binary operation to the target type!

The type of the statement (uint)b shall be uint, only uint and nothing but uint. It can be nothing else, the explicit cast tells the compiler it is so. Yet the compiler seemingly thinks it's not (without telling us what it has really converted that into).


Original Comments

Visual Studio Feedback System on 3/22/2020, 06:46 PM:

We have directed your feedback to the appropriate engineering team for further evaluation. The team will review the feedback and notify you about the next steps.


Original Solutions

(no solutions)

@CyrusNajmabadi
Copy link
Member

Closing out. This is both a language quesiton, but is currently by design as how the C# language works. If you wish to make proposals to change this, please file them at github.com/dotnet/csharplang.

@CyrusNajmabadi CyrusNajmabadi added Area-External Resolution-External The behavior lies outside the functionality covered by this repository labels Mar 26, 2020
@SuuperW
Copy link

SuuperW commented Nov 19, 2020

Closing out. This is both a language quesiton, but is currently by design as how the C# language works. If you wish to make proposals to change this, please file them at github.com/dotnet/csharplang.

This is not a helpful comment, as it says nothing about why the behavior is what it is, or about what is actually happening under the hood. Could you please explain, or link to an explanation, if this is actually intended behavior?

I have a slightly different situation, where I am casting an int to a ulong (as opposed to OP casting a sbyte to to uint). I get the same warning if I use a single cast. But if I cast twice, I still get this warning along with a hint in VS saying the cast is redundant. If I cast to uint or long before casting to ulong, the warning goes away but I still have the "cast is redundant" hint.

@CyrusNajmabadi
Copy link
Member

but I still have the "cast is redundant" hint.

That's an ide bug. Can you please file a simple example? I'll look to getting a fix in that regard.

In terms of the language itself, it's due to how the language extends values when using these operators. The language extends things toward signed values in cases like this. So, if you don't want that, you need an explicit unsigned cast.

If you'd like that changed, you'd need to file a language suggestion. However at a cursory glance, it seems like that could break code.

@SuuperW
Copy link

SuuperW commented Nov 19, 2020

Here are some examples:
First, create two variables of types ulong and int:
ulong a = 1; int b = 2;
Then, the line in question:

  1. This gives an error, which helpfully describes the problem. (can't use operator | on ulong and int)
    ulong c = a | b;
  2. I would expect that an explicit cast as given here would result in no problems, but it gives the aforementioned warning.
    ulong c = a | (ulong)b;
  3. This results in a redundant cast note for the ulong cast. (but no warning)
    ulong c = a | (ulong)(uint)b;
  4. This results in a redundant cast note for the long cast. (but no warning)
    ulong c = a | (ulong)(long)b;

EDIT: I will add that the warning description and the operator description (obtained by hovering over the | character) disagree. The warning states that the bitwise-or operator is being used on a sign-extended operand, while the operator description says "ulong ulong.operator |(ulong left, ulong right)" indicating only unsigned operands.

@CyrusNajmabadi
Copy link
Member

I'm going to reactivate as i do want clarification on at least one of those cases.

@jaredpar this case is odd to me:

ulong a = 1; int b = 2;
ulong d = a | (ulong)b;

This should be equivalent to:

ulong a = 1; int b = 2;
ulong c = (ulong)b;
ulong d = a | c;

However, the former warns, but hte latter does not. That does actually seem like a bug to me afaict.

Thanks for the clarifying case!

@jaredpar
Copy link
Member

Did a bit of digging and this appears to be "By Strange Design".

Best place to get context on this is the comment block in the following locations:

The big take aways from those are essentially:

  • This is a heuristic not a right / wrong warning
  • The native compiler also had this as a heuristic and it did some truly strange behaviors that Roslyn attempted to maintain.

The gist of the heuristic here though is that the compiler wants to warn when there are surprising sign extensions in the code. The definition of surprising is essentially the heuristic nature of the warning and it can be summarized by the following:

// A "surprising" sign extension is:
//
// * a conversion with no cast in source code that goes from a smaller
//   signed type to a larger signed or unsigned type.
//
// * a conversion (with or without a cast) from a smaller
//   signed type to a larger unsigned type.

The case which includes the cast falls into the later rule here: cast from smaller signed type to larger unsigned type. While the case without the cast hits neither of these rules.

Does that clear it up?

@CyrusNajmabadi
Copy link
Member

Ah, got it. Will have to update the IDE here to understand that quirk.

@CyrusNajmabadi
Copy link
Member

// This results in a redundant cast note for the ulong cast. (but no warning)
ulong c = a | (ulong)(uint)b;

looks like this case was fixed.

This results in a redundant cast note for the long cast. (but no warning)
ulong c = a | (ulong)(long)b;

looks like this case was not.

@SuuperW
Copy link

SuuperW commented Nov 19, 2020

One of the comments that you linked to states (where y is a short) ""uint z = (uint)x | (uint)y;" should still produce the warning because the user might not realize that converting y to uint does a sign extension."
Indeed, I did not realize that this happens. It seems then that my problem was in not understanding what the warning text was trying to tell me. Perhaps a re-wording would be helpful? Something like "Bitwise-or operator used on the result of a sign-extending cast; consider casting to a smaller unsigned type first"? Or maybe most people would understand it as-is, Idk.

This also makes sense of why there is a difference if the operand is cast and stored in a ulong variable first. Unless all casts from int to ulong were to cause a warning, there is no cause to warn there. And there is no cause to warn bitwise-oring two ulong variables either. The compiler isn't looking for these kinds of multiple-line-spanning mistakes, and I wouldn't expect it to.

Also the IDE notes make some sense as well. The redundant casts indicated in my above post are indeed unnecessary, since they will be implicitly converted. However I can still see an argument for not flagging them as redundant in this case, since it may be desirable to include them for clarity... but not everyone may agree.

@CyrusNajmabadi
Copy link
Member

CyrusNajmabadi commented Nov 19, 2020

I think this makes sense:

// * a conversion with no cast in source code that goes from a smaller
//   signed type to a larger signed or unsigned type.

I'm not sure about this:

// * a conversion (with or without a cast) from a smaller
//   signed type to a larger unsigned type.

Specifically the "with ... a cast".

If the conversion has the cast, then it's no longer surprising afaict. Note: this is the logic the IDE implements here for determining if we can remove things.

@jaredpar we could always relax this a bit. That should not be breaking as it would be removing warnings, not adding them.

@jaredpar
Copy link
Member

I agree the "with a cast" part is odd and doesn't really seem to make sense. This is not a strong area for me though so wanted to see what @cston and @tannergooding think here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-External Resolution-External The behavior lies outside the functionality covered by this repository
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants