[PATCH] D30357: [ObjectYAML] NFC. Refactor DWARFYAML CompileUnit dump code

Adrian Prantl via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Mar 2 15:43:32 PST 2017


aprantl added inline comments.


================
Comment at: lib/ObjectYAML/DWARFVisitor.cpp:38
+template <typename T>
+size_t DWARFYAML::VisitorImpl<T>::GetRefSize(const DWARFYAML::Unit &Unit) {
+  if (Unit.Version == 2)
----------------
beanz wrote:
> aprantl wrote:
> > we probably want a target-independent type like uint64_t here?
> > 
> > why not make this static uint64_t getRefSize(DebugInfo, Unit) instead?
> I think using `size_type` here is arguably more correct, because it is a size value.
> 
> That said, since the value will never be larger than the max value of AddrSize (which is uint8_t), using uint64_t would be overkill, and has performance issues on some 32-bit hosts.
> 
> Do you have strong feelings on this?
If my memory serves correctly C doesn't guarantee that size_t is larger than unsigned.
I don't feel strongly about it and in this particular case it shouldn't matter. Personally I would probably use unsigned here.


https://reviews.llvm.org/D30357





More information about the llvm-commits mailing list