-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
Add treatment for C-style arrays in friendly class names #25477
Conversation
The code-checks are being triggered in jenkins. |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-25477/7582 |
A new Pull Request was created by @makortel (Matti Kortelainen) for master. It involves the following packages: FWCore/Utilities @cmsbuild, @smuzaffar, @Dr15Jones can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
@cmsbuild, please test |
The tests are being triggered in jenkins. |
Given |
(summarizing short discussion with @Dr15Jones for later reference) |
@Dr15Jones Sounds good (I used pointer as the example). |
@cmsbuild, please abort |
Jenkins tests are aborted. |
78181b3
to
2d312b7
Compare
The code-checks are being triggered in jenkins. |
Done (squashed to the same commit). |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-25477/7584 |
Pull request #25477 was updated. @cmsbuild, @smuzaffar, @Dr15Jones can you please check and sign again. |
please test |
The tests are being triggered in jenkins. |
+1 |
This pull request is fully signed and it will be integrated in one of the next master IBs after it passes the integration tests. 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 job queued. |
I realized that this one is not sufficient to solve the problem I had in hand, but I end to infinite recursion because of |
Comparison is ready Comparison Summary:
|
@makortel not sure how to interpret you last remark: are you still willing to move forward with this code as approved by @Dr15Jones or are you having in mind some further update to this PR? |
@fabiocos We discussed with @Dr15Jones and came to the conclusion that it's better to have this PR in as it covers a corner case not covered before. |
+1 |
I noticed that product class names like
bar<foo<int[]>>
lead to infinite loop within the friendly name system (to reproduce runedmToFriendlyClassName "bar<foo<int[]>>"
). I suppose that happens because the brackets[]
have a special meaning in the regex syntax, and this replacementcmssw/FWCore/Utilities/src/FriendlyName.cc
Lines 123 to 125 in 623eb35
ends up doing nothing when
theSub
isint[]
. Rather than to mess with the regexes, this PR suggests to add a "standard rename" for C-style arrays.Tested in 10_4_0_pre3, no changes expected.