Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Simon Marchi <simark@simark.ca>
To: "Jan Vraný" <Jan.Vrany@labware.com>, "tom@tromey.com" <tom@tromey.com>
Cc: "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>
Subject: Re: [PATCH 2/2] gdb: use std::vector<> to hold on blocks in struct blockvector
Date: Tue, 14 Oct 2025 22:00:30 -0400	[thread overview]
Message-ID: <0420c102-7ef0-40c7-a4e8-76004a6a2214@simark.ca> (raw)
In-Reply-To: <86e1dd07aea13acd5e7ded0217f053a0db64c63d.camel@labware.com>



On 2025-10-14 16:40, Jan Vraný wrote:
> On Tue, 2025-10-14 at 16:19 -0400, Simon Marchi wrote:
>> On 10/14/25 4:05 PM, Tom Tromey wrote:
>>>>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:
>>>
>>> Simon> I also wish we could avoid having a "set_block" method, because it
>>> Simon> requires having the blockvector in an invalid state (some blocks slots
>>> Simon> set to nullptr) while we build it, which could be error prone.  The
>>> Simon> users of the set_block method are buildsym and jit.  From what i saw,
>>> Simon> they could both very well prepare an std::vector with their blocks and
>>> Simon> std::move that vector into a blockvector constructor.  That constructor
>>> Simon> could assert that the blocks are correctly ordered, or to the sort
>>> Simon> itself (something that both buildsym and jit do already).  buildsym
>>> Simon> starts with a singly-linked list (m_pending_blocks), moves the blocks to a
>>> Simon> temporary vector (in end_compunit_symtab_get_static_block), sorts that
>>> Simon> vector, builds a singly-linked list again, and then traverses that list
>>> Simon> to insert the blocks into the blockvector.  I think that could all be
>>> Simon> simplified by making m_pending_blocks an std::vector<block *> from the
>>> Simon> start.
>>>
>>> Are you proposing that Jan do this in this patch?  Or is this just a
>>> future-looking comment?  It wasn't clear to me.
>>>
>>> Tom
>>
>> Sorry about that.  I would not ask Jan to refactor or clean up
>> everything in buildsym and jit.  But perhaps we can agree on what a good
>> (easy to use, hard to mis-use) API for blockvector is, and then Jan can
>> do the bare minimum to wire up buildsym and jit to that API.  We can
>> then do some cleanups later to remove some unnecessary steps in buildsym
>> and jit.
> 
> At the moment it seems to me that the first step is to drop add_block(),
> add append_block() and common ordering predicate and take it from there.
> I'll try to have a look tomorrow.

Sounds fine to me.  And I think it's fine to have set_block for now.
Code today pokes the block list directly, so set_block isn't introducing
anything new.

Simon

      reply	other threads:[~2025-10-15  2:01 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-13 18:23 [PATCH 0/2] " Jan Vrany
2025-10-13 18:23 ` [PATCH 1/2] gdb: allocate blockvector on heap Jan Vrany
2025-10-14 18:41   ` Simon Marchi
2025-10-13 18:23 ` [PATCH 2/2] gdb: use std::vector<> to hold on blocks in struct blockvector Jan Vrany
2025-10-14 15:00   ` Andrew Burgess
2025-10-14 19:41     ` Simon Marchi
2025-10-15 21:34       ` Jan Vraný
2025-10-14 19:29   ` Simon Marchi
2025-10-14 20:05     ` Tom Tromey
2025-10-14 20:19       ` Simon Marchi
2025-10-14 20:40         ` Jan Vraný
2025-10-15  2:00           ` Simon Marchi [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=0420c102-7ef0-40c7-a4e8-76004a6a2214@simark.ca \
    --to=simark@simark.ca \
    --cc=Jan.Vrany@labware.com \
    --cc=gdb-patches@sourceware.org \
    --cc=tom@tromey.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox