HazardyKnusperkeks added inline comments.
================ Comment at: clang/lib/Format/UnwrappedLineParser.cpp:1217 + assert(PPBranchLevel >= -1); + if (PPBranchLevel <= -1) + conditionalCompilationStart(/*Unreachable=*/true); ---------------- You assert >= -1, that means this has to be == -1. ================ Comment at: clang/unittests/Format/FormatTest.cpp:5196-5206 + std::function<void(std::string, unsigned)> FormatBadBranches = + [&](std::string Prefix, unsigned Lines) { + const std::string Directives[] = {"", "#if X\n", "#else X\n", + "#endif\n"}; + if (Lines == 0) + verifyNoCrash(Prefix); + else ---------------- sstwcw wrote: > owenpan wrote: > > Can we have individual `verifyFormat` or `verifyNoCrash`tests for easy > > reading and to avoid the overhead? Some examples: > > ``` > > #else > > a; > > #if X > > b; > > #endif > > #endif > > > > #elif X > > a; > > #endif > > #ifdef X > > b; > > #endif > > ``` > I added some tests like the examples. But I kept the generated cases. I > feel more secure that way. At first I only found the bug because of the > generated cases. As for overhead, the debug build of this test took 1.6s on > my laptop. > I added some tests like the examples. But I kept the generated cases. I > feel more secure that way. At first I only found the bug because of the > generated cases. As for overhead, the debug build of this test took 1.6s on > my laptop. Couldn't you just take the previously crashing ones, you find important, as test cases? The overhead is not only compile and runtime, it's also while trying to understand what's happening here. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135972/new/ https://reviews.llvm.org/D135972 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits