-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove boost lexical_cast dependency in EventFilter #35168
Remove boost lexical_cast dependency in EventFilter #35168
Conversation
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35168/25096
|
A new Pull Request was created by @Purva-Chaudhari for master. It involves the following packages:
@emeschi, @smorovic, @cmsbuild, @rekovic, @slava77, @jpata, @cecilecaillol can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
return boost::lexical_cast<int>(data); | ||
} catch (boost::bad_lexical_cast const& e) { | ||
return std::stoi(data); | ||
} catch (const std::exception& e) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @Purva-Chaudhari,
std::runtime
and std::exception
are caught below this catch statement (lines 1343 and 1348), so this would make them always ignored.
It would be better instead to catch specifically std::invalid_argument
and std::out_of_range
, both of which can be thrown by std::stoi
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh alright thanks, I will fix it
@@ -1432,7 +1431,7 @@ namespace evf { | |||
// Another process grabbed the file and NFS did not register this | |||
unlockFULocal(); | |||
edm::LogError("EvFDaqDirector") << "grabNextFile runtime Exception -: " << e.what(); | |||
} catch (boost::bad_lexical_cast const&) { | |||
} catch (const std::out_of_range&) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
std::invalid_argument
should also be also caught in addition to std::out_of_range
to keep the same behavior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, adding it
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35168/25098
|
+1 |
@cmsbuild please test |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-36eb2d/18343/summary.html Comparison SummarySummary:
|
merge |
PR description:
Remove boost lexical cast dependency in EventFilter with corresponding stl alternatives.
PR validation:
Passed runtests
if this PR is a backport:
@davidlange6 @vgvassilev