replaced global emptyString used for const reference results with local instances
#7250
Conversation
lib/checkother.cpp
Outdated
| @@ -4364,7 +4364,8 @@ void CheckOther::overlappingWriteUnion(const Token *tok) | |||
|
|
|||
| void CheckOther::overlappingWriteFunction(const Token *tok) | |||
| { | |||
| const std::string &funcname = tok ? tok->str() : emptyString; | |||
There was a problem hiding this comment.
Seems like this overlaps with the intent of #7251 (should just use "")
There was a problem hiding this comment.
Good catch. That should never be nullptr otherwise the error message would have an empty function name. I will align it to match existing code and integrate with the other PR.
There was a problem hiding this comment.
I assume that code exists for the getErrorMessages() invocation.
firewave
force-pushed
the
emptystring-x
branch
from
3e1195c to
dad4fa3
Compare
lib/symboldatabase.cpp
Outdated
| else if (!start->isName()) | ||
| return emptyString; | ||
| else if (!start->isName()) { | ||
| static const std::string s_empty_string; |
There was a problem hiding this comment.
Has anyone ever benchmarked this? I wonder if the static object can provide any benefits, since it is returned immediately, and RVO should just construct the object at the target location anyway.
There was a problem hiding this comment.
Oh. I expected this to be a const reference. Will back out.
Has anyone ever benchmarked this?
I have at one point with it being dropped completely and it caused a sizable performance impact. That's why I am doing the removal in steps so it is easier to pinpoint potential regressions. Will profile it again after I get rid of the cold stuff.
The whole thing might only be necessary with older C++ standards (pre-C++17?).
There was a problem hiding this comment.
With current GCC, the static version is even slower: https://quick-bench.com/q/goY88i2idDwpvg2VMh9bquG1Tds
No difference with clang. Have not checked older standards.
There was a problem hiding this comment.
Interesting. Good to know ahead of doing the profiling for this.
FYI some slightly related things:
llvm/llvm-project#58002
llvm/llvm-project#60165
firewave
force-pushed
the
emptystring-x
branch
from
dad4fa3 to
ff016c9
Compare
No description provided.