tejohnson added a comment. In D152741#4419366 <https://reviews.llvm.org/D152741#4419366>, @modimo wrote:
> In D152741#4419324 <https://reviews.llvm.org/D152741#4419324>, @tejohnson > wrote: > >> In D152741#4419265 <https://reviews.llvm.org/D152741#4419265>, @modimo wrote: >> >>> In D152741#4418831 <https://reviews.llvm.org/D152741#4418831>, @tejohnson >>> wrote: >>> >>>> I think I understand the motivation, but not sure I agree this is the >>>> right approach - can you simply not pass -flto-unit and >>>> -fwhole-program-vtables for these files? >>> >>> For our third-party libraries, they're pre-built into native files by GCC >>> so that's unfortunately not an option. >> >> I'm confused - how would you pass this new option then? I was assuming you >> were passing this option to some LLVM built files at the interface of those >> libraries. In which case not passing -flto-unit and >> -fwhole-program-visibility should have a similar effect (suppress the type >> metadata). > > Oh I see, I misunderstood. Yes this is being passed to LLVM built files. We > want to avoid manual allowlists/blocklists because code changes make it less > flexible and scalable than an automatic option. It seems like you need allowlists or blocklists in either case - either it is passed as a regex via the option proposed here, or the build system modifies the options for that set of files. > This can also be pretty tricky to do correctly since we can get type metadata > from multiple TUs and all of them would need to be opted out for WPD to not > kick in. But clang is presumably compiling a single TU at a time, so your regex needs to cover them all anyway? I'm not sure I understand the distinction between doing something like -fskip-vtable-filepaths=third-party/.* vs something like applying -funknown-vtable-visibility to each third-party/*.cc compile. I really think the logic for which files to apply this option to belongs in the build system, not in the clang driver - just like any other clang option. It isn't clear to me why this particular option should be applied based on a file regex. I like the approach of communicating this via vcall visibility, but just think that it should be applied to the current TU whenever a TBD new option is provided. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152741/new/ https://reviews.llvm.org/D152741 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
