gracelang / minigrace

Self-hosting compiler for the Grace programming language
39 stars 22 forks source link

Layout not enforced for if(_)then_()else(_) #221

Closed apblack closed 7 years ago

apblack commented 8 years ago

This compiles

if {funk>7} then {
    print "Good"
} 
else {
    print "not so good"
}

whereas the compiler ought to complain that there is no method else.

KimBruce commented 8 years ago

Do you really want to ban this? There are a variety of understandable ways to indent if-then-else:

if (true) then {
    print "hello"
}
else
{
    print "goodbye"
}

or

if (true) 
    then {print "hello"}
    else {print "goodbye"}

or

if (true) 
then {print "hello"}
else {print "goodbye"}

and finally, my favorite:

if (true) then {
    print "hello"
} else {
    print "goodbye"
}

All of these currently work. While the next to last perhaps shouldn't work, I don't see the problem with the others. In particular, the last example is what I use rather consistently. However, I understand the above might violate our currently written rules. Can they be rewritten (gracefully) to accommodate these?

kjx commented 8 years ago

If this is one request

On 11/10/2016, at 19:32PM, Kim Bruce notifications@github.com wrote:

if (true) then {
print "hello"
}
else
{
print "goodbye"
}

(where "if" and "else" are indented the same), then isn't this one request too?

print "foo"
print "bar" 

ditto for

if (true) 
then {print "hello"}
else {print "goodbye"}

J

apblack commented 7 years ago

This is fixed in commit 826f1578. By "fixed" I mean that @KimBruce's examples 1 and 3 now generate static errors, while examples 2 and 4 continue to work.

Because the minigrace parser deals with if(_)then(_)else(_) as a special case, the error message is also a special case, and says that the then part of an if(_)then(_)… must be indented more than the if.

The general case of the body of a block needing to be indented was also not being caught, as I discovered when I lectured to my students about the importance of indentation. Since commit 0b6e223e40, this case is also reported as a syntax error.