| Message ID | 4F6A70C5.7000109@oracle.com |
|---|---|
| State | New |
| Headers | show |
On Wed, Mar 21, 2012 at 7:22 PM, Paolo Carlini <paolo.carlini@oracle.com> wrote: > Hi, > > this diagnostic issue is about not even trying to print expressions in error > messages involving operators, and print operand types instead. Just as an > example, for: > > struct X { int x; }; > void trigger (X x []) { x [01] = 0; } > > we currently print: > > error: no match for ‘operator=’ in ‘*(x + 4u) = 0’ > > which the patch changes to: > > error: no match for ‘operator=’ (operand types are ‘X’ and ‘int’) > > Or, for the existing other/error10.C, from: > > error: no match for ‘operator-’ in ‘-(* & a)’ > > to > > error: no match for ‘operator-’ (operand type is ‘A<0>’) > Usually these things appear in much less simpler expressions, possibly involving the same symbol but withe different meanings. There out be the a way to give an indication of which symbol the diagnostic is about. Withe the (imperfect) approach of printing expressions, at least some indication is given on the expression involved. Just printing the types with no indication of what expression is causing trouble is more head-scratching. > Jon and Manuel checked clang and I checked what icc does: without the caret, > I don't think we can do *much* better here, but, wrt the audit trail I think printing part or all of the expression is better in absence of carets. We can improve on the pretty printing of expressions, for example. > discussion, I'm proposing printing the actual operand types between > parentheses - I got the general idea from icc - because we don't want to > confuse parameters and arguments. > > Tested x86_64-linux. > > Thanks, > Paolo. > > ///////////////////////// > > > >
Hi, On 03/22/2012 05:25 AM, Gabriel Dos Reis wrote: > On Wed, Mar 21, 2012 at 7:22 PM, Paolo Carlini<paolo.carlini@oracle.com> wrote: >> Hi, >> >> this diagnostic issue is about not even trying to print expressions in error >> messages involving operators, and print operand types instead. Just as an >> example, for: >> >> struct X { int x; }; >> void trigger (X x []) { x [01] = 0; } >> >> we currently print: >> >> error: no match for ‘operator=’ in ‘*(x + 4u) = 0’ >> >> which the patch changes to: >> >> error: no match for ‘operator=’ (operand types are ‘X’ and ‘int’) >> >> Or, for the existing other/error10.C, from: >> >> error: no match for ‘operator-’ in ‘-(*& a)’ >> >> to >> >> error: no match for ‘operator-’ (operand type is ‘A<0>’) >> > Usually these things appear in much less simpler expressions, possibly involving > the same symbol but withe different meanings. There out be the a > way to give an indication of which symbol the diagnostic is about. > Withe the (imperfect) approach of printing expressions, at least some indication > is given on the expression involved. Just printing the types with no indication > of what expression is causing trouble is more head-scratching. Yes, you are right. Yesterday I had another look to the audit trail and thought, mistakenly, that people had a consensus about printing the types (by the way we are dealing also with much worse cases, like the one in Comment #0 there). Thus I don't think for the time being I can do much here, maybe the PR could be even closes as duplicate of the one about missing caret. Thanks, Paolo.
On Thu, 22 Mar 2012, Paolo Carlini wrote: > Hi, > > On 03/22/2012 05:25 AM, Gabriel Dos Reis wrote: >> On Wed, Mar 21, 2012 at 7:22 PM, Paolo Carlini<paolo.carlini@oracle.com> >> wrote: >>> Hi, >>> >>> this diagnostic issue is about not even trying to print expressions in >>> error >>> messages involving operators, and print operand types instead. Just as an >>> example, for: >>> >>> struct X { int x; }; >>> void trigger (X x []) { x [01] = 0; } >>> >>> we currently print: >>> >>> error: no match for ‘operator=’ in ‘*(x + 4u) = 0’ >>> >>> which the patch changes to: >>> >>> error: no match for ‘operator=’ (operand types are ‘X’ and ‘int’) >>> >>> Or, for the existing other/error10.C, from: >>> >>> error: no match for ‘operator-’ in ‘-(*& a)’ >>> >>> to >>> >>> error: no match for ‘operator-’ (operand type is ‘A<0>’) >>> >> Usually these things appear in much less simpler expressions, possibly >> involving >> the same symbol but withe different meanings. There out be the a >> way to give an indication of which symbol the diagnostic is about. >> Withe the (imperfect) approach of printing expressions, at least some >> indication >> is given on the expression involved. Just printing the types with no >> indication >> of what expression is causing trouble is more head-scratching. > Yes, you are right. Yesterday I had another look to the audit trail and > thought, mistakenly, that people had a consensus about printing the types (by > the way we are dealing also with much worse cases, like the one in Comment #0 > there). Thus I don't think for the time being I can do much here, maybe the > PR could be even closes as duplicate of the one about missing caret. I haven't followed the whole diagnostic discussion, but what about printing both the reconstructed expression and the types?
On Thu, Mar 22, 2012 at 11:13 AM, Marc Glisse <marc.glisse@inria.fr> wrote: > I haven't followed the whole diagnostic discussion, but what about printing > both the reconstructed expression and the types? Printing both isn't really the issue -- and we probably should. (And I thought we did in some cases.) What is at issue is that the tree we print is one that is of a too low-level form, sometimes a bit removed from what user wrote (e.g. containing too much information to be usefully processed by the user) therefore adding some confusion. There are several ways around this: (a) print the expression as written by the user (this usually means dumping verbatim the expression from the input source file); (b) reconstruct the expression from the tree but with a much higher level view (this may be possible if the C++ front-end preserves certain properties from the source code, but currently it does not.) People usually refers to (a) as diagnostics with caret, because some popular compilers dump the user input source file and use carets to delimit the boundaries of the expression under scrutiny. -- Gaby > > -- > Marc Glisse
.. hi all, hi Gaby, a couple of days ago, Manuel suggested in the audit trail of the main "caret diagnostics" PR, that now that we actually have got a form of it, the kind of change I proposed to resolve PR 49152 may make much more sense. In any case, my original patch still regtests fine today. What do you think? Thanks, Paolo.
On Sun, Apr 15, 2012 at 8:54 PM, Paolo Carlini <paolo.carlini@oracle.com> wrote: > .. hi all, hi Gaby, > > a couple of days ago, Manuel suggested in the audit trail of the main "caret > diagnostics" PR, that now that we actually have got a form of it, the kind > of change I proposed to resolve PR 49152 may make much more sense. In any > case, my original patch still regtests fine today. > > What do you think? a hybrid approach; I would suggest something like this: (a) if caret is in effect, then print the caret pointing to the symbol in question; otherwise (b) print the symbol and the type (as suggested by Marc). This is all best abstracted in a separate function.
On Sun, 15 Apr 2012, Gabriel Dos Reis wrote: > a hybrid approach; I would suggest something like this: (a) if caret > is in effect, then print > the caret pointing to the symbol in question; otherwise (b) print the > symbol and the type (as suggested by Marc). I may have forgotten the details, but looking at the beginning of the PR, don't we always want the types? Show where the failing operator== is, and print the types of the 2 arguments? I tried to parenthesize your sentence so it meant that, but it wasn't convincing...
On Mon, Apr 16, 2012 at 12:42 AM, Marc Glisse <marc.glisse@inria.fr> wrote: > On Sun, 15 Apr 2012, Gabriel Dos Reis wrote: > >> a hybrid approach; I would suggest something like this: (a) if caret >> is in effect, then print >> the caret pointing to the symbol in question; otherwise (b) print the >> symbol and the type (as suggested by Marc). > > > I may have forgotten the details, but looking at the beginning of the PR, > don't we always want the types? Yes. > Show where the failing operator== is, and > print the types of the 2 arguments? I tried to parenthesize your sentence so > it meant that, but it wasn't convincing... > > -- > Marc Glisse
Index: testsuite/g++.dg/ext/label5.C =================================================================== --- testsuite/g++.dg/ext/label5.C (revision 185603) +++ testsuite/g++.dg/ext/label5.C (working copy) @@ -2,5 +2,5 @@ // PR c++/24052 struct A { }; -int main() { b: A() && && b; } // { dg-error "A\\(\\) && && *b" } +int main() { b: A() && && b; } // { dg-error "operand types are 'A' and 'void\\*'" } // { dg-message "candidate|operator&&|no known conversion" "additional" { target *-*-* } 5 } Index: testsuite/g++.dg/ext/va-arg1.C =================================================================== --- testsuite/g++.dg/ext/va-arg1.C (revision 185603) +++ testsuite/g++.dg/ext/va-arg1.C (working copy) @@ -4,5 +4,5 @@ struct A {}; void foo() { - ++__builtin_va_arg(0, A); // { dg-error "'\\+\\+va_arg\\(0, A\\)'" } + ++__builtin_va_arg(0, A); // { dg-error "operand type is 'A'" } } Index: testsuite/g++.dg/other/error20.C =================================================================== --- testsuite/g++.dg/other/error20.C (revision 185603) +++ testsuite/g++.dg/other/error20.C (working copy) @@ -8,6 +8,6 @@ struct A // { dg-message "operator=|no known con void bar (A& a) { - a.foo () = 0; // { dg-error "A::foo\\(\\) = 0" } + a.foo () = 0; // { dg-error "operand types are 'A' and 'int'" } // { dg-message "candidate" "candidate note" { target *-*-* } 11 } } Index: testsuite/g++.dg/other/error16.C =================================================================== --- testsuite/g++.dg/other/error16.C (revision 185603) +++ testsuite/g++.dg/other/error16.C (working copy) @@ -10,5 +10,5 @@ typedef Outer<X> XOuter; int main() { Outer<int> ab; - ab.foo() == 1; // { dg-error "ab.Outer" } + ab.foo() == 1; // { dg-error "operand types are 'Outer<int>::Inner' and 'int'" } } Index: testsuite/g++.dg/other/error10.C =================================================================== --- testsuite/g++.dg/other/error10.C (revision 185603) +++ testsuite/g++.dg/other/error10.C (working copy) @@ -6,10 +6,9 @@ template<int> struct A {}; template<int N> void foo(const A<N> &a) -{ -A<N>(a); } // { dg-error "\\(\\* & a\\)" "" } +{ -A<N>(a); } // { dg-error "operand type is 'A<0>'" } void bar() { foo(A<0>()); // { dg-message "required from here" "" } } - Index: testsuite/g++.dg/diagnostic/operator1.C =================================================================== --- testsuite/g++.dg/diagnostic/operator1.C (revision 0) +++ testsuite/g++.dg/diagnostic/operator1.C (revision 0) @@ -0,0 +1,4 @@ +// PR c++/49152 + +struct X { int x; }; +void trigger (X x []) { x [01] = 0; } // { dg-error "operand types are 'X' and 'int'" } Index: testsuite/g++.dg/parse/error30.C =================================================================== --- testsuite/g++.dg/parse/error30.C (revision 185603) +++ testsuite/g++.dg/parse/error30.C (working copy) @@ -8,5 +8,5 @@ struct A A(int); }; -A a = -A(); // { dg-error "10:no match for.*operator-.*in.*-A\\(\\)" } -A b = -A(5); // { dg-error "11:no match for.*operator-.*in.*-A\\(5\\)" } +A a = -A(); // { dg-error "operand type is 'A'" } +A b = -A(5); // { dg-error "operand type is 'A'" } Index: testsuite/g++.dg/cpp0x/lambda/lambda-err1.C =================================================================== --- testsuite/g++.dg/cpp0x/lambda/lambda-err1.C (revision 185603) +++ testsuite/g++.dg/cpp0x/lambda/lambda-err1.C (working copy) @@ -4,5 +4,5 @@ void foo() { int x[1]; - [x]{} = 0; // { dg-error "lambda closure" } + [x]{} = 0; // { dg-error "lambda" } } Index: cp/call.c =================================================================== --- cp/call.c (revision 185603) +++ cp/call.c (working copy) @@ -4155,6 +4155,10 @@ op_error (enum tree_code code, enum tree_code code { const char *opname; + arg1 = arg1 ? TREE_TYPE (arg1) : NULL_TREE; + arg2 = arg2 ? TREE_TYPE (arg2) : NULL_TREE; + arg3 = arg3 ? TREE_TYPE (arg3) : NULL_TREE; + if (code == MODIFY_EXPR) opname = assignment_operator_name_info[code2].name; else @@ -4165,56 +4169,60 @@ op_error (enum tree_code code, enum tree_code code case COND_EXPR: if (match) error ("ambiguous overload for ternary %<operator?:%> " - "in %<%E ? %E : %E%>", arg1, arg2, arg3); + "(operand types are %qT, %qT, and %qT)", + arg1, arg2, arg3); else error ("no match for ternary %<operator?:%> " - "in %<%E ? %E : %E%>", arg1, arg2, arg3); + "(operand types are %qT, %qT, and %qT)", + arg1, arg2, arg3); break; case POSTINCREMENT_EXPR: case POSTDECREMENT_EXPR: if (match) - error ("ambiguous overload for %<operator%s%> in %<%E%s%>", - opname, arg1, opname); + error ("ambiguous overload for %<operator%s%> " + "(operand type is %qT)", opname, arg1); else - error ("no match for %<operator%s%> in %<%E%s%>", - opname, arg1, opname); + error ("no match for %<operator%s%> " + "(operand type is %qT)", opname, arg1); break; case ARRAY_REF: if (match) - error ("ambiguous overload for %<operator[]%> in %<%E[%E]%>", - arg1, arg2); + error ("ambiguous overload for %<operator[]%> " + "(operand types are %qT and %qT)", arg1, arg2); else - error ("no match for %<operator[]%> in %<%E[%E]%>", - arg1, arg2); + error ("no match for %<operator[]%> " + "(operand types are %qT and %qT)", arg1, arg2); break; case REALPART_EXPR: case IMAGPART_EXPR: if (match) - error ("ambiguous overload for %qs in %<%s %E%>", - opname, opname, arg1); + error ("ambiguous overload for %qs (operand type is %qT)", + opname, arg1); else - error ("no match for %qs in %<%s %E%>", - opname, opname, arg1); + error ("no match for %qs (operand type is %qT)", + opname, arg1); break; default: if (arg2) if (match) - error ("ambiguous overload for %<operator%s%> in %<%E %s %E%>", - opname, arg1, opname, arg2); + error ("ambiguous overload for %<operator%s%> " + "(operand types are %qT and %qT)", + opname, arg1, arg2); else - error ("no match for %<operator%s%> in %<%E %s %E%>", - opname, arg1, opname, arg2); + error ("no match for %<operator%s%> " + "(operand types are %qT and %qT)", + opname, arg1, arg2); else if (match) - error ("ambiguous overload for %<operator%s%> in %<%s%E%>", - opname, opname, arg1); + error ("ambiguous overload for %<operator%s%> " + "(operand type is %qT)", opname, arg1); else - error ("no match for %<operator%s%> in %<%s%E%>", - opname, opname, arg1); + error ("no match for %<operator%s%> (operand type is %qT)", + opname, arg1); break; } }
Hi, this diagnostic issue is about not even trying to print expressions in error messages involving operators, and print operand types instead. Just as an example, for: struct X { int x; }; void trigger (X x []) { x [01] = 0; } we currently print: error: no match for ‘operator=’ in ‘*(x + 4u) = 0’ which the patch changes to: error: no match for ‘operator=’ (operand types are ‘X’ and ‘int’) Or, for the existing other/error10.C, from: error: no match for ‘operator-’ in ‘-(* & a)’ to error: no match for ‘operator-’ (operand type is ‘A<0>’) Jon and Manuel checked clang and I checked what icc does: without the caret, I don't think we can do *much* better here, but, wrt the audit trail discussion, I'm proposing printing the actual operand types between parentheses - I got the general idea from icc - because we don't want to confuse parameters and arguments. Tested x86_64-linux. Thanks, Paolo. ///////////////////////// /cp 2012-03-21 Paolo Carlini <paolo.carlini@oracle.com> PR c++/49152 * call.c (op_error): Don't try to print expressions, print types. /testsuite 2012-03-21 Paolo Carlini <paolo.carlini@oracle.com> PR c++/49152 * g++.dg/diagnostic/operator1.C: New. * g++.dg/ext/label5.C: Adjust. * g++.dg/ext/va-arg1.C: Likewise. * g++.dg/other/error20.C: Likewise. * g++.dg/other/error20.C: Likewise. * g++.dg/other/error16.C: Likewise. * g++.dg/other/error10.C: Likewise. * g++.dg/parse/error30.C: Likewise. * g++.dg/cpp0x/lambda/lambda-err1.C: Likewise.