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

[SYCL][Sema] Only imply float size conversion warnings in SYCL #16857

Open
wants to merge 4 commits into
base: sycl
Choose a base branch
from

Conversation

Maetveis
Copy link
Contributor

@Maetveis Maetveis commented Jan 31, 2025

Before this change -Wimplicit-float-conversion (part of -Wconversion) enabled -Wimplicit-float-size-conversion for all language modes.

This is problematic because it causes DPC++ to emit warnings where upstream clang does not, even when used as a plain C++ compiler. Projects that enable -Werror don't expect these (questionable) floating point size warnings when they enable -Wconversion, and as such cannot be compiled with the DPC++ compiler.

Change -Wimplicit-float-conversion to only be emitted in SYCL language mode. This preserves existing behaviour for SYCL users, but otherwise matches upstream Clang behaviour.

Fixes: #16393

Before this change `-Wimplicit-float-conversion` (part of `-Wconversion`)
enabled `-Wimplicit-float-size-conversion` for all language modes.

This is problematic because it causes it makes DPC++ emit warnings where
upstream clang does not, even when used as a plain C++ compiler.
Projects that enable `-Werror` don't expect these (questionable)
floating point size warnings when they enable `-Wconversion`, and as such
cannot be compiled with the DPC++ compiler.

Change `-Wimplicit-float-conversion` to imply
`-Wsycl-implicit-float-size-conversion` instead, which is only emitted
in SYCL language mode. This preserves existing behaviour for SYCL users.

Explicitly passing `-Wimplicit-float-size-conversion` still applies to all
language modes.

This change only affects users who enable `-Wimplicit-float-conversion`
but do not enable `-Wimplicit-float-size-conversion` explicitly and do
not compile in SYCL language mode. These users would need to enable
the option explicitly to keep the existing behaviour.
@Maetveis Maetveis requested a review from a team as a code owner January 31, 2025 15:01
@Maetveis
Copy link
Contributor Author

Maetveis commented Feb 4, 2025

@intel/dpcpp-cfe-reviewers please review :)

clang/lib/Sema/SemaChecking.cpp Outdated Show resolved Hide resolved
clang/lib/Sema/SemaChecking.cpp Outdated Show resolved Hide resolved
clang/test/Sema/conversion.c Show resolved Hide resolved
clang/test/Sema/precision-and-size-conversion.c Outdated Show resolved Hide resolved
- Only emit `-Wimplicit-float-size-conversion` warnings in SYCL language
  mode
- Remove `-Wsycl-implicit-float-size-conversion`
- Revert changes to llvm community tests
- Update tests
@Maetveis
Copy link
Contributor Author

Maetveis commented Feb 5, 2025

@elizabethandrews

Simplified the logic now -Wimplicit-float-size-conversion is SYCL only. Is the warning option name still good as -Wimplicit-float-size-conversion?

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

Successfully merging this pull request may close these issues.

-Wimplicit-float-size-conversion is implied by -Wimplicit-float-conversion
3 participants