Filename and line numbers in JSScript

[Nicholas Nethercote]

Hi,

JSScript::NewScriptFromEmitter() has this code:

const char *filename = bce->parser->tokenStream.getFilename();
if (filename) {
script->filename = SaveScriptFilename(cx, filename);
if (!script->filename)
return false;
}
script->lineno = bce->firstLine;

Basically, it gets the filename for the JSScript from |tokenStream|,
but gets the lineno from |bce|. This is inconsistent, because if you
have a "//@line 42 foo.js" comment, |tokenStream|'s filename and
lineno get updated, but |bce->firstLine| does not.

Any suggestions which of these two cases is correct? I feel like
lineno, which isn't affected by //@line comments, is the correct
behaviour -- i.e. JSScript's filename/lineno should be what they were
at the start of the script. But I'm not sure.

Nick

[Nicholas Nethercote]

I tried changing JSScript::filename to have the same behaviour as
JSScript::lineno. I get a test failure in
js1_8_5/regress/regress-618652.js, which looks like this:

options('atline')

var F, L;

eval('//@line 42 "foo"\n' +
'try { nosuchname; } catch (e) { F = e.fileName; L = e.lineNumber; }');
assertEq(F, "foo");
assertEq(L, 42);

reportCompare(0, 0, "ok");

After my change, |F| isn't "foo", but |L| is 42. That surprised me,
because I thought JSScript::lineno should not be affected by the
//@line, i.e. it should be 1, not 42. And it turns out that's
correct. But what happens is that e.fileName comes from
JSScript::filename, but e.lineNumber *does not* come from
JSScript::lineno! Why not? Well, in PopulateReportBlame() we have
this:

report->filename = iter.script()->filename;
report->lineno = PCToLineNumber(iter.script(), iter.pc());

PCToLineNumber looks at the source notes for the JSScript, and those
source notes are computed from TokenStream::lineno, not
JSScript::lineno.

Argh, what a muddle. I guess the key question is this: what are
JSScript::{filename,lineno} meant to mean? And if the answer is "the
filename/lineno at the start of the script", then we probably
shouldn't be using JSScript::filename to generate exceptions/errors,
because it won't account for //@line comments.

In fact, I suspect JSScript::{filename,lineno} are almost never what
you really want -- they made sense before //@line comments were added,
but no longer do and should probably be removed...

Nick

[Dave Mandelin]

It's hard for me to imagine what meaning 'script source location' could mean other than 'what the client says the source location is'.

But I can't make much sense out of what @line is doing now. I tried a little shell test, and the filename part seemed never to get used. And it seemed to apply to code inside a script only if the @line was within the script. Is there documentation on what it's supposed to do?

Dave

[Dave Mandelin]

On Thursday, May 24, 2012 7:17:02 AM UTC-7, Nicholas Nethercote wrote:

[Brendan Eich]

Dave Mandelin wrote:
> It's hard for me to imagine what meaning 'script source location' could mean other than 'what the client says the source location is'.

This was for the XUL pre-processor, to get line numbers to reflect
primary source coordinates, not expanded output. As with CPP's #line
directives.

> But I can't make much sense out of what @line is doing now. I tried a little shell test, and the filename part seemed never to get used.

Did you set the atline option?

$ cat /tmp/atline.js
function f(o) {
print('hi'); //@line 43 "foo"
o.x.y;
}
f({});

$ ./Darwin_DBG.OBJ/js -o atline -f /tmp/atline.js
hi
foo:43: TypeError: o.x is undefined

> And it seemed to apply to code inside a script only if the @line was within the script.

Seems to persist for me:

$ cat /tmp/atline2.js
function f(o) {
print('hi'); //@line 43 "foo"
o.x.y;
}
try { f({}); } catch (e) {}
g();

$ ./Darwin_DBG.OBJ/js -o atline -f /tmp/atline2.js
hi
foo:46: ReferenceError: g is not defined

> Is there documentation on what it's supposed to do?

Not sure there's doc, this is old (XULpp was early 2000s).

/be

[Brendan Eich]

Brendan Eich wrote:
> Did you set the atline option?

This option is enabled for XUL script only. Why is there an option?
Because magic comments that change source line and maybe file are not
standard and we didn't want to take a chance on the web.

/be

[Brendan Eich]

Nicholas Nethercote wrote:
> Hi,

>
> JSScript::NewScriptFromEmitter() has this code:
>
> const char *filename = bce->parser->tokenStream.getFilename();
> if (filename) {
> script->filename = SaveScriptFilename(cx, filename);
> if (!script->filename)
> return false;
> }
> script->lineno = bce->firstLine;
>
> Basically, it gets the filename for the JSScript from |tokenStream|,
> but gets the lineno from |bce|. This is inconsistent, because if you
> have a "//@line 42 foo.js" comment, |tokenStream|'s filename and
> lineno get updated, but |bce->firstLine| does not.

There's a required staging or phase structure: lexing/parsing of a
compileable unit (top level statement or function declaration), then
code generation. So updating the tokenStream's line number should
suffice if the bytecode emitter's firstLine, etc. state is set only
after ward.

Did something change in the phase structure to break this, or did you
leave out the JSOPTION_ATLINE configury required to see effects of
//@line at all? The tests I showed in reply to dmandelin seem to work
same as in a FF3-era js shell. Seems like a testing gap exists, though :-(.

The source maps work is all much more important than this old
XULpp-motivated hack, but it will want the same kind of control.

/be

[Dave Mandelin]

On Thursday, May 24, 2012 12:06:38 PM UTC-7, Brendan Eich wrote:
> Dave Mandelin wrote:
> > It's hard for me to imagine what meaning 'script source location' could mean other than 'what the client says the source location is'.
>
> This was for the XUL pre-processor, to get line numbers to reflect
> primary source coordinates, not expanded output. As with CPP's #line
> directives.

Er, that's the sort of thing I meant. I was trying to agree with Nick, I think.

> > But I can't make much sense out of what @line is doing now. I tried a little shell test, and the filename part seemed never to get used.
>
> Did you set the atline option?

I set it with |options('atline')|, like in Nick's example. Strangely, now it doesn't seem to work at all:

$ cat /tmp/c.js
options('atline')

function f() {
//@line 100 "foo"
d = u.asdf
}

f()

$ shell/js /tmp/c.js
C:/Users/Dave/AppData/Local/Temp/c.js:5: ReferenceError: u is not defined

> $ cat /tmp/atline.js
> function f(o) {
> print('hi'); //@line 43 "foo"
> o.x.y;
> }
> f({});
>
> $ ./Darwin_DBG.OBJ/js -o atline -f /tmp/atline.js
> hi
> foo:43: TypeError: o.x is undefined

Hmmm, the -o option doesn't exist on trunk AFAIK.

[Dave Mandelin]

On Thursday, May 24, 2012 12:06:38 PM UTC-7, Brendan Eich wrote:

> Dave Mandelin wrote:
> > It's hard for me to imagine what meaning 'script source location' could mean other than 'what the client says the source location is'.
>
> This was for the XUL pre-processor, to get line numbers to reflect
> primary source coordinates, not expanded output. As with CPP's #line
> directives.

Er, that's the sort of thing I meant. I was trying to agree with Nick, I think.

> > But I can't make much sense out of what @line is doing now. I tried a little shell test, and the filename part seemed never to get used.
>
> Did you set the atline option?

I set it with |options('atline')|, like in Nick's example. Strangely, now it doesn't seem to work at all:

$ cat /tmp/c.js
options('atline')

function f() {
//@line 100 "foo"
d = u.asdf
}

f()

$ shell/js /tmp/c.js
C:/Users/Dave/AppData/Local/Temp/c.js:5: ReferenceError: u is not defined

> $ cat /tmp/atline.js
> function f(o) {
> print('hi'); //@line 43 "foo"
> o.x.y;
> }
> f({});
>
> $ ./Darwin_DBG.OBJ/js -o atline -f /tmp/atline.js
> hi
> foo:43: TypeError: o.x is undefined

Hmmm, the -o option doesn't exist on trunk AFAIK.

[Brendan Eich]

Dave Mandelin wrote:
> Hmmm, the -o option doesn't exist on trunk AFAIK.

Did this get removed recently? Sure enough, my shell was from early last
week and it works. A rebuild doesn't.

The call to options('atline') of course happens after we've tokenized
the entire file (and parsed it, and compiled it into bytecode which runs
and calls that Options shell built-in ;-). Kind of a pain to use. You
have to call options('atline') before compiling via eval, Function, or
similar.

Why was -o removed? It's handy, especially for atline testing in light
of the previous paragraph :-P.

/be

[Dave Mandelin]

On Thursday, May 24, 2012 1:10:58 PM UTC-7, Brendan Eich wrote:
> Dave Mandelin wrote:
> > Hmmm, the -o option doesn't exist on trunk AFAIK.
> Did this get removed recently? Sure enough, my shell was from early last
> week and it works. A rebuild doesn't.
>
> The call to options('atline') of course happens after we've tokenized
> the entire file (and parsed it, and compiled it into bytecode which runs
> and calls that Options shell built-in ;-).

Ah, right.

> Kind of a pain to use. You
> have to call options('atline') before compiling via eval, Function, or
> similar.
>
> Why was -o removed? It's handy, especially for atline testing in light
> of the previous paragraph :-P.

That happened a while ago, during a cleanup of the many shell options. Ones not used and not complained about soon after got removed.

shell/js -e 'options("atline")' /tmp/b.js

works, though, and is not too bad.

Dave

[Dave Mandelin]

On Thursday, May 24, 2012 1:10:58 PM UTC-7, Brendan Eich wrote:
> Dave Mandelin wrote:
> > Hmmm, the -o option doesn't exist on trunk AFAIK.
> Did this get removed recently? Sure enough, my shell was from early last
> week and it works. A rebuild doesn't.
>
> The call to options('atline') of course happens after we've tokenized
> the entire file (and parsed it, and compiled it into bytecode which runs
> and calls that Options shell built-in ;-).

Ah, right.

> Kind of a pain to use. You
> have to call options('atline') before compiling via eval, Function, or
> similar.
>
> Why was -o removed? It's handy, especially for atline testing in light
> of the previous paragraph :-P.

[Brendan Eich]

Dave Mandelin wrote:
> shell/js -e 'options("atline")' /tmp/b.js
>
> works, though, and is not too bad.

Good enough, thanks.

Nick, is there a bug here other than the lack of tests and the
awkwardness, and the whole thing being superseded (I hope) by Source Maps?

/be

[Nicholas Nethercote]

On Thu, May 24, 2012 at 12:22 PM, Brendan Eich <bre...@mozilla.com> wrote:
>
> There's a required staging or phase structure: lexing/parsing of a
> compileable unit (top level statement or function declaration), then code
> generation. So updating the tokenStream's line number should suffice if the
> bytecode emitter's firstLine, etc. state is set only after ward.

BytecodeEmitter's constructor sets firstLine -- i.e. before codegen
happens -- and it's never changed after that. It's been like that for
some time, certainly prior to my recent front-end refactorings.

> Did something change in the phase structure to break this, or did you leave
> out the JSOPTION_ATLINE configury required to see effects of //@line at all?
> The tests I showed in reply to dmandelin seem to work same as in a FF3-era
> js shell. Seems like a testing gap exists, though :-(.

The phase structure hasn't changed, AFAICT.

It's still not clear to me what JSScript::{filename,lineno} are
supposed to mean when //@line comments exist. Actually, it does make
sense if you assume that //@line comments only ever appear at the top
of a script (i.e. top of the file or top of a function). But if you
have multiple //@line comments within a script, they don't make sense.
(At least, updating them based on //@line comments doesn't make
sense.)

I suspect //@line comments were implemented just enough to get the XUL
case working, but nobody pushed on them harder than that to work
through all the consequences. There are only two tests that use them
-- js1_8_5/regress/regress-618652.js and jsapi-tests/testXDR.cpp --
and together they include only 3 //@line comments.

(BTW, I'm not trying to lay blame here so much as gather evidence for
my "this doesn't make sense" hypothesis.)

> The source maps work is all much more important than this old
> XULpp-motivated hack, but it will want the same kind of control.

Unfortunately it seems to suffer similar problems. At least, there
currently can be at most one source map filename associated with a
JSScript. Is that a valid assumption?

Nick

[Wes Garland]

I'm not sure what source maps are intended to do -- but I have been
considering using @line functionality to keep line numbers correct when the
day finally comes that I can't do CommonJS/1.1 modules without adding a
function(){-prologue onto each module before compiling it. Pretty much
exactly how the C preprocessor works.

("Finally comes" -- will certainly happen if web-focussed optimizations
continue on the current roap map, the prologue hack is already ~ required
in the browser)

Wes

> _______________________________________________
> dev-tech-js-engine-internals mailing list
> dev-tech-js-en...@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-tech-js-engine-internals
>



--
Wesley W. Garland
Director, Product Development
PageMail, Inc.
+1 613 542 2787 x 102

[Brendan Eich]

Nicholas Nethercote wrote:
> After my change,|F| isn't "foo", but|L| is 42. That surprised me,
> because I thought JSScript::lineno should not be affected by the
> //@line, i.e. it should be 1, not 42. And it turns out that's
> correct. But what happens is that e.fileName comes from

> JSScript::filename, but e.lineNumber*does not* come from

> JSScript::lineno! Why not? Well, in PopulateReportBlame() we have
> this:
>
> report->filename = iter.script()->filename;
> report->lineno = PCToLineNumber(iter.script(), iter.pc());

PCToLineNumber uses iter.script()->lineno as its starting point, though.

> PCToLineNumber looks at the source notes for the JSScript, and those
> source notes are computed from TokenStream::lineno, not
> JSScript::lineno.

js::PCToLineNumber(JSScript *script, jsbytecode *pc)
{
/* Cope with StackFrame.pc value prior to entering js_Interpret. */
if (!pc)
return 0;

return PCToLineNumber(script->lineno, script->notes(),
script->code, pc);
}

and just above this,

unsigned
js::PCToLineNumber(unsigned startLine, jssrcnote *notes, jsbytecode
*code, jsbytecode *pc)
{
unsigned lineno = startLine;

/*
* Walk through source notes accumulating their deltas, keeping
track of
* line-number notes, until we pass the note for pc's offset within
* script->code.
*/
ptrdiff_t offset = 0;
ptrdiff_t target = pc - code;
for (jssrcnote *sn = notes; !SN_IS_TERMINATOR(sn); sn = SN_NEXT(sn)) {
offset += SN_DELTA(sn);
SrcNoteType type = (SrcNoteType) SN_TYPE(sn);
if (type == SRC_SETLINE) {
if (offset <= target)
lineno = (unsigned) js_GetSrcNoteOffset(sn, 0);
} else if (type == SRC_NEWLINE) {
if (offset <= target)
lineno++;
}
if (offset > target)
break;
}

return lineno;
}

So "but e.lineNumber *does not* come from JSScript::lineno!" is true,
but that's by design, and you have not shown that JSScript::lineno is
unused (useless). In the absence of a SRC_SETLINE, it is absolutely the
basis-case line number and required for the correct run time line number
result.

Sorry if I'm being dense, but I don't see a bug. Do you have a test-case
showing one?

/be

[Brendan Eich]

Nicholas Nethercote wrote:
> It's still not clear to me what JSScript::{filename,lineno} are
> supposed to mean when //@line comments exist.

JSScript::filename is the file name set by the last @line, and you're
right, it has to agree with previous settings, it's a pigeon-hole. Just
enough for the XULpp, as you note later.

JSScript::lineno is the starting line number, and source notes notes can
override it, including for multiple embedded //@line uses.

> Actually, it does make
> sense if you assume that //@line comments only ever appear at the top
> of a script (i.e. top of the file or top of a function).

Let's separate JSScript::filename from JSScript::lineno. filename has a
pigeon-hole problem as you note. lineno does not.

>> The source maps work is all much more important than this old
>> XULpp-motivated hack, but it will want the same kind of control.
>
> Unfortunately it seems to suffer similar problems. At least, there
> currently can be at most one source map filename associated with a
> JSScript. Is that a valid assumption?

Yes, this is exactly the assumption behind the JSScript::filename
pigeon-hole. It still holds for compilers such as CoffeeScript or
Traceur. It wouldn't be enough for a language that extended JS with
#include where the inclusions did not force JSScript boundaries, but I
don't know of such a language.

/be

[Nicholas Nethercote]

I just stepped through the code in gdb. In
js1_8_5/regress/regress-618652.js there is a SRC_SETLINE, and it sets
lineno to 42, replacing the its prior value of 1. I'm not saying that
JSScript::lineno is useless, but I suspect if there is a //@line
comment there will always be a corresponding SRC_SETLINE.

> JSScript::lineno is the starting line number, and source notes notes can
> override it, including for multiple embedded //@line uses.

Nope, precisely because of the:

script->lineno = bce->firstLine;

in NewScriptFromEmitter().

----

Anyway, the starting point of this thread was that filename and lineno
are handled inconsistently. To summarize:

- JSScript::lineno is the line number for the start of the script. It
is not affected by //@line comments within the script.
JSScript::startLine would arguably be a better name.

- JSScript::filename is assigned to TokenStream::getFilename() at the
end of script compilation. In the absence of //@line comments in the
script, this is the filename inherited from the surrounding context.
In the presence of //@line comments that have a filename, it's the
filename from the last such comment. There's an unstated assumption
that there will only be one such //@line comment per script.

Now that this is clear, I can move forward. Thanks for the clarifications!

(Actually, now that I think of it: is there an unstated assumption
that line numbers specified by //@line comments increase
monotonically? I ask because it affects
https://bugzilla.mozilla.org/show_bug.cgi?id=747831 significantly --
it would be great if the answer was "yes".)

Nick

[Brendan Eich]

Nicholas Nethercote wrote:
> I'm not saying that JSScript::lineno is useless, but I suspect if
> there is a //@line comment there will always be a corresponding
> SRC_SETLINE.

Almost always -- if the line numbers don't differ enough there'll be one
or two SRC_NEWLINEs. If the lines happen to match then nothing.

>> JSScript::lineno is the starting line number,

Argh, I used the ambiguous phrase. Sorry, should have written starting
line number according to prior //@line directives but not any nested in
the embedded (function) script case. More below.

>> and source notes notes can
>> override it, including for multiple embedded //@line uses.
>
> Nope, precisely because of the:
>
> script->lineno = bce->firstLine;
>
> in NewScriptFromEmitter().

Yes, but bce->firstLine is set directly by BytecodeEmitter's ctor from
the ctor's lineno parameter. There are three calls to this ctor:

$ grep 'BytecodeEmitter [a-zA-Z_][a-zA-Z0-9_]*(' *cpp */*cpp
frontend/BytecodeCompiler.cpp: BytecodeEmitter bce(&parser, &sc,
lineno, noScriptRval, needScriptGlobal);
frontend/BytecodeCompiler.cpp: BytecodeEmitter funbce(&parser,
&funsc, lineno,
frontend/BytecodeEmitter.cpp: BytecodeEmitter bce2(bce->parser,
&sc, pn->pn_pos.begin.lineno,

The first two are API entry points, so the starting line number is
whatever the API caller wants. The last is for a function nested in
another script or function, and pn->pn_pos.begin.lineno is the starting
line number of the nested function.

A function pn's pn_pos.begin.lineno is set under Parser::functionDef
before the function's body is parsed, and so it may have been farbled by
a prior //@line, but not by any in the nested function. Any //@line
comments embedded in the function may, depending on deltas, cause source
notes to be emitted.

> Anyway, the starting point of this thread was that filename and lineno
> are handled inconsistently. To summarize:
>
> - JSScript::lineno is the line number for the start of the script. It
> is not affected by //@line comments within the script.
> JSScript::startLine would arguably be a better name.

Sure. This name dates from 1995, there must be a better name! But this
is "just a name", not a functional change. I was hoping for a bug! :-P

> - JSScript::filename is assigned to TokenStream::getFilename() at the
> end of script compilation. In the absence of //@line comments in the
> script, this is the filename inherited from the surrounding context.
> In the presence of //@line comments that have a filename, it's the
> filename from the last such comment. There's an unstated assumption
> that there will only be one such //@line comment per script.

Yes, this is the pigeon-hole design limit, good enough for one filename
change per script, not meant for #include scenarios.

Should we assert that the name is only changed via //@line to a novel
string (not only changed, just never changed to a different string
value) more than once?

> Now that this is clear, I can move forward. Thanks for the clarifications!

Thanks for clarifying!. The filename pigeon-hole is worth a comment or
perhaps even an assertion.

> (Actually, now that I think of it: is there an unstated assumption
> that line numbers specified by //@line comments increase
> monotonically?

No, SRC_SETLINE can handle reducing the current line number. Translators
such as the XULpp do not do this, AFAIK, but any line number
discontinuity will cause a SRC_SETLINE to be emitted.

> I ask because it affects
> https://bugzilla.mozilla.org/show_bug.cgi?id=747831 significantly --
> it would be great if the answer was "yes".)

Even though SRC_SETLINE can reduce the current line number for a given
next-bytecode, if no one needs this we can do without. What does the
SourceMap spec say?

https://wiki.mozilla.org/DevTools/Features/SourceMap

Out of time to find myself, appreciate anyone answering definitively.

/be

[Nicholas Nethercote]

On Mon, May 28, 2012 at 5:45 PM, Brendan Eich <bre...@mozilla.com> wrote:
>
> I was hoping for a bug! :-P

No bugs in the present code AFAIK. I raised this only because the
present code was stymying my refactoring attempts. Thanks for the
additional clarifications.

Nick

[Nicholas Nethercote]

On Mon, May 28, 2012 at 6:03 PM, Nicholas Nethercote >

> No bugs in the present code AFAIK.

Well... there are numerous places where we use script->filename and
script->lineno as a pair, in a fashion which suggests that the author
thought (not unreasonably!) that they have similar semantics. For
example, a simple search which catches a subset of these cases:

[bayou:~/moz/mi7/js/src] rgrep "script->filename.*script->lineno" '*.cpp'
./jsapi.cpp: JS_snprintf(buf, bufsize, " %s:%u",
script->filename, unsigned(script->lineno));
./jsinfer.cpp: printf(" #%u %s (line %d):\n", script->id(),
script->filename, script->lineno);
./jsdbgapi.cpp: fprintf(stdout, "--- SCRIPT %s:%d ---\n",
script->filename, script->lineno);
./jsdbgapi.cpp: fprintf(stdout, "--- END SCRIPT %s:%d ---\n",
script->filename, script->lineno);
./jsdbgapi.cpp: fprintf(stdout, "--- SCRIPT %s:%d ---\n",
script->filename, script->lineno);
./jsdbgapi.cpp: fprintf(stdout, "--- END SCRIPT %s:%d ---\n",
script->filename, script->lineno);
./jsobj.cpp: script->filename ? script->filename :
"", script->lineno);
./methodjit/InvokeHelpers.cpp: script->filename,
script->lineno, OpcodeNames[op], PCToLineNumber(script, pc));
./methodjit/Compiler.cpp: script->filename, script->lineno);
./methodjit/Compiler.cpp: script->filename, script->lineno);
./methodjit/MethodJIT.cpp: script->filename, script->lineno);
./methodjit/Retcon.cpp: script->filename,
script->lineno, script->length);
./jsscript.cpp: hook(cx, script->filename, script->lineno, script, fun,

A lot of these are just debugging output things, where an
inconsistency between the filename and lineno won't be a big deal, and
could easily go unnoticed. There are some others where it's less
obvious, and I haven't looked in detail at them yet.

Nick

[Nicholas Nethercote]

On Wed, May 30, 2012 at 11:35 PM, Nicholas Nethercote
<n.neth...@gmail.com> wrote:
> On Mon, May 28, 2012 at 6:03 PM, Nicholas Nethercote >
>> No bugs in the present code AFAIK.
>
> Well... there are numerous places where we use script->filename and
> script->lineno as a pair, in a fashion which suggests that the author
> thought (not unreasonably!) that they have similar semantics. For
> example, a simple search which catches a subset of these cases:

Here's a more radical thought: can we just get rid of //@line support
altogether? I see three reasons for this.

- In general, //@line is under-specified -- it's not clear (or
documented!) how it's supposed to work. The line numbering works
fairly well, but if you specify more than one file only the last one
will be used. And see
https://bugzilla.mozilla.org/show_bug.cgi?id=663934 for an old bug
about the filenames. The unclear semantics means it's hard to clean
up the related code.

- Within Mozilla code, AFAICT it's only used in cases where it doesn't
add much. For example, from browser/components/nsBrowserGlue.js we
generate dist/bin/webapprt/chrome/webapprt/content/webapp.js, which is
the same except it has a few #ifdef-y things stripped out, and the
line number remapping provides very little value.

- JS has source maps now, which are much more powerful and official.

Thoughts?

Nick

[Benjamin Peterson]

2013/2/18 Nicholas Nethercote <n.neth...@gmail.com>:

> On Wed, May 30, 2012 at 11:35 PM, Nicholas Nethercote
> <n.neth...@gmail.com> wrote:
>> On Mon, May 28, 2012 at 6:03 PM, Nicholas Nethercote >
>>> No bugs in the present code AFAIK.
>>
>> Well... there are numerous places where we use script->filename and
>> script->lineno as a pair, in a fashion which suggests that the author
>> thought (not unreasonably!) that they have similar semantics. For
>> example, a simple search which catches a subset of these cases:
>
> Here's a more radical thought: can we just get rid of //@line support
> altogether?

+100



--
Regards,
Benjamin

[Nicholas Nethercote]

On Mon, Feb 18, 2013 at 2:59 PM, Nicholas Nethercote
<n.neth...@gmail.com> wrote:
>
> Here's a more radical thought: can we just get rid of //@line support

> altogether? I see three reasons for this.
>

> - Within Mozilla code, AFAICT it's only used in cases where it doesn't
> add much. For example, from browser/components/nsBrowserGlue.js we
> generate dist/bin/webapprt/chrome/webapprt/content/webapp.js, which is
> the same except it has a few #ifdef-y things stripped out, and the
> line number remapping provides very little value.

To clarify: it's only used in
http://mxr.mozilla.org/mozilla-central/source/config/Preprocessor.py#132,
which is "a very primitive line based preprocessor, for times when
using a C preprocessor isn't an option."

Even better: Benjamin tells me the "atline" option isn't set in the browser!

Nick

[Brendan Eich]

Of course Source Maps supersede //@line. Just test twice, cut once.

/be

[Bill McCloskey]

> To clarify: it's only used in
> http://mxr.mozilla.org/mozilla-central/source/config/Preprocessor.py#132,
> which is "a very primitive line based preprocessor, for times when
> using a C preprocessor isn't an option."
>
> Even better: Benjamin tells me the "atline" option isn't set in the
> browser!

I looked into this a few months ago because I was interested in getting proper line numbers for errors in chrome code. When I enabled the @line stuff in the browser, it caused crashes on startup. The problem didn't seem to be specific to the browser code in question; I think this feature is just broken. There's one extremely simple test for @line in the test suite. Aside from that, I can't imagine anyone can be using it.

Boris told me that someone tried to enable @line for browser code a while ago. However, there's an existing feature where errors in the error console have hyperlinks to the error location, and for chrome code they link directly to the preprocessed output that's packaged in the Firefox omnijar. So if we enabled @line, we'd get the right line numbers for the original source code but the wrong ones for the omnijar versions, which would break the hyperlinks. That's why @line was disabled.

I suspect that source maps would have the same problem as @line did, so it probably isn't worth the trouble of fixing the preprocessor to generate them. Unless somebody can think of a solution for the hyperlinks.

-Bill

[Nicholas Nethercote]

On Mon, Feb 18, 2013 at 4:58 PM, Brendan Eich <bre...@mozilla.com> wrote:
> Of course Source Maps supersede //@line. Just test twice, cut once.

I wrote patches and filed https://bugzilla.mozilla.org/show_bug.cgi?id=842438.

Nick

[Jim Blandy]

When a compressed/preprocessed/translated JS file contains a source map
URL, the source map can provide a new URL for the original source code.
So as long as we use distinct URLs for the preprocessed and original
("pre-preprocessed"?) sources, and ensure both can be followed, I think
the console links would work out.

What's more interesting - Brendan said this briefly - is that source
maps re-introduce the potential for a single JSScript to have code
attributed to several different URLs. Such source maps describe the
results of concatenating several .js files, for example. So it won't
make sense to ask for the URL of a JSScript: one will ask for the URL of
a <JSScript, bytecode offset> pair.