[llvm-rc] Allow -1 for control IDs in old style dialogs with 16 bit fields

[mstorsjo]

Summary

-1 is commonly used as ID for controls that one don't want to refer to later. For DIALOG resources, the IDs are 16 bit numbers, and -1 gets interpreted as UINT32_MAX earlier, which then later is too large to write into a uint16_t.

[mstorsjo]

Ping

[amccarth]

LGTM, but consider inline suggestion.

tools/llvm-rc/ResourceFileWriter.cpp

986

This is fine, but I had to stop and wonder if there's an assumption about two's-complement.

Why not:

if (Ctl.ID != static_cast<uint32_t>(-1))

[mstorsjo]

tools/llvm-rc/ResourceFileWriter.cpp

986

Thanks, that's even more readable, will try that before commiting!

test/tools/llvm-rc/Inputs/tag-dialog.rc

Types DIALOGEX 12345, -11215, 0x1234, 0x1EED, 0x51525354 {
   LTEXT 65535, 3001, 3002, 3003, 3004, 3005
 }
 
 EmptyOld DIALOG 1, 2, 3, 4 {}
 
 ArgsOld DIALOG 1, 2, 3, 4 {
   LTEXT "L", 1, 2, 3, 4, 5
   LTEXT "L2", 6, 7, 8, 9, 10, 11
-  LTEXT "L3", 12, 13, 14, 15, 16, 17, 18
+  LTEXT "L3", -1, 13, 14, 15, 16, 17, 18
 
   EDITTEXT 19, 20, 21, 22, 23
   EDITTEXT 24, 25, 26, 27, 28, 29
   EDITTEXT 30, 31, 32, 33, 34, 35, 36
 }
 

test/tools/llvm-rc/tag-dialog.test

 ; DIALOG-NEXT: Data size: 198
 ; DIALOG-NEXT: Data: (
 ; DIALOG-NEXT:   0000: 00008880 00000000 06000100 02000300  |................|
 ; DIALOG-NEXT:   0010: 04000000 00000000 00000250 00000000  |...........P....|
 ; DIALOG-NEXT:   0020: 02000300 04000500 0100FFFF 82004C00  |..............L.|
 ; DIALOG-NEXT:   0030: 00000000 0B000250 00000000 07000800  |.......P........|
 ; DIALOG-NEXT:   0040: 09000A00 0600FFFF 82004C00 32000000  |..........L.2...|
 ; DIALOG-NEXT:   0050: 00000000 11000250 12000000 0D000E00  |.......P........|
-; DIALOG-NEXT:   0060: 0F001000 0C00FFFF 82004C00 33000000  |..........L.3...|
+; DIALOG-NEXT:   0060: 0F001000 FFFFFFFF 82004C00 33000000  |..........L.3...|
 ; DIALOG-NEXT:   0070: 00000000 00008150 00000000 14001500  |.......P........|
 ; DIALOG-NEXT:   0080: 16001700 1300FFFF 81000000 00000000  |................|
 ; DIALOG-NEXT:   0090: 1D008150 00000000 19001A00 1B001C00  |...P............|
 ; DIALOG-NEXT:   00A0: 1800FFFF 81000000 00000000 23008150  |............#..P|
 ; DIALOG-NEXT:   00B0: 24000000 1F002000 21002200 1E00FFFF  |$..... .!.".....|
 ; DIALOG-NEXT:   00C0: 81000000 0000                        |......|
 ; DIALOG-NEXT: )
 

tools/llvm-rc/ResourceFileWriter.cpp

struct {
     ulittle16_t Width;
     ulittle16_t Height;
   } Middle{ulittle16_t(Ctl.X), ulittle16_t(Ctl.Y), ulittle16_t(Ctl.Width),
            ulittle16_t(Ctl.Height)};
   writeObject(Middle);
 
   // ID; it's 16-bit in DIALOG and 32-bit in DIALOGEX.
   if (!IsExtended) {
+    // It's common to use -1, i.e. UINT32_MAX, for controls one doesn't
+    // want to refer to later.
+    if (Ctl.ID != std::numeric_limits<uint32_t>::max())
       RETURN_IF_ERROR(checkNumberFits<uint16_t>(
           Ctl.ID, "Control ID in simple DIALOG resource"));
     writeInt<uint16_t>(Ctl.ID);
   } else {
     writeInt<uint32_t>(Ctl.ID);
   }
 
   // Window class - either 0xFFFF + 16-bit integer or a string.
   RETURN_IF_ERROR(writeIntOrString(IntOrString(TypeInfo.CtlClass)));