[obv] compile-print.exp: xfail->kfail for '@' GDB array operator

Message ID 20150604193820.GA18453@host1.jankratochvil.net
State New, archived
Headers

Commit Message

Jan Kratochvil June 4, 2015, 7:38 p.m. UTC
  Patch implementing '@' GDB array operator in GCC has been rejected:
	https://gcc.gnu.org/ml/gcc-patches/2015-06/msg00414.html
and so there is now a GDB tracker to implement it just in GDB:
	https://sourceware.org/bugzilla/show_bug.cgi?id=18489

gdb/testsuite/ChangeLog
2015-06-04  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.compile/compile-print.exp (compile print *vararray@3)
	(compile print *vararrayp@3): Change xfail to kfail compile/18489.
---
 gdb/testsuite/ChangeLog                     | 5 +++++
 gdb/testsuite/gdb.compile/compile-print.exp | 4 ++--
 2 files changed, 7 insertions(+), 2 deletions(-)
  

Comments

Yao Qi June 5, 2015, 9:34 a.m. UTC | #1
Jan Kratochvil <jan.kratochvil@redhat.com> writes:

Hi, Jan,
This patch isn't obvious to me... see my question below,

> Patch implementing '@' GDB array operator in GCC has been rejected:
> 	https://gcc.gnu.org/ml/gcc-patches/2015-06/msg00414.html
> and so there is now a GDB tracker to implement it just in GDB:
> 	https://sourceware.org/bugzilla/show_bug.cgi?id=18489

If '@' is rejected in GCC, why do we need to support it in "compile"
feature in GDB?  The manual of command "compile print" says:

  Compile and execute expr with the compiler language found as the
  current language in GDB

'@' isn't a valid operator for C language, we should emit error here.
IMO, "compile" feature should *only* accept valid source code according
to the language spec and the compiler.  It will be really confusing if
we add some other things (like '@' in this case) which is out of the
scope of the language.

> @@ -38,7 +38,7 @@ gdb_test_multiple $test $test {
>  	pass $test
>      }
>      -re ": error: stray '@' in program\r\n.*\r\n$gdb_prompt $" {
> -	xfail "$test (gcc does not support '@')"
> +	kfail compile/18489 "$test"
>      }
>  }

I think xfail is correct as gcc doesn't support '@'.
  
Jan Kratochvil June 5, 2015, 12:40 p.m. UTC | #2
On Fri, 05 Jun 2015 11:34:43 +0200, Yao Qi wrote:
> Jan Kratochvil <jan.kratochvil@redhat.com> writes:
> > Patch implementing '@' GDB array operator in GCC has been rejected:
> > 	https://gcc.gnu.org/ml/gcc-patches/2015-06/msg00414.html
> > and so there is now a GDB tracker to implement it just in GDB:
> > 	https://sourceware.org/bugzilla/show_bug.cgi?id=18489
> 
> If '@' is rejected in GCC, why do we need to support it in "compile"
> feature in GDB?  The manual of command "compile print" says:
> 
>   Compile and execute expr with the compiler language found as the
>   current language in GDB
> 
> '@' isn't a valid operator for C language, we should emit error here.

'compile print' should one day replace the standard 'print' command, like in
LLDB.  Otherwise 'compile print' makes no sense.

Now we can argue whether the '@' GDB operator is useful or not but I think
majority of GDB users considers it as useful.

Whether the manual wording is right or not is up to Eli.


> IMO, "compile" feature should *only* accept valid source code according
> to the language spec and the compiler.

GDB 'compile print' should ideally accept all the features of GDB 'print'
command.  There is an internal TODO list:
------------------------------------------------------------------------------
   * Add some gdb extensions to the GCC C parser.
     * "@" could be handled by having it create a new array
     * Convenience variables could be handled with perhaps some difficulty
       * Hack the parser to recognize them and call into gdb
       * Use copy-in/copy-out semantics as we do for registers
       * Use  the variable value's current type and recompile the expression when the  type changes (even trickier than it seems because the variable can be  reassigned from inside the code)
     * Convenience  functions are much harder.  Maybe they could be done by annotating them  with type information and then arranging for call-backs to gdb.  It's  likely that this isn't worth the effort; perhaps better would be to  expose interesting and relevant gdb state to the compiled expressions  somehow
     * "::" could be handled by hacking the parser
     * gdb's extended "." could also probably be handled
     * Not sure if the two {...} extensions are doable or not -- would need research to see if they introduce ambiguous parses.  These are:
       * you can use {...} to make an array:
                        (gdb) print {1,2,3,4}
                        (gdb) ptype {1,2,3,4}
                        type = int [4]
       * the cast-like '{int} foo'
------------------------------------------------------------------------------

These GDB operators (currently only '@') probably should not be supported by
'compile code' / 'compile file'.


> It will be really confusing if we add some other things (like '@' in this
> case) which is out of the scope of the language.

So why were these operators added to the GDB expression evaluator in the first
place?


> > -	xfail "$test (gcc does not support '@')"
> > +	kfail compile/18489 "$test"
> 
> I think xfail is correct as gcc doesn't support '@'.

This does not match the original plan of the 'compile' project.


Jan
  
Jan Kratochvil June 5, 2015, 1:24 p.m. UTC | #3
On Fri, 05 Jun 2015 11:34:43 +0200, Yao Qi wrote:
> If '@' is rejected in GCC,

BTW '@' was implemented in the GCC patch only under condition of being run by
GDB/libcc1.


> '@' isn't a valid operator for C language,

So even if the GCC patch was accepted '@' still would not be a valid operator
for C language; that is correct.


Jan
  
Doug Evans June 5, 2015, 3:19 p.m. UTC | #4
On Fri, Jun 5, 2015 at 5:40 AM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
> 'compile print' should one day replace the standard 'print' command, like in
> LLDB.  Otherwise 'compile print' makes no sense.

Just so we're on the same page,
the plan isn't to remove the "standard 'print' command", right?
[The print (output, etc.) commands would either use the builtin
support or compile print.]
  
Jan Kratochvil June 5, 2015, 3:26 p.m. UTC | #5
On Fri, 05 Jun 2015 17:19:05 +0200, Doug Evans wrote:
> the plan isn't to remove the "standard 'print' command", right?
> [The print (output, etc.) commands would either use the builtin
> support or compile print.]

Sure GCC will probably never become a hard requirement for GDB.

I think there could be:
  set compile print never = current state
  set compile print always = never use internal GDB evaluation
  set compile print preferred = first try GCC then GDB evaluation
   - IMO to be the default (as long as GCC is available)
  set compile print deferred = first try GDB then GCC evaluation

But the setting above has never been implemented/posted/discussed.


Jan
  
Yao Qi June 5, 2015, 4:13 p.m. UTC | #6
Jan Kratochvil <jan.kratochvil@redhat.com> writes:

> 'compile print' should one day replace the standard 'print' command, like in
> LLDB.  Otherwise 'compile print' makes no sense.
>
> Now we can argue whether the '@' GDB operator is useful or not but I think
> majority of GDB users considers it as useful.

'@' is useful in command print, and it is reasonable for me to replace
command print with 'compile print' one day, I agree on them.  If gcc
doesn't accept '@', 'compile print' shouldn't as well, but 'print' can.

Command 'print' can be built on top of the 'compile print' stuff, with
extra functionality to transform '@' to a valid language form.  Is it
possible?

Anyway, I suggest we consider support '@' when we really start
considering replace 'print' command with 'compile print' command.  Once
we support '@' in 'compile print', we can't remove it.  I am inclined to
postpone the decision to some time we really need it.

>> It will be really confusing if we add some other things (like '@' in this
>> case) which is out of the scope of the language.
>
> So why were these operators added to the GDB expression evaluator in the first
> place?
>

I don't know, but GDB expression (which supports '@') is not a language,
so it is flexible to add and remove operators.  Commands 'compile XXX'
are different, because their input is source language, and IMO we
shouldn't add any extensions to the source language.

>
>> > -	xfail "$test (gcc does not support '@')"
>> > +	kfail compile/18489 "$test"
>> 
>> I think xfail is correct as gcc doesn't support '@'.
>
> This does not match the original plan of the 'compile' project.

Where is the original plan?
https://sourceware.org/gdb/wiki/GCCCompileAndExecute is the only thing I
can find, but I don't see the anything about replacing command 'print' with
'compile print'.
  
Jan Kratochvil June 5, 2015, 9:23 p.m. UTC | #7
Hi Yao,

I have replied to all your arguments but I think it is just enough to say:
	'compile print' is a work in progress to become future 'print'.


On Fri, 05 Jun 2015 18:13:07 +0200, Yao Qi wrote:
> '@' is useful in command print, and it is reasonable for me to replace
> command print with 'compile print' one day, I agree on them.  If gcc
> doesn't accept '@', 'compile print' shouldn't as well, but 'print' can.

That depends what 'compile print' should do.  IMO it is "future 'print'".
So I do not understand why you say:
	'compile print' shouldn't [...], but 'print' can.


> Command 'print' can be built on top of the 'compile print' stuff,

'print' should be replaced by 'compile print'.  'compile print' is a work in
progress towards that goal.

If you want we can create 'compile print2' which will be "future 'print'"
while 'compile print' can remain ISO C (or whatever language GCC is configure
to accept) clean.  But then I do not see what 'compile print' would be good
for, 'compile print2' would supersede it in all aspects.

Besides that there is currently no kind of 'print' command unsupporting GDB
extensions so I do not understand why you want 'compile print' to have one.


> with extra functionality to transform '@' to a valid language form.  Is it
> possible?

Everything is possible but I do not see usefulness of multiple commands where
one is a subset of functionality of other.  The general goal should be
reducing the number of commands/options of GDB, not the opposite.


> Anyway, I suggest we consider support '@' when we really start
> considering replace 'print' command with 'compile print' command.  Once
> we support '@' in 'compile print', we can't remove it.  I am inclined to
> postpone the decision to some time we really need it.

'compile print' should be already useful in some cases (such as evaluating
macros containing statement expressions) although I admit I haven't yet had an
opportunity to utilize it (primarily because there is no inferior C++
'compile' support yet).

I was proposing to implement the discussed command
	set compile print {never|always|preferred|deferred}
as it makes sense already; but I was assigned to other priorities so far.


> I don't know, but GDB expression (which supports '@') is not a language,
> so it is flexible to add and remove operators.  Commands 'compile XXX'
> are different, because their input is source language, and IMO we
> shouldn't add any extensions to the source language.

I do not understand why you find the goals of 'compile print' and 'print' to
be different.


> Where is the original plan?
> https://sourceware.org/gdb/wiki/GCCCompileAndExecute is the only thing I
> can find, but I don't see the anything about replacing command 'print' with
> 'compile print'.

The plan is to support all the features of LLDB which uses clang/llvm for its
'print' command.  Although it may not be a great argument as LLDB does not
support the '@' operator at all.


Jan
  

Patch

diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 5ee74eb..246d6a8 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,8 @@ 
+2015-06-04  Jan Kratochvil  <jan.kratochvil@redhat.com>
+
+	* gdb.compile/compile-print.exp (compile print *vararray@3)
+	(compile print *vararrayp@3): Change xfail to kfail compile/18489.
+
 2015-06-03  Jan Kratochvil  <jan.kratochvil@redhat.com>
 
 	* gdb.compile/compile.exp (keep jit in memory): Rename to ...
diff --git a/gdb/testsuite/gdb.compile/compile-print.exp b/gdb/testsuite/gdb.compile/compile-print.exp
index 92c6240..de2f0e7 100644
--- a/gdb/testsuite/gdb.compile/compile-print.exp
+++ b/gdb/testsuite/gdb.compile/compile-print.exp
@@ -38,7 +38,7 @@  gdb_test_multiple $test $test {
 	pass $test
     }
     -re ": error: stray '@' in program\r\n.*\r\n$gdb_prompt $" {
-	xfail "$test (gcc does not support '@')"
+	kfail compile/18489 "$test"
     }
 }
 
@@ -48,7 +48,7 @@  gdb_test_multiple $test $test {
 	pass $test
     }
     -re ": error: stray '@' in program\r\n.*\r\n$gdb_prompt $" {
-	xfail "$test (gcc does not support '@')"
+	kfail compile/18489 "$test"
     }
 }