[m-rev.] for review: implement RTTI for io__write on .NET

Tyson Dowd trd at cs.mu.OZ.AU
Wed Sep 26 21:29:33 AEST 2001


On 26-Sep-2001, Mark Brown <dougl at cs.mu.OZ.AU> wrote:
> Hi Tyson,
> 
> Aside from the things mentioned below, this diff looks okay to me.
> 
> Cheers,
> Mark.
> 
> On Thu, Sep 20, 2001 at 02:56:30PM +0200, Tyson Dowd wrote:
> > Hi,
> > 
> > Most of the time spent on this was actually spent grappling with the 
> > problem that generated structs were being generated as nested arrays,
> > and not nested structures.
> > 
> > I attribute that debugging time to this change because the behaviour
> > might be considered correct, only that for the purposes of RTTI it isn't
> > what we want to be able to effectively use these data structures.
> > 
> > As a side effect of this change uncaught exceptions are now printed
> > correctly.
> 
> This comment would be useful in the log message, I think.
> 
> > 
> > ===================================================================
> > 
> > 
> > Estimated hours taken: 50
> > Branches: main
> > 
> > Implement most of the RTTI required for io__write to work in the .NET
> > backend.  With this code most of tests/hard_coded/write.m work (up until
> > the point where we try to write a univ).
> > 
> > We don't yet handle higher-order terms or existentially quantified type
> > variables.
> > 
> > library/io.m:
> > 	Prepend an "_" to some unused variables.
> > 	Move unsafe_cast from io.m into rtti_implementation.m -- it is
> > 	useful in rtti_implementation (and possibly elsewhere), and it's
> > 	better to have io depend on rtti_implementation than vice-versa.
> > 
> > library/rtti_implementation.m:
> > 	Implement type_ctor_name_and_arity for commonly occuring data
> > 	representations.
> > 
> > 	Add type_ctor_is_variable_arity to simplfy this test.
> > 	Rename index as type_info_index to make it clear what we are
> > 	indexing into.
> > 
> > library/std_util.m:
> > 	Improve some of the error messages to make it easier to track
> > 	down unimplemented code.
> > 	Call into rtti_implementation for type_ctor_and_args.
> > 	Use pragma export to generate ML_call_rtti_compare_type_infos
> > 	(it wasn't available before so we jumped through a few more
> > 	hoops to call into rtti_implementation).
> > 
> > 
> > 
> > Index: library/io.m
> > ===================================================================
> > RCS file: /home/mercury1/repository/mercury/library/io.m,v
> > retrieving revision 1.231
> > diff -u -r1.231 io.m
> > --- library/io.m	12 Sep 2001 10:34:45 -0000	1.231
> > +++ library/io.m	20 Sep 2001 10:23:28 -0000
> > @@ -1120,6 +1120,7 @@
> >  :- import_module map, dir, term, term_io, varset, require, benchmarking, array.
> >  :- import_module bool, int, parser, exception.
> >  :- use_module table_builtin.
> > +:- use_module rtti_implementation.
> >  
> >  :- type io__state ---> io__state(c_pointer).
> >  	% Values of type `io__state' are never really used:
> > @@ -1594,7 +1595,7 @@
> >  		MR_PROC_LABEL, RetStr);
> >  }").
> >  
> > -:- pragma foreign_proc("MC++", ferror(Stream::in, RetVal::out, RetStr::out,
> > +:- pragma foreign_proc("MC++", ferror(_Stream::in, RetVal::out, _RetStr::out,
> >  		IO0::di, IO::uo),
> >  		[will_not_call_mercury, thread_safe],
> >  "{
> > @@ -2410,23 +2411,9 @@
> >  :- pred io__write_private_builtin_type_info(private_builtin__type_info(T)::in,
> >  		io__state::di, io__state::uo) is det.
> >  io__write_private_builtin_type_info(PrivateBuiltinTypeInfo) -->
> > -	{ TypeInfo = unsafe_cast(PrivateBuiltinTypeInfo) },
> > +	{ TypeInfo = rtti_implementation__unsafe_cast(PrivateBuiltinTypeInfo) },
> >  	io__write_type_desc(TypeInfo).
> >  
> > -:- func unsafe_cast(T1::in) = (T2::out) is det.
> > -:- pragma foreign_proc("C",
> > -	unsafe_cast(VarIn::in) = (VarOut::out),
> > -		[will_not_call_mercury, thread_safe],
> > -"
> > -	VarOut = VarIn;
> > -").
> > -:- pragma foreign_proc("C#",
> > -	unsafe_cast(VarIn::in) = (VarOut::out),
> > -		[will_not_call_mercury, thread_safe],
> > -"
> > -	VarOut = VarIn;
> > -").
> > -
> >  %-----------------------------------------------------------------------------%
> >  
> >  io__write_list([], _Separator, _OutputPred) --> [].
> > @@ -3621,7 +3608,7 @@
> >  }").
> >  
> >  :- pragma foreign_proc("MC++",
> > -	io__putback_byte(File::in, Character::in, IO0::di, IO::uo),
> > +	io__putback_byte(File::in, _Character::in, IO0::di, IO::uo),
> >  		may_call_mercury, "{
> >  
> >  	MR_MercuryFile mf = ML_DownCast(MR_MercuryFile, 
> > Index: library/rtti_implementation.m
> > ===================================================================
> > RCS file: /home/mercury1/repository/mercury/library/rtti_implementation.m,v
> > retrieving revision 1.4
> > diff -u -r1.4 rtti_implementation.m
> > --- library/rtti_implementation.m	22 Aug 2001 12:29:01 -0000	1.4
> > +++ library/rtti_implementation.m	20 Sep 2001 10:23:29 -0000
> > @@ -28,6 +28,10 @@
> >  
> >  :- interface.
> >  
> > +:- import_module list.
> > +
> > +:- use_module std_util.
> > +
> >  	% Our type_info and type_ctor_info implementations are both
> >  	% abstract types.
> >  :- type type_info.
> > @@ -42,12 +46,26 @@
> >  :- pred compare_type_infos(comparison_result::out,
> >  		type_info::in, type_info::in) is det.
> >  
> > +:- pred type_ctor_and_args(type_info::in,
> > +		type_ctor_info::out,
> > +		list(type_info)::out) is det.
> > +
> > +:- pred type_ctor_name_and_arity(type_ctor_info::in,
> > +		string::out, string::out, int::out) is det.
> > +
> > +:- pred deconstruct(T::in, string::out, int::out,
> > +		list(std_util__univ)::out) is det.
> 
> The log message should mention all of these additions.
> 
> > +
> > +	% This is useful in a few places, so we'd like to share the code, but
> > +	% it's better to put it into an implementation module such as this one.
> > +:- func unsafe_cast(T1::in) = (T2::out) is det.
> > +
> >  %-----------------------------------------------------------------------------%
> >  %-----------------------------------------------------------------------------%
> >  
> >  :- implementation.
> >  
> > -:- import_module require, string.
> > +:- import_module require, string, int.
> >  
> >  	% std_util has a lot of types and functions with the same names,
> >  	% so we prefer to keep the namespace separate.
> > @@ -80,7 +98,7 @@
> >  	;	array
> >  	;	succip
> >  	;	hp
> > -	;	currfr
> > +	;	curfr
> 
> The log message doesn't mention this change.
> 

> > +:- func get_type(type_info, P, T, du_functor_descriptor) = type_info.
> > +
> > +get_type(TypeInfoParams, PseudoTypeInfo, Term, FunctorDesc) = (ArgTypeInfo) :-
> > +	( 
> > +		typeinfo_is_variable(PseudoTypeInfo, VarNum)
> > +	->
> > +		ExpandedTypeInfo = get_type_info_for_var(TypeInfoParams,
> > +			VarNum, Term, FunctorDesc),
> > +		( typeinfo_is_variable(ExpandedTypeInfo, _) ->
> > +			error("unbound type variable")
> 
> It would be better if this error string mentioned the function.

Fixed.

> > +		iterate_foldl(StartRegionSize, UpperBound,
> > +			(pred(I::in, TI0::in, TI::out) is det :-
> > +
> > +				PTI = get_pti_from_type_info(CastTypeInfo, I),
> > +				ETypeInfo = get_type(
> > +					TypeInfoParams, PTI, Term, FunctorDesc),
> > +						% this comparison is not
> > +						% right...???
> 
> I'm not sure what this comment means.  Is this supposed to be an XXX?
> 

I've removed this in a subsequent change -- it's a leftover comment from
when I was debugging this code, the code is OK now, the comment just
needs to be removed.

> > +
> > +	% XXX this is completely unimplemented.
> > +:- func pseudotypeinfo_get_higher_order_arity(type_info) = int.
> > +pseudotypeinfo_get_higher_order_arity(_) = 1 :-
> > +	det_unimplemented("pseudotypeinfo_get_higher_order_arity").
> > +
> 
> Why not give the "unimplemented" predicate a determinism of erroneous?
> That way you wouldn't have to supply a dummy return value.  (I realise
> it wasn't added as part of this change, but I thought I'd mention it
> here anyway.)

Then all the callers are inferred as having an erroneous determinism, and the
consequent warning messages halt compilation of the library.
Fixing it requires providing code path that is det, which always
requires a dummy value.  

If someone knows a fancy trick to get around this, I'd love to hear it,
as I find the dummy values annoying.

> > +
> > +	% Make a new type-info with the given arity, using the given type_info
> > +	% as the basis.
> > +
> > +:- func new_type_info(type_info, int) = type_info.
> > +new_type_info(TypeInfo::in, _::in) = (TypeInfo::out) :- 
> > +	det_unimplemented("new_type_info").
> > +
> > +:- pragma foreign_proc("C#",
> > +	new_type_info(OldTypeInfo::in, Arity::in) = (NewTypeInfo::out), [], "
> > +	NewTypeInfo = new object[Arity + 1];
> > +	System.Array.Copy(OldTypeInfo, NewTypeInfo, OldTypeInfo.Length);
> > +").
> > +
> > +
> > +	% Get the pseudo-typeinfo at the given index from the argument types.
> > +	
> > +:- some [T] func get_pti_from_arg_types(arg_types, int) = T.
> > +
> > +get_pti_from_arg_types(_::in, _::in) = (42::out) :-
> > +	det_unimplemented("get_pti_from_arg_types").
> > +
> > +:- pragma foreign_proc("C#",
> > +	get_pti_from_arg_types(ArgTypes::in, Index::in) =
> > +		(ArgTypeInfo::out), [], "
> > +	ArgTypeInfo = ArgTypes[Index];
> > +").
> > +
> > +
> > +	% Get the pseudo-typeinfo at the given index from a type-info.
> > +
> > +:- some [T] func get_pti_from_type_info(type_info, int) = T.
> > +
> > +get_pti_from_type_info(_::in, _::in) = (42::out) :-
> > +	det_unimplemented("get_pti_from_type_info").
> > +
> > +:- pragma foreign_proc("C#",
> > +	get_pti_from_type_info(TypeInfo::in, Index::in) = (PTI::out), [], "
> > +	PTI = TypeInfo[Index];
> > +").
> > +
> > +
> > +
> > +	% Get the type info for a particular type variable number
> > +	% (it might be in the type_info or in the term itself).
> > +	%
> > +	% XXX existentially quantified vars are not yet handled.
> > +	
> > +:- func get_type_info_for_var(
> > +		type_info, int, T, du_functor_descriptor) = type_info.
> > +


> > +	% An unchecked cast to type_info (for pseudo-typeinfos).
> > +
> > +:- func type_info_cast(T) = type_info.
> > +
> > +type_info_cast(X::in) = (unsafe_cast(X)::out) :-
> > +	det_unimplemented("type_info_cast").
> 
> It looks implemented to me.  What else needs to be done other than call
> unsafe_cast?
> 
> > +
> > +:- pragma foreign_proc("C#",
> > +	type_info_cast(PseudoTypeInfo::in) = (TypeInfo::out), [], "
> > +
> > +	TypeInfo = (object[]) PseudoTypeInfo;
> > +").
> 
> If you can use the previous clause, then this clause won't be needed at all.
> The C# code in the implementation of unsafe_cast will be used instead.

You are of course completely right.  I've fixed this.

> > +	% Test whether a type info is variable.
> > +
> > +:- pred typeinfo_is_variable(T::in, int::out) is semidet.
> > +
> > +typeinfo_is_variable(_::in, 42::out) :-
> > +	std_util__semidet_succeed,
> > +	det_unimplemented("typeinfo_is_variable").
> 
> That would be like calling semidet_unimplemented.  ;-)

Another fine catch.

> > +update_type_info_index(_::in, _::in, _::in) :- 
> > +	det_unimplemented("type_info_index").
> > +
> > +:- pred update_type_info_index(int::in, type_info::in, type_info::in) is det.
> > +:- pragma foreign_proc("C#",
> > +	update_type_info_index(X::in, OldTypeInfo::in, NewValue::in), [], "
> > +	OldTypeInfo[X] = NewValue;
> > +").
> > +
> 
> This implementation performs destructive update on the type info, so you
> should thread it through the predicate using unique modes to ensure that
> the compiler can't optimize away calls to the predicate, and to ensure that
> no attempts are made to use the previous value.

Yep, I was being lazy here and I have fixed it now.

I've fixed all these things, and I will include them with the
diff to add support for existentials.

Although I have already committed I will improve the log message too
(sneaky CVS admin commands to the rescue).

Tyson.
--------------------------------------------------------------------------
mercury-reviews mailing list
post:  mercury-reviews at cs.mu.oz.au
administrative address: owner-mercury-reviews at cs.mu.oz.au
unsubscribe: Address: mercury-reviews-request at cs.mu.oz.au Message: unsubscribe
subscribe:   Address: mercury-reviews-request at cs.mu.oz.au Message: subscribe
--------------------------------------------------------------------------



More information about the reviews mailing list