[PATCH] D14171: [ELF2] merge-string.s test fixed for win32 configuraton.

George Rimar via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 29 08:28:50 PDT 2015


grimar added a comment.

Idea of virtual space is clear for me. I still can imagine implementations of StringTableBuilder where size_t will not be enough (like keeping strings in multiple files), but that is not real case, I agree.

One last though:
When using size_t instead of strict type like uint_x we have different behavior of linker depending on environment. And that can cause the fails of tests like we had. Using of strict types can help to avoid that. So I mean even uint32_t here can be better that size_t since behavior between 32x/64x configurations will be consistent. But at the same time I dont see the solid reasons to artifically cap to 32 bits variable. That returns to idea to have template size type for StringTableBuilder, because truncation of 64 bits size_t to something like uint32_t is still bad to have. Or If StringTableBuilder really does not need that (I would agree now) then using of uint_x (assuming its size >= size_t) is still looks like a solution for behavior consistency.


http://reviews.llvm.org/D14171





More information about the llvm-commits mailing list