Opened 9 years ago
Last modified 3 years ago
#3535 assigned defect
duplicate function names are not detected
Reported by: | Willi Braun | Owned by: | Adrian Pop |
---|---|---|---|
Priority: | blocker | Milestone: | 1.19.0 |
Component: | Interactive Environment | Version: | |
Keywords: | Cc: | Adrian Pop, francois.beaude@… |
Description
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.
Change History (20)
comment:1 by , 9 years ago
Component: | MetaModelica → Frontend |
---|---|
Owner: | changed from | to
comment:2 by , 9 years ago
Cc: | added |
---|
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.
comment:3 by , 9 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
I remember we did this check right after parsing via SCodeUtil. I'll ask Per to have a look.
comment:4 by , 9 years ago
Status: | assigned → accepted |
---|
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.
comment:5 by , 8 years ago
Cc: | added |
---|---|
Milestone: | Future → 2.0.0 |
Priority: | normal → blocker |
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.
comment:6 by , 8 years ago
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.
comment:7 by , 8 years ago
Priority: | blocker → high |
---|
comment:8 by , 8 years ago
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>;
comment:9 by , 6 years ago
Component: | Frontend → New Instantiation |
---|
comment:10 by , 6 years ago
Milestone: | 2.0.0 → 1.14.0 |
---|---|
Priority: | high → blocker |
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
comment:12 by , 5 years ago
Component: | New Instantiation → OMEdit |
---|---|
Owner: | changed from | to
Status: | accepted → assigned |
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.
comment:14 by , 5 years ago
Component: | OMEdit → Interactive Environment |
---|---|
Owner: | changed from | to
I think its more of an API issue then OMEdit.
comment:15 by , 5 years ago
Milestone: | 1.14.0 → 1.15.0 |
---|
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
comment:16 by , 5 years ago
Milestone: | 1.15.0 → 1.16.0 |
---|
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
We used to have detection of duplicate classes, but it seems we no longer do. Try: