r340080 - Update comments in CGDebugInfo to reflect changes in the MS mangler, NFC

[Reid Kleckner via cfe-commits]

Author: rnk
Date: Fri Aug 17 13:59:52 2018
New Revision: 340080

URL: http://llvm.org/viewvc/llvm-project?rev=340080&view=rev
Log:
Update comments in CGDebugInfo to reflect changes in the MS mangler, NFC

I've tried to elaborate on the purpose of these type identifiers and why
and when clang uses them.

Modified:
    cfe/trunk/lib/CodeGen/CGDebugInfo.cpp

Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=340080&r1=340079&r2=340080&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Fri Aug 17 13:59:52 2018
@@ -829,31 +829,45 @@ static bool hasCXXMangling(const TagDecl
   }
 }
 
-// Determines if the tag declaration will require a type identifier.
+// Determines if the debug info for this tag declaration needs a type
+// identifier. The purpose of the unique identifier is to deduplicate type
+// information for identical types across TUs. Because of the C++ one definition
+// rule (ODR), it is valid to assume that the type is defined the same way in
+// every TU and its debug info is equivalent.
+//
+// C does not have the ODR, and it is common for codebases to contain multiple
+// different definitions of a struct with the same name in different TUs.
+// Therefore, if the type doesn't have a C++ mangling, don't give it an
+// identifer. Type information in C is smaller and simpler than C++ type
+// information, so the increase in debug info size is negligible.
+//
+// If the type is not externally visible, it should be unique to the current TU,
+// and should not need an identifier to participate in type deduplication.
+// However, when emitting CodeView, the format internally uses these
+// unique type name identifers for references between debug info. For example,
+// the method of a class in an anonymous namespace uses the identifer to refer
+// to its parent class. The Microsoft C++ ABI attempts to provide unique names
+// for such types, so when emitting CodeView, always use identifiers for C++
+// types. This may create problems when attempting to emit CodeView when the MS
+// C++ ABI is not in use.
 static bool needsTypeIdentifier(const TagDecl *TD, CodeGenModule &CGM,
                                 llvm::DICompileUnit *TheCU) {
   // We only add a type identifier for types with C++ name mangling.
   if (!hasCXXMangling(TD, TheCU))
     return false;
 
-  // CodeView types with C++ mangling need a type identifier.
-  if (CGM.getCodeGenOpts().EmitCodeView)
-    return true;
-
   // Externally visible types with C++ mangling need a type identifier.
   if (TD->isExternallyVisible())
     return true;
 
+  // CodeView types with C++ mangling need a type identifier.
+  if (CGM.getCodeGenOpts().EmitCodeView)
+    return true;
+
   return false;
 }
 
-// When emitting CodeView debug information we need to produce a type
-// identifier for all types which have a C++ mangling.  Until a GUID is added
-// to the identifier (not currently implemented) the result will not be unique
-// across compilation units.
-// When emitting DWARF debug information, we need to produce a type identifier
-// for all externally visible types with C++ name mangling. This identifier
-// should be unique across ODR-compliant compilation units.
+// Returns a unique type identifier string if one exists, or an empty string.
 static SmallString<256> getTypeIdentifier(const TagType *Ty, CodeGenModule &CGM,
                                           llvm::DICompileUnit *TheCU) {
   SmallString<256> Identifier;