Re: [SFN+LVU+IEPM v4 7/9] [LVU] Introduce location views

[Jakub Jelinek <jakub@redhat.com>]

On Thu, Jan 25, 2018 at 05:58:37PM -0200, Alexandre Oliva wrote:
> > This looks wrong.  The proposal has not been accepted yet, so you
> > really can't know if DW_LLE_view_pair will be like that or whether it
> > will have value of 9.  Unfortunately, the DWARF standard doesn't specify a
> > vendor range for DW_LLE_* values.  I'd use 0xf0 or so, and don't pretend
> > there is DW_LLE_view_pair at all, just use DW_LLE_GNU_view_pair everywhere.
> > Jason, what do you think?
> 
> My reasoning was that, since we'd only emit this as an
> explicitly-requested backward-incompatible extension to DWARF-5 (by
> specifying -gdwarf-6 in this patch, but the option spelling could be
> changed), any LLE number whatsoever would do.  The point of the #define
> was to write the code in GCC so that, once DW_LLE_view_pair was
> standardized (if it ever was), we'd just set up an enum for it and we'd
> be done, but that would only happen at DWARF6+ anyway, so we'd be able
> to tell, since we'd then have actual DWARF6, rather than DWARF5 with an
> incompatible extensions (which is what we emit with the current
> -gdwarf-6 option; see below).

Having to tweak debug info consumers so that they treat DW_LLE_* of 9
one way for .debug_loclist of version 5 and another way for .debug_loclist
of version 6 isn't a good idea.
Why don't you emit the DW_LLE_GNU_view_pair stuff in .debug_loclists
already with -gdwarf-5, if needed paired without some TU attribute that says
that views are emitted?

> >> +static inline bool
> >> +dwarf2out_locviews_in_attribute ()
> >> +{
> >> +  return debug_variable_location_views
> >> +    && dwarf_version <= 5;
> 
> > Formatting, should be
> >   return debug_variable_location_views && dwarf_version <= 5;
> > or
> >   return (debug_variable_location_views
> > 	  && dwarf_version <= 5);
> > if it wouldn't fit (but it does).
> 
> Hmm...  Where does that mandate come from?, if you don't mind my asking.
> I have just revised both GNU Coding Standards, and GCC-specific
> conventions, to make sure I hadn't missed a change or some long-ago
> established convention, and I couldn't find anything that supports that
> "should".

Haven't looked for it it in the coding standards, but it is something
I've been asked to do in my code by various reviewers over the years and
what I've been asking others in my patch reviews from others too.

I'm personally not using emacs and so the extra ()s isn't something I'd
want myself, it is just something others asked for multiple times.
I feel strongly about indenting stuff right, which if it wouldn't fit would
be
  return verylongcondition____________________________________
	 && otherlongcondition__________________________________;
rather than
  return verylongcondition____________________________________
      && otherlongcondition__________________________________;
or similar, it would surprise me if it is not in the coding standard.

> Personally, I don't see that line breaks before operators are only
> permitted when the operand that follows wouldn't fit; the standards are

Yes, you can break it, but why, it doesn't make the code more readable,
but less in this case.

> No, that means something else, namely emit location view lists in a
> separate attribute, matching corresponding ranges.
> 
> Maybe we shouldn't even have an option to emit that at this point, or
> make it a very different spelling.  But I'd argue there's no point in
> discarding the code that implements the format that was proposed for
> standardization; at the very least it serves as a reference.  That's
> also an argument for retaining the ability to emit it somehow, even if
> with an option that we document as to-be-dropped if/when there's a
> standard representation.

So, what is the reason not to emit the format you are proposing already with
-gdwarf-5, perhaps with some extra attribute on the TU (of course not when
-gstrict-dwarf)?
Debuggers are still barely catching up with DWARF5, so even if they would be
upset by the DW_LLE_GNU_view_pair stuff, they can be still tweaked to ignore
those (skip over them) if they don't want to deal with the views.

	Jakub