On Nov 5, 2012, at 8:58 AM, Jordan Rose <[email protected]> wrote:

> 
> On Nov 2, 2012, at 20:33 , Jordan Rose <[email protected]> wrote:
> 
>> 
>> On Nov 2, 2012, at 18:40 , Anna Zaks <[email protected]> wrote:
>> 
>>> 
>>> On Nov 1, 2012, at 6:54 PM, Jordan Rose <[email protected]> wrote:
>>> 
>>>> Author: jrose
>>>> Date: Thu Nov  1 20:54:06 2012
>>>> New Revision: 167276
>>>> 
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=167276&view=rev
>>>> Log:
>>>> [analyzer] Use nice macros for the common ProgramStateTraits (map, set, 
>>>> list).
>>>> 
>>>> Also, move the REGISTER_*_WITH_PROGRAMSTATE macros to ProgramStateTrait.h.
>>>> 
>>> 
>>> I prefer the macros to live in the CheckerContext.h. I did see your comment 
>>> about the original commit.
>>> 
>>> As you pointed out, the negatives of putting the macros into the 
>>> CheckerContext is that the 2 internal users(maybe more in the future) will 
>>> not be able to use it.
>>> 
>>> However, on the positive side, someone writing a checker would have all the 
>>> main building blocks in one header file (no need to look in the obscure 
>>> ProgramStateTrait.h). 
>>> 
>>> Anyway, that's the reason I did not move it.
>> 
>> CheckerContext.h includes ExprEngine.h, which includes ProgramStateTrait.h, 
>> so they don't have to include a second header file. And I'd be fine with 
>> adding an explicit include to CheckerContext.h. But you're right that 
>> someone looking for the macros and not knowing the names ahead of time would 
>> have to look somewhere else.
>> 
>> Hm. Let me think about it for a bit.
> 
> I compromised in r167385. The base macro, REGISTER_TRAIT_WITH_PROGRAMSTATE, 
> still lives in ProgramStateTrait.h, but the ones for specific types are in 
> CheckerContext.h. That makes them easier to find in the common case, but Core 
> can still hide the template goop. If a checker writer ever needs to pick 
> their own type, I think they'll be able to jump from CheckerContext.h to 
> ProgramStateTrait.h
> 
> What do you think?
> 

Looks good. Thanks.

> Jordan
> 

_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to