From: fche@redhat.com (Frank Ch. Eigler)
To: rarora@redhat.com
Cc: systemtap@sources.redhat.com
Subject: Re: BZ 6701 - Improve error messages (patch)
Date: Wed, 01 Oct 2008 01:23:00 -0000 [thread overview]
Message-ID: <y0mk5ct5cui.fsf@ton.toronto.redhat.com> (raw)
In-Reply-To: <48E29E22.4070708@redhat.com> (Rajan Arora's message of "Tue, 30 Sep 2008 17:46:10 -0400")
Hi, Rajan -
> This is a proposed patch for the Bugzilla bug 6701 [...]
Thanks, nice work!
> # stap -ve 'probe begin { log("hello world") exit () }'
> Pass 1: parsed user script and 45 library script(s) in 150usr/10sys/172real ms.
> semantic error: unresolved arity-0 function: identifier 'exxit' at <input>:1:34
> source: probe begin { log("hello world") exxit () }
> ^
Please check the rendering heuristics for the case of the inputs
containing tabs and for lines perhaps too long to draw in their
entirety.
> + //Do file_contents exist? If not, error is in tapset!
> + if (e.tok1 && user_file->file_contents.size() != 0)
> + {
> + errormsg.str ("");
> + print_error_source (errormsg, user_file->file_contents, e.tok1);
> + cerr << errormsg.str();
> + }
For this and similar cases, is there some reason for the errormsg
object? Would this not work just as well?
print_error_source (cerr, user_file->file_contents, e.tok1);
> +systemtap_session::print_error_source (std::stringstream& message,
> + std::string& file_contents, const token* tok)
> +{
> [...]
> + message << "\tsource: " << file_contents.substr (start_pos, end_pos-start_pos-1) << endl;
> + message << "\t ";
> + message.fill (' ');
> + message.width (col);
> + message << "^" << endl;
Yeah, this won't work right if the input contains tabs. Instead of
the .fill()/.width() way of lining up with the input, you could try
looping over the contents of file_contents[start_pos .. end_pos]. You
could copy each isspace(char) and emit a ' ' otherwise.
> +void
> +lexer::get_input_contents (std::string& file_contents)
> +{
> + unsigned i;
> + for (i=0; i<input_contents.size(); i++)
> + file_contents += input_contents[i];
> +}
Could you look into making that input_contents[] widget into a plain
std::string? That should turn this into a simple assignment ...
or rather a "return input_contents;" - the function might as well
return a std::string instead of a value by reference.
> int
> lexer::input_peek (unsigned n)
> [...]
> + //Assuming tapsets are error-free, only user's script contents are fetched
> + if (!strstr (input_name.c_str(),"tapset/"))
> + input.get_input_contents (f->file_contents);
Nothing's error free :-) ... except my wife's mother. Drop this
assumption - we can have semantic errors that occur with e.g. $context
variables or probe points that exist only on a ragtag collection of
kernel versions.
> +++ b/testsuite/parseko/source_context.stp
> @@ -0,0 +1,11 @@
> +global count_test
> +probe timer.ms(123)
> +{
> +printf("%d loops have executed\n",count_test)
> +count_test ++
> +iffff(count_test == 5)
> +{
> +priiintf("Done execution 5 times and about to exit. . .\n")
> + eeexit ()
> +}
> +}
OK (though one error per file might be more assertive).
- FChE
next prev parent reply other threads:[~2008-10-01 1:23 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-09-30 21:46 Rajan Arora
2008-10-01 1:23 ` Frank Ch. Eigler [this message]
2008-10-01 14:01 ` Frank Ch. Eigler
2008-10-02 14:39 ` BZ 6701 - Improve error messages (updated-patch) Rajan Arora
2008-10-02 16:47 ` Frank Ch. Eigler
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=y0mk5ct5cui.fsf@ton.toronto.redhat.com \
--to=fche@redhat.com \
--cc=rarora@redhat.com \
--cc=systemtap@sources.redhat.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).