Hidden clang-tidy warnings in code

[ClausKlein]

When I run clang-tidy over src/example with clang-11 I get a lot of warnings?

run-clang-tidy -header-filter='/Users/clausklein/Workspace/cpp/taocpp-json/include/tao/json/.*\.hpp' -checks='-*non-private-member-*' src/example 
# ...

Claus-iMac:taocpp-json clausklein$ egrep -vw '(ubjson|external|jaxn|cbor|msgpack|events)' builddriver.log 
WarningErrorEntry(path='/Users/clausklein/Workspace/cpp/taocpp-json/include/tao/json/span.hpp', lineno='337', severity='warning', message="function 'front' should be marked [[nodiscard]] [modernize-use-nodiscard]", column='7')
WarningErrorEntry(path='/Users/clausklein/Workspace/cpp/taocpp-json/include/tao/json/span.hpp', lineno='343', severity='warning', message="function 'back' should be marked [[nodiscard]] [modernize-use-nodiscard]", column='7')
WarningErrorEntry(path='/Users/clausklein/Workspace/cpp/taocpp-json/include/tao/json/span.hpp', lineno='349', severity='warning', message="function 'data' should be marked [[nodiscard]] [modernize-use-nodiscard]", column='7')
WarningErrorEntry(path='/Users/clausklein/Workspace/cpp/taocpp-json/include/tao/json/span.hpp', lineno='354', severity='warning', message="function 'begin' should be marked [[nodiscard]] [modernize-use-nodiscard]", column='7')
WarningErrorEntry(path='/Users/clausklein/Workspace/cpp/taocpp-json/include/tao/json/span.hpp', lineno='359', severity='warning', message="function 'end' should be marked [[nodiscard]] [modernize-use-nodiscard]", column='7')
WarningErrorEntry(path='/Users/clausklein/Workspace/cpp/taocpp-json/include/tao/json/span.hpp', lineno='364', severity='warning', message="function 'rbegin' should be marked [[nodiscard]] [modernize-use-nodiscard]", column='7')
WarningErrorEntry(path='/Users/clausklein/Workspace/cpp/taocpp-json/include/tao/json/span.hpp', lineno='369', severity='warning', message="function 'rend' should be marked [[nodiscard]] [modernize-use-nodiscard]", column='7')
WarningErrorEntry(path='/Users/clausklein/Workspace/cpp/taocpp-json/include/tao/json/span.hpp', lineno='399', severity='warning', message="function 'first' should be marked [[nodiscard]] [modernize-use-nodiscard]", column='7')
WarningErrorEntry(path='/Users/clausklein/Workspace/cpp/taocpp-json/include/tao/json/span.hpp', lineno='406', severity='warning', message="function 'last' should be marked [[nodiscard]] [modernize-use-nodiscard]", column='7')
WarningErrorEntry(path='/Users/clausklein/Workspace/cpp/taocpp-json/include/tao/json/span.hpp', lineno='413', severity='warning', message="function 'subspan' should be marked [[nodiscard]] [modernize-use-nodiscard]", column='7')
WarningErrorEntry(path='/Users/clausklein/Workspace/cpp/taocpp-json/include/tao/json/binding/internal/../../basic_value.hpp', lineno='927', severity='warning', message="function 'variant' should be marked [[nodiscard]] [modernize-use-nodiscard]", column='7')
Claus-iMac:taocpp-json clausklein$ grep -vw external builddriver.log | wc
     217    3515   89961
Claus-iMac:taocpp-json clausklein$ 

[ClausKlein]

and what is about the 217 other not in external?

[d-frey]

I had to disable the warnings for span as this is modeled after the standard and those functions are not marked [[nodiscard]]. Not sure which version of clang-tidy you were using, but that might explain why you got a few more warnings while the CI job stayed silent.

Note that we are using a .clang-tidy file to define which clang-tidy checks are supported and which ones are not. In "external" you will find a lot of reports of wrong header guards, but that is because we copy the PEGTL and run a simple sed-command to replace macro prefixes. That said, as a user one should not be required to run clang-tidy on our code. If you want to address specific improvements that we can implement in the CI system, please be specific and we are happy to discuss the issues.

[ClausKlein]

I fix the warnings about header guards to focus me on the other warnings. "external" is code from tao too, why are different QA rules used?

Claus-iMac:taocpp-json clausklein$ clang-tidy --version LLVM (http://llvm.org/): LLVM version 11.0.0 Optimized build. Default target: x86_64-apple-darwin20.2.0 Host CPU: skylake Claus-iMac:taocpp-json clausklein$

[d-frey]

On GitHub Actions we use clang-tidy v10, you are using v11 (and the version from Apple, which might be modified). That might explain small differences.

For fixing the include guards: Please don't, I will not accept a PR like that. If they get in your way, why not just disable this one check?

To explain why I won't "fix" the header guards to pass the clang-tidy check here: The reason for a header-guard check is to prevent typos or copy-paste-errors. The header guards of the embedded PEGTL are already tested for those issues in the main project, https://github.com/taocpp/PEGTL. The embedded version that is copied into taoJSON is adapted with an sed-command, there is no potential for those errors here. Running the clang-tidy check for header guards is therefore counter-productive. In fact, having to make a distinction between header-guards and other macros just to "fix" those clang-tidy warnings only adds complexity and therefore potential for actual errors.

Likewise, warnings for the other external code (ryu, ...) will also only lead to more differences between the code's original form and our version. We try to minimize the potential for errors here. If you want to improve the code quality of any of the external libraries, it should be done at the origin where the code is coming from - not on the copy in taoJSON's external/ path.

[ClausKlein]

I know this, it is only to show the problem. after merging the .clang-tidy config with PEGTL try to upload the log ...

see https://raw.githubusercontent.com/ClausKlein/json/develop/.run-clang-tidy.log

P.S: the clang is original LLVM installed with brew.