diff --git a/Changelog.md b/Changelog.md index a4063640c..1d3c47c12 100644 --- a/Changelog.md +++ b/Changelog.md @@ -16,6 +16,9 @@ Bugfixes: * Imports: ``import ".dir/a"`` is not a relative path. Relative paths begin with directory ``.`` or ``..``. * Type checker, disallow inheritances of different kinds (e.g. a function and a modifier) of members of the same name +Features: + * Contracts and libraries are now unique to their source files, rather than globally. + ### 0.4.7 (2016-12-15) Features: diff --git a/libsolidity/ast/AST.cpp b/libsolidity/ast/AST.cpp index 3db4627a2..fcd6e38c0 100644 --- a/libsolidity/ast/AST.cpp +++ b/libsolidity/ast/AST.cpp @@ -191,7 +191,6 @@ void ContractDefinition::setUserDocumentation(Json::Value const& _userDocumentat m_userDocumentation = _userDocumentation; } - vector const& ContractDefinition::inheritableMembers() const { if (!m_inheritableMembers) diff --git a/libsolidity/ast/AST.h b/libsolidity/ast/AST.h index e9df2e7d5..d11a246c2 100644 --- a/libsolidity/ast/AST.h +++ b/libsolidity/ast/AST.h @@ -165,6 +165,7 @@ public: /// @returns the source name this declaration is present in. /// Can be combined with annotation().canonicalName to form a globally unique name. std::string sourceUnitName() const; + std::string fullyQualifiedName() const { return sourceUnitName() + ":" + name(); } virtual bool isLValue() const { return false; } virtual bool isPartOfExternalInterface() const { return false; } diff --git a/libsolidity/codegen/ContractCompiler.cpp b/libsolidity/codegen/ContractCompiler.cpp index a0f196bc3..fa77c1aad 100644 --- a/libsolidity/codegen/ContractCompiler.cpp +++ b/libsolidity/codegen/ContractCompiler.cpp @@ -575,7 +575,7 @@ bool ContractCompiler::visit(InlineAssembly const& _inlineAssembly) else if (auto contract = dynamic_cast(decl)) { solAssert(contract->isLibrary(), ""); - _assembly.appendLibraryAddress(contract->name()); + _assembly.appendLibraryAddress(contract->fullyQualifiedName()); } else solAssert(false, "Invalid declaration type."); diff --git a/libsolidity/codegen/ExpressionCompiler.cpp b/libsolidity/codegen/ExpressionCompiler.cpp index 3922da881..37bd14583 100644 --- a/libsolidity/codegen/ExpressionCompiler.cpp +++ b/libsolidity/codegen/ExpressionCompiler.cpp @@ -892,7 +892,7 @@ bool ExpressionCompiler::visit(MemberAccess const& _memberAccess) solAssert(funType->location() == FunctionType::Location::DelegateCall, ""); auto contract = dynamic_cast(funType->declaration().scope()); solAssert(contract && contract->isLibrary(), ""); - m_context.appendLibraryAddress(contract->name()); + m_context.appendLibraryAddress(contract->fullyQualifiedName()); m_context << funType->externalIdentifier(); utils().moveIntoStack(funType->selfType()->sizeOnStack(), 2); } @@ -1270,7 +1270,7 @@ void ExpressionCompiler::endVisit(Identifier const& _identifier) else if (auto contract = dynamic_cast(declaration)) { if (contract->isLibrary()) - m_context.appendLibraryAddress(contract->name()); + m_context.appendLibraryAddress(contract->fullyQualifiedName()); } else if (dynamic_cast(declaration)) { diff --git a/libsolidity/interface/CompilerStack.cpp b/libsolidity/interface/CompilerStack.cpp index 08b217155..61fc77287 100644 --- a/libsolidity/interface/CompilerStack.cpp +++ b/libsolidity/interface/CompilerStack.cpp @@ -21,6 +21,7 @@ * Full-stack compiler that converts a source code string to bytecode. */ + #include #include @@ -180,11 +181,15 @@ bool CompilerStack::parse() if (!resolver.updateDeclaration(*m_globalContext->currentThis())) return false; if (!resolver.updateDeclaration(*m_globalContext->currentSuper())) return false; if (!resolver.resolveNamesAndTypes(*contract)) return false; - m_contracts[contract->name()].contract = contract; - } - if (!checkLibraryNameClashes()) - noErrors = false; + // Note that we now reference contracts by their fully qualified names, and + // thus contracts can only conflict if declared in the same source file. This + // already causes a double-declaration error elsewhere, so we do not report + // an error here and instead silently drop any additional contracts we find. + + if (m_contracts.find(contract->fullyQualifiedName()) == m_contracts.end()) + m_contracts[contract->fullyQualifiedName()].contract = contract; + } for (Source const* source: m_sourceOrder) for (ASTPointer const& node: source->ast->nodes()) @@ -201,7 +206,13 @@ bool CompilerStack::parse() else noErrors = false; - m_contracts[contract->name()].contract = contract; + // Note that we now reference contracts by their fully qualified names, and + // thus contracts can only conflict if declared in the same source file. This + // already causes a double-declaration error elsewhere, so we do not report + // an error here and instead silently drop any additional contracts we find. + + if (m_contracts.find(contract->fullyQualifiedName()) == m_contracts.end()) + m_contracts[contract->fullyQualifiedName()].contract = contract; } if (noErrors) @@ -315,6 +326,27 @@ string const* CompilerStack::runtimeSourceMapping(string const& _contractName) c return c.runtimeSourceMapping.get(); } +std::string const CompilerStack::filesystemFriendlyName(string const& _contractName) const +{ + // Look up the contract (by its fully-qualified name) + Contract const& matchContract = m_contracts.at(_contractName); + // Check to see if it could collide on name + for (auto const& contract: m_contracts) + { + if (contract.second.contract->name() == matchContract.contract->name() && + contract.second.contract != matchContract.contract) + { + // If it does, then return its fully-qualified name, made fs-friendly + std::string friendlyName = boost::algorithm::replace_all_copy(_contractName, "/", "_"); + boost::algorithm::replace_all(friendlyName, ":", "_"); + boost::algorithm::replace_all(friendlyName, ".", "_"); + return friendlyName; + } + } + // If no collision, return the contract's name + return matchContract.contract->name(); +} + eth::LinkerObject const& CompilerStack::object(string const& _contractName) const { return contract(_contractName).object; @@ -569,37 +601,6 @@ void CompilerStack::resolveImports() swap(m_sourceOrder, sourceOrder); } -bool CompilerStack::checkLibraryNameClashes() -{ - bool clashFound = false; - map libraries; - for (Source const* source: m_sourceOrder) - for (ASTPointer const& node: source->ast->nodes()) - if (ContractDefinition* contract = dynamic_cast(node.get())) - if (contract->isLibrary()) - { - if (libraries.count(contract->name())) - { - auto err = make_shared(Error::Type::DeclarationError); - *err << - errinfo_sourceLocation(contract->location()) << - errinfo_comment( - "Library \"" + contract->name() + "\" declared twice " - "(will create ambiguities during linking)." - ) << - errinfo_secondarySourceLocation(SecondarySourceLocation().append( - "The other declaration is here:", libraries[contract->name()] - )); - - m_errors.push_back(err); - clashFound = true; - } - else - libraries[contract->name()] = contract->location(); - } - return !clashFound; -} - string CompilerStack::absolutePath(string const& _path, string const& _reference) const { using path = boost::filesystem::path; @@ -628,7 +629,7 @@ void CompilerStack::compileContract( compileContract(*dependency, _compiledContracts); shared_ptr compiler = make_shared(m_optimize, m_optimizeRuns); - Contract& compiledContract = m_contracts.at(_contract.name()); + Contract& compiledContract = m_contracts.at(_contract.fullyQualifiedName()); string onChainMetadata = createOnChainMetadata(compiledContract); bytes cborEncodedMetadata = // CBOR-encoding of {"bzzr0": dev::swarmHash(onChainMetadata)} @@ -674,10 +675,27 @@ CompilerStack::Contract const& CompilerStack::contract(string const& _contractNa for (auto const& it: m_sources) for (ASTPointer const& node: it.second.ast->nodes()) if (auto contract = dynamic_cast(node.get())) - contractName = contract->name(); + contractName = contract->fullyQualifiedName(); auto it = m_contracts.find(contractName); - if (it == m_contracts.end()) + // To provide a measure of backward-compatibility, if a contract is not located by its + // fully-qualified name, a lookup will be attempted purely on the contract's name to see + // if anything will satisfy. + if (it == m_contracts.end() && contractName.find(":") == string::npos) + { + for (auto const& contractEntry: m_contracts) + { + stringstream ss; + ss.str(contractEntry.first); + // All entries are : + string source; + string foundName; + getline(ss, source, ':'); + getline(ss, foundName, ':'); + if (foundName == contractName) return contractEntry.second; + } + // If we get here, both lookup methods failed. BOOST_THROW_EXCEPTION(CompilerError() << errinfo_comment("Contract " + _contractName + " not found.")); + } return it->second; } diff --git a/libsolidity/interface/CompilerStack.h b/libsolidity/interface/CompilerStack.h index d49a8df14..61edc284a 100644 --- a/libsolidity/interface/CompilerStack.h +++ b/libsolidity/interface/CompilerStack.h @@ -148,6 +148,10 @@ public: /// @returns the string that provides a mapping between runtime bytecode and sourcecode. /// if the contract does not (yet) have bytecode. std::string const* runtimeSourceMapping(std::string const& _contractName = "") const; + + /// @returns either the contract's name or a mixture of its name and source file, sanitized for filesystem use + std::string const filesystemFriendlyName(std::string const& _contractName) const; + /// @returns hash of the runtime bytecode for the contract, i.e. the code that is /// returned by the constructor or the zero-h256 if the contract still needs to be linked or /// does not have runtime code. @@ -230,9 +234,6 @@ private: StringMap loadMissingSources(SourceUnit const& _ast, std::string const& _path); std::string applyRemapping(std::string const& _path, std::string const& _context); void resolveImports(); - /// Checks whether there are libraries with the same name, reports that as an error and - /// @returns false in this case. - bool checkLibraryNameClashes(); /// @returns the absolute path corresponding to @a _path relative to @a _reference. std::string absolutePath(std::string const& _path, std::string const& _reference) const; /// Helper function to return path converted strings. diff --git a/solc/CommandLineInterface.cpp b/solc/CommandLineInterface.cpp index e322455b8..e49e8517d 100644 --- a/solc/CommandLineInterface.cpp +++ b/solc/CommandLineInterface.cpp @@ -186,7 +186,7 @@ void CommandLineInterface::handleBinary(string const& _contract) if (m_args.count(g_argBinary)) { if (m_args.count(g_argOutputDir)) - createFile(_contract + ".bin", m_compiler->object(_contract).toHex()); + createFile(m_compiler->filesystemFriendlyName(_contract) + ".bin", m_compiler->object(_contract).toHex()); else { cout << "Binary: " << endl; diff --git a/test/libsolidity/Imports.cpp b/test/libsolidity/Imports.cpp index 56895fdc7..6aa96fb87 100644 --- a/test/libsolidity/Imports.cpp +++ b/test/libsolidity/Imports.cpp @@ -106,6 +106,7 @@ BOOST_AUTO_TEST_CASE(library_name_clash) CompilerStack c; c.addSource("a", "library A {} pragma solidity >=0.0;"); c.addSource("b", "library A {} pragma solidity >=0.0;"); + c.addSource("c", "import {A} from \"./a\"; import {A} from \"./b\";"); BOOST_CHECK(!c.compile()); } diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp index 191618317..03aa26999 100644 --- a/test/libsolidity/SolidityEndToEndTest.cpp +++ b/test/libsolidity/SolidityEndToEndTest.cpp @@ -3192,7 +3192,7 @@ BOOST_AUTO_TEST_CASE(library_call_in_homestead) } )"; compileAndRun(sourceCode, 0, "Lib"); - compileAndRun(sourceCode, 0, "Test", bytes(), map{{"Lib", m_contractAddress}}); + compileAndRun(sourceCode, 0, "Test", bytes(), map{{":Lib", m_contractAddress}}); BOOST_CHECK(callContractFunction("f()") == encodeArgs()); BOOST_CHECK(callContractFunction("sender()") == encodeArgs(u160(m_sender))); } @@ -6191,7 +6191,7 @@ BOOST_AUTO_TEST_CASE(library_call) } )"; compileAndRun(sourceCode, 0, "Lib"); - compileAndRun(sourceCode, 0, "Test", bytes(), map{{"Lib", m_contractAddress}}); + compileAndRun(sourceCode, 0, "Test", bytes(), map{{":Lib", m_contractAddress}}); BOOST_CHECK(callContractFunction("f(uint256)", u256(33)) == encodeArgs(u256(33) * 9)); } @@ -6208,7 +6208,7 @@ BOOST_AUTO_TEST_CASE(library_stray_values) } )"; compileAndRun(sourceCode, 0, "Lib"); - compileAndRun(sourceCode, 0, "Test", bytes(), map{{"Lib", m_contractAddress}}); + compileAndRun(sourceCode, 0, "Test", bytes(), map{{":Lib", m_contractAddress}}); BOOST_CHECK(callContractFunction("f(uint256)", u256(33)) == encodeArgs(u256(42))); } @@ -6341,7 +6341,7 @@ BOOST_AUTO_TEST_CASE(internal_types_in_library) } )"; compileAndRun(sourceCode, 0, "Lib"); - compileAndRun(sourceCode, 0, "Test", bytes(), map{{"Lib", m_contractAddress}}); + compileAndRun(sourceCode, 0, "Test", bytes(), map{{":Lib", m_contractAddress}}); BOOST_CHECK(callContractFunction("f()") == encodeArgs(u256(4), u256(17))); } @@ -6368,7 +6368,7 @@ BOOST_AUTO_TEST_CASE(using_library_structs) } )"; compileAndRun(sourceCode, 0, "Lib"); - compileAndRun(sourceCode, 0, "Test", bytes(), map{{"Lib", m_contractAddress}}); + compileAndRun(sourceCode, 0, "Test", bytes(), map{{":Lib", m_contractAddress}}); BOOST_CHECK(callContractFunction("f()") == encodeArgs(u256(7), u256(8))); } @@ -6902,7 +6902,7 @@ BOOST_AUTO_TEST_CASE(using_for_function_on_int) } )"; compileAndRun(sourceCode, 0, "D"); - compileAndRun(sourceCode, 0, "C", bytes(), map{{"D", m_contractAddress}}); + compileAndRun(sourceCode, 0, "C", bytes(), map{{":D", m_contractAddress}}); BOOST_CHECK(callContractFunction("f(uint256)", u256(9)) == encodeArgs(u256(2 * 9))); } @@ -6920,7 +6920,7 @@ BOOST_AUTO_TEST_CASE(using_for_function_on_struct) } )"; compileAndRun(sourceCode, 0, "D"); - compileAndRun(sourceCode, 0, "C", bytes(), map{{"D", m_contractAddress}}); + compileAndRun(sourceCode, 0, "C", bytes(), map{{":D", m_contractAddress}}); BOOST_CHECK(callContractFunction("f(uint256)", u256(7)) == encodeArgs(u256(3 * 7))); BOOST_CHECK(callContractFunction("x()") == encodeArgs(u256(3 * 7))); } @@ -6943,7 +6943,7 @@ BOOST_AUTO_TEST_CASE(using_for_overload) } )"; compileAndRun(sourceCode, 0, "D"); - compileAndRun(sourceCode, 0, "C", bytes(), map{{"D", m_contractAddress}}); + compileAndRun(sourceCode, 0, "C", bytes(), map{{":D", m_contractAddress}}); BOOST_CHECK(callContractFunction("f(uint256)", u256(7)) == encodeArgs(u256(6 * 7))); BOOST_CHECK(callContractFunction("x()") == encodeArgs(u256(6 * 7))); } @@ -6962,7 +6962,7 @@ BOOST_AUTO_TEST_CASE(using_for_by_name) } )"; compileAndRun(sourceCode, 0, "D"); - compileAndRun(sourceCode, 0, "C", bytes(), map{{"D", m_contractAddress}}); + compileAndRun(sourceCode, 0, "C", bytes(), map{{":D", m_contractAddress}}); BOOST_CHECK(callContractFunction("f(uint256)", u256(7)) == encodeArgs(u256(6 * 7))); BOOST_CHECK(callContractFunction("x()") == encodeArgs(u256(6 * 7))); } @@ -6982,7 +6982,7 @@ BOOST_AUTO_TEST_CASE(bound_function_in_var) } )"; compileAndRun(sourceCode, 0, "D"); - compileAndRun(sourceCode, 0, "C", bytes(), map{{"D", m_contractAddress}}); + compileAndRun(sourceCode, 0, "C", bytes(), map{{":D", m_contractAddress}}); BOOST_CHECK(callContractFunction("f(uint256)", u256(7)) == encodeArgs(u256(6 * 7))); BOOST_CHECK(callContractFunction("x()") == encodeArgs(u256(6 * 7))); } @@ -7005,7 +7005,7 @@ BOOST_AUTO_TEST_CASE(bound_function_to_string) } )"; compileAndRun(sourceCode, 0, "D"); - compileAndRun(sourceCode, 0, "C", bytes(), map{{"D", m_contractAddress}}); + compileAndRun(sourceCode, 0, "C", bytes(), map{{":D", m_contractAddress}}); BOOST_CHECK(callContractFunction("f()") == encodeArgs(u256(3))); BOOST_CHECK(callContractFunction("g()") == encodeArgs(u256(3))); } @@ -7751,7 +7751,7 @@ BOOST_AUTO_TEST_CASE(payable_function_calls_library) } )"; compileAndRun(sourceCode, 0, "L"); - compileAndRun(sourceCode, 0, "C", bytes(), map{{"L", m_contractAddress}}); + compileAndRun(sourceCode, 0, "C", bytes(), map{{":L", m_contractAddress}}); BOOST_CHECK(callContractFunctionWithValue("f()", 27) == encodeArgs(u256(7))); }