[llvm-branch-commits] [clang] 70195e0 - [C++20] [Modules] Remove previous workaround for odr hashing enums

Tom Stellard via llvm-branch-commits llvm-branch-commits at lists.llvm.org
Tue Feb 6 21:31:45 PST 2024


Author: Chuanqi Xu
Date: 2024-02-06T21:30:14-08:00
New Revision: 70195e0d67080c2d0d1f321db69ebb40042a1c7a

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

LOG: [C++20] [Modules] Remove previous workaround for odr hashing enums

Previosly we land
https://github.com/llvm/llvm-project/commit/085eae6b863881fb9fda323e5b672b04a00ed19e
to workaround the false positive ODR violations in
https://github.com/llvm/llvm-project/issues/76638.

However, we decided to not perform ODR checks for decls from GMF in
https://github.com/llvm/llvm-project/issues/79240 and we land the
corresponding change. So we should be able to remove the workaround now.

The original tests get remained.

Added: 
    clang/test/Modules/cxx20-modules-enum-odr.cppm

Modified: 
    clang/docs/ReleaseNotes.rst
    clang/lib/AST/ODRHash.cpp

Removed: 
    


################################################################################
diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index ce94b3e44b955..d335be78a499b 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -1041,9 +1041,6 @@ Bug Fixes to C++ Support
   in 
diff erent visibility.
   Fixes (`#67893 <https://github.com/llvm/llvm-project/issues/67893>`_)
 
-- Fix a false-positive ODR violation for 
diff erent definitions for `std::align_val_t`.
-  Fixes (`#76638 <https://github.com/llvm/llvm-project/issues/76638>`_)
-
 - Remove recorded `#pragma once` state for headers included in named modules.
   Fixes (`#77995 <https://github.com/llvm/llvm-project/issues/77995>`_)
 

diff  --git a/clang/lib/AST/ODRHash.cpp b/clang/lib/AST/ODRHash.cpp
index 5b98646a1e8dc..2dbc259138a89 100644
--- a/clang/lib/AST/ODRHash.cpp
+++ b/clang/lib/AST/ODRHash.cpp
@@ -745,55 +745,8 @@ void ODRHash::AddEnumDecl(const EnumDecl *Enum) {
   if (Enum->isScoped())
     AddBoolean(Enum->isScopedUsingClassTag());
 
-  if (Enum->getIntegerTypeSourceInfo()) {
-    // FIMXE: This allows two enums with 
diff erent spellings to have the same
-    // hash.
-    //
-    //  // mod1.cppm
-    //  module;
-    //  extern "C" {
-    //      typedef unsigned __int64 size_t;
-    //  }
-    //  namespace std {
-    //      using :: size_t;
-    //  }
-    //
-    //  extern "C++" {
-    //      namespace std {
-    //          enum class align_val_t : std::size_t {};
-    //      }
-    //  }
-    //
-    //  export module mod1;
-    //  export using std::align_val_t;
-    //
-    //  // mod2.cppm
-    //  module;
-    //  extern "C" {
-    //      typedef unsigned __int64 size_t;
-    //  }
-    //
-    //  extern "C++" {
-    //      namespace std {
-    //          enum class align_val_t : size_t {};
-    //      }
-    //  }
-    //
-    //  export module mod2;
-    //  import mod1;
-    //  export using std::align_val_t;
-    //
-    // The above example should be disallowed since it violates
-    // [basic.def.odr]p14:
-    //
-    //    Each such definition shall consist of the same sequence of tokens
-    //
-    // The definitions of `std::align_val_t` in two module units have 
diff erent
-    // spellings but we failed to give an error here.
-    //
-    // See https://github.com/llvm/llvm-project/issues/76638 for details.
+  if (Enum->getIntegerTypeSourceInfo())
     AddQualType(Enum->getIntegerType().getCanonicalType());
-  }
 
   // Filter out sub-Decls which will not be processed in order to get an
   // accurate count of Decl's.

diff  --git a/clang/test/Modules/cxx20-modules-enum-odr.cppm b/clang/test/Modules/cxx20-modules-enum-odr.cppm
new file mode 100644
index 0000000000000..831c01143a27b
--- /dev/null
+++ b/clang/test/Modules/cxx20-modules-enum-odr.cppm
@@ -0,0 +1,51 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -std=c++20 %t/mod1.cppm -emit-module-interface -o %t/mod1.pcm
+// RUN: %clang_cc1 -std=c++20 %t/mod2.cppm -emit-module-interface -o %t/mod2.pcm
+// RUN: %clang_cc1 -std=c++20 %t/test.cpp -fprebuilt-module-path=%t -verify -fsyntax-only
+
+//--- size_t.h
+
+extern "C" {
+    typedef unsigned int size_t;
+}
+
+//--- csize_t
+namespace std {
+            using :: size_t;
+}
+
+//--- align.h
+namespace std {
+    enum class align_val_t : size_t {};
+}
+
+//--- mod1.cppm
+module;
+#include "size_t.h"
+#include "align.h"
+export module mod1;
+namespace std {
+export using std::align_val_t;
+}
+
+//--- mod2.cppm
+module;
+#include "size_t.h"
+#include "csize_t"
+#include "align.h"
+export module mod2;
+namespace std {
+export using std::align_val_t;
+}
+
+//--- test.cpp
+// expected-no-diagnostics
+import mod1;
+import mod2;
+void test() {
+    std::align_val_t v;
+}
+


        


More information about the llvm-branch-commits mailing list