[PATCH] Pass pratt parsed token to expr parser functions to fix expr locus
Mark Wielaard
mark@klomp.org
Fri Jul 30 01:03:13 GMT 2021
Hi,
On Thu, Jul 29, 2021 at 05:18:50PM +0200, Thomas Schwinge wrote:
> On 2021-07-29T12:55:38+0200, Mark Wielaard <mark@klomp.org> wrote:
> > On Thu, 2021-07-29 at 09:25 +0800, The Other via Gcc-rust wrote:
> >> But isn’t it overkill to pass the token in instead of just the
> >> location? You can avoid a fairly expensive shared_ptr copy by doing
> >> so.
> >
> > I hadn't even noticed it was a smart pointer. At least the abstraction
> > works transparently.
>
> Wouldn't 'const_TokenPtr &pratt_parsed_token' work, passing a C++
> reference? (Or am I missing some C++ "detail"; I haven't looked
> carefully.)
I think the issue is not the passing of the actual smart pointer, but
the construction in the default case. When we define the method
argument as const_TokenPtr pratt_parsed_token = nullptr we are
actually constructing a shared_ptr wrapper around the nullptr.
Of course in the case of passing a Location we also always have to
construct an object. But the Location class is very simple, so
hopefully it is cheaper to construct.
> > But yes, things might be even simpler by passing the location directly.
> > I'll try to rewrite the code tonight to pass a location and see how
> > that looks.
That variant is attached and can also be found here:
https://code.wildebeest.org/git/user/mjw/gccrs/commit/?h=pass-pratt-parse-loc
The original is also here:
https://code.wildebeest.org/git/user/mjw/gccrs/commit/?h=pass-pratt-parse-token
Hopefully one of the two is acceptable. If not please let me know how
to rewrite it in a cheaper more idiomatic way.
Thanks,
Mark
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Pass-pratt-parsed-location-to-expr-parser-functions-.patch
Type: text/x-diff
Size: 22004 bytes
Desc: not available
URL: <https://gcc.gnu.org/pipermail/gcc-rust/attachments/20210730/ab09a5ad/attachment-0001.bin>
More information about the Gcc-rust
mailing list