Re: Diagnostic Messaging Suggestion

2009-04-17 Thread 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

2009-04-17 Thread Tom Tromey
 Joe == Joe Buck joe.b...@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

2009-04-17 Thread 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

2009-04-16 Thread James Dennett
On Thu, Apr 16, 2009 at 12:06 PM, Arthur Schwarz
aschwarz1...@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


Re: Diagnostic Messaging Suggestion

2009-04-16 Thread 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] : Ndx0..BufferSize-1
//  if (Ndx == BufferSize) TempStream.write(Ranges[Ndx]; Size += 
BufferSize; Ndx = 0;
fstream TempStream;  // Temporary Stream
static
long const  BufferSize;  // Input buffer size
longNdx; // Index in Ranges buffer
longSize;// Total number ranges used
cRange  ErrorRange;
protected:
cRange* Ranges;  // All input ranges
longErrorCount;  // 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)  ErrorCountendl;
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
  longDatum; };  // 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

2009-04-16 Thread 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

2009-04-16 Thread 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

2009-04-16 Thread 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

2009-04-09 Thread Ian Lance Taylor
Arthur Schwarz aschwarz1...@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


Re: Diagnostic Messaging Suggestion

2009-04-09 Thread Joe Buck
On Thu, Apr 09, 2009 at 11:03:20AM -0700, Ian Lance Taylor wrote:
 Arthur Schwarz aschwarz1...@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

2009-04-09 Thread 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

2009-04-09 Thread Ian Lance Taylor
Arthur Schwarz aschwarz1...@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

2009-04-09 Thread 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)