fix #13155: Tokenizer::simplifyTypedef does not set info correctly for typedef in function #6888
Conversation
There was a problem hiding this comment.
I think some unit testing would be good. test/testsimplifytypedef.cpp to start with.
However since you change TokenList::copyTokens I feel there is a danger you change behavior for other simplifications also. So ideally we would check the various copyTokens usages and determine that the result is wanted.
| @@ -292,6 +294,9 @@ Token *TokenList::copyTokens(Token *dest, const Token *first, const Token *last, | |||
|
|
|||
| links.pop(); | |||
| } | |||
|
|
|||
| column += tok->next() ? tok->next()->column() - tok->column() : tok->str().length() + 1; | |||
There was a problem hiding this comment.
I am not so sure about this. At least when we replace a token it's better that the column just match the replaced token. imagine MY_INT generates a warning the warning should point at MY_INT not at some other location later..
| @@ -267,12 +267,14 @@ Token *TokenList::copyTokens(Token *dest, const Token *first, const Token *last, | |||
| std::stack<Token *> links; | |||
| Token *tok2 = dest; | |||
| int linenr = dest->linenr(); | |||
| int column = dest->next() ? dest->next()->column() : dest->column() + dest->str().length() + 1; | |||
There was a problem hiding this comment.
the dest->column() + dest->str().length() + 1 is unfortunate. It's an educated guess but could be very wrong.
| @@ -267,12 +267,14 @@ Token *TokenList::copyTokens(Token *dest, const Token *first, const Token *last, | |||
| std::stack<Token *> links; | |||
| Token *tok2 = dest; | |||
| int linenr = dest->linenr(); | |||
There was a problem hiding this comment.
this looks inconsistent. we read linenr from dest but column from dest->next().
should there be some more option for the location?
No description provided.