[PATCH] D58321: Support for relative vtables
John McCall via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Wed May 29 21:39:19 PDT 2019
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")
> 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.
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
More information about the cfe-commits