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

[Kaelyn Uhrain]

On Fri, Jun 10, 2011 at 9:18 AM, Douglas Gregor <dgregor at apple.com> wrote:

>
> On Jun 9, 2011, at 6:18 PM, Kaelyn Uhrain wrote:
>
>
>
> On Wed, Jun 1, 2011 at 9:56 AM, Douglas Gregor <dgregor at apple.com> wrote:
>
>>
>> +    // Only perform the qualified lookups for C++
>> +    // FIXME: this breaks
>> test/CXX/basic/basic.lookup/basic.lookup.argdep/p4.cpp
>> +    if (getLangOptions().CPlusPlus) {
>> +      TmpRes.suppressDiagnostics();
>> +      for (llvm::SmallPtrSet<IdentifierInfo*,
>> +                             16>::iterator QRI =
>> QualifiedResults.begin(),
>> +                                        QRIEnd = QualifiedResults.end();
>> +           QRI != QRIEnd; ++QRI) {
>> +        for (llvm::SmallPtrSet<NamespaceDecl*,
>> +                               16>::iterator KNI =
>> KnownNamespaces.begin(),
>> +                                          KNIEnd = KnownNamespaces.end();
>> +             KNI != KNIEnd; ++KNI) {
>> +          NameSpecifierAndSize NSS;
>> +          DeclContext *Ctx = dyn_cast<DeclContext>(*KNI);
>> +          TmpRes.clear();
>> +          TmpRes.setLookupName(*QRI);
>> +          if (!LookupQualifiedName(TmpRes, Ctx)) continue;
>> +
>> +          switch (TmpRes.getResultKind()) {
>> +          case LookupResult::Found:
>> +          case LookupResult::FoundOverloaded:
>> +          case LookupResult::FoundUnresolvedValue:
>> +            if (SpecifierMap.find(Ctx) != SpecifierMap.end()) {
>> +              NSS = SpecifierMap[Ctx];
>> +            } else {
>> +              NSS = BuildSpecifier(Context, CurContextChain, Ctx);
>> +              SpecifierMap[Ctx] = NSS;
>> +            }
>> +            Consumer.addName((*QRI)->getName(),
>> TmpRes.getAsSingle<NamedDecl>(),
>> +                             ED + NSS.second, NSS.first);
>> +            break;
>> +          case LookupResult::NotFound:
>> +          case LookupResult::NotFoundInCurrentInstantiation:
>> +          case LookupResult::Ambiguous:
>> +            break;
>> +          }
>> +        }
>>        }
>>      }
>>
>> I suggest always calling TmpRes.suppressDiagnostics() in the ambiguous
>> case, otherwise you may end up getting ambiguity warnings. I don't know if
>> that's the p4.cpp issue or not.
>>
>> I suspect that we'll want to order the KnownNamespaces traversal from
>> shortest NNS to longest NNS, so we can avoid performing name lookups (which
>> are very expensive in the PCH case) when the NNS length + edit distance are
>> worse than the best.
>>
>
> Quick question about doing this: would it be more cost effective to use a
> std::multimap to allow iterating through the namespaces in ascending order
> of NNS length and exiting the loop once NNS length + edit distance are worse
> than the best, or to use an llvm::SmallVector instead and always looping
> over all of the namespaces, but skipping the lookup if (NNS length + ED) is
> too long? Or is there a better way than either option buried in the
> LLVM/Clang codebase (keeping in mind that there are three pieces of info to
> be kept together in some fashion: a DeclContext*, its NestedNameSpecifer*,
> and the "edit distance" of the NestedNameSpecifer*)?
>
>
> It seems like a SmallVector indexed by NNS length would be best.
>

Indexing by NNS length might work (I keep thinking of the identifier edit
distances that span a much larger range of values than the NNS length in
most cases would)... just would have to a bit of funky nested SmallVector
handling to deal with multiple NNSes having the same length.


> Also, any thoughts on how to handle typo corrections in the presence of
> argument-dependent lookup (for solving the p4.cpp issue) would be
> appreciated...
>
>
> When ADL is involved, we shouldn't attempt any typo correction unless the
> call fails because there are no overload candidates. At that point (after
> overload resolution has failed), we could attempt typo correction and
> overload resolution again. I suggest tackling this separately.
>

Yeah, I was just thinking on the way to work this morning that for this
patch to just disable the namespace search when ADL is involved, then work
on making typo correction more type-aware later.


>
> - Doug
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20110610/f714cfcf/attachment.html>