From ef133aa670db20d34177f674c927c2c82f3aeb54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20=C5=9Aliwak?= Date: Fri, 29 May 2020 21:21:42 +0200 Subject: [PATCH 1/6] CommandLineInterface: Display an error instead of crashing when assembly variant has no Ewasm translation --- solc/CommandLineInterface.cpp | 6 ++++++ test/cmdlineTests/evm_to_wasm_unsupported_translation/args | 1 + test/cmdlineTests/evm_to_wasm_unsupported_translation/err | 1 + test/cmdlineTests/evm_to_wasm_unsupported_translation/exit | 1 + .../evm_to_wasm_unsupported_translation/input.yul | 3 +++ .../cmdlineTests/evm_to_wasm_unsupported_translation/output | 1 + 6 files changed, 13 insertions(+) create mode 100644 test/cmdlineTests/evm_to_wasm_unsupported_translation/args create mode 100644 test/cmdlineTests/evm_to_wasm_unsupported_translation/err create mode 100644 test/cmdlineTests/evm_to_wasm_unsupported_translation/exit create mode 100644 test/cmdlineTests/evm_to_wasm_unsupported_translation/input.yul create mode 100644 test/cmdlineTests/evm_to_wasm_unsupported_translation/output diff --git a/solc/CommandLineInterface.cpp b/solc/CommandLineInterface.cpp index a8e5cc5b7..1cee24807 100644 --- a/solc/CommandLineInterface.cpp +++ b/solc/CommandLineInterface.cpp @@ -1172,6 +1172,12 @@ bool CommandLineInterface::processInput() endl; return false; } + if (targetMachine == Machine::Ewasm && inputLanguage != Input::StrictAssembly && inputLanguage != Input::Ewasm) + { + serr() << "The selected input language is not directly supported when targeting the Ewasm machine "; + serr() << "and automatic translation is not available." << endl; + return false; + } serr() << "Warning: Yul is still experimental. Please use the output with care." << endl; diff --git a/test/cmdlineTests/evm_to_wasm_unsupported_translation/args b/test/cmdlineTests/evm_to_wasm_unsupported_translation/args new file mode 100644 index 000000000..cc6c07083 --- /dev/null +++ b/test/cmdlineTests/evm_to_wasm_unsupported_translation/args @@ -0,0 +1 @@ +--assemble --machine ewasm diff --git a/test/cmdlineTests/evm_to_wasm_unsupported_translation/err b/test/cmdlineTests/evm_to_wasm_unsupported_translation/err new file mode 100644 index 000000000..5ef050f64 --- /dev/null +++ b/test/cmdlineTests/evm_to_wasm_unsupported_translation/err @@ -0,0 +1 @@ +The selected input language is not directly supported when targeting the Ewasm machine and automatic translation is not available. diff --git a/test/cmdlineTests/evm_to_wasm_unsupported_translation/exit b/test/cmdlineTests/evm_to_wasm_unsupported_translation/exit new file mode 100644 index 000000000..d00491fd7 --- /dev/null +++ b/test/cmdlineTests/evm_to_wasm_unsupported_translation/exit @@ -0,0 +1 @@ +1 diff --git a/test/cmdlineTests/evm_to_wasm_unsupported_translation/input.yul b/test/cmdlineTests/evm_to_wasm_unsupported_translation/input.yul new file mode 100644 index 000000000..f21cd2b7e --- /dev/null +++ b/test/cmdlineTests/evm_to_wasm_unsupported_translation/input.yul @@ -0,0 +1,3 @@ +{ + sstore(0, 1) +} diff --git a/test/cmdlineTests/evm_to_wasm_unsupported_translation/output b/test/cmdlineTests/evm_to_wasm_unsupported_translation/output new file mode 100644 index 000000000..8b1378917 --- /dev/null +++ b/test/cmdlineTests/evm_to_wasm_unsupported_translation/output @@ -0,0 +1 @@ + From 37e8d78cff0fdfd93edf56b932476130c0ae8312 Mon Sep 17 00:00:00 2001 From: chriseth Date: Thu, 28 May 2020 17:39:08 +0200 Subject: [PATCH 2/6] [Sol->Yul] Implement getters. --- libsolidity/codegen/YulUtilFunctions.cpp | 12 +- libsolidity/codegen/ir/IRGenerator.cpp | 206 +++++++++++------- .../semanticTests/getters/arrays.sol | 14 ++ .../semanticTests/getters/mapping.sol | 11 + .../semanticTests/getters/small_types.sol | 19 ++ .../storage/accessors_mapping_for_array.sol | 2 + 6 files changed, 179 insertions(+), 85 deletions(-) create mode 100644 test/libsolidity/semanticTests/getters/arrays.sol create mode 100644 test/libsolidity/semanticTests/getters/mapping.sol create mode 100644 test/libsolidity/semanticTests/getters/small_types.sol diff --git a/libsolidity/codegen/YulUtilFunctions.cpp b/libsolidity/codegen/YulUtilFunctions.cpp index 067fe3fea..87142099e 100644 --- a/libsolidity/codegen/YulUtilFunctions.cpp +++ b/libsolidity/codegen/YulUtilFunctions.cpp @@ -1276,7 +1276,8 @@ string YulUtilFunctions::mappingIndexAccessFunction(MappingType const& _mappingT string YulUtilFunctions::readFromStorage(Type const& _type, size_t _offset, bool _splitFunctionTypes) { - solUnimplementedAssert(!_splitFunctionTypes, ""); + if (_type.category() == Type::Category::Function) + solUnimplementedAssert(!_splitFunctionTypes, ""); string functionName = "read_from_storage_" + string(_splitFunctionTypes ? "split_" : "") + @@ -1299,7 +1300,8 @@ string YulUtilFunctions::readFromStorage(Type const& _type, size_t _offset, bool string YulUtilFunctions::readFromStorageDynamic(Type const& _type, bool _splitFunctionTypes) { - solUnimplementedAssert(!_splitFunctionTypes, ""); + if (_type.category() == Type::Category::Function) + solUnimplementedAssert(!_splitFunctionTypes, ""); string functionName = "read_from_storage_dynamic" + string(_splitFunctionTypes ? "split_" : "") + @@ -1429,7 +1431,8 @@ string YulUtilFunctions::writeToMemoryFunction(Type const& _type) string YulUtilFunctions::extractFromStorageValueDynamic(Type const& _type, bool _splitFunctionTypes) { - solUnimplementedAssert(!_splitFunctionTypes, ""); + if (_type.category() == Type::Category::Function) + solUnimplementedAssert(!_splitFunctionTypes, ""); string functionName = "extract_from_storage_value_dynamic" + @@ -1474,7 +1477,8 @@ string YulUtilFunctions::extractFromStorageValue(Type const& _type, size_t _offs string YulUtilFunctions::cleanupFromStorageFunction(Type const& _type, bool _splitFunctionTypes) { solAssert(_type.isValueType(), ""); - solUnimplementedAssert(!_splitFunctionTypes, ""); + if (_type.category() == Type::Category::Function) + solUnimplementedAssert(!_splitFunctionTypes, ""); string functionName = string("cleanup_from_storage_") + (_splitFunctionTypes ? "split_" : "") + _type.identifier(); return m_functionCollector.createFunction(functionName, [&] { diff --git a/libsolidity/codegen/ir/IRGenerator.cpp b/libsolidity/codegen/ir/IRGenerator.cpp index 1bb6c7089..92df1ba83 100644 --- a/libsolidity/codegen/ir/IRGenerator.cpp +++ b/libsolidity/codegen/ir/IRGenerator.cpp @@ -268,101 +268,145 @@ string IRGenerator::generateFunction(FunctionDefinition const& _function) string IRGenerator::generateGetter(VariableDeclaration const& _varDecl) { string functionName = IRNames::function(_varDecl); + return m_context.functionCollector().createFunction(functionName, [&]() { + Type const* type = _varDecl.annotation().type; - Type const* type = _varDecl.annotation().type; - - solAssert(_varDecl.isStateVariable(), ""); - - if (auto const* mappingType = dynamic_cast(type)) - return m_context.functionCollector().createFunction(functionName, [&]() { - solAssert(!_varDecl.isConstant() && !_varDecl.immutable(), ""); - pair slot_offset = m_context.storageLocationOfVariable(_varDecl); - solAssert(slot_offset.second == 0, ""); - FunctionType funType(_varDecl); - solUnimplementedAssert(funType.returnParameterTypes().size() == 1, ""); - TypePointer returnType = funType.returnParameterTypes().front(); - unsigned num_keys = 0; - stringstream indexAccesses; - string slot = m_context.newYulVariable(); - do - { - solUnimplementedAssert( - mappingType->keyType()->sizeOnStack() == 1, - "Multi-slot mapping key unimplemented - might not be a problem" - ); - indexAccesses << - slot << - " := " << - m_utils.mappingIndexAccessFunction(*mappingType, *mappingType->keyType()) << - "(" << - slot; - if (mappingType->keyType()->sizeOnStack() > 0) - indexAccesses << - ", " << - suffixedVariableNameList("key", num_keys, num_keys + mappingType->keyType()->sizeOnStack()); - indexAccesses << ")\n"; - num_keys += mappingType->keyType()->sizeOnStack(); - } - while ((mappingType = dynamic_cast(mappingType->valueType()))); + solAssert(_varDecl.isStateVariable(), ""); + FunctionType accessorType(_varDecl); + TypePointers paramTypes = accessorType.parameterTypes(); + if (_varDecl.immutable()) + { + solAssert(paramTypes.empty(), ""); + solUnimplementedAssert(type->sizeOnStack() == 1, ""); return Whiskers(R"( - function () -> rval { - let := - - rval := () + function () -> rval { + rval := loadimmutable("") } )") ("functionName", functionName) - ("keys", suffixedVariableNameList("key", 0, num_keys)) - ("readStorage", m_utils.readFromStorage(*returnType, 0, false)) - ("indexAccesses", indexAccesses.str()) - ("slot", slot) - ("base", slot_offset.first.str()) + ("id", to_string(_varDecl.id())) .render(); - }); - else - { - solUnimplementedAssert(type->isValueType(), ""); + } + else if (_varDecl.isConstant()) + { + solAssert(paramTypes.empty(), ""); + return Whiskers(R"( + function () -> { + := () + } + )") + ("functionName", functionName) + ("constantValueFunction", IRGeneratorForStatements(m_context, m_utils).constantValueFunction(_varDecl)) + ("ret", suffixedVariableNameList("ret_", 0, _varDecl.type()->sizeOnStack())) + .render(); + } - return m_context.functionCollector().createFunction(functionName, [&]() { - if (_varDecl.immutable()) + string code; + + auto const& location = m_context.storageLocationOfVariable(_varDecl); + code += Whiskers(R"( + let slot := + let offset := + )") + ("slot", location.first.str()) + ("offset", to_string(location.second)) + .render(); + + if (!paramTypes.empty()) + solAssert( + location.second == 0, + "If there are parameters, we are dealing with structs or mappings and thus should have offset zero." + ); + + // The code of an accessor is of the form `x[a][b][c]` (it is slightly more complicated + // if the final type is a struct). + // In each iteration of the loop below, we consume one parameter, perform an + // index access, reassign the yul variable `slot` and move @a currentType further "down". + // The initial value of @a currentType is only used if we skip the loop completely. + TypePointer currentType = _varDecl.annotation().type; + + vector parameters; + vector returnVariables; + + for (size_t i = 0; i < paramTypes.size(); ++i) + { + MappingType const* mappingType = dynamic_cast(currentType); + ArrayType const* arrayType = dynamic_cast(currentType); + solAssert(mappingType || arrayType, ""); + + vector keys = IRVariable("key_" + to_string(i), + mappingType ? *mappingType->keyType() : *TypeProvider::uint256() + ).stackSlots(); + parameters += keys; + code += Whiskers(R"( + slot, offset := (slot, ) + )") + ( + "indexAccess", + mappingType ? + m_utils.mappingIndexAccessFunction(*mappingType, *mappingType->keyType()) : + m_utils.storageArrayIndexAccessFunction(*arrayType) + ) + ("array", arrayType != nullptr) + ("keys", joinHumanReadable(keys)) + .render(); + + currentType = mappingType ? mappingType->valueType() : arrayType->baseType(); + } + + auto returnTypes = accessorType.returnParameterTypes(); + solAssert(returnTypes.size() >= 1, ""); + if (StructType const* structType = dynamic_cast(currentType)) + { + solAssert(location.second == 0, ""); + auto const& names = accessorType.returnParameterNames(); + for (size_t i = 0; i < names.size(); ++i) { - solUnimplementedAssert(type->sizeOnStack() == 1, ""); - return Whiskers(R"( - function () -> rval { - rval := loadimmutable("") - } + if (returnTypes[i]->category() == Type::Category::Mapping) + continue; + if (auto arrayType = dynamic_cast(returnTypes[i])) + if (!arrayType->isByteArray()) + continue; + + // TODO conversion from storage byte arrays is not yet implemented. + pair const& offsets = structType->storageOffsetsOfMember(names[i]); + vector retVars = IRVariable("ret_" + to_string(returnVariables.size()), *returnTypes[i]).stackSlots(); + returnVariables += retVars; + code += Whiskers(R"( + := (add(slot, )) )") - ("functionName", functionName) - ("id", to_string(_varDecl.id())) + ("ret", joinHumanReadable(retVars)) + ("readStorage", m_utils.readFromStorage(*returnTypes[i], offsets.second, true)) + ("slotOffset", offsets.first.str()) .render(); } - else if (_varDecl.isConstant()) - return Whiskers(R"( - function () -> { - := () - } - )") - ("functionName", functionName) - ("constantValueFunction", IRGeneratorForStatements(m_context, m_utils).constantValueFunction(_varDecl)) - ("ret", suffixedVariableNameList("ret_", 0, _varDecl.type()->sizeOnStack())) - .render(); - else - { - pair slot_offset = m_context.storageLocationOfVariable(_varDecl); + } + else + { + solAssert(returnTypes.size() == 1, ""); + vector retVars = IRVariable("ret", *returnTypes.front()).stackSlots(); + returnVariables += retVars; + // TODO conversion from storage byte arrays is not yet implemented. + code += Whiskers(R"( + := (slot, offset) + )") + ("ret", joinHumanReadable(retVars)) + ("readStorage", m_utils.readFromStorageDynamic(*returnTypes.front(), true)) + .render(); + } - return Whiskers(R"( - function () -> rval { - rval := () - } - )") - ("functionName", functionName) - ("readStorage", m_utils.readFromStorage(*type, slot_offset.second, false)) - ("slot", slot_offset.first.str()) - .render(); + return Whiskers(R"( + function () -> { + } - }); - } + )") + ("functionName", functionName) + ("params", joinHumanReadable(parameters)) + ("retVariables", joinHumanReadable(returnVariables)) + ("code", std::move(code)) + .render(); + }); } string IRGenerator::generateInitialAssignment(VariableDeclaration const& _varDecl) diff --git a/test/libsolidity/semanticTests/getters/arrays.sol b/test/libsolidity/semanticTests/getters/arrays.sol new file mode 100644 index 000000000..2a8641a9b --- /dev/null +++ b/test/libsolidity/semanticTests/getters/arrays.sol @@ -0,0 +1,14 @@ +contract C { + uint8[][2] public a; + constructor() public { + a[1].push(3); + a[1].push(4); + } +} +// ==== +// compileViaYul: also +// ---- +// a(uint256,uint256): 0, 0 -> FAILURE +// a(uint256,uint256): 1, 0 -> 3 +// a(uint256,uint256): 1, 1 -> 4 +// a(uint256,uint256): 2, 0 -> FAILURE diff --git a/test/libsolidity/semanticTests/getters/mapping.sol b/test/libsolidity/semanticTests/getters/mapping.sol new file mode 100644 index 000000000..5b28a0f06 --- /dev/null +++ b/test/libsolidity/semanticTests/getters/mapping.sol @@ -0,0 +1,11 @@ +contract C { + mapping(uint => mapping(uint => uint)) public x; + constructor() public { + x[1][2] = 3; + } +} +// ==== +// compileViaYul: also +// ---- +// x(uint256,uint256): 1, 2 -> 3 +// x(uint256,uint256): 0, 0 -> 0 \ No newline at end of file diff --git a/test/libsolidity/semanticTests/getters/small_types.sol b/test/libsolidity/semanticTests/getters/small_types.sol new file mode 100644 index 000000000..d970f9138 --- /dev/null +++ b/test/libsolidity/semanticTests/getters/small_types.sol @@ -0,0 +1,19 @@ +contract C { + uint8 public a; + uint16 public b; + uint128 public c; + uint public d; + constructor() public { + a = 3; + b = 4; + c = 5; + d = 6; + } +} +// ==== +// compileViaYul: also +// ---- +// a() -> 3 +// b() -> 4 +// c() -> 5 +// d() -> 6 diff --git a/test/libsolidity/semanticTests/storage/accessors_mapping_for_array.sol b/test/libsolidity/semanticTests/storage/accessors_mapping_for_array.sol index 94bc11ff0..e7f43d48d 100644 --- a/test/libsolidity/semanticTests/storage/accessors_mapping_for_array.sol +++ b/test/libsolidity/semanticTests/storage/accessors_mapping_for_array.sol @@ -8,6 +8,8 @@ contract test { dynamicData[2][2] = 8; } } +// ==== +// compileViaYul: also // ---- // data(uint256,uint256): 2, 2 -> 8 // data(uint256,uint256): 2, 8 -> FAILURE # NB: the original code contained a bug here # From e7f3c042b6923ddd5f51665ef22c2ea7ca1d585e Mon Sep 17 00:00:00 2001 From: chriseth Date: Thu, 28 May 2020 21:11:06 +0200 Subject: [PATCH 3/6] Struct member access for storage and memory. --- .../codegen/ir/IRGeneratorForStatements.cpp | 38 ++++++++++++++++++- .../getters/mapping_to_struct.sol | 20 ++++++++++ .../storage/accessors_mapping_for_array.sol | 2 +- .../semanticTests/storage/struct_accessor.sol | 2 + 4 files changed, 60 insertions(+), 2 deletions(-) create mode 100644 test/libsolidity/semanticTests/getters/mapping_to_struct.sol diff --git a/libsolidity/codegen/ir/IRGeneratorForStatements.cpp b/libsolidity/codegen/ir/IRGeneratorForStatements.cpp index 2924ea535..075cad1f3 100644 --- a/libsolidity/codegen/ir/IRGeneratorForStatements.cpp +++ b/libsolidity/codegen/ir/IRGeneratorForStatements.cpp @@ -1467,7 +1467,43 @@ void IRGeneratorForStatements::endVisit(MemberAccess const& _memberAccess) break; case Type::Category::Struct: { - solUnimplementedAssert(false, ""); + auto const& structType = dynamic_cast(*_memberAccess.expression().annotation().type); + + IRVariable expression(_memberAccess.expression()); + switch (structType.location()) + { + case DataLocation::Storage: + { + pair const& offsets = structType.storageOffsetsOfMember(member); + string slot = m_context.newYulVariable(); + m_code << "let " << slot << " := " << + ("add(" + expression.name() + ", " + offsets.first.str() + ")\n"); + setLValue(_memberAccess, IRLValue{ + type(_memberAccess), + IRLValue::Storage{slot, offsets.second} + }); + break; + } + case DataLocation::Memory: + { + string pos = m_context.newYulVariable(); + m_code << "let " << pos << " := " << + ("add(" + expression.part("mpos").name() + ", " + structType.memoryOffsetOfMember(member).str() + ")\n"); + setLValue(_memberAccess, IRLValue{ + type(_memberAccess), + IRLValue::Memory{pos} + }); + break; + } + case DataLocation::CallData: + { + solUnimplementedAssert(false, ""); + break; + } + default: + solAssert(false, "Illegal data location for struct."); + } + break; } case Type::Category::Enum: { diff --git a/test/libsolidity/semanticTests/getters/mapping_to_struct.sol b/test/libsolidity/semanticTests/getters/mapping_to_struct.sol new file mode 100644 index 000000000..fed9f6ec5 --- /dev/null +++ b/test/libsolidity/semanticTests/getters/mapping_to_struct.sol @@ -0,0 +1,20 @@ +contract C { + struct S { + uint8 a; + uint16 b; + uint128 c; + uint d; + } + mapping(uint => mapping(uint => S)) public x; + constructor() public { + x[1][2].a = 3; + x[1][2].b = 4; + x[1][2].c = 5; + x[1][2].d = 6; + } +} +// ==== +// compileViaYul: also +// ---- +// x(uint256,uint256): 1, 2 -> 3, 4, 5, 6 +// x(uint256,uint256): 0, 0 -> 0x00, 0x00, 0x00, 0x00 diff --git a/test/libsolidity/semanticTests/storage/accessors_mapping_for_array.sol b/test/libsolidity/semanticTests/storage/accessors_mapping_for_array.sol index e7f43d48d..01460a1c8 100644 --- a/test/libsolidity/semanticTests/storage/accessors_mapping_for_array.sol +++ b/test/libsolidity/semanticTests/storage/accessors_mapping_for_array.sol @@ -12,6 +12,6 @@ contract test { // compileViaYul: also // ---- // data(uint256,uint256): 2, 2 -> 8 -// data(uint256,uint256): 2, 8 -> FAILURE # NB: the original code contained a bug here # +// data(uint256,uint256): 2, 8 -> FAILURE # NB: the original code contained a bug here # // dynamicData(uint256,uint256): 2, 2 -> 8 // dynamicData(uint256,uint256): 2, 8 -> FAILURE diff --git a/test/libsolidity/semanticTests/storage/struct_accessor.sol b/test/libsolidity/semanticTests/storage/struct_accessor.sol index 398cf750b..de57d4697 100644 --- a/test/libsolidity/semanticTests/storage/struct_accessor.sol +++ b/test/libsolidity/semanticTests/storage/struct_accessor.sol @@ -8,5 +8,7 @@ contract test { data[7].d = true; } } +// ==== +// compileViaYul: also // ---- // data(uint256): 7 -> 1, 2, true From 5b1426b55e59819df0b7e5ba30c14c0c29050071 Mon Sep 17 00:00:00 2001 From: Djordje Mijovic Date: Tue, 2 Jun 2020 15:45:03 +0200 Subject: [PATCH 4/6] Adding fixes for signedness warnings in liblangutil --- liblangutil/CharStream.cpp | 7 ++-- liblangutil/CharStream.h | 2 +- liblangutil/ParserBase.cpp | 2 +- liblangutil/Scanner.cpp | 42 +++++++++---------- liblangutil/Scanner.h | 6 +-- liblangutil/SemVerHandler.cpp | 12 +++--- liblangutil/SourceLocation.h | 8 +++- liblangutil/SourceReferenceExtractor.cpp | 21 +++++++--- liblangutil/SourceReferenceFormatter.cpp | 4 +- liblangutil/SourceReferenceFormatterHuman.cpp | 20 +++++---- liblangutil/Token.cpp | 2 +- 11 files changed, 71 insertions(+), 55 deletions(-) diff --git a/liblangutil/CharStream.cpp b/liblangutil/CharStream.cpp index 046aca5d3..6f2d47c48 100644 --- a/liblangutil/CharStream.cpp +++ b/liblangutil/CharStream.cpp @@ -85,7 +85,7 @@ string CharStream::lineAtPosition(int _position) const { // if _position points to \n, it returns the line before the \n using size_type = string::size_type; - size_type searchStart = min(m_source.size(), _position); + size_type searchStart = min(m_source.size(), size_type(_position)); if (searchStart > 0) searchStart--; size_type lineStart = m_source.rfind('\n', searchStart); @@ -105,8 +105,9 @@ string CharStream::lineAtPosition(int _position) const tuple CharStream::translatePositionToLineColumn(int _position) const { using size_type = string::size_type; - size_type searchPosition = min(m_source.size(), _position); - int lineNumber = count(m_source.begin(), m_source.begin() + searchPosition, '\n'); + using diff_type = string::difference_type; + size_type searchPosition = min(m_source.size(), size_type(_position)); + int lineNumber = count(m_source.begin(), m_source.begin() + diff_type(searchPosition), '\n'); size_type lineStart; if (searchPosition == 0) lineStart = 0; diff --git a/liblangutil/CharStream.h b/liblangutil/CharStream.h index 6962864b9..aa8f01af2 100644 --- a/liblangutil/CharStream.h +++ b/liblangutil/CharStream.h @@ -72,7 +72,7 @@ public: explicit CharStream(std::string _source, std::string name): m_source(std::move(_source)), m_name(std::move(name)) {} - int position() const { return m_position; } + size_t position() const { return m_position; } bool isPastEndOfInput(size_t _charsForward = 0) const { return (m_position + _charsForward) >= m_source.size(); } char get(size_t _charsForward = 0) const { return m_source[m_position + _charsForward]; } diff --git a/liblangutil/ParserBase.cpp b/liblangutil/ParserBase.cpp index be8ec3eb9..c2162044a 100644 --- a/liblangutil/ParserBase.cpp +++ b/liblangutil/ParserBase.cpp @@ -106,7 +106,7 @@ void ParserBase::expectTokenOrConsumeUntil(Token _value, string const& _currentN if (m_scanner->currentToken() == Token::EOS) { // rollback to where the token started, and raise exception to be caught at a higher level. - m_scanner->setPosition(startPosition); + m_scanner->setPosition(static_cast(startPosition)); m_inParserRecovery = true; fatalParserError(1957_error, errorLoc, msg); } diff --git a/liblangutil/Scanner.cpp b/liblangutil/Scanner.cpp index 6cf319067..e703b5376 100644 --- a/liblangutil/Scanner.cpp +++ b/liblangutil/Scanner.cpp @@ -171,7 +171,7 @@ void Scanner::supportPeriodInIdentifier(bool _value) bool Scanner::scanHexByte(char& o_scannedByte) { char x = 0; - for (int i = 0; i < 2; i++) + for (size_t i = 0; i < 2; i++) { int d = hexValue(m_char); if (d < 0) @@ -189,7 +189,7 @@ bool Scanner::scanHexByte(char& o_scannedByte) std::optional Scanner::scanUnicode() { unsigned x = 0; - for (int i = 0; i < 4; i++) + for (size_t i = 0; i < 4; i++) { int d = hexValue(m_char); if (d < 0) @@ -197,7 +197,7 @@ std::optional Scanner::scanUnicode() rollback(i); return {}; } - x = x * 16 + d; + x = x * 16 + static_cast(d); advance(); } return x; @@ -207,17 +207,17 @@ std::optional Scanner::scanUnicode() void Scanner::addUnicodeAsUTF8(unsigned codepoint) { if (codepoint <= 0x7f) - addLiteralChar(codepoint); + addLiteralChar(char(codepoint)); else if (codepoint <= 0x7ff) { - addLiteralChar(0xc0 | (codepoint >> 6)); - addLiteralChar(0x80 | (codepoint & 0x3f)); + addLiteralChar(char(0xc0u | (codepoint >> 6u))); + addLiteralChar(char(0x80u | (codepoint & 0x3fu))); } else { - addLiteralChar(0xe0 | (codepoint >> 12)); - addLiteralChar(0x80 | ((codepoint >> 6) & 0x3f)); - addLiteralChar(0x80 | (codepoint & 0x3f)); + addLiteralChar(char(0xe0u | (codepoint >> 12u))); + addLiteralChar(char(0x80u | ((codepoint >> 6u) & 0x3fu))); + addLiteralChar(char(0x80u | (codepoint & 0x3fu))); } } @@ -225,10 +225,10 @@ void Scanner::rescan() { size_t rollbackTo = 0; if (m_skippedComments[Current].literal.empty()) - rollbackTo = m_tokens[Current].location.start; + rollbackTo = static_cast(m_tokens[Current].location.start); else - rollbackTo = m_skippedComments[Current].location.start; - m_char = m_source->rollback(size_t(m_source->position()) - rollbackTo); + rollbackTo = static_cast(m_skippedComments[Current].location.start); + m_char = m_source->rollback(m_source->position() - rollbackTo); next(); next(); next(); @@ -260,7 +260,7 @@ Token Scanner::selectToken(char _next, Token _then, Token _else) bool Scanner::skipWhitespace() { - int const startPosition = sourcePos(); + size_t const startPosition = sourcePos(); while (isWhiteSpace(m_char)) advance(); // Return whether or not we skipped any characters. @@ -269,7 +269,7 @@ bool Scanner::skipWhitespace() bool Scanner::skipWhitespaceExceptUnicodeLinebreak() { - int const startPosition = sourcePos(); + size_t const startPosition = sourcePos(); while (isWhiteSpace(m_char) && !isUnicodeLinebreak()) advance(); // Return whether or not we skipped any characters. @@ -309,10 +309,10 @@ bool Scanner::tryScanEndOfLine() return false; } -int Scanner::scanSingleLineDocComment() +size_t Scanner::scanSingleLineDocComment() { LiteralScope literal(this, LITERAL_TYPE_COMMENT); - int endPosition = m_source->position(); + size_t endPosition = m_source->position(); advance(); //consume the last '/' at /// skipWhitespaceExceptUnicodeLinebreak(); @@ -429,7 +429,7 @@ Token Scanner::scanMultiLineDocComment() Token Scanner::scanSlash() { - int firstSlashPosition = sourcePos(); + int firstSlashPosition = static_cast(sourcePos()); advance(); if (m_char == '/') { @@ -441,7 +441,7 @@ Token Scanner::scanSlash() m_skippedComments[NextNext].location.start = firstSlashPosition; m_skippedComments[NextNext].location.source = m_source; m_skippedComments[NextNext].token = Token::CommentLiteral; - m_skippedComments[NextNext].location.end = scanSingleLineDocComment(); + m_skippedComments[NextNext].location.end = static_cast(scanSingleLineDocComment()); return Token::Whitespace; } else @@ -467,7 +467,7 @@ Token Scanner::scanSlash() m_skippedComments[NextNext].location.start = firstSlashPosition; m_skippedComments[NextNext].location.source = m_source; comment = scanMultiLineDocComment(); - m_skippedComments[NextNext].location.end = sourcePos(); + m_skippedComments[NextNext].location.end = static_cast(sourcePos()); m_skippedComments[NextNext].token = comment; if (comment == Token::Illegal) return Token::Illegal; // error already set @@ -495,7 +495,7 @@ void Scanner::scanToken() do { // Remember the position of the next token - m_tokens[NextNext].location.start = sourcePos(); + m_tokens[NextNext].location.start = static_cast(sourcePos()); switch (m_char) { case '"': @@ -690,7 +690,7 @@ void Scanner::scanToken() // whitespace. } while (token == Token::Whitespace); - m_tokens[NextNext].location.end = sourcePos(); + m_tokens[NextNext].location.end = static_cast(sourcePos()); m_tokens[NextNext].location.source = m_source; m_tokens[NextNext].token = token; m_tokens[NextNext].extendedTokenInfo = make_tuple(m, n); diff --git a/liblangutil/Scanner.h b/liblangutil/Scanner.h index d54bb7532..5e613cfdf 100644 --- a/liblangutil/Scanner.h +++ b/liblangutil/Scanner.h @@ -196,7 +196,7 @@ private: ///@} bool advance() { m_char = m_source->advanceAndGet(); return !m_source->isPastEndOfInput(); } - void rollback(int _amount) { m_char = m_source->rollback(_amount); } + void rollback(size_t _amount) { m_char = m_source->rollback(_amount); } /// Rolls back to the start of the current token and re-runs the scanner. void rescan(); @@ -231,7 +231,7 @@ private: Token scanString(); Token scanHexString(); /// Scans a single line comment and returns its corrected end position. - int scanSingleLineDocComment(); + size_t scanSingleLineDocComment(); Token scanMultiLineDocComment(); /// Scans a slash '/' and depending on the characters returns the appropriate token Token scanSlash(); @@ -245,7 +245,7 @@ private: bool isUnicodeLinebreak(); /// Return the current source position. - int sourcePos() const { return m_source->position(); } + size_t sourcePos() const { return m_source->position(); } bool isSourcePastEndOfInput() const { return m_source->isPastEndOfInput(); } bool m_supportPeriodInIdentifier = false; diff --git a/liblangutil/SemVerHandler.cpp b/liblangutil/SemVerHandler.cpp index 04b81206e..a626c5aa7 100644 --- a/liblangutil/SemVerHandler.cpp +++ b/liblangutil/SemVerHandler.cpp @@ -37,7 +37,7 @@ SemVerVersion::SemVerVersion(string const& _versionString) { unsigned v = 0; for (; i != end && '0' <= *i && *i <= '9'; ++i) - v = v * 10 + (*i - '0'); + v = v * 10 + unsigned(*i - '0'); numbers[level] = v; if (level < 2) { @@ -100,10 +100,10 @@ bool SemVerMatchExpression::MatchComponent::matches(SemVerVersion const& _versio int cmp = 0; bool didCompare = false; for (unsigned i = 0; i < levelsPresent && cmp == 0; i++) - if (version.numbers[i] != unsigned(-1)) + if (version.numbers[i] != std::numeric_limits::max()) { didCompare = true; - cmp = _version.numbers[i] - version.numbers[i]; + cmp = static_cast(_version.numbers[i] - version.numbers[i]); } if (cmp == 0 && !_version.prerelease.empty() && didCompare) @@ -245,14 +245,14 @@ unsigned SemVerMatchExpressionParser::parseVersionPart() return 0; else if ('1' <= c && c <= '9') { - unsigned v = c - '0'; + unsigned v(c - '0'); // If we skip to the next token, the current number is terminated. while (m_pos == startPos && '0' <= currentChar() && currentChar() <= '9') { c = currentChar(); - if (v * 10 < v || v * 10 + (c - '0') < v * 10) + if (v * 10 < v || v * 10 + unsigned(c - '0') < v * 10) throw SemVerError(); - v = v * 10 + c - '0'; + v = v * 10 + unsigned(c - '0'); nextChar(); } return v; diff --git a/liblangutil/SourceLocation.h b/liblangutil/SourceLocation.h index e6f97942e..283c8a524 100644 --- a/liblangutil/SourceLocation.h +++ b/liblangutil/SourceLocation.h @@ -85,7 +85,7 @@ struct SourceLocation assertThrow(0 <= start, SourceLocationError, "Invalid source location."); assertThrow(start <= end, SourceLocationError, "Invalid source location."); assertThrow(end <= int(source->source().length()), SourceLocationError, "Invalid source location."); - return source->source().substr(start, end - start); + return source->source().substr(size_t(start), size_t(end - start)); } /// @returns the smallest SourceLocation that contains both @param _a and @param _b. @@ -113,7 +113,11 @@ struct SourceLocation std::shared_ptr source; }; -SourceLocation const parseSourceLocation(std::string const& _input, std::string const& _sourceName, size_t _maxIndex = -1); +SourceLocation const parseSourceLocation( + std::string const& _input, + std::string const& _sourceName, + size_t _maxIndex = std::numeric_limits::max() +); /// Stream output for Location (used e.g. in boost exceptions). inline std::ostream& operator<<(std::ostream& _out, SourceLocation const& _location) diff --git a/liblangutil/SourceReferenceExtractor.cpp b/liblangutil/SourceReferenceExtractor.cpp index 0817e7750..0a6ff0421 100644 --- a/liblangutil/SourceReferenceExtractor.cpp +++ b/liblangutil/SourceReferenceExtractor.cpp @@ -58,11 +58,15 @@ SourceReference SourceReferenceExtractor::extract(SourceLocation const* _locatio string line = source->lineAtPosition(_location->start); - int locationLength = isMultiline ? line.length() - start.column : end.column - start.column; + int locationLength = + isMultiline ? + int(line.length()) - start.column : + end.column - start.column; + if (locationLength > 150) { - int const lhs = start.column + 35; - int const rhs = (isMultiline ? line.length() : end.column) - 35; + auto const lhs = static_cast(start.column) + 35; + string::size_type const rhs = (isMultiline ? line.length() : static_cast(end.column)) - 35; line = line.substr(0, lhs) + " ... " + line.substr(rhs); end.column = start.column + 75; locationLength = 75; @@ -70,8 +74,13 @@ SourceReference SourceReferenceExtractor::extract(SourceLocation const* _locatio if (line.length() > 150) { - int const len = line.length(); - line = line.substr(max(0, start.column - 35), min(start.column, 35) + min(locationLength + 35, len - start.column)); + int const len = static_cast(line.length()); + line = line.substr( + static_cast(max(0, start.column - 35)), + static_cast(min(start.column, 35)) + static_cast( + min(locationLength + 35,len - start.column) + ) + ); if (start.column + locationLength + 35 < len) line += " ..."; if (start.column > 35) @@ -79,7 +88,7 @@ SourceReference SourceReferenceExtractor::extract(SourceLocation const* _locatio line = " ... " + line; start.column = 40; } - end.column = start.column + locationLength; + end.column = start.column + static_cast(locationLength); } return SourceReference{ diff --git a/liblangutil/SourceReferenceFormatter.cpp b/liblangutil/SourceReferenceFormatter.cpp index 6b1a8286a..965755a10 100644 --- a/liblangutil/SourceReferenceFormatter.cpp +++ b/liblangutil/SourceReferenceFormatter.cpp @@ -51,7 +51,7 @@ void SourceReferenceFormatter::printSourceLocation(SourceReference const& _ref) ); m_stream << "^"; if (_ref.endColumn > _ref.startColumn + 2) - m_stream << string(_ref.endColumn - _ref.startColumn - 2, '-'); + m_stream << string(static_cast(_ref.endColumn - _ref.startColumn - 2), '-'); if (_ref.endColumn > _ref.startColumn + 1) m_stream << "^"; m_stream << endl; @@ -60,7 +60,7 @@ void SourceReferenceFormatter::printSourceLocation(SourceReference const& _ref) m_stream << _ref.text << endl << - string(_ref.startColumn, ' ') << + string(static_cast(_ref.startColumn), ' ') << "^ (Relevant source part starts here and spans across multiple lines)." << endl; } diff --git a/liblangutil/SourceReferenceFormatterHuman.cpp b/liblangutil/SourceReferenceFormatterHuman.cpp index b8fdba163..3560955c7 100644 --- a/liblangutil/SourceReferenceFormatterHuman.cpp +++ b/liblangutil/SourceReferenceFormatterHuman.cpp @@ -104,7 +104,7 @@ void SourceReferenceFormatterHuman::printSourceLocation(SourceReference const& _ if (!_ref.multiline) { - int const locationLength = _ref.endColumn - _ref.startColumn; + auto const locationLength = static_cast(_ref.endColumn - _ref.startColumn); // line 1: m_stream << leftpad << ' '; @@ -113,15 +113,17 @@ void SourceReferenceFormatterHuman::printSourceLocation(SourceReference const& _ // line 2: frameColored() << line << " |"; - m_stream << ' ' << text.substr(0, _ref.startColumn); - highlightColored() << text.substr(_ref.startColumn, locationLength); - m_stream << text.substr(_ref.endColumn) << '\n'; + + m_stream << ' ' << text.substr(0, static_cast(_ref.startColumn)); + highlightColored() << text.substr(static_cast(_ref.startColumn), locationLength); + m_stream << text.substr(static_cast(_ref.endColumn)) << '\n'; // line 3: m_stream << leftpad << ' '; frameColored() << '|'; - m_stream << ' ' << replaceNonTabs(text.substr(0, _ref.startColumn), ' '); - diagColored() << replaceNonTabs(text.substr(_ref.startColumn, locationLength), '^'); + + m_stream << ' ' << replaceNonTabs(text.substr(0, static_cast(_ref.startColumn)), ' '); + diagColored() << replaceNonTabs(text.substr(static_cast(_ref.startColumn), locationLength), '^'); m_stream << '\n'; } else @@ -133,13 +135,13 @@ void SourceReferenceFormatterHuman::printSourceLocation(SourceReference const& _ // line 2: frameColored() << line << " |"; - m_stream << ' ' << text.substr(0, _ref.startColumn); - highlightColored() << text.substr(_ref.startColumn) << '\n'; + m_stream << ' ' << text.substr(0, static_cast(_ref.startColumn)); + highlightColored() << text.substr(static_cast(_ref.startColumn)) << '\n'; // line 3: m_stream << leftpad << ' '; frameColored() << '|'; - m_stream << ' ' << replaceNonTabs(text.substr(0, _ref.startColumn), ' '); + m_stream << ' ' << replaceNonTabs(text.substr(0, static_cast(_ref.startColumn)), ' '); diagColored() << "^ (Relevant source part starts here and spans across multiple lines)."; m_stream << '\n'; } diff --git a/liblangutil/Token.cpp b/liblangutil/Token.cpp index e051bb552..70b645992 100644 --- a/liblangutil/Token.cpp +++ b/liblangutil/Token.cpp @@ -130,7 +130,7 @@ int parseSize(string::const_iterator _begin, string::const_iterator _end) { try { - unsigned int m = boost::lexical_cast(boost::make_iterator_range(_begin, _end)); + int m = boost::lexical_cast(boost::make_iterator_range(_begin, _end)); return m; } catch(boost::bad_lexical_cast const&) From e4e3f4984458b17ccd7e9aaf83513b35adfebf48 Mon Sep 17 00:00:00 2001 From: Djordje Mijovic Date: Tue, 2 Jun 2020 15:10:14 +0200 Subject: [PATCH 5/6] Fixing signedness conversion warnings in libevmasm Co-authored-by: Harikrishnan Mulackal --- libevmasm/Assembly.cpp | 50 ++++++++++----------- libevmasm/AssemblyItem.cpp | 38 ++++++++-------- libevmasm/AssemblyItem.h | 6 +-- libevmasm/BlockDeduplicator.cpp | 7 +-- libevmasm/CommonSubexpressionEliminator.cpp | 16 ++++--- libevmasm/ConstantOptimiser.cpp | 4 +- libevmasm/ControlFlowGraph.h | 4 +- libevmasm/ExpressionClasses.cpp | 4 +- libevmasm/Instruction.cpp | 6 +-- libevmasm/JumpdestRemover.cpp | 4 +- libevmasm/KnownState.cpp | 26 +++++------ libevmasm/LinkerObject.cpp | 2 +- libevmasm/PeepholeOptimiser.cpp | 10 ++--- test/libevmasm/Assembler.cpp | 4 +- test/libevmasm/Optimiser.cpp | 10 ++--- 15 files changed, 98 insertions(+), 93 deletions(-) diff --git a/libevmasm/Assembly.cpp b/libevmasm/Assembly.cpp index 296a330ab..8588832fe 100644 --- a/libevmasm/Assembly.cpp +++ b/libevmasm/Assembly.cpp @@ -41,7 +41,7 @@ using namespace solidity::util; AssemblyItem const& Assembly::append(AssemblyItem const& _i) { assertThrow(m_deposit >= 0, AssemblyException, "Stack underflow."); - m_deposit += _i.deposit(); + m_deposit += static_cast(_i.deposit()); m_items.emplace_back(_i); if (!m_items.back().location().isValid() && m_currentSourceLocation.isValid()) m_items.back().setLocation(m_currentSourceLocation); @@ -77,10 +77,10 @@ string locationFromSources(StringMap const& _sourceCodes, SourceLocation const& return ""; string const& source = it->second; - if (size_t(_location.start) >= source.size()) + if (static_cast(_location.start) >= source.size()) return ""; - string cut = source.substr(_location.start, _location.end - _location.start); + string cut = source.substr(static_cast(_location.start), static_cast(_location.end - _location.start)); auto newLinePos = cut.find_first_of("\n"); if (newLinePos != string::npos) cut = cut.substr(0, newLinePos) + "..."; @@ -106,7 +106,7 @@ public: if (!( _item.canBeFunctional() && _item.returnValues() <= 1 && - _item.arguments() <= int(m_pending.size()) + _item.arguments() <= m_pending.size() )) { flush(); @@ -117,7 +117,7 @@ public: if (_item.arguments() > 0) { expression += "("; - for (int i = 0; i < _item.arguments(); ++i) + for (size_t i = 0; i < _item.arguments(); ++i) { expression += m_pending.back(); m_pending.pop_back(); @@ -225,12 +225,12 @@ Json::Value Assembly::assemblyJSON(map const& _sourceIndices) Json::Value& collection = root[".code"] = Json::arrayValue; for (AssemblyItem const& i: m_items) { - unsigned sourceIndex = unsigned(-1); + int sourceIndex = -1; if (i.location().source) { auto iter = _sourceIndices.find(i.location().source->name()); if (iter != _sourceIndices.end()) - sourceIndex = iter->second; + sourceIndex = static_cast(iter->second); } switch (i.type()) @@ -340,7 +340,7 @@ AssemblyItem Assembly::namedTag(string const& _name) { assertThrow(!_name.empty(), AssemblyException, "Empty named tag."); if (!m_namedTags.count(_name)) - m_namedTags[_name] = size_t(newTag().data()); + m_namedTags[_name] = static_cast(newTag().data()); return AssemblyItem{Tag, m_namedTags.at(_name)}; } @@ -441,7 +441,7 @@ map Assembly::optimiseInternal( for (auto const& replacement: deduplicator.replacedTags()) { assertThrow( - replacement.first <= size_t(-1) && replacement.second <= size_t(-1), + replacement.first <= numeric_limits::max() && replacement.second <= numeric_limits::max(), OptimizerException, "Invalid tag replacement." ); @@ -451,8 +451,8 @@ map Assembly::optimiseInternal( "Replacement already known." ); tagReplacements[replacement.first] = replacement.second; - if (_tagsReferencedFromOutside.erase(size_t(replacement.first))) - _tagsReferencedFromOutside.insert(size_t(replacement.second)); + if (_tagsReferencedFromOutside.erase(static_cast(replacement.first))) + _tagsReferencedFromOutside.insert(static_cast(replacement.second)); } count++; } @@ -479,7 +479,7 @@ map Assembly::optimiseInternal( try { optimisedChunk = eliminator.getOptimizedItems(); - shouldReplace = (optimisedChunk.size() < size_t(iter - orig)); + shouldReplace = (optimisedChunk.size() < static_cast(iter - orig)); } catch (StackTooDeepException const&) { @@ -544,7 +544,7 @@ LinkerObject const& Assembly::assemble() const immutableReferencesBySub = linkerObject.immutableReferences; } for (size_t tagPos: sub->m_tagPositionsInBytecode) - if (tagPos != size_t(-1) && tagPos > subTagSize) + if (tagPos != numeric_limits::max() && tagPos > subTagSize) subTagSize = tagPos; } @@ -567,7 +567,7 @@ LinkerObject const& Assembly::assemble() const ); size_t bytesRequiredForCode = bytesRequired(subTagSize); - m_tagPositionsInBytecode = vector(m_usedTags, -1); + m_tagPositionsInBytecode = vector(m_usedTags, numeric_limits::max()); map> tagRef; multimap dataRef; multimap subRef; @@ -586,7 +586,7 @@ LinkerObject const& Assembly::assemble() const for (AssemblyItem const& i: m_items) { // store position of the invalid jump destination - if (i.type() != Tag && m_tagPositionsInBytecode[0] == size_t(-1)) + if (i.type() != Tag && m_tagPositionsInBytecode[0] == numeric_limits::max()) m_tagPositionsInBytecode[0] = ret.bytecode.size(); switch (i.type()) @@ -629,15 +629,15 @@ LinkerObject const& Assembly::assemble() const ret.bytecode.resize(ret.bytecode.size() + bytesPerDataRef); break; case PushSub: - assertThrow(i.data() <= size_t(-1), AssemblyException, ""); + assertThrow(i.data() <= numeric_limits::max(), AssemblyException, ""); ret.bytecode.push_back(dataRefPush); - subRef.insert(make_pair(size_t(i.data()), ret.bytecode.size())); + subRef.insert(make_pair(static_cast(i.data()), ret.bytecode.size())); ret.bytecode.resize(ret.bytecode.size() + bytesPerDataRef); break; case PushSubSize: { - assertThrow(i.data() <= size_t(-1), AssemblyException, ""); - auto s = m_subs.at(size_t(i.data()))->assemble().bytecode.size(); + assertThrow(i.data() <= numeric_limits::max(), AssemblyException, ""); + auto s = m_subs.at(static_cast(i.data()))->assemble().bytecode.size(); i.setPushedValue(u256(s)); uint8_t b = max(1, util::bytesRequired(s)); ret.bytecode.push_back((uint8_t)Instruction::PUSH1 - 1 + b); @@ -683,10 +683,10 @@ LinkerObject const& Assembly::assemble() const break; case Tag: assertThrow(i.data() != 0, AssemblyException, "Invalid tag position."); - assertThrow(i.splitForeignPushTag().first == size_t(-1), AssemblyException, "Foreign tag."); + assertThrow(i.splitForeignPushTag().first == numeric_limits::max(), AssemblyException, "Foreign tag."); assertThrow(ret.bytecode.size() < 0xffffffffL, AssemblyException, "Tag too large."); - assertThrow(m_tagPositionsInBytecode[size_t(i.data())] == size_t(-1), AssemblyException, "Duplicate tag position."); - m_tagPositionsInBytecode[size_t(i.data())] = ret.bytecode.size(); + assertThrow(m_tagPositionsInBytecode[static_cast(i.data())] == numeric_limits::max(), AssemblyException, "Duplicate tag position."); + m_tagPositionsInBytecode[static_cast(i.data())] = ret.bytecode.size(); ret.bytecode.push_back((uint8_t)Instruction::JUMPDEST); break; default: @@ -722,14 +722,14 @@ LinkerObject const& Assembly::assemble() const size_t subId; size_t tagId; tie(subId, tagId) = i.second; - assertThrow(subId == size_t(-1) || subId < m_subs.size(), AssemblyException, "Invalid sub id"); + assertThrow(subId == numeric_limits::max() || subId < m_subs.size(), AssemblyException, "Invalid sub id"); std::vector const& tagPositions = - subId == size_t(-1) ? + subId == numeric_limits::max() ? m_tagPositionsInBytecode : m_subs[subId]->m_tagPositionsInBytecode; assertThrow(tagId < tagPositions.size(), AssemblyException, "Reference to non-existing tag."); size_t pos = tagPositions[tagId]; - assertThrow(pos != size_t(-1), AssemblyException, "Reference to tag without position."); + assertThrow(pos != numeric_limits::max(), AssemblyException, "Reference to tag without position."); assertThrow(util::bytesRequired(pos) <= bytesPerTag, AssemblyException, "Tag too large for reserved space."); bytesRef r(ret.bytecode.data() + i.first, bytesPerTag); toBigEndian(pos, r); diff --git a/libevmasm/AssemblyItem.cpp b/libevmasm/AssemblyItem.cpp index 610c6f5dd..c2e9056fe 100644 --- a/libevmasm/AssemblyItem.cpp +++ b/libevmasm/AssemblyItem.cpp @@ -34,7 +34,7 @@ AssemblyItem AssemblyItem::toSubAssemblyTag(size_t _subId) const { assertThrow(data() < (u256(1) << 64), util::Exception, "Tag already has subassembly set."); assertThrow(m_type == PushTag || m_type == Tag, util::Exception, ""); - size_t tag = size_t(u256(data()) & 0xffffffffffffffffULL); + auto tag = static_cast(u256(data()) & 0xffffffffffffffffULL); AssemblyItem r = *this; r.m_type = PushTag; r.setPushTagSubIdAndTag(_subId, tag); @@ -45,8 +45,8 @@ pair AssemblyItem::splitForeignPushTag() const { assertThrow(m_type == PushTag || m_type == Tag, util::Exception, ""); u256 combined = u256(data()); - size_t subId = size_t((combined >> 64) - 1); - size_t tag = size_t(combined & 0xffffffffffffffffULL); + size_t subId = static_cast((combined >> 64) - 1); + size_t tag = static_cast(combined & 0xffffffffffffffffULL); return make_pair(subId, tag); } @@ -54,7 +54,7 @@ void AssemblyItem::setPushTagSubIdAndTag(size_t _subId, size_t _tag) { assertThrow(m_type == PushTag || m_type == Tag, util::Exception, ""); u256 data = _tag; - if (_subId != size_t(-1)) + if (_subId != numeric_limits::max()) data |= (u256(_subId) + 1) << 64; setData(data); } @@ -93,22 +93,22 @@ unsigned AssemblyItem::bytesRequired(unsigned _addressLength) const assertThrow(false, InvalidOpcode, ""); } -int AssemblyItem::arguments() const +size_t AssemblyItem::arguments() const { if (type() == Operation) - return instructionInfo(instruction()).args; + return static_cast(instructionInfo(instruction()).args); else if (type() == AssignImmutable) return 1; else return 0; } -int AssemblyItem::returnValues() const +size_t AssemblyItem::returnValues() const { switch (m_type) { case Operation: - return instructionInfo(instruction()).ret; + return static_cast(instructionInfo(instruction()).ret); case Push: case PushString: case PushTag: @@ -193,7 +193,7 @@ string AssemblyItem::toAssemblyText() const size_t sub{0}; size_t tag{0}; tie(sub, tag) = splitForeignPushTag(); - if (sub == size_t(-1)) + if (sub == numeric_limits::max()) text = string("tag_") + to_string(tag); else text = string("tag_") + to_string(sub) + "_" + to_string(tag); @@ -201,16 +201,16 @@ string AssemblyItem::toAssemblyText() const } case Tag: assertThrow(data() < 0x10000, AssemblyException, "Declaration of sub-assembly tag."); - text = string("tag_") + to_string(size_t(data())) + ":"; + text = string("tag_") + to_string(static_cast(data())) + ":"; break; case PushData: text = string("data_") + util::toHex(data()); break; case PushSub: - text = string("dataOffset(sub_") + to_string(size_t(data())) + ")"; + text = string("dataOffset(sub_") + to_string(static_cast(data())) + ")"; break; case PushSubSize: - text = string("dataSize(sub_") + to_string(size_t(data())) + ")"; + text = string("dataSize(sub_") + to_string(static_cast(data())) + ")"; break; case PushProgramSize: text = string("bytecodeSize"); @@ -262,7 +262,7 @@ ostream& solidity::evmasm::operator<<(ostream& _out, AssemblyItem const& _item) case PushTag: { size_t subId = _item.splitForeignPushTag().first; - if (subId == size_t(-1)) + if (subId == numeric_limits::max()) _out << " PushTag " << _item.splitForeignPushTag().second; else _out << " PushTag " << subId << ":" << _item.splitForeignPushTag().second; @@ -272,13 +272,13 @@ ostream& solidity::evmasm::operator<<(ostream& _out, AssemblyItem const& _item) _out << " Tag " << _item.data(); break; case PushData: - _out << " PushData " << hex << (unsigned)_item.data() << dec; + _out << " PushData " << hex << static_cast(_item.data()) << dec; break; case PushSub: - _out << " PushSub " << hex << size_t(_item.data()) << dec; + _out << " PushSub " << hex << static_cast(_item.data()) << dec; break; case PushSubSize: - _out << " PushSubSize " << hex << size_t(_item.data()) << dec; + _out << " PushSubSize " << hex << static_cast(_item.data()) << dec; break; case PushProgramSize: _out << " PushProgramSize"; @@ -317,7 +317,7 @@ std::string AssemblyItem::computeSourceMapping( int prevStart = -1; int prevLength = -1; int prevSourceIndex = -1; - size_t prevModifierDepth = -1; + int prevModifierDepth = -1; char prevJump = 0; for (auto const& item: _items) { @@ -328,14 +328,14 @@ std::string AssemblyItem::computeSourceMapping( int length = location.start != -1 && location.end != -1 ? location.end - location.start : -1; int sourceIndex = location.source && _sourceIndicesMap.count(location.source->name()) ? - _sourceIndicesMap.at(location.source->name()) : + static_cast(_sourceIndicesMap.at(location.source->name())) : -1; char jump = '-'; if (item.getJumpType() == evmasm::AssemblyItem::JumpType::IntoFunction) jump = 'i'; else if (item.getJumpType() == evmasm::AssemblyItem::JumpType::OutOfFunction) jump = 'o'; - size_t modifierDepth = item.m_modifierDepth; + int modifierDepth = static_cast(item.m_modifierDepth); unsigned components = 5; if (modifierDepth == prevModifierDepth) diff --git a/libevmasm/AssemblyItem.h b/libevmasm/AssemblyItem.h index fc8c63c67..6c5e01942 100644 --- a/libevmasm/AssemblyItem.h +++ b/libevmasm/AssemblyItem.h @@ -134,9 +134,9 @@ public: /// @returns an upper bound for the number of bytes required by this item, assuming that /// the value of a jump tag takes @a _addressLength bytes. unsigned bytesRequired(unsigned _addressLength) const; - int arguments() const; - int returnValues() const; - int deposit() const { return returnValues() - arguments(); } + size_t arguments() const; + size_t returnValues() const; + size_t deposit() const { return returnValues() - arguments(); } /// @returns true if the assembly item can be used in a functional context. bool canBeFunctional() const; diff --git a/libevmasm/BlockDeduplicator.cpp b/libevmasm/BlockDeduplicator.cpp index decfee650..95aba2a01 100644 --- a/libevmasm/BlockDeduplicator.cpp +++ b/libevmasm/BlockDeduplicator.cpp @@ -63,8 +63,9 @@ bool BlockDeduplicator::deduplicate() if (_j < m_items.size() && m_items.at(_j).type() == Tag) pushSecondTag = m_items.at(_j).pushTag(); - BlockIterator first{m_items.begin() + _i, m_items.end(), &pushFirstTag, &pushSelf}; - BlockIterator second{m_items.begin() + _j, m_items.end(), &pushSecondTag, &pushSelf}; + using diff_type = BlockIterator::difference_type; + BlockIterator first{m_items.begin() + diff_type(_i), m_items.end(), &pushFirstTag, &pushSelf}; + BlockIterator second{m_items.begin() + diff_type(_j), m_items.end(), &pushSecondTag, &pushSelf}; BlockIterator end{m_items.end(), m_items.end()}; if (first != end && (*first).type() == Tag) @@ -120,7 +121,7 @@ bool BlockDeduplicator::applyTagReplacement( if (it != _replacements.end()) { changed = true; - item.setPushTagSubIdAndTag(subId, size_t(it->second)); + item.setPushTagSubIdAndTag(subId, static_cast(it->second)); } } return changed; diff --git a/libevmasm/CommonSubexpressionEliminator.cpp b/libevmasm/CommonSubexpressionEliminator.cpp index 747f78f24..7fa5c938b 100644 --- a/libevmasm/CommonSubexpressionEliminator.cpp +++ b/libevmasm/CommonSubexpressionEliminator.cpp @@ -386,7 +386,11 @@ void CSECodeGenerator::generateClassElement(Id _c, bool _allowSequenced) "Opcodes with more than two arguments not implemented yet." ); for (size_t i = 0; i < arguments.size(); ++i) - assertThrow(m_stack[m_stackHeight - i] == arguments[i], OptimizerException, "Expected arguments not present." ); + assertThrow( + m_stack[m_stackHeight - static_cast(i)] == arguments[i], + OptimizerException, + "Expected arguments not present." + ); while (SemanticInformation::isCommutativeOperation(*expr.item) && !m_generatedItems.empty() && @@ -395,8 +399,8 @@ void CSECodeGenerator::generateClassElement(Id _c, bool _allowSequenced) appendOrRemoveSwap(m_stackHeight - 1, itemLocation); for (size_t i = 0; i < arguments.size(); ++i) { - m_classPositions[m_stack[m_stackHeight - i]].erase(m_stackHeight - i); - m_stack.erase(m_stackHeight - i); + m_classPositions[m_stack[m_stackHeight - static_cast(i)]].erase(m_stackHeight - static_cast(i)); + m_stack.erase(m_stackHeight - static_cast(i)); } appendItem(*expr.item); if (expr.item->type() != Operation || instructionInfo(expr.item->instruction()).ret == 1) @@ -467,7 +471,7 @@ void CSECodeGenerator::appendDup(int _fromPosition, SourceLocation const& _locat int instructionNum = 1 + m_stackHeight - _fromPosition; assertThrow(instructionNum <= 16, StackTooDeepException, "Stack too deep, try removing local variables."); assertThrow(1 <= instructionNum, OptimizerException, "Invalid stack access."); - appendItem(AssemblyItem(dupInstruction(instructionNum), _location)); + appendItem(AssemblyItem(dupInstruction(static_cast(instructionNum)), _location)); m_stack[m_stackHeight] = m_stack[_fromPosition]; m_classPositions[m_stack[m_stackHeight]].insert(m_stackHeight); } @@ -480,7 +484,7 @@ void CSECodeGenerator::appendOrRemoveSwap(int _fromPosition, SourceLocation cons int instructionNum = m_stackHeight - _fromPosition; assertThrow(instructionNum <= 16, StackTooDeepException, "Stack too deep, try removing local variables."); assertThrow(1 <= instructionNum, OptimizerException, "Invalid stack access."); - appendItem(AssemblyItem(swapInstruction(instructionNum), _location)); + appendItem(AssemblyItem(swapInstruction(static_cast(instructionNum)), _location)); if (m_stack[m_stackHeight] != m_stack[_fromPosition]) { @@ -502,5 +506,5 @@ void CSECodeGenerator::appendOrRemoveSwap(int _fromPosition, SourceLocation cons void CSECodeGenerator::appendItem(AssemblyItem const& _item) { m_generatedItems.push_back(_item); - m_stackHeight += _item.deposit(); + m_stackHeight += static_cast(_item.deposit()); } diff --git a/libevmasm/ConstantOptimiser.cpp b/libevmasm/ConstantOptimiser.cpp index 8e6d991a5..018be38ea 100644 --- a/libevmasm/ConstantOptimiser.cpp +++ b/libevmasm/ConstantOptimiser.cpp @@ -262,7 +262,7 @@ bool ComputeMethod::checkRepresentation(u256 const& _value, AssemblyItems const& { case Operation: { - if (stack.size() < size_t(item.arguments())) + if (stack.size() < item.arguments()) return false; u256* sp = &stack.back(); switch (item.instruction()) @@ -320,7 +320,7 @@ bool ComputeMethod::checkRepresentation(u256 const& _value, AssemblyItems const& bigint ComputeMethod::gasNeeded(AssemblyItems const& _routine) const { - size_t numExps = count(_routine.begin(), _routine.end(), Instruction::EXP); + auto numExps = static_cast(count(_routine.begin(), _routine.end(), Instruction::EXP)); return combineGas( simpleRunGas(_routine) + numExps * (GasCosts::expGas + GasCosts::expByteGas(m_params.evmVersion)), // Data gas for routine: Some bytes are zero, but we ignore them. diff --git a/libevmasm/ControlFlowGraph.h b/libevmasm/ControlFlowGraph.h index a0c2b9632..5fceb6388 100644 --- a/libevmasm/ControlFlowGraph.h +++ b/libevmasm/ControlFlowGraph.h @@ -45,8 +45,8 @@ public: BlockId() { *this = invalid(); } explicit BlockId(unsigned _id): m_id(_id) {} explicit BlockId(u256 const& _id); - static BlockId initial() { return BlockId(-2); } - static BlockId invalid() { return BlockId(-1); } + static BlockId initial() { return BlockId(std::numeric_limits::max() - 1); } + static BlockId invalid() { return BlockId(std::numeric_limits::max()); } bool operator==(BlockId const& _other) const { return m_id == _other.m_id; } bool operator!=(BlockId const& _other) const { return m_id != _other.m_id; } diff --git a/libevmasm/ExpressionClasses.cpp b/libevmasm/ExpressionClasses.cpp index 04686ec94..a208fabe7 100644 --- a/libevmasm/ExpressionClasses.cpp +++ b/libevmasm/ExpressionClasses.cpp @@ -190,7 +190,7 @@ ExpressionClasses::Id ExpressionClasses::tryToSimplify(Expression const& _expr) _expr.item->type() != Operation || !SemanticInformation::isDeterministic(*_expr.item) ) - return -1; + return numeric_limits::max(); if (auto match = rules.findFirstMatch(_expr, *this)) { @@ -208,7 +208,7 @@ ExpressionClasses::Id ExpressionClasses::tryToSimplify(Expression const& _expr) return rebuildExpression(ExpressionTemplate(match->action(), _expr.item->location())); } - return -1; + return numeric_limits::max(); } ExpressionClasses::Id ExpressionClasses::rebuildExpression(ExpressionTemplate const& _template) diff --git a/libevmasm/Instruction.cpp b/libevmasm/Instruction.cpp index 34fef1642..0a61fbea0 100644 --- a/libevmasm/Instruction.cpp +++ b/libevmasm/Instruction.cpp @@ -330,8 +330,8 @@ void solidity::evmasm::eachInstruction( { for (auto it = _mem.begin(); it < _mem.end(); ++it) { - Instruction instr = Instruction(*it); - size_t additional = 0; + auto instr = Instruction(*it); + int additional = 0; if (isValidInstruction(instr)) additional = instructionInfo(instr).additional; @@ -357,7 +357,7 @@ string solidity::evmasm::disassemble(bytes const& _mem) stringstream ret; eachInstruction(_mem, [&](Instruction _instr, u256 const& _data) { if (!isValidInstruction(_instr)) - ret << "0x" << std::uppercase << std::hex << int(_instr) << " "; + ret << "0x" << std::uppercase << std::hex << static_cast(_instr) << " "; else { InstructionInfo info = instructionInfo(_instr); diff --git a/libevmasm/JumpdestRemover.cpp b/libevmasm/JumpdestRemover.cpp index 6be05b198..cbb498c01 100644 --- a/libevmasm/JumpdestRemover.cpp +++ b/libevmasm/JumpdestRemover.cpp @@ -30,7 +30,7 @@ using namespace solidity::evmasm; bool JumpdestRemover::optimise(set const& _tagsReferencedFromOutside) { - set references{referencedTags(m_items, -1)}; + set references{referencedTags(m_items, numeric_limits::max())}; references.insert(_tagsReferencedFromOutside.begin(), _tagsReferencedFromOutside.end()); size_t initialSize = m_items.size(); @@ -43,7 +43,7 @@ bool JumpdestRemover::optimise(set const& _tagsReferencedFromOutside) if (_item.type() != Tag) return false; auto asmIdAndTag = _item.splitForeignPushTag(); - assertThrow(asmIdAndTag.first == size_t(-1), OptimizerException, "Sub-assembly tag used as label."); + assertThrow(asmIdAndTag.first == numeric_limits::max(), OptimizerException, "Sub-assembly tag used as label."); size_t tag = asmIdAndTag.second; return !references.count(tag); } diff --git a/libevmasm/KnownState.cpp b/libevmasm/KnownState.cpp index 5f1ca736c..10e4a4f10 100644 --- a/libevmasm/KnownState.cpp +++ b/libevmasm/KnownState.cpp @@ -41,7 +41,7 @@ ostream& KnownState::stream(ostream& _out) const if (!expr.item) _out << " no item"; else if (expr.item->type() == UndefinedItem) - _out << " unknown " << int(expr.item->data()); + _out << " unknown " << static_cast(expr.item->data()); else _out << *expr.item; if (expr.sequenceNumber) @@ -112,21 +112,21 @@ KnownState::StoreOperation KnownState::feedItem(AssemblyItem const& _item, bool setStackElement( m_stackHeight + 1, stackElement( - m_stackHeight - int(instruction) + int(Instruction::DUP1), + m_stackHeight - static_cast(instruction) + static_cast(Instruction::DUP1), _item.location() ) ); else if (SemanticInformation::isSwapInstruction(_item)) swapStackElements( m_stackHeight, - m_stackHeight - 1 - int(instruction) + int(Instruction::SWAP1), + m_stackHeight - 1 - static_cast(instruction) + static_cast(Instruction::SWAP1), _item.location() ); else if (instruction != Instruction::POP) { - vector arguments(info.args); - for (int i = 0; i < info.args; ++i) - arguments[i] = stackElement(m_stackHeight - i, _item.location()); + vector arguments(static_cast(info.args)); + for (size_t i = 0; i < static_cast(info.args); ++i) + arguments[i] = stackElement(m_stackHeight - static_cast(i), _item.location()); switch (_item.instruction()) { case Instruction::SSTORE: @@ -134,7 +134,7 @@ KnownState::StoreOperation KnownState::feedItem(AssemblyItem const& _item, bool break; case Instruction::SLOAD: setStackElement( - m_stackHeight + _item.deposit(), + m_stackHeight + static_cast(_item.deposit()), loadFromStorage(arguments[0], _item.location()) ); break; @@ -143,13 +143,13 @@ KnownState::StoreOperation KnownState::feedItem(AssemblyItem const& _item, bool break; case Instruction::MLOAD: setStackElement( - m_stackHeight + _item.deposit(), + m_stackHeight + static_cast(_item.deposit()), loadFromMemory(arguments[0], _item.location()) ); break; case Instruction::KECCAK256: setStackElement( - m_stackHeight + _item.deposit(), + m_stackHeight + static_cast(_item.deposit()), applyKeccak256(arguments.at(0), arguments.at(1), _item.location()) ); break; @@ -167,16 +167,16 @@ KnownState::StoreOperation KnownState::feedItem(AssemblyItem const& _item, bool assertThrow(info.ret <= 1, InvalidDeposit, ""); if (info.ret == 1) setStackElement( - m_stackHeight + _item.deposit(), + m_stackHeight + static_cast(_item.deposit()), m_expressionClasses->find(_item, arguments, _copyItem) ); } } m_stackElements.erase( - m_stackElements.upper_bound(m_stackHeight + _item.deposit()), + m_stackElements.upper_bound(m_stackHeight + static_cast(_item.deposit())), m_stackElements.end() ); - m_stackHeight += _item.deposit(); + m_stackHeight += static_cast(_item.deposit()); } return op; } @@ -388,7 +388,7 @@ KnownState::Id KnownState::applyKeccak256( bytes data; for (Id a: arguments) data += util::toBigEndian(*m_expressionClasses->knownConstant(a)); - data.resize(size_t(*l)); + data.resize(static_cast(*l)); v = m_expressionClasses->find(AssemblyItem(u256(util::keccak256(data)), _location)); } else diff --git a/libevmasm/LinkerObject.cpp b/libevmasm/LinkerObject.cpp index 21b213101..b7ffaac66 100644 --- a/libevmasm/LinkerObject.cpp +++ b/libevmasm/LinkerObject.cpp @@ -40,7 +40,7 @@ void LinkerObject::link(map const& _libraryAddresses) std::map remainingRefs; for (auto const& linkRef: linkReferences) if (h160 const* address = matchLibrary(linkRef.second, _libraryAddresses)) - copy(address->data(), address->data() + 20, bytecode.begin() + linkRef.first); + copy(address->data(), address->data() + 20, bytecode.begin() + vector::difference_type(linkRef.first)); else remainingRefs.insert(linkRef); linkReferences.swap(remainingRefs); diff --git a/libevmasm/PeepholeOptimiser.cpp b/libevmasm/PeepholeOptimiser.cpp index e05abd3e4..861cae0f2 100644 --- a/libevmasm/PeepholeOptimiser.cpp +++ b/libevmasm/PeepholeOptimiser.cpp @@ -84,7 +84,7 @@ struct SimplePeepholeOptimizerMethod { if ( _state.i + WindowSize <= _state.items.size() && - ApplyRule::applyRule(_state.items.begin() + _state.i, _state.out) + ApplyRule::applyRule(_state.items.begin() + static_cast(_state.i), _state.out) ) { _state.i += WindowSize; @@ -303,7 +303,7 @@ struct UnreachableCode { static bool apply(OptimiserState& _state) { - auto it = _state.items.begin() + _state.i; + auto it = _state.items.begin() + static_cast(_state.i); auto end = _state.items.end(); if (it == end) return false; @@ -317,13 +317,13 @@ struct UnreachableCode ) return false; - size_t i = 1; + ptrdiff_t i = 1; while (it + i != end && it[i].type() != Tag) i++; if (i > 1) { *_state.out = it[0]; - _state.i += i; + _state.i += static_cast(i); return true; } else @@ -345,7 +345,7 @@ void applyMethods(OptimiserState& _state, Method, OtherMethods... _other) size_t numberOfPops(AssemblyItems const& _items) { - return std::count(_items.begin(), _items.end(), Instruction::POP); + return static_cast(std::count(_items.begin(), _items.end(), Instruction::POP)); } } diff --git a/test/libevmasm/Assembler.cpp b/test/libevmasm/Assembler.cpp index a402101c4..cdb7046be 100644 --- a/test/libevmasm/Assembler.cpp +++ b/test/libevmasm/Assembler.cpp @@ -85,7 +85,7 @@ BOOST_AUTO_TEST_CASE(all_assembly_items) // PushSubSize auto sub = _assembly.appendSubroutine(_subAsmPtr); // PushSub - _assembly.pushSubroutineOffset(size_t(sub.data())); + _assembly.pushSubroutineOffset(static_cast(sub.data())); // PushDeployTimeAddress _assembly.append(PushDeployTimeAddress); // AssignImmutable. @@ -187,7 +187,7 @@ BOOST_AUTO_TEST_CASE(immutable) _assembly.appendImmutableAssignment("someOtherImmutable"); auto sub = _assembly.appendSubroutine(_subAsmPtr); - _assembly.pushSubroutineOffset(size_t(sub.data())); + _assembly.pushSubroutineOffset(static_cast(sub.data())); checkCompilation(_assembly); diff --git a/test/libevmasm/Optimiser.cpp b/test/libevmasm/Optimiser.cpp index 536eaa951..4690ceddc 100644 --- a/test/libevmasm/Optimiser.cpp +++ b/test/libevmasm/Optimiser.cpp @@ -1227,7 +1227,7 @@ BOOST_AUTO_TEST_CASE(jumpdest_removal_subassemblies) sub->append(t4.pushTag()); sub->append(Instruction::JUMP); - size_t subId = size_t(main.appendSubroutine(sub).data()); + size_t subId = static_cast(main.appendSubroutine(sub).data()); main.append(t1.toSubAssemblyTag(subId)); main.append(t1.toSubAssemblyTag(subId)); main.append(u256(8)); @@ -1281,10 +1281,10 @@ BOOST_AUTO_TEST_CASE(cse_remove_redundant_shift_masking) if (!solidity::test::CommonOptions::get().evmVersion().hasBitwiseShifting()) return; - for (int i = 1; i < 256; i++) + for (size_t i = 1; i < 256; i++) { checkCSE({ - u256(boost::multiprecision::pow(u256(2), i)-1), + u256(boost::multiprecision::pow(u256(2), i) - 1), Instruction::CALLVALUE, u256(256-i), Instruction::SHR, @@ -1309,10 +1309,10 @@ BOOST_AUTO_TEST_CASE(cse_remove_redundant_shift_masking) } // Check that opt. does NOT trigger - for (int i = 1; i < 255; i++) + for (size_t i = 1; i < 255; i++) { checkCSE({ - u256(boost::multiprecision::pow(u256(2), i)-1), + u256(boost::multiprecision::pow(u256(2), i) - 1), Instruction::CALLVALUE, u256(255-i), Instruction::SHR, From 6f8b5fe53b1e28191cbbc17fd8db499890b2abfb Mon Sep 17 00:00:00 2001 From: Mathias Baumann Date: Wed, 3 Jun 2020 13:53:11 +0200 Subject: [PATCH 6/6] Disallow override with non-public state variables --- Changelog.md | 1 + libsolidity/analysis/OverrideChecker.cpp | 5 +++++ .../inheritance/override/private_state_variable.sol | 9 +++++++++ 3 files changed, 15 insertions(+) create mode 100644 test/libsolidity/syntaxTests/inheritance/override/private_state_variable.sol diff --git a/Changelog.md b/Changelog.md index ec12a214b..a1592946b 100644 --- a/Changelog.md +++ b/Changelog.md @@ -23,6 +23,7 @@ Bugfixes: * Type Checker: Fix internal compiler error when trying to decode too large static arrays. * Type Checker: Fix wrong compiler error when referencing an overridden function without calling it. * Type Checker: Fix internal compiler error when forward referencing non-literal constants from inline assembly. + * Type Checker: Disallow usage of override with non-public state variables. * NatSpec: DocString block is terminated when encountering an empty line. * Scanner: Fix bug when two empty NatSpec comments lead to scanning past EOL. * Code Generator: Trigger proper unimplemented errors on certain array copy operations. diff --git a/libsolidity/analysis/OverrideChecker.cpp b/libsolidity/analysis/OverrideChecker.cpp index 4b9a3b7f3..89f9a6637 100644 --- a/libsolidity/analysis/OverrideChecker.cpp +++ b/libsolidity/analysis/OverrideChecker.cpp @@ -481,7 +481,12 @@ void OverrideChecker::checkIllegalOverrides(ContractDefinition const& _contract) for (auto const* stateVar: _contract.stateVariables()) { if (!stateVar->isPublic()) + { + if (stateVar->overrides()) + m_errorReporter.typeError(8022_error, stateVar->location(), "Override can only be used with public state variables."); + continue; + } if (contains_if(inheritedMods, MatchByName{stateVar->name()})) m_errorReporter.typeError(1456_error, stateVar->location(), "Override changes modifier to public state variable."); diff --git a/test/libsolidity/syntaxTests/inheritance/override/private_state_variable.sol b/test/libsolidity/syntaxTests/inheritance/override/private_state_variable.sol new file mode 100644 index 000000000..400da4a8e --- /dev/null +++ b/test/libsolidity/syntaxTests/inheritance/override/private_state_variable.sol @@ -0,0 +1,9 @@ +contract C1 { + function f() external pure returns(int) { return 42; } +} + +contract C is C1 { + int override f; +} +// ---- +// TypeError: (96-110): Override can only be used with public state variables.