Feature #11982
Patch for C++ Warnings issued by Clang compiler
Description
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
-Wall
diagnostic (hence it appearing withCAUTIOUS
diagnostics) 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-tags
to 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. - Many
-Wmissing-brace
warnings 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-braces
to 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.
Associated revisions
Fix narrowing conversion warnings per issue #11982.
Add missing override decoration per issue #11982.
Add template decoration per issue #11982.
Merge branch 'Issue11982-ClangPatches' -- improvements for clang operation from Ben Morgan per issue #11982.
History
#1 Updated by Kyle Knoepfel almost 5 years ago
- Tracker changed from Bug to Feature
- Status changed from New to Accepted
- Estimated time set to 4.00 h
Thanks, Ben. We will review the patches and apply as necessary.
#2 Updated by Kyle Knoepfel almost 5 years ago
- Assignee set to Christopher Green
#3 Updated by Christopher Green almost 5 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 static_cast<std::size_t>(-1)
.
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.
#4 Updated by Kyle Knoepfel over 4 years ago
- Target version changed from 2.01.00 to 2.00.01
#5 Updated by Kyle Knoepfel over 4 years ago
- Status changed from Resolved to Closed
Use copy-list initialization per issue #11982.