Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#6428 closed defect (fixed)

[MM] Add Dangerous.listAppendDestroy to the MetaModelica

Reported by: Adrian Pop Owned by: Martin Sjölund
Priority: high Milestone: 1.17.1
Component: MetaModelica Version: 1.16.2
Keywords: Cc: Karim Adbdelhak

Description


Change History (12)

comment:1 by Adrian Pop, 4 years ago

Cc: Karim Adbdelhak added

PR: https://github.com/OpenModelica/OpenModelica/pull/7303

Note that if you do:

Dangerous.listAppendDestroy(constantList, anyList) you will get a segfault as you cannot write the end of a constant list to point to the second list.

comment:2 by Adrian Pop, 4 years ago

Resolution: fixed
Status: newclosed

comment:3 by Karim Adbdelhak, 4 years ago

I basically have something like the following example (extremely simplified). Will this work? And if not, is there a way to make it work? The lists contain ComponentRef in my current implementation but that could also be smth else.

function test
  input ComponentRef i1;
  input ComponentRef i2;
  input UnorderedMap<Integer, list<...>> map;
protected
  list<...> list1, list2;
algorithm
  // if both are in the map merge them
  if UnorderedMap.contains(i1, map) and UnorderedMap.contains(i2, map) then
    list1 := UnorderedMap.get(i1, map);
    list2 := UnorderedMap.get(i2, map);
    Dangerous.listAppendDestroy(list1,list2);
    UnorderedMap.add(i1, list2);
    UnorderedMap.add(i2, list2); // is this needed?
  else
    // smth else
  end if;

end test;
Last edited 4 years ago by Karim Adbdelhak (previous) (diff)

comment:4 by Adrian Pop, 4 years ago

it should be: list1 := Dangerous.listAppendDestroy(list1,list2);
as when list1 is empty I return list2.

comment:5 by Adrian Pop, 4 years ago

It depends on how the lists in the UnorderedMap are created, if they are created via cons then is fine.

comment:6 by Karim Adbdelhak, 4 years ago

They cannot be empty, they will always have at least one entry. And they are created step by step either by appending one element at a time or merged like we see here.

in reply to:  4 comment:7 by Karim Adbdelhak, 4 years ago

Replying to adrpo:

it should be: list1 := Dangerous.listAppendDestroy(list1,list2);
as when list1 is empty I return list2.

Good to know! So list1 will be the list i have to use and whatever i put in i cannot use anymore right?

comment:8 by Adrian Pop, 4 years ago

list1 will be destroyed if not empty, if is empty then list2 will be returned and list1 stays empty.

comment:9 by Adrian Pop, 4 years ago

This is the C code for it, maybe is easier to understand how it works:

modelica_metatype listAppendDestroy(modelica_metatype lstFirstDestroyed, modelica_metatype listSecondKept)
{
  modelica_metatype lst = lstFirstDestroyed;
  if (MMC_NILTEST(lstFirstDestroyed)) {
    return listSecondKept;
  }
  while (!MMC_NILTEST(lst))
  {
    /* reached the end, set the element */
    if (MMC_NILTEST(MMC_CDR(lst)))
    {
      MMC_CDR(lst) = listSecondKept;
      break;
    }
    lst = MMC_CDR(lst);
  }
  return lstFirstDestroyed;
}

comment:10 by Philip Hannebohm, 4 years ago

Since we are talking about performance here, does this save one if-statement?

modelica_metatype listAppendDestroy(modelica_metatype lstFirstDestroyed, modelica_metatype listSecondKept)
{
  modelica_metatype lst = lstFirstDestroyed;
  if (MMC_NILTEST(lstFirstDestroyed)) {
    return listSecondKept;
  }
  while (!MMC_NILTEST(MMC_CDR(lst))) {
    lst = MMC_CDR(lst);
  }
  /* reached the end, set the element */
  MMC_CDR(lst) = listSecondKept;
  return lstFirstDestroyed;
}

comment:11 by Adrian Pop, 4 years ago

Sure, make a PR with it.

comment:12 by Karim Adbdelhak, 4 years ago

Seems to work perfectly fine, thanks adrian!

Note: See TracTickets for help on using tickets.