[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