[PATCH] D27991: Optimize objdump -objc-meta-data
Saleem Abdulrasool via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Jan 3 15:20:35 PST 2017
compnerd added inline comments.
================
Comment at: lib/Object/MachOObjectFile.cpp:2827
+ assert(
+ (Opcodes.data() == Other.Opcodes.data() || Opcodes == Other.Opcodes) &&
+ "compare iterators of different files");
----------------
kastiglione wrote:
> compnerd wrote:
> > Why not just change this to:
> >
> > assert(Opcodes.data() == Other.Opcodes.data() && Opcodes.size() == Other.Opcodes.size() && "comparing differing opcodes");
> The current assertion has value semantics, this change would switch the assertion to allow reference equality only. I'm not sure why the assertion is currently constructed the way it is. I don't think any of llvm would break, but some external code could in theory. If you think it's best to change, I'll go with it.
Except that after your change, you are short-circuiting the check to pointer equality rather than value equality. So, changing it entirely to pointer equality might be the best approach. Are expensive asserts enabled by default? We could just demote this to an expensive check and leave it as a full value comparison as well.
https://reviews.llvm.org/D27991
More information about the llvm-commits
mailing list