Re: Diagnostic Messaging Suggestion

Re: Diagnostic Messaging Suggestion

[Arthur Schwarz]

I forgot to say 'thanks James', thanks.

Well, spurred on by the whimsy that I need a solution to the problem (however dolorous), I experimented. I've commented most everything at least once and the net effect is that only the 'operator<<' gets a nasty message. I've checked the include files that I've written and they all seem clean of heresies. The 'operator>>' files are unaffected, and with them gone, I still get an error on the 'operator<<' function.

The only thing that I can think of, and I think it remote, is that the functions refer to a public structure internal to a class. The 'operator>>' functions refer to their respective classes. Again, I've removed all of my friends but one 'operator<<' and this gets the error.

Now I think I know C++ (although, to be honest, I have strong, personal, doubts) and I can't think what I missed.

However, the initial statement still holds, the second diagnostic messages adds no clarity and seems redundant. Further, if there was a cause of conflict with a redefinition, it would be useful to include the original conflicting declaration. Perhaps something like:

<><line no>: error: redefinition of function at <><line no> illegal.
             note: istream& operator>> (bool& val );
                   istream& operator>> (short& val );
                        o o o

the note: stuff is from cplusplus.com @ 
     http://cplusplus.com/reference/iostream/istream/operator%3E%3E/

So, a concrete suggestion at to messaging and an individual first, a puzzler. I really need help on the puzzler.

thanks
art

Re: Diagnostic Messaging Suggestion

[Daniel Jacobowitz]

On Fri, Apr 17, 2009 at 11:58:48AM -0600, Tom Tromey wrote:
> Chris> Clang just prints the include stack information when anything
> Chris> in the include stack differs between two consecutive
> Chris> diagnostics.
> 
> We could easily do that too.

FWIW, I think this would be quite useful.

-- 
Daniel Jacobowitz
CodeSourcery

Re: Diagnostic Messaging Suggestion

[Tom Tromey]

>>>>> "Joe" == Joe Buck <Joe.Buck@synopsys.COM> writes:

Joe> If, for definitions, the compiler keeps track of this detail, it
Joe> would be possible to reliably print
Joe> foo.h:11 error: redefinition of `a' (file was included more than once)
Joe> if the printable line number is the same but the internal line number
Joe> is different.

You could certainly implement that in today's GCC.

The "internal line number" comparison can be done by directly
comparing source_locations.

Then "printable line number" comparison can be done by expanding the
locations and comparing the contents.

Chris> Clang just prints the include stack information when anything
Chris> in the include stack differs between two consecutive
Chris> diagnostics.

We could easily do that too.

Tom

Re: Diagnostic Messaging Suggestion

[Chris Lattner]

On Apr 16, 2009, at 8:44 PM, Joe Buck wrote:

> On Thu, Apr 16, 2009 at 03:40:47PM -0700, Arthur Schwarz wrote:
>> The rock has dropped. The answer is quoted below:
>>
>> "My best guess is that a header file is included twice, and lacks  
>> guards, hence the message is correct: the function is being defined  
>> twice, from the same source location."
>
> Yes, I've had to diagnose this one before; it doesn't happen with my
> own code because I use include guards, but I've had to help others.
>
> It would be cool if there were a way of distinguishing the case where
> the same header is being processed twice.
>
> We might see
>
> foo.h:11 error: redefinition of `a'
> foo.h:11 error: `a' previously defined here
>
> but the first "foo.h:11" might represent the 2300'th line read by the
> compiler, while the second "foo.h:11" might represent the 2194'th  
> line.
> If, for definitions, the compiler keeps track of this detail, it
> would be possible to reliably print
>
> foo.h:11 error: redefinition of `a' (file was included more than once)

Clang just prints the include stack information when anything in the  
include stack differs between two consecutive diagnostics.

$ cat t.h
int x = 4;
$ cat t.c

#include "t.h"
#include "t.h"
$ clang t.c
In file included from t.c:3:
./t.h:1:5: error: redefinition of 'x'
int x = 4;
     ^
In file included from t.c:2:
./t.h:1:5: note: previous definition is here
int x = 4;
     ^

This clearly says that the first definition was coming from t.c:2 and  
the second from t.c:3.  Of course, if you get multiple diagnostics  
from the same header, you only show the include stack for the first.

-Chris

Re: Diagnostic Messaging Suggestion

[Joe Buck]

On Thu, Apr 16, 2009 at 03:40:47PM -0700, Arthur Schwarz wrote:
> The rock has dropped. The answer is quoted below:
> 
> "My best guess is that a header file is included twice, and lacks guards, hence the message is correct: the function is being defined twice, from the same source location."

Yes, I've had to diagnose this one before; it doesn't happen with my
own code because I use include guards, but I've had to help others.

It would be cool if there were a way of distinguishing the case where
the same header is being processed twice.

We might see

foo.h:11 error: redefinition of `a'
foo.h:11 error: `a' previously defined here

but the first "foo.h:11" might represent the 2300'th line read by the
compiler, while the second "foo.h:11" might represent the 2194'th line.
If, for definitions, the compiler keeps track of this detail, it
would be possible to reliably print

foo.h:11 error: redefinition of `a' (file was included more than once)

if the printable line number is the same but the internal line number
is different.

Re: Diagnostic Messaging Suggestion

[Arthur Schwarz]

Thanks to everyone. 

The rock has dropped. The answer is quoted below:

"My best guess is that a header file is included twice, and lacks guards, hence the message is correct: the function is being defined twice, from the same source location."

I had put my friends following my 'include guard'. As we all know, when your guard is down you can get sucker-punched.

Although you should never try to teach an old dog new tricks, with luck and a good tail wind this will never happen again.

thanks
art

PS: You are now all my best friend. Sorry.

Re: Diagnostic Messaging Suggestion

[Arthur Schwarz]

> 
> Can you show code that reproduces the issue?  My best
> guess is that a
> header file is included twice, and lacks guards, hence the
> message is
> correct: the function is being defined twice, from the same
> source
> location.
> 
> -- James
> 

---- Code (unadulterated and full of original lacerations) -----

/* 
 * File:   irange.h
 * Author: Arthur Schwarz
 *
 * Created on April 13, 2009, 1:23 PM
 */

#ifndef _IRANGE_H
#define	_IRANGE_H
# include <ios>
# include <iomanip>
# include <istream>
# include <ostream>
# include <fstream>
# include <sstream>
# include "common.h"

class iRange_List : public cDebug{               // input ranges
private:
    // Processing of range buffer: <Ndx, Size, BufferSize, Ranges, TempStream>
    //  Ranges[Ndx] : Ndx<0..BufferSize-1>
    //  if (Ndx == BufferSize) TempStream.write(Ranges[Ndx]; Size += BufferSize; Ndx = 0;
    fstream     TempStream;                      // Temporary Stream
    static
    long const  BufferSize;                      // Input buffer size
    long        Ndx;                             // Index in Ranges buffer
    long        Size;                            // Total number ranges used
    cRange      ErrorRange;
protected:
    cRange*     Ranges;                          // All input ranges
    long        ErrorCount;                      // Number of errors found
protected:
//    double Atod(string& Range, long id);
    virtual
    void DData () { string str(80, ' ');
                    ostringstream stream(str);
                    stream << "iRange_List<"
                           << setw(9) << Ndx  << ", "
                           << setw(9) << Size << ", "
                           << setw(9) << ErrorCount << ">" << endl;
                    cDebug::DData(str);
    };
    long Next() { if ( ++Ndx < BufferSize) return Ndx;
                  TempStream.write((char*)Ranges, sizeof(Ranges));
                  Size += BufferSize;
                  return (Ndx = 0);
                };
    void Flush();
public:
    iRange_List();
    iRange_List(const iRange_List& orig);
    virtual ~iRange_List();

    long GetSize()  { return Size; }
    void PrintRange() { for(long i = 0; i < Ndx; i++) Stdout << setw(9) << i << ": "
                                                              << Ranges[i]
                                                              << endl;
    }; // PrintRange()
    virtual
    bool Read() = 0;
    cRange& operator[](long Ndx) { return ((Ndx >=0 ) && (Ndx < Size))? Ranges[Ndx]: ErrorRange; }
}; // class iRange_List

class ciRange_2 : public iRange_List {           // <low, high>
public:
    struct sRange_2 { double RLo;                // Low  range value
                      double RHi; };             // High range value

    ciRange_2() : iRange_List() { }
    virtual
    bool Read();
    friend istream& operator>>(istream& stream, ciRange_2::sRange_2& Range);
    friend ostream& operator<<(ostream& stream, ciRange_2::sRange_2& Range);
}; // class ciRange_2

class ciRange_3 : public iRange_List {           // <low, high, id>
public:
    struct sRange_3 { double  RLo;               // Low  range value
                      double  RHi;               // High range value
                      long    Datum; };          // User supplied datum
    ciRange_3() : iRange_List() { }
    virtual
    bool Read();
    friend istream& operator>>(istream& stream, sRange_3& Range);
    friend ostream& operator<<(ostream& stream, ciRange_3::sRange_3& Range);
}; // class ciRange_3

#endif	/* _IRANGE_H */


    //----------------------------------------
    //            Debug Streams
    //----------------------------------------

istream& operator>>(istream& stream, ciRange_2::sRange_2& Range);
istream& operator>>(istream& stream, ciRange_3::sRange_3& Range);

ostream& operator<<(ostream& stream, ciRange_2::sRange_2& Range) {
   stream << setw(9) << Range.RLo << " " << setw(9) << Range.RHi;
}

ostream& operator<<(ostream& stream, ciRange_3::sRange_3& Range) {
   stream << setw(9) << Range.RLo << " " << setw(9) << Range.RHi << " " << setw(9) << Range.Datum;
}

Re: Diagnostic Messaging Suggestion

[James Dennett]

On Thu, Apr 16, 2009 at 12:06 PM, Arthur Schwarz
<aschwarz1309@verizon.net> wrote:
>
>
>
> Suggested Messaging: Messaging seems redundant in indicating that function has been redifined twice. One of the messages should be removed. More to the point, I think the messaging may be erroneous - code fragment follows.
>
>
> g++-4 Diagnostic Messaging
>
> In file included from partition.cpp:66:
> irange.h: In function 'std::ostream& operator<<(std::ostream&, ciRange_2::sRange_2&)':
> irange.h:102: error: redefinition of 'std::ostream& operator<<(std::ostream&, ciRange_2::sRange_2&)'
> irange.h:102: error: 'std::ostream& operator<<(std::ostream&, ciRange_2::sRange_2&)' previously defined here
>
>
> ---- Code Fragment ----
> class ciRange_2 : public iRange_List {           // <low, high>
> public:
>    struct sRange_2 { double RLo;                // Low  range value
>                      double RHi; };             // High range value
>
>    friend istream& operator>>(istream& stream, ciRange_2::sRange_2& Range);
>    friend ostream& operator<<(ostream& stream, ciRange_2::sRange_2& Range);
> }; // class ciRange_2
>
> istream& operator>>(istream& stream, ciRange_2::sRange_2& Range);
>
> ostream& operator<<(ostream& stream, ciRange_2::sRange_2& Range) {
>   stream << setw(9) << Range.RLo << " " << setw(9) << Range.RHi;
> }

Can you show code that reproduces the issue?  My best guess is that a
header file is included twice, and lacks guards, hence the message is
correct: the function is being defined twice, from the same source
location.

-- James

Diagnostic Messaging Suggestion

[Arthur Schwarz]

Suggested Messaging: Messaging seems redundant in indicating that function has been redifined twice. One of the messages should be removed. More to the point, I think the messaging may be erroneous - code fragment follows.


g++-4 Diagnostic Messaging

In file included from partition.cpp:66:
irange.h: In function 'std::ostream& operator<<(std::ostream&, ciRange_2::sRange_2&)':
irange.h:102: error: redefinition of 'std::ostream& operator<<(std::ostream&, ciRange_2::sRange_2&)'
irange.h:102: error: 'std::ostream& operator<<(std::ostream&, ciRange_2::sRange_2&)' previously defined here


---- Code Fragment ----
class ciRange_2 : public iRange_List {           // <low, high>
public:
    struct sRange_2 { double RLo;                // Low  range value
                      double RHi; };             // High range value

    friend istream& operator>>(istream& stream, ciRange_2::sRange_2& Range);
    friend ostream& operator<<(ostream& stream, ciRange_2::sRange_2& Range);
}; // class ciRange_2

istream& operator>>(istream& stream, ciRange_2::sRange_2& Range);

ostream& operator<<(ostream& stream, ciRange_2::sRange_2& Range) {
   stream << setw(9) << Range.RLo << " " << setw(9) << Range.RHi;
}

Re: Diagnostic Messaging Suggestion

[Dave Korn]

Arthur Schwarz wrote:

> 
> (I've already provided an indication of a willingness to do something to
> Robert Dewar).

  Does his wife know? :-O

    cheers,
      DaveK

(I'll get my coat)

Re: Diagnostic Messaging Suggestion

[Ian Lance Taylor]

Arthur Schwarz <aschwarz1309@verizon.net> writes:

> I didn't mean to volunteer. I'm retired and therefor am both
> unhireable and hove no free time. But if you really (really) want
> ... I'd be glad to contribute in any way that I can. Provide what you
> need that I can do, and a means to give you feedback and I'm yours.

I think what we need most is somebody to collect a set of examples in
which g++ generates bad warning messages, suggest a different set of
warning messages which g++ could generate instead, and build some
consensus that the different set of warnings are indeed better in a
range of different scenarios.

Ian

Re: Diagnostic Messaging Suggestion

[Arthur Schwarz]

I didn't mean to volunteer. I'm retired and therefor am both unhireable and hove no free time. But if you really (really) want ... I'd be glad to contribute in any way that I can. Provide what you need that I can do, and a means to give you feedback and I'm yours.

(I've already provided an indication of a willingness to do something to Robert Dewar).

art

Re: Diagnostic Messaging Suggestion

[Joe Buck]

On Thu, Apr 09, 2009 at 11:03:20AM -0700, Ian Lance Taylor wrote:
> Arthur Schwarz <aschwarz1309@verizon.net> writes:
> > 2: Stream errors are tortuous to decode to find the cause of
> >    error..Any thought to simplification?
> >
> >    In particular: 'stream << something_wrong;' can produce a
> >    multiline message with copious use of the templates used -
> >    but no simple statement as what the error is.
> 
> I completely agree that the C++ diagnostics need to be improved.  gcc
> mainline recently took a step forward with simplification of template
> error messages.  However, much more can be done.
> 
> One problem we face is that compiler developers tend to be too close to
> the problem.  It helps for users to give examples and to give specific
> suggestions for how the diagnostics could be improved.

The template message simplifications help, but in cases where there
are five or more overloads for some function, the front end needs to
be smarter.  It's pointless to spew out the 20 different overloads,
and would be better to try to hone in on the real cause of the error.

stream << something_wrong is a good example.  Perhaps something_wrong
is of struct or class type, and there are no overloads in scope to
print that class, and no conversions that will coerce that class into
something we can print.  It's pointless to print all of the operator<<
functions the compiler knows about!  Saying that there's no match
suffices.

In other cases the compiler could try to rank the possible close matches,
putting those that would match except for a const attribute at the top
(since in my experience const issues, like passing a temporary to
a function that has a non-const reference is common), then if a
& or a * would make the argument match an overload, put those in next.
Just print the top two possibilities and say if there are more.

Re: Diagnostic Messaging Suggestion

[Ian Lance Taylor]

Arthur Schwarz <aschwarz1309@verizon.net> writes:

> 1: Any thought to including column numbers with line numbers on 
>    ERROR messages? Looking at a diagnostic message for a complex
>    statement  without a column number has lead to making an 
>    incorrect assumption as to what in the line is faulty. A 
>    column number would help to identify the fault.

This can already be done using -fshow-column.  However, the column
numbers in released versions of gcc are not very accurate.  There is
work under way to improve that:
http://gcc.gnu.org/wiki/Better_Diagnostics


> 2: Stream errors are tortuous to decode to find the cause of  
>    error..Any thought to simplification? 
>
>    In particular: 'stream << something_wrong;' can produce a 
>    multiline message with copious use of the templates used - 
>    but no simple statement as what the error is.
>
> 3: A simple error:
>    'ifstream istream;'
>    Leads to an diagnostic message which seems remote from
>    the problem.
>
> Most diagnostic messages contain some indication of what the failure 
> is and what to do to repair them. Some messages are outstanding. 
> But then again, some could probably use a little tweaking. Any thoughts?

I completely agree that the C++ diagnostics need to be improved.  gcc
mainline recently took a step forward with simplification of template
error messages.  However, much more can be done.

One problem we face is that compiler developers tend to be too close to
the problem.  It helps for users to give examples and to give specific
suggestions for how the diagnostics could be improved.

Another problem is that among users there is significant disagreement
over how warnings can be most useful.  Controlling the warning output by
options can help, but most users do not read the manual and will not be
aware of the options.  So choosing the best default behaviour is both
important and difficult, as reasonable people disagree.

Fortunately gcc is free software, and that means that you can help.
Even if you don't know how to change the code, if you can build a
consensus for what the best warning output should be, that would be an
enormous help in guiding how the warnings should be changed.

Thanks for your note.

Ian

Diagnostic Messaging Suggestion

[Arthur Schwarz]

Please take the comments as constructive and not adversarial. I use GCC
and have hopes of sending it a Valentine's Day gift.

1: Any thought to including column numbers with line numbers on 
   ERROR messages? Looking at a diagnostic message for a complex
   statement  without a column number has lead to making an 
   incorrect assumption as to what in the line is faulty. A 
   column number would help to identify the fault.

2: Stream errors are tortuous to decode to find the cause of  
   error..Any thought to simplification? 

   In particular: 'stream << something_wrong;' can produce a 
   multiline message with copious use of the templates used - 
   but no simple statement as what the error is.

3: A simple error:
   'ifstream istream;'
   Leads to an diagnostic message which seems remote from
   the problem.

Most diagnostic messages contain some indication of what the failure 
is and what to do to repair them. Some messages are outstanding. 
But then again, some could probably use a little tweaking. Any thoughts?

art