[PATCH] D33735: [DebugInfo] Add ThisOrSelf attribute for emission of FlagObjectPointer.

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.


================
Comment at: include/clang/AST/Decl.h:1387
+    IPK_CapturedContext, /// Parameter for captured context
+    IPK_GeneralParam,    /// General implicit parameter
+  };
----------------
I would just call this "Other" and document it as being for kinds of implicit parameters that we haven't seen a purpose in categorizing yet.  (Or you could just classify them all, I suppose.)

We can use C++11 features in Clang now, so I would recommend hoisting this type out of ImplicitParamDecl and making it an enum class.  You can then drop the "IPK_" prefixes.


================
Comment at: include/clang/AST/Decl.h:1394
+  /// Whether this parameter represents implicit 'this'|'self' argument in
+  /// member functions.
+  ImplicitParamKind ParamKind;
----------------
Comment is out-of-date.

Also, please compress this into VarDecl's bit-field storage.


================
Comment at: include/clang/AST/Decl.h:1405
+  static ImplicitParamDecl *Create(ASTContext &C, QualType T,
+                                   DeclContext *DC = nullptr);
 
----------------
I see the point in having a convenience constructor like this, but it should probably still take an IPK.


================
Comment at: include/clang/AST/Decl.h:1426
     setImplicit();
   }
 
----------------
Same idea.  This should probably still take an IPK.


================
Comment at: include/clang/AST/Decl.h:1429
+  /// Returns true if the parameter represents implicit 'this' or 'self'
+  /// argument in member functions.
+  ImplicitParamKind getParameterKind() const { return ParamKind; }
----------------
This comment is out-of-date.


================
Comment at: lib/CodeGen/CGDebugInfo.cpp:3475
+        Flags |= llvm::DINode::FlagObjectPointer;
+    }
 
----------------
Looks like this whole block is still unnecessarily indented.


================
Comment at: lib/CodeGen/CGException.cpp:1654
+      IPD->setLocation(StartLoc);
+      IPD->setDeclName(&getContext().Idents.get("exception_pointers"));
+      Args.push_back(IPD);
----------------
These do not feel like good uses of your shorthand constructors.


https://reviews.llvm.org/D33735