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

[Douglas Gregor]

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.

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

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