Skip to content
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

Implementations of formatting traits for signed integers can be ambiguous #42860

Closed
Enet4 opened this issue Jun 23, 2017 · 6 comments
Closed
Labels
A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools C-enhancement Category: An issue proposing an enhancement or a PR with one. P-medium Medium priority

Comments

@Enet4
Copy link
Contributor

Enet4 commented Jun 23, 2017

I was recently formatting a signed integer primitive to hexadecimal using the standard formatter API, hoping that the result would be aware of the value's sign. It turns out that the implementations of UpperHex (and relatives such as LowerHex and Binary) for signed integers simply treat these numbers as unsigned (or just a sequence of bits).

println!("{:X}", -15i32);   // prints "FFFFFFF1",   expected "-F"

I posted this concern first as an SO question. A way around this is to make a newtype with another formatting implementation.

I can make arguments on both sides whether it should (or not) behave like this, but what actually concerns me most is that there seems to be no mention of this behaviour in the documentation. It appears that formatting trait implementations do not have to abide to a value's sign, but then the fact that a negative integer is treated as an unsigned number for formatting purposes can be unexpected for some people, especially when the docs do not clarify this situation.

To sum up: should we improve the documentation regarding what makes a valid formatting trait implementation? Should we also (or just) document further their implementations for integers in particular? I am willing to collaborate with the necessary changes one we're clear about what should be improved.

@Mark-Simulacrum Mark-Simulacrum added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Jun 23, 2017
@Mark-Simulacrum Mark-Simulacrum added the C-feature-request Category: A feature request, i.e: not implemented / a PR. label Jul 28, 2017
@dtolnay
Copy link
Member

dtolnay commented Nov 18, 2017

This representation dates back to #1653 which implemented UpperHex using *self as $unsigned in 6feb58e.

For what it's worth, iostream in C++ and printf in C both handle this the same way we do.

#include <iostream>

int main() {
  int32_t x = -15;
  std::cout << std::uppercase << std::hex << x << '\n';
}
#include <stdint.h>
#include <stdio.h>

int main(void) {
  int32_t x = -15;
  printf("%X\n", x);
}

I don't believe we need to change the behavior but I do agree that the expectations around implementation of UpperHex and friends needs to be documented better.

@dtolnay dtolnay added A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools and removed C-feature-request Category: A feature request, i.e: not implemented / a PR. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Nov 18, 2017
@Stargateur
Copy link
Contributor

Your code in C invoke undefined behavior, printf() family expect an unsigned int when you use the specifier X. If you don't send the right type to printf() the behavior is undefined. Plus, you use int32_t where an uint32_t will be undefined too.

@dtolnay
Copy link
Member

dtolnay commented Nov 19, 2017

Thanks @Stargateur. What would be the defined way to print a signed 32-bit integer in uppercase hex?

@Stargateur
Copy link
Contributor

Stargateur commented Nov 20, 2017

There are no way,

7.8.1 Macros for format specifiers (C11)
...
2 The fprintf macros for signed integers are:
PRIdN PRIdLEASTN PRIdFASTN PRIdMAX PRIdPTR
PRIiN PRIiLEASTN PRIiFASTN PRIiMAX PRIiPTR
3 The fprintf macros for unsigned integers are:
PRIoN PRIoLEASTN PRIoFASTN PRIoMAX PRIoPTR
PRIuN PRIuLEASTN PRIuFASTN PRIuMAX PRIuPTR
PRIxN PRIxLEASTN PRIxFASTN PRIxMAX PRIxPTR
PRIXN PRIXLEASTN PRIXFASTN PRIXMAX PRIXPTR

Like you see, standard don't give any specifier to print an intN_t in hexadecimal format. The correct way to print it would be to cast it.

#include <stdint.h>
#include <stdio.h>
#include <inttypes.h>

int main(void) {
  int32_t x = -15;
  printf("%"PRIX32"\n", (uint32_t)x);
}

Overflow of unsigned integer are defined by C standard so the cast is not undefined. AFAIK

@steveklabnik steveklabnik added the P-medium Medium priority label Nov 21, 2017
@XAMPPRocky XAMPPRocky added the C-enhancement Category: An issue proposing an enhancement or a PR with one. label Jan 22, 2018
@Phlosioneer
Copy link
Contributor

I think this was addressed by #46285

@Enet4
Copy link
Contributor Author

Enet4 commented Mar 11, 2018

@Phlosioneer That is correct! :) Closing.

@Enet4 Enet4 closed this as completed Mar 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools C-enhancement Category: An issue proposing an enhancement or a PR with one. P-medium Medium priority
Projects
None yet
Development

No branches or pull requests

7 participants