efriedma added inline comments.

================
Comment at: clang/lib/Basic/Targets/AVR.cpp:313
+  Builder.defineMacro("__UINT16_TYPE__", "unsigned int");
+  Builder.defineMacro("__INT16_TYPE__", "int");
 
----------------
benshi001 wrote:
> efriedma wrote:
> > Redefining `__INT16_TYPE__` like this is unusual.  The macro is normally 
> > defined in InitPreprocessor.cpp; if the logic there is wrong, I'd prefer to 
> > fix it there.
> > 
> > For specific types int16_t and uint16_t, I'm not sure if anything inside 
> > the compiler actually uses them at the moment, but I'd prefer to encode the 
> > type in the TargetInfo, along the lines of getInt64Type(), in case we need 
> > these types elsewhere in the future.
> Making changes to InitPreprocessor.cpp will affect all targets, and I not 
> familar with other targets. Actually AVR is so special that it defines  
> `__INT16_TYPE__` to `int`.
> Making changes to InitPreprocessor.cpp will affect all targets, and I not 
> familar with other targets.

That shouldn't be a problem as long as you have an appropriate reviewer; I 
volunteer. :)

You should be able to use getInt64Type() as a model for the necessary changes.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100701/new/

https://reviews.llvm.org/D100701

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to