Supporting end column on JSErrorReport

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Supporting end column on JSErrorReport

Ehsan Akhgari
Hi everyone,

Currently JSErrorReport doesn't have the notion of the end column, which is
something that is exposed by the corresponding V8 data structure
(v8::Message).  Is the end column something that we can easily support?

Thanks,
--
Ehsan
_______________________________________________
dev-tech-js-engine mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-js-engine
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Supporting end column on JSErrorReport

Jason Orendorff-2
I would just set the end column to the start column plus 1.

But! This would be fairly easy to fix for SyntaxErrors. Keeping precise
location information around for every part of a script that could possibly
go wrong would take a ridiculous amount of memory; I doubt V8 is really all
that specific with its runtime error messages. We're not. By comparison, if
you're parsing a script and hit an error, noting the precise location (and
length) of the current token is a piece of cake.

-j



On Wed, May 4, 2016 at 6:22 AM, Ehsan Akhgari <[hidden email]>
wrote:

> Hi everyone,
>
> Currently JSErrorReport doesn't have the notion of the end column, which is
> something that is exposed by the corresponding V8 data structure
> (v8::Message).  Is the end column something that we can easily support?
>
> Thanks,
> --
> Ehsan
> _______________________________________________
> dev-tech-js-engine mailing list
> [hidden email]
> https://lists.mozilla.org/listinfo/dev-tech-js-engine
>
_______________________________________________
dev-tech-js-engine mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-js-engine
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Supporting end column on JSErrorReport

Bobby Holley-2
On that note, please keep in mine that JSErrorReporters (though not
necessarily JSErrorReports) are going away, so please don't add any new
dependencies on them for SpiderNode. The Right Way to do it is to use an
RAII guard on the embedding side around any calls where you want exceptions
to be handled, and set autoJSAPIOwnsErrorReporting everwhere.

On Wed, May 4, 2016 at 8:38 AM, Jason Orendorff <[hidden email]>
wrote:

> I would just set the end column to the start column plus 1.
>
> But! This would be fairly easy to fix for SyntaxErrors. Keeping precise
> location information around for every part of a script that could possibly
> go wrong would take a ridiculous amount of memory; I doubt V8 is really all
> that specific with its runtime error messages. We're not. By comparison, if
> you're parsing a script and hit an error, noting the precise location (and
> length) of the current token is a piece of cake.
>
> -j
>
>
>
> On Wed, May 4, 2016 at 6:22 AM, Ehsan Akhgari <[hidden email]>
> wrote:
>
> > Hi everyone,
> >
> > Currently JSErrorReport doesn't have the notion of the end column, which
> is
> > something that is exposed by the corresponding V8 data structure
> > (v8::Message).  Is the end column something that we can easily support?
> >
> > Thanks,
> > --
> > Ehsan
> > _______________________________________________
> > dev-tech-js-engine mailing list
> > [hidden email]
> > https://lists.mozilla.org/listinfo/dev-tech-js-engine
> >
> _______________________________________________
> dev-tech-js-engine mailing list
> [hidden email]
> https://lists.mozilla.org/listinfo/dev-tech-js-engine
>
_______________________________________________
dev-tech-js-engine mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-js-engine
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Supporting end column on JSErrorReport

Ehsan Akhgari
In reply to this post by Jason Orendorff-2
On 2016-05-04 11:38 AM, Jason Orendorff wrote:
> I would just set the end column to the start column plus 1.

OK, I guess that makes sense!

> But! This would be fairly easy to fix for SyntaxErrors.

Filed bug 1270354.

> Keeping precise
> location information around for every part of a script that could
> possibly go wrong would take a ridiculous amount of memory; I doubt V8
> is really all that specific with its runtime error messages. We're not.
> By comparison, if you're parsing a script and hit an error, noting the
> precise location (and length) of the current token is a piece of cake.

Yeah this makes perfect sense.  I don't think this is a particularly
crucial part of the API to support, so it's fine even if V8 exposes more
info here.

> On Wed, May 4, 2016 at 6:22 AM, Ehsan Akhgari <[hidden email]
> <mailto:[hidden email]>> wrote:
>
>     Hi everyone,
>
>     Currently JSErrorReport doesn't have the notion of the end column,
>     which is
>     something that is exposed by the corresponding V8 data structure
>     (v8::Message).  Is the end column something that we can easily support?
>
>     Thanks,
>     --
>     Ehsan
>     _______________________________________________
>     dev-tech-js-engine mailing list
>     [hidden email]
>     <mailto:[hidden email]>
>     https://lists.mozilla.org/listinfo/dev-tech-js-engine
>
>

_______________________________________________
dev-tech-js-engine mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-js-engine
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Supporting end column on JSErrorReport

Ehsan Akhgari
In reply to this post by Bobby Holley-2
On 2016-05-04 12:27 PM, Bobby Holley wrote:
> On that note, please keep in mine that JSErrorReporters (though not
> necessarily JSErrorReports) are going away, so please don't add any new
> dependencies on them for SpiderNode. The Right Way to do it is to use an
> RAII guard on the embedding side around any calls where you want
> exceptions to be handled, and set autoJSAPIOwnsErrorReporting everwhere.

I'm already using autoJSAPIOwnsErrorReporting since that matches the
semantics of V8 APIs.  But even with that the JS engine uses the
JSErrorReporter to notify about warnings, so I don't think it's
reasonable to not use a JSErrorReporter in SpiderNode for now.  (We
currently output this information when the VERBOSE environment variable
is set.)

Is there going to be a specific API for warnings?  If not, what else
should I be doing instead?

Thanks,
Ehsan

> On Wed, May 4, 2016 at 8:38 AM, Jason Orendorff <[hidden email]
> <mailto:[hidden email]>> wrote:
>
>     I would just set the end column to the start column plus 1.
>
>     But! This would be fairly easy to fix for SyntaxErrors. Keeping precise
>     location information around for every part of a script that could
>     possibly
>     go wrong would take a ridiculous amount of memory; I doubt V8 is
>     really all
>     that specific with its runtime error messages. We're not. By
>     comparison, if
>     you're parsing a script and hit an error, noting the precise
>     location (and
>     length) of the current token is a piece of cake.
>
>     -j
>
>
>
>     On Wed, May 4, 2016 at 6:22 AM, Ehsan Akhgari
>     <[hidden email] <mailto:[hidden email]>>
>     wrote:
>
>     > Hi everyone,
>     >
>     > Currently JSErrorReport doesn't have the notion of the end column,
>     which is
>     > something that is exposed by the corresponding V8 data structure
>     > (v8::Message).  Is the end column something that we can easily
>     support?
>     >
>     > Thanks,
>     > --
>     > Ehsan
>     > _______________________________________________
>     > dev-tech-js-engine mailing list
>     > [hidden email]
>     <mailto:[hidden email]>
>     > https://lists.mozilla.org/listinfo/dev-tech-js-engine
>     >
>     _______________________________________________
>     dev-tech-js-engine mailing list
>     [hidden email]
>     <mailto:[hidden email]>
>     https://lists.mozilla.org/listinfo/dev-tech-js-engine
>
>

_______________________________________________
dev-tech-js-engine mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-js-engine
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Supporting end column on JSErrorReport

Bobby Holley-2
On Wed, May 4, 2016 at 9:30 PM, Ehsan Akhgari <[hidden email]>
wrote:

> On 2016-05-04 12:27 PM, Bobby Holley wrote:
> > On that note, please keep in mine that JSErrorReporters (though not
> > necessarily JSErrorReports) are going away, so please don't add any new
> > dependencies on them for SpiderNode. The Right Way to do it is to use an
> > RAII guard on the embedding side around any calls where you want
> > exceptions to be handled, and set autoJSAPIOwnsErrorReporting everwhere.
>
> I'm already using autoJSAPIOwnsErrorReporting since that matches the
> semantics of V8 APIs.  But even with that the JS engine uses the
> JSErrorReporter to notify about warnings, so I don't think it's
> reasonable to not use a JSErrorReporter in SpiderNode for now.  (We
> currently output this information when the VERBOSE environment variable
> is set.)
>
> Is there going to be a specific API for warnings?  If not, what else
> should I be doing instead?
>

Yes, warnings will probably use something like the current mechanism for
the foreseeable future. This is why the JSErrorReporter that Gecko installs
is called WarningOnlyErrorReporter, and asserts JSREPORT_IS_WARNING.
Spidernode should do something similar.


>
> Thanks,
> Ehsan
>
> > On Wed, May 4, 2016 at 8:38 AM, Jason Orendorff <[hidden email]
> > <mailto:[hidden email]>> wrote:
> >
> >     I would just set the end column to the start column plus 1.
> >
> >     But! This would be fairly easy to fix for SyntaxErrors. Keeping
> precise
> >     location information around for every part of a script that could
> >     possibly
> >     go wrong would take a ridiculous amount of memory; I doubt V8 is
> >     really all
> >     that specific with its runtime error messages. We're not. By
> >     comparison, if
> >     you're parsing a script and hit an error, noting the precise
> >     location (and
> >     length) of the current token is a piece of cake.
> >
> >     -j
> >
> >
> >
> >     On Wed, May 4, 2016 at 6:22 AM, Ehsan Akhgari
> >     <[hidden email] <mailto:[hidden email]>>
> >     wrote:
> >
> >     > Hi everyone,
> >     >
> >     > Currently JSErrorReport doesn't have the notion of the end column,
> >     which is
> >     > something that is exposed by the corresponding V8 data structure
> >     > (v8::Message).  Is the end column something that we can easily
> >     support?
> >     >
> >     > Thanks,
> >     > --
> >     > Ehsan
> >     > _______________________________________________
> >     > dev-tech-js-engine mailing list
> >     > [hidden email]
> >     <mailto:[hidden email]>
> >     > https://lists.mozilla.org/listinfo/dev-tech-js-engine
> >     >
> >     _______________________________________________
> >     dev-tech-js-engine mailing list
> >     [hidden email]
> >     <mailto:[hidden email]>
> >     https://lists.mozilla.org/listinfo/dev-tech-js-engine
> >
> >
>
>
_______________________________________________
dev-tech-js-engine mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-js-engine
Loading...