[PATCH] D30956: Introduce NativeEnumModules and NativeCompilandSymbol

[Bob Haarman via Phabricator via llvm-commits]

inglorion accepted this revision.
inglorion added a comment.

lgtm2



================
Comment at: llvm/lib/DebugInfo/PDB/Native/NativeEnumModules.cpp:32
+NativeEnumModules::getChildAtIndex(uint32_t Index) const {
+  if (Index >= Modules.size())
+    return nullptr;
----------------
amccarth wrote:
> inglorion wrote:
> > You're choosing to return nullptr here, instead of something else, e.g. crashing or asserting. I'm curious why you prefer to return nullptr. Could you explain your rationale?
> I'm following the lead set by the DIA versions of the enumerators I've looked at.  For example, DIAEnumDebugStreams.cpp returns nullptr if anything goes wrong querying DIA for the desired item, and calling with an invalid index is one possibility.
> 
> I'd be happy to assert instead if you think it's important.
I'm perfectly happy with us doing the same thing DIA does. Thanks for the explanation!


https://reviews.llvm.org/D30956