Finally, the problem is solved.
I’ve found that EmitObjCBoxedExpr uses EmitAnyExpr to handle SubExpr (which
might be treated as a method call' argument), while EmitObjCMessageExpr uses
EmitCallArgs.
To solve the problem I just unified a bit EmitObjCBoxedExpr, so now it also
uses EmitCallArgs.
Here is the part of the full diff, which illustrates unification of
EmitObjCBoxedExpr and EmitObjCMessageExpr:
diff --git a/include/clang/AST/ExprObjC.h b/include/clang/AST/ExprObjC.h
index 817c0cc..82738da 100644
--- a/include/clang/AST/ExprObjC.h
+++ b/include/clang/AST/ExprObjC.h
@@ -124,6 +124,15 @@ class ObjCBoxedExpr : public Expr {
// Iterators
child_range children() { return child_range(&SubExpr, &SubExpr+1); }
+
+ typedef ConstExprIterator const_arg_iterator;
+
+ const_arg_iterator arg_begin() const {
+ return reinterpret_cast<Stmt const * const*>(&SubExpr);
+ }
+ const_arg_iterator arg_end() const {
+ return reinterpret_cast<Stmt const * const*>(&SubExpr + 1);
+ }
friend class ASTStmtReader;
};
diff --git a/lib/CodeGen/CGObjC.cpp b/lib/CodeGen/CGObjC.cpp
index ca67c4b..d2975b8 100644
--- a/lib/CodeGen/CGObjC.cpp
+++ b/lib/CodeGen/CGObjC.cpp
@@ -55,12 +55,12 @@ llvm::Value *CodeGenFunction::EmitObjCStringLiteral(const
ObjCStringLiteral *E)
/// EmitObjCBoxedExpr - This routine generates code to call
/// the appropriate expression boxing method. This will either be
-/// one of +[NSNumber numberWith<Type>:], or +[NSString stringWithUTF8String:].
+/// one of +[NSNumber numberWith<Type>:], or +[NSString stringWithUTF8String:],
+/// or [NSValue valueWith<Type>:].
///
llvm::Value *
CodeGenFunction::EmitObjCBoxedExpr(const ObjCBoxedExpr *E) {
// Generate the correct selector for this literal's concrete type.
- const Expr *SubExpr = E->getSubExpr();
// Get the method.
const ObjCMethodDecl *BoxingMethod = E->getBoxingMethod();
assert(BoxingMethod && "BoxingMethod is null");
@@ -73,12 +73,9 @@ CodeGenFunction::EmitObjCBoxedExpr(const ObjCBoxedExpr *E) {
CGObjCRuntime &Runtime = CGM.getObjCRuntime();
const ObjCInterfaceDecl *ClassDecl = BoxingMethod->getClassInterface();
llvm::Value *Receiver = Runtime.GetClass(*this, ClassDecl);
-
- const ParmVarDecl *argDecl = *BoxingMethod->param_begin();
- QualType ArgQT = argDecl->getType().getUnqualifiedType();
- RValue RV = EmitAnyExpr(SubExpr);
+
CallArgList Args;
- Args.add(RV, ArgQT);
+ EmitCallArgs(Args, BoxingMethod, E->arg_begin(), E->arg_end());
RValue result = Runtime.GenerateMessageSend(
*this, ReturnValueSlot(), BoxingMethod->getReturnType(), Sel, Receiver,
The following steps have to be done before final submission:
- add tests for non-arc environment. I’m going to look for a better way to
handle this, but any suggestions on best practices are welcome.
- add full support for NSValue:
- valueWithEdgeInsets:(NSEdgeInsets), availability: OS X 10.10 and
higher, iOS 8 and higher.
- valueWithNonretainedObject:(id), availability: OS X, iOS
- valueWithPointer:(const void *), availability: OS X, iOS
- valueWithCATansform3D:(CATansform3D), availability: OSX, iOS
- valueWithCGAffineTransform:(CGAffineTransform), availability: iOS
- valueWithUIOffset:(UIOffset), availability: iOS 5.0 and higher
- valueWithUIEdgeInsets:(UIEdgeInsets), availability: iOS
- valueWithCGVector:(CGVector), availability: iOS
If I missed something or there any objections, please, let me know.
--
AlexDenisov
Software Engineer, https://github.com/AlexDenisov
On 22 Nov 2014 at 20:56:26, AlexDenisov ([email protected]) wrote:
Here is another patch, with tests for CodeGen, it covers the same data types:
NS/CG Rect, Size, Point and NSRange.
I’ve compiled test file with calls like ‘NSValue valueWith<Type>:' into LLVM
IR, then added checks into the test file, they all passed.
After that I replaced direct NSValue calls with boxed expressions: @(cg_point),
@(ns_rect), etc.
All the tests passes, excerpt of two which use NS/CGRect.
For the both cases CGM generates %ns_rect (%cg_rect respectively) variable and
fills it with some data.
In case of direct call (valueWithRect) it just passes the variable to
objc_msgSend, but in case of boxed expression there are some extra steps.
CGM creates additional variable (%agg-temp), copies %ns_rect’s value into it
and passes the new var into objc_msgSend.
Actually, generated program has correct behaviour, but it has extra variable,
which means that something is wrong.
Currently, I have no idea what is wrong and how to fix the issue, but,
definitely, I’m going to dive deeper into the CodeGenModule.
I would really appreciate any suggestions or advice where to start. Also, any
feedback on newly added CG tests are welcome.
In attachment you can find the patch and diff, which explains my problem in a
better way.
Best regards.
--
AlexDenisov
Software Engineer, https://github.com/AlexDenisov
On 21 Nov 2014 at 18:10:46, AlexDenisov ([email protected]) wrote:
Great, so let’s talk later, with another patch.
Thank you for help!
--
AlexDenisov
Software Engineer, https://github.com/AlexDenisov
On 21 Nov 2014 at 17:28:18, jahanian ([email protected]) wrote:
On Nov 21, 2014, at 4:45 AM, AlexDenisov <[email protected]> wrote:
- It is important to have a complete IRGen tests for all the supported boxed
items.
Could you, please, point me to the right direction? I’ve found only few tests
for literals under CodeGenObjC: boxing.m and objc-literal-tests.m. Should I put
new one there as well or just create another file?
The best one to look at is arc-literals.m. And it is good to produce the test
under -fobjc-arc and without.
Generally, you need to compile your test with -emit-llvm -o - and save the
output and then put the CHECKS
around the code you care enough (and make the register references generic,
etc.).
It will take several iterations to get it right.
Since you need many tests of a variety of APIs it is good to have a brand new
test.
- CGPoint, CGSize, CGRect are iOS APIs only. But I haven’t been able to find
their APIs in
the later version of iPhone SDKs. If they have been removed recently, then we
should make sure to check
the deployment target and issue proper diagnostics if need be. This is
generally the case for all the APIs.
If we are going to use auto boxing, then we need to make sure the APIs
available in earlier (or later) versions
of the SDKs.
CG-structs are part of the CoreGraphics.framework. On OS X NSValue' factory
methods is a part of Foundation.framework (NSGeometry.h,
NSValueGeometryExtensions), while on iOS they’re part of UIKit (UIGeometry.h,
NSValueUIGeometryExtensions). I guess they’re not going to disappear. Bu, I
totally agree - deployment target should be checked for methods/structs, which
is unavailable at some target version (e.g. NSEdgeInsets, UIOffset).
Great thanks.
- Have you though of supporting other item types?
NSEdgeInsets (CGEdgeInsets for iOS?) macosx 10.10, iPhone 8.0
non-retaining Objective-C objects.
Any C pointers
I wasn’t sure if this feature will be accepted (and still don't), so decided to
sent patch only for these structs. But, yes, in my opinion support for the
whole ‘NSValueBoxable’ data types should be implemented.
OK. While you are focusing on the structs these can wait.
Also, I've realized later, there should be support for modernizer to convert
‘old’ syntax to the new one.
Definitely, we should do that too. (I can help with that).
I will add more tests, and, probably add other data types support, if there are
no objections.
I would wait on the other data type support until we have gone through the
review process as it might impact implementation.
Thank you for feedback.
Thank you so much for working on this.
- Fariborz
--
AlexDenisov
Software Engineer, https://github.com/AlexDenisov
On 20 Nov 2014 at 20:14:39, jahanian ([email protected]) wrote:
While, the review process is ongoing, I looked at the patch. It is generally
very good and closely follows the convention for
doing auto boxing of other types. I have few general comments to make.
- It is important to have a complete IRGen tests for all the supported boxed
items.
- CGPoint, CGSize, CGRect are iOS APIs only. But I haven’t been able to find
their APIs in
the later version of iPhone SDKs. If they have been removed recently, then we
should make sure to check
the deployment target and issue proper diagnostics if need be. This is
generally the case for all the APIs.
If we are going to use auto boxing, then we need to make sure the APIs
available in earlier (or later) versions
of the SDKs.
- Have you though of supporting other item types?
NSEdgeInsets (CGEdgeInsets for iOS?) macosx 10.10, iPhone 8.0
non-retaining Objective-C objects.
Any C pointers
One code comment: getNSValueFactoryMethod has two default arguments but do not
have non-default value.
- Fariborz
On Nov 16, 2014, at 10:15 AM, AlexDenisov <[email protected]> wrote:
Patch extends boxing expressions to support NSValue.
Some C structures might be boxed into a NSValue, e.g.: NSPoint, CGPoint,
NSRect, etc.
This patch extends boxing expressions to accept the structures, that could be
used to construct NSValue object:
NSPoint p;
NSValue *point = @(p);
CGRect r;
NSValue rect = @(r);
Full list of supported structures:
NSPoint, NSSize, NSRect, CGPoint, CGSize, CGRect, NSRange.
--
AlexDenisov
Software Engineer, https://github.com/AlexDenisov
<nsvalue_literals.diff>_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
