[llvm-rc] Support EXSTYLE statement.

[phosek]

LGTM although you may want to get a review from someone who's more familiar with llvm-rc.

tools/llvm-rc/ResourceScriptStmt.h

872

Nit: I'd consider naming it just ExStyle to match the name of the statement precisely.

[mstorsjo]

Looks great in general, but I think I agree that having it named ExStyle would be less prone to confusion.

tools/llvm-rc/ResourceVisitor.h

53

These were alpabetically ordered before. The same goes for a few other places in the patch as well.

[jacek]

Thanks for reviews, I addressed comments.

FWIW, I used ExtStyle instead of ExStyle in previous version because that's what's used in a few places in sources already, but I agree that ExStyle fits better.

[mstorsjo]

LGTM

However, I can't commit it as is, as the diff for test/tools/llvm-rc/Inputs/tag-dialog-headers.rc is missing, as it's treated as binary. I'm not sure exactly why we have that there, but you can e.g. remove the relevant line from .gitattributes before diffing, to get that file included in the diff.

tools/llvm-rc/ResourceVisitor.h

30

This one is still unordered, but I could change that before committing.

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

 ; HEADERS-NEXT: Version (minor): 0
 ; HEADERS-NEXT: Characteristics: 0
 ; HEADERS-NEXT: Data size: 26
 ; HEADERS-NEXT: Data: (
 ; HEADERS-NEXT:   0000: 00008880 00000000 00000500 05000500  |................|
 ; HEADERS-NEXT:   0010: 05000000 FFFF2A00 0000               |......*...|
 ; HEADERS-NEXT: )
 
+; HEADERS-DAG: Resource type (int): 5
+; HEADERS-NEXT: Resource name (int): 29
+; HEADERS-NEXT: Data version: 0
+; HEADERS-NEXT: Memory flags: 0x1030
+; HEADERS-NEXT: Language ID: 1033
+; HEADERS-NEXT: Version (major): 0
+; HEADERS-NEXT: Version (minor): 0
+; HEADERS-NEXT: Characteristics: 0
+; HEADERS-NEXT: Data size: 32
+; HEADERS-NEXT: Data: (
+; HEADERS-NEXT:   0000: 0100FFFF 00000000 67452301 00008880  |........gE#.....|
+; HEADERS-NEXT:   0010: 00000000 01000000 09000000 00000000  |................|
+; HEADERS-NEXT: )
+
+; HEADERS-DAG: Resource type (int): 5
+; HEADERS-NEXT: Resource name (int): 30
+; HEADERS-NEXT: Data version: 0
+; HEADERS-NEXT: Memory flags: 0x1030
+; HEADERS-NEXT: Language ID: 1033
+; HEADERS-NEXT: Version (major): 0
+; HEADERS-NEXT: Version (minor): 0
+; HEADERS-NEXT: Characteristics: 0
+; HEADERS-NEXT: Data size: 24
+; HEADERS-NEXT: Data: (
+; HEADERS-NEXT:   0000: 00008880 67452301 00000000 01000000  |....gE#.........|
+; HEADERS-NEXT:   0010: 09000000 00000000                    |........|
+; HEADERS-NEXT: )
+
+; HEADERS-DAG: Resource type (int): 5
+; HEADERS-NEXT: Resource name (int): 31
+; HEADERS-NEXT: Data version: 0
+; HEADERS-NEXT: Memory flags: 0x1030
+; HEADERS-NEXT: Language ID: 1033
+; HEADERS-NEXT: Version (major): 0
+; HEADERS-NEXT: Version (minor): 0
+; HEADERS-NEXT: Characteristics: 0
+; HEADERS-NEXT: Data size: 32
+; HEADERS-NEXT: Data: (
+; HEADERS-NEXT:   0000: 0100FFFF 00000000 67452301 10325476  |........gE#..2Tv|
+; HEADERS-NEXT:   0010: 00000000 01000000 09000000 00000000  |................|
+; HEADERS-NEXT: )
+
+; HEADERS-DAG: Resource type (int): 5
+; HEADERS-NEXT: Resource name (int): 32
+; HEADERS-NEXT: Data version: 0
+; HEADERS-NEXT: Memory flags: 0x1030
+; HEADERS-NEXT: Language ID: 1033
+; HEADERS-NEXT: Version (major): 0
+; HEADERS-NEXT: Version (minor): 0
+; HEADERS-NEXT: Characteristics: 0
+; HEADERS-NEXT: Data size: 24
+; HEADERS-NEXT: Data: (
+; HEADERS-NEXT:   0000: 10325476 56341200 00000000 01000000  |.2TvV4..........|
+; HEADERS-NEXT:   0010: 09000000 00000000                    |........|
+; HEADERS-NEXT: )
+
 
 ; RUN: not llvm-rc /FO %t %p/Inputs/tag-dialog-large-coord.rc 2>&1 | FileCheck %s --check-prefix COORD1
 
 ; COORD1: llvm-rc: Error in DIALOGEX statement (ID 1):
 ; COORD1-NEXT: Dialog x-coordinate (50000) does not fit in 16-bit signed integer type.
 
 
 ; RUN: not llvm-rc /FO %t %p/Inputs/tag-dialog-large-coord-neg.rc 2>&1 | FileCheck %s --check-prefix COORD2

tools/llvm-rc/ResourceFileWriter.h

public:
   Error visitUserDefinedResource(const RCResource *) override;
 
   Error visitCaptionStmt(const CaptionStmt *) override;
   Error visitCharacteristicsStmt(const CharacteristicsStmt *) override;
   Error visitClassStmt(const ClassStmt *) override;
   Error visitFontStmt(const FontStmt *) override;
   Error visitLanguageStmt(const LanguageResource *) override;
   Error visitStyleStmt(const StyleStmt *) override;
+  Error visitExtStyleStmt(const ExtStyleStmt *) override;
   Error visitVersionStmt(const VersionStmt *) override;
 
   // Stringtables are output at the end of .res file. We need a separate
   // function to do it.
   Error dumpAllStringTables();
 
   bool AppendNull; // Append '\0' to each existing STRINGTABLE element?
 
   struct ObjectInfo {
     uint16_t LanguageInfo;
     uint32_t Characteristics;
     uint32_t VersionInfo;
 
     Optional<uint32_t> Style;
+    Optional<uint32_t> ExtStyle;
     StringRef Caption;
     struct FontInfo {
       uint32_t Size;
       StringRef Typeface;
       uint32_t Weight;
       bool IsItalic;
       uint32_t Charset;
     };

tools/llvm-rc/ResourceFileWriter.cpp

Error ResourceFileWriter::visitLanguageStmt(const LanguageResource *Stmt) {
   return Error::success();
 }
 
 Error ResourceFileWriter::visitStyleStmt(const StyleStmt *Stmt) {
   ObjectData.Style = Stmt->Value;
   return Error::success();
 }
 
+Error ResourceFileWriter::visitExtStyleStmt(const ExtStyleStmt *Stmt) {
+  ObjectData.ExtStyle = Stmt->Value;
+  return Error::success();
+}
+
 Error ResourceFileWriter::visitVersionStmt(const VersionStmt *Stmt) {
   ObjectData.VersionInfo = Stmt->Value;
   return Error::success();
 }
 
 Error ResourceFileWriter::writeResource(
     const RCResource *Res,
     Error (ResourceFileWriter::*BodyWriter)(const RCResource *)) {
else
     UsedStyle &= ~StyleFontFlag;
 
   // Actually, in case of empty (but existent) caption, the examined field
   // is equal to "\"\"". That's why empty captions are still noticed.
   if (ObjectData.Caption != "")
     UsedStyle |= StyleCaptionFlag;
 
   const uint16_t DialogExMagic = 0xFFFF;
+  uint32_t ExtStyle = ObjectData.ExtStyle.getValueOr(0);
 
   // Write DIALOG(EX) header prefix. These are pretty different.
   if (!Res->IsExtended) {
     // We cannot let the higher word of DefaultStyle be equal to 0xFFFF.
     // In such a case, whole object (in .res file) is equivalent to a
     // DIALOGEX. It might lead to access violation/segmentation fault in
     // resource readers. For example,
     //   1 DIALOG 0, 0, 0, 65432
     //   STYLE 0xFFFF0001 {}
     // would be compiled to a DIALOGEX with 65432 controls.
     if ((UsedStyle >> 16) == DialogExMagic)
       return createError("16 higher bits of DIALOG resource style cannot be"
                          " equal to 0xFFFF");
 
     struct {
       ulittle32_t Style;
       ulittle32_t ExtStyle;
     } Prefix{ulittle32_t(UsedStyle),
-             ulittle32_t(0)}; // As of now, we don't keep EXSTYLE.
+             ulittle32_t(ExtStyle)};
 
     writeObject(Prefix);
   } else {
     struct {
       ulittle16_t Version;
       ulittle16_t Magic;
       ulittle32_t HelpID;
       ulittle32_t ExtStyle;
       ulittle32_t Style;
     } Prefix{ulittle16_t(1), ulittle16_t(DialogExMagic),
-             ulittle32_t(Res->HelpID), ulittle32_t(0), ulittle32_t(UsedStyle)};
+             ulittle32_t(Res->HelpID), ulittle32_t(ExtStyle), ulittle32_t(UsedStyle)};
 
     writeObject(Prefix);
   }
 
   // Now, a common part. First, fixed-length fields.
   RETURN_IF_ERROR(checkNumberFits<uint16_t>(Res->Controls.size(),
                                             "Number of dialog controls"));
   RETURN_IF_ERROR(

tools/llvm-rc/ResourceScriptParser.h

private:
   // Optional statement parsers.
   ParseOptionType parseLanguageStmt();
   ParseOptionType parseCharacteristicsStmt();
   ParseOptionType parseVersionStmt();
   ParseOptionType parseCaptionStmt();
   ParseOptionType parseClassStmt();
   ParseOptionType parseFontStmt(OptStmtType DialogType);
   ParseOptionType parseStyleStmt();
+  ParseOptionType parseExtStyleStmt();
 
   // Raises an error. If IsAlreadyRead = false (default), this complains about
   // the token that couldn't be parsed. If the flag is on, this complains about
   // the correctly read token that makes no sense (that is, the current parser
   // state is beyond the erroneous token.)
   Error getExpectedError(const Twine &Message, bool IsAlreadyRead = false);
 
   std::vector<RCToken> Tokens;
   LocIter CurLoc;
   const LocIter End;
 };
 
 } // namespace rc
 } // namespace llvm
 
 #endif

tools/llvm-rc/ResourceScriptParser.cpp

if (StmtsType != OptStmtType::BasicStmt) {
     if (TypeToken->equals_lower("CAPTION"))
       return parseCaptionStmt();
     if (TypeToken->equals_lower("CLASS"))
       return parseClassStmt();
     if (TypeToken->equals_lower("FONT"))
       return parseFontStmt(StmtsType);
     if (TypeToken->equals_lower("STYLE"))
       return parseStyleStmt();
+    if (TypeToken->equals_lower("EXSTYLE"))
+      return parseExtStyleStmt();
   }
 
   return getExpectedError("optional statement type, BEGIN or '{'",
                           /* IsAlreadyRead = */ true);
 }
 
 RCParser::ParseType RCParser::parseLanguageResource() {
   // Read LANGUAGE as an optional statement. If it's read correctly, we can
return llvm::make_unique<FontStmt>(*SizeResult, *NameResult, FontWeight,
                                      FontItalic, FontCharset);
 }
 
 RCParser::ParseOptionType RCParser::parseStyleStmt() {
   ASSIGN_OR_RETURN(Arg, readInt());
   return llvm::make_unique<StyleStmt>(*Arg);
 }
 
+RCParser::ParseOptionType RCParser::parseExtStyleStmt() {
+  ASSIGN_OR_RETURN(Arg, readInt());
+  return llvm::make_unique<ExtStyleStmt>(*Arg);
+}
+
 Error RCParser::getExpectedError(const Twine &Message, bool IsAlreadyRead) {
   return make_error<ParserError>(
       Message, IsAlreadyRead ? std::prev(CurLoc) : CurLoc, End);
 }
 
 } // namespace rc
 } // namespace llvm

tools/llvm-rc/ResourceScriptStmt.h

public:
   uint32_t Value;
 
   StyleStmt(uint32_t Style) : Value(Style) {}
   raw_ostream &log(raw_ostream &) const override;
   Twine getResourceTypeName() const override { return "STYLE"; }
   Error visit(Visitor *V) const override { return V->visitStyleStmt(this); }
 };
 
+// EXSTYLE optional statement.
+//
+// Ref: docs.microsoft.com/en-us/windows/desktop/menurc/exstyle-statement
+class ExtStyleStmt : public OptionalStmt {
+public:
+  uint32_t Value;
+
+  ExtStyleStmt(uint32_t ExtStyle) : Value(ExtStyle) {}
+  raw_ostream &log(raw_ostream &) const override;
+  Twine getResourceTypeName() const override { return "EXSTYLE"; }
+  Error visit(Visitor *V) const override { return V->visitExtStyleStmt(this); }
+};
+
 // CLASS optional statement.
 //
 // Ref: msdn.microsoft.com/en-us/library/windows/desktop/aa380883(v=vs.85).aspx
 class ClassStmt : public OptionalStmt {
 public:
   IntOrString Value;
 
   ClassStmt(IntOrString Class) : Value(Class) {}

tools/llvm-rc/ResourceScriptStmt.cpp

if (Italic)
     OS << ", italic";
   return OS << ", charset = " << Charset << "\n";
 }
 
 raw_ostream &StyleStmt::log(raw_ostream &OS) const {
   return OS << "Style: " << Value << "\n";
 }
 
+raw_ostream &ExtStyleStmt::log(raw_ostream &OS) const {
+  return OS << "ExtStyle: " << Value << "\n";
+}
+
 } // namespace rc
 } // namespace llvm

tools/llvm-rc/ResourceVisitor.h

 class RCResource;
 
 class CaptionStmt;
 class ClassStmt;
 class CharacteristicsStmt;
 class FontStmt;
 class LanguageResource;
 class StyleStmt;
+class ExtStyleStmt;
 class VersionStmt;
 
 class Visitor {
 public:
   virtual Error visitNullResource(const RCResource *) = 0;
   virtual Error visitAcceleratorsResource(const RCResource *) = 0;
   virtual Error visitBitmapResource(const RCResource *) = 0;
   virtual Error visitCursorResource(const RCResource *) = 0;
   virtual Error visitDialogResource(const RCResource *) = 0;
   virtual Error visitHTMLResource(const RCResource *) = 0;
   virtual Error visitIconResource(const RCResource *) = 0;
   virtual Error visitMenuResource(const RCResource *) = 0;
   virtual Error visitStringTableResource(const RCResource *) = 0;
   virtual Error visitUserDefinedResource(const RCResource *) = 0;
   virtual Error visitVersionInfoResource(const RCResource *) = 0;
 
   virtual Error visitCaptionStmt(const CaptionStmt *) = 0;
   virtual Error visitClassStmt(const ClassStmt *) = 0;
   virtual Error visitCharacteristicsStmt(const CharacteristicsStmt *) = 0;
   virtual Error visitFontStmt(const FontStmt *) = 0;
   virtual Error visitLanguageStmt(const LanguageResource *) = 0;
   virtual Error visitStyleStmt(const StyleStmt *) = 0;
+  virtual Error visitExtStyleStmt(const ExtStyleStmt *) = 0;
   virtual Error visitVersionStmt(const VersionStmt *) = 0;
 
   virtual ~Visitor() {}
 };
 
 } // namespace rc
 } // namespace llvm
 
 #endif