Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Command Line REPL doesn't like simple module names? #10

Closed
Ingo60 opened this issue Oct 27, 2014 · 9 comments
Closed

Command Line REPL doesn't like simple module names? #10

Ingo60 opened this issue Oct 27, 2014 · 9 comments
Assignees

Comments

@Ingo60
Copy link
Member

Ingo60 commented Oct 27, 2014

Here is a transcript:

frege> :load /home/ingo/frege/examples/CommandLineClock.fr
Module examples.CommandLineClock
frege> :browse examples.CommandLineClock
data type Date :: native java.util.Date
value current :: IO String
value main :: [String] -> IO ()
frege> current
[ERROR: 4]: can't resolve `current`, did you mean `curry` perhaps?
frege> import examples.CommandLineClock
frege> current
"Mon Oct 27 21:12:35 CET 2014"

So far, so good.
But then:

frege> :load /home/ingo/frege/frege/Scrap.fr
Module Scrap
frege> :browse Scrap
function toDouble :: Integral a => a -> Double
value foo :: [Double]
frege> import Scrap
frege> foo
[ERROR: 1]: The import Scrap cannot be resolved

To reproduce, try to load and import a module with a simple name, and then use definitions from it.

@mmhelloworld
Copy link
Member

I think the problem is that in Java, classes in the default package cannot be imported into another packaged class.

Since Scrap.fr doesn't have a package element, it is compiled into default package which can be accessible only using reflection. :browse uses reflection hence it is able to access the class and print the members while the import in the generated Java source cannot access default package classes.

@Ingo60
Copy link
Member Author

Ingo60 commented Oct 28, 2014

So are you saying that error comes from the Java compiler?

@mmhelloworld
Copy link
Member

Most likely but it could also be Frege compiler. I am not sure even how
Frege compiler can import a class in the default package into a packaged
module. Even if it does that, I don't think it is possible with Java
compiler.

On Tuesday, October 28, 2014, Ingo Wechsung notifications@github.com
wrote:

So are you saying that error comes from the Java compiler?


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

@Ingo60
Copy link
Member Author

Ingo60 commented Oct 28, 2014

The compiler reads only the annotations on an import, and then rebuilds the symbol table based on the information (type FregePackage).
I guess this counts also as reflection.

A short test reveals that importing some class from the unnamed package is indeed impossible, the SDK javac does not even allow an unqualified name after import:

ingo@ibinti:~/frege$ fregec -d /tmp/x /tmp/B.fr /tmp/A.fr # A imports B
calling: javac -cp build:/tmp -d /tmp -sourcepath . -encoding UTF-8 /tmp/x/B.java 
calling: javac -cp build:/tmp -d /tmp -sourcepath . -encoding UTF-8 /tmp/x/A.java 
/tmp/x/A.java:16: error: '.' expected
import B;
        ^
/tmp/x/A.java:16: error: ';' expected
import B;
         ^
/tmp/x/A.java:17: error: class, interface, or enum expected
import frege.control.Category;
       ^
3 errors
E /tmp/A.fr:5: java compiler errors are most likely caused by erronous
    native definitions
runtime 6.443 wallclock seconds.

However, we could nevertheless just use B from A. I confirmed this by removing the import B from A.java and recompiling.

So, it turns out the code generator must not generate import statements for simple packages!

However, we can use such classes only from classes in other unnamed packages.

So, as I see it, we need following fixes:

  • code generation must not generate imports of classes from unnamed packages
  • a frege import of a simple name in a module with a non-simple name must get flagged (I hate to do this, because it makes absolutely no sense! But, no choice.)
  • the repl session package must itself be simple (i.e. Console, instead of frege.repl.Console)

If you agree with me, I shall prepare the first two fixes relative to 3.21.500.

I consider this crucial, for the simple reason that it is most likely that beginners begin with something like

module Test where

and then get disappointed when they try that in the REPL and get errors without them having done anything wrong.

@mmhelloworld
Copy link
Member

Thanks Ingo! That clears up a lot of things.

I also agree with those fixes you proposed. I will make the necessary
changes in the REPL later today.

On Tuesday, October 28, 2014, Ingo Wechsung notifications@github.com
wrote:

The compiler reads only the annotations on an import, and then rebuilds
the symbol table based on the information (type FregePackage).
I guess this counts also as reflection.

A short test reveals that importing some class from the unnamed package is
indeed impossible, the SDK javac does not even allow an unqualified name
after import:

ingo@ibinti:~/frege$ fregec -d /tmp/x /tmp/B.fr /tmp/A.fr # A imports B
calling: javac -cp build:/tmp -d /tmp -sourcepath . -encoding UTF-8 /tmp/x/B.java
calling: javac -cp build:/tmp -d /tmp -sourcepath . -encoding UTF-8 /tmp/x/A.java
/tmp/x/A.java:16: error: '.' expected
import B;
^
/tmp/x/A.java:16: error: ';' expected
import B;
^
/tmp/x/A.java:17: error: class, interface, or enum expected
import frege.control.Category;
^
3 errors
E /tmp/A.fr:5: java compiler errors are most likely caused by erronous
native definitions
runtime 6.443 wallclock seconds.

However, we could nevertheless just use B from A. I confirmed this by
removing the import B from A.java and recompiling.

So, it turns out the code generator must not generate import statements
for simple packages!

However, we can use such classes only from classes in other unnamed
packages.

So, as I see it, we need following fixes:

  • code generation must not generate imports of classes from unnamed
    packages
  • a frege import of a simple name in a module with a non-simple name
    must get flagged (I hate to do this, because it makes absolutely no sense!
    But, no choice.)
  • the repl session package must itself be simple (i.e. Console,
    instead of frege.repl.Console)

If you agree with me, I shall prepare the first two fixes relative to
3.21.500.

I consider this crucial, for the simple reason that it is most likely that
beginners begin with something like

module Test where

and then get disappointed when they try that in the REPL and get errors
without them having done anything wrong.


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

@Ingo60
Copy link
Member Author

Ingo60 commented Oct 28, 2014

Fine then. I have created an issue accordingly for the frege project .
Note I will work on the release21 branch, which corresponds to the "FInal 21" release, so the fixes will be relative to 3.21.586 (not 500, as I said before).
Keep watching for commits to frege!

@mmhelloworld mmhelloworld self-assigned this Oct 29, 2014
@mmhelloworld
Copy link
Member

REPL session is now compiled into the default package. Both online and command line REPL are updated and are also upgraded to 3.21.586.

$ java -jar frege-repl-1.0.3-SNAPSHOT.jar 
Welcome to Frege 3.21.586-g026e8d7 (Oracle Corporation Java HotSpot(TM) 64-Bit Server VM, 1.8.0_25)

frege> module Foo where { bar = "I am Bar!" }
Module Foo

frege> pure native bar Foo.bar :: String
native function bar :: String

frege> bar
I am Bar!

@Ingo60
Copy link
Member Author

Ingo60 commented Dec 6, 2014

Should be fixed with commit 8b2dcd87b8dc41f1ef808eb83ffa8d23e0babf7a
Or does it?

@mmhelloworld
Copy link
Member

Sorry, I am not sure how I missed your comment. Yes, this issue is fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants