kuganv created this revision.
Herald added subscribers: usaxena95, kadircet, arphaman.
Herald added a project: All.
kuganv requested review of this revision.
Herald added subscribers: cfe-commits, ilya-biryukov.
Herald added projects: clang, clang-tools-extra.

1.

Preamble generated by clangd is generated with WriteCommentListToPCH = false;
However, if we are using precompiled modules that have comments in the
serialised AST, following sanity check in the clangd fails.

// Sanity check that the comment does not come from the PCH. We choose to not
// write them into PCH, because they are racy and slow to load.
assert(!Ctx.getSourceManager().isLoadedSourceLocation(RC->getBeginLoc()));

In this patch when we are generating Preamble with
WriteCommentListToPCH, we do not load the comments from AST.

1.

If we have modules that are built with RetainCommentsFromSystemHeaders
difference, that wouldn prevent modules from getting loaded. Since this
difference is not critical that should be stopping modules from being loaded,
this patch changes it to be a COMPATIBLE_LANGOPT.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D124909

Files:
  clang-tools-extra/clangd/test/modules-options-compatablity-test/A.h
  clang-tools-extra/clangd/test/modules-options-compatablity-test/B.h
  
clang-tools-extra/clangd/test/modules-options-compatablity-test/compile_commands.json
  
clang-tools-extra/clangd/test/modules-options-compatablity-test/definition.jsonrpc
  
clang-tools-extra/clangd/test/modules-options-compatablity-test/module.modulemap
  clang-tools-extra/clangd/test/modules-options-compatablity-test/use.c
  clang-tools-extra/clangd/test/modules-options-compatablity.test
  clang/include/clang/Basic/LangOptions.def
  clang/lib/Serialization/ASTReader.cpp

Index: clang/lib/Serialization/ASTReader.cpp
===================================================================
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -3079,6 +3079,10 @@
           return Err;
         if (llvm::Error Err = ReadBlockAbbrevs(C, COMMENTS_BLOCK_ID))
           return Err;
+        if (PP.getPreprocessorOpts().GeneratePreamble &&
+            !PP.getPreprocessorOpts().WriteCommentListToPCH) {
+          break;
+        }
         CommentsCursors.push_back(std::make_pair(C, &F));
         break;
       }
Index: clang/include/clang/Basic/LangOptions.def
===================================================================
--- clang/include/clang/Basic/LangOptions.def
+++ clang/include/clang/Basic/LangOptions.def
@@ -387,7 +387,7 @@
 
 LANGOPT(XLPragmaPack, 1, 0, "IBM XL #pragma pack handling")
 
-LANGOPT(RetainCommentsFromSystemHeaders, 1, 0, "retain documentation comments from system headers in the AST")
+COMPATIBLE_LANGOPT(RetainCommentsFromSystemHeaders, 1, 0, "retain documentation comments from system headers in the AST")
 
 LANGOPT(SanitizeAddressFieldPadding, 2, 0, "controls how aggressive is ASan "
                                            "field padding (0: none, 1:least "
Index: clang-tools-extra/clangd/test/modules-options-compatablity.test
===================================================================
--- /dev/null
+++ clang-tools-extra/clangd/test/modules-options-compatablity.test
@@ -0,0 +1,10 @@
+
+# RUN: rm -rf %t && mkdir -p %t
+# RUN: cp -r %S/modules-options-compatablity-test/* %t
+# RUN: mkdir -p %t/prebuilt
+# RUN: sed -i -e "s|DIRECTORY|%t|g" %t/definition.jsonrpc
+# RUN: sed -i -e "s|DIRECTORY|%t|g" %t/compile_commands.json
+# RUN: %clang -cc1 -emit-module -o %t/prebuilt/A.pcm -fmodules %t/module.modulemap -fmodule-name=A
+# RUN: %clang -cc1 -fretain-comments-from-system-headers -emit-module -o %t/prebuilt/B.pcm -fmodules %t/module.modulemap -fmodule-name=B -fprebuilt-module-path=%t/prebuilt
+# RUN: rm %t/*.h
+# RUN: clangd -background-index -lit-test < %t/definition.jsonrpc
Index: clang-tools-extra/clangd/test/modules-options-compatablity-test/use.c
===================================================================
--- /dev/null
+++ clang-tools-extra/clangd/test/modules-options-compatablity-test/use.c
@@ -0,0 +1,5 @@
+/* use */
+#include <A.h>
+#include <B.h>
+
+void use() { a(); }
Index: clang-tools-extra/clangd/test/modules-options-compatablity-test/module.modulemap
===================================================================
--- /dev/null
+++ clang-tools-extra/clangd/test/modules-options-compatablity-test/module.modulemap
@@ -0,0 +1,8 @@
+/* module.modulemap */
+module A {
+  header "A.h"
+}
+module B {
+  header "B.h"
+  export *
+}
Index: clang-tools-extra/clangd/test/modules-options-compatablity-test/definition.jsonrpc
===================================================================
--- /dev/null
+++ clang-tools-extra/clangd/test/modules-options-compatablity-test/definition.jsonrpc
@@ -0,0 +1,28 @@
+{
+  "jsonrpc": "2.0",
+    "id": 0,
+    "method": "initialize",
+    "params": {
+      "processId": 123,
+      "rootPath": "clangd",
+      "capabilities": { "window": { "workDoneProgress": true, "implicitWorkDoneProgressCreate": true} },
+      "trace": "off"
+    }
+}
+---
+{
+  "jsonrpc": "2.0",
+    "method": "textDocument/didOpen",
+    "params": {
+      "textDocument": {
+        "uri": "file://DIRECTORY/use.c",
+        "languageId": "cpp",
+        "version": 1,
+        "text": "#include \"B.h\"\nvoid use(){\na();\n}"
+      }
+    }
+}
+---
+{"jsonrpc":"2.0","id":3,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}
Index: clang-tools-extra/clangd/test/modules-options-compatablity-test/compile_commands.json
===================================================================
--- /dev/null
+++ clang-tools-extra/clangd/test/modules-options-compatablity-test/compile_commands.json
@@ -0,0 +1,8 @@
+[{
+  "directory" : ".",
+  "command" : "clang -emit-obj DIRECTORY/use.c -fno-modules-global -fmodules "
+              "-fmodules-cache-path=/DOES/NOT/EXIST  -fno-builtin-module-map "
+              "-fno-implicit-modules  -fimplicit-module-maps "
+              "-fprebuilt-module-path=DIRECTORY/prebuilt",
+  "file" : "DIRECTORY/use.c"
+}]
Index: clang-tools-extra/clangd/test/modules-options-compatablity-test/B.h
===================================================================
--- /dev/null
+++ clang-tools-extra/clangd/test/modules-options-compatablity-test/B.h
@@ -0,0 +1,3 @@
+// Comment for B
+// ORDINARY COMMENT
+#include "A.h"
Index: clang-tools-extra/clangd/test/modules-options-compatablity-test/A.h
===================================================================
--- /dev/null
+++ clang-tools-extra/clangd/test/modules-options-compatablity-test/A.h
@@ -0,0 +1,2 @@
+// Comment for void a()
+void a() {}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to