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

Kaelyn Uhrain rikka at google.com
Wed Jun 22 10:48:32 PDT 2011


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.

Cheers,
Kaelyn

On Mon, Jun 20, 2011 at 3:28 PM, Kaelyn Uhrain <rikka at google.com> wrote:

> Same day, yet another version of my patch. ;)
>
> I've eliminated the use of std::multimap and moved the building and sorting
> of the sets of NestedNameSpecifiers into a separate class.  I've also fixed
> up the typo correction cache to store TypoCorrections instead of just the
> StringRef name and a keyword boolean (I moved the declaration of the
> TypoCorrection class out of Sema/Lookup.h and into its own header so that
> Sema/Sema.h could use the class in an llvm::DenseMap). The only failing unit
> test is the new one I added which is dependent on more of the users of
> CorrectTypo being made aware of namespace corrections.
>
> Chandler, if you get to a point where you can easily do so, I'd be curious
> to know if your benchmarking shows any noticeable difference between these
> last two patches (ns-20110620.diff and ns-20110620-nomultimap.diff; I can
> easily provide an incremental diff between the two if you want/need it).
>
> Now to fix up those call sites!
>
> Cheers,
> Kaelyn
>
>
> On Mon, Jun 20, 2011 at 10:14 AM, Kaelyn Uhrain <rikka at google.com> wrote:
>
>> Managed to not reply to all... which is okay as the patch was against the
>> clang git tree as of a couple weeks ago and recent changes causes a couple
>> of minor merge conflicts.  Here's the patch based on the current git HEAD.
>>
>>
>> On Mon, Jun 20, 2011 at 9:54 AM, Kaelyn Uhrain <rikka at google.com> wrote:
>>
>>> Here ya go.  It has most of the smaller changes based on dgregor's most
>>> recent feedback.  It also makes CorrectTypo know when argument-dependent
>>> lookup is required and skips trying namespace qualifiers, which allows
>>> basic.lookup.argdep/p4.cpp to pass. The main pieces I'm still working on
>>> (within CorrectTypo at least) are improving the caching and updating the
>>> CXXScopeSpec if/when possible.  I also still need to remove the use of
>>> std::multimap, though you're welcome to benchmark it and alternatives. ;)
>>>
>>>
>>> On Mon, Jun 20, 2011 at 9:37 AM, Chandler Carruth <chandlerc at google.com>wrote:
>>>
>>>> On Mon, Jun 20, 2011 at 9:35 AM, Kaelyn Uhrain <rikka at google.com>wrote:
>>>>
>>>>> Don't quite have it ready yet.  Being sick for much of last week didn't
>>>>> help...
>>>>>
>>>>> I could post for submission my last patch + the smaller cleanups that I
>>>>> already have done and finish up the work as a second patch, so that folks
>>>>> can get started on the serialization, etc.
>>>>>
>>>>
>>>> I'm just looking for a WIP patch that I can run with.
>>>>
>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20110622/44a08255/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ns-pt1-20110622.diff
Type: text/x-diff
Size: 44618 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20110622/44a08255/attachment.diff>


More information about the cfe-commits mailing list