[PATCH] D85157: [Sema] Add casting check for fixed to fixed point conversions

[Bjorn Pettersson via Phabricator via cfe-commits]

bjope added inline comments.


================
Comment at: clang/test/Sema/warn-bad-function-cast.c:49
+#ifdef FIXED_POINT
+  (void)(_Fract) if1(); // no warning
+#endif
----------------
bjope wrote:
> vabridgers wrote:
> > bjope wrote:
> > > bjope wrote:
> > > > bjope wrote:
> > > > > This should be added before the line saying `/* All following casts issue warning */`.
> > > > Is the `(void)` needed/relevant here?
> > > As questioned earlier, shouldn't we expect a warning for this scenario?
> > > 
> > > There is however a problem that we get the warning for _Fract to _Fract conversion. And it would be nice with a more complete set of tests involving both FixedPoint->FixedPoint, FixedPoint->Integer and Integer->FixedPoint casts.
> > If you have any *specific* suggestions for test cases, I'm open to that.
> Add:
> 
> ```
> _Fract ff1(void);
> ```
> 
> And inside foo add these three tests (you'll need to add appropriate expects):
> ```
> (_Fract)ff1();  // No warning expected.
> (_Fract)if1();  // Warning expected.
> (int)ff1();  // Warning expected.
> ```
I still think it would be nice not to break the structure of this test. Tests seem to be divided into three categories:

/* Casts to void types are always OK.  */

  /* Casts to the same type or similar types are OK.  */

/* All following casts issue warning */

And you have currently inserted all new tests in the last section.



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85157/new/

https://reviews.llvm.org/D85157