aaron.ballman added inline comments.
================
Comment at: lib/CodeGen/CGBuiltin.cpp:1208
+
+ assert(RT && "The first argument must be a record type");
+
----------------
paulsemel wrote:
> aaron.ballman wrote:
> > I don't see anything enforcing this constraint, so users are likely to hit
> > this assertion rather than a compile error.
> I actually didn't manage to enforce this in the builtin declaration as we can
> only declare simple types (I am probably missing something)
I think this builtin will require custom type checking (so its signature needs
a `t`), and then you can add your code to `Sema::CheckBuiltinFunctionCall()` to
do the actual semantic checking.
================
Comment at: lib/CodeGen/CGBuiltin.cpp:1258
+
+ //Need to handle bitfield here
+
----------------
paulsemel wrote:
> aaron.ballman wrote:
> > Are you intending to implement this as part of this functionality?
> Yes, my goal is to be able to dump the bitfields correctly, particularly if
> the structure is packed (for dumping a GDT for example).
> I just didn't manage to do it properly for the moment.
Okay, this should probably be a FIXME comment if you don't intend to handle it
in the initial patch. It should also have a test case with some comments about
the test behavior.
================
Comment at: lib/CodeGen/CGBuiltin.cpp:1206
+ QualType Arg0Type = Arg0->getType()->getPointeeType();
+ const RecordType *RT = Arg0Type->getAs<RecordType>();
+
----------------
You can use `const auto *` here because the type is spelled out in the
initializer.
================
Comment at: lib/CodeGen/CGBuiltin.cpp:1211
+ RecordDecl *RD = RT->getDecl()->getDefinition();
+ ASTContext &ASTContext = RD->getASTContext();
+ const ASTRecordLayout &RL = ASTContext.getASTRecordLayout(RD);
----------------
Please don't name the variable the same name as a type.
================
Comment at: lib/CodeGen/CGBuiltin.cpp:1231
+ Types[getContext().VoidPtrTy] = "%p";
+ Types[getContext().FloatTy] = "%f";
+ Types[getContext().DoubleTy] = "%f";
----------------
It's unfortunate that you cannot distinguish between `float` and `double`. Do
you need to use the format specifiers exactly?
What about other type information, like qualifiers or array extents? How should
this handle types that do not exist in this mapping, like structure or enum
types?
================
Comment at: lib/CodeGen/CGBuiltin.cpp:1238
+ /* field : RecordDecl::field_iterator */
+ for (auto *FD : RD->fields()) {
+ uint64_t Off = RL.getFieldOffset(FD->getFieldIndex());
----------------
`const auto *`
Repository:
rC Clang
https://reviews.llvm.org/D44093
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
