From 9702d8605e4a163d24c62c3d3f43842032c01cc9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Storsj=C3=B6?= 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()) {