Include source position in the AST

[wiiznokes]

I'm creating a tool to check if there is more than two identifiers with the same name. How can i know where the error is in the file?

Idk what is the standard way of doing this, i'm thinking of providing a field pos: Option<usize>, inside the ast types, but i'm not sure.

wdyt?

[zbraniecki]

Ugh, I understand what you're trying to achieve but the architecture is particularly not suited for that. We do not associate AST with any textual representation, and binding AST to offsets would do just that.

If you were able to achieve adding that without inducing performance, memory penalty or increasing AST maintenance complexity, I'd be open to that, but I'm concerned it won't be easy to do.

For your particular purpose, you could extend the parser to handle optional visitor/inspect callback before inserting an entry to the AST, and check if it's an ID that already has been added. I know it's not perfect, but parser is tied to source format, resulting AST is not.

[wiiznokes]

For your particular purpose, you could extend the parser to handle optional visitor/inspect callback before inserting an entry to the AST, and check if it's an ID that already has been added.

while this could work, i'm also planning to do more verification like compare two files, so having to parse multiple times is not great.

I know it doesn't really fill the maintenance constraint but would a feature gate could be considered?

[zbraniecki]

Sure, I'd like to just ask for a good design that minimizes the maintenance burden. I don't want to have each edit to AST/parser be slowed down by source position considerations.

Maybe a trait that can be implemented on each node and parser #[cfg(...)]node.setPositionInSource(offset) ?

[wiiznokes]

But how will the offset be stored ?

I was thinking of something like #[cfg(...)] pos: usize, and then when parsing, we just need to insert the position #[cfg(...)] pos: self.ptr

To be clear, if the ast is changed, the positions will not be in sync, and the serializer will not take the offset into account.

[alerque]

This seems to be a duplicate of #270.