Skip to content

feat: .len() method on array, dictionary and string - #292

Merged
MichaelTheSynthCat merged 3 commits into
avast:masterfrom
MichaelTheSynthCat:feature/len_method
Jun 9, 2026
Merged

feat: .len() method on array, dictionary and string#292
MichaelTheSynthCat merged 3 commits into
avast:masterfrom
MichaelTheSynthCat:feature/len_method

Conversation

@MichaelTheSynthCat

Copy link
Copy Markdown
Collaborator

No description provided.

@MichaelTheSynthCat
MichaelTheSynthCat marked this pull request as ready for review June 4, 2026 16:03
std::vector<std::vector<std::string>> _overloadArgumentsNames; ///< Names of arguments of all known overloads
};

inline std::optional<std::shared_ptr<Symbol>> ValueSymbol::getAttribute(const std::string& name) const

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does ValueSymbol need this method too? 🤔

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even Strings (represented by ValueSymbol) have .len() method.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh that makes sense in that case. But wouldn't it probably make sense to have getAttribute on global Symbol level then and override it for classes where it makes sense?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking about that because it would simplify the code a lot. I was just not sure if we would be okay with adding a new virtual method on Symbol.

Comment thread src/parser/parser_driver.cpp Outdated
if (!attr)
error_handle(args[2].getTokenIt()->getLocation(), "Unrecognized identifier '" + symbol_token->getString() + "' referenced on array/dictionary");
}
else if (parentSymbol->isValue())

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if this is really needed or is it?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was intended for Strings too. Right now it has benn removed thanks to the new common Symbol::getAttribute() method.

@MatejKastak MatejKastak left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would also suggest to also test the Visitors.

Comment thread tests/cpp/parser_tests.cpp

@MatejKastak MatejKastak left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thx for the tests

@MichaelTheSynthCat
MichaelTheSynthCat merged commit f2894a0 into avast:master Jun 9, 2026
15 checks passed
@MichaelTheSynthCat
MichaelTheSynthCat deleted the feature/len_method branch June 9, 2026 08:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants