Clean up error reporting for duplicate tags

Split the test case duplicate_tags/1 into two parts. Do the
error checking test in error_SUITE. Keep the SeqOptional2
specification and compile it from the per/1 and ber_other/1
test cases (for coverage).

5 files changed, 106 insertions, 155 deletions

diff --git a/lib/asn1/src/asn1ct_check.erl b/lib/asn1/src/asn1ct_check.erl
index d76ecee3b9..fa70f7b18c 100644
--- a/lib/asn1/src/asn1ct_check.erl
+++ b/lib/asn1/src/asn1ct_check.erl
@@ -4565,7 +4565,7 @@ check_sequenceof(S,Type,Component) when is_record(Component,type) ->
 
 check_set(S,Type,Components) ->
     {TableCInf,NewComponents} = check_sequence(S,Type,Components),
-    check_distinct_tags(NewComponents,[]),
+    check_unique_tags(S, collect_components(NewComponents), []),
     case {lists:member(der,S#state.options),S#state.erule} of
 	{true,_} ->
 	    {Sorted,SortedComponents} = sort_components(der,S,NewComponents),
@@ -4577,35 +4577,21 @@ check_set(S,Type,Components) ->
 	    {false,TableCInf,NewComponents}
     end.
 
-
-%% check that all tags are distinct according to X.680 26.3
-check_distinct_tags({C1,C2,C3},Acc) when is_list(C1),is_list(C2),is_list(C3) ->
-    check_distinct_tags(C1++C2++C3,Acc);
-check_distinct_tags({C1,C2},Acc) when is_list(C1),is_list(C2) ->
-    check_distinct_tags(C1++C2,Acc);
-check_distinct_tags([#'ComponentType'{tags=[T]}|Cs],Acc) ->
-    check_distinct(T,Acc),
-    check_distinct_tags(Cs,[T|Acc]);
-check_distinct_tags([C=#'ComponentType'{tags=[T|Ts]}|Cs],Acc) ->
-    check_distinct(T,Acc),
-    check_distinct_tags([C#'ComponentType'{tags=Ts}|Cs],[T|Acc]);
-check_distinct_tags([#'ComponentType'{tags=[]}|_Cs],_Acc) ->
-    throw({error,"Not distinct tags in SET"});
-check_distinct_tags([],_) ->
-    ok.
-check_distinct(T,Acc) ->
-    case lists:member(T,Acc) of
-	true ->
-	    throw({error,"Not distinct tags in SET"});
-	_ -> ok
-    end.
+collect_components({C1,C2,C3}) ->
+    collect_components(C1++C2++C3);
+collect_components({C1,C2}) ->
+    collect_components(C1++C2);
+collect_components(Cs) ->
+    %% Assert that tags are not empty
+    [] = [EmptyTag || EmptyTag = #'ComponentType'{tags=[]} <- Cs],
+    Cs.
 
 %% sorting in canonical order according to X.680 8.6, X.691 9.2
 %% DER: all components shall be sorted in canonical order.
 %% PER: only root components shall be sorted in canonical order. The
 %%      extension components shall remain in textual order.
 %%
-sort_components(der,S=#state{tname=TypeName},Components) ->
+sort_components(der, S, Components) ->
     {R1,Ext,R2} = extension(textual_order(Components)),
     CompsList = case Ext of
 		    noext -> R1;
@@ -4613,88 +4599,34 @@ sort_components(der,S=#state{tname=TypeName},Components) ->
 		end,
     case {untagged_choice(S,CompsList),Ext} of
 	{false,noext} ->
-	    {true,sort_components1(S,TypeName,CompsList,[],[],[],[])};
+	    {true,sort_components1(CompsList)};
 	{false,_} ->
-	    {true,{sort_components1(S,TypeName,CompsList,[],[],[],[]), []}};
+	    {true,{sort_components1(CompsList),[]}};
 	{true,noext} ->
 	    %% sort in run-time
 	    {dynamic,R1};
 	_ ->
 	    {dynamic,{R1, Ext, R2}}
     end;
-sort_components(per,S=#state{tname=TypeName},Components) ->
+sort_components(per, S, Components) ->
     {R1,Ext,R2} = extension(textual_order(Components)),
     Root = tag_untagged_choice(S,R1++R2),
     case Ext of
 	noext ->
-	    {true,sort_components1(S,TypeName,Root,[],[],[],[])};
+	    {true,sort_components1(Root)};
 	_ ->
-	    {true,{sort_components1(S,TypeName,Root,[],[],[],[]),
-		   Ext}}
+	    {true,{sort_components1(Root),Ext}}
     end.
 
-sort_components1(S,TypeName,[C=#'ComponentType'{tags=[{'UNIVERSAL',_}|_R]}|Cs],
-		 UnivAcc,ApplAcc,ContAcc,PrivAcc) ->
-    sort_components1(S,TypeName,Cs,[C|UnivAcc],ApplAcc,ContAcc,PrivAcc);
-sort_components1(S,TypeName,[C=#'ComponentType'{tags=[{'APPLICATION',_}|_R]}|Cs],
-		 UnivAcc,ApplAcc,ContAcc,PrivAcc) ->
-    sort_components1(S,TypeName,Cs,UnivAcc,[C|ApplAcc],ContAcc,PrivAcc);
-sort_components1(S,TypeName,[C=#'ComponentType'{tags=[{'CONTEXT',_}|_R]}|Cs],
-		 UnivAcc,ApplAcc,ContAcc,PrivAcc) ->
-    sort_components1(S,TypeName,Cs,UnivAcc,ApplAcc,[C|ContAcc],PrivAcc);
-sort_components1(S,TypeName,[C=#'ComponentType'{tags=[{'PRIVATE',_}|_R]}|Cs],
-		 UnivAcc,ApplAcc,ContAcc,PrivAcc) ->
-    sort_components1(S,TypeName,Cs,UnivAcc,ApplAcc,ContAcc,[C|PrivAcc]);
-sort_components1(S,TypeName,[],UnivAcc,ApplAcc,ContAcc,PrivAcc) ->
-    I = #'ComponentType'.tags,
-    ascending_order_check(S,TypeName,sort_universal_type(UnivAcc)) ++
-	ascending_order_check(S,TypeName,lists:keysort(I,ApplAcc)) ++
-	ascending_order_check(S,TypeName,lists:keysort(I,ContAcc)) ++
-	ascending_order_check(S,TypeName,lists:keysort(I,PrivAcc)).
-
-ascending_order_check(S,TypeName,Components) ->
-    ascending_order_check1(S,TypeName,Components),
-    Components.
-
-ascending_order_check1(S,TypeName,
-		       [C1 = #'ComponentType'{tags=[{_,T}|_]},
-			C2 = #'ComponentType'{tags=[{_,T}|_]}|Rest]) ->
-    asn1ct:warning("Indistinct tag ~p in SET ~p, components ~p and ~p~n",
-		   [T,TypeName,C1#'ComponentType'.name,C2#'ComponentType'.name],S,
-		   "Indistinct tag in SET"),
-    ascending_order_check1(S,TypeName,[C2|Rest]);
-ascending_order_check1(S,TypeName,
-		       [C1 = #'ComponentType'{tags=[{'UNIVERSAL',T1}|_]},
-			C2 = #'ComponentType'{tags=[{'UNIVERSAL',T2}|_]}|Rest]) ->
-    case (decode_type(T1) == decode_type(T2)) of
-	true ->
-	    asn1ct:warning("Indistinct tags ~p and ~p in"
-			   " SET ~p, components ~p and ~p~n",
-			   [T1,T2,TypeName,C1#'ComponentType'.name,
-			    C2#'ComponentType'.name],S,
-			   "Indistinct tags and in SET"),
-	    ascending_order_check1(S,TypeName,[C2|Rest]);
-	_ ->
-	    ascending_order_check1(S,TypeName,[C2|Rest])
-    end;
-ascending_order_check1(S,N,[_|Rest]) ->
-    ascending_order_check1(S,N,Rest);
-ascending_order_check1(_,_,[]) ->
-    ok.
-    
-sort_universal_type(Components) ->
-    List = lists:map(fun(C) ->
-			     #'ComponentType'{tags=[{_,T}|_]} = C,
-			     {decode_type(T),C}
-		     end,
-		     Components),
-    SortedList = lists:keysort(1,List),
-    lists:map(fun(X)->element(2,X) end,SortedList).
-
-decode_type(I) when is_integer(I) ->
-    I;
-decode_type(T) ->
-    asn1ct_gen_ber_bin_v2:decode_type(T).
+sort_components1(Cs0) ->
+    Cs1 = [{tag_key(Tag),C} || #'ComponentType'{tags=[Tag|_]}=C <- Cs0],
+    Cs = lists:sort(Cs1),
+    [C || {_,C} <- Cs].
+
+tag_key({'UNIVERSAL',Tag}) ->   {0,Tag};
+tag_key({'APPLICATION',Tag}) -> {1,Tag};
+tag_key({'CONTEXT',Tag}) ->     {2,Tag};
+tag_key({'PRIVATE',Tag}) ->     {3,Tag}.
 
 untagged_choice(_S,[#'ComponentType'{typespec=#type{tag=[],def={'CHOICE',_}}}|_Rest]) ->
     true;
@@ -4948,25 +4880,30 @@ check_unique_tags(S,C) ->
     case (S#state.module)#module.tagdefault of
 	'AUTOMATIC' ->
 	    case any_manual_tag(C) of
-		false -> true;
-		_ -> collect_and_sort_tags(C,[])
+		false ->
+		    true;
+		true ->
+		    check_unique_tags(S, C, [])
 	    end;
 	_ ->
-    	    collect_and_sort_tags(C,[])
+	    check_unique_tags(S, C, [])
     end.
 
-collect_and_sort_tags([C|Rest],Acc) when is_record(C,'ComponentType') ->
-    collect_and_sort_tags(Rest,C#'ComponentType'.tags ++ Acc);
-collect_and_sort_tags([_|Rest],Acc) ->
-    collect_and_sort_tags(Rest,Acc);
-collect_and_sort_tags([],Acc) ->
-    {Dupl,_}= lists:mapfoldl(fun(El,El)->{{dup,El},El};(El,_Prev)-> {El,El} end,notag,lists:sort(Acc)),
-    Dupl2 = [Dup|| {dup,Dup} <- Dupl],
-    if 
-	length(Dupl2) > 0 ->
-	    throw({error,{asn1,{duplicates_of_the_tags,Dupl2}}});
-	true ->
-	    true
+check_unique_tags(S, [#'ComponentType'{name=Name,tags=Tags0}|T], Acc) ->
+    Tags = [{Tag,Name} || Tag <- Tags0],
+    check_unique_tags(S, T, Tags ++ Acc);
+check_unique_tags(S, [_|T], Acc) ->
+    check_unique_tags(S, T, Acc);
+check_unique_tags(S, [], Acc) ->
+    R0 = sofs:relation(Acc),
+    R1 = sofs:relation_to_family(R0),
+    R2 = sofs:to_external(R1),
+    Dup = [Els || {_,[_,_|_]=Els} <- R2],
+    case Dup of
+	[] ->
+	    ok;
+	[FirstDupl|_] ->
+	    asn1_error(S, {duplicate_tags,FirstDupl})
     end.
 
 check_unique(L,Pos) ->
@@ -5921,6 +5858,9 @@ asn1_error(S, Item, Error) ->
 format_error({already_defined,Name,PrevLine}) ->
     io_lib:format("the name ~p has already been defined at line ~p",
 		  [Name,PrevLine]);
+format_error({duplicate_tags,Elements}) ->
+    io_lib:format("duplicate tags in the elements: ~s",
+		  [format_elements(Elements)]);
 format_error(illegal_bitstring_value) ->
     "expecting a BIT STRING value";
 format_error({illegal_class_name,Class}) ->
@@ -6003,6 +5943,11 @@ format_fields([H|T]) ->
     [io_lib:format("fields '&~s'", [H])|
      [io_lib:format(", '&~s'", [F]) || F <- T]].
 
+format_elements([H1,H2|T]) ->
+    [io_lib:format("~p, ", [H1])|format_elements([H2|T])];
+format_elements([H]) ->
+    io_lib:format("~p", [H]).
+
 error({_,{structured_error,_,_,_}=SE,_}) ->
     SE;
 error({type,Msg,#state{mname=Mname,type=Type,tname=Typename}}) 
diff --git a/lib/asn1/src/asn1ct_gen_ber_bin_v2.erl b/lib/asn1/src/asn1ct_gen_ber_bin_v2.erl
index 41f86046f0..4dedd6305c 100644
--- a/lib/asn1/src/asn1ct_gen_ber_bin_v2.erl
+++ b/lib/asn1/src/asn1ct_gen_ber_bin_v2.erl
@@ -24,7 +24,7 @@
 
 -include("asn1_records.hrl").
 
--export([decode_class/1, decode_type/1]).
+-export([decode_class/1]).
 -export([gen_encode/2,gen_encode/3,gen_decode/2,gen_decode/3]).
 -export([gen_encode_prim/4]).
 -export([gen_dec_prim/3]).
@@ -1544,39 +1544,6 @@ decode_class('CONTEXT') ->
 decode_class('PRIVATE') ->
     ?PRIVATE.
 
-decode_type('BOOLEAN') -> 1;
-decode_type('INTEGER') -> 2;
-decode_type('BIT STRING') -> 3; 
-decode_type('OCTET STRING') -> 4; 
-decode_type('NULL') -> 5;
-decode_type('OBJECT IDENTIFIER') -> 6;
-decode_type('ObjectDescriptor') -> 7;
-decode_type('EXTERNAL') -> 8;
-decode_type('REAL') -> 9;
-decode_type('ENUMERATED') -> 10;
-decode_type('EMBEDDED_PDV') -> 11;
-decode_type('UTF8String') -> 12;
-decode_type('RELATIVE-OID') -> 13;
-decode_type('SEQUENCE') -> 16;
-decode_type('SEQUENCE OF') -> 16;
-decode_type('SET') -> 17;
-decode_type('SET OF') -> 17;
-decode_type('NumericString') -> 18;  
-decode_type('PrintableString') -> 19;  
-decode_type('TeletexString') -> 20;  
-decode_type('T61String') -> 20;
-decode_type('VideotexString') -> 21;  
-decode_type('IA5String') -> 22;  
-decode_type('UTCTime') -> 23;  
-decode_type('GeneralizedTime') -> 24;  
-decode_type('GraphicString') -> 25;  
-decode_type('VisibleString') -> 26;  
-decode_type('GeneralString') -> 27;  
-decode_type('UniversalString') -> 28;  
-decode_type('BMPString') -> 30;
-decode_type('CHOICE') -> 'CHOICE'; % choice gets the tag from the actual alternative  
-decode_type(Else) -> exit({error,{asn1,{unrecognized_type,Else}}}).
-
 mkfuncname(#'Externaltypereference'{module=Mod,type=EType}, DecOrEnc) ->
     CurrMod = get(currmod),
     case CurrMod of
diff --git a/lib/asn1/test/asn1_SUITE.erl b/lib/asn1/test/asn1_SUITE.erl
index 3b6e18c46d..04ebbab599 100644
--- a/lib/asn1/test/asn1_SUITE.erl
+++ b/lib/asn1/test/asn1_SUITE.erl
@@ -148,7 +148,6 @@ groups() ->
        testMvrasn6,
        testContextSwitchingTypes,
        testOpenTypeImplicitTag,
-       duplicate_tags,
        testROSE,
        testINSTANCE_OF,
        testTCAP,
@@ -869,14 +868,6 @@ testOpenTypeImplicitTag(Config, Rule, Opts) ->
     asn1_test_lib:compile("OpenTypeImplicitTag", Config, [Rule|Opts]),
     testOpenTypeImplicitTag:main(Rule).
 
-duplicate_tags(Config) ->
-    DataDir = ?config(data_dir, Config),
-    CaseDir = ?config(case_dir, Config),
-    {error, [{error, {type, _, _, 'SeqOpt1Imp',
-			     {asn1, {duplicates_of_the_tags, _}}}}]} =
-	asn1ct:compile(filename:join(DataDir, "SeqOptional2"),
-		       [abs, {outdir, CaseDir}]).
-
 rtUI(Config) -> test(Config, fun rtUI/3).
 rtUI(Config, Rule, Opts) ->
     asn1_test_lib:compile("Prim", Config, [Rule|Opts]),
@@ -1147,7 +1138,8 @@ test_modules() ->
      "Def",
      "Opt",
      "ELDAPv3",
-     "LDAP"].
+     "LDAP",
+     "SeqOptional2"].
 
 test_OTP_9688(Config) ->
     PrivDir = ?config(case_dir, Config),
diff --git a/lib/asn1/test/asn1_SUITE_data/SeqOptional2.asn b/lib/asn1/test/asn1_SUITE_data/SeqOptional2.asn
index 7de9134096..bb85c9e418 100644
--- a/lib/asn1/test/asn1_SUITE_data/SeqOptional2.asn
+++ b/lib/asn1/test/asn1_SUITE_data/SeqOptional2.asn
@@ -15,10 +15,10 @@ SeqOpt1Imp ::= SEQUENCE
   bool1 [1] BOOLEAN OPTIONAL,
   int1  INTEGER,
   seq1  [2] SeqIn OPTIONAL,
-  seq2  [2] SeqIn OPTIONAL,
+  seq2  [3] SeqIn OPTIONAL,
   ...,
-  int2  [3] SeqIn,
-  int3  [3] SeqIn
+  int2  [4] SeqIn,
+  int3  [5] SeqIn
 }
 
 SeqOpt1Exp ::= SEQUENCE 
diff --git a/lib/asn1/test/error_SUITE.erl b/lib/asn1/test/error_SUITE.erl
index c386fd6d31..83ef5f226e 100644
--- a/lib/asn1/test/error_SUITE.erl
+++ b/lib/asn1/test/error_SUITE.erl
@@ -24,7 +24,7 @@
 	 imports_exports/1,instance_of/1,integers/1,objects/1,
 	 object_field_extraction/1,oids/1,rel_oids/1,
 	 object_sets/1,parameterization/1,
-	 syntax/1,table_constraints/1,values/1]).
+	 syntax/1,table_constraints/1,tags/1,values/1]).
 
 -include_lib("test_server/include/test_server.hrl").
 
@@ -51,6 +51,7 @@ groups() ->
        parameterization,
        syntax,
        table_constraints,
+       tags,
        values]}].
 
 parallel() ->
@@ -652,6 +653,52 @@ table_constraints(Config) ->
      ]} = run(P, Config),
     ok.
 
+tags(Config) ->
+    M = 'Tags',
+    P = {M,
+	 <<"Tags DEFINITIONS AUTOMATIC TAGS ::= BEGIN\n"
+	   "SeqOpt1 ::= SEQUENCE\n"
+	   "{\n"
+	   "bool1  BOOLEAN OPTIONAL,\n"
+	   "int1  INTEGER,\n"
+	   "seq1  SeqIn OPTIONAL\n"
+	   "}\n"
+
+	   "SeqOpt1Imp ::= SEQUENCE \n"
+	   "{\n"
+	   "bool1 [1] BOOLEAN OPTIONAL,\n"
+	   "int1  INTEGER,\n"
+	   "seq1  [2] SeqIn OPTIONAL,\n"
+	   "seq2  [2] SeqIn OPTIONAL,\n"
+	   "...,\n"
+	   "int2  [3] SeqIn,\n"
+	   "int3  [3] SeqIn\n"
+	   "}\n"
+
+	   "SeqIn ::= SEQUENCE \n"
+	   "{\n"
+	   "boolIn  BOOLEAN,\n"
+	   "intIn  INTEGER\n"
+	   "}\n"
+	   "\n"
+
+	   "Set1 ::= SET {\n"
+	   " os [0] OCTET STRING,\n"
+	   " bool [0] BOOLEAN\n"
+	   "}\n"
+
+	   "END\n">>},
+    {error,
+     [{structured_error,
+       {M,8},asn1ct_check,
+       {duplicate_tags,[seq1,seq2]}},
+      {structured_error,
+       {M,24},asn1ct_check,
+       {duplicate_tags,[bool,os]}}
+     ]} = run(P, Config),
+    ok.
+
+
 values(Config) ->
     M = 'Values',
     P = {M,