TC: name confusion #493

PaulKlint · 2014-02-11T09:36:45Z

Compiling demo::lang::Exp::Combined::Manual::Load:

module demo::lang::Exp::Combined::Manual::Load

import demo::lang::Exp::Concrete::WithLayout::Syntax;  /*1*/
import demo::lang::Exp::Abstract::Syntax;              /*2*/
import demo::lang::Exp::Combined::Manual::Parse;       /*3*/
import String;

public Exp load(str txt) = load(parse(txt));           /*4*/

public Exp load((Exp)`<IntegerLiteral l>`)             /*5*/
       = con(toInt("<l>"));       
public Exp load((Exp)`<Exp e1> * <Exp e2>`) 
       = mul(load(e1), load(e2));  
public Exp load((Exp)`<Exp e1> + <Exp e2>`)
       = add(load(e1), load(e2)); 
public Exp load((Exp)`( <Exp e> )`) 
       = load(e);

leads to a lot of name confusion:

error("Multiple functions found which could be applied",|rascal:///demo/lang/Exp/Combined/Manual/Load.rsc|(616,7,<17,9>,<17,16>))
error("Syntax type Exp is not defined",|rascal:///demo/lang/Exp/Combined/Manual/Load.rsc|(508,3,<14,17>,<14,20>))
error("Syntax type Exp is not defined",|rascal:///demo/lang/Exp/Combined/Manual/Load.rsc|(444,3,<12,34>,<12,37>))
error("Multiple functions found which could be applied",|rascal:///demo/lang/Exp/Combined/Manual/Load.rsc|(468,8,<13,13>,<13,21>))
error("Multiple functions found which could be applied",|rascal:///demo/lang/Exp/Combined/Manual/Load.rsc|(548,8,<15,13>,<15,21>))
error("Syntax type Exp is not defined",|rascal:///demo/lang/Exp/Combined/Manual/Load.rsc|(525,3,<14,34>,<14,37>))
error("Syntax type Exp is not defined",|rascal:///demo/lang/Exp/Combined/Manual/Load.rsc|(433,3,<12,23>,<12,26>))
error("Type Exp not declared",|project://rascal/src/org/rascalmpl/library/demo/lang/Exp/Combined/Manual/Parse.rsc|(128,3,<5,7>,<5,10>))
error("Syntax type Exp is not defined",|rascal:///demo/lang/Exp/Combined/Manual/Load.rsc|(514,3,<14,23>,<14,26>))
error("Multiple functions found which could be applied",|rascal:///demo/lang/Exp/Combined/Manual/Load.rsc|(478,8,<13,23>,<13,31>))
error("Multiple functions found which could be applied",|rascal:///demo/lang/Exp/Combined/Manual/Load.rsc|(558,8,<15,23>,<15,31>))
error("Syntax type Exp is not defined",|rascal:///demo/lang/Exp/Combined/Manual/Load.rsc|(331,3,<10,17>,<10,20>))
error("Syntax type Exp is not defined",|rascal:///demo/lang/Exp/Combined/Manual/Load.rsc|(427,3,<12,17>,<12,20>))
error("Unexpected type: type of body expression, Exp, must be a subtype of the function return type, fail",|rascal:///demo/lang/Exp/Combined/Manual/Load.rsc|(384,17,<11,9>,<11,26>))
error("Syntax type Exp is not defined",|rascal:///demo/lang/Exp/Combined/Manual/Load.rsc|(587,3,<16,17>,<16,20>))
error("Syntax type Exp is not defined",|rascal:///demo/lang/Exp/Combined/Manual/Load.rsc|(595,3,<16,25>,<16,28>))

The text was updated successfully, but these errors were encountered:

mahills · 2014-02-11T20:07:37Z

I think the problem here is that there is also an abstract type named Exp. When that is introduced, it knocks the concrete type out of scope (along with itself, it's imported as well). You would normally then be able to access it using a qualified name, like demo::lang::Exp::Concrete::WithLayout::Syntax::Exp, but you cannot use qualified names in the parens before the concrete pattern.

PaulKlint · 2014-02-11T21:57:35Z

@mahills you are completely right. In fact all problems in the Exp example disappear by separating the concrete sort (Exp) and the abstract sort (AExp) but then we can no longer use implode. The troubling thing is that the Exp example is one of the first examples you want to show to a new user, but the assessment is (unfortunately):

The Abstract and Concrete versions are as good as they can be: concise and understandable.
The Combined versions (both Manual and Automatic) are too hard to understand due to the naming issues with the non-terminal Exp.

The current dilemma is:

We want to get rid of this problem (but how?)
We want to get the compiler working for all the examples we currently have. Unfortunately, none of the examples that use implode can work due to these naming issues.

We have written the type checker and the compiler mostly relying on concrete patterns, but we will occasionally want a bridge between concrete and abstract syntax.

Has anyone ideas for a solution?

mahills · 2014-02-11T22:07:09Z

@PaulKlint , at least for this it would work to be able to give a qualified name in the concrete pattern, but in general this is just painful because the types are separate. It probably works in the interpreter because the interpreter just does bizarre stuff with these scenarios, it is probably merging the two types and then picking the right one out when it needs to (but, on the other hand, potentially turning things to value in other places).

One solution may be to say that these are both the same type, and just keep track of whether a type can have productions, constructors, or both. I'm not sure what other problems that would cause, though.

PaulKlint · 2014-02-11T22:42:54Z

@mahills Indeed, qualified names in concrete patterns are not the solution either.

I like you idea about merging constructors and productions in a single type since that eliminates the current confusion. Find it hard to oversee the implications of this.

PaulKlint · 2014-02-11T22:51:56Z

This solution would imply that instead of

module demo::lang::Exp::Combined::Automatic::Load

import demo::lang::Exp::Combined::Automatic::Parse;  /*1*/
import demo::lang::Exp::Abstract::Syntax;            /*2*/
import ParseTree;                                    /*3*/

public demo::lang::Exp::Abstract::Syntax::Exp load(str txt) = 
       implode(#demo::lang::Exp::Abstract::Syntax::Exp, parse(txt));

we can write

module demo::lang::Exp::Combined::Automatic::Load

import demo::lang::Exp::Combined::Automatic::Parse;  /*1*/
import demo::lang::Exp::Abstract::Syntax;            /*2*/
import ParseTree;                                    /*3*/

public Exp load(str txt) = 
       implode(#Exp, parse(txt));

and that looks great!

mahills · 2014-02-12T00:50:26Z

I'm not sure it has any problems, I just want to be careful. It would definitely simplify things.

On Feb 11, 2014, at 5:43 PM, "Paul Klint" <notifications@github.com mailto:notifications@github.com> wrote:

@mahillshttps://github.com/mahills Indeed, qualified names in concrete patterns are not the solution either.

I like you idea about merging constructors and productions in a single type since that eliminates the current confusion. Find it hard to oversee the implications of this.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/493#issuecomment-34817654.

mahills · 2014-02-12T15:39:21Z

Well, the subtyping of concrete nonterminal types with Tree and of algebraic datatypes with node could be interesting to handle (since Tree is also a subtype of node, especially).

jurgenvinju · 2014-02-12T17:16:57Z

We have considered such option before and did not arrive at a good design yet. Indeed the Tree type is in the way, but also the fact that a parse tree is really different from an ast. We also need a common super type of all parse trees for language independent processing...—
Jurgen J. Vinju
CWI SWAT
INRIA Lille
UvA master software engineering
http://jurgen.vinju.org

On Wed, Feb 12, 2014 at 4:39 PM, mahills notifications@github.com wrote:

Well, the subtyping of concrete nonterminal types with Tree and of algebraic datatypes with node could be interesting to handle (since Tree is also a subtype of node, especially).

Reply to this email directly or view it on GitHub:
#493 (comment)

swatbot · 2014-02-12T17:20:52Z

Also there would be type safety problems where asts could suddenly be nested inside parse trees. —
Jurgen J. Vinju
CWI SWAT
INRIA Lille
UvA master software engineering
http://jurgen.vinju.org

On Wed, Feb 12, 2014 at 6:17 PM, Jurgen J. Vinju notifications@github.com
wrote:

We have considered such option before and did not arrive at a good design yet. Indeed the Tree type is in the way, but also the fact that a parse tree is really different from an ast. We also need a common super type of all parse trees for language independent processing...—
Jurgen J. Vinju
CWI SWAT
INRIA Lille
UvA master software engineering
http://jurgen.vinju.org
On Wed, Feb 12, 2014 at 4:39 PM, mahills notifications@github.com wrote:

Well, the subtyping of concrete nonterminal types with Tree and of algebraic datatypes with node could be interesting to handle (since Tree is also a subtype of node, especially).

Reply to this email directly or view it on GitHub:

#493 (comment)

Reply to this email directly or view it on GitHub:
#493 (comment)

mahills · 2014-02-12T17:37:41Z

Ah, good point, we really can't keep track of whether the type was
constructed based on a constructor or a production.

swatbot mailto:notifications@github.com
February 12, 2014 at 12:20 PM
Also there would be type safety problems where asts could suddenly be
nested inside parse trees. —
Jurgen J. Vinju
CWI SWAT
INRIA Lille
UvA master software engineering
http://jurgen.vinju.org

On Wed, Feb 12, 2014 at 6:17 PM, Jurgen J. Vinju
notifications@github.com
wrote:

We have considered such option before and did not arrive at a good
design yet. Indeed the Tree type is in the way, but also the fact that
a parse tree is really different from an ast. We also need a common
super type of all parse trees for language independent processing...—
Jurgen J. Vinju
CWI SWAT
INRIA Lille
UvA master software engineering
http://jurgen.vinju.org
On Wed, Feb 12, 2014 at 4:39 PM, mahills notifications@github.com
wrote:

Well, the subtyping of concrete nonterminal types with Tree and
of algebraic datatypes with node could be interesting to handle

(since Tree is also a subtype of node, especially).

Reply to this email directly or view it on GitHub:

#493 (comment)

Reply to this email directly or view it on GitHub:
#493 (comment)

—
Reply to this email directly or view it on GitHub
#493 (comment).

Paul Klint mailto:notifications@github.com
February 11, 2014 at 4:36 AM

Compiling |demo::lang::Exp::Combined::Manual::Load|:

|module demo::lang::Exp::Combined::Manual::Load

import demo::lang::Exp::Concrete::WithLayout::Syntax; /1/
import demo::lang::Exp::Abstract::Syntax; /2/
import demo::lang::Exp::Combined::Manual::Parse; /3/
import String;

public Exp load(str txt) = load(parse(txt)); /4/

public Exp load((Exp)<IntegerLiteral l>) /5/
= con(toInt(""));
public Exp load((Exp)<Exp e1> * <Exp e2>)
= mul(load(e1), load(e2));
public Exp load((Exp)<Exp e1> + <Exp e2>)
= add(load(e1), load(e2));
public Exp load((Exp)( <Exp e> ))
= load(e);
|

leads to a lot of name confusion:

|error("Multiple functions found which could be
applied",|rascal:///demo/lang/Exp/Combined/Manual/Load.rsc|(616,7,<17,9>,<17,16>))
error("Syntax type Exp is not
defined",|rascal:///demo/lang/Exp/Combined/Manual/Load.rsc|(508,3,<14,17>,<14,20>))
error("Syntax type Exp is not
defined",|rascal:///demo/lang/Exp/Combined/Manual/Load.rsc|(444,3,<12,34>,<12,37>))
error("Multiple functions found which could be
applied",|rascal:///demo/lang/Exp/Combined/Manual/Load.rsc|(468,8,<13,13>,<13,21>))
error("Multiple functions found which could be
applied",|rascal:///demo/lang/Exp/Combined/Manual/Load.rsc|(548,8,<15,13>,<15,21>))
error("Syntax type Exp is not
defined",|rascal:///demo/lang/Exp/Combined/Manual/Load.rsc|(525,3,<14,34>,<14,37>))
error("Syntax type Exp is not
defined",|rascal:///demo/lang/Exp/Combined/Manual/Load.rsc|(433,3,<12,23>,<12,26>))
error("Type Exp not
declared",|project://rascal/src/org/rascalmpl/library/demo/lang/Exp/Combined/Manual/Parse.rsc|(128,3,<5,7>,<5,10>))
error("Syntax type Exp is not
defined",|rascal:///demo/lang/Exp/Combined/Manual/Load.rsc|(514,3,<14,23>,<14,26>))
error("Multiple functions found which could be
applied",|rascal:///demo/lang/Exp/Combined/Manual/Load.rsc|(478,8,<13,23>,<13,31>))
error("Multiple functions found which could be
applied",|rascal:///demo/lang/Exp/Combined/Manual/Load.rsc|(558,8,<15,23>,<15,31>))
error("Syntax type Exp is not
defined",|rascal:///demo/lang/Exp/Combined/Manual/Load.rsc|(331,3,<10,17>,<10,20>))
error("Syntax type Exp is not
defined",|rascal:///demo/lang/Exp/Combined/Manual/Load.rsc|(427,3,<12,17>,<12,20>))
error("Unexpected type: type of body expression, Exp, must be a
subtype of the function return type,
fail",|rascal:///demo/lang/Exp/Combined/Manual/Load.rsc|(384,17,<11,9>,<11,26>))
error("Syntax type Exp is not
defined",|rascal:///demo/lang/Exp/Combined/Manual/Load.rsc|(587,3,<16,17>,<16,20>))
error("Syntax type Exp is not
defined",|rascal:///demo/lang/Exp/Combined/Manual/Load.rsc|(595,3,<16,25>,<16,28>))
|

—
Reply to this email directly or view it on GitHub
#493.

PaulKlint · 2014-02-14T20:57:51Z

I am further exploring this issue and a bit simpler example is this:

module experiments::Compiler::Examples::ExpTst

import ParseTree;
import demo::lang::Exp::Concrete::WithLayout::Syntax;
import demo::lang::Exp::Abstract::Syntax; 

demo::lang::Exp::Concrete::WithLayout::Syntax::Exp v = (Exp) `1`;

which gives error("Syntax type Exp is not defined",|rascal:///experiments/Compiler/Examples/ExpTst.rsc|(220,3,<7,56>,<7,59>))

@mahills you said that "the abstract type knocks the concrete type out of scope", but would it in any way be possible to use the knocked out type in the case of (Exp) that prefixes a concrete pattern or term?

mahills · 2014-06-18T16:07:04Z

I'm looking at this again, so it's probably best we restart the conversation on this topic :)

Honestly, I'm still not sure the best approach here. The problem remains as before, even after the various changes -- because the current module would import two distinct types named Exp, one ADT and one nonterminal, Exp is not added into scope, but instead the longer, qualified names would be used. It may be possible to maintain a separate scope just for uses where we can distinguish between concrete and abstract types, like we have above (here, we would know that Exp has to be concrete, and can react accordingly), but my concern would be that this could lead to confusion. It may be hard to explain why, in some places, you can say Exp, but in others, even in the same statement, you cannot.

swatbot · 2014-06-18T16:46:34Z

We need a radical solution here, i.e. A language change. I believe a type constructor to separate the two name spaces would help or a different solution for implode which does not require the names to be equal or both..—
Jurgen J. Vinju
CWI SWAT
INRIA Lille
UvA master software engineering
http://jurgen.vinju.org

On Wed, Jun 18, 2014 at 6:07 PM, mahills notifications@github.com wrote:

I'm looking at this again, so it's probably best we restart the conversation on this topic :)

Honestly, I'm still not sure the best approach here. The problem remains as before, even after the various changes -- because the current module would import two distinct types named Exp, one ADT and one nonterminal, Exp is not added into scope, but instead the longer, qualified names would be used. It may be possible to maintain a separate scope just for uses where we can distinguish between concrete and abstract types, like we have above (here, we would know that Exp has to be concrete, and can react accordingly), but my concern would be that this could lead to confusion. It may be hard to explain why, in some places, you can say Exp, but in others, even in the same statement, you cannot.

Reply to this email directly or view it on GitHub:
#493 (comment)

mahills · 2014-06-18T16:54:06Z

I agree. Given code like this:

Exp myfun(Exp e) = ...

where Exp has been imported both as an ADT and as a nonterminal, this really is ambiguous -- not only can the checker not determine which of the two should be used here, a human reading this code may not be able to figure this out either (except, maybe, for whoever wrote it).

PaulKlint · 2014-06-18T21:02:21Z

@mahills I agree that the current situation is unacceptable and that we need something better. I will discuss this the coming days with @jurgenvinju and @tvdstorm and maybe we can have a Skype session after that. Any ideas/suggestions appreciated.

jurgenvinju · 2014-06-19T05:55:13Z

For now such ambiguity should be flagged as an error and resolvable using qualification. Is that possible without a big effort?—
Jurgen J. Vinju
CWI SWAT
INRIA Lille
UvA master software engineering
http://jurgen.vinju.org

On Wed, Jun 18, 2014 at 6:54 PM, mahills notifications@github.com wrote:

I agree. Given code like this:
Exp myfun(Exp e) = ...

where Exp has been imported both as an ADT and as a nonterminal, this really is ambiguous -- not only can the checker not determine which of the two should be used here, a human reading this code may not be able to figure this out either (except, maybe, for whoever wrote it).

Reply to this email directly or view it on GitHub:
#493 (comment)

mahills · 2014-06-19T11:25:43Z

We can flag it as an error, I'll just need to carry around information about which names were not added because they would have created ambiguities, like Exp in this example. It is already possible to use the qualified versions of the names, at least in the checker -- I'm not sure if all the positions in the program above allow fully qualified names, some may only allow unqualified names. I'll have to check that later.

tvdstorm · 2014-06-19T11:46:02Z

Not sure it was mentioned yet, but the current solution is to keep both ADT and grammar in separate modules, and use qualified names only in a custom Implode module. The other unfortunate thing is is that qualified non-terminal names don't work in start[X].

jurgenvinju · 2014-06-19T13:38:48Z

start[X] could be an exception because X can only be a non-terminal.

On Thu, Jun 19, 2014 at 1:25 PM, mahills notifications@github.com wrote:

We can flag it as an error, I'll just need to carry around information
about which names were not added because they would have created
ambiguities, like Exp in this example. It is already possible to use the
qualified versions of the names, at least in the checker -- I'm not sure if
all the positions in the program above allow fully qualified names, some
may only allow unqualified names. I'll have to check that later.

—
Reply to this email directly or view it on GitHub
#493 (comment).

Jurgen Vinju

Centrum Wiskunde & Informatica - SEN1
INRIA Lille - ATEAMS
Universiteit van Amsterdam

www: http://jurgen.vinju.org, http://www.rascal-mpl.nl,
http://twitter.com/jurgenvinju
skype: jurgen.vinju

mahills · 2014-06-19T13:47:55Z

@tvdstorm I thought that was the case, but wanted to check the grammar first to make sure my memory was accurate.

@jurgenvinju we can do that in multiple places (we could say the same of the use of the names inside concrete syntax patterns, for instance), but that would still leave open the question of how we differentiate a concrete Exp from an abstract Exp in function parameters, function return types, etc. I can always keep around separate namespaces inside the checker (a concrete namespace for cases where only a concrete type can be used, an abstract namespace for cases where only an abstract type can be used, plus the current combined namespace) but this is making the concept of scope for Rascal more complex.

swatbot · 2014-06-19T15:39:09Z

Agreed. We have a simple concept for the other names and conflicts, namely merging the functions, datatypes, whenever possible, but we don't know how to merge abstract and concrete syntax yet.. A solution could be in this direction, where we truly consider data A and syntax A to produce concrete terms of the same abstract sort.

From the perspective of the liskov principle, embedding a concrete tree in an abstract one is fine, but vice versa is a problem. We need a step that automatically lifts an abstract tree to a concrete one to fix this.... Then we would always generate type safe trees and the abstract type and the syntax type could be unified.
—
Jurgen J. Vinju
CWI SWAT
INRIA Lille
UvA master software engineering
http://jurgen.vinju.org

On Thu, Jun 19, 2014 at 3:48 PM, mahills notifications@github.com wrote:

@tvdstorm I thought that was the case, but wanted to check the grammar first to make sure my memory was accurate.

@jurgenvinju we can do that in multiple places (we could say the same of the use of the names inside concrete syntax patterns, for instance), but that would still leave open the question of how we differentiate a concrete Exp from an abstract Exp in function parameters, function return types, etc. I can always keep around separate namespaces inside the checker (a concrete namespace for cases where only a concrete type can be used, an abstract namespace for cases where only an abstract type can be used, plus the current combined namespace) but this is making the concept of scope for Rascal more complex.

Reply to this email directly or view it on GitHub:
#493 (comment)

PaulKlint · 2014-12-12T21:59:54Z

This file is still pestering us, @mahills what are your current thoughts on this?

Before delving into language changes (as suggested above and in the related issues) we need to get this one up and running.

Currently the messages are:

error("Syntax type Exp is not defined",|project://rascal/src/org/rascalmpl/library/demo/lang/Exp/Combined/Manual/Load.rsc|(422,3,<11,52>,<11,55>))
error("Syntax type Exp is not defined",|project://rascal/src/org/rascalmpl/library/demo/lang/Exp/Combined/Manual/Load.rsc|(669,3,<15,52>,<15,55>))
error("Syntax type Exp is not defined",|project://rascal/src/org/rascalmpl/library/demo/lang/Exp/Combined/Manual/Load.rsc|(553,3,<13,52>,<13,55>))
error("Syntax type Exp is not defined",|project://rascal/src/org/rascalmpl/library/demo/lang/Exp/Combined/Manual/Load.rsc|(559,3,<13,58>,<13,61>))
error("Syntax type Exp is not defined",|project://rascal/src/org/rascalmpl/library/demo/lang/Exp/Combined/Manual/Load.rsc|(675,3,<15,58>,<15,61>))
error("Syntax type Exp is not defined",|project://rascal/src/org/rascalmpl/library/demo/lang/Exp/Combined/Manual/Load.rsc|(791,3,<17,60>,<17,63>))
error("Syntax type Exp is not defined",|project://rascal/src/org/rascalmpl/library/demo/lang/Exp/Combined/Manual/Load.rsc|(783,3,<17,52>,<17,55>))
error("Syntax type Exp is not defined",|project://rascal/src/org/rascalmpl/library/demo/lang/Exp/Combined/Manual/Load.rsc|(686,3,<15,69>,<15,72>))
error("Syntax type Exp is not defined",|project://rascal/src/org/rascalmpl/library/demo/lang/Exp/Combined/Manual/Load.rsc|(570,3,<13,69>,<13,72>))
error("Function of type fun Exp(Exp) cannot be called with argument types (Tree)",|project://rascal/src/org/rascalmpl/library/demo/lang/Exp/Combined/Manual/Load.rsc|(330,19,<9,65>,<9,84>))

mahills · 2014-12-16T22:16:39Z

I now have this as the message in all the places where Syntax type Exp is not defined was being shown:

Nonterminal Exp was not imported, use one of the following fully qualified type names instead: demo::lang::Exp::Concrete::WithLayout::Syntax::Exp,demo::lang::Exp::Abstract::Syntax::Exp

This could also do with some work (since it gives names for both concrete and abstract types), but at least it's more informative. It also gives a better description of the error -- Exp wasn't imported as an unqualified name since there are two incompatible versions.

PaulKlint · 2014-12-16T22:47:06Z

That is certainly an improvement.

We keep running in circles since, sadly, all roads to a solution are blocked by the fact that we can currently not use qualified names in certain places:

inside concrete patterns or strings, e..g., <demo::lang::Exp::Concrete::WithLayout::Syntax::Exp e2>.
as type before a concrete string or pattern, e.g., (demo::lang::Exp::Concrete::WithLayout::Syntax::Exp) 1
start[X]

I propose therefore that we put this issue on hold and reenact it when we become more liberal in the use of qualified names.

In the mean time I will rewrite/delete some of the demo that cause these problems.

mahills · 2014-12-17T00:21:37Z

I'll see what I can do to handle those in the checker as well -- I may be able to handle all those cases without too much trouble.

PaulKlint · 2014-12-17T08:11:58Z

But this will also require changes to the Rascal syntax and I am reluctant to do that now since that will break things in various places.

jurgenvinju · 2014-12-17T08:26:45Z

@PaulKlint for the three cases you mention the semantics is unambiguous; it can not be anything but a concrete syntax type. I suspect @mahills is planning to hard code this fact into the type checker as an intermediate solution.

mahills · 2014-12-17T12:04:51Z

@jurgenvinju something like that -- I'm just trying to decide if I should look them up manually, using the information I already have, or add another map to the configuration (which would be slightly faster but would take up even more space, then, and require maintenance to keep it up to date)

jurgenvinju · 2014-12-17T12:21:00Z

@mahills I guess you answered that one. Look up manually 👍

mahills · 2014-12-17T15:03:35Z

@PaulKlint I believe all 3 cases are now supported

PaulKlint · 2014-12-17T15:11:10Z

Super, will check this in a moment.

PaulKlint · 2014-12-17T15:39:14Z

But on second thoughts, how could I test this? Any example will be syntactically incorrect!

Do you have an example where this works? Or should we just postpone testing this until we have syntactic support for the above 3 cases?

mahills · 2014-12-17T15:41:57Z

What I've done is to allow cases like the above to work with unqualified names. For instance, if you import a concrete and an abstract version of Exp, Exp will not be added to the type environment, but the qualified versions are available. If you use Exp in a case where it can only be a sort, the code is first looking it up as it was before, but then is falling back to checking that Exp was actually imported, wasn't added because of a name clash, and is in fact the name of a sort. In those cases, it is resolved anyway, since that is not ambiguous.

mahills · 2014-12-17T15:42:43Z

All the checking also makes sure it was really imported, instead of somehow "leaking" through (since we bring in all the grammar info transitively anyway).

jurgenvinju · 2014-12-17T16:03:45Z

Great! @PaulKlint so we need a test where Exp is imported twice once as data and once as syntax, and used in a start[Exp] but does not lead to a type check error.

mahills · 2014-12-17T16:04:55Z

The module that started this issue is a perfect example, since it does this (except for using start[Exp]).

PaulKlint · 2014-12-17T16:25:07Z

Indeed, the above example (with disambiguated names) now works fine (I was under the assumption that more disambiguation was needed). I am inclined to close this issue for now.

We end up with the following, quite ugly, but working code. I may remove this from the demos to avoid confronting Rascal novices with this.

module demo::lang::Exp::Combined::Manual::Load

import demo::lang::Exp::Concrete::WithLayout::Syntax;  /*1*/
import demo::lang::Exp::Abstract::Syntax;              /*2*/
import demo::lang::Exp::Combined::Manual::Parse;       /*3*/
import String;

public demo::lang::Exp::Abstract::Syntax::Exp loadExp(str txt) = load(parseExp(txt));        /*4*/

public demo::lang::Exp::Abstract::Syntax::Exp load((Exp)`<IntegerLiteral l>`)             /*5*/
       = con(toInt("<l>"));       
public demo::lang::Exp::Abstract::Syntax::Exp load((Exp)`<Exp e1> * <Exp e2>`) 
       = mul(load(e1), load(e2));  
public demo::lang::Exp::Abstract::Syntax::Exp load((Exp)`<Exp e1> + <Exp e2>`)
       = add(load(e1), load(e2)); 
public demo::lang::Exp::Abstract::Syntax::Exp load((Exp)`( <Exp e> )`) 
       = load(e);

jurgenvinju · 2014-12-17T16:41:05Z

One way to make the code look better is to choose different names for Exp. Is @tvdstorm already finished with the new implode such that we can implode to different data?

PaulKlint · 2014-12-17T16:44:54Z

Actually, in this specific example we do the mapping manually and could rename as we like.

However, to be compatible with the implode-based solution the names are the same here. I don't think the new implode is already ready for prime time.

mahills · 2014-12-17T17:12:11Z

I guess we could also use aliases -- I haven't tried it here, but think it would work.

jurgenvinju · 2014-12-17T17:27:57Z

certainly aliases should work as well to make the code look better.

mahills · 2014-12-17T17:51:34Z

Right, then maybe add

alias AbstractExp = demo::lang::Exp::Abstract::Syntax::Exp;

and then use that for the return types, which would leave it well-typed while making it look better. That would at least take care of it for now. I'm happy to make the changes in a bit...

PaulKlint · 2014-12-17T20:22:31Z

Thanks @mahills, but I have some other changes in mind as well. So will do this.

PaulKlint · 2014-12-17T20:23:41Z

With 43 comments this is one of the longer discussions, nonetheless closing it now.

PaulKlint added this to the Rascal compiler milestone Feb 11, 2014

PaulKlint added bug labels Feb 11, 2014

mahills mentioned this issue Aug 16, 2014

Allow qualified names in concrete patterns #638

Closed

jurgenvinju mentioned this issue Aug 17, 2014

Overloading constructors: who is right? #502

Closed

PaulKlint closed this as completed Dec 17, 2014

TC: name confusion #493

TC: name confusion #493

Comments

PaulKlint commented Feb 11, 2014

mahills commented Feb 11, 2014

PaulKlint commented Feb 11, 2014

mahills commented Feb 11, 2014

PaulKlint commented Feb 11, 2014

PaulKlint commented Feb 11, 2014

mahills commented Feb 12, 2014

mahills commented Feb 12, 2014

jurgenvinju commented Feb 12, 2014

Well, the subtyping of concrete nonterminal types with Tree and of algebraic datatypes with node could be interesting to handle (since Tree is also a subtype of node, especially).

swatbot commented Feb 12, 2014

Well, the subtyping of concrete nonterminal types with Tree and of algebraic datatypes with node could be interesting to handle (since Tree is also a subtype of node, especially).

#493 (comment)

mahills commented Feb 12, 2014

(since Tree is also a subtype of node, especially).

#493 (comment)

PaulKlint commented Feb 14, 2014

mahills commented Jun 18, 2014

swatbot commented Jun 18, 2014

mahills commented Jun 18, 2014

PaulKlint commented Jun 18, 2014

jurgenvinju commented Jun 19, 2014

where Exp has been imported both as an ADT and as a nonterminal, this really is ambiguous -- not only can the checker not determine which of the two should be used here, a human reading this code may not be able to figure this out either (except, maybe, for whoever wrote it).

mahills commented Jun 19, 2014

tvdstorm commented Jun 19, 2014

jurgenvinju commented Jun 19, 2014

mahills commented Jun 19, 2014

swatbot commented Jun 19, 2014

PaulKlint commented Dec 12, 2014

mahills commented Dec 16, 2014

PaulKlint commented Dec 16, 2014

mahills commented Dec 17, 2014

PaulKlint commented Dec 17, 2014

jurgenvinju commented Dec 17, 2014

mahills commented Dec 17, 2014

jurgenvinju commented Dec 17, 2014

mahills commented Dec 17, 2014

PaulKlint commented Dec 17, 2014

PaulKlint commented Dec 17, 2014

mahills commented Dec 17, 2014

mahills commented Dec 17, 2014

jurgenvinju commented Dec 17, 2014

mahills commented Dec 17, 2014

PaulKlint commented Dec 17, 2014

jurgenvinju commented Dec 17, 2014

PaulKlint commented Dec 17, 2014

mahills commented Dec 17, 2014

jurgenvinju commented Dec 17, 2014

mahills commented Dec 17, 2014

PaulKlint commented Dec 17, 2014

PaulKlint commented Dec 17, 2014

Well, the subtyping of concrete nonterminal types with `Tree` and of algebraic datatypes with `node` could be interesting to handle (since `Tree` is also a subtype of `node`, especially).

Well, the subtyping of concrete nonterminal types with `Tree` and of algebraic datatypes with `node` could be interesting to handle (since `Tree` is also a subtype of `node`, especially).

(since `Tree` is also a subtype of `node`, especially).

where `Exp` has been imported both as an ADT and as a nonterminal, this really is ambiguous -- not only can the checker not determine which of the two should be used here, a human reading this code may not be able to figure this out either (except, maybe, for whoever wrote it).