On Mar 10, 2011, at 1:22 PM, Ted Kremenek wrote: > 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.
Right, I get that... but why is this to right policy? -Chris > > 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
