Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make messages mandatory in errors #15671

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
+70 −64
Open

Make messages mandatory in errors #15671

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c0612d1
ErrorReporter: Require all errors to have a non-empty message
cameel Dec 7, 2024
dcbd7a3
Make message mandatory in solUnimplementedAssert
cameel Dec 7, 2024
923a483
Provide messages for all instances of solUnimplemented() and solUnimp…
cameel Dec 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions liblangutil/ErrorReporter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ void ErrorReporter::warning(

void ErrorReporter::error(ErrorId _errorId, Error::Type _type, SourceLocation const& _location, std::string const& _description)
{
solAssert(!_description.empty(), "Errors must include a message for the user.");
if (checkForExcessiveErrors(_type))
return;

Expand All @@ -71,6 +72,7 @@ void ErrorReporter::error(ErrorId _errorId, Error::Type _type, SourceLocation co

void ErrorReporter::error(ErrorId _errorId, Error::Type _type, SourceLocation const& _location, SecondarySourceLocation const& _secondaryLocation, std::string const& _description)
{
solAssert(!_description.empty(), "Errors must include a message for the user.");
if (checkForExcessiveErrors(_type))
return;

Expand Down
11 changes: 1 addition & 10 deletions liblangutil/Exceptions.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,16 +72,7 @@ struct InvalidAstError: virtual util::Exception {};


/// Assertion that throws an UnimplementedFeatureError containing the given description if it is not met.
#if !BOOST_PP_VARIADICS_MSVC
#define solUnimplementedAssert(...) BOOST_PP_OVERLOAD(solUnimplementedAssert_,__VA_ARGS__)(__VA_ARGS__)
#else
#define solUnimplementedAssert(...) BOOST_PP_CAT(BOOST_PP_OVERLOAD(solUnimplementedAssert_,__VA_ARGS__)(__VA_ARGS__),BOOST_PP_EMPTY())
#endif

#define solUnimplementedAssert_1(CONDITION) \
solUnimplementedAssert_2((CONDITION), "")

#define solUnimplementedAssert_2(CONDITION, DESCRIPTION) \
#define solUnimplementedAssert(CONDITION, DESCRIPTION) \
assertThrowWithDefaultDescription( \
(CONDITION), \
::solidity::langutil::UnimplementedFeatureError, \
Expand Down
2 changes: 1 addition & 1 deletion libsolidity/analysis/ControlFlowBuilder.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -604,7 +604,7 @@ void ControlFlowBuilder::operator()(yul::FunctionDefinition const&)
void ControlFlowBuilder::operator()(yul::Leave const&)
{
// This has to be implemented, if we ever decide to visit functions.
solUnimplemented("");
solAssert(false);
}

bool ControlFlowBuilder::visit(VariableDeclaration const& _variableDeclaration)
Expand Down
10 changes: 7 additions & 3 deletions libsolidity/codegen/ExpressionCompiler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2302,11 +2302,12 @@ bool ExpressionCompiler::visit(IndexRangeAccess const& _indexAccess)
if (ArraySliceType const* sliceType = dynamic_cast<ArraySliceType const*>(&baseType))
arrayType = &sliceType->arrayType();

solAssert(arrayType, "");
solAssert(arrayType);
solUnimplementedAssert(
arrayType->location() == DataLocation::CallData &&
arrayType->isDynamicallySized() &&
!arrayType->baseType()->isDynamicallyEncoded()
!arrayType->baseType()->isDynamicallyEncoded(),
"Slicing is supported only for dynamically-sized calldata arrays of statically-encoded types."
);

if (_indexAccess.startExpression())
Expand Down Expand Up @@ -2440,7 +2441,10 @@ void ExpressionCompiler::appendCompareOperatorCode(Token _operator, Type const&
FunctionType const* functionType = dynamic_cast<decltype(functionType)>(&_type);
if (functionType && functionType->kind() == FunctionType::Kind::External)
{
solUnimplementedAssert(functionType->sizeOnStack() == 2, "");
solUnimplementedAssert(
functionType->sizeOnStack() == 2,
"Comparisons are implemented only for external function types."
);
m_context << Instruction::SWAP3;

m_context << ((u256(1) << 160) - 1) << Instruction::AND;
Expand Down
8 changes: 4 additions & 4 deletions libsolidity/codegen/LValue.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ ImmutableItem::ImmutableItem(CompilerContext& _compilerContext, VariableDeclarat

void ImmutableItem::retrieveValue(SourceLocation const&, bool) const
{
solUnimplementedAssert(m_dataType->isValueType());
solUnimplementedAssert(m_dataType->isValueType(), "Immutables of non-value types not supported yet.");

if (m_context.runtimeContext())
CompilerUtils(m_context).loadFromMemory(
Expand All @@ -172,8 +172,8 @@ void ImmutableItem::retrieveValue(SourceLocation const&, bool) const
void ImmutableItem::storeValue(Type const& _sourceType, SourceLocation const&, bool _move) const
{
CompilerUtils utils(m_context);
solUnimplementedAssert(m_dataType->isValueType());
solAssert(_sourceType.isValueType(), "");
solUnimplementedAssert(m_dataType->isValueType(), "Immutables of non-value types not supported yet.");
solAssert(_sourceType.isValueType());

utils.convertType(_sourceType, *m_dataType, true);
m_context << m_context.immutableMemoryOffset(m_variable);
Expand All @@ -188,7 +188,7 @@ void ImmutableItem::storeValue(Type const& _sourceType, SourceLocation const&, b
void ImmutableItem::setToZero(SourceLocation const&, bool _removeReference) const
{
CompilerUtils utils(m_context);
solUnimplementedAssert(m_dataType->isValueType());
solUnimplementedAssert(m_dataType->isValueType(), "Immutables of non-value types not supported yet.");
solAssert(_removeReference);

m_context << m_context.immutableMemoryOffset(m_variable);
Expand Down
37 changes: 23 additions & 14 deletions libsolidity/codegen/YulUtilFunctions.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1364,8 +1364,8 @@ std::string YulUtilFunctions::extractByteArrayLengthFunction()

std::string YulUtilFunctions::resizeArrayFunction(ArrayType const& _type)
{
solAssert(_type.location() == DataLocation::Storage, "");
solUnimplementedAssert(_type.baseType()->storageBytes() <= 32);
solAssert(_type.location() == DataLocation::Storage);
solUnimplementedAssert(_type.baseType()->storageBytes() <= 32, "Resizing not implemented for packed arrays.");

if (_type.isByteArrayOrString())
return resizeDynamicByteArrayFunction(_type);
Expand Down Expand Up @@ -1404,10 +1404,10 @@ std::string YulUtilFunctions::resizeArrayFunction(ArrayType const& _type)

std::string YulUtilFunctions::cleanUpStorageArrayEndFunction(ArrayType const& _type)
{
solAssert(_type.location() == DataLocation::Storage, "");
solAssert(_type.baseType()->category() != Type::Category::Mapping, "");
solAssert(!_type.isByteArrayOrString(), "");
solUnimplementedAssert(_type.baseType()->storageBytes() <= 32);
solAssert(_type.location() == DataLocation::Storage);
solAssert(_type.baseType()->category() != Type::Category::Mapping);
solAssert(!_type.isByteArrayOrString());
solUnimplementedAssert(_type.baseType()->storageBytes() <= 32, "Resizing not implemented for packed arrays.");

std::string functionName = "cleanup_storage_array_end_" + _type.identifier();
return m_functionCollector.createFunction(functionName, [&](std::vector<std::string>& _args, std::vector<std::string>&) {
Expand Down Expand Up @@ -1698,12 +1698,15 @@ std::string YulUtilFunctions::storageByteArrayPopFunction(ArrayType const& _type

std::string YulUtilFunctions::storageArrayPushFunction(ArrayType const& _type, Type const* _fromType)
{
solAssert(_type.location() == DataLocation::Storage, "");
solAssert(_type.isDynamicallySized(), "");
solAssert(_type.location() == DataLocation::Storage);
solAssert(_type.isDynamicallySized());
if (!_fromType)
_fromType = _type.baseType();
else if (_fromType->isValueType())
solUnimplementedAssert(*_fromType == *_type.baseType());
solUnimplementedAssert(
*_fromType == *_type.baseType(),
".push() to a storage array with a different base type not implemented."
);

std::string functionName =
std::string{"array_push_from_"} +
Expand Down Expand Up @@ -3527,10 +3530,10 @@ std::string YulUtilFunctions::conversionFunction(Type const& _from, Type const&
if (auto const* toFixedBytes = dynamic_cast<FixedBytesType const*>(&_to))
convert = shiftLeftFunction(256 - toFixedBytes->numBytes() * 8);
else if (dynamic_cast<FixedPointType const*>(&_to))
solUnimplemented("");
solUnimplemented("Conversions from integers to fixed-point types not implemented");
else if (dynamic_cast<IntegerType const*>(&_to))
{
solUnimplementedAssert(fromCategory != Type::Category::FixedPoint);
solUnimplementedAssert(fromCategory != Type::Category::FixedPoint, "Fixed point types not implemented.");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you add a semantic test for this (or any other assert)? The legacy codegen one has a different error message, which is not ideal, but at this point I don't think it makes sense to invest the time in updating them.

convert = identityFunction();
}
else if (toCategory == Type::Category::Enum)
Expand Down Expand Up @@ -3569,8 +3572,14 @@ std::string YulUtilFunctions::conversionFunction(Type const& _from, Type const&
body = "converted := value";
else
{
solUnimplementedAssert(toStructType.location() == DataLocation::Memory);
solUnimplementedAssert(fromStructType.location() != DataLocation::Memory);
solUnimplementedAssert(
toStructType.location() == DataLocation::Memory,
"Conversions changing the location of a struct to non-memory not implemented."
);
solUnimplementedAssert(
fromStructType.location() != DataLocation::Memory,
"Conversions changing the location of a memory struct not implemented."
);

if (fromStructType.location() == DataLocation::CallData)
body = Whiskers(R"(
Expand Down Expand Up @@ -4335,7 +4344,7 @@ std::string YulUtilFunctions::zeroValueFunction(Type const& _type, bool _splitFu
else if (auto const* structType = dynamic_cast<StructType const*>(&_type))
templ("zeroValue", allocateAndInitializeMemoryStructFunction(*structType) + "()");
else
solUnimplemented("");
solUnimplemented("zeroValue for type " + _type.identifier() + " not yet implemented!");
}

return templ.render();
Expand Down
4 changes: 2 additions & 2 deletions libsolidity/codegen/ir/IRGenerationContext.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,8 +153,8 @@ size_t IRGenerationContext::reservedMemory()
size_t immutableVariablesSize = 0;
for (auto const* var: keys(m_immutableVariables))
{
solUnimplementedAssert(var->type()->isValueType());
solUnimplementedAssert(var->type()->sizeOnStack() == 1);
solUnimplementedAssert(var->type()->isValueType(), "Immutables of non-value types not supported yet.");
solUnimplementedAssert(var->type()->sizeOnStack() == 1, "Multi-slot immutables not supported yet.");
immutableVariablesSize += var->type()->sizeOnStack() * 32;
}

Expand Down
6 changes: 3 additions & 3 deletions libsolidity/codegen/ir/IRGenerator.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -562,7 +562,7 @@ std::string IRGenerator::generateGetter(VariableDeclaration const& _varDecl)
if (_varDecl.immutable())
{
solAssert(paramTypes.empty(), "");
solUnimplementedAssert(type->sizeOnStack() == 1);
solUnimplementedAssert(type->sizeOnStack() == 1, "Multi-slot immutables not supported yet.");

auto t = Whiskers(R"(
<astIDComment><sourceLocationComment>
Expand Down Expand Up @@ -1019,8 +1019,8 @@ std::string IRGenerator::deployCode(ContractDefinition const& _contract)
{
for (VariableDeclaration const* immutable: ContractType(_contract).immutableVariables())
{
solUnimplementedAssert(immutable->type()->isValueType());
solUnimplementedAssert(immutable->type()->sizeOnStack() == 1);
solUnimplementedAssert(immutable->type()->isValueType(), "Immutables of non-value types not supported yet.");
solUnimplementedAssert(immutable->type()->sizeOnStack() == 1, "Multi-slot immutables not supported yet.");
if (!eof)
immutables.emplace_back(std::map<std::string, std::string>{
{"immutableName"s, std::to_string(immutable->id())},
Expand Down
16 changes: 8 additions & 8 deletions libsolidity/codegen/ir/IRGeneratorForStatements.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ struct CopyTranslate: public yul::ASTCopier
{
auto const& reference = m_references.at(&_identifier);
auto const varDecl = dynamic_cast<VariableDeclaration const*>(reference.declaration);
solUnimplementedAssert(varDecl);
solUnimplementedAssert(varDecl, "translateReference() is only implemented for variable declarations.");
std::string const& suffix = reference.suffix;

std::string value;
Expand Down Expand Up @@ -888,7 +888,7 @@ bool IRGeneratorForStatements::visit(BinaryOperation const& _binOp)

if (functionType && functionType->kind() == FunctionType::Kind::External)
{
solUnimplementedAssert(functionType->sizeOnStack() == 2, "");
solUnimplementedAssert(functionType->sizeOnStack() == 2, "Comparisons are implemented only for external function types.");
expr = m_utils.externalFunctionPointersEqualFunction() +
"(" +
IRVariable{_binOp.leftExpression()}.part("address").name() + "," +
Expand Down Expand Up @@ -1732,7 +1732,7 @@ void IRGeneratorForStatements::endVisit(FunctionCallOptions const& _options)
setLocation(_options);
FunctionType const& previousType = dynamic_cast<FunctionType const&>(*_options.expression().annotation().type);

solUnimplementedAssert(!previousType.hasBoundFirstArgument());
solUnimplementedAssert(!previousType.hasBoundFirstArgument(), "Call options not implemented for bound library functions.");

// Copy over existing values.
for (auto const& item: previousType.stackItems())
Expand Down Expand Up @@ -1913,7 +1913,7 @@ void IRGeneratorForStatements::endVisit(MemberAccess const& _memberAccess)
}
else if (member == "address")
{
solUnimplementedAssert(
solAssert(
dynamic_cast<FunctionType const&>(*_memberAccess.expression().annotation().type).kind() ==
FunctionType::Kind::External
);
Expand Down Expand Up @@ -3202,8 +3202,8 @@ void IRGeneratorForStatements::writeToLValue(IRLValue const& _lvalue, IRVariable
[&](IRLValue::Stack const& _stack) { assign(_stack.variable, _value); },
[&](IRLValue::Immutable const& _immutable)
{
solUnimplementedAssert(_lvalue.type.isValueType());
solUnimplementedAssert(_lvalue.type.sizeOnStack() == 1);
solUnimplementedAssert(_lvalue.type.isValueType(), "Immutables of non-value types not supported yet.");
solUnimplementedAssert(_lvalue.type.sizeOnStack() == 1, "Multi-slot immutables not supported yet.");
solAssert(_lvalue.type == *_immutable.variable->type());
size_t memOffset = m_context.immutableMemoryOffset(*_immutable.variable);

Expand Down Expand Up @@ -3280,8 +3280,8 @@ IRVariable IRGeneratorForStatements::readFromLValue(IRLValue const& _lvalue)
define(result, _stack.variable);
},
[&](IRLValue::Immutable const& _immutable) {
solUnimplementedAssert(_lvalue.type.isValueType());
solUnimplementedAssert(_lvalue.type.sizeOnStack() == 1);
solUnimplementedAssert(_lvalue.type.isValueType(), "Immutables of non-value types not supported yet.");
solUnimplementedAssert(_lvalue.type.sizeOnStack() == 1, "Multi-slot immutables not supported yet.");
solAssert(_lvalue.type == *_immutable.variable->type());
if (m_context.executionContext() == IRGenerationContext::ExecutionContext::Creation)
{
Expand Down
2 changes: 1 addition & 1 deletion libsolidity/experimental/codegen/IRGeneratorForStatements.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ bool IRGeneratorForStatements::visit(TupleExpression const& _tupleExpression)
std::vector<std::string> components;
for (auto const& component: _tupleExpression.components())
{
solUnimplementedAssert(component);
solUnimplementedAssert(component, "Tuple expressions with empty components not supported.");
component->accept(*this);
components.emplace_back(IRNames::localVariable(*component));
}
Expand Down
Loading
Loading