xml/json parse error location in catch clause

Can you remind me what the relevant file(s) are again? I.e., the one with the enum that needs to be augmented?

src/compiler/translator/translator.cpp:
- begin_visit(const CatchExpr& v)

src/runtime/core/trycatch.h
- enum var_type

src/runtime/core/trycatch.cpp
- TryCatchIterator::bindErrorVars(...)

BTW: this can't be put into 2.9 because it means changing the public API in a non-backwards-compatible way for XQueryException due to the addition of a "data location."

Why is the addition a incompatible API change?

On Feb 4, 2013, at 8:30 PM, "Paul J. Lucas" <email address hidden> wrote:

> BTW: this can't be put into 2.9 because it means changing the public API
> in a non-backwards-compatible way for XQueryException due to the
> addition of a "data location."
>
> ** Changed in: zorba
> Milestone: 2.9 => 3.0
>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/1111786
>
> Title:
> xml/json parse error location in catch clause
>
> Status in Zorba - The XQuery Processor:
> In Progress
>
> Bug description:
> There should be a way to retrieve the error location of a xml/json parser error in a XQuery catch clause.
> One way of doing this would be to always declare and eventually bind the following variables:
>
> $zerr:data-line, $zerr:data-column, $zerr:data-uri
>
> For non-parser related errors, those variables would be bound to the
> empty sequence.
>
> Alternatively, there could be only one variable $zerr:data-error which is bound to
> either the empty sequence, an xml element, or a json object.
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/zorba/+bug/1111786/+subscriptions

On Feb 4, 2013, at 10:16 PM, Matthias Brantner <email address hidden> wrote:

> Why is the addition a incompatible API change?

I believe it's if they use the copy constructor in their own code to construct a local copy of an XQueryException on the stack. Their compiler allocated sizeof(old-XQueryException) but the sizeof(new-XQueryException) is now bigger and trample all over their stack. Chaos ensues.

For adding the JSON location, if you look at runtime/json/json_impl.cpp, the parser location is always set to the query location. If it's reading JSON from a file, it should set the location to that of the FILE and not the query. How do I find out (a) if it's reading from a file and (b) that file's URI?

On Feb 5, 2013, at 7:30 AM, Paul J. Lucas <email address hidden> wrote:

> On Feb 4, 2013, at 10:16 PM, Matthias Brantner
> <email address hidden> wrote:
>
>> Why is the addition a incompatible API change?
>
> I believe it's if they use the copy constructor in their own code to
> construct a local copy of an XQueryException on the stack. Their
> compiler allocated sizeof(old-XQueryException) but the sizeof(new-
> XQueryException) is now bigger and trample all over their stack. Chaos
> ensues.
But that's an ABI incompatibility, right? The API is still compatible.

>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/1111786
>
> Title:
> xml/json parse error location in catch clause
>
> Status in Zorba - The XQuery Processor:
> In Progress
>
> Bug description:
> There should be a way to retrieve the error location of a xml/json parser error in a XQuery catch clause.
> One way of doing this would be to always declare and eventually bind the following variables:
>
> $zerr:data-line, $zerr:data-column, $zerr:data-uri
>
> For non-parser related errors, those variables would be bound to the
> empty sequence.
>
> Alternatively, there could be only one variable $zerr:data-error which is bound to
> either the empty sequence, an xml element, or a json object.
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/zorba/+bug/1111786/+subscriptions

Oh, I thought we cared about ABI incompatibilities as well. If we don't fine.

Adding Chris.

That's a question that has never been answered. In certain cases, we don't care. Sometimes
we care but have no way/test to make sure except looking very carefully.

@Chris: what's your take on that?

Matthias

On Feb 5, 2013, at 10:10 AM, "Paul J. Lucas" <email address hidden> wrote:

> Oh, I thought we cared about ABI incompatibilities as well. If we don't
> fine.
>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/1111786
>
> Title:
> xml/json parse error location in catch clause
>
> Status in Zorba - The XQuery Processor:
> In Progress
>
> Bug description:
> There should be a way to retrieve the error location of a xml/json parser error in a XQuery catch clause.
> One way of doing this would be to always declare and eventually bind the following variables:
>
> $zerr:data-line, $zerr:data-column, $zerr:data-uri
>
> For non-parser related errors, those variables would be bound to the
> empty sequence.
>
> Alternatively, there could be only one variable $zerr:data-error which is bound to
> either the empty sequence, an xml element, or a json object.
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/zorba/+bug/1111786/+subscriptions

I don't think we should change the ABI just for the heck of it. We've made ABI- and API-incompatible changes before, but we're "trying to be better".

I have a bug against me to introduce ABI checking somehow, as well as to start versioning our ABI (as reflected by libzorba version numbers, which currently are tied to Zorba releases but probably shouldn't be). Starting with Zorba 3.0 for sure we're going to be a lot more anal about that.

As for this specific bug: if it's somehow vital for Zorba or 28msec, I'm OK with making the change now. However, if it's not, I'd prefer to leave it targeted for 3.0 (which is very loosely scheduled for May/June).

It's extremely important for us. Having no way to retrieve the error location
of a parsed string in the program itself is really nasty.

In the future, we will have to make such decisions. That's the price we pay
if we put classes like XQueryException into the API.

Matthias

On Feb 5, 2013, at 10:31 AM, Chris Hillery <email address hidden> wrote:

> I don't think we should change the ABI just for the heck of it. We've
> made ABI- and API-incompatible changes before, but we're "trying to be
> better".
>
> I have a bug against me to introduce ABI checking somehow, as well as to
> start versioning our ABI (as reflected by libzorba version numbers,
> which currently are tied to Zorba releases but probably shouldn't be).
> Starting with Zorba 3.0 for sure we're going to be a lot more anal about
> that.
>
> As for this specific bug: if it's somehow vital for Zorba or 28msec, I'm
> OK with making the change now. However, if it's not, I'd prefer to leave
> it targeted for 3.0 (which is very loosely scheduled for May/June).
>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/1111786
>
> Title:
> xml/json parse error location in catch clause
>
> Status in Zorba - The XQuery Processor:
> In Progress
>
> Bug description:
> There should be a way to retrieve the error location of a xml/json parser error in a XQuery catch clause.
> One way of doing this would be to always declare and eventually bind the following variables:
>
> $zerr:data-line, $zerr:data-column, $zerr:data-uri
>
> For non-parser related errors, those variables would be bound to the
> empty sequence.
>
> Alternatively, there could be only one variable $zerr:data-error which is bound to
> either the empty sequence, an xml element, or a json object.
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/zorba/+bug/1111786/+subscriptions