[cfe-commits] r63939 - in /cfe/trunk: include/clang/Basic/DiagnosticSemaKinds.def lib/Sema/SemaDecl.cpp test/SemaCXX/nested-name-spec.cpp test/SemaCXX/qualified-id-lookup.cpp

[Douglas Gregor]

Hi Piotr,

On Feb 6, 2009, at 12:43 PM, Piotr Rak wrote:
> I have two minor comments.

Okay, thanks.

> 2009/2/6 Douglas Gregor <dgregor at apple.com>:
>> Author: dgregor
>> Date: Fri Feb  6 11:46:57 2009
>> New Revision: 63939
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=63939&view=rev
>> Log:
>> Diagnose attempts to define a namespace member out-of-line when no
>> matching member exists. Thanks to Piotr Rak for reporting the  
>> problem!
>>
>> Modified:
>>   cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.def
>>   cfe/trunk/lib/Sema/SemaDecl.cpp
>>   cfe/trunk/test/SemaCXX/nested-name-spec.cpp
>>   cfe/trunk/test/SemaCXX/qualified-id-lookup.cpp
>> [snip]
>> +    // Complain about this problem, and attempt to suggest close
>> +    // matches (e.g., those that differ only in cv-qualifiers and
>> +    // whether the parameter types are references).
>> +    DeclarationName CtxName;
>> +    if (DC->isRecord())
>> +      CtxName = cast<RecordDecl>(DC)->getDeclName();
>> +    else if (DC->isNamespace())
>> +      CtxName = cast<NamespaceDecl>(DC)->getDeclName();
>> +    // FIXME: global scope
>
> This FIXME is not needed, since D.getCXXScopeSpec().isSet() so is
> qualified name lookup and things like:
>
>  void ::f() {}
>
> are already diagnosed. We will never get TranslationUnitDecl here,  
> right?

Right. Thanks!

>> +    Diag(D.getIdentifierLoc(), diag::err_member_def_does_not_match)
>> +      << CtxName << D.getCXXScopeSpec().getRange();
>> +    InvalidDecl = true;
>> +
>> +    LookupResult Prev = LookupQualifiedName(DC, Name,  
>> LookupOrdinaryName,
>> +                                            true);
>> +    assert(!Prev.isAmbiguous() &&
>> +           "Cannot have an ambiguity in previous-declaration  
>> lookup");
>> +    for (LookupResult::iterator Func = Prev.begin(), FuncEnd =  
>> Prev.end();
>> +         Func != FuncEnd; ++Func) {
>> +      if (isa<FunctionDecl>(*Func) &&
>> +          isNearlyMatchingFunction(Context, cast<FunctionDecl> 
>> (*Func), NewFD))
>> +        Diag((*Func)->getLocation(),  
>> diag::note_member_def_close_match);
>>    }
>
> This does not display any possible function declarations for case  
> like:
>
> namespace A {
>  int f();
>  int f(char);
> }
>
> void A::f(double) {
> }
>
> Is that what we want here? Maybe we should handle case (!Prev)
> separatly form (!Redeclaration), and use err_ovl_candidate instead?

The intent was to keep the set of declarations we print relatively  
small, since the length of C++ error messages is a cause for concern  
when overload sets are involved. How about this: if we don't find any  
"nearly-matching" functions, then we print the full list of functions  
(possibly filtering out any functions that have already been defined).

	- Doug