On Sat, Mar 21, 2020 at 11:31 PM David Blaikie <dblai...@gmail.com> wrote: > > Why the change? this seems counter to LLVM's style which pretty consistently > uses unreachable rather than assert(false), so far as I know?
We're not super consistent (at least within Clang), but the rules as I've generally understood them are to use llvm_unreachable only for truly unreachable code and to use assert(false) when the code is technically reachable but is a programmer mistake to have gotten there. In this particular case, the code is very much reachable and I spent a lot more time debugging than I should have because I was using a release + asserts build and the semantics of llvm_unreachable made unfortunate codegen (switching to an assert makes the issue immediately obvious). > > On Tue, Mar 10, 2020 at 11:22 AM Aaron Ballman via cfe-commits > <cfe-commits@lists.llvm.org> wrote: >> >> >> Author: Aaron Ballman >> Date: 2020-03-10T14:22:21-04:00 >> New Revision: 4a0267e3ad8c4d47f267d7d960f127e099fb4818 >> >> URL: >> https://github.com/llvm/llvm-project/commit/4a0267e3ad8c4d47f267d7d960f127e099fb4818 >> DIFF: >> https://github.com/llvm/llvm-project/commit/4a0267e3ad8c4d47f267d7d960f127e099fb4818.diff >> >> LOG: Convert a reachable llvm_unreachable into an assert. >> >> Added: >> >> >> Modified: >> clang/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp >> >> Removed: >> >> >> >> ################################################################################ >> diff --git a/clang/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp >> b/clang/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp >> index 01ac2bc83bb6..99e16752b51a 100644 >> --- a/clang/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp >> +++ b/clang/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp >> @@ -134,9 +134,9 @@ StringRef >> AnalyzerOptions::getCheckerStringOption(StringRef CheckerName, >> CheckerName = CheckerName.substr(0, Pos); >> } while (!CheckerName.empty() && SearchInParents); >> >> - llvm_unreachable("Unknown checker option! Did you call getChecker*Option " >> - "with incorrect parameters? User input must've been " >> - "verified by CheckerRegistry."); >> + assert(false && "Unknown checker option! Did you call getChecker*Option " >> + "with incorrect parameters? User input must've been " >> + "verified by CheckerRegistry."); >> >> return ""; >> } >> >> >> >> _______________________________________________ >> cfe-commits mailing list >> cfe-commits@lists.llvm.org >> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
