From: John McCall Date: Fri, 8 Oct 2010 02:01:28 +0000 (+0000) Subject: Track the location of the context requiring an implicit conversion and use it X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=b4eb64d8426c0eaa58d398961e0e74ff85063d7c;p=clang Track the location of the context requiring an implicit conversion and use it to white-list conversions required by system headers. rdar://problem/8232669 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@116029 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h index 08645dcee4..3ef942b2e8 100644 --- a/include/clang/Sema/Sema.h +++ b/include/clang/Sema/Sema.h @@ -4472,7 +4472,7 @@ private: 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. /// diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp index ae2fef21c4..f03e1f9310 100644 --- a/lib/Sema/SemaChecking.cpp +++ b/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,12 +2564,14 @@ 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(); @@ -2577,6 +2579,13 @@ void CheckImplicitConversion(Sema &S, Expr *E, QualType T, 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; @@ -2584,7 +2593,7 @@ void CheckImplicitConversion(Sema &S, Expr *E, QualType T, // Strip vector types. if (isa(Source)) { if (!isa(Target)) - return DiagnoseImpCast(S, E, T, diag::warn_impcast_vector_scalar); + return DiagnoseImpCast(S, E, T, CC, diag::warn_impcast_vector_scalar); Source = cast(Source)->getElementType().getTypePtr(); Target = cast(Target)->getElementType().getTypePtr(); @@ -2593,7 +2602,7 @@ void CheckImplicitConversion(Sema &S, Expr *E, QualType T, // Strip complex types. if (isa(Source)) { if (!isa(Target)) - return DiagnoseImpCast(S, E, T, diag::warn_impcast_complex_scalar); + return DiagnoseImpCast(S, E, T, CC, diag::warn_impcast_complex_scalar); Source = cast(Source)->getElementType().getTypePtr(); Target = cast(Target)->getElementType().getTypePtr(); @@ -2621,7 +2630,7 @@ 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; } @@ -2629,7 +2638,7 @@ void CheckImplicitConversion(Sema &S, Expr *E, QualType T, // 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(E)) return CheckConditionalOperator(S, cast(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(E)) { E = cast(E)->getSubExpr()->IgnoreParenImpCasts(); - return AnalyzeImplicitConversions(S, E); + return AnalyzeImplicitConversions(S, E, CC); } // Do a somewhat different check with comparison operators. @@ -2767,9 +2778,10 @@ void AnalyzeImplicitConversions(Sema &S, Expr *OrigE) { if (isa(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(*I)); + AnalyzeImplicitConversions(S, cast(*I), CC); } } // end anonymous namespace @@ -2777,7 +2789,11 @@ void AnalyzeImplicitConversions(Sema &S, Expr *OrigE) { /// 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 diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp index 6cb2bcf512..025f696e09 100644 --- a/lib/Sema/SemaDecl.cpp +++ b/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); diff --git a/lib/Sema/SemaDeclCXX.cpp b/lib/Sema/SemaDeclCXX.cpp index 181b7169c5..a27785e4f3 100644 --- a/lib/Sema/SemaDeclCXX.cpp +++ b/lib/Sema/SemaDeclCXX.cpp @@ -136,6 +136,7 @@ Sema::SetParamDefaultArgument(ParmVarDecl *Param, Expr *Arg, return true; Arg = Result.takeAs(); + 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(); + 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()); @@ -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(), diff --git a/lib/Sema/SemaExprCXX.cpp b/lib/Sema/SemaExprCXX.cpp index c674031219..4618dcf216 100644 --- a/lib/Sema/SemaExprCXX.cpp +++ b/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); } diff --git a/lib/Sema/SemaStmt.cpp b/lib/Sema/SemaStmt.cpp index 1e7912cbdc..fd196af198 100644 --- a/lib/Sema/SemaStmt.cpp +++ b/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(); 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); } diff --git a/test/Sema/Inputs/conversion.h b/test/Sema/Inputs/conversion.h index 9f6ed2e70f..768190f8d2 100644 --- a/test/Sema/Inputs/conversion.h +++ b/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) diff --git a/test/Sema/conversion.c b/test/Sema/conversion.c index 5fbd64de5b..411ca32b5a 100644 --- a/test/Sema/conversion.c +++ b/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}} } + +// : 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}} +}