nikic accepted this revision. nikic added a comment. This revision is now accepted and ready to land.
LGTM In D140800#4044983 <https://reviews.llvm.org/D140800#4044983>, @serge-sans-paille wrote: > In D140800#4043723 <https://reviews.llvm.org/D140800#4043723>, @nikic wrote: > >> Just to check, this isn't going to cause some warning spew about all those >> OptTable implementations being non-final? > > nope. Why would there be? Something about missing virtual destructors on a non-final class? Not sure when exactly that warning applies. ================ Comment at: llvm/unittests/Option/OptionParsingTest.cpp:327 -TEST(DISABLED_Option, FindNearestFIXME) { - TestOptTable T; ---------------- serge-sans-paille wrote: > @nikic: the `DISABLED_` prefix seems to be a gtest convention, see > https://github.com/google/googletest/blob/main/docs/advanced.md#temporarily-disabling-tests Huh, TIL. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140800/new/ https://reviews.llvm.org/D140800 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
