[PATCH] D44093: [BUILTINS] structure pretty printer

[Francis Visoiu Mistrih via Phabricator via cfe-commits]

thegameg added a comment.

Thanks for working on this! Few remarks in the comments.



================
Comment at: lib/CodeGen/CGBuiltin.cpp:934
 
+static Value *dumpRecord(CodeGenFunction &CGF, QualType RType,
+                         Value*& RecordPtr, CharUnits Align,
----------------
`llvm::Value` for consistency?


================
Comment at: lib/CodeGen/CGBuiltin.cpp:966
+    Types[Context.getPointerType(Context.CharTy)] = "%s";
+    }
+
----------------
Indentation failed here.


================
Comment at: lib/CodeGen/CGBuiltin.cpp:976
+      FieldPtr = CGF.Builder.CreateAdd(FieldPtr, ConstantInt::get(CGF.IntPtrTy, Off));
+      FieldPtr = CGF.Builder.CreateIntToPtr(FieldPtr, CGF.VoidPtrTy);
+    }
----------------
I think you should use `getelementptr` instead of `ptrtoint` -> `inttoptr` https://llvm.org/docs/GetElementPtr.html


================
Comment at: lib/CodeGen/CGBuiltin.cpp:997
+    if (Types.find(CanonicalType) == Types.end())
+        Format = Types[Context.VoidPtrTy];
+    else
----------------
Indentation failed here too.


================
Comment at: lib/CodeGen/CGBuiltin.cpp:1003
+    llvm::Type *ResType = CGF.ConvertType(ResPtrType);
+    FieldPtr = CGF.Builder.CreatePointerCast(FieldPtr, ResType);
+    Address FieldAddress = Address(FieldPtr, Align);
----------------
If you use GEP you should be able to get rid of this cast here.


================
Comment at: lib/CodeGen/CGBuiltin.cpp:1009
+
+    GString = CGF.Builder.CreateGlobalStringPtr(Format + "\n");
+    TmpRes = CGF.Builder.CreateCall(Func, {GString, FieldPtr});
----------------
You can probably use `llvm::Twine` for the concatenation of `Format`: http://llvm.org/doxygen/classllvm_1_1Twine.html.


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


Repository:
  rC Clang

https://reviews.llvm.org/D44093