[cfe-commits] r90280 - /cfe/trunk/lib/Basic/SourceManager.cpp

[Chris Lattner]

On Dec 1, 2009, at 3:07 PM, Daniel Dunbar wrote:

> Author: ddunbar
> Date: Tue Dec  1 17:07:57 2009
> New Revision: 90280
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=90280&view=rev
> Log:
> In SourceManager::isBeforeInTranslationUnit, if we are trying to compare two source locations with no common ancestor in the include stack, determine order by assuming memory buffers preceed files, and then that FileIDs are created in order.
> 
> The later assumption is patently false, but this was already broken -- this situation is conceptually impossible, my feeling is we should fix SourceManager and friends to make it impossible in practice as well. However, we need to fix PR5662 and perhaps some other things involving memory buffers first. In the short term I'm pretty sure this is reliable.
> 
> Chris, Argiris, is this going to break anything that wasn't already broken?

My recollection is that this code was used to "binary search" a file/line/col position to a decl, I'm not sure if it matters.  Incidentally, ROffsMap in that function should really use a DenseMap. :)

-Chris

> 
> Modified:
>    cfe/trunk/lib/Basic/SourceManager.cpp
> 
> Modified: cfe/trunk/lib/Basic/SourceManager.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/SourceManager.cpp?rev=90280&r1=90279&r2=90280&view=diff
> 
> ==============================================================================
> --- cfe/trunk/lib/Basic/SourceManager.cpp (original)
> +++ cfe/trunk/lib/Basic/SourceManager.cpp Tue Dec  1 17:07:57 2009
> @@ -1087,30 +1087,20 @@
>       return LastResForBeforeTUCheck = LOffs.second < I->second;
>   }
> 
> -  // No common ancestor.
> -  // Now we are getting into murky waters. Most probably this is because one
> -  // location is in the predefines buffer.
> -
> -  const FileEntry *LEntry =
> -    getSLocEntry(LOffs.first).getFile().getContentCache()->Entry;
> -  const FileEntry *REntry =
> -    getSLocEntry(ROffs.first).getFile().getContentCache()->Entry;
> -
> -  // If the locations are in two memory buffers we give up, we can't answer
> -  // which one should be considered first.
> -  // FIXME: Should there be a way to "include" memory buffers in the translation
> -  // unit ?
> -  assert((LEntry != 0 || REntry != 0) && "Locations in memory buffers.");
> -  (void) REntry;
> -
> -  // Consider the memory buffer as coming before the file in the translation
> -  // unit.
> -  if (LEntry == 0)
> -    return LastResForBeforeTUCheck = true;
> -  else {
> -    assert(REntry == 0 && "Locations in not #included files ?");
> -    return LastResForBeforeTUCheck = false;
> -  }
> +  // There is no common ancestor, most probably because one location is in the
> +  // predefines buffer.
> +  //
> +  // FIXME: We should rearrange the external interface so this simply never
> +  // happens; it can't conceptually happen. Also see PR5662.
> +
> +  // If exactly one location is a memory buffer, assume it preceeds the other.
> +  bool LIsMB = !getSLocEntry(LOffs.first).getFile().getContentCache()->Entry;
> +  bool RIsMB = !getSLocEntry(ROffs.first).getFile().getContentCache()->Entry;
> +  if (LIsMB != RIsMB)
> +    return LastResForBeforeTUCheck = LIsMB;
> +
> +  // Otherwise, just assume FileIDs were created in order.
> +  return LastResForBeforeTUCheck = (LOffs.first < ROffs.first);
> }
> 
> void SourceManager::truncateFileAt(const FileEntry *Entry, unsigned Line, 
> 
> 
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits