[m-rev.] for review: pack args next to remote sectags

Peter Wang novalazy at gmail.com
Tue Aug 28 17:10:29 AEST 2018


On Tue, 28 Aug 2018 16:49:15 +1000 (AEST), Julien Fischer <jfischer at opturion.com> wrote:
> 
> Hi Zoltan,
> 
> On Sun, 26 Aug 2018, Zoltan Somogyi wrote:
> 
> > For review by Peter or Julien. The diff is with -b due to systematic
> > changes in indentation levels in some files.
> >
> > The part of this diff that required the most work was the changes
> > to the RTTI system. I tried to add test cases for each part of the system
> > that was affected by this, from library/construct.m, library/deconstructs.m,
> > deep copying and tabling, but there is one affected module for which
> > I don't feel comfortable writing tests: library/store.m. If someone
> > could add a test case checking the proper operation of store.m
> > on terms that use the new data representation, I would appreciate it.
> 
> I'll have a go at that.   Based on the XXXs within the store module,
> it's probably broken and will be likely more broken after this change.
> (Does anyone actually use the generic_ref stuff?)

Never, it always seemed like a hack to me.

> 
> Did you intend to enable these by default now?  (If so, that should definitely
> be mentioned in the log message.)
> 
> The rest of the diff looks ok -- unless Peter has any additional comments
> I suggest you go ahead and commit.

I only paged through to see how big the diff was. MR_arg_value_uncommon
looks unused now.

(If we want to promote a ROTD soon, it might be prudent to pick one
created prior to this patch going in.)

Peter


More information about the reviews mailing list