[cfe-commits] r167276 - in /cfe/trunk: include/clang/StaticAnalyzer/Core/PathSensitive/ lib/StaticAnalyzer/Checkers/ lib/StaticAnalyzer/Core/

[Anna Zaks]

On Nov 5, 2012, at 8:58 AM, Jordan Rose <jordan_rose at apple.com> wrote:

> 
> On Nov 2, 2012, at 20:33 , Jordan Rose <jordan_rose at apple.com> wrote:
> 
>> 
>> On Nov 2, 2012, at 18:40 , Anna Zaks <ganna at apple.com> wrote:
>> 
>>> 
>>> On Nov 1, 2012, at 6:54 PM, Jordan Rose <jordan_rose at apple.com> 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
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20121105/3bc25784/attachment.html>