Re: 'make check' still fails

[Richard Heck]

On 11/24/2015 02:37 PM, Georg Baum wrote:
> Scott Kostyshak wrote:
>
>> On Sun, Nov 22, 2015 at 06:40:45PM +, Guillaume Munch wrote:
>>> Le 22/11/2015 17:38, Georg Baum a écrit :
 It is now also clear that the MSVC workaround is not needed, so I
 deleted it. Is the attached patch OK to go in?
>>> Looks good (I trust that the small details are ok). I have the same
>>> understanding. Isn't the "FIXME: Unicode" trivial to fix btw?
>> I would say commit then.
> Done. To my knowledge there are no known problems with std::regex anymore.

That may mean we are ready for a second alpha.

Richard

>
>
> Georg
>

Re: 'make check' still fails

[Georg Baum]

Scott Kostyshak wrote:

> On Sun, Nov 22, 2015 at 06:40:45PM +, Guillaume Munch wrote:
>> Le 22/11/2015 17:38, Georg Baum a écrit :
>> >It is now also clear that the MSVC workaround is not needed, so I
>> >deleted it. Is the attached patch OK to go in?
>> 
>> Looks good (I trust that the small details are ok). I have the same
>> understanding. Isn't the "FIXME: Unicode" trivial to fix btw?
> 
> I would say commit then.

Done. To my knowledge there are no known problems with std::regex anymore.


Georg

Re: 'make check' still fails

[Scott Kostyshak]

On Sun, Nov 22, 2015 at 06:40:45PM +, Guillaume Munch wrote:
> Le 22/11/2015 17:38, Georg Baum a écrit :
> >It is now also clear that the MSVC workaround is not needed, so I deleted
> >it. Is the attached patch OK to go in?
> 
> Looks good (I trust that the small details are ok). I have the same
> understanding. Isn't the "FIXME: Unicode" trivial to fix btw?

I would say commit then.

Scott


signature.asc
Description: PGP signature

Re: 'make check' still fails

[Georg Baum]

Guillaume Munch wrote:

> Looks good (I trust that the small details are ok). I have the same
> understanding. Isn't the "FIXME: Unicode" trivial to fix btw?

With C++11 and after docstring is changed to be std::u32string yes. I 
believe that it is possible, but far from easy and not worth the effort for 
the current docstring implementation on windows.


Georg

Re: 'make check' still fails

[Guillaume Munch]

Le 22/11/2015 17:42, Georg Baum a écrit :

Richard Heck wrote:


Actually, is there some reason we do not use QRegExp? My limited
research says it is in QtCore. We could certainly replace boost::regex
with that. I don't know if it would play nicer with std::regex or not,
but we could also just use it throughout.


I guess we could implement lyx::regex without boost or std and always use
QRegExp. However, this would require additional testing, and I do not think
that it is the right time now to do such a change. I believe the reason why
QRegExp is not used currently comes from the time when qt was forbidden
outside frontend.




Also, a quick google search shows that QRegExp (which by the way is
already used in parts of the code) is buggy and is being replaced by
another class in qt5. Also it seems to force conversions QString ↔
docstring.

Re: 'make check' still fails

[Guillaume Munch]

Le 22/11/2015 17:38, Georg Baum a écrit :

Guillaume Munch wrote:


Le 20/11/2015 19:43, Georg Baum a écrit :


It does. The reason is that the output of escape_special_chars() changes
when compiling with std::regex. The attached patch would fix the test for
std::regex, but then make check would fail when using boost::regex.


Of course, the source should be corrected, not the test.


Sure. This was to demonstrate the differences.


BTW I read the new documentation for tests and it did not help me at
all. It said nothing for "make check", and when I run "make check" it is
still a mystery to me where I can get any log file that says what went
wrong. (The file test_biblio.log is always empty.)


Yes, these tests are unfortunately not documented yet. Apart from that: The
autotools test machinery is too complicated. I do not know an equivalent for
make check, but you can run the equivalent of make alltests also with cmake
which I would recommend:

cd $BUILDTREE/src/test
make test


This shows that the regex engines we use do still interpret the
expressions in different ways, and I believe I also know the reason: The
expressions have changed to ECMAScript syntax, but the boost::regex
engine does not default to ECMAScript, but to boost perl style.


But "perl" for Boost is just a synonym for "ECMAScript", and the
difference between std and boost in the current master is only
supposed to be a few perl extensions added. (See


.)

Yes, I realized this now as well after reading the sources. I was lead on
the wrong track by the MSVC workaround in regex.h, and the fact that
boost::regex also has some undocumented features (like the one I got rid of
with dfe3a7d9fc57)


Therefore the problem is probably still in the expressions. What I
understand from your patch is that the output in std::regex is still
wrong: the regex adds \\ instead of \. The result of the test you showed
with the patch. I am worried by the line:

return lyx::regex_replace(expr, reg, "$&");

This should be:

return lyx::regex_replace(expr, reg, "\\$&");

but then this would no longer work with Boost, because it interprets the
backslash as an escape sequence... This appears to be a bug in Boost:

format_default: Specifies that when a regular expression match is to
be replaced by a new string, that the new string is constructed using
the rules used by the ECMAScript replace function in ECMA-262,
ECMAScript Language Specification, Chapter 15 part 5.4.11
String.prototype.replace. (FWD.1).
This is functionally identical to the Perl format string rules.

But the "Perl format" treats backslashes as a character one must escape,
which contradicts the preceding sentence.


The reference to ECMAScript may be wrong,


So the documentation is wrong.


but boost behaves as documented:
"Perl format string rules" in the original text at
http://www.boost.org/doc/libs/1_57_0/libs/regex/doc/html/boost_regex/ref/match_flag_type.html
is a link pointing to
http://www.boost.org/doc/libs/master/libs/regex/doc/html/boost_regex/format/perl_format.html.

That page states that a backslash represents the literal character following
the backslash, unless the following character is listed as exception. If we
read the expression "\$&" (as seen by the regex engine, in C++ source it
would be "\\$&") from left to right we see a backslash, followed by a dollar
sign. The dollar sign is not listed as exception => \$ is read as a literal
$, and the back reference $& is not recognized.

BTW, references to Perl syntax in boost::regex do not mean 100% perl
compatibilty, but the boost::regex "perl" flavour, which is "based on that
used by the programming language Perl" (see
http://www.boost.org/doc/libs/1_57_0/libs/regex/doc/html/boost_regex/syntax/perl_syntax.html).

std::regex behaves as documented as well: Escape sequences for backslashes
are not mentioned for the default format (see e.g.
http://www.cplusplus.com/reference/regex/regex_replace/ or
https://msdn.microsoft.com/en-us/library/bb982727.aspx), so backslashes are
always taken literally. I did some experiments with gcc 4.9 and MSVC
(compiler version: 19.00.23427.0(x86) using the online service at
http://webcompiler.cloudapp.net/ for the latter), and they support this
understanding.

Therefore, we simply have two regex implementations that treat the format
string of regex_replace() according to different rules. Fortunately, this
seems to be the only difference we need to care of.


On my side master still works fine with Boost so I don't see an inverted
situation. But, if we did the correct thing and changed the format
string to \$& as it should be, we would indeed end up in a symmetric
situation.


Yes, this was my misunderstanding that perl and ECMAScript would be
different. After much more reading and testing I see this now as well.


And my hope was that ECMAScript would 

Re: 'make check' still fails

[Georg Baum]

Guillaume Munch wrote:

> Le 22/11/2015 18:23, Richard Heck a écrit :
>>
>> Just one question: The regex that caused the original crash I reported
>> is all right now? I.e., no conditional compilation needs to be used for
>> it?

Yes, the crash was already fixed by Guillaume. This patch has nothing to do 
with it, it fixes an additional problem when using std::regex.

> That's my understanding (without having tested the patch). That is,
> valid regexps for std::regex are valid for boost. Only format strings
> used by std::regex_replace are possibly incompatible with boost.

That is my understanding as well.


Georg

Re: 'make check' still fails

[Guillaume Munch]

Le 22/11/2015 18:23, Richard Heck a écrit :

On 11/22/2015 12:38 PM, Georg Baum wrote:

It is now also clear that the MSVC workaround is not needed, so I deleted it. 
Is the attached patch OK to go in?


Just one question: The regex that caused the original crash I reported
is all right now? I.e., no conditional compilation needs to be used for it?



That's my understanding (without having tested the patch). That is, 
valid regexps for std::regex are valid for boost. Only format strings 
used by std::regex_replace are possibly incompatible with boost.

Re: 'make check' still fails

[Richard Heck]

On 11/22/2015 12:38 PM, Georg Baum wrote:
> It is now also clear that the MSVC workaround is not needed, so I deleted it. 
> Is the attached patch OK to go in?

Just one question: The regex that caused the original crash I reported
is all right now? I.e., no conditional compilation needs to be used for it?

I'm afraid I don't know this stuff well enough to have an informed
opinion as to whether the patch is correct.

Richard

Re: 'make check' still fails

[Georg Baum]

Richard Heck wrote:

> Actually, is there some reason we do not use QRegExp? My limited
> research says it is in QtCore. We could certainly replace boost::regex
> with that. I don't know if it would play nicer with std::regex or not,
> but we could also just use it throughout.

I guess we could implement lyx::regex without boost or std and always use 
QRegExp. However, this would require additional testing, and I do not think 
that it is the right time now to do such a change. I believe the reason why 
QRegExp is not used currently comes from the time when qt was forbidden 
outside frontend.


Georg

Re: 'make check' still fails

[Georg Baum]

Guillaume Munch wrote:

> Le 20/11/2015 19:43, Georg Baum a écrit :
>>
>> It does. The reason is that the output of escape_special_chars() changes
>> when compiling with std::regex. The attached patch would fix the test for
>> std::regex, but then make check would fail when using boost::regex.
> 
> Of course, the source should be corrected, not the test.

Sure. This was to demonstrate the differences.

> BTW I read the new documentation for tests and it did not help me at
> all. It said nothing for "make check", and when I run "make check" it is
> still a mystery to me where I can get any log file that says what went
> wrong. (The file test_biblio.log is always empty.)

Yes, these tests are unfortunately not documented yet. Apart from that: The 
autotools test machinery is too complicated. I do not know an equivalent for 
make check, but you can run the equivalent of make alltests also with cmake 
which I would recommend:

cd $BUILDTREE/src/test
make test

>> This shows that the regex engines we use do still interpret the
>> expressions in different ways, and I believe I also know the reason: The
>> expressions have changed to ECMAScript syntax, but the boost::regex
>> engine does not default to ECMAScript, but to boost perl style.
> 
> But "perl" for Boost is just a synonym for "ECMAScript", and the
> difference between std and boost in the current master is only
> supposed to be a few perl extensions added. (See
> 
.)

Yes, I realized this now as well after reading the sources. I was lead on 
the wrong track by the MSVC workaround in regex.h, and the fact that 
boost::regex also has some undocumented features (like the one I got rid of 
with dfe3a7d9fc57)

> Therefore the problem is probably still in the expressions. What I
> understand from your patch is that the output in std::regex is still
> wrong: the regex adds \\ instead of \. The result of the test you showed
> with the patch. I am worried by the line:
> 
>return lyx::regex_replace(expr, reg, "$&");
> 
> This should be:
> 
>return lyx::regex_replace(expr, reg, "\\$&");
> 
> but then this would no longer work with Boost, because it interprets the
> backslash as an escape sequence... This appears to be a bug in Boost:
> 
>format_default: Specifies that when a regular expression match is to
>be replaced by a new string, that the new string is constructed using
>the rules used by the ECMAScript replace function in ECMA-262,
>ECMAScript Language Specification, Chapter 15 part 5.4.11
>String.prototype.replace. (FWD.1).
>This is functionally identical to the Perl format string rules.
> 
> But the "Perl format" treats backslashes as a character one must escape,
> which contradicts the preceding sentence.

The reference to ECMAScript may be wrong, but boost behaves as documented: 
"Perl format string rules" in the original text at 
http://www.boost.org/doc/libs/1_57_0/libs/regex/doc/html/boost_regex/ref/match_flag_type.html
 
is a link pointing to 
http://www.boost.org/doc/libs/master/libs/regex/doc/html/boost_regex/format/perl_format.html.

That page states that a backslash represents the literal character following 
the backslash, unless the following character is listed as exception. If we 
read the expression "\$&" (as seen by the regex engine, in C++ source it 
would be "\\$&") from left to right we see a backslash, followed by a dollar 
sign. The dollar sign is not listed as exception => \$ is read as a literal 
$, and the back reference $& is not recognized.

BTW, references to Perl syntax in boost::regex do not mean 100% perl 
compatibilty, but the boost::regex "perl" flavour, which is "based on that 
used by the programming language Perl" (see 
http://www.boost.org/doc/libs/1_57_0/libs/regex/doc/html/boost_regex/syntax/perl_syntax.html).

std::regex behaves as documented as well: Escape sequences for backslashes 
are not mentioned for the default format (see e.g. 
http://www.cplusplus.com/reference/regex/regex_replace/ or 
https://msdn.microsoft.com/en-us/library/bb982727.aspx), so backslashes are 
always taken literally. I did some experiments with gcc 4.9 and MSVC 
(compiler version: 19.00.23427.0(x86) using the online service at 
http://webcompiler.cloudapp.net/ for the latter), and they support this 
understanding.

Therefore, we simply have two regex implementations that treat the format 
string of regex_replace() according to different rules. Fortunately, this 
seems to be the only difference we need to care of.

> On my side master still works fine with Boost so I don't see an inverted
> situation. But, if we did the correct thing and changed the format
> string to \$& as it should be, we would indeed end up in a symmetric
> situation.

Yes, this was my misunderstanding that perl and ECMAScript would be 
different. After much more reading and testing I see this now as well.

> I am af

Re: 'make check' still fails

[Richard Heck]

On 11/21/2015 10:40 AM, Richard Heck wrote:
> On 11/21/2015 10:19 AM, Guillaume Munch wrote:
>> Le 20/11/2015 19:43, Georg Baum a écrit :
>>> Scott Kostyshak wrote:
>>>
 I was hoping the recent commit fixing regex issues would fix the
 failing
 test (test_biblio) of 'make check'. I just wanted to confirm that the
 test fails for others on current master also.
>>> It does. The reason is that the output of escape_special_chars() changes
>>> when compiling with std::regex. The attached patch would fix the test
>>> for
>>> std::regex, but then make check would fail when using boost::regex.
>> Of course, the source should be corrected, not the test.
>>
>> BTW I read the new documentation for tests and it did not help me at
>> all. It said nothing for "make check", and when I run "make check" it is
>> still a mystery to me where I can get any log file that says what went
>> wrong. (The file test_biblio.log is always empty.)
>>
>>> This shows that the regex engines we use do still interpret the
>>> expressions
>>> in different ways, and I believe I also know the reason: The expressions
>>> have changed to ECMAScript syntax, but the boost::regex engine does not
>>> default to ECMAScript, but to boost perl style.
>> But "perl" for Boost is just a synonym for "ECMAScript", and the
>> difference between std and boost in the current master is only
>> supposed to be a few perl extensions added. (See
>> .)
>>
>>
>> Therefore the problem is probably still in the expressions. What I
>> understand from your patch is that the output in std::regex is still
>> wrong: the regex adds \\ instead of \. The result of the test you showed
>> with the patch. I am worried by the line:
>>
>>   return lyx::regex_replace(expr, reg, "$&");
>>
>> This should be:
>>
>>   return lyx::regex_replace(expr, reg, "\\$&");
>>
>> but then this would no longer work with Boost, because it interprets the
>> backslash as an escape sequence... This appears to be a bug in Boost:
>>
>>   format_default: Specifies that when a regular expression match is to
>>   be replaced by a new string, that the new string is constructed using
>>   the rules used by the ECMAScript replace function in ECMA-262,
>>   ECMAScript Language Specification, Chapter 15 part 5.4.11
>>   String.prototype.replace. (FWD.1).
>>   This is functionally identical to the Perl format string rules.
>>
>> But the "Perl format" treats backslashes as a character one must escape,
>> which contradicts the preceding sentence.
>>
>>> Therefore, we do now have
>>> exactly the inverted situation than before dbce5cafcc167: regexes are
>>> interpreted correctly when using std::regx, but not when using
>>> boost::regex.
>> On my side master still works fine with Boost so I don't see an inverted
>> situation. But, if we did the correct thing and changed the format
>> string to \$& as it should be, we would indeed end up in a symmetric
>> situation.
>>
>>> What needs to be done now is to call boost::regex in ECMAScript mode.
>>> Then
>>> escape_special_chars() will produce the same output for both regex
>>> engines.
>> As I understand, both are already set to ECMAScript, the problem being
>> that Boost does not implement ECMAScript.
>>
>>> What I am not 100% sure yet is whether it is correct, there seem to
>>> be too
>>> many backslashes.
>> I am afraid that for now there are only two unpleasant solutions:
>> * fix this particular replacement using a hack, or
> I think we have used conditional compilation for such things in the
> past. Ugly in the source, but it works.
>
>> * use Boost on every platform until we can get rid of it entirely.
> I'm guessing people will not like this option, but it would also work.

Actually, is there some reason we do not use QRegExp? My limited
research says it is in QtCore. We could certainly replace boost::regex
with that. I don't know if it would play nicer with std::regex or not,
but we could also just use it throughout.

Richard

Re: 'make check' still fails

[Richard Heck]

On 11/21/2015 10:19 AM, Guillaume Munch wrote:
> Le 20/11/2015 19:43, Georg Baum a écrit :
>> Scott Kostyshak wrote:
>>
>>> I was hoping the recent commit fixing regex issues would fix the
>>> failing
>>> test (test_biblio) of 'make check'. I just wanted to confirm that the
>>> test fails for others on current master also.
>>
>> It does. The reason is that the output of escape_special_chars() changes
>> when compiling with std::regex. The attached patch would fix the test
>> for
>> std::regex, but then make check would fail when using boost::regex.
>
> Of course, the source should be corrected, not the test.
>
> BTW I read the new documentation for tests and it did not help me at
> all. It said nothing for "make check", and when I run "make check" it is
> still a mystery to me where I can get any log file that says what went
> wrong. (The file test_biblio.log is always empty.)
>
>>
>> This shows that the regex engines we use do still interpret the
>> expressions
>> in different ways, and I believe I also know the reason: The expressions
>> have changed to ECMAScript syntax, but the boost::regex engine does not
>> default to ECMAScript, but to boost perl style.
>
> But "perl" for Boost is just a synonym for "ECMAScript", and the
> difference between std and boost in the current master is only
> supposed to be a few perl extensions added. (See
> .)
>
>
> Therefore the problem is probably still in the expressions. What I
> understand from your patch is that the output in std::regex is still
> wrong: the regex adds \\ instead of \. The result of the test you showed
> with the patch. I am worried by the line:
>
>   return lyx::regex_replace(expr, reg, "$&");
>
> This should be:
>
>   return lyx::regex_replace(expr, reg, "\\$&");
>
> but then this would no longer work with Boost, because it interprets the
> backslash as an escape sequence... This appears to be a bug in Boost:
>
>   format_default: Specifies that when a regular expression match is to
>   be replaced by a new string, that the new string is constructed using
>   the rules used by the ECMAScript replace function in ECMA-262,
>   ECMAScript Language Specification, Chapter 15 part 5.4.11
>   String.prototype.replace. (FWD.1).
>   This is functionally identical to the Perl format string rules.
>
> But the "Perl format" treats backslashes as a character one must escape,
> which contradicts the preceding sentence.
>
>> Therefore, we do now have
>> exactly the inverted situation than before dbce5cafcc167: regexes are
>> interpreted correctly when using std::regx, but not when using
>> boost::regex.
>
> On my side master still works fine with Boost so I don't see an inverted
> situation. But, if we did the correct thing and changed the format
> string to \$& as it should be, we would indeed end up in a symmetric
> situation.
>
>>
>> What needs to be done now is to call boost::regex in ECMAScript mode.
>> Then
>> escape_special_chars() will produce the same output for both regex
>> engines.
>
> As I understand, both are already set to ECMAScript, the problem being
> that Boost does not implement ECMAScript.
>
>> What I am not 100% sure yet is whether it is correct, there seem to
>> be too
>> many backslashes.
>
> I am afraid that for now there are only two unpleasant solutions:
> * fix this particular replacement using a hack, or

I think we have used conditional compilation for such things in the
past. Ugly in the source, but it works.

> * use Boost on every platform until we can get rid of it entirely.

I'm guessing people will not like this option, but it would also work.

Richard

Re: 'make check' still fails

[Guillaume Munch]

Le 20/11/2015 19:43, Georg Baum a écrit :

Scott Kostyshak wrote:


I was hoping the recent commit fixing regex issues would fix the failing
test (test_biblio) of 'make check'. I just wanted to confirm that the
test fails for others on current master also.


It does. The reason is that the output of escape_special_chars() changes
when compiling with std::regex. The attached patch would fix the test for
std::regex, but then make check would fail when using boost::regex.


Of course, the source should be corrected, not the test.

BTW I read the new documentation for tests and it did not help me at
all. It said nothing for "make check", and when I run "make check" it is
still a mystery to me where I can get any log file that says what went
wrong. (The file test_biblio.log is always empty.)



This shows that the regex engines we use do still interpret the expressions
in different ways, and I believe I also know the reason: The expressions
have changed to ECMAScript syntax, but the boost::regex engine does not
default to ECMAScript, but to boost perl style.


But "perl" for Boost is just a synonym for "ECMAScript", and the
difference between std and boost in the current master is only
supposed to be a few perl extensions added. (See
.)

Therefore the problem is probably still in the expressions. What I
understand from your patch is that the output in std::regex is still
wrong: the regex adds \\ instead of \. The result of the test you showed
with the patch. I am worried by the line:

  return lyx::regex_replace(expr, reg, "$&");

This should be:

  return lyx::regex_replace(expr, reg, "\\$&");

but then this would no longer work with Boost, because it interprets the
backslash as an escape sequence... This appears to be a bug in Boost:

  format_default: Specifies that when a regular expression match is to
  be replaced by a new string, that the new string is constructed using
  the rules used by the ECMAScript replace function in ECMA-262,
  ECMAScript Language Specification, Chapter 15 part 5.4.11
  String.prototype.replace. (FWD.1).
  This is functionally identical to the Perl format string rules.

But the "Perl format" treats backslashes as a character one must escape,
which contradicts the preceding sentence.


Therefore, we do now have
exactly the inverted situation than before dbce5cafcc167: regexes are
interpreted correctly when using std::regx, but not when using boost::regex.


On my side master still works fine with Boost so I don't see an inverted
situation. But, if we did the correct thing and changed the format
string to \$& as it should be, we would indeed end up in a symmetric
situation.



What needs to be done now is to call boost::regex in ECMAScript mode. Then
escape_special_chars() will produce the same output for both regex engines.


As I understand, both are already set to ECMAScript, the problem being
that Boost does not implement ECMAScript.


What I am not 100% sure yet is whether it is correct, there seem to be too
many backslashes.


I am afraid that for now there are only two unpleasant solutions:
* fix this particular replacement using a hack, or
* use Boost on every platform until we can get rid of it entirely.


Guillaume

Re: 'make check' still fails

[Georg Baum]

Richard Heck wrote:

> This is a bit worrying, and it makes me wonder about the wisdom of
> trying to support different regex engines. What was the reason not just
> to use the boost one? if only for consistency?

We want to get rid of the boost one. I think it is possible to use both in 
ECMAScript mode, and then it would not be any problem to have two 
implementations. Since we have unit tests for the regex stuff I am quite 
relaxed about unwanted differences: If we make sure that for the next 
release all unit tests pass on the main compilers, then we are very safe.


Georg

Re: 'make check' still fails

[Richard Heck]

On 11/20/2015 02:43 PM, Georg Baum wrote:
> Scott Kostyshak wrote:
>
>> I was hoping the recent commit fixing regex issues would fix the failing
>> test (test_biblio) of 'make check'. I just wanted to confirm that the
>> test fails for others on current master also.
> It does. The reason is that the output of escape_special_chars() changes 
> when compiling with std::regex. The attached patch would fix the test for 
> std::regex, but then make check would fail when using boost::regex.
>
> This shows that the regex engines we use do still interpret the expressions 
> in different ways, and I believe I also know the reason: The expressions 
> have changed to ECMAScript syntax, but the boost::regex engine does not 
> default to ECMAScript, but to boost perl style. Therefore, we do now have 
> exactly the inverted situation than before dbce5cafcc167: regexes are 
> interpreted correctly when using std::regx, but not when using boost::regex.
>
> What needs to be done now is to call boost::regex in ECMAScript mode. Then  
> escape_special_chars() will produce the same output for both regex engines. 
> What I am not 100% sure yet is whether it is correct, there seem to be too 
> many backslashes.

This is a bit worrying, and it makes me wonder about the wisdom of
trying to support different regex engines. What was the reason not just
to use the boost one? if only for consistency?

Richard

Re: 'make check' still fails

[Scott Kostyshak]

On Fri, Nov 20, 2015 at 08:43:10PM +0100, Georg Baum wrote:
> Scott Kostyshak wrote:
> 
> > I was hoping the recent commit fixing regex issues would fix the failing
> > test (test_biblio) of 'make check'. I just wanted to confirm that the
> > test fails for others on current master also.
> 
> It does. The reason is that the output of escape_special_chars() changes 
> when compiling with std::regex. The attached patch would fix the test for 
> std::regex, but then make check would fail when using boost::regex.
> 
> This shows that the regex engines we use do still interpret the expressions 
> in different ways, and I believe I also know the reason: The expressions 
> have changed to ECMAScript syntax, but the boost::regex engine does not 
> default to ECMAScript, but to boost perl style. Therefore, we do now have 
> exactly the inverted situation than before dbce5cafcc167: regexes are 
> interpreted correctly when using std::regx, but not when using boost::regex.
> 
> What needs to be done now is to call boost::regex in ECMAScript mode. Then  
> escape_special_chars() will produce the same output for both regex engines. 
> What I am not 100% sure yet is whether it is correct, there seem to be too 
> many backslashes.

Thanks for the explanation, Georg. Let me know if I should test the
patch.

Scott


signature.asc
Description: PGP signature

Re: 'make check' still fails

[Georg Baum]

Scott Kostyshak wrote:

> I was hoping the recent commit fixing regex issues would fix the failing
> test (test_biblio) of 'make check'. I just wanted to confirm that the
> test fails for others on current master also.

It does. The reason is that the output of escape_special_chars() changes 
when compiling with std::regex. The attached patch would fix the test for 
std::regex, but then make check would fail when using boost::regex.

This shows that the regex engines we use do still interpret the expressions 
in different ways, and I believe I also know the reason: The expressions 
have changed to ECMAScript syntax, but the boost::regex engine does not 
default to ECMAScript, but to boost perl style. Therefore, we do now have 
exactly the inverted situation than before dbce5cafcc167: regexes are 
interpreted correctly when using std::regx, but not when using boost::regex.

What needs to be done now is to call boost::regex in ECMAScript mode. Then  
escape_special_chars() will produce the same output for both regex engines. 
What I am not 100% sure yet is whether it is correct, there seem to be too 
many backslashes.


Georg
diff --git a/src/frontends/tests/regfiles/biblio b/src/frontends/tests/regfiles/biblio
index a3880d2..bb699f3 100644
--- a/src/frontends/tests/regfiles/biblio
+++ b/src/frontends/tests/regfiles/biblio
@@ -1,6 +1,6 @@
 abcd
 ab&cd
-\.\|\*\?\+\(\)\{\}\[\]\^\$"
-\.\.\|\|\*\*\?\?\+\+\(\(\)\)\{\{\}\}\[\[\]\]\^\^\$\$
+\\.\\|\\*\\?\\+\\(\\)\\{\\}\\[\\]\\^\\$"
+\\.\\.\\|\\|\\*\\*\\?\\?\\+\\+\\(\\(\\)\\)\\{\\{\\}\\}\\[\\[\\]\\]\\^\\^\\$\\$\\
 1
 0

'make check' still fails

[Scott Kostyshak]

I was hoping the recent commit fixing regex issues would fix the failing
test (test_biblio) of 'make check'. I just wanted to confirm that the
test fails for others on current master also.

Scott


signature.asc
Description: PGP signature