Skip to content

testrunner: more Tokenizer related refactoring #7440

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

Merged
firewave merged 4 commits into from
Apr 13, 2025
Merged

testrunner: more Tokenizer related refactoring #7440

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
fdb4a45
TokenList: got rid of test-only `createTokens()` overload
firewave Apr 7, 2025
2c6fd37
testrunner: moved some redundant code into `TokenListHelper`
firewave Apr 7, 2025
a989b35
testunusedvar.cpp: removed unused `CheckStructMemberUsageOptions::s`
firewave Apr 8, 2025
aaee6ae
testrunner: avoid some duplicated tokenizing
firewave Apr 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 45 additions & 45 deletions Makefile
View file
Open in desktop

Large diffs are not rendered by default.

17 changes: 3 additions & 14 deletions lib/tokenlist.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,8 @@ void TokenList::determineCppC()

int TokenList::appendFileIfNew(std::string fileName)
{
ASSERT_LANG(!fileName.empty());

// Has this file been tokenized already?
auto it = std::find_if(mFiles.cbegin(), mFiles.cend(), [&](const std::string& f) {
return Path::sameFileName(f, fileName);
Expand Down Expand Up @@ -340,19 +342,6 @@ void TokenList::insertTokens(Token *dest, const Token *src, nonneg int n)
}
}

//---------------------------------------------------------------------------
// Tokenize - tokenizes a given file.
//---------------------------------------------------------------------------

bool TokenList::createTokens(std::istream &code, const std::string& file0)
{
ASSERT_LANG(!file0.empty());

appendFileIfNew(file0);

return createTokensInternal(code, file0);
}

//---------------------------------------------------------------------------

bool TokenList::createTokens(std::istream &code, Standards::Language lang)
Expand All @@ -364,7 +353,7 @@ bool TokenList::createTokens(std::istream &code, Standards::Language lang)
ASSERT_LANG(lang == mLang);
}

return createTokensInternal(code, "");
return createTokensInternal(code, mFiles.empty() ? "" : *mFiles.cbegin());
}

//---------------------------------------------------------------------------
Expand Down
3 changes: 1 addition & 2 deletions lib/tokenlist.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,9 +103,8 @@ class CPPCHECKLIB TokenList {
* - UTF in the code are not handled.
* - comments are not handled.
* @param code input stream for code
* @param file0 source file name
* @param lang the language of the code
*/
bool createTokens(std::istream &code, const std::string& file0);
bool createTokens(std::istream &code, Standards::Language lang);

void createTokens(simplecpp::TokenList&& tokenList);
Expand Down
17 changes: 16 additions & 1 deletion test/helpers.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#define helpersH

#include "library.h"
#include "path.h"
#include "settings.h"
#include "standards.h"
#include "tokenize.h"
Expand Down Expand Up @@ -111,7 +112,10 @@ class SimpleTokenizer : public Tokenizer {
const std::string& filename,
const std::string &configuration = "")
{
if (!list.createTokens(istr, filename))
if (list.front())
throw std::runtime_error("token list is not empty");
list.appendFileIfNew(filename);
if (!list.createTokens(istr, Path::identify(filename, false)))
return false;

return simplifyTokens1(configuration);
Expand Down Expand Up @@ -278,4 +282,15 @@ class SimpleTokenizer2 : public Tokenizer {
std::vector<std::string> mFiles;
};

struct TokenListHelper
{
static bool createTokens(TokenList& tokenlist, std::istream& istr, const std::string& file)
{
if (tokenlist.front())
throw std::runtime_error("token list is not empty");
tokenlist.appendFileIfNew(file);
return tokenlist.createTokens(istr, Path::identify(file, false));
}
};

#endif // helpersH
11 changes: 7 additions & 4 deletions test/testsimplifytemplate.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include "errortypes.h"
#include "fixture.h"
#include "helpers.h"
#include "path.h"
#include "settings.h"
#include "templatesimplifier.h"
#include "token.h"
Expand Down Expand Up @@ -5429,7 +5430,8 @@ class TestSimplifyTemplate : public TestFixture {
Tokenizer tokenizer(settings, *this);

std::istringstream istr(code);
if (!tokenizer.list.createTokens(istr, "test.cpp"))
tokenizer.list.appendFileIfNew("test.cpp");
if (!tokenizer.list.createTokens(istr, Path::identify("test.cpp", false)))
return false;
tokenizer.createLinks();
tokenizer.splitTemplateRightAngleBrackets(false);
Expand Down Expand Up @@ -5497,7 +5499,8 @@ class TestSimplifyTemplate : public TestFixture {
Tokenizer tokenizer(settings, *this);

std::istringstream istr(code);
if (!tokenizer.list.createTokens(istr, "test.cpp"))
tokenizer.list.appendFileIfNew("test.cpp");
if (!tokenizer.list.createTokens(istr, Path::identify("test.cpp", false)))
return false;
tokenizer.createLinks();
tokenizer.splitTemplateRightAngleBrackets(false);
Expand Down Expand Up @@ -5568,7 +5571,7 @@ class TestSimplifyTemplate : public TestFixture {
Tokenizer tokenizer(settings, *this);

std::istringstream istr(code);
if (!tokenizer.list.createTokens(istr, "test.cpp"))
if (!TokenListHelper::createTokens(tokenizer.list, istr, "test.cpp"))
return false;
tokenizer.createLinks();
tokenizer.splitTemplateRightAngleBrackets(false);
Expand Down Expand Up @@ -5598,7 +5601,7 @@ class TestSimplifyTemplate : public TestFixture {
Tokenizer tokenizer(settings, *this);

std::istringstream istr(code);
if (!tokenizer.list.createTokens(istr, "test.cpp"))
if (!TokenListHelper::createTokens(tokenizer.list, istr, "test.cpp"))
return false;
tokenizer.createLinks();
tokenizer.splitTemplateRightAngleBrackets(false);
Expand Down
11 changes: 5 additions & 6 deletions test/testsimplifytypedef.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
#include <cstddef>
#include <sstream>
#include <string>
#include <vector>

class TestSimplifyTypedef : public TestFixture {
public:
Expand Down Expand Up @@ -310,7 +309,7 @@ class TestSimplifyTypedef : public TestFixture {
Tokenizer tokenizer(settings1, *this);

std::istringstream istr(code);
if (!tokenizer.list.createTokens(istr, "file.c"))
if (!TokenListHelper::createTokens(tokenizer.list, istr, "file.c"))
return "";
tokenizer.createLinks();
tokenizer.simplifyTypedef();
Expand All @@ -326,7 +325,7 @@ class TestSimplifyTypedef : public TestFixture {
Tokenizer tokenizer(settings1, *this);

std::istringstream istr(code);
if (!tokenizer.list.createTokens(istr, "file.c"))
if (!TokenListHelper::createTokens(tokenizer.list, istr, "file.c"))
return {};
tokenizer.createLinks();
tokenizer.simplifyTypedef();
Expand Down Expand Up @@ -4454,7 +4453,7 @@ class TestSimplifyTypedef : public TestFixture {

Tokenizer tokenizer(settings1, *this);
std::istringstream istr(code);
ASSERT(tokenizer.list.createTokens(istr, "file.c"));
ASSERT(TokenListHelper::createTokens(tokenizer.list, istr, "file.c"));
tokenizer.createLinks();
tokenizer.simplifyTypedef();

Expand Down Expand Up @@ -4496,7 +4495,7 @@ class TestSimplifyTypedef : public TestFixture {

Tokenizer tokenizer(settings1, *this);
std::istringstream istr(code);
ASSERT(tokenizer.list.createTokens(istr, "file.c"));
ASSERT(TokenListHelper::createTokens(tokenizer.list, istr, "file.c"));
tokenizer.createLinks();
tokenizer.simplifyTypedef();

Expand All @@ -4514,7 +4513,7 @@ class TestSimplifyTypedef : public TestFixture {

Tokenizer tokenizer(settings1, *this);
std::istringstream istr(code);
ASSERT(tokenizer.list.createTokens(istr, "file.c"));
ASSERT(TokenListHelper::createTokens(tokenizer.list, istr, "file.c"));
tokenizer.createLinks();
tokenizer.simplifyTypedef();

Expand Down
7 changes: 1 addition & 6 deletions test/testsimplifyusing.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,10 @@
#include "platform.h"
#include "settings.h"
#include "token.h"
#include "tokenlist.h"
#include "utils.h"

#include <cstddef>
#include <sstream>
#include <string>
#include <vector>

class TestSimplifyUsing : public TestFixture {
public:
Expand Down Expand Up @@ -115,8 +112,6 @@ class TestSimplifyUsing : public TestFixture {
if (options.preprocess) {
SimpleTokenizer2 tokenizer(settings, *this, code, "test.cpp");

std::istringstream istr(code);
ASSERT_LOC(tokenizer.list.createTokens(istr, "test.cpp"), file, line); // TODO: this creates the tokens a second time
ASSERT_LOC(tokenizer.simplifyTokens1(""), file, line);
return tokenizer.tokens()->stringifyList(nullptr);
}
Expand Down Expand Up @@ -1588,7 +1583,7 @@ class TestSimplifyUsing : public TestFixture {
"STAMP(B, A);\n"
"STAMP(C, B);\n";
(void)tok(code, dinit(TokOptions, $.preprocess = true));
ASSERT(startsWith(errout_str(), "[test.cpp:6]: (debug) Failed to parse 'using C = S < S < S < int"));
TODO_ASSERT(startsWith(errout_str(), "[test.cpp:6]: (debug) Failed to parse 'using C = S < S < S < int"));
Comment on lines -1591 to +1586
Copy link
Copy Markdown
Collaborator Author

@firewave firewave Apr 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chrchr-github This was producing an error because the code contained the preprocessed and non-preprocessed source because it was tokenized twice (I tracked it back to the introduction and it has always been that way). I think this is currently fine because it was about a hang/crash but if we expect to test for the error we might need to add another test.

Copy link
Copy Markdown
Collaborator Author

@firewave firewave Apr 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original code was added in #5018.

Copy link
Copy Markdown
Collaborator

@chrchr-github chrchr-github Apr 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is errout_str() after the change? I agree the output was not important, since the issue was a hang.

Copy link
Copy Markdown
Collaborator Author

@firewave firewave Apr 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is empty.

Copy link
Copy Markdown
Collaborator

@chrchr-github chrchr-github Apr 11, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That even seems like progress. But then I don't understand why the refactoring made the debug message go away.
If it's related to what you wrote above, just assert on the empty errout_str.

Copy link
Copy Markdown
Collaborator Author

@firewave firewave Apr 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But then I don't understand why the refactoring made the debug message go away.

See previous comment.

This was producing an error because the code contained the preprocessed and non-preprocessed source because it was tokenized twice

I was asking because I think we should have a test which actually detected the debug message.

}

void scopeInfo1() {
Expand Down
7 changes: 5 additions & 2 deletions test/testtokenize.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include "errortypes.h"
#include "fixture.h"
#include "helpers.h"
#include "path.h"
#include "platform.h"
#include "preprocessor.h" // usually tests here should not use preprocessor...
#include "settings.h"
Expand Down Expand Up @@ -865,7 +866,8 @@ class TestTokenizer : public TestFixture {
Tokenizer tokenizer(settings1, *this);
const char code[] = "void foo(int i) { reinterpret_cast<char>(i) };";
std::istringstream istr(code);
ASSERT(tokenizer.list.createTokens(istr, "test.h"));
tokenizer.list.appendFileIfNew("test.h");
ASSERT(tokenizer.list.createTokens(istr, Path::identify("test.h", false)));
ASSERT_THROW_INTERNAL(tokenizer.simplifyTokens1(""), SYNTAX);
}
}
Expand Down Expand Up @@ -6133,7 +6135,8 @@ class TestTokenizer : public TestFixture {
// tokenize given code..
Tokenizer tokenizer(settings0, *this);
std::istringstream istr(code);
if (!tokenizer.list.createTokens(istr,"test.cpp"))
tokenizer.list.appendFileIfNew("test.cpp");
if (!tokenizer.list.createTokens(istr,Path::identify("test.cpp", false)))
return "ERROR";

tokenizer.combineStringAndCharLiterals();
Expand Down
7 changes: 5 additions & 2 deletions test/testtokenlist.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include "settings.h"
#include "fixture.h"
#include "helpers.h"
#include "path.h"
#include "platform.h"
#include "preprocessor.h"
#include "standards.h"
Expand Down Expand Up @@ -129,7 +130,8 @@ class TestTokenList : public TestFixture {
const Settings s = settingsBuilder().c(Standards::C89).build();
TokenList tokenlist(&s);
std::istringstream istr(code2);
ASSERT(tokenlist.createTokens(istr, "a.c"));
tokenlist.appendFileIfNew("a.c");
ASSERT(tokenlist.createTokens(istr, Path::identify("a.c", false)));
ASSERT_EQUALS(false, tokenlist.front()->isKeyword());
}

Expand All @@ -150,7 +152,8 @@ class TestTokenList : public TestFixture {
const Settings s = settingsBuilder().cpp(Standards::CPP03).build();
TokenList tokenlist(&s);
std::istringstream istr(code2);
ASSERT(tokenlist.createTokens(istr, "a.cpp"));
tokenlist.appendFileIfNew("a.cpp");
ASSERT(tokenlist.createTokens(istr, Path::identify("a.cpp", false)));
ASSERT_EQUALS(false, tokenlist.front()->isKeyword());
}
}
Expand Down
18 changes: 6 additions & 12 deletions test/testunusedvar.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,9 @@
#include "helpers.h"
#include "preprocessor.h"
#include "settings.h"
#include "standards.h"

#include <list>
#include <string>
#include <vector>

class TestUnusedVar : public TestFixture {
public:
Expand Down Expand Up @@ -281,21 +279,19 @@ class TestUnusedVar : public TestFixture {
{
CheckStructMemberUsageOptions() = default;
const std::list<Directive>* directives = nullptr;
const Settings *s = nullptr;
bool cpp = true;
};

#define checkStructMemberUsage(...) checkStructMemberUsage_(__FILE__, __LINE__, __VA_ARGS__)
void checkStructMemberUsage_(const char* file, int line, const char code[], const CheckStructMemberUsageOptions& options = make_default_obj()) {
const Settings *settings1 = options.s ? options.s : &settings;

// Tokenize..
SimpleTokenizer tokenizer(*settings1, *this);
SimpleTokenizer tokenizer(settings, *this);
if (options.directives)
tokenizer.setDirectives(*options.directives);
ASSERT_LOC(tokenizer.tokenize(code), file, line);
ASSERT_LOC(tokenizer.tokenize(code, options.cpp), file, line);

// Check for unused variables..
CheckUnusedVar checkUnusedVar(&tokenizer, settings1, this);
CheckUnusedVar checkUnusedVar(&tokenizer, &settings, this);
(checkUnusedVar.checkStructMemberUsage)();
}

Expand Down Expand Up @@ -1831,8 +1827,6 @@ class TestUnusedVar : public TestFixture {
ASSERT_EQUALS("[test.cpp:1]: (style) struct member 'A::i' is never used.\n",
errout_str());

/*const*/ Settings s = settings;
s.enforcedLang = Standards::Language::C;
checkStructMemberUsage("struct A {\n" // #10852
" struct B {\n"
" int x;\n"
Expand All @@ -1841,7 +1835,7 @@ class TestUnusedVar : public TestFixture {
"void f() {\n"
" struct B* pb = &a.b;\n"
" pb->x = 1;\n"
"}\n", dinit(CheckStructMemberUsageOptions, $.s = &s));
"}\n", dinit(CheckStructMemberUsageOptions, $.cpp = false));
ASSERT_EQUALS("", errout_str());

checkStructMemberUsage("union U {\n"
Expand All @@ -1859,7 +1853,7 @@ class TestUnusedVar : public TestFixture {
" pb->x = 1;\n"
" struct C* pc = &u.c;\n"
" pc->s[0] = 1;\n"
"}\n", dinit(CheckStructMemberUsageOptions, $.s = &s));
"}\n", dinit(CheckStructMemberUsageOptions, $.cpp = false));
ASSERT_EQUALS("", errout_str());
}

Expand Down
Loading