On Thu, May 6, 2021 at 10:09 PM David Gow <davidgow@xxxxxxxxxx> wrote: > > The use of typecheck() in KUNIT_EXPECT_EQ() and friends is causing more > problems than I think it's worth. Things like enums need to have their > values explicitly cast, and literals all need to be very precisely typed > for the code to compile. nit: I have not had the typecheck() call prevent any code from compiling, just generating warnings. I guess you can have a build set to cause any warning to be promoted to an error; still, I think this statement is misleading. > While typechecking does have its uses, the additional overhead of having > lots of needless casts -- combined with the awkward error messages which > don't mention which types are involved -- makes tests less readable and > more difficult to write. > > By removing the typecheck() call, the two arguments still need to be of > compatible types, but don't need to be of exactly the same time, which > seems a less confusing and more useful compromise. > > Signed-off-by: David Gow <davidgow@xxxxxxxxxx> Looks good to me. Reviewed-by: Brendan Higgins <brendanhiggins@xxxxxxxxxx> > --- > > I appreciate that this is probably a bit controversial (and, indeed, I > was a bit hesitant about sending it out myself), but after sitting on it > for a few days, I still think this is probably an improvement overall. > > The second patch does fix what I think is an actual bug, though, so even > if this isn't determined to be a good idea, it (or some equivalent) > should probably go through. I don't remember being a huge fan of the typecheck when it was asked for either. I think I am a little bit more indifferent than you; nevertheless, I support this change.
