-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Fix LogDebug in HLTrigger/Muon package #25580
Conversation
The constructor of HLTMuonIsoFilter would not compile using clang's standard C++ library implementation. This fixes that problem and makes a few improvements. - Call LogDebug::log to only do work if logging is on this avoids the compilation problem by not trying to use '<<' on an std::ostringstream - Converted to range based for loop - Preallocate space in std::vector - Avoid unnecessary resetting of member variable
This fixes errors found when we run the clang static analyzer. |
The code-checks are being triggered in jenkins. |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-25580/7819 |
A new Pull Request was created by @Dr15Jones (Chris Jones) for master. It involves the following packages: HLTrigger/Muon @Martin-Grunewald, @cmsbuild, @fwyzard can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
please test |
The tests are being triggered in jenkins. |
It is not that the clang C++ standard library is different. What is happening is the static analyzer is testing using 'EDM_ML_DEBUG' to true and the LogDebug message was not compileable. |
There is no standard operator<< for std::vector. The code will now compile with EDM_ML_DEBUG compiler flag.
-Switch to LogDebug::log call and fix compilation problem when using EDM_ML_DEBUG -Call std::vector::reserve -Switch to using range based for
The code-checks are being triggered in jenkins. |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-25580/7821 |
The code-checks are being triggered in jenkins. |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-25580/7823 |
Pull request #25580 was updated. @Martin-Grunewald, @cmsbuild, @fwyzard can you please check and sign again. |
please test |
The tests are being triggered in jenkins. |
Comparison job queued. |
+1 |
This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @davidlange6, @slava77, @smuzaffar, @fabiocos (and backports should be raised in the release meeting by the corresponding L2) |
Comparison is ready Comparison Summary:
|
+1 |
The clang static analyzer appears to be using the compilation option EDM_ML_DEBUG as true. This forces actually compiling LogDebug which was failing for this package.