================
Comment at: lib/Basic/Targets.cpp:5769
@@ +5768,3 @@
+        .Case("mips32r3", NanLegacy | Nan2008)
+        .Case("mips32r5", NanLegacy | Nan2008)
+        .Case("mips32r6", Nan2008)
----------------
dsanders wrote:
> petarj wrote:
> > It might be me only, but seeing a function that returns enum but can 
> > actually return non-enumerated value seems wrong (I am not saying it is 
> > illegal though).
> > I see this is already done for ArchDefineTypes within PPCTargetInfo, but it 
> > still looks wrong (to me).
> > Would not this trigger a runtime error in clang if it was compiled with 
> > "-fsanitize=enum"?
> I think this is the main remaining comment. I agree in principle that enum 
> types should have the value of one of the enum values. Like Petar, I don't 
> think it's technically wrong though. Have you tried the -fsanitize=enum 
> option that was suggested?
> 
> Elsewhere, an integer type is used instead but given that there's only two 
> values and I would be very surprised if we ever add more, we should probably 
> just define NanBoth = 3 if it's a problem.
I tried -fsanitize=enum there wasn't runtime error. Should i define NanBoth?

================
Comment at: test/Preprocessor/init.c:4419
@@ -4418,3 +4418,1 @@
 //
-// RUN: %clang_cc1 -target-feature +nan2008 \
-// RUN:   -E -dM -triple=mips-none-none < /dev/null \
----------------
atanasyan wrote:
> By the way, why do you decide to remove that test at all? Does it fail now?
It doesn't fail now, but is wrong to pass +nan2008 with the default cpu 
(mips32r2), because mips32r2 architecture is on the legacy-only list.

http://reviews.llvm.org/D8170

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/



_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to