Replace boost with std
replace boost::regex with std::regex, and boost::variant with std::variant
Merge request reports
Activity
added 1 commit
- 3d344ab1 - Replace boost::regex with std::regex, and boost::variant with std::variant
added 1 commit
- 420a698e - Replace boost::regex with std::regex, and boost::variant with std::variant
added 1 commit
- 14ae20b5 - Replace boost::regex with std::regex, and boost::variant with std::variant
added 1 commit
- 3ab51f5e - Replace boost::regex with std::regex, and boost::variant with std::variant
added 1 commit
- 22f06b8a - Replace boost::regex with std::regex, and boost::variant with std::variant
added 1 commit
- 8c05b740 - Replace boost::regex with std::regex, and boost::variant with std::variant
added 1 commit
- 0a015c62 - Replace boost::regex with std::regex, and boost::variant with std::variant
- [2019-02-26 07:06] Validation started with lhcb-tdr-test#466
- [2019-02-27 00:10] Validation started with lhcb-tdr-test#467
- [2019-02-28 00:07] Validation started with lhcb-tdr-test#468
- [2019-03-01 00:08] Validation started with lhcb-tdr-test#469
- [2019-03-02 00:08] Validation started with lhcb-tdr-test#470
- [2019-03-03 00:10] Validation started with lhcb-tdr-test#471
- [2019-03-04 00:07] Validation started with lhcb-tdr-test#472
- [2019-03-05 00:07] Validation started with lhcb-head#2165
- [2019-03-05 00:08] Validation started with lhcb-gaudi-head#2179
- [2019-03-05 00:08] Validation started with lhcb-lcg-dev4#828
- [2019-03-05 00:08] Validation started with lhcb-sanitizers#175
- [2019-03-05 00:08] Validation started with lhcb-lcg-dev3#822
- [2019-03-05 00:10] Validation started with lhcb-tdr-test#473
Edited by Software for LHCb@graven please have a look at the two failing tests in https://lhcb-nightlies.cern.ch/logs/tests/nightly/lhcb-tdr-test/466/x86_64-centos7-gcc8-opt/LHCb/
Interesting -- seems to be due to
LoKi::HLT::TurboPass::LoKi::HLT::TurboPass(const string& name) => Invalid special open parenthesis. (C++ exception of type regex_error)
which throws an exception because the regex
'^Hlt2.*(?<!TurboCalib)Decision$'
isn't recognized as a valid regex...Edited by Gerhard Ravenlooks like boost regex defaults to perl regex), whereas std regex assumes modified ECMAScript, which is very close, but not entirely the same...
The problematic regex uses '?<!' which in perl is a 'lookbehind' operation which basically says that in the matched piece between
Hlt2
and 'Decision' the patternTurboCalib
should not appear -- in ECMAScript this is done using assertions, in particular '?!', but then that doesn't do 'lookbehind' -- so presumably the pattern should be'^Hlt2(?!.*TurboCalib).*Decision$'
instead... looking at https://coliru.stacked-crooked.com/a/7e9128d68047246b that seems to do the right thing (provided I correctly understood the perl regex!), i.e. require a string which starts with
Hlt2
, then not followed by anything andTurboCalib
, and finally not ending inDecision
.Now, whether we can migrate from one regex grammar to another needs some thought about backwards compatibility (even if replacing a lookbehind with a forward looking assertion would probably be more efficient/faster).
What's the motivation for moving away from
boost::regex
, given that it supports this additional pattern (the negative lookbehind)?in general, I think it is a-priori preferable to use code from the ISO standard instead of a third-party library, i.e. boost, as the std version is typically more performant / future proof than boost. Often, the ISO standard evolves from the original code in boost, and in the process gets improved, and uses newer language features.
Now, in the case of regex support, that seems to have resulted in a version which has less features (albeit I think that I can still write the same functionality with a different, and somewhat 'cheaper' pattern -- see the linked example), at which point it may well be that it is better to stick to boost regex as it is a 'user facing' feature and we may not have the test coverage to be sure there are no regressions due to this. On the other hand, if we were to change, now (i.e. between run2 and run3) is probably the best time to do so...
Thanks for the explanation! I've no preference, though it does seem a bit arbitrary to me to move in this case (in which case why bother).
Anyway, on a (more?) pedantic point, your proposed regex isn't equivalent in intent. The original one rejects anything that ends in
TurboCalibDecision
, but will acceptTurboCalib
elsewhere, e.g. your test caseHlt2FooTurboCalibBarDecision
.I am very tempted to say that we drop this modernisation. The gain in going to the standard in this particular case seems more than offset by the migration effort, having to learn a slightly different syntax, and introducing possible time bombs due to the likely insufficient test coverage.