<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>On Nov 5, 2012, at 8:58 AM, Jordan Rose <<a href="mailto:jordan_rose@apple.com">jordan_rose@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><meta http-equiv="Content-Type" content="text/html charset=us-ascii"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>On Nov 2, 2012, at 20:33 , Jordan Rose <<a href="mailto:jordan_rose@apple.com">jordan_rose@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><meta http-equiv="Content-Type" content="text/html charset=us-ascii"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>On Nov 2, 2012, at 18:40 , Anna Zaks <<a href="mailto:ganna@apple.com">ganna@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><br>On Nov 1, 2012, at 6:54 PM, Jordan Rose <<a href="mailto:jordan_rose@apple.com">jordan_rose@apple.com</a>> wrote:<br><br><blockquote type="cite">Author: jrose<br>Date: Thu Nov 1 20:54:06 2012<br>New Revision: 167276<br><br>URL: <a href="http://llvm.org/viewvc/llvm-project?rev=167276&view=rev">http://llvm.org/viewvc/llvm-project?rev=167276&view=rev</a><br>Log:<br>[analyzer] Use nice macros for the common ProgramStateTraits (map, set, list).<br><br>Also, move the REGISTER_*_WITH_PROGRAMSTATE macros to ProgramStateTrait.h.<br><br></blockquote><br>I prefer the macros to live in the CheckerContext.h. I did see your comment about the original commit.<br><br>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.<br><br>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). <br><br>Anyway, that's the reason I did not move it.</blockquote><br></div><div>CheckerContext.h includes ExprEngine.h, which includes ProgramStateTrait.h, so they don't have to <i>include</i> a second header file. And I'd be fine with adding an explicit include to CheckerContext.h. But you're right that someone <i>looking</i> for the macros and not knowing the names ahead of time would have to look somewhere else.</div><div><br></div><div>Hm. Let me think about it for a bit.</div></div></blockquote><br></div><div>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</div><div><br></div><div>What do you think?</div><div><br></div></div></blockquote><div><br></div>Looks good. Thanks.<br><br><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div>Jordan</div><br></div></blockquote></div><br></body></html>