Export oft-used types

[mheiber]

Export more types that meet these criteria:

• Documented
• Convenient for client code. They are already
  used in specs of public functions of their respective
  modules.
• These types do not expose implementation details.

The types are:

ets.erl:

• type() :: set | ordered_set | bag | duplicate_bag.

supervisor.erl:

• `restart() :: 'permanent' | 'transient' | 'temporary'
• shutdown() :: 'brutal_kill' | timeout().

The reason for this change is to fix the status quo, which is
that teams either must:

• copy/pasta the types into client code, leading to code bloat and having
  to manually keep the types in sync.
• OR refer to non-exported types, which will mean either more noise
  or less accuracy from code analysis tools. For example,
  Dialyzer will either warn or treat the types as term().

[IngelaAndin]

We discussed this in the PS team, and we feel that the exports are motivated. We would however prefer that the type mfargs() is changed to only mfa(), which we think is the natural name. @rickard-green what do the VM team think?

Or just realized it is probably because of the inclusion of the internal undefined. Maybe it should be changed to mfa() | sup_internal() which can be specified as undefined. I just remembered after reading the docs again that is used for temporary processes instead of saving potentially big argument lists for no purpose. That could make it clearer that undefined is nothing that should be used by the user.

[paulo-ferraz-oliveira]

Do you think this reasoning (probably clearer than mine, initially) could be applied to #2961, which I could resurrect?

Edit: I closed it (#2961), at that time, understanding that it wouldn't be accepted, but it was probably not discussed enough.

[mheiber]

@IngelaAndin , I updated the PR to not export mfargs(), since that type requires more care than the others.
I can do a follow-on for mfargs, following your suggestion in #2994 (comment).

[KennethL]

We are not through the internal discussion about when and why to export types in general yet. Will try to finish that soon.
When it comes to the specific types in this PR I have the following so far personal opinions:

• ets:type(), could be ok to export but I really think it should be named table_type() in that case.
• re:mp(), today there are no types at all exported from the re module and the mp() type is intended to be opaque so I wonder why you propose to export that one? Exporting that one requires some thinking both about its name and its spec.
• supervisor:shutdown() , would be ok to export and is analogous with the newly introduced type auto_shutdown() which is exported. For consistency this one should also be exported.

[mheiber]

We are not through the internal discussion about when and why to export types in general yet. Will try to finish that soon.

What is the best way for me to contribute and to follow the work? Since you're taking a broader look at the general problem, would it be easier for you if I close this PR and open an issue in the bug tracker instead?

[mheiber]

• re:mp(), today there are no types at all exported from the re module and the mp() type is intended to be opaque so I wonder why you propose to export that one? Exporting that one requires some thinking both about its name and its spec.

We have code that takes regexes as parameters and then uses the re module under the hood.

fwiw, my two cents is there is sufficient reason to expose a type (or an opaque wrapper of a type) when the type appears in the signature of an exported function. Otherwise it can be hard to spec functions that use those exported functions.

[KennethL]

We are not through the internal discussion about when and why to export types in general yet. Will try to finish that soon.

What is the best way for me to contribute and to follow the work? Since you're taking a broader look at the general problem, would it be easier for you if I close this PR and open an issue in the bug tracker instead?

Yes I think it is a good idea to close the PR and that you create an issue per API where you think there are types that should be exported. In this case one for ets one for supervisor and one for re. This since the PR does not add any valuable code and we might want to change the name if a type goes from internal to external.

[mheiber]

Yes I think it is a good idea to close the PR and that you create an issue per API where you think there are types that should be exported.

Thanks @KennethL , I've converted to issues:

• export type ets:type/1 #4968
• export opaque wrapper for re:mp/0 #4969
• export supervisor types: restart/0, mfargs/0 shutdown/0 #4970