State variable accessors code is now more organized

- FunctionDescription is the abstraction of what should describe a
  function. It can either be a VariableDeclaration of a
  FunctionDefinition.

- ParamDescription is what FunctionDescription uses to describe its
  parameters for outside use purposes with a pair of (name, type)
  strings

- Modified code around Solidity and especially interface handler to
  adapt to this change

6 files changed, 199 insertions, 47 deletions

diff --git a/AST.cpp b/AST.cpp
index 7620eeec..e4b5ed7d 100644
--- a/AST.cpp
+++ b/AST.cpp
@@ -71,17 +71,19 @@ void ContractDefinition::checkTypeRequirements()
 		FixedHash<4> const& hash = std::get<0>(hashAndFunction);
 		if (hashes.count(hash))
 			BOOST_THROW_EXCEPTION(createTypeError(
-									  "Function signature hash collision for " +
-									  std::get<1>(hashAndFunction)>->getCanonicalSignature(std::get<2>(hashAndFunction)->getName())));
+									  std::string("Function signature hash collision for ") +
+									  std::get<1>(hashAndFunction)->getCanonicalSignature(std::get<2>(hashAndFunction)->getName())));
 		hashes.insert(hash);
 	}
 }
 
-map<FixedHash<4>, pair<FunctionType const*, FunctionDefinition const*>> ContractDefinition::getInterfaceFunctions() const
+map<FixedHash<4>, FunctionDescription> ContractDefinition::getInterfaceFunctions() const
 {
-	vector<tuple<FixedHash<4>, FunctionType const*, Declaration const*>>> exportedFunctionList = getInterfaceFunctionList();
-	map<FixedHash<4>, pair<FunctionType *, Declaration const*>> exportedFunctions(exportedFunctionList.begin(),
-																				  exportedFunctionList.end());
+	auto exportedFunctionList = getInterfaceFunctionList();
+
+	map<FixedHash<4>, FunctionDescription> exportedFunctions;
+	for (auto const& it: exportedFunctionList)
+		exportedFunctions[std::get<0>(it)] = std::move(FunctionDescription(std::get<1>(it), std::get<2>(it)));
 
 	solAssert(exportedFunctionList.size() == exportedFunctions.size(),
 			  "Hash collision at Function Definition Hash calculation");
@@ -136,12 +138,12 @@ void ContractDefinition::checkIllegalOverrides() const
 	}
 }
 
-vector<tuple<FixedHash<4>, FunctionType const*, Declaration const*>> const& ContractDefinition::getInterfaceFunctionList() const
+vector<tuple<FixedHash<4>, std::shared_ptr<FunctionType const>, Declaration const*>> const& ContractDefinition::getInterfaceFunctionList() const
 {
 	if (!m_interfaceFunctionList)
 	{
 		set<string> functionsSeen;
-		m_interfaceFunctionList.reset(new vector<tuple<FixedHash<4>, FunctionType const*, Declaration const*>>());
+		m_interfaceFunctionList.reset(new vector<tuple<FixedHash<4>, std::shared_ptr<FunctionType const>, Declaration const*>>());
 		for (ContractDefinition const* contract: getLinearizedBaseContracts())
 		{
 			for (ASTPointer<FunctionDefinition> const& f: contract->getDefinedFunctions())
@@ -149,7 +151,7 @@ vector<tuple<FixedHash<4>, FunctionType const*, Declaration const*>> const& Cont
 				{
 					functionsSeen.insert(f->getName());
 					FixedHash<4> hash(dev::sha3(f->getCanonicalSignature()));
-					m_interfaceFunctionList->push_back(make_tuple(hash, FunctionType(*f), f.get()));
+					m_interfaceFunctionList->push_back(make_tuple(hash, make_shared<FunctionType>(*f), f.get()));
 				}
 
 			for (ASTPointer<VariableDeclaration> const& v: contract->getStateVariables())
@@ -157,8 +159,8 @@ vector<tuple<FixedHash<4>, FunctionType const*, Declaration const*>> const& Cont
 				{
 					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()));
+					FixedHash<4> hash(dev::sha3(ftype.getCanonicalSignature(v->getName())));
+					m_interfaceFunctionList->push_back(make_tuple(hash, make_shared<FunctionType>(*v), v.get()));
 				}
 		}
 	}
@@ -518,17 +520,104 @@ void Literal::checkTypeRequirements()
 }
 
 
-ASTPointer<ASTString> FunctionDescription::getDocumentation()
+bool ParamDescription::operator!=(ParamDescription const& _other) const
+{
+	return m_description.first == _other.getName() && m_description.second == _other.getType();
+}
+
+std::ostream& ParamDescription::operator<<(std::ostream& os) const
+{
+	return os << m_description.first << ":" << m_description.second;
+}
+
+std::string ParamDescription::getName() const
+{
+	return m_description.first;
+}
+
+std::string ParamDescription::getType() const
+{
+	return m_description.second;
+}
+
+
+ASTPointer<ASTString> FunctionDescription::getDocumentation() const
 {
 	auto function = dynamic_cast<FunctionDefinition const*>(m_description.second);
 	if (function)
 		return function->getDocumentation();
+
+	return ASTPointer<ASTString>();
 }
 
-string FunctionDescription::getSignature()
+string FunctionDescription::getSignature() const
 {
 	return m_description.first->getCanonicalSignature(m_description.second->getName());
 }
 
+string FunctionDescription::getName() const
+{
+	return m_description.second->getName();
+}
+
+bool FunctionDescription::isConstant() const
+{
+	auto function = dynamic_cast<FunctionDefinition const*>(m_description.second);
+	if (function)
+		return function->isDeclaredConst();
+
+	return true;
+}
+
+vector<ParamDescription> const FunctionDescription::getParameters() const
+{
+	auto function = dynamic_cast<FunctionDefinition const*>(m_description.second);
+	if (function)
+	{
+		vector<ParamDescription> paramsDescription;
+		for (auto const& param: function->getParameters())
+			paramsDescription.push_back(ParamDescription(param->getName(), param->getType()->toString()));
+
+		return paramsDescription;
+	}
+
+	// else for now let's assume no parameters to accessors
+	// LTODO: fix this for mapping types
+	return {};
+}
+
+vector<ParamDescription> const FunctionDescription::getReturnParameters() const
+{
+	auto function = dynamic_cast<FunctionDefinition const*>(m_description.second);
+	if (function)
+	{
+		vector<ParamDescription> paramsDescription;
+		for (auto const& param: function->getParameters())
+			paramsDescription.push_back(ParamDescription(param->getName(), param->getType()->toString()));
+
+		return paramsDescription;
+	}
+
+	auto vardecl = dynamic_cast<VariableDeclaration const*>(m_description.second);
+	return {ParamDescription(vardecl->getName(), vardecl->getType()->toString())};
+}
+
+Declaration const* FunctionDescription::getDeclaration() const
+{
+	return m_description.second;
+}
+
+shared_ptr<FunctionType const> FunctionDescription::getFunctionTypeShared() const
+{
+	return m_description.first;
+}
+
+
+FunctionType const* FunctionDescription::getFunctionType() const
+{
+	return m_description.first.get();
+}
+
+
 }
 }
diff --git a/AST.h b/AST.h
index 695e504a..89daf80f 100755
--- a/AST.h
+++ b/AST.h
@@ -158,18 +158,58 @@ private:
 
 
 /**
+* Generic Parameter description used by @see FunctionDescription to return
+* a descripton of its parameters.
+*/
+struct ParamDescription
+{
+	ParamDescription(std::string const& _name, std::string const& _type):
+		m_description(_name, _type){}
+
+	bool operator!=(ParamDescription const& _other) const;
+	std::ostream& operator<<(std::ostream& os) const;
+
+	std::string getName() const;
+	std::string getType() const;
+
+	std::pair<std::string, std::string> m_description;
+};
+
+
+/**
  * 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):
+	FunctionDescription(std::shared_ptr<FunctionType const> _type, Declaration const* _decl):
 		m_description(_type, _decl){}
 
-	ASTPointer<ASTString> getDocumentation();
-	std::string getSignature();
+	FunctionDescription():
+		m_description(nullptr, nullptr){}
 
-	std::pair<FunctionType const*, Declaration const*> m_description;
+	/// @returns the natspec documentation of the function if existing. Accessor (for now) don't have natspec doc
+	ASTPointer<ASTString> getDocumentation() const;
+	/// @returns the canonical signature of the function
+	std::string getSignature() const;
+	/// @returns the name of the function, basically that of the declaration
+	std::string getName() const;
+	/// @returns whether the function is constant. IF it's an accessor this is always true
+	bool isConstant() const;
+	/// @returns the argument parameters of the function
+	std::vector<ParamDescription> const getParameters() const;
+	/// @returns the return parameters of the function
+	std::vector<ParamDescription> const getReturnParameters() const;
+	/// @returns the Declaration AST Node pointer
+	Declaration const* getDeclaration() const;
+	/// @returns a created shared pointer with the type of the function
+	std::shared_ptr<FunctionType> makeFunctionType() const;
+	/// @returns a pointer to the function type
+	FunctionType const* getFunctionType() const;
+	/// @returns a shared pointer to the function type
+	std::shared_ptr<FunctionType const> getFunctionTypeShared() const;
+
+	std::pair<std::shared_ptr<FunctionType const>, Declaration const*> m_description;
 };
 
 
@@ -219,7 +259,7 @@ public:
 
 	/// @returns a map of canonical function signatures to FunctionDefinitions
 	/// as intended for use by the ABI.
-	std::map<FixedHash<4>, std::pair<FunctionType const*, Declaration const*>> getInterfaceFunctions() const;
+	std::map<FixedHash<4>, FunctionDescription> getInterfaceFunctions() const;
 
 	/// List of all (direct and indirect) base contracts in order from derived to base, including
 	/// the contract itself. Available after name resolution
@@ -232,7 +272,7 @@ public:
 private:
 	void checkIllegalOverrides() const;
 
-	std::vector<std::tuple<FixedHash<4>, FunctionType const*, Declaration const*>> const& getInterfaceFunctionList() const;
+	std::vector<std::tuple<FixedHash<4>, std::shared_ptr<FunctionType const>, Declaration const*>> const& getInterfaceFunctionList() const;
 
 	std::vector<ASTPointer<InheritanceSpecifier>> m_baseContracts;
 	std::vector<ASTPointer<StructDefinition>> m_definedStructs;
@@ -242,7 +282,7 @@ private:
 	ASTPointer<ASTString> m_documentation;
 
 	std::vector<ContractDefinition const*> m_linearizedBaseContracts;
-	mutable std::unique_ptr<std::vector<std::tuple<FixedHash<4>, FunctionType const*, Declaration const*>>> m_interfaceFunctionList;
+	mutable std::unique_ptr<std::vector<std::tuple<FixedHash<4>, std::shared_ptr<FunctionType const>, Declaration const*>>> m_interfaceFunctionList;
 };
 
 class InheritanceSpecifier: public ASTNode
diff --git a/Compiler.cpp b/Compiler.cpp
index 13f8282e..f6f48a8c 100644
--- a/Compiler.cpp
+++ b/Compiler.cpp
@@ -177,13 +177,14 @@ void Compiler::appendConstructorCall(FunctionDefinition const& _constructor)
 	unsigned argumentSize = 0;
 	for (ASTPointer<VariableDeclaration> const& var: _constructor.getParameters())
 		argumentSize += CompilerUtils::getPaddedSize(var->getType()->getCalldataEncodedSize());
+
 	if (argumentSize > 0)
 	{
 		m_context << u256(argumentSize);
 		m_context.appendProgramSize();
 		m_context << u256(CompilerUtils::dataStartOffset); // copy it to byte four as expected for ABI calls
 		m_context << eth::Instruction::CODECOPY;
-		appendCalldataUnpacker(_constructor, true);
+		appendCalldataUnpacker(FunctionType(_constructor).getParameterTypes(), true);
 	}
 	m_context.appendJumpTo(m_context.getFunctionEntryLabel(_constructor));
 	m_context << returnTag;
@@ -201,7 +202,7 @@ set<FunctionDefinition const*> Compiler::getFunctionsCalled(set<ASTNode const*>
 
 void Compiler::appendFunctionSelector(ContractDefinition const& _contract)
 {
-	map<FixedHash<4>, FunctionType const*, Declaration const*> interfaceFunctions = _contract.getInterfaceFunctions();
+	map<FixedHash<4>, FunctionDescription> interfaceFunctions = _contract.getInterfaceFunctions();
 	map<FixedHash<4>, const eth::AssemblyItem> callDataUnpackerEntryPoints;
 
 	// retrieve the function signature hash from the calldata
@@ -219,11 +220,11 @@ void Compiler::appendFunctionSelector(ContractDefinition const& _contract)
 
 	for (auto const& it: interfaceFunctions)
 	{
-		FunctionType const* functionType = *it.second.first;
+		FunctionType const* functionType = it.second.getFunctionType();
 		m_context << callDataUnpackerEntryPoints.at(it.first);
 		eth::AssemblyItem returnTag = m_context.pushNewTag();
 		appendCalldataUnpacker(functionType->getParameterTypes());
-		m_context.appendJumpTo(m_context.getFunctionEntryLabel(it.second.second));
+		m_context.appendJumpTo(m_context.getFunctionEntryLabel(*it.second.getDeclaration()));
 		m_context << returnTag;
 		appendReturnValuePacker(functionType->getReturnParameterTypes());
 	}
diff --git a/InterfaceHandler.cpp b/InterfaceHandler.cpp
index 1adce8cb..9b632778 100644
--- a/InterfaceHandler.cpp
+++ b/InterfaceHandler.cpp
@@ -45,23 +45,23 @@ std::unique_ptr<std::string> InterfaceHandler::getABIInterface(ContractDefinitio
 		Json::Value inputs(Json::arrayValue);
 		Json::Value outputs(Json::arrayValue);
 
-		auto populateParameters = [](std::vector<ASTPointer<VariableDeclaration>> const& _vars)
+		auto populateParameters = [](vector<ParamDescription> const& _params)
 		{
 			Json::Value params(Json::arrayValue);
-			for (ASTPointer<VariableDeclaration> const& var: _vars)
+			for (auto const& param: _params)
 			{
 				Json::Value input;
-				input["name"] = var->getName();
-				input["type"] = var->getType()->toString();
+				input["name"] = param.getName();
+				input["type"] = param.getType();
 				params.append(input);
 			}
 			return params;
 		};
 
-		method["name"] = it.second->getName();
-		method["constant"] = it.second->isDeclaredConst();
-		method["inputs"] = populateParameters(it.second->getParameters());
-		method["outputs"] = populateParameters(it.second->getReturnParameters());
+		method["name"] = it.second.getName();
+		method["constant"] = it.second.isConstant();
+		method["inputs"] = populateParameters(it.second.getParameters());
+		method["outputs"] = populateParameters(it.second.getReturnParameters());
 		methods.append(method);
 	}
 	return std::unique_ptr<std::string>(new std::string(m_writer.write(methods)));
@@ -72,17 +72,16 @@ unique_ptr<string> InterfaceHandler::getABISolidityInterface(ContractDefinition
 	string ret = "contract " + _contractDef.getName() + "{";
 	for (auto const& it: _contractDef.getInterfaceFunctions())
 	{
-		FunctionDefinition const* f = it.second;
-		auto populateParameters = [](vector<ASTPointer<VariableDeclaration>> const& _vars)
+		auto populateParameters = [](vector<ParamDescription> const& _params)
 		{
 			string r = "";
-			for (ASTPointer<VariableDeclaration> const& var: _vars)
-				r += (r.size() ? "," : "(") + var->getType()->toString() + " " + var->getName();
+			for (auto const& param: _params)
+				r += (r.size() ? "," : "(") + param.getType() + " " + param.getName();
 			return r.size() ? r + ")" : "()";
 		};
-		ret += "function " + f->getName() + populateParameters(f->getParameters()) + (f->isDeclaredConst() ? "constant " : "");
-		if (f->getReturnParameters().size())
-			ret += "returns" + populateParameters(f->getReturnParameters());
+		ret += "function " + it.second.getName() + populateParameters(it.second.getParameters()) + (it.second.isConstant() ? "constant " : "");
+		if (it.second.getReturnParameters().size())
+			ret += "returns" + populateParameters(it.second.getReturnParameters());
 		else if (ret.back() == ' ')
 			ret.pop_back();
 		ret += "{}";
@@ -98,7 +97,7 @@ std::unique_ptr<std::string> InterfaceHandler::getUserDocumentation(ContractDefi
 	for (auto const& it: _contractDef.getInterfaceFunctions())
 	{
 		Json::Value user;
-		auto strPtr = it.second->getDocumentation();
+		auto strPtr = it.second.getDocumentation();
 		if (strPtr)
 		{
 			resetUser();
@@ -106,7 +105,7 @@ std::unique_ptr<std::string> InterfaceHandler::getUserDocumentation(ContractDefi
 			if (!m_notice.empty())
 			{// since @notice is the only user tag if missing function should not appear
 				user["notice"] = Json::Value(m_notice);
-				methods[it.second->getCanonicalSignature()] = user;
+				methods[it.second.getSignature()] = user;
 			}
 		}
 	}
@@ -139,7 +138,7 @@ std::unique_ptr<std::string> InterfaceHandler::getDevDocumentation(ContractDefin
 	for (auto const& it: _contractDef.getInterfaceFunctions())
 	{
 		Json::Value method;
-		auto strPtr = it.second->getDocumentation();
+		auto strPtr = it.second.getDocumentation();
 		if (strPtr)
 		{
 			resetDev();
@@ -162,7 +161,7 @@ std::unique_ptr<std::string> InterfaceHandler::getDevDocumentation(ContractDefin
 				method["return"] = m_return;
 
 			if (!method.empty()) // add the function, only if we have any documentation to add
-				methods[it.second->getCanonicalSignature()] = method;
+				methods[it.second.getSignature()] = method;
 		}
 	}
 	doc["methods"] = methods;
diff --git a/Types.cpp b/Types.cpp
index d5b00163..c5f1a3ae 100644
--- a/Types.cpp
+++ b/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.second->getName()] = make_shared<FunctionType>(*it.second.second, false);
+			members[it.second.getName()] = it.second.getFunctionTypeShared();
 		m_members.reset(new MemberList(members));
 	}
 	return *m_members;
@@ -511,9 +511,9 @@ shared_ptr<FunctionType const> const& ContractType::getConstructorType() const
 u256 ContractType::getFunctionIdentifier(string const& _functionName) const
 {
 	auto interfaceFunctions = m_contract.getInterfaceFunctions();
-	for (auto it = interfaceFunctions.cbegin(); it != interfaceFunctions.cend(); ++it)
-		if (it->second.second->getName() == _functionName)
-			return FixedHash<4>::Arith(it->first);
+	for (auto const& it: m_contract.getInterfaceFunctions())
+		if (it.second.getName() == _functionName)
+			return FixedHash<4>::Arith(it.first);
 
 	return Invalid256;
 }
@@ -582,28 +582,47 @@ FunctionType::FunctionType(FunctionDefinition const& _function, bool _isInternal
 	m_location(_isInternal ? Location::INTERNAL : Location::EXTERNAL)
 {
 	TypePointers params;
+	vector<string> paramNames;
 	TypePointers retParams;
+	vector<string> retParamNames;
 	params.reserve(_function.getParameters().size());
+	paramNames.reserve(_function.getParameters().size());
 	for (ASTPointer<VariableDeclaration> const& var: _function.getParameters())
+	{
+		paramNames.push_back(var->getName());
 		params.push_back(var->getType());
+	}
 	retParams.reserve(_function.getReturnParameters().size());
+	retParamNames.reserve(_function.getReturnParameters().size());
 	for (ASTPointer<VariableDeclaration> const& var: _function.getReturnParameters())
+	{
+		retParamNames.push_back(var->getName());
 		retParams.push_back(var->getType());
+	}
 	swap(params, m_parameterTypes);
+	swap(paramNames, m_parameterNames);
 	swap(retParams, m_returnParameterTypes);
+	swap(retParamNames, m_returnParameterNames);
 }
 
 FunctionType::FunctionType(VariableDeclaration const& _varDecl):
 	m_location(Location::INTERNAL)
 {
 	TypePointers params;
+	vector<string> paramNames;
 	TypePointers retParams;
+	vector<string> retParamNames;
 	// for now, no input parameters LTODO: change for some things like mapping
 	params.reserve(0);
+	paramNames.reserve(0);
 	retParams.reserve(1);
+	retParamNames.reserve(1);
 	retParams.push_back(_varDecl.getType());
+	retParamNames.push_back(_varDecl.getName());
 	swap(params, m_parameterTypes);
+	swap(paramNames, m_parameterNames);
 	swap(retParams, m_returnParameterTypes);
+	swap(retParamNames, m_returnParameterNames);
 }
 
 bool FunctionType::operator==(Type const& _other) const
diff --git a/Types.h b/Types.h
index b3a92021..b26157b3 100644
--- a/Types.h
+++ b/Types.h
@@ -365,7 +365,9 @@ public:
 		m_location(_location), m_gasSet(_gasSet), m_valueSet(_valueSet) {}
 
 	TypePointers const& getParameterTypes() const { return m_parameterTypes; }
+	std::vector<std::string> const& getParameterNames() const { return m_parameterNames; }
 	TypePointers const& getReturnParameterTypes() const { return m_returnParameterTypes; }
+	std::vector<std::string> const& getReturnParameterNames() const { return m_returnParameterNames; }
 
 	virtual bool operator==(Type const& _other) const override;
 	virtual std::string toString() const override;
@@ -390,6 +392,8 @@ private:
 
 	TypePointers m_parameterTypes;
 	TypePointers m_returnParameterTypes;
+	std::vector<std::string> m_parameterNames;
+	std::vector<std::string> m_returnParameterNames;
 	Location const m_location;
 	bool const m_gasSet = false; ///< true iff the gas value to be used is on the stack
 	bool const m_valueSet = false; ///< true iff the value to be sent is on the stack