ebevhan added inline comments.
================
Comment at: include/clang/AST/ASTContext.h:2882
+
+ QualType getCorrespondingSaturatedType(const QualType &Ty) const;
};
----------------
This probably belongs up near the other predicates.
Also I think it's more common to simply pass `QualType` instead of a `const
QualType&`.
================
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);
----------------
leonardchan wrote:
> 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.
That sounds reasonable. I have no strong opinions on it and I don't think we
use the functionality anyway.
================
Comment at: lib/Sema/SemaType.cpp:1612
+ // Only fixed point types can be saturated
+ if (DS.isTypeSpecSat()) {
+ if (!IsFixedPointType) {
----------------
The braces aren't needed.
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
