Skip to content

Conversation

lf-
Copy link
Contributor

@lf- lf- commented Oct 30, 2023

It looks like this has been there for so long that my git-blaming ran into the end of history when libc++ was imported from svn in 2010. This change will undoubtedly break some downstream code with broken includes, but libstdc++ does not do the same here, so such code was already broken on libstdc++; as such this change improves compatibility between the two implementations.

It looks like this has been there for so long that my git-blaming ran
into the end of history when libc++ was imported from svn in 2010. This
change will undoubtedly break some downstream code with broken includes,
but libstdc++ does not do the same here, so such code was already broken
on libstdc++.
@lf- lf- requested a review from a team as a code owner October 30, 2023 00:49
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Oct 30, 2023
@llvmbot
Copy link
Member

llvmbot commented Oct 30, 2023

@llvm/pr-subscribers-libcxx

Author: Jade Lovelace (lf-)

Changes

It looks like this has been there for so long that my git-blaming ran into the end of history when libc++ was imported from svn in 2010. This change will undoubtedly break some downstream code with broken includes, but libstdc++ does not do the same here, so such code was already broken on libstdc++; as such this change improves compatibility between the two implementations.


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

1 Files Affected:

  • (modified) libcxx/include/string (-1)
diff --git a/libcxx/include/string b/libcxx/include/string
index cf9f0c847eb43af..68f13414c7b6948 100644
--- a/libcxx/include/string
+++ b/libcxx/include/string
@@ -614,7 +614,6 @@ basic_string<char32_t> operator""s( const char32_t *str, size_t len );
 #include <__utility/swap.h>
 #include <__utility/unreachable.h>
 #include <climits>
-#include <cstdint>
 #include <cstdio>  // EOF
 #include <cstring>
 #include <limits>

@philnik777
Copy link
Contributor

It looks like <cstdint> is still transitively included, so this doesn't actually do anything. This looks basically good, since we're still including <cstdint>, but please update your commit message.

@ldionne
Copy link
Member

ldionne commented Oct 31, 2023

Why is the include redundant? We don't use anything from <cstdint> inside <string>? LGTM if that's the case.

@philnik777
Copy link
Contributor

Why is the include redundant? We don't use anything from <cstdint> inside <string>? LGTM if that's the case.

Nothing that I could find. If we did, the modules build should also have complained.

@ldionne ldionne merged commit a3ee0d4 into llvm:main Oct 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code-cleanup 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