[llvm] r261152 - [DebugInfoPDB] Teach Variant to support string types.
David Blaikie via llvm-commits
llvm-commits at lists.llvm.org
Wed Feb 17 14:14:58 PST 2016
On Wed, Feb 17, 2016 at 2:04 PM, Zachary Turner <zturner at google.com> wrote:
> Actually *this = Variant() in the destructor is recursive, since that
> requires destroying the temporary.
>
Ah, fair point indeed.
>
> On Wed, Feb 17, 2016 at 1:34 PM Zachary Turner <zturner at google.com> wrote:
>
>> Yea, sounds reasonable. I'll get that in as a followup.
>>
>> On Wed, Feb 17, 2016 at 1:32 PM David Blaikie <dblaikie at gmail.com> wrote:
>>
>>> On Wed, Feb 17, 2016 at 1:13 PM, Zachary Turner via llvm-commits <
>>> llvm-commits at lists.llvm.org> wrote:
>>>
>>>> Author: zturner
>>>> Date: Wed Feb 17 15:13:15 2016
>>>> New Revision: 261152
>>>>
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=261152&view=rev
>>>> Log:
>>>> [DebugInfoPDB] Teach Variant to support string types.
>>>>
>>>> The IDiaSymbol::getValue() method returns a variant. Until now,
>>>> I had never encountered a string value, so the Variant wrapper
>>>> did not support VT_BSTR. Now we have need to support string
>>>> values, so this patch just adds support for one extra type to
>>>> Variant.
>>>>
>>>> Modified:
>>>> llvm/trunk/include/llvm/DebugInfo/PDB/PDBTypes.h
>>>> llvm/trunk/lib/DebugInfo/PDB/DIA/DIARawSymbol.cpp
>>>> llvm/trunk/lib/DebugInfo/PDB/PDBExtras.cpp
>>>>
>>>> Modified: llvm/trunk/include/llvm/DebugInfo/PDB/PDBTypes.h
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/DebugInfo/PDB/PDBTypes.h?rev=261152&r1=261151&r2=261152&view=diff
>>>>
>>>> ==============================================================================
>>>> --- llvm/trunk/include/llvm/DebugInfo/PDB/PDBTypes.h (original)
>>>> +++ llvm/trunk/include/llvm/DebugInfo/PDB/PDBTypes.h Wed Feb 17
>>>> 15:13:15 2016
>>>> @@ -350,6 +350,7 @@ enum PDB_VariantType {
>>>> UInt32,
>>>> UInt64,
>>>> Bool,
>>>> + String
>>>> };
>>>>
>>>> struct Variant {
>>>> @@ -357,6 +358,15 @@ struct Variant {
>>>> : Type(PDB_VariantType::Empty) {
>>>> }
>>>>
>>>> + Variant(const Variant &Other) : Type(PDB_VariantType::Empty) {
>>>> + *this = Other;
>>>> + }
>>>> +
>>>> + ~Variant() {
>>>> + if (Type == PDB_VariantType::String && Value.String != nullptr)
>>>>
>>>
>>> delete is a no-op on null pointers, so there's no need for the
>>> Value.String != nullptr condition here & in the assignment operator.
>>>
>>> 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.
>>>
>>>
>>>> + delete[] Value.String;
>>>> + }
>>>> +
>>>> PDB_VariantType Type;
>>>> union {
>>>> bool Bool;
>>>> @@ -370,10 +380,11 @@ struct Variant {
>>>> uint16_t UInt16;
>>>> uint32_t UInt32;
>>>> uint64_t UInt64;
>>>> - };
>>>> + char *String;
>>>> + } Value;
>>>> #define VARIANT_EQUAL_CASE(Enum)
>>>> \
>>>> case PDB_VariantType::Enum:
>>>> \
>>>> - return Enum == Other.Enum;
>>>> + return Value.Enum == Other.Value.Enum;
>>>> bool operator==(const Variant &Other) const {
>>>> if (Type != Other.Type)
>>>> return false;
>>>> @@ -389,12 +400,27 @@ struct Variant {
>>>> VARIANT_EQUAL_CASE(UInt16)
>>>> VARIANT_EQUAL_CASE(UInt32)
>>>> VARIANT_EQUAL_CASE(UInt64)
>>>> + VARIANT_EQUAL_CASE(String)
>>>> default:
>>>> return true;
>>>> }
>>>> }
>>>> #undef VARIANT_EQUAL_CASE
>>>> bool operator!=(const Variant &Other) const { return !(*this ==
>>>> Other); }
>>>> + Variant &operator=(const Variant &Other) {
>>>> + if (this == &Other)
>>>> + return *this;
>>>> + if (Type == PDB_VariantType::String && Value.String != nullptr)
>>>> + delete[] Value.String;
>>>> + Type = Other.Type;
>>>> + Value = Other.Value;
>>>> + if (Other.Type == PDB_VariantType::String &&
>>>> + Other.Value.String != nullptr) {
>>>> + Value.String = new char[strlen(Other.Value.String) + 1];
>>>> + ::strcpy(Value.String, Other.Value.String);
>>>> + }
>>>> + return *this;
>>>> + }
>>>> };
>>>>
>>>> namespace PDB {
>>>>
>>>> Modified: llvm/trunk/lib/DebugInfo/PDB/DIA/DIARawSymbol.cpp
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/DebugInfo/PDB/DIA/DIARawSymbol.cpp?rev=261152&r1=261151&r2=261152&view=diff
>>>>
>>>> ==============================================================================
>>>> --- llvm/trunk/lib/DebugInfo/PDB/DIA/DIARawSymbol.cpp (original)
>>>> +++ llvm/trunk/lib/DebugInfo/PDB/DIA/DIARawSymbol.cpp Wed Feb 17
>>>> 15:13:15 2016
>>>> @@ -22,49 +22,60 @@ Variant VariantFromVARIANT(const VARIANT
>>>> Variant Result;
>>>> switch (V.vt) {
>>>> case VT_I1:
>>>> - Result.Int8 = V.cVal;
>>>> + Result.Value.Int8 = V.cVal;
>>>> Result.Type = PDB_VariantType::Int8;
>>>> break;
>>>> case VT_I2:
>>>> - Result.Int16 = V.iVal;
>>>> + Result.Value.Int16 = V.iVal;
>>>> Result.Type = PDB_VariantType::Int16;
>>>> break;
>>>> case VT_I4:
>>>> - Result.Int32 = V.intVal;
>>>> + Result.Value.Int32 = V.intVal;
>>>> Result.Type = PDB_VariantType::Int32;
>>>> break;
>>>> case VT_I8:
>>>> - Result.Int64 = V.llVal;
>>>> + Result.Value.Int64 = V.llVal;
>>>> Result.Type = PDB_VariantType::Int64;
>>>> break;
>>>> case VT_UI1:
>>>> - Result.UInt8 = V.bVal;
>>>> + Result.Value.UInt8 = V.bVal;
>>>> Result.Type = PDB_VariantType::UInt8;
>>>> break;
>>>> case VT_UI2:
>>>> - Result.UInt16 = V.uiVal;
>>>> + Result.Value.UInt16 = V.uiVal;
>>>> Result.Type = PDB_VariantType::UInt16;
>>>> break;
>>>> case VT_UI4:
>>>> - Result.UInt32 = V.uintVal;
>>>> + Result.Value.UInt32 = V.uintVal;
>>>> Result.Type = PDB_VariantType::UInt32;
>>>> break;
>>>> case VT_UI8:
>>>> - Result.UInt64 = V.ullVal;
>>>> + Result.Value.UInt64 = V.ullVal;
>>>> Result.Type = PDB_VariantType::UInt64;
>>>> break;
>>>> case VT_BOOL:
>>>> - Result.Bool = (V.boolVal == VARIANT_TRUE) ? true : false;
>>>> + Result.Value.Bool = (V.boolVal == VARIANT_TRUE) ? true : false;
>>>> Result.Type = PDB_VariantType::Bool;
>>>> break;
>>>> case VT_R4:
>>>> - Result.Single = V.fltVal;
>>>> + Result.Value.Single = V.fltVal;
>>>> Result.Type = PDB_VariantType::Single;
>>>> break;
>>>> case VT_R8:
>>>> - Result.Double = V.dblVal;
>>>> + Result.Value.Double = V.dblVal;
>>>> Result.Type = PDB_VariantType::Double;
>>>> break;
>>>> + case VT_BSTR: {
>>>> + const char *SrcBytes = reinterpret_cast<const char *>(V.bstrVal);
>>>> + llvm::ArrayRef<char> SrcByteArray(SrcBytes,
>>>> SysStringByteLen(V.bstrVal));
>>>> + std::string Result8;
>>>> + if (!llvm::convertUTF16ToUTF8String(SrcByteArray, Result8))
>>>> + Result.Value.String = nullptr;
>>>> + Result.Value.String = new char[Result8.length() + 1];
>>>> + ::strcpy(Result.Value.String, Result8.c_str());
>>>> + Result.Type = PDB_VariantType::String;
>>>> + break;
>>>> + }
>>>> default:
>>>> Result.Type = PDB_VariantType::Unknown;
>>>> break;
>>>>
>>>> Modified: llvm/trunk/lib/DebugInfo/PDB/PDBExtras.cpp
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/DebugInfo/PDB/PDBExtras.cpp?rev=261152&r1=261151&r2=261152&view=diff
>>>>
>>>> ==============================================================================
>>>> --- llvm/trunk/lib/DebugInfo/PDB/PDBExtras.cpp (original)
>>>> +++ llvm/trunk/lib/DebugInfo/PDB/PDBExtras.cpp Wed Feb 17 15:13:15 2016
>>>> @@ -284,37 +284,40 @@ raw_ostream &llvm::operator<<(raw_ostrea
>>>> raw_ostream &llvm::operator<<(raw_ostream &OS, const Variant &Value) {
>>>> switch (Value.Type) {
>>>> case PDB_VariantType::Bool:
>>>> - OS << (Value.Bool ? "true" : "false");
>>>> + OS << (Value.Value.Bool ? "true" : "false");
>>>> break;
>>>> case PDB_VariantType::Double:
>>>> - OS << Value.Double;
>>>> + OS << Value.Value.Double;
>>>> break;
>>>> case PDB_VariantType::Int16:
>>>> - OS << Value.Int16;
>>>> + OS << Value.Value.Int16;
>>>> break;
>>>> case PDB_VariantType::Int32:
>>>> - OS << Value.Int32;
>>>> + OS << Value.Value.Int32;
>>>> break;
>>>> case PDB_VariantType::Int64:
>>>> - OS << Value.Int64;
>>>> + OS << Value.Value.Int64;
>>>> break;
>>>> case PDB_VariantType::Int8:
>>>> - OS << static_cast<int>(Value.Int8);
>>>> + OS << static_cast<int>(Value.Value.Int8);
>>>> break;
>>>> case PDB_VariantType::Single:
>>>> - OS << Value.Single;
>>>> + OS << Value.Value.Single;
>>>> break;
>>>> case PDB_VariantType::UInt16:
>>>> - OS << Value.Double;
>>>> + OS << Value.Value.Double;
>>>> break;
>>>> case PDB_VariantType::UInt32:
>>>> - OS << Value.UInt32;
>>>> + OS << Value.Value.UInt32;
>>>> break;
>>>> case PDB_VariantType::UInt64:
>>>> - OS << Value.UInt64;
>>>> + OS << Value.Value.UInt64;
>>>> break;
>>>> case PDB_VariantType::UInt8:
>>>> - OS << static_cast<unsigned>(Value.UInt8);
>>>> + OS << static_cast<unsigned>(Value.Value.UInt8);
>>>> + break;
>>>> + case PDB_VariantType::String:
>>>> + OS << Value.Value.String;
>>>> break;
>>>> default:
>>>> OS << Value.Type;
>>>>
>>>>
>>>> _______________________________________________
>>>> llvm-commits mailing list
>>>> llvm-commits at lists.llvm.org
>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>>
>>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160217/ec3bbc86/attachment.html>
More information about the llvm-commits
mailing list