From 2cd8ea83a74befe6e59579466814f1a4cc1340f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Sun, 22 Nov 2020 16:43:36 +0100 Subject: [PATCH] Fixed #9860 (unused template not removed properly by default) --- lib/token.cpp | 12 +++----- lib/tokenize.cpp | 56 ++++++++++++++++++----------------- lib/tokenize.h | 2 +- test/testconstructors.cpp | 14 ++------- test/testgarbage.cpp | 2 +- test/testother.cpp | 17 +++++++---- test/testsimplifytemplate.cpp | 16 ++++++---- test/testsimplifytokens.cpp | 8 ++++- test/testsimplifytypedef.cpp | 12 ++++++-- test/testsimplifyusing.cpp | 8 ++++- test/testsymboldatabase.cpp | 4 +++ test/testtokenize.cpp | 50 ++++++++++++++++++++++--------- test/testvarid.cpp | 3 ++ 13 files changed, 126 insertions(+), 78 deletions(-) diff --git a/lib/token.cpp b/lib/token.cpp index d3f699fbe..afb9ad8a1 100644 --- a/lib/token.cpp +++ b/lib/token.cpp @@ -293,18 +293,14 @@ void Token::deleteThis() takeData(mNext); mNext->link(nullptr); // mark as unlinked deleteNext(); - } else if (mPrevious && mPrevious->mPrevious) { // Copy previous to this and delete previous + } else if (mPrevious) { // Copy previous to this and delete previous takeData(mPrevious); - - Token* toDelete = mPrevious; - mPrevious = mPrevious->mPrevious; - mPrevious->mNext = this; - - delete toDelete; + mPrevious->link(nullptr); + deletePrevious(); } else { // We are the last token in the list, we can't delete // ourselves, so just make us empty - str(""); + str(";"); } } diff --git a/lib/tokenize.cpp b/lib/tokenize.cpp index 481438682..2c868f034 100644 --- a/lib/tokenize.cpp +++ b/lib/tokenize.cpp @@ -4365,7 +4365,7 @@ bool Tokenizer::simplifyTokenList1(const char FileName[]) reportUnknownMacros(); - simplifyHeaders(); + simplifyHeadersAndUnusedTemplates(); // Remove __asm.. simplifyAsm(); @@ -5027,7 +5027,7 @@ void Tokenizer::dump(std::ostream &out) const list.front()->printValueFlow(true, out); } -void Tokenizer::simplifyHeaders() +void Tokenizer::simplifyHeadersAndUnusedTemplates() { if (mSettings->checkHeaders && mSettings->checkUnusedTemplates) // Full analysis. All information in the headers are kept. @@ -5053,7 +5053,13 @@ void Tokenizer::simplifyHeaders() // functions and types to keep std::set keep; for (const Token *tok = list.front(); tok; tok = tok->next()) { - if (!tok->isName()) + if (isCPP() && Token::simpleMatch(tok, "template <")) { + const Token *closingBracket = tok->next()->findClosingBracket(); + if (Token::Match(closingBracket, "> class|struct %name% {")) + tok = closingBracket->linkAt(3); + } + + if (!tok->isName() || tok->isKeyword()) continue; if (!checkHeaders && tok->fileIndex() != 0) @@ -5088,49 +5094,42 @@ void Tokenizer::simplifyHeaders() } } - if (Token::Match(tok, "[;{}]")) { + if (!tok->previous() || Token::Match(tok->previous(), "[;{}]")) { // Remove unused function declarations if (isIncluded && removeUnusedIncludedFunctions) { while (1) { - Token *start = tok->next(); + Token *start = tok; while (start && functionStart.find(start->str()) != functionStart.end()) start = start->next(); - if (Token::Match(start, "%name% (") && Token::Match(start->linkAt(1), ") const| ;") && keep.find(start->str()) == keep.end()) + if (Token::Match(start, "%name% (") && Token::Match(start->linkAt(1), ") const| ;") && keep.find(start->str()) == keep.end()) { Token::eraseTokens(tok, start->linkAt(1)->tokAt(2)); - else + tok->deleteThis(); + } else break; } } if (isIncluded && removeUnusedIncludedClasses) { - if (Token::Match(tok, "[;{}] class|struct %name% [:{]") && keep.find(tok->strAt(2)) == keep.end()) { + if (Token::Match(tok, "class|struct %name% [:{]") && keep.find(tok->strAt(1)) == keep.end()) { // Remove this class/struct - const Token *endToken = tok->tokAt(3); + const Token *endToken = tok->tokAt(2); if (endToken->str() == ":") { endToken = endToken->next(); while (Token::Match(endToken, "%name%|,")) endToken = endToken->next(); } - if (endToken && endToken->str() == "{" && Token::simpleMatch(endToken->link(), "} ;")) + if (endToken && endToken->str() == "{" && Token::simpleMatch(endToken->link(), "} ;")) { Token::eraseTokens(tok, endToken->link()->next()); + tok->deleteThis(); + } } } if (removeUnusedTemplates || (isIncluded && removeUnusedIncludedTemplates)) { - if (Token::Match(tok->next(), "template < %name%")) { - const Token *tok2 = tok->tokAt(3); - while (Token::Match(tok2, "%name% %name% [,=>]") || Token::Match(tok2, "typename|class ... %name% [,>]")) { - if (Token::Match(tok2, "typename|class ...")) - tok2 = tok2->tokAt(3); - else - tok2 = tok2->tokAt(2); - if (Token::Match(tok2, "= %name% [,>]")) - tok2 = tok2->tokAt(2); - if (tok2->str() == ",") - tok2 = tok2->next(); - } - if (Token::Match(tok2, "> class|struct %name% [;:{]") && keep.find(tok2->strAt(2)) == keep.end()) { - const Token *endToken = tok2->tokAt(3); + if (Token::Match(tok, "template < %name%")) { + const Token *closingBracket = tok->next()->findClosingBracket(); + if (Token::Match(closingBracket, "> class|struct %name% [;:{]") && keep.find(closingBracket->strAt(2)) == keep.end()) { + const Token *endToken = closingBracket->tokAt(3); if (endToken->str() == ":") { endToken = endToken->next(); while (Token::Match(endToken, "%name%|,")) @@ -5138,11 +5137,14 @@ void Tokenizer::simplifyHeaders() } if (endToken && endToken->str() == "{") endToken = endToken->link()->next(); - if (endToken && endToken->str() == ";") + if (endToken && endToken->str() == ";") { Token::eraseTokens(tok, endToken); - } else if (Token::Match(tok2, "> %type% %name% (") && Token::simpleMatch(tok2->linkAt(3), ") {") && keep.find(tok2->strAt(2)) == keep.end()) { - const Token *endToken = tok2->linkAt(3)->linkAt(1)->next(); + tok->deleteThis(); + } + } else if (Token::Match(closingBracket, "> %type% %name% (") && Token::simpleMatch(closingBracket->linkAt(3), ") {") && keep.find(closingBracket->strAt(2)) == keep.end()) { + const Token *endToken = closingBracket->linkAt(3)->linkAt(1)->next(); Token::eraseTokens(tok, endToken); + tok->deleteThis(); } } } diff --git a/lib/tokenize.h b/lib/tokenize.h index 2ad4eb391..c54ed82c9 100644 --- a/lib/tokenize.h +++ b/lib/tokenize.h @@ -169,7 +169,7 @@ public: * - All executable code. * - Unused types/variables/etc */ - void simplifyHeaders(); + void simplifyHeadersAndUnusedTemplates(); /** * Deletes dead code between 'begin' and 'end'. diff --git a/test/testconstructors.cpp b/test/testconstructors.cpp index f3a68a1d6..58d2f463b 100644 --- a/test/testconstructors.cpp +++ b/test/testconstructors.cpp @@ -31,11 +31,11 @@ public: private: Settings settings; - void check(const char code[], bool showAll = false) { + void check(const char code[], bool inconclusive = false) { // Clear the error buffer.. errout.str(""); - settings.inconclusive = showAll; + settings.inconclusive = inconclusive; // Tokenize.. Tokenizer tokenizer(&settings, this); @@ -348,16 +348,6 @@ private: " int x;\n" "};"); ASSERT_EQUALS("", errout.str()); - - check("template struct A {\n" - " A() : x(0) { }\n" - " A(const T & t) : x(t.x) { }\n" - "private:\n" - " int x;\n" - " int y;\n" - "};"); - ASSERT_EQUALS("[test.cpp:2]: (warning) Member variable 'A::y' is not initialized in the constructor.\n" - "[test.cpp:3]: (warning) Member variable 'A::y' is not initialized in the constructor.\n", errout.str()); } void simple7() { // ticket #4531 diff --git a/test/testgarbage.cpp b/test/testgarbage.cpp index 814d2ac21..8320cd982 100644 --- a/test/testgarbage.cpp +++ b/test/testgarbage.cpp @@ -1378,7 +1378,7 @@ private: ); // #3449 - ASSERT_EQUALS("template < typename T > struct A ;\n" + ASSERT_EQUALS(";\n" "struct B { template < typename T > struct C } ;\n" "{ } ;", checkCode("template struct A;\n" diff --git a/test/testother.cpp b/test/testother.cpp index 0832facbd..93b589694 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -6101,10 +6101,14 @@ private: ASSERT_EQUALS("", errout.str()); } - check("template void foo(unsigned int x) {\n" - "if (x <= 0);\n" - "}\n"); - ASSERT_EQUALS("[test.cpp:2]: (style) Checking if unsigned expression 'x' is less than zero.\n", errout.str()); + { + Settings keepTemplates; + keepTemplates.checkUnusedTemplates = true; + check("template void foo(unsigned int x) {\n" + "if (x <= 0);\n" + "}\n", &keepTemplates); + ASSERT_EQUALS("[test.cpp:2]: (style) Checking if unsigned expression 'x' is less than zero.\n", errout.str()); + } // #8836 check("uint32_t value = 0xFUL;\n" @@ -8743,11 +8747,14 @@ private: } void forwardAndUsed() { + Settings keepTemplates; + keepTemplates.checkUnusedTemplates = true; + check("template\n" "void f(T && t) {\n" " g(std::forward(t));\n" " T s = t;\n" - "}"); + "}", &keepTemplates); ASSERT_EQUALS("[test.cpp:4]: (warning) Access of forwarded variable 't'.\n", errout.str()); } diff --git a/test/testsimplifytemplate.cpp b/test/testsimplifytemplate.cpp index 67ef684b9..c79a040f0 100644 --- a/test/testsimplifytemplate.cpp +++ b/test/testsimplifytemplate.cpp @@ -41,6 +41,9 @@ private: void run() OVERRIDE { settings.addEnabled("portability"); + // If there are unused templates, keep those + settings.checkUnusedTemplates = true; + TEST_CASE(template1); TEST_CASE(template2); TEST_CASE(template3); @@ -458,7 +461,8 @@ private: // The expected result.. const char expected[] = "class A ; " - "void f ( ) { A a ; } ; " + "void f ( ) { A a ; } " + "template < typename T > class B { void g ( ) { A < T > b ; b = A < T > :: h ( ) ; } } ; " "class A { } ;"; ASSERT_EQUALS(expected, tok(code)); @@ -1472,6 +1476,7 @@ private: "template C3::C3(const C3 &v) { C1 c1; }\n" "C3 c3;"; const char exp[] = "struct C1 ; " + "template < class T > void f ( ) { x = y ? ( C1 < int > :: allocate ( 1 ) ) : 0 ; } " "class C3 ; " "C3 c3 ; " "class C3 { } ; " @@ -2549,8 +2554,8 @@ private: "template class Fred {};\n" "ObjectCache _cache;"; const char exp[] = "class ObjectCache ; " - "ObjectCache _cache ; " - "class ObjectCache { } ;"; + "template < typename T > class Fred { } ; " + "ObjectCache _cache ; class ObjectCache { } ;"; ASSERT_EQUALS(exp, tok(code)); } @@ -4092,7 +4097,8 @@ private: "template < class T > struct Unconst < const T & > { } ; " "template < class T > struct Unconst < T * const > { } ; " "template < class T1 , class T2 > struct type_equal { enum Anonymous0 { value = 0 } ; } ; " - "template < class T > struct type_equal < T , T > { enum Anonymous1 { value = 1 } ; } ;"; + "template < class T > struct type_equal < T , T > { enum Anonymous1 { value = 1 } ; } ; " + "template < class T > struct template_is_const { enum Anonymous2 { value = ! type_equal < T , Unconst < T > :: type > :: value } ; } ;"; ASSERT_EQUALS(exp1, tok(code1)); } @@ -4372,7 +4378,7 @@ private: const char code[] = "class Fred {\n" " template explicit Fred(T t) { }\n" "}"; - ASSERT_EQUALS("class Fred { }", tok(code)); + ASSERT_EQUALS("class Fred { template < class T > explicit Fred ( T t ) { } }", tok(code)); // #3532 const char code2[] = "class Fred {\n" diff --git a/test/testsimplifytokens.cpp b/test/testsimplifytokens.cpp index f7e1b4452..5c6f065f4 100644 --- a/test/testsimplifytokens.cpp +++ b/test/testsimplifytokens.cpp @@ -46,6 +46,12 @@ private: settings1.addEnabled("style"); settings_windows.addEnabled("portability"); + // If there are unused templates, keep those + settings0.checkUnusedTemplates = true; + settings1.checkUnusedTemplates = true; + settings_std.checkUnusedTemplates = true; + settings_windows.checkUnusedTemplates = true; + // Make sure the Tokenizer::simplifyTokenList works. // The order of the simplifications is important. So this test // case shall make sure the simplifications are done in the @@ -2883,7 +2889,7 @@ private: { const char code[] = "namespace std { }"; - ASSERT_EQUALS("", tok(code)); + ASSERT_EQUALS(";", tok(code)); } { diff --git a/test/testsimplifytypedef.cpp b/test/testsimplifytypedef.cpp index b232a0037..79f19253a 100644 --- a/test/testsimplifytypedef.cpp +++ b/test/testsimplifytypedef.cpp @@ -42,6 +42,11 @@ private: settings0.addEnabled("style"); settings2.addEnabled("style"); + // If there are unused templates, keep those + settings0.checkUnusedTemplates = true; + settings1.checkUnusedTemplates = true; + settings2.checkUnusedTemplates = true; + TEST_CASE(simplifyTypedef1); TEST_CASE(simplifyTypedef2); TEST_CASE(simplifyTypedef3); @@ -1797,7 +1802,7 @@ private: void simplifyTypedef75() { // ticket #2426 const char code[] = "typedef _Packed struct S { long l; };"; - ASSERT_EQUALS("", tok(code, true, Settings::Native, false)); + ASSERT_EQUALS(";", tok(code, true, Settings::Native, false)); ASSERT_EQUALS("", errout.str()); } @@ -2047,7 +2052,7 @@ private: const char code[] = "typedef long Long;\n" "namespace NS {\n" "}"; - ASSERT_EQUALS("", tok(code)); + ASSERT_EQUALS(";", tok(code)); ASSERT_EQUALS("", errout.str()); } @@ -2532,7 +2537,8 @@ private: "template struct c; " "template struct d { enum { e = c::f }; };"; const char exp [] = "class a ; " - "template < long , class > struct c ;"; + "template < long , class > struct c ; " + "template < int g > struct d { enum Anonymous0 { e = c < g , int ( a :: * ) > :: f } ; } ;"; ASSERT_EQUALS(exp, tok(code, false)); } diff --git a/test/testsimplifyusing.cpp b/test/testsimplifyusing.cpp index d43a85543..60d1cd349 100644 --- a/test/testsimplifyusing.cpp +++ b/test/testsimplifyusing.cpp @@ -42,6 +42,11 @@ private: settings0.addEnabled("style"); settings2.addEnabled("style"); + // If there are unused templates, keep those + settings0.checkUnusedTemplates = true; + settings1.checkUnusedTemplates = true; + settings2.checkUnusedTemplates = true; + TEST_CASE(simplifyUsing1); TEST_CASE(simplifyUsing2); TEST_CASE(simplifyUsing3); @@ -492,7 +497,8 @@ private: "class c { " "int i ; i = 0 ; " "c ( ) { i -- ; } " - "} ;"; + "} ; " + "template < class T > class s { } ;"; ASSERT_EQUALS(exp, tok(code, true, Settings::Win64)); } diff --git a/test/testsymboldatabase.cpp b/test/testsymboldatabase.cpp index b2615d390..2af37c431 100644 --- a/test/testsymboldatabase.cpp +++ b/test/testsymboldatabase.cpp @@ -115,6 +115,10 @@ private: LOAD_LIB_2(settings1.library, "std.cfg"); settings2.platform(Settings::Unspecified); + // If there are unused templates, keep those + settings1.checkUnusedTemplates = true; + settings2.checkUnusedTemplates = true; + TEST_CASE(array); TEST_CASE(stlarray1); TEST_CASE(stlarray2); diff --git a/test/testtokenize.cpp b/test/testtokenize.cpp index 0a740a0be..ae97537cb 100644 --- a/test/testtokenize.cpp +++ b/test/testtokenize.cpp @@ -47,6 +47,12 @@ private: void run() OVERRIDE { LOAD_LIB_2(settings_windows.library, "windows.cfg"); + // If there are unused templates, keep those + settings0.checkUnusedTemplates = true; + settings1.checkUnusedTemplates = true; + settings2.checkUnusedTemplates = true; + settings_windows.checkUnusedTemplates = true; + TEST_CASE(tokenize1); TEST_CASE(tokenize2); TEST_CASE(tokenize4); @@ -95,7 +101,8 @@ private: TEST_CASE(longtok); - TEST_CASE(removeUnusedTemplates); + TEST_CASE(simplifyHeadersAndUnusedTemplates1); + TEST_CASE(simplifyHeadersAndUnusedTemplates2); TEST_CASE(simplifyCasts1); TEST_CASE(simplifyCasts2); @@ -1028,7 +1035,7 @@ private: } - void removeUnusedTemplates() { + void simplifyHeadersAndUnusedTemplates1() { Settings s; s.checkUnusedTemplates = false; ASSERT_EQUALS(";", @@ -1044,6 +1051,22 @@ private: "}", s)); } + void simplifyHeadersAndUnusedTemplates2() { + const char code[] = "; template< typename T, u_int uBAR = 0 >\n" + "class Foo {\n" + "public:\n" + " void FooBar() {\n" + " new ( (uBAR ? uBAR : sizeof(T))) T;\n" + " }\n" + "};"; + + Settings s; + s.checkUnusedTemplates = false; + ASSERT_EQUALS(";", tokenizeAndStringify(code, s)); + + s.checkUnusedTemplates = true; + ASSERT_THROW(tokenizeAndStringify(code, s), InternalError); + } // Don’t remove "(int *)".. void simplifyCasts1() { @@ -7467,11 +7490,11 @@ private: } void simplifyEmptyNamespaces() { - ASSERT_EQUALS("", tokenizeAndStringify("namespace { }")); - ASSERT_EQUALS("", tokenizeAndStringify("namespace foo { }")); - ASSERT_EQUALS("", tokenizeAndStringify("namespace foo { namespace { } }")); - ASSERT_EQUALS("", tokenizeAndStringify("namespace { namespace { } }")); // Ticket #9512 - ASSERT_EQUALS("", tokenizeAndStringify("namespace foo { namespace bar { } }")); + ASSERT_EQUALS(";", tokenizeAndStringify("namespace { }")); + ASSERT_EQUALS(";", tokenizeAndStringify("namespace foo { }")); + ASSERT_EQUALS(";", tokenizeAndStringify("namespace foo { namespace { } }")); + ASSERT_EQUALS(";", tokenizeAndStringify("namespace { namespace { } }")); // Ticket #9512 + ASSERT_EQUALS(";", tokenizeAndStringify("namespace foo { namespace bar { } }")); } void prepareTernaryOpForAST() { @@ -8625,12 +8648,12 @@ private: } } - std::string checkHeaders(const char code[], bool f) { + std::string checkHeaders(const char code[], bool checkHeadersFlag) { // Clear the error buffer.. errout.str(""); Settings settings; - settings.checkHeaders = f; + settings.checkHeaders = checkHeadersFlag; // Raw tokens.. std::vector files(1, "test.cpp"); @@ -8670,12 +8693,11 @@ private: "5: } ; void A :: g ( int x ) { a = 2 ; }\n", checkHeaders(code, true)); - ASSERT_EQUALS("\n\n##file 1\n" - "1: struct A {\n" - "2: int a ; a = 1 ;\n" - "3: void f ( ) ;\n" + ASSERT_EQUALS("\n\n##file 1\n\n" + "1:\n" + "|\n" "4:\n" - "5: } ;\n", + "5: ;\n", checkHeaders(code, false)); } }; diff --git a/test/testvarid.cpp b/test/testvarid.cpp index 58f30c33d..f54c786a2 100644 --- a/test/testvarid.cpp +++ b/test/testvarid.cpp @@ -221,6 +221,7 @@ private: settings.platform(Settings::Unix64); settings.standards.c = Standards::C89; settings.standards.cpp = Standards::CPP11; + settings.checkUnusedTemplates = true; Tokenizer tokenizer(&settings, this); std::istringstream istr(code); @@ -239,6 +240,7 @@ private: settings.platform(Settings::Unix64); settings.standards.c = Standards::C89; settings.standards.cpp = Standards::CPP11; + settings.checkUnusedTemplates = true; Tokenizer tokenizer(&settings, this); std::istringstream istr(code); @@ -257,6 +259,7 @@ private: settings.platform(Settings::Unix64); settings.standards.c = Standards::C89; settings.standards.cpp = Standards::CPP11; + settings.checkUnusedTemplates = true; Tokenizer tokenizer(&settings, this); std::istringstream istr(code);