Re: [PATCH 0/3 v2] C/C++: show pertinent open token when missing a close token

[Trevor Saunders <tbsaunde at tbsaunde dot org>]

On Tue, Aug 01, 2017 at 04:21:41PM -0400, David Malcolm wrote:
> On Wed, 2017-07-12 at 09:13 -0400, Trevor Saunders wrote:
> > On Tue, Jul 11, 2017 at 11:24:45AM -0400, David Malcolm wrote:
> > > + public:
> > > +  /* token_pair's ctor.  */
> > > +  token_pair () : m_open_loc (UNKNOWN_LOCATION) {}
> > 
> > What do you think of passing the parser to the ctor, and dropping it
> > from the other arguments?  It doesn't seem to make sense to support
> > passing in different parsers?
> 
> I'm in two minds about this:
> 
> (a) yes: the parser ptr is always going to unchanged, and could be
>     passed in to the ctor, which would avoid needing to pass it in
>     everywhere the token_pair<T> instancd is used
> 
> (b) is the optimizer good enough to realize this, and avoid storing
>     a second copy of the parser ptr on the stack?  (presumably to
>     optimize away the 2nd copy of the parser ptr, every call to methods
>     of the class would need to be inlined, and then the two on-stack
>     member fields be split up into individual fields, and then copy
>     propagation could eliminate it).

I think storing an extra copy of the parser ptr is the wrong thing to
worry about, its only going to be 8 bytes or so, and you aren't going to
have *that* many of these objects on the stack at any one point.  On the
other hand it might be worth considering which is faster, but I'd be
pretty shocked if these methods are called nearly enough to move this
out of the noise.  I'm not really sure what would be faster in theory,
and can believe it depends on the host arch.  On x86 I could see the
instructions to deal with the parser arg on the stack are slower than
copying the parser pointer into the object.

> In this version of the kit I opted to keep passing in the parser ptr
> at all the usage sites, but I'm open to making it a field of the
> class.
> 
> I'm hoping for input on this from the C/C++ frontend maintainers.

Fair enough, part of me thinks if there is a performance problem its
something we should fix by making gcc optimize better.

So I'd tend to think its better to go for clearer code, but we already
have a ton of explicit parser arguments, so no big deal either way.

Trev