[clang] a2772fc - [C++20] [Modules] Disable preferred_name when writing a C++20 Module interface

Chuanqi Xu via cfe-commits cfe-commits at lists.llvm.org
Tue Jul 26 08:59:07 PDT 2022


Author: Chuanqi Xu
Date: 2022-07-26T23:58:07+08:00
New Revision: a2772fc806af7db5d58c7e3d604270a92fff79de

URL: https://github.com/llvm/llvm-project/commit/a2772fc806af7db5d58c7e3d604270a92fff79de
DIFF: https://github.com/llvm/llvm-project/commit/a2772fc806af7db5d58c7e3d604270a92fff79de.diff

LOG: [C++20] [Modules] Disable preferred_name when writing a C++20 Module interface

Currently, the use of preferred_name would block implementing std
modules in libcxx. See https://github.com/llvm/llvm-project/issues/56490
for example.
The problem is pretty hard and it looks like we couldn't solve it in a
short time. So we sent this patch as a workaround to avoid blocking us
to modularize STL. This is intended to be fixed properly in the future.

Reviewed By: erichkeane, aaron.ballman, tahonermann

Differential Revision: https://reviews.llvm.org/D130331

Added: 
    clang/test/Modules/preferred_name.cppm

Modified: 
    clang/docs/ReleaseNotes.rst
    clang/include/clang/Basic/AttrDocs.td
    clang/include/clang/Serialization/ASTWriter.h
    clang/lib/Serialization/ASTReaderDecl.cpp
    clang/lib/Serialization/ASTWriter.cpp

Removed: 
    


################################################################################
diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 39d776332f286..8b44e90949ec9 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -435,6 +435,11 @@ Attribute Changes in Clang
   ``__attribute__((function_return("keep")))`` was added. This is intended to
   be used by the Linux kernel to mitigate RETBLEED.
 
+- Ignore the `__preferred_name__` attribute when writing for C++20 module interfaces.
+  This is a short-term workaround intentionally since clang doesn't take care of the
+  serialization and deserialization of `__preferred_name__`.  See
+  https://github.com/llvm/llvm-project/issues/56490 for example.
+
 Windows Support
 ---------------
 

diff  --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td
index aff0dbbdd94d3..5c84e2fc5b77d 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -5040,6 +5040,12 @@ general this requires the template to be declared at least twice. For example:
                                 clang::preferred_name(wstring)]] basic_string {
     // ...
   };
+
+
+Note that the ``preferred_name`` attribute will be ignored when the compiler
+writes a C++20 Module interface now. This is due to a compiler issue
+(https://github.com/llvm/llvm-project/issues/56490) that blocks users to modularize
+declarations with `preferred_name`. This is intended to be fixed in the future.
   }];
 }
 

diff  --git a/clang/include/clang/Serialization/ASTWriter.h b/clang/include/clang/Serialization/ASTWriter.h
index 6a3532d7272d8..83bc7dcdfde3b 100644
--- a/clang/include/clang/Serialization/ASTWriter.h
+++ b/clang/include/clang/Serialization/ASTWriter.h
@@ -703,6 +703,10 @@ class ASTWriter : public ASTDeserializationListener,
   bool hasChain() const { return Chain; }
   ASTReader *getChain() const { return Chain; }
 
+  bool isWritingNamedModules() const {
+    return WritingModule && WritingModule->isModulePurview();
+  }
+
 private:
   // ASTDeserializationListener implementation
   void ReaderInitialized(ASTReader *Reader) override;

diff  --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp
index d70e824224dfe..73800191dfc1b 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -2926,7 +2926,8 @@ Attr *ASTRecordReader::readAttr() {
 /// Reads attributes from the current stream position.
 void ASTRecordReader::readAttributes(AttrVec &Attrs) {
   for (unsigned I = 0, E = readInt(); I != E; ++I)
-    Attrs.push_back(readAttr());
+    if (auto *A = readAttr())
+      Attrs.push_back(A);
 }
 
 //===----------------------------------------------------------------------===//

diff  --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index fac8fc141d2c7..0739dcc1ce600 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -4347,8 +4347,12 @@ void ASTWriter::WriteModuleFileExtension(Sema &SemaRef,
 
 void ASTRecordWriter::AddAttr(const Attr *A) {
   auto &Record = *this;
-  if (!A)
+  // FIXME: Clang can't handle the serialization/deserialization of
+  // preferred_name properly now. See
+  // https://github.com/llvm/llvm-project/issues/56490 for example.
+  if (!A || (isa<PreferredNameAttr>(A) && Writer->isWritingNamedModules()))
     return Record.push_back(0);
+
   Record.push_back(A->getKind() + 1); // FIXME: stable encoding, target attrs
 
   Record.AddIdentifierRef(A->getAttrName());

diff  --git a/clang/test/Modules/preferred_name.cppm b/clang/test/Modules/preferred_name.cppm
new file mode 100644
index 0000000000000..46ad96cb1abc3
--- /dev/null
+++ b/clang/test/Modules/preferred_name.cppm
@@ -0,0 +1,50 @@
+// Tests that the ODR check wouldn't produce false-positive result for preferred_name attribute.
+//
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -std=c++20 %t/A.cppm -emit-module-interface -o %t/A.pcm
+// RUN: %clang_cc1 -std=c++20 -fprebuilt-module-path=%t -I%t %t/Use.cppm -verify -fsyntax-only
+// RUN: %clang_cc1 -std=c++20 -fprebuilt-module-path=%t -I%t %t/Use1.cpp -verify -fsyntax-only
+// RUN: %clang_cc1 -std=c++20 -fprebuilt-module-path=%t -I%t %t/Use2.cpp -verify -fsyntax-only
+//
+//--- foo.h
+template<class _CharT>
+class foo_templ;
+
+typedef foo_templ<char> foo;
+
+template<class _CharT>
+class
+__attribute__((__preferred_name__(foo)))
+foo_templ {
+public:
+    foo_templ() {}
+};
+
+inline foo_templ<char> bar()
+{
+  return foo_templ<char>();
+}
+
+//--- A.cppm
+module;
+#include "foo.h"
+export module A;
+
+//--- Use.cppm
+// expected-no-diagnostics
+module;
+#include "foo.h"
+export module Use;
+import A;
+
+//--- Use1.cpp
+import A;         // expected-warning at foo.h:8 {{attribute declaration must precede definition}}
+#include "foo.h"  // expected-note at foo.h:9 {{previous definition is here}}
+
+//--- Use2.cpp
+// expected-no-diagnostics
+#include "foo.h"
+import A;


        


More information about the cfe-commits mailing list