Take out automatic insertion of 'undefined' from typed record fields

Background
-----------
In record fields with a type declaration but without an initializer, the
Erlang parser inserted automatically the singleton type 'undefined' to
the list of declared types, if that value was not present there.
I.e. the record declaration:

   -record(rec, {f1      :: float(),
                 f2 = 42 :: integer(),
                 f3      :: some_mod:some_typ()}).

was translated by the parser to:

   -record(rec, {f1      :: float() | 'undefined',
                 f2 = 42 :: integer(),
                 f3      :: some_mod:some_typ() | 'undefined'}).

The rationale for this was that creation of a "dummy" #rec{} record
should not result in a warning from dialyzer that e.g. the implicit
initialization of the #rec.f1 field violates its type declaration.

Problems
---------
This seemingly innocent action has some unforeseen consequences.

For starters, there is no way for programmers to declare that e.g. only
floats make sense for the f1 field of #rec{} records when there is no
`obvious' default initializer for this field. (This also affects tools
like PropEr that use these declarations produced by the Erlang parser to
generate random instances of records for testing purposes.)

It also means that dialyzer does not warn if e.g. an is_atom/1 test or
something more exotic like an atom_to_list/1 call is performed on the
value of the f1 field.

Similarly, there is no way to extend dialyzer to warn if it finds record
constructions where f1 is not initialized to some float.

Last but not least, it is semantically problematic when the type of the
field is an opaque type: creating a union of an opaque and a structured
type is very problematic for analysis because it fundamentally breaks
the opacity of the term at that point.

Change
-------
To solve these problems the parser will not automatically insert the
'undefined' value anymore; instead the user has the option to choose the
places where this value makes sense (for the field) and where it does
not and insert the | 'undefined' there manually.

Consequences of this change
----------------------------
This change means that dialyzer will issue a warning for all places
where records with uninitialized fields are created and those fields have
a declared type that is incompatible with 'undefined' (e.g. float()).
This warning can be suppressed easily by adding | 'undefined' to the
type of this field.  This also adds documentation that the user really
intends to create records where this field is uninitialized.

14 files changed, 63 insertions, 49 deletions

diff --git a/lib/dialyzer/test/behaviour_SUITE_data/results/callbacks_and_specs b/lib/dialyzer/test/behaviour_SUITE_data/results/callbacks_and_specs
index 33d135048e..38999e8919 100644
--- a/lib/dialyzer/test/behaviour_SUITE_data/results/callbacks_and_specs
+++ b/lib/dialyzer/test/behaviour_SUITE_data/results/callbacks_and_specs
@@ -1,5 +1,5 @@
 
-my_callbacks_wrong.erl:26: The return type #state{parent::'undefined' | pid(),status::'closed' | 'init' | 'open',subscribe::[{pid(),integer()}],counter::integer()} in the specification of callback_init/1 is not a subtype of {'ok',_}, which is the expected return type for the callback of my_behaviour behaviour
-my_callbacks_wrong.erl:28: The inferred return type of callback_init/1 (#state{parent::'undefined' | pid(),status::'init',subscribe::[],counter::1}) has nothing in common with {'ok',_}, which is the expected return type for the callback of my_behaviour behaviour
-my_callbacks_wrong.erl:30: The return type {'reply',#state{parent::'undefined' | pid(),status::'closed' | 'init' | 'open',subscribe::[{pid(),integer()}],counter::integer()}} in the specification of callback_cast/3 is not a subtype of {'noreply',_}, which is the expected return type for the callback of my_behaviour behaviour
+my_callbacks_wrong.erl:26: The return type #state{parent::pid(),status::'closed' | 'init' | 'open',subscribe::[{pid(),integer()}],counter::integer()} in the specification of callback_init/1 is not a subtype of {'ok',_}, which is the expected return type for the callback of my_behaviour behaviour
+my_callbacks_wrong.erl:28: The inferred return type of callback_init/1 (#state{parent::pid(),status::'init',subscribe::[],counter::1}) has nothing in common with {'ok',_}, which is the expected return type for the callback of my_behaviour behaviour
+my_callbacks_wrong.erl:30: The return type {'reply',#state{parent::pid(),status::'closed' | 'init' | 'open',subscribe::[{pid(),integer()}],counter::integer()}} in the specification of callback_cast/3 is not a subtype of {'noreply',_}, which is the expected return type for the callback of my_behaviour behaviour
 my_callbacks_wrong.erl:39: The specified type for the 2nd argument of callback_call/3 (atom()) is not a supertype of pid(), which is expected type for this argument in the callback of the my_behaviour behaviour
diff --git a/lib/dialyzer/test/opaque_SUITE_data/results/crash b/lib/dialyzer/test/opaque_SUITE_data/results/crash
index 69bdc00257..d63389f79c 100644
--- a/lib/dialyzer/test/opaque_SUITE_data/results/crash
+++ b/lib/dialyzer/test/opaque_SUITE_data/results/crash
@@ -1,6 +1,6 @@
 
-crash_1.erl:45: Record construction #targetlist{list::[]} violates the declared type of field list::'undefined' | crash_1:target()
-crash_1.erl:48: The call crash_1:get_using_branch2(Branch::maybe_improper_list(),L::'undefined' | crash_1:target()) will never return since it differs in the 2nd argument from the success typing arguments: (any(),maybe_improper_list())
-crash_1.erl:50: The pattern <_Branch, []> can never match the type <maybe_improper_list(),'undefined' | crash_1:target()>
-crash_1.erl:52: The pattern <Branch, [H = {'target', _, _} | _T]> can never match the type <maybe_improper_list(),'undefined' | crash_1:target()>
-crash_1.erl:54: The pattern <Branch, [{'target', _, _} | T]> can never match the type <maybe_improper_list(),'undefined' | crash_1:target()>
+crash_1.erl:45: Record construction #targetlist{list::[]} violates the declared type of field list::crash_1:target()
+crash_1.erl:48: The call crash_1:get_using_branch2(Branch::maybe_improper_list(),L::crash_1:target()) will never return since it differs in the 2nd argument from the success typing arguments: (any(),maybe_improper_list())
+crash_1.erl:50: The pattern <_Branch, []> can never match the type <maybe_improper_list(),crash_1:target()>
+crash_1.erl:52: The pattern <Branch, [H = {'target', _, _} | _T]> can never match the type <maybe_improper_list(),crash_1:target()>
+crash_1.erl:54: The pattern <Branch, [{'target', _, _} | T]> can never match the type <maybe_improper_list(),crash_1:target()>
diff --git a/lib/dialyzer/test/opaque_SUITE_data/results/simple b/lib/dialyzer/test/opaque_SUITE_data/results/simple
index 1a7a139d6e..5f58f69730 100644
--- a/lib/dialyzer/test/opaque_SUITE_data/results/simple
+++ b/lib/dialyzer/test/opaque_SUITE_data/results/simple
@@ -18,7 +18,7 @@ rec_api.erl:104: Matching of pattern {'r2', 10} tagged with a record name violat
 rec_api.erl:113: The attempt to match a term of type #r3{f1::queue:queue(_)} against the pattern {'r3', 'a'} breaks the opaqueness of queue:queue(_)
 rec_api.erl:118: Record construction #r3{f1::10} violates the declared type of field f1::queue:queue(_)
 rec_api.erl:123: The attempt to match a term of type #r3{f1::10} against the pattern {'r3', 10} breaks the opaqueness of queue:queue(_)
-rec_api.erl:24: Record construction #r1{f1::10} violates the declared type of field f1::'undefined' | rec_api:a()
+rec_api.erl:24: Record construction #r1{f1::10} violates the declared type of field f1::rec_api:a()
 rec_api.erl:29: Matching of pattern {'r1', 10} tagged with a record name violates the declared type of #r1{f1::10}
 rec_api.erl:33: The attempt to match a term of type rec_adt:r1() against the pattern {'r1', 'a'} breaks the opaqueness of the term
 rec_api.erl:35: Invalid type specification for function rec_api:adt_t1/1. The success typing is (#r1{f1::'a'}) -> #r1{f1::'a'}
diff --git a/lib/dialyzer/test/r9c_SUITE_data/results/inets b/lib/dialyzer/test/r9c_SUITE_data/results/inets
index d377f34978..89be9652e3 100644
--- a/lib/dialyzer/test/r9c_SUITE_data/results/inets
+++ b/lib/dialyzer/test/r9c_SUITE_data/results/inets
@@ -37,6 +37,7 @@ mod_cgi.erl:372: The pattern {'http_response', NewAccResponse} can never match t
 mod_dir.erl:101: The call lists:flatten(nonempty_improper_list(atom() | [any()] | char(),atom() | {'no_translation',binary()})) will never return since it differs in the 1st argument from the success typing arguments: ([any()])
 mod_dir.erl:72: The pattern {'error', Reason} can never match the type {'ok',[[[any()] | char()],...]}
 mod_get.erl:135: The pattern <{'enfile', _}, _Info, Path> can never match the type <atom(),#mod{},atom() | binary() | [atom() | [any()] | char()]>
+mod_get.erl:150: Record construction #file_info{} violates the declared type of field size::non_neg_integer() and type::'device' | 'directory' | 'other' | 'regular' | 'symlink' and access::'none' | 'read' | 'read_write' | 'write' and atime::non_neg_integer() | {{non_neg_integer(),1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 | 10 | 11 | 12,1..255},{byte(),byte(),byte()}} and mtime::non_neg_integer() | {{non_neg_integer(),1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 | 10 | 11 | 12,1..255},{byte(),byte(),byte()}} and ctime::non_neg_integer() | {{non_neg_integer(),1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 | 10 | 11 | 12,1..255},{byte(),byte(),byte()}} and mode::non_neg_integer() and links::non_neg_integer() and major_device::non_neg_integer() and minor_device::non_neg_integer() and inode::non_neg_integer() and uid::non_neg_integer() and gid::non_neg_integer()
 mod_head.erl:80: The pattern <{'enfile', _}, _Info, Path> can never match the type <atom(),#mod{},atom() | binary() | [atom() | [any()] | char()]>
 mod_htaccess.erl:460: The pattern {'error', BadData} can never match the type {'ok',_}
 mod_include.erl:193: The pattern {_, Name, {[], []}} can never match the type {[any()],[any()],maybe_improper_list()}
@@ -47,6 +48,7 @@ mod_include.erl:692: The pattern <{'read', Reason}, Info, Path> can never match
 mod_include.erl:706: The pattern <{'enfile', _}, _Info, Path> can never match the type <atom(),#mod{},atom() | binary() | [atom() | [any()] | char()]>
 mod_include.erl:716: Function read_error/3 will never be called
 mod_include.erl:719: Function read_error/4 will never be called
+mod_range.erl:308: Record construction #file_info{} violates the declared type of field size::non_neg_integer() and type::'device' | 'directory' | 'other' | 'regular' | 'symlink' and access::'none' | 'read' | 'read_write' | 'write' and atime::non_neg_integer() | {{non_neg_integer(),1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 | 10 | 11 | 12,1..255},{byte(),byte(),byte()}} and mtime::non_neg_integer() | {{non_neg_integer(),1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 | 10 | 11 | 12,1..255},{byte(),byte(),byte()}} and ctime::non_neg_integer() | {{non_neg_integer(),1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 | 10 | 11 | 12,1..255},{byte(),byte(),byte()}} and mode::non_neg_integer() and links::non_neg_integer() and major_device::non_neg_integer() and minor_device::non_neg_integer() and inode::non_neg_integer() and uid::non_neg_integer() and gid::non_neg_integer()
 mod_security_server.erl:386: The variable O can never match since previous clauses completely covered the type [tuple()]
 mod_security_server.erl:433: The variable Other can never match since previous clauses completely covered the type [tuple()]
 mod_security_server.erl:585: The variable _ can never match since previous clauses completely covered the type [tuple()]
diff --git a/lib/dialyzer/test/small_SUITE_data/results/literals b/lib/dialyzer/test/small_SUITE_data/results/literals
index 03e161ca71..222d2c0cdb 100644
--- a/lib/dialyzer/test/small_SUITE_data/results/literals
+++ b/lib/dialyzer/test/small_SUITE_data/results/literals
@@ -1,14 +1,14 @@
 
 literals.erl:11: Function t1/0 has no local return
-literals.erl:12: Record construction #r{id::'a'} violates the declared type of field id::'integer' | 'undefined'
+literals.erl:12: Record construction #r{id::'a'} violates the declared type of field id::'integer'
 literals.erl:14: Function t2/0 has no local return
-literals.erl:15: Record construction #r{id::'a'} violates the declared type of field id::'integer' | 'undefined'
+literals.erl:15: Record construction #r{id::'a'} violates the declared type of field id::'integer'
 literals.erl:17: Function t3/0 has no local return
-literals.erl:18: Record construction #r{id::'a'} violates the declared type of field id::'integer' | 'undefined'
-literals.erl:21: Record construction #r{id::'a'} violates the declared type of field id::'integer' | 'undefined'
+literals.erl:18: Record construction #r{id::'a'} violates the declared type of field id::'integer'
+literals.erl:21: Record construction #r{id::'a'} violates the declared type of field id::'integer'
 literals.erl:23: Function m1/1 has no local return
-literals.erl:23: Matching of pattern {'r', 'a'} tagged with a record name violates the declared type of #r{id::'integer' | 'undefined'}
+literals.erl:23: Matching of pattern {'r', 'a'} tagged with a record name violates the declared type of #r{id::'integer'}
 literals.erl:26: Function m2/1 has no local return
-literals.erl:26: Matching of pattern {'r', 'a'} tagged with a record name violates the declared type of #r{id::'integer' | 'undefined'}
+literals.erl:26: Matching of pattern {'r', 'a'} tagged with a record name violates the declared type of #r{id::'integer'}
 literals.erl:29: Function m3/1 has no local return
 literals.erl:29: The pattern {{'r', 'a'}} can never match the type any()
diff --git a/lib/dialyzer/test/small_SUITE_data/results/record_creation_diffs b/lib/dialyzer/test/small_SUITE_data/results/record_creation_diffs
index f00c4b10ff..c971935bbf 100644
--- a/lib/dialyzer/test/small_SUITE_data/results/record_creation_diffs
+++ b/lib/dialyzer/test/small_SUITE_data/results/record_creation_diffs
@@ -1,3 +1,3 @@
 
 record_creation_diffs.erl:10: Function foo/1 has no local return
-record_creation_diffs.erl:11: Record construction #bar{some_list::{'this','is','a','tuple'}} violates the declared type of field some_list::'undefined' | [any()]
+record_creation_diffs.erl:11: Record construction #bar{some_list::{'this','is','a','tuple'}} violates the declared type of field some_list::[any()]
diff --git a/lib/dialyzer/test/small_SUITE_data/results/record_pat b/lib/dialyzer/test/small_SUITE_data/results/record_pat
index a46be6c451..8317ea041a 100644
--- a/lib/dialyzer/test/small_SUITE_data/results/record_pat
+++ b/lib/dialyzer/test/small_SUITE_data/results/record_pat
@@ -1,2 +1,2 @@
 
-record_pat.erl:14: Matching of pattern {'foo', 'baz'} tagged with a record name violates the declared type of #foo{bar::'undefined' | integer()}
+record_pat.erl:14: Matching of pattern {'foo', 'baz'} tagged with a record name violates the declared type of #foo{bar::integer()}
diff --git a/lib/dialyzer/test/small_SUITE_data/results/relevant_record_warning b/lib/dialyzer/test/small_SUITE_data/results/relevant_record_warning
index 2e417e1b2a..ea3ac92d96 100644
--- a/lib/dialyzer/test/small_SUITE_data/results/relevant_record_warning
+++ b/lib/dialyzer/test/small_SUITE_data/results/relevant_record_warning
@@ -1,3 +1,3 @@
 
 relevant_record_warning.erl:22: Function test/1 has no local return
-relevant_record_warning.erl:23: Record construction #r{field::<<_:8>>} violates the declared type of field field::'binary' | 'undefined'
+relevant_record_warning.erl:23: Record construction #r{field::<<_:8>>} violates the declared type of field field::'binary'
diff --git a/lib/dialyzer/test/small_SUITE_data/results/undefined b/lib/dialyzer/test/small_SUITE_data/results/undefined
new file mode 100644
index 0000000000..9daa8640d3
--- /dev/null
+++ b/lib/dialyzer/test/small_SUITE_data/results/undefined
@@ -0,0 +1,2 @@
+
+r.erl:16: Record construction #r{a::{'fi'},b::{'a','b'},c::[],d::'undefined',e::[],f::'undefined'} violates the declared type of field b::[any()] and d::[any()] and f::[any()]
diff --git a/lib/dialyzer/test/small_SUITE_data/src/trec.erl b/lib/dialyzer/test/small_SUITE_data/src/trec.erl
index 06706162c1..516358f7c6 100644
--- a/lib/dialyzer/test/small_SUITE_data/src/trec.erl
+++ b/lib/dialyzer/test/small_SUITE_data/src/trec.erl
@@ -8,7 +8,7 @@
 -module(trec).
 -export([test/0, mk_foo_exp/2]).
 
--record(foo, {a :: integer(), b :: [atom()]}).
+-record(foo, {a :: integer() | 'undefined', b :: [atom()]}).
 
 %%
 %% For these functions we currently get the following warnings:
diff --git a/lib/dialyzer/test/small_SUITE_data/undefined.erl b/lib/dialyzer/test/small_SUITE_data/undefined.erl
new file mode 100644
index 0000000000..8549f2e161
--- /dev/null
+++ b/lib/dialyzer/test/small_SUITE_data/undefined.erl
@@ -0,0 +1,29 @@
+-module(undefined).
+
+-export([t/0]).
+
+%% As of OTP 19.0 'undefined' is no longer added to fields with a type
+%% declaration but without an initializer. The pretty printing of
+%% records (erl_types:t_to_string()) is updated to reflect this: if a
+%% field is of type 'undefined', it is output if 'undefined' is not in
+%% the declared type of the field. (It used to be the case that the
+%% singleton type 'undefined' was never output.)
+%%
+%% One consequence is shown by the example below: the warning about
+%% the record construction violating the the declared type shows
+%% #r{..., d::'undefined', ...} which is meant to be of help to the
+%% user, who could otherwise get confused the first time (s)he gets
+%% confronted by the warning.
+
+-record(r,
+        {
+          a = {fi},
+          b = {a,b} :: list(),                  % violation
+          c = {a,b} :: list(),
+          d :: list(),                          % violation
+          e = [] :: list(),
+          f = undefined :: list()               % violation
+        }).
+
+t() ->
+    #r{c = []}.
diff --git a/lib/hipe/cerl/erl_types.erl b/lib/hipe/cerl/erl_types.erl
index cd2d2fe207..420d7e2a8f 100644
--- a/lib/hipe/cerl/erl_types.erl
+++ b/lib/hipe/cerl/erl_types.erl
@@ -3974,18 +3974,17 @@ record_to_string(Tag, [_|Fields], FieldNames, RecDict) ->
   FieldStrings = record_fields_to_string(Fields, FieldNames, RecDict, []),
   "#" ++ atom_to_string(Tag) ++ "{" ++ string:join(FieldStrings, ",") ++ "}".
 
-record_fields_to_string([F|Fs], [{FName, _Abstr, _DefType}|FDefs],
+record_fields_to_string([F|Fs], [{FName, _Abstr, DefType}|FDefs],
                         RecDict, Acc) ->
   NewAcc =
-    case t_is_equal(F, t_any()) orelse t_is_any_atom('undefined', F) of
+    case
+      t_is_equal(F, t_any()) orelse
+      (t_is_any_atom('undefined', F) andalso
+       not t_is_none(t_inf(F, DefType)))
+    of
       true -> Acc;
       false ->
 	StrFV = atom_to_string(FName) ++ "::" ++ t_to_string(F, RecDict),
-	%% ActualDefType = t_subtract(DefType, t_atom('undefined')),
-	%% Str = case t_is_any(ActualDefType) of
-	%% 	  true -> StrFV;
-	%% 	  false -> StrFV ++ "::" ++ t_to_string(ActualDefType, RecDict)
-	%%	end,
 	[StrFV|Acc]
     end,
   record_fields_to_string(Fs, FDefs, RecDict, NewAcc);
diff --git a/lib/stdlib/src/erl_parse.yrl b/lib/stdlib/src/erl_parse.yrl
index 206b0c6e7b..ae42a8f0b1 100644
--- a/lib/stdlib/src/erl_parse.yrl
+++ b/lib/stdlib/src/erl_parse.yrl
@@ -790,31 +790,11 @@ record_fields([{match,_Am,{atom,Aa,A},Expr}|Fields]) ->
     [{record_field,Aa,{atom,Aa,A},Expr}|record_fields(Fields)];
 record_fields([{typed,Expr,TypeInfo}|Fields]) ->
     [Field] = record_fields([Expr]),
-    TypeInfo1 =
-	case Expr of
-	    {match, _, _, _} -> TypeInfo; %% If we have an initializer.
-	    {atom, Aa, _} ->
-                case has_undefined(TypeInfo) of
-                    false ->
-                        lift_unions(abstract2(undefined, Aa), TypeInfo);
-                    true ->
-                        TypeInfo
-                end
-	end,
-    [{typed_record_field,Field,TypeInfo1}|record_fields(Fields)];
+    [{typed_record_field,Field,TypeInfo}|record_fields(Fields)];
 record_fields([Other|_Fields]) ->
     ret_err(?anno(Other), "bad record field");
 record_fields([]) -> [].
 
-has_undefined({atom,_,undefined}) ->
-    true;
-has_undefined({ann_type,_,[_,T]}) ->
-    has_undefined(T);
-has_undefined({type,_,union,Ts}) ->
-    lists:any(fun has_undefined/1, Ts);
-has_undefined(_) ->
-    false.
-
 term(Expr) ->
     try normalise(Expr)
     catch _:_R -> ret_err(?anno(Expr), "bad attribute")
diff --git a/lib/stdlib/test/erl_pp_SUITE.erl b/lib/stdlib/test/erl_pp_SUITE.erl
index 389fd059f6..9bb193d865 100644
--- a/lib/stdlib/test/erl_pp_SUITE.erl
+++ b/lib/stdlib/test/erl_pp_SUITE.erl
@@ -928,7 +928,9 @@ otp_8522(Config) when is_list(Config) ->
     ?line {ok, _} = compile:file(FileName, [{outdir,?privdir},debug_info]),
     BF = filename("otp_8522", Config),
     ?line {ok, A} = beam_lib:chunks(BF, [abstract_code]),
-    ?line 5 = count_atom(A, undefined),
+    %% OTP-12719: Since 'undefined' is no longer added by the Erlang
+    %% Parser, the number of 'undefined' is 4. It used to be 5.
+    ?line 4 = count_atom(A, undefined),
     ok.
 
 count_atom(A, A) ->
@@ -1062,7 +1064,7 @@ otp_9147(Config) when is_list(Config) ->
     ?line {ok, Bin} = file:read_file(PFileName),
     %% The parentheses around "F1 :: a | b" are new (bugfix).
     ?line true = 
-        lists:member("-record(undef,{f1 :: undefined | (F1 :: a | b)}).",
+        lists:member("-record(undef,{f1 :: F1 :: a | b}).",
                      string:tokens(binary_to_list(Bin), "\n")),
     ok.