[PATCH] D67536: [WIP] [clangd] Add support for an inactive regions notification

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

Could you add tests for this?



================
Comment at: clang-tools-extra/clangd/Compiler.cpp:66
   CI->getLangOpts()->CommentOpts.ParseAllComments = true;
+  CI->getPreprocessorOpts().DetailedRecord = true;
   return CI;
----------------
I'm not sure how does this flag impact the size of Preamble/AST, @ilya-biryukov any thoughts?


================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:143
+    for (const SourceRange &R :
+         AST.getPreprocessor().getPreprocessingRecord()->getSkippedRanges()) {
+      if (isInsideMainFile(R.getBegin(), SM)) {
----------------
I think the current implementation only collects the skipped preprocessing ranges **after preamble** region in the main file.  We probably need to store these ranges in PreambleData to make the ranges in the preamble region work, no need to do it in this patch, but we'd better have a test reflecting this fact. 

```
#ifdef ActiveCOde
// inactive code here
#endif

#include "foo.h"
// preamble ends here

namespace ns {
// ..
}  
```


================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:152
+          // Don't bother computing the offset for the end of the line, just use
+          // zero. The client will treat this highlighting kind specially, and
+          // highlight the entire line visually (i.e. not just to where the text
----------------
This seems too couple with VSCode client, I would prefer to calculate the range of the line and return to the client.

Is there any big differences in VSCode between highlighting with the `isWholeLine` and highlighting with the range of the line? 


================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.h:43
   Primitive,
+  InactivePreprocessorBranch,
   Macro,
----------------
This is a different kind group, I would put it after the Macro,  we'd need to update the LastKind. 

The name seems too specific, how about "UnreachableCode"?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67536