[3/3] Let Python breakpoints be created silently

Message ID 20221208191804.3819129-4-tromey@adacore.com
State New
Headers
Series Add "announce" flag to Python breakpoint creation |

Commit Message

Tom Tromey Dec. 8, 2022, 7:18 p.m. UTC
  Currently, a breakpoint created from Python will always announce its
presence; and in some cases (for example a pending breakpoint), other
information will be printed as well.

When scripting gdb, it's useful to be able to control the output in
cases like this.  I debated whether to simply disable the output
entirely, but I thought perhaps some existing code acts as a simple
"break"-like command and wants the output.

This patch adds a new "announce" flag to gdb.Breakpoint.  Setting this
to False will cause gdb to be silent here.
---
 gdb/doc/python.texi                        | 10 ++++++--
 gdb/python/py-breakpoint.c                 | 30 +++++++++++++++++-----
 gdb/testsuite/gdb.python/py-breakpoint.exp |  3 ++-
 3 files changed, 34 insertions(+), 9 deletions(-)
  

Comments

Tom Tromey Jan. 11, 2023, 5:31 p.m. UTC | #1
>>>>> "Tom" == Tom Tromey via Gdb-patches <gdb-patches@sourceware.org> writes:

Tom> Currently, a breakpoint created from Python will always announce its
Tom> presence; and in some cases (for example a pending breakpoint), other
Tom> information will be printed as well.

Tom> When scripting gdb, it's useful to be able to control the output in
Tom> cases like this.  I debated whether to simply disable the output
Tom> entirely, but I thought perhaps some existing code acts as a simple
Tom> "break"-like command and wants the output.

Tom> This patch adds a new "announce" flag to gdb.Breakpoint.  Setting this
Tom> to False will cause gdb to be silent here.

I think this one still needs a documentation review.

Tom
  
Eli Zaretskii Jan. 12, 2023, 9:42 a.m. UTC | #2
> From: Tom Tromey <tromey@adacore.com>
> Cc: Tom Tromey <tromey@adacore.com>, Eli Zaretskii <eliz@gnu.org>
> Date: Wed, 11 Jan 2023 10:31:33 -0700
> 
> >>>>> "Tom" == Tom Tromey via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
> Tom> Currently, a breakpoint created from Python will always announce its
> Tom> presence; and in some cases (for example a pending breakpoint), other
> Tom> information will be printed as well.
> 
> Tom> When scripting gdb, it's useful to be able to control the output in
> Tom> cases like this.  I debated whether to simply disable the output
> Tom> entirely, but I thought perhaps some existing code acts as a simple
> Tom> "break"-like command and wants the output.
> 
> Tom> This patch adds a new "announce" flag to gdb.Breakpoint.  Setting this
> Tom> to False will cause gdb to be silent here.
> 
> I think this one still needs a documentation review.

Sorry for missing it.

The patch is okay, with a single gotcha:

> +The optional @var{announce} argument is a boolean that controls
> +whether @var{GDBN} announces the existence of the breakpoint.  The
           ^^^^
That should be @value, not @var.
  
Simon Marchi Jan. 12, 2023, 5:49 p.m. UTC | #3
> @@ -5940,9 +5940,15 @@ the function passed in @code{spec} as a fully-qualified name.  It is equivalent
>  to @code{break}'s @code{-qualified} flag (@pxref{Linespec Locations} and
>  @ref{Explicit Locations}).
>  
> +The optional @var{announce} argument is a boolean that controls
> +whether @var{GDBN} announces the existence of the breakpoint.  The
> +default is to announce, meaning that a message is printed.  Setting
> +this argument to false will suppress all output from breakpoint
> +creation.

I assume this means "all CLI output", but any MI / DAP frontend would
still be notified?

Otherwise, the code LGTM.

Simon
  
Pedro Alves Jan. 13, 2023, 12:59 p.m. UTC | #4
On 2022-12-08 7:18 p.m., Tom Tromey via Gdb-patches wrote:
> Currently, a breakpoint created from Python will always announce its
> presence; and in some cases (for example a pending breakpoint), other
> information will be printed as well.
> 
> When scripting gdb, it's useful to be able to control the output in
> cases like this.  I debated whether to simply disable the output
> entirely, but I thought perhaps some existing code acts as a simple
> "break"-like command and wants the output.
> 
> This patch adds a new "announce" flag to gdb.Breakpoint.  Setting this
> to False will cause gdb to be silent here.

Wouldn't making the breakpoint "internal" work for the same effect?
  
Tom Tromey Jan. 13, 2023, 6:55 p.m. UTC | #5
>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:

>> @@ -5940,9 +5940,15 @@ the function passed in @code{spec} as a fully-qualified name.  It is equivalent
>> to @code{break}'s @code{-qualified} flag (@pxref{Linespec Locations} and
>> @ref{Explicit Locations}).
>> 
>> +The optional @var{announce} argument is a boolean that controls
>> +whether @var{GDBN} announces the existence of the breakpoint.  The
>> +default is to announce, meaning that a message is printed.  Setting
>> +this argument to false will suppress all output from breakpoint
>> +creation.

Simon> I assume this means "all CLI output", but any MI / DAP frontend would
Simon> still be notified?

Yes, I think that happens through an observer.

Tom
  
Tom Tromey Jan. 13, 2023, 6:56 p.m. UTC | #6
>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:

Pedro> Wouldn't making the breakpoint "internal" work for the same effect?

No:

(gdb) python gdb.Breakpoint("nosuch", internal=True)
No symbol table is loaded.  Use the "file" command.
(gdb) python gdb.Breakpoint("nosuch", announce=False)
(gdb)

I didn't really think about it though.  I guess 'internal' could maybe
be reused to disable the other message as well.

Tom
  

Patch

diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index 9cbb2f9f57d..bcbd3b271e8 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -5910,7 +5910,7 @@  create both breakpoints and watchpoints.  The second accepts separate Python
 arguments similar to @ref{Explicit Locations}, and can only be used to create
 breakpoints.
 
-@defun Breakpoint.__init__ (spec @r{[}, type @r{][}, wp_class @r{][}, internal @r{][}, temporary @r{][}, qualified @r{]})
+@defun Breakpoint.__init__ (spec @r{[}, type @r{][}, wp_class @r{][}, internal @r{][}, temporary @r{][}, qualified @r{][}, announce @r{]})
 Create a new breakpoint according to @var{spec}, which is a string naming the
 location of a breakpoint, or an expression that defines a watchpoint.  The
 string should describe a location in a format recognized by the @code{break}
@@ -5940,9 +5940,15 @@  the function passed in @code{spec} as a fully-qualified name.  It is equivalent
 to @code{break}'s @code{-qualified} flag (@pxref{Linespec Locations} and
 @ref{Explicit Locations}).
 
+The optional @var{announce} argument is a boolean that controls
+whether @var{GDBN} announces the existence of the breakpoint.  The
+default is to announce, meaning that a message is printed.  Setting
+this argument to false will suppress all output from breakpoint
+creation.
+
 @end defun
 
-@defun Breakpoint.__init__ (@r{[} source @r{][}, function @r{][}, label @r{][}, line @r{]}, @r{][} internal @r{][}, temporary @r{][}, qualified @r{]})
+@defun Breakpoint.__init__ (@r{[} source @r{][}, function @r{][}, label @r{][}, line @r{]}, @r{][} internal @r{][}, temporary @r{][}, qualified @r{][}, announce @r{]})
 This second form of creating a new breakpoint specifies the explicit
 location (@pxref{Explicit Locations}) using keywords.  The new breakpoint will
 be created in the specified source file @var{source}, at the specified
diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
index 39d9bd5dff6..f942a1c631e 100644
--- a/gdb/python/py-breakpoint.c
+++ b/gdb/python/py-breakpoint.c
@@ -903,7 +903,8 @@  bppy_init (PyObject *self, PyObject *args, PyObject *kwargs)
 {
   static const char *keywords[] = { "spec", "type", "wp_class", "internal",
 				    "temporary","source", "function",
-				    "label", "line", "qualified", NULL };
+				    "label", "line", "qualified",
+				    "announce", nullptr };
   const char *spec = NULL;
   enum bptype type = bp_breakpoint;
   int access_type = hw_write;
@@ -918,13 +919,15 @@  bppy_init (PyObject *self, PyObject *args, PyObject *kwargs)
   char *function = NULL;
   PyObject *qualified_obj = nullptr;
   int qualified = 0;
+  PyObject *announce_obj = nullptr;
+  int announce = 1;
 
-  if (!gdb_PyArg_ParseTupleAndKeywords (args, kwargs, "|siiOOsssOO", keywords,
+  if (!gdb_PyArg_ParseTupleAndKeywords (args, kwargs, "|siiOOsssOOO", keywords,
 					&spec, &type, &access_type,
 					&internal,
 					&temporary, &source,
 					&function, &label, &lineobj,
-					&qualified_obj))
+					&qualified_obj, &announce_obj))
     return -1;
 
 
@@ -963,6 +966,13 @@  bppy_init (PyObject *self, PyObject *args, PyObject *kwargs)
 	return -1;
     }
 
+  if (announce_obj != nullptr)
+    {
+      announce = PyObject_IsTrue (announce_obj);
+      if (announce == -1)
+	return -1;
+    }
+
   if (bppy_init_validate_args (spec, source, function, label, line.get (),
 			       type) == -1)
     return -1;
@@ -973,9 +983,17 @@  bppy_init (PyObject *self, PyObject *args, PyObject *kwargs)
 
   try
     {
-      bppy_create_breakpoint (type, access_type, temporary_bp, internal_bp,
-			      spec, qualified, source, function, label,
-			      line.get ());
+      if (announce)
+	bppy_create_breakpoint (type, access_type, temporary_bp, internal_bp,
+				spec, qualified, source, function,
+				label, line.get ());
+      else
+	execute_fn_to_ui_file (&null_stream, [&] ()
+	  {
+	    bppy_create_breakpoint (type, access_type, temporary_bp,
+				    internal_bp, spec, qualified,
+				    source, function, label, line.get ());
+	  });
     }
   catch (const gdb_exception &except)
     {
diff --git a/gdb/testsuite/gdb.python/py-breakpoint.exp b/gdb/testsuite/gdb.python/py-breakpoint.exp
index e36e87dc291..27f0619443e 100644
--- a/gdb/testsuite/gdb.python/py-breakpoint.exp
+++ b/gdb/testsuite/gdb.python/py-breakpoint.exp
@@ -544,7 +544,8 @@  proc_with_prefix test_bkpt_address {} {
 
 proc_with_prefix test_bkpt_pending {} {
     delete_breakpoints
-    gdb_breakpoint "nosuchfunction" allow-pending
+    gdb_test_no_output "python gdb.Breakpoint(\"nosuchfunction\", announce=False)" \
+	"create pending breakpoint"
     gdb_test "python print (gdb.breakpoints()\[0\].pending)" "True" \
 	"Check pending status of pending breakpoint"
 }