Merge pull request #1747 from ethereum/fixICEInternalConstructor

Move privateness of constructor into AST itself.

Changelog.md

@@ -18,6 +18,7 @@ Bugfixes:
  * Type system: Fix a crash related to invalid binary operators.
  * Type system: Disallow ``var`` declaration with empty tuple type.
  * Type system: Correctly convert function argument types to pointers for member functions.
+ * Type system: Move privateness of constructor into AST itself.
  * Inline assembly: Charge one stack slot for non-value types during analysis.
  * Assembly output: Print source location before the operation it refers to instead of after.
  * Optimizer: Stop trying to optimize tricky constants after a while.

libsolidity/analysis/TypeChecker.cpp

@@ -77,8 +77,6 @@ bool TypeChecker::visit(ContractDefinition const& _contract)
 	FunctionDefinition const* function = _contract.constructor();
 	if (function)
 	{
-		if (!function->isPublic())
-			_contract.annotation().hasPublicConstructor = false;
 		if (!function->returnParameters().empty())
 			typeError(function->returnParameterList()->location(), "Non-empty \"returns\" directive for constructor.");
 		if (function->isDeclaredConst())
@@ -1305,7 +1303,7 @@ void TypeChecker::endVisit(NewExpression const& _newExpression)
 			fatalTypeError(_newExpression.location(), "Identifier is not a contract.");
 		if (!contract->annotation().isFullyImplemented)
 			typeError(_newExpression.location(), "Trying to create an instance of an abstract contract.");
-		if (!contract->annotation().hasPublicConstructor)
+		if (!contract->constructorIsPublic())
 			typeError(_newExpression.location(), "Contract with internal constructor cannot be created directly.");
 
 		solAssert(!!m_scope, "");

libsolidity/ast/AST.cpp

@@ -83,7 +83,7 @@ SourceUnitAnnotation& SourceUnit::annotation() const
 {
 	if (!m_annotation)
 		m_annotation = new SourceUnitAnnotation();
-	return static_cast<SourceUnitAnnotation&>(*m_annotation);
+	return dynamic_cast<SourceUnitAnnotation&>(*m_annotation);
 }
 
 string Declaration::sourceUnitName() const
@@ -99,7 +99,7 @@ ImportAnnotation& ImportDirective::annotation() const
 {
 	if (!m_annotation)
 		m_annotation = new ImportAnnotation();
-	return static_cast<ImportAnnotation&>(*m_annotation);
+	return dynamic_cast<ImportAnnotation&>(*m_annotation);
 }
 
 TypePointer ImportDirective::type() const
@@ -132,6 +132,12 @@ FunctionDefinition const* ContractDefinition::constructor() const
 	return nullptr;
 }
 
+bool ContractDefinition::constructorIsPublic() const
+{
+	FunctionDefinition const* f = constructor();
+	return !f || f->isPublic();
+}
+
 FunctionDefinition const* ContractDefinition::fallbackFunction() const
 {
 	for (ContractDefinition const* contract: annotation().linearizedBaseContracts)
@@ -255,14 +261,14 @@ ContractDefinitionAnnotation& ContractDefinition::annotation() const
 {
 	if (!m_annotation)
 		m_annotation = new ContractDefinitionAnnotation();
-	return static_cast<ContractDefinitionAnnotation&>(*m_annotation);
+	return dynamic_cast<ContractDefinitionAnnotation&>(*m_annotation);
 }
 
 TypeNameAnnotation& TypeName::annotation() const
 {
 	if (!m_annotation)
 		m_annotation = new TypeNameAnnotation();
-	return static_cast<TypeNameAnnotation&>(*m_annotation);
+	return dynamic_cast<TypeNameAnnotation&>(*m_annotation);
 }
 
 TypePointer StructDefinition::type() const
@@ -274,7 +280,7 @@ TypeDeclarationAnnotation& StructDefinition::annotation() const
 {
 	if (!m_annotation)
 		m_annotation = new TypeDeclarationAnnotation();
-	return static_cast<TypeDeclarationAnnotation&>(*m_annotation);
+	return dynamic_cast<TypeDeclarationAnnotation&>(*m_annotation);
 }
 
 TypePointer EnumValue::type() const
@@ -293,7 +299,7 @@ TypeDeclarationAnnotation& EnumDefinition::annotation() const
 {
 	if (!m_annotation)
 		m_annotation = new TypeDeclarationAnnotation();
-	return static_cast<TypeDeclarationAnnotation&>(*m_annotation);
+	return dynamic_cast<TypeDeclarationAnnotation&>(*m_annotation);
 }
 
 shared_ptr<FunctionType> FunctionDefinition::functionType(bool _internal) const
@@ -349,7 +355,7 @@ FunctionDefinitionAnnotation& FunctionDefinition::annotation() const
 {
 	if (!m_annotation)
 		m_annotation = new FunctionDefinitionAnnotation();
-	return static_cast<FunctionDefinitionAnnotation&>(*m_annotation);
+	return dynamic_cast<FunctionDefinitionAnnotation&>(*m_annotation);
 }
 
 TypePointer ModifierDefinition::type() const
@@ -361,7 +367,7 @@ ModifierDefinitionAnnotation& ModifierDefinition::annotation() const
 {
 	if (!m_annotation)
 		m_annotation = new ModifierDefinitionAnnotation();
-	return static_cast<ModifierDefinitionAnnotation&>(*m_annotation);
+	return dynamic_cast<ModifierDefinitionAnnotation&>(*m_annotation);
 }
 
 TypePointer EventDefinition::type() const
@@ -381,14 +387,14 @@ EventDefinitionAnnotation& EventDefinition::annotation() const
 {
 	if (!m_annotation)
 		m_annotation = new EventDefinitionAnnotation();
-	return static_cast<EventDefinitionAnnotation&>(*m_annotation);
+	return dynamic_cast<EventDefinitionAnnotation&>(*m_annotation);
 }
 
 UserDefinedTypeNameAnnotation& UserDefinedTypeName::annotation() const
 {
 	if (!m_annotation)
 		m_annotation = new UserDefinedTypeNameAnnotation();
-	return static_cast<UserDefinedTypeNameAnnotation&>(*m_annotation);
+	return dynamic_cast<UserDefinedTypeNameAnnotation&>(*m_annotation);
 }
 
 bool VariableDeclaration::isLValue() const
@@ -460,70 +466,70 @@ VariableDeclarationAnnotation& VariableDeclaration::annotation() const
 {
 	if (!m_annotation)
 		m_annotation = new VariableDeclarationAnnotation();
-	return static_cast<VariableDeclarationAnnotation&>(*m_annotation);
+	return dynamic_cast<VariableDeclarationAnnotation&>(*m_annotation);
 }
 
 StatementAnnotation& Statement::annotation() const
 {
 	if (!m_annotation)
 		m_annotation = new StatementAnnotation();
-	return static_cast<StatementAnnotation&>(*m_annotation);
+	return dynamic_cast<StatementAnnotation&>(*m_annotation);
 }
 
 InlineAssemblyAnnotation& InlineAssembly::annotation() const
 {
 	if (!m_annotation)
 		m_annotation = new InlineAssemblyAnnotation();
-	return static_cast<InlineAssemblyAnnotation&>(*m_annotation);
+	return dynamic_cast<InlineAssemblyAnnotation&>(*m_annotation);
 }
 
 ReturnAnnotation& Return::annotation() const
 {
 	if (!m_annotation)
 		m_annotation = new ReturnAnnotation();
-	return static_cast<ReturnAnnotation&>(*m_annotation);
+	return dynamic_cast<ReturnAnnotation&>(*m_annotation);
 }
 
 VariableDeclarationStatementAnnotation& VariableDeclarationStatement::annotation() const
 {
 	if (!m_annotation)
 		m_annotation = new VariableDeclarationStatementAnnotation();
-	return static_cast<VariableDeclarationStatementAnnotation&>(*m_annotation);
+	return dynamic_cast<VariableDeclarationStatementAnnotation&>(*m_annotation);
 }
 
 ExpressionAnnotation& Expression::annotation() const
 {
 	if (!m_annotation)
 		m_annotation = new ExpressionAnnotation();
-	return static_cast<ExpressionAnnotation&>(*m_annotation);
+	return dynamic_cast<ExpressionAnnotation&>(*m_annotation);
 }
 
 MemberAccessAnnotation& MemberAccess::annotation() const
 {
 	if (!m_annotation)
 		m_annotation = new MemberAccessAnnotation();
-	return static_cast<MemberAccessAnnotation&>(*m_annotation);
+	return dynamic_cast<MemberAccessAnnotation&>(*m_annotation);
 }
 
 BinaryOperationAnnotation& BinaryOperation::annotation() const
 {
 	if (!m_annotation)
 		m_annotation = new BinaryOperationAnnotation();
-	return static_cast<BinaryOperationAnnotation&>(*m_annotation);
+	return dynamic_cast<BinaryOperationAnnotation&>(*m_annotation);
 }
 
 FunctionCallAnnotation& FunctionCall::annotation() const
 {
 	if (!m_annotation)
 		m_annotation = new FunctionCallAnnotation();
-	return static_cast<FunctionCallAnnotation&>(*m_annotation);
+	return dynamic_cast<FunctionCallAnnotation&>(*m_annotation);
 }
 
 IdentifierAnnotation& Identifier::annotation() const
 {
 	if (!m_annotation)
 		m_annotation = new IdentifierAnnotation();
-	return static_cast<IdentifierAnnotation&>(*m_annotation);
+	return dynamic_cast<IdentifierAnnotation&>(*m_annotation);
 }
 
 bool Literal::looksLikeAddress() const

libsolidity/ast/AST.h

@@ -356,6 +356,8 @@ public:
 
 	/// Returns the constructor or nullptr if no constructor was specified.
 	FunctionDefinition const* constructor() const;
+	/// @returns true iff the constructor of this contract is public (or non-existing).
+	bool constructorIsPublic() const;
 	/// Returns the fallback function or nullptr if no fallback function was specified.
 	FunctionDefinition const* fallbackFunction() const;
 

libsolidity/ast/ASTAnnotations.h

@@ -80,8 +80,6 @@ struct ContractDefinitionAnnotation: TypeDeclarationAnnotation, DocumentedAnnota
 {
 	/// Whether all functions are implemented.
 	bool isFullyImplemented = true;
-	/// Whether a public constructor (even the default one) is available.
-	bool hasPublicConstructor = true;
 	/// List of all (direct and indirect) base contracts in order from derived to
 	/// base, including the contract itself.
 	std::vector<ContractDefinition const*> linearizedBaseContracts;

libsolidity/interface/CompilerStack.cpp

@@ -637,7 +637,7 @@ void CompilerStack::compileContract(
 	if (
 		_compiledContracts.count(&_contract) ||
 		!_contract.annotation().isFullyImplemented ||
-		!_contract.annotation().hasPublicConstructor
+		!_contract.constructorIsPublic()
 	)
 		return;
 	for (auto const* dependency: _contract.annotation().contractDependencies)

test/libsolidity/SolidityNameAndTypeResolution.cpp

@@ -5101,6 +5101,24 @@ BOOST_AUTO_TEST_CASE(inconstructible_internal_constructor)
 	CHECK_ERROR(text, TypeError, "Contract with internal constructor cannot be created directly.");
 }
 
+BOOST_AUTO_TEST_CASE(inconstructible_internal_constructor_inverted)
+{
+	// Previously, the type information for A was not yet available at the point of
+	// "new A".
+	char const* text = R"(
+		contract B {
+			A a;
+			function B() {
+				a = new A(this);
+			}
+		}
+		contract A {
+			function A(address a) internal {}
+		}
+	)";
+	CHECK_ERROR(text, TypeError, "Contract with internal constructor cannot be created directly.");
+}
+
 BOOST_AUTO_TEST_CASE(constructible_internal_constructor)
 {
 	char const* text = R"(