[clang] 4a0267e - Convert a reachable llvm_unreachable into an assert.

[Aaron Ballman via cfe-commits]

On Sat, Mar 21, 2020 at 11:31 PM David Blaikie <dblaikie at 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 at 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 at lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits