[Laszlo-dev] For Review: Change 20080518-dda-6 Summary: Script compiler line number rework
Donald Anderson
dda at ddanderson.com
Mon May 19 11:42:58 PDT 2008
On May 19, 2008, at 2:19 PM, P T Withington wrote:
> Approved. Comments below:
>
> 1) This looks odd:
>
>> + if (!options.containsKey(TRACK_LINES) &&
>> + !options.containsKey(DISABLE_TRACK_LINES) &&
>>
I'll clean that up to read
if (!options.containsKey(DISABLE_TRACK_LINES) &&
options.getBoolean(NAME_FUNCTIONS)) {
options.putBoolean(TRACK_LINES, true);
}
It has the same effect, but is clearer.
>>
>
> If not track lines and not not track lines?
>
> 2) I wonder if you would get around your space explosion if you
> interned the file name strings? That would also speed up comparing
> them.
I don't think that helps. We are embedding the strings within longer
and longer strings
as the unparser is making the entire program bottom up.
So:
"i = i + 2;"
might become
"\u0001Finputfile.lzx#234.5\u0001 i = i + 2;"
and then assembled into larger and larger parts of the program. The
embedded
information stays in the string until we're all done and about to emit
it.
Things get even worse with debugging mode when each statement can turn
into multiple function calls. Callexpressions each get a line number
annotation, so
that is part of the explosion.
Another approach is that we could probably prune some annotations as
we go
(in light of the rule that we should not split lines).
In response to your first comment about why lines cannot be split, I
think it
is important to be explicit about never splitting. So I've changed
shouldShowSourceLocation() to this. It's also clearer, I think.
public boolean shouldShowSourceLocation(LineNumberState os,
LineNumberState ns,
char op,
boolean atBol) {
boolean fileSame = os.filename.equals(ns.filename);
boolean lineSame = (os.linediff == ns.linediff);
boolean showSrcloc = false;
// We need to emit at the beginning of the line,
// even if the file has changed. If we break up lines,
// we may alter the meaning of the program.
// 'return foo' can become
// return
// foo
// (two separate statements).
if (atBol) {
// Show source location if we are tracing linenums one
// of our filenames is 'real'.
if (!fileSame && trackLines && (ns.hasfile ||
os.hasfile)) {
showSrcloc = true;
}
// Show source location if we are 'forced' to and have a
name
else if (op == ANNOTATE_OP_FILE_LINENUM_FORCE &&
ns.filename.length() > 0) {
showSrcloc = true;
}
// Otherwise, show it if it has changed.
else if (trackLines && ns.linenum > 0 && (!lineSame || !
fileSame)) {
showSrcloc = true;
}
}
// If debugging, indicate the reasons we are or are not
showing loc
if (DEBUG_LINE_NUMBER && trackLines) {
....
>
>
> On 2008-05-18, at 16:16 EDT, Donald Anderson wrote:
>
>> Change 20080518-dda-6 by dda at lester.local on 2008-05-18 16:02:23 EDT
>> in /Users/dda/laszlo/src/svn/openlaszlo/trunk-c
>> for http://svn.openlaszlo.org/openlaszlo/trunk
>>
>> Summary: Script compiler line number rework
>>
>> New Features:
>>
>> Bugs Fixed: LPP-5784
>>
>> Technical Reviewer: ptw (pending)
>> QA Reviewer: promanik (pending)
>> Doc Reviewer: (pending)
>>
>> Documentation:
>>
>> Release Notes:
>>
>> Details:
>> General:
>> A fair amount of rework of the line numbers. We needed an
>> integrated
>> approach so that internally at least, /* file: name#xx.yy */
>> wasn't
>> treated separately from /* file: #xx */. More line
>> information needed
>> to be preserved to capture all the various line transitions.
>> These
>> had various ripple effects, one being that versions of the LFC
>> (compiled with nameFunctions) needed to have trackLines turned
>> off,
>> otherwise heap space blew up. This may be fixable in the
>> future if it
>> is important. LFC still has /* -*- file: .... */ comments for
>> the
>> top of functions.
>>
>> Also observed during testing is the necessity of not breaking
>> output
>> lines to insert /* -*- file: .... */ comments. This breaks
>> lzunit
>> test/lztest/lztest-node-options.js in a strange manner:
>>
>> [exec] js: "test/lztest/lztest-node-options.js", line
>> 108: uncaught JavaScript runtime exception: TypeError:
>> org.mozilla.javascript.Undefined at 992bae is not a function, it is
>> org.mozilla.javascript.Undefined.
>>
>> A working lztest-node-options.js differed from the broken one
>> only in
>> the comments produced. This remains a mystery, but without much
>> loss of line number accuracy, we now should not break lines.
>>
>> lfc/build.xml
>> Turn off -tracklines when building the LFC, it can overflow
>> heap space
>> due to enormous strings generated internally.
>>
>> ScriptCompiler.java, Compiler.java:
>> Eliminate needless call to Compiler.setProperties(),
>> and eliminate that method. Compiler.defaultOptions() is
>> now only called once.
>>
>> Compiler.java:
>> Support disableTrackLines option.
>>
>> Compiler.java, CommonGenerator.java:
>> Fragments generated with fake filenames are now issued with
>> [filename]
>>
>> SWF9ParseTreePrinter.java:
>> More thorough checking of class 'type'
>>
>> ParseTreePrinter.java
>> - Added extra debugging output for line numbers
>> (DEBUG_LINE_NUMBER)
>>
>> - Integrate file/line numbering to a common approach.
>> There are still two annotations, but they both contain file/
>> line
>> numbers and they are processed the same. One is marked as a
>> 'forced' line number so that a line number will sure to be
>> emitted at the beginning of a function.
>>
>> - allow more internal line annotations to be created -
>> don't automatically elim an annotation just because it is
>> followed by another annotation. The intelligence about
>> what to finally produce happens at the end.
>>
>> - added capability to extract a single annotation.
>>
>> - encapsulate current and last line number info in
>> a LineNumberState struct to make comparison and updating
>> easier and clearer.
>>
>> - when outputting the line numbers, fine tune the conditions for
>> when we need a new source location. Some of the factors in
>> the decision are:
>> -- is it the same as can be predicted?
>> -- does the current/previous filename have a name?
>> and is the name internal (used for compiling a fragment)?
>> and the line number >0?
>> -- are we at the beginning of a line (the preferred spot,
>> but there are exceptions)?
>> -- is this the beginning of a function?
>> -- is trackLines on?
>> this is all managed in shouldShowSourceLocation().
>>
>> - there are now three variants of line number (source locations
>> using the nexb terminology) emitted:
>> /* -*- file: -*- */
>> There is no file attached to the following code
>> Don't expect to see line numbers updated.
>> /* -*- file: FILENAME#NN.YY -*- */
>> The filename has changed. The following output
>> line
>> relates to line NN (and column YY) of FILENAME
>> as the
>> input file. Subsequent output lines numbers can
>> be
>> determined by simple incrementing.
>> /* -*- file: #NN */-*- */
>> The following output line relates to line NN of
>> the
>> previously mentioned FILENAME as the input file.
>> Subsequent output lines numbers can be
>> determined by simple incrementing.
>> At the beginning of output, there is no file attached
>> until the first source location is seen. That is there is
>> an implicit /* -*- file: -*- */ at the beginning of output.
>>
>> Tests:
>> Functionality:
>> Test cases from nexb #449 and #443 work by examining output JS.
>> In particular output not relating to particular input source
>> lines
>> are indicated with /* -*- file: -*- */
>> https://nexb.laszlosystems.com/trac/main/ticket/443
>> https://nexb.laszlosystems.com/trac/main/ticket/449
>> Compiled for example:
>> lzc -DnameFunctions --runtime=dhtml -S nexb443.lzx
>>
>> Regression:
>> ant lzunit
>> swf9 hello
>> (smokecheck,weather,lzunit) x (swf8,dhtml)
>>
>> Files:
>> M WEB-INF/lps/lfc/build.xml
>> M WEB-INF/lps/server/src/org/openlaszlo/sc/ScriptCompiler.java
>> M WEB-INF/lps/server/src/org/openlaszlo/sc/CommonGenerator.java
>> M WEB-INF/lps/server/src/org/openlaszlo/sc/
>> SWF9ParseTreePrinter.java
>> M WEB-INF/lps/server/src/org/openlaszlo/sc/Compiler.java
>> M WEB-INF/lps/server/src/org/openlaszlo/sc/ParseTreePrinter.java
>>
>> Changeset: http://svn.openlaszlo.org/openlaszlo/patches/20080518-dda-6.tar
>>
>>
>>
>> --
>>
>> Don Anderson
>> Java/C/C++, Berkeley DB, systems consultant
>>
>> voice: 617-547-7881
>> email: dda at ddanderson.com
>> www: http://www.ddanderson.com
>>
>>
>>
>>
>
--
Don Anderson
Java/C/C++, Berkeley DB, systems consultant
voice: 617-547-7881
email: dda at ddanderson.com
www: http://www.ddanderson.com
More information about the Laszlo-dev
mailing list