[clang] [libclang/python] Fix bug in `SourceRange.__contains__`, add tests (PR #101802)

Aaron Ballman via cfe-commits cfe-commits at lists.llvm.org
Mon Aug 12 08:31:02 PDT 2024


================
@@ -386,6 +386,10 @@ def __contains__(self, other):
         # same file, in between lines
         if self.start.line < other.line < self.end.line:
             return True
+        # between columns in one-liner range
+        elif self.start.line == other.line == self.end.line:
----------------
AaronBallman wrote:

> I'm against anything non file-sensitive, because that's buggy and is going to surprise everyone. I'm less concerned with cleanliness of implementation in the face of this concern.

+1, that would be surprising behavior.

> At this point I'm even more concerned by amount of places that interpret raw encoding of SourceLocation, even though (in my understanding) SourceManager is the only entity that should do so.

The raw encoding of source locations is an implementation detail and should not be relied on. You don't always have to go to the `SourceManager` (for example, you can get the raw encoding from one `SourceLocation` and use it to construct an equivalent `SourceLocation` through the correct constructor), but we should not be relying on the representation to be stable.

It should also be noted that modules may break everything that's comparing raw locations:
```
/// Technically, a source location is simply an offset into the manager's view
/// of the input source, which is all input buffers (including macro
/// expansions) concatenated in an effectively arbitrary order. The manager
/// actually maintains two blocks of input buffers. One, starting at offset
/// 0 and growing upwards, contains all buffers from this module. The other,
/// starting at the highest possible offset and growing downwards, contains
/// buffers of loaded modules.
```

https://github.com/llvm/llvm-project/pull/101802


More information about the cfe-commits mailing list