r177329 - This code works around what appears to be a bug in another part of clang.

[reed kotler]

On 03/18/2013 04:36 PM, Rafael Espíndola wrote:
>> Hi Rafael,
>>
>> I don't think I should have to revert this patch.
>>
>> I'm not even sure if there is another bug.
>>
>> I don't know the code that is calling this. It's just my opinion that there
>> is some other issue.
> Then you should investigate that. What we should never do is add
>
>      if (!Fn) return; // should not happen
>
> If it should not happen, this should be an assert (as it was before
> your patch). It it is a logically valid condition, instead of the
> wrong comment we should have a test where Fn is null.
>
>> Someone that knows clang can explain how FD is a function declaration but GV
>> is not a Function.
>> Why is that my responsibility to sort that out?
> Because you changed the code.
>
>> This code is need for the mips test-suite to not regress and for my
>> attribute work to continue.
> Sorry. You should not push problems to others. Looks like you actually
> have a testcase and you just need to reduce it. Please revert this
> patch and reduce the testcase. Depending on what you find you can put
> this patch back, but without the comment and with a testcase where Fn
> is null.
>
>> Reed
>>
>>
> Cheers,
> Rafael
THere are several patches that need to be reverted to set this back and 
we will just break other code and you will impede work that I'm doing 
that needs this.

I will look into what is causing this.

Give me some time to figure out what is causing this.

Please don't revert this. You will just make  a lot of unnecessary work 
for me and the Mips team.

My original patch for attributes did not have this issue and I changed 
it to suite the reviewers.
I did things exactly how the x86 port did them and they avoid this issue 
by luck or else maybe they
already found this problem and worked around it.

There is code already identical to this in this same .cpp for other targets.

We are talking about a two line patch.

You are punishing me for being honest about the situation.

I don't believe that I should be getting called in this situation with 
those parameter values. Maybe I'm wrong. There is no documentation on 
any of this so how would I know. Other code also assumes the same. I've 
filed a bug against clang for this and duly noted the issue both in the 
code and in the putback notes.

There are no mips target attributes other than mips16/nomips16 and those 
are for functions.

This test case has no  Mips target specific attributes at all so why is 
this method even getting called?


Reed