[SourceManager] don't check invalid param of getLocalSLocEntry()

[nickdesaulniers]

Summary

Forked from D80681.

getLocalSLocEntry() has an unused parameter used to satisfy an interface
of libclang (see getInclusions() in
clang/tools/libclang/CIndexInclusionStack.cpp). It's pointless for
callers to construct/pass/check this inout parameter that can never
signify that a FileID is invalid.

[kadircet]

clang/include/clang/Basic/SourceManager.h

1648

i think we should drop Invalid from the signature completely if we are going for this.

I've sent out D82532 to get rid of the libclang dependency on this, after it lands this should be possible to clear.

[kadircet]

clang/include/clang/Basic/SourceManager.h

1648

landed that patch please rebase and get rid of this parameter completely. Also please add a comment to getSLocEntryByID saying that Invalid won't be modified for local sloc entries.

[nickdesaulniers]

• drop Invalid param from getLocalSLocEntry

[kadircet]

thanks, lgtm!

clang/include/clang/Basic/SourceManager.h

public:
   void PrintStats() const;
 
   void dump() const;
 
   /// Get the number of local SLocEntries we have.
   unsigned local_sloc_entry_size() const { return LocalSLocEntryTable.size(); }
 
   /// Get a local SLocEntry. This is exposed for indexing.
-  const SrcMgr::SLocEntry &getLocalSLocEntry(unsigned Index,
-                                             bool *Invalid = nullptr) const {
+  const SrcMgr::SLocEntry &getLocalSLocEntry(unsigned Index) const {
     assert(Index < LocalSLocEntryTable.size() && "Invalid index");
     return LocalSLocEntryTable[Index];
   }
 
   /// Get the number of loaded SLocEntries we have.
   unsigned loaded_sloc_entry_size() const { return LoadedSLocEntryTable.size();}
 
   /// Get a loaded SLocEntry. This is exposed for indexing.
private:
   friend class ASTWriter;
 
   llvm::MemoryBuffer *getFakeBufferForRecovery() const;
   const SrcMgr::ContentCache *getFakeContentCacheForRecovery() const;
 
   const SrcMgr::SLocEntry &loadSLocEntry(unsigned Index, bool *Invalid) const;
 
   /// Get the entry with the given unwrapped FileID.
+  /// Invalid will not be modified for Local IDs.
   const SrcMgr::SLocEntry &getSLocEntryByID(int ID,
                                             bool *Invalid = nullptr) const {
     assert(ID != -1 && "Using FileID sentinel value");
     if (ID < 0)
       return getLoadedSLocEntryByID(ID, Invalid);
-    return getLocalSLocEntry(static_cast<unsigned>(ID), Invalid);
+    return getLocalSLocEntry(static_cast<unsigned>(ID));
   }
 
   const SrcMgr::SLocEntry &
   getLoadedSLocEntryByID(int ID, bool *Invalid = nullptr) const {
     return getLoadedSLocEntry(static_cast<unsigned>(-ID - 2), Invalid);
   }
 
   /// Implements the common elements of storing an expansion info struct into

clang/lib/Basic/SourceManager.cpp

FileID SourceManager::getFileIDLocal(unsigned SLocOffset) const {
   // larger than the offset we are looking for.
   unsigned GreaterIndex = I - LocalSLocEntryTable.begin();
   // LessIndex - This is the lower bound of the range that we're searching.
   // We know that the offset corresponding to the FileID is is less than
   // SLocOffset.
   unsigned LessIndex = 0;
   NumProbes = 0;
   while (true) {
-    bool Invalid = false;
     unsigned MiddleIndex = (GreaterIndex-LessIndex)/2+LessIndex;
-    unsigned MidOffset = getLocalSLocEntry(MiddleIndex, &Invalid).getOffset();
-    if (Invalid)
-      return FileID::get(0);
+    unsigned MidOffset = getLocalSLocEntry(MiddleIndex).getOffset();
 
     ++NumProbes;
 
     // If the offset of the midpoint is too large, chop the high side of the
     // range to the midpoint.
     if (MidOffset > SLocOffset) {
       GreaterIndex = MiddleIndex;
       continue;
if (MainSLoc.isFile()) {
       if (MainContentCache && MainContentCache->OrigEntry == SourceFile)
         return MainFileID;
     }
   }
 
   // The location we're looking for isn't in the main file; look
   // through all of the local source locations.
   for (unsigned I = 0, N = local_sloc_entry_size(); I != N; ++I) {
-    bool Invalid = false;
-    const SLocEntry &SLoc = getLocalSLocEntry(I, &Invalid);
-    if (Invalid)
-      return FileID();
-
+    const SLocEntry &SLoc = getLocalSLocEntry(I);
     if (SLoc.isFile() && SLoc.getFile().getContentCache() &&
         SLoc.getFile().getContentCache()->OrigEntry == SourceFile)
       return FileID::get(I);
   }
 
   // If that still didn't help, try the modules.
   for (unsigned I = 0, N = loaded_sloc_entry_size(); I != N; ++I) {
     const SLocEntry &SLoc = getLoadedSLocEntry(I);