Allow constructor to be external?

[ethers]

One way to see is that a public is both internal and external.
So why can't a constructor be external given that it can be public?
Is public on a constructor identical to public for a function? If not, should there be a different keyword than changing the meaning of public for a constructor?

It would also be useful if marking a constructor external, prevented the contract from being inherited from.

(Related but wasn't resolved in #3132)

[chriseth]

Your argumentation makes sense, but somehow I feel that the keywords here do not directly translate to constructors, since you call a constructor only implicitly. Using an external external constructor to essentially create final contracts is also a little bit indirect for me.

[ethers]

I agree the keywords are awkward; and we should avoid external if the effect is intended to prevent inheritance since something like final is more understood.

Maybe following is the topic to focus on and find some other keyword than public for the constructor?

Is public on a constructor identical to public for a function? If not, should there be a different keyword than changing the meaning of public for a constructor?

[MicahZoltu]

IMO constructor should be external. It CANNOT be called internally, it can only ever be called by some external party. To put this another way, any function that you cannot flag as internal should not be flaggable as public.

Recommended Remediation: Remove visibility modifier from constructor entirely (on the argument that it doesn't really make sense) or make external the only option since internal doesn't make sense, and thus public doesn't make sense.

[MicahZoltu]

Thinking more on this, I think this may currently be used for abstract classes (where they should not be instantiated directly, always derived from). I would rather see a dedicated keyword for this like abstract contract than re-use internal on a constructor to represent this concept.

[jalextowle]

IMO constructor should be external. It CANNOT be called internally, it can only ever be called by some external party. To put this another way, any function that you cannot flag as internal should not be flaggable as public.

Recommended Remediation: Remove visibility modifier from constructor entirely (on the argument that it doesn't really make sense) or make external the only option since internal doesn't make sense, and thus public doesn't make sense.

Yep, I would second this. Even when calling new SomeContract(), create is used to deploy the bytecode to an address, which is definitely an external call. IMO the usage of the new keyword constitutes an internal call, but contract creation doesn’t.

I would push back a bit on the remediation though, but I think that your suggestion of using the abstract keyword rather than marking the constructor as internal would significantly clarify the behavior.

Thanks for commenting on this! I remember learning about the difference between constructor and function visibility a while back, and public really felt like a misnomer.

[chriseth]

Note that constructors of base contracts are called internally, while the most derived constructor could be say do be called externally. Once we have calldata data location for function parameters of internal function, this distinction is not too relevant anymore.

I agree that removing visibility is probably the cleanest solution.

[chriseth]

Calldata data locations for all variables is tracked here: #5545

[chriseth]

@Marenz remarks that there are internal types that can only be used with internal constructors.

[axic]

@Marenz remarks that there are internal types that can only be used with internal constructors.

Cannot we loosen that so constructors can see every type in the given contract?

[chriseth]

I understood that the problems are internal function pointers or storage pointers.

[axic]

@Marenz can you clarify your concern here?

[Marenz]

It's not exactly a concern, just a point to consider: A c'tor like constructor(function() internal pure _fp) currently must be internal. Without the keywords we have to decide how this case should be handled. Would it be implicitly internal because it uses internal arguments? Would be require abstract to be used for the contract?

[chriseth]

The solution I see is to require the contract to be abstract.

[chriseth]

Closing since external does not work with inheritance and we aim to remove visibility completely (#8162).