All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jilles Tjoelker <jilles@stack.nl>
To: Harald van Dijk <harald@gigawatt.nl>
Cc: dash@vger.kernel.org
Subject: Re: [PATCH] Improved LINENO support
Date: Wed, 10 Nov 2010 23:00:08 +0100	[thread overview]
Message-ID: <20101110220008.GC17871@stack.nl> (raw)
In-Reply-To: <4CD85E90.6000308@gigawatt.nl>

On Mon, Nov 08, 2010 at 09:32:56PM +0059, Harald van Dijk wrote:
> This patch improves LINENO support by storing line numbers in the parse 
> tree, for commands as well as for function definitions. It makes LINENO 
> behaves properly when calling functions, and has the added benefit of 
> improved line numbers in error messages when the last-parsed command is 
> not the last-executed one. It removes the earlier LINENO support, and 
> instead sets LINENO from evaltree when a command is executed. I have 
> used it for a while, and have seen correct line numbers in basic testing 
> and in error messages in fairly large and complicated autoconf-generated 
> shell scripts. Does this look good enough to add to dash? If there are 
> any problems with it, please let me know, I will be happy to look at 
> this further.

What I was thinking of was adding a lineno field to narg instead of to
all command types. The lineno variable would then be set by expand.c. I
think that leads to a smaller patch and it should still give a sensible
value for almost all errors. Downside is a few more int-to-ascii
conversions. A few divisions is not that bad relative to other things
that are done but printf in modern libcs is bloated and slow and might
slow down things measurably.

FreeBSD's code subtracts the function's starting line number in the
parser rather than at run time. I wonder which is best. (FreeBSD
neglects to save and restore the value, so nested function definitions
may lead to incorrect line numbers, but that could be fixed.)

I think this patch will increase dash's code size at least a little,
which is always a consideration here. I can also look at getting this
into FreeBSD sh.

Thanks,
-- 
Jilles Tjoelker

  reply	other threads:[~2010-11-10 22:00 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-11-08 20:33 [PATCH] Improved LINENO support Harald van Dijk
2010-11-10 22:00 ` Jilles Tjoelker [this message]
2010-11-11 20:33   ` Harald van Dijk
2010-11-12 18:17   ` Harald van Dijk
2010-11-12 18:35     ` Eric Blake
2010-11-12 19:08       ` Harald van Dijk
2010-11-12 19:40         ` Harald van Dijk
2010-11-12 21:29         ` Jilles Tjoelker
2010-11-27 16:56           ` Harald van Dijk
2011-03-10  8:10             ` Herbert Xu
2011-03-11 21:56               ` Harald van Dijk
2011-03-15  7:52                 ` Herbert Xu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20101110220008.GC17871@stack.nl \
    --to=jilles@stack.nl \
    --cc=dash@vger.kernel.org \
    --cc=harald@gigawatt.nl \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.