[PATCH] D58321: Support for relative vtables

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.


================
Comment at: clang/include/clang/Basic/LangOptions.def:329
+        "Whether to use clang's relative C++ ABI "
+        "for classes with vtables")
+
----------------
leonardchan wrote:
> rjmccall wrote:
> > Yeah, see, this plays into the question above.  I would not want to provide this as a language option for general use.  The attribute seems good enough for testing, and if you want a -cc1 option to apply the attribute by default for experimentation during Fuchsia bring-up that's fair, but I don't want something that suggests to users that it's okay to pass this attribute and change the system default.
> Ok. So is this the reason for the white list approach mentioned in D17893? As an alternative, would you also be ok with creating a `FuchsiaCXXABI` that subclasses `ItaniumCXXABI`, similar to the ARM and WebAssembly ones? This way it doesn't change the system default.
That design for relative v-tables was intended (at least as it was described to me) as an optimization for certain classes in a project that still needed to interoperate with system and other non-project C++ code.  It was initially attempting to apply the optimization eagerly to all classes that didn't match a blacklist of classes (and namespaces of classes) that needed to follow the system ABI, which I objected to, which is why I asked for it to be redesigned around applying the optimization only to whitelisted classes/namespaces.  None of that applies if you're changing the system ABI rule unless you decide you want to allow classes to opt into standard Itanium v-tables.

If introducing a `FuchsiaCXXABI` class makes the code easier, go for it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58321