[d16-5][Runtime] Move some of the changes back to int. (#7529) - #7606
Conversation
Some of the fixes done for the warnings have breaking changes. Move back to int and ensure that we do not have any compilation errors (we are using -Werror). Fixes: dotnet#7509
mandel-macaque
changed the title
| monotouch_set_monodevelop_port (int port) | ||
| { | ||
| monodevelop_port = port; | ||
| monodevelop_port = (long) port; |
There was a problem hiding this comment.
My understanding is that we are trying to remove long/unsigned long types because their size changes depending on the platform and somehow that is a breaking change?
It doesn't seem like there is any harm in casting an int to a long, but why are we doing this here when everywhere else we are removing the long type?
There was a problem hiding this comment.
remove long/unsigned long types
Not quite. They ([u]int/[u]long) are all fine individually... until you mix them up.
long->intcan overflow;sign/unsignedchanges can be issues too, e.g.-1to unsigned;
The goal is not to remove anything specific but to be consistent (correctness) in their usage.
It doesn't seem like there is any harm in casting an int to a long
Right, it is generally not an issue... unless mapped to memory, e.g. change a structure member from int to long can (alignment dependent) change its size or element position, which might be defined elsewhere (e.g. mono or in managed code).
An earlier PR enabled the compiler to spot those changes (i.e. emit warnings) and also turn warnings into errors.
However there were issues in the previous fixes (the compiler does not see everything, having no warning/error does not mean the code works). @rolfbjarne listed them inside #7509
|
Build failure Test results1 tests failed, 87 tests passed.Failed tests
|
|
Known issue: https://github.com/xamarin/maccore/issues/581 |
5 participants
Some of the fixes done for the warnings have breaking changes. Move back
to int and ensure that we do not have any compilation errors (we are
using -Werror).
Fixes: #7509