Fix `string.concat` and `bytes.concat` assignment to a constant variable. #16341

rodiazet · 2025-12-10T10:38:21Z

Initially reviewed here #16335

This PR fixes assignment to a constant variable of a result of string or bytes concatenation, if their arguments are values known in the compile-time.
Closes: #16188

Additionally

Add unit tests
Update the Changelog.md
Update existing unit tests:
- Modify tests, which trigger "no-effect" expression warning to silence the warning.
- Update error message, because now they fail in different (but very similar) way.
- Add more tests to cover all the builtins

cameel · 2025-12-10T13:41:30Z

test/libsolidity/syntaxTests/constants/type_information_pure_assignment_error.sol

type_information_pure_assignment_error.sol

I don't see any errors in this file :)

Yeah. Too fast as always. Need to slow down a little. So basically I thought initially that runtime/creation codes cannot be compile time constants, but they can and it works well. Added test to type_information_pure_assignment.sol. As discussed async there is only one small issues with this. Namely we should verify that there is no circular dependency in the code. So a contract contains a constant variable which stores code (runtime or creation) of the contract it is being defined. Reported #16345

test/libsolidity/syntaxTests/constants/type_information_pure_assignment_error.sol

test/libsolidity/syntaxTests/constants/math_functions_pure_assignment_error.sol

test/libsolidity/syntaxTests/constants/type_information_pure_assignment_error.sol

cameel · 2025-12-11T15:17:45Z

libsolidity/analysis/TypeChecker.cpp

+		if (
+			// This covers `bytes.concat` and `string.concat`.
+			tt->actualType()->category() == Type::Category::Array &&
+			memberName == "concat"


It would be safer to check for FunctionType::Kind::StringConcat/FunctionType::Kind::BytesConcat rather than function name.

I would also add an assert at the end of this function that for function types the annotation matches the result of FunctionType::isPure().

cameel · 2025-12-11T15:25:12Z

test/libsolidity/semanticTests/constants/type_information_pure_assignment.sol

+    function isEmptyCode() public pure returns (bool) {
+        return creationCode.length > 0 && runtimeCode.length > 0;


The name does not match the condition.

Suggested change

function isEmptyCode() public pure returns (bool) {

return creationCode.length > 0 && runtimeCode.length > 0;

function nonEmptyCode() public pure returns (bool) {

return creationCode.length > 0 && runtimeCode.length > 0;

cameel · 2025-12-11T15:48:43Z

test/libsolidity/syntaxTests/constants/bytes_concat_pure_assignment.sol

+bytes constant abcGlobal = bytes.concat(
+    hex"aaaa",
+    hex"bbbb",
+    hex"cccc"
+);


For completeness I'd also make one of the arguments a constant.

And in bytes_concat_pure_assignment_error.sol I'd add a call to a function that is marked pure.

cameel · 2025-12-11T15:50:36Z

test/libsolidity/syntaxTests/constants/bytes_concat_pure_assignment_error.sol

+    bytes constant abData = bytes.concat(
+        hex"aaaa",
+        hex"bbbb",
+        msg.data
+    );


The args in these tests are pretty short, so I'd put them on a single line to make these it less verbose.

Suggested change

bytes constant abData = bytes.concat(

hex"aaaa",

hex"bbbb",

msg.data

);

bytes constant abData = bytes.concat(hex"aaaa", hex"bbbb", msg.data);

cameel · 2025-12-11T15:54:42Z

test/libsolidity/syntaxTests/constants/math_functions_opcodes_pure_assignment_error.sol

+    uint256 k = 7;
+    uint256 constant amod = addmod(1, 8, k);
+    uint256 constant mmod = mulmod(1, 8, k);


Do we have a variant of this test but with constant args?

Same for math_functions_precompiles_pure_assignment_error.sol

cameel · 2025-12-11T15:58:35Z

test/libsolidity/syntaxTests/constants/math_functions_pure_assignment.sol

+bytes32 constant sGlobal = sha256(hex"ffff");
+address constant addrGlobal = ecrecover("1234", 1, "0", abi.decode("", (bytes2)));


Please add ripemd160() for completeness.

cameel · 2025-12-11T16:03:18Z

test/libsolidity/syntaxTests/constants/type_information_pure_assignment.sol

+    bytes creationCodeB = type(B).creationCode;
+    bytes runtimeCodeB = type(B).runtimeCode;


These are not marked constant:

Suggested change

bytes creationCodeB = type(B).creationCode;

bytes runtimeCodeB = type(B).runtimeCode;

bytes constant creationCodeB = type(B).creationCode;

bytes constant runtimeCodeB = type(B).runtimeCode;

And I'd add them at file level too.

cameel · 2025-12-11T16:19:12Z

test/libsolidity/syntaxTests/constants/abi_encoding_constant_error.sol

It would be good to use a more regular naming scheme for these tests. Checking if we already have tests covering something is always a challenge and this would make it a bit easier. Here's how I'd name these:

abi_decode_pure_assignment.sol -> assign_abi_decode_non_const_args.sol abi_decode_pure_assignment_error.sol -> assign_abi_decode_const_args.sol abi_encoding_constant_error.sol -> assign_abi_encoding_builtin_non_const_args.sol abi_encode_call_pure_assignment.sol -> assign_abi_encode_const_args.sol abi_encode_call_pure_assignment_error.sol -> assign_abi_encode_non_const_args.sol block_and_tx_properties_pure_assignment_error.sol -> assign_block_tx_msg_property.sol bytes_concat_pure_assignment.sol -> assign_bytes_concat_const_args.sol bytes_concat_pure_assignment_error.sol -> assign_bytes_concat_non_const_args.sol string_concat_pure_assignment.sol -> assign_string_concat_const_args.sol string_concat_pure_assignment_error.sol -> assign_string_concat_non_const_args.sol math_functions_opcodes_pure_assignment_error.sol -> assign_math_builtin_opcode_based_non_const_args.sol math_functions_precompiles_pure_assignment_error.sol -> assign_math_builtin_precompile_based_non_const_args.sol math_functions_pure_assignment.sol -> assign_math_builtin_const_args.sol type_information_pure_assignment.sol -> assign_type_info.sol

General remarks:

Instead of appending _error I'd rather have the test name say what exactly makes the test different. It makes it easier to see what the test is about just from the name and sometimes these errors are there only temporarily, because some features is not yet implemented.

Instead of the assign_ prefix I suggested above you could also group them by moving them into a subdir, e.g. initialization/.

The names of existing tests use pure and constant interchangeably. That's because I think initially pure was meant to be a compile-time constant. With time they have diverged and pure will likely remain its own thing. It's fine to stick to original naming if it's consistent, but otherwise we should distinguish these.

rodiazet mentioned this pull request Dec 10, 2025

Fix string.concat and bytes.concat assignment to a constant variable. #16335

Closed

rodiazet changed the title ~~Fix string bytes concat assigment~~ Fix string.concat and bytes.concat assignment to a constant variable. Dec 10, 2025

rodiazet requested a review from cameel December 10, 2025 10:50

cameel reviewed Dec 10, 2025

View reviewed changes

rodiazet added 6 commits December 11, 2025 12:37

Fix string|bytes.concat assignment to a constant variable.

0670546

Update changelog

767c58f

Update tests. Modify to avoid the no effect warning.

8aa5346

Update tests. Change error message.

356f582

Add negative tests

440a437

Add more tests to cover all builtins.

fb51ba2

rodiazet force-pushed the fix-string-bytes-concat-assigment branch from 7974cf9 to 9cdd7bb Compare December 11, 2025 12:24

Address PR review commnets.

aee6051

rodiazet force-pushed the fix-string-bytes-concat-assigment branch from 9cdd7bb to aee6051 Compare December 11, 2025 13:28

cameel reviewed Dec 11, 2025

View reviewed changes

		function isEmptyCode() public pure returns (bool) {
		return creationCode.length > 0 && runtimeCode.length > 0;

		bytes32 constant sGlobal = sha256(hex"ffff");
		address constant addrGlobal = ecrecover("1234", 1, "0", abi.decode("", (bytes2)));

		bytes creationCodeB = type(B).creationCode;
		bytes runtimeCodeB = type(B).runtimeCode;

Fix string.concat and bytes.concat assignment to a constant variable. #16341

Are you sure you want to change the base?

Fix string.concat and bytes.concat assignment to a constant variable. #16341

Uh oh!

Conversation

rodiazet commented Dec 10, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

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!

Choose a reason for hiding this comment

Uh oh!

cameel Dec 11, 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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Fix `string.concat` and `bytes.concat` assignment to a constant variable. #16341

Fix `string.concat` and `bytes.concat` assignment to a constant variable. #16341

cameel Dec 11, 2025 •

edited

Loading