[libcxx] r304891 - Implement LWG 2904.

[Michael Park via cfe-commits]

Author: mpark
Date: Wed Jun  7 05:22:43 2017
New Revision: 304891

URL: http://llvm.org/viewvc/llvm-project?rev=304891&view=rev
Log:
Implement LWG 2904.

Summary:
- Removed the move-constructibe requirement from copy-assignable.
- Updated `__assign_alt` such that we direct initialize if
  `_Tp` can be `nothrow`-constructible from `_Arg`, or `_Tp`'s
  move construction can throw. Otherwise, construct a temporary and move it.
- Updated the tests to remove the pre-LWG2904 path.

Depends on D32671.

Reviewers: EricWF, CaseyCarter

Reviewed By: EricWF

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

Modified:
libcxx/trunk/include/variant

libcxx/trunk/test/std/utilities/variant/variant.variant/variant.assign/T.pass.cpp

libcxx/trunk/test/std/utilities/variant/variant.variant/variant.assign/copy.pass.cpp

libcxx/trunk/test/std/utilities/variant/variant.variant/variant.assign/move.pass.cpp

Modified: libcxx/trunk/include/variant
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/variant?rev=304891&r1=304890&r2=304891&view=diff
==
--- libcxx/trunk/include/variant (original)
+++ libcxx/trunk/include/variant Wed Jun  7 05:22:43 2017
@@ -358,7 +358,6 @@ struct __traits {
 
   static constexpr _Trait __copy_assignable_trait = __common_trait(
   {__copy_constructible_trait,
-   __move_constructible_trait,
__trait<_Types, is_trivially_copy_assignable, is_copy_assignable>...});
 
   static constexpr _Trait __move_assignable_trait = __common_trait(
@@ -877,25 +876,24 @@ public:
   }
 
 protected:
-  template 
+  template 
   inline _LIBCPP_INLINE_VISIBILITY
-  void __assign_alt(__alt<_Ip, _Tp>& __a,
-_Arg&& __arg,
-bool_constant<_CopyAssign> __tag) {
+  void __assign_alt(__alt<_Ip, _Tp>& __a, _Arg&& __arg) {
 if (this->index() == _Ip) {
   __a.__value = _VSTD::forward<_Arg>(__arg);
 } else {
   struct {
 void operator()(true_type) const {
-  __this->__emplace<_Ip>(_Tp(_VSTD::forward<_Arg>(__arg)));
+  __this->__emplace<_Ip>(_VSTD::forward<_Arg>(__arg));
 }
 void operator()(false_type) const {
-  __this->__emplace<_Ip>(_VSTD::forward<_Arg>(__arg));
+  __this->__emplace<_Ip>(_Tp(_VSTD::forward<_Arg>(__arg)));
 }
 __assignment* __this;
 _Arg&& __arg;
   } __impl{this, _VSTD::forward<_Arg>(__arg)};
-  __impl(__tag);
+  __impl(bool_constant ||
+   !is_nothrow_move_constructible_v<_Tp>>{});
 }
   }
 
@@ -912,8 +910,7 @@ protected:
   [this](auto& __this_alt, auto&& __that_alt) {
 this->__assign_alt(
 __this_alt,
-_VSTD::forward(__that_alt).__value,
-is_lvalue_reference<_That>{});
+_VSTD::forward(__that_alt).__value);
   },
   *this, _VSTD::forward<_That>(__that));
 }
@@ -1013,8 +1010,7 @@ public:
   inline _LIBCPP_INLINE_VISIBILITY
   void __assign(_Arg&& __arg) {
 this->__assign_alt(__access::__base::__get_alt<_Ip>(*this),
-   _VSTD::forward<_Arg>(__arg),
-   false_type{});
+   _VSTD::forward<_Arg>(__arg));
   }
 
   inline _LIBCPP_INLINE_VISIBILITY
@@ -1088,7 +1084,6 @@ class _LIBCPP_TEMPLATE_VIS variant
   __all...>::value>,
   private __sfinae_assign_base<
   __all<(is_copy_constructible_v<_Types> &&
- is_move_constructible_v<_Types> &&
  is_copy_assignable_v<_Types>)...>::value,
   __all<(is_move_constructible_v<_Types> &&
  is_move_assignable_v<_Types>)...>::value> {

Modified: 
libcxx/trunk/test/std/utilities/variant/variant.variant/variant.assign/T.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/utilities/variant/variant.variant/variant.assign/T.pass.cpp?rev=304891&r1=304890&r2=304891&view=diff
==
--- 
libcxx/trunk/test/std/utilities/variant/variant.variant/variant.assign/T.pass.cpp
 (original)
+++ 
libcxx/trunk/test/std/utilities/variant/variant.variant/variant.assign/T.pass.cpp
 Wed Jun  7 05:22:43 2017
@@ -199,12 +199,8 @@ void test_T_assignment_performs_construc
   assert(false);
 } catch (...) { /* ... */
 }
-#ifdef _LIBCPP_VERSION // LWG2904
-assert(v.valueless_by_exception());
-#else // _LIBCPP_VERSION
 assert(v.index() == 0);
 assert(std::get<0>(v) == "hello");
-#endif // _LIBCPP_VERSION
   }
   {
 using V = std::variant;
@@ -213,28 +209,6 @@ void test_T_assignment_performs_construc
 assert(v.index() == 0);
 assert(std::get<0>(v).value == 42);
   }
-#ifdef _LIBCPP_VERSION // LWG2904
-  {
-// Test that nothrow direct construction is preferred to nothrow move.
-using V = std::variant;
-V v(std::in_place_type, "hello");
-v = 42

[libcxx] r304893 - Mark LWG 2904 as complete.

[Michael Park via cfe-commits]

Author: mpark
Date: Wed Jun  7 05:27:17 2017
New Revision: 304893

URL: http://llvm.org/viewvc/llvm-project?rev=304893&view=rev
Log:
Mark LWG 2904 as complete.

Modified:
libcxx/trunk/www/cxx1z_status.html

Modified: libcxx/trunk/www/cxx1z_status.html
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/www/cxx1z_status.html?rev=304893&r1=304892&r2=304893&view=diff
==
--- libcxx/trunk/www/cxx1z_status.html (original)
+++ libcxx/trunk/www/cxx1z_status.html Wed Jun  7 05:27:17 2017
@@ -46,7 +46,7 @@
 SG1 - Study group #1 (Concurrency working group)
   
   
-  
+
   Paper Status
   
Paper #GroupPaper 
NameMeetingStatusFirst released version
@@ -477,7 +477,7 @@
http://wg21.link/LWG2890";>2890The 
definition of 'object state' applies only to class 
typesKonaComplete
http://wg21.link/LWG2900";>2900The copy 
and move constructors of optional are not 
constexprKonaComplete
http://wg21.link/LWG2903";>2903The form of 
initialization for the emplace-constructors is not 
specifiedKona
-   http://wg21.link/LWG2904";>2904Make 
variant move-assignment more exception safeKona
+   http://wg21.link/LWG2904";>2904Make 
variant move-assignment more exception 
safeKonaComplete
http://wg21.link/LWG2905";>2905is_constructible_v, P, D const &> should be false when D is not copy 
constructibleKonaComplete
http://wg21.link/LWG2908";>2908The 
less-than operator for shared pointers could do 
moreKona
http://wg21.link/LWG2911";>2911An 
is_aggregate type trait is neededKonaComplete


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[libcxx] r305370 - Add an `__is_inplace_index` metafunction.

[Michael Park via cfe-commits]

Author: mpark
Date: Wed Jun 14 00:51:18 2017
New Revision: 305370

URL: http://llvm.org/viewvc/llvm-project?rev=305370&view=rev
Log:
Add an `__is_inplace_index` metafunction.

Summary: This is used to constrain `variant`'s converting constructor correctly.

Reviewers: EricWF, mclow.lists

Reviewed By: EricWF, mclow.lists

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

Added:
libcxx/trunk/test/libcxx/utilities/utility/__is_inplace_index.pass.cpp
libcxx/trunk/test/libcxx/utilities/utility/__is_inplace_type.pass.cpp
Modified:
libcxx/trunk/include/utility

Modified: libcxx/trunk/include/utility
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/utility?rev=305370&r1=305369&r2=305370&view=diff
==
--- libcxx/trunk/include/utility (original)
+++ libcxx/trunk/include/utility Wed Jun 14 00:51:18 2017
@@ -930,6 +930,12 @@ template  struct __is_inplace
 template 
 using __is_inplace_type = __is_inplace_type_imp<__uncvref_t<_Tp>>;
 
+template  struct __is_inplace_index_imp : false_type {};
+template  struct __is_inplace_index_imp> : 
true_type {};
+
+template 
+using __is_inplace_index = __is_inplace_index_imp<__uncvref_t<_Tp>>;
+
 #endif // _LIBCPP_STD_VER > 14
 
 template 

Added: libcxx/trunk/test/libcxx/utilities/utility/__is_inplace_index.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/libcxx/utilities/utility/__is_inplace_index.pass.cpp?rev=305370&view=auto
==
--- libcxx/trunk/test/libcxx/utilities/utility/__is_inplace_index.pass.cpp 
(added)
+++ libcxx/trunk/test/libcxx/utilities/utility/__is_inplace_index.pass.cpp Wed 
Jun 14 00:51:18 2017
@@ -0,0 +1,32 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// template  using __is_inplace_index
+
+#include 
+
+struct S {};
+
+int main() {
+  using I = std::in_place_index_t<0>;
+  static_assert( std::__is_inplace_index::value, "");
+  static_assert( std::__is_inplace_index::value, "");
+  static_assert( std::__is_inplace_index::value, "");
+  static_assert( std::__is_inplace_index::value, "");
+  static_assert( std::__is_inplace_index::value, "");
+  static_assert( std::__is_inplace_index::value, "");
+  static_assert( std::__is_inplace_index::value, "");
+  static_assert( std::__is_inplace_index::value, "");
+  static_assert( std::__is_inplace_index::value, "");
+  static_assert(!std::__is_inplace_index>::value, 
"");
+  static_assert(!std::__is_inplace_index::value, "");
+  static_assert(!std::__is_inplace_index::value, "");
+  static_assert(!std::__is_inplace_index::value, "");
+  static_assert(!std::__is_inplace_index::value, "");
+}

Added: libcxx/trunk/test/libcxx/utilities/utility/__is_inplace_type.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/libcxx/utilities/utility/__is_inplace_type.pass.cpp?rev=305370&view=auto
==
--- libcxx/trunk/test/libcxx/utilities/utility/__is_inplace_type.pass.cpp 
(added)
+++ libcxx/trunk/test/libcxx/utilities/utility/__is_inplace_type.pass.cpp Wed 
Jun 14 00:51:18 2017
@@ -0,0 +1,32 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// template  using __is_inplace_type
+
+#include 
+
+struct S {};
+
+int main() {
+  using T = std::in_place_type_t;
+  static_assert( std::__is_inplace_type::value, "");
+  static_assert( std::__is_inplace_type::value, "");
+  static_assert( std::__is_inplace_type::value, "");
+  static_assert( std::__is_inplace_type::value, "");
+  static_assert( std::__is_inplace_type::value, "");
+  static_assert( std::__is_inplace_type::value, "");
+  static_assert( std::__is_inplace_type::value, "");
+  static_assert( std::__is_inplace_type::value, "");
+  static_assert( std::__is_inplace_type::value, "");
+  static_assert(!std::__is_inplace_type>::value, "");
+  static_assert(!std::__is_inplace_type::value, "");
+  static_assert(!std::__is_inplace_type::value, "");
+  static_assert(!std::__is_inplace_type::value, "");
+  static_assert(!std::__is_inplace_type::value, "");
+}


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[libcxx] r305373 - Mark `__is_inplace_*` tests as UNSUPPORTED in <= C++14.

[Michael Park via cfe-commits]

Author: mpark
Date: Wed Jun 14 02:12:55 2017
New Revision: 305373

URL: http://llvm.org/viewvc/llvm-project?rev=305373&view=rev
Log:
Mark `__is_inplace_*` tests as UNSUPPORTED in <= C++14.

Modified:
libcxx/trunk/test/libcxx/utilities/utility/__is_inplace_index.pass.cpp
libcxx/trunk/test/libcxx/utilities/utility/__is_inplace_type.pass.cpp

Modified: libcxx/trunk/test/libcxx/utilities/utility/__is_inplace_index.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/libcxx/utilities/utility/__is_inplace_index.pass.cpp?rev=305373&r1=305372&r2=305373&view=diff
==
--- libcxx/trunk/test/libcxx/utilities/utility/__is_inplace_index.pass.cpp 
(original)
+++ libcxx/trunk/test/libcxx/utilities/utility/__is_inplace_index.pass.cpp Wed 
Jun 14 02:12:55 2017
@@ -7,6 +7,8 @@
 //
 
//===--===//
 
+// UNSUPPORTED: c++98, c++03, c++11, c++14
+
 // template  using __is_inplace_index
 
 #include 

Modified: libcxx/trunk/test/libcxx/utilities/utility/__is_inplace_type.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/libcxx/utilities/utility/__is_inplace_type.pass.cpp?rev=305373&r1=305372&r2=305373&view=diff
==
--- libcxx/trunk/test/libcxx/utilities/utility/__is_inplace_type.pass.cpp 
(original)
+++ libcxx/trunk/test/libcxx/utilities/utility/__is_inplace_type.pass.cpp Wed 
Jun 14 02:12:55 2017
@@ -7,6 +7,8 @@
 //
 
//===--===//
 
+// UNSUPPORTED: c++98, c++03, c++11, c++14
+
 // template  using __is_inplace_type
 
 #include 


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[libcxx] r305668 - Add a missing SFINAE condition to the `variant`'s converting constructor.

[Michael Park via cfe-commits]

Author: mpark
Date: Mon Jun 19 03:25:57 2017
New Revision: 305668

URL: http://llvm.org/viewvc/llvm-project?rev=305668&view=rev
Log:
Add a missing SFINAE condition to the `variant`'s converting constructor.

Remarks: This function shall not participate in overload resolution unless
 `is_same_v, variant>` is false, unless `decay_t` is
 neither a specialization of `in_place_type_t` nor a specialization of
 `in_place_index_t`, unless `is_constructible_v` is true, and
 unless the expression `FUN(std::forward(t))` (with `FUN` being the
 above-mentioned set of imaginary functions) is well formed.

Depends on D34111.

Reviewers: EricWF, K-ballo

Reviewed By: EricWF

Subscribers: fhahn

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

Modified:
libcxx/trunk/include/variant

libcxx/trunk/test/std/utilities/variant/variant.variant/variant.ctor/T.pass.cpp

Modified: libcxx/trunk/include/variant
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/variant?rev=305668&r1=305667&r2=305668&view=diff
==
--- libcxx/trunk/include/variant (original)
+++ libcxx/trunk/include/variant Mon Jun 19 03:25:57 2017
@@ -1116,6 +1116,8 @@ public:
   template <
   class _Arg,
   enable_if_t, variant>, int> = 0,
+  enable_if_t>::value, int> = 0,
+  enable_if_t>::value, int> = 0,
   class _Tp = __variant_detail::__best_match_t<_Arg, _Types...>,
   size_t _Ip =
   __find_detail::__find_unambiguous_index_sfinae<_Tp, 
_Types...>::value,

Modified: 
libcxx/trunk/test/std/utilities/variant/variant.variant/variant.ctor/T.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/utilities/variant/variant.variant/variant.ctor/T.pass.cpp?rev=305668&r1=305667&r2=305668&view=diff
==
--- 
libcxx/trunk/test/std/utilities/variant/variant.variant/variant.ctor/T.pass.cpp 
(original)
+++ 
libcxx/trunk/test/std/utilities/variant/variant.variant/variant.ctor/T.pass.cpp 
Mon Jun 19 03:25:57 2017
@@ -37,6 +37,9 @@ struct NoThrowT {
   NoThrowT(int) noexcept(true) {}
 };
 
+struct AnyConstructible { template  AnyConstructible(T&&) {} };
+struct NoConstructible { NoConstructible() = delete; };
+
 void test_T_ctor_noexcept() {
   {
 using V = std::variant;
@@ -62,6 +65,17 @@ void test_T_ctor_sfinae() {
 static_assert(!std::is_constructible::value,
   "no matching constructor");
   }
+  {
+using V = std::variant;
+static_assert(
+!std::is_constructible>::value,
+"no matching constructor");
+static_assert(!std::is_constructible>::value,
+  "no matching constructor");
+  }
+
+
+
 #if !defined(TEST_VARIANT_HAS_NO_REFERENCES)
   {
 using V = std::variant;


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[libcxx] r291344 - Added "Michael Park" to `CREDITS.TXT`.

[Michael Park via cfe-commits]

Author: mpark
Date: Sat Jan  7 04:19:24 2017
New Revision: 291344

URL: http://llvm.org/viewvc/llvm-project?rev=291344&view=rev
Log:
Added "Michael Park" to `CREDITS.TXT`.

Summary: Test commit + give myself credit.

Reviewers: EricWF

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

Modified:
libcxx/trunk/CREDITS.TXT

Modified: libcxx/trunk/CREDITS.TXT
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/CREDITS.TXT?rev=291344&r1=291343&r2=291344&view=diff
==
--- libcxx/trunk/CREDITS.TXT (original)
+++ libcxx/trunk/CREDITS.TXT Sat Jan  7 04:19:24 2017
@@ -80,6 +80,10 @@ N: Andrew Morrow
 E: andrew.c.mor...@gmail.com
 D: Minor patches and Linux fixes.
 
+N: Michael Park
+E: mp...@apache.org
+D: Implementation of .
+
 N: Arvid Picciani
 E: aep at exys dot org
 D: Minor patches and musl port.


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[libcxx] r298581 - Worked around GCC bug 56480. Explicit specialization in a different namespace.

[Michael Park via cfe-commits]

Author: mpark
Date: Thu Mar 23 01:21:24 2017
New Revision: 298581

URL: http://llvm.org/viewvc/llvm-project?rev=298581&view=rev
Log:
Worked around GCC bug 56480. Explicit specialization in a different namespace.

Summary: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56480

Reviewers: EricWF

Reviewed By: EricWF

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

Modified:
libcxx/trunk/test/std/utilities/optional/optional.hash/hash.pass.cpp
libcxx/trunk/test/std/utilities/variant/variant.hash/hash.pass.cpp

Modified: libcxx/trunk/test/std/utilities/optional/optional.hash/hash.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/utilities/optional/optional.hash/hash.pass.cpp?rev=298581&r1=298580&r2=298581&view=diff
==
--- libcxx/trunk/test/std/utilities/optional/optional.hash/hash.pass.cpp 
(original)
+++ libcxx/trunk/test/std/utilities/optional/optional.hash/hash.pass.cpp Thu 
Mar 23 01:21:24 2017
@@ -22,11 +22,15 @@
 struct A {};
 struct B {};
 
+namespace std {
+
 template <>
-struct std::hash {
+struct hash {
   size_t operator()(B const&) { return 0; }
 };
 
+}
+
 int main()
 {
 using std::optional;

Modified: libcxx/trunk/test/std/utilities/variant/variant.hash/hash.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/utilities/variant/variant.hash/hash.pass.cpp?rev=298581&r1=298580&r2=298581&view=diff
==
--- libcxx/trunk/test/std/utilities/variant/variant.hash/hash.pass.cpp 
(original)
+++ libcxx/trunk/test/std/utilities/variant/variant.hash/hash.pass.cpp Thu Mar 
23 01:21:24 2017
@@ -125,13 +125,17 @@ void test_hash_variant_duplicate_element
 struct A {};
 struct B {};
 
+namespace std {
+
 template <>
-struct std::hash {
+struct hash {
   size_t operator()(B const&) const {
 return 0;
   }
 };
 
+}
+
 void test_hash_variant_enabled() {
   {
 test_hash_enabled_for_type >();


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[libcxx] r292097 - Added a workaround for a `-fdelayed-template-parsing` bug.

[Michael Park via cfe-commits]

Author: mpark
Date: Mon Jan 16 02:14:25 2017
New Revision: 292097

URL: http://llvm.org/viewvc/llvm-project?rev=292097&view=rev
Log:
Added a workaround for a `-fdelayed-template-parsing` bug.

Summary:
There seems to be an additional bug in `-fdelayed-template-parsing`
similar to
http://llvm.org/viewvc/llvm-project?view=revision&revision=236063.

This is a workaround for it for  to compile with `clang-cl` on Windows.

Reviewers: EricWF

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

Modified:
libcxx/trunk/include/variant
libcxx/trunk/test/std/utilities/variant/variant.visit/visit.pass.cpp

Modified: libcxx/trunk/include/variant
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/variant?rev=292097&r1=292096&r2=292097&view=diff
==
--- libcxx/trunk/include/variant (original)
+++ libcxx/trunk/include/variant Mon Jan 16 02:14:25 2017
@@ -466,17 +466,22 @@ private:
 return __result{{_VSTD::forward<_Fs>(__fs)...}};
   }
 
-  template 
-  inline _LIBCPP_INLINE_VISIBILITY
-  static constexpr auto __make_dispatch(index_sequence<_Is...>) {
-struct __dispatcher {
-  static constexpr decltype(auto) __dispatch(_Fp __f, _Vs... __vs) {
+  template 
+  struct __dispatcher {
+template 
+inline _LIBCPP_INLINE_VISIBILITY
+static constexpr decltype(auto) __dispatch(_Fp __f, _Vs... __vs) {
 return __invoke_constexpr(
 static_cast<_Fp>(__f),
 __access::__base::__get_alt<_Is>(static_cast<_Vs>(__vs))...);
-  }
-};
-return _VSTD::addressof(__dispatcher::__dispatch);
+}
+  };
+
+  template 
+  inline _LIBCPP_INLINE_VISIBILITY
+  static constexpr auto __make_dispatch(index_sequence<_Is...>) {
+return _VSTD::addressof(
+__dispatcher<_Is...>::template __dispatch<_Fp, _Vs...>);
   }
 
   template 

Modified: libcxx/trunk/test/std/utilities/variant/variant.visit/visit.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/utilities/variant/variant.visit/visit.pass.cpp?rev=292097&r1=292096&r2=292097&view=diff
==
--- libcxx/trunk/test/std/utilities/variant/variant.visit/visit.pass.cpp 
(original)
+++ libcxx/trunk/test/std/utilities/variant/variant.visit/visit.pass.cpp Mon 
Jan 16 02:14:25 2017
@@ -10,9 +10,6 @@
 
 // UNSUPPORTED: c++98, c++03, c++11, c++14
 
-// FIXME: This test hangs for an unknown reason on Windows. See 
llvm.org/PR31642
-// UNSUPPORTED: windows
-
 // 
 // template 
 // constexpr see below visit(Visitor&& vis, Variants&&... vars);


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[libcxx] r296558 - Updated the XFAIL comment in variant tests.

[Michael Park via cfe-commits]

Author: mpark
Date: Tue Feb 28 19:07:56 2017
New Revision: 296558

URL: http://llvm.org/viewvc/llvm-project?rev=296558&view=rev
Log:
Updated the XFAIL comment in variant tests.

Summary:
`ConstexprTestTypes::NoCtors` is an aggregate type (and consequently a literal 
type) in C++17,
but not in C++14 since it has a base class. This patch updates the comment to 
accurately describe the reason for the XFAIL.

Reviewers: EricWF

Reviewed By: EricWF

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

Modified:

libcxx/trunk/test/std/utilities/variant/variant.variant/variant.status/index.pass.cpp

libcxx/trunk/test/std/utilities/variant/variant.variant/variant.status/valueless_by_exception.pass.cpp

Modified: 
libcxx/trunk/test/std/utilities/variant/variant.variant/variant.status/index.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/utilities/variant/variant.variant/variant.status/index.pass.cpp?rev=296558&r1=296557&r2=296558&view=diff
==
--- 
libcxx/trunk/test/std/utilities/variant/variant.variant/variant.status/index.pass.cpp
 (original)
+++ 
libcxx/trunk/test/std/utilities/variant/variant.variant/variant.status/index.pass.cpp
 Tue Feb 28 19:07:56 2017
@@ -10,7 +10,10 @@
 
 // UNSUPPORTED: c++98, c++03, c++11, c++14
 
-// The following compilers don't allow constexpr variables of non-literal type.
+// The following compilers don't consider a type an aggregate type (and
+// consequently not a literal type) if it has a base class at all.
+// In C++17, an aggregate type is allowed to have a base class if it's not
+// virtual, private, nor protected (e.g. ConstexprTestTypes:::NoCtors).
 // XFAIL: gcc-5, gcc-6
 // XFAIL: clang-3.5, clang-3.6, clang-3.7, clang-3.8
 // XFAIL: apple-clang-6, apple-clang-7, apple-clang-8

Modified: 
libcxx/trunk/test/std/utilities/variant/variant.variant/variant.status/valueless_by_exception.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/utilities/variant/variant.variant/variant.status/valueless_by_exception.pass.cpp?rev=296558&r1=296557&r2=296558&view=diff
==
--- 
libcxx/trunk/test/std/utilities/variant/variant.variant/variant.status/valueless_by_exception.pass.cpp
 (original)
+++ 
libcxx/trunk/test/std/utilities/variant/variant.variant/variant.status/valueless_by_exception.pass.cpp
 Tue Feb 28 19:07:56 2017
@@ -10,7 +10,10 @@
 
 // UNSUPPORTED: c++98, c++03, c++11, c++14
 
-// The following compilers don't allow constexpr variables of non-literal type.
+// The following compilers don't consider a type an aggregate type (and
+// consequently not a literal type) if it has a base class at all.
+// In C++17, an aggregate type is allowed to have a base class if it's not
+// virtual, private, nor protected (e.g. ConstexprTestTypes:::NoCtors).
 // XFAIL: gcc-5, gcc-6
 // XFAIL: clang-3.5, clang-3.6, clang-3.7, clang-3.8
 // XFAIL: apple-clang-6, apple-clang-7, apple-clang-8


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[libcxx] r297065 - Updated email address in `CREDITS.txt`.

[Michael Park via cfe-commits]

Author: mpark
Date: Mon Mar  6 14:46:55 2017
New Revision: 297065

URL: http://llvm.org/viewvc/llvm-project?rev=297065&view=rev
Log:
Updated email address in `CREDITS.txt`.

Modified:
libcxx/trunk/CREDITS.TXT

Modified: libcxx/trunk/CREDITS.TXT
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/CREDITS.TXT?rev=297065&r1=297064&r2=297065&view=diff
==
--- libcxx/trunk/CREDITS.TXT (original)
+++ libcxx/trunk/CREDITS.TXT Mon Mar  6 14:46:55 2017
@@ -81,7 +81,7 @@ E: andrew.c.mor...@gmail.com
 D: Minor patches and Linux fixes.
 
 N: Michael Park
-E: mp...@apache.org
+E: mcyp...@gmail.com
 D: Implementation of .
 
 N: Arvid Picciani


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[libcxx] r302773 - Fix `std::visit` for the zero variants case.

[Michael Park via cfe-commits]

Author: mpark
Date: Thu May 11 02:17:12 2017
New Revision: 302773

URL: http://llvm.org/viewvc/llvm-project?rev=302773&view=rev
Log:
Fix `std::visit` for the zero variants case.

Summary:
The following code is broken:

```
std::visit([]{});
```

Reviewers: EricWF

Reviewed By: EricWF

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

Modified:
libcxx/trunk/include/variant
libcxx/trunk/test/std/utilities/variant/variant.visit/visit.pass.cpp

Modified: libcxx/trunk/include/variant
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/variant?rev=302773&r1=302772&r2=302773&view=diff
==
--- libcxx/trunk/include/variant (original)
+++ libcxx/trunk/include/variant Thu May 11 02:17:12 2017
@@ -425,30 +425,21 @@ struct __base {
 constexpr auto __fmatrix =
 __make_fmatrix<_Visitor&&,
decltype(_VSTD::forward<_Vs>(__vs).__as_base())...>();
-const size_t __indices[] = {__vs.index()...};
-return __at(__fmatrix, __indices)(_VSTD::forward<_Visitor>(__visitor),
-  
_VSTD::forward<_Vs>(__vs).__as_base()...);
+return __at(__fmatrix, __vs.index()...)(
+_VSTD::forward<_Visitor>(__visitor),
+_VSTD::forward<_Vs>(__vs).__as_base()...);
   }
 
 private:
   template 
   inline _LIBCPP_INLINE_VISIBILITY
-  static constexpr const _Tp& __at_impl(const _Tp& __elem, const size_t*) {
-return __elem;
-  }
-
-  template 
-  inline _LIBCPP_INLINE_VISIBILITY
-  static constexpr auto&& __at_impl(const array<_Tp, _Np>& __elems,
-const size_t* __index) {
-return __at_impl(__elems[*__index], __index + 1);
-  }
+  static constexpr const _Tp& __at(const _Tp& __elem) { return __elem; }
 
-  template 
+  template 
   inline _LIBCPP_INLINE_VISIBILITY
   static constexpr auto&& __at(const array<_Tp, _Np>& __elems,
-   const size_t (&__indices)[_Ip]) {
-return __at_impl(__elems, begin(__indices));
+   size_t __index, _Indices... __indices) {
+return __at(__elems[__index], __indices...);
   }
 
   template 

Modified: libcxx/trunk/test/std/utilities/variant/variant.visit/visit.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/utilities/variant/variant.visit/visit.pass.cpp?rev=302773&r1=302772&r2=302773&view=diff
==
--- libcxx/trunk/test/std/utilities/variant/variant.visit/visit.pass.cpp 
(original)
+++ libcxx/trunk/test/std/utilities/variant/variant.visit/visit.pass.cpp Thu 
May 11 02:17:12 2017
@@ -94,6 +94,16 @@ void test_call_operator_forwarding() {
   using Fn = ForwardingCallObject;
   Fn obj{};
   const Fn &cobj = obj;
+  { // test call operator forwarding - no variant
+std::visit(obj);
+assert(Fn::check_call<>(CT_NonConst | CT_LValue));
+std::visit(cobj);
+assert(Fn::check_call<>(CT_Const | CT_LValue));
+std::visit(std::move(obj));
+assert(Fn::check_call<>(CT_NonConst | CT_RValue));
+std::visit(std::move(cobj));
+assert(Fn::check_call<>(CT_Const | CT_RValue));
+  }
   { // test call operator forwarding - single variant, single arg
 using V = std::variant;
 V v(42);


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clangd] Enable passing a `FeatureModuleSet` to `clangdMain`. (PR #97255)

[Michael Park via cfe-commits]

https://github.com/mpark created https://github.com/llvm/llvm-project/pull/97255

This diff adds an overload of `clangdMain` that makes a `FeatureModuleSet`. 
`clangdMain` was initially added in 56ac9d46a7c1 to allow custom builds of 
clangd outside of the LLVM repo that link against clangd. Currently, there's no 
way for such custom builds to use their own `FeatureModule`s.

This adds a new overload of `clangdMain` rather than adding a new default 
parameter since that would break existing code that links against the library.

It's also possible that there is a use that takes the address of `clangdMain` 
which this change would break. That seems very unlikely, and is not addressed 
in this diff. If this turns out to be a problem, we can add a 
`clangdMainWithFeatures` instead of adding a new overload.

>From 01ae6c9aee33d3b2b0a00484bf7c041f6b90e710 Mon Sep 17 00:00:00 2001
From: Michael Park 
Date: Sun, 30 Jun 2024 21:41:09 -0700
Subject: [PATCH] Enable passing a `FeatureModuleSet` to `clangdMain`.

This diff adds an overload of `clangdMain` that makes a `FeatureModuleSet`.
`clangdMain` was initially added in 56ac9d46a7c1 to allow custom builds
of clangd outside of the LLVM repo that link against clangd. Currently,
there's no way for such custom builds to use their own `FeatureModule`s.

This adds a new overload of `clangdMain` rather than adding a new default
parameter since that would break existing code that links against the library.

It's also possible that there is a use that takes the address of `clangdMain`
which this change would break. That seems very unlikely, and is not addressed
in this diff. If this turns out to be a problem, we can add a
`clangdMainWithFeatures` instead of adding a new overload.
---
 clang-tools-extra/clangd/tool/ClangdMain.cpp | 6 ++
 clang-tools-extra/clangd/tool/ClangdMain.h   | 2 ++
 2 files changed, 8 insertions(+)

diff --git a/clang-tools-extra/clangd/tool/ClangdMain.cpp 
b/clang-tools-extra/clangd/tool/ClangdMain.cpp
index c3ba655ee2dc6..4caa1425c5ab6 100644
--- a/clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ b/clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -13,6 +13,7 @@
 #include "Config.h"
 #include "ConfigProvider.h"
 #include "Feature.h"
+#include "FeatureModuleSet.h"
 #include "IncludeCleaner.h"
 #include "PathMapping.h"
 #include "Protocol.h"
@@ -712,6 +713,10 @@ enum class ErrorResultCode : int {
 };
 
 int clangdMain(int argc, char *argv[]) {
+  return clangdMain(arc, argv, nullptr);
+}
+
+int clangdMain(int argc, char *argv[], FeatureModuleSet* FeatureModules) {
   // Clang could run on the main thread. e.g., when the flag '-check' or 
'-sync'
   // is enabled.
   clang::noteBottomOfStack();
@@ -898,6 +903,7 @@ clangd accepts flags on the commandline, and in the 
CLANGD_FLAGS environment var
   Opts.StaticIndex = PAI.get();
   Opts.AsyncThreadsCount = WorkerThreadsCount;
   Opts.MemoryCleanup = getMemoryCleanupFunction();
+  Opts.FeatureModules = FeatureModules;
 
   Opts.CodeComplete.IncludeIneligibleResults = IncludeIneligibleResults;
   Opts.CodeComplete.Limit = LimitResults;
diff --git a/clang-tools-extra/clangd/tool/ClangdMain.h 
b/clang-tools-extra/clangd/tool/ClangdMain.h
index bd0f51aac83bb..88b590be58d5e 100644
--- a/clang-tools-extra/clangd/tool/ClangdMain.h
+++ b/clang-tools-extra/clangd/tool/ClangdMain.h
@@ -11,8 +11,10 @@
 
 namespace clang {
 namespace clangd {
+class FeatureModuleSet;
 // clangd main function (clangd server loop)
 int clangdMain(int argc, char *argv[]);
+int clangdMain(int argc, char *argv[], FeatureModuleSet *FeatureModules);
 } // namespace clangd
 } // namespace clang
 

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clangd] Enable passing a `FeatureModuleSet` to `clangdMain`. (PR #97255)

[Michael Park via cfe-commits]

https://github.com/mpark closed https://github.com/llvm/llvm-project/pull/97255
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [NFC] Fix an inaccurate comment about typo-correction. (PR #108143)

[Michael Park via cfe-commits]

https://github.com/mpark created 
https://github.com/llvm/llvm-project/pull/108143

The comment describes "If the identifier was typo-corrected", but it doesn't 
need to have been typo-corrected, just being annotated is enough to retry.

>From a5cb61e79b928dae0272dc9b3c5448050361f775 Mon Sep 17 00:00:00 2001
From: Michael Park 
Date: Fri, 6 Sep 2024 02:27:47 -0700
Subject: [PATCH] [NFC] Fix an inaccurate comment about typo-correction.

The comment describes "If the identifier was typo-corrected",
but it doesn't need to have been typo-corrected, just being
annotated is enough to retry.
---
 clang/lib/Parse/ParseStmt.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/lib/Parse/ParseStmt.cpp b/clang/lib/Parse/ParseStmt.cpp
index bdb3fc051d0b35..9188799fce13e6 100644
--- a/clang/lib/Parse/ParseStmt.cpp
+++ b/clang/lib/Parse/ParseStmt.cpp
@@ -228,7 +228,7 @@ StmtResult 
Parser::ParseStatementOrDeclarationAfterAttributes(
 return StmtError();
   }
 
-  // If the identifier was typo-corrected, try again.
+  // If the identifier was annotated, try again.
   if (Tok.isNot(tok::identifier))
 goto Retry;
 }

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [NFC] Comment fix: "does specify state that" -> "does specify that" (PR #106338)

[Michael Park via cfe-commits]

https://github.com/mpark created 
https://github.com/llvm/llvm-project/pull/106338

None

>From 76b247844ba302fc9b62431d1abaa4720372e51f Mon Sep 17 00:00:00 2001
From: Michael Park 
Date: Tue, 27 Aug 2024 22:32:58 -0700
Subject: [PATCH] [NFC] Comment fix: "does specify state that" -> "does specify
 that"

---
 clang/lib/AST/ComputeDependence.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/lib/AST/ComputeDependence.cpp 
b/clang/lib/AST/ComputeDependence.cpp
index 62ca15ea398f5c..6ef49790481aca 100644
--- a/clang/lib/AST/ComputeDependence.cpp
+++ b/clang/lib/AST/ComputeDependence.cpp
@@ -164,7 +164,7 @@ ExprDependence clang::computeDependence(BinaryOperator *E) {
 ExprDependence clang::computeDependence(ConditionalOperator *E) {
   // The type of the conditional operator depends on the type of the 
conditional
   // to support the GCC vector conditional extension. Additionally,
-  // [temp.dep.expr] does specify state that this should be dependent on ALL 
sub
+  // [temp.dep.expr] does specify that this should be dependent on ALL sub
   // expressions.
   return E->getCond()->getDependence() | E->getLHS()->getDependence() |
  E->getRHS()->getDependence();

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] Emit `BeginSourceFile` failure with `elog`. (PR #104845)

[Michael Park via cfe-commits]

https://github.com/mpark created 
https://github.com/llvm/llvm-project/pull/104845

There are 3 ways in which `ParseAST::build` can fail and return `std::nullopt`. 
2 of the ways we emit the error message using `elog`, but for the 3rd way, 
`log` is used. We should emit all 3 of these reasons with `elog`.

>From 5e27ddec1ef275fe4401fa9f57827058610597f5 Mon Sep 17 00:00:00 2001
From: Michael Park 
Date: Mon, 19 Aug 2024 13:00:02 -0700
Subject: [PATCH] Emit `BeginSourceFile` failure with `elog`.

There are 3 ways in which `ParseAST::build` can fail and return `std::nullopt`.
2 of the ways we emit the error message using `elog`, but for the 3rd way,
`log` is used. We should emit all 3 of these reasons with `elog`.
---
 clang-tools-extra/clangd/ParsedAST.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/clang-tools-extra/clangd/ParsedAST.cpp 
b/clang-tools-extra/clangd/ParsedAST.cpp
index a2f1504db7e880..14440acd08b353 100644
--- a/clang-tools-extra/clangd/ParsedAST.cpp
+++ b/clang-tools-extra/clangd/ParsedAST.cpp
@@ -512,8 +512,8 @@ ParsedAST::build(llvm::StringRef Filename, const 
ParseInputs &Inputs,
   auto Action = std::make_unique();
   const FrontendInputFile &MainInput = Clang->getFrontendOpts().Inputs[0];
   if (!Action->BeginSourceFile(*Clang, MainInput)) {
-log("BeginSourceFile() failed when building AST for {0}",
-MainInput.getFile());
+elog("BeginSourceFile() failed when building AST for {0}",
+ MainInput.getFile());
 return std::nullopt;
   }
   // If we saw an include guard in the preamble section of the main file,

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Fix a typo in InternalsManual: ActOnCXX -> ActOnXXX (PR #105207)

[Michael Park via cfe-commits]

https://github.com/mpark created 
https://github.com/llvm/llvm-project/pull/105207

This part of the manual describes uses of `ActOnXXX` and `BuildXXX`.

>From 524de01a9759a5f9ba0cad67f8351405fbfa37db Mon Sep 17 00:00:00 2001
From: Michael Park 
Date: Tue, 20 Aug 2024 10:59:03 -0700
Subject: [PATCH] Fix a typo in InternalsManual: ActOnCXX -> ActOnXXX

---
 clang/docs/InternalsManual.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/docs/InternalsManual.rst b/clang/docs/InternalsManual.rst
index 3d21e37784b363..b70ec35f3da229 100644
--- a/clang/docs/InternalsManual.rst
+++ b/clang/docs/InternalsManual.rst
@@ -3200,7 +3200,7 @@ are similar.
always involve two functions: an ``ActOnXXX`` function that will be called
directly from the parser, and a ``BuildXXX`` function that performs the
actual semantic analysis and will (eventually!) build the AST node.  It's
-   fairly common for the ``ActOnCXX`` function to do very little (often just
+   fairly common for the ``ActOnXXX`` function to do very little (often just
some minor translation from the parser's representation to ``Sema``'s
representation of the same thing), but the separation is still important:
C++ template instantiation, for example, should always call the ``BuildXXX``

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Fix a typo in InternalsManual: ActOnCXX -> ActOnXXX (PR #105207)

[Michael Park via cfe-commits]

mpark wrote:

> LGTM
> 
> We do have some `ActOnCXX...` functions, so that’s probably how that happened 
> ;Þ

Haha yep. I suspect that as well.

https://github.com/llvm/llvm-project/pull/105207
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [NFC] Fix a typo in InternalsManual: ActOnCXX -> ActOnXXX (PR #105207)

[Michael Park via cfe-commits]

https://github.com/mpark edited https://github.com/llvm/llvm-project/pull/105207
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [NFC] Fix a typo in InternalsManual: ActOnCXX -> ActOnXXX (PR #105207)

[Michael Park via cfe-commits]

https://github.com/mpark closed https://github.com/llvm/llvm-project/pull/105207
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [NFC] Fix an incorrect comment about operator precedence. (PR #105784)

[Michael Park via cfe-commits]

https://github.com/mpark created 
https://github.com/llvm/llvm-project/pull/105784

The comment talks about left-associative operators twice, when the latter 
mention is actually describing right-associative operators.

>From 9c90592726137fabbcbc7388b3ce0d4bc496bfc6 Mon Sep 17 00:00:00 2001
From: Michael Park 
Date: Thu, 22 Aug 2024 22:14:19 -0700
Subject: [PATCH] [NFC] Fix an incorrect comment about operator precedence.

The comment talks about left-associative operators twice, when
the latter mention is actually describing right-associative operators.
---
 clang/lib/Parse/ParseExpr.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/lib/Parse/ParseExpr.cpp b/clang/lib/Parse/ParseExpr.cpp
index 1405aef700bec5..64f284d78b24db 100644
--- a/clang/lib/Parse/ParseExpr.cpp
+++ b/clang/lib/Parse/ParseExpr.cpp
@@ -607,7 +607,7 @@ Parser::ParseRHSOfBinaryExpression(ExprResult LHS, 
prec::Level MinPrec) {
 RHS = ExprError();
   }
   // If this is left-associative, only parse things on the RHS that bind
-  // more tightly than the current operator.  If it is left-associative, it
+  // more tightly than the current operator.  If it is right-associative, 
it
   // is okay, to bind exactly as tightly.  For example, compile A=B=C=D as
   // A=(B=(C=D)), where each paren is a level of recursion here.
   // The function takes ownership of the RHS.

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [NFC] Fix an incorrect comment about operator precedence. (PR #105784)

[Michael Park via cfe-commits]

mpark wrote:

> Thanks (Hi!). Do you have commit access or should I merge that for you?

Hi Corentin! I have commit access 🙂

https://github.com/llvm/llvm-project/pull/105784
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [NFC] Fix an incorrect comment about operator precedence. (PR #105784)

[Michael Park via cfe-commits]

https://github.com/mpark closed https://github.com/llvm/llvm-project/pull/105784
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clangd] Fix the modification detection logic in `ClangdLSPServer::applyConfiguration`. (PR #115438)

[Michael Park via cfe-commits]

https://github.com/mpark edited https://github.com/llvm/llvm-project/pull/115438
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clangd] Fix the modification detection logic in `ClangdLSPServer::applyConfiguration`. (PR #115438)

[Michael Park via cfe-commits]

https://github.com/mpark edited https://github.com/llvm/llvm-project/pull/115438
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] Fix the modification detection logic in `ClangdLSPServer::applyConfiguration`. (PR #115438)

[Michael Park via cfe-commits]

https://github.com/mpark created 
https://github.com/llvm/llvm-project/pull/115438

The current `ClangdLSPServer::applyConfiguration` logic tests whether the 
stored CDB for a file has changed.

https://github.com/llvm/llvm-project/blob/1adca7af21f1d8cc12b0f1c33db8ab869b36ae48/clang-tools-extra/clangd/ClangdLSPServer.cpp#L1424-L1432

`OverlayCDB::getCompileCommand` applies a mangling operation if the CDB has one 
set. A `CommandMangler` mangles the provided command line, for example, adding 
flags such as `--driver-mode=g++`. See this example test case:

https://github.com/llvm/llvm-project/blob/d4525b016f5a1ab2852acb2108742b2f9d0bd3bd/clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp#L53-L61

This means that the `Old != New` test is **always** true because `New` is a 
"raw" (pre-mangling) compile command whereas `Old` a "cooked" (post-mangling) 
compile command.

This PR proposes to fix this by moving the check into `setCompileCommand`. The 
comparison is done between the "raw" compile commands, and a boolean 
representing whether there has been a change in the command line is returned.

>From 78b08dae9386435f734861a2f5428c88ca9be1e4 Mon Sep 17 00:00:00 2001
From: Michael Park 
Date: Thu, 7 Nov 2024 17:12:23 -0800
Subject: [PATCH] Fix the modification detection logic in
 `ClangdLSPServer::applyConfiguration`.

`OverlayCDB::getCompileCommand` applies a mangling operation if
the CDB has one set. Given a `CommandMangler`, the provided command line
is mangled to have additional flags such as `--driver-mode=g++`.

Example: 
https://github.com/llvm/llvm-project/blob/d4525b016f5a1ab2852acb2108742b2f9d0bd3bd/clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp#L46-L62

Considering the previous logic of:

```
auto Old = CDB->getCompileCommand(File);
auto New = tooling::CompileCommand(...);
if (Old != New) {
  ...
}
```

The `Old != New` here is always true because `New` is the "raw" compile
command whereas `Old` is the "cooked" (mangling aplied) compile command.

This diff performs the change check inside of `setCompileCommand`
instead to compare the "raw" compile commands.
---
 clang-tools-extra/clangd/ClangdLSPServer.cpp  | 13 +--
 .../clangd/GlobalCompilationDatabase.cpp  | 15 +
 .../clangd/GlobalCompilationDatabase.h|  4 +++-
 .../GlobalCompilationDatabaseTests.cpp| 22 ++-
 4 files changed, 31 insertions(+), 23 deletions(-)

diff --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp 
b/clang-tools-extra/clangd/ClangdLSPServer.cpp
index 06573a57554245..05dd313d0a0d35 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -1419,15 +1419,12 @@ void ClangdLSPServer::applyConfiguration(
 const ConfigurationSettings &Settings) {
   // Per-file update to the compilation database.
   llvm::StringSet<> ModifiedFiles;
-  for (auto &Entry : Settings.compilationDatabaseChanges) {
-PathRef File = Entry.first;
-auto Old = CDB->getCompileCommand(File);
-auto New =
-tooling::CompileCommand(std::move(Entry.second.workingDirectory), File,
-std::move(Entry.second.compilationCommand),
+  for (auto &[File, Command] : Settings.compilationDatabaseChanges) {
+auto Cmd =
+tooling::CompileCommand(std::move(Command.workingDirectory), File,
+std::move(Command.compilationCommand),
 /*Output=*/"");
-if (Old != New) {
-  CDB->setCompileCommand(File, std::move(New));
+if (CDB->setCompileCommand(File, std::move(Cmd))) {
   ModifiedFiles.insert(File);
 }
   }
diff --git a/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp 
b/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
index 1d96667a8e9f4a..71e97ac4efd673 100644
--- a/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
+++ b/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
@@ -807,7 +807,7 @@ tooling::CompileCommand 
OverlayCDB::getFallbackCommand(PathRef File) const {
   return Cmd;
 }
 
-void OverlayCDB::setCompileCommand(PathRef File,
+bool OverlayCDB::setCompileCommand(PathRef File,
std::optional Cmd) 
{
   // We store a canonical version internally to prevent mismatches between set
   // and get compile commands. Also it assures clients listening to broadcasts
@@ -815,12 +815,19 @@ void OverlayCDB::setCompileCommand(PathRef File,
   std::string CanonPath = removeDots(File);
   {
 std::unique_lock Lock(Mutex);
-if (Cmd)
-  Commands[CanonPath] = std::move(*Cmd);
-else
+if (Cmd) {
+  if (auto [It, Inserted] =
+  Commands.try_emplace(CanonPath, std::move(*Cmd));
+  !Inserted) {
+if (It->second == *Cmd)
+  return false;
+It->second = *Cmd;
+  }
+} else
   Commands.erase(CanonPath);
   }
   OnCommandChanged.broadcast({CanonPath});
+  return true;
 }
 
 DelegatingCDB::DelegatingCDB(

[clang] Delay marking pending incomplete decl chains until the end of `finishPendingActions`. (PR #121245)

[Michael Park via cfe-commits]

https://github.com/mpark edited https://github.com/llvm/llvm-project/pull/121245
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Delay marking pending incomplete decl chains until the end of `finishPendingActions`. (PR #121245)

[Michael Park via cfe-commits]

https://github.com/mpark created 
https://github.com/llvm/llvm-project/pull/121245

This is fix for an unreachable code path being reached, for an internal use 
case at Meta. I'm unfortunately still not able to reproduce a minimal example 
that I can share 😞

# Description

There is a defaulted constructor `_Hashtable_alloc() = default;` which ends up 
in 
[`CodeGenFunction::GenerateCode`](https://github.com/llvm/llvm-project/blob/48bf0a9457fd60d0872d9b9b4804a95c833a72e1/clang/lib/CodeGen/CodeGenFunction.cpp#L1448)
 and eventually calls 
[`FunctionProtoType::canThrow`](https://github.com/llvm/llvm-project/blob/48bf0a9457fd60d0872d9b9b4804a95c833a72e1/clang/lib/AST/Type.cpp#L3758)
 with `EST_Unevaluated`.

In the "good" cases I observed, I see that the decl is also loaded with the 
`noexcept-unevaluated` state, but it gets fixed up later by a [call to 
`adjustExceptionSpec`](https://github.com/llvm/llvm-project/blob/48bf0a9457fd60d0872d9b9b4804a95c833a72e1/clang/lib/Serialization/ASTReader.cpp#L10656).
 The update gets [added to `PendingExceptionSpecUpdates` 
here](https://github.com/llvm/llvm-project/blob/48bf0a9457fd60d0872d9b9b4804a95c833a72e1/clang/lib/Serialization/ASTReaderDecl.cpp#L4749-L4752).

In the "bad" case, the update does not get added to 
`PendingExceptionSpecUpdates` and hence the call to `adjustExceptionSpec` also 
does not occur.

# Observations

I made two observations in Clang Discord: 
[[1](https://discord.com/channels/636084430946959380/636732781086638081/1317290104431185921)]
 and 
[[2](https://discord.com/channels/636084430946959380/636732781086638081/1317291980413206608)].

1. 
[FinishedDeserializating](https://github.com/llvm/llvm-project/blob/main/clang/lib/Serialization/ASTReader.cpp#L10641)
 can exit with stuff still in `PendingIncompleteDeclChains`.

`FinishedDeserializing` [calls `finishPendingActions` only if 
`NumCurrentElementsDeserializing == 
1`](https://github.com/llvm/llvm-project/blob/main/clang/lib/Serialization/ASTReader.cpp#L10647).
 In 
[`finishPendingActions`](https://github.com/llvm/llvm-project/blob/main/clang/lib/Serialization/ASTReader.cpp#L10028),
 it tries to [clear out 
`PendingIncompleteDeclChains`](https://github.com/llvm/llvm-project/blob/main/clang/lib/Serialization/ASTReader.cpp#L10082-L10087).
 This is done in a loop, which is fine. But, later in `finishPendingActions`, 
it calls things like `hasBody` 
[here](https://github.com/llvm/llvm-project/blob/main/clang/lib/Serialization/ASTReader.cpp#L10296).
 These can call `getMostRecentDecl` / `getNextRedeclaration` that ultimately 
calls 
[`CompleteDeclChain`](https://github.com/llvm/llvm-project/blob/main/clang/lib/Serialization/ASTReader.cpp#L7719).
 `CompleteDeclChain` is "called each time we need the most recent declaration 
of a declaration after the generation count is incremented." according to [this 
comment](https://github.com/llvm/llvm-project/blob/main/clang/include/clang/AST/ExternalASTSource.h#L213-L215).
 Anyway, since we're still in `finishPendingActions` with 
`NumCurrentElementsDeserializing == 1`, calls to `CompleteDeclChain` simply 
[adds more stuff to 
`PendingIncompleteDeclChains`](https://github.com/llvm/llvm-project/blob/main/clang/lib/Serialization/ASTReader.cpp#L7725).
 ~~I think the emptying out of `PendingIncompleteDeclChains` should actually 
happen in `FinishedDeserializing`, in [this 
loop](https://github.com/llvm/llvm-project/blob/main/clang/lib/Serialization/ASTReader.cpp#L10657-L10658)
 instead.~~

2. `LazyGenerationalUpdatePtr::get` seems a bit dubious. In the `LazyData` 
case, it does [the 
following](https://github.com/llvm/llvm-project/blob/main/clang/include/clang/AST/ExternalASTSource.h#L482-L486):

```cpp
  if (LazyVal->LastGeneration != LazyVal->ExternalSource->getGeneration()) {
LazyVal->LastGeneration = LazyVal->ExternalSource->getGeneration();
(LazyVal->ExternalSource->*Update)(O);
  }
  return LazyVal->LastValue;
```

so for example, after `markIncomplete`, `LastGeneration` gets set to `0` to 
force the update. For example, `LazyVal->LastGeneration == 0` and 
`LazyVal->ExternalSource->getGeneration() == 6`. The `Update` function pointer 
calls `CompleteDeclChain`, which, if we're in the middle of deserialization, do 
nothing and just add the decl to `PendingIncompleteDeclChains`. So the update 
was not completed, but the `LastGeneration` is updated to `6` now... that seems 
potentially problematic, since subsequent calls will simply return 
`LazyVal->LastValue` since the generation numbers match now. I would've maybe 
expected something like:
```
  if (LazyVal->LastGeneration != LazyVal->ExternalSource->getGeneration() &&
  (LazyVal->ExternalSource->*Update)(O)) {
LazyVal->LastGeneration = LazyVal->ExternalSource->getGeneration();
  }
  return LazyVal->LastValue;
```

where the generation is updated once the intended update actually happens.

# Solution

The proposed solution is to perform the marking of

[clang] Delay marking pending incomplete decl chains until the end of `finishPendingActions`. (PR #121245)

[Michael Park via cfe-commits]

https://github.com/mpark edited https://github.com/llvm/llvm-project/pull/121245
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Delay marking pending incomplete decl chains until the end of `finishPendingActions`. (PR #121245)

[Michael Park via cfe-commits]

mpark wrote:

> Did you try creduce/cvise (although using that with modules might be a bit 
> challenging) It would be really nice to have a repro so that we have a test 
> case

I did. It did help me reduce it significantly, but it still involves a few 
modules that are difficult to reduce. I still can make progress by running it 
on the modules themselves though. I agree it'd be really nice to have a test 
case.

https://github.com/llvm/llvm-project/pull/121245
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C++20] [Modules] [Serialization] Delay marking pending incomplete decl chains until the end of `finishPendingActions`. (PR #121245)

[Michael Park via cfe-commits]

mpark wrote:

> +1. It is best to have more test case. Generally I reduce it by hand in this 
> case.

Yeah, I did already spent quite a bit of effort trying to reduce my hand but 
have not been successful so far. I'll try again and report back. Would you 
still review the logic / whether the suggested solution seems sensible?

https://github.com/llvm/llvm-project/pull/121245
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Delay marking pending incomplete decl chains until the end of `finishPendingActions`. (PR #121245)

[Michael Park via cfe-commits]

https://github.com/mpark edited https://github.com/llvm/llvm-project/pull/121245
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Delay marking pending incomplete decl chains until the end of `finishPendingActions`. (PR #121245)

[Michael Park via cfe-commits]

https://github.com/mpark edited https://github.com/llvm/llvm-project/pull/121245
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Delay marking pending incomplete decl chains until the end of `finishPendingActions`. (PR #121245)

[Michael Park via cfe-commits]

https://github.com/mpark edited https://github.com/llvm/llvm-project/pull/121245
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C++20][Modules][Serialization] Delay marking pending incomplete decl chains until the end of `finishPendingActions`. (PR #121245)

[Michael Park via cfe-commits]

https://github.com/mpark closed https://github.com/llvm/llvm-project/pull/121245
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C++20][Modules][Serialization] Add an additional test case for #120277. (PR #126349)

[Michael Park via cfe-commits]

https://github.com/mpark created 
https://github.com/llvm/llvm-project/pull/126349

https://github.com/llvm/llvm-project/commit/4b35dd57b88a59b169c3471cbc398113d3bf98e8
 was shipped to address https://github.com/llvm/llvm-project/issues/120277 .

It was thought to be a regression in 19.x according to this comment: 
https://github.com/llvm/llvm-project/issues/120277#issuecomment-2558991129

This is a test case that fails even in 17.x but nevertheless is also fixed by: 
https://github.com/llvm/llvm-project/commit/4b35dd57b88a59b169c3471cbc398113d3bf98e8
 .

>From 832f6dc5a6b4d8cb72ba161e2a5d9e808ec9b340 Mon Sep 17 00:00:00 2001
From: Michael Park 
Date: Fri, 7 Feb 2025 21:23:26 -0800
Subject: [PATCH] [C++20][Modules][Serialization] Add an additional test case
 for #120277.

https://github.com/llvm/llvm-project/commit/4b35dd57b88a59b169c3471cbc398113d3bf98e8
was shipped to address https://github.com/llvm/llvm-project/issues/120277 .

It was thought to be a regression in 19.x according to this comment:
https://github.com/llvm/llvm-project/issues/120277#issuecomment-2558991129

This is a test case that fails even in 17.x but nevertheless is also fixed by:
https://github.com/llvm/llvm-project/commit/4b35dd57b88a59b169c3471cbc398113d3bf98e8
 .
---
 clang/test/Modules/pr120277-2.cpp | 69 +++
 1 file changed, 69 insertions(+)
 create mode 100644 clang/test/Modules/pr120277-2.cpp

diff --git a/clang/test/Modules/pr120277-2.cpp 
b/clang/test/Modules/pr120277-2.cpp
new file mode 100644
index 000..1ed8fc52cb1ef7a
--- /dev/null
+++ b/clang/test/Modules/pr120277-2.cpp
@@ -0,0 +1,69 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+// RUN: cd %t
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header %t/hu-01.h \
+// RUN:  -fcxx-exceptions -o %t/hu-01.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header %t/hu-02.h \
+// RUN:  -Wno-experimental-header-units -fcxx-exceptions \
+// RUN:  -fmodule-file=%t/hu-01.pcm -o %t/hu-02.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header %t/hu-03.h \
+// RUN:  -Wno-experimental-header-units -fcxx-exceptions \
+// RUN:  -fmodule-file=%t/hu-01.pcm -o %t/hu-03.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header %t/hu-04.h \
+// RUN:  -Wno-experimental-header-units -fcxx-exceptions \
+// RUN:  -fmodule-file=%t/hu-02.pcm -fmodule-file=%t/hu-03.pcm -o %t/hu-04.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-obj %t/main.cpp \
+// RUN:  -Wno-experimental-header-units -fcxx-exceptions \
+// RUN:  -fmodule-file=%t/hu-04.pcm
+//--- hu-01.h
+template 
+struct A {
+  ~A() { f(); }
+  auto f() const { return 0; }
+};
+
+template 
+struct B {
+  int g() const { return a.f(); }
+  A a;
+};
+
+//--- hu-02.h
+import "hu-01.h";
+
+template 
+struct C {
+  void h() {
+B().g();
+  }
+};
+
+template struct A;
+
+//--- hu-03.h
+import "hu-01.h";
+
+inline B b() {
+  return {};
+}
+
+//--- hu-04.h
+import "hu-02.h";
+import "hu-03.h";
+
+inline void f4() {
+  C{}.h();
+}
+
+//--- main.cpp
+import "hu-04.h";
+
+int main() {
+  f4();
+}

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C++20][Modules][Serialization] Add an additional test case for #120277. (PR #126349)

[Michael Park via cfe-commits]

https://github.com/mpark updated 
https://github.com/llvm/llvm-project/pull/126349

>From eac633d78359245f1ce478b5f3cff26c6f5c858f Mon Sep 17 00:00:00 2001
From: Michael Park 
Date: Fri, 7 Feb 2025 21:23:26 -0800
Subject: [PATCH] [C++20][Modules][Serialization] Add an additional test case
 for #120277.

https://github.com/llvm/llvm-project/commit/4b35dd57b88a59b169c3471cbc398113d3bf98e8
was shipped to address https://github.com/llvm/llvm-project/issues/120277 .

It was thought to be a regression in 19.x according to this comment:
https://github.com/llvm/llvm-project/issues/120277#issuecomment-2558991129

This is a test case that fails even in 17.x but nevertheless is also fixed by:
https://github.com/llvm/llvm-project/commit/4b35dd57b88a59b169c3471cbc398113d3bf98e8
 .
---
 clang/test/Modules/pr120277-2.cpp | 67 +++
 1 file changed, 67 insertions(+)
 create mode 100644 clang/test/Modules/pr120277-2.cpp

diff --git a/clang/test/Modules/pr120277-2.cpp 
b/clang/test/Modules/pr120277-2.cpp
new file mode 100644
index 000..21d24207c920dac
--- /dev/null
+++ b/clang/test/Modules/pr120277-2.cpp
@@ -0,0 +1,67 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+// RUN: cd %t
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header %t/hu-01.h \
+// RUN:  -o %t/hu-01.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header %t/hu-02.h \
+// RUN:  -Wno-experimental-header-units -fmodule-file=%t/hu-01.pcm -o 
%t/hu-02.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header %t/hu-03.h \
+// RUN:  -Wno-experimental-header-units \
+// RUN:  -fmodule-file=%t/hu-01.pcm -o %t/hu-03.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header %t/hu-04.h \
+// RUN:  -Wno-experimental-header-units -fmodule-file=%t/hu-02.pcm \
+// RUN:  -fmodule-file=%t/hu-03.pcm -o %t/hu-04.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-obj %t/main.cpp \
+// RUN:  -Wno-experimental-header-units -fmodule-file=%t/hu-04.pcm
+//--- hu-01.h
+template 
+struct A {
+  ~A() { f(); }
+  auto f() const { return 0; }
+};
+
+template 
+struct B {
+  int g() const { return a.f(); }
+  A a;
+};
+
+//--- hu-02.h
+import "hu-01.h";
+
+template 
+struct C {
+  void h() {
+B().g();
+  }
+};
+
+template struct A;
+
+//--- hu-03.h
+import "hu-01.h";
+
+inline B b() {
+  return {};
+}
+
+//--- hu-04.h
+import "hu-02.h";
+import "hu-03.h";
+
+inline void f4() {
+  C{}.h();
+}
+
+//--- main.cpp
+import "hu-04.h";
+
+int main() {
+  f4();
+}

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C++20][Modules][Serialization] Delay marking pending incomplete decl chains until the end of `finishPendingActions`. (PR #121245)

[Michael Park via cfe-commits]

https://github.com/mpark milestoned 
https://github.com/llvm/llvm-project/pull/121245
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C++20][Modules][Serialization] Delay marking pending incomplete decl chains until the end of `finishPendingActions`. (PR #121245)

[Michael Park via cfe-commits]

mpark wrote:

/cherry-pick a9e249f

https://github.com/llvm/llvm-project/pull/121245
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C++20][Modules][Serialization] Delay marking pending incomplete decl chains until the end of `finishPendingActions`. (PR #121245)

[Michael Park via cfe-commits]

mpark wrote:

/cherry-pick a9e249f64e800fbb20a3b26c0cfb68c1a1aee5e1

https://github.com/llvm/llvm-project/pull/121245
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C++20][Modules][Serialization] Add an additional test case for #120277. (PR #126349)

[Michael Park via cfe-commits]

https://github.com/mpark closed https://github.com/llvm/llvm-project/pull/126349
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C++20][Modules][Serialization] Add an additional test case for #120277. (PR #126349)

[Michael Park via cfe-commits]

https://github.com/mpark updated 
https://github.com/llvm/llvm-project/pull/126349

>From eac633d78359245f1ce478b5f3cff26c6f5c858f Mon Sep 17 00:00:00 2001
From: Michael Park 
Date: Fri, 7 Feb 2025 21:23:26 -0800
Subject: [PATCH 1/2] [C++20][Modules][Serialization] Add an additional test
 case for #120277.

https://github.com/llvm/llvm-project/commit/4b35dd57b88a59b169c3471cbc398113d3bf98e8
was shipped to address https://github.com/llvm/llvm-project/issues/120277 .

It was thought to be a regression in 19.x according to this comment:
https://github.com/llvm/llvm-project/issues/120277#issuecomment-2558991129

This is a test case that fails even in 17.x but nevertheless is also fixed by:
https://github.com/llvm/llvm-project/commit/4b35dd57b88a59b169c3471cbc398113d3bf98e8
 .
---
 clang/test/Modules/pr120277-2.cpp | 67 +++
 1 file changed, 67 insertions(+)
 create mode 100644 clang/test/Modules/pr120277-2.cpp

diff --git a/clang/test/Modules/pr120277-2.cpp 
b/clang/test/Modules/pr120277-2.cpp
new file mode 100644
index 000..21d24207c920dac
--- /dev/null
+++ b/clang/test/Modules/pr120277-2.cpp
@@ -0,0 +1,67 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+// RUN: cd %t
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header %t/hu-01.h \
+// RUN:  -o %t/hu-01.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header %t/hu-02.h \
+// RUN:  -Wno-experimental-header-units -fmodule-file=%t/hu-01.pcm -o 
%t/hu-02.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header %t/hu-03.h \
+// RUN:  -Wno-experimental-header-units \
+// RUN:  -fmodule-file=%t/hu-01.pcm -o %t/hu-03.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header %t/hu-04.h \
+// RUN:  -Wno-experimental-header-units -fmodule-file=%t/hu-02.pcm \
+// RUN:  -fmodule-file=%t/hu-03.pcm -o %t/hu-04.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-obj %t/main.cpp \
+// RUN:  -Wno-experimental-header-units -fmodule-file=%t/hu-04.pcm
+//--- hu-01.h
+template 
+struct A {
+  ~A() { f(); }
+  auto f() const { return 0; }
+};
+
+template 
+struct B {
+  int g() const { return a.f(); }
+  A a;
+};
+
+//--- hu-02.h
+import "hu-01.h";
+
+template 
+struct C {
+  void h() {
+B().g();
+  }
+};
+
+template struct A;
+
+//--- hu-03.h
+import "hu-01.h";
+
+inline B b() {
+  return {};
+}
+
+//--- hu-04.h
+import "hu-02.h";
+import "hu-03.h";
+
+inline void f4() {
+  C{}.h();
+}
+
+//--- main.cpp
+import "hu-04.h";
+
+int main() {
+  f4();
+}

>From 8fd10c5268bcd4732cf1b69eaed23694f8bf69d9 Mon Sep 17 00:00:00 2001
From: Michael Park 
Date: Sat, 8 Feb 2025 22:28:06 -0800
Subject: [PATCH 2/2] Update clang/test/Modules/pr120277-2.cpp

Co-authored-by: Chuanqi Xu 
---
 clang/test/Modules/pr120277-2.cpp | 1 -
 1 file changed, 1 deletion(-)

diff --git a/clang/test/Modules/pr120277-2.cpp 
b/clang/test/Modules/pr120277-2.cpp
index 21d24207c920dac..f3a7e4743184864 100644
--- a/clang/test/Modules/pr120277-2.cpp
+++ b/clang/test/Modules/pr120277-2.cpp
@@ -1,7 +1,6 @@
 // RUN: rm -rf %t
 // RUN: mkdir -p %t
 // RUN: split-file %s %t
-// RUN: cd %t
 
 // RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header %t/hu-01.h \
 // RUN:  -o %t/hu-01.pcm

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C++20][Modules][Serialization] Delay marking pending incomplete decl chains until the end of `finishPendingActions`. (PR #121245)

[Michael Park via cfe-commits]

https://github.com/mpark updated 
https://github.com/llvm/llvm-project/pull/121245

>From f1871418523dbd2915e4245e655d283114595730 Mon Sep 17 00:00:00 2001
From: Michael Park 
Date: Tue, 28 Jan 2025 22:01:37 -0800
Subject: [PATCH 1/3] [C++20][Modules][Serialization] Add a unit test for
 #121245.

---
 clang/test/Modules/pr121245.cpp | 93 +
 1 file changed, 93 insertions(+)
 create mode 100644 clang/test/Modules/pr121245.cpp

diff --git a/clang/test/Modules/pr121245.cpp b/clang/test/Modules/pr121245.cpp
new file mode 100644
index 00..0e276ad0e435d1
--- /dev/null
+++ b/clang/test/Modules/pr121245.cpp
@@ -0,0 +1,93 @@
+// If this test fails, it should be investigated under Debug builds.
+// Before the PR, this test was encountering an `llvm_unreachable()`.
+
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+// RUN: cd %t
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header %t/hu-01.h \
+// RUN:  -fcxx-exceptions -o %t/hu-01.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header %t/hu-02.h \
+// RUN:  -Wno-experimental-header-units -fcxx-exceptions \
+// RUN:  -fmodule-file=%t/hu-01.pcm -o %t/hu-02.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header %t/hu-03.h \
+// RUN:  -Wno-experimental-header-units -fcxx-exceptions \
+// RUN:  -fmodule-file=%t/hu-01.pcm -o %t/hu-03.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header %t/hu-04.h \
+// RUN:  -Wno-experimental-header-units -fcxx-exceptions \
+// RUN:  -fmodule-file=%t/hu-01.pcm -o %t/hu-04.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header %t/hu-05.h \
+// RUN:  -Wno-experimental-header-units -fcxx-exceptions \
+// RUN:  -fmodule-file=%t/hu-03.pcm -fmodule-file=%t/hu-04.pcm \
+// RUN:  -fmodule-file=%t/hu-01.pcm -o %t/hu-05.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-obj %t/main.cpp \
+// RUN:  -Wno-experimental-header-units -fcxx-exceptions \
+// RUN:  -fmodule-file=%t/hu-02.pcm -fmodule-file=%t/hu-05.pcm \
+// RUN:  -fmodule-file=%t/hu-04.pcm -fmodule-file=%t/hu-03.pcm \
+// RUN:  -fmodule-file=%t/hu-01.pcm
+
+//--- hu-01.h
+template 
+struct A {
+  A() {}
+  ~A() {}
+};
+
+template 
+struct EBO : T {
+  EBO() = default;
+};
+
+template 
+struct HT : EBO> {};
+
+//--- hu-02.h
+import "hu-01.h";
+
+inline void f() {
+  HT();
+}
+
+//--- hu-03.h
+import "hu-01.h";
+
+struct C {
+  C();
+
+  HT _;
+};
+
+//--- hu-04.h
+import "hu-01.h";
+
+void g(HT = {});
+
+//--- hu-05.h
+import "hu-03.h";
+import "hu-04.h";
+import "hu-01.h";
+
+struct B {
+  virtual ~B() = default;
+
+  virtual void f() {
+HT();
+  }
+};
+
+//--- main.cpp
+import "hu-02.h";
+import "hu-05.h";
+import "hu-03.h";
+
+int main() {
+  f();
+  C();
+  B();
+}

>From ac5ff8f1d93be542ce8f5a11e0c9dd1d9c9902bf Mon Sep 17 00:00:00 2001
From: Michael Park 
Date: Fri, 27 Dec 2024 17:52:19 -0800
Subject: [PATCH 2/3] [C++20][Modules][Serialization] Delay marking pending
 incomplete decl chains until the end of `finishPendingActions`.

---
 clang/lib/Serialization/ASTReader.cpp | 26 +-
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/clang/lib/Serialization/ASTReader.cpp 
b/clang/lib/Serialization/ASTReader.cpp
index f524251c48ddd7..69e6703dbdeb1c 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -10186,12 +10186,12 @@ void ASTReader::visitTopLevelModuleMaps(
 }
 
 void ASTReader::finishPendingActions() {
-  while (
-  !PendingIdentifierInfos.empty() || !PendingDeducedFunctionTypes.empty() 
||
-  !PendingDeducedVarTypes.empty() || !PendingIncompleteDeclChains.empty() 
||
-  !PendingDeclChains.empty() || !PendingMacroIDs.empty() ||
-  !PendingDeclContextInfos.empty() || !PendingUpdateRecords.empty() ||
-  !PendingObjCExtensionIvarRedeclarations.empty()) {
+  while (!PendingIdentifierInfos.empty() ||
+ !PendingDeducedFunctionTypes.empty() ||
+ !PendingDeducedVarTypes.empty() || !PendingDeclChains.empty() ||
+ !PendingMacroIDs.empty() || !PendingDeclContextInfos.empty() ||
+ !PendingUpdateRecords.empty() ||
+ !PendingObjCExtensionIvarRedeclarations.empty()) {
 // If any identifiers with corresponding top-level declarations have
 // been loaded, load those declarations now.
 using TopLevelDeclsMap =
@@ -10239,13 +10239,6 @@ void ASTReader::finishPendingActions() {
 }
 PendingDeducedVarTypes.clear();
 
-// For each decl chain that we wanted to complete while deserializing, mark
-// it as "still needs to be completed".
-for (unsigned I = 0; I != PendingIncompleteDeclChains.size(); ++I) {
-  markIncompleteDeclChain(PendingIncompleteDeclChains[I]);
-}
-PendingIncompleteDeclChains.clear();
-
 // Load pending declaration chains.
 for (unsigned I = 0; I != PendingDeclChains.size(); ++I)
   loadPendingDeclChain(PendingDeclChains[I].first,
@@ -10483,6 +10476,

[clang] [C++20] [Modules] [Serialization] Delay marking pending incomplete decl chains until the end of `finishPendingActions`. (PR #121245)

[Michael Park via cfe-commits]

https://github.com/mpark updated 
https://github.com/llvm/llvm-project/pull/121245

>From 9879eb1e68c7799f14ddef37cf43f43e65ce735e Mon Sep 17 00:00:00 2001
From: Michael Park 
Date: Fri, 27 Dec 2024 17:52:19 -0800
Subject: [PATCH] Delay marking pending incomplete decl chains until the end of
 `finishPendingActions`.

---
 clang/lib/Serialization/ASTReader.cpp | 26 
 clang/test/Modules/pr121245.cpp   | 90 +++
 2 files changed, 103 insertions(+), 13 deletions(-)
 create mode 100644 clang/test/Modules/pr121245.cpp

diff --git a/clang/lib/Serialization/ASTReader.cpp 
b/clang/lib/Serialization/ASTReader.cpp
index f524251c48ddd7..69e6703dbdeb1c 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -10186,12 +10186,12 @@ void ASTReader::visitTopLevelModuleMaps(
 }
 
 void ASTReader::finishPendingActions() {
-  while (
-  !PendingIdentifierInfos.empty() || !PendingDeducedFunctionTypes.empty() 
||
-  !PendingDeducedVarTypes.empty() || !PendingIncompleteDeclChains.empty() 
||
-  !PendingDeclChains.empty() || !PendingMacroIDs.empty() ||
-  !PendingDeclContextInfos.empty() || !PendingUpdateRecords.empty() ||
-  !PendingObjCExtensionIvarRedeclarations.empty()) {
+  while (!PendingIdentifierInfos.empty() ||
+ !PendingDeducedFunctionTypes.empty() ||
+ !PendingDeducedVarTypes.empty() || !PendingDeclChains.empty() ||
+ !PendingMacroIDs.empty() || !PendingDeclContextInfos.empty() ||
+ !PendingUpdateRecords.empty() ||
+ !PendingObjCExtensionIvarRedeclarations.empty()) {
 // If any identifiers with corresponding top-level declarations have
 // been loaded, load those declarations now.
 using TopLevelDeclsMap =
@@ -10239,13 +10239,6 @@ void ASTReader::finishPendingActions() {
 }
 PendingDeducedVarTypes.clear();
 
-// For each decl chain that we wanted to complete while deserializing, mark
-// it as "still needs to be completed".
-for (unsigned I = 0; I != PendingIncompleteDeclChains.size(); ++I) {
-  markIncompleteDeclChain(PendingIncompleteDeclChains[I]);
-}
-PendingIncompleteDeclChains.clear();
-
 // Load pending declaration chains.
 for (unsigned I = 0; I != PendingDeclChains.size(); ++I)
   loadPendingDeclChain(PendingDeclChains[I].first,
@@ -10483,6 +10476,13 @@ void ASTReader::finishPendingActions() {
   for (auto *ND : PendingMergedDefinitionsToDeduplicate)
 getContext().deduplicateMergedDefinitonsFor(ND);
   PendingMergedDefinitionsToDeduplicate.clear();
+
+  // For each decl chain that we wanted to complete while deserializing, mark
+  // it as "still needs to be completed".
+  for (unsigned I = 0; I != PendingIncompleteDeclChains.size(); ++I) {
+markIncompleteDeclChain(PendingIncompleteDeclChains[I]);
+  }
+  PendingIncompleteDeclChains.clear();
 }
 
 void ASTReader::diagnoseOdrViolations() {
diff --git a/clang/test/Modules/pr121245.cpp b/clang/test/Modules/pr121245.cpp
new file mode 100644
index 00..1ced5f286d3acc
--- /dev/null
+++ b/clang/test/Modules/pr121245.cpp
@@ -0,0 +1,90 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+// RUN: cd %t
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header %t/hu-01.h \
+// RUN:  -fcxx-exceptions -o %t/hu-01.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header %t/hu-02.h \
+// RUN:  -Wno-experimental-header-units -fcxx-exceptions \
+// RUN:  -fmodule-file=%t/hu-01.pcm -o %t/hu-02.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header %t/hu-03.h \
+// RUN:  -Wno-experimental-header-units -fcxx-exceptions \
+// RUN:  -fmodule-file=%t/hu-01.pcm -o %t/hu-03.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header %t/hu-04.h \
+// RUN:  -Wno-experimental-header-units -fcxx-exceptions \
+// RUN:  -fmodule-file=%t/hu-01.pcm -o %t/hu-04.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header %t/hu-05.h \
+// RUN:  -Wno-experimental-header-units -fcxx-exceptions \
+// RUN:  -fmodule-file=%t/hu-03.pcm -fmodule-file=%t/hu-04.pcm \
+// RUN:  -fmodule-file=%t/hu-01.pcm -o %t/hu-05.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-obj %t/main.cpp \
+// RUN:  -Wno-experimental-header-units -fcxx-exceptions \
+// RUN:  -fmodule-file=%t/hu-02.pcm -fmodule-file=%t/hu-05.pcm \
+// RUN:  -fmodule-file=%t/hu-04.pcm -fmodule-file=%t/hu-03.pcm \
+// RUN:  -fmodule-file=%t/hu-01.pcm
+
+//--- hu-01.h
+template 
+struct A {
+  A() {}
+  ~A() {}
+};
+
+template 
+struct EBO : T {
+  EBO() = default;
+};
+
+template 
+struct HT : EBO> {};
+
+//--- hu-02.h
+import "hu-01.h";
+
+inline void f() {
+  HT();
+}
+
+//--- hu-03.h
+import "hu-01.h";
+
+struct C {
+  C();
+
+  HT _;
+};
+
+//--- hu-04.h
+import "hu-01.h";
+
+void g(HT = {});
+
+//--- hu-05.h
+import "hu-03.h";
+import "hu-04.h";
+import "hu-01.h";
+
+struct B {
+  virtual ~B() = default;
+
+  virtual void f() {
+HT();
+  }
+};
+
+//--

[clang] [C++20] [Modules] [Serialization] Delay marking pending incomplete decl chains until the end of `finishPendingActions`. (PR #121245)

[Michael Park via cfe-commits]

https://github.com/mpark updated 
https://github.com/llvm/llvm-project/pull/121245

>From f8d317ce75e51e467487cb10a89e44b73e6b386a Mon Sep 17 00:00:00 2001
From: Michael Park 
Date: Tue, 28 Jan 2025 22:01:37 -0800
Subject: [PATCH 1/2] [C++20][Modules][Serialization] Add a unit test for
 #121245.

---
 clang/test/Modules/pr121245.cpp | 90 +
 1 file changed, 90 insertions(+)
 create mode 100644 clang/test/Modules/pr121245.cpp

diff --git a/clang/test/Modules/pr121245.cpp b/clang/test/Modules/pr121245.cpp
new file mode 100644
index 00..1ced5f286d3acc
--- /dev/null
+++ b/clang/test/Modules/pr121245.cpp
@@ -0,0 +1,90 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+// RUN: cd %t
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header %t/hu-01.h \
+// RUN:  -fcxx-exceptions -o %t/hu-01.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header %t/hu-02.h \
+// RUN:  -Wno-experimental-header-units -fcxx-exceptions \
+// RUN:  -fmodule-file=%t/hu-01.pcm -o %t/hu-02.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header %t/hu-03.h \
+// RUN:  -Wno-experimental-header-units -fcxx-exceptions \
+// RUN:  -fmodule-file=%t/hu-01.pcm -o %t/hu-03.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header %t/hu-04.h \
+// RUN:  -Wno-experimental-header-units -fcxx-exceptions \
+// RUN:  -fmodule-file=%t/hu-01.pcm -o %t/hu-04.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header %t/hu-05.h \
+// RUN:  -Wno-experimental-header-units -fcxx-exceptions \
+// RUN:  -fmodule-file=%t/hu-03.pcm -fmodule-file=%t/hu-04.pcm \
+// RUN:  -fmodule-file=%t/hu-01.pcm -o %t/hu-05.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-obj %t/main.cpp \
+// RUN:  -Wno-experimental-header-units -fcxx-exceptions \
+// RUN:  -fmodule-file=%t/hu-02.pcm -fmodule-file=%t/hu-05.pcm \
+// RUN:  -fmodule-file=%t/hu-04.pcm -fmodule-file=%t/hu-03.pcm \
+// RUN:  -fmodule-file=%t/hu-01.pcm
+
+//--- hu-01.h
+template 
+struct A {
+  A() {}
+  ~A() {}
+};
+
+template 
+struct EBO : T {
+  EBO() = default;
+};
+
+template 
+struct HT : EBO> {};
+
+//--- hu-02.h
+import "hu-01.h";
+
+inline void f() {
+  HT();
+}
+
+//--- hu-03.h
+import "hu-01.h";
+
+struct C {
+  C();
+
+  HT _;
+};
+
+//--- hu-04.h
+import "hu-01.h";
+
+void g(HT = {});
+
+//--- hu-05.h
+import "hu-03.h";
+import "hu-04.h";
+import "hu-01.h";
+
+struct B {
+  virtual ~B() = default;
+
+  virtual void f() {
+HT();
+  }
+};
+
+//--- main.cpp
+import "hu-02.h";
+import "hu-05.h";
+import "hu-03.h";
+
+int main() {
+  f();
+  C();
+  B();
+}

>From e6c5d9a10d18a632c99dc437225e66634c1a Mon Sep 17 00:00:00 2001
From: Michael Park 
Date: Fri, 27 Dec 2024 17:52:19 -0800
Subject: [PATCH 2/2] Delay marking pending incomplete decl chains until the
 end of `finishPendingActions`.

---
 clang/lib/Serialization/ASTReader.cpp | 26 +-
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/clang/lib/Serialization/ASTReader.cpp 
b/clang/lib/Serialization/ASTReader.cpp
index f524251c48ddd7..69e6703dbdeb1c 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -10186,12 +10186,12 @@ void ASTReader::visitTopLevelModuleMaps(
 }
 
 void ASTReader::finishPendingActions() {
-  while (
-  !PendingIdentifierInfos.empty() || !PendingDeducedFunctionTypes.empty() 
||
-  !PendingDeducedVarTypes.empty() || !PendingIncompleteDeclChains.empty() 
||
-  !PendingDeclChains.empty() || !PendingMacroIDs.empty() ||
-  !PendingDeclContextInfos.empty() || !PendingUpdateRecords.empty() ||
-  !PendingObjCExtensionIvarRedeclarations.empty()) {
+  while (!PendingIdentifierInfos.empty() ||
+ !PendingDeducedFunctionTypes.empty() ||
+ !PendingDeducedVarTypes.empty() || !PendingDeclChains.empty() ||
+ !PendingMacroIDs.empty() || !PendingDeclContextInfos.empty() ||
+ !PendingUpdateRecords.empty() ||
+ !PendingObjCExtensionIvarRedeclarations.empty()) {
 // If any identifiers with corresponding top-level declarations have
 // been loaded, load those declarations now.
 using TopLevelDeclsMap =
@@ -10239,13 +10239,6 @@ void ASTReader::finishPendingActions() {
 }
 PendingDeducedVarTypes.clear();
 
-// For each decl chain that we wanted to complete while deserializing, mark
-// it as "still needs to be completed".
-for (unsigned I = 0; I != PendingIncompleteDeclChains.size(); ++I) {
-  markIncompleteDeclChain(PendingIncompleteDeclChains[I]);
-}
-PendingIncompleteDeclChains.clear();
-
 // Load pending declaration chains.
 for (unsigned I = 0; I != PendingDeclChains.size(); ++I)
   loadPendingDeclChain(PendingDeclChains[I].first,
@@ -10483,6 +10476,13 @@ void ASTReader::finishPendingActions() {
   for (auto *ND : PendingMergedDefinitionsToDeduplicate)
 getContext().deduplicateMergedDefinitonsFor(ND);
   PendingMerged

[clang] [C++20] [Modules] [Serialization] Delay marking pending incomplete decl chains until the end of `finishPendingActions`. (PR #121245)

[Michael Park via cfe-commits]

mpark wrote:

Okay folks, I've finally managed to get a reasonable repro as a unit test!

The new unit test, with `Debug` build fails like this:
```
FAIL: Clang :: Modules/pr121245.cpp (8522 of 22154)
 TEST 'Clang :: Modules/pr121245.cpp' FAILED 

Exit Code: 134

Command Output (stderr):
--
RUN: at line 1: rm -rf 
/home/mcypark/llvm-project/build-debug/tools/clang/test/Modules/Output/pr121245.cpp.tmp
+ rm -rf 
/home/mcypark/llvm-project/build-debug/tools/clang/test/Modules/Output/pr121245.cpp.tmp
RUN: at line 2: mkdir -p 
/home/mcypark/llvm-project/build-debug/tools/clang/test/Modules/Output/pr121245.cpp.tmp
+ mkdir -p 
/home/mcypark/llvm-project/build-debug/tools/clang/test/Modules/Output/pr121245.cpp.tmp
RUN: at line 3: split-file 
/home/mcypark/llvm-project/clang/test/Modules/pr121245.cpp 
/home/mcypark/llvm-project/build-debug/tools/clang/test/Modules/Output/pr121245.cpp.tmp
+ split-file /home/mcypark/llvm-project/clang/test/Modules/pr121245.cpp 
/home/mcypark/llvm-project/build-debug/tools/clang/test/Modules/Output/pr121245.cpp.tmp
RUN: at line 4: cd 
/home/mcypark/llvm-project/build-debug/tools/clang/test/Modules/Output/pr121245.cpp.tmp
+ cd 
/home/mcypark/llvm-project/build-debug/tools/clang/test/Modules/Output/pr121245.cpp.tmp
RUN: at line 6: /home/mcypark/llvm-project/build-debug/bin/clang -cc1 
-internal-isystem /home/mcypark/llvm-project/build-debug/lib/clang/21/include 
-nostdsysteminc -std=c++20 -emit-header-unit -xc++-user-header 
/home/mcypark/llvm-project/build-debug/tools/clang/test/Modules/Output/pr121245.cpp.tmp/hu-01.h
   -fcxx-exceptions -o 
/home/mcypark/llvm-project/build-debug/tools/clang/test/Modules/Output/pr121245.cpp.tmp/hu-01.pcm
+ /home/mcypark/llvm-project/build-debug/bin/clang -cc1 -internal-isystem 
/home/mcypark/llvm-project/build-debug/lib/clang/21/include -nostdsysteminc 
-std=c++20 -emit-header-unit -xc++-user-header 
/home/mcypark/llvm-project/build-debug/tools/clang/test/Modules/Output/pr121245.cpp.tmp/hu-01.h
 -fcxx-exceptions -o 
/home/mcypark/llvm-project/build-debug/tools/clang/test/Modules/Output/pr121245.cpp.tmp/hu-01.pcm
RUN: at line 9: /home/mcypark/llvm-project/build-debug/bin/clang -cc1 
-internal-isystem /home/mcypark/llvm-project/build-debug/lib/clang/21/include 
-nostdsysteminc -std=c++20 -emit-header-unit -xc++-user-header 
/home/mcypark/llvm-project/build-debug/tools/clang/test/Modules/Output/pr121245.cpp.tmp/hu-02.h
   -Wno-experimental-header-units -fcxx-exceptions   
-fmodule-file=/home/mcypark/llvm-project/build-debug/tools/clang/test/Modules/Output/pr121245.cpp.tmp/hu-01.pcm
 -o 
/home/mcypark/llvm-project/build-debug/tools/clang/test/Modules/Output/pr121245.cpp.tmp/hu-02.pcm
+ /home/mcypark/llvm-project/build-debug/bin/clang -cc1 -internal-isystem 
/home/mcypark/llvm-project/build-debug/lib/clang/21/include -nostdsysteminc 
-std=c++20 -emit-header-unit -xc++-user-header 
/home/mcypark/llvm-project/build-debug/tools/clang/test/Modules/Output/pr121245.cpp.tmp/hu-02.h
 -Wno-experimental-header-units -fcxx-exceptions 
-fmodule-file=/home/mcypark/llvm-project/build-debug/tools/clang/test/Modules/Output/pr121245.cpp.tmp/hu-01.pcm
 -o 
/home/mcypark/llvm-project/build-debug/tools/clang/test/Modules/Output/pr121245.cpp.tmp/hu-02.pcm
RUN: at line 13: /home/mcypark/llvm-project/build-debug/bin/clang -cc1 
-internal-isystem /home/mcypark/llvm-project/build-debug/lib/clang/21/include 
-nostdsysteminc -std=c++20 -emit-header-unit -xc++-user-header 
/home/mcypark/llvm-project/build-debug/tools/clang/test/Modules/Output/pr121245.cpp.tmp/hu-03.h
   -Wno-experimental-header-units -fcxx-exceptions   
-fmodule-file=/home/mcypark/llvm-project/build-debug/tools/clang/test/Modules/Output/pr121245.cpp.tmp/hu-01.pcm
 -o 
/home/mcypark/llvm-project/build-debug/tools/clang/test/Modules/Output/pr121245.cpp.tmp/hu-03.pcm
+ /home/mcypark/llvm-project/build-debug/bin/clang -cc1 -internal-isystem 
/home/mcypark/llvm-project/build-debug/lib/clang/21/include -nostdsysteminc 
-std=c++20 -emit-header-unit -xc++-user-header 
/home/mcypark/llvm-project/build-debug/tools/clang/test/Modules/Output/pr121245.cpp.tmp/hu-03.h
 -Wno-experimental-header-units -fcxx-exceptions 
-fmodule-file=/home/mcypark/llvm-project/build-debug/tools/clang/test/Modules/Output/pr121245.cpp.tmp/hu-01.pcm
 -o 
/home/mcypark/llvm-project/build-debug/tools/clang/test/Modules/Output/pr121245.cpp.tmp/hu-03.pcm
RUN: at line 17: /home/mcypark/llvm-project/build-debug/bin/clang -cc1 
-internal-isystem /home/mcypark/llvm-project/build-debug/lib/clang/21/include 
-nostdsysteminc -std=c++20 -emit-header-unit -xc++-user-header 
/home/mcypark/llvm-project/build-debug/tools/clang/test/Modules/Output/pr121245.cpp.tmp/hu-04.h
   -Wno-experimental-header-units -fcxx-exceptions   
-fmodule-file=/home/mcypark/llvm-project/build-debug/tools/clang/test/Modules/Output/pr121245.cpp.tmp/hu-01.pcm
 -o 
/home/mcypark/llvm-project/build-debug/tools/clang/test/Modules/Output/pr121245.cpp.tmp/hu-0

[clang] [C++20] [Modules] [Serialization] Delay marking pending incomplete decl chains until the end of `finishPendingActions`. (PR #121245)

[Michael Park via cfe-commits]

https://github.com/mpark updated 
https://github.com/llvm/llvm-project/pull/121245

>From f8d317ce75e51e467487cb10a89e44b73e6b386a Mon Sep 17 00:00:00 2001
From: Michael Park 
Date: Tue, 28 Jan 2025 22:01:37 -0800
Subject: [PATCH 1/2] [C++20][Modules][Serialization] Add a unit test for
 #121245.

---
 clang/test/Modules/pr121245.cpp | 90 +
 1 file changed, 90 insertions(+)
 create mode 100644 clang/test/Modules/pr121245.cpp

diff --git a/clang/test/Modules/pr121245.cpp b/clang/test/Modules/pr121245.cpp
new file mode 100644
index 00..1ced5f286d3acc
--- /dev/null
+++ b/clang/test/Modules/pr121245.cpp
@@ -0,0 +1,90 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+// RUN: cd %t
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header %t/hu-01.h \
+// RUN:  -fcxx-exceptions -o %t/hu-01.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header %t/hu-02.h \
+// RUN:  -Wno-experimental-header-units -fcxx-exceptions \
+// RUN:  -fmodule-file=%t/hu-01.pcm -o %t/hu-02.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header %t/hu-03.h \
+// RUN:  -Wno-experimental-header-units -fcxx-exceptions \
+// RUN:  -fmodule-file=%t/hu-01.pcm -o %t/hu-03.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header %t/hu-04.h \
+// RUN:  -Wno-experimental-header-units -fcxx-exceptions \
+// RUN:  -fmodule-file=%t/hu-01.pcm -o %t/hu-04.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header %t/hu-05.h \
+// RUN:  -Wno-experimental-header-units -fcxx-exceptions \
+// RUN:  -fmodule-file=%t/hu-03.pcm -fmodule-file=%t/hu-04.pcm \
+// RUN:  -fmodule-file=%t/hu-01.pcm -o %t/hu-05.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-obj %t/main.cpp \
+// RUN:  -Wno-experimental-header-units -fcxx-exceptions \
+// RUN:  -fmodule-file=%t/hu-02.pcm -fmodule-file=%t/hu-05.pcm \
+// RUN:  -fmodule-file=%t/hu-04.pcm -fmodule-file=%t/hu-03.pcm \
+// RUN:  -fmodule-file=%t/hu-01.pcm
+
+//--- hu-01.h
+template 
+struct A {
+  A() {}
+  ~A() {}
+};
+
+template 
+struct EBO : T {
+  EBO() = default;
+};
+
+template 
+struct HT : EBO> {};
+
+//--- hu-02.h
+import "hu-01.h";
+
+inline void f() {
+  HT();
+}
+
+//--- hu-03.h
+import "hu-01.h";
+
+struct C {
+  C();
+
+  HT _;
+};
+
+//--- hu-04.h
+import "hu-01.h";
+
+void g(HT = {});
+
+//--- hu-05.h
+import "hu-03.h";
+import "hu-04.h";
+import "hu-01.h";
+
+struct B {
+  virtual ~B() = default;
+
+  virtual void f() {
+HT();
+  }
+};
+
+//--- main.cpp
+import "hu-02.h";
+import "hu-05.h";
+import "hu-03.h";
+
+int main() {
+  f();
+  C();
+  B();
+}

>From 2a5a87e62ebfff8f92eebf3f598ac87a359d4e53 Mon Sep 17 00:00:00 2001
From: Michael Park 
Date: Fri, 27 Dec 2024 17:52:19 -0800
Subject: [PATCH 2/2] [C++20][Modules][Serialization] Delay marking pending
 incomplete decl chains until the end of `finishPendingActions`.

---
 clang/lib/Serialization/ASTReader.cpp | 26 +-
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/clang/lib/Serialization/ASTReader.cpp 
b/clang/lib/Serialization/ASTReader.cpp
index f524251c48ddd7..69e6703dbdeb1c 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -10186,12 +10186,12 @@ void ASTReader::visitTopLevelModuleMaps(
 }
 
 void ASTReader::finishPendingActions() {
-  while (
-  !PendingIdentifierInfos.empty() || !PendingDeducedFunctionTypes.empty() 
||
-  !PendingDeducedVarTypes.empty() || !PendingIncompleteDeclChains.empty() 
||
-  !PendingDeclChains.empty() || !PendingMacroIDs.empty() ||
-  !PendingDeclContextInfos.empty() || !PendingUpdateRecords.empty() ||
-  !PendingObjCExtensionIvarRedeclarations.empty()) {
+  while (!PendingIdentifierInfos.empty() ||
+ !PendingDeducedFunctionTypes.empty() ||
+ !PendingDeducedVarTypes.empty() || !PendingDeclChains.empty() ||
+ !PendingMacroIDs.empty() || !PendingDeclContextInfos.empty() ||
+ !PendingUpdateRecords.empty() ||
+ !PendingObjCExtensionIvarRedeclarations.empty()) {
 // If any identifiers with corresponding top-level declarations have
 // been loaded, load those declarations now.
 using TopLevelDeclsMap =
@@ -10239,13 +10239,6 @@ void ASTReader::finishPendingActions() {
 }
 PendingDeducedVarTypes.clear();
 
-// For each decl chain that we wanted to complete while deserializing, mark
-// it as "still needs to be completed".
-for (unsigned I = 0; I != PendingIncompleteDeclChains.size(); ++I) {
-  markIncompleteDeclChain(PendingIncompleteDeclChains[I]);
-}
-PendingIncompleteDeclChains.clear();
-
 // Load pending declaration chains.
 for (unsigned I = 0; I != PendingDeclChains.size(); ++I)
   loadPendingDeclChain(PendingDeclChains[I].first,
@@ -10483,6 +10476,13 @@ void ASTReader::finishPendingActions() {
   for (auto *ND : PendingMergedDefinitionsToDeduplicate)
 getContext().deduplicateMergedDef

[clang] [C++20][Modules][Serialization] Delay marking pending incomplete decl chains until the end of `finishPendingActions`. (PR #121245)

[Michael Park via cfe-commits]

https://github.com/mpark edited https://github.com/llvm/llvm-project/pull/121245
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C++20][Modules][Serialization] Delay marking pending incomplete decl chains until the end of `finishPendingActions`. (PR #121245)

[Michael Park via cfe-commits]

mpark wrote:

> Presumably, we want to cherry pick that in clang 20?

Yeah, I'd think so if it's not too late.

https://github.com/llvm/llvm-project/pull/121245
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C++20][Modules][Serialization] Delay marking pending incomplete decl chains until the end of `finishPendingActions`. (PR #121245)

[Michael Park via cfe-commits]

https://github.com/mpark updated 
https://github.com/llvm/llvm-project/pull/121245

>From f8d317ce75e51e467487cb10a89e44b73e6b386a Mon Sep 17 00:00:00 2001
From: Michael Park 
Date: Tue, 28 Jan 2025 22:01:37 -0800
Subject: [PATCH 1/3] [C++20][Modules][Serialization] Add a unit test for
 #121245.

---
 clang/test/Modules/pr121245.cpp | 90 +
 1 file changed, 90 insertions(+)
 create mode 100644 clang/test/Modules/pr121245.cpp

diff --git a/clang/test/Modules/pr121245.cpp b/clang/test/Modules/pr121245.cpp
new file mode 100644
index 00..1ced5f286d3acc
--- /dev/null
+++ b/clang/test/Modules/pr121245.cpp
@@ -0,0 +1,90 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+// RUN: cd %t
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header %t/hu-01.h \
+// RUN:  -fcxx-exceptions -o %t/hu-01.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header %t/hu-02.h \
+// RUN:  -Wno-experimental-header-units -fcxx-exceptions \
+// RUN:  -fmodule-file=%t/hu-01.pcm -o %t/hu-02.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header %t/hu-03.h \
+// RUN:  -Wno-experimental-header-units -fcxx-exceptions \
+// RUN:  -fmodule-file=%t/hu-01.pcm -o %t/hu-03.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header %t/hu-04.h \
+// RUN:  -Wno-experimental-header-units -fcxx-exceptions \
+// RUN:  -fmodule-file=%t/hu-01.pcm -o %t/hu-04.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header %t/hu-05.h \
+// RUN:  -Wno-experimental-header-units -fcxx-exceptions \
+// RUN:  -fmodule-file=%t/hu-03.pcm -fmodule-file=%t/hu-04.pcm \
+// RUN:  -fmodule-file=%t/hu-01.pcm -o %t/hu-05.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-obj %t/main.cpp \
+// RUN:  -Wno-experimental-header-units -fcxx-exceptions \
+// RUN:  -fmodule-file=%t/hu-02.pcm -fmodule-file=%t/hu-05.pcm \
+// RUN:  -fmodule-file=%t/hu-04.pcm -fmodule-file=%t/hu-03.pcm \
+// RUN:  -fmodule-file=%t/hu-01.pcm
+
+//--- hu-01.h
+template 
+struct A {
+  A() {}
+  ~A() {}
+};
+
+template 
+struct EBO : T {
+  EBO() = default;
+};
+
+template 
+struct HT : EBO> {};
+
+//--- hu-02.h
+import "hu-01.h";
+
+inline void f() {
+  HT();
+}
+
+//--- hu-03.h
+import "hu-01.h";
+
+struct C {
+  C();
+
+  HT _;
+};
+
+//--- hu-04.h
+import "hu-01.h";
+
+void g(HT = {});
+
+//--- hu-05.h
+import "hu-03.h";
+import "hu-04.h";
+import "hu-01.h";
+
+struct B {
+  virtual ~B() = default;
+
+  virtual void f() {
+HT();
+  }
+};
+
+//--- main.cpp
+import "hu-02.h";
+import "hu-05.h";
+import "hu-03.h";
+
+int main() {
+  f();
+  C();
+  B();
+}

>From 2a5a87e62ebfff8f92eebf3f598ac87a359d4e53 Mon Sep 17 00:00:00 2001
From: Michael Park 
Date: Fri, 27 Dec 2024 17:52:19 -0800
Subject: [PATCH 2/3] [C++20][Modules][Serialization] Delay marking pending
 incomplete decl chains until the end of `finishPendingActions`.

---
 clang/lib/Serialization/ASTReader.cpp | 26 +-
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/clang/lib/Serialization/ASTReader.cpp 
b/clang/lib/Serialization/ASTReader.cpp
index f524251c48ddd7..69e6703dbdeb1c 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -10186,12 +10186,12 @@ void ASTReader::visitTopLevelModuleMaps(
 }
 
 void ASTReader::finishPendingActions() {
-  while (
-  !PendingIdentifierInfos.empty() || !PendingDeducedFunctionTypes.empty() 
||
-  !PendingDeducedVarTypes.empty() || !PendingIncompleteDeclChains.empty() 
||
-  !PendingDeclChains.empty() || !PendingMacroIDs.empty() ||
-  !PendingDeclContextInfos.empty() || !PendingUpdateRecords.empty() ||
-  !PendingObjCExtensionIvarRedeclarations.empty()) {
+  while (!PendingIdentifierInfos.empty() ||
+ !PendingDeducedFunctionTypes.empty() ||
+ !PendingDeducedVarTypes.empty() || !PendingDeclChains.empty() ||
+ !PendingMacroIDs.empty() || !PendingDeclContextInfos.empty() ||
+ !PendingUpdateRecords.empty() ||
+ !PendingObjCExtensionIvarRedeclarations.empty()) {
 // If any identifiers with corresponding top-level declarations have
 // been loaded, load those declarations now.
 using TopLevelDeclsMap =
@@ -10239,13 +10239,6 @@ void ASTReader::finishPendingActions() {
 }
 PendingDeducedVarTypes.clear();
 
-// For each decl chain that we wanted to complete while deserializing, mark
-// it as "still needs to be completed".
-for (unsigned I = 0; I != PendingIncompleteDeclChains.size(); ++I) {
-  markIncompleteDeclChain(PendingIncompleteDeclChains[I]);
-}
-PendingIncompleteDeclChains.clear();
-
 // Load pending declaration chains.
 for (unsigned I = 0; I != PendingDeclChains.size(); ++I)
   loadPendingDeclChain(PendingDeclChains[I].first,
@@ -10483,6 +10476,13 @@ void ASTReader::finishPendingActions() {
   for (auto *ND : PendingMergedDefinitionsToDeduplicate)
 getContext().deduplicateMergedDef

[clang] [C++20][Modules][Serialization] Delay marking pending incomplete decl chains until the end of `finishPendingActions`. (PR #121245)

[Michael Park via cfe-commits]

@@ -10483,6 +10476,13 @@ void ASTReader::finishPendingActions() {
   for (auto *ND : PendingMergedDefinitionsToDeduplicate)
 getContext().deduplicateMergedDefinitonsFor(ND);
   PendingMergedDefinitionsToDeduplicate.clear();
+
+  // For each decl chain that we wanted to complete while deserializing, mark
+  // it as "still needs to be completed".
+  for (unsigned I = 0; I != PendingIncompleteDeclChains.size(); ++I) {
+markIncompleteDeclChain(PendingIncompleteDeclChains[I]);
+  }

mpark wrote:

Yeah, why not

https://github.com/llvm/llvm-project/pull/121245
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C++20][Modules][Serialization] Delay marking pending incomplete decl chains until the end of `finishPendingActions`. (PR #121245)

[Michael Park via cfe-commits]

mpark wrote:

Hi @zixu-w, thanks for the ping. I can't take a look this week, but can on 
Monday. It would be great if I can look at a small repro though. I'll follow on 
the issue.

https://github.com/llvm/llvm-project/pull/121245
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [lldb] [C++20][Modules] Do not update the declaration generation number if the redeclaration chain completion was delayed. (PR #129982)

[Michael Park via cfe-commits]

@@ -9180,6 +9180,12 @@ bool Sema::hasAcceptableDefinition(NamedDecl *D, 
NamedDecl **Suggested,
   if (!getLangOpts().Modules && !getLangOpts().ModulesLocalVisibility)
 return true;
 
+  // The external source may have additional definitions of this entity that 
are
+  // visible, so complete the redeclaration chain now.
+  if (auto *Source = Context.getExternalSource()) {
+Source->CompleteRedeclChain(D);
+  }

mpark wrote:

Further findings: in this case, what I'm seeing is that 
[`LazyGenerationalUpdatePtr::get`](https://github.com/llvm/llvm-project/blob/main/clang/include/clang/AST/ExternalASTSource.h#L487-L497)
 is called twice for [4].

The first invocation, the generation numbers don't match, and 
`CompleteRedeclChain` is called (*outside* of the deserialization stage) and 
gets to 
[`DC->lookup(Name);`](https://github.com/llvm/llvm-project/blob/c6d95c441a29a45782ff72d6cb82839b86fd0e4a/clang/lib/Serialization/ASTReader.cpp#L7862).
 We don't discover the definition in the first invocation.

The second invocation is from 
[here](https://github.com/llvm/llvm-project/blob/c6d95c441a29a45782ff72d6cb82839b86fd0e4a/clang/lib/Sema/SemaType.cpp#L9174)
 (via the `getMostRecentDecl()` call within `getDefinition`). This time, the 
generation numbers **match**, and `CompleteRedeclChain` is **not** called. 
However, if I explicitly invoke `CompleteRedeclChain` on it, it gets to the 
same `DC->lookup(Name);` line as before with exactly the same values for `DC` 
and `Name`. However, this time the definition **is** found. To be clear, this 
is invoking `CompleteRedeclChain` on [4] **directly** rather than on [1], [2] 
or [3], and also not **indirectly** via something like `getMostRecentDecl()` or 
`redecls()` since `CompleteRedeclChain` will not be called through those 
functions because the generation numbers match.

In short, I'm seeing that the `DC->lookup(Name)` has different behaviors 
between two invocations, without either generation numbers changing. I wonder 
if maybe another module was loaded in between the two calls, and the 
`ExternalASTSource`'s generation number should have been incremented but it 
wasn't for some reason...

https://github.com/llvm/llvm-project/pull/129982
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [lldb] [C++20][Modules] Do not update the declaration generation number if the redeclaration chain completion was delayed. (PR #129982)

[Michael Park via cfe-commits]

https://github.com/mpark edited https://github.com/llvm/llvm-project/pull/129982
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [lldb] [C++20][Modules] Do not update the declaration generation number if the redeclaration chain completion was delayed. (PR #129982)

[Michael Park via cfe-commits]

@@ -9180,6 +9180,12 @@ bool Sema::hasAcceptableDefinition(NamedDecl *D, 
NamedDecl **Suggested,
   if (!getLangOpts().Modules && !getLangOpts().ModulesLocalVisibility)
 return true;
 
+  // The external source may have additional definitions of this entity that 
are
+  // visible, so complete the redeclaration chain now.
+  if (auto *Source = Context.getExternalSource()) {
+Source->CompleteRedeclChain(D);
+  }

mpark wrote:

Hm, okay... so the `DC->lookup(Name)` have different behaviors because of 
[`Source->FindExternalVisibleDeclsByName(...)`](https://github.com/llvm/llvm-project/blob/release/20.x/clang/lib/AST/DeclBase.cpp#L1913).
 In `ASTReader::FindExternalVisibleDeclsByName`, we get to 
[here](https://github.com/llvm/llvm-project/blob/c6d95c441a29a45782ff72d6cb82839b86fd0e4a/clang/lib/Serialization/ASTReader.cpp#L8494-L8501)
 looking through `Lookups` data structure. In the first call, 
`Lookups[DC].Table[Name]` has 1 entry (just a decl), whereas in the second call 
it has 2 entries (the decl and a defn).

In summary, we have `LazyGenerationalUpdatePtr::get` calling 
`CompleteRedeclChain` where `Lookups` isn't populated with the definition, and 
updates the generation number. The second time `LazyGenerationUpdatePtr::get` 
is called, `Lookups` **is** populated with the definition, but the generation 
number matches, and so it doesn't call `CompleteRedeclChain` even though it 
would yield a different result.

https://github.com/llvm/llvm-project/pull/129982
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [lldb] [C++20][Modules] Do not update the declaration generation number if the redeclaration chain completion was delayed. (PR #129982)

[Michael Park via cfe-commits]

https://github.com/mpark edited https://github.com/llvm/llvm-project/pull/129982
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [lldb] [C++20][Modules] Do not update the declaration generation number if the redeclaration chain completion was delayed. (PR #129982)

[Michael Park via cfe-commits]

@@ -9180,6 +9180,12 @@ bool Sema::hasAcceptableDefinition(NamedDecl *D, 
NamedDecl **Suggested,
   if (!getLangOpts().Modules && !getLangOpts().ModulesLocalVisibility)
 return true;
 
+  // The external source may have additional definitions of this entity that 
are
+  // visible, so complete the redeclaration chain now.
+  if (auto *Source = Context.getExternalSource()) {
+Source->CompleteRedeclChain(D);
+  }

mpark wrote:

On one thought, it seems like the decls inside `DC` should be notified of the 
fact that there have been new additions made to `Lookups[DC]`... but on the 
other hand, it's not clear to me whether this is an expected sequence of events 
in the first place. If this is an expected sequence of events, I'm a bit 
surprised that this doesn't seem to be a bigger issue. If this is not an 
expected sequence of events, do we expect that the first call to 
`CompleteRedeclChain` should have the `Lookups` populated with everything 
already?

CC @ChuanqiXu9, maybe @Bigcheese can help here too?

https://github.com/llvm/llvm-project/pull/129982
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [lldb] [C++20][Modules] Do not update the declaration generation number if the redeclaration chain completion was delayed. (PR #129982)

[Michael Park via cfe-commits]

https://github.com/mpark edited https://github.com/llvm/llvm-project/pull/129982
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [lldb] [C++20][Modules] Do not update the declaration generation number if the redeclaration chain completion was delayed. (PR #129982)

[Michael Park via cfe-commits]

https://github.com/mpark edited https://github.com/llvm/llvm-project/pull/129982
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C++20][Modules] Do not update the declaration generation number if the redeclaration chain completion was delayed. (PR #129982)

[Michael Park via cfe-commits]

https://github.com/mpark updated 
https://github.com/llvm/llvm-project/pull/129982

>From 39b845fd64c2ce173b95ddcafbbab2d5116aa68e Mon Sep 17 00:00:00 2001
From: Michael Park 
Date: Wed, 5 Mar 2025 18:46:10 -0800
Subject: [PATCH 1/2] [clang][modules] Add a unit test for the assertion
 failure in bootstrap build on OS X with XCode.

---
 clang/test/Modules/pr129982.cpp | 107 
 1 file changed, 107 insertions(+)
 create mode 100644 clang/test/Modules/pr129982.cpp

diff --git a/clang/test/Modules/pr129982.cpp b/clang/test/Modules/pr129982.cpp
new file mode 100644
index 0..b19cccfd07687
--- /dev/null
+++ b/clang/test/Modules/pr129982.cpp
@@ -0,0 +1,107 @@
+// If this test fails, it should be investigated under Debug builds.
+// Before the PR, this test was violating an assertion.
+
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+
+// RUN: %clang_cc1 -std=c++20 -emit-obj -fmodules \
+// RUN:  -fmodule-map-file=%t/module.modulemap \
+// RUN:  -fmodules-cache-path=%t %t/a.cpp
+
+//--- module.modulemap
+module ebo {
+  header "ebo.h"
+}
+
+module fwd {
+  header "fwd.h"
+}
+
+module s {
+  header "s.h"
+  export *
+}
+
+module mod {
+  header "a.h"
+  header "b.h"
+}
+
+//--- ebo.h
+#pragma once
+
+namespace N { inline namespace __1 {
+
+template 
+struct EBO : T {
+  EBO() = default;
+};
+
+}}
+
+//--- fwd.h
+#pragma once
+
+namespace N { inline namespace __1 {
+
+template 
+struct Empty;
+
+template 
+struct BS;
+
+using S = BS>;
+
+}}
+
+//--- s.h
+#pragma once
+
+#include "fwd.h"
+#include "ebo.h"
+
+namespace N { inline namespace __1 {
+
+template 
+struct Empty {};
+
+template 
+struct BS {
+EBO _;
+void f();
+};
+
+extern template void BS>::f();
+
+}}
+
+//--- b.h
+#pragma once
+
+#include "s.h"
+
+struct B {
+  void f() {
+N::S{}.f();
+  }
+};
+
+//--- a.h
+#pragma once
+
+#include "s.h"
+
+struct A {
+  void f(int) {}
+  void f(const N::S &) {}
+
+  void g();
+};
+
+//--- a.cpp
+#include "a.h"
+
+void A::g() { f(0); }
+
+// expected-no-diagnostics

>From e38c598fc4ff7226287883f5cb6a8355f576db38 Mon Sep 17 00:00:00 2001
From: Michael Park 
Date: Fri, 7 Mar 2025 01:09:38 -0800
Subject: [PATCH 2/2] =?UTF-8?q?Reapply=20"[C++20][Modules][Serialization]?=
 =?UTF-8?q?=20Delay=20marking=20pending=20incompl=E2=80=A6=20(#127136)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This reverts commit 912b154f3a3f8c3cebf5cc5731fd8b0749762da5.
---
 clang/lib/Serialization/ASTReader.cpp | 25 ---
 clang/test/Modules/pr121245.cpp   | 93 +++
 2 files changed, 105 insertions(+), 13 deletions(-)
 create mode 100644 clang/test/Modules/pr121245.cpp

diff --git a/clang/lib/Serialization/ASTReader.cpp 
b/clang/lib/Serialization/ASTReader.cpp
index ca09c3d79d941..17d07f8535346 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -10178,12 +10178,12 @@ void ASTReader::visitTopLevelModuleMaps(
 }
 
 void ASTReader::finishPendingActions() {
-  while (
-  !PendingIdentifierInfos.empty() || !PendingDeducedFunctionTypes.empty() 
||
-  !PendingDeducedVarTypes.empty() || !PendingIncompleteDeclChains.empty() 
||
-  !PendingDeclChains.empty() || !PendingMacroIDs.empty() ||
-  !PendingDeclContextInfos.empty() || !PendingUpdateRecords.empty() ||
-  !PendingObjCExtensionIvarRedeclarations.empty()) {
+  while (!PendingIdentifierInfos.empty() ||
+ !PendingDeducedFunctionTypes.empty() ||
+ !PendingDeducedVarTypes.empty() || !PendingDeclChains.empty() ||
+ !PendingMacroIDs.empty() || !PendingDeclContextInfos.empty() ||
+ !PendingUpdateRecords.empty() ||
+ !PendingObjCExtensionIvarRedeclarations.empty()) {
 // If any identifiers with corresponding top-level declarations have
 // been loaded, load those declarations now.
 using TopLevelDeclsMap =
@@ -10231,13 +10231,6 @@ void ASTReader::finishPendingActions() {
 }
 PendingDeducedVarTypes.clear();
 
-// For each decl chain that we wanted to complete while deserializing, mark
-// it as "still needs to be completed".
-for (unsigned I = 0; I != PendingIncompleteDeclChains.size(); ++I) {
-  markIncompleteDeclChain(PendingIncompleteDeclChains[I]);
-}
-PendingIncompleteDeclChains.clear();
-
 // Load pending declaration chains.
 for (unsigned I = 0; I != PendingDeclChains.size(); ++I)
   loadPendingDeclChain(PendingDeclChains[I].first,
@@ -10475,6 +10468,12 @@ void ASTReader::finishPendingActions() {
   for (auto *ND : PendingMergedDefinitionsToDeduplicate)
 getContext().deduplicateMergedDefinitonsFor(ND);
   PendingMergedDefinitionsToDeduplicate.clear();
+
+  // For each decl chain that we wanted to complete while deserializing, mark
+  // it as "still needs to be completed".
+  for (Decl *D : PendingIncompleteDeclChains)
+markIncompleteDeclChain(D);
+  PendingIncompleteDeclChains.clear();
 }

[clang] [lldb] [C++20][Modules] Do not update the declaration generation number if the redeclaration chain completion was delayed. (PR #129982)

[Michael Park via cfe-commits]

https://github.com/mpark converted_to_draft 
https://github.com/llvm/llvm-project/pull/129982
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C++20][Modules] Fix incomplete decl chains (PR #129982)

[Michael Park via cfe-commits]

https://github.com/mpark edited https://github.com/llvm/llvm-project/pull/129982
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C++20][Modules] Fix incomplete decl chains (PR #129982)

[Michael Park via cfe-commits]

https://github.com/mpark edited https://github.com/llvm/llvm-project/pull/129982
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C++20][Modules] Do not update the declaration generation number if the redeclaration chain completion was delayed. (PR #129982)

[Michael Park via cfe-commits]

@@ -9180,6 +9180,12 @@ bool Sema::hasAcceptableDefinition(NamedDecl *D, 
NamedDecl **Suggested,
   if (!getLangOpts().Modules && !getLangOpts().ModulesLocalVisibility)
 return true;
 
+  // The external source may have additional definitions of this entity that 
are
+  // visible, so complete the redeclaration chain now.
+  if (auto *Source = Context.getExternalSource()) {
+Source->CompleteRedeclChain(D);
+  }

mpark wrote:

CC @cor3ntin, @mizvekov, @dmpolukhin since they helped look at #121245 and 
might have some answers around invariants/expectations here.

https://github.com/llvm/llvm-project/pull/129982
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C++20][Modules] Fix incomplete decl chains (PR #129982)

[Michael Park via cfe-commits]

@@ -9180,6 +9180,12 @@ bool Sema::hasAcceptableDefinition(NamedDecl *D, 
NamedDecl **Suggested,
   if (!getLangOpts().Modules && !getLangOpts().ModulesLocalVisibility)
 return true;
 
+  // The external source may have additional definitions of this entity that 
are
+  // visible, so complete the redeclaration chain now.
+  if (auto *Source = Context.getExternalSource()) {
+Source->CompleteRedeclChain(D);
+  }

mpark wrote:

Yeah, I agree. I'm kind of trying to figure out what the most recent pattern 
(😂) is for most recent decls.

Some options I've come across are:

1. `CompleteRedeclChain` if `getExternalSource()` is present. e.g. 
[SemaType.cpp](https://github.com/llvm/llvm-project/blob/release/20.x/clang/lib/Sema/SemaType.cpp#L9225-L9230)
 (current state of the PR)
```cpp
  // The external source may have additional definitions of this entity that are
  // visible, so complete the redeclaration chain now.
  if (auto *Source = Context.getExternalSource()) {
Source->CompleteRedeclChain(D);
  }
```

2. "Dummy" invocation of `getMostRecentDecl()` if `getExternalSource()` is 
present. e.g. 
[DeclBase.cpp](https://github.com/llvm/llvm-project/blob/release/20.x/clang/lib/AST/DeclBase.cpp#L1884-L1889)
```cpp
  // If we have an external source, ensure that any later redeclarations of this
  // context have been loaded, since they may add names to the result of this
  // lookup (or add external visible storage).
  ExternalASTSource *Source = getParentASTContext().getExternalSource();
  if (Source)
(void)cast(this)->getMostRecentDecl();
```

3. Unconditional "dummy" invocation of `getMostRecentDecl()`. e.g. 
[CXXRecordDecl::dataPtr](https://github.com/llvm/llvm-project/blob/release/20.x/clang/include/clang/AST/DeclCXX.h#L457-L461)
 and 
[RecordLayoutBuilder.cpp](https://github.com/llvm/llvm-project/blob/release/20.x/clang/lib/AST/RecordLayoutBuilder.cpp#L3329-L3330)

I'm not quite sure I fully understood what you meant by:
> I feel better to do this in redecls.

We don't need to iterate through the `redecls()` here, and while I see dummy 
invocations of `getMostRecentDecl` as a way to trigger redecl completion, I 
don't see examples of `redecls` being called to force that to happen.

https://github.com/llvm/llvm-project/pull/129982
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C++20][Modules] Fix incomplete decl chains (PR #129982)

[Michael Park via cfe-commits]

https://github.com/mpark edited https://github.com/llvm/llvm-project/pull/129982
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C++20][Modules] Fix incomplete decl chains (PR #129982)

[Michael Park via cfe-commits]

@@ -10178,12 +10178,12 @@ void ASTReader::visitTopLevelModuleMaps(
 }
 
 void ASTReader::finishPendingActions() {
-  while (
-  !PendingIdentifierInfos.empty() || !PendingDeducedFunctionTypes.empty() 
||
-  !PendingDeducedVarTypes.empty() || !PendingIncompleteDeclChains.empty() 
||
-  !PendingDeclChains.empty() || !PendingMacroIDs.empty() ||
-  !PendingDeclContextInfos.empty() || !PendingUpdateRecords.empty() ||
-  !PendingObjCExtensionIvarRedeclarations.empty()) {
+  while (!PendingIdentifierInfos.empty() ||
+ !PendingDeducedFunctionTypes.empty() ||
+ !PendingDeducedVarTypes.empty() || !PendingDeclChains.empty() ||
+ !PendingMacroIDs.empty() || !PendingDeclContextInfos.empty() ||
+ !PendingUpdateRecords.empty() ||
+ !PendingObjCExtensionIvarRedeclarations.empty()) {

mpark wrote:

This is the effect of reapplying #121245, which moved 
`PendingIncompleteDeclChains` out of this loop.

https://github.com/llvm/llvm-project/pull/129982
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C++20][Modules] Fix incomplete decl chains (PR #129982)

[Michael Park via cfe-commits]

https://github.com/mpark edited https://github.com/llvm/llvm-project/pull/129982
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C++20][Modules] Fix incomplete decl chains (PR #129982)

[Michael Park via cfe-commits]

https://github.com/mpark edited https://github.com/llvm/llvm-project/pull/129982
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C++20][Modules] Fix incomplete decl chains (PR #129982)

[Michael Park via cfe-commits]

https://github.com/mpark edited https://github.com/llvm/llvm-project/pull/129982
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C++20][Modules] Fix incomplete decl chains (PR #129982)

[Michael Park via cfe-commits]

@@ -9180,6 +9180,12 @@ bool Sema::hasAcceptableDefinition(NamedDecl *D, 
NamedDecl **Suggested,
   if (!getLangOpts().Modules && !getLangOpts().ModulesLocalVisibility)
 return true;
 
+  // The external source may have additional definitions of this entity that 
are
+  // visible, so complete the redeclaration chain now.
+  if (auto *Source = Context.getExternalSource()) {
+Source->CompleteRedeclChain(D);
+  }

mpark wrote:

Yes, calling `redecls` or `getMostRecentDecl` (similar to 
[CXXRecordDecl::dataPtr](https://github.com/llvm/llvm-project/blob/e4c3d258b7a1f335cfd3a90bcf3d28ea220c999d/clang/include/clang/AST/DeclCXX.h#L457-L461)
 here would work as well. The reason I used this "pattern" here is because of 
[this 
code](https://github.com/llvm/llvm-project/blob/d6dbfd6f01710b8fed2303e66f60903efd2283f2/clang/lib/Sema/SemaType.cpp#L9257-L9262)
 at the end of the same function.

I'm very much open to refining the solution. I wanted to submit this PR now 
because the unit-test-level repro was something we were missing from #126973 

https://github.com/llvm/llvm-project/pull/129982
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Fix incomplete decl chains (PR #129982)

[Michael Park via cfe-commits]

https://github.com/mpark created 
https://github.com/llvm/llvm-project/pull/129982

None

>From 3f883067a2fda4147003de9a53b88e52c33d1280 Mon Sep 17 00:00:00 2001
From: Michael Park 
Date: Tue, 25 Feb 2025 14:33:41 -0800
Subject: [PATCH 1/3] =?UTF-8?q?Reapply=20"[C++20][Modules][Serialization]?=
 =?UTF-8?q?=20Delay=20marking=20pending=20incompl=E2=80=A6=20(#127136)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This reverts commit 912b154f3a3f8c3cebf5cc5731fd8b0749762da5.
---
 clang/lib/Serialization/ASTReader.cpp | 25 ---
 clang/test/Modules/pr121245.cpp   | 93 +++
 2 files changed, 105 insertions(+), 13 deletions(-)
 create mode 100644 clang/test/Modules/pr121245.cpp

diff --git a/clang/lib/Serialization/ASTReader.cpp 
b/clang/lib/Serialization/ASTReader.cpp
index ca09c3d79d941..17d07f8535346 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -10178,12 +10178,12 @@ void ASTReader::visitTopLevelModuleMaps(
 }
 
 void ASTReader::finishPendingActions() {
-  while (
-  !PendingIdentifierInfos.empty() || !PendingDeducedFunctionTypes.empty() 
||
-  !PendingDeducedVarTypes.empty() || !PendingIncompleteDeclChains.empty() 
||
-  !PendingDeclChains.empty() || !PendingMacroIDs.empty() ||
-  !PendingDeclContextInfos.empty() || !PendingUpdateRecords.empty() ||
-  !PendingObjCExtensionIvarRedeclarations.empty()) {
+  while (!PendingIdentifierInfos.empty() ||
+ !PendingDeducedFunctionTypes.empty() ||
+ !PendingDeducedVarTypes.empty() || !PendingDeclChains.empty() ||
+ !PendingMacroIDs.empty() || !PendingDeclContextInfos.empty() ||
+ !PendingUpdateRecords.empty() ||
+ !PendingObjCExtensionIvarRedeclarations.empty()) {
 // If any identifiers with corresponding top-level declarations have
 // been loaded, load those declarations now.
 using TopLevelDeclsMap =
@@ -10231,13 +10231,6 @@ void ASTReader::finishPendingActions() {
 }
 PendingDeducedVarTypes.clear();
 
-// For each decl chain that we wanted to complete while deserializing, mark
-// it as "still needs to be completed".
-for (unsigned I = 0; I != PendingIncompleteDeclChains.size(); ++I) {
-  markIncompleteDeclChain(PendingIncompleteDeclChains[I]);
-}
-PendingIncompleteDeclChains.clear();
-
 // Load pending declaration chains.
 for (unsigned I = 0; I != PendingDeclChains.size(); ++I)
   loadPendingDeclChain(PendingDeclChains[I].first,
@@ -10475,6 +10468,12 @@ void ASTReader::finishPendingActions() {
   for (auto *ND : PendingMergedDefinitionsToDeduplicate)
 getContext().deduplicateMergedDefinitonsFor(ND);
   PendingMergedDefinitionsToDeduplicate.clear();
+
+  // For each decl chain that we wanted to complete while deserializing, mark
+  // it as "still needs to be completed".
+  for (Decl *D : PendingIncompleteDeclChains)
+markIncompleteDeclChain(D);
+  PendingIncompleteDeclChains.clear();
 }
 
 void ASTReader::diagnoseOdrViolations() {
diff --git a/clang/test/Modules/pr121245.cpp b/clang/test/Modules/pr121245.cpp
new file mode 100644
index 0..0e276ad0e435d
--- /dev/null
+++ b/clang/test/Modules/pr121245.cpp
@@ -0,0 +1,93 @@
+// If this test fails, it should be investigated under Debug builds.
+// Before the PR, this test was encountering an `llvm_unreachable()`.
+
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+// RUN: cd %t
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header %t/hu-01.h \
+// RUN:  -fcxx-exceptions -o %t/hu-01.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header %t/hu-02.h \
+// RUN:  -Wno-experimental-header-units -fcxx-exceptions \
+// RUN:  -fmodule-file=%t/hu-01.pcm -o %t/hu-02.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header %t/hu-03.h \
+// RUN:  -Wno-experimental-header-units -fcxx-exceptions \
+// RUN:  -fmodule-file=%t/hu-01.pcm -o %t/hu-03.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header %t/hu-04.h \
+// RUN:  -Wno-experimental-header-units -fcxx-exceptions \
+// RUN:  -fmodule-file=%t/hu-01.pcm -o %t/hu-04.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header %t/hu-05.h \
+// RUN:  -Wno-experimental-header-units -fcxx-exceptions \
+// RUN:  -fmodule-file=%t/hu-03.pcm -fmodule-file=%t/hu-04.pcm \
+// RUN:  -fmodule-file=%t/hu-01.pcm -o %t/hu-05.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-obj %t/main.cpp \
+// RUN:  -Wno-experimental-header-units -fcxx-exceptions \
+// RUN:  -fmodule-file=%t/hu-02.pcm -fmodule-file=%t/hu-05.pcm \
+// RUN:  -fmodule-file=%t/hu-04.pcm -fmodule-file=%t/hu-03.pcm \
+// RUN:  -fmodule-file=%t/hu-01.pcm
+
+//--- hu-01.h
+template 
+struct A {
+  A() {}
+  ~A() {}
+};
+
+template 
+struct EBO : T {
+  EBO() = default;
+};
+
+template 
+struct HT : EBO> {};
+
+//--- hu-02.h
+import "hu-01.h";
+
+inline void f() {
+  HT();
+}
+
+//--- hu-0

[clang] Fix incomplete decl chains (PR #129982)

[Michael Park via cfe-commits]

https://github.com/mpark updated 
https://github.com/llvm/llvm-project/pull/129982

>From 3f883067a2fda4147003de9a53b88e52c33d1280 Mon Sep 17 00:00:00 2001
From: Michael Park 
Date: Tue, 25 Feb 2025 14:33:41 -0800
Subject: [PATCH 1/3] =?UTF-8?q?Reapply=20"[C++20][Modules][Serialization]?=
 =?UTF-8?q?=20Delay=20marking=20pending=20incompl=E2=80=A6=20(#127136)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This reverts commit 912b154f3a3f8c3cebf5cc5731fd8b0749762da5.
---
 clang/lib/Serialization/ASTReader.cpp | 25 ---
 clang/test/Modules/pr121245.cpp   | 93 +++
 2 files changed, 105 insertions(+), 13 deletions(-)
 create mode 100644 clang/test/Modules/pr121245.cpp

diff --git a/clang/lib/Serialization/ASTReader.cpp 
b/clang/lib/Serialization/ASTReader.cpp
index ca09c3d79d941..17d07f8535346 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -10178,12 +10178,12 @@ void ASTReader::visitTopLevelModuleMaps(
 }
 
 void ASTReader::finishPendingActions() {
-  while (
-  !PendingIdentifierInfos.empty() || !PendingDeducedFunctionTypes.empty() 
||
-  !PendingDeducedVarTypes.empty() || !PendingIncompleteDeclChains.empty() 
||
-  !PendingDeclChains.empty() || !PendingMacroIDs.empty() ||
-  !PendingDeclContextInfos.empty() || !PendingUpdateRecords.empty() ||
-  !PendingObjCExtensionIvarRedeclarations.empty()) {
+  while (!PendingIdentifierInfos.empty() ||
+ !PendingDeducedFunctionTypes.empty() ||
+ !PendingDeducedVarTypes.empty() || !PendingDeclChains.empty() ||
+ !PendingMacroIDs.empty() || !PendingDeclContextInfos.empty() ||
+ !PendingUpdateRecords.empty() ||
+ !PendingObjCExtensionIvarRedeclarations.empty()) {
 // If any identifiers with corresponding top-level declarations have
 // been loaded, load those declarations now.
 using TopLevelDeclsMap =
@@ -10231,13 +10231,6 @@ void ASTReader::finishPendingActions() {
 }
 PendingDeducedVarTypes.clear();
 
-// For each decl chain that we wanted to complete while deserializing, mark
-// it as "still needs to be completed".
-for (unsigned I = 0; I != PendingIncompleteDeclChains.size(); ++I) {
-  markIncompleteDeclChain(PendingIncompleteDeclChains[I]);
-}
-PendingIncompleteDeclChains.clear();
-
 // Load pending declaration chains.
 for (unsigned I = 0; I != PendingDeclChains.size(); ++I)
   loadPendingDeclChain(PendingDeclChains[I].first,
@@ -10475,6 +10468,12 @@ void ASTReader::finishPendingActions() {
   for (auto *ND : PendingMergedDefinitionsToDeduplicate)
 getContext().deduplicateMergedDefinitonsFor(ND);
   PendingMergedDefinitionsToDeduplicate.clear();
+
+  // For each decl chain that we wanted to complete while deserializing, mark
+  // it as "still needs to be completed".
+  for (Decl *D : PendingIncompleteDeclChains)
+markIncompleteDeclChain(D);
+  PendingIncompleteDeclChains.clear();
 }
 
 void ASTReader::diagnoseOdrViolations() {
diff --git a/clang/test/Modules/pr121245.cpp b/clang/test/Modules/pr121245.cpp
new file mode 100644
index 0..0e276ad0e435d
--- /dev/null
+++ b/clang/test/Modules/pr121245.cpp
@@ -0,0 +1,93 @@
+// If this test fails, it should be investigated under Debug builds.
+// Before the PR, this test was encountering an `llvm_unreachable()`.
+
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+// RUN: cd %t
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header %t/hu-01.h \
+// RUN:  -fcxx-exceptions -o %t/hu-01.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header %t/hu-02.h \
+// RUN:  -Wno-experimental-header-units -fcxx-exceptions \
+// RUN:  -fmodule-file=%t/hu-01.pcm -o %t/hu-02.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header %t/hu-03.h \
+// RUN:  -Wno-experimental-header-units -fcxx-exceptions \
+// RUN:  -fmodule-file=%t/hu-01.pcm -o %t/hu-03.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header %t/hu-04.h \
+// RUN:  -Wno-experimental-header-units -fcxx-exceptions \
+// RUN:  -fmodule-file=%t/hu-01.pcm -o %t/hu-04.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header %t/hu-05.h \
+// RUN:  -Wno-experimental-header-units -fcxx-exceptions \
+// RUN:  -fmodule-file=%t/hu-03.pcm -fmodule-file=%t/hu-04.pcm \
+// RUN:  -fmodule-file=%t/hu-01.pcm -o %t/hu-05.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-obj %t/main.cpp \
+// RUN:  -Wno-experimental-header-units -fcxx-exceptions \
+// RUN:  -fmodule-file=%t/hu-02.pcm -fmodule-file=%t/hu-05.pcm \
+// RUN:  -fmodule-file=%t/hu-04.pcm -fmodule-file=%t/hu-03.pcm \
+// RUN:  -fmodule-file=%t/hu-01.pcm
+
+//--- hu-01.h
+template 
+struct A {
+  A() {}
+  ~A() {}
+};
+
+template 
+struct EBO : T {
+  EBO() = default;
+};
+
+template 
+struct HT : EBO> {};
+
+//--- hu-02.h
+import "hu-01.h";
+
+inline void f() {
+  HT();
+}
+
+//--- hu-03.h
+i

[clang] Fix incomplete decl chains (PR #129982)

[Michael Park via cfe-commits]

https://github.com/mpark edited https://github.com/llvm/llvm-project/pull/129982
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Fix incomplete decl chains (PR #129982)

[Michael Park via cfe-commits]

https://github.com/mpark edited https://github.com/llvm/llvm-project/pull/129982
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C++20][Modules][Serialization] Fix incomplete decl chains (PR #129982)

[Michael Park via cfe-commits]

https://github.com/mpark edited https://github.com/llvm/llvm-project/pull/129982
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C++20][Modules] Fix incomplete decl chains (PR #129982)

[Michael Park via cfe-commits]

https://github.com/mpark edited https://github.com/llvm/llvm-project/pull/129982
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C++20][Modules] Fix incomplete decl chains (PR #129982)

[Michael Park via cfe-commits]

@@ -9180,6 +9180,12 @@ bool Sema::hasAcceptableDefinition(NamedDecl *D, 
NamedDecl **Suggested,
   if (!getLangOpts().Modules && !getLangOpts().ModulesLocalVisibility)
 return true;
 
+  // The external source may have additional definitions of this entity that 
are
+  // visible, so complete the redeclaration chain now.
+  if (auto *Source = Context.getExternalSource()) {
+Source->CompleteRedeclChain(D);
+  }

mpark wrote:

Hm, is that right? when I step through `getDefinition` for [4]:
```cpp
CXXRecordDecl *getDefinition() const {
  // We only need an update if we don't already know which
  // declaration is the definition.
  auto *DD = DefinitionData ? DefinitionData : dataPtr();
  return DD ? DD->Definition : nullptr;
}
```
I see that `DefinitionData` is `nullptr`, followed by `DD` also being `nullptr`.

https://github.com/llvm/llvm-project/pull/129982
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C++20][Modules] Fix incomplete decl chains (PR #129982)

[Michael Park via cfe-commits]

@@ -9180,6 +9180,12 @@ bool Sema::hasAcceptableDefinition(NamedDecl *D, 
NamedDecl **Suggested,
   if (!getLangOpts().Modules && !getLangOpts().ModulesLocalVisibility)
 return true;
 
+  // The external source may have additional definitions of this entity that 
are
+  // visible, so complete the redeclaration chain now.
+  if (auto *Source = Context.getExternalSource()) {
+Source->CompleteRedeclChain(D);
+  }

mpark wrote:

Ah I see. Okay, I'm thinking now that we might be back to the second 
observation I made in #121245, where the generation numbers get updated without 
the update actually happening. When I step through the `dataPtr()` invocation, 
I see that the `getMostRecentDecl()` call on [4] doesn't do anything because 
the generation numbers already match. I'll have to continue tomorrow.

https://github.com/llvm/llvm-project/pull/129982
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C++20][Modules] Fix incomplete decl chains (PR #129982)

[Michael Park via cfe-commits]

@@ -9180,6 +9180,12 @@ bool Sema::hasAcceptableDefinition(NamedDecl *D, 
NamedDecl **Suggested,
   if (!getLangOpts().Modules && !getLangOpts().ModulesLocalVisibility)
 return true;
 
+  // The external source may have additional definitions of this entity that 
are
+  // visible, so complete the redeclaration chain now.
+  if (auto *Source = Context.getExternalSource()) {
+Source->CompleteRedeclChain(D);
+  }

mpark wrote:

So the issue has to do with `ClassTemplateDecl`, 
`ClassTemplateSpecializationDecl`, and `CXXRecordDecl`.
I'll describe my observations in the problematic invocation of 
`hasAcceptableDefinition`.

I believe there are 4 relevant pieces here:

1. `ClassTemplateSpecializationDecl` for `BS>`
2. `CXXRecordDecl` for `BS>` (base portion of (1))
3. `ClassTemplateDecl` for `BS`
4. `CXXRecordDecl` for `BS`

`NamedDecl* D` coming into `hasAcceptableDefinition` points to a `NamedDecl` 
base portion of `ClassTemplateSpecializationDecl` for `BS>` [1]. 
`auto* RD = dyn_cast(D)` is the `CXXRecordDecl` portion of 
`BS>` [2]. `auto *Pattern = RD->getTemplateInstantiationPattern()` 
is an instance of `CXXRecordDecl` for `BS` [4].

Whatever the reason, at this point the *definition* of [4] has not been loaded 
yet. I would've expected that the call to `RD->getDefinition()` (which has a 
dummy invocation to `getMostRecentDecl()` under the hood) should load the 
definition, but this does not happen for some reason.

However, calling `getMostRecentDecl()` on any of [1], [2] or [3] have the 
desired effect of loading the definition of [4]... e.g. 
`RD->getMostRecentDecl()` calls it on [2] which loads the definition of [4], 
and `Pattern->getDescribedClassTemplate()->getMostRecentDecl()` calls it on [3] 
which also gives us the definition of [4] we're looking for...

The current solution here effectively is to call `getMostRecentDecl` on [2] 
such that it loads the redecl chain which somehow includes the definition of 
[4]... but maybe the real solution is to figure out why the `getMostRecentDecl` 
call on [4] itself doesn't load the corresponding definition?

https://github.com/llvm/llvm-project/pull/129982
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C++20][Modules] Fix incomplete decl chains (PR #129982)

[Michael Park via cfe-commits]

https://github.com/mpark updated 
https://github.com/llvm/llvm-project/pull/129982

>From 3f883067a2fda4147003de9a53b88e52c33d1280 Mon Sep 17 00:00:00 2001
From: Michael Park 
Date: Tue, 25 Feb 2025 14:33:41 -0800
Subject: [PATCH 1/3] =?UTF-8?q?Reapply=20"[C++20][Modules][Serialization]?=
 =?UTF-8?q?=20Delay=20marking=20pending=20incompl=E2=80=A6=20(#127136)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This reverts commit 912b154f3a3f8c3cebf5cc5731fd8b0749762da5.
---
 clang/lib/Serialization/ASTReader.cpp | 25 ---
 clang/test/Modules/pr121245.cpp   | 93 +++
 2 files changed, 105 insertions(+), 13 deletions(-)
 create mode 100644 clang/test/Modules/pr121245.cpp

diff --git a/clang/lib/Serialization/ASTReader.cpp 
b/clang/lib/Serialization/ASTReader.cpp
index ca09c3d79d941..17d07f8535346 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -10178,12 +10178,12 @@ void ASTReader::visitTopLevelModuleMaps(
 }
 
 void ASTReader::finishPendingActions() {
-  while (
-  !PendingIdentifierInfos.empty() || !PendingDeducedFunctionTypes.empty() 
||
-  !PendingDeducedVarTypes.empty() || !PendingIncompleteDeclChains.empty() 
||
-  !PendingDeclChains.empty() || !PendingMacroIDs.empty() ||
-  !PendingDeclContextInfos.empty() || !PendingUpdateRecords.empty() ||
-  !PendingObjCExtensionIvarRedeclarations.empty()) {
+  while (!PendingIdentifierInfos.empty() ||
+ !PendingDeducedFunctionTypes.empty() ||
+ !PendingDeducedVarTypes.empty() || !PendingDeclChains.empty() ||
+ !PendingMacroIDs.empty() || !PendingDeclContextInfos.empty() ||
+ !PendingUpdateRecords.empty() ||
+ !PendingObjCExtensionIvarRedeclarations.empty()) {
 // If any identifiers with corresponding top-level declarations have
 // been loaded, load those declarations now.
 using TopLevelDeclsMap =
@@ -10231,13 +10231,6 @@ void ASTReader::finishPendingActions() {
 }
 PendingDeducedVarTypes.clear();
 
-// For each decl chain that we wanted to complete while deserializing, mark
-// it as "still needs to be completed".
-for (unsigned I = 0; I != PendingIncompleteDeclChains.size(); ++I) {
-  markIncompleteDeclChain(PendingIncompleteDeclChains[I]);
-}
-PendingIncompleteDeclChains.clear();
-
 // Load pending declaration chains.
 for (unsigned I = 0; I != PendingDeclChains.size(); ++I)
   loadPendingDeclChain(PendingDeclChains[I].first,
@@ -10475,6 +10468,12 @@ void ASTReader::finishPendingActions() {
   for (auto *ND : PendingMergedDefinitionsToDeduplicate)
 getContext().deduplicateMergedDefinitonsFor(ND);
   PendingMergedDefinitionsToDeduplicate.clear();
+
+  // For each decl chain that we wanted to complete while deserializing, mark
+  // it as "still needs to be completed".
+  for (Decl *D : PendingIncompleteDeclChains)
+markIncompleteDeclChain(D);
+  PendingIncompleteDeclChains.clear();
 }
 
 void ASTReader::diagnoseOdrViolations() {
diff --git a/clang/test/Modules/pr121245.cpp b/clang/test/Modules/pr121245.cpp
new file mode 100644
index 0..0e276ad0e435d
--- /dev/null
+++ b/clang/test/Modules/pr121245.cpp
@@ -0,0 +1,93 @@
+// If this test fails, it should be investigated under Debug builds.
+// Before the PR, this test was encountering an `llvm_unreachable()`.
+
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+// RUN: cd %t
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header %t/hu-01.h \
+// RUN:  -fcxx-exceptions -o %t/hu-01.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header %t/hu-02.h \
+// RUN:  -Wno-experimental-header-units -fcxx-exceptions \
+// RUN:  -fmodule-file=%t/hu-01.pcm -o %t/hu-02.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header %t/hu-03.h \
+// RUN:  -Wno-experimental-header-units -fcxx-exceptions \
+// RUN:  -fmodule-file=%t/hu-01.pcm -o %t/hu-03.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header %t/hu-04.h \
+// RUN:  -Wno-experimental-header-units -fcxx-exceptions \
+// RUN:  -fmodule-file=%t/hu-01.pcm -o %t/hu-04.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header %t/hu-05.h \
+// RUN:  -Wno-experimental-header-units -fcxx-exceptions \
+// RUN:  -fmodule-file=%t/hu-03.pcm -fmodule-file=%t/hu-04.pcm \
+// RUN:  -fmodule-file=%t/hu-01.pcm -o %t/hu-05.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-obj %t/main.cpp \
+// RUN:  -Wno-experimental-header-units -fcxx-exceptions \
+// RUN:  -fmodule-file=%t/hu-02.pcm -fmodule-file=%t/hu-05.pcm \
+// RUN:  -fmodule-file=%t/hu-04.pcm -fmodule-file=%t/hu-03.pcm \
+// RUN:  -fmodule-file=%t/hu-01.pcm
+
+//--- hu-01.h
+template 
+struct A {
+  A() {}
+  ~A() {}
+};
+
+template 
+struct EBO : T {
+  EBO() = default;
+};
+
+template 
+struct HT : EBO> {};
+
+//--- hu-02.h
+import "hu-01.h";
+
+inline void f() {
+  HT();
+}
+
+//--- hu-03.h
+i

[clang] [C++20][Modules] Fix incomplete decl chains (PR #129982)

[Michael Park via cfe-commits]

https://github.com/mpark edited https://github.com/llvm/llvm-project/pull/129982
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [lldb] [C++20][Modules] Do not update the generation number if the redeclaration chain completion was delayed. (PR #129982)

[Michael Park via cfe-commits]

https://github.com/mpark edited https://github.com/llvm/llvm-project/pull/129982
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [lldb] [C++20][Modules] Do not update the declaration generation number if the redeclaration chain completion was delayed. (PR #129982)

[Michael Park via cfe-commits]

https://github.com/mpark edited https://github.com/llvm/llvm-project/pull/129982
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [lldb] [C++20][Modules] Do not update the generation number if the redecl chain completion was delayed. (PR #129982)

[Michael Park via cfe-commits]

https://github.com/mpark edited https://github.com/llvm/llvm-project/pull/129982
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [lldb] [C++20][Modules] Fix incomplete decl chains (PR #129982)

[Michael Park via cfe-commits]

https://github.com/mpark updated 
https://github.com/llvm/llvm-project/pull/129982

>From 39b845fd64c2ce173b95ddcafbbab2d5116aa68e Mon Sep 17 00:00:00 2001
From: Michael Park 
Date: Wed, 5 Mar 2025 18:46:10 -0800
Subject: [PATCH 1/3] [clang][modules] Add a unit test for the assertion
 failure in bootstrap build on OS X with XCode.

---
 clang/test/Modules/pr129982.cpp | 107 
 1 file changed, 107 insertions(+)
 create mode 100644 clang/test/Modules/pr129982.cpp

diff --git a/clang/test/Modules/pr129982.cpp b/clang/test/Modules/pr129982.cpp
new file mode 100644
index 0..b19cccfd07687
--- /dev/null
+++ b/clang/test/Modules/pr129982.cpp
@@ -0,0 +1,107 @@
+// If this test fails, it should be investigated under Debug builds.
+// Before the PR, this test was violating an assertion.
+
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+
+// RUN: %clang_cc1 -std=c++20 -emit-obj -fmodules \
+// RUN:  -fmodule-map-file=%t/module.modulemap \
+// RUN:  -fmodules-cache-path=%t %t/a.cpp
+
+//--- module.modulemap
+module ebo {
+  header "ebo.h"
+}
+
+module fwd {
+  header "fwd.h"
+}
+
+module s {
+  header "s.h"
+  export *
+}
+
+module mod {
+  header "a.h"
+  header "b.h"
+}
+
+//--- ebo.h
+#pragma once
+
+namespace N { inline namespace __1 {
+
+template 
+struct EBO : T {
+  EBO() = default;
+};
+
+}}
+
+//--- fwd.h
+#pragma once
+
+namespace N { inline namespace __1 {
+
+template 
+struct Empty;
+
+template 
+struct BS;
+
+using S = BS>;
+
+}}
+
+//--- s.h
+#pragma once
+
+#include "fwd.h"
+#include "ebo.h"
+
+namespace N { inline namespace __1 {
+
+template 
+struct Empty {};
+
+template 
+struct BS {
+EBO _;
+void f();
+};
+
+extern template void BS>::f();
+
+}}
+
+//--- b.h
+#pragma once
+
+#include "s.h"
+
+struct B {
+  void f() {
+N::S{}.f();
+  }
+};
+
+//--- a.h
+#pragma once
+
+#include "s.h"
+
+struct A {
+  void f(int) {}
+  void f(const N::S &) {}
+
+  void g();
+};
+
+//--- a.cpp
+#include "a.h"
+
+void A::g() { f(0); }
+
+// expected-no-diagnostics

>From e3aebf17e0ea72492564a3652518e6ad20f37330 Mon Sep 17 00:00:00 2001
From: Michael Park 
Date: Thu, 6 Mar 2025 11:49:54 -0800
Subject: [PATCH 2/3] [clang][modules] Re-add the unit test from #121245.

---
 clang/test/Modules/pr121245.cpp | 93 +
 1 file changed, 93 insertions(+)
 create mode 100644 clang/test/Modules/pr121245.cpp

diff --git a/clang/test/Modules/pr121245.cpp b/clang/test/Modules/pr121245.cpp
new file mode 100644
index 0..0e276ad0e435d
--- /dev/null
+++ b/clang/test/Modules/pr121245.cpp
@@ -0,0 +1,93 @@
+// If this test fails, it should be investigated under Debug builds.
+// Before the PR, this test was encountering an `llvm_unreachable()`.
+
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+// RUN: cd %t
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header %t/hu-01.h \
+// RUN:  -fcxx-exceptions -o %t/hu-01.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header %t/hu-02.h \
+// RUN:  -Wno-experimental-header-units -fcxx-exceptions \
+// RUN:  -fmodule-file=%t/hu-01.pcm -o %t/hu-02.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header %t/hu-03.h \
+// RUN:  -Wno-experimental-header-units -fcxx-exceptions \
+// RUN:  -fmodule-file=%t/hu-01.pcm -o %t/hu-03.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header %t/hu-04.h \
+// RUN:  -Wno-experimental-header-units -fcxx-exceptions \
+// RUN:  -fmodule-file=%t/hu-01.pcm -o %t/hu-04.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header %t/hu-05.h \
+// RUN:  -Wno-experimental-header-units -fcxx-exceptions \
+// RUN:  -fmodule-file=%t/hu-03.pcm -fmodule-file=%t/hu-04.pcm \
+// RUN:  -fmodule-file=%t/hu-01.pcm -o %t/hu-05.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-obj %t/main.cpp \
+// RUN:  -Wno-experimental-header-units -fcxx-exceptions \
+// RUN:  -fmodule-file=%t/hu-02.pcm -fmodule-file=%t/hu-05.pcm \
+// RUN:  -fmodule-file=%t/hu-04.pcm -fmodule-file=%t/hu-03.pcm \
+// RUN:  -fmodule-file=%t/hu-01.pcm
+
+//--- hu-01.h
+template 
+struct A {
+  A() {}
+  ~A() {}
+};
+
+template 
+struct EBO : T {
+  EBO() = default;
+};
+
+template 
+struct HT : EBO> {};
+
+//--- hu-02.h
+import "hu-01.h";
+
+inline void f() {
+  HT();
+}
+
+//--- hu-03.h
+import "hu-01.h";
+
+struct C {
+  C();
+
+  HT _;
+};
+
+//--- hu-04.h
+import "hu-01.h";
+
+void g(HT = {});
+
+//--- hu-05.h
+import "hu-03.h";
+import "hu-04.h";
+import "hu-01.h";
+
+struct B {
+  virtual ~B() = default;
+
+  virtual void f() {
+HT();
+  }
+};
+
+//--- main.cpp
+import "hu-02.h";
+import "hu-05.h";
+import "hu-03.h";
+
+int main() {
+  f();
+  C();
+  B();
+}

>From 3134ad3f6fccfa3216ddcb750cdfa93f05ec Mon Sep 17 00:00:00 2001
From: Michael Park 
Date: Thu, 6 Mar 2025 11:47:09 -0800
Subject: [PATCH 3/3] [clang][modules] Only update the generation number if the
 update was not delayed.

---
 clang/include/clang

[clang] [C++20][Modules] Do not update the declaration generation number if the redeclaration chain completion was delayed. (PR #129982)

[Michael Park via cfe-commits]

@@ -10475,6 +10468,12 @@ void ASTReader::finishPendingActions() {
   for (auto *ND : PendingMergedDefinitionsToDeduplicate)
 getContext().deduplicateMergedDefinitonsFor(ND);
   PendingMergedDefinitionsToDeduplicate.clear();
+
+  // For each decl chain that we wanted to complete while deserializing, mark
+  // it as "still needs to be completed".
+  for (Decl *D : PendingIncompleteDeclChains)
+markIncompleteDeclChain(D);
+  PendingIncompleteDeclChains.clear();

mpark wrote:

> Sorry, I've been away from this code for a long time and have forgotten a lot 
> about how it's supposed to work.

Not a problem at all. In trying to dig through some of the history, I've seen 
that a bunch of this code is from 2015 😄

> We're just marking the chains as incomplete here, not actually completing 
> them, and we won't do the work to complete them until someone actually calls 
> `redecls()` or similar.

What I'm uncertain about is the expectation of the parts **within** 
`finishPendingActions` that call `redecls()` or similar. Because for those, we 
**still** won't do the work to complete them. More specifically, the parts 
within `finishPendingActions` that say "we require the redecl chain to be fully 
wired" seem to me like they wouldn't have the requirement guaranteed to be 
satisfied.

> I would expect that when we do go and wire up the redecl chains, we're 
> propagating the `Definition` value along the chain. Also, when chains aren't 
> wired up properly, each declaration still does correctly point to its 
> canonical declaration, so we will still detect things like multiple 
> definitions for an entity because each of those will try to update the 
> canonical declaration to have a different definition.

Ah, I see! This is great. Thanks for sharing. It's exactly this kind of 
high-level architectural understanding is what I'm needing more of.

> Moving the marking of pending incomplete decl chains as incomplete to the end 
> of the function makes sense to me.

This is exactly what I had done in #121245, but unfortunately it caused #126973 
so it got reverted. The reduced test case of #126973 is 
[pr129982.cpp](https://github.com/llvm/llvm-project/pull/129982/files#diff-972832d26c9b2be8952ac137e86510a1eaf0197f98df6245688b1c3b5303ad65).

My gathering of that situation so far is that `RD->addedMember(MD);` (near the 
end of `finishPendingActions`) effectively calls `RD->data()` which calls 
`RD->getMostRecentDecl()`. Anyway, for some reason leaving the `RD` in 
`PendingIncompleteDeclChains` as before is okay, but marking it incomplete 
caused another issue. The other issue in short:
1. There is a call to `getMostRecentDecl()` which calls `CompleteRedeclChain` 
which calls `DC->lookup(Name)`. During this process, the definition of `Name` 
is not found. 
[`Lookups[DC].Table[Name]`](https://github.com/llvm/llvm-project/blob/c6d95c441a29a45782ff72d6cb82839b86fd0e4a/clang/lib/Serialization/ASTReader.cpp#L8494-L8501)
 doesn't have the definition ID.
2. A pending update record populates `Lookups[DC].Table[Name]` with the 
definition ID.
3. Another call to `getMostRecentDecl()` which, if it were to call 
`CompleteRedeclChain` would call `DC->lookup(Name)` and find the definition. 
However, we updated the lazy ptr generation number in step (1), so we do not 
invoke `CompleteRedeclChain`, and we can't find the definition.

> Can we also add an assert that all of the other "pending" lists are still 
> empty at that point? (Or do we already have an assert for that somewhere?)

Yes, I can certainly do that. No, we don't have that assertion currently.

https://github.com/llvm/llvm-project/pull/129982
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C++20][Modules] Do not update the declaration generation number if the redeclaration chain completion was delayed. (PR #129982)

[Michael Park via cfe-commits]

https://github.com/mpark edited https://github.com/llvm/llvm-project/pull/129982
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C++20][Modules] Do not update the declaration generation number if the redeclaration chain completion was delayed. (PR #129982)

[Michael Park via cfe-commits]

https://github.com/mpark edited https://github.com/llvm/llvm-project/pull/129982
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C++20][Modules] Do not update the declaration generation number if the redeclaration chain completion was delayed. (PR #129982)

[Michael Park via cfe-commits]

@@ -9180,6 +9180,12 @@ bool Sema::hasAcceptableDefinition(NamedDecl *D, 
NamedDecl **Suggested,
   if (!getLangOpts().Modules && !getLangOpts().ModulesLocalVisibility)
 return true;
 
+  // The external source may have additional definitions of this entity that 
are
+  // visible, so complete the redeclaration chain now.
+  if (auto *Source = Context.getExternalSource()) {
+Source->CompleteRedeclChain(D);
+  }

mpark wrote:

Thanks @ChuanqiXu9 for following up!

> why it was not a problem but it is after we delay pending the complete decl 
> chain?

I'll look into this again in more detail, but previous investigations what I 
know is that the 
[`RD->addedMember(MD);`](https://github.com/llvm/llvm-project/blob/release/20.x/clang/lib/Serialization/ASTReader.cpp#L10478)
 adds more entries to `PendingIncompleteDeclChains`. Somewhat surprisingly, the 
entries that get added are the `RD` portion of the call: `Empty`. When 
`markIncompleteDeclChain` happens prior to this (the current state, after the 
revert), we leave `finishPendingActions` with `Empty` still in 
`PendingIncompleteDeclChains`, which is not actually marked as incomplete until 
the next time `finishPendingActions` is invoked. With #121245, the 
`markIncompleteDeclChain` happens after this, which marks `Empty` 
incomplete and clears out `PendingIncompleteDeclChain` fully before leaving 
`finishPendingActions`.

That tends to change the order of how things get processed quite a bit. One 
place this effects for sure is this call to 
[getMostRecentDecl()](https://github.com/llvm/llvm-project/blob/release/20.x/clang/lib/AST/DeclBase.cpp#L1889).
 In the current state (where `Empty` is left in 
`PendingIncompleteDeclChains`), when we come across that `getMostRecentDecl()` 
call it does nothing. With #121245, `Empty` has been marked incomplete, 
the `getMostRecentDecl()` call there actually does work.

I know these are rather specific, narrow-scoped observations. I'll need to dig 
a bit deeper to understand the bigger picture of what really changes.

https://github.com/llvm/llvm-project/pull/129982
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C++20][Modules] Fix incomplete decl chains (PR #129982)

[Michael Park via cfe-commits]

https://github.com/mpark edited https://github.com/llvm/llvm-project/pull/129982
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C++20][Modules] Fix incomplete decl chains (PR #129982)

[Michael Park via cfe-commits]

https://github.com/mpark edited https://github.com/llvm/llvm-project/pull/129982
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C++20][Modules] Prevent premature calls to PassInterestingDeclsToConsumer() within FinishedDeserializing(). (PR #129982)

[Michael Park via cfe-commits]

https://github.com/mpark edited https://github.com/llvm/llvm-project/pull/129982
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C++20][Modules] Prevent premature calls to PassInterestingDeclsToConsumer() within FinishedDeserializing(). (PR #129982)

[Michael Park via cfe-commits]

https://github.com/mpark closed https://github.com/llvm/llvm-project/pull/129982
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C++20][Modules] Prevent premature calls to PassInterestingDeclsToConsumer() within FinishedDeserializing(). (PR #129982)

[Michael Park via cfe-commits]

mpark wrote:

Fixes #126973

https://github.com/llvm/llvm-project/pull/129982
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [NFC] Add a leading comment to boolean arguments. (PR #131746)

[Michael Park via cfe-commits]

https://github.com/mpark closed https://github.com/llvm/llvm-project/pull/131746
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C++20][Modules] Do not update the declaration generation number if the redeclaration chain completion was delayed. (PR #129982)

[Michael Park via cfe-commits]

https://github.com/mpark updated 
https://github.com/llvm/llvm-project/pull/129982

>From ebf6cd9c13ae6dbd9bfcece78491bdf01f0f170f Mon Sep 17 00:00:00 2001
From: Michael Park 
Date: Wed, 5 Mar 2025 18:46:10 -0800
Subject: [PATCH 1/3] [clang][modules] Add a unit test for the assertion
 failure in bootstrap build on OS X with XCode.

---
 clang/test/Modules/pr129982.cpp | 107 
 1 file changed, 107 insertions(+)
 create mode 100644 clang/test/Modules/pr129982.cpp

diff --git a/clang/test/Modules/pr129982.cpp b/clang/test/Modules/pr129982.cpp
new file mode 100644
index 0..b19cccfd07687
--- /dev/null
+++ b/clang/test/Modules/pr129982.cpp
@@ -0,0 +1,107 @@
+// If this test fails, it should be investigated under Debug builds.
+// Before the PR, this test was violating an assertion.
+
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+
+// RUN: %clang_cc1 -std=c++20 -emit-obj -fmodules \
+// RUN:  -fmodule-map-file=%t/module.modulemap \
+// RUN:  -fmodules-cache-path=%t %t/a.cpp
+
+//--- module.modulemap
+module ebo {
+  header "ebo.h"
+}
+
+module fwd {
+  header "fwd.h"
+}
+
+module s {
+  header "s.h"
+  export *
+}
+
+module mod {
+  header "a.h"
+  header "b.h"
+}
+
+//--- ebo.h
+#pragma once
+
+namespace N { inline namespace __1 {
+
+template 
+struct EBO : T {
+  EBO() = default;
+};
+
+}}
+
+//--- fwd.h
+#pragma once
+
+namespace N { inline namespace __1 {
+
+template 
+struct Empty;
+
+template 
+struct BS;
+
+using S = BS>;
+
+}}
+
+//--- s.h
+#pragma once
+
+#include "fwd.h"
+#include "ebo.h"
+
+namespace N { inline namespace __1 {
+
+template 
+struct Empty {};
+
+template 
+struct BS {
+EBO _;
+void f();
+};
+
+extern template void BS>::f();
+
+}}
+
+//--- b.h
+#pragma once
+
+#include "s.h"
+
+struct B {
+  void f() {
+N::S{}.f();
+  }
+};
+
+//--- a.h
+#pragma once
+
+#include "s.h"
+
+struct A {
+  void f(int) {}
+  void f(const N::S &) {}
+
+  void g();
+};
+
+//--- a.cpp
+#include "a.h"
+
+void A::g() { f(0); }
+
+// expected-no-diagnostics

>From 7bb10f498c55b8c1ec9e10bd5efad7a7ab90437f Mon Sep 17 00:00:00 2001
From: Michael Park 
Date: Fri, 7 Mar 2025 01:09:38 -0800
Subject: [PATCH 2/3] =?UTF-8?q?Reapply=20"[C++20][Modules][Serialization]?=
 =?UTF-8?q?=20Delay=20marking=20pending=20incompl=E2=80=A6=20(#127136)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This reverts commit 912b154f3a3f8c3cebf5cc5731fd8b0749762da5.
---
 clang/lib/Serialization/ASTReader.cpp | 25 ---
 clang/test/Modules/pr121245.cpp   | 93 +++
 2 files changed, 105 insertions(+), 13 deletions(-)
 create mode 100644 clang/test/Modules/pr121245.cpp

diff --git a/clang/lib/Serialization/ASTReader.cpp 
b/clang/lib/Serialization/ASTReader.cpp
index 8e9978829c512..41b6c2a4d64ab 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -10182,12 +10182,12 @@ void ASTReader::visitTopLevelModuleMaps(
 }
 
 void ASTReader::finishPendingActions() {
-  while (
-  !PendingIdentifierInfos.empty() || !PendingDeducedFunctionTypes.empty() 
||
-  !PendingDeducedVarTypes.empty() || !PendingIncompleteDeclChains.empty() 
||
-  !PendingDeclChains.empty() || !PendingMacroIDs.empty() ||
-  !PendingDeclContextInfos.empty() || !PendingUpdateRecords.empty() ||
-  !PendingObjCExtensionIvarRedeclarations.empty()) {
+  while (!PendingIdentifierInfos.empty() ||
+ !PendingDeducedFunctionTypes.empty() ||
+ !PendingDeducedVarTypes.empty() || !PendingDeclChains.empty() ||
+ !PendingMacroIDs.empty() || !PendingDeclContextInfos.empty() ||
+ !PendingUpdateRecords.empty() ||
+ !PendingObjCExtensionIvarRedeclarations.empty()) {
 // If any identifiers with corresponding top-level declarations have
 // been loaded, load those declarations now.
 using TopLevelDeclsMap =
@@ -10235,13 +10235,6 @@ void ASTReader::finishPendingActions() {
 }
 PendingDeducedVarTypes.clear();
 
-// For each decl chain that we wanted to complete while deserializing, mark
-// it as "still needs to be completed".
-for (unsigned I = 0; I != PendingIncompleteDeclChains.size(); ++I) {
-  markIncompleteDeclChain(PendingIncompleteDeclChains[I]);
-}
-PendingIncompleteDeclChains.clear();
-
 // Load pending declaration chains.
 for (unsigned I = 0; I != PendingDeclChains.size(); ++I)
   loadPendingDeclChain(PendingDeclChains[I].first,
@@ -10479,6 +10472,12 @@ void ASTReader::finishPendingActions() {
   for (auto *ND : PendingMergedDefinitionsToDeduplicate)
 getContext().deduplicateMergedDefinitonsFor(ND);
   PendingMergedDefinitionsToDeduplicate.clear();
+
+  // For each decl chain that we wanted to complete while deserializing, mark
+  // it as "still needs to be completed".
+  for (Decl *D : PendingIncompleteDeclChains)
+markIncompleteDeclChain(D);
+  PendingIncompleteDeclChains.clear();
 }

[clang] [C++20][Modules] Prevent premature call to PassInterestingDeclsToConsumer() within FinishedDeserializing(). (PR #129982)

[Michael Park via cfe-commits]

https://github.com/mpark updated 
https://github.com/llvm/llvm-project/pull/129982

>From ebf6cd9c13ae6dbd9bfcece78491bdf01f0f170f Mon Sep 17 00:00:00 2001
From: Michael Park 
Date: Wed, 5 Mar 2025 18:46:10 -0800
Subject: [PATCH 1/3] [clang][modules] Add a unit test for the assertion
 failure in bootstrap build on OS X with XCode.

---
 clang/test/Modules/pr129982.cpp | 107 
 1 file changed, 107 insertions(+)
 create mode 100644 clang/test/Modules/pr129982.cpp

diff --git a/clang/test/Modules/pr129982.cpp b/clang/test/Modules/pr129982.cpp
new file mode 100644
index 0..b19cccfd07687
--- /dev/null
+++ b/clang/test/Modules/pr129982.cpp
@@ -0,0 +1,107 @@
+// If this test fails, it should be investigated under Debug builds.
+// Before the PR, this test was violating an assertion.
+
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+
+// RUN: %clang_cc1 -std=c++20 -emit-obj -fmodules \
+// RUN:  -fmodule-map-file=%t/module.modulemap \
+// RUN:  -fmodules-cache-path=%t %t/a.cpp
+
+//--- module.modulemap
+module ebo {
+  header "ebo.h"
+}
+
+module fwd {
+  header "fwd.h"
+}
+
+module s {
+  header "s.h"
+  export *
+}
+
+module mod {
+  header "a.h"
+  header "b.h"
+}
+
+//--- ebo.h
+#pragma once
+
+namespace N { inline namespace __1 {
+
+template 
+struct EBO : T {
+  EBO() = default;
+};
+
+}}
+
+//--- fwd.h
+#pragma once
+
+namespace N { inline namespace __1 {
+
+template 
+struct Empty;
+
+template 
+struct BS;
+
+using S = BS>;
+
+}}
+
+//--- s.h
+#pragma once
+
+#include "fwd.h"
+#include "ebo.h"
+
+namespace N { inline namespace __1 {
+
+template 
+struct Empty {};
+
+template 
+struct BS {
+EBO _;
+void f();
+};
+
+extern template void BS>::f();
+
+}}
+
+//--- b.h
+#pragma once
+
+#include "s.h"
+
+struct B {
+  void f() {
+N::S{}.f();
+  }
+};
+
+//--- a.h
+#pragma once
+
+#include "s.h"
+
+struct A {
+  void f(int) {}
+  void f(const N::S &) {}
+
+  void g();
+};
+
+//--- a.cpp
+#include "a.h"
+
+void A::g() { f(0); }
+
+// expected-no-diagnostics

>From 7bb10f498c55b8c1ec9e10bd5efad7a7ab90437f Mon Sep 17 00:00:00 2001
From: Michael Park 
Date: Fri, 7 Mar 2025 01:09:38 -0800
Subject: [PATCH 2/3] =?UTF-8?q?Reapply=20"[C++20][Modules][Serialization]?=
 =?UTF-8?q?=20Delay=20marking=20pending=20incompl=E2=80=A6=20(#127136)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This reverts commit 912b154f3a3f8c3cebf5cc5731fd8b0749762da5.
---
 clang/lib/Serialization/ASTReader.cpp | 25 ---
 clang/test/Modules/pr121245.cpp   | 93 +++
 2 files changed, 105 insertions(+), 13 deletions(-)
 create mode 100644 clang/test/Modules/pr121245.cpp

diff --git a/clang/lib/Serialization/ASTReader.cpp 
b/clang/lib/Serialization/ASTReader.cpp
index 8e9978829c512..41b6c2a4d64ab 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -10182,12 +10182,12 @@ void ASTReader::visitTopLevelModuleMaps(
 }
 
 void ASTReader::finishPendingActions() {
-  while (
-  !PendingIdentifierInfos.empty() || !PendingDeducedFunctionTypes.empty() 
||
-  !PendingDeducedVarTypes.empty() || !PendingIncompleteDeclChains.empty() 
||
-  !PendingDeclChains.empty() || !PendingMacroIDs.empty() ||
-  !PendingDeclContextInfos.empty() || !PendingUpdateRecords.empty() ||
-  !PendingObjCExtensionIvarRedeclarations.empty()) {
+  while (!PendingIdentifierInfos.empty() ||
+ !PendingDeducedFunctionTypes.empty() ||
+ !PendingDeducedVarTypes.empty() || !PendingDeclChains.empty() ||
+ !PendingMacroIDs.empty() || !PendingDeclContextInfos.empty() ||
+ !PendingUpdateRecords.empty() ||
+ !PendingObjCExtensionIvarRedeclarations.empty()) {
 // If any identifiers with corresponding top-level declarations have
 // been loaded, load those declarations now.
 using TopLevelDeclsMap =
@@ -10235,13 +10235,6 @@ void ASTReader::finishPendingActions() {
 }
 PendingDeducedVarTypes.clear();
 
-// For each decl chain that we wanted to complete while deserializing, mark
-// it as "still needs to be completed".
-for (unsigned I = 0; I != PendingIncompleteDeclChains.size(); ++I) {
-  markIncompleteDeclChain(PendingIncompleteDeclChains[I]);
-}
-PendingIncompleteDeclChains.clear();
-
 // Load pending declaration chains.
 for (unsigned I = 0; I != PendingDeclChains.size(); ++I)
   loadPendingDeclChain(PendingDeclChains[I].first,
@@ -10479,6 +10472,12 @@ void ASTReader::finishPendingActions() {
   for (auto *ND : PendingMergedDefinitionsToDeduplicate)
 getContext().deduplicateMergedDefinitonsFor(ND);
   PendingMergedDefinitionsToDeduplicate.clear();
+
+  // For each decl chain that we wanted to complete while deserializing, mark
+  // it as "still needs to be completed".
+  for (Decl *D : PendingIncompleteDeclChains)
+markIncompleteDeclChain(D);
+  PendingIncompleteDeclChains.clear();
 }

[clang] [C++20][Modules] Prevent premature call to PassInterestingDeclsToConsumer() within FinishedDeserializing(). (PR #129982)

[Michael Park via cfe-commits]

https://github.com/mpark edited https://github.com/llvm/llvm-project/pull/129982
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C++20][Modules] Prevent premature call to PassInterestingDeclsToConsumer() within FinishedDeserializing(). (PR #129982)

[Michael Park via cfe-commits]

https://github.com/mpark updated 
https://github.com/llvm/llvm-project/pull/129982

>From ebf6cd9c13ae6dbd9bfcece78491bdf01f0f170f Mon Sep 17 00:00:00 2001
From: Michael Park 
Date: Wed, 5 Mar 2025 18:46:10 -0800
Subject: [PATCH 1/3] [clang][modules] Add a unit test for the assertion
 failure in bootstrap build on OS X with XCode.

---
 clang/test/Modules/pr129982.cpp | 107 
 1 file changed, 107 insertions(+)
 create mode 100644 clang/test/Modules/pr129982.cpp

diff --git a/clang/test/Modules/pr129982.cpp b/clang/test/Modules/pr129982.cpp
new file mode 100644
index 0..b19cccfd07687
--- /dev/null
+++ b/clang/test/Modules/pr129982.cpp
@@ -0,0 +1,107 @@
+// If this test fails, it should be investigated under Debug builds.
+// Before the PR, this test was violating an assertion.
+
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+
+// RUN: %clang_cc1 -std=c++20 -emit-obj -fmodules \
+// RUN:  -fmodule-map-file=%t/module.modulemap \
+// RUN:  -fmodules-cache-path=%t %t/a.cpp
+
+//--- module.modulemap
+module ebo {
+  header "ebo.h"
+}
+
+module fwd {
+  header "fwd.h"
+}
+
+module s {
+  header "s.h"
+  export *
+}
+
+module mod {
+  header "a.h"
+  header "b.h"
+}
+
+//--- ebo.h
+#pragma once
+
+namespace N { inline namespace __1 {
+
+template 
+struct EBO : T {
+  EBO() = default;
+};
+
+}}
+
+//--- fwd.h
+#pragma once
+
+namespace N { inline namespace __1 {
+
+template 
+struct Empty;
+
+template 
+struct BS;
+
+using S = BS>;
+
+}}
+
+//--- s.h
+#pragma once
+
+#include "fwd.h"
+#include "ebo.h"
+
+namespace N { inline namespace __1 {
+
+template 
+struct Empty {};
+
+template 
+struct BS {
+EBO _;
+void f();
+};
+
+extern template void BS>::f();
+
+}}
+
+//--- b.h
+#pragma once
+
+#include "s.h"
+
+struct B {
+  void f() {
+N::S{}.f();
+  }
+};
+
+//--- a.h
+#pragma once
+
+#include "s.h"
+
+struct A {
+  void f(int) {}
+  void f(const N::S &) {}
+
+  void g();
+};
+
+//--- a.cpp
+#include "a.h"
+
+void A::g() { f(0); }
+
+// expected-no-diagnostics

>From 7bb10f498c55b8c1ec9e10bd5efad7a7ab90437f Mon Sep 17 00:00:00 2001
From: Michael Park 
Date: Fri, 7 Mar 2025 01:09:38 -0800
Subject: [PATCH 2/3] =?UTF-8?q?Reapply=20"[C++20][Modules][Serialization]?=
 =?UTF-8?q?=20Delay=20marking=20pending=20incompl=E2=80=A6=20(#127136)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This reverts commit 912b154f3a3f8c3cebf5cc5731fd8b0749762da5.
---
 clang/lib/Serialization/ASTReader.cpp | 25 ---
 clang/test/Modules/pr121245.cpp   | 93 +++
 2 files changed, 105 insertions(+), 13 deletions(-)
 create mode 100644 clang/test/Modules/pr121245.cpp

diff --git a/clang/lib/Serialization/ASTReader.cpp 
b/clang/lib/Serialization/ASTReader.cpp
index 8e9978829c512..41b6c2a4d64ab 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -10182,12 +10182,12 @@ void ASTReader::visitTopLevelModuleMaps(
 }
 
 void ASTReader::finishPendingActions() {
-  while (
-  !PendingIdentifierInfos.empty() || !PendingDeducedFunctionTypes.empty() 
||
-  !PendingDeducedVarTypes.empty() || !PendingIncompleteDeclChains.empty() 
||
-  !PendingDeclChains.empty() || !PendingMacroIDs.empty() ||
-  !PendingDeclContextInfos.empty() || !PendingUpdateRecords.empty() ||
-  !PendingObjCExtensionIvarRedeclarations.empty()) {
+  while (!PendingIdentifierInfos.empty() ||
+ !PendingDeducedFunctionTypes.empty() ||
+ !PendingDeducedVarTypes.empty() || !PendingDeclChains.empty() ||
+ !PendingMacroIDs.empty() || !PendingDeclContextInfos.empty() ||
+ !PendingUpdateRecords.empty() ||
+ !PendingObjCExtensionIvarRedeclarations.empty()) {
 // If any identifiers with corresponding top-level declarations have
 // been loaded, load those declarations now.
 using TopLevelDeclsMap =
@@ -10235,13 +10235,6 @@ void ASTReader::finishPendingActions() {
 }
 PendingDeducedVarTypes.clear();
 
-// For each decl chain that we wanted to complete while deserializing, mark
-// it as "still needs to be completed".
-for (unsigned I = 0; I != PendingIncompleteDeclChains.size(); ++I) {
-  markIncompleteDeclChain(PendingIncompleteDeclChains[I]);
-}
-PendingIncompleteDeclChains.clear();
-
 // Load pending declaration chains.
 for (unsigned I = 0; I != PendingDeclChains.size(); ++I)
   loadPendingDeclChain(PendingDeclChains[I].first,
@@ -10479,6 +10472,12 @@ void ASTReader::finishPendingActions() {
   for (auto *ND : PendingMergedDefinitionsToDeduplicate)
 getContext().deduplicateMergedDefinitonsFor(ND);
   PendingMergedDefinitionsToDeduplicate.clear();
+
+  // For each decl chain that we wanted to complete while deserializing, mark
+  // it as "still needs to be completed".
+  for (Decl *D : PendingIncompleteDeclChains)
+markIncompleteDeclChain(D);
+  PendingIncompleteDeclChains.clear();
 }