kuganv created this revision.
Herald added subscribers: dexonsmith, usaxena95, kadircet, arphaman.
Herald added a project: All.
kuganv requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added projects: clang, clang-tools-extra.
Fixes following incompatibility issues with c++ modules
(clangd with c++ modules results in assertion failure and pch_langopt_mismatch
#1105)
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.
2. If we have modules that are built with RetainCommentsFromSystemHeaders
difference, that would 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/D124432
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
@@ -3077,6 +3077,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,5 @@
+[{
+ "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
