[Bug translator/5679] New: inline documentation for scripts/tapsets

[Bug translator/5679] New: inline documentation for scripts/tapsets

[fche at redhat dot com]

We need a way to embed documentation inside scripts and especially tapsets,
to permit:
- man pages to be generated
- more helpful error messages (typo correction suggestions)

One possible syntax was outlined at the bottom of this message:
http://sourceware.org/ml/systemtap/2007-q3/msg00466.html

The implementation could result in adding a documentation-string
field to certain staptree.h objects (perhaps just classes probe,
functiondecl, vardecl).

Bug #2949 (probe listing command line option) should use the
results of this work.  Other probe types (e.g. dwarf, markers)
can synthesize/extend documentation strings from data they pull
out of debuginfo/etc. files.

-- 
           Summary: inline documentation for scripts/tapsets
           Product: systemtap
           Version: unspecified
            Status: NEW
          Severity: normal
          Priority: P2
         Component: translator
        AssignedTo: systemtap at sources dot redhat dot com
        ReportedBy: fche at redhat dot com
OtherBugsDependingO 2949
             nThis:


http://sourceware.org/bugzilla/show_bug.cgi?id=5679

------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

[Bug translator/5679] inline documentation for scripts/tapsets

[fche at redhat dot com]

------- Additional Comments From fche at redhat dot com  2008-02-05 14:42 -------
I have a patch that supports a syntax like this:

probe foo   @@ "this documentats `foo'" 
            @@ "and so does this"  {
   var = $expr   @@ "this documents `var'"
}


-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|systemtap at sources dot    |fche at redhat dot com
                   |redhat dot com              |
             Status|NEW                         |ASSIGNED


http://sourceware.org/bugzilla/show_bug.cgi?id=5679

------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

Re: [Bug translator/5679] inline documentation for scripts/tapsets

[Frank Ch. Eigler]

Hi -

mhiramat wrote:

> I have a question about this syntax.
> 
> What happened in following case?
> if (cond) {
>    var = $var1 @@ "this is var1"
> } else {
>    var = $var2 @@ "this is var2"
> }

Good question.  The "@@" binds at elaboration time (i.e., not at run
time, so ordinary conditions cannot be evaluated, but preprocessor
conditionals are).  Multiple "@@" operators on the same variable are
additive.  So this would result in "var" being documented with both
strings.

> AFAIK, some probe alias (ex. syscall.fork) set variables like as
> above...

Indeed.  But remember, we want to document the purpose of the variable
(rather than its actual value on one actual control flow branch, which
is in any case invisible to the user), so one string is enough:

   if (cond) {  name = "foo"   @@ "System call name."  }
   else { name = "bar" }



By the way, here are some excerpts from what a converted tapset may
look like.  It combines information currently in the man pages and in
previous comment blocks, so is best compared to a union of the tapset
source code AND the man page.


----------------------------------------
// IO scheduler tapset
// [...]
probe ioscheduler.elv_next_request
	=  kernel.function("elv_next_request")
        @@ "Probe the retrieval of a I/O request from request queue."
{
        elevator_name =
        %( kernel_v >= "2.6.10" %?
           kernel_string($q->elevator->elevator_type->elevator_name) %:
           kernel_string($q->elevator->elevator_name) %)
        @@ "The elevator name."
}

probe ioscheduler.elv_add_request
	= kernel.function("__elv_add_request")
        @@ "Probe adding a request into the I/O request queue."
{
%( kernel_v >= "2.6.10" %?
	elevator_name = kernel_string($q->elevator->elevator_type->elevator_name)
%:
	elevator_name = kernel_string($q->elevator->elevator_name)
%)      @@ "The elevator name."

	if($rq == 0)  {
		disk_major = -1   @@ "Disk major number of the request."
		disk_minor = -1   @@ "Disk minor number of the request."
	} else {
		if($rq->rq_disk == 0)  {
			disk_major = -1
			disk_minor = -1
		} else {
			disk_major = $rq->rq_disk->major
			disk_minor = $rq->rq_disk->first_minor
		}
	}

	req = $rq   @@ "Address of the request."
%( kernel_v >= "2.6.19" %?
	req_flags = $rq==0? 0:$rq->cmd_flags
%:
	req_flags = $rq==0? 0:$rq->flags
%)      @@ "Request flags."
}


----------------------------------------
// context tapset
// [...]

function print_regs
    @@ "Print a register dump (for kprobes, uprobes, profiling)."
    () 
%{
    	if (CONTEXT->regs) {
	   	_stp_print_regs (CONTEXT->regs);
	}
%}

function print_backtrace
    @@ "Equivalent to print_stack(backtrace()), except that"
    @@ "deeper stack nesting may be supported."
    () 
%{
	if (CONTEXT->regs) {
		_stp_stack_print(CONTEXT->regs, 1, CONTEXT->pi);
	} else {
		_stp_printf("Systemtap probe: %s\n", CONTEXT->probe_point);
	}
%}

----------------------------------------
// queue_stats.stp

function qsq_utilization
  @@ "Return the fraction of time that any request was being serviced."
  (qname @@ "Queue name", scale @@ "Time scaling factor") {
  elapsed = qs_time() - qs_stime[qname]
  return (scale * qs_rtime[qname]) / elapsed
}

Re: [Bug translator/5679] inline documentation for scripts/tapsets

[David Smith]

Frank Ch. Eigler wrote:
> By the way, here are some excerpts from what a converted tapset may
> look like.  It combines information currently in the man pages and in
> previous comment blocks, so is best compared to a union of the tapset
> source code AND the man page.
> 
> 
> ----------------------------------------
> // IO scheduler tapset
> // [...]
> probe ioscheduler.elv_next_request
> 	=  kernel.function("elv_next_request")
>         @@ "Probe the retrieval of a I/O request from request queue."
> {
>         elevator_name =
>         %( kernel_v >= "2.6.10" %?
>            kernel_string($q->elevator->elevator_type->elevator_name) %:
>            kernel_string($q->elevator->elevator_name) %)
>         @@ "The elevator name."
> }

Hmm.  That looks a bit intrusive to me, at least on a first glance,
especially for the variable documentation.  I believe you mentioned that
you had found some inspiration in emacs lisp's documentation feature.
In emacs, you only document the function itself, not variables within it
(as far as I can remember).

When would you see a user needing the documentation for 'elevator_name'?
  How would he get it?  Why is this documentation better than just a
comment?

An unrelated question - we currently merge two probes that have the same
body.  If we had 2 probes that were the same, except for their
documentation, are they still identical?

-- 
David Smith
dsmith@redhat.com
Red Hat
http://www.redhat.com
256.217.0141 (direct)
256.837.0057 (fax)

Re: [Bug translator/5679] inline documentation for scripts/tapsets

[Frank Ch. Eigler]

Hi -

dsmith wrote:
> [...]
> > probe ioscheduler.elv_next_request
> > 	=  kernel.function("elv_next_request")
> >         @@ "Probe the retrieval of a I/O request from request queue."
> > {
> >         elevator_name =
> >         %( kernel_v >= "2.6.10" %?
> >            kernel_string($q->elevator->elevator_type->elevator_name) %:
> >            kernel_string($q->elevator->elevator_name) %)
> >         @@ "The elevator name."
> > }
> 
> Hmm.  That looks a bit intrusive to me, at least on a first glance,
> especially for the variable documentation.  

Yeah, it's a bit odd, but consider the alternatives: separately
written man pages and/or javadoc-style comment markup.


> I believe you mentioned that you had found some inspiration in emacs
> lisp's documentation feature.  In emacs, you only document the
> function itself, not variables within it (as far as I can remember).

Yes, but emacs is different in this regard.  Here, the variables
provided by aliases are first class interfaces being provided to
end-user scripts.


> When would you see a user needing the documentation for
> 'elevator_name'?

When she finds a probe on the topic of i/o schedulers, she'll want to
know which variables the alias makes available, so that she doesn't
have to use $expressions.

> How would he get it?

Oh, it's a "he" now?  Oh well, that happens too.  He could get
it with a verbose listing, a la:

  % stap -lv 'ioschedule.el*'

> Why is this documentation better than just a comment?

It's not a huge difference, but it could reduce the necessity for
someone to pop open a tapset script.


> An unrelated question - we currently merge two probes that have the
> same body.  If we had 2 probes that were the same, except for their
> documentation, are they still identical?

Documentation strings are not propagated into the generated code, so
yeah, they should merge the same way with the existing algorithms.

- FChE

Re: [Bug translator/5679] inline documentation for scripts/tapsets

[David Smith]

Frank Ch. Eigler wrote:
> Hi -
> 
> dsmith wrote:
>> [...]
>>> probe ioscheduler.elv_next_request
>>> 	=  kernel.function("elv_next_request")
>>>         @@ "Probe the retrieval of a I/O request from request queue."
>>> {
>>>         elevator_name =
>>>         %( kernel_v >= "2.6.10" %?
>>>            kernel_string($q->elevator->elevator_type->elevator_name) %:
>>>            kernel_string($q->elevator->elevator_name) %)
>>>         @@ "The elevator name."
>>> }
>> Hmm.  That looks a bit intrusive to me, at least on a first glance,
>> especially for the variable documentation.  
> 
> Yeah, it's a bit odd, but consider the alternatives: separately
> written man pages and/or javadoc-style comment markup.

...

>> When would you see a user needing the documentation for
>> 'elevator_name'?
> 
> When she finds a probe on the topic of i/o schedulers, she'll want to
> know which variables the alias makes available, so that she doesn't
> have to use $expressions.

Why couldn't the probe's documentation string list the available
variables?  Taking the example from above:

probe ioscheduler.elv_next_request
	=  kernel.function("elv_next_request")
        @@ "Probe the retrieval of a I/O request from request queue."
        @@ "Provides:"
        @@ "  elevator_name: the elevator name"
{
        elevator_name =
        %( kernel_v >= "2.6.10" %?
           kernel_string($q->elevator->elevator_type->elevator_name) %:
           kernel_string($q->elevator->elevator_name) %)
}

-- 
David Smith
dsmith@redhat.com
Red Hat
http://www.redhat.com
256.217.0141 (direct)
256.837.0057 (fax)

Re: [Bug translator/5679] inline documentation for scripts/tapsets

[Frank Ch. Eigler]

Hi -

dsmith wrote:
> [...]
> > Yeah, it's a bit odd, but consider the alternatives: separately
> > written man pages and/or javadoc-style comment markup.
> [...]
> Why couldn't the probe's documentation string list the available
> variables?  Taking the example from above:
> 
> probe ioscheduler.elv_next_request
> 	=  kernel.function("elv_next_request")
>         @@ "Probe the retrieval of a I/O request from request queue."
>         @@ "Provides:"
>         @@ "  elevator_name: the elevator name"
> {
>         elevator_name =
>         %( kernel_v >= "2.6.10" %?
>            kernel_string($q->elevator->elevator_type->elevator_name) %:
>            kernel_string($q->elevator->elevator_name) %)
> }

Perhaps.  One problem here is that then it's heard to create a
non-verbose listing (just to show the probe "headline"), or else the
text would be structured and would have to be internally parsed.
Another problem: it duplicates and/or makes it harder to mechanically
enforce "documentation coverage".

- FChE

[Bug translator/5679] inline documentation for scripts/tapsets

[fche at redhat dot com]

------- Additional Comments From fche at redhat dot com  2008-06-27 16:19 -------
Handing this documentation back to the pool.
Perhaps we'll end up with javadoc or somesuch; now that
listings mode exists, this seems not so urgent.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|fche at redhat dot com      |systemtap at sources dot
                   |                            |redhat dot com


http://sourceware.org/bugzilla/show_bug.cgi?id=5679

------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

[Bug translator/5679] inline documentation for scripts/tapsets

[fche at redhat dot com]

------- Additional Comments From fche at redhat dot com  2009-09-13 12:47 -------
wcohen's kerneldoc-based infrastructure has been in for a while.
We still need to complete the markup in the tapsets.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED


http://sourceware.org/bugzilla/show_bug.cgi?id=5679

------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.