[PATCH] D46915: [Fixed Point Arithmetic] Fixed Point Precision Bits and Fixed Point Literals

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In https://reviews.llvm.org/D46915#1162095, @leonardchan wrote:

> In https://reviews.llvm.org/D46915#1162038, @erichkeane wrote:
>
> > See this bug here: https://bugs.llvm.org/show_bug.cgi?id=38161
> >
> > This patch doesn't seem to properly consider integers, and ignores octals, which I suspect shouldn't be the case.
>
>
> I just requested an account on bugs.llvm.org and will respond here for now until I can comment on the thread there.
>
> So the syntax for fixed point literals should be the same as that for floating point constants according to clause 6.4.4.2a in the embedded C spec (http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1169.pdf). Integers and octal literals should not be supported since floating point literals can only be declared in decimal or hexadecimal notation.


Then it seems you've missed that part... These literals are allowing integers without issue at the moment.

> I can see how the error messages don't seem clear and portray this, and should eventually change it so if the user does provide an integer/octal literal an error along the lines of `"fixed point literals cannot be declared as integers"` should be thrown instead.

Seems like that could work.

> The assertion is also something that I will address since this should not be thrown.
> 
> To sidetrack a bit also, we are limiting fixed point types to just C for now (https://reviews.llvm.org/D46084) so using `auto` to complete the type is not supported. This is until we come up with an itanium mangling for fixed point types (https://github.com/itanium-cxx-abi/cxx-abi/issues/56).

At the moment you are NOT limiting to C.  You'll have to add quite a view checks at the moment in order to make that limitation.  That said `auto' was for convienience, it had nothign to do with the bug itself.


Repository:
  rC Clang

https://reviews.llvm.org/D46915