TestPreprocessor: removed usage of std::istringstream#6873
TestPreprocessor: removed usage of std::istringstream#6873firewave merged 1 commit intodanmar:mainfrom
std::istringstream#6873Conversation
| private: | ||
| static std::string expandMacros(const char code[], ErrorLogger &errorLogger) { | ||
| std::istringstream istr(code); | ||
| template<size_t size> |
There was a problem hiding this comment.
I don't see the advantage at all here neither. What exactly is the advantage?
There was a problem hiding this comment.
We do not need the stream and get the size.
There was a problem hiding this comment.
I am not very convinced about this. It feels like using a template and complicating the code just for fun.
imho it's a bit weird syntax const char (&code)[size] we could have used a strlen or a std::string. this is test code.
There was a problem hiding this comment.
That's not for fun. Some code in the core is just provided to accommodate the tests and that should be get rid of (and will be done in other changes).
And we run the test code mostly in debug i.e. no optimizations so it is slower than production code and thus having slower code for convenience should be avoided. Makes me wonder if unoptimized matchcompiled code might behave better.
firewave
changed the title
std::istringstream
firewave
force-pushed
the
preproc-xxx-3
branch
from
922432b to
1a0f92d
Compare
|
danmar
left a comment
danmar
left a comment
There was a problem hiding this comment.
I don't like it really. I feel you uglify/complicate the testcode.
| private: | ||
| static std::string expandMacros(const char code[], ErrorLogger &errorLogger) { | ||
| std::istringstream istr(code); | ||
| template<size_t size> |
There was a problem hiding this comment.
I am not very convinced about this. It feels like using a template and complicating the code just for fun.
imho it's a bit weird syntax const char (&code)[size] we could have used a strlen or a std::string. this is test code.
It only "uglifies" the functions you call and in most cases it does not have much on the tests themselves. As seen in #6379 it helps with getting rid of unnecessary (slow) wrappers and overloads in the production code. I did not adjust tests where it really makes things worse. There's also a simplecpp PR which makes things better. |
2 participants
No description provided.