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

[C++] Compiler warnings with clang + -Wconversion -Wno-sign-conversion in public headers #39185

Closed
paleolimbot opened this issue Dec 11, 2023 · 0 comments · Fixed by #39186
Closed
Assignees
Milestone

Comments

@paleolimbot
Copy link
Member

paleolimbot commented Dec 11, 2023

Describe the bug, including details regarding any error messages, version, and platform.

There are also a number of R-specific warnings that occur (#39138), but a few are in the C++ library in public headers. I am not sure if this is a compiler warning scheme that is meaningful to adhere to for the entire C++ library; however, it seems reasonable to add the requisite static casts in headers included by other projects.

Component(s)

C++

paleolimbot added a commit that referenced this issue Dec 12, 2023
…-conversion` in public headers (#39186)

### Rationale for this change

The R package has a warning from CRAN to fix a failure to compile with `-Wconversion -Wno-sign-conversion -Werror`. Some of these errors we control and can patch easily; however, the ones in the Arrow C++ portion are more difficult to work around (hence the separate PR). See #39138 for all reported errors (including those in just the R package).

### What changes are included in this PR?

The requisite `static_cast<>()`s were added to silence the warnings.

### Are these changes tested?

By existing tests. We may add a future R nightly job that runs with these warning flags.

### Are there any user-facing changes?

No
* Closes: #39185

Authored-by: Dewey Dunnington <dewey@fishandwhistle.net>
Signed-off-by: Dewey Dunnington <dewey@fishandwhistle.net>
@paleolimbot paleolimbot added this to the 15.0.0 milestone Dec 12, 2023
mapleFU pushed a commit to mapleFU/arrow that referenced this issue Dec 13, 2023
…o-sign-conversion` in public headers (apache#39186)

### Rationale for this change

The R package has a warning from CRAN to fix a failure to compile with `-Wconversion -Wno-sign-conversion -Werror`. Some of these errors we control and can patch easily; however, the ones in the Arrow C++ portion are more difficult to work around (hence the separate PR). See apache#39138 for all reported errors (including those in just the R package).

### What changes are included in this PR?

The requisite `static_cast<>()`s were added to silence the warnings.

### Are these changes tested?

By existing tests. We may add a future R nightly job that runs with these warning flags.

### Are there any user-facing changes?

No
* Closes: apache#39185

Authored-by: Dewey Dunnington <dewey@fishandwhistle.net>
Signed-off-by: Dewey Dunnington <dewey@fishandwhistle.net>
assignUser pushed a commit that referenced this issue Dec 23, 2023
…-conversion` in public headers (#39186)

### Rationale for this change

The R package has a warning from CRAN to fix a failure to compile with `-Wconversion -Wno-sign-conversion -Werror`. Some of these errors we control and can patch easily; however, the ones in the Arrow C++ portion are more difficult to work around (hence the separate PR). See #39138 for all reported errors (including those in just the R package).

### What changes are included in this PR?

The requisite `static_cast<>()`s were added to silence the warnings.

### Are these changes tested?

By existing tests. We may add a future R nightly job that runs with these warning flags.

### Are there any user-facing changes?

No
* Closes: #39185

Authored-by: Dewey Dunnington <dewey@fishandwhistle.net>
Signed-off-by: Dewey Dunnington <dewey@fishandwhistle.net>
clayburn pushed a commit to clayburn/arrow that referenced this issue Jan 23, 2024
…o-sign-conversion` in public headers (apache#39186)

### Rationale for this change

The R package has a warning from CRAN to fix a failure to compile with `-Wconversion -Wno-sign-conversion -Werror`. Some of these errors we control and can patch easily; however, the ones in the Arrow C++ portion are more difficult to work around (hence the separate PR). See apache#39138 for all reported errors (including those in just the R package).

### What changes are included in this PR?

The requisite `static_cast<>()`s were added to silence the warnings.

### Are these changes tested?

By existing tests. We may add a future R nightly job that runs with these warning flags.

### Are there any user-facing changes?

No
* Closes: apache#39185

Authored-by: Dewey Dunnington <dewey@fishandwhistle.net>
Signed-off-by: Dewey Dunnington <dewey@fishandwhistle.net>
dgreiss pushed a commit to dgreiss/arrow that referenced this issue Feb 19, 2024
…o-sign-conversion` in public headers (apache#39186)

### Rationale for this change

The R package has a warning from CRAN to fix a failure to compile with `-Wconversion -Wno-sign-conversion -Werror`. Some of these errors we control and can patch easily; however, the ones in the Arrow C++ portion are more difficult to work around (hence the separate PR). See apache#39138 for all reported errors (including those in just the R package).

### What changes are included in this PR?

The requisite `static_cast<>()`s were added to silence the warnings.

### Are these changes tested?

By existing tests. We may add a future R nightly job that runs with these warning flags.

### Are there any user-facing changes?

No
* Closes: apache#39185

Authored-by: Dewey Dunnington <dewey@fishandwhistle.net>
Signed-off-by: Dewey Dunnington <dewey@fishandwhistle.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant