[llvm] r304814 - UnitTests: Do not use assert() for error checking

[Matthias Braun via llvm-commits]

Changed in r305519

> On Jun 12, 2017, at 11:02 AM, David Blaikie <dblaikie at gmail.com> wrote:
> 
> Not sure I understand - what's the motivation for that? What difference in usability/behavior comes from that?
> 
> Seems to me that gunit ASSERT is a good tool (compared to report_fatal_error) for checking setup invariants - it provides more precise error messages, and allows tests to continue running (well, actually, I'm not sure what happens when a test crashes like that - I guess gunit probably still manages to recover and run more tests), etc.
> 
> On Mon, Jun 12, 2017 at 10:36 AM Matthias Braun <matze at braunis.de <mailto:matze at braunis.de>> wrote:
> This is not about the thing being tested here, it's just sanity checking that we can parse the inputs before starting the actual test. I think it is better to only use the gunit ASSERTs for the partsthat the test actually wants to test.
> 
> - Matthias
> 
> On Jun 12, 2017, at 10:06 AM, David Blaikie <dblaikie at gmail.com <mailto:dblaikie at gmail.com>> wrote:
> 
>> Shouldn't this use the gunit ASSERT macros instead? probably.
>> 
>> On Tue, Jun 6, 2017 at 12:01 PM Matthias Braun via llvm-commits <llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>> wrote:
>> Author: matze
>> Date: Tue Jun  6 14:00:54 2017
>> New Revision: 304814
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=304814&view=rev <http://llvm.org/viewvc/llvm-project?rev=304814&view=rev>
>> Log:
>> UnitTests: Do not use assert() for error checking
>> 
>> Use `if (!X) report_fatal_error()` instead of `assert()` for the ad-hoc
>> error handling in two unittests. This reduces unnecessary differences
>> between release and debug builds (motivated by unused variable warnings
>> triggered in release builds).
>> 
>> Modified:
>>     llvm/trunk/unittests/MI/LiveIntervalTest.cpp
>>     llvm/trunk/unittests/Target/AArch64/InstSizes.cpp
>> 
>> Modified: llvm/trunk/unittests/MI/LiveIntervalTest.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/MI/LiveIntervalTest.cpp?rev=304814&r1=304813&r2=304814&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/MI/LiveIntervalTest.cpp?rev=304814&r1=304813&r2=304814&view=diff>
>> ==============================================================================
>> --- llvm/trunk/unittests/MI/LiveIntervalTest.cpp (original)
>> +++ llvm/trunk/unittests/MI/LiveIntervalTest.cpp Tue Jun  6 14:00:54 2017
>> @@ -151,7 +151,8 @@ body: |
>>    std::unique_ptr<MIRParser> MIR;
>>    std::unique_ptr<Module> M = parseMIR(Context, PM, MIR, *TM, MIRString,
>>                                         "func");
>> -  assert(M && "MIR parsing successfull");
>> +  if (!M)
>> +    report_fatal_error("Could not parse MIR code\n");
>> 
>>    PM.add(new TestPass(T));
>> 
>> 
>> Modified: llvm/trunk/unittests/Target/AArch64/InstSizes.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Target/AArch64/InstSizes.cpp?rev=304814&r1=304813&r2=304814&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Target/AArch64/InstSizes.cpp?rev=304814&r1=304813&r2=304814&view=diff>
>> ==============================================================================
>> --- llvm/trunk/unittests/Target/AArch64/InstSizes.cpp (original)
>> +++ llvm/trunk/unittests/Target/AArch64/InstSizes.cpp Tue Jun  6 14:00:54 2017
>> @@ -21,7 +21,8 @@ std::unique_ptr<TargetMachine> createTar
>> 
>>    std::string Error;
>>    const Target *TheTarget = TargetRegistry::lookupTarget(TT, Error);
>> -  assert(TheTarget && "Target not registered");
>> +  if (!TheTarget)
>> +    report_fatal_error("Target not registered");
>> 
>>    return std::unique_ptr<TargetMachine>(
>>        TheTarget->createTargetMachine(TT, CPU, FS, TargetOptions(), None,
>> @@ -58,21 +59,24 @@ void runChecks(
>>    std::unique_ptr<MemoryBuffer> MBuffer = MemoryBuffer::getMemBuffer(MIRString);
>>    std::unique_ptr<MIRParser> MParser =
>>        createMIRParser(std::move(MBuffer), Context);
>> -  assert(MParser && "Couldn't create MIR parser");
>> +  if (!MParser)
>> +    report_fatal_error("Couldn't create MIR parser");
>> 
>>    std::unique_ptr<Module> M = MParser->parseIRModule();
>> -  assert(M && "Couldn't parse module");
>> +  if (!M)
>> +    report_fatal_error("Couldn't parse module");
>> 
>>    M->setTargetTriple(TM->getTargetTriple().getTriple());
>>    M->setDataLayout(TM->createDataLayout());
>> 
>>    MachineModuleInfo MMI(TM);
>>    bool Res = MParser->parseMachineFunctions(*M, MMI);
>> -  (void)Res;
>> -  assert(!Res && "Couldn't parse MIR functions");
>> +  if (Res)
>> +    report_fatal_error("Couldn't parse MIR functions");
>> 
>>    auto F = M->getFunction("sizes");
>> -  assert(F && "Couldn't find intended function");
>> +  if (!F)
>> +    report_fatal_error("Couldn't find intended function");
>>    auto &MF = MMI.getOrCreateMachineFunction(*F);
>> 
>>    Checks(*II, MF);
>> 
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits <http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170615/5bb56da5/attachment.html>