[cfe-commits] r127425 - /cfe/trunk/lib/Sema/SemaChecking.cpp

[Ted Kremenek]

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
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>