DelphiAST icon indicating copy to clipboard operation
DelphiAST copied to clipboard

Constant and variable nodes don't have a name attribute but a sub node of type ntName

Open sglienke opened this issue 10 years ago • 7 comments

This is inconsistent with say typedecl or method nodes:

<CONSTANT line="181" col="3"> // <- expected name attribute
  <NAME line="181" col="3" value="DBXVersion25"/>
  <VALUE line="181" col="22">
    <EXPRESSION line="181" col="22">
      <LITERAL line="181" col="27" type="string" value="2.5"/>
    </EXPRESSION>
  </VALUE>
</CONSTANT>

sglienke avatar May 22 '15 07:05 sglienke

I think it's generally better to have a sub node instead of an attribute. Nodes have always line number and column. Sometimes this information can be important. Let's say you have code like this:

function DoSometing: integer;

If you have the name stored as an attribute it's going to be represented in AST exactly like this code:

function                             DoSometing:                      integer;
function 
    DoSometing: integer;

This fact makes it really hard to find the syntax node by line number and column. And it's almost impossible to write a code file with the same custom format by running over the AST. I'd suggest to use attributes rarely.

Wosi avatar Jun 05 '15 18:06 Wosi

@Wosi agree

RomanYankovsky avatar Jun 13 '15 12:06 RomanYankovsky

I think to answer this we have to define what actually should be achieved with this project. Maybe just its name is wrong. An abstract syntax tree does not contain certain information to be able to fully reconstruct the original source from it as it does not contain any information about whitespaces or formatting - afaik this is what a full syntax or parse tree does.

sglienke avatar Mar 16 '16 12:03 sglienke

It is very useful to have source locations, though.

Currently you can't completely reverse from DelphiAST back to the source (I think comments aren't preserved, for example) but having code position (and thus some elicit whitespace and formatting) information is very useful for some purposes.

On 16 March 2016 at 13:51, Stefan Glienke [email protected] wrote:

I think to answer this we have to define what actually should be achieved with this project. Maybe just its name is wrong. An abstract syntax tree does not contain certain information to be able to fully reconstruct the original source from it as it does not contain any information about whitespaces or formatting - afaik this is what a full syntax or parse tree does.

— You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub https://github.com/RomanYankovsky/DelphiAST/issues/91#issuecomment-197307286

vintagedave avatar Mar 16 '16 14:03 vintagedave

I think comments aren't preserved, for example

They do :) As a separated list.

RomanYankovsky avatar Mar 16 '16 14:03 RomanYankovsky

Good news :)

On 16 March 2016 at 15:06, Roman Yankovsky [email protected] wrote:

I think comments aren't preserved, for example

They do :) As a separated list.

— You are receiving this because you commented. Reply to this email directly or view it on GitHub https://github.com/RomanYankovsky/DelphiAST/issues/91#issuecomment-197347202

vintagedave avatar Mar 16 '16 14:03 vintagedave

It is very useful to have source locations, though.

Yes, however their use depends on what you are doing with them. What if I want to write a refactoring feature (rename) with DAST and I have the code that Christopher wrote earlier. What happens when the new name of DoSomething has a different length? Should it keep the number of spaces between the colon and Integer or keep the absolute position in that line? So then you just don't have to store the absolute positions but also the relative ones to the previous token.

Anyhow I think right now it is more like a parse tree than an abstract syntax tree. And imo we need both but not put both into one.

sglienke avatar Mar 16 '16 15:03 sglienke