<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Feb 17, 2016 at 2:04 PM, Zachary Turner <span dir="ltr"><<a href="mailto:zturner@google.com" target="_blank">zturner@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Actually *this = Variant() in the destructor is recursive, since that requires destroying the temporary.</div></blockquote><div><br></div><div>Ah, fair point indeed. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5"><br><div class="gmail_quote"><div dir="ltr">On Wed, Feb 17, 2016 at 1:34 PM Zachary Turner <<a href="mailto:zturner@google.com" target="_blank">zturner@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Yea, sounds reasonable. I'll get that in as a followup.</div><br><div class="gmail_quote"><div dir="ltr">On Wed, Feb 17, 2016 at 1:32 PM David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Wed, Feb 17, 2016 at 1:13 PM, Zachary Turner via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: zturner<br>
Date: Wed Feb 17 15:13:15 2016<br>
New Revision: 261152<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=261152&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=261152&view=rev</a><br>
Log:<br>
[DebugInfoPDB] Teach Variant to support string types.<br>
<br>
The IDiaSymbol::getValue() method returns a variant. Until now,<br>
I had never encountered a string value, so the Variant wrapper<br>
did not support VT_BSTR. Now we have need to support string<br>
values, so this patch just adds support for one extra type to<br>
Variant.<br>
<br>
Modified:<br>
llvm/trunk/include/llvm/DebugInfo/PDB/PDBTypes.h<br>
llvm/trunk/lib/DebugInfo/PDB/DIA/DIARawSymbol.cpp<br>
llvm/trunk/lib/DebugInfo/PDB/PDBExtras.cpp<br>
<br>
Modified: llvm/trunk/include/llvm/DebugInfo/PDB/PDBTypes.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/DebugInfo/PDB/PDBTypes.h?rev=261152&r1=261151&r2=261152&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/DebugInfo/PDB/PDBTypes.h?rev=261152&r1=261151&r2=261152&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/include/llvm/DebugInfo/PDB/PDBTypes.h (original)<br>
+++ llvm/trunk/include/llvm/DebugInfo/PDB/PDBTypes.h Wed Feb 17 15:13:15 2016<br>
@@ -350,6 +350,7 @@ enum PDB_VariantType {<br>
UInt32,<br>
UInt64,<br>
Bool,<br>
+ String<br>
};<br>
<br>
struct Variant {<br>
@@ -357,6 +358,15 @@ struct Variant {<br>
: Type(PDB_VariantType::Empty) {<br>
}<br>
<br>
+ Variant(const Variant &Other) : Type(PDB_VariantType::Empty) {<br>
+ *this = Other;<br>
+ }<br>
+<br>
+ ~Variant() {<br>
+ if (Type == PDB_VariantType::String && Value.String != nullptr)<br></blockquote><div><br></div></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>delete is a no-op on null pointers, so there's no need for the Value.String != nullptr condition here & in the assignment operator.<br><br>It might be nice to unify the code in these two places, perhaps? Maybe just implement the dtor as "*this = Variant()", the extra couple of assignments are probably cheap enough? No big deal, though, if you prefer it as-is.</div></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ delete[] Value.String;<br>
+ }<br>
+<br>
PDB_VariantType Type;<br>
union {<br>
bool Bool;<br>
@@ -370,10 +380,11 @@ struct Variant {<br>
uint16_t UInt16;<br>
uint32_t UInt32;<br>
uint64_t UInt64;<br>
- };<br>
+ char *String;<br>
+ } Value;<br>
#define VARIANT_EQUAL_CASE(Enum) \<br>
case PDB_VariantType::Enum: \<br>
- return Enum == Other.Enum;<br>
+ return Value.Enum == Other.Value.Enum;<br>
bool operator==(const Variant &Other) const {<br>
if (Type != Other.Type)<br>
return false;<br>
@@ -389,12 +400,27 @@ struct Variant {<br>
VARIANT_EQUAL_CASE(UInt16)<br>
VARIANT_EQUAL_CASE(UInt32)<br>
VARIANT_EQUAL_CASE(UInt64)<br>
+ VARIANT_EQUAL_CASE(String)<br>
default:<br>
return true;<br>
}<br>
}<br>
#undef VARIANT_EQUAL_CASE<br>
bool operator!=(const Variant &Other) const { return !(*this == Other); }<br>
+ Variant &operator=(const Variant &Other) {<br>
+ if (this == &Other)<br>
+ return *this;<br>
+ if (Type == PDB_VariantType::String && Value.String != nullptr)<br>
+ delete[] Value.String;<br>
+ Type = Other.Type;<br>
+ Value = Other.Value;<br>
+ if (Other.Type == PDB_VariantType::String &&<br>
+ Other.Value.String != nullptr) {<br>
+ Value.String = new char[strlen(Other.Value.String) + 1];<br>
+ ::strcpy(Value.String, Other.Value.String);<br>
+ }<br>
+ return *this;<br>
+ }<br>
};<br>
<br>
namespace PDB {<br>
<br>
Modified: llvm/trunk/lib/DebugInfo/PDB/DIA/DIARawSymbol.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/DebugInfo/PDB/DIA/DIARawSymbol.cpp?rev=261152&r1=261151&r2=261152&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/DebugInfo/PDB/DIA/DIARawSymbol.cpp?rev=261152&r1=261151&r2=261152&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/DebugInfo/PDB/DIA/DIARawSymbol.cpp (original)<br>
+++ llvm/trunk/lib/DebugInfo/PDB/DIA/DIARawSymbol.cpp Wed Feb 17 15:13:15 2016<br>
@@ -22,49 +22,60 @@ Variant VariantFromVARIANT(const VARIANT<br>
Variant Result;<br>
switch (V.vt) {<br>
case VT_I1:<br>
- Result.Int8 = V.cVal;<br>
+ Result.Value.Int8 = V.cVal;<br>
Result.Type = PDB_VariantType::Int8;<br>
break;<br>
case VT_I2:<br>
- Result.Int16 = V.iVal;<br>
+ Result.Value.Int16 = V.iVal;<br>
Result.Type = PDB_VariantType::Int16;<br>
break;<br>
case VT_I4:<br>
- Result.Int32 = V.intVal;<br>
+ Result.Value.Int32 = V.intVal;<br>
Result.Type = PDB_VariantType::Int32;<br>
break;<br>
case VT_I8:<br>
- Result.Int64 = V.llVal;<br>
+ Result.Value.Int64 = V.llVal;<br>
Result.Type = PDB_VariantType::Int64;<br>
break;<br>
case VT_UI1:<br>
- Result.UInt8 = V.bVal;<br>
+ Result.Value.UInt8 = V.bVal;<br>
Result.Type = PDB_VariantType::UInt8;<br>
break;<br>
case VT_UI2:<br>
- Result.UInt16 = V.uiVal;<br>
+ Result.Value.UInt16 = V.uiVal;<br>
Result.Type = PDB_VariantType::UInt16;<br>
break;<br>
case VT_UI4:<br>
- Result.UInt32 = V.uintVal;<br>
+ Result.Value.UInt32 = V.uintVal;<br>
Result.Type = PDB_VariantType::UInt32;<br>
break;<br>
case VT_UI8:<br>
- Result.UInt64 = V.ullVal;<br>
+ Result.Value.UInt64 = V.ullVal;<br>
Result.Type = PDB_VariantType::UInt64;<br>
break;<br>
case VT_BOOL:<br>
- Result.Bool = (V.boolVal == VARIANT_TRUE) ? true : false;<br>
+ Result.Value.Bool = (V.boolVal == VARIANT_TRUE) ? true : false;<br>
Result.Type = PDB_VariantType::Bool;<br>
break;<br>
case VT_R4:<br>
- Result.Single = V.fltVal;<br>
+ Result.Value.Single = V.fltVal;<br>
Result.Type = PDB_VariantType::Single;<br>
break;<br>
case VT_R8:<br>
- Result.Double = V.dblVal;<br>
+ Result.Value.Double = V.dblVal;<br>
Result.Type = PDB_VariantType::Double;<br>
break;<br>
+ case VT_BSTR: {<br>
+ const char *SrcBytes = reinterpret_cast<const char *>(V.bstrVal);<br>
+ llvm::ArrayRef<char> SrcByteArray(SrcBytes, SysStringByteLen(V.bstrVal));<br>
+ std::string Result8;<br>
+ if (!llvm::convertUTF16ToUTF8String(SrcByteArray, Result8))<br>
+ Result.Value.String = nullptr;<br>
+ Result.Value.String = new char[Result8.length() + 1];<br>
+ ::strcpy(Result.Value.String, Result8.c_str());<br>
+ Result.Type = PDB_VariantType::String;<br>
+ break;<br>
+ }<br>
default:<br>
Result.Type = PDB_VariantType::Unknown;<br>
break;<br>
<br>
Modified: llvm/trunk/lib/DebugInfo/PDB/PDBExtras.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/DebugInfo/PDB/PDBExtras.cpp?rev=261152&r1=261151&r2=261152&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/DebugInfo/PDB/PDBExtras.cpp?rev=261152&r1=261151&r2=261152&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/DebugInfo/PDB/PDBExtras.cpp (original)<br>
+++ llvm/trunk/lib/DebugInfo/PDB/PDBExtras.cpp Wed Feb 17 15:13:15 2016<br>
@@ -284,37 +284,40 @@ raw_ostream &llvm::operator<<(raw_ostrea<br>
raw_ostream &llvm::operator<<(raw_ostream &OS, const Variant &Value) {<br>
switch (Value.Type) {<br>
case PDB_VariantType::Bool:<br>
- OS << (Value.Bool ? "true" : "false");<br>
+ OS << (Value.Value.Bool ? "true" : "false");<br>
break;<br>
case PDB_VariantType::Double:<br>
- OS << Value.Double;<br>
+ OS << Value.Value.Double;<br>
break;<br>
case PDB_VariantType::Int16:<br>
- OS << Value.Int16;<br>
+ OS << Value.Value.Int16;<br>
break;<br>
case PDB_VariantType::Int32:<br>
- OS << Value.Int32;<br>
+ OS << Value.Value.Int32;<br>
break;<br>
case PDB_VariantType::Int64:<br>
- OS << Value.Int64;<br>
+ OS << Value.Value.Int64;<br>
break;<br>
case PDB_VariantType::Int8:<br>
- OS << static_cast<int>(Value.Int8);<br>
+ OS << static_cast<int>(Value.Value.Int8);<br>
break;<br>
case PDB_VariantType::Single:<br>
- OS << Value.Single;<br>
+ OS << Value.Value.Single;<br>
break;<br>
case PDB_VariantType::UInt16:<br>
- OS << Value.Double;<br>
+ OS << Value.Value.Double;<br>
break;<br>
case PDB_VariantType::UInt32:<br>
- OS << Value.UInt32;<br>
+ OS << Value.Value.UInt32;<br>
break;<br>
case PDB_VariantType::UInt64:<br>
- OS << Value.UInt64;<br>
+ OS << Value.Value.UInt64;<br>
break;<br>
case PDB_VariantType::UInt8:<br>
- OS << static_cast<unsigned>(Value.UInt8);<br>
+ OS << static_cast<unsigned>(Value.Value.UInt8);<br>
+ break;<br>
+ case PDB_VariantType::String:<br>
+ OS << Value.Value.String;<br>
break;<br>
default:<br>
OS << Value.Type;<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div></div></div></blockquote></div></blockquote></div>
</div></div></blockquote></div><br></div></div>