[llvm] r190322 - Debug Info: Rename DITypeRef to DIScopeRef.
Manman Ren
manman.ren at gmail.com
Mon Sep 9 13:29:39 PDT 2013
On Mon, Sep 9, 2013 at 1:12 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
>
>
> On Mon, Sep 9, 2013 at 12:03 PM, Manman Ren <manman.ren at gmail.com> wrote:
>
>> Author: mren
>> Date: Mon Sep 9 14:03:51 2013
>> New Revision: 190322
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=190322&view=rev
>> Log:
>> Debug Info: Rename DITypeRef to DIScopeRef.
>>
>
> This is a little unfortunate to drop some of the type safety/implicit
> documentation by muddying every DIType member in the DI* hierarchy with
> DIScopes. Did you consider other designs that would avoid this
> conflation/ambiguity? Might it be worth having two types (DIScopeRef and
> DITypeRef - could even be typedefs of specializations of a common template)?
>
Yes, I thought about DITypeRef being a subclass of DIScopeRef, but since
they resolve in the same way, and
the only difference is during verification, I choose to implement two
different verification helpers (isScopeRef vs isTypeRef).
DITypeRef should differ from DIScopeRef in the constructor: assertion on
isTypeRef instead of on isScopeRef.
>
>>
>> A reference to a scope is more general than a reference to a type since
>> DIType is a subclass of DIScope.
>>
>> A reference to a type can be either an identifier for the type or
>> the DIType itself, while a reference to a scope can be either an
>> identifier for the type (when the scope is indeed a type) or the
>> DIScope itself. A reference to a type and a reference to a scope
>> will be resolved in the same way. The only difference is in the
>> verifier when a field is a reference to a type (i.e. the containing
>> type field of a DICompositeType) or a field is a reference to a scope
>> (i.e. the context field of a DIType).
>>
>> This is to get ready for switching DIType::getContext to return
>> DIScopeRef instead of DIScope.
>>
>> Tighten up isTypeRef and isScopeRef to make sure the identifier is not
>> empty and the MDNode is DIType for TypeRef and DIScope for ScopeRef.
>>
>> Modified:
>> llvm/trunk/include/llvm/DebugInfo.h
>> llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
>> llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h
>> llvm/trunk/lib/IR/DebugInfo.cpp
>>
>> Modified: llvm/trunk/include/llvm/DebugInfo.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/DebugInfo.h?rev=190322&r1=190321&r2=190322&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/include/llvm/DebugInfo.h (original)
>> +++ llvm/trunk/include/llvm/DebugInfo.h Mon Sep 9 14:03:51 2013
>> @@ -46,7 +46,7 @@ namespace llvm {
>> class DILexicalBlockFile;
>> class DIVariable;
>> class DIType;
>> - class DITypeRef;
>> + class DIScopeRef;
>> class DIObjCProperty;
>>
>> /// Maps from type identifier to the actual MDNode.
>> @@ -56,9 +56,9 @@ namespace llvm {
>> /// This should not be stored in a container, because the underlying
>> MDNode
>> /// may change in certain situations.
>> class DIDescriptor {
>> - // Befriends DITypeRef so DITypeRef can befriend the protected member
>> - // function: getFieldAs<DITypeRef>.
>> - friend class DITypeRef;
>> + // Befriends DIScopeRef so DIScopeRef can befriend the protected
>> member
>> + // function: getFieldAs<DIScopeRef>.
>> + friend class DIScopeRef;
>> public:
>> enum {
>> FlagPrivate = 1 << 0,
>> @@ -151,9 +151,9 @@ namespace llvm {
>> void dump() const;
>> };
>>
>> - /// Specialize getFieldAs to handle fields that are references to
>> DITypes.
>> + /// npecialize getFieldAs to handle fields that are references to
>> DIScopes.
>> template <>
>> - DITypeRef DIDescriptor::getFieldAs<DITypeRef>(unsigned Elt) const;
>> + DIScopeRef DIDescriptor::getFieldAs<DIScopeRef>(unsigned Elt) const;
>>
>> /// DISubrange - This is used to represent ranges, for array bounds.
>> class DISubrange : public DIDescriptor {
>> @@ -205,6 +205,25 @@ namespace llvm {
>> DIScope getContext() const;
>> StringRef getFilename() const;
>> StringRef getDirectory() const;
>> +
>> + /// Generate a reference to this DIScope. Uses the type identifier
>> instead
>> + /// of the actual MDNode if possible, to help type uniquing.
>> + Value *generateRef();
>> + };
>> +
>> + /// Represents reference to a DIScope, abstracts over direct and
>> + /// identifier-based metadata scope references.
>> + class DIScopeRef {
>> + template <typename DescTy>
>> + friend DescTy DIDescriptor::getFieldAs(unsigned Elt) const;
>> +
>> + /// Val can be either a MDNode or a MDString, in the latter,
>> + /// MDString specifies the type identifier.
>> + const Value *Val;
>> + explicit DIScopeRef(const Value *V);
>> + public:
>> + DIScope resolve(const DITypeIdentifierMap &Map) const;
>> + operator Value *() const { return const_cast<Value*>(Val); }
>> };
>>
>> /// DIType - This is a wrapper for a type.
>> @@ -271,32 +290,12 @@ namespace llvm {
>> /// isUnsignedDIType - Return true if type encoding is unsigned.
>> bool isUnsignedDIType();
>>
>> - /// Generate a reference to this DIType. Uses the type identifier
>> instead
>> - /// of the actual MDNode if possible, to help type uniquing.
>> - DITypeRef generateRef();
>> -
>> /// replaceAllUsesWith - Replace all uses of debug info referenced by
>> /// this descriptor.
>> void replaceAllUsesWith(DIDescriptor &D);
>> void replaceAllUsesWith(MDNode *D);
>> };
>>
>> - /// Represents reference to a DIType, abstracts over direct and
>> - /// identifier-based metadata type references.
>> - class DITypeRef {
>> - template <typename DescTy>
>> - friend DescTy DIDescriptor::getFieldAs(unsigned Elt) const;
>> - friend DITypeRef DIType::generateRef();
>> -
>> - /// TypeVal can be either a MDNode or a MDString, in the latter,
>> - /// MDString specifies the type identifier.
>> - const Value *TypeVal;
>> - explicit DITypeRef(const Value *V);
>> - public:
>> - DIType resolve(const DITypeIdentifierMap &Map) const;
>> - operator Value *() const { return const_cast<Value*>(TypeVal); }
>> - };
>> -
>> /// DIBasicType - A basic type, like 'int' or 'float'.
>> class DIBasicType : public DIType {
>> public:
>> @@ -328,9 +327,9 @@ namespace llvm {
>> /// associated with one.
>> MDNode *getObjCProperty() const;
>>
>> - DITypeRef getClassType() const {
>> + DIScopeRef getClassType() const {
>> assert(getTag() == dwarf::DW_TAG_ptr_to_member_type);
>> - return getFieldAs<DITypeRef>(10);
>> + return getFieldAs<DIScopeRef>(10);
>> }
>>
>> Constant *getConstant() const {
>> @@ -358,8 +357,8 @@ namespace llvm {
>> void setTypeArray(DIArray Elements, DIArray TParams = DIArray());
>> void addMember(DIDescriptor D);
>> unsigned getRunTimeLang() const { return getUnsignedField(11); }
>> - DITypeRef getContainingType() const {
>> - return getFieldAs<DITypeRef>(12);
>> + DIScopeRef getContainingType() const {
>> + return getFieldAs<DIScopeRef>(12);
>> }
>> void setContainingType(DICompositeType ContainingType);
>> DIArray getTemplateParams() const { return getFieldAs<DIArray>(13); }
>> @@ -426,8 +425,8 @@ namespace llvm {
>> unsigned getVirtuality() const { return getUnsignedField(10); }
>> unsigned getVirtualIndex() const { return getUnsignedField(11); }
>>
>> - DITypeRef getContainingType() const {
>> - return getFieldAs<DITypeRef>(12);
>> + DIScopeRef getContainingType() const {
>> + return getFieldAs<DIScopeRef>(12);
>> }
>>
>> unsigned getFlags() const {
>>
>> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp?rev=190322&r1=190321&r2=190322&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp (original)
>> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp Mon Sep 9 14:03:51
>> 2013
>> @@ -2633,7 +2633,7 @@ void DwarfDebug::emitDebugStrDWO() {
>> OffSec, StrSym);
>> }
>>
>> -/// Find the MDNode for the given type reference.
>> -MDNode *DwarfDebug::resolve(DITypeRef TRef) const {
>> - return TRef.resolve(TypeIdentifierMap);
>> +/// Find the MDNode for the given scope reference.
>> +DIScope DwarfDebug::resolve(DIScopeRef SRef) const {
>> + return SRef.resolve(TypeIdentifierMap);
>> }
>>
>> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h?rev=190322&r1=190321&r2=190322&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h (original)
>> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h Mon Sep 9 14:03:51
>> 2013
>> @@ -683,8 +683,8 @@ public:
>> /// Returns the Dwarf Version.
>> unsigned getDwarfVersion() const { return DwarfVersion; }
>>
>> - /// Find the MDNode for the given type reference.
>> - MDNode *resolve(DITypeRef TRef) const;
>> + /// Find the MDNode for the given scope reference.
>> + DIScope resolve(DIScopeRef SRef) const;
>>
>> };
>> } // End of namespace llvm
>>
>> Modified: llvm/trunk/lib/IR/DebugInfo.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/DebugInfo.cpp?rev=190322&r1=190321&r2=190322&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/IR/DebugInfo.cpp (original)
>> +++ llvm/trunk/lib/IR/DebugInfo.cpp Mon Sep 9 14:03:51 2013
>> @@ -426,17 +426,26 @@ static bool fieldIsMDString(const MDNode
>> return !Fld || isa<MDString>(Fld);
>> }
>>
>> -/// Check if a value can be a TypeRef.
>> +/// Check if a value can be a reference to a type.
>> static bool isTypeRef(const Value *Val) {
>> - return !Val || isa<MDString>(Val) || isa<MDNode>(Val);
>> + return !Val ||
>> + (isa<MDString>(Val) &&
>> !cast<MDString>(Val)->getString().empty()) ||
>> + (isa<MDNode>(Val) && DIType(cast<MDNode>(Val)).isType());
>> }
>>
>> -/// Check if a field at position Elt of a MDNode can be a TypeRef.
>> +/// Check if a field at position Elt of a MDNode can be a reference to a
>> type.
>> static bool fieldIsTypeRef(const MDNode *DbgNode, unsigned Elt) {
>> Value *Fld = getField(DbgNode, Elt);
>> return isTypeRef(Fld);
>> }
>>
>> +/// Check if a value can be a ScopeRef.
>> +static bool isScopeRef(const Value *Val) {
>> + return !Val ||
>> + (isa<MDString>(Val) &&
>> !cast<MDString>(Val)->getString().empty()) ||
>> + (isa<MDNode>(Val) && DIScope(cast<MDNode>(Val)).isScope());
>> +}
>> +
>> /// Verify - Verify that a type descriptor is well formed.
>> bool DIType::Verify() const {
>> if (!isType())
>> @@ -710,13 +719,13 @@ void DICompositeType::addMember(DIDescri
>>
>> /// Generate a reference to this DIType. Uses the type identifier instead
>> /// of the actual MDNode if possible, to help type uniquing.
>> -DITypeRef DIType::generateRef() {
>> +Value *DIScope::generateRef() {
>>
>
> Why did the return type of this function change?
>
This function is used by DIBuilder only, which uses the return value as a
"Value *", so I feel
it is unnecessary to invoke the constructor then another conversion
operator.
>
>
>> if (!isCompositeType())
>> - return DITypeRef(*this);
>> + return *this;
>> DICompositeType DTy(DbgNode);
>> if (!DTy.getIdentifier())
>> - return DITypeRef(*this);
>> - return DITypeRef(DTy.getIdentifier());
>> + return *this;
>> + return DTy.getIdentifier();
>> }
>>
>> /// \brief Set the containing type.
>> @@ -1428,33 +1437,30 @@ void DIVariable::printExtendedName(raw_o
>> }
>> }
>>
>> -DITypeRef::DITypeRef(const Value *V) : TypeVal(V) {
>> - assert(isTypeRef(V) && "DITypeRef should be a MDString or MDNode");
>> +DIScopeRef::DIScopeRef(const Value *V) : Val(V) {
>> + assert(isScopeRef(V) && "DIScopeRef should be a MDString or MDNode");
>> }
>>
>> /// Given a DITypeIdentifierMap, tries to find the corresponding
>> -/// DIType for a DITypeRef.
>> -DIType DITypeRef::resolve(const DITypeIdentifierMap &Map) const {
>> - if (!TypeVal)
>> - return NULL;
>> -
>> - if (const MDNode *MD = dyn_cast<MDNode>(TypeVal)) {
>> - assert(DIType(MD).isType() &&
>> - "MDNode in DITypeRef should be a DIType.");
>> - return MD;
>> - }
>> +/// DIScope for a DIScopeRef.
>> +DIScope DIScopeRef::resolve(const DITypeIdentifierMap &Map) const {
>> + if (!Val)
>> + return DIScope();
>>
>
> Why this rather than the original "return NULL;"?
>
>
>> +
>> + if (const MDNode *MD = dyn_cast<MDNode>(Val))
>> + return DIScope(MD);
>>
>
> Why this rather than the original "return MD;"?
>
>
>>
>> - const MDString *MS = cast<MDString>(TypeVal);
>> + const MDString *MS = cast<MDString>(Val);
>> // Find the corresponding MDNode.
>> DITypeIdentifierMap::const_iterator Iter = Map.find(MS);
>> assert(Iter != Map.end() && "Identifier not in the type map?");
>> assert(DIType(Iter->second).isType() &&
>> "MDNode in DITypeIdentifierMap should be a DIType.");
>> - return Iter->second;
>> + return DIScope(Iter->second);
>>
>
> Why did this explicit ctor invocation get added?
>
> I'm guessing because DIScope's DIScope(MDNode*) ctor is explicit? Unlike
> all the other DI* types MDNode* ctors? Perhaps we could change it to be
> consistent (assuming there is such consistency).
>
True, unfortunately, I didn't find any consistency ;) Most of the
constructors are explicit, except DIType.
Thanks,
Manman
>
>
>
>> }
>>
>> -/// Specialize getFieldAs to handle fields that are references to
>> DITypes.
>> +/// Specialize getFieldAs to handle fields that are references to
>> DIScopes.
>> template <>
>> -DITypeRef DIDescriptor::getFieldAs<DITypeRef>(unsigned Elt) const {
>> - return DITypeRef(getField(DbgNode, Elt));
>> +DIScopeRef DIDescriptor::getFieldAs<DIScopeRef>(unsigned Elt) const {
>> + return DIScopeRef(getField(DbgNode, Elt));
>> }
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130909/8f8b6e2a/attachment.html>
More information about the llvm-commits
mailing list