Work in progress for state variable accessors

- Changed the code so that a generic declaration with the combination of
  a function type can be used wherer a function definition was used
  before

- Since using an std::pair everywhere is really tiring with this commit
  I am in the process of abstracting it into a function

AST.cpp

@@ -68,19 +68,21 @@ void ContractDefinition::checkTypeRequirements()
 	set<FixedHash<4>> hashes;
 	for (auto const& hashAndFunction: getInterfaceFunctionList())
 	{
-		FixedHash<4> const& hash = hashAndFunction.first;
+		FixedHash<4> const& hash = std::get<0>(hashAndFunction);
 		if (hashes.count(hash))
-			BOOST_THROW_EXCEPTION(createTypeError("Function signature hash collision for " +
+			BOOST_THROW_EXCEPTION(createTypeError(
-												  hashAndFunction.second->getCanonicalSignature()));
+									  "Function signature hash collision for " +
+									  std::get<1>(hashAndFunction)>->getCanonicalSignature(std::get<2>(hashAndFunction)->getName())));
 		hashes.insert(hash);
 	}
 }
 
-map<FixedHash<4>, FunctionDefinition const*> ContractDefinition::getInterfaceFunctions() const
+map<FixedHash<4>, pair<FunctionType const*, FunctionDefinition const*>> ContractDefinition::getInterfaceFunctions() const
 {
-	vector<pair<FixedHash<4>, FunctionDefinition const*>> exportedFunctionList = getInterfaceFunctionList();
+	vector<tuple<FixedHash<4>, FunctionType const*, Declaration const*>>> exportedFunctionList = getInterfaceFunctionList();
-	map<FixedHash<4>, FunctionDefinition const*> exportedFunctions(exportedFunctionList.begin(),
+	map<FixedHash<4>, pair<FunctionType *, Declaration const*>> exportedFunctions(exportedFunctionList.begin(),
 																				  exportedFunctionList.end());
+
 	solAssert(exportedFunctionList.size() == exportedFunctions.size(),
 			  "Hash collision at Function Definition Hash calculation");
 
@@ -134,19 +136,30 @@ void ContractDefinition::checkIllegalOverrides() const
 	}
 }
 
-vector<pair<FixedHash<4>, FunctionDefinition const*>> const& ContractDefinition::getInterfaceFunctionList() const
+vector<tuple<FixedHash<4>, FunctionType const*, Declaration const*>> const& ContractDefinition::getInterfaceFunctionList() const
 {
 	if (!m_interfaceFunctionList)
 	{
 		set<string> functionsSeen;
-		m_interfaceFunctionList.reset(new vector<pair<FixedHash<4>, FunctionDefinition const*>>());
+		m_interfaceFunctionList.reset(new vector<tuple<FixedHash<4>, FunctionType const*, Declaration const*>>());
 		for (ContractDefinition const* contract: getLinearizedBaseContracts())
+		{
 			for (ASTPointer<FunctionDefinition> const& f: contract->getDefinedFunctions())
 				if (f->isPublic() && !f->isConstructor() && functionsSeen.count(f->getName()) == 0)
 				{
 					functionsSeen.insert(f->getName());
 					FixedHash<4> hash(dev::sha3(f->getCanonicalSignature()));
-					m_interfaceFunctionList->push_back(make_pair(hash, f.get()));
+					m_interfaceFunctionList->push_back(make_tuple(hash, FunctionType(*f), f.get()));
+				}
+
+			for (ASTPointer<VariableDeclaration> const& v: contract->getStateVariables())
+				if (v->isPublic())
+				{
+					FunctionType ftype(*v);
+					functionsSeen.insert(v->getName());
+					FixedHash<4> hash(dev::sha3(ftype.getCanonicalSignature(v->getName()));
+					m_interfaceFunctionList->push_back(make_tuple(hash, ftype, v.get()));
+				}
 		}
 	}
 	return *m_interfaceFunctionList;
@@ -219,7 +232,7 @@ void FunctionDefinition::checkTypeRequirements()
 
 string FunctionDefinition::getCanonicalSignature() const
 {
-	return getName() + FunctionType(*this).getCanonicalSignature();
+	return FunctionType(*this).getCanonicalSignature(getName());
 }
 
 Declaration::LValueType VariableDeclaration::getLValueType() const
@@ -504,5 +517,18 @@ void Literal::checkTypeRequirements()
 		BOOST_THROW_EXCEPTION(createTypeError("Invalid literal value."));
 }
 
+
+ASTPointer<ASTString> FunctionDescription::getDocumentation()
+{
+	auto function = dynamic_cast<FunctionDefinition const*>(m_description.second);
+	if (function)
+		return function->getDocumentation();
+}
+
+string FunctionDescription::getSignature()
+{
+	return m_description.first->getCanonicalSignature(m_description.second->getName());
+}
+
 }
 }

AST.h

@@ -156,6 +156,23 @@ private:
 	Declaration const* m_scope;
 };
 
+
+/**
+ * Generic function description able to describe both normal functions and
+ * functions that should be made as accessors to state variables
+ */
+struct FunctionDescription
+{
+	FunctionDescription(FunctionType const *_type, Declaration const* _decl):
+		m_description(_type, _decl){}
+
+	ASTPointer<ASTString> getDocumentation();
+	std::string getSignature();
+
+	std::pair<FunctionType const*, Declaration const*> m_description;
+};
+
+
 /**
  * Definition of a contract. This is the only AST nodes where child nodes are not visited in
  * document order. It first visits all struct declarations, then all variable declarations and
@@ -202,7 +219,7 @@ public:
 
 	/// @returns a map of canonical function signatures to FunctionDefinitions
 	/// as intended for use by the ABI.
-	std::map<FixedHash<4>, FunctionDefinition const*> getInterfaceFunctions() const;
+	std::map<FixedHash<4>, std::pair<FunctionType const*, Declaration const*>> getInterfaceFunctions() const;
 
 	/// List of all (direct and indirect) base contracts in order from derived to base, including
 	/// the contract itself. Available after name resolution
@@ -215,7 +232,7 @@ public:
 private:
 	void checkIllegalOverrides() const;
 
-	std::vector<std::pair<FixedHash<4>, FunctionDefinition const*>> const& getInterfaceFunctionList() const;
+	std::vector<std::tuple<FixedHash<4>, FunctionType const*, Declaration const*>> const& getInterfaceFunctionList() const;
 
 	std::vector<ASTPointer<InheritanceSpecifier>> m_baseContracts;
 	std::vector<ASTPointer<StructDefinition>> m_definedStructs;
@@ -225,7 +242,7 @@ private:
 	ASTPointer<ASTString> m_documentation;
 
 	std::vector<ContractDefinition const*> m_linearizedBaseContracts;
-	mutable std::unique_ptr<std::vector<std::pair<FixedHash<4>, FunctionDefinition const*>>> m_interfaceFunctionList;
+	mutable std::unique_ptr<std::vector<std::tuple<FixedHash<4>, FunctionType const*, Declaration const*>>> m_interfaceFunctionList;
 };
 
 class InheritanceSpecifier: public ASTNode
@@ -372,8 +389,8 @@ class VariableDeclaration: public Declaration
 {
 public:
 	VariableDeclaration(Location const& _location, ASTPointer<TypeName> const& _type,
-						ASTPointer<ASTString> const& _name):
+						ASTPointer<ASTString> const& _name, bool _isPublic):
-		Declaration(_location, _name), m_typeName(_type) {}
+		Declaration(_location, _name), m_typeName(_type), m_isPublic(_isPublic) {}
 	virtual void accept(ASTVisitor& _visitor) override;
 	virtual void accept(ASTConstVisitor& _visitor) const override;
 
@@ -385,10 +402,13 @@ public:
 	void setType(std::shared_ptr<Type const> const& _type) { m_type = _type; }
 
 	virtual LValueType getLValueType() const override;
+	bool isLocalVariable() const { return !!dynamic_cast<FunctionDefinition const*>(getScope()); }
+	bool isPublic() const { return m_isPublic; }
+
 
 private:
 	ASTPointer<TypeName> m_typeName;    ///< can be empty ("var")
-
+	bool m_isPublic;                    ///< Whether there is an accessor for it or not
 	std::shared_ptr<Type const> m_type; ///< derived type, initially empty
 };
 
@@ -1076,5 +1096,6 @@ private:
 
 /// @}
 
+
 }
 }

Compiler.cpp

@@ -201,7 +201,7 @@ set<FunctionDefinition const*> Compiler::getFunctionsCalled(set<ASTNode const*>
 
 void Compiler::appendFunctionSelector(ContractDefinition const& _contract)
 {
-	map<FixedHash<4>, FunctionDefinition const*> interfaceFunctions = _contract.getInterfaceFunctions();
+	map<FixedHash<4>, FunctionType const*, Declaration const*> interfaceFunctions = _contract.getInterfaceFunctions();
 	map<FixedHash<4>, const eth::AssemblyItem> callDataUnpackerEntryPoints;
 
 	// retrieve the function signature hash from the calldata
@@ -209,7 +209,6 @@ void Compiler::appendFunctionSelector(ContractDefinition const& _contract)
 	CompilerUtils(m_context).loadFromMemory(0, 4, false, true);
 
 	// stack now is: 1 0 <funhash>
-	// for (auto it = interfaceFunctions.cbegin(); it != interfaceFunctions.cend(); ++it)
 	for (auto const& it: interfaceFunctions)
 	{
 		callDataUnpackerEntryPoints.insert(std::make_pair(it.first, m_context.newTag()));
@@ -220,29 +219,28 @@ void Compiler::appendFunctionSelector(ContractDefinition const& _contract)
 
 	for (auto const& it: interfaceFunctions)
 	{
-		FunctionDefinition const& function = *it.second;
+		FunctionType const* functionType = *it.second.first;
 		m_context << callDataUnpackerEntryPoints.at(it.first);
 		eth::AssemblyItem returnTag = m_context.pushNewTag();
-		appendCalldataUnpacker(function);
+		appendCalldataUnpacker(functionType->getParameterTypes());
-		m_context.appendJumpTo(m_context.getFunctionEntryLabel(function));
+		m_context.appendJumpTo(m_context.getFunctionEntryLabel(it.second.second));
 		m_context << returnTag;
-		appendReturnValuePacker(function);
+		appendReturnValuePacker(functionType->getReturnParameterTypes());
 	}
 }
 
-unsigned Compiler::appendCalldataUnpacker(FunctionDefinition const& _function, bool _fromMemory)
+unsigned Compiler::appendCalldataUnpacker(TypePointers const& _typeParameters, bool _fromMemory)
 {
 	// We do not check the calldata size, everything is zero-padded.
 	unsigned dataOffset = CompilerUtils::dataStartOffset; // the 4 bytes of the function hash signature
 	//@todo this can be done more efficiently, saving some CALLDATALOAD calls
-	for (ASTPointer<VariableDeclaration> const& var: _function.getParameters())
+	for (TypePointer const& type: _typeParameters)
 	{
-		unsigned const c_numBytes = var->getType()->getCalldataEncodedSize();
+		unsigned const c_numBytes = type->getCalldataEncodedSize();
 		if (c_numBytes > 32)
 			BOOST_THROW_EXCEPTION(CompilerError()
-								  << errinfo_sourceLocation(var->getLocation())
+								  << errinfo_comment("Type " + type->toString() + " not yet supported."));
-								  << errinfo_comment("Type " + var->getType()->toString() + " not yet supported."));
+		bool const c_leftAligned = type->getCategory() == Type::Category::STRING;
-		bool const c_leftAligned = var->getType()->getCategory() == Type::Category::STRING;
 		bool const c_padToWords = true;
 		dataOffset += CompilerUtils(m_context).loadFromMemory(dataOffset, c_numBytes, c_leftAligned,
 															  !_fromMemory, c_padToWords);
@@ -250,26 +248,26 @@ unsigned Compiler::appendCalldataUnpacker(FunctionDefinition const& _function, b
 	return dataOffset;
 }
 
-void Compiler::appendReturnValuePacker(FunctionDefinition const& _function)
+void Compiler::appendReturnValuePacker(TypePointers const& _typeParameters)
 {
 	//@todo this can be also done more efficiently
 	unsigned dataOffset = 0;
-	vector<ASTPointer<VariableDeclaration>> const& parameters = _function.getReturnParameters();
+	unsigned stackDepth = 0;
-	unsigned stackDepth = CompilerUtils(m_context).getSizeOnStack(parameters);
+	for (TypePointer const& type: _typeParameters)
-	for (unsigned i = 0; i < parameters.size(); ++i)
+		stackDepth += type->getSizeOnStack();
+
+	for (TypePointer const& type: _typeParameters)
 	{
-		Type const& paramType = *parameters[i]->getType();
+		unsigned numBytes = type->getCalldataEncodedSize();
-		unsigned numBytes = paramType.getCalldataEncodedSize();
 		if (numBytes > 32)
 			BOOST_THROW_EXCEPTION(CompilerError()
-								  << errinfo_sourceLocation(parameters[i]->getLocation())
+								  << errinfo_comment("Type " + type->toString() + " not yet supported."));
-								  << errinfo_comment("Type " + paramType.toString() + " not yet supported."));
+		CompilerUtils(m_context).copyToStackTop(stackDepth, *type);
-		CompilerUtils(m_context).copyToStackTop(stackDepth, paramType);
+		ExpressionCompiler::appendTypeConversion(m_context, *type, *type, true);
-		ExpressionCompiler::appendTypeConversion(m_context, paramType, paramType, true);
+		bool const c_leftAligned = type->getCategory() == Type::Category::STRING;
-		bool const c_leftAligned = paramType.getCategory() == Type::Category::STRING;
 		bool const c_padToWords = true;
 		dataOffset += CompilerUtils(m_context).storeInMemory(dataOffset, numBytes, c_leftAligned, c_padToWords);
-		stackDepth -= paramType.getSizeOnStack();
+		stackDepth -= type->getSizeOnStack();
 	}
 	// note that the stack is not cleaned up here
 	m_context << u256(dataOffset) << u256(0) << eth::Instruction::RETURN;

Compiler.h

@@ -56,10 +56,10 @@ private:
 					std::function<FunctionDefinition const*(std::string const&)> const& _resolveFunctionOverride,
 					std::function<ModifierDefinition const*(std::string const&)> const& _resolveModifierOverride);
 	void appendFunctionSelector(ContractDefinition const& _contract);
-	/// Creates code that unpacks the arguments for the given function, from memory if
+	/// Creates code that unpacks the arguments for the given function represented by a vector of TypePointers.
-	/// @a _fromMemory is true, otherwise from call data. @returns the size of the data in bytes.
+	/// From memory if @a _fromMemory is true, otherwise from call data. @returns the size of the data in bytes.
-	unsigned appendCalldataUnpacker(FunctionDefinition const& _function, bool _fromMemory = false);
+	unsigned appendCalldataUnpacker(TypePointers const& _typeParameters, bool _fromMemory = false);
-	void appendReturnValuePacker(FunctionDefinition const& _function);
+	void appendReturnValuePacker(TypePointers const& _typeParameters);
 
 	void registerStateVariables(ContractDefinition const& _contract);
 

CompilerContext.cpp

@@ -83,9 +83,9 @@ bool CompilerContext::isLocalVariable(Declaration const* _declaration) const
 	return m_localVariables.count(_declaration);
 }
 
-eth::AssemblyItem CompilerContext::getFunctionEntryLabel(FunctionDefinition const& _function) const
+eth::AssemblyItem CompilerContext::getFunctionEntryLabel(Declaration const& _declaration) const
 {
-	auto res = m_functionEntryLabels.find(&_function);
+	auto res = m_functionEntryLabels.find(&_declaration);
 	solAssert(res != m_functionEntryLabels.end(), "Function entry label not found.");
 	return res->second.tag();
 }

CompilerContext.h

@@ -58,7 +58,7 @@ public:
 	bool isLocalVariable(Declaration const* _declaration) const;
 	bool isStateVariable(Declaration const* _declaration) const { return m_stateVariables.count(_declaration) != 0; }
 
-	eth::AssemblyItem getFunctionEntryLabel(FunctionDefinition const& _function) const;
+	eth::AssemblyItem getFunctionEntryLabel(Declaration const& _declaration) const;
 	/// @returns the entry label of the given function and takes overrides into account.
 	eth::AssemblyItem getVirtualFunctionEntryLabel(FunctionDefinition const& _function) const;
 	ModifierDefinition const& getFunctionModifier(std::string const& _name) const;
@@ -115,9 +115,12 @@ private:
 	u256 m_stateVariablesSize = 0;
 	/// Storage offsets of state variables
 	std::map<Declaration const*, u256> m_stateVariables;
-	/// Positions of local variables on the stack.
+	/// Offsets of local variables on the stack (relative to stack base).
 	std::map<Declaration const*, unsigned> m_localVariables;
-	/// Labels pointing to the entry points of funcitons.
+	/// Sum of stack sizes of local variables
+	unsigned m_localVariablesSize;
+	/// Labels pointing to the entry points of functions.
+
 	std::map<Declaration const*, eth::AssemblyItem> m_functionEntryLabels;
 	/// Labels pointing to the entry points of function overrides.
 	std::map<std::string, eth::AssemblyItem> m_virtualFunctionEntryLabels;

Parser.cpp

@@ -109,30 +109,6 @@ ASTPointer<ImportDirective> Parser::parseImportDirective()
 	return nodeFactory.createNode<ImportDirective>(url);
 }
 
-void Parser::addStateVariableAccessor(ASTPointer<VariableDeclaration> const& _varDecl,
-									  vector<ASTPointer<FunctionDefinition>> & _functions)
-{
-	ASTNodeFactory nodeFactory(*this);
-	nodeFactory.setLocationEmpty();
-	ASTPointer<ASTString> emptyDoc;
-
-	vector<ASTPointer<VariableDeclaration>> parameters;
-	auto expression = nodeFactory.createNode<Identifier>(make_shared<ASTString>(_varDecl->getName()));
-	vector<ASTPointer<Statement>> block_statements = {nodeFactory.createNode<Return>(expression)};
-
-	_functions.push_back(nodeFactory.createNode<FunctionDefinition>(
-							 make_shared<ASTString>(_varDecl->getName()),
-							 true,               // isPublic
-							 false,              // not a Constructor
-							 emptyDoc,           // no documentation
-							 nodeFactory.createNode<ParameterList>(vector<ASTPointer<VariableDeclaration>>()),
-							 true,               // is constant
-							 nodeFactory.createNode<ParameterList>(vector<ASTPointer<VariableDeclaration>>()),
-							 nodeFactory.createNode<Block>(block_statements)
-						 )
-	);
-}
-
 ASTPointer<ContractDefinition> Parser::parseContractDefinition()
 {
 	ASTNodeFactory nodeFactory(*this);
@@ -174,9 +150,7 @@ ASTPointer<ContractDefinition> Parser::parseContractDefinition()
 				 Token::isElementaryTypeName(currentToken))
 		{
 			bool const allowVar = false;
-			stateVariables.push_back(parseVariableDeclaration(allowVar));
+			stateVariables.push_back(parseVariableDeclaration(allowVar, visibilityIsPublic));
-			if (visibilityIsPublic)
-				addStateVariableAccessor(stateVariables.back(), functions);
 			expectToken(Token::SEMICOLON);
 		}
 		else if (currentToken == Token::MODIFIER)
@@ -271,12 +245,12 @@ ASTPointer<StructDefinition> Parser::parseStructDefinition()
 	return nodeFactory.createNode<StructDefinition>(name, members);
 }
 
-ASTPointer<VariableDeclaration> Parser::parseVariableDeclaration(bool _allowVar)
+ASTPointer<VariableDeclaration> Parser::parseVariableDeclaration(bool _allowVar, bool _isPublic)
 {
 	ASTNodeFactory nodeFactory(*this);
 	ASTPointer<TypeName> type = parseTypeName(_allowVar);
 	nodeFactory.markEndPosition();
-	return nodeFactory.createNode<VariableDeclaration>(type, expectIdentifierToken());
+	return nodeFactory.createNode<VariableDeclaration>(type, expectIdentifierToken(), _isPublic);
 }
 
 ASTPointer<ModifierDefinition> Parser::parseModifierDefinition()

Parser.h

@@ -52,7 +52,7 @@ private:
 	ASTPointer<InheritanceSpecifier> parseInheritanceSpecifier();
 	ASTPointer<FunctionDefinition> parseFunctionDefinition(bool _isPublic, ASTString const* _contractName);
 	ASTPointer<StructDefinition> parseStructDefinition();
-	ASTPointer<VariableDeclaration> parseVariableDeclaration(bool _allowVar);
+	ASTPointer<VariableDeclaration> parseVariableDeclaration(bool _allowVar, bool _isPublic = false);
 	ASTPointer<ModifierDefinition> parseModifierDefinition();
 	ASTPointer<ModifierInvocation> parseModifierInvocation();
 	ASTPointer<Identifier> parseIdentifier();
@@ -78,10 +78,6 @@ private:
 	///@{
 	///@name Helper functions
 
-	/// Depending on whether a state Variable is Public, appends an accessor to the contract's functions
-	void addStateVariableAccessor(ASTPointer<VariableDeclaration> const& _varDecl,
-								  std::vector<ASTPointer<FunctionDefinition>> & _functions);
-
 	/// Peeks ahead in the scanner to determine if a variable definition is going to follow
 	bool peekVariableDefinition();
 

Types.cpp

@@ -489,7 +489,7 @@ MemberList const& ContractType::getMembers() const
 		map<string, shared_ptr<Type const>> members(IntegerType::AddressMemberList.begin(),
 													IntegerType::AddressMemberList.end());
 		for (auto const& it: m_contract.getInterfaceFunctions())
-			members[it.second->getName()] = make_shared<FunctionType>(*it.second, false);
+			members[it.second.second->getName()] = make_shared<FunctionType>(*it.second.second, false);
 		m_members.reset(new MemberList(members));
 	}
 	return *m_members;
@@ -512,7 +512,7 @@ u256 ContractType::getFunctionIdentifier(string const& _functionName) const
 {
 	auto interfaceFunctions = m_contract.getInterfaceFunctions();
 	for (auto it = interfaceFunctions.cbegin(); it != interfaceFunctions.cend(); ++it)
-		if (it->second->getName() == _functionName)
+		if (it->second.second->getName() == _functionName)
 			return FixedHash<4>::Arith(it->first);
 
 	return Invalid256;
@@ -593,6 +593,19 @@ FunctionType::FunctionType(FunctionDefinition const& _function, bool _isInternal
 	swap(retParams, m_returnParameterTypes);
 }
 
+FunctionType::FunctionType(VariableDeclaration const& _varDecl):
+	m_location(Location::INTERNAL)
+{
+	TypePointers params;
+	TypePointers retParams;
+	// for now, no input parameters LTODO: change for some things like mapping
+	params.reserve(0);
+	retParams.reserve(1);
+	retParams.push_back(_varDecl.getType());
+	swap(params, m_parameterTypes);
+	swap(retParams, m_returnParameterTypes);
+}
+
 bool FunctionType::operator==(Type const& _other) const
 {
 	if (_other.getCategory() != getCategory())
@@ -672,9 +685,9 @@ MemberList const& FunctionType::getMembers() const
 	}
 }
 
-string FunctionType::getCanonicalSignature() const
+string FunctionType::getCanonicalSignature(std::string const& _name) const
 {
-	string ret = "(";
+	string ret = _name + "(";
 
 	for (auto it = m_parameterTypes.cbegin(); it != m_parameterTypes.cend(); ++it)
 		ret += (*it)->toString() + (it + 1 == m_parameterTypes.cend() ? "" : ",");

Types.h

@@ -353,6 +353,7 @@ public:
 
 	virtual Category getCategory() const override { return Category::FUNCTION; }
 	explicit FunctionType(FunctionDefinition const& _function, bool _isInternal = true);
+	explicit FunctionType(VariableDeclaration const& _varDecl);
 	FunctionType(strings const& _parameterTypes, strings const& _returnParameterTypes,
 				 Location _location = Location::INTERNAL):
 		FunctionType(parseElementaryTypeVector(_parameterTypes), parseElementaryTypeVector(_returnParameterTypes),
@@ -375,7 +376,7 @@ public:
 	virtual MemberList const& getMembers() const override;
 
 	Location const& getLocation() const { return m_location; }
-	std::string getCanonicalSignature() const;
+	std::string getCanonicalSignature(std::string const &_name) const;
 
 	bool gasSet() const { return m_gasSet; }
 	bool valueSet() const { return m_valueSet; }