[ALCA] [GCC12] Fix build warnings #39520

smuzaffar · 2022-09-28T10:42:00Z

This should fix the compilation warnings for GCC12 IBs

In file included from ...lcg/root/6.24.07-b2cee796fd454a5d25662f56310655d2/include/TNamed.h:26,
                 from ...lcg/root/6.24.07-b2cee796fd454a5d25662f56310655d2/include/TSystem.h:34,
                 from ...cms/cmssw/CMSSW_12_6_X_2022-09-24-1100/src/CalibCalorimetry/EcalCorrelatedNoiseAnalysisAlgos/interface/TEcnaGui.h:5,
                 from ...cms/cmssw/CMSSW_12_6_X_2022-09-24-1100/src/CalibCalorimetry/EcalCorrelatedNoiseAnalysisAlgos/src/TEcnaGui.cc:5:
In member function 'TString& TString::Append(const char*)',
    inlined from 'void TEcnaGui::SubmitOnBatchSystem(const TString&)' at ...cms/cmssw/CMSSW_12_6_X_2022-09-24-1100/src/CalibCalorimetry/EcalCorrelatedNoiseAnalysisAlgos/src/TEcnaGui.cc:6254:30:
  ...lcg/root/6.24.07-b2cee796fd454a5d25662f56310655d2/include/TString.h:565:46: warning: dangling pointer to an unnamed temporary may be used [-Wdangling-pointer=]
   565 | { return Replace(Length(), 0, cs, cs ? strlen(cs) : 0); }

cmsbuild · 2022-09-28T10:48:00Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-39520/32284

This PR adds an extra 80KB to repository

cmsbuild · 2022-09-28T10:48:22Z

A new Pull Request was created by @smuzaffar (Malik Shahzad Muzaffar) for master.

It involves the following packages:

CalibCalorimetry/EcalCorrelatedNoiseAnalysisAlgos (alca)

@malbouis, @yuanchao, @cmsbuild, @saumyaphor4252, @francescobrivio, @ChrisMisan, @tvami can you please review it and eventually sign? Thanks.
@rchatter, @tocheng, @argiro, @thomreis, @simonepigazzini, @mmusich this is something you requested to watch as well.
@perrotta, @dpiparo, @rappoccio you are the release manager for this.

cms-bot commands are listed here

smuzaffar · 2022-09-28T11:01:41Z

please test

tvami · 2022-09-28T12:20:06Z

FYI @thomreis

cmsbuild · 2022-09-28T17:31:45Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-1ca198/27812/summary.html
COMMIT: fb72f32
CMSSW: CMSSW_12_6_X_2022-09-27-2300/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/39520/27812/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 8 differences found in the comparisons
DQMHistoTests: Total files compared: 51
DQMHistoTests: Total histograms compared: 3624368
DQMHistoTests: Total failures: 6
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3624340
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 50 files compared)
Checked 212 log files, 49 edm output root files, 51 DQM output files
TriggerResults: no differences found

ChrisMisan · 2022-09-28T18:14:01Z

CalibCalorimetry/EcalCorrelatedNoiseAnalysisAlgos/interface/TEcnaParPaths.h

+  const TString &ResultsRootFilePath() const;
+  const TString &ResultsAsciiFilePath() const;
+  const TString &HistoryRunListFilePath() const;
+  const TString &CMSSWBase() const;
+  const TString &CMSSWSubsystem() const;
+  const TString &SCRAMArch() const;


this is just a minor issue but I think this should either be

const TString & ResultsRootFilePath()

or

const TString& ResultsRootFilePath()

Like it is now, "&" is attached to a function name which I see as a little bit confusing.

ChrisMisan · 2022-09-28T18:14:13Z

CalibCalorimetry/EcalCorrelatedNoiseAnalysisAlgos/interface/TEcnaRead.h

+  const TString &GetStartDate() const;
+  const TString &GetStopDate() const;
+  const TString &GetRootFileName() const;
+  const TString &GetRootFileNameShort() const;


ChrisMisan · 2022-09-28T18:14:23Z

CalibCalorimetry/EcalCorrelatedNoiseAnalysisAlgos/src/TEcnaParPaths.cc

+const TString &TEcnaParPaths::ResultsRootFilePath() const { return fCfgResultsRootFilePath; }
+const TString &TEcnaParPaths::ResultsAsciiFilePath() const { return fCfgResultsAsciiFilePath; }
+const TString &TEcnaParPaths::HistoryRunListFilePath() const { return fCfgHistoryRunListFilePath; }
+const TString &TEcnaParPaths::CMSSWBase() const { return fCfgCMSSWBase; }
+const TString &TEcnaParPaths::CMSSWSubsystem() const { return fCfgCMSSWSubsystem; }
+const TString &TEcnaParPaths::SCRAMArch() const { return fCfgSCRAMArch; }


ChrisMisan · 2022-09-28T18:14:33Z

CalibCalorimetry/EcalCorrelatedNoiseAnalysisAlgos/src/TEcnaRead.cc

+const TString &TEcnaRead::GetStartDate() const { return fFileHeader->fStartDate; }
+const TString &TEcnaRead::GetStopDate() const { return fFileHeader->fStopDate; }


ChrisMisan · 2022-09-28T18:14:42Z

CalibCalorimetry/EcalCorrelatedNoiseAnalysisAlgos/src/TEcnaRead.cc

+const TString &TEcnaRead::GetRootFileName() const { return fCnaWrite->GetRootFileName(); }
+const TString &TEcnaRead::GetRootFileNameShort() const { return fCnaWrite->GetRootFileNameShort(); }


ChrisMisan · 2022-09-28T18:14:52Z

CalibCalorimetry/EcalCorrelatedNoiseAnalysisAlgos/src/TEcnaRead.cc

@@ -815,7 +815,7 @@ void TEcnaRead::FileParameters(const TString &typ_ana,
 //   GetLastReqEvtNumber, GetReqNbOfEvts, GetStexNumber
 //
 //=========================================================================
-TString TEcnaRead::GetAnalysisName() { return fFileHeader->fTypAna; }
+const TString &TEcnaRead::GetAnalysisName() const { return fFileHeader->fTypAna; }


smuzaffar · 2022-09-28T18:22:51Z

thanks @ChrisMisan , I will update.

smuzaffar · 2022-09-28T18:54:04Z

@ChrisMisan , I tried both your suggestion const TString& NAME and const TString & NAME and re-ran code-checks. As clang-format follow the style of the code unit so in this case clang-format recommanded to use const TSTring &NAME . As code checks passed so I would suggest to approve these changes as it is. If we really want to use const TString& NAME then I would suggest to open a new PR to fix the whole code unit.

ChrisMisan · 2022-09-28T19:02:25Z

@smuzaffar I guess it's fine, for my own knowledge, what is this "code unit" you've mentioned?

ChrisMisan · 2022-09-28T19:05:01Z

+alca

differences only in msgLogger

cmsbuild · 2022-09-28T19:05:24Z

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. @perrotta, @dpiparo, @rappoccio (and backports should be raised in the release meeting by the corresponding L2)

smuzaffar · 2022-09-28T19:09:53Z

code unit means each individual source file :-) e.g. for

TEcnaParPaths.h clang-format suggested const TSTring &NAME style because that is the style used in this file
TEcnaRun.h clang-format suggested const TString& NAME style

perrotta · 2022-09-28T21:07:49Z

+1

[ALCA] [GCC12] Fix build warnings

fb72f32

cmsbuild added this to the CMSSW_12_6_X milestone Sep 28, 2022

cmsbuild added alca-pending code-checks-pending orp-pending pending-signatures tests-pending labels Sep 28, 2022

cmsbuild added code-checks-approved and removed code-checks-pending labels Sep 28, 2022

cmsbuild added tests-started and removed tests-pending labels Sep 28, 2022

cmsbuild added tests-approved and removed tests-started labels Sep 28, 2022

ChrisMisan reviewed Sep 28, 2022

View reviewed changes

cmsbuild added alca-approved fully-signed and removed alca-pending pending-signatures labels Sep 28, 2022

cmsbuild added orp-approved and removed orp-pending labels Sep 28, 2022

cmsbuild merged commit d06beb0 into cms-sw:master Sep 28, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ALCA] [GCC12] Fix build warnings #39520

[ALCA] [GCC12] Fix build warnings #39520

smuzaffar commented Sep 28, 2022

cmsbuild commented Sep 28, 2022

cmsbuild commented Sep 28, 2022

smuzaffar commented Sep 28, 2022

tvami commented Sep 28, 2022

cmsbuild commented Sep 28, 2022

ChrisMisan Sep 28, 2022

ChrisMisan Sep 28, 2022

ChrisMisan Sep 28, 2022

ChrisMisan Sep 28, 2022

ChrisMisan Sep 28, 2022

ChrisMisan Sep 28, 2022

smuzaffar commented Sep 28, 2022

smuzaffar commented Sep 28, 2022

ChrisMisan commented Sep 28, 2022

ChrisMisan commented Sep 28, 2022

cmsbuild commented Sep 28, 2022

smuzaffar commented Sep 28, 2022 •

edited

Loading

perrotta commented Sep 28, 2022

		const TString &TEcnaRead::GetStartDate() const { return fFileHeader->fStartDate; }
		const TString &TEcnaRead::GetStopDate() const { return fFileHeader->fStopDate; }

		const TString &TEcnaRead::GetRootFileName() const { return fCnaWrite->GetRootFileName(); }
		const TString &TEcnaRead::GetRootFileNameShort() const { return fCnaWrite->GetRootFileNameShort(); }

[ALCA] [GCC12] Fix build warnings #39520

[ALCA] [GCC12] Fix build warnings #39520

Conversation

smuzaffar commented Sep 28, 2022

cmsbuild commented Sep 28, 2022

cmsbuild commented Sep 28, 2022

smuzaffar commented Sep 28, 2022

tvami commented Sep 28, 2022

cmsbuild commented Sep 28, 2022

Comparison Summary

ChrisMisan Sep 28, 2022

Choose a reason for hiding this comment

ChrisMisan Sep 28, 2022

Choose a reason for hiding this comment

ChrisMisan Sep 28, 2022

Choose a reason for hiding this comment

ChrisMisan Sep 28, 2022

Choose a reason for hiding this comment

ChrisMisan Sep 28, 2022

Choose a reason for hiding this comment

ChrisMisan Sep 28, 2022

Choose a reason for hiding this comment

smuzaffar commented Sep 28, 2022

smuzaffar commented Sep 28, 2022

ChrisMisan commented Sep 28, 2022

ChrisMisan commented Sep 28, 2022

cmsbuild commented Sep 28, 2022

smuzaffar commented Sep 28, 2022 • edited Loading

perrotta commented Sep 28, 2022

smuzaffar commented Sep 28, 2022 •

edited

Loading