rjmccall added inline comments.
================
Comment at: clang/include/clang/AST/DeclCXX.h:531
+ /// \brief Whether the class uses the relative C++ vtable ABI.
+ unsigned IsRelativeCXXABI : 1;
+
----------------
Should we proactively generalize this as a "CXXABIVariant" enum, which for now
can just be "Standard" and "RelativeVTables"?
Also, I don't want to pre-empt your secret plans, but if Fuchsia is just going
to use this as its system C++ ABI, maybe we should plan for that, too.
================
Comment at: clang/include/clang/AST/VTableBuilder.h:267
+ VTableComponent &getVTableComponent(size_t i) const {
+ return VTableComponents[i];
----------------
`const VTableComponent &`, I think.
================
Comment at: clang/include/clang/Basic/LangOptions.def:329
+ "Whether to use clang's relative C++ ABI "
+ "for classes with vtables")
+
----------------
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.
================
Comment at: clang/include/clang/Sema/Sema.h:10976
+ /// Determine if this class can use the relative vtable ABI.
+ void checkClassABI(CXXRecordDecl *RD);
+
----------------
Comment / method-name mismatch?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D58321/new/
https://reviews.llvm.org/D58321
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
