[Lldb-commits] [PATCH] D16563: Set symbol types for function symbols loaded from PE/COFF

[Adrian McCarthy via lldb-commits]

amccarth marked 2 inline comments as done.

================
Comment at: include/lldb/Core/RangeMap.h:1230
@@ -1229,3 +1229,3 @@
                 typename Collection::const_iterator pos;
-                for(pos = m_entries.begin(); pos != m_entries.end(); pos++)
+                for (pos = m_entries.begin(); pos != m_entries.end(); pos++)
                 {
----------------
zturner wrote:
> If you're making cleanup changes anyway, how about a ranged based for.
I had thought about it, especially since it violates the LLVM style guide to call end() on every iteration, but I was trying not to venture too far from the point of the patch.  Done now.

================
Comment at: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:183
@@ +182,3 @@
+    }
+    return lldb::eSymbolTypeInvalid;
+}
----------------
zturner wrote:
> Instead of returning `eSymbolTypeInvalid` here, how about `eSymbolTypeData`?  If you look in `llvm/Support/COFF.h` all of the non function types are data.
I'm not sure about that.  Some of the symbols are clearly sections (.text, etc.).  Those have 0 for the COFF type, which COFF.h says means "No type information or unknown base type."  If .text has a "valid" symbol type, then it will be found (instead of, say, "_main"), and I'm not sure if the unwinding/stepping would work right.


http://reviews.llvm.org/D16563