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