Re: [PATCH 2/3] allow builtins to have prototype and evaluate/expand methods

From: Chris Li <christ.li@gmail.com>
To: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
Cc: Linux-Sparse <linux-sparse@vger.kernel.org>,
	Johannes Berg <johannes@sipsolutions.net>
Subject: Re: [PATCH 2/3] allow builtins to have prototype and evaluate/expand methods
Date: Wed, 8 Feb 2017 04:26:47 +0800	[thread overview]
Message-ID: <CANeU7QnVqeOASzooDPuNocKNm3pzx_B6T2D=eA39vp=1MiGCEw@mail.gmail.com> (raw)
In-Reply-To: <20170207201253.4i2ye6inqwma65rf@macpro.local>

On Wed, Feb 8, 2017 at 4:12 AM, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
> On Wed, Feb 08, 2017 at 03:49:54AM +0800, Chris Li wrote:
>> On Tue, Jan 24, 2017 at 5:37 AM, Luc Van Oostenryck
>> <luc.vanoostenryck@gmail.com> wrote:
>> > So far, builtin functions which had some evaluate/expand method
>> > couldn't also have a prototype because each would have its own symbol
>> > and only the one for the prototype will be seen.
>> > This also meant that the evaluate/expand functions had to take care
>> > to set the correct types for they argumenst & results, which is fine
>> > for some generic builtins like __builtin_constant_p() it's much less
>> > practical for the ones like __builtin_bswap{16,32,64}().
>> >
>> > Fix this by marking the idents for the builtins we declare some
>> > evaluate/expand methods has being the ident of a builtin function
>> > and later at evaluation time, to share the methods between all the symbols
>> > corresponding to an identifier so marked.
>>
>> This certainly fix the problem for builtin functions. However, I think there
>> is more general bug in sparse not limit to builtin functions. When the symbol
>> is declare twice, the later one did not migrate all the declare information from
>> the previous one.
>
> I'm very well aware of this bug/problem, it creates all sort of complications
> but I vaguely understood it was a design choice. To be 100%, you're talking
> the fact that each declaration create a new symbol only related by their
> identifier chain, right?
>
>> In function external_declaration()
>>
>>         check_declaration(decl);
>>         if (decl->same_symbol)
>>                 decl->definition = decl->same_symbol->definition;
>>
>> Here it only migrates the definition. I think if you add symbol->op in
>> the migration
>> as well, it *should* work for your case. I haven't test this myself.
>> If that works for you, it is a more general fix. Want to give it a try?
>>
>> We might want to extract the migration code into a new function and
>> later adding new code it. e.g. function attributes.
>
> Yes and diagnose any compatibility.
> I'll give it a try but I won't be able to do that before the weekend.
>
> Luc