Seems like Fan is subject to the same kinds of woes faced by most ;-doesn't-matter languages. I think it's safe to say that this isn't expected behavior. Personally, I like Python's consider-newline-the-end-if-at-all-possible mentality. Well, maybe it goes a bit overboard, but I think erring on that side is safer than defaulting to trying to continue to the next line. Note also that I prefer { on the end, rather than on the next line. (And as mentioned before, Ruby-style end works around this issue somewhat, too.)
But it seems Brian and Andy are next-line folks, and I don't want to go down that venerable subject line, it seems not safe to require { on the end, and so we need to look at the next line at least some when parsing statements.
So, as a workaround for now, I at least made ( require being on the same line. Seems to work fine. I only addressed normal calls, not the keywords (though that leaves things inconsistent). I'm not sure if any changes are acceptable in this arena at all, but here's my diff of Parser.fan:
1555c1555
< if (curt === Token.lparen)
---
> if (argStart(cur))
1616c1616
< if (peekt === Token.lparen)
---
> if (argStart(peek))
2114a2115,2122
> ** Check if we have an lparen for starting arguments
> ** but only if on the same line.
> **
> private Bool argStart(TokenVal token) {
> return token.kind == Token.lparen && !token.newline
> }
>
> **
That's based on the 1.0.27 download.
Again, no complaints if this goes unused (or if I haven't learned the right way to hack the compiler yet). Just felt like mentioning the mini adventure in case it's of value.
andyFri 20 Jun 2008
That looks like a bug in the compiler - Brian can comment.
cbeustFri 20 Jun 2008
Thanks, I was scratching my head over that one, trying to find what this "obvious" error was (Tom, you never shared your interpretation of the compiler error...).
-- Cedric
tompalmerFri 20 Jun 2008
To clarify, here's an equivalent example from ECMAScript:
var item = window
(1 + 1).toString()
alert(item)
That window (1 + 1) is seen as being a function call, and it causes a similar error to what I described above. They want to fix this in ES4 but are afraid of backwards compatibility (last I was following the conversation).
In both cases adding a semicolon fixes things, for example name := "Tom";. My Parser.fan patch fixes it so the ; isn't needed.
I think the second error from the Fan compiler is probably a compiler bug. I didn't address that. I only addressed the "args on next line" case.
andyFri 20 Jun 2008
Requiring the opening paren for a method to be on the same line seems reasonable to me, though I already always use that convention, so thats a bit biased.
The second error is correct - if (1..10) is interpreted as a method call on "Tom" - then name is not defined yet.
tompalmerFri 20 Jun 2008
OK. I get the "$name" issue now.
brianSat 21 Jun 2008
Tom,
You are spot on, one of the main problems with the grammar not using ; is when the following line starts with an open paren.
I think your suggestion is pretty sweet myself. Most all of the existing Fan codebase compiled OK with it - all of Andy's and my code. A couple lines written by a third guy used syntax like this:
if (colDefType == null)
throw HavenErr.make
("$type.qname: Unsupported type for column: $field.qname : $field.of")
But my personal opinion is that one way or the other you are going to get a weird compiler error. So I'm going to say from now on that the open paren of a method call must be on the same line as the method name.
So I rolled your patch in for the next build (I changed the new method to be on TokenVal). I added a new test testCompiler::MiscTest.testCallParens and updated the docLang::Statements chapter.
Congratulations and thank you - yours is the first patch ever for Fan!
On a side note - I'm assuming you must have setup a boot build environment to hack the compiler. How was your experience getting it setup and figuring your way around the compiler?
tompalmerSat 21 Jun 2008
Glad to hear it worked out (even if it required more work on your end still). As for setting up the environment, I've found it awkward. It seems like it should be possible simply to use a prebuilt compiler from one place to build a new version of the compiler from somewhere else, without any configuration between the two. I haven't dug into the reason behind the current requirements.
As for the code itself, what I've read I find clean and clear.
I have to admit missing Eclipse features. I feel blind without "find references" and "call hierarchy" these days. (And as a side note on that, it really should be just one feature in Eclipse: "reference hierarchy". Reminds me of how people throw tagging onto existing systems with complicated folders and ontologies and expect it to just work. Sometimes features need removed and/or merged.)
brianSun 6 Jul 2008
I think we should enforce the same requirement for the opening "[" in a get/set/slice operation that it must be on the same line as the target expression.
doSomething
[1, 2, 3].each |Int i| { echo(i) }
tompalmerMon 7 Jul 2008
Agreed. I've thought about it some but hadn't investigated.
brianWed 9 Jul 2008
I think we should enforce the same requirement for the opening "[" in a get/set/slice operation that it must be on the same line as the target expression.
I've made this change.
brianFri 10 Apr 2009
I think we need to apply this same technique to return statements. Currently there is an ambiguity if not using semicolons whether to parse a return expression or not. I handle this today because I keep track of whether I am in a void method or not.
But if we do closure inference that technique won't work anymore - I won't know whether a return value was expected or not until well after parse time.
So the proposal is that a return expression must start on the same line as the return keyword:
// ok
return x
return call(
x,
y)
// not ok
return
call(x, y)
tompalmerFri 10 Apr 2009
I like the proposal.
Personally, I like not ever wrapping except when explicitly allowed. That is, default deny. One example needed for Fan currently is the { opening blocks (because of your preferred conventions), but that's okay if expected. I know better than to start that topic here.
I think mid-expression is also okay. I hate having to say a + \ in Python. Uh, of course I didn't finish my statement. But any time you can safely end a statement should always count as the end of the statement, except for prescribed (and hopefully intuitive) exceptions.
tompalmer Fri 20 Jun 2008
In testing around with Fan, I ran across a case looking something like this:
Seems safe enough, but I get these errors:
Seems like Fan is subject to the same kinds of woes faced by most
;
-doesn't-matter languages. I think it's safe to say that this isn't expected behavior. Personally, I like Python's consider-newline-the-end-if-at-all-possible mentality. Well, maybe it goes a bit overboard, but I think erring on that side is safer than defaulting to trying to continue to the next line. Note also that I prefer{
on the end, rather than on the next line. (And as mentioned before, Ruby-styleend
works around this issue somewhat, too.)But it seems Brian and Andy are next-line folks, and I don't want to go down that venerable subject line, it seems not safe to require
{
on the end, and so we need to look at the next line at least some when parsing statements.So, as a workaround for now, I at least made
(
require being on the same line. Seems to work fine. I only addressed normal calls, not the keywords (though that leaves things inconsistent). I'm not sure if any changes are acceptable in this arena at all, but here's my diff of Parser.fan:That's based on the 1.0.27 download.
Again, no complaints if this goes unused (or if I haven't learned the right way to hack the compiler yet). Just felt like mentioning the mini adventure in case it's of value.
andy Fri 20 Jun 2008
That looks like a bug in the compiler - Brian can comment.
cbeust Fri 20 Jun 2008
Thanks, I was scratching my head over that one, trying to find what this "obvious" error was (Tom, you never shared your interpretation of the compiler error...).
-- Cedric
tompalmer Fri 20 Jun 2008
To clarify, here's an equivalent example from ECMAScript:
That
window (1 + 1)
is seen as being a function call, and it causes a similar error to what I described above. They want to fix this in ES4 but are afraid of backwards compatibility (last I was following the conversation).In both cases adding a semicolon fixes things, for example
name := "Tom";
. My Parser.fan patch fixes it so the;
isn't needed.I think the second error from the Fan compiler is probably a compiler bug. I didn't address that. I only addressed the "args on next line" case.
andy Fri 20 Jun 2008
Requiring the opening paren for a method to be on the same line seems reasonable to me, though I already always use that convention, so thats a bit biased.
The second error is correct - if (1..10) is interpreted as a method call on "Tom" - then
name
is not defined yet.tompalmer Fri 20 Jun 2008
OK. I get the "$name" issue now.
brian Sat 21 Jun 2008
Tom,
You are spot on, one of the main problems with the grammar not using ; is when the following line starts with an open paren.
I think your suggestion is pretty sweet myself. Most all of the existing Fan codebase compiled OK with it - all of Andy's and my code. A couple lines written by a third guy used syntax like this:
But my personal opinion is that one way or the other you are going to get a weird compiler error. So I'm going to say from now on that the open paren of a method call must be on the same line as the method name.
So I rolled your patch in for the next build (I changed the new method to be on TokenVal). I added a new test
testCompiler::MiscTest.testCallParens
and updated the docLang::Statements chapter.Congratulations and thank you - yours is the first patch ever for Fan!
On a side note - I'm assuming you must have setup a boot build environment to hack the compiler. How was your experience getting it setup and figuring your way around the compiler?
tompalmer Sat 21 Jun 2008
Glad to hear it worked out (even if it required more work on your end still). As for setting up the environment, I've found it awkward. It seems like it should be possible simply to use a prebuilt compiler from one place to build a new version of the compiler from somewhere else, without any configuration between the two. I haven't dug into the reason behind the current requirements.
As for the code itself, what I've read I find clean and clear.
I have to admit missing Eclipse features. I feel blind without "find references" and "call hierarchy" these days. (And as a side note on that, it really should be just one feature in Eclipse: "reference hierarchy". Reminds me of how people throw tagging onto existing systems with complicated folders and ontologies and expect it to just work. Sometimes features need removed and/or merged.)
brian Sun 6 Jul 2008
I think we should enforce the same requirement for the opening "[" in a get/set/slice operation that it must be on the same line as the target expression.
tompalmer Mon 7 Jul 2008
Agreed. I've thought about it some but hadn't investigated.
brian Wed 9 Jul 2008
I've made this change.
brian Fri 10 Apr 2009
I think we need to apply this same technique to return statements. Currently there is an ambiguity if not using semicolons whether to parse a return expression or not. I handle this today because I keep track of whether I am in a void method or not.
But if we do closure inference that technique won't work anymore - I won't know whether a return value was expected or not until well after parse time.
So the proposal is that a return expression must start on the same line as the return keyword:
tompalmer Fri 10 Apr 2009
I like the proposal.
Personally, I like not ever wrapping except when explicitly allowed. That is, default deny. One example needed for Fan currently is the
{
opening blocks (because of your preferred conventions), but that's okay if expected. I know better than to start that topic here.I think mid-expression is also okay. I hate having to say
a + \
in Python. Uh, of course I didn't finish my statement. But any time you can safely end a statement should always count as the end of the statement, except for prescribed (and hopefully intuitive) exceptions.