leonardchan added inline comments.
================
Comment at: lib/Sema/DeclSpec.cpp:1123
+ if (!(TypeSpecType == TST_accum || TypeSpecType == TST_fract)) {
+ S.Diag(TSSatLoc, diag::err_invalid_saturation_spec)
+ << getSpecifierName((TST)TypeSpecType, Policy);
----------------
ebevhan wrote:
> Handling this case here means that placing _Sat on something other than
> exactly a fixed-point type is a parsing error rather than a semantic error.
> How does this handle _Sat on sugared types? Should _Sat on things like
> typedefs work?
>
> typedef _Fract myfract;
> _Sat myfract F;
>
> The primary issue (and this is one that we have encountered as well) is that
> you cannot have a true _Sat typedef since _Sat only exists as part of builtin
> types. You need to desugar/canonicalize the type and then do
> getCorrespondingSaturatingType (or have getCorrespondingSaturatingType look
> at the canonical type internally).
I think _Sat is analogous to _Complex where it only works with specific builtin
types, albeit the only builtin type _Complex doesn't work with is _Bool.
Currently this example would throw the error `'_Sat' specifier is only valid on
'_Fract' or '_Accum', not 'type-name'` which is similar to what _Complex does
when paired with a typedef:
```
typedef double mydouble;
mydouble _Complex D; // _Complex type-name' is invalid
```
I don't see this as a big problem right now, but am willing to come back to
this in the future if it becomes more urgent. For now, I added a test that
asserts this error is thrown.
================
Comment at: lib/Sema/SemaType.cpp:1410
+
+ if (DS.getTypeSpecSign() != DeclSpec::TSS_unsigned)
+ Result = fixedpoint::getCorrespondingSignedType(Context, Result);
----------------
ebevhan wrote:
> The logic is a bit reversed. The default should be to select the signed
> variant, and if the TSS is unsigned, then convert it to the unsigned variant.
Using getCorrespondingUnsignedType(). Also changed
getCorrespondingUnsignedType() to accept fixed point types.
Repository:
rC Clang
https://reviews.llvm.org/D46911
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
