[cfe-dev] david's integer overflow stuff

[David Chisnall]

On 10 Sep 2010, at 23:11, Chris Lattner wrote:

> On Sep 10, 2010, at 2:51 PM, David Chisnall wrote:
>>>> Chris, I note that you made this change last month, without any discussion, removing the original, generic, behaviour that was discussed on-list, in favour of GCC's more limited behaviour which does not provide options for recovery.  
>>> 
>>> I thought that I emailed you or cc'd you on the commit.  But yes you're right.  -ftrapv is a gcc flag and we got numerous bug reports from people who were trying to use it and getting link errors.
>> 
>> Numerous?  I think I remember two.  Anyway, that would have been a good reason for starting a discussion, not a good reason for removing a feature that has been in clang for over a year and that people are using.
> 
> I thought I emailed you about this.  

Nope, the first I saw was when I read the patch just now and discovered that it had been removed.  Checking the svn log, I discovered that you did it a month ago.

You even put in your commit message 'David is using this...' indicating that you knew that removing it would break existing code, but did not discuss it prior to making the change - even if you thought you emailed me, you clearly did not wait for a reply.  

> In any case, I don't think that there is any reasonable discussion to be had: it's the wrong thing to do for -ftrapv, as I said before, it breaks an established model.  

What established model?  Every time -ftrapv is mentioned on the GCC lists, the instructions are not to use it.  

> Adding a new option that does the new behavior may be reasonable.

Indeed, and the correct time for doing this was when refactoring the code.  Now, since 2.8 has already branched, I am faced with another six months of telling people that they need to use clang from trunk instead of the release version.  

If you had bothered to email me about this (and, when removing a feature that you know people use, the correct time to make the commit is after you have received a reply, not after you think you have sent an email), then I could have fixed it before the release was branched.

>>> I didn't realize that you were implementing it in a gcc compatible way.
>> 
>> I provided a default handler function that would call abort (after printing a helpful error) along with the original patch.  It never got committed (I didn't have commit access back then, other people were committing my patches).  Mike said it would go in a clang runtime lib when one existed.
> 
> Are you sure that I approved the patch and not mike?  Do you have a link?

Mike committed it.  You reviewed it and asked that the -ftrapu option be removed.  Check the mailing list archives for April 2, 2009.  We also discussed it in IRC, where I explained both the implementation and my use of it in some detail (if you log private chats, check around April 2-4, 2009 - I don't keep logs).

> In any case, it doesn't matter if you patch included it or not, the end result is that the compiler was broken.  It was reported in more than two places.

The compiler is now broken for anyone who was using the -ftrapv functionality that clang has had for over a year, and which has been in two releases.  Worse; while it is possible to work around the previous 'breakage' by simply linking in the file that I included with the original patch and mirroring the gcc's behaviour, it is not possible to work around this behaviour.  

David