jkorous marked 12 inline comments as done.
jkorous added inline comments.
================
Comment at: xpc/XPCJSONConversions.cpp:62
+ if (objType == XPC_TYPE_UINT64)
+ return json::Expr(xpc_uint64_get_value(xpcObj));
+ if (objType == XPC_TYPE_STRING)
----------------
sammccall wrote:
> hmm, I think this shouldn't compile anymore, rather require you to explicitly
> do the narrowing conversion to int64 or double.
Explicitly narroving now. Thanks.
================
Comment at: xpc/XPCJSONConversions.cpp:69
+ xpcObj,
+ ^bool(size_t /* index */, xpc_object_t value) {
+ result.emplace_back(xpcToJson(value));
----------------
jkorous wrote:
> sammccall wrote:
> > this looks like objC syntax, I'm not familiar with it, but should this file
> > be `.mm`?
> >
> > Alternatively, it seems like you could write a C++ loop with
> > `xpc_array_get_value` (though I don't know if there are performance
> > concerns).
> >
> > (and dictionary)
> Oh, sorry. These are actually C blocks - a nonstandard C extension.
> https://en.wikipedia.org/wiki/Blocks_(C_language_extension)
> https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/Blocks/Articles/00_Introduction.html
>
> There was no performance concern on my side - just liked this approach
> better. Didn't realize how confusing it might be, will rewrite this.
Allright, by trying to get rid of C block in dictionary conversion I remembered
that there's unfortunately no sane reason how to iterate XPC dictionary without
using xpc_dictionary_apply() which uses C block for functor parameter.
https://developer.apple.com/documentation/xpc/1505404-xpc_dictionary_apply?language=objc
https://developer.apple.com/documentation/xpc/xpc_dictionary_applier_t?language=objc
Anyway, even had we succeeded in removing C blocks from conversion they still
would be necessary for dispatch as xpc_connection_set_event_handler() is
another part of XPC API that uses it.
I guess that there's no point in removing the xpc_array_apply() then. Is that
ok with you?
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D48560
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
