Skip to content

Conversation

jyknight
Copy link
Member

@jyknight jyknight commented Jul 1, 2024

PR #96165 broke code similar to this test, and was subsequently reverted. Add a test-case, to ensure the problem won't reoccur.

This error is potentially related to issues #59292 and #59966.

PR llvm#96165 broke code similar to this test, and was subsequently
reverted. Add a test-case, to ensure the problem won't reoccur.
@jyknight jyknight requested review from ldionne and philnik777 July 1, 2024 21:49
@jyknight jyknight requested a review from a team as a code owner July 1, 2024 21:49
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Jul 1, 2024
@llvmbot
Copy link
Member

llvmbot commented Jul 1, 2024

@llvm/pr-subscribers-libcxx

Author: James Y Knight (jyknight)

Changes

PR #96165 broke code similar to this test, and was subsequently reverted. Add a test-case, to ensure the problem won't reoccur.


Full diff: https://github.com/llvm/llvm-project/pull/97355.diff

1 Files Affected:

  • (added) libcxx/test/libcxx/utilities/utility/pairs/pairs.pair/pair.incomplete.verify.cpp (+15)
diff --git a/libcxx/test/libcxx/utilities/utility/pairs/pairs.pair/pair.incomplete.verify.cpp b/libcxx/test/libcxx/utilities/utility/pairs/pairs.pair/pair.incomplete.verify.cpp
new file mode 100644
index 0000000000000..d0832ea311690
--- /dev/null
+++ b/libcxx/test/libcxx/utilities/utility/pairs/pairs.pair/pair.incomplete.verify.cpp
@@ -0,0 +1,15 @@
+// Check that instantiating pair doesn't look up type traits "too early", before
+// the contained types have been completed.
+//
+// This is a regression test, to prevent a reoccurrance of the issue introduced in
+// 5e1de27f680591a870d78e9952b23f76aed7f456.
+
+// expected-no-diagnostics
+#include <utility>
+#include <vector>
+
+struct Test {
+  std::vector<std::pair<int, Test>> v;
+};
+
+std::pair<int, Test> p;

@frederick-vs-ja
Copy link
Contributor

Seems related to #59292 and #59966. Should we mention these issues?

@@ -0,0 +1,15 @@
// Check that instantiating pair doesn't look up type traits "too early", before
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a license header

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Copy link
Member

@ldionne ldionne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM w/ comments addressed. Thanks!

// This is a regression test, to prevent a reoccurrance of the issue introduced in
// 5e1de27f680591a870d78e9952b23f76aed7f456.

// expected-no-diagnostics
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's make this a .compile.pass.cpp test instead of .verify.pass.cpp, that way it can run under all compiler.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Copy link
Member Author

@jyknight jyknight left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a reference to the possibly-related issues as suggested.

@@ -0,0 +1,15 @@
// Check that instantiating pair doesn't look up type traits "too early", before
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

// This is a regression test, to prevent a reoccurrance of the issue introduced in
// 5e1de27f680591a870d78e9952b23f76aed7f456.

// expected-no-diagnostics
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@jyknight jyknight merged commit cfbad45 into llvm:main Jul 2, 2024
@jyknight jyknight deleted the libcxx-test branch July 2, 2024 15:20
cpiaseque pushed a commit to cpiaseque/llvm-project that referenced this pull request Jul 3, 2024
PR llvm#96165 broke code similar to this test, and was subsequently
reverted. Add a test-case, to ensure the problem won't reoccur.

This error is potentially related to issues llvm#59292 and llvm#59966.
kbluck pushed a commit to kbluck/llvm-project that referenced this pull request Jul 6, 2024
PR llvm#96165 broke code similar to this test, and was subsequently
reverted. Add a test-case, to ensure the problem won't reoccur.

This error is potentially related to issues llvm#59292 and llvm#59966.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants