Skip to content

Accept nested templates in tokenizer-simplify#1070

Merged
danmar merged 1 commit into
cppcheck-opensource:masterfrom
jokva:master
Feb 4, 2018
Merged

Accept nested templates in tokenizer-simplify#1070
danmar merged 1 commit into
cppcheck-opensource:masterfrom
jokva:master

Conversation

@jokva

@jokva jokva commented Jan 30, 2018 •
edited
Loading

Copy link
Copy Markdown

The following snippet triggerd the error:

template<typename DerivedT>
template<typename T>
auto ComposableParserImpl<DerivedT>::operator|( T const &other ) const -> Parser {
    return Parser() | static_cast<DerivedT const &>( *this ) | other;
}

Whenever simplifyFunctionParameters was called on a templated class'
templated member function (and probably any nested template), the
tokenizer would recognise it as a syntax error, assuming that return
type must come after a template<> token.

@amai2012

amai2012 commented Jan 31, 2018

Copy link
Copy Markdown
Collaborator

Thanks.
Your change has caused a couple of test failures, see https://ci.appveyor.com/project/danmar/cppcheck/build/1.73.5066/job/076f2egoopjhxraa and also check testrunner on your local instance please.

amai2012
amai2012 requested changes Jan 31, 2018
View reviewed changes

@amai2012 amai2012 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please try to repair the broken tests.

@jokva
Accept nested templates in tokenizer-simplify
f656abe 
The following snippet triggerd the error:

template<typename DerivedT>
template<typename T>
auto ComposableParserImpl<DerivedT>::operator|( T const &other ) const -> Parser {
    return Parser() | static_cast<DerivedT const &>( *this ) | other;
}

Whenever simplifyFunctionParameters was called on a templated class'
templated member function (and probably any nested template), the
tokenizer would recognise it as a syntax error, assuming that return
type *must* come after a template<> token.
@jokva

jokva commented Jan 31, 2018

Copy link
Copy Markdown
Author

Hi,

You're right, the previous patch was rubbish and didn't work properly at all. I've updated it, and the test pass on my machine.

diff --git a/lib/tokenize.cpp b/lib/tokenize.cpp
index 2830c6d..a98ea79 100755
--- a/lib/tokenize.cpp
+++ b/lib/tokenize.cpp
@@ -8420,9 +8420,7 @@ const Token * Tokenizer::findGarbageCode() const
         for (const Token *tok = tokens(); tok; tok = tok->next()) {
             if (!Token::simpleMatch(tok, "template <"))
                 continue;
-            if (tok->previous() && Token::simpleMatch(tok, "template <"))
-                continue;
-            if (tok->previous() && !Token::Match(tok->previous(), "[:;{})]"))
+            if (tok->previous() && !Token::Match(tok->previous(), "[:;{})>]"))
                 return tok;
             const Token * const tok1 = tok;
             tok = tok->next()->findClosingBracket();

I suspect this will now accept in-the-wild > that doesn't actually close a template, allowing some carefully crafted input to slip through. Is that worth investigating?

@amai2012 amai2012 requested a review from danmar January 31, 2018 17:01
@danmar danmar merged commit a61f21d into cppcheck-opensource:master Feb 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@amai2012 amai2012 amai2012 approved these changes

@danmar danmar Awaiting requested review from danmar

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@jokva @amai2012 @danmar