On Wed, 2021-06-16 at 11:21 -0600, Martin Sebor wrote: > On 6/16/21 10:35 AM, Jason Merrill wrote: > > On 6/16/21 12:11 PM, Martin Sebor wrote: > > > On 6/16/21 9:06 AM, David Malcolm wrote: > > > > On Wed, 2021-06-16 at 08:52 -0600, Martin Sebor wrote: > > > > > On 6/16/21 6:38 AM, David Malcolm wrote: > > > > > > On Tue, 2021-06-15 at 19:48 -0600, Martin Sebor wrote: > > > > > > > > > > > > Thanks for writing the patch. > > > > > > > > > > > > > While debugging locations I noticed the semi_embedded_vec > > > > > > > template > > > > > > > in line-map.h doesn't declare a copy ctor or copy > > > > > > > assignment, but > > > > > > > is being copied in a couple of places in the C++ parser > > > > > > > (via > > > > > > > gcc_rich_location). It gets away with it most likely > > > > > > > because it > > > > > > > never grows beyond the embedded buffer. > > > > > > > > > > > > Where are these places? I wasn't aware of this. > > > > > > > > > > They're in the attached file along with the diff to reproduce > > > > > the errors. > > > > > > > > Thanks. > > > > > > > > Looks like: > > > > > > > > gcc_rich_location richloc = tok->location; > > > > > > > > is implicitly constructing a gcc_rich_location, then copying it > > > > to > > > > richloc. This should instead be simply: > > > > > > > > gcc_rich_location richloc (tok->location); > > > > > > > > which directly constructs the richloc in place, as I understand > > > > it. > > > > > > I see, tok->location is location_t here, and the > > > gcc_rich_location > > > ctor that takes it is not declared explicit (that would prevent > > > the implicit conversion). > > > > > > The attached patch solves the rich_location problem by a) making > > > the ctor explicit, b) disabling the rich_location copy ctor, c) > > > changing the parser to use direct initialization. (I CC Jason > > > as a heads up on the C++ FE bits.) > > > > The C++ bits are fine. > > > > > The semi_embedded_vec should be fixed as well, regardless of this > > > particular use and patch. Let me know if it's okay to commit > > > (I'm > > > not open to disabling its copy ctor). > > > > > + /* Not copyable or assignable. */ > > > > This comment needs a rationale. > > Done in the attached patch.
LGTM; thanks Dave > > Providing a rationale in each instance sounds like a good addition > to the coding conventions. Let me propose a patch for that. > > Martin
