[PATCH] D96816: [ObjC] Encode pointers to C++ classes as "^v" if the encoded string would otherwise include template specialization types

[David Chisnall via Phabricator via cfe-commits]

theraven added inline comments.


================
Comment at: clang/include/clang/Driver/Options.td:2113
+def fno_objc_encode_cxx_class_template_spec :
+  Flag<["-"], "fno-objc-encode-cxx-class-template-spec">, Group<f_Group>;
 defm objc_convert_messages_to_runtime_calls : BoolFOption<"objc-convert-messages-to-runtime-calls",
----------------
I think there's some magic in Options.td for defining the thing and its no- variant together.  I don't think you want the f prefix in the name either if it's an f-option.


================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5784
+                   options::OPT_fno_objc_encode_cxx_class_template_spec,
+                   !Triple.isOSDarwin()))
+    CmdArgs.push_back("-fobjc-encode-cxx-class-template-spec");
----------------
I don't think this should be guarded on Darwin, it should be guarded on whether the [ObjCRuntime is a NeXT-family runtime](https://github.com/llvm/llvm-project/blob/62ec4ac90738a5f2d209ed28c822223e58aaaeb7/clang/include/clang/Basic/ObjCRuntime.h#L134).  It might be fine to enable this for all non-GNUstep runtimes (the GCC ABI has typed selectors, but doesn't use the type information for dispatch and doesn't have non-fragile ivars.  I believe the ObjFW runtime is the same).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96816/new/

https://reviews.llvm.org/D96816