[PATCH] D44093: [BUILTINS] structure pretty printer

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: lib/CodeGen/CGBuiltin.cpp:935
+static llvm::Value *dumpRecord(CodeGenFunction &CGF, QualType RType,
+                         Value*& RecordPtr, CharUnits Align,
+                         Value *Func, int Lvl)
----------------
Formatting looks off here and elsewhere. You should run the patch through clang-format. https://clang.llvm.org/docs/ClangFormat.html#script-for-patch-reformatting


================
Comment at: lib/CodeGen/CGBuiltin.cpp:938
+{
+  const RecordType *RT = RType->getAs<RecordType>();
+  ASTContext& Context = CGF.getContext();
----------------
Can use `const auto *` rather than spelling the type twice.


================
Comment at: lib/Sema/SemaChecking.cpp:1128
+
+    const RecordType *RT = PtrArgType->getPointeeType()->getAs<RecordType>();
+    if (!RT) {
----------------
`const auto *`


================
Comment at: lib/Sema/SemaChecking.cpp:1129
+    const RecordType *RT = PtrArgType->getPointeeType()->getAs<RecordType>();
+    if (!RT) {
+      this->Diag(PtrArg->getLocStart(), diag::err_typecheck_convert_incompatible)
----------------
I'd hoist this test to avoid duplicated code.
```
if (!PtrArgType->isPointerType() ||
    !PtrArgType->getPointeeType()->isRecordType())
```


================
Comment at: lib/Sema/SemaChecking.cpp:1147-1148
+
+    const FunctionType *FuncType =
+      FnPtrArgType->getPointeeType()->getAs<FunctionType>();
+
----------------
`const auto *`


================
Comment at: lib/Sema/SemaChecking.cpp:1158
+
+    if (const FunctionProtoType *FT = dyn_cast<FunctionProtoType>(FuncType)) {
+      if (!FT->isVariadic() ||
----------------
`const auto *`


================
Comment at: lib/Sema/SemaChecking.cpp:1159-1160
+    if (const FunctionProtoType *FT = dyn_cast<FunctionProtoType>(FuncType)) {
+      if (!FT->isVariadic() ||
+          FT->getReturnType() != Context.IntTy) {
+      this->Diag(FnPtrArg->getLocStart(), diag::err_typecheck_convert_incompatible)
----------------
You should also check that the first argument is `const char *` and add test cases for when it's not.


Repository:
  rC Clang

https://reviews.llvm.org/D44093