[cfe-dev] [Patch] Refactor Sema::CppLookupName

[Douglas Gregor]

Hi Piotr,

On Feb 6, 2009, at 2:29 AM, Piotr Rak wrote:
> 2009/2/5 Douglas Gregor <dgregor at apple.com>:
>>
>> On Feb 4, 2009, at 7:44 PM, Piotr Rak wrote:
>>>
>>> Current approach (LookupResult::OverloadedDeclSingleDecl), still has
>>> one flaw, unfortunately consumers of LookupResult using new  
>>> interface
>>> (begin/end) to access overloads, need to check if we store
>>> OverloadedFunctionDecl or iterators, to free memory. I don't see any
>>> obvious solution for that now, I will keep that in mind.
>>
>> Okay. We can add a Destroy method that knows how to deallocate  
>> LookupResults
>> data structures. That would also be useful to deallocate base paths  
>> and
>> such.
>
> I'll do that, but we will have to be extra careful to avoid leaks  
> anyway.
> Could we just add destructor only for debug mode, something like:
>
> #ifndef NDEBUG
> LookupResult::~LookupResult() {
>  assert(<did_cleanup>);
> }
> #endif

Yes, we can do that. The trick is to keep LookupResult a POD, because  
everything related to name lookup is performance-critical.

>
>> +
>> +  // No name lookup results, return early.
>> +  if (I == End) return LResult::CreateLookupResult(Context, 0);
>> +
>>
>> We could also return early if there's only one lookup result, right?
>
> Indeed, I added this to my local copy (where I have basicly
> implemented [namespace.qual]).
> Unfortunately Sema::ActOnFunctionDeclarator() does pretty wired stuff,
> things like:
>
> namespace A {}
>
> void A::f() {}
>
> compiles silently with trunk, and few other funny things..., so I must
> first straighten this out. I think I already know how to fix it.

Yikes! This is probably just a misplaced/missing diagnostic. I'll look  
into it.

>> Interesting, this test uncovered an unfortunate issue. Since we  
>> added code
>> to report ambiguous type-name lookups in Sema::getTypeName, we  
>> actually get
>> *two* sets of ambiguity diagnostics regarding
>>
>>       X();
>>
>
> Do we want 'clang -verify' warn when such things happen? Now in such
> case tests will silently passes.

It would be helpful to teach -verify to do that, yes.

>
>> The first diagnostic comes when we check whether 'X' is a type name  
>> (using
>> Sema::getTypeName, to see if this is a C++ functional cast), and  
>> then later
>> we check whether we can build an expression for 'X' (using
>> Sema::ActOnIdentifierExpr), and both of those issue the diagnostic.  
>> Ideas
>> welcome!
>
> I will look into it.

Thanks!

Looking forward to qualified name lookup into namespaces with using  
directives :)

	- Doug