[PATCH] D58189: [clang][Index] Fix usage of IndexImplicitInstantiation

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Only have a few NITs, will dig deeper into the change tomorrow.
Added @arphaman too as an owner of the index library. Alex, feel free to reassign if you're the wrong person to take a look at this



================
Comment at: unittests/Index/IndexTests.cpp:31
+struct Position {
+  size_t Line;
+  size_t Column;
----------------
NIT: initialize with 0 to avoid UB.


================
Comment at: unittests/Index/IndexTests.cpp:40
+    Position P;
+    P.Line = static_cast<int>(SM.getLineNumber(FID, Offset)) - 1;
+    P.Column = SM.getColumnNumber(FID, Offset) - 1;
----------------
Why do we need to `static_cast` to int? Can we leave out the cast?


================
Comment at: unittests/Index/IndexTests.cpp:45
+
+  bool operator==(const Position &Other) const {
+    return std::tie(Line, Column) == std::tie(Other.Line, Other.Column);
----------------
NIT: maybe make it a free-standing function, accepting two parameters:
```
struct Position {
  friend bool operator==(const Pos& L, const Pos& R) {
    // ...
  }
};
```
Doesn't really matter much here, though, just a general best practice.


================
Comment at: unittests/Index/IndexTests.cpp:97
   std::vector<TestSymbol> Symbols;
+  const ASTContext *AST;
 };
----------------
NIT: initialize with null to make UB less likely


Repository:
  rC Clang

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

https://reviews.llvm.org/D58189