[PATCH] D26343: [RFC] Refactor TargetParserTests

[Renato Golin via Phabricator via llvm-commits]

rengolin accepted this revision.
rengolin added a comment.
This revision is now accepted and ready to land.

Ok, I think this looks **much** better than what we had, and it's a more meaningful testing strategy.

The repetitiveness will help other people adding tests whenever they add a new core/extension, too.

With the small change I proposed in the inline comment, LGTM.

Thanks!
--renato



================
Comment at: unittests/Support/TargetParserTest.cpp:40
+  unsigned FPUKind = ARM::getDefaultFPU(CPUName, ArchKind);
+  ret &= ARM::getFPUName(FPUKind).equals(ExpectedFPU);
+
----------------
jojo wrote:
> rengolin wrote:
> > Shouldn't this be `ret |=`?
> It will return "true" here, only when all the items are the values that we expect.
> So it should return a value after "&=".
Right, my mind was in the "true if it fails" which is pervasive throughout the back-end, mainly because you initialised it as "false". :)

Since the first ret will override, I think you can declare `ret` at the same time:

    bool ret = ARM::getArchName(ArchKind).equals(ExpectedArch);
    ...

Maybe even call it `pass`, so it's clearer what it means.


https://reviews.llvm.org/D26343