[clang-tools-extra] r187736 - Fixed incorrect include file exit detection. Added work-around to avoid error on header guard in nested include. Fixed a couple of coding standard issues on variable names.

[Thompson, John]

Sorry, I'm still getting the hang of things and didn't think to watch for build breakage emails since what I thought was a simple change passed the tests for me.  An uninitialized variable got me.  I've fixed it and recommitted and will do better in the future.

-John

-----Original Message-----
From: cfe-commits-bounces at cs.uiuc.edu [mailto:cfe-commits-bounces at cs.uiuc.edu] On Behalf Of Arnold Schwaighofer
Sent: Monday, August 05, 2013 3:08 PM
To: John Thompson
Cc: cfe-commits at cs.uiuc.edu
Subject: Re: [clang-tools-extra] r187736 - Fixed incorrect include file exit detection. Added work-around to avoid error on header guard in nested include. Fixed a couple of coding standard issues on variable names.

Hi John,


I have reverted the commit in 187746.

The following bots were broken for the past 2 hours:

http://lab.llvm.org:8011/builders/clang-native-arm-cortex-a9/builds/10248
http://bb.pgr.jp/builders/cmake-clang-i686-mingw32/builds/3327


Thanks,
Arnold



On Aug 5, 2013, at 3:13 PM, Arnold Schwaighofer <aschwaighofer at apple.com> wrote:

> Hi John,
> 
> This commit (187736) appears to have broken
> 
> http://bb.pgr.jp/builders/cmake-clang-i686-mingw32/builds/3327
> 
> and our internal bots. Can you please fix or revert?
> 
> 
> Thanks,
> Arnold
> 
> On Aug 5, 2013, at 2:15 PM, John Thompson <John.Thompson.JTSoftware at gmail.com> wrote:
> 
>> Author: jtsoftware
>> Date: Mon Aug  5 14:15:50 2013
>> New Revision: 187736
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=187736&view=rev
>> Log:
>> Fixed incorrect include file exit detection.  Added work-around to avoid error on header guard in nested include.  Fixed a couple of coding standard issues on variable names.
>> 
>> Modified:
>>   clang-tools-extra/trunk/modularize/PreprocessorTracker.cpp
>> 
>> Modified: clang-tools-extra/trunk/modularize/PreprocessorTracker.cpp
>> URL: 
>> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/modulariz
>> e/PreprocessorTracker.cpp?rev=187736&r1=187735&r2=187736&view=diff
>> =====================================================================
>> =========
>> --- clang-tools-extra/trunk/modularize/PreprocessorTracker.cpp 
>> (original)
>> +++ clang-tools-extra/trunk/modularize/PreprocessorTracker.cpp Mon 
>> +++ Aug  5 14:15:50 2013
>> @@ -818,8 +818,12 @@ public:
>>    if (HeaderPath.startswith("<"))
>>      return;
>>    HeaderHandle H = addHeader(HeaderPath);
>> -    if (H != getCurrentHeaderHandle())
>> +    if (H != getCurrentHeaderHandle()) {
>> +      // Check for nested header.
>> +      if (!InNestedHeader)
>> +        InNestedHeader = isHeaderHandleInStack(H);
>>      pushHeaderHandle(H);
>> +    }
>>  }
>>  // Handle exiting a header source file.
>>  void handleHeaderExit(llvm::StringRef HeaderPath) { @@ -831,6 +835,7 
>> @@ public:
>>      while ((H != getCurrentHeaderHandle()) && (HeaderStack.size() != 0))
>>        popHeaderHandle();
>>    }
>> +    InNestedHeader = false;
>>  }
>> 
>>  // Lookup/add string.
>> @@ -839,11 +844,13 @@ public:
>>  // Get the handle of a header file entry.
>>  // Return HeaderHandleInvalid if not found.
>>  HeaderHandle findHeaderHandle(llvm::StringRef HeaderPath) const {
>> +    std::string CanonicalPath(HeaderPath);
>> +    std::replace(CanonicalPath.begin(), CanonicalPath.end(), '\\', 
>> + '/');
>>    HeaderHandle H = 0;
>>    for (std::vector<StringHandle>::const_iterator I = HeaderPaths.begin(),
>>                                                   E = HeaderPaths.end();
>>         I != E; ++I, ++H) {
>> -      if (**I == HeaderPath)
>> +      if (**I == CanonicalPath)
>>        return H;
>>    }
>>    return HeaderHandleInvalid;
>> @@ -852,12 +859,12 @@ public:
>>  // Add a new header file entry, or return existing handle.
>>  // Return the header handle.
>>  HeaderHandle addHeader(llvm::StringRef HeaderPath) {
>> -    std::string canonicalPath(HeaderPath);
>> -    std::replace(canonicalPath.begin(), canonicalPath.end(), '\\', '/');
>> -    HeaderHandle H = findHeaderHandle(canonicalPath);
>> +    std::string CanonicalPath(HeaderPath);
>> +    std::replace(CanonicalPath.begin(), CanonicalPath.end(), '\\', '/');
>> +    HeaderHandle H = findHeaderHandle(CanonicalPath);
>>    if (H == HeaderHandleInvalid) {
>>      H = HeaderPaths.size();
>> -      HeaderPaths.push_back(addString(canonicalPath));
>> +      HeaderPaths.push_back(addString(CanonicalPath));
>>    }
>>    return H;
>>  }
>> @@ -993,6 +1000,9 @@ public:
>>                                  bool ConditionValue,
>>                                  llvm::StringRef ConditionUnexpanded,
>>                                  InclusionPathHandle 
>> InclusionPathHandle) {
>> +    // Ignore header guards, assuming the header guard is the only conditional.
>> +    if (InNestedHeader)
>> +      return;
>>    StringHandle ConditionUnexpandedHandle(addString(ConditionUnexpanded));
>>    PPItemKey InstanceKey(PP, ConditionUnexpandedHandle, H, InstanceLoc);
>>    ConditionalExpansionMapIter I = 
>> ConditionalExpansions.find(InstanceKey);
>> @@ -1002,7 +1012,8 @@ public:
>>          getSourceLocationString(PP, InstanceLoc) + ":\n" +
>>          getSourceLine(PP, InstanceLoc) + "\n";
>>      ConditionalExpansions[InstanceKey] =
>> -          ConditionalTracker(DirectiveKind, ConditionValue, ConditionUnexpandedHandle,
>> +          ConditionalTracker(DirectiveKind, ConditionValue,
>> +                             ConditionUnexpandedHandle,
>>                             InclusionPathHandle);
>>    } else {
>>      // We've seen the conditional before.  Get its tracker.
>> @@ -1157,6 +1168,7 @@ private:
>>  InclusionPathHandle CurrentInclusionPathHandle;  MacroExpansionMap 
>> MacroExpansions;  ConditionalExpansionMap ConditionalExpansions;
>> +  bool InNestedHeader;
>> };
>> 
>> // PreprocessorTracker functions.
>> @@ -1180,10 +1192,12 @@ void PreprocessorCallbacks::FileChanged(
>>    PPTracker.handleHeaderEntry(PP, getSourceLocationFile(PP, Loc));
>>    break;
>>  case ExitFile:
>> -    if (PrevFID.isInvalid())
>> -      PPTracker.handleHeaderExit(RootHeaderFile);
>> -    else
>> -      PPTracker.handleHeaderExit(getSourceLocationFile(PP, Loc));
>> +    {
>> +      const clang::FileEntry *F =
>> +        PP.getSourceManager().getFileEntryForID(PrevFID);
>> +      if (F != NULL)
>> +        PPTracker.handleHeaderExit(F->getName());
>> +    }
>>    break;
>>  case SystemHeaderPragma:
>>  case RenameFile:
>> 
>> 
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> 
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

_______________________________________________
cfe-commits mailing list
cfe-commits at cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits