Skip to content

[clang][Fuchsia] Use unsigned int for wint_t on *-fuchsia targets #95499

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

Merged
merged 2 commits into from
Jun 14, 2024

Conversation

frobtech
Copy link
Contributor

This aligns Fuchsia targets with other similar OS targets such as
Linux. Fuchsia's libc already uses unsigned rather than the
compiler-provided WINT_TYPE macro for its wint_t typedef, so
this just makes the compiler consistent with the OS's actual ABI.
The only known manifestation of the mismatch is -Wformat warnings
for %lc no matching wint_t arguments.

The closest thing I could see to existing tests for each target's
wint_t type setting was the predefine tests that check various
macros including WINT_TYPE on a per-machine and/or per-OS
basis. While the setting is done per-OS in most of the target
implementations rather than actually varying by machine, the only
existing tests for WINT_TYPE are in per-machine checks that
are also wholly or partly tagged as per-OS. x86_64 and riscv64
tests for respective *-linux-gnu targets now check for the same
definitions in the respective *-fuchsia targets. WINT_TYPE
is not among the type checked in the aarch64 tests and those lack
a section that's specifically tested for aarch64-linux-gnu; if
such is added then it can similarly be made to check for most or
all of the same value on aarch64-fuchsia as aarch64-linux-gnu.
But since the actual implementation of choosing the type is done
per-OS and not per-machine for the three machines with Fuchsia
target support, the x86 and riscv64 tests are already redundantly
testing that same code and seem sufficient.

frobtech added 2 commits June 13, 2024 20:25
This aligns Fuchsia targets with other similar OS targets such as
Linux.  Fuchsia's libc already uses unsigned rather than the
compiler-provided __WINT_TYPE__ macro for its wint_t typedef, so
this just makes the compiler consistent with the OS's actual ABI.
The only known manifestation of the mismatch is -Wformat warnings
for %lc no matching wint_t arguments.

The closest thing I could see to existing tests for each target's
wint_t type setting was the predefine tests that check various
macros including __WINT_TYPE__ on a per-machine and/or per-OS
basis.  While the setting is done per-OS in most of the target
implementations rather than actually varying by machine, the only
existing tests for __WINT_TYPE__ are in per-machine checks that
are also wholly or partly tagged as per-OS.  x86_64 and riscv64
tests for respective *-linux-gnu targets now check for the same
definitions in the respective *-fuchsia targets.  __WINT_TYPE__
is not among the type checked in the aarch64 tests and those lack
a section that's specifically tested for aarch64-linux-gnu; if
such is added then it can similarly be made to check for most or
all of the same value on aarch64-fuchsia as aarch64-linux-gnu.
But since the actual implementation of choosing the type is done
per-OS and not per-machine for the three machines with Fuchsia
target support, the x86 and riscv64 tests are already redundantly
testing that same code and seem sufficient.
@frobtech frobtech marked this pull request as ready for review June 14, 2024 03:49
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Jun 14, 2024
@llvmbot
Copy link
Member

llvmbot commented Jun 14, 2024

@llvm/pr-subscribers-clang

Author: Roland McGrath (frobtech)

Changes

This aligns Fuchsia targets with other similar OS targets such as
Linux. Fuchsia's libc already uses unsigned rather than the
compiler-provided WINT_TYPE macro for its wint_t typedef, so
this just makes the compiler consistent with the OS's actual ABI.
The only known manifestation of the mismatch is -Wformat warnings
for %lc no matching wint_t arguments.

The closest thing I could see to existing tests for each target's
wint_t type setting was the predefine tests that check various
macros including WINT_TYPE on a per-machine and/or per-OS
basis. While the setting is done per-OS in most of the target
implementations rather than actually varying by machine, the only
existing tests for WINT_TYPE are in per-machine checks that
are also wholly or partly tagged as per-OS. x86_64 and riscv64
tests for respective *-linux-gnu targets now check for the same
definitions in the respective *-fuchsia targets. WINT_TYPE
is not among the type checked in the aarch64 tests and those lack
a section that's specifically tested for aarch64-linux-gnu; if
such is added then it can similarly be made to check for most or
all of the same value on aarch64-fuchsia as aarch64-linux-gnu.
But since the actual implementation of choosing the type is done
per-OS and not per-machine for the three machines with Fuchsia
target support, the x86 and riscv64 tests are already redundantly
testing that same code and seem sufficient.


Full diff: https://github.com/llvm/llvm-project/pull/95499.diff

3 Files Affected:

  • (modified) clang/lib/Basic/Targets/OSTargets.h (+1)
  • (modified) clang/test/Preprocessor/init-x86.c (+1)
  • (modified) clang/test/Preprocessor/init.c (+2)
diff --git a/clang/lib/Basic/Targets/OSTargets.h b/clang/lib/Basic/Targets/OSTargets.h
index 4366c1149e405..5f27c3469f861 100644
--- a/clang/lib/Basic/Targets/OSTargets.h
+++ b/clang/lib/Basic/Targets/OSTargets.h
@@ -868,6 +868,7 @@ class LLVM_LIBRARY_VISIBILITY FuchsiaTargetInfo : public OSTargetInfo<Target> {
 public:
   FuchsiaTargetInfo(const llvm::Triple &Triple, const TargetOptions &Opts)
       : OSTargetInfo<Target>(Triple, Opts) {
+    this->WIntType = TargetInfo::UnsignedInt;
     this->MCountName = "__mcount";
     this->TheCXXABI.set(TargetCXXABI::Fuchsia);
   }
diff --git a/clang/test/Preprocessor/init-x86.c b/clang/test/Preprocessor/init-x86.c
index 1268419e18a5c..6f5aa5674e48e 100644
--- a/clang/test/Preprocessor/init-x86.c
+++ b/clang/test/Preprocessor/init-x86.c
@@ -999,6 +999,7 @@
 // X32:#define __x86_64__ 1
 
 // RUN: %clang_cc1 -E -dM -ffreestanding -fgnuc-version=4.2.1 -triple=x86_64-pc-linux-gnu < /dev/null | FileCheck -match-full-lines -check-prefix X86_64-LINUX %s
+// RUN: %clang_cc1 -E -dM -ffreestanding -fgnuc-version=4.2.1 -triple=x86_64-unknown-fuchsia < /dev/null | FileCheck -match-full-lines -check-prefix X86_64-LINUX %s
 //
 // X86_64-LINUX:#define _LP64 1
 // X86_64-LINUX:#define __BIGGEST_ALIGNMENT__ 16
diff --git a/clang/test/Preprocessor/init.c b/clang/test/Preprocessor/init.c
index 2641fee940231..6e7c0ea5c730b 100644
--- a/clang/test/Preprocessor/init.c
+++ b/clang/test/Preprocessor/init.c
@@ -2527,6 +2527,8 @@
 // RUN:   | FileCheck -match-full-lines -check-prefix=RISCV64 %s
 // RUN: %clang_cc1 -E -dM -ffreestanding -fgnuc-version=4.2.1 -triple=riscv64-unknown-linux < /dev/null \
 // RUN:   | FileCheck -match-full-lines -check-prefixes=RISCV64,RISCV64-LINUX %s
+// RUN: %clang_cc1 -E -dM -ffreestanding -fgnuc-version=4.2.1 -triple=riscv64-unknown-fuchsia < /dev/null \
+// RUN:   | FileCheck -match-full-lines -check-prefixes=RISCV64 %s
 // RISCV64: #define _LP64 1
 // RISCV64: #define __ATOMIC_ACQUIRE 2
 // RISCV64: #define __ATOMIC_ACQ_REL 4

@frobtech frobtech merged commit 4ab37e4 into llvm:main Jun 14, 2024
9 of 10 checks passed
@frobtech frobtech deleted the p/fuchsia-wint_t branch June 14, 2024 06:40
EthanLuisMcDonough pushed a commit to EthanLuisMcDonough/llvm-project that referenced this pull request Aug 13, 2024
…vm#95499)

This aligns Fuchsia targets with other similar OS targets such as
Linux.  Fuchsia's libc already uses unsigned rather than the
compiler-provided __WINT_TYPE__ macro for its wint_t typedef, so
this just makes the compiler consistent with the OS's actual ABI.
The only known manifestation of the mismatch is -Wformat warnings
for %lc no matching wint_t arguments.

The closest thing I could see to existing tests for each target's
wint_t type setting was the predefine tests that check various
macros including __WINT_TYPE__ on a per-machine and/or per-OS
basis.  While the setting is done per-OS in most of the target
implementations rather than actually varying by machine, the only
existing tests for __WINT_TYPE__ are in per-machine checks that
are also wholly or partly tagged as per-OS.  x86_64 and riscv64
tests for respective *-linux-gnu targets now check for the same
definitions in the respective *-fuchsia targets.  __WINT_TYPE__
is not among the type checked in the aarch64 tests and those lack
a section that's specifically tested for aarch64-linux-gnu; if
such is added then it can similarly be made to check for most or
all of the same value on aarch64-fuchsia as aarch64-linux-gnu.
But since the actual implementation of choosing the type is done
per-OS and not per-machine for the three machines with Fuchsia
target support, the x86 and riscv64 tests are already redundantly
testing that same code and seem sufficient.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants