[libcxx] don't #include <cwchar> if wide chars aren't enabled
#99911
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -237,21 +237,21 @@ namespace std { | |
| # include <cstdint> | ||
| # include <cstdlib> | ||
| # include <cstring> | ||
| # include <initializer_list> | ||
| # include <limits> | ||
| # include <locale> | ||
| # include <new> | ||
| # include <optional> | ||
| # include <queue> | ||
| # include <stack> | ||
| # include <stdexcept> | ||
| # include <string> | ||
| # include <string_view> | ||
| # include <tuple> | ||
|
|
||
|
There was a problem hiding this comment. Why are we changing the behaviour here? This PR is protecting the include for There was a problem hiding this comment. We're not changing the behaviour. If you expand this window to see the lines above, you'll see that the exact same conditional guards the includes above. I merged the two sections to make the code clearer. There was a problem hiding this comment. Can that be made clearer in the commit message? There was a problem hiding this comment. +1 – if two of us misread the patch in the same way, then I think it's likely that future readers would benefit from the clarification! There was a problem hiding this comment. As I think we'll loose or bury this convo if I force-push, I've updated the description in my automated first comment in this thread. I'll update the description when I merge it. Unless there's a better way to handle this. There was a problem hiding this comment. I have no idea whether that will do what you want, so good luck with getting the right version of the commit message into the final commit! There was a problem hiding this comment. If you force-push over the current commit it should keep this conversation, not sure GitHub will delete it. There was a problem hiding this comment. ok, let me give it a shot now that I've got approval. There was a problem hiding this comment. Ah that did work. Googling says comments don't disappear if the lines they're on don't change. Seems like they do still disappear if the lines change: https://github.com/orgs/community/discussions/3478. |
||
| # if !defined(_LIBCPP_HAS_NO_WIDE_CHARACTERS) | ||
| # include <cwchar> | ||
| # endif | ||
| #endif | ||
|
|
||
| #endif // _LIBCPP_FORMAT | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The commit message doesn't say anything about why you've also made the includes of
<locale>,<queue>and<stack>unconditional. Was that change included in this patch by mistake? If it was on purpose, could you update the commit message to explain why it's related or necessary?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're not making them unconditional. If you expand this window to see the lines above, you'll see that the exact same conditional guards the includes above.
I merged the two sections to make the code clearer.