Fix this instantiation

Author: pfultz2

lib/templatesimplifier.cpp

@@ -4699,11 +4699,13 @@ void TemplateSimplifier::simplifyTemplates(const std::time_t maxtime)
                     Token * tok = it->token();
                     tok->deleteNext(2);
                     tok->deleteThis();
-                } else if (shouldDeferRemoval(*it)) {
+                } else if (shouldDeferRemoval(*it) || hasRemainingCalls(*it)) {
                     // don't remove the declaration yet: pending type deductions may
                     // instantiate this function template again, and during the type
                     // deduction rounds the symbol database has references to these
-                    // tokens. It is removed later by removeDeferredTemplateDeclarations().
+                    // tokens. It is removed later by removeDeferredTemplateDeclarations()
+                    // - unless the template is still called without template arguments:
+                    // those calls refer to the generic definition.
                     auto it1 = mTemplateForwardDeclarationsMap.find(it->token());
                     if (it1 != mTemplateForwardDeclarationsMap.end())
                         deferRemoval(it1->second);
@@ -4871,6 +4873,75 @@ void TemplateSimplifier::rememberDirtyCallSite(Token* nameTok)
     mDirtyCallSites.push_back(nameTok);
 }
 
+// The tokens of the given template declarations, from "template" to the end of each
+// declaration
+static std::unordered_set<const Token*> declarationTokens(const std::vector<const Token*>& declarations)
+{
+    std::unordered_set<const Token*> tokens;
+    for (const Token* declTok : declarations) {
+        const Token* end = TemplateSimplifier::findTemplateDeclarationEnd(declTok);
+        if (!end)
+            continue;
+        for (const Token* tok = declTok; tok; tok = tok->next()) {
+            tokens.insert(tok);
+            if (tok == end)
+                break;
+        }
+    }
+    return tokens;
+}
+
+// Is the token the name of a plain call "f ( x )" - not a declaration "int f ( int x )"?
+static bool isPlainCall(const Token* tok)
+{
+    if (!tok->isName() || tok->isKeyword() || tok->strAt(1) != "(")
+        return false;
+    // skip the qualification: "N :: f ( x )"
+    const Token* prev = tok->previous();
+    while (Token::simpleMatch(prev, "::") && prev->previous() && prev->previous()->isName())
+        prev = prev->tokAt(-2);
+    if (!prev)
+        return false;
+    // "return f ( x )" is a call, "int f ( int x )" / "T * f ( T t )" is a declaration
+    if (Token::Match(prev, "return|case|throw|else|do|co_return|co_yield|co_await|sizeof"))
+        return true;
+    return !Token::Match(prev, "%name%|*|&|&&|>");
+}
+
+// Are there plain calls "name ( x )" of the function template left in the token list,
+// outside the tokens of its declarations? Such a call - in an uninstantiated template,
+// or with arguments whose types are unknown - could not be instantiated and refers to
+// the generic definition, so the definition must not be removed.
+static bool hasRemainingCalls(const TokenList& tokenList,
+                              const std::string& name,
+                              const std::unordered_set<const Token*>& declarations)
+{
+    for (const Token* tok = tokenList.front(); tok; tok = tok->next()) {
+        if (tok->str() == name && declarations.count(tok) == 0 && isPlainCall(tok))
+            return true;
+    }
+    return false;
+}
+
+// Is the name token the name of a function template whose calls can be written without
+// template arguments? A member function of a class template ("C < T > :: f") is
+// instantiated with its class, never from a call.
+static bool isDeducibleFunctionTemplateName(const Token* nameTok)
+{
+    return nameTok && nameTok->strAt(1) == "(" && !Token::simpleMatch(nameTok->tokAt(-2), "> ::");
+}
+
+bool TemplateSimplifier::hasRemainingCalls(const TokenAndName& declaration) const
+{
+    if (!declaration.isFunction() || !isDeducibleFunctionTemplateName(declaration.nameToken()))
+        return false;
+    std::vector<const Token*> declarations{declaration.token()};
+    const auto it = mTemplateForwardDeclarationsMap.find(declaration.token());
+    if (it != mTemplateForwardDeclarationsMap.cend())
+        declarations.push_back(it->second);
+    return ::hasRemainingCalls(mTokenList, declaration.name(), declarationTokens(declarations));
+}
+
 bool TemplateSimplifier::removeDeferredTemplateDeclarations(SymbolDatabase* symbolDatabase)
 {
     if (mDeferredRemovals.empty())
@@ -4886,18 +4957,27 @@ bool TemplateSimplifier::removeDeferredTemplateDeclarations(SymbolDatabase* symb
     }
     mDeferredRemovals.clear();
 
-    // all tokens that are going to be removed
-    std::unordered_set<const Token*> removedTokens;
+    // a function template that is still called without template arguments (the
+    // arguments could not be deduced) is kept - the calls refer to the generic definition
+    const std::unordered_set<const Token*> deferredTokens(
+        declarationTokens(std::vector<const Token*>(declarations.cbegin(), declarations.cend())));
+    std::vector<Token*> removedDeclarations;
     for (Token* declTok : declarations) {
-        const Token* end = findTemplateDeclarationEnd(declTok);
-        if (!end)
-            continue;
-        for (const Token* tok = declTok; tok; tok = tok->next()) {
-            removedTokens.insert(tok);
-            if (tok == end)
-                break;
+        const Token* nameTok = nullptr;
+        if (const Token* paramEnd = declTok->next()->findClosingBracket()) {
+            const int namepos = getTemplateNamePosition(paramEnd);
+            if (namepos > 0)
+                nameTok = paramEnd->tokAt(namepos);
         }
+        if (isDeducibleFunctionTemplateName(nameTok) && ::hasRemainingCalls(mTokenList, nameTok->str(), deferredTokens))
+            continue;
+        removedDeclarations.push_back(declTok);
     }
+    declarations = std::move(removedDeclarations);
+
+    // all tokens that are going to be removed
+    const std::unordered_set<const Token*> removedTokens(
+        declarationTokens(std::vector<const Token*>(declarations.cbegin(), declarations.cend())));
 
     // dirty call sites inside the removed declarations are gone
     mDirtyCallSites.erase(std::remove_if(mDirtyCallSites.begin(),

lib/templatesimplifier.h

@@ -448,6 +448,14 @@ class CPPCHECKLIB TemplateSimplifier {
      */
     void deferRemoval(Token* declTok);
 
+    /**
+     * Are there calls of the function template left in the token list whose template
+     * arguments could not be deduced ("f ( x )" in an uninstantiated template, or with
+     * arguments of unknown type)? Those calls refer to the generic definition, so the
+     * declaration must not be removed after the instantiation.
+     */
+    bool hasRemainingCalls(const TokenAndName& declaration) const;
+
     /**
      * Whether removing the given instantiated template declaration must be deferred
      * with deferRemoval() instead of removing it immediately.

test/testsimplifytemplate.cpp

@@ -319,6 +319,7 @@ class TestSimplifyTemplate : public TestFixture {
         TEST_CASE(templateTypeDeductionOutOfLine);   // member template defined out of line, class in a namespace
         TEST_CASE(templateTypeDeductionShadowedType); // the deduced type name means another type in the template's scope
         TEST_CASE(templateTypeDeductionRecursion);   // recursive calls are deduced, not renamed to the current instantiation
+        TEST_CASE(templateTypeDeductionRemainingCalls); // the generic definition stays while undeduced calls remain
         TEST_CASE(templateTypeDeductionEnum);        // enum arguments deduce the enum, not its underlying type
         TEST_CASE(templateTypeDeductionArray);       // array arguments: only the decayed dimension is a pointer
         TEST_CASE(templateTypeDeductionConstPointer); // "const T *": the parameter provides the pointee const
@@ -7562,6 +7563,63 @@ class TestSimplifyTemplate : public TestFixture {
         }
     }
 
+    void templateTypeDeductionRemainingCalls()
+    { // a call whose template arguments can not be deduced - in an uninstantiated class
+      // template here - refers to the generic definition: it must not be removed after
+      // another call instantiated the template
+        {
+            const char code[] = "template<class T> int get(const T& t) { return t.count; }\n"
+                                "template<class U> struct Box { U item; int count; int size() const { return get(*this); } };\n"
+                                "struct Bag { int count; };\n"
+                                "int total(const Bag& b) { return get(b); }";
+            const char exp[] = "int get<Bag> ( const Bag & t ) ; "
+                               "template < class T > int get ( const T & t ) { return t . count ; } "
+                               "template < class U > struct Box { U item ; int count ; int size ( ) const { return get ( * this ) ; } } ; "
+                               "struct Bag { int count ; } ; "
+                               "int total ( const Bag & b ) { return get<Bag> ( b ) ; } "
+                               "int get<Bag> ( const Bag & t ) { return t . count ; }";
+            ASSERT_EQUALS(exp, tok(code));
+            ASSERT_EQUALS(exp, tok(code, dinit(CheckOptions, $.templateFullRebuild = true)));
+        }
+        { // explicit template arguments: same result
+            const char code[] = "template<class T> int get(const T& t) { return t.count; }\n"
+                                "template<class U> struct Box { U item; int count; int size() const { return get(*this); } };\n"
+                                "struct Bag { int count; };\n"
+                                "int total(const Bag& b) { return get<Bag>(b); }";
+            const char exp[] = "int get<Bag> ( const Bag & t ) ; "
+                               "template < class T > int get ( const T & t ) { return t . count ; } "
+                               "template < class U > struct Box { U item ; int count ; int size ( ) const { return get ( * this ) ; } } ; "
+                               "struct Bag { int count ; } ; "
+                               "int total ( const Bag & b ) { return get<Bag> ( b ) ; } "
+                               "int get<Bag> ( const Bag & t ) { return t . count ; }";
+            ASSERT_EQUALS(exp, tok(code));
+        }
+        { // a forward declaration is kept with the definition
+            const char code[] = "template<class T> int get(const T& t);\n"
+                                "template<class T> int get(const T& t) { return t.count; }\n"
+                                "template<class U> struct Box { int count; int size() const { return get(*this); } };\n"
+                                "struct Bag { int count; };\n"
+                                "int total(const Bag& b) { return get(b); }";
+            const std::string result = tok(code);
+            ASSERT(result.find("template < class T > int get ( const T & t ) ;") != std::string::npos);
+            ASSERT(result.find("template < class T > int get ( const T & t ) { return t . count ; }") != std::string::npos);
+            ASSERT(result.find("int get<Bag> ( const Bag & t ) { return t . count ; }") != std::string::npos);
+        }
+        { // when all calls are instantiated the generic definition is removed as before; a
+          // function with the same name in another class is not a call
+            const char code[] = "template<class T> int get(const T& t) { return t.count; }\n"
+                                "struct Other { int get(int x); };\n"
+                                "struct Bag { int count; };\n"
+                                "int total(const Bag& b) { return get(b); }";
+            const char exp[] = "int get<Bag> ( const Bag & t ) ; "
+                               "struct Other { int get ( int x ) ; } ; "
+                               "struct Bag { int count ; } ; "
+                               "int total ( const Bag & b ) { return get<Bag> ( b ) ; } "
+                               "int get<Bag> ( const Bag & t ) { return t . count ; }";
+            ASSERT_EQUALS(exp, tok(code));
+        }
+    }
+
     void templateTypeDeductionEnum()
     {
         { // an enum variable has the ValueType of the underlying integer type - the enum
@@ -7647,10 +7705,14 @@ class TestSimplifyTemplate : public TestFixture {
                             "    f(a); g(a); f(b); h(b); k(b); f(s.m); f(p->m); f(&a); f(q); f(*b); g(c ? a : a);\n"
                             "    f(i); h(a); k(a); f(a[0]); h(b[0]); k(q); h(&a[0]); k(&i);\n"
                             "}";
+        // the generic definitions stay because of the calls that are not deduced
         const char exp[] = "void f<int> ( int & x ) ; "
+                           "template < class T > void f ( T & x ) { } "
                            "template < class T > void g ( const T & x ) { } "
                            "void h<int> ( int * x ) ; "
+                           "template < class T > void h ( T * x ) { } "
                            "void k<int*> ( int * x ) ; void k<int**> ( int * * x ) ; "
+                           "template < class T > void k ( T x ) { } "
                            "struct S { int m [ 2 ] ; } ; "
                            "void test ( S s , S * p , bool c ) { "
                            "int a [ 3 ] ; int b [ 2 ] [ 3 ] ; int * q [ 2 ] ; int i ; "

test/testunusedvar.cpp

@@ -83,6 +83,7 @@ class TestUnusedVar : public TestFixture {
         TEST_CASE(structmember35);
         TEST_CASE(structmember_macro);
         TEST_CASE(structmember_template_argument); // #13887 - do not report that member used in template argument is unused
+        TEST_CASE(structmember_template_function); // member used through a function template that is instantiated for another type
         TEST_CASE(classmember);
         TEST_CASE(structmemberStructuredBinding); // #13107
 
@@ -2125,6 +2126,22 @@ class TestUnusedVar : public TestFixture {
         ASSERT_EQUALS("", errout_str());
     }
 
+    void structmember_template_function() {
+        // the generic get() is still called from the uninstantiated Box: it must not be
+        // removed when get<Bag> is instantiated, otherwise Box::count has no use left
+        checkStructMemberUsage("template<class T> int get(const T& t) { return t.count; }\n"
+                               "template<class U> struct Box { U item; int count; int size() const { return get(*this); } };\n"
+                               "struct Bag { int count; };\n"
+                               "int total(const Bag& b) { return get(b); }\n");
+        ASSERT_EQUALS("", errout_str());
+
+        checkStructMemberUsage("template<class T> int get(const T& t) { return t.count; }\n"
+                               "template<class U> struct Box { U item; int count; int size() const { return get(*this); } };\n"
+                               "struct Bag { int count; };\n"
+                               "int total(const Bag& b) { return get<Bag>(b); }\n");
+        ASSERT_EQUALS("", errout_str());
+    }
+
     void structmember_template_argument() { // #13887 - False positive
         checkStructMemberUsage("template <class T, int i> struct A{ T buf[i]; }\n"
                                "struct B {\n"