-
Notifications
You must be signed in to change notification settings - Fork 30
Conversation
These were deprecated or removed in C++17.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I have included two remarks to consider.
LOL 651987a I thought there is something fishy about this when I wrote it. But it "compiled" so I thought ok, probably some partial application is going on in there - that method has one argument but was called with zero. |
The comparison function should take two arguments and return [](const auto& i1, const auto& i2) { return i1.biggerFileOffset(i2); } Member function bool PELIB_IMAGE_SECTION_HEADER::biggerFileOffset(const PELIB_IMAGE_SECTION_HEADER& ish) const
{
return PointerToRawData < ish.PointerToRawData;
} So, the comparator is correct. As for the compilation, when a compiler encounters a template, it first only checks its syntax. It checks its semantic only after the template is instantiated with particular types/values. |
Thanks. I will merge this once RetDec tests that use this pass. |
This simplifies the code and allows us to use more readable lambdas than std::bind().
Everything looks good now, but I will still wait for RetDec regression tests to pass witch this branch - I'm currently running this code inside RetDec's |
C++17 deprecated and removed some language features. This made this project not compile inside RetDec.
I replaced these features and bumped Travis and Appveyor environments. I also bumped stuff in
README.md
andCHANGELOG.md
.Now it compiles without warnings or errors.
Since PeLib does not have its own unit or regression tests, can one (both?) of you (@s3rvac, @metthal) please look at it? I was careful, but it is easy to mess up those lambdas :-)
I will need this for avast/retdec#650. But for now, I will use reference to this branch (instead of
master
) in RetDec, so it is not a total blocker. It will also run RetDec tests with these changes, so it may catch some problems before we merge here.