[Mono-dev] Overflowed double->int casts on ARM in MS referencesource

[Alexander Köplinger]

Thanks for your response Sebastien!
 
I agree that it's pure luck those edge cases showed up in the unit tests :)
I just wanted confirmation that you guys think it's worthwhile fixing those cases nevertheless and let's hope there won't be too many more of them in the codebase. I'll wait until my PR is merged and revert your commit as you said.
 
-- Alex
 
Date: Mon, 23 Mar 2015 20:20:28 -0400
Subject: Re: Overflowed double->int casts on ARM in MS referencesource
From: sebastien.pouliot at gmail.com
To: alex.koeplinger at outlook.com
CC: mono-devel-list at lists.ximian.com

Hey,
On Mon, Mar 23, 2015 at 7:47 PM, Alexander Köplinger <alex.koeplinger at outlook.com> wrote:



I just noticed this commit by @spouliot: https://github.com/mono/mono/commit/298962b7ddd5e3af33c3177e8523cc36da4de553
 
In my opinion, this isn't the right approach, we should rather fix the cases where a cast would overflow in MS referencesource code rather than changing the tests.
 
I sent a PR a week or so ago that fixes the particular DateTime tests on ARM by explictly checking if the value fits into long, which I think is better: https://github.com/mono/referencesource/pull/8
Feel free to revert that commit once you PR lands. It's goal is to clear up issues* for the next XI release.
* or non-issue in that case, DateTime works just fine  
There are a couple more of these overflows in MS code that make tests fail and I think we should discuss what the best approach is. What are your thoughts?

The real problems with unspecified values (double casted to long) is that:

a) it's unrelated to the feature being tested (i.e. it was not a DateTime failure);

b) you can only fix the BCL for the cases we know. It's pure luck that those tests exists, i.e. they were not written to test that condition. That means it solves one of potentially many cases.

That being said it's still worth fixing the known cases inside the BCL source itself - even if it requires a bit more code and is not complete :-)

Sebastien

 		 	   		  
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ximian.com/pipermail/mono-devel-list/attachments/20150324/66d1dd47/attachment.html>