rjmccall added a comment.
You should add a __has_feature check for this
(__has_feature(objc_arc_fields)?), as well as documentation. The documentation
would go in the ARC spec.
================
Comment at: include/clang/AST/Decl.h:3575
+ /// Functions to query basic properties of non-trivial C structs.
+ bool nonTrivialToDefaultInitialize() const {
+ return NonTrivialToDefaultInitialize;
----------------
Naming style would be "isNonTrivialToDefualtInitialize" and so on for all of
these.
If these aren't guaranteed to be consistent with the C++ definitions (and the
copying one almost certainly isn't, since there isn't a single definition of
copying in C++), I think you probably need to name these in a way that makes it
clear that it's the C bit that's being queried/changed. Perhaps adding
"Primitive" to each one, e.g. isNonTrivialToPrimitiveCopy? Richard is probably
opinionated about this.
================
Comment at: lib/AST/ASTContext.cpp:5780
+ // Return true if Ty is a non-trivial C struct type that is non-trivial to
+ // destructly move or destroy.
+ if (Ty.hasNonTrivialToDestructiveMoveStruct() ||
----------------
"Return true" is just a description of the code that follows. I would suggest
something like "The block needs copy/destroy helpers if...".
================
Comment at: lib/CodeGen/CGBlocks.cpp:2244
+ CGM, byrefInfo, NonTrivialCStructByrefHelpers(valueAlignment, type));
+
// Otherwise, if we don't have a retainable type, there's nothing to do.
----------------
You've updated the code for __block captures, but I think you missed
computeBlockInfo for direct captures.
================
Comment at: lib/CodeGen/CGDecl.cpp:1421
+ destroyer = CodeGenFunction::destroyNonTrivialCStruct;
+ cleanupKind = getARCCleanupKind();
+ break;
----------------
This can only be getARCCleanupKind() if it's non-trivial to destroy solely
because of __strong members. Since the setup here is significantly more
general than ARC, I think you need to default to the more-correct behavior; if
you want to add a special-case check for only __strong members, you ought to do
that explicitly.
================
Comment at: lib/CodeGen/CGExprAgg.cpp:315
+ }
+ }
}
----------------
Do these functions have a memcpy as a precondition? I would expect them to do
the full copy (for code size if nothing else).
================
Comment at: lib/CodeGen/CMakeLists.txt:73
CodeGenAction.cpp
+ CodeGenCStruct.cpp
CodeGenFunction.cpp
----------------
I think CGNonTrivialStruct.cpp would be a better name.
https://reviews.llvm.org/D41228
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
