myhsu added inline comments.

================
Comment at: clang/lib/Basic/Targets/M68k.cpp:77-79
+  Builder.defineMacro("M68k");
+  Builder.defineMacro("__M68k__");
+  Builder.defineMacro("__M68K__");
----------------
jrtc27 wrote:
> myhsu wrote:
> > jrtc27 wrote:
> > > Where are these coming from? GCC only defines `__m68k__`.
> > I think GCC does: 
> > https://github.com/gcc-mirror/gcc/blob/b90d051ecbc1d8972ae1bf0cd7588fcc66df0722/gcc/config/m68k/m68k.h#L64
> > 
> > Also I found this page: https://sourceforge.net/p/predef/wiki/Architectures
> Yes, and as you can see none of those three are defined by GCC. Defining 
> `M68k` is particularly bad as it will collide with the C++ namespace used by 
> this very backend so you won't be able to build Clang with Clang on m68k! And 
> the others are just a waste of time. Defining fewer macros is best, that way 
> people don't come to rely on non-standard ones and instead use the single 
> option that exists everywhere (`__m68k__`).
> it will collide with the C++ namespace used by this very backend so you won't 
> be able to build Clang with Clang on m68k

good point :-)

> Defining fewer macros is best, that way people don't come to rely on 
> non-standard ones and instead use the single option that exists everywhere

fair enough


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

https://reviews.llvm.org/D88393

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

Reply via email to