modelica / ModelicaSpecification

Specification of the Modelica Language
https://specification.modelica.org
Creative Commons Attribution Share Alike 4.0 International
104 stars 40 forks source link

Should a conflict between an import and a declaration cause an error? #1573

Closed modelica-trac-importer closed 6 years ago

modelica-trac-importer commented 6 years ago

Reported by stefanv on 24 Sep 2014 13:33 UTC Consider a package like this:

package MyLibrary
    import Modelica.Math; // or import Math = Modelica.Math
    ...
    package Math
        ...
    end Math;
    ...
    model M
        ...
        y = Math.SomeFunction(x);
    end M;
end MyLibrary;

Section 5.3.1 of the specification says that when looking up Math.SomeFunction, we first look up Math, and that we first look for it among the declared elements of M, then among the qualified import statements in the lexical scope, and then we do the same thing in the surrounding scope.

It's not clear to me whether this will find Modelica.Math, or MyLibrary.Math, or whether the conflict between Modelica.Math and MyLibrary.Math should be considered an error.

Ignoring the latter possibility, the wording suggests that the import statement in MyLibraryis considered while looking for Math in M, since that import statement is in scope. But I'm not sure that that is what was intended.

I think this needs clarification.


Migrated-From: https://trac.modelica.org/Modelica/ticket/1573

modelica-trac-importer commented 6 years ago

Comment by hansolsson on 25 Sep 2014 06:27 UTC To me it seems obvious that we proceed in three steps (given as three bullets): declared named elements, qualified import, and unqualified import - and the first step that gives a match is the result.

However, we should clarify this latter point, since it's one of those implicit assumption that may seem obvious, but still should be explicit in a specification.

This means that import Modelica.Math cannot be used in the example; which means that the import itself is useless. I don't see that we need anything special for that in the specifiation; it's more a style-check issue than a formal error.

modelica-trac-importer commented 6 years ago

Comment by jmattsson on 24 Nov 2014 15:22 UTC Replying to [comment:1 hansolsson]:

it's more a style-check issue than a formal error.

I disagree - allowing an import with the same name as a local class will make it confusing for the user to understand what an access refers to. Such things can easily lead to hours of debugging fun. That combined with that it is not possible to use such an import for anything suggests to me that it should be forbidden.

modelica-trac-importer commented 6 years ago

Comment by choeger on 25 Nov 2014 00:55 UTC Replying to [comment:2 jmattsson]:

Replying to [comment:1 hansolsson]:

it's more a style-check issue than a formal error. I disagree - allowing an import with the same name as a local class will make it confusing for the user to understand what an access refers to. Such things can easily lead to hours of debugging fun. That combined with that it is not possible to use such an import for anything suggests to me that it should be forbidden.

I agree with Hans. Just because something is confusing, we should not forbid it. Some people might find recursion confusing. Others differential equations ;). In fact, IMO we should only forbid things that (might) cause runtime errors.

modelica-trac-importer commented 6 years ago

Comment by hansolsson on 1 Dec 2014 17:11 UTC Language group: Agreement that bad style and should generate diagnostics. Discussion whether it should stop translation or not.

The current priorities are:

Proposal: It is an error if a qualified import-statement has the same name as a local or inherited element.

Poll:

  1. Must give an error

  2. Should give a warning

  3. Don't change specification for this (a tool may still give a warning)

  4. 4

  5. 8

  6. 4

Also change: This lookup in each scope is performed as follows -> This lookup in each scope is performed in the following order, only proceeding to the next step if nothing was found in the current step:

(Note that import-statements must refer to existing classes or constants.)

modelica-trac-importer commented 6 years ago

Modified by hansolsson on 10 Dec 2014 15:40 UTC

modelica-trac-importer commented 6 years ago

Comment by jmattsson on 10 Dec 2014 15:46 UTC Decision was made at design meeting. Now need to update spec.

modelica-trac-importer commented 6 years ago

Comment by hansolsson on 16 Jul 2015 16:15 UTC Resolved in r8350

Also clarified that the same applies to multiple import (the new "import Modelica.Math.{sin,cos};"; and added a note stating that non-qualified import shall not generate warnings.

modelica-trac-importer commented 6 years ago

Modified by dietmarw on 17 Jul 2015 06:24 UTC