Skip to content

[libc++] Do not forward-declare syncstream outside experimental #82511

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Mar 4, 2024

Conversation

ldionne
Copy link
Member

@ldionne ldionne commented Feb 21, 2024

We only define the classes in <syncstream> when experimental library features are enabled, but we would forward-declare them in <iosfwd> even when they are disabled. This led to confusing error messages about being unable to instantiate an undefined template.

@ldionne ldionne requested a review from a team as a code owner February 21, 2024 17:29
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Feb 21, 2024
@llvmbot
Copy link
Member

llvmbot commented Feb 21, 2024

@llvm/pr-subscribers-libcxx

Author: Louis Dionne (ldionne)

Changes

We only define the classes in <syncstream> when experimental library features are enabled, but we would forward-declare them in <iosfwd> even when they are disabled. This led to confusing error messages about being unable to instantiate an undefined template.


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

1 Files Affected:

  • (modified) libcxx/include/iosfwd (+2-2)
diff --git a/libcxx/include/iosfwd b/libcxx/include/iosfwd
index e28998d004156d..728700853fb331 100644
--- a/libcxx/include/iosfwd
+++ b/libcxx/include/iosfwd
@@ -143,7 +143,7 @@ typedef fpos<mbstate_t> u8streampos;
 typedef fpos<mbstate_t> u16streampos;
 typedef fpos<mbstate_t> u32streampos;
 
-#if _LIBCPP_STD_VER >= 20
+#if _LIBCPP_STD_VER >= 20 && !defined(_LIBCPP_HAS_NO_EXPERIMENTAL_SYNCSTREAM)
 
 template <class _CharT, class _Traits = char_traits<_CharT>, class _Allocator = allocator<_CharT>>
 class basic_syncbuf;
@@ -161,7 +161,7 @@ using osyncstream = basic_osyncstream<char>;
 using wosyncstream = basic_osyncstream<wchar_t>;
 #  endif
 
-#endif // _LIBCPP_STD_VER >=20
+#endif // _LIBCPP_STD_VER >= 20 && !defined(_LIBCPP_HAS_NO_EXPERIMENTAL_SYNCSTREAM)
 
 // Include other forward declarations here
 template <class _Tp, class _Alloc = allocator<_Tp> >

We only define the classes in <syncstream> when experimental
library features are enabled, but we would forward-declare
them in <iosfwd> even when they are disabled. This led to
confusing error messages about being unable to instantiate
an undefined template.
@ldionne ldionne force-pushed the review/fix-syncstream-forward-decl branch from 39ec2d5 to eb443dc Compare February 26, 2024 19:37
Copy link

github-actions bot commented Feb 27, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

Copy link
Member

@mordante mordante left a comment

Choose a reason for hiding this comment

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

Thanks for catching this. LGTM!

@ldionne ldionne merged commit 4d80df0 into llvm:main Mar 4, 2024
@ldionne ldionne deleted the review/fix-syncstream-forward-decl branch March 4, 2024 23:16
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.

4 participants