Skip to content

Rework purity flag setting for constant variables accessed via member access. #16376

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

Open
rodiazet wants to merge 9 commits into
base: purity-flag-struct
Choose a base branch
from
Open

Rework purity flag setting for constant variables accessed via member access. #16376

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
820f20f
Member access via contract instance.
rodiazet Mar 20, 2026
da755f8
Member access via a contract type name.
rodiazet Mar 19, 2026
1ad02a1
Member access via module type.
rodiazet Mar 20, 2026
871ac53
Add accessible variables sanity check.
rodiazet Mar 20, 2026
1fee9bc
Add error arguments purity tests.
rodiazet Mar 23, 2026
5c407e8
Add constant access getter test which reproduces the bug.
rodiazet Mar 23, 2026
75b7fcd
Add purity tests of different types and usage.
rodiazet Mar 23, 2026
d477a6a
Fix `constantValueFunction` to properly support function returning mo…
rodiazet Mar 25, 2026
93fd415
Add semantic tests.
rodiazet Mar 25, 2026
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
195 changes: 175 additions & 20 deletions libsolidity/analysis/TypeChecker.cpp
View file
Open in desktop
Copy link
Copy Markdown
Collaborator

@cameel cameel Feb 2, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

The PR is missing a changelog entry. With how much it's changing, we'll probably need more than one.

Original file line number Diff line number Diff line change
Expand Up @@ -3329,6 +3329,19 @@ bool TypeChecker::visit(MemberAccess const& _memberAccess)

_memberAccess.annotation().requiredLookup = requiredLookup;

// Sanity check. Only module, struct and contract instances as well as contract types can have accessible variables.
if (dynamic_cast<VariableDeclaration const*>(_memberAccess.annotation().referencedDeclaration))
{
if (owningObjectType->category() == Type::Category::TypeType)
solAssert(static_cast<TypeType const*>(owningObjectType)->actualType()->category() == Type::Category::Contract);
else
solAssert(
owningObjectType->category() == Type::Category::Module ||
owningObjectType->category() == Type::Category::Struct ||
owningObjectType->category() == Type::Category::Contract
);
}

switch (owningObjectType->category())
{
case Type::Category::Struct:
Expand Down Expand Up @@ -3430,13 +3443,75 @@ bool TypeChecker::visit(MemberAccess const& _memberAccess)
}
case Type::Category::Contract:
{
// Member of a contract accessed by the contract name (not contract instance).

// ContractType has only user-defined members, so accessedMemberAnnotation.referencedDeclaration is not `NULL`.
// See `ContractType::nativeMembers` for details.
solAssert(_memberAccess.annotation().referencedDeclaration);
_memberAccess.annotation().isLValue = _memberAccess.annotation().referencedDeclaration->isLValue();
if (
auto const* accessedMemberFunctionType = dynamic_cast<FunctionType const*>(type(_memberAccess));
accessedMemberFunctionType &&
accessedMemberFunctionType->kind() == FunctionType::Kind::Declaration
)
_memberAccess.annotation().isPure = *_memberAccess.expression().annotation().isPure;
// Expressions like `C.foo;`, `C.Ev;` are pure and they must generate `Statement has no effect.` warning.
// TODO: However, in case a function this does not allow to assign the expression to a constant variable,
// TODO: because of different kind. Left-hand side of the variable declaration never has `Declaration` kind.
if (auto const* functionTypeMember = dynamic_cast<FunctionType const*>(_memberAccess.annotation().type))
{
// By default, all pure function invocation kinds are pure. Additionally, `C.Ev` is pure too.
// Note: This means also that a member function of a foreign contract accessed via the contract type
// name is pure, but a member function of a library accessed via the library name is not pure, because
// `C.foo` cannot be called (it needs a contract instance instead), but `Lib.foo` can be called.
if (
functionTypeMember->isPure() ||
functionTypeMember->kind() == FunctionType::Kind::Event
)
_memberAccess.annotation().isPure = true;
else if (functionTypeMember->kind() == FunctionType::Kind::Internal)
{
// A variable declaration of constant function pointer.
if (
auto const* variableDeclarationMember =
dynamic_cast<VariableDeclaration const*>(_memberAccess.annotation().referencedDeclaration)
)
_memberAccess.annotation().isPure = variableDeclarationMember->isConstant();
else if (dynamic_cast<FunctionDefinition const*>(_memberAccess.annotation().referencedDeclaration))
_memberAccess.annotation().isPure = true;
else
solAssert(false, "Impossible declaration type for internal function call kind");
}
else if (functionTypeMember->kind() == FunctionType::Kind::External)
{
// A variable declaration of constant function pointer.
if (
auto const* variableDeclarationMember =
dynamic_cast<VariableDeclaration const*>(_memberAccess.annotation().referencedDeclaration)
)
_memberAccess.annotation().isPure = variableDeclarationMember->isConstant();
else
solAssert(
dynamic_cast<FunctionDefinition const*>(_memberAccess.annotation().referencedDeclaration),
"Impossible declaration type for external function call kind"
);
}
else
// Library function declaration is not pure. It requires the library address.
solAssert(functionTypeMember->kind() == FunctionType::Kind::DelegateCall);
}
else if (auto const* typeTypeMember = dynamic_cast<TypeType const*>(_memberAccess.annotation().type))
{
solAssert(
typeTypeMember->actualType()->category() == Type::Category::Struct ||
typeTypeMember->actualType()->category() == Type::Category::Enum ||
// Note: We add Contract intentionally, to cover a possible contract nesting case.
typeTypeMember->actualType()->category() == Type::Category::Contract ||
typeTypeMember->actualType()->category() == Type::Category::UserDefinedValueType,
"Impossible `TypeType` category as contract member."
);
_memberAccess.annotation().isPure = true;
}
// In case `Base.value` or `Lib.value` and when `value` is constant, the expression is pure.
else if (auto const* varDecl = dynamic_cast<VariableDeclaration const*>(_memberAccess.annotation().referencedDeclaration))
_memberAccess.annotation().isPure = varDecl->isConstant();
else
solAssert(false, "Unexpected annotation type");

break;
}
case Type::Category::Enum:
Expand Down Expand Up @@ -3547,9 +3622,57 @@ bool TypeChecker::visit(MemberAccess const& _memberAccess)
break;
}
case Type::Category::Module:
_memberAccess.annotation().isPure = *_memberAccess.expression().annotation().isPure;
{
// Module has only exported symbols members, so accessedMemberAnnotation.referencedDeclaration is not `NULL`.
// See `ModuleType::nativeMembers` for details.
solAssert(_memberAccess.annotation().referencedDeclaration);
// All currently accessible members via a module type are pure.
_memberAccess.annotation().isPure = true;
_memberAccess.annotation().isLValue = false;

// Below only the sanity checks.
solAssert(
_memberAccess.annotation().type->category() == Type::Category::Function ||
_memberAccess.annotation().type->category() == Type::Category::TypeType ||
_memberAccess.annotation().type->category() == Type::Category::Module ||
dynamic_cast<VariableDeclaration const*>(_memberAccess.annotation().referencedDeclaration),
"Impossible member type for module type member access"
);

if (_memberAccess.annotation().type->category() == Type::Category::Function)
{
auto const* functionTypeMember = static_cast<FunctionType const*>(_memberAccess.annotation().type);
solAssert (
functionTypeMember->isPure() ||
functionTypeMember->kind() == FunctionType::Kind::Event ||
functionTypeMember->kind() == FunctionType::Kind::Internal,
"Impossible declaration type for function call kind"
);

if (functionTypeMember->kind() == FunctionType::Kind::Internal)
solAssert(dynamic_cast<FunctionDefinition const*>(_memberAccess.annotation().referencedDeclaration), "Impossible declaration type for internal function call kind");
}

if (_memberAccess.annotation().type->category() == Type::Category::TypeType)
{
auto const* typeTypeMember = static_cast<TypeType const*>(_memberAccess.annotation().type);
solAssert(
typeTypeMember->actualType()->category() == Type::Category::Struct ||
typeTypeMember->actualType()->category() == Type::Category::Enum ||
typeTypeMember->actualType()->category() == Type::Category::Contract ||
typeTypeMember->actualType()->category() == Type::Category::UserDefinedValueType,
"Impossible `TypeType` category as module member."
);
}

if (
auto const* accessedMemberVariableDeclaration =
dynamic_cast<VariableDeclaration const*>(_memberAccess.annotation().referencedDeclaration)
)
solAssert(accessedMemberVariableDeclaration->isConstant(), "Only constant variables are allowed at file level.");

break;
}
case Type::Category::Address:
if (memberName == "codehash" && !m_evmVersion.hasExtCodeHash())
m_errorReporter.typeError(
Expand All @@ -3559,6 +3682,47 @@ bool TypeChecker::visit(MemberAccess const& _memberAccess)
);
_memberAccess.annotation().isLValue = false;
break;
// Contract instance case
case Type::Category::Contract:
{
solAssert(
_memberAccess.annotation().type->category() == Type::Category::Function,
"Via contract instance only a function or a variable getter can be accessed."
);
// When contract is constant its members are also constant.
_memberAccess.annotation().isPure = *_memberAccess.expression().annotation().isPure;
_memberAccess.annotation().isLValue = false;

// Below only the sanity checks.
if (dynamic_cast<FunctionDefinition const*>(_memberAccess.annotation().referencedDeclaration))
{
auto const* accessedMemberFunctionType = static_cast<FunctionType const*>(_memberAccess.annotation().type);
// In case when an internal library function is attached to a contract, the function invoke kind can be
// `Internal` or `DelegateCall`. It depends on the function declaration in the library.
solAssert(
accessedMemberFunctionType->kind() == FunctionType::Kind::Internal ||
accessedMemberFunctionType->kind() == FunctionType::Kind::External ||
accessedMemberFunctionType->kind() == FunctionType::Kind::DelegateCall,
"Impossible function call kind for contract type member."
);
}
else if (dynamic_cast<VariableDeclaration const*>(_memberAccess.annotation().referencedDeclaration))
{
// If a constant variable of contract type (owning expression) is pure, then the accessed member is pure.
// Note: It does not matter that the being accessed declaration is constant, because when accessing via
// a contract instance we always have a getter function but not the variable itself. Moreover, the getter of
// a constant variable contained by non-constant contract should not be constant.
auto const* accessedMemberFunctionType = static_cast<FunctionType const*>(_memberAccess.annotation().type);
solAssert(
accessedMemberFunctionType->kind() == FunctionType::Kind::External,
"Impossible function call kind for contract type member."
);
}
else
solAssert(false, "Invalid declaration type for contract instance member.");

break;
}
case Type::Category::Integer:
case Type::Category::RationalNumber:
case Type::Category::StringLiteral:
Expand All @@ -3567,7 +3731,6 @@ bool TypeChecker::visit(MemberAccess const& _memberAccess)
case Type::Category::FixedBytes:
case Type::Category::Array:
case Type::Category::ArraySlice:
case Type::Category::Contract:
case Type::Category::Enum:
case Type::Category::UserDefinedValueType:
case Type::Category::Tuple:
Expand All @@ -3580,18 +3743,10 @@ bool TypeChecker::visit(MemberAccess const& _memberAccess)

solAssert(_memberAccess.annotation().isLValue.set());

// TODO: Leave it for now, but it should be moved to TypeType -> Contract case.
// We do not want to change the logic in refactor PR.
if (
auto const* varDecl = dynamic_cast<VariableDeclaration const*>(_memberAccess.annotation().referencedDeclaration);
!_memberAccess.annotation().isPure.set() &&
varDecl &&
varDecl->isConstant()
)
{
solAssert(owningObjectType->category() != Type::Category::Magic);
_memberAccess.annotation().isPure = true;
}
// // TODO: Leave it for now, but it should be moved to TypeType -> Contract case.
// // We do not want to change the logic in refactor PR.
// if (dynamic_cast<VariableDeclaration const*>(accessedMemberAnnotation.referencedDeclaration))
// solAssert(accessedMemberAnnotation.isPure.set());


if (!_memberAccess.annotation().isPure.set())
Expand Down
4 changes: 3 additions & 1 deletion libsolidity/ast/Types.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3714,7 +3714,9 @@ bool FunctionType::isPure() const
m_kind == Kind::Unwrap ||
m_kind == Kind::BytesConcat ||
m_kind == Kind::StringConcat ||
m_kind == Kind::ERC7201;
m_kind == Kind::ERC7201 ||
m_kind == Kind::Error ||
m_kind == Kind::Declaration;
}

TypePointers FunctionType::parseElementaryTypeVector(strings const& _types)
Expand Down
4 changes: 3 additions & 1 deletion libsolidity/codegen/ir/IRGeneratorForStatements.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -346,14 +346,16 @@ std::string IRGeneratorForStatements::constantValueFunction(VariableDeclaration
<sourceLocationComment>
function <functionName>() -> <ret> {
<code>
<ret> := <value>
<ret> := <identity>(<value>)
}
)");
templ("sourceLocationComment", dispenseLocationComment(_constant, m_context));
templ("functionName", functionName);
IRGeneratorForStatements generator(m_context, m_utils, m_optimiserSettings);
solAssert(_constant.value());
Type const& constantType = *_constant.type();
// Need to use the convert function which is an identity function to handle a case with more than one value.
templ("identity", m_utils.conversionFunction(constantType, constantType));
templ("value", generator.evaluateExpression(*_constant.value(), constantType).commaSeparatedList());
templ("code", generator.code());
templ("ret", IRVariable("ret", constantType).commaSeparatedList());
Expand Down
36 changes: 18 additions & 18 deletions test/cmdlineTests/standard_debug_info_in_yul_location/output.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -303,14 +303,18 @@ object \"C_54\" {

}

function cleanup_t_rational_41_by_1(value) -> cleaned {
cleaned := value
}

function identity(value) -> ret {
ret := value
}

function convert_t_int256_to_t_int256(value) -> converted {
converted := cleanup_t_int256(identity(cleanup_t_int256(value)))
}

function cleanup_t_rational_41_by_1(value) -> cleaned {
cleaned := value
}

function convert_t_rational_41_by_1_to_t_int256(value) -> converted {
converted := cleanup_t_int256(identity(cleanup_t_rational_41_by_1(value)))
}
Expand All @@ -321,7 +325,7 @@ object \"C_54\" {
let expr_4 := 0x29
let _1 := convert_t_rational_41_by_1_to_t_int256(expr_4)

ret := _1
ret := convert_t_int256_to_t_int256(_1)
}

/// @ast-id 5
Expand Down Expand Up @@ -428,10 +432,6 @@ object \"C_54\" {
result := or(value, and(toInsert, mask))
}

function convert_t_int256_to_t_int256(value) -> converted {
converted := cleanup_t_int256(identity(cleanup_t_int256(value)))
}

function prepare_store_t_int256(value) -> ret {
ret := value
}
Expand Down Expand Up @@ -1140,14 +1140,18 @@ object \"D_72\" {

}

function cleanup_t_rational_41_by_1(value) -> cleaned {
cleaned := value
}

function identity(value) -> ret {
ret := value
}

function convert_t_int256_to_t_int256(value) -> converted {
converted := cleanup_t_int256(identity(cleanup_t_int256(value)))
}

function cleanup_t_rational_41_by_1(value) -> cleaned {
cleaned := value
}

function convert_t_rational_41_by_1_to_t_int256(value) -> converted {
converted := cleanup_t_int256(identity(cleanup_t_rational_41_by_1(value)))
}
Expand All @@ -1158,7 +1162,7 @@ object \"D_72\" {
let expr_4 := 0x29
let _1 := convert_t_rational_41_by_1_to_t_int256(expr_4)

ret := _1
ret := convert_t_int256_to_t_int256(_1)
}

/// @ast-id 5
Expand Down Expand Up @@ -1265,10 +1269,6 @@ object \"D_72\" {
result := or(value, and(toInsert, mask))
}

function convert_t_int256_to_t_int256(value) -> converted {
converted := cleanup_t_int256(identity(cleanup_t_int256(value)))
}

function prepare_store_t_int256(value) -> ret {
ret := value
}
Expand Down
Loading