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. Dave > > I was seeing strange behavior in my tests that led me to rich_location > and the m_ranges member. The problem turned out to be unrelated but > before I figured it out I noticed the missing copy ctor and deleted > it to see if it was being used. Since that's such a pervasive bug > in GCC code (and likely elsewhere as well) I'm thinking I should take > the time to develop the warning I've been thinking about to detect it. > > > > > The attached patch defines the copy ctor and also copy assignment > > > and adds the corresponding move functions. > > > > Note that rich_location::m_fixit_hints "owns" the fixit_hint > > instances, > > manually deleting them in rich_location's dtor, so simply doing a > > shallow copy of it would be wrong. > > > > Also, a rich_location stores other pointers (to range_labels and > > diagnostic_path), which are borrowed pointers, where their lifetime > > is > > assumed to outlive any (non-dtor) calls to the rich_location. So I'm > > nervous about code that copies rich_location instances. > > > > I think I'd prefer to forbid copying them; what's the use-case for > > copying them? Am I missing something here? > > I noticed and fixed just the one problem I uncovered by accident with > the missing copy ctor. If there are others I don't know about them. > Preventing code from copying rich_location might make sense > independently of fixing the vec class to be safely copyable. > > Martin > > > > > > > > > Tested on x86_64-linux. > > > > > > Martin > > > > Thanks > > Dave > > >
