stdlib: fix exported types in supervisor, gen_server, gen_event #7205

kikofernandez · 2023-05-03T18:27:21Z

Exports types mentioned in the documentation of supervisor, gen_server, and gen_event

Closes #6893

github-actions · 2023-05-03T19:40:04Z

CT Test Results

      4 files   167 suites 2h 23m 32s ⏱️
2 624 tests 2 361 ✔️ 263 💤 0 ❌
3 027 runs 2 749 ✔️ 278 💤 0 ❌

Results for commit e2043c8.

♻️ This comment has been updated with latest results.

IngelaAndin · 2023-05-08T09:53:03Z

This feels kind of backwards! Should we not have a generic type that used by gen_statem, gen_server and supervisors? Supervisors actually are implemented as gen_servers.

KennethL

I think this needs to be taken up at an OTB since it has quite much impact on the public API.
If types are exported they should have well thought names that we are happy with and would like to keep.
When exporting types they become part of the public API that we have to keep compatible, When the types where only mentioned in the doc but not exported they where only for manual use and we could change them whenever we wanted.

kikofernandez · 2023-05-10T05:51:15Z

This feels kind of backwards! Should we not have a generic type that used by gen_statem, gen_server and supervisors? Supervisors actually are implemented as gen_servers.

What do you mean with this? that the types from gen_servers are the ones that everyone references? or that the types from gen_server are the ones that everyone references under an alias (similar to what we have now but I chose supervisor to be the "global" type to be shared and every other module creates an alias towards it?

IngelaAndin · 2023-05-10T07:17:14Z

@kikofernandez I mean that not all gen_statems or gen_servers are supervisors, rather they are mostly workers and supervisors are implemented as gen_servers. So the generic process behaviors should not reference types of the specific supervisor behavior rather the other way around, or probably we should export a gen-type that can be referenced by all the behaviors.

kikofernandez · 2023-06-02T09:17:31Z

@IngelaAndin @KennethL I have made changes as suggested in the OTB thread, where proc_lib is the one that exports those types, which are consumed by gen_XXX.erl modules

IngelaAndin

I think proc_lib:sup_name should be proc_lib:process_name, or maybe shorter proc_name. Otherwise I like it now.

kikofernandez · 2023-06-07T14:43:46Z

@IngelaAndin for consistency, I think that if we have not ever exported gen_xxx:sup_ref() and others, then these should be renamed as well as gen_xxx:process_ref()?

IngelaAndin · 2023-06-09T07:17:17Z

@kikofernandez I agree with you. I think it makes much more sense to use the names from the main building blocks to represent common types.

kikofernandez · 2023-06-12T20:20:09Z

@IngelaAndin I have updated the gen_xxx:server_ref() and gen_xxx:server_name() to gen_xxx:process_ref() and gen_xxx:process_name(). Exactly the same for gen.erl. There is a file named peer.erl that already exports server_ref(), so I cannot touch that one. Let me know if this changes are ok with you :)

IngelaAndin · 2023-06-14T14:38:21Z

lib/stdlib/doc/src/gen_event.xml

@@ -142,8 +142,12 @@ gen_event:stop             ----->  Module:terminate/2
    </datatype>

    <datatype>
-      <name name="emgr_ref"/>
-    </datatype>    
+      <name name="process_ref"/>


emgr is not a clear name I think, I guess it stands for event manager. But in this particular place I am not sure if it is obvious if we should use the generalized type or if we should have a type for this abstraction?! I think the same goes for gen_server and supervisor module. I will make a comment in those place too, so that we can have comments from the @erlang/team-ps

Or maybe it is the case that we should not have new types with the same name but let the documentation refer to
proc_lib:process_name and so on!

The point on making aliases an exporting the aliases is to provide a type abstraction over it, so that theoretically, in a common type system, gen_event:process_ref() is not the same as other type that happens to match its internal representation. That said, the current type checker (Dialyzer) will expand the types to its constituents, so for Dialyzer gen_event:process_ref() and proc_lib:process_ref() are the same.
(AFAIK, Dialyzer is a structural type system and will establish the equality of types as mentioned above)

Regarding the generalisation of the type, I know think you mean that emgr_ref() is more concrete than process_ref().
I would like to favour uniformity, so that we can create a "type" understanding without having to know that an emgr_ref() is equivalent to what is known in other modules as process_ref()

IngelaAndin · 2023-06-14T14:39:10Z

lib/stdlib/doc/src/gen_server.xml

            </p>
          </item>
        </taglist>
      </desc>
    </datatype>

    <datatype>
-      <name name="server_ref"/>
+      <name name="process_ref"/>


Here is one place

IngelaAndin · 2023-06-14T14:40:20Z

lib/stdlib/doc/src/gen_server.xml

@@ -122,7 +122,7 @@ gen_server:abcast     -----> Module:handle_cast/2

  <datatypes>
    <datatype>
-      <name name="server_name"/>
+      <name name="process_name"/>
      <desc>


Here is one place

IngelaAndin · 2023-06-14T14:41:46Z

lib/stdlib/doc/src/supervisor.xml

@@ -402,7 +402,10 @@ child_spec() = #{id => child_id(),             % mandatory
        <seeerl marker="#sup_flags">above</seeerl>.</p></desc>
    </datatype>
    <datatype>
-      <name name="sup_ref"/>
+      <name name="process_ref"/>
+    </datatype>


Here is one place

IngelaAndin · 2023-06-14T14:43:05Z

lib/stdlib/doc/src/supervisor.xml

@@ -606,7 +609,7 @@ child_spec() = #{id => child_id(),             % mandatory
      <fsummary>Create a supervisor process.</fsummary>
      <type name="startlink_ret"/>
      <type name="startlink_err"/>
-      <type name="sup_name"/>
+      <type name="process_name"/>
      <desc>


Here is one place

IngelaAndin · 2023-06-14T14:43:45Z

lib/stdlib/doc/src/gen_statem.xml

@@ -512,26 +512,26 @@ handle_event(_, _, State, Data) ->

  <datatypes>
    <datatype>
-      <name name="server_name"/>
+      <name name="process_name"/>


Here is one place

kikofernandez · 2023-08-16T12:08:28Z

@rickard-green Could you take a look at this PR and let us know what you think of the types?

rickard-green added the team:PS Assigned to OTP team PS label May 3, 2023

kikofernandez self-assigned this May 8, 2023

IngelaAndin requested a review from KennethL May 8, 2023 07:58

kikofernandez requested a review from IngelaAndin May 8, 2023 07:58

KennethL suggested changes May 8, 2023

View reviewed changes

kikofernandez force-pushed the kiko/stdlib/fix-types-supervisor-and-behaviours/OTP-18573 branch from 81bd801 to 3eb6e8e Compare June 2, 2023 13:38

kikofernandez requested review from KennethL and IngelaAndin and removed request for IngelaAndin June 2, 2023 13:56

kikofernandez force-pushed the kiko/stdlib/fix-types-supervisor-and-behaviours/OTP-18573 branch 2 times, most recently from ade2f5c to ad49f80 Compare June 5, 2023 07:48

IngelaAndin requested changes Jun 5, 2023

View reviewed changes

IngelaAndin approved these changes Jun 5, 2023

View reviewed changes

IngelaAndin requested a review from rickard-green June 5, 2023 11:49

kikofernandez force-pushed the kiko/stdlib/fix-types-supervisor-and-behaviours/OTP-18573 branch 2 times, most recently from ab7170d to 944ce98 Compare June 5, 2023 12:29

kikofernandez added the testing currently being tested, tag is used by OTP internal CI label Jun 5, 2023

kikofernandez mentioned this pull request Jun 7, 2023

supervisor: export sup_ref type #6893

Closed

kikofernandez removed the testing currently being tested, tag is used by OTP internal CI label Jun 12, 2023

kikofernandez added the testing currently being tested, tag is used by OTP internal CI label Jun 13, 2023

IngelaAndin reviewed Jun 14, 2023

View reviewed changes

kikofernandez added 8 commits August 16, 2023 13:54

proc_lib: export shared types

8789b60

gen_event: replace supervisor types by proc_libs

39d8e6c

gen: replace types by proc_libs

c13ddca

gen_statem: replace types by proc_libs

0d98b98

supervisor: use proc_libs types

129e047

gen_server: use proc_libs types

14dbf86

stdlib: replace proc_lib's name

19a37c1

stdlib: add consistent naming to gen_XXX

e2043c8

kikofernandez force-pushed the kiko/stdlib/fix-types-supervisor-and-behaviours/OTP-18573 branch from 5965f42 to e2043c8 Compare August 16, 2023 11:56

IngelaAndin added the stalled waiting for input by the Erlang/OTP team label Aug 30, 2023

kikofernandez removed the testing currently being tested, tag is used by OTP internal CI label Sep 11, 2023

IngelaAndin added team:VM Assigned to OTP team VM and removed team:PS Assigned to OTP team PS labels Jan 23, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

stdlib: fix exported types in supervisor, gen_server, gen_event #7205

stdlib: fix exported types in supervisor, gen_server, gen_event #7205

kikofernandez commented May 3, 2023 •

edited

Loading

github-actions bot commented May 3, 2023 •

edited

Loading

IngelaAndin commented May 8, 2023

KennethL left a comment

kikofernandez commented May 10, 2023

IngelaAndin commented May 10, 2023 •

edited

Loading

kikofernandez commented Jun 2, 2023

IngelaAndin left a comment

kikofernandez commented Jun 7, 2023

IngelaAndin commented Jun 9, 2023

kikofernandez commented Jun 12, 2023

IngelaAndin Jun 14, 2023

IngelaAndin Jun 14, 2023

kikofernandez Aug 16, 2023

kikofernandez Aug 16, 2023

IngelaAndin Jun 14, 2023

IngelaAndin Jun 14, 2023

IngelaAndin Jun 14, 2023

IngelaAndin Jun 14, 2023

IngelaAndin Jun 14, 2023

kikofernandez commented Aug 16, 2023

stdlib: fix exported types in supervisor, gen_server, gen_event #7205

Are you sure you want to change the base?

stdlib: fix exported types in supervisor, gen_server, gen_event #7205

Conversation

kikofernandez commented May 3, 2023 • edited Loading

github-actions bot commented May 3, 2023 • edited Loading

CT Test Results

IngelaAndin commented May 8, 2023

KennethL left a comment

Choose a reason for hiding this comment

kikofernandez commented May 10, 2023

IngelaAndin commented May 10, 2023 • edited Loading

kikofernandez commented Jun 2, 2023

IngelaAndin left a comment

Choose a reason for hiding this comment

kikofernandez commented Jun 7, 2023

IngelaAndin commented Jun 9, 2023

kikofernandez commented Jun 12, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kikofernandez commented Aug 16, 2023

kikofernandez commented May 3, 2023 •

edited

Loading

github-actions bot commented May 3, 2023 •

edited

Loading

IngelaAndin commented May 10, 2023 •

edited

Loading