[PATCH] D129689: [limits.h] USHRT_MAX fix for 16 bit architectures

aaron.ballman added a comment. In D129689#4194096 , @SebastianPerta wrote: > @cor3ntin can I close it? You can abandon this PR, it was handled by 0fecac18ffad476b5a4682770f6d8b1f0f176b40

[PATCH] D129689: [limits.h] USHRT_MAX fix for 16 bit architectures

SebastianPerta added a comment. @cor3ntin can I close it? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129689/new/ https://reviews.llvm.org/D129689 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D129689: [limits.h] USHRT_MAX fix for 16 bit architectures

cor3ntin added a comment. @aaron.ballman I think we should close this change as it's superseeded by https://reviews.llvm.org/D144218 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129689/new/ https://reviews.llvm.org/D129689 ___ cfe-commits

[PATCH] D129689: [limits.h] USHRT_MAX fix for 16 bit architectures

SebastianPerta added a comment. >>are you planning to update the patch Yes, sorry about the delay. I will update the patch before the end of the week. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129689/new/ https://reviews.llvm.org/D129689

[PATCH] D129689: [limits.h] USHRT_MAX fix for 16 bit architectures

aaron.ballman added a comment. This seems to have fallen through the cracks -- @SebastianPerta are you planning to update the patch, or would you be okay if I commandeered it? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129689/new/ https://reviews.llvm.org/D129689

[PATCH] D129689: [limits.h] USHRT_MAX fix for 16 bit architectures

tahonermann added a comment. > Sorry about this, I actually tried this but the patch showed a lot more than > what I've changed, not sure what I did wrong. You didn't do anything wrong; the `-U9` option will effectively include the entire file for each changed file. That is actually the

[PATCH] D129689: [limits.h] USHRT_MAX fix for 16 bit architectures

cor3ntin added a comment. In D129689#3652010 , @aaron.ballman wrote: > In D129689#3651965 , > @SebastianPerta wrote: > But can you add some test coverage for this? >> In clang\test\Headers\limits.cpp there

[PATCH] D129689: [limits.h] USHRT_MAX fix for 16 bit architectures

aaron.ballman added a comment. In D129689#3651965 , @SebastianPerta wrote: >>>But can you add some test coverage for this? > In clang\test\Headers\limits.cpp there is: > _Static_assert(USHRT_MAX == (unsigned short)~0ULL, ""); > However as I said this

[PATCH] D129689: [limits.h] USHRT_MAX fix for 16 bit architectures

SebastianPerta added a comment. >>But can you add some test coverage for this? In clang\test\Headers\limits.cpp there is: _Static_assert(USHRT_MAX == (unsigned short)~0ULL, ""); However as I said this needs a 16 bits target. Looking at the RUN commands at the top of the file I think I can solve

[PATCH] D129689: [limits.h] USHRT_MAX fix for 16 bit architectures

erichkeane added a comment. Please include the full context of the patch with -U9. See https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129689/new/ https://reviews.llvm.org/D129689