Some high-level comments, and hopefully Doug can make more specific comments on some of these issues:<div><br></div><div>- I suspect performance is going to be a real problem here. Thinking more about this, I think the RAV is probably the wrong tool (sorry for leading you astray!) for the job. Specifically, this has the unfortunate behavior in the presence of a lazily deserialized PCH of forcibly deserializing a huge chunk of the AST, if not *all* of it. Here are my ideas of how to address this aspect:</div>
<div><br></div><div>1) switch to a recursive function -- because we're only ever traversing namespaces, the traversal logic itself is extremely simple (a for loop).</div><div>2) start from the nearest enclosing namespace and walk up, and then back down</div>
<div>3) keep track of the distance from the original namespace while walking (the penalty you're adding at the end to represent qualifiers) and only walk until the max edit distance has been reached. This should bound the number of namespaces we look at at least a small amount.</div>
<div><br></div><div>(if the above isn't sufficient)</div><div>4) do the typo correction iteratively. at each namespace (starting with the existing typo correction logic) compute the best correction and its edit distance. if less than the max edit distance, replace the max with the new edit distance, and keep that correction as a candidate, and walk up/down one more namespace iff the new max edit distance is >1. That should allow us to stop walking even sooner when a good candidate is found.</div>
<div><br></div><div><br></div><div>I haven't looked at the details of the implementation yet, but I'm mostly interested in seeing if we can get the performance threshold down to a more reasonable level.</div><div>
<br></div><div>-Chandler</div><div><br><div class="gmail_quote">On Tue, Feb 22, 2011 at 11:21 AM, Kaelyn Uhrain <span dir="ltr"><<a href="mailto:rikka@google.com">rikka@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
This patch implements checking available C++ namespaces when trying to find typo corrections for unknown identifiers.  There are currently 4 broken unit tests (CXX/basic/basic.lookup/basic.lookup.argdep/p4.cpp, CXX/class/class.local/p1.cpp, FixIt/typo.cpp, and SemaObjC/synth-provisional-ivars.m) which I'm fairly sure are cases where the tests need to be updated to reflect the new behavior instead of being indicative of actual code breakage, but I'm not 100% certain.  The new unit test, test/Sema/missing-namespace-qualifier-typo-corrections.cpp, is a good demonstration of the improved suggestions provided by this patch as most of the errors found by "clang -fsyntax-only" lack suggestions without this patch..<br>

<br>Cheers,<br><font color="#888888">Kaelyn<br>
</font><br>_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
<br></blockquote></div><br></div>