Support constants in custom storage layout expression #15944

matheusaaguiar · 2025-03-15T15:42:37Z

part of #15727.

matheusaaguiar · 2025-03-27T02:19:48Z

libsolidity/analysis/PostTypeContractLevelChecker.cpp

-			"The base slot of the storage layout must evaluate to an integer."
-		);
-		return;
+		if (integerType->isSigned())


This could also be removed from here and then replace the assert in line 173 as suggested in a previous PR #15528 (comment)

Depends. The range check should catch it anyway and it provides a message that will be clearer to the user so I'd rather not check it. Removing this and adding an assert would be a better choice.

But I'm not sure if we will actually reach that check with constants. What happens in this case if you remove the isSigned(). Will it fail in constant evaluator instead?

int constant X = -1; contract C layout at X {}

It still fails the range check and the message is indeed clearer as you said:

Error: The base slot of the storage layout evaluates to -1, which is outside the range of type uint256.

zaqk · 2025-04-04T13:08:56Z

thank you for supporting constants! this is a huge composability unlock!

zaqk · 2025-04-04T14:46:03Z

Would this also apply to immutables?

matheusaaguiar · 2025-04-11T13:10:16Z

Would this also apply to immutables?

No, this only targets constants. Immutables are a little bit different and demand a different handling.

cameel · 2025-04-11T14:47:25Z

We can't really support immutables here, because the layout must already be defined at creation time (constructor can initialize state variables), but their values can still change at that point. In case where they do not, the immutable can be replaced by a constant anyway.

@matheusaaguiar Speaking of immutables, please make sure you have a test case against #15989. I.e. one showing that immutables are rejected even if they end up being treated as pure by the compiler.

cameel · 2025-04-22T23:40:52Z

libsolidity/analysis/PostTypeContractLevelChecker.cpp

 	{
 		m_errorReporter.typeError(
 			6396_error,
 			baseSlotExpression.location(),
-			"The base slot of the storage layout must evaluate to a rational number."
+			"The base slot of the storage layout must evaluate to a rational integer number."


Suggested change

"The base slot of the storage layout must evaluate to a rational integer number."

"The base slot of the storage layout must evaluate to an integer."

We could also say a rational number or an integer, but not sure if the user will get the distinction. Unfortunately, out terminology here is a bit ambiguous.

cameel · 2025-04-23T00:21:53Z

test/libsolidity/syntaxTests/storageLayoutSpecifier/constant_from_base_contract.sol

@@ -4,4 +4,4 @@ contract A {

 contract C is A layout at A.x { }
 // ----
-// TypeError 6396: (68-71): The base slot of the storage layout must evaluate to a rational number.
+// TypeError 1505: (68-71): The base slot expression cannot be evaluated during compilation.


This one is weird. Why are we getting this and not something like:

Error: Member "x" not found or not visible after argument-dependent lookup in type(contract A).

Is ConstantEvaluator silencing this error and just returning nullopt?

Yes, ConstantEvaluator can't handle member access. At the moment it can only process, literals, variables with no member acess from modules or other contracts and most of binary/unary operations.

Interesting. So apparently it's not just layuouts, but ConstantEvaluator does not support such constants in general. It does not seem hard to add that though. Looking at ConstantEvaluator, it may be as simple as defining endVisit(MemberAccess) similar to endVisit(Identifier) it already has. I think we should do it: #16055.

This should not block this PR though. It's fine to leave it unsupported. You could add an explicit check for MemberAccess to PostTypeContractLevelChecker to issue a better message, but it will only catch cases where it's the top-level expression, so it's a half-measure.

cameel · 2025-04-23T00:23:20Z

test/libsolidity/syntaxTests/storageLayoutSpecifier/constant_from_module.sol

+import "A" as M;
+contract C layout at M.x{ }
+// ----
+// TypeError 1505: (B:38-41): The base slot expression cannot be evaluated during compilation.


This one should work without errors.

Same for constant_initialized_from_cast.sol

cameel · 2025-04-23T00:27:43Z

test/libsolidity/syntaxTests/storageLayoutSpecifier/immutables.sol

+    address immutable a = 0x0000000000000000000000000000000000000001;
+    uint immutable x = 1;


Please also add an immutable that's not pure (i.e. one that's not initialized with a literal) since that's a separate case. Also a comment explaining how they differ.

cameel · 2025-04-23T00:35:25Z

test/libsolidity/syntaxTests/storageLayoutSpecifier/bitwise_negation_after_cast.sol

@@ -1,3 +1,3 @@
 contract C layout at ~uint(0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF) {}
 // ----
-// TypeError 6396: (21-94): The base slot of the storage layout must evaluate to a rational number.
+// TypeError 1505: (21-94): The base slot expression cannot be evaluated during compilation.


I think that users will be confused about why we can't evaluate this, because fundamentally it should be possible. It's documented, but maybe it would not hurt to just say that the constant evaluator is just too limited for this?

Suggested change

// TypeError 1505: (21-94): The base slot expression cannot be evaluated during compilation.

// TypeError 1505: (21-94): The base slot expression contains elements that are not yet supported by the internal constant evaluator and therefore cannot be evaluated at compilation time.

cameel · 2025-04-23T00:45:28Z

test/libsolidity/syntaxTests/storageLayoutSpecifier/type_uint_max.sol

 contract at layout at type(uint).max { }
 // ----
-// TypeError 6396: (22-36): The base slot of the storage layout must evaluate to a rational number.
+// TypeError 1505: (22-36): The base slot expression cannot be evaluated during compilation.


I was hoping this would work already in this PR, but apparently it's another thing that requires proper compile-time evaluation... #11183 (comment)

cameel · 2025-05-16T16:32:20Z

libsolidity/analysis/PostTypeContractLevelChecker.cpp

We should mention in the docs that constants are now supported.

Just replace an integer literal expression with an integer literal expression or a constant.

The feature also needs a changelog entry.

matheusaaguiar self-assigned this Mar 15, 2025

matheusaaguiar force-pushed the allowConstantsCustomStorageLayout branch 2 times, most recently from 944dc30 to 059349f Compare March 25, 2025 23:27

matheusaaguiar force-pushed the allowConstantsCustomStorageLayout branch from 4aae044 to 00ee32e Compare March 27, 2025 02:03

matheusaaguiar commented Mar 27, 2025

View reviewed changes

matheusaaguiar marked this pull request as ready for review March 27, 2025 02:20

matheusaaguiar force-pushed the allowConstantsCustomStorageLayout branch from ec9785d to d1dbd2e Compare April 22, 2025 19:47

cameel reviewed Apr 23, 2025

View reviewed changes

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

matheusaaguiar removed the stale The issue/PR was marked as stale because it has been open for too long. label May 7, 2025

matheusaaguiar force-pushed the allowConstantsCustomStorageLayout branch from d1dbd2e to 0d68b8c Compare May 15, 2025 15:20

ethereum deleted a comment from github-actions bot May 16, 2025

cameel mentioned this pull request May 16, 2025

Compile-time evaluation of constants of member constants #16055

Open

cameel reviewed May 16, 2025

View reviewed changes

matheusaaguiar added 6 commits May 21, 2025 09:36

Update and add tests

1bce2af

Allow constants in custom storage layout expression

5ceb50d

fixup! error on types not convertible to uint256

5cb35ce

fixup! Update and add tests

3cb6335

Adjust docs

8174ebf

Changelog

d19c5dc

matheusaaguiar force-pushed the allowConstantsCustomStorageLayout branch from ced73ae to d19c5dc Compare May 21, 2025 12:36

	"The base slot of the storage layout must evaluate to a rational integer number."
	"The base slot of the storage layout must evaluate to an integer."

		address immutable a = 0x0000000000000000000000000000000000000001;
		uint immutable x = 1;

	// TypeError 1505: (21-94): The base slot expression cannot be evaluated during compilation.
	// TypeError 1505: (21-94): The base slot expression contains elements that are not yet supported by the internal constant evaluator and therefore cannot be evaluated at compilation time.

Support constants in custom storage layout expression #15944

Are you sure you want to change the base?

Support constants in custom storage layout expression #15944

Uh oh!

Conversation

matheusaaguiar commented Mar 15, 2025

Uh oh!

matheusaaguiar Mar 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zaqk commented Apr 4, 2025

Uh oh!

zaqk commented Apr 4, 2025

Uh oh!

matheusaaguiar commented Apr 11, 2025

Uh oh!

cameel commented Apr 11, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cameel May 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

matheusaaguiar Mar 27, 2025 •

edited

Loading

cameel May 16, 2025 •

edited

Loading