Parser support for explicit storage locations #15463

matheusaaguiar · 2024-09-28T00:54:24Z

First step for #597.

matheusaaguiar · 2024-09-29T17:58:34Z

libsolidity/ast/AST.h

@@ -591,6 +593,7 @@ class ContractDefinition: public Declaration, public StructurallyDocumented, pub
 	std::vector<ASTPointer<ASTNode>> m_subNodes;
 	ContractKind m_contractKind;
 	bool m_abstract{false};
+	ASTPointer<Expression> m_storageBaseLocationExpression;


Should we create a dedicated AST node for it?

We'll likely allow more and more expressions in the future - just packing it into an expression seems fine to me.
We can still promote it to a full AST node, once we add more complex layout specifiers.

matheusaaguiar · 2024-09-30T00:18:11Z

libsolidity/parsing/Parser.cpp

+		);
+
+	advance();
+	return parseExpression();


I guess it should be something more specific, because expression will accept anything. Or is it fine?

Yes, I also believe that we should have something more specific. If I recall correctly, it must be a constant expression that can be evaluate at compile-time. So maybe we should have a new parse function that is restricted to TokenTraits::isArithmeticOp, Literals and the erc7201 helper only.

I am assuming that we will check those kind of things later (constant, compile-time eval, etc) at type checking phase.
I am worried about parsing something weird (like a function declaration or a nested contract declaration), but I think I should look more closely to the behaviour of parseExpression to get a better feeling for that.

Parsing arbitrary expressions here and then rejecting complex expressions later (in type checking or even already in syntax checking) seems fine to me.
Might be valuable to quickly confirm that there's no really pathological weird cases that may arise because of it, but I don't anticipate any out of my head.

test/libsolidity/syntaxTests/contractBaseLocation/contract_named_at.sol

libsolidity/parsing/Parser.cpp

github-actions · 2024-11-14T12:06:29Z

This pull request is stale because it has been open for 14 days with no activity.
It will be closed in 7 days unless the stale label is removed.

github-actions · 2024-12-18T12:06:12Z

This pull request is stale because it has been open for 14 days with no activity.
It will be closed in 7 days unless the stale label is removed.

github-actions · 2024-12-26T12:05:44Z

This pull request was closed due to a lack of activity for 7 days after it was stale.

github-actions · 2025-01-10T12:06:02Z

This pull request is stale because it has been open for 14 days with no activity.
It will be closed in 7 days unless the stale label is removed.

nikola-matic

First step for #597.

Do we have another issue detailing the actual steps? I.e. syntax/parsing (this), type checking/analysis, code gen, docs, etc.?

Otherwise, this particular PR looks good, you seem to have covered all of the edge cases I could think of.

nikola-matic · 2025-01-10T12:26:13Z

libsolidity/parsing/Parser.cpp

+				m_errorReporter.parserError(
+					6668_error,
+					m_scanner->currentLocation(),
+					SecondarySourceLocation().append("First inheritance definition is here", baseContracts[0]->location()),


Is this actually output anywhere? I can't see the secondary source location messages anywhere in the tests (not just for this particular one, but all of them in general).

They appear in the CLI, not sure if they do in the JSON output.

Hmm, weird, I did a bit of grepping around and didn't see any occurrences in the tests, but I might be mistaken.

Yeah, I hadn't added any cmdline tests, so the secondary source location/error message was not showing.
I added two more tests covering this case (inheritance) and the repetead base location definition.

Changelog.md

matheusaaguiar · 2025-01-17T20:53:01Z

Do we have another issue detailing the actual steps? I.e. syntax/parsing (this), type checking/analysis, code gen, docs, etc.?

I just created #15727 in order to keep track of the tasks.

libsolidity/ast/AST.h

matheusaaguiar force-pushed the storageLocationsParserSupport branch from 42172bc to a632251 Compare September 28, 2024 19:59

matheusaaguiar self-assigned this Sep 28, 2024

matheusaaguiar commented Sep 29, 2024

View reviewed changes

matheusaaguiar commented Sep 30, 2024

View reviewed changes

r0qs reviewed Sep 30, 2024

View reviewed changes

test/libsolidity/syntaxTests/contractBaseLocation/contract_named_at.sol Show resolved Hide resolved

matheusaaguiar force-pushed the storageLocationsParserSupport branch from fef6ff4 to 26bb5e5 Compare October 14, 2024 13:16

matheusaaguiar marked this pull request as ready for review October 14, 2024 18:34

matheusaaguiar force-pushed the storageLocationsParserSupport branch from 26bb5e5 to bf640be Compare October 21, 2024 01:32

matheusaaguiar mentioned this pull request Oct 21, 2024

Type checking for contract explicit storage base location #15528

Open

1 task

matheusaaguiar commented Oct 21, 2024

View reviewed changes

libsolidity/parsing/Parser.cpp Outdated Show resolved Hide resolved

github-actions bot added the stale The issue/PR was marked as stale because it has been open for too long. label Nov 14, 2024

matheusaaguiar removed the stale The issue/PR was marked as stale because it has been open for too long. label Nov 15, 2024

matheusaaguiar force-pushed the storageLocationsParserSupport branch from bf640be to 666b499 Compare November 21, 2024 13:57

matheusaaguiar force-pushed the storageLocationsParserSupport branch from fb75168 to 2579486 Compare December 3, 2024 17:11

github-actions bot added the stale The issue/PR was marked as stale because it has been open for too long. label Dec 18, 2024

github-actions bot added the closed-due-inactivity label Dec 26, 2024

github-actions bot closed this Dec 26, 2024

matheusaaguiar reopened this Dec 26, 2024

matheusaaguiar removed stale The issue/PR was marked as stale because it has been open for too long. closed-due-inactivity labels Dec 26, 2024

github-actions bot added the stale The issue/PR was marked as stale because it has been open for too long. label Jan 10, 2025

nikola-matic removed the stale The issue/PR was marked as stale because it has been open for too long. label Jan 10, 2025

nikola-matic reviewed Jan 10, 2025

View reviewed changes

matheusaaguiar force-pushed the storageLocationsParserSupport branch 3 times, most recently from f0f7997 to 2839eed Compare January 17, 2025 15:16

matheusaaguiar mentioned this pull request Jan 17, 2025

Support for arbitrary contract storage base location #15727

Open

ekpyron reviewed Jan 27, 2025

View reviewed changes

libsolidity/ast/AST.h Outdated Show resolved Hide resolved

matheusaaguiar added 8 commits January 28, 2025 00:42

Parse contract layout specification

13bd005

update grammar

25b1736

Changelog

b785bc6

Add StorageBaseLocation AST node

d326734

Parse/create StorageBaseLocation AST Node

c317eb2

AST Json import/export

4abdd38

Refactor: change ast node name to StorageLayoutSpecifier

37197d5

Add tests

c2d5d9a

matheusaaguiar force-pushed the storageLocationsParserSupport branch from 7a42733 to c2d5d9a Compare January 28, 2025 03:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Parser support for explicit storage locations #15463

Parser support for explicit storage locations #15463

matheusaaguiar commented Sep 28, 2024

matheusaaguiar Sep 29, 2024

ekpyron Dec 2, 2024

matheusaaguiar Sep 30, 2024 •

edited

Loading

r0qs Sep 30, 2024 •

edited

Loading

matheusaaguiar Sep 30, 2024 •

edited

Loading

ekpyron Dec 2, 2024

github-actions bot commented Nov 14, 2024

github-actions bot commented Dec 18, 2024

github-actions bot commented Dec 26, 2024

github-actions bot commented Jan 10, 2025

nikola-matic left a comment

nikola-matic Jan 10, 2025

matheusaaguiar Jan 10, 2025

nikola-matic Jan 13, 2025

matheusaaguiar Jan 13, 2025

matheusaaguiar commented Jan 17, 2025

Parser support for explicit storage locations #15463

Are you sure you want to change the base?

Parser support for explicit storage locations #15463

Conversation

matheusaaguiar commented Sep 28, 2024

matheusaaguiar Sep 29, 2024

Choose a reason for hiding this comment

ekpyron Dec 2, 2024

Choose a reason for hiding this comment

matheusaaguiar Sep 30, 2024 • edited Loading

Choose a reason for hiding this comment

r0qs Sep 30, 2024 • edited Loading

Choose a reason for hiding this comment

matheusaaguiar Sep 30, 2024 • edited Loading

Choose a reason for hiding this comment

ekpyron Dec 2, 2024

Choose a reason for hiding this comment

github-actions bot commented Nov 14, 2024

github-actions bot commented Dec 18, 2024

github-actions bot commented Dec 26, 2024

github-actions bot commented Jan 10, 2025

nikola-matic left a comment

Choose a reason for hiding this comment

nikola-matic Jan 10, 2025

Choose a reason for hiding this comment

matheusaaguiar Jan 10, 2025

Choose a reason for hiding this comment

nikola-matic Jan 13, 2025

Choose a reason for hiding this comment

matheusaaguiar Jan 13, 2025

Choose a reason for hiding this comment

matheusaaguiar commented Jan 17, 2025

matheusaaguiar Sep 30, 2024 •

edited

Loading

r0qs Sep 30, 2024 •

edited

Loading

matheusaaguiar Sep 30, 2024 •

edited

Loading