Better ImmutableList #3816
Conversation
|
There is one stupid error in |
# Conflicts: # key.core/src/main/java/de/uka/ilkd/key/proof/init/InitConfig.java # key.core/src/main/java/de/uka/ilkd/key/taclettranslation/lemma/TacletLoader.java
Drodt
left a comment
There was a problem hiding this comment.
Nice work! Do you have any idea why tests are failing?
| */ | ||
| protected void fireProofGoalsAdded(Goal goal) { | ||
| fireProofGoalsAdded(ImmutableSLList.<Goal>nil().prepend(goal)); | ||
| fireProofGoalsAdded(ImmutableList.<Goal>nil().prepend(goal)); |
There was a problem hiding this comment.
Probably singleton is better here
There was a problem hiding this comment.
I know. On the other side, I wanted to limit my impact. More changes => more errors => more debugging.
Let us wait, until it is green again.
| : "p_toMatch and assumes sequent must have same number of elements"; | ||
| newMC = matchAssumes( | ||
| ImmutableSLList.<AssumesFormulaInstantiation>nil().prepend(candidateInst), | ||
| ImmutableList.<AssumesFormulaInstantiation>nil().prepend(candidateInst), |
| .createProgramSV(new ProgramElementName("#res_sv"), ProgramSVSort.VARIABLE, false); | ||
|
|
||
| final ImmutableList<KeYJavaType> sig = ImmutableSLList.<KeYJavaType>nil() | ||
| final ImmutableList<KeYJavaType> sig = ImmutableList.<KeYJavaType>nil() |
| tb.apply(update, tb.prog(modality.kind(), newJavaBlock, target.sub(0)))), | ||
| ruleApp.pio); | ||
| return ImmutableSLList.<Goal>nil().append(goal); | ||
| return ImmutableList.<Goal>nil().append(goal); |
There was a problem hiding this comment.
| return ImmutableList.<Goal>nil().append(goal); | |
| return ImmutableList.<Goal>singleton(goal); |
| ifInsts.toArray(new PosInOccurrence[0]); | ||
| ImmutableList<PosInOccurrence> immutableIfInsts = | ||
| ImmutableSLList.<PosInOccurrence>nil().append(ifInstsArr); | ||
| ImmutableList.<PosInOccurrence>nil().append(ifInstsArr); |
There was a problem hiding this comment.
| ImmutableList.<PosInOccurrence>nil().append(ifInstsArr); | |
| ImmutableList.<PosInOccurrence>singleton(ifInstsArr); |
|
|
||
| protected SubIterator(Term t, MutableState mState, LogicServices services) { | ||
| termStack = ImmutableSLList.<Term>nil().prepend(t); | ||
| termStack = ImmutableList.<Term>nil().prepend(t); |
There was a problem hiding this comment.
| termStack = ImmutableList.<Term>nil().prepend(t); | |
| termStack = ImmutableList.<Term>singleton(t); |
| // No functionality is allowed in this method body! | ||
| mediator.getUI().getProofControl().startAutoMode(r.getSelectedProof(), | ||
| ImmutableSLList.<Goal>nil().prepend(invokedGoal)); | ||
| ImmutableList.<Goal>nil().prepend(invokedGoal)); |
There was a problem hiding this comment.
| ImmutableList.<Goal>nil().prepend(invokedGoal)); | |
| ImmutableList.<Goal>singleton(invokedGoal)); |
| PosInOccurrence pos) { | ||
| if (pos == null) { | ||
| return ImmutableSLList.<Integer>nil().prepend(0); | ||
| return ImmutableList.<Integer>nil().prepend(0); |
There was a problem hiding this comment.
| return ImmutableList.<Integer>nil().prepend(0); | |
| return ImmutableList.<Integer>singleton(0); |
| */ | ||
| private DefaultImmutableSet(T element) { | ||
| elementList = (ImmutableSLList.<T>nil()).prepend(element); | ||
| elementList = (ImmutableList.<T>nil()).prepend(element); |
There was a problem hiding this comment.
| elementList = (ImmutableList.<T>nil()).prepend(element); | |
| elementList = ImmutableList.<T>singleton(element); |
| */ | ||
| ImmutableList<T> append(@SuppressWarnings("unchecked") T... array); | ||
| default ImmutableList<T> append(ImmutableList<T> list) { | ||
| return new ImmutableListConcat<>(this, ImmutableList.fromList(list)); |
There was a problem hiding this comment.
| return new ImmutableListConcat<>(this, ImmutableList.fromList(list)); | |
| return new ImmutableListConcat<>(this, list); |
Would this not also work?
Drodt
left a comment
There was a problem hiding this comment.
Nice work! Do you have any idea why tests are failing?
| public ImmutableList<T> take(int n) { | ||
| List<T> seq = new ArrayList<>(Math.min(size(), n)); | ||
| ImmutableList<T> list = this; | ||
| for (int i = 0; i < seq.size(); i++) { |
There was a problem hiding this comment.
seq.size() is 0 here so the loop is never executed
|
|
||
| @Override | ||
| public ImmutableList<T> removeAll(T obj) { | ||
| return stream().filter(it -> Objects.equals(obj, it)).collect(ImmutableList.collector()); |
| @Override | ||
| public int hashCode() { | ||
| return Objects.hash(list, start, end); | ||
| } |
There was a problem hiding this comment.
here and in ListConcat and ListReverse the hascode is structural, while in Cons and ListList element-wise
equals is always element-wise
Issue
We currently have two implementations of immutable sequences with
ImmutableListandImmutableArray. This requires conversions sometimes, but mainly it leads to confusion about which implementation should be taken.Intended Change
This PR unifies everything under
ImmutableList.There are two fresh children classes:
ImmutableListArrayandImmutableListList. Both behave likeImmutableList, but use different data storage (T[]orList<T>) in the background.Some operations from
ImmutableListare quite expensive on this data storages, e.g.,tail()orreverse(). Therefore, I introduced views that emulate these operations, without copying the data, i.e.,ImmutableListConcatasImmutableList#prepend(other)ImmutableListReverseforImmutableList#reverse()ImmutableListSubListforImmutableList#take(n),ImmutableList#skip(n),ImmutableList#tail()These operations are now in$O(1)$ .
The class
ImmutableArrayshould be considered obsolete.Developers should use only the methods and functions of
ImmutableList. Implementation may override them if there is a faster implementation, e.g,tail()onImmutableSList.Plan
ImmutableSListtoImmutableList.Type of pull request
Refactoring (behaviour should not change or only minimally change)
New feature (non-breaking change which adds functionality)
There are changes to the (Java) code
Ensuring quality