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

Reply via email to