Track the location of the context requiring an implicit conversion and use it

to white-list conversions required by system headers.  rdar://problem/8232669

llvm-svn: 116029

Author: rjmccall

clang/include/clang/Sema/Sema.h

@@ -4472,7 +4472,7 @@ class Sema {
   void CheckReturnStackAddr(Expr *RetValExp, QualType lhsType,
                             SourceLocation ReturnLoc);
   void CheckFloatComparison(SourceLocation loc, Expr* lex, Expr* rex);
-  void CheckImplicitConversions(Expr *E);
+  void CheckImplicitConversions(Expr *E, SourceLocation CC = SourceLocation());
 
   /// \brief The parser's current scope.
   ///

clang/lib/Sema/SemaChecking.cpp

@@ -2434,7 +2434,7 @@ bool IsSameFloatAfterCast(const APValue &value,
           IsSameFloatAfterCast(value.getComplexFloatImag(), Src, Tgt));
 }
 
-void AnalyzeImplicitConversions(Sema &S, Expr *E);
+void AnalyzeImplicitConversions(Sema &S, Expr *E, SourceLocation CC);
 
 static bool IsZero(Sema &S, Expr *E) {
   // Suppress cases where we are comparing against an enum constant.
@@ -2487,8 +2487,8 @@ void CheckTrivialUnsignedComparison(Sema &S, BinaryOperator *E) {
 /// Analyze the operands of the given comparison.  Implements the
 /// fallback case from AnalyzeComparison.
 void AnalyzeImpConvsInComparison(Sema &S, BinaryOperator *E) {
-  AnalyzeImplicitConversions(S, E->getLHS());
-  AnalyzeImplicitConversions(S, E->getRHS());
+  AnalyzeImplicitConversions(S, E->getLHS(), E->getOperatorLoc());
+  AnalyzeImplicitConversions(S, E->getRHS(), E->getOperatorLoc());
 }
 
 /// \brief Implements -Wsign-compare.
@@ -2533,8 +2533,8 @@ void AnalyzeComparison(Sema &S, BinaryOperator *E) {
 
   // Go ahead and analyze implicit conversions in the operands.  Note
   // that we skip the implicit conversions on both sides.
-  AnalyzeImplicitConversions(S, lex);
-  AnalyzeImplicitConversions(S, rex);
+  AnalyzeImplicitConversions(S, lex, E->getOperatorLoc());
+  AnalyzeImplicitConversions(S, rex, E->getOperatorLoc());
 
   // If the signed range is non-negative, -Wsign-compare won't fire,
   // but we should still check for comparisons which are always true
@@ -2564,27 +2564,36 @@ void AnalyzeComparison(Sema &S, BinaryOperator *E) {
 }
 
 /// Diagnose an implicit cast;  purely a helper for CheckImplicitConversion.
-void DiagnoseImpCast(Sema &S, Expr *E, QualType T, unsigned diag) {
-  S.Diag(E->getExprLoc(), diag) << E->getType() << T << E->getSourceRange();
+void DiagnoseImpCast(Sema &S, Expr *E, QualType T, SourceLocation CContext,
+                     unsigned diag) {
+  S.Diag(E->getExprLoc(), diag)
+    << E->getType() << T << E->getSourceRange() << SourceRange(CContext);
 }
 
 void CheckImplicitConversion(Sema &S, Expr *E, QualType T,
-                             bool *ICContext = 0) {
+                             SourceLocation CC, bool *ICContext = 0) {
   if (E->isTypeDependent() || E->isValueDependent()) return;
 
   const Type *Source = S.Context.getCanonicalType(E->getType()).getTypePtr();
   const Type *Target = S.Context.getCanonicalType(T).getTypePtr();
   if (Source == Target) return;
   if (Target->isDependentType()) return;
 
+  // If the conversion context location is invalid or instantiated
+  // from a system macro, don't complain.
+  if (CC.isInvalid() ||
+      (CC.isMacroID() && S.Context.getSourceManager().isInSystemHeader(
+                           S.Context.getSourceManager().getSpellingLoc(CC))))
+    return;
+
   // Never diagnose implicit casts to bool.
   if (Target->isSpecificBuiltinType(BuiltinType::Bool))
     return;
 
   // Strip vector types.
   if (isa<VectorType>(Source)) {
     if (!isa<VectorType>(Target))
-      return DiagnoseImpCast(S, E, T, diag::warn_impcast_vector_scalar);
+      return DiagnoseImpCast(S, E, T, CC, diag::warn_impcast_vector_scalar);
 
     Source = cast<VectorType>(Source)->getElementType().getTypePtr();
     Target = cast<VectorType>(Target)->getElementType().getTypePtr();
@@ -2593,7 +2602,7 @@ void CheckImplicitConversion(Sema &S, Expr *E, QualType T,
   // Strip complex types.
   if (isa<ComplexType>(Source)) {
     if (!isa<ComplexType>(Target))
-      return DiagnoseImpCast(S, E, T, diag::warn_impcast_complex_scalar);
+      return DiagnoseImpCast(S, E, T, CC, diag::warn_impcast_complex_scalar);
 
     Source = cast<ComplexType>(Source)->getElementType().getTypePtr();
     Target = cast<ComplexType>(Target)->getElementType().getTypePtr();
@@ -2621,15 +2630,15 @@ void CheckImplicitConversion(Sema &S, Expr *E, QualType T,
             return;
         }
 
-        DiagnoseImpCast(S, E, T, diag::warn_impcast_float_precision);
+        DiagnoseImpCast(S, E, T, CC, diag::warn_impcast_float_precision);
       }
       return;
     }
 
     // If the target is integral, always warn.
     if ((TargetBT && TargetBT->isInteger()))
       // TODO: don't warn for integer values?
-      DiagnoseImpCast(S, E, T, diag::warn_impcast_float_integer);
+      DiagnoseImpCast(S, E, T, CC, diag::warn_impcast_float_integer);
 
     return;
   }
@@ -2644,8 +2653,8 @@ void CheckImplicitConversion(Sema &S, Expr *E, QualType T,
     // People want to build with -Wshorten-64-to-32 and not -Wconversion
     // and by god we'll let them.
     if (SourceRange.Width == 64 && TargetRange.Width == 32)
-      return DiagnoseImpCast(S, E, T, diag::warn_impcast_integer_64_32);
-    return DiagnoseImpCast(S, E, T, diag::warn_impcast_integer_precision);
+      return DiagnoseImpCast(S, E, T, CC, diag::warn_impcast_integer_64_32);
+    return DiagnoseImpCast(S, E, T, CC, diag::warn_impcast_integer_precision);
   }
 
   if ((TargetRange.NonNegative && !SourceRange.NonNegative) ||
@@ -2663,7 +2672,7 @@ void CheckImplicitConversion(Sema &S, Expr *E, QualType T,
       *ICContext = true;
     }
 
-    return DiagnoseImpCast(S, E, T, DiagID);
+    return DiagnoseImpCast(S, E, T, CC, DiagID);
   }
 
   return;
@@ -2672,24 +2681,26 @@ void CheckImplicitConversion(Sema &S, Expr *E, QualType T,
 void CheckConditionalOperator(Sema &S, ConditionalOperator *E, QualType T);
 
 void CheckConditionalOperand(Sema &S, Expr *E, QualType T,
-                             bool &ICContext) {
+                             SourceLocation CC, bool &ICContext) {
   E = E->IgnoreParenImpCasts();
 
   if (isa<ConditionalOperator>(E))
     return CheckConditionalOperator(S, cast<ConditionalOperator>(E), T);
 
-  AnalyzeImplicitConversions(S, E);
+  AnalyzeImplicitConversions(S, E, CC);
   if (E->getType() != T)
-    return CheckImplicitConversion(S, E, T, &ICContext);
+    return CheckImplicitConversion(S, E, T, CC, &ICContext);
   return;
 }
 
 void CheckConditionalOperator(Sema &S, ConditionalOperator *E, QualType T) {
-  AnalyzeImplicitConversions(S, E->getCond());
+  SourceLocation CC = E->getQuestionLoc();
+
+  AnalyzeImplicitConversions(S, E->getCond(), CC);
 
   bool Suspicious = false;
-  CheckConditionalOperand(S, E->getTrueExpr(), T, Suspicious);
-  CheckConditionalOperand(S, E->getFalseExpr(), T, Suspicious);
+  CheckConditionalOperand(S, E->getTrueExpr(), T, CC, Suspicious);
+  CheckConditionalOperand(S, E->getFalseExpr(), T, CC, Suspicious);
 
   // If -Wconversion would have warned about either of the candidates
   // for a signedness conversion to the context type...
@@ -2708,10 +2719,10 @@ void CheckConditionalOperator(Sema &S, ConditionalOperator *E, QualType T) {
   if (E->getType() != T) {
     Suspicious = false;
     CheckImplicitConversion(S, E->getTrueExpr()->IgnoreParenImpCasts(),
-                            E->getType(), &Suspicious);
+                            E->getType(), CC, &Suspicious);
     if (!Suspicious)
       CheckImplicitConversion(S, E->getFalseExpr()->IgnoreParenImpCasts(),
-                              E->getType(), &Suspicious);
+                              E->getType(), CC, &Suspicious);
     if (!Suspicious)
       return;
   }
@@ -2727,7 +2738,7 @@ void CheckConditionalOperator(Sema &S, ConditionalOperator *E, QualType T) {
 /// AnalyzeImplicitConversions - Find and report any interesting
 /// implicit conversions in the given expression.  There are a couple
 /// of competing diagnostics here, -Wconversion and -Wsign-compare.
-void AnalyzeImplicitConversions(Sema &S, Expr *OrigE) {
+void AnalyzeImplicitConversions(Sema &S, Expr *OrigE, SourceLocation CC) {
   QualType T = OrigE->getType();
   Expr *E = OrigE->IgnoreParenImpCasts();
 
@@ -2743,14 +2754,14 @@ void AnalyzeImplicitConversions(Sema &S, Expr *OrigE) {
   // The non-canonical typecheck is just an optimization;
   // CheckImplicitConversion will filter out dead implicit conversions.
   if (E->getType() != T)
-    CheckImplicitConversion(S, E, T);
+    CheckImplicitConversion(S, E, T, CC);
 
   // Now continue drilling into this expression.
 
   // Skip past explicit casts.
   if (isa<ExplicitCastExpr>(E)) {
     E = cast<ExplicitCastExpr>(E)->getSubExpr()->IgnoreParenImpCasts();
-    return AnalyzeImplicitConversions(S, E);
+    return AnalyzeImplicitConversions(S, E, CC);
   }
 
   // Do a somewhat different check with comparison operators.
@@ -2767,17 +2778,22 @@ void AnalyzeImplicitConversions(Sema &S, Expr *OrigE) {
   if (isa<SizeOfAlignOfExpr>(E)) return;
 
   // Now just recurse over the expression's children.
+  CC = E->getExprLoc();
   for (Stmt::child_iterator I = E->child_begin(), IE = E->child_end();
          I != IE; ++I)
-    AnalyzeImplicitConversions(S, cast<Expr>(*I));
+    AnalyzeImplicitConversions(S, cast<Expr>(*I), CC);
 }
 
 } // end anonymous namespace
 
 /// Diagnoses "dangerous" implicit conversions within the given
 /// expression (which is a full expression).  Implements -Wconversion
 /// and -Wsign-compare.
-void Sema::CheckImplicitConversions(Expr *E) {
+///
+/// \param CC the "context" location of the implicit conversion, i.e.
+///   the most location of the syntactic entity requiring the implicit
+///   conversion
+void Sema::CheckImplicitConversions(Expr *E, SourceLocation CC) {
   // Don't diagnose in unevaluated contexts.
   if (ExprEvalContexts.back().Context == Sema::Unevaluated)
     return;
@@ -2786,7 +2802,8 @@ void Sema::CheckImplicitConversions(Expr *E) {
   if (E->isTypeDependent() || E->isValueDependent())
     return;
 
-  AnalyzeImplicitConversions(*this, E);
+  // This is not the right CC for (e.g.) a variable initialization.
+  AnalyzeImplicitConversions(*this, E, CC);
 }
 
 /// CheckParmsForFunctionDef - Check that the parameters of the given

clang/lib/Sema/SemaDecl.cpp

@@ -4326,6 +4326,9 @@ void Sema::AddInitializerToDecl(Decl *RealDecl, Expr *Init, bool DirectInit) {
     Init->setType(DclT);
   }
 
+  // Check any implicit conversions within the expression.
+  CheckImplicitConversions(Init, VDecl->getLocation());
+
   Init = MaybeCreateCXXExprWithTemporaries(Init);
   // Attach the initializer to the decl.
   VDecl->setInit(Init);

clang/lib/Sema/SemaDeclCXX.cpp

@@ -136,6 +136,7 @@ Sema::SetParamDefaultArgument(ParmVarDecl *Param, Expr *Arg,
     return true;
   Arg = Result.takeAs<Expr>();
 
+  CheckImplicitConversions(Arg, EqualLoc);
   Arg = MaybeCreateCXXExprWithTemporaries(Arg);
 
   // Okay: add the default argument to the parameter
@@ -1276,6 +1277,8 @@ Sema::BuildMemberInitializer(FieldDecl *Member, Expr **Args,
                     MultiExprArg(*this, Args, NumArgs), 0);
   if (MemberInit.isInvalid())
     return true;
+
+  CheckImplicitConversions(MemberInit.get(), LParenLoc);
 
   // C++0x [class.base.init]p7:
   //   The initialization of each base and member constitutes a 
@@ -1407,6 +1410,8 @@ Sema::BuildBaseInitializer(QualType BaseType, TypeSourceInfo *BaseTInfo,
                     MultiExprArg(*this, Args, NumArgs), 0);
   if (BaseInit.isInvalid())
     return true;
+
+  CheckImplicitConversions(BaseInit.get(), LParenLoc);
 
   // C++0x [class.base.init]p7:
   //   The initialization of each base and member constitutes a 
@@ -5356,6 +5361,7 @@ bool Sema::InitializeVarWithConstructor(VarDecl *VD,
     return true;
 
   Expr *Temp = TempResult.takeAs<Expr>();
+  CheckImplicitConversions(Temp, VD->getLocation());
   MarkDeclarationReferenced(VD->getLocation(), Constructor);
   Temp = MaybeCreateCXXExprWithTemporaries(Temp);
   VD->setInit(Temp);
@@ -5489,6 +5495,8 @@ void Sema::AddCXXDirectInitializerToDecl(Decl *RealDecl,
     VDecl->setInvalidDecl();
     return;
   }
+
+  CheckImplicitConversions(Result.get(), LParenLoc);
 
   Result = MaybeCreateCXXExprWithTemporaries(Result.get());
   VDecl->setInit(Result.takeAs<Expr>());
@@ -6834,7 +6842,7 @@ void Sema::SetIvarInitializers(ObjCImplementationDecl *ObjCImplementation) {
       // is required (e.g., because it would call a trivial default constructor)
       if (!MemberInit.get() || MemberInit.isInvalid())
         continue;
-      
+
       Member =
         new (Context) CXXBaseOrMemberInitializer(Context,
                                                  Field, SourceLocation(),

clang/lib/Sema/SemaExprCXX.cpp

@@ -2962,9 +2962,6 @@ ExprResult Sema::MaybeBindToTemporary(Expr *E) {
 Expr *Sema::MaybeCreateCXXExprWithTemporaries(Expr *SubExpr) {
   assert(SubExpr && "sub expression can't be null!");
 
-  // Check any implicit conversions within the expression.
-  CheckImplicitConversions(SubExpr);
-
   unsigned FirstTemporary = ExprEvalContexts.back().NumTemporaries;
   assert(ExprTemporaries.size() >= FirstTemporary);
   if (ExprTemporaries.size() == FirstTemporary)
@@ -3381,5 +3378,7 @@ ExprResult Sema::ActOnNoexceptExpr(SourceLocation KeyLoc, SourceLocation,
 
 ExprResult Sema::ActOnFinishFullExpr(Expr *FullExpr) {
   if (!FullExpr) return ExprError();
+
+  CheckImplicitConversions(FullExpr);
   return MaybeCreateCXXExprWithTemporaries(FullExpr);
 }

clang/lib/Sema/SemaStmt.cpp

@@ -446,6 +446,7 @@ Sema::ActOnStartOfSwitchStmt(SourceLocation SwitchLoc, Expr *Cond,
   Cond = CondResult.take();
 
   if (!CondVar) {
+    CheckImplicitConversions(Cond, SwitchLoc);
     CondResult = MaybeCreateCXXExprWithTemporaries(Cond);
     if (CondResult.isInvalid())
       return StmtError();
@@ -889,6 +890,7 @@ Sema::ActOnDoStmt(SourceLocation DoLoc, Stmt *Body,
   if (CheckBooleanCondition(Cond, DoLoc))
     return StmtError();
 
+  CheckImplicitConversions(Cond, DoLoc);
   ExprResult CondResult = MaybeCreateCXXExprWithTemporaries(Cond);
   if (CondResult.isInvalid())
     return StmtError();
@@ -1173,8 +1175,10 @@ Sema::ActOnBlockReturnStmt(SourceLocation ReturnLoc, Expr *RetValExp) {
         return StmtError();
       }
 
-      if (RetValExp)
+      if (RetValExp) {
+        CheckImplicitConversions(RetValExp, ReturnLoc);
         RetValExp = MaybeCreateCXXExprWithTemporaries(RetValExp);
+      }
 
       RetValExp = Res.takeAs<Expr>();
       if (RetValExp) 
@@ -1227,6 +1231,7 @@ Sema::ActOnReturnStmt(SourceLocation ReturnLoc, Expr *RetValExp) {
           << RetValExp->getSourceRange();
       }
 
+      CheckImplicitConversions(RetValExp, ReturnLoc);
       RetValExp = MaybeCreateCXXExprWithTemporaries(RetValExp);
     }
 
@@ -1269,8 +1274,10 @@ Sema::ActOnReturnStmt(SourceLocation ReturnLoc, Expr *RetValExp) {
         CheckReturnStackAddr(RetValExp, FnRetType, ReturnLoc);
     }
 
-    if (RetValExp)
+    if (RetValExp) {
+      CheckImplicitConversions(RetValExp, ReturnLoc);
       RetValExp = MaybeCreateCXXExprWithTemporaries(RetValExp);
+    }
     Result = new (Context) ReturnStmt(ReturnLoc, RetValExp, NRVOCandidate);
   }
 

clang/test/Sema/Inputs/conversion.h

@@ -1,3 +1,4 @@
 /* Fake system header for Sema/conversion.c */
 
 #define LONG_MAX __LONG_MAX__
+#define SETBIT(set,bit) do { int i = bit; set[i/(8*sizeof(set[0]))] |= (1 << (i%(8*sizeof(set)))); } while(0)

clang/test/Sema/conversion.c

@@ -297,3 +297,16 @@ void test_7904686(void) {
   unsigned u2 = -1; // expected-warning {{implicit conversion changes signedness}}  
   u2 = -1; // expected-warning {{implicit conversion changes signedness}}  
 }
+
+// <rdar://problem/8232669>: don't warn about conversions required by
+// contexts in system headers
+void test_8232669(void) {
+  unsigned bitset[20];
+  SETBIT(bitset, 0);
+
+  unsigned y = 50;
+  SETBIT(bitset, y);
+
+#define USER_SETBIT(set,bit) do { int i = bit; set[i/(8*sizeof(set[0]))] |= (1 << (i%(8*sizeof(set)))); } while(0)
+  USER_SETBIT(bitset, 0); // expected-warning 2 {{implicit conversion changes signedness}}
+}