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
[LLVM10] EventFilter fix warnings for clang 10 #29592
Conversation
The code-checks are being triggered in jenkins. |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-29592/14920
|
A new Pull Request was created by @mrodozov (Mircho Rodozov) for master. It involves the following packages: EventFilter/CSCRawToDigi @perrotta, @mommsen, @cmsbuild, @emeschi, @slava77 can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
please test |
The tests are being triggered in jenkins. |
+1 |
Comparison job queued. |
Comparison is ready Comparison Summary:
|
@@ -95,7 +95,7 @@ void RPCTwinMuxDigiToRaw::produce(edm::Event& event, edm::EventSetup const& setu | |||
|
|||
std::map<int, FEDRawData> fed_data; | |||
// Loop over the FEDs | |||
for (std::pair<int, std::vector<RPCAMCLink> > const& fed_amcs : fed_amcs_) { | |||
for (std::pair<int, std::vector<RPCAMCLink> > const fed_amcs : fed_amcs_) { |
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.
this is strange.
why is a simple const OK here? Isn't this going to trigger a copy of a vector?
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.
Most of this fixes are suggested by the compiler. It says you need copy here or your need a reference here, in this case it said remove the reference
https://cmssdt.cern.ch/SDT/cgi-bin/buildlogs/slc7_amd64_gcc820/CMSSW_11_1_CLANG_X_2020-04-27-2300/EventFilter/RPCRawToDigi
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.
@mrodozov , the correct fux is to use std::pair<const int
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.
@mrodozov , please remove this fix. It is already part of https://github.com/cms-sw/cmssw/pull/29572/files#diff-d409478036411a5bded20820a924b87a
please test |
The tests are being triggered in jenkins. |
+1 |
Comparison job queued. |
Comparison is ready Comparison Summary:
|
+1 Sorry for the delay in signing this PR. |
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. @silviodonato, @dpiparo (and backports should be raised in the release meeting by the corresponding L2) |
+1 |
PR description:
Fixes warnings in clang 10
PR validation:
Builds without warnings with CLANG IB
tokenArraySeparator and tokenArrayEnd belong to the same enum so thats probably or not and