ebevhan added inline comments.
================ Comment at: include/clang/Sema/DeclSpec.h:670 const PrintingPolicy &Policy); + bool SetTypeSpecSat(SourceLocation Loc, const char *&PrevSpec, + unsigned &DiagID); ---------------- leonardchan wrote: > ebevhan wrote: > > This should take a PrintingPolicy like the others. > I think the PrintingPolicy may not be necessary because it's only used for > getting the name of the current TypeSpecType. More specifically, for just > differentiating vetween "bool" and "_Bool" for `TST_bool`. It also seems that > other setters that don't touch TypeSpecType use PrintingPolicy, like > `SetTypeSpecComplex` or `SetTypeSpecSign` You are correct, the ones that don't need it don't take it. I'm just being selfish since I need it downstream to disambiguate `__sat` and `_Sat`. It's fine the way it is. ================ Comment at: lib/Sema/SemaType.cpp:1612 + // Only fixed point types can be saturated + if (DS.isTypeSpecSat() && !IsFixedPointType) + S.Diag(DS.getTypeSpecSatLoc(), diag::err_invalid_saturation_spec) ---------------- leonardchan wrote: > ebevhan wrote: > > Also, this does not seem to invalidate the declarator. > How so? I have tests under `test/Frontend/fixed_point_errors.c` that check > for an error thrown if `_Sat` is used with non-fixed point types. Hm, that is true. We don't have it downstream either. I'm not sure what the purpose of doing it is, but other invalid specifiers do `declarator.setInvalidType(true);` I guess it isn't needed, just curious as to why it's done for some other ones. 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