[llvm] r261152 - [DebugInfoPDB] Teach Variant to support string types.

Zachary Turner via llvm-commits llvm-commits at lists.llvm.org
Wed Feb 17 14:04:49 PST 2016


Actually *this = Variant() in the destructor is recursive, since that
requires destroying the temporary.

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/a91dce59/attachment.html>


More information about the llvm-commits mailing list