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

[Douglas Gregor]

On Jun 22, 2011, at 1:47 PM, Kaelyn Uhrain wrote:

> On Wed, Jun 22, 2011 at 1:23 PM, Douglas Gregor <dgregor at apple.com> wrote:
> 
> On Jun 22, 2011, at 10:48 AM, Kaelyn Uhrain wrote:
> 
> > So in the interest of keeping the overall changes a bit more digestible, I've split my dev branch in two. The first part, for which I've attached the latest diff, contains just the changes for the improved version of Sema::CorrectTypo and does not include the new unit test or any changes to any of the callers of CorrectTypo (all calls are going through the compatibility wrapper, and all unit tests pass). The second part, which I'm still working on (and which is already a third of the combined patch size), already has many of the callers modified to use the new version of CorrectTypo so I'd be fine with getting the first part submitted. Splitting it apart should also make the changes a bit easier to bisect if something breaks down the road.
> 
> Thanks for breaking this up. However, I'd like to have at least one test in the initial patch, so that we know that things are starting to work. Then follow-on commits/patches can switch over call sites to get the improved typo correction everywhere.
> 
> Well the catch is that until at least one of the call sites are switched to the new version of CorrectTypo, the code would at best give an incorrect/incomplete suggestion--it would suggest using an identifier from another namespace without the proper qualifiers to make the identifier visible, so the suggestion would (in the fixit case) end up tripping another warning.  If you'd like I could fix up one of the CorrectTypo callers and add stripped-down version of missing-namespace-qualifier-typo-corrections.cpp that only tests that one instance.

Sorry, I meant to say that explicitly: I'd like one call site to switch along with the initial patch, so we see some testable change in behavior from the commit. Later patches/commits could add more call sites and tests, as appropriate. Thanks!

	- Doug

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20110622/9163e10d/attachment.html>