[PATCH] D63316: [clangd] Include the diagnostics's code when comparing diagnostics

[Nathan Ridge via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL363889: [clangd] Include the diagnosticss code when 
comparing diagnostics (authored by nridge, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D63316?vs=205694=205695#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D63316

Files:
  clang-tools-extra/trunk/clangd/Protocol.h
  clang-tools-extra/trunk/clangd/test/fixits-duplication.test

Index: clang-tools-extra/trunk/clangd/test/fixits-duplication.test
===
--- clang-tools-extra/trunk/clangd/test/fixits-duplication.test
+++ clang-tools-extra/trunk/clangd/test/fixits-duplication.test
@@ -0,0 +1,221 @@
+# RUN: clangd -lit-test -clang-tidy-checks=modernize-use-nullptr,hicpp-use-nullptr < %s | FileCheck -strict-whitespace %s
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{"textDocument":{"codeAction":{"codeActionLiteralSupport":{,"trace":"off"}}
+---
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///foo.cpp","languageId":"cpp","version":1,"text":"void foo() { char* p = 0; }"}}}
+#  CHECK:"method": "textDocument/publishDiagnostics",
+# CHECK-NEXT:  "params": {
+# CHECK-NEXT:"diagnostics": [
+# CHECK-NEXT:  {
+# CHECK-NEXT:"code": "hicpp-use-nullptr",
+# CHECK-NEXT:"message": "Use nullptr (fix available)",
+# CHECK-NEXT:"range": {
+# CHECK-NEXT:  "end": {
+# CHECK-NEXT:"character": 24,
+# CHECK-NEXT:"line": 0
+# CHECK-NEXT:  },
+# CHECK-NEXT:  "start": {
+# CHECK-NEXT:"character": 23,
+# CHECK-NEXT:"line": 0
+# CHECK-NEXT:  }
+# CHECK-NEXT:},
+# CHECK-NEXT:"severity": 2,
+# CHECK-NEXT:"source": "clang-tidy"
+# CHECK-NEXT:  },
+# CHECK-NEXT:  {
+# CHECK-NEXT:"code": "modernize-use-nullptr",
+# CHECK-NEXT:"message": "Use nullptr (fix available)",
+# CHECK-NEXT:"range": {
+# CHECK-NEXT:  "end": {
+# CHECK-NEXT:"character": 24,
+# CHECK-NEXT:"line": 0
+# CHECK-NEXT:  },
+# CHECK-NEXT:  "start": {
+# CHECK-NEXT:"character": 23,
+# CHECK-NEXT:"line": 0
+# CHECK-NEXT:  }
+# CHECK-NEXT:},
+# CHECK-NEXT:"severity": 2,
+# CHECK-NEXT:"source": "clang-tidy"
+# CHECK-NEXT:  }
+# CHECK-NEXT:],
+# CHECK-NEXT:"uri": "file:///{{.*}}/foo.cpp"
+# CHECK-NEXT:  }
+---
+{"jsonrpc":"2.0","id":2,"method":"textDocument/codeAction","params":{"textDocument":{"uri":"test:///foo.cpp"},"range":{"start":{"line":0,"character":23},"end":{"line":0,"character":24}},"context":{"diagnostics":[{"range":{"start": {"line": 0, "character": 23}, "end": {"line": 0, "character": 24}},"severity":2,"message":"Use nullptr (fix available)", "code": "hicpp-use-nullptr", "source": "clang-tidy"},{"range":{"start": {"line": 0, "character": 23}, "end": {"line": 0, "character": 24}},"severity":2,"message":"Use nullptr (fix available)", "code": "modernize-use-nullptr", "source": "clang-tidy"}]}}}
+#  CHECK:  "id": 2,
+# CHECK-NEXT:  "jsonrpc": "2.0",
+# CHECK-NEXT:  "result": [
+# CHECK-NEXT:{
+# CHECK-NEXT:  "diagnostics": [
+# CHECK-NEXT:{
+# CHECK-NEXT:  "code": "hicpp-use-nullptr",
+# CHECK-NEXT:  "message": "Use nullptr (fix available)",
+# CHECK-NEXT:  "range": {
+# CHECK-NEXT:"end": {
+# CHECK-NEXT:  "character": 24,
+# CHECK-NEXT:  "line": 0
+# CHECK-NEXT:},
+# CHECK-NEXT:"start": {
+# CHECK-NEXT:  "character": 23,
+# CHECK-NEXT:  "line": 0
+# CHECK-NEXT:}
+# CHECK-NEXT:  },
+# CHECK-NEXT:  "severity": 2,
+# CHECK-NEXT:  "source": "clang-tidy"
+# CHECK-NEXT:}
+# CHECK-NEXT:  ],
+# CHECK-NEXT:  "edit": {
+# CHECK-NEXT:"changes": {
+# CHECK-NEXT:  "file://{{.*}}/foo.cpp": [
+# CHECK-NEXT:{
+# CHECK-NEXT:  "newText": "nullptr",
+# CHECK-NEXT:  "range": {
+# CHECK-NEXT:"end": {
+# CHECK-NEXT:  "character": 24,
+# CHECK-NEXT:  "line": 0
+# CHECK-NEXT:},
+# CHECK-NEXT:"start": {
+# CHECK-NEXT:  "character": 23,
+# CHECK-NEXT:  "line": 0
+# CHECK-NEXT:}
+# CHECK-NEXT:  }
+# CHECK-NEXT:}
+# CHECK-NEXT:  ]
+# CHECK-NEXT:}
+# CHECK-NEXT:  },
+# CHECK-NEXT:  "kind": "quickfix",
+# CHECK-NEXT:  "title": "change '0' to 'nullptr'"
+# CHECK-NEXT:},
+# CHECK-NEXT:{
+# CHECK-NEXT:  "diagnostics": [
+# 

[PATCH] D63316: [clangd] Include the diagnostics's code when comparing diagnostics

[Nathan Ridge via Phabricator via cfe-commits]

nridge updated this revision to Diff 205694.
nridge added a comment.

Add requested test case


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63316

Files:
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/test/fixits-duplication.test

Index: clang-tools-extra/clangd/test/fixits-duplication.test
===
--- /dev/null
+++ clang-tools-extra/clangd/test/fixits-duplication.test
@@ -0,0 +1,221 @@
+# RUN: clangd -lit-test -clang-tidy-checks=modernize-use-nullptr,hicpp-use-nullptr < %s | FileCheck -strict-whitespace %s
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{"textDocument":{"codeAction":{"codeActionLiteralSupport":{,"trace":"off"}}
+---
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///foo.cpp","languageId":"cpp","version":1,"text":"void foo() { char* p = 0; }"}}}
+#  CHECK:"method": "textDocument/publishDiagnostics",
+# CHECK-NEXT:  "params": {
+# CHECK-NEXT:"diagnostics": [
+# CHECK-NEXT:  {
+# CHECK-NEXT:"code": "hicpp-use-nullptr",
+# CHECK-NEXT:"message": "Use nullptr (fix available)",
+# CHECK-NEXT:"range": {
+# CHECK-NEXT:  "end": {
+# CHECK-NEXT:"character": 24,
+# CHECK-NEXT:"line": 0
+# CHECK-NEXT:  },
+# CHECK-NEXT:  "start": {
+# CHECK-NEXT:"character": 23,
+# CHECK-NEXT:"line": 0
+# CHECK-NEXT:  }
+# CHECK-NEXT:},
+# CHECK-NEXT:"severity": 2,
+# CHECK-NEXT:"source": "clang-tidy"
+# CHECK-NEXT:  },
+# CHECK-NEXT:  {
+# CHECK-NEXT:"code": "modernize-use-nullptr",
+# CHECK-NEXT:"message": "Use nullptr (fix available)",
+# CHECK-NEXT:"range": {
+# CHECK-NEXT:  "end": {
+# CHECK-NEXT:"character": 24,
+# CHECK-NEXT:"line": 0
+# CHECK-NEXT:  },
+# CHECK-NEXT:  "start": {
+# CHECK-NEXT:"character": 23,
+# CHECK-NEXT:"line": 0
+# CHECK-NEXT:  }
+# CHECK-NEXT:},
+# CHECK-NEXT:"severity": 2,
+# CHECK-NEXT:"source": "clang-tidy"
+# CHECK-NEXT:  }
+# CHECK-NEXT:],
+# CHECK-NEXT:"uri": "file:///{{.*}}/foo.cpp"
+# CHECK-NEXT:  }
+---
+{"jsonrpc":"2.0","id":2,"method":"textDocument/codeAction","params":{"textDocument":{"uri":"test:///foo.cpp"},"range":{"start":{"line":0,"character":23},"end":{"line":0,"character":24}},"context":{"diagnostics":[{"range":{"start": {"line": 0, "character": 23}, "end": {"line": 0, "character": 24}},"severity":2,"message":"Use nullptr (fix available)", "code": "hicpp-use-nullptr", "source": "clang-tidy"},{"range":{"start": {"line": 0, "character": 23}, "end": {"line": 0, "character": 24}},"severity":2,"message":"Use nullptr (fix available)", "code": "modernize-use-nullptr", "source": "clang-tidy"}]}}}
+#  CHECK:  "id": 2,
+# CHECK-NEXT:  "jsonrpc": "2.0",
+# CHECK-NEXT:  "result": [
+# CHECK-NEXT:{
+# CHECK-NEXT:  "diagnostics": [
+# CHECK-NEXT:{
+# CHECK-NEXT:  "code": "hicpp-use-nullptr",
+# CHECK-NEXT:  "message": "Use nullptr (fix available)",
+# CHECK-NEXT:  "range": {
+# CHECK-NEXT:"end": {
+# CHECK-NEXT:  "character": 24,
+# CHECK-NEXT:  "line": 0
+# CHECK-NEXT:},
+# CHECK-NEXT:"start": {
+# CHECK-NEXT:  "character": 23,
+# CHECK-NEXT:  "line": 0
+# CHECK-NEXT:}
+# CHECK-NEXT:  },
+# CHECK-NEXT:  "severity": 2,
+# CHECK-NEXT:  "source": "clang-tidy"
+# CHECK-NEXT:}
+# CHECK-NEXT:  ],
+# CHECK-NEXT:  "edit": {
+# CHECK-NEXT:"changes": {
+# CHECK-NEXT:  "file://{{.*}}/foo.cpp": [
+# CHECK-NEXT:{
+# CHECK-NEXT:  "newText": "nullptr",
+# CHECK-NEXT:  "range": {
+# CHECK-NEXT:"end": {
+# CHECK-NEXT:  "character": 24,
+# CHECK-NEXT:  "line": 0
+# CHECK-NEXT:},
+# CHECK-NEXT:"start": {
+# CHECK-NEXT:  "character": 23,
+# CHECK-NEXT:  "line": 0
+# CHECK-NEXT:}
+# CHECK-NEXT:  }
+# CHECK-NEXT:}
+# CHECK-NEXT:  ]
+# CHECK-NEXT:}
+# CHECK-NEXT:  },
+# CHECK-NEXT:  "kind": "quickfix",
+# CHECK-NEXT:  "title": "change '0' to 'nullptr'"
+# CHECK-NEXT:},
+# CHECK-NEXT:{
+# CHECK-NEXT:  "diagnostics": [
+# CHECK-NEXT:{
+# CHECK-NEXT:  "code": "modernize-use-nullptr",
+# CHECK-NEXT:  "message": "Use nullptr (fix available)",
+# CHECK-NEXT:  "range": {
+# CHECK-NEXT:"end": {
+# CHECK-NEXT:  "character": 24,
+# CHECK-NEXT:  "line": 0
+# CHECK-NEXT:

[PATCH] D63316: [clangd] Include the diagnostics's code when comparing diagnostics

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet accepted this revision.
kadircet added a comment.
This revision is now accepted and ready to land.

LGTM with one more test case request. Thanks!




Comment at: clang-tools-extra/clangd/test/fixits-duplication.test:44
+---
+{"jsonrpc":"2.0","id":2,"method":"textDocument/codeAction","params":{"textDocument":{"uri":"test:///foo.cpp"},"range":{"start":{"line":0,"character":23},"end":{"line":0,"character":24}},"context":{"diagnostics":[{"range":{"start":
 {"line": 0, "character": 23}, "end": {"line": 0, "character": 
24}},"severity":2,"message":"Use nullptr (fix available)", "code": 
"hicpp-use-nullptr", "source": "clang-tidy"},{"range":{"start": {"line": 0, 
"character": 23}, "end": {"line": 0, "character": 
24}},"severity":2,"message":"Use nullptr (fix available)", "code": 
"modernize-use-nullptr", "source": "clang-tidy"}]}}}
+#  CHECK:  "id": 2,

could you also add another codeAction request with one of the codes missing?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63316



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63316: [clangd] Include the diagnostics's code when comparing diagnostics

[Nathan Ridge via Phabricator via cfe-commits]

nridge updated this revision to Diff 204939.
nridge added a comment.

Revise fix and add test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63316

Files:
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/test/fixits-duplication.test

Index: clang-tools-extra/clangd/test/fixits-duplication.test
===
--- /dev/null
+++ clang-tools-extra/clangd/test/fixits-duplication.test
@@ -0,0 +1,135 @@
+# RUN: clangd -lit-test -clang-tidy-checks=modernize-use-nullptr,hicpp-use-nullptr < %s | FileCheck -strict-whitespace %s
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{"textDocument":{"codeAction":{"codeActionLiteralSupport":{,"trace":"off"}}
+---
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///foo.cpp","languageId":"cpp","version":1,"text":"void foo() { char* p = 0; }"}}}
+#  CHECK:"method": "textDocument/publishDiagnostics",
+# CHECK-NEXT:  "params": {
+# CHECK-NEXT:"diagnostics": [
+# CHECK-NEXT:  {
+# CHECK-NEXT:"code": "hicpp-use-nullptr",
+# CHECK-NEXT:"message": "Use nullptr (fix available)",
+# CHECK-NEXT:"range": {
+# CHECK-NEXT:  "end": {
+# CHECK-NEXT:"character": 24,
+# CHECK-NEXT:"line": 0
+# CHECK-NEXT:  },
+# CHECK-NEXT:  "start": {
+# CHECK-NEXT:"character": 23,
+# CHECK-NEXT:"line": 0
+# CHECK-NEXT:  }
+# CHECK-NEXT:},
+# CHECK-NEXT:"severity": 2,
+# CHECK-NEXT:"source": "clang-tidy"
+# CHECK-NEXT:  },
+# CHECK-NEXT:  {
+# CHECK-NEXT:"code": "modernize-use-nullptr",
+# CHECK-NEXT:"message": "Use nullptr (fix available)",
+# CHECK-NEXT:"range": {
+# CHECK-NEXT:  "end": {
+# CHECK-NEXT:"character": 24,
+# CHECK-NEXT:"line": 0
+# CHECK-NEXT:  },
+# CHECK-NEXT:  "start": {
+# CHECK-NEXT:"character": 23,
+# CHECK-NEXT:"line": 0
+# CHECK-NEXT:  }
+# CHECK-NEXT:},
+# CHECK-NEXT:"severity": 2,
+# CHECK-NEXT:"source": "clang-tidy"
+# CHECK-NEXT:  }
+# CHECK-NEXT:],
+# CHECK-NEXT:"uri": "file:///{{.*}}/foo.cpp"
+# CHECK-NEXT:  }
+---
+{"jsonrpc":"2.0","id":2,"method":"textDocument/codeAction","params":{"textDocument":{"uri":"test:///foo.cpp"},"range":{"start":{"line":0,"character":23},"end":{"line":0,"character":24}},"context":{"diagnostics":[{"range":{"start": {"line": 0, "character": 23}, "end": {"line": 0, "character": 24}},"severity":2,"message":"Use nullptr (fix available)", "code": "hicpp-use-nullptr", "source": "clang-tidy"},{"range":{"start": {"line": 0, "character": 23}, "end": {"line": 0, "character": 24}},"severity":2,"message":"Use nullptr (fix available)", "code": "modernize-use-nullptr", "source": "clang-tidy"}]}}}
+#  CHECK:  "id": 2,
+# CHECK-NEXT:  "jsonrpc": "2.0",
+# CHECK-NEXT:  "result": [
+# CHECK-NEXT:{
+# CHECK-NEXT:  "diagnostics": [
+# CHECK-NEXT:{
+# CHECK-NEXT:  "code": "hicpp-use-nullptr",
+# CHECK-NEXT:  "message": "Use nullptr (fix available)",
+# CHECK-NEXT:  "range": {
+# CHECK-NEXT:"end": {
+# CHECK-NEXT:  "character": 24,
+# CHECK-NEXT:  "line": 0
+# CHECK-NEXT:},
+# CHECK-NEXT:"start": {
+# CHECK-NEXT:  "character": 23,
+# CHECK-NEXT:  "line": 0
+# CHECK-NEXT:}
+# CHECK-NEXT:  },
+# CHECK-NEXT:  "severity": 2,
+# CHECK-NEXT:  "source": "clang-tidy"
+# CHECK-NEXT:}
+# CHECK-NEXT:  ],
+# CHECK-NEXT:  "edit": {
+# CHECK-NEXT:"changes": {
+# CHECK-NEXT:  "file://{{.*}}/foo.cpp": [
+# CHECK-NEXT:{
+# CHECK-NEXT:  "newText": "nullptr",
+# CHECK-NEXT:  "range": {
+# CHECK-NEXT:"end": {
+# CHECK-NEXT:  "character": 24,
+# CHECK-NEXT:  "line": 0
+# CHECK-NEXT:},
+# CHECK-NEXT:"start": {
+# CHECK-NEXT:  "character": 23,
+# CHECK-NEXT:  "line": 0
+# CHECK-NEXT:}
+# CHECK-NEXT:  }
+# CHECK-NEXT:}
+# CHECK-NEXT:  ]
+# CHECK-NEXT:}
+# CHECK-NEXT:  },
+# CHECK-NEXT:  "kind": "quickfix",
+# CHECK-NEXT:  "title": "change '0' to 'nullptr'"
+# CHECK-NEXT:},
+# CHECK-NEXT:{
+# CHECK-NEXT:  "diagnostics": [
+# CHECK-NEXT:{
+# CHECK-NEXT:  "code": "modernize-use-nullptr",
+# CHECK-NEXT:  "message": "Use nullptr (fix available)",
+# CHECK-NEXT:  "range": {
+# CHECK-NEXT:"end": {
+# CHECK-NEXT:  "character": 24,
+# CHECK-NEXT:  "line": 0
+# CHECK-NEXT:

[PATCH] D63316: [clangd] Include the diagnostics's code when comparing diagnostics

[Nathan Ridge via Phabricator via cfe-commits]

nridge marked an inline comment as done.
nridge added a comment.

The fix actually broke another lit test, because in that test the client was 
not sending the diagnostic's code back in the `codeAction` request, resulting 
in a mismatch with the new comparison function.

I think a real client could potentially could this too (not send the code 
back), so I revised the fix to address this case, by only using the code in the 
comparison if it's present in both objects.

Also added the requested new lit test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63316



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63316: [clangd] Include the diagnostics's code when comparing diagnostics

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

Could you also add a lit test for the case you mentioned in the github issue?

You can find pointers in clang-tools-extra/clangd/test/diagnostics.test and 
clang-tools-extra/clangd/test/fixits-codeaction.test




Comment at: clang-tools-extra/clangd/Protocol.h:654
   bool operator()(const Diagnostic , const Diagnostic ) const {
-return std::tie(LHS.range, LHS.message) < std::tie(RHS.range, RHS.message);
+return std::tie(LHS.code, LHS.range, LHS.message) <
+   std::tie(RHS.code, RHS.range, RHS.message);

could you rather put `code` after range and message so that we somewhat 
preserve the ordering. I am not sure if it is used anywhere but looks like we 
are using an ordered container(std::map) to store these.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63316



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63316: [clangd] Include the diagnostics's code when comparing diagnostics

[Nathan Ridge via Phabricator via cfe-commits]

nridge created this revision.
nridge added a reviewer: kadircet.
Herald added subscribers: cfe-commits, arphaman, jkorous, MaskRay, 
ilya-biryukov.
Herald added a project: clang.

This fixes https://github.com/clangd/clangd/issues/60


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D63316

Files:
  clang-tools-extra/clangd/Protocol.h


Index: clang-tools-extra/clangd/Protocol.h
===
--- clang-tools-extra/clangd/Protocol.h
+++ clang-tools-extra/clangd/Protocol.h
@@ -597,7 +597,6 @@
 };
 bool fromJSON(const llvm::json::Value &, DocumentSymbolParams &);
 
-
 /// Represents a related message and source code location for a diagnostic.
 /// This should be used to point to code locations that cause or related to a
 /// diagnostics, e.g when duplicating a symbol in a scope.
@@ -647,12 +646,13 @@
 
 /// A LSP-specific comparator used to find diagnostic in a container like
 /// std:map.
-/// We only use the required fields of Diagnostic to do the comparsion to avoid
-/// any regression issues from LSP clients (e.g. VScode), see
-/// https://git.io/vbr29
+/// We only use as many fields of Diagnostic as is needed to make distinct
+/// diagnostics unique in practice, to avoid  regression issues from LSP 
clients
+/// (e.g. VScode), see https://git.io/vbr29
 struct LSPDiagnosticCompare {
   bool operator()(const Diagnostic , const Diagnostic ) const {
-return std::tie(LHS.range, LHS.message) < std::tie(RHS.range, RHS.message);
+return std::tie(LHS.code, LHS.range, LHS.message) <
+   std::tie(RHS.code, RHS.range, RHS.message);
   }
 };
 bool fromJSON(const llvm::json::Value &, Diagnostic &);


Index: clang-tools-extra/clangd/Protocol.h
===
--- clang-tools-extra/clangd/Protocol.h
+++ clang-tools-extra/clangd/Protocol.h
@@ -597,7 +597,6 @@
 };
 bool fromJSON(const llvm::json::Value &, DocumentSymbolParams &);
 
-
 /// Represents a related message and source code location for a diagnostic.
 /// This should be used to point to code locations that cause or related to a
 /// diagnostics, e.g when duplicating a symbol in a scope.
@@ -647,12 +646,13 @@
 
 /// A LSP-specific comparator used to find diagnostic in a container like
 /// std:map.
-/// We only use the required fields of Diagnostic to do the comparsion to avoid
-/// any regression issues from LSP clients (e.g. VScode), see
-/// https://git.io/vbr29
+/// We only use as many fields of Diagnostic as is needed to make distinct
+/// diagnostics unique in practice, to avoid  regression issues from LSP clients
+/// (e.g. VScode), see https://git.io/vbr29
 struct LSPDiagnosticCompare {
   bool operator()(const Diagnostic , const Diagnostic ) const {
-return std::tie(LHS.range, LHS.message) < std::tie(RHS.range, RHS.message);
+return std::tie(LHS.code, LHS.range, LHS.message) <
+   std::tie(RHS.code, RHS.range, RHS.message);
   }
 };
 bool fromJSON(const llvm::json::Value &, Diagnostic &);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits