<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Jul 8, 2016 at 3:10 PM, Adrian McCarthy <span dir="ltr"><<a href="mailto:amccarth@google.com" target="_blank">amccarth@google.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">This wasn't actually a revert. </div></blockquote><div><br></div><div>*nod* Just meant in general - git hashes shouldn't be included, SVN revisions should be (I usually include both "recommit rXXXXX originally reverted in rYYYYY" to make it easy to keep track of the path when doing histrionics).</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"> It was another attempt to land the patch, with a correction to the new test to account for changes made by others between the LGTM and the dcommit.<div><br></div><div>Good to know about those scripts.  Is anybody working on Windows-compatible versions of them? :-)</div></div></blockquote><div><br></div><div>Not that I'm aware of - though I've not looked at them/considered what that would take.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5"><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Jul 8, 2016 at 3:01 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><div class="gmail_quote"><span>On Wed, Jul 6, 2016 at 12:49 PM, Adrian McCarthy via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: amccarth<br>
Date: Wed Jul  6 14:49:51 2016<br>
New Revision: 274661<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=274661&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=274661&view=rev</a><br>
Log:<br>
Retry: "Emit CodeView type records for nested classes."<br>
<br>
Now with a corrected test to account for a recently supported properties bit in the debug info of a struct.<br>
<br>
Original review: <a href="http://reviews.llvm.org/D21939" rel="noreferrer" target="_blank">http://reviews.llvm.org/D21939</a><br>
<br>
This reverts commit 970c3fd497a28d25dd69526eb52594a696c37968.<br></blockquote><div><br></div></span><div>Best to include SVN revisions in the commit log so it's easy for everyone to follow.<br><br>LLVM has a utility for this: utils/git-svn/git-svnrevert that works just like git revert, but produces a commit message that has the SVN revision number instead of the git hash.</div><div><div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Added:<br>
    llvm/trunk/test/DebugInfo/COFF/types-nested-class.ll<br>
Modified:<br>
    llvm/trunk/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp<br>
    llvm/trunk/lib/CodeGen/AsmPrinter/CodeViewDebug.h<br>
<br>
Modified: llvm/trunk/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp?rev=274661&r1=274660&r2=274661&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp?rev=274661&r1=274660&r2=274661&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp (original)<br>
+++ llvm/trunk/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp Wed Jul  6 14:49:51 2016<br>
@@ -1414,6 +1414,8 @@ struct llvm::ClassInfo {<br>
   MemberList Members;<br>
   // Direct overloaded methods gathered by name.<br>
   MethodsMap Methods;<br>
+<br>
+  std::vector<const DICompositeType *> NestedClasses;<br>
 };<br>
<br>
 void CodeViewDebug::clear() {<br>
@@ -1466,8 +1468,8 @@ ClassInfo CodeViewDebug::collectClassInf<br>
         // friends in the past, but modern versions do not.<br>
       }<br>
       // FIXME: Get Clang to emit function virtual table here and handle it.<br>
-      // FIXME: Get clang to emit nested types here and do something with<br>
-      // them.<br>
+    } else if (auto *Composite = dyn_cast<DICompositeType>(Element)) {<br>
+      Info.NestedClasses.push_back(Composite);<br>
     }<br>
     // Skip other unrecognized kinds of elements.<br>
   }<br>
@@ -1496,7 +1498,12 @@ TypeIndex CodeViewDebug::lowerCompleteTy<br>
   TypeIndex FieldTI;<br>
   TypeIndex VShapeTI;<br>
   unsigned FieldCount;<br>
-  std::tie(FieldTI, VShapeTI, FieldCount) = lowerRecordFieldList(Ty);<br>
+  bool ContainsNestedClass;<br>
+  std::tie(FieldTI, VShapeTI, FieldCount, ContainsNestedClass) =<br>
+      lowerRecordFieldList(Ty);<br>
+<br>
+  if (ContainsNestedClass)<br>
+    CO |= ClassOptions::ContainsNestedClass;<br>
<br>
   std::string FullName = getFullyQualifiedName(Ty);<br>
<br>
@@ -1532,7 +1539,13 @@ TypeIndex CodeViewDebug::lowerCompleteTy<br>
   ClassOptions CO = getCommonClassOptions(Ty);<br>
   TypeIndex FieldTI;<br>
   unsigned FieldCount;<br>
-  std::tie(FieldTI, std::ignore, FieldCount) = lowerRecordFieldList(Ty);<br>
+  bool ContainsNestedClass;<br>
+  std::tie(FieldTI, std::ignore, FieldCount, ContainsNestedClass) =<br>
+      lowerRecordFieldList(Ty);<br>
+<br>
+  if (ContainsNestedClass)<br>
+    CO |= ClassOptions::ContainsNestedClass;<br>
+<br>
   uint64_t SizeInBytes = Ty->getSizeInBits() / 8;<br>
   std::string FullName = getFullyQualifiedName(Ty);<br>
<br>
@@ -1550,7 +1563,7 @@ TypeIndex CodeViewDebug::lowerCompleteTy<br>
   return UnionTI;<br>
 }<br>
<br>
-std::tuple<TypeIndex, TypeIndex, unsigned><br>
+std::tuple<TypeIndex, TypeIndex, unsigned, bool><br>
 CodeViewDebug::lowerRecordFieldList(const DICompositeType *Ty) {<br>
   // Manually count members. MSVC appears to count everything that generates a<br>
   // field list record. Each individual overload in a method overload group<br>
@@ -1645,8 +1658,17 @@ CodeViewDebug::lowerRecordFieldList(cons<br>
           OverloadedMethodRecord(Methods.size(), MethodList, Name));<br>
     }<br>
   }<br>
+<br>
+  // Create nested classes.<br>
+  for (const DICompositeType *Nested : Info.NestedClasses) {<br>
+    NestedTypeRecord R(getTypeIndex(DITypeRef(Nested)), Nested->getName());<br>
+    Fields.writeNestedType(R);<br>
+    MemberCount++;<br>
+  }<br>
+<br>
   TypeIndex FieldTI = TypeTable.writeFieldList(Fields);<br>
-  return std::make_tuple(FieldTI, TypeIndex(), MemberCount);<br>
+  return std::make_tuple(FieldTI, TypeIndex(), MemberCount,<br>
+                         !Info.NestedClasses.empty());<br>
 }<br>
<br>
 TypeIndex CodeViewDebug::getVBPTypeIndex() {<br>
<br>
Modified: llvm/trunk/lib/CodeGen/AsmPrinter/CodeViewDebug.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/CodeViewDebug.h?rev=274661&r1=274660&r2=274661&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/CodeViewDebug.h?rev=274661&r1=274660&r2=274661&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/CodeGen/AsmPrinter/CodeViewDebug.h (original)<br>
+++ llvm/trunk/lib/CodeGen/AsmPrinter/CodeViewDebug.h Wed Jul  6 14:49:51 2016<br>
@@ -275,7 +275,7 @@ class LLVM_LIBRARY_VISIBILITY CodeViewDe<br>
   /// Common record member lowering functionality for record types, which are<br>
   /// structs, classes, and unions. Returns the field list index and the member<br>
   /// count.<br>
-  std::tuple<codeview::TypeIndex, codeview::TypeIndex, unsigned><br>
+  std::tuple<codeview::TypeIndex, codeview::TypeIndex, unsigned, bool><br>
   lowerRecordFieldList(const DICompositeType *Ty);<br>
<br>
   /// Inserts {{Node, ClassTy}, TI} into TypeIndices and checks for duplicates.<br>
<br>
Added: llvm/trunk/test/DebugInfo/COFF/types-nested-class.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/DebugInfo/COFF/types-nested-class.ll?rev=274661&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/DebugInfo/COFF/types-nested-class.ll?rev=274661&view=auto</a><br>
==============================================================================<br>
--- llvm/trunk/test/DebugInfo/COFF/types-nested-class.ll (added)<br>
+++ llvm/trunk/test/DebugInfo/COFF/types-nested-class.ll Wed Jul  6 14:49:51 2016<br>
@@ -0,0 +1,98 @@<br>
+; RUN: llc < %s -filetype=obj | llvm-readobj - -codeview | FileCheck %s<br>
+<br>
+; C++ source to regenerate:<br>
+; $ cat hello.cpp<br>
+; struct A {<br>
+;   struct Nested {};<br>
+; } a;<br>
+; $ clang hello.cpp -S -emit-llvm -g -gcodeview -o t.ll<br>
+<br>
+; CHECK: CodeViewTypes [<br>
+; CHECK:   Section: .debug$T (5)<br>
+; CHECK:   Magic: 0x4<br>
+; CHECK:   Struct (0x1000) {<br>
+; CHECK:     TypeLeafKind: LF_STRUCTURE (0x1505)<br>
+; CHECK:     MemberCount: 0<br>
+; CHECK:     Properties [ (0x280)<br>
+; CHECK:       ForwardReference (0x80)<br>
+; CHECK:       HasUniqueName (0x200)<br>
+; CHECK:     ]<br>
+; CHECK:     FieldList: 0x0<br>
+; CHECK:     DerivedFrom: 0x0<br>
+; CHECK:     VShape: 0x0<br>
+; CHECK:     SizeOf: 0<br>
+; CHECK:     Name: A<br>
+; CHECK:     LinkageName: .?AUA@@<br>
+; CHECK:   }<br>
+; CHECK:   Struct (0x1001) {<br>
+; CHECK:     TypeLeafKind: LF_STRUCTURE (0x1505)<br>
+; CHECK:     MemberCount: 0<br>
+; CHECK:     Properties [ (0x288)<br>
+; CHECK:       ForwardReference (0x80)<br>
+; CHECK:       HasUniqueName (0x200)<br>
+; CHECK:     ]<br>
+; CHECK:     FieldList: 0x0<br>
+; CHECK:     DerivedFrom: 0x0<br>
+; CHECK:     VShape: 0x0<br>
+; CHECK:     SizeOf: 0<br>
+; CHECK:     Name: A::Nested<br>
+; CHECK:     LinkageName: .?AUNested@A@@<br>
+; CHECK:   }<br>
+; CHECK:   FieldList (0x1002) {<br>
+; CHECK:     TypeLeafKind: LF_FIELDLIST (0x1203)<br>
+; CHECK:     NestedType {<br>
+; CHECK:       Type: A::Nested (0x1001)<br>
+; CHECK:       Name: Nested<br>
+; CHECK:     }<br>
+; CHECK:   }<br>
+; CHECK:   Struct (0x1003) {<br>
+; CHECK:     TypeLeafKind: LF_STRUCTURE (0x1505)<br>
+; CHECK:     MemberCount: 1<br>
+; CHECK:     Properties [ (0x210)<br>
+; CHECK:       ContainsNestedClass (0x10)<br>
+; CHECK:       HasUniqueName (0x200)<br>
+; CHECK:     ]<br>
+; CHECK:     FieldList: <field list> (0x1002)<br>
+; CHECK:     DerivedFrom: 0x0<br>
+; CHECK:     VShape: 0x0<br>
+; CHECK:     SizeOf: 1<br>
+; CHECK:     Name: A<br>
+; CHECK:     LinkageName: .?AUA@@<br>
+; CHECK:   }<br>
+; CHECK:   StringId (0x1004) {<br>
+; CHECK:     TypeLeafKind: LF_STRING_ID (0x1605)<br>
+; CHECK:     Id: 0x0<br>
+; CHECK:     StringData: D:\src\hello\hello.cpp<br>
+; CHECK:   }<br>
+; CHECK:   UdtSourceLine (0x1005) {<br>
+; CHECK:     TypeLeafKind: LF_UDT_SRC_LINE (0x1606)<br>
+; CHECK:     UDT: A (0x1003)<br>
+; CHECK:     SourceFile: D:\src\hello\hello.cpp (0x1004)<br>
+; CHECK:     LineNumber: 1<br>
+; CHECK:   }<br>
+; CHECK: ]<br>
+<br>
+; ModuleID = 'hello.cpp'<br>
+source_filename = "hello.cpp"<br>
+target datalayout = "e-m:x-p:32:32-i64:64-f80:32-n8:16:32-a:0:32-S32"<br>
+target triple = "i686-pc-windows-msvc19.0.23918"<br>
+<br>
+%struct.A = type { i8 }<br>
+<br>
+@"\01?a@@3UA@@A" = global %struct.A zeroinitializer, align 1<br>
+<br>
+!<a href="http://llvm.dbg.cu" rel="noreferrer" target="_blank">llvm.dbg.cu</a> = !{!0}<br>
+!llvm.module.flags = !{!8, !9}<br>
+!llvm.ident = !{!10}<br>
+<br>
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !1, producer: "clang version 3.9.0 ", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, globals: !3)<br>
+!1 = !DIFile(filename: "hello.cpp", directory: "D:\5Csrc\5Chello")<br>
+!2 = !{}<br>
+!3 = !{!4}<br>
+!4 = distinct !DIGlobalVariable(name: "a", linkageName: "\01?a@@3UA@@A", scope: !0, file: !1, line: 3, type: !5, isLocal: false, isDefinition: true, variable: %struct.A* @"\01?a@@3UA@@A")<br>
+!5 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "A", file: !1, line: 1, size: 8, align: 8, elements: !6, identifier: ".?AUA@@")<br>
+!6 = !{!7}<br>
+!7 = !DICompositeType(tag: DW_TAG_structure_type, name: "Nested", scope: !5, file: !1, line: 2, size: 8, align: 8, flags: DIFlagFwdDecl, identifier: ".?AUNested@A@@")<br>
+!8 = !{i32 2, !"CodeView", i32 1}<br>
+!9 = !{i32 2, !"Debug Info Version", i32 3}<br>
+!10 = !{!"clang version 3.9.0 "}<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div></div></div><br></div></div>
</blockquote></div><br></div>
</div></div></blockquote></div><br></div></div>