Re: [PATCH] Add extra location information - PR43486

[Manuel López-Ibáñez <lopezibanez at gmail dot com>]

On 18 September 2012 12:58, Arnaud Charlet <charlet@adacore.com> wrote:
> This is a relatively large patch which tackles the difficult issue of
> generating more sloc information in front-ends (in particular C and C++
> front-ends), so that "static analyzers" based on GCC can get more useful
> and detailed information.
>
> This happens to be related/similar to the need mentioned in PR43486, hence
> the mention of this PR in the subject line.
>
> In my work, as I mentioned a few times in the past, this patch is the
> first foundation for another patch, which will provide source navigation
> capabilities in IDEs/editors, by generating source cross reference information
> based on the C and C++ front-ends (introducing a new switch -fdump-xref).
> Ideally I'd like to submit both patches.
>
> Since this issue is more general, I have split my changes and introduced a new
> tentative switch called -fextra-slocs, which is the subject of this email.
>
> In order to provide more sloc info attached to nodes (and in particular
> VAR_DECLs), there are basically two possibilities:
>
> - the one mentioned in comment 4 of PR43486:
>   << Jason Merrill 2010-03-23 01:51:34 UTC
>
>   I suppose we could wrap rvalue uses in NOP_EXPR and lvalue uses in
>   VIEW_CONVERT_EXPR.
>   >>
>
>   I investigated this option, and unfortunately this does not work
>   because of folding occurring all over the place in the C and
>   C++ front-ends, removing these extra EXPRs very early.

These are issues that require to be fixed. The front-ends should only
fold when actually required, and then, they should keep around the
original expression.

>   So I opted for the second approach:
> - Keep another data structure which will associate extra slocs with some
>   node expressions. The idea is that since you can't associate slocs with
>   VAR_DECLs in an expr, instead we store extra slocs in the containing
>   expression node. For instance, all binary expressions (add ADD, MINUS, ...)
>   will contain two extra slocs: one for the left hand side, and one for the
>   right hand side of the expression. One extra sloc for unary operators,
>   etc...

Consider bugs like: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55173,
where a warning ocurs in a system-header, but GCC does not see this
because the expression that triggers that warning does not have a
location, so input_location is used, which is not at the header.  The
proposed approach does not fully solve such bugs, because  at the
point of warning, only the operands are available, and not the
containing expression node, so the additional locations are not
available. Thus, this second approach would still require passing an
explicit location all over the FE. Admitedly, this is what we are
currently doing in many cases. But do we want to add one extra (or
worse, one per tree argument) explicit location argument on almost
every FE function? For functions that accept tree nodes with location
and without location, this is a waste, because depending on the
particular expression, we will pass two times the same location
(explicitly and implicitly as part of the node). Even worse, many
functions that don't care about locations will have to pass around
locations.

I think, in the long-term, your proposed approach is less useful than
fixing early folding and adding locations (via a wrapper or an
on-the-side index) to the tree nodes that don't have one. But, since
nobody is working on the latter, the "long term" may be longer than
the time until someone makes a LLVM-to-GIMPLE conversor, and replaces
the C/C++ FEs by clang. ;-)  So, as a short-term fix, your proposed
approach is definitely better than the status-quo, so I hope you
manage to get it finished.

Cheers,

Manuel.