Skip to content

Allow NatSpec comments for state variables#8399

Closed
erak wants to merge 3 commits intodevelopfrom
structured-docs-variables
Closed

Allow NatSpec comments for state variables#8399
erak wants to merge 3 commits intodevelopfrom
structured-docs-variables

Conversation

@erak
Copy link
Collaborator

@erak erak commented Feb 28, 2020

Fixes #3418.

Checklist

  • Code compiles correctly
  • All tests are passing
  • New tests have been created which fail without the change (if possible)
  • README / documentation was extended, if necessary
  • Changelog entry (if change is visible to the user)
  • Used meaningful commit messages

@erak erak force-pushed the structured-docs-variables branch from f0edb95 to 11a7c37 Compare February 28, 2020 12:23
@christianparpart christianparpart requested a review from ekpyron March 2, 2020 13:57
ekpyron
ekpyron requested changes Mar 2, 2020
View reviewed changes
libsolidity/analysis/ReferencesResolver.cpp Outdated

void ReferencesResolver::endVisit(VariableDeclaration const& _variable)
{
bool isStateVariable = dynamic_cast<VariableDeclaration const*>(&_variable);
Copy link
Collaborator

@ekpyron ekpyron Mar 2, 2020

Choose a reason for hiding this comment

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

Suggested change
bool isStateVariable = dynamic_cast<VariableDeclaration const*>(&_variable);
bool isStateVariable = dynamic_cast<StateVariableDeclaration const*>(&_variable);

?

Copy link
Collaborator Author

@erak erak Mar 2, 2020

Choose a reason for hiding this comment

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

That's a defect ;-) Thanks!

Copy link
Collaborator

@ekpyron ekpyron Mar 2, 2020

Choose a reason for hiding this comment

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

It's interesting compiler errors in CI: the compiler can assume that the address of ‘_variable’ will never be NULL - never seen those, but it's legit :-)!

@erak erak force-pushed the structured-docs-variables branch from 11a7c37 to ea0d331 Compare March 5, 2020 21:01
@stackenbotten
Copy link

stackenbotten commented Mar 5, 2020

There was an error when running chk_coding_style for commit ea0d33199489956e2f7c97a1861c9b673fd98c04:

Coding style error:
 libsolidity/ast/AST.h:825: ASTPointer<StructuredDocumentation> const& _documentation,
 libsolidity/ast/AST.h:833: StructurallyDocumented(_documentation),
 libsolidity/ast/ASTJsonImporter.cpp:420: _node["documentation"].isNull() ? nullptr : createDocumentation(member(_node, "documentation")),
 libsolidity/parsing/Parser.cpp:681: ASTPointer<StructuredDocumentation> documentation = parseStructuredDocumentation();
 libsolidity/parsing/Parser.cpp:795: documentation,
 libsolidity/parsing/Parser.cpp:1554: Visibility::Default,
 libsolidity/parsing/Parser.cpp:1555: nullptr

Please check that your changes are working as intended.

@erak erak added the takeover Can be taken over by someone other than label giver label Mar 5, 2020
@erak
Copy link
Collaborator Author

erak commented Mar 5, 2020

I think that this goes into the right direction now. There are some parts which need some more thoughts (e.g. how to handle comments on non-state-variables, see commented code in Parser.cpp), but I'm not able to continue on this any further. Would be good, if someone else could take over.

@erak erak mentioned this pull request Mar 5, 2020
Closed
1 task
@aarlt aarlt mentioned this pull request Mar 24, 2020
Merged
@chriseth
Copy link
Contributor

chriseth commented Mar 25, 2020

Replaced by #8532

@chriseth chriseth closed this Mar 25, 2020
@leonardoalt leonardoalt deleted the structured-docs-variables branch September 4, 2020 11:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ekpyron ekpyron ekpyron requested changes

Assignees

No one assigned

Labels

takeover Can be taken over by someone other than label giver

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Allow NatSpec comments for variables

4 participants

@erak @stackenbotten @chriseth @ekpyron