Patch for C++ Warnings issued by Clang compiler
Attached are a sequence of patches to resolve compilation warnings (and hence errors with cetbuildtools
CAUTIOUS diagnostics) when using Clang to compile fhicl-cpp. I have separated by file to assist in evaluating the fixes as I realise Clang is not directly supported - though I believe the fixes to be genuine issues with C++ Standard and/or code cruft.
Note that some additional warnings were found, but I feel are due to Clang being overzealous:
- Use of source:fhiclcpp/type_traits.h triggers Clang's "mismatched-tags" warning due to fhicl-cpp objects being forward declared as structs when the physical declaration will be a class. I believe Clang adds this warning to their
-Walldiagnostic (hence it appearing with
CAUTIOUSdiagnostics) for back compatibility with older versions of MSVC that mangled structs and classes differently (see, for example https://bugzilla.mozilla.org/show_bug.cgi?id=780474). I've handled this by adding
-Wno-mismatched-tagsto cetbuildtools' flags when using Clang, which matches GCC's default behaviour and shouldn't affect anything as long as the Itanium ABI mangling scheme is used.
-Wmissing-bracewarnings are emitted for innocuous code such as source:fhiclcpp/types/Sequence.h#L86
- I think this is running into these bugs: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=25137 and https://llvm.org/bugs/show_bug.cgi?id=21629 which upstream Clang hasn't fixed yet.
- As with the previous warning, I've handled this by adding
-Wno-missing-bracesto the cetbuildtools set flags when using Clang to match the GCC behaviour.
So these additional "-Wno-" options are really things to be added in cetbuildtools as and when it supports Clang, so I just note the above issues here for information.
#3 Updated by Christopher Green over 3 years ago
- Status changed from Accepted to Resolved
- Target version set to 2.01.00
- % Done changed from 0 to 100
Implemented (with minor changes) with af25370 :
* af25370 - (HEAD -> master, origin/master, origin/HEAD) Merge branch 'Issue11982-ClangPatches' -- improvements for clang operation from Ben Morgan per issue #11982. (2 minutes ago) <Chris Green> |\ | * f638f97 - Add template decoration per issue #11982. (89 minutes ago) <Chris Green> | * c976d23 - Add missing override decoration per issue #11982. (2 hours ago) <Chris Green> | * b570f1b - Fix narrowing conversion warnings per issue #11982. (2 hours ago) <Chris Green> | * df5f801 - Use copy-list initialization per issue #11982. (2 hours ago) <Chris Green> | * 1c6f964 - Use explicit instance in OptionalTupleAs (2 hours ago) <Ben Morgan> | * 65d57d9 - Include <numeric> header for std::iota (2 hours ago) <Ben Morgan> | * 2b6770b - Add explicit initializer to const ParameterSet construction (2 hours ago) <Ben Morgan> | * ed9ebb6 - Remove unused local typedef (2 hours ago) <Ben Morgan> | * d736c7a - Fix potentially-evaluated-expression warning on Clang (2 hours ago) <Ben Morgan> | * f405634 - Remove unused private function "literal_nil" (2 hours ago) <Ben Morgan> |/ *
Patch 0012-Use-copy-list-initialization.patch failed to apply due to conflicts, so changes were made manually.
Narrowing conversions addressed in 0010-Fix-narrowing-and-oveeride-warnings-for-Clang.patch were fixed with
-1ull instead of
Override and template decoration changes in that same patch were made as separate commits.
Please let us know if there's anything we failed to address.