search

metaeducation / ren-c

Library for embedding a Rebol interpreter into C codebases
GNU Lesser General Public License v3.0
128 stars 27 forks source link

off-by-one error in `help` #1113

Closed MarkoSchuetz closed 3 years ago

MarkoSchuetz commented 3 years ago

The descriptions of the refinements seem to be off by one, for example

>> help call*
USAGE:
    CALL* command /wait /console /shell /info /input /output /error

DESCRIPTION:
    Run another program by spawning a new process
    CALL* is an ACTION!

RETURNS: [text! block! file!]
    OS-local command line, block with arguments, executable file

ARGUMENTS:
    command
        Wait for command to terminate before returning

REFINEMENTS:
    /wait
        Runs command with I/O redirected to console
    /console
        Forces command to be run from shell
    /shell
        Returns process information object
    /info [text! binary! file! logic!]
        Redirects stdin (false=/dev/null, true=inherit)
    /input [text! binary! file! logic!]
        Redirects stdout (false=/dev/null, true=inherit)
    /output [text! binary! file! logic!]
        Redirects stderr (false=/dev/null, true=inherit)
    /error
giuliolunati commented 3 years ago

The bug isn't in HELP source: ?? (meta-of :call*)/parameter-notes also has off-by-one error

giuliolunati commented 3 years ago

First bad commit: c0408f94 (Restore RETURN type checking functionality)

hostilefork commented 3 years ago

Thanks for finding the commit...!

The issue affects any function that doesn't have a RETURN: in its spec. Most natives do, but that CALL* was an alias for a native that did not.

>> x: func [a {alpha} b {beta}] [return a]
== #[action! [a b]]

>> help x
USAGE:
    X a b

DESCRIPTION:
    (undocumented)
    X is an ACTION!

RETURNS: (undocumented)
    alpha

ARGUMENTS:
    a
        beta
    b

What's a headache and holding this up is that I need to pin down a bit more about frame mechanics when it comes to the oddities of RETURN and multi-return.

I was in the middle of trying to do a generalized fix, so I didn't put my attention on a patch just for this issue. However that generalized fix wound up on hold and I didn't come back to this.

I'm actually back to looking at related issues so I'll raise the priority on it.

hostilefork commented 3 years ago

Sorry this took so long to get around to fixing. I should have just patched it instead of trying to finish the rewrite I was doing...

https://github.com/metaeducation/ren-c/commit/adb36325f92ead632f8b7d8f6eaa93f73efaece7

As part of the commit I added some primordial tests of the kind that I think are needed if people are going to be concerned with HELP staying in working order. But there's still a lot of nuts & bolts that have to be tended to in terms of type checking...what's in this auxiliary information and what its invariants are. So worrying about "help inheritance" is a whole new angle of problem, especially when it's typical that you want to override it. Might seem easy, but it's non-trivial stuff!