bootstrapping w/ new compiler: Merged #233 onto master #241

Merged
merged 66 commits into from Jun 16, 2014

Conversation

Projects
None yet
2 participants
@baroquebobcat
Member

baroquebobcat commented Jan 2, 2014

see #233 for details

@ribrdb

This comment has been minimized.

Show comment
Hide comment
@ribrdb

ribrdb Feb 11, 2014

Contributor

Actually the whole point of the proxies is to make sure different versions of a type do not compare as equal in the typer. We should fix the test use isSameType or toString or something like that.

Contributor

ribrdb commented on d306a89 Feb 11, 2014

Actually the whole point of the proxies is to make sure different versions of a type do not compare as equal in the typer. We should fix the test use isSameType or toString or something like that.

This comment has been minimized.

Show comment
Hide comment
Member

baroquebobcat replied Feb 17, 2014

Ok.

baroquebobcat added some commits Feb 17, 2014

Revert "fix mirror lub equality assertion"
This reverts commit d306a89.

Turns out this is incorrect: d306a89#commitcomment-5345748
improve compile, parse_and_type new backend test helpers
now at 1 failures, 4 errors, 10 pendings instead of 2 failures, 5 errors, 10 pendings. Slightly better
change field setter test so it doesn't depend on ast
it use to depend on mirah.lang.ast, but that's not in the classpath by default
replace System.out.println w/ puts in tests
since the puts macro is back, we should use it in tests instead of System.out.println
fix annotation value test
fixtures are not added to the classpath anymore, so you can't get at them easily from jruby

additionally, the annotations should use RUNTIME retention to ensure it's accessible.
oops. fix typo in args code
forgot to run compile.

baroquebobcat added some commits Feb 18, 2014

rm mirahp
mirahp isn't very interesting. I'd like to have some debug tools, but it's not it. as far as I can tell, it's not used. Partly because it doesn't do very much
use File::PATH_SEPARATOR for classpath when constructing classpaths i…
…n Rakefile

':' isn't the path separator on windows, so hardcoding the path separator leads to an inability to build on windows. :(

This fixes it.
delete all the things! rm JRuby Compiler bits
the self hosted compiler covers everything at this point, so I'm deleting as much of the JRuby compiler components as I can get away with
deprecate -c for classpath in favor of -cp / --classpath
-c now works w/ new compiler, but spits out some stderr
fix up logging. typo, multi install, rm ruby impl
The Ruby log formatter isn't needed.
The Mirah one had a couple bugs
- it used split('.'), but java's split takes regex, so fixed that
- it would install itself multiple times, so I added a check to not create a new handler if one already existed
fix BlocksTest#test_block_with_method_def
we were inferring block args too early and more importantly, introducing a scope around them. That put those args in a scope that was captured prematurely

Because they were captured, the compiler would think they were fields in a binding, even if the block was inlined as part of a macro, causing bytecode for an invalid field to be generated.
fix map collection extension method
backtick in pipes doesn't work when putting the block args in it. You need to put the contents of the args there.
In theory you could use backticks w/o pipes, but I haven't gotten that working

a little more discussion is at https://gist.github.com/baroquebobcat/0236307218d989615ffa
Merge branch 'master' into bootstrap_with_new_compiler
Conflicts:
	src/org/mirah/tool/mirahc.mirah
	test/jvm/bytecode_test_helper.rb

@baroquebobcat baroquebobcat merged commit 5dbd5b9 into master Jun 16, 2014

@ribrdb

This comment has been minimized.

Show comment
Hide comment
@ribrdb

ribrdb Jul 2, 2014

Contributor

Do you actually think supporting shadowing is a good idea, or are you just trying to fix the test?

Contributor

ribrdb commented on 9cd7f6e Jul 2, 2014

Do you actually think supporting shadowing is a good idea, or are you just trying to fix the test?

This comment has been minimized.

Show comment
Hide comment
@baroquebobcat

baroquebobcat Jul 14, 2014

Member

Just trying to fix the test. I've been trying to treat the new backend as a port of the existing one, with the same behavior. The JRuby implemented Mirah compiler currently shadows the variable in this case. The following code compiles and prints 1

a = 1
begin
  raise "wut"
rescue => a
end
puts a # a is 1

In Java, shadowing isn't allowed.

class Test {
    public static void main(String[] args){
    Object e = 1;
    try{
        throw new RuntimeException("wut");
    } catch (Exception e){
        System.out.println(e);
    }
    System.out.println(e);
    }
}

gives you a compile error

Test.java:6: e is already defined in main(java.lang.String[])
    } catch (Exception e){
                       ^
1 error

which I think is a reasonable thing to do.

Ruby doesn't treat rescues as introducing scope, so the variable defined at that point exists in the outer scope and is overridden.

 a=1;begin; raise 'wut';rescue => a;end
# a is #<RuntimeError: wut>

Interestingly for Ruby, this also means that

begin raise 'wut'; rescue => b;end

adds b to the outer scope.

I see a few options:

  • keep existing behavior: introducing a new scope that shadows vars outside it
  • adopt Java's behavior: introduce a new scope, but don't allow shadowing and error like it's a variable redeclaration.
  • add a little Ruby-ish: don't shadow, and if the types don't work out, error, but if the types do work out, treat it as assignment.

That was longer than I thought it was going to be.

Because Mirah has static types, I don't think emulating Ruby all the way is a good idea because we'd have to widen types to much and rely on dynamic dispatch. Following Java's approach would be pretty understandable.

I think the current behavior makes some sense in that it allows you to have multiple es in a method without complaining too much, so the compiler stays out of your way. On the other hand, it means that if you assume e outside the rescue will be overwritten by the rescue after the rescue finishes, you're going to have a bad time.

Member

baroquebobcat replied Jul 14, 2014

Just trying to fix the test. I've been trying to treat the new backend as a port of the existing one, with the same behavior. The JRuby implemented Mirah compiler currently shadows the variable in this case. The following code compiles and prints 1

a = 1
begin
  raise "wut"
rescue => a
end
puts a # a is 1

In Java, shadowing isn't allowed.

class Test {
    public static void main(String[] args){
    Object e = 1;
    try{
        throw new RuntimeException("wut");
    } catch (Exception e){
        System.out.println(e);
    }
    System.out.println(e);
    }
}

gives you a compile error

Test.java:6: e is already defined in main(java.lang.String[])
    } catch (Exception e){
                       ^
1 error

which I think is a reasonable thing to do.

Ruby doesn't treat rescues as introducing scope, so the variable defined at that point exists in the outer scope and is overridden.

 a=1;begin; raise 'wut';rescue => a;end
# a is #<RuntimeError: wut>

Interestingly for Ruby, this also means that

begin raise 'wut'; rescue => b;end

adds b to the outer scope.

I see a few options:

  • keep existing behavior: introducing a new scope that shadows vars outside it
  • adopt Java's behavior: introduce a new scope, but don't allow shadowing and error like it's a variable redeclaration.
  • add a little Ruby-ish: don't shadow, and if the types don't work out, error, but if the types do work out, treat it as assignment.

That was longer than I thought it was going to be.

Because Mirah has static types, I don't think emulating Ruby all the way is a good idea because we'd have to widen types to much and rely on dynamic dispatch. Following Java's approach would be pretty understandable.

I think the current behavior makes some sense in that it allows you to have multiple es in a method without complaining too much, so the compiler stays out of your way. On the other hand, it means that if you assume e outside the rescue will be overwritten by the rescue after the rescue finishes, you're going to have a bad time.

This comment has been minimized.

Show comment
Hide comment
@ribrdb

ribrdb Jul 15, 2014

Contributor

My problem with your first two options is that the rescue statement is the only place we create new scopes.
What if you create new variables inside the rescue block - do these exist outside?

If we want to support new scopes for rescue, I think we need to follow java and create new scopes for every block. But then we also need to change the ast to keep track of blocks. These all parse to if statements, with no indication of which should get a scope:

a = b if c
a && (b = c)
if a
 b = c
end
Contributor

ribrdb replied Jul 15, 2014

My problem with your first two options is that the rescue statement is the only place we create new scopes.
What if you create new variables inside the rescue block - do these exist outside?

If we want to support new scopes for rescue, I think we need to follow java and create new scopes for every block. But then we also need to change the ast to keep track of blocks. These all parse to if statements, with no indication of which should get a scope:

a = b if c
a && (b = c)
if a
 b = c
end

This comment has been minimized.

Show comment
Hide comment
@baroquebobcat

baroquebobcat Jul 15, 2014

Member

After sleeping on it, I think following Ruby's behavior makes the most sense to me. There are a few cases where this gets a little tricky with primitives, but I think it'd be worth it for the ease of use / less time fighting w/ the compiler.

I started putting some thoughts together in this gist

https://gist.github.com/baroquebobcat/8c608c58b2e1dad1ead2

speaking of gists. What do you think about me throwing my aspirational docs on what I'd like Mirah's behavior to be into the repo? I think doing that'd make my thoughts more visible / discussable.

Member

baroquebobcat replied Jul 15, 2014

After sleeping on it, I think following Ruby's behavior makes the most sense to me. There are a few cases where this gets a little tricky with primitives, but I think it'd be worth it for the ease of use / less time fighting w/ the compiler.

I started putting some thoughts together in this gist

https://gist.github.com/baroquebobcat/8c608c58b2e1dad1ead2

speaking of gists. What do you think about me throwing my aspirational docs on what I'd like Mirah's behavior to be into the repo? I think doing that'd make my thoughts more visible / discussable.

This comment has been minimized.

Show comment
Hide comment
@ribrdb

ribrdb Jul 16, 2014

Contributor

Sure, checking in your ideas seems like a good idea.

Contributor

ribrdb replied Jul 16, 2014

Sure, checking in your ideas seems like a good idea.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment