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

Kaelyn Uhrain rikka at google.com
Thu Jun 9 18:18:11 PDT 2011


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*)?

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

Thanks,
Kaelyn
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20110609/92e5e3cd/attachment.html>


More information about the cfe-commits mailing list