<html><head><meta http-equiv="Content-Type" content="text/html charset=iso-8859-1"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>On Jan 2, 2013, at 2:21 , Manuel Klimek <<a href="mailto:klimek@google.com">klimek@google.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="font-family: arial, helvetica, sans-serif; font-size: 10pt"><div dir="ltr"><div class="gmail_default" style="">On Wed, Dec 12, 2012 at 11:25 AM, Manuel Klimek <span dir="ltr"><<a href="mailto:klimek@google.com" target="_blank">klimek@google.com</a>></span> wrote:<br>
</div><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex; position: static; z-index: auto; "><div style="font-family:arial,helvetica,sans-serif;font-size:10pt">
<div dir="ltr"><div class="im">On Wed, Dec 12, 2012 at 2:19 AM, Jordan Rose <span dir="ltr"><<a href="mailto:jordan_rose@apple.com" target="_blank">jordan_rose@apple.com</a>></span> wrote:<br>
</div><div class="gmail_extra"><div class="gmail_quote"><div class="im"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div>(+Olaf Krzikalla, a known external user of the classic ParentMap)</div>
<div><br></div><div>The analyzer seems to be the primary user of the "classic" ParentMap; it's also scattered over the rewriters, but I'm not sure it's ever actually used there. All of the uses of our ParentMap seem like they could easily be implemented on top of ASTMatcher's DenseMap implementation, though I'm a little concerned about (a) the ability to <i>update</i> parents, which Olaf uses, and (b) the careful handling of PseudoObjectExprs and OpaqueValueExprs used for proper source locations in the analyzer.</div>
<div><br></div><div>I also don't like the idea that building a ParentMap traverses the <i>entire</i> AST. That includes everything pulled in in every header file, when usually the analyzer only needs to deal with a single function at a time. I'm not sure what to say about this, though; making ParentMaps specific to Decls seems problematic in a different way: you can't match across <i>everything</i> as easily. And if you allow both per-function and global parent maps, you're doing redundant work.</div>
<div><br></div><div>Hm. I'm not sure what I think yet, but I'm leaning towards using your lazily constructed global map on ASTContext and yet still keeping the per-function maps on AnalysisDeclContext. Unifying the two types is fine, though.</div>
</div></blockquote><div><br></div></div><div>One thing I'm concerned about is how performance critical the parent map for the analyzer is - we'll need to expand ours to work for at least all types of nodes that have pointer identity (including TypeLoc and NestedNameSpecifierLoc, as those are crucial for refactorings), while the analyzer (currently) seems to only need statements.</div>
<div><br></div></div></div></div></div></blockquote><div style="">Ping :) Or should I just come up with a patch so we have something concrete to look at?</div></div></div></div></div></blockquote><br></div><div>Whoops, sorry I missed this. Unfortunately, ParentMap is indeed in the analyzer's hot path, which is probably already not a great thing. I'm not sure how much making it more generic will cost, though. The way we use it, every statement's parent should be another statement or NULL; that would change to being another statement, a CXXInitializer, or the enclosing function Decl. Slightly more expensive to check the "expression is consumed" during the analyzer run; we're okay with taking the hit during the diagnostics emission for finding good path note locations.</div><div><br></div><div><br></div><div><blockquote type="cite"><div style="font-family: arial, helvetica, sans-serif; font-size: 10pt; "><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex; position: static; z-index: auto; "><div style="font-size: 10pt; "><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">Before trying to come up with a patch I'd be interested whether you expect the analyzer to need parents for different node types anyway, and whether you think the additional performance hit of building a more generic parent map would be prohibitive.</div></div></div></div></blockquote></div></div></div></div></blockquote></div><div><br></div><div>I can think of uses outside of just statement->statement mappings, but obviously we've gotten this far with only that.</div><div><br></div><div>I'm not so worried about the cost of <i>building</i> the map; the analyzer run tends to dwarf any linear-time operations anyway. I just don't want to build it for the entire translation unit, since we won't use most of it and we're already caching the per-function ones. I don't think that's a design issue, though.</div><div><br></div><div>So, okay, let's see a patch. :-)</div><div>Jordan</div><div><br></div></body></html>