duplicate function names are not detected

[Willi Braun]

When a function name is accidentally used twice in a module one might get cryptic messages: Either a not really helping
... Failed to elaborate expression ...
or a in some cases a more helpful C compiling error:
redefinition of ... with a different type ...
Which one depends one usage of the duplicate function.

[Martin Sjölund]

We used to have detection of duplicate classes, but it seems we no longer do. Try:

model M
end M;

model M
end M;

[Willi Braun]

That issue might lead to really annoying behaviour and
not really helping error messages.
Case1:

loadString("
model M
  Real y = sin(time);
  Real x = cos(time);
end M;
 // some pages down [...]
model M
  Real x = sin(time)*y;
  Real z=2;
end M;
");

One might wonder why model M does not have variable z.

Case2:

loadString("
package p
 model xy
   Real x = sin(time)*der(x);
   Real y = sin(time);
 end xy;

 // some pages down [...]

 model xy
   Real x = cos(time)*der(x);
   Real y = cos(time);
   //Real z = x*y;
 end xy;
end p;
");

In this second case we throw a quite cryptic error message

Error: Internal error Env.avlTreeReplace failed
Error: Error occurred while flattening model p.A

which occurs only if you activate the line with
variable z and it's quite hard to find out that
the mistake was done before and not by this line.

[Adrian Pop]

I remember we did this check right after parsing via SCodeUtil. I'll ask Per to have a look.

[Per Östlund]

The check should only be done for classes that are used, i.e. when building a class scope. There's already code for that in FNode.avlTreeAdd2, but it's commented out. But enabling the code causes a lot of false positives, it seems the same elements are added multiple times in some cases. We probably just need to use e.g. avlTreeReplace instead in some places. I will investigate further next week.

[Francesco Casella]

Another test case reported in #4211, which is closed as duplicate.

I think this should really be fixed for 2.0.0. We cannot afford to have unpredictable or cryptic beaviour of the compiler when there are duplicate class definitions in the code. This case should be explicitly reported as an error.

[Per Östlund]

Fixed in r860bb91 and 669113d. The check for duplicate elements is disabled for MetaModelica though, because otherwise the bootstrapping fails. This is caused by actual duplicate elements, see ​Hudson log. Maybe Martin wants to take a look at that.

[Martin Sjölund]

For bootstrapping, it seems to be the interface files when dumped since they use SCode. Any type variables are duplicated since Absyn to SCode fixes some classes...

encapsulated uniontype DoubleEndedList<T>  

to:

encapsulated uniontype DoubleEndedList<T>  
  final type T = polymorphic<Any>;

[Francesco Casella]

I tried loading this package:

package p
 model xy
   Real x = sin(time)*der(x);
   Real y = sin(time);
 end xy;

 // some pages down [...]

 model xy
   Real x = cos(time)*der(x);
   Real y = cos(time);
   //Real z = x*y;
 end xy;
end p;

in OMEdit. The package is loaded without complaining, which is probably not good. When I expand the package, two classes with the same name are shown, which is odd. However, the following error message is displayed:

[1] 10:24:44 Translation Notification
[p: 2:2-5:8]: From here:

[2] 10:24:44 Translation Error
[p: 9:2-13:8]: An element with name xy is already declared in this scope.

However, it's really not clear which of the two xy classes will be used.

If I load package p with loadString in OMShell, I get no error messages whatsoever.

I think this should be improved, let's wait to see what happens with Adrian's new API. It would be best if we could fix this for 1.14.0

[Francesco Casella]

@perost what is the status with this issue?

[Per Östlund]

Replying to casella:

@perost what is the status with this issue?

The status is that I don't know why this is still assigned to me, I fixed the check in the OF earlier while the NF didn't need any fixing. So neither frontends will instantiate p.xy due to the duplicated elements. The remaining issues seems to be OMEdit and/or API related.

[Francesco Casella]

OK, thanks!

[Adeel Asghar]

I think its more of an API issue then OMEdit.

[Francesco Casella]

Releasing 1.14.0 which is stable and has many improvements w.r.t. 1.13.2.

This issue, previously marked as blocker for 1.14.0, is rescheduled to 1.15.0

[Francesco Casella]

Release 1.15.0 was scrapped, because replaceable support eventually turned out to be more easily implemented in 1.16.0. Hence, all 1.15.0 tickets are rescheduled to 1.16.0

[Francesco Casella]

Retargeted to 1.17.0 after 1.16.0 release

[Francesco Casella]

Rescheduled to 1.18.0

[Francesco Casella]

Ticket retargeted after milestone closed

[Francesco Casella]

1.18.0 blocker tickets moved to 1.19.0