apply more review suggestions

Author: zyx-billy

mlir/include/mlir/Dialect/LLVMIR/LLVMInterfaces.td

@@ -352,12 +352,6 @@ def LLVM_DIRecursiveTypeAttrInterface
     Note that a rec-self without an outer rec-decl with the same recId is
     conceptually the same as an "unbound" variable. The context needs to provide
     meaning to the rec-self.
-
-    This can be avoided by calling the `getUnfoldedBaseType()` method on a
-    rec-decl, which returns the same type with all matching rec-self instances
-    replaced with this rec-decl again. This is useful, for example, for fetching
-    a field out of a recursive struct and maintaining the legality of the field
-    type.
   }];
   let cppNamespace = "::mlir::LLVM";
   let methods = [

mlir/lib/Dialect/LLVMIR/IR/LLVMAttrs.cpp

@@ -69,8 +69,8 @@ bool DINodeAttr::classof(Attribute attr) {
 //===----------------------------------------------------------------------===//
 
 bool DIScopeAttr::classof(Attribute attr) {
-  return llvm::isa<DICompileUnitAttr, DIFileAttr, DILocalScopeAttr,
-                   DIModuleAttr, DINamespaceAttr, DITypeAttr>(attr);
+  return llvm::isa<DICompileUnitAttr, DICompositeTypeAttr, DIFileAttr,
+                   DILocalScopeAttr, DIModuleAttr, DINamespaceAttr>(attr);
 }
 
 //===----------------------------------------------------------------------===//

mlir/lib/Target/LLVMIR/DebugImporter.cpp

@@ -318,10 +318,10 @@ DINodeAttr DebugImporter::translate(llvm::DINode *node) {
   if (DINodeAttr attr = translateNode(node)) {
     // If this node was marked as recursive, set its recId.
     if (auto recType = dyn_cast<DIRecursiveTypeAttrInterface>(attr)) {
-      if (DistinctAttr id = translationStack.lookup(node)) {
-        attr = cast<DINodeAttr>(recType.withRecId(id));
+      if (DistinctAttr recId = translationStack.lookup(node)) {
+        attr = cast<DINodeAttr>(recType.withRecId(recId));
         // Remove the unbound recursive DistinctAttr ID.
-        unboundRecursiveSelfRefs.back().erase(id);
+        unboundRecursiveSelfRefs.back().erase(recId);
       }
     }
 
@@ -394,8 +394,7 @@ DistinctAttr DebugImporter::getOrCreateDistinctID(llvm::DINode *node) {
 
 function_ref<DIRecursiveTypeAttrInterface(DistinctAttr)>
 DebugImporter::getRecSelfConstructor(llvm::DINode *node) {
-  using CtorType =
-      llvm::function_ref<DIRecursiveTypeAttrInterface(DistinctAttr)>;
+  using CtorType = function_ref<DIRecursiveTypeAttrInterface(DistinctAttr)>;
   return TypeSwitch<llvm::DINode *, CtorType>(node)
       .Case<llvm::DICompositeType>([](auto *concreteNode) {
         return CtorType(decltype(translateImpl(concreteNode))::getRecSelf);

mlir/lib/Target/LLVMIR/DebugImporter.h

@@ -88,7 +88,7 @@ class DebugImporter {
 
   /// Get the `getRecSelf` constructor for the translated type of `node` if its
   /// translated DITypeAttr supports recursion. Otherwise, returns nullptr.
-  llvm::function_ref<DIRecursiveTypeAttrInterface(DistinctAttr)>
+  function_ref<DIRecursiveTypeAttrInterface(DistinctAttr)>
   getRecSelfConstructor(llvm::DINode *node);
 
   /// A mapping between LLVM debug metadata and the corresponding attribute.

mlir/lib/Target/LLVMIR/DebugTranslation.cpp

@@ -145,7 +145,7 @@ DebugTranslation::translateImplGetPlaceholder(DICompositeTypeAttr attr) {
 void DebugTranslation::translateImplFillPlaceholder(
     DICompositeTypeAttr attr, llvm::DICompositeType *placeholder) {
   SmallVector<llvm::Metadata *> elements;
-  for (auto member : attr.getElements())
+  for (DINodeAttr member : attr.getElements())
     elements.push_back(translate(member));
   placeholder->replaceElements(llvm::MDNode::get(llvmCtx, elements));
 }