mirror of
https://github.com/ethereum/solidity
synced 2023-10-03 13:03:40 +00:00
Merge pull request #10908 from a3d4/fix-9231-struct-member-names-shadow-type-names
Fix shadowing struct types by struct member names
This commit is contained in:
commit
0fff4e6743
@ -21,6 +21,7 @@ Bugfixes:
|
||||
* Control Flow Graph: Take internal calls to functions that always revert into account for reporting unused or unassigned variables.
|
||||
* Control Flow Graph: Assume unimplemented modifiers use a placeholder.
|
||||
* Function Call Graph: Fix internal error connected with circular constant references.
|
||||
* Name Resolver: Do not issue shadowing warning if the shadowing name is not directly accessible.
|
||||
* Natspec: Allow multiple ``@return`` tags on public state variable documentation.
|
||||
* SMTChecker: Fix internal error on struct constructor with fixed bytes member initialized with string literal.
|
||||
* SMTChecker: Fix internal error on external calls from the constructor.
|
||||
|
@ -26,6 +26,9 @@
|
||||
#include <libsolidity/ast/AST.h>
|
||||
#include <libsolutil/StringUtils.h>
|
||||
|
||||
#include <range/v3/view/filter.hpp>
|
||||
#include <range/v3/range/conversion.hpp>
|
||||
|
||||
using namespace std;
|
||||
using namespace solidity;
|
||||
using namespace solidity::frontend;
|
||||
@ -120,11 +123,7 @@ bool DeclarationContainer::registerDeclaration(
|
||||
if (conflictingDeclaration(_declaration, _name))
|
||||
return false;
|
||||
|
||||
// Do not warn about shadowing for structs and enums because their members are
|
||||
// not accessible without prefixes. Also do not warn about event parameters
|
||||
// because they do not participate in any proper scope.
|
||||
bool special = _declaration.scope() && (_declaration.isStructMember() || _declaration.isEnumValue() || _declaration.isEventOrErrorParameter());
|
||||
if (m_enclosingContainer && !special)
|
||||
if (m_enclosingContainer && _declaration.isVisibleAsUnqualifiedName())
|
||||
m_homonymCandidates.emplace_back(*_name, _location ? _location : &_declaration.location());
|
||||
}
|
||||
|
||||
@ -143,16 +142,35 @@ bool DeclarationContainer::registerDeclaration(
|
||||
return registerDeclaration(_declaration, nullptr, nullptr, _invisible, _update);
|
||||
}
|
||||
|
||||
vector<Declaration const*> DeclarationContainer::resolveName(ASTString const& _name, bool _recursive, bool _alsoInvisible) const
|
||||
vector<Declaration const*> DeclarationContainer::resolveName(
|
||||
ASTString const& _name,
|
||||
bool _recursive,
|
||||
bool _alsoInvisible,
|
||||
bool _onlyVisibleAsUnqualifiedNames
|
||||
) const
|
||||
{
|
||||
solAssert(!_name.empty(), "Attempt to resolve empty name.");
|
||||
vector<Declaration const*> result;
|
||||
|
||||
if (m_declarations.count(_name))
|
||||
result = m_declarations.at(_name);
|
||||
{
|
||||
if (_onlyVisibleAsUnqualifiedNames)
|
||||
result += m_declarations.at(_name) | ranges::views::filter(&Declaration::isVisibleAsUnqualifiedName) | ranges::to_vector;
|
||||
else
|
||||
result += m_declarations.at(_name);
|
||||
}
|
||||
|
||||
if (_alsoInvisible && m_invisibleDeclarations.count(_name))
|
||||
result += m_invisibleDeclarations.at(_name);
|
||||
{
|
||||
if (_onlyVisibleAsUnqualifiedNames)
|
||||
result += m_invisibleDeclarations.at(_name) | ranges::views::filter(&Declaration::isVisibleAsUnqualifiedName) | ranges::to_vector;
|
||||
else
|
||||
result += m_invisibleDeclarations.at(_name);
|
||||
}
|
||||
|
||||
if (result.empty() && _recursive && m_enclosingContainer)
|
||||
result = m_enclosingContainer->resolveName(_name, true, _alsoInvisible);
|
||||
result = m_enclosingContainer->resolveName(_name, true, _alsoInvisible, _onlyVisibleAsUnqualifiedNames);
|
||||
|
||||
return result;
|
||||
}
|
||||
|
||||
|
@ -39,11 +39,10 @@ class DeclarationContainer
|
||||
public:
|
||||
using Homonyms = std::vector<std::pair<langutil::SourceLocation const*, std::vector<Declaration const*>>>;
|
||||
|
||||
explicit DeclarationContainer(
|
||||
ASTNode const* _enclosingNode = nullptr,
|
||||
DeclarationContainer* _enclosingContainer = nullptr
|
||||
):
|
||||
m_enclosingNode(_enclosingNode), m_enclosingContainer(_enclosingContainer)
|
||||
DeclarationContainer() = default;
|
||||
explicit DeclarationContainer(ASTNode const* _enclosingNode, DeclarationContainer* _enclosingContainer):
|
||||
m_enclosingNode(_enclosingNode),
|
||||
m_enclosingContainer(_enclosingContainer)
|
||||
{
|
||||
if (_enclosingContainer)
|
||||
_enclosingContainer->m_innerContainers.emplace_back(this);
|
||||
@ -57,7 +56,20 @@ public:
|
||||
bool registerDeclaration(Declaration const& _declaration, ASTString const* _name, langutil::SourceLocation const* _location, bool _invisible, bool _update);
|
||||
bool registerDeclaration(Declaration const& _declaration, bool _invisible, bool _update);
|
||||
|
||||
std::vector<Declaration const*> resolveName(ASTString const& _name, bool _recursive = false, bool _alsoInvisible = false) const;
|
||||
/// Finds all declarations that in the current scope can be referred to using specified name.
|
||||
/// @param _name the name to look for.
|
||||
/// @param _recursive if true and there are no matching declarations in the current container,
|
||||
/// recursively searches the enclosing containers as well.
|
||||
/// @param _alsoInvisible if true, include invisible declaration in the results.
|
||||
/// @param _onlyVisibleAsUnqualifiedNames if true, do not include declarations which can never
|
||||
/// actually be referenced using their name alone (without being qualified with the name
|
||||
/// of scope in which they are declared).
|
||||
std::vector<Declaration const*> resolveName(
|
||||
ASTString const& _name,
|
||||
bool _recursive = false,
|
||||
bool _alsoInvisible = false,
|
||||
bool _onlyVisibleAsUnqualifiedNames = false
|
||||
) const;
|
||||
ASTNode const* enclosingNode() const { return m_enclosingNode; }
|
||||
DeclarationContainer const* enclosingContainer() const { return m_enclosingContainer; }
|
||||
std::map<ASTString, std::vector<Declaration const*>> const& declarations() const { return m_declarations; }
|
||||
@ -80,8 +92,8 @@ public:
|
||||
void populateHomonyms(std::back_insert_iterator<Homonyms> _it) const;
|
||||
|
||||
private:
|
||||
ASTNode const* m_enclosingNode;
|
||||
DeclarationContainer const* m_enclosingContainer;
|
||||
ASTNode const* m_enclosingNode = nullptr;
|
||||
DeclarationContainer const* m_enclosingContainer = nullptr;
|
||||
std::vector<DeclarationContainer const*> m_innerContainers;
|
||||
std::map<ASTString, std::vector<Declaration const*>> m_declarations;
|
||||
std::map<ASTString, std::vector<Declaration const*>> m_invisibleDeclarations;
|
||||
|
@ -182,7 +182,13 @@ vector<Declaration const*> NameAndTypeResolver::nameFromCurrentScope(ASTString c
|
||||
Declaration const* NameAndTypeResolver::pathFromCurrentScope(vector<ASTString> const& _path) const
|
||||
{
|
||||
solAssert(!_path.empty(), "");
|
||||
vector<Declaration const*> candidates = m_currentScope->resolveName(_path.front(), true);
|
||||
vector<Declaration const*> candidates = m_currentScope->resolveName(
|
||||
_path.front(),
|
||||
/* _recursive */ true,
|
||||
/* _alsoInvisible */ false,
|
||||
/* _onlyVisibleAsUnqualifiedNames */ true
|
||||
);
|
||||
|
||||
for (size_t i = 1; i < _path.size() && candidates.size() == 1; i++)
|
||||
{
|
||||
if (!m_scopes.count(candidates.front()))
|
||||
@ -627,7 +633,10 @@ void DeclarationRegistrationHelper::enterNewSubScope(ASTNode& _subScope)
|
||||
solAssert(dynamic_cast<SourceUnit const*>(&_subScope), "Unexpected scope type.");
|
||||
else
|
||||
{
|
||||
bool newlyAdded = m_scopes.emplace(&_subScope, make_shared<DeclarationContainer>(m_currentScope, m_scopes[m_currentScope].get())).second;
|
||||
bool newlyAdded = m_scopes.emplace(
|
||||
&_subScope,
|
||||
make_shared<DeclarationContainer>(m_currentScope, m_scopes[m_currentScope].get())
|
||||
).second;
|
||||
solAssert(newlyAdded, "Unable to add new scope.");
|
||||
}
|
||||
m_currentScope = &_subScope;
|
||||
|
@ -606,6 +606,18 @@ bool Declaration::isEventOrErrorParameter() const
|
||||
return dynamic_cast<EventDefinition const*>(scope()) || dynamic_cast<ErrorDefinition const*>(scope());
|
||||
}
|
||||
|
||||
bool Declaration::isVisibleAsUnqualifiedName() const
|
||||
{
|
||||
if (!scope())
|
||||
return true;
|
||||
if (isStructMember() || isEnumValue() || isEventOrErrorParameter())
|
||||
return false;
|
||||
if (auto const* functionDefinition = dynamic_cast<FunctionDefinition const*>(scope()))
|
||||
if (!functionDefinition->isImplemented())
|
||||
return false; // parameter of a function without body
|
||||
return true;
|
||||
}
|
||||
|
||||
DeclarationAnnotation& Declaration::annotation() const
|
||||
{
|
||||
return initAnnotation<DeclarationAnnotation>();
|
||||
|
@ -265,6 +265,12 @@ public:
|
||||
/// @returns true if this is a declaration of a parameter of an event.
|
||||
bool isEventOrErrorParameter() const;
|
||||
|
||||
/// @returns false if the declaration can never be referenced without being qualified with a scope.
|
||||
/// Usually the name alone can be used to refer to the corresponding entity.
|
||||
/// But, for example, struct member names or enum member names always require a prefix.
|
||||
/// Another example is event parameter names, which do not participate in any proper scope.
|
||||
bool isVisibleAsUnqualifiedName() const;
|
||||
|
||||
/// @returns the type of expressions referencing this declaration.
|
||||
/// This can only be called once types of variable declarations have already been resolved.
|
||||
virtual Type const* type() const = 0;
|
||||
|
@ -0,0 +1,26 @@
|
||||
pragma abicoder v2;
|
||||
|
||||
contract C {
|
||||
enum EnumType {A, B, C}
|
||||
|
||||
struct StructType {
|
||||
uint x;
|
||||
}
|
||||
|
||||
error E1(StructType StructType);
|
||||
error E2(EnumType StructType, StructType EnumType);
|
||||
|
||||
function f() public {
|
||||
revert E1({StructType: StructType(42)});
|
||||
}
|
||||
|
||||
function g() public {
|
||||
revert E2({EnumType: StructType(42), StructType: EnumType.B});
|
||||
}
|
||||
}
|
||||
// ====
|
||||
// compileToEwasm: also
|
||||
// compileViaYul: also
|
||||
// ----
|
||||
// f() -> FAILURE, hex"33a54193", hex"000000000000000000000000000000000000000000000000000000000000002a"
|
||||
// g() -> FAILURE, hex"374b9387", hex"0000000000000000000000000000000000000000000000000000000000000001", hex"000000000000000000000000000000000000000000000000000000000000002a"
|
12
test/libsolidity/syntaxTests/enums/enum_member_shadowing.sol
Normal file
12
test/libsolidity/syntaxTests/enums/enum_member_shadowing.sol
Normal file
@ -0,0 +1,12 @@
|
||||
contract C {
|
||||
struct S {
|
||||
uint x;
|
||||
}
|
||||
|
||||
enum E {E, S, C, a, f}
|
||||
|
||||
uint a;
|
||||
|
||||
function f() public pure {}
|
||||
}
|
||||
// ----
|
@ -0,0 +1,5 @@
|
||||
contract C {
|
||||
error E(int bytes, bytes x);
|
||||
}
|
||||
// ----
|
||||
// ParserError 2314: (29-34): Expected ',' but got 'bytes'
|
@ -0,0 +1,12 @@
|
||||
contract C {
|
||||
enum EnumType {A, B, C}
|
||||
|
||||
struct StructType {
|
||||
uint x;
|
||||
}
|
||||
|
||||
error E1(StructType StructType);
|
||||
error E2(EnumType EnumType);
|
||||
error E3(EnumType StructType, StructType EnumType);
|
||||
}
|
||||
// ----
|
@ -0,0 +1,13 @@
|
||||
contract C {
|
||||
enum EnumType {A, B, C}
|
||||
|
||||
struct StructType {
|
||||
uint x;
|
||||
}
|
||||
|
||||
event E1(StructType StructType);
|
||||
event E2(EnumType EnumType);
|
||||
event E3(EnumType StructType, StructType EnumType);
|
||||
event E4(StructType indexed StructType) anonymous;
|
||||
}
|
||||
// ----
|
@ -0,0 +1,20 @@
|
||||
interface I {
|
||||
function f(uint I) external; // OK
|
||||
}
|
||||
|
||||
library L {
|
||||
function f(uint L) public pure {} // warning
|
||||
}
|
||||
|
||||
abstract contract A {
|
||||
function f(uint A) public pure {} // warning
|
||||
function g(uint A) public virtual; // OK
|
||||
}
|
||||
|
||||
contract C {
|
||||
function f(uint C) public pure {} // warning
|
||||
}
|
||||
// ----
|
||||
// Warning 2519: (91-97): This declaration shadows an existing declaration.
|
||||
// Warning 2519: (168-174): This declaration shadows an existing declaration.
|
||||
// Warning 2519: (283-289): This declaration shadows an existing declaration.
|
@ -0,0 +1,5 @@
|
||||
contract C {
|
||||
function f(uint f) pure public {}
|
||||
}
|
||||
// ----
|
||||
// Warning 2519: (28-34): This declaration shadows an existing declaration.
|
@ -0,0 +1,20 @@
|
||||
contract C {
|
||||
enum EnumType {A, B, C}
|
||||
|
||||
struct StructType {
|
||||
uint x;
|
||||
}
|
||||
|
||||
function f(StructType memory StructType) external {}
|
||||
function g(EnumType EnumType) external {}
|
||||
function h(EnumType StructType, StructType memory EnumType) external {}
|
||||
|
||||
function z(EnumType e) external returns (uint EnumType) {}
|
||||
}
|
||||
// ----
|
||||
// Warning 2519: (104-132): This declaration shadows an existing declaration.
|
||||
// Warning 2519: (161-178): This declaration shadows an existing declaration.
|
||||
// Warning 2519: (207-226): This declaration shadows an existing declaration.
|
||||
// Warning 2519: (228-254): This declaration shadows an existing declaration.
|
||||
// Warning 2519: (314-327): This declaration shadows an existing declaration.
|
||||
// TypeError 5172: (104-114): Name has to refer to a struct, enum or contract.
|
@ -0,0 +1,20 @@
|
||||
library C {
|
||||
enum EnumType {A, B, C}
|
||||
|
||||
struct StructType {
|
||||
uint x;
|
||||
}
|
||||
|
||||
function f1(function (StructType memory StructType) external f) external {}
|
||||
function f2(function (EnumType EnumType) external f) external {}
|
||||
function f3(function (EnumType StructType, StructType memory EnumType) external f) external {}
|
||||
}
|
||||
// ----
|
||||
// Warning 6162: (114-142): Naming function type parameters is deprecated.
|
||||
// Warning 6162: (194-211): Naming function type parameters is deprecated.
|
||||
// Warning 6162: (263-282): Naming function type parameters is deprecated.
|
||||
// Warning 6162: (284-310): Naming function type parameters is deprecated.
|
||||
// Warning 2519: (114-142): This declaration shadows an existing declaration.
|
||||
// Warning 2519: (194-211): This declaration shadows an existing declaration.
|
||||
// Warning 2519: (263-282): This declaration shadows an existing declaration.
|
||||
// Warning 2519: (284-310): This declaration shadows an existing declaration.
|
@ -0,0 +1,20 @@
|
||||
contract C {
|
||||
enum EnumType {A, B, C}
|
||||
|
||||
struct StructType {
|
||||
uint x;
|
||||
}
|
||||
|
||||
function f() external returns (StructType memory StructType) {}
|
||||
function g() external returns (EnumType EnumType) {}
|
||||
function h() external returns (EnumType StructType, StructType memory EnumType) {}
|
||||
|
||||
function z(uint EnumType) external returns (EnumType e) {}
|
||||
}
|
||||
// ----
|
||||
// Warning 2519: (124-152): This declaration shadows an existing declaration.
|
||||
// Warning 2519: (192-209): This declaration shadows an existing declaration.
|
||||
// Warning 2519: (249-268): This declaration shadows an existing declaration.
|
||||
// Warning 2519: (270-296): This declaration shadows an existing declaration.
|
||||
// Warning 2519: (317-330): This declaration shadows an existing declaration.
|
||||
// TypeError 5172: (124-134): Name has to refer to a struct, enum or contract.
|
@ -0,0 +1,20 @@
|
||||
contract C {
|
||||
enum EnumType {A, B, C}
|
||||
|
||||
struct StructType {
|
||||
uint x;
|
||||
}
|
||||
|
||||
function (StructType memory StructType) external ext1;
|
||||
function (EnumType EnumType) external ext2;
|
||||
function (EnumType StructType, StructType memory EnumType) external ext3;
|
||||
}
|
||||
// ----
|
||||
// Warning 6162: (103-131): Naming function type parameters is deprecated.
|
||||
// Warning 6162: (162-179): Naming function type parameters is deprecated.
|
||||
// Warning 6162: (210-229): Naming function type parameters is deprecated.
|
||||
// Warning 6162: (231-257): Naming function type parameters is deprecated.
|
||||
// Warning 2519: (103-131): This declaration shadows an existing declaration.
|
||||
// Warning 2519: (162-179): This declaration shadows an existing declaration.
|
||||
// Warning 2519: (210-229): This declaration shadows an existing declaration.
|
||||
// Warning 2519: (231-257): This declaration shadows an existing declaration.
|
@ -0,0 +1,20 @@
|
||||
contract C {
|
||||
enum EnumType {A, B, C}
|
||||
|
||||
struct StructType {
|
||||
uint x;
|
||||
}
|
||||
|
||||
function () external returns (StructType memory StructType) ext1;
|
||||
function () external returns (EnumType EnumType) ext2;
|
||||
function () external returns (EnumType StructType, StructType memory EnumType) ext3;
|
||||
}
|
||||
// ----
|
||||
// SyntaxError 7304: (123-151): Return parameters in function types may not be named.
|
||||
// SyntaxError 7304: (193-210): Return parameters in function types may not be named.
|
||||
// SyntaxError 7304: (252-271): Return parameters in function types may not be named.
|
||||
// SyntaxError 7304: (273-299): Return parameters in function types may not be named.
|
||||
// Warning 2519: (123-151): This declaration shadows an existing declaration.
|
||||
// Warning 2519: (193-210): This declaration shadows an existing declaration.
|
||||
// Warning 2519: (252-271): This declaration shadows an existing declaration.
|
||||
// Warning 2519: (273-299): This declaration shadows an existing declaration.
|
@ -3,4 +3,4 @@ contract C {
|
||||
function f(function(S memory) external) public {}
|
||||
}
|
||||
// ----
|
||||
// TypeError 5172: (25-26): Name has to refer to a struct, enum or contract.
|
||||
// DeclarationError 7920: (25-26): Identifier not found or not unique.
|
||||
|
@ -0,0 +1,6 @@
|
||||
contract C {
|
||||
enum E {a, b, c}
|
||||
struct S {E X; uint E;}
|
||||
struct T {E T; uint E;}
|
||||
struct U {E E;}
|
||||
}
|
@ -0,0 +1,12 @@
|
||||
contract C {
|
||||
enum E {a, b, c}
|
||||
struct S {function (E X) external f; uint E;}
|
||||
struct T {function (E T) external f; uint E;}
|
||||
struct U {function (E E) external f;}
|
||||
}
|
||||
// ----
|
||||
// Warning 6162: (58-61): Naming function type parameters is deprecated.
|
||||
// Warning 6162: (108-111): Naming function type parameters is deprecated.
|
||||
// Warning 6162: (158-161): Naming function type parameters is deprecated.
|
||||
// Warning 2519: (108-111): This declaration shadows an existing declaration.
|
||||
// Warning 2519: (158-161): This declaration shadows an existing declaration.
|
Loading…
Reference in New Issue
Block a user