Draft: CxxUtils: remove suppression of preprocessorErrorDirective

Remove the suppression of the preprocessor error in CachedPointer.h and instead define the appropriate compiler variable. Not having this defined (and the error being suppressed) is one of the reasons why cppcheck was blind to a few defects in our builds (see e.g. !76484 (merged)).

cc @sroe

added full-build label

build area is cleaned as MR is labeled as full-build

This merge request affects 1 package:

• Control/CxxUtils

This merge request affects 3 files:

• Control/CxxUtils/CxxUtils/CachedPointer.h
• Control/CxxUtils/CxxUtils/features.h
• Control/CxxUtils/cmake/CxxUtilsSettingsConfig.cmake

Adding @ssnyder as watcher

added Core main review-pending-level-1 labels

CI Result SUCCESS (hash 9cb93855)

	Athena	AthSimulation	AthGeneration	AnalysisBase	AthAnalysis	DetCommon
externals
cmake
make
tests

Full details available on this CI monitor view. Check the JIRA CI status board for known problems
Athena: number of compilation errors 0, warnings 281
AthSimulation: number of compilation errors 0, warnings 0
AthGeneration: number of compilation errors 0, warnings 0
AnalysisBase: number of compilation errors 0, warnings 0
AthAnalysis: number of compilation errors 0, warnings 0
DetCommon: number of compilation errors 0, warnings 0
For experts only: Jenkins output (remote access info)

@fwinkl : so this adds ~ 280 defects? :-|

And even worse, the full build now takes 4h50 instead of 2h10. Looks like we were ignoring a huge part of our code in cppcheck so far. This will need some thinking...

...it might be that only 4-5 files are taking all that extra time; prime candidates are: CalibrateIBL, MonitoringFile_RPCPostProcess,SUSYCrossSection, validateDigitization

Oh, and the primary one which I always exclude: CudaFitter.cpp

I somewhat doubt it. E.g. compiling/checking AthContainers now takes almost twice as long and I see similar patterns across the board. The offending #error was in CachedPointer.h which is used in ElementLinkBase.h. That means a huge portion of our code-base was not being checked at all.

I think part of the problem is that when cppcheck is used within cmake, it sees a huge amount of athena headers which is not the case if you run it standalone (unless you specify them via -I). Another problem could be the large amount of preprocessor #define we have in our code-base and which cppcheck tries to check many permutations of. We could experiment with lowering --max-configs. Or we may have to fix some of them explicitly via -D.

FWIW, i add -DATOMIC_POINTER_LOCK_FREE=2 to the command line when i run cppcheck. I guess we could also test __CPPCHECK__ in CachedPointer.h.

It might be worth generally omitting the 'missingReturn' defect: cppcheck is giving this as a false positive on some templated constructors, and the compiler warns of this anyway.

Sure. But that will not fix the performance problems. If we cannot solve this, I am afraid we'll have to disable cppcheck again in our builds.

It would significantly complicate things, perhaps, but cppcheck can use a folder of previously cached results and do incremental scans, with the --cppcheck-build-dir=cachedir option.

removed review-pending-level-1 label

added review-user-action-required label

marked this merge request as draft

mentioned in merge request !76523 (merged)

mentioned in merge request !76524 (merged)

mentioned in merge request !76527 (merged)

mentioned in merge request !76528 (merged)

mentioned in merge request !76529 (merged)

mentioned in merge request !76533 (merged)

mentioned in merge request !76535 (merged)

mentioned in merge request !76536 (merged)

mentioned in merge request !76539 (merged)

mentioned in merge request !76540 (merged)

mentioned in merge request !76542 (merged)

mentioned in merge request !76544 (merged)

mentioned in merge request !76547 (merged)

mentioned in merge request !76554 (merged)

mentioned in merge request !76557 (merged)

mentioned in merge request !76559 (merged)

removed review-user-action-required label

mentioned in merge request !76578 (merged)

mentioned in merge request !76579 (closed)

mentioned in merge request !76580 (merged)

mentioned in merge request !76581 (merged)

mentioned in merge request !76590 (merged)

mentioned in merge request !76592 (merged)

Jenkins please retry a build

build area is cleaned as MR is labeled as full-build

This merge request affects 1 package:

• Control/CxxUtils

This merge request affects 3 files:

• Control/CxxUtils/CxxUtils/CachedPointer.h
• Control/CxxUtils/CxxUtils/features.h
• Control/CxxUtils/cmake/CxxUtilsSettingsConfig.cmake

Adding @ssnyder as watcher

CI Result SUCCESS (hash 9cb93855)

	Athena	AthSimulation	AthGeneration	AnalysisBase	AthAnalysis	DetCommon
externals
cmake
make
tests

Full details available on this CI monitor view. Check the JIRA CI status board for known problems
Athena: number of compilation errors 0, warnings 228
AthSimulation: number of compilation errors 0, warnings 0
AthGeneration: number of compilation errors 0, warnings 0
AnalysisBase: number of compilation errors 0, warnings 0
AthAnalysis: number of compilation errors 0, warnings 0
DetCommon: number of compilation errors 0, warnings 0
For experts only: Jenkins output (remote access info)

mentioned in merge request !76615 (merged)

I can take the Reconstruction domain defects, but I will be slower than the initial push on the InnerDetector ones; I'll clean up any remaining InnerDetector ones first.

mentioned in merge request !76616 (merged)

mentioned in merge request !76617 (merged)

mentioned in merge request !76619 (merged)

mentioned in merge request !76620 (merged)

mentioned in merge request !76621 (merged)

mentioned in merge request !76625 (merged)

added 269 commits

• 9cb93855...d3325dd5 - 268 commits from branch atlas:main
• 4e155962 - CachedPointer: remove preprocessor error suppression

Compare with previous version

mentioned in merge request !76627 (merged)

mentioned in merge request !76629 (merged)

mentioned in merge request !76632 (merged)

mentioned in merge request !76634 (merged)

mentioned in merge request !76636 (merged)