~technomancy/fennel

This thread contains a patchset. You're looking at the original emails, but you may wish to use the patch review UI. Review patch
3 2

[PATCH] Enable lua style method calls

Details
Message ID
<20190830202640.2530-1-benaiah@mischenko.com>
DKIM signature
missing
Download raw message
Patch: +60 -5
(foo:bar baz) is now syntax sugar for (: foo :bar baz)
---
 fennel.lua   | 45 ++++++++++++++++++++++++++++++++++++++++-----
 reference.md | 10 ++++++++++
 test.lua     | 10 ++++++++++
 3 files changed, 60 insertions(+), 5 deletions(-)

diff --git a/fennel.lua b/fennel.lua
index f68a38e..6b40634 100644
--- a/fennel.lua
+++ b/fennel.lua
@@ -496,12 +496,28 @@ local function isMultiSym(str)
        return isMultiSym(tostring(str))
    end
    if type(str) ~= 'string' then return end
    if str:match(':%.') or str:match('%.:') or str:match('::') then
        error("malformed multisym")
    end
    local parts = {}
    for part in str:gmatch('[^%.]+') do
        parts[#parts + 1] = part
    local foundColon = false
    for part in str:gmatch('[^%.%:]+[%.%:]?') do
        local lastChar = part:sub(-1)
        if (lastChar == "." or lastChar == ":") and foundColon then
            error("method call must be last component of multisym")
        end
        if lastChar == ":" then
            foundColon = true
            parts.multiSymMethodCall = true
        end
        if lastChar == ":" or lastChar == "." then
            parts[#parts + 1] = part:sub(1, -2)
        else
            parts[#parts + 1] = part
        end
    end
    return #parts > 0 and
        str:match('%.') and
        (str:match('%.') or str:match(':')) and
        (not str:match('%.%.')) and
        str:byte() ~= string.byte '.' and
        str:byte(-1) ~= string.byte '.' and
@@ -571,7 +587,11 @@ local function combineParts(parts, scope)
    local ret = scope.manglings[parts[1]] or globalMangling(parts[1])
    for i = 2, #parts do
        if isValidLuaIdentifier(parts[i]) then
            ret = ret .. '.' .. parts[i]
            if parts.multiSymMethodCall and i == #parts then
                ret = ret .. ':' .. parts[i]
            else
                ret = ret .. '.' .. parts[i]
            end
        else
            ret = ret .. '[' .. serializeString(parts[i]) .. ']'
        end
@@ -640,7 +660,10 @@ local function symbolToExpression(symbol, scope, isReference)
    local name = symbol[1]
    if scope.hashfn and name == '$' then name = '$1' end
    local parts = isMultiSym(name) or {name}
    local etype = (#parts > 1) and "expression" or "sym"
    if parts.multiSymMethodCall then
        error("multisym method calls may only be in call position")
    end
    local etype = (#parts > 1) and 'expression' or "sym"
    local isLocal = scope.manglings[parts[1]]
    if isLocal and scope.symmeta[name] then scope.symmeta[name].used = true end
    -- if it's a reference and not a symbol which introduces a new binding
@@ -866,6 +889,7 @@ local function compile1(ast, scope, parent, opts)
        if isSym(first) then -- Resolve symbol
            first = first[1]
        end
        local multiSymParts = isMultiSym(first)
        local special = scope.specials[first]
        if special and isSym(ast[1]) then
            -- Special form
@@ -883,6 +907,17 @@ local function compile1(ast, scope, parent, opts)
            end
            exprs.returned = true
            return exprs
        elseif multiSymParts and multiSymParts.multiSymMethodCall then
            local tableWithMethod = table.concat({
                    unpack(multiSymParts, 1, #multiSymParts - 1)
                                                 }, '.')
            local methodToCall = multiSymParts[#multiSymParts]
            local newAST = list(sym(':', scope), sym(tableWithMethod, scope), methodToCall)
            for i = 2, len do
                newAST[#newAST + 1] = ast[i]
            end
            local compiled = compile1(newAST, scope, parent, opts)
            exprs = compiled
        else
            -- Function call
            local fargs = {}
diff --git a/reference.md b/reference.md
index b9897cf..7284120 100644
--- a/reference.md
+++ b/reference.md
@@ -403,6 +403,16 @@ Equivalent to:
  (f.close f))
```

If you know the method you wish to call at compile time, you can also
use this equivalent form:

```
(let [f (assert (io.open "hello" "w"))]
  (f:write "world")
  (f:close))
`
```

### `values` multi-valued return

Returns multiple values from a function. Usually used to signal
diff --git a/test.lua b/test.lua
index 2c627c3..e20ba2d 100644
--- a/test.lua
+++ b/test.lua
@@ -304,6 +304,12 @@ local cases = {
        -- Mixed $ types
        ["(let [f #(+ $ $1 $2)] (f 1 2))"]=4,
    },
    methodcalls = {
        -- multisym method call
        ["(let [x {:foo (fn [self arg1] (.. self.bar arg1)) :bar :baz}] (x:foo :quux))"]="bazquux",
        -- multisym method call on property
        ["(let [x {:y {:foo (fn [self arg1] (.. self.bar arg1)) :bar :baz}}] (x.y:foo :quux))"]="bazquux",
    },
    match = {
        -- basic literal
        ["(match (+ 1 6) 7 8)"]=8,
@@ -504,10 +510,14 @@ local compile_failures = {
    ["(global 48 :forty-eight)"]="unable to bind 48",
    ["(let [t []] (set t.47 :forty-seven))"]=
        "can't start multisym segment with digit: t.47",
    ["(let [t []] (set t.:x :y))"]="malformed multisym",
    ["(let [t []] (set t:.x :y))"]="malformed multisym",
    ["(let [t []] (set t::x :y))"]="malformed multisym",
    -- other
    ["(match [1 2 3] [a & b c] nil)"]="rest argument in final position",
    ["(x(y))"]="expected whitespace before opening delimiter %(",
    ["(x[1 2])"]="expected whitespace before opening delimiter %[",
    ["(let [x {:foo (fn [self] self.bar) :bar :baz}] x:foo)"]="multisym method calls may only be in call position",
}

print("Running tests for compile errors...")
-- 
2.23.0.rc1
Details
Message ID
<87mufpkt9o.fsf@hagelb.org>
In-Reply-To
<20190830202640.2530-1-benaiah@mischenko.com> (view parent)
DKIM signature
missing
Download raw message
Benaiah Mischenko <benaiah@mischenko.com> writes:

> +    if str:match(':%.') or str:match('%.:') or str:match('::') then
> +        error("malformed multisym")
> +    end

I think the rules about what constitutes a valid symbol or multisymbol
should be enforced during parsing rather than in the ismultisym
function; that way the errors will have line numbers. You can see how I
enforced multisym rules here:

    https://github.com/bakpakin/Fennel/commit/2f43f19797838772061e3a0e2563b51269189f2b

In general, using `error' or `assert' directly in the compiler is best
avoided; instead either use `parseError' or `assertCompile' which will
handle the reporting better. There's a few other calls in this patch that
should also be converted.

> +If you know the method you wish to call at compile time, you can also
> +use this equivalent form:
> +
> +```
> +(let [f (assert (io.open "hello" "w"))]
> +  (f:write "world")
> +  (f:close))
> +`
> +```

I think in the reference docs it would be better to introduce the new
style first, and then introduce the `:' special form as a fallback for
situations where the method name isn't known at compile time.

But those are pretty minor issues; overall this is a great patch. I
think this new style is closer to what folks would intuitively expect
from Fennel if they already know Lua. Nice work!

-Phil
Details
Message ID
<87blw3h7o9.fsf@mischenko.com>
In-Reply-To
<87mufpkt9o.fsf@hagelb.org> (view parent)
DKIM signature
missing
Download raw message
I've attached the patch instead of using git send-email; maybe it will
thread better?

Phil Hagelberg <phil@hagelb.org> writes:

> I think the rules about what constitutes a valid symbol or multisymbol
> should be enforced during parsing rather than in the ismultisym
> function; that way the errors will have line numbers. You can see how I
> enforced multisym rules here:
>
>     https://github.com/bakpakin/Fennel/commit/2f43f19797838772061e3a0e2563b51269189f2b
>
> In general, using `error' or `assert' directly in the compiler is best
> avoided; instead either use `parseError' or `assertCompile' which will
> handle the reporting better. There's a few other calls in this patch that
> should also be converted.

Ok, I've changed the error calls to parseError and assertCompile calls
and moved them to the parser and compile1 functions instead of the
helpers.

> I think in the reference docs it would be better to introduce the new
> style first, and then introduce the `:' special form as a fallback for
> situations where the method name isn't known at compile time.

Roger - I've switched around the order so the lua-style call is
explained first.

> But those are pretty minor issues; overall this is a great patch. I
> think this new style is closer to what folks would intuitively expect
> from Fennel if they already know Lua. Nice work!

Thanks! I think it'll make Lua library tutorials which themselves use
Lua (a.k.a. all of them) a lot easier to follow.

- benaiah
Details
Message ID
<87imqbl5up.fsf@hagelb.org>
In-Reply-To
<87blw3h7o9.fsf@mischenko.com> (view parent)
DKIM signature
missing
Download raw message
Benaiah Mischenko <benaiah@mischenko.com> writes:

> I've attached the patch instead of using git send-email; maybe it will
> thread better?

Thanks; looks great. I've applied and pushed it out.

-Phil