[PATCH] D108045: [clangd] Fix clangd crash when including a header

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.


================
Comment at: clang-tools-extra/clangd/Preamble.cpp:148
   bool operator==(const TextualPPDirective &RHS) const {
-    return std::tie(DirectiveLine, Text) ==
-           std::tie(RHS.DirectiveLine, RHS.Text);
+    return std::tie(DirectiveLine, Text, Offset) ==
+           std::tie(RHS.DirectiveLine, RHS.Text, RHS.Offset);
----------------
nit: i'd put offset before text, since it's faster to compare.


================
Comment at: clang-tools-extra/clangd/Preamble.cpp:533
 
+PreamblePatch PreamblePatch::createMacroPatch(llvm::StringRef FileName,
+                                              const ParseInputs &Modified,
----------------
sorry for not being explicit about this on the first round but rather than duplicating the logic, can we have a shared private factory function that takes an extra parameter to control whether includes/macros should be included in the patch? It can probably look something like this:

```
class PreamblePatch {
public:
  enum class PatchType {
     MacroDirectives,
     All,
  };
  // these two will just call the create with relevant patchtype
  static PreamblePatch createFullPatch(FileName, Modified, Baseline);
  static PreamblePatch createMacroPatch(FileName, Modified, Baseline);
private:
  static PreamblePatch create(filename, Modified, Baseline, PatchType);
}
```


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

https://reviews.llvm.org/D108045