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

Resolve implicit trunc TODOs #112510

Open
nikic opened this issue Oct 16, 2024 · 1 comment
Open

Resolve implicit trunc TODOs #112510

nikic opened this issue Oct 16, 2024 · 1 comment
Labels

Comments

@nikic
Copy link
Contributor

nikic commented Oct 16, 2024

#106524 introduces a consistency assertion in the APInt constructor (currently still disabled by default), which verifies that the passed value is indeed a signed/unsigned value of the indicated bit width.

To allow this assertion to be enabled by default, a number of // TODO: Avoid implicit trunc? comments were left behind, in cases where it was not immediately clear whether/how the implicit truncation in the APInt constructor could be avoided.

Long term, these should be resolved in one three ways:

  • Determine that the implicit truncation is actually desirable/acceptable in the given context. Replace the TODO with a comment explaining why.
  • Change the code to avoid passing values that require implicit truncation.
  • Make an API change, e.g. by propagating the implicitTrunc parameter up, so that some users can use it and some not. Or by accepting an APInt instead of a plain integer and leaving the proper construction to callers.
@llvmbot
Copy link
Member

llvmbot commented Oct 16, 2024

@llvm/issue-subscribers-mlir

Author: Nikita Popov (nikic)

https://github.com//pull/106524 introduces a consistency assertion in the APInt constructor (currently still disabled by default), which verifies that the passed value is indeed a signed/unsigned value of the indicated bit width.

To allow this assertion to be enabled by default, a number of // TODO: Avoid implicit trunc? comments were left behind, in cases where it was not immediately clear whether/how the implicit truncation in the APInt constructor could be avoided.

Long term, these should be resolved in one three ways:

  • Determine that the implicit truncation is actually desirable/acceptable in the given context. Replace the TODO with a comment explaining why.
  • Change the code to avoid passing values that require implicit truncation.
  • Make an API change, e.g. by propagating the implicitTrunc parameter up, so that some users can use it and some not. Or by accepting an APInt instead of a plain integer and leaving the proper construction to callers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants