<div dir="ltr">Not sure I understand - what's the motivation for that? What difference in usability/behavior comes from that?<br><br>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.</div><br><div class="gmail_quote"><div dir="ltr">On Mon, Jun 12, 2017 at 10:36 AM Matthias Braun <<a href="mailto:matze@braunis.de">matze@braunis.de</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="auto"><div></div><div>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.</div></div><div dir="auto"><div><br></div><div>- Matthias</div></div><div dir="auto"><div><br>On Jun 12, 2017, at 10:06 AM, David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>> wrote:<br><br></div><blockquote type="cite"><div><div dir="ltr">Shouldn't this use the gunit ASSERT macros instead? probably.</div><br><div class="gmail_quote"><div dir="ltr">On Tue, Jun 6, 2017 at 12:01 PM Matthias Braun via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: matze<br>
Date: Tue Jun 6 14:00:54 2017<br>
New Revision: 304814<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=304814&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=304814&view=rev</a><br>
Log:<br>
UnitTests: Do not use assert() for error checking<br>
<br>
Use `if (!X) report_fatal_error()` instead of `assert()` for the ad-hoc<br>
error handling in two unittests. This reduces unnecessary differences<br>
between release and debug builds (motivated by unused variable warnings<br>
triggered in release builds).<br>
<br>
Modified:<br>
llvm/trunk/unittests/MI/LiveIntervalTest.cpp<br>
llvm/trunk/unittests/Target/AArch64/InstSizes.cpp<br>
<br>
Modified: llvm/trunk/unittests/MI/LiveIntervalTest.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/MI/LiveIntervalTest.cpp?rev=304814&r1=304813&r2=304814&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/MI/LiveIntervalTest.cpp?rev=304814&r1=304813&r2=304814&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/unittests/MI/LiveIntervalTest.cpp (original)<br>
+++ llvm/trunk/unittests/MI/LiveIntervalTest.cpp Tue Jun 6 14:00:54 2017<br>
@@ -151,7 +151,8 @@ body: |<br>
std::unique_ptr<MIRParser> MIR;<br>
std::unique_ptr<Module> M = parseMIR(Context, PM, MIR, *TM, MIRString,<br>
"func");<br>
- assert(M && "MIR parsing successfull");<br>
+ if (!M)<br>
+ report_fatal_error("Could not parse MIR code\n");<br>
<br>
PM.add(new TestPass(T));<br>
<br>
<br>
Modified: llvm/trunk/unittests/Target/AArch64/InstSizes.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Target/AArch64/InstSizes.cpp?rev=304814&r1=304813&r2=304814&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Target/AArch64/InstSizes.cpp?rev=304814&r1=304813&r2=304814&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/unittests/Target/AArch64/InstSizes.cpp (original)<br>
+++ llvm/trunk/unittests/Target/AArch64/InstSizes.cpp Tue Jun 6 14:00:54 2017<br>
@@ -21,7 +21,8 @@ std::unique_ptr<TargetMachine> createTar<br>
<br>
std::string Error;<br>
const Target *TheTarget = TargetRegistry::lookupTarget(TT, Error);<br>
- assert(TheTarget && "Target not registered");<br>
+ if (!TheTarget)<br>
+ report_fatal_error("Target not registered");<br>
<br>
return std::unique_ptr<TargetMachine>(<br>
TheTarget->createTargetMachine(TT, CPU, FS, TargetOptions(), None,<br>
@@ -58,21 +59,24 @@ void runChecks(<br>
std::unique_ptr<MemoryBuffer> MBuffer = MemoryBuffer::getMemBuffer(MIRString);<br>
std::unique_ptr<MIRParser> MParser =<br>
createMIRParser(std::move(MBuffer), Context);<br>
- assert(MParser && "Couldn't create MIR parser");<br>
+ if (!MParser)<br>
+ report_fatal_error("Couldn't create MIR parser");<br>
<br>
std::unique_ptr<Module> M = MParser->parseIRModule();<br>
- assert(M && "Couldn't parse module");<br>
+ if (!M)<br>
+ report_fatal_error("Couldn't parse module");<br>
<br>
M->setTargetTriple(TM->getTargetTriple().getTriple());<br>
M->setDataLayout(TM->createDataLayout());<br>
<br>
MachineModuleInfo MMI(TM);<br>
bool Res = MParser->parseMachineFunctions(*M, MMI);<br>
- (void)Res;<br>
- assert(!Res && "Couldn't parse MIR functions");<br>
+ if (Res)<br>
+ report_fatal_error("Couldn't parse MIR functions");<br>
<br>
auto F = M->getFunction("sizes");<br>
- assert(F && "Couldn't find intended function");<br>
+ if (!F)<br>
+ report_fatal_error("Couldn't find intended function");<br>
auto &MF = MMI.getOrCreateMachineFunction(*F);<br>
<br>
Checks(*II, MF);<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div>
</div></blockquote></div></blockquote></div>