The spelling location is checked because the diagnostic gets pruned if: 1) The diagnostic was instantiated from a macro
AND 2) That macro was from a system header. #2 is the reason getSpellingLoc() is called. On Mar 10, 2011, at 1:09 PM, Chris Lattner wrote: > > On Mar 10, 2011, at 12:03 PM, Ted Kremenek wrote: > >> Author: kremenek >> Date: Thu Mar 10 14:03:42 2011 >> New Revision: 127425 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=127425&view=rev >> Log: >> Profiling showed that 'CheckImplicitConversions' was very slow because of >> the call to getSpellingLoc(). On 'aes.c' >> in the LLVM test suite, this function was consuming 7.4% of -fsyntax-only >> time. This change fixes this issue >> by delaying the check that the warning would be issued within a system macro >> by as long as possible. The >> main negative of this change is now the logic for this check is done in >> multiple places in this function instead >> of just in one place up front. > > Nice catch Ted. Why are we checking the spelling location in any case? > Whether a token came from a system header doesn't sound that interesting to > me, and *is* very expensive to compute. > > -Chris > >> >> Modified: >> cfe/trunk/lib/Sema/SemaChecking.cpp >> >> Modified: cfe/trunk/lib/Sema/SemaChecking.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=127425&r1=127424&r2=127425&view=diff >> ============================================================================== >> --- cfe/trunk/lib/Sema/SemaChecking.cpp (original) >> +++ cfe/trunk/lib/Sema/SemaChecking.cpp Thu Mar 10 14:03:42 2011 >> @@ -2762,6 +2762,11 @@ >> return ValueInRange.toString(10); >> } >> >> +static bool isFromSystemMacro(Sema &S, SourceLocation loc) { >> + SourceManager &smgr = S.Context.getSourceManager(); >> + return loc.isMacroID() && smgr.isInSystemHeader(smgr.getSpellingLoc(loc)); >> +} >> + >> void CheckImplicitConversion(Sema &S, Expr *E, QualType T, >> SourceLocation CC, bool *ICContext = 0) { >> if (E->isTypeDependent() || E->isValueDependent()) return; >> @@ -2771,11 +2776,12 @@ >> 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)))) >> + // If the conversion context location is invalid don't complain. >> + // We also don't want to emit a warning if the issue occurs from the >> + // instantiation of a system macro. The problem is that >> 'getSpellingLoc()' >> + // is slow, so we delay this check as long as possible. Once we detect >> + // we are in that scenario, we just return. >> + if (CC.isInvalid()) >> return; >> >> // Never diagnose implicit casts to bool. >> @@ -2784,8 +2790,11 @@ >> >> // Strip vector types. >> if (isa<VectorType>(Source)) { >> - if (!isa<VectorType>(Target)) >> + if (!isa<VectorType>(Target)) { >> + if (isFromSystemMacro(S, CC)) >> + return; >> return DiagnoseImpCast(S, E, T, CC, diag::warn_impcast_vector_scalar); >> + } >> >> Source = cast<VectorType>(Source)->getElementType().getTypePtr(); >> Target = cast<VectorType>(Target)->getElementType().getTypePtr(); >> @@ -2793,8 +2802,12 @@ >> >> // Strip complex types. >> if (isa<ComplexType>(Source)) { >> - if (!isa<ComplexType>(Target)) >> + if (!isa<ComplexType>(Target)) { >> + if (isFromSystemMacro(S, CC)) >> + return; >> + >> return DiagnoseImpCast(S, E, T, CC, diag::warn_impcast_complex_scalar); >> + } >> >> Source = cast<ComplexType>(Source)->getElementType().getTypePtr(); >> Target = cast<ComplexType>(Target)->getElementType().getTypePtr(); >> @@ -2822,13 +2835,19 @@ >> return; >> } >> >> + if (isFromSystemMacro(S, CC)) >> + return; >> + >> DiagnoseImpCast(S, E, T, CC, diag::warn_impcast_float_precision); >> } >> return; >> } >> >> - // If the target is integral, always warn. >> + // If the target is integral, always warn. >> if ((TargetBT && TargetBT->isInteger())) { >> + if (isFromSystemMacro(S, CC)) >> + return; >> + >> Expr *InnerE = E->IgnoreParenImpCasts(); >> if (FloatingLiteral *LiteralExpr = dyn_cast<FloatingLiteral>(InnerE)) { >> DiagnoseImpCast(S, LiteralExpr, T, CC, >> @@ -2852,6 +2871,9 @@ >> // TODO: this should happen for bitfield stores, too. >> llvm::APSInt Value(32); >> if (E->isIntegerConstantExpr(Value, S.Context)) { >> + if (isFromSystemMacro(S, CC)) >> + return; >> + >> std::string PrettySourceValue = Value.toString(10); >> std::string PrettyTargetValue = PrettyPrintInRange(Value, TargetRange); >> >> @@ -2863,6 +2885,10 @@ >> >> // People want to build with -Wshorten-64-to-32 and not -Wconversion >> // and by god we'll let them. >> + >> + if (isFromSystemMacro(S, CC)) >> + return; >> + >> if (SourceRange.Width == 64 && TargetRange.Width == 32) >> return DiagnoseImpCast(S, E, T, CC, diag::warn_impcast_integer_64_32); >> return DiagnoseImpCast(S, E, T, CC, diag::warn_impcast_integer_precision); >> @@ -2871,6 +2897,10 @@ >> if ((TargetRange.NonNegative && !SourceRange.NonNegative) || >> (!TargetRange.NonNegative && SourceRange.NonNegative && >> SourceRange.Width == TargetRange.Width)) { >> + >> + if (isFromSystemMacro(S, CC)) >> + return; >> + >> unsigned DiagID = diag::warn_impcast_integer_sign; >> >> // Traditionally, gcc has warned about this under -Wsign-compare. >> @@ -2893,9 +2923,13 @@ >> SourceEnum->getDecl()->getTypedefForAnonDecl()) && >> (TargetEnum->getDecl()->getIdentifier() || >> TargetEnum->getDecl()->getTypedefForAnonDecl()) && >> - SourceEnum != TargetEnum) >> + SourceEnum != TargetEnum) { >> + if (isFromSystemMacro(S, CC)) >> + return; >> + >> return DiagnoseImpCast(S, E, T, CC, >> diag::warn_impcast_different_enum_types); >> + } >> >> return; >> } >> >> >> _______________________________________________ >> cfe-commits mailing list >> [email protected] >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
