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

fix #12658 Remove constructor initialize also with checkHeaders = false #6375

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

fix #12658 Remove constructor initialize also with checkHeaders = false #6375

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b30aa4c
fix #12658L Remove constructor initialize also witj checkHeaders = false
RobkeBaer May 2, 2024
b8d036f
Fix handling curled braces and multiple variables
RobkeBaer May 3, 2024
b9b2f02
Improve detection of constructor initializer
RobkeBaer May 7, 2024
f5b88ef
Add more explicit check
RobkeBaer May 28, 2024
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
23 changes: 23 additions & 0 deletions lib/tokenize.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6225,6 +6225,29 @@ void Tokenizer::simplifyHeadersAndUnusedTemplates()
for (Token *tok = list.front(); tok; tok = tok->next()) {
const bool isIncluded = (tok->fileIndex() != 0);

// Remove constructor initializer
if (isIncluded && !mSettings.checkHeaders && Token::Match(tok, "%name% (")) {
Token *start = tok->next()->link();
if (start) {
start = start->next();
}
if (start && start->str() == ":") {
const Token *next = start;
while (Token::Match(next, "[,:]")) {
Copy link
Owner

Choose a reason for hiding this comment

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

we expect that there is a name token after next right?

while (Token::Match(next, "[,:] %name%"))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct

Copy link
Owner

Choose a reason for hiding this comment

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

in my opinion it wouldn't hurt to add that more explicit check.

// Skip to ( or {
while (next && !next->link())
Copy link
Owner

Choose a reason for hiding this comment

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

I don't want to skip any random tokens. something more like:

next = next->next();
while (Token::Match(next, "%name%|::|<")) {
    if (next->str() == "<")
         // find end token.. I believe there is a utility function in template simplifier
    next = next->next();
}
if (!Token::Match(next, "(|{"))
    // not initializer list..
next = next->link()->next(); // <- goto "," or "{"

Copy link
Contributor Author

@RobkeBaer RobkeBaer May 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Looking better at it, If after the previous change 'Token::Match(next, "[,:] %name%")' The symbol after %name% should always be a link in a constructor initializer list. If not is is something else. %name% should always be a member or delegate constructor.

Only thing now failing is a parameter pack expansion in an initializer list

template<class... Mixins>
class X : public Mixins...
{
public:
X(const Mixins&... mixins) : Mixins(mixins)... {}
};

next = next->next();
if (next) {
next = next->link();
next = next->next(); // Go to next, either ',' or '{'
}
}
if (next && next->str() == "{") {
Token::eraseTokens(start->previous(), next);
}
}
}

// Remove executable code
if (isIncluded && !mSettings.checkHeaders && tok->str() == "{") {
// TODO: We probably need to keep the executable code if this function is called from the source file.
Expand Down
48 changes: 48 additions & 0 deletions test/testtokenize.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -433,6 +433,8 @@ class TestTokenizer : public TestFixture {
TEST_CASE(cppcast);

TEST_CASE(checkHeader1);
TEST_CASE(checkHeader2);
TEST_CASE(checkHeader3);

TEST_CASE(removeExtraTemplateKeywords);

Expand Down Expand Up @@ -7750,6 +7752,52 @@ class TestTokenizer : public TestFixture {
checkHdrs(code, false));
}

void checkHeader2() {
// #9977
const char code[] = "# 1 \"test.h\"\n"
"A::A(const int &value)\n"
" :data(value),\n"
" data2(value)\n"
" {}\n";

ASSERT_EQUALS("\n\n##file 1\n"
"1: A :: A ( const int & value )\n"
"2: : data ( value ) ,\n"
"3: data2 ( value )\n"
"4: { }\n",
checkHdrs(code, true));

ASSERT_EQUALS("\n\n##file 1\n"
"1: A :: A ( const int & value )\n"
"2:\n"
"3:\n"
"4: ;\n",
checkHdrs(code, false));
}

void checkHeader3() {
// #9977
const char code[] = "# 1 \"test.h\"\n"
"A::A(const int &value)\n"
" :data{value},\n"
" data2{value}\n"
" {}\n";

ASSERT_EQUALS("\n\n##file 1\n"
"1: A :: A ( const int & value )\n"
"2: : data { value } ,\n"
"3: data2 { value }\n"
"4: { }\n",
checkHdrs(code, true));

ASSERT_EQUALS("\n\n##file 1\n"
"1: A :: A ( const int & value )\n"
"2:\n"
"3:\n"
"4: ;\n",
checkHdrs(code, false));
}

void removeExtraTemplateKeywords() {
const char code1[] = "typename GridView::template Codim<0>::Iterator iterator;";
const char expected1[] = "GridView :: Codim < 0 > :: Iterator iterator ;";
Expand Down
Loading