[llvm] r190322 - Debug Info: Rename DITypeRef to DIScopeRef.

David Blaikie dblaikie at gmail.com
Mon Sep 9 13:12:07 PDT 2013


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)?


>
> 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?


>    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).



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


More information about the llvm-commits mailing list