[PATCH] make rich_location safe to copy

Martin Sebor msebor@gmail.com
Wed Jun 16 17:21:48 GMT 2021


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.

Providing a rationale in each instance sounds like a good addition
to the coding conventions.  Let me propose a patch for that.

Martin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: gcc-rich-location.diff
Type: text/x-patch
Size: 2737 bytes
Desc: not available
URL: <https://gcc.gnu.org/pipermail/gcc-patches/attachments/20210616/a6896b65/attachment.bin>


More information about the Gcc-patches mailing list