[cfe-commits] r153970 - in /cfe/trunk: include/clang/AST/DeclBase.h include/clang/AST/DeclLookups.h lib/Sema/SemaCodeComplete.cpp lib/Sema/SemaLookup.cpp test/SemaCXX/function-redecl.cpp test/SemaCXX/qualified-id-lookup.cpp test/SemaCXX/typo-correction.cpp

Douglas Gregor dgregor at apple.com
Tue Apr 3 20:25:59 PDT 2012


On Apr 3, 2012, at 2:44 PM, Nick Lewycky wrote:

> Author: nicholas
> Date: Tue Apr  3 16:44:08 2012
> New Revision: 153970
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=153970&view=rev
> Log:
> Remove more redundant lookups. Add a new "all_lookups_iterator" which provides
> a view over the contents of a DeclContext without exposing the implementation
> details of the StoredDeclsMap. Use this in LookupVisibleDecls to find the
> visible declarations. Fixes PR12339!
> 
> Added:
>    cfe/trunk/include/clang/AST/DeclLookups.h
> Modified:
>    cfe/trunk/include/clang/AST/DeclBase.h
>    cfe/trunk/lib/Sema/SemaCodeComplete.cpp
>    cfe/trunk/lib/Sema/SemaLookup.cpp
>    cfe/trunk/test/SemaCXX/function-redecl.cpp
>    cfe/trunk/test/SemaCXX/qualified-id-lookup.cpp
>    cfe/trunk/test/SemaCXX/typo-correction.cpp
> 
> Modified: cfe/trunk/include/clang/AST/DeclBase.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclBase.h?rev=153970&r1=153969&r2=153970&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/AST/DeclBase.h (original)
> +++ cfe/trunk/include/clang/AST/DeclBase.h Tue Apr  3 16:44:08 2012
> @@ -1438,6 +1438,14 @@
>   /// replaced with D.
>   void makeDeclVisibleInContext(NamedDecl *D);
> 
> +  /// all_lookups_iterator - An iterator that provides a view over the results
> +  /// of looking up every possible name.
> +  class all_lookups_iterator;
> +
> +  all_lookups_iterator lookups_begin() const;
> +
> +  all_lookups_iterator lookups_end() const;
> +
>   /// udir_iterator - Iterates through the using-directives stored
>   /// within this context.
>   typedef UsingDirectiveDecl * const * udir_iterator;
> 
> Added: cfe/trunk/include/clang/AST/DeclLookups.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclLookups.h?rev=153970&view=auto
> ==============================================================================
> --- cfe/trunk/include/clang/AST/DeclLookups.h (added)
> +++ cfe/trunk/include/clang/AST/DeclLookups.h Tue Apr  3 16:44:08 2012
> @@ -0,0 +1,84 @@
> +//===-- DeclLookups.h - Low-level interface to all names in a DC-*- C++ -*-===//
> +//
> +//                     The LLVM Compiler Infrastructure
> +//
> +// This file is distributed under the University of Illinois Open Source
> +// License. See LICENSE.TXT for details.
> +//
> +//===----------------------------------------------------------------------===//
> +//
> +//  This file defines DeclContext::all_lookups_iterator.
> +//
> +//===----------------------------------------------------------------------===//
> +
> +#ifndef LLVM_CLANG_AST_DECLLOOKUPS_H
> +#define LLVM_CLANG_AST_DECLLOOKUPS_H
> +
> +#include "clang/AST/DeclBase.h"
> +#include "clang/AST/DeclContextInternals.h"
> +#include "clang/AST/DeclarationName.h"
> +
> +namespace clang {
> +
> +/// all_lookups_iterator - An iterator that provides a view over the results
> +/// of looking up every possible name.
> +class DeclContext::all_lookups_iterator {
> +  StoredDeclsMap::iterator It, End;
> +public:
> +  typedef lookup_result             value_type;
> +  typedef lookup_result             reference;
> +  typedef lookup_result             pointer;
> +  typedef std::forward_iterator_tag iterator_category;
> +  typedef std::ptrdiff_t            difference_type;
> +
> +  all_lookups_iterator() {}
> +  all_lookups_iterator(StoredDeclsMap::iterator It,
> +                       StoredDeclsMap::iterator End)
> +      : It(It), End(End) {}
> +
> +  reference operator*() const { return It->second.getLookupResult(); }
> +  pointer operator->() const { return It->second.getLookupResult(); }
> +
> +  all_lookups_iterator& operator++() {
> +    // Filter out using directives. They don't belong as results from name
> +    // lookup anyways, except as an implementation detail. Users of the API
> +    // should not expect to get them (or worse, rely on it).
> +    do {
> +      ++It;
> +    } while (It != End &&
> +             It->first == DeclarationName::getUsingDirectiveName());
> +             
> +    return *this;
> +  }
> +
> +  all_lookups_iterator operator++(int) {
> +    all_lookups_iterator tmp(*this);
> +    ++(*this);
> +    return tmp;
> +  }
> +
> +  friend bool operator==(all_lookups_iterator x, all_lookups_iterator y) {
> +    return x.It == y.It;
> +  }
> +  friend bool operator!=(all_lookups_iterator x, all_lookups_iterator y) {
> +    return x.It != y.It;
> +  }
> +};
> +
> +DeclContext::all_lookups_iterator DeclContext::lookups_begin() const {
> +  DeclContext *Primary = const_cast<DeclContext*>(this)->getPrimaryContext();
> +  if (StoredDeclsMap *Map = Primary->buildLookup())
> +    return all_lookups_iterator(Map->begin(), Map->end());
> +  return all_lookups_iterator();
> +}
> +
> +DeclContext::all_lookups_iterator DeclContext::lookups_end() const {
> +  DeclContext *Primary = const_cast<DeclContext*>(this)->getPrimaryContext();
> +  if (StoredDeclsMap *Map = Primary->buildLookup())
> +    return all_lookups_iterator(Map->end(), Map->end());
> +  return all_lookups_iterator();
> +}

This isn't quite correct in the case where the DeclContext is backed by a precompiled header because, alas, buildLookup() doesn't always build the same lookup table that Sema would build. Here, if the DeclContext has external visible storage, we'll need to ask the external AST source to enumerate all of its contents.

	- Doug

> +} // end namespace clang
> +
> +#endif
> 
> Modified: cfe/trunk/lib/Sema/SemaCodeComplete.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCodeComplete.cpp?rev=153970&r1=153969&r2=153970&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaCodeComplete.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaCodeComplete.cpp Tue Apr  3 16:44:08 2012
> @@ -514,14 +514,6 @@
>         return false;
>     }
>   }
> -   
> -  // Skip out-of-line declarations and definitions.
> -  // NOTE: Unless it's an Objective-C property, method, or ivar, where
> -  // the contexts can be messy.
> -  if (!ND->getDeclContext()->Equals(ND->getLexicalDeclContext()) &&
> -      !(isa<ObjCPropertyDecl>(ND) || isa<ObjCIvarDecl>(ND) ||
> -        isa<ObjCMethodDecl>(ND)))
> -    return false;
> 
>   if (Filter == &ResultBuilder::IsNestedNameSpecifier ||
>       ((isa<NamespaceDecl>(ND) || isa<NamespaceAliasDecl>(ND)) &&
> @@ -904,7 +896,7 @@
> 
>   if (Hiding && CheckHiddenResult(R, CurContext, Hiding))
>     return;
> -      
> +
>   // Make sure that any given declaration only shows up in the result set once.
>   if (!AllDeclsFound.insert(R.Declaration->getCanonicalDecl()))
>     return;
> 
> Modified: cfe/trunk/lib/Sema/SemaLookup.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaLookup.cpp?rev=153970&r1=153969&r2=153970&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaLookup.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaLookup.cpp Tue Apr  3 16:44:08 2012
> @@ -25,6 +25,7 @@
> #include "clang/AST/CXXInheritance.h"
> #include "clang/AST/Decl.h"
> #include "clang/AST/DeclCXX.h"
> +#include "clang/AST/DeclLookups.h"
> #include "clang/AST/DeclObjC.h"
> #include "clang/AST/DeclTemplate.h"
> #include "clang/AST/Expr.h"
> @@ -2875,26 +2876,16 @@
>     Result.getSema().ForceDeclarationOfImplicitMembers(Class);
> 
>   // Enumerate all of the results in this context.
> -  llvm::SmallVector<DeclContext *, 2> Contexts;
> -  Ctx->collectAllContexts(Contexts);
> -  for (unsigned I = 0, N = Contexts.size(); I != N; ++I) {
> -    DeclContext *CurCtx = Contexts[I];
> -    for (DeclContext::decl_iterator D = CurCtx->decls_begin(),
> -                                 DEnd = CurCtx->decls_end();
> -         D != DEnd; ++D) {
> -      if (NamedDecl *ND = dyn_cast<NamedDecl>(*D)) {
> +  for (DeclContext::all_lookups_iterator L = Ctx->lookups_begin(),
> +                                      LEnd = Ctx->lookups_end();
> +       L != LEnd; ++L) {
> +    for (DeclContext::lookup_result R = *L; R.first != R.second; ++R.first) {
> +      if (NamedDecl *ND = dyn_cast<NamedDecl>(*R.first)) {
>         if ((ND = Result.getAcceptableDecl(ND))) {
>           Consumer.FoundDecl(ND, Visited.checkHidden(ND), Ctx, InBaseClass);
>           Visited.add(ND);
>         }
>       }
> -      
> -      // Visit transparent contexts and inline namespaces inside this context.
> -      if (DeclContext *InnerCtx = dyn_cast<DeclContext>(*D)) {
> -        if (InnerCtx->isTransparentContext() || InnerCtx->isInlineNamespace())
> -          LookupVisibleDecls(InnerCtx, Result, QualifiedNameLookup, InBaseClass,
> -                             Consumer, Visited);
> -      }
>     }
>   }
> 
> 
> Modified: cfe/trunk/test/SemaCXX/function-redecl.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/function-redecl.cpp?rev=153970&r1=153969&r2=153970&view=diff
> ==============================================================================
> --- cfe/trunk/test/SemaCXX/function-redecl.cpp (original)
> +++ cfe/trunk/test/SemaCXX/function-redecl.cpp Tue Apr  3 16:44:08 2012
> @@ -76,9 +76,12 @@
>   void GetCart(int count) const;
> };
> // This out-of-line definition was fine...
> -void Crash::cart(int count) const {} // expected-error {{out-of-line definition of 'cart' does not match any declaration in 'Crash'}}
> +void Crash::cart(int count) const {} // expected-error {{out-of-line definition of 'cart' does not match any declaration in 'Crash'}} \
> +                                     // expected-note {{'cart' declared here}} \
> +                                     // expected-note {{previous definition is here}}
> // ...while this one crashed clang
> -void Crash::chart(int count) const {} // expected-error {{out-of-line definition of 'chart' does not match any declaration in 'Crash'}}
> +void Crash::chart(int count) const {} // expected-error {{out-of-line definition of 'chart' does not match any declaration in 'Crash'; did you mean 'cart'?}} \
> +                                      // expected-error {{redefinition of 'cart'}}
> 
> class TestConst {
>  public:
> 
> Modified: cfe/trunk/test/SemaCXX/qualified-id-lookup.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/qualified-id-lookup.cpp?rev=153970&r1=153969&r2=153970&view=diff
> ==============================================================================
> --- cfe/trunk/test/SemaCXX/qualified-id-lookup.cpp (original)
> +++ cfe/trunk/test/SemaCXX/qualified-id-lookup.cpp Tue Apr  3 16:44:08 2012
> @@ -146,3 +146,8 @@
>     Z(foo::X()).Work();
>   }
> }
> +
> +namespace pr12339 {
> +  extern "C" void i;
> +  pr12339::FOO  // expected-error{{no type named 'FOO' in namespace 'pr12339'}}
> +}  // expected-error{{expected unqualified-id}}
> 
> Modified: cfe/trunk/test/SemaCXX/typo-correction.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/typo-correction.cpp?rev=153970&r1=153969&r2=153970&view=diff
> ==============================================================================
> --- cfe/trunk/test/SemaCXX/typo-correction.cpp (original)
> +++ cfe/trunk/test/SemaCXX/typo-correction.cpp Tue Apr  3 16:44:08 2012
> @@ -183,3 +183,10 @@
>         getExprAs<ConstructExpr>(); // expected-error{{use of undeclared identifier 'ConstructExpr'; did you mean 'clash::ConstructExpr'?}}
>   }
> };
> +
> +namespace test1 {
> +  struct S {
> +    struct Foobar *f;  // expected-note{{'Foobar' declared here}}
> +  };
> +  test1::FooBar *b;  // expected-error{{no type named 'FooBar' in namespace 'test1'; did you mean 'Foobar'?}}
> +}
> 
> 
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits




More information about the cfe-commits mailing list