astutils.cpp: added more bailouts in isMutableExpression()
#6959
Conversation
|
@pfultz2 Please have a look. I obviously have no idea what I am doing. The printing of the tokens we pass to The namespace prefix does not seem like any expression that might have any impact. |
lib/astutils.cpp
Outdated
| if (tok->isLiteral() || tok->isKeyword() || tok->isStandardType() || tok->isEnumerator()) | ||
| return false; | ||
| if (Token::Match(tok, ",|;|:|]|)|}")) | ||
| if (Token::Match(tok, ",|;|:|]|)|}|::")) |
There was a problem hiding this comment.
You shouldn't bail on ::, this will miss variable changes. Instead you should check astOperand2()(or perhaps both) when its a :::
if (Token::Match(tok, "::"))
return isMutableExpression(tok->astOperand2());There was a problem hiding this comment.
Given the example above in case of std :: strncmp that will pass any of that to this function and my understanding would be that it makes no sense for std :: in that case. Same for a type like std :: string which I also see.
The case you refer to would be a scoped variable. I wonder if all parts would also be passed as an expression that should be changed.
I have the feeling that namespaces are not handled properly here. I guess it requires some tests to be written.
There was a problem hiding this comment.
You shouldn't bail on
::, this will miss variable changes.
It is called on all tokens (at least in terms of find aliases) so I think it would only make sense for the actual variable one to proceed on but not the scoping stuff or type. And neither for any function calls.
I dont really have a context of what you are trying to do here, but it is definitely problematic. |
I want to reduce the amount of calls to |
firewave
force-pushed
the
expr-chg
branch
5 times, most recently
from
c8642a7 to
fc864e2
Compare
|
|
Closing in favor of #8145. This was mostly about starting a discussion. |
2 participants
No description provided.