[PATCH] Add DWARFFormValue::getAsSignedConstant().
vonosmas at gmail.com
Thu Oct 9 18:31:04 PDT 2014
Comment at: unittests/DebugInfo/DWARFFormValueTest.cpp:10
@@ -9,2 +9,3 @@
> samsonov wrote:
> > friss wrote:
> > > samsonov wrote:
> > > > Unittests may only include headers from include/llvm (i.e., public headers). If we want to add unittests that instantiate DWARFContext, we should make corresponding header public.
> > > I was actually wondering about this, but I found one prior example of including ../lib/. I'm not sure what the best way forward is for this, if we make the Context public, then everything gets public. Not that I'm opposed to that, it would make some of my planned work easier, and I might propose that in the near future.
> > >
> > > What would people think about making the whole DWARF stuff a pubilc interface to libDebugInfo ? (Note that right now this is just to get some testing for this very small feature added just to be able to print AT_lower_bound as a signed decimal... I was already embarrassed by the amount of Dummy* instantiations to get my small test running, but getting the whole interface made public just for that seems a bit too much, doesn't it?)
> > I don't really know. Certainly, this set up is an overkill for a small unit test you have. Testing isolated components of DWARF parser are hard because of all the relationship between various sections. However, we can expose DWARFContext.h in public headers somewhere under include/llvm/DebugInfo/DWARF, then create a DWARFTestContext in unittests/, and reuse it in various unit tests. I'm not sure if it would be convenient, or we'd actually need that many unit tests.
> But if we put DWARFContext.h in the public headers, we have to put (nearly) everything else. Just look at the list of includes at the beginning of DWARFContext.h.
Sure, I understand this and therefore feel reluctant about this shift. We can do this, but only if it allows us to write a lot of good and helpful unit-tests. I don't yet see how unit-testing a DWARF parser routines can be made easily.
More information about the llvm-commits