Hello!

Here is an update of the progress made since the last update [1]:
- Brought all DMD as a library additions hidden behind a version under DMDLIB: CallbackAPI seemed to be the only one fitting this description so far [2]
- Added the start location to CPPMangleDeclaration, CPPNamespaceDeclaration and LinkDeclaration [3]

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?

The remaining nodes:
- StorageClassDeclaration
- DeprecatedDeclaration
- 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
- UserAttributeDeclaration: Jacob Carlborg made an attempt in this direction a while ago [4], I'll look into it and see what could be done in this regard

Next steps:
- Add locations to the remaining nodes
- Add token length

The tasks I planned for this milestone can be found here [5]

#SAOC2020

[1] https://forum.dlang.org/post/fodopjdbdxqclclsdcrn@forum.dlang.org
[2] https://github.com/dlang/dmd/pull/11923
[3] https://github.com/dlang/dmd/pull/11934
[4] https://github.com/dlang/dmd/pull/9359
[5] https://forum.dlang.org/post/kfcxcjauafpcrmfuajam@forum.dlang.org

On Saturday, 7 November 2020 at 11:28:34 UTC, Mihaela Chirea wrote:
> Hello!
>
> Here is an update of the progress made since the last update [1]:
> - Brought all DMD as a library additions hidden behind a version under DMDLIB: CallbackAPI seemed to be the only one fitting this description so far [2]
> - Added the start location to CPPMangleDeclaration, CPPNamespaceDeclaration and LinkDeclaration [3]
>
> [...]

Nice. Keep up the good work

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

> 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.

That makes sense. Thanks for explaining!

>> - 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?

StaticForeach[1] is not an AST node. It's just an object used to keep the common parts of the StaticForeachDeclaration[2] and StaticForeachStatement[3] nodes.

[1] https://github.com/dlang/dmd/blob/master/src/dmd/cond.d#L95
[2] https://github.com/dlang/dmd/blob/master/src/dmd/attrib.d#L1114
[3] https://github.com/dlang/dmd/blob/master/src/dmd/statement.d#L1532

On Monday, 9 November 2020 at 18:32:38 UTC, Mihaela Chirea wrote:

> StaticForeach[1] is not an AST node. It's just an object used to keep the common parts of the StaticForeachDeclaration[2] and StaticForeachStatement[3] nodes.

Ok, I see. All AST nodes should have a location. I recommend adding methods to `StaticForeachDeclaration` and `StaticForeachStatement` which forwards to the location stored in `StaticForeach`.

--
/Jacob Carlborg