[cfe-dev] Start of source range for Decl nodes.

[Douglas Gregor]

On Mar 4, 2011, at 9:03 AM, Abramo Bagnara wrote:

> Il 04/03/2011 16:30, Douglas Gregor ha scritto:
>> 
>> On Mar 1, 2011, at 10:44 AM, Enea Zaffanella wrote:
>> 
>>> Hello.
>>> 
>>> In our application we are facing a problem related to the quality of
>>> source location information for Decl nodes. When using method
>>>   Decl::getSourceRange()
>>> we noticed a few cases where we obtain not so accurate location info for
>>> the start of the range.
>>> Here are a few examples.
>>> 
>>> For DeclaratorDecl nodes:
>>> const int a;
>>> static int b;
>>> inline int foo() { return 5; }
>>> the range starts from "int", rather than "const" or "static" or "inline".
>>> 
>>> For a NamespaceDecl node such as
>>> namespace N { }
>>> the range starts from the identifier N, rather than from the "namespace"
>>> keyword. Similarly for a linkage specification such as
>>> extern "C" { ... }
>>> 
>>> All of these minor problems can be easily fixed as soon as there will be
>>> a place where the start for the range can be recorded. For instance,
>>> when creating a new VarDecl node in SemaDecl.cpp:2956
>>> 
>>>   NewVD = VarDecl::Create(Context, DC, D.getIdentifierLoc(),
>>>                           II, R, TInfo, SC, SCAsWritten);
>>> 
>>> the Declarator object D could provide accurate source range info, but
>>> the VarDecl constructor has no parameter accepting the start of such a
>>> range: it has a SourceLocation for the identifier loc and a
>>> TypeSourceInfo, which does not take into account syntactic elements such
>>> as storage classes, CVR qualifiers, inline specifiers, etc.
>>> As a result, we obtain strange-looking differences whereby declaration
>>>   int const a;
>>> has an accurate source range, whereas
>>>   const int a;
>>> is skipping the const keyword.
>>> 
>>> Assuming that the goal of obtaining precise source location info should
>>> be achieved by all means, now the question is: which is best place where
>>> this additional source location (the start of the range) for the Decl
>>> hierarchy should be stored?
>>> Would it be OK to store them at the top of the hierarchy?
>>> Are there better places?
>> 
>> I'd rather that we only add this information to those AST nodes that need it.
> 
> I've proposed to put it in Decl as I'm unable to find an AST node that
> has no need to store start source location...
> 
> I'm missing something?

I don't know without studying every Decl again,. If we're truly missing this information *everywhere*, then it's fine. However, please make sure that you're not introducing redundancy: for example, NamespaceAliasDecl already has its starting location in the member "NamespaceLoc". If we add a StartLoc to Decl, we need to remove NamespaceLoc from NamespaceAliasDecl.

>> 
>> Also, the source range for a DeclaratorDecl (and anything else that stares with declaration specifiers) gets messy when there are multiple declarations, e.g.,
>> 
>>  static struct Point { int x, y; } p1, *p2, get_point(int x, int y);
>> 
>> Who claims the "static" keyword? Point? p1? Everyone?
> 
> IMHO opinion there is no doubts that the full source range of p2 is from
> "static" to "p2" and that the full source range of "get_point" is from
> static to ")".
> 
> The concept underlying the source range can be easily defined as the
> minimal contiguous textual area that includes all the text about the
> declaration.
> 
> Of course if some library client wish to build a source range between
> name and declaration end source location is free to do that.
> 
> This solves also easily the problem to know if two declaration are
> inside the same group: if two consecutive declaration have same start
> location they are inside the same group.


Seems reasonable. So the starting location of Point is still 'static'?

  - Doug