Skip to content

libcxx: remove redundant <cstdint> include from <string> #70613

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 1 commit into from
Oct 31, 2023

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