[cfe-commits] PATCH: Add support for C++ namespace-aware typo correcting

[Chandler Carruth]

Some high-level comments, and hopefully Doug can make more specific comments
on some of these issues:

- 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:

1) switch to a recursive function -- because we're only ever traversing
namespaces, the traversal logic itself is extremely simple (a for loop).
2) start from the nearest enclosing namespace and walk up, and then back
down
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.

(if the above isn't sufficient)
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.


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.

-Chandler

On Tue, Feb 22, 2011 at 11:21 AM, Kaelyn Uhrain <rikka at google.com> wrote:

> 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..
>
> Cheers,
> Kaelyn
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20110225/360613a5/attachment.html>