Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

clang-tidy gives warnings of unused variables in test cases when compiled without doctest #691

Open
garyo opened this issue Aug 30, 2022 · 3 comments · May be fixed by #708
Open

clang-tidy gives warnings of unused variables in test cases when compiled without doctest #691

garyo opened this issue Aug 30, 2022 · 3 comments · May be fixed by #708

Comments

@garyo
Copy link

garyo commented Aug 30, 2022

Description

clang-tidy gives warnings of unused variables in test cases when compiled without doctest.

Clang-tidy uses some clever methods to omit test cases from the binary when compiled with DOCTEST_CONFIG_DISABLE, but the test case code is still present (not #ifdefed out) so clang-tidy still sees it, and since the CHECKs are compiled out, it sees unused vars.

We run clang-tidy with warnings as errors, so this causes clang-tidy failures for us.

Steps to reproduce

TEST_CASE("foo") {
  int i = 100;
  CHECK(i == 100);
}

When compiled with DOCTEST_CONFIG_DISABLE, the test case function declares an unused variable i, which clang-tidy complains about. I'm not sure if anything can be done about this; doctest can insert a // NOLINTBEGIN but it can't insert the final // NOLINTEND for the same reason it can't just #ifdef out the whole test -- there's no marker for the test end.

Extra information

  • doctest version: v2.4.8
  • Operating System: MacOS 12.5.1
  • Compiler+version: Apple clang 
    Apple clang version 13.1.6 (clang-1316.0.21.2.5)
Target: arm64-apple-darwin21.6.0
Thread model: posix
InstalledDir: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin
@Saalvage Saalvage linked a pull request Oct 4, 2022 that will close this issue
Open
@Saalvage Saalvage linked a pull request Oct 4, 2022 that will close this issue
Clean clang-tidy with DOCTEST_CONFIG_DISABLE #708
Open
@Saalvage
Copy link
Member

Saalvage commented Oct 4, 2022

I was originally looking into introducing a custom config option to consume the input of the macro invocations, but on second thought the combination of defining DOCTEST_CONFIG_EVALUATE_ASSERTS_EVEN_WHEN_DISABLED and DOCTEST_CONFIG_ASSERTS_RETURN_VALUES should be able to accomplish the wanted functionality. The only downside to that approach is that all usages of THROWS_WITH macros result in compilation errors, although that can be fixed once the general idea can be confirmed to be working.
Could you try this solution and report back?

@garyo
Copy link
Author

garyo commented Oct 10, 2022

It's closer, but not quite there. Try the following code:

#include "doctest/doctest.h"
#include <string>

static std::string read_file(const std::string &filename) {
  return filename;              // just for quick test
}

TEST_CASE("export") {
    auto input = "test1";
    CHECK(input == read_file(input));
}

Compile like this:

cc -c -std=c++20 -Wall -Wpedantic -Werror -g -DDOCTEST_CONFIG_DISABLE -DDOCTEST_CONFIG_EVALUATE_ASSERTS_EVEN_WHEN_DISABLED -DDOCTEST_CONFIG_ASSERTS_RETURN_VALUES -I/path/to/doctest foo.cpp

On my MacOS machine (Apple clang version 14.0.0 (clang-1400.0.29.102)), this gives the following error:

foo.cpp:4:20: error: function 'read_file' is not needed and will not be emitted [-Werror,-Wunneeded-internal-declaration]
static std::string read_file(const std::string &filename) {
                   ^
1 error generated.

If I remove read_file altogether, I get a different error:

foo.cpp:6:20: error: use of undeclared identifier 'read_file'
    CHECK(input == read_file(input));
                   ^

So whether I include read_file or not, I get an error either way. (And yes, I know the error is pedantic. Sorry about that.)

@bilderbuchi bilderbuchi mentioned this issue Mar 17, 2023
Open
@bilderbuchi
Copy link

Any news on this? I also seem to be hitting this problem (#748).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Clean clang-tidy with DOCTEST_CONFIG_DISABLE doctest/doctest
3 participants
@garyo @bilderbuchi @Saalvage