<div dir="ltr">Hey,<div><br></div><div class="gmail_extra"><div class="gmail_quote">On Mon, Mar 23, 2015 at 7:47 PM, Alexander Köplinger <span dir="ltr"><<a href="mailto:alex.koeplinger@outlook.com" target="_blank">alex.koeplinger@outlook.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">


<div><div dir="ltr"><font color="#000000" face="Calibri,sans-serif">I just noticed this commit by @spouliot: <a href="https://github.com/mono/mono/commit/298962b7ddd5e3af33c3177e8523cc36da4de553" target="_blank">https://github.com/mono/mono/commit/298962b7ddd5e3af33c3177e8523cc36da4de553</a></font><br><font color="#000000" face="Calibri,sans-serif"></font> <br><font color="#000000" face="Calibri,sans-serif">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.</font><br> <br><font color="#000000" face="Calibri,sans-serif">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: </font><a href="https://github.com/mono/referencesource/pull/8" target="_blank">https://github.com/mono/referencesource/pull/8</a></div></div></blockquote><div><br></div><div>Feel free to revert that commit once you PR lands. It's goal is to clear up issues* for the next XI release.</div><div><br></div><div>* or non-issue in that case, DateTime works just fine</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div><div dir="ltr"> <br>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?<br></div></div></blockquote><div><br></div><div>The real problems with <span style="color:rgb(51,51,51);font-size:13px;line-height:1.42857143;white-space:pre-wrap;background-color:rgb(245,245,245)">unspecified values (double casted to long) is that:</span><br></div><div><span style="color:rgb(51,51,51);font-size:13px;line-height:1.42857143;white-space:pre-wrap;background-color:rgb(245,245,245)"><br></span></div><div><span style="color:rgb(51,51,51);font-size:13px;line-height:1.42857143;white-space:pre-wrap;background-color:rgb(245,245,245)">a) it's unrelated to the feature being tested (i.e. it was not a DateTime failure);</span><br></div><div><span style="color:rgb(51,51,51);font-size:13px;line-height:1.42857143;white-space:pre-wrap;background-color:rgb(245,245,245)"><br></span></div><div><span style="color:rgb(51,51,51);font-size:13px;line-height:1.42857143;white-space:pre-wrap;background-color:rgb(245,245,245)">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.</span><br></div><div><span style="color:rgb(51,51,51);font-size:13px;line-height:1.42857143;white-space:pre-wrap;background-color:rgb(245,245,245)"><br></span></div><div><span style="color:rgb(51,51,51);font-size:13px;line-height:1.42857143;white-space:pre-wrap;background-color:rgb(245,245,245)">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 :-)</span><br></div><div><span style="color:rgb(51,51,51);font-size:13px;line-height:1.42857143;white-space:pre-wrap;background-color:rgb(245,245,245)"><br></span></div><div>Sebastien<br></div><div><br></div></div></div></div>