[PATCH] D48560: [clangd] JSON <-> XPC conversions

[Jan Korous via Phabricator via cfe-commits]

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