[cfe-commits] [patch] Lookup

[Douglas Gregor]

On Oct 7, 2009, at 5:42 PM, John McCall wrote:

> This patch refactors and hopefully simplifies and clarifies some of  
> the logic in name lookup, mostly for the benefit of C++.  The major  
> structural change is that a LookupResult is now a non-POD type which  
> is passed by reference to the lookup routines;  this allows us to  
> eliminate a lot of unnecessary copies from C++ lookups that reach  
> into multiple scopes.  No allocations or extra logic are required in  
> the common case, and along the common C paths, where only a single  
> declaration is found.

Excellent.

> In addition, the logic to replace declarations in scope chains is  
> now identical to that in declarationReplaces,

This is so much cleaner:

+  // If this replaces anything in the current scope,
+  IdentifierResolver::iterator I = IdResolver.begin(D->getDeclName()),
+                               IEnd = IdResolver.end();
+  for (; I != IEnd; ++I) {
+    if (S->isDeclScope(DeclPtrTy::make(*I)) && D->declarationReplaces 
(*I)) {
+      S->RemoveDecl(DeclPtrTy::make(*I));
+      IdResolver.RemoveDecl(*I);

than what we did previously. Nice cleanup!

> and we no longer rely on the ordering of declarations within a  
> single scope to implement C++ tag-name shadowing.

Ah, because this is centralized in Sema::LookupResult::resolveKind().

> In addition, users which assume that lookup will find at most one  
> declaration must now explicitly call either LookupSingleName or, if  
> working with a LookupResult, getAsSingleDecl().  These methods exist  
> to mark users making this assumption.  Users which for some reason  
> are certain that at most one declaration can be returned should use  
> getFoundDecl() instead.

... which means that we can easily audit our handling of ambiguous/ 
overloaded lookups by searching for these few names.

> My performance evaluations show a (very) small gain on the Cocoa.h  
> benchmark.

Thanks for checking this!

I hope to have more time to poke through the nitty-gritty details of  
SemaLookup.cpp later, but this looks great. Please commit!

	- Doug