[PATCH] move implementation from IndexedList to List; cleanup

25 views
Skip to first unread message

oldk1331

unread,
Dec 18, 2018, 5:28:29 AM12/18/18
to fricas...@googlegroups.com
I tried to make this change over a year ago, now I'll try again.

The attached 2 patches tries to simplify and cleanup domain
IndexedList and List.

First patch, it moves almost all functions from IndexedList to
List.  Because these functions are not related with index at all.

Second patch, it removes many usage of Lisp function call by
SPAD function call.  This will not harm performance, because
in domain List we can do inline optimization (we can't inline
in IndexedList).  There is also a benefit that these function
call will be type checked.
02-reduce-lisp-call-by-inline.patch
01-move-indexedlist-to-list.patch

oldk1331

unread,
Apr 24, 2019, 8:13:03 AM4/24/19
to fricas...@googlegroups.com
Hi all, can we review these two patches please?

The original thread and attachments are in:

https://groups.google.com/forum/#!msg/fricas-devel/XtVQA0xxES4/g_8MbobFBgAJ

Ralf Hemmecke

unread,
Apr 24, 2019, 9:57:45 AM4/24/19
to fricas...@googlegroups.com
Hi Qian,

Patch 1 basically moves all of the function definitions from
IndexedList to List (except, minIndex and maxIndex). Maybe maxIndex can
even be avoided, because it is inherited from the default implementation
of LinearAggregate.

https://github.com/hemmecke/fricas/blob/master-hemmecke/src/algebra/aggcat.spad#L1831

I first was a bit sceptical about such a move, because IndexedList
seemed more general to me than List, but, in fact, this patch removes an
interdependency between List and IndexedList (List is currently used in
IndexedList).

I first thought that this patch would be problematic, but seemingly all
index related functions like elt etc. are implemented by category
defaults, so Qian's suggestion works.

Patch 2
In general I like the removal of unnecessary macros and their
replacement by actual function calls. The Lisp reference is still
concentrated in one place (at the beginning).
Maybe, I would reorder the function definition so that all references to
Lisp and all pretend's come first, i.e. move the elt and setelt!
functions further down.

Ralf

Waldek Hebisch

unread,
Apr 27, 2019, 10:07:52 AM4/27/19
to fricas...@googlegroups.com
oldk1331 wrote:
>
> --0000000000007ac98c0587459fbb
> Content-Type: text/plain; charset="UTF-8"
>
> Hi all, can we review these two patches please?
>
> The original thread and attachments are in:
>
> https://groups.google.com/forum/#!msg/fricas-devel/XtVQA0xxES4/g_8MbobFBgAJ

OK for both. Sorry for long delay.

--
Waldek Hebisch
Reply all
Reply to author
Forward
0 new messages