[Sema] Diagnose floating point conversions based on target semantics

...instead of just comparing rank. Also, fix a bad warning about
_Float16, since its declared out of order in BuiltinTypes.def,
meaning comparing rank using BuiltinType::getKind() is incorrect.

Differential revision: https://reviews.llvm.org/D58254

llvm-svn: 354190

4 files changed, 49 insertions, 11 deletions

diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h
index 0d5752abeeb..b8944a05227 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -2501,6 +2501,11 @@ public:
   /// \p LHS < \p RHS, return -1.
   int getFloatingTypeOrder(QualType LHS, QualType RHS) const;
 
+  /// Compare the rank of two floating point types as above, but compare equal
+  /// if both types have the same floating-point semantics on the target (i.e.
+  /// long double and double on AArch64 will return 0).
+  int getFloatingTypeSemanticOrder(QualType LHS, QualType RHS) const;
+
   /// Return a real floating point or a complex type (based on
   /// \p typeDomain/\p typeSize).
   ///
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 6af0cf15184..46cdd9a93f6 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -5608,6 +5608,12 @@ int ASTContext::getFloatingTypeOrder(QualType LHS, QualType RHS) const {
   return -1;
 }
 
+int ASTContext::getFloatingTypeSemanticOrder(QualType LHS, QualType RHS) const {
+  if (&getFloatTypeSemantics(LHS) == &getFloatTypeSemantics(RHS))
+    return 0;
+  return getFloatingTypeOrder(LHS, RHS);
+}
+
 /// getIntegerRank - Return an integer conversion rank (C99 6.3.1.1p1). This
 /// routine will assert if passed a built-in type that isn't an integer or enum,
 /// or if it is not canonicalized.
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 71882b571c4..5d9ae18267b 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -10626,14 +10626,16 @@ static void AnalyzeCompoundAssignment(Sema &S, BinaryOperator *E) {
 
   // If source is floating point but target is an integer.
   if (ResultBT->isInteger())
-    DiagnoseImpCast(S, E, E->getRHS()->getType(), E->getLHS()->getType(),
-                    E->getExprLoc(), diag::warn_impcast_float_integer);
-  // If both source and target are floating points. Builtin FP kinds are ordered
-  // by increasing FP rank. FIXME: except _Float16, we currently emit a bogus
-  // warning.
-  else if (ResultBT->isFloatingPoint() && ResultBT->getKind() < RBT->getKind() &&
-           // We don't want to warn for system macro.
-           !S.SourceMgr.isInSystemMacro(E->getOperatorLoc()))
+    return DiagnoseImpCast(S, E, E->getRHS()->getType(), E->getLHS()->getType(),
+                           E->getExprLoc(), diag::warn_impcast_float_integer);
+
+  if (!ResultBT->isFloatingPoint())
+    return;
+
+  // If both source and target are floating points, warn about losing precision.
+  int Order = S.getASTContext().getFloatingTypeSemanticOrder(
+      QualType(ResultBT, 0), QualType(RBT, 0));
+  if (Order < 0 && !S.SourceMgr.isInSystemMacro(E->getOperatorLoc()))
     // warn about dropping FP rank.
     DiagnoseImpCast(S, E->getRHS(), E->getLHS()->getType(), E->getOperatorLoc(),
                     diag::warn_impcast_float_result_precision);
@@ -10952,8 +10954,9 @@ CheckImplicitConversion(Sema &S, Expr *E, QualType T, SourceLocation CC,
     if (TargetBT && TargetBT->isFloatingPoint()) {
       // ...then warn if we're dropping FP rank.
 
-      // Builtin FP kinds are ordered by increasing FP rank.
-      if (SourceBT->getKind() > TargetBT->getKind()) {
+      int Order = S.getASTContext().getFloatingTypeSemanticOrder(
+          QualType(SourceBT, 0), QualType(TargetBT, 0));
+      if (Order > 0) {
         // Don't warn about float constants that are precisely
         // representable in the target type.
         Expr::EvalResult result;
@@ -10971,7 +10974,7 @@ CheckImplicitConversion(Sema &S, Expr *E, QualType T, SourceLocation CC,
         DiagnoseImpCast(S, E, T, CC, diag::warn_impcast_float_precision);
       }
       // ... or possibly if we're increasing rank, too
-      else if (TargetBT->getKind() > SourceBT->getKind()) {
+      else if (Order < 0) {
         if (S.SourceMgr.isInSystemMacro(CC))
           return;
 
diff --git a/clang/test/Sema/conversion-target-dep.c b/clang/test/Sema/conversion-target-dep.c
new file mode 100644
index 00000000000..e16685fa067
--- /dev/null
+++ b/clang/test/Sema/conversion-target-dep.c
@@ -0,0 +1,24 @@
+// RUN: %clang_cc1 -Wdouble-promotion -Wimplicit-float-conversion %s -triple x86_64-apple-macosx10.12 -verify=x86,expected
+// RUN: %clang_cc1 -Wdouble-promotion -Wimplicit-float-conversion %s -triple armv7-apple-ios9.0 -verify=arm,expected
+
+// On ARM, long double and double both map to double precision 754s, so there
+// isn't any reason to warn on conversions back and forth.
+
+long double ld;
+double d;
+_Float16 f16; // x86-error {{_Float16 is not supported on this target}}
+
+int main() {
+  ld = d; // x86-warning {{implicit conversion increases floating-point precision: 'double' to 'long double'}}
+  d = ld; // x86-warning {{implicit conversion loses floating-point precision: 'long double' to 'double'}}
+
+  ld += d; // x86-warning {{implicit conversion increases floating-point precision: 'double' to 'long double'}}
+  d += ld; // x86-warning {{implicit conversion when assigning computation result loses floating-point precision: 'long double' to 'double'}}
+
+  f16 = ld; // expected-warning {{implicit conversion loses floating-point precision: 'long double' to '_Float16'}}
+  ld = f16; // expected-warning {{implicit conversion increases floating-point precision: '_Float16' to 'long double'}}
+
+  f16 += ld; // expected-warning {{implicit conversion when assigning computation result loses floating-point precision: 'long double' to '_Float16'}}
+  ld += f16; // expected-warning {{implicit conversion increases floating-point precision: '_Float16' to 'long double'}}
+}
+