[llvm] [llvm-rc] Allow ALT on non-virtkey accelerators (PR #143374)
Martin Storsjö via llvm-commits
llvm-commits at lists.llvm.org
Mon Jun 9 05:21:42 PDT 2025
https://github.com/mstorsjo created https://github.com/llvm/llvm-project/pull/143374
While https://learn.microsoft.com/en-us/windows/win32/menurc/accelerators-resource specifies that ALT only applies to virtkeys, this doesn't seem to be the case in reality.
https://learn.microsoft.com/en-us/windows/win32/menurc/using-keyboard-accelerators contains an example that uses this combination:
"B", ID_ACCEL5, ALT ; ALT_SHIFT+B
Also Microsoft also includes such cases in their repo of test cases: https://github.com/microsoft/Windows-classic-samples/blob/263dd514ad215d0a40d1ec44b4df84b30ec11dcf/Samples/Win7Samples/begin/sdkdiff/sdkdiff.rc#L161-L164
Also MS rc.exe doesn't warn/error about this. However if applying SHIFT or CONTROL on a non-virtkey accelerator, MS rc.exe does produce this warning:
warning RC4203 : SHIFT or CONTROL used without VIRTKEY
Hence, keep the checks for SHIFT and CONTROL, but remove the checks for ALT, which seems to have been incorrect.
This fixes one aspect of
https://github.com/llvm/llvm-project/issues/143157.
>From 9702d8605e4a163d24c62c3d3f43842032c01cc9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Martin=20Storsj=C3=B6?= <martin at martin.st>
Date: Mon, 9 Jun 2025 15:16:23 +0300
Subject: [PATCH] [llvm-rc] Allow ALT on non-virtkey accelerators
While https://learn.microsoft.com/en-us/windows/win32/menurc/accelerators-resource
specifies that ALT only applies to virtkeys, this doesn't seem
to be the case in reality.
https://learn.microsoft.com/en-us/windows/win32/menurc/using-keyboard-accelerators
contains an example that uses this combination:
"B", ID_ACCEL5, ALT ; ALT_SHIFT+B
Also Microsoft also includes such cases in their repo of test
cases: https://github.com/microsoft/Windows-classic-samples/blob/263dd514ad215d0a40d1ec44b4df84b30ec11dcf/Samples/Win7Samples/begin/sdkdiff/sdkdiff.rc#L161-L164
Also MS rc.exe doesn't warn/error about this. However if applying
SHIFT or CONTROL on a non-virtkey accelerator, MS rc.exe does
produce this warning:
warning RC4203 : SHIFT or CONTROL used without VIRTKEY
Hence, keep the checks for SHIFT and CONTROL, but remove the checks
for ALT, which seems to have been incorrect.
This fixes one aspect of
https://github.com/llvm/llvm-project/issues/143157.
---
.../llvm-rc/Inputs/tag-accelerators-ascii-alt.rc | 4 ----
.../test/tools/llvm-rc/Inputs/tag-accelerators.rc | 1 +
llvm/test/tools/llvm-rc/tag-accelerators.test | 15 +++++----------
llvm/tools/llvm-rc/ResourceFileWriter.cpp | 4 ++--
4 files changed, 8 insertions(+), 16 deletions(-)
delete mode 100644 llvm/test/tools/llvm-rc/Inputs/tag-accelerators-ascii-alt.rc
diff --git a/llvm/test/tools/llvm-rc/Inputs/tag-accelerators-ascii-alt.rc b/llvm/test/tools/llvm-rc/Inputs/tag-accelerators-ascii-alt.rc
deleted file mode 100644
index 363263bfe4cf2..0000000000000
--- a/llvm/test/tools/llvm-rc/Inputs/tag-accelerators-ascii-alt.rc
+++ /dev/null
@@ -1,4 +0,0 @@
-2 ACCELERATORS {
- "A", 15, ASCII, ALT
-}
-
diff --git a/llvm/test/tools/llvm-rc/Inputs/tag-accelerators.rc b/llvm/test/tools/llvm-rc/Inputs/tag-accelerators.rc
index 90e7f926cc087..bcfc35bdeab68 100644
--- a/llvm/test/tools/llvm-rc/Inputs/tag-accelerators.rc
+++ b/llvm/test/tools/llvm-rc/Inputs/tag-accelerators.rc
@@ -110,5 +110,6 @@ LANGUAGE 5, 1
"7", 71, VIRTKEY, NOINVERT, CONTROL, SHIFT, ALT
"^j", 72, ASCII
"^j", 73, ASCII, NOINVERT
+ "A", 15, ASCII, ALT
}
diff --git a/llvm/test/tools/llvm-rc/tag-accelerators.test b/llvm/test/tools/llvm-rc/tag-accelerators.test
index 336727f617687..4f44aebc75011 100644
--- a/llvm/test/tools/llvm-rc/tag-accelerators.test
+++ b/llvm/test/tools/llvm-rc/tag-accelerators.test
@@ -37,7 +37,7 @@
; ACCELERATORS-NEXT: Version (major): 0
; ACCELERATORS-NEXT: Version (minor): 0
; ACCELERATORS-NEXT: Characteristics: 0
-; ACCELERATORS-NEXT: Data size: 592
+; ACCELERATORS-NEXT: Data size: 600
; ACCELERATORS-NEXT: Data: (
; ACCELERATORS-NEXT: 0000: 00002A00 00000000 01002A00 01000000 |..*.......*.....|
; ACCELERATORS-NEXT: 0010: 02002A00 02000000 03002A00 03000000 |..*.......*.....|
@@ -75,7 +75,8 @@
; ACCELERATORS-NEXT: 0210: 15003700 42000000 0F003700 43000000 |..7.B.....7.C...|
; ACCELERATORS-NEXT: 0220: 1B003700 44000000 17003700 45000000 |..7.D.....7.E...|
; ACCELERATORS-NEXT: 0230: 1D003700 46000000 1F003700 47000000 |..7.F.....7.G...|
-; ACCELERATORS-NEXT: 0240: 00000A00 48000000 82000A00 49000000 |....H.......I...|
+; ACCELERATORS-NEXT: 0240: 00000A00 48000000 02000A00 49000000 |....H.......I...|
+; ACCELERATORS-NEXT: 0250: 90004100 0F000000 |..A.....|
; ACCELERATORS-NEXT: )
@@ -94,19 +95,13 @@
; RUN: not llvm-rc -no-preprocess /FO %t -- %p/Inputs/tag-accelerators-ascii-control.rc 2>&1 | FileCheck %s --check-prefix ASCII2
; ASCII2: llvm-rc: Error in ACCELERATORS statement (ID 2):
-; ASCII2-NEXT: Accelerator ID 15: Can only apply ALT, SHIFT or CONTROL to VIRTKEY accelerators
+; ASCII2-NEXT: Accelerator ID 15: Can only apply SHIFT or CONTROL to VIRTKEY accelerators
; RUN: not llvm-rc -no-preprocess /FO %t -- %p/Inputs/tag-accelerators-ascii-shift.rc 2>&1 | FileCheck %s --check-prefix ASCII3
; ASCII3: llvm-rc: Error in ACCELERATORS statement (ID 2):
-; ASCII3-NEXT: Accelerator ID 15: Can only apply ALT, SHIFT or CONTROL to VIRTKEY accelerators
-
-
-; RUN: not llvm-rc -no-preprocess /FO %t -- %p/Inputs/tag-accelerators-ascii-alt.rc 2>&1 | FileCheck %s --check-prefix ASCII4
-
-; ASCII4: llvm-rc: Error in ACCELERATORS statement (ID 2):
-; ASCII4-NEXT: Accelerator ID 15: Can only apply ALT, SHIFT or CONTROL to VIRTKEY accelerators
+; ASCII3-NEXT: Accelerator ID 15: Can only apply SHIFT or CONTROL to VIRTKEY accelerators
; RUN: not llvm-rc -no-preprocess /FO %t -- %p/Inputs/tag-accelerators-bad-key-id.rc 2>&1 | FileCheck %s --check-prefix BADKEYID
diff --git a/llvm/tools/llvm-rc/ResourceFileWriter.cpp b/llvm/tools/llvm-rc/ResourceFileWriter.cpp
index dc21f63a682aa..2c35c5ad365dc 100644
--- a/llvm/tools/llvm-rc/ResourceFileWriter.cpp
+++ b/llvm/tools/llvm-rc/ResourceFileWriter.cpp
@@ -631,8 +631,8 @@ Error ResourceFileWriter::writeSingleAccelerator(
if (IsASCII && IsVirtKey)
return createAccError("Accelerator can't be both ASCII and VIRTKEY");
- if (!IsVirtKey && (Obj.Flags & (Opt::ALT | Opt::SHIFT | Opt::CONTROL)))
- return createAccError("Can only apply ALT, SHIFT or CONTROL to VIRTKEY"
+ if (!IsVirtKey && (Obj.Flags & (Opt::SHIFT | Opt::CONTROL)))
+ return createAccError("Can only apply SHIFT or CONTROL to VIRTKEY"
" accelerators");
if (Obj.Event.isInt()) {
More information about the llvm-commits
mailing list