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 [EMAIL PROTECTED] 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:
[EMAIL PROTECTED] 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: [EMAIL PROTECTED]
www: http://www.ddanderson.com
--
Don Anderson
Java/C/C++, Berkeley DB, systems consultant
voice: 617-547-7881
email: [EMAIL PROTECTED]
www: http://www.ddanderson.com
