Mihaela Chirea - SAOC 2020 Milestone 2 Update 2 - Improving DMD as a Library

[Jacob Carlborg]

On 2020-11-07 12:28, Mihaela Chirea wrote:

> Currently, the CPPNamespaceDeclaration location starts at `extern`, just 
> like Nspace, but moving it to the exact start of the name is possible. 
> Would you consider this a better option?

Absolutely not. I feel like I have already mentioned this several times: 
The start position needs to point to the first byte of the AST node. If 
you want to get the location of some specific part of a node, you should 
access the smallest sub node and then re-lex the source range the sub 
node occupies (when source ranges are available). Then you can get the 
location of the specific token you're interested in, like the 
identifier. At this point you already know the semantic meanings of the 
tokens, so it shouldn't be any problem.

> - StaticForeachDeclaration: this one contains a StaticForeach which 
> already starts at `static` so I'm not sure if adding it here as well 
> would be necessary

All AST nodes should have a location. Otherwise more special casing in 
the code is required when operating on the nodes. Do you know why there 
are two nodes? Do they have the same meaning?

Either you can duplicate the location, or can (possibly) declare a 
method in `StaticForeachDeclaration` which forwards to the location in 
`StaticForeach`.

-- 
/Jacob Carlborg