[cfe-commits] r59502 - in /cfe/trunk: Driver/ include/clang/Analysis/ include/clang/Analysis/PathSensitive/ include/clang/Basic/ include/clang/Driver/ lib/Analysis/ lib/Basic/ lib/CodeGen/ lib/Driver/ lib/Lex/ lib/Parse/ lib/Sema/

[Douglas Gregor]

On Nov 20, 2008, at 12:27 AM, Chris Lattner wrote:

> On Nov 18, 2008, at 8:13 AM, Douglas Gregor wrote:
>>> +  DiagnosticInfo &operator<<(const SourceRange &R) {
>>> +    assert((unsigned)DiagObj->NumDiagArgs <
>>> +           sizeof(DiagObj->DiagRanges)/sizeof(DiagObj-
>>>> DiagRanges[0]) &&
>>> +           "Too many arguments to diagnostic!");
>>> +    DiagObj->DiagRanges[DiagObj->NumDiagRanges++] = &R;
>>> +    return *this;
>>> +  }
>>> +
>>> };
>>
>> These operator<<'s should be free functions, because we will  
>> certainly
>> have other operator<<'s for DiagnosticInfo elsewhere in the compiler
>> that must be free functions, e.g.,
>>
>> 	DiagnosticInfo& operator<<(DiagnosticInfo& DI, QualType Ty); // in
>> clang/AST/Type.h
>>
>> and we don't want non-member and member operator<<'s overloaded
>> because the available conversions on the first argument differ, and
>> that causes overloading weirdness.
>
> Yes, I totally agree.  Okay, I'm to the point where I want to do  
> this.  However, this is not my day.  Please take a look at this patch:
>
> <operators.patch>
>
> I just move the operators out of the class in a straightforward  
> way.  With this patch, I get build errors like these:
>
> Lexer.cpp: In function ‘char DecodeTrigraphChar(const char*,  
> clang::Lexer*)’:
> Lexer.cpp:352: error: no match for ‘operator<<’ in  
> ‘clang::Lexer::Diag(const char*, unsigned int) const((CP - 2u), 9u)  
> << std::operator+(const std::basic_string<_CharT, _Traits, _Alloc>&,  
> _CharT) [with _Char
> T = char, _Traits = std::char_traits<char>, _Alloc =  
> std::allocator<char>](((int)Res))’
> /Users/sabre/llvm/tools/clang/lib/Lex/../../include/clang/Basic/ 
> Diagnostic.h:366: note: candidates are: clang::DiagnosticInfo&  
> clang::operator<<(clang::DiagnosticInfo&, const std::string&)
> /Users/sabre/llvm/tools/clang/lib/Lex/../../include/clang/Basic/ 
> Diagnostic.h:371: note:                 clang::DiagnosticInfo&  
> clang::operator<<(clang::DiagnosticInfo&, const char*)
> /Users/sabre/llvm/tools/clang/lib/Lex/../../include/clang/Basic/ 
> Diagnostic.h:376: note:                 clang::DiagnosticInfo&  
> clang::operator<<(clang::DiagnosticInfo&, int)
> /Users/sabre/llvm/tools/clang/lib/Lex/../../include/clang/Basic/ 
> Diagnostic.h:381: note:                 clang::DiagnosticInfo&  
> clang::operator<<(clang::DiagnosticInfo&, unsigned int)
> /Users/sabre/llvm/tools/clang/lib/Lex/../../include/clang/Basic/ 
> Diagnostic.h:386: note:                 clang::DiagnosticInfo&  
> clang::operator<<(clang::DiagnosticInfo&, const  
> clang::IdentifierInfo*)
> /Users/sabre/llvm/tools/clang/lib/Lex/../../include/clang/Basic/ 
> Diagnostic.h:392: note:                 clang::DiagnosticInfo&  
> clang::operator<<(clang::DiagnosticInfo&, const clang::SourceRange&)
>
> This is the source line:
> Lexer *L; char Res;
> ...
>   L->Diag(CP-2, diag::trigraph_converted) << std::string()+Res;

The basic problem is that Diag() is returning a temporary, but a non- 
const (lvalue) reference can't bind to a temporary. I suggest making  
the first parameter to these operator<<'s either a const  
DiagnosticInfo& or a DiagnosticInfo. (The former will require a bunch  
of other member functions to be const, but so what?)

> This is a good point where we can step back and look at the amazing  
> awfulness of this diagnostic.  It's 'pretty' printing out the  
> expression in question, in a horribly mangled form.  Then it dumps  
> out a candidate list, but manages to not tell me the types that it  
> actually *has* on the LHS/RHS of the <<.

We should use this kind of thing as a test case for our diagnostics,  
to see how clear we can make them.

	- Doug
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20081120/d5292dc1/attachment.html>