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

[clang][Sema] Question: Why is -Wshorten-64-to-32 disjoint from -Wimplicit-int-conversion? #69444

Closed
whisperity opened this issue Oct 18, 2023 · 3 comments · Fixed by #80814
Closed
Labels
c++ clang:frontend Language frontend issues, e.g. anything involving "Sema" enhancement Improving things as opposed to bug fixing, e.g. new or missing feature

Comments

@whisperity
Copy link
Member

An interesting case was encountered where conversions from larger bit-width integers to smaller bit-width ones is generally reported by -Wimplicit-int-conversion (32 to 16, 16 to 8, 128 to 64, etc.) except for 64 to 32, which was given its own warning: -Wshorten-64-to-32.

See http://godbolt.org/z/6q4GMPsxs. Lines 19 (uint64_t -> uint32_t) and 35 (int64_t -> int32_t) are not warned about, unless "Compiler 2"'s warning is enabled.

Although -Weverything enables both of them at the same time (and neither is enabled by default, AFAIK), it is strange that 64->32 is such a specific dedicated case.

Unfortunately, all I could track down was commit 263a48b where this warning was introduced, and that is ancient, but history runs dry at that point.

CC @dwblaikie, @rjmccall. Your names appeared in the relevant code path.


What would be the cons against enabling -Wshorten-64-to-32 under -Wimplicit-int-conversion implicitly?

@whisperity whisperity added enhancement Improving things as opposed to bug fixing, e.g. new or missing feature question A question, not bug report. Check out https://llvm.org/docs/GettingInvolved.html instead! c++ clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Oct 18, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Oct 18, 2023

@llvm/issue-subscribers-c-1

Author: None (whisperity)

An interesting case was encountered where conversions from larger bit-width integers to smaller bit-width ones is *generally* reported by `-Wimplicit-int-conversion` (32 to 16, 16 to 8, 128 to 64, etc.) **except** for 64 to 32, which was given its own warning: `-Wshorten-64-to-32`.

See http://godbolt.org/z/6q4GMPsxs. Lines 19 (uint64_t -> uint32_t) and 35 (int64_t -> int32_t) are not warned about, unless "Compiler 2"'s warning is enabled.

Although -Weverything enables both of them at the same time (and neither is enabled by default, AFAIK), it is strange that 64->32 is such a specific dedicated case.

Unfortunately, all I could track down was commit 263a48b where this warning was introduced, and that is ancient, but history runs dry at that point.

> CC @dwblaikie, @rjmccall. Your names appeared in the relevant code path.


What would be the cons against enabling -Wshorten-64-to-32 under -Wimplicit-int-conversion implicitly?

@llvmbot
Copy link
Collaborator

llvmbot commented Oct 18, 2023

@llvm/issue-subscribers-clang-frontend

Author: None (whisperity)

An interesting case was encountered where conversions from larger bit-width integers to smaller bit-width ones is *generally* reported by `-Wimplicit-int-conversion` (32 to 16, 16 to 8, 128 to 64, etc.) **except** for 64 to 32, which was given its own warning: `-Wshorten-64-to-32`.

See http://godbolt.org/z/6q4GMPsxs. Lines 19 (uint64_t -> uint32_t) and 35 (int64_t -> int32_t) are not warned about, unless "Compiler 2"'s warning is enabled.

Although -Weverything enables both of them at the same time (and neither is enabled by default, AFAIK), it is strange that 64->32 is such a specific dedicated case.

Unfortunately, all I could track down was commit 263a48b where this warning was introduced, and that is ancient, but history runs dry at that point.

> CC @dwblaikie, @rjmccall. Your names appeared in the relevant code path.


What would be the cons against enabling -Wshorten-64-to-32 under -Wimplicit-int-conversion implicitly?

@rjmccall
Copy link
Contributor

rjmccall commented Oct 18, 2023

IIRC, -Wshorten-64-to-32 was an earlier warning implemented specifically to help with 32-to-64-bit portability problems, and then the other warnings were generalizations of that logic. I can't think of any reason why they should be disjoint, though; the more general groups should include -Wshorten-64-to-32.

@EugeneZelenko EugeneZelenko removed the question A question, not bug report. Check out https://llvm.org/docs/GettingInvolved.html instead! label Feb 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ clang:frontend Language frontend issues, e.g. anything involving "Sema" enhancement Improving things as opposed to bug fixing, e.g. new or missing feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants