[PATCH] D109603: add a new API seek for the Cursor class in the DataExtractor.cpp

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/include/llvm/Support/DataExtractor.h:73
 
+    /// Set the cursor to the new offset.
+    void seek(uint64_t NewOffSet) { Offset = NewOffSet; }
----------------
Perhaps add another sentence saying "This does not impact the error state."


================
Comment at: llvm/unittests/Support/DataExtractorTest.cpp:181
+TEST(DataExtractorTest, Cursor_seek) {
+  DataExtractor DE(StringRef("ABCDEF"), false, 8);
+  DataExtractor::Cursor C(0);
----------------
I think you can simplify this testing: `Cursor` doesn't depend on `DataExtractor`, so you can just instantiate a Cursor and then call seek on it a couple of times to show what happens, like the following:

```
Cursor C(5);
C.seek(3);
EXPECT_EQ(3u, C.tell());
C.seek(8);
EXPECT_EQ(8u, C.tell());
```

This is different to `tell`, where reading data impacts the `tell` location.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D109603/new/

https://reviews.llvm.org/D109603