[llvm-rc] Support not expressions.

[mstorsjo]

LGTM. I presume that you've tested the testcase against rc.exe and that the output is bitexact with it?

I'll leave out the accept stamp for now to hopefully trigger @amccarth or someone else to give it another set of eyes during the next round of US daytime, else I'll stamp and commit it tomorrow.

tools/llvm-rc/ResourceScriptParser.cpp

124

"is mostly useful for style values"?

[mstorsjo]

test/tools/llvm-rc/Inputs/not-expr.rc

14

Isn't this actually 0x50000022, not just 0x22? The testcase would be easier to grasp if you'd point out what the default style values involved are.

[jacek]

Thanks for review.

In D55242#1318061, @mstorsjo wrote:

LGTM. I presume that you've tested the testcase against rc.exe and that the output is bitexact with it?

Yes, I verified that the result is identical to rc.exe.

[mstorsjo]

test/tools/llvm-rc/Inputs/not-expr.rc

14

0x34 & ~0x15 = 0x20, not 0x22. So this should be "expression evaluates to 0x20". The 0x02 part comes from the default groupbox class style.

[jacek]

test/tools/llvm-rc/Inputs/not-expr.rc

14

Oh, right, thanks. I will attach a fixed version.

[mstorsjo]

LGTM, will commit.

test/tools/llvm-rc/Inputs/not-expr.rc

+not DIALOGEX // not as an identifier
+0,
+3 | not 1, // = 2
+61 + not 1, // = 60
+not 1 | 3,  // = 3
+7 | (not 1 + not 2) // = 4
+STYLE   0xff00ff03 & ~(not (1|2)) // = 0xff00ff03
+EXSTYLE 0xff00ff03 | ~(not (1|2)) | not 8 // = 0xfffffff7
+{
+    GROUPBOX "", 0,
+        not 1, // = 0
+        15 + (((not 1 | not 2 | not 4) | 2)), // = 10
+        0, 0, 0x34 - (not 1) + (not 0x14) // expression evaluates to 0x20 with not mask 0x15, which clears 0x5 bits of default
+                                          // groupbox class style 0x50000007, resulting with style 0x50000022
+    GROUPBOX "", 1, 0,0,0,0,(not 7) | 1   // expression evaluates to 1 with not mask 0x7, which clears 0x7 bits of groupbox default
+                                          // class style 0x50000007, but sets bit 1, resulting with style 0x50000001
+}

test/tools/llvm-rc/not-expr.test

+; RUN: llvm-rc /FO %t %p/Inputs/not-expr.rc
+; RUN: llvm-readobj %t | FileCheck %s --check-prefix=NOTEXPR
+
+; NOTEXPR: Resource type (int): 5
+; NOTEXPR-NEXT: Resource name (string): NOT
+; NOTEXPR-NEXT: Data version: 0
+; NOTEXPR-NEXT: Memory flags: 0x1030
+; NOTEXPR-NEXT: Language ID: 1033
+; NOTEXPR-NEXT: Version (major): 0
+; NOTEXPR-NEXT: Version (minor): 0
+; NOTEXPR-NEXT: Characteristics: 0
+; NOTEXPR-NEXT: Data size: 96
+; NOTEXPR-NEXT: Data: (
+; NOTEXPR-NEXT:   0000: 0100FFFF 04000000 F7FFFFFF 03FF00FF  |................|
+; NOTEXPR-NEXT:   0010: 02000000 02003C00 03000000 00000000  |......<.........|
+; NOTEXPR-NEXT:   0020: 00000000 00000000 22000050 00000A00  |........"..P....|
+; NOTEXPR-NEXT:   0030: 00000000 00000000 FFFF8000 00000000  |................|
+; NOTEXPR-NEXT:   0040: 00000000 00000000 01000050 00000000  |...........P....|
+; NOTEXPR-NEXT:   0050: 00000000 01000000 FFFF8000 00000000  |................|
+; NOTEXPR-NEXT: )

tools/llvm-rc/ResourceFileWriter.cpp

 // --- DialogResource helpers. --- //
 
 Error ResourceFileWriter::writeSingleDialogControl(const Control &Ctl,
                                                    bool IsExtended) {
   // Each control should be aligned to DWORD.
   padStream(sizeof(uint32_t));
 
   auto TypeInfo = Control::SupportedCtls.lookup(Ctl.Type);
-  uint32_t CtlStyle = TypeInfo.Style | Ctl.Style.getValueOr(0);
+  IntWithNotMask CtlStyle(TypeInfo.Style);
+  CtlStyle |= Ctl.Style.getValueOr(RCInt(0));
   uint32_t CtlExtStyle = Ctl.ExtStyle.getValueOr(0);
 
   // DIALOG(EX) item header prefix.
   if (!IsExtended) {
     struct {
       ulittle32_t Style;
       ulittle32_t ExtStyle;
-    } Prefix{ulittle32_t(CtlStyle), ulittle32_t(CtlExtStyle)};
+    } Prefix{ulittle32_t(CtlStyle.getValue()), ulittle32_t(CtlExtStyle)};
     writeObject(Prefix);
   } else {
     struct {
       ulittle32_t HelpID;
       ulittle32_t ExtStyle;
       ulittle32_t Style;
     } Prefix{ulittle32_t(Ctl.HelpID.getValueOr(0)), ulittle32_t(CtlExtStyle),
-             ulittle32_t(CtlStyle)};
+             ulittle32_t(CtlStyle.getValue())};
     writeObject(Prefix);
   }
 
   // Common fixed-length part.
   RETURN_IF_ERROR(checkSignedNumberFits<int16_t>(
       Ctl.X, "Dialog control x-coordinate", true));
   RETURN_IF_ERROR(checkSignedNumberFits<int16_t>(
       Ctl.Y, "Dialog control y-coordinate", true));

tools/llvm-rc/ResourceScriptParser.h

private:
   Expected<RCInt> readInt();               // Parse an integer.
   Expected<StringRef> readString();        // Parse a string.
   Expected<StringRef> readIdentifier();    // Parse an identifier.
   Expected<StringRef> readFilename();      // Parse a filename.
   Expected<IntOrString> readIntOrString(); // Parse an integer or a string.
   Expected<IntOrString> readTypeOrName();  // Parse an integer or an identifier.
 
   // Helper integer expression parsing methods.
-  Expected<RCInt> parseIntExpr1();
-  Expected<RCInt> parseIntExpr2();
+  Expected<IntWithNotMask> parseIntExpr1();
+  Expected<IntWithNotMask> parseIntExpr2();
 
   // Advance the state by one, discarding the current token.
   // If the discarded token had an incorrect type, fail.
   Error consumeType(Kind TokenKind);
 
   // Check the current token type. If it's TokenKind, discard it.
   // Return true if the parser consumed this token successfully.
   bool consumeOptionalType(Kind TokenKind);

tools/llvm-rc/ResourceScriptParser.cpp

 
 void RCParser::consume() {
   assert(!isEof());
   CurLoc++;
 }
 
 // An integer description might consist of a single integer or
 // an arithmetic expression evaluating to the integer. The expressions
-// can contain the following tokens: <int> ( ) + - | & ~. Their meaning
-// is the same as in C++.
+// can contain the following tokens: <int> ( ) + - | & ~ not. Their meaning
+// is the same as in C++ except for 'not' expression.
 // The operators in the original RC implementation have the following
 // precedence:
-//   1) Unary operators (- ~),
+//   1) Unary operators (- ~ not),
 //   2) Binary operators (+ - & |), with no precedence.
 //
+// 'not' expression is mostly useful for style values. It evaluates to 0,
+// but value given to the operator is stored separately from integer value.
+// It's mostly useful for control style expressions and causes bits from
+// default control style to be excluded from generated style. For binary
+// operators the mask from the right operand is applied to the left operand
+// and masks from both operands are combined in operator result.
+//
 // The following grammar is used to parse the expressions Exp1:
 //   Exp1 ::= Exp2 || Exp1 + Exp2 || Exp1 - Exp2 || Exp1 | Exp2 || Exp1 & Exp2
-//   Exp2 ::= -Exp2 || ~Exp2 || Int || (Exp1).
+//   Exp2 ::= -Exp2 || ~Exp2 || not Expr2 || Int || (Exp1).
 // (More conveniently, Exp1 is a non-empty sequence of Exp2 expressions,
 // separated by binary operators.)
 //
 // Expressions of type Exp1 are read by parseIntExpr1(Inner) method, while Exp2
 // is read by parseIntExpr2().
 //
 // The original Microsoft tool handles multiple unary operators incorrectly.
 // For example, in 16-bit little-endian integers:
 //    1 => 01 00, -1 => ff ff, --1 => ff ff, ---1 => 01 00;
 //    1 => 01 00, ~1 => fe ff, ~~1 => fd ff, ~~~1 => fc ff.
 // Our implementation differs from the original one and handles these
 // operators correctly:
 //    1 => 01 00, -1 => ff ff, --1 => 01 00, ---1 => ff ff;
 //    1 => 01 00, ~1 => fe ff, ~~1 => 01 00, ~~~1 => fe ff.
 
-Expected<RCInt> RCParser::readInt() { return parseIntExpr1(); }
+Expected<RCInt> RCParser::readInt() {
+  ASSIGN_OR_RETURN(Value, parseIntExpr1());
+  return (*Value).getValue();
+}
 
-Expected<RCInt> RCParser::parseIntExpr1() {
+Expected<IntWithNotMask> RCParser::parseIntExpr1() {
   // Exp1 ::= Exp2 || Exp1 + Exp2 || Exp1 - Exp2 || Exp1 | Exp2 || Exp1 & Exp2.
   ASSIGN_OR_RETURN(FirstResult, parseIntExpr2());
-  RCInt Result = *FirstResult;
+  IntWithNotMask Result = *FirstResult;
 
   while (!isEof() && look().isBinaryOp()) {
     auto OpToken = read();
     ASSIGN_OR_RETURN(NextResult, parseIntExpr2());
 
     switch (OpToken.kind()) {
     case Kind::Plus:
       Result += *NextResult;
while (!isEof() && look().isBinaryOp()) {
     default:
       llvm_unreachable("Already processed all binary ops.");
     }
   }
 
   return Result;
 }
 
-Expected<RCInt> RCParser::parseIntExpr2() {
-  // Exp2 ::= -Exp2 || ~Exp2 || Int || (Exp1).
+Expected<IntWithNotMask> RCParser::parseIntExpr2() {
+  // Exp2 ::= -Exp2 || ~Exp2 || not Expr2 || Int || (Exp1).
   static const char ErrorMsg[] = "'-', '~', integer or '('";
 
   if (isEof())
     return getExpectedError(ErrorMsg);
 
   switch (look().kind()) {
   case Kind::Minus: {
     consume();
Expected<IntWithNotMask> RCParser::parseIntExpr2() {
 
   case Kind::LeftParen: {
     consume();
     ASSIGN_OR_RETURN(Result, parseIntExpr1());
     RETURN_IF_ERROR(consumeType(Kind::RightParen));
     return *Result;
   }
 
+  case Kind::Identifier: {
+    if (!read().value().equals_lower("not"))
+      return getExpectedError(ErrorMsg, true);
+    ASSIGN_OR_RETURN(Result, parseIntExpr2());
+    return IntWithNotMask(0, (*Result).getValue());
+  }
+
   default:
     return getExpectedError(ErrorMsg);
   }
 }
 
 Expected<StringRef> RCParser::readString() {
   if (!isNextTokenKind(Kind::String))
     return getExpectedError("string");
if (CtlInfo->getValue().HasTitle) {
     RETURN_IF_ERROR(consumeType(Kind::Comma));
     Caption = *CaptionResult;
   }
 
   ASSIGN_OR_RETURN(ID, readInt());
   RETURN_IF_ERROR(consumeType(Kind::Comma));
 
   IntOrString Class;
-  Optional<uint32_t> Style;
+  Optional<IntWithNotMask> Style;
   if (ClassUpper == "CONTROL") {
     // CONTROL text, id, class, style, x, y, width, height [, exstyle] [, helpID]
     ASSIGN_OR_RETURN(ClassStr, readString());
     RETURN_IF_ERROR(consumeType(Kind::Comma));
     Class = *ClassStr;
-    ASSIGN_OR_RETURN(StyleVal, readInt());
+    ASSIGN_OR_RETURN(StyleVal, parseIntExpr1());
     RETURN_IF_ERROR(consumeType(Kind::Comma));
     Style = *StyleVal;
   } else {
     Class = CtlInfo->getValue().CtlClass;
   }
 
   // x, y, width, height
   ASSIGN_OR_RETURN(Args, readIntsWithCommas(4, 4));
 
   if (ClassUpper != "CONTROL") {
     if (consumeOptionalType(Kind::Comma)) {
-      ASSIGN_OR_RETURN(Val, readInt());
+      ASSIGN_OR_RETURN(Val, parseIntExpr1());
       Style = *Val;
     }
   }
 
   Optional<uint32_t> ExStyle;
   if (consumeOptionalType(Kind::Comma)) {
     ASSIGN_OR_RETURN(Val, readInt());
     ExStyle = *Val;

tools/llvm-rc/ResourceScriptStmt.h

public:
   RCInt operator-() const { return {-Val, Long}; }
   RCInt operator~() const { return {~Val, Long}; }
 
   friend raw_ostream &operator<<(raw_ostream &OS, const RCInt &Int) {
     return OS << Int.Val << (Int.Long ? "L" : "");
   }
 };
 
+class IntWithNotMask {
+private:
+  RCInt Value;
+  int32_t NotMask;
+
+public:
+  IntWithNotMask() : IntWithNotMask(RCInt(0)) {}
+  IntWithNotMask(RCInt Value, int32_t NotMask = 0) : Value(Value), NotMask(NotMask) {}
+
+  RCInt getValue() const {
+    return Value;
+  }
+
+  uint32_t getNotMask() const {
+    return NotMask;
+  }
+
+  IntWithNotMask &operator+=(const IntWithNotMask &Rhs) {
+    Value &= ~Rhs.NotMask;
+    Value += Rhs.Value;
+    NotMask |= Rhs.NotMask;
+    return *this;
+  }
+
+  IntWithNotMask &operator-=(const IntWithNotMask &Rhs) {
+    Value &= ~Rhs.NotMask;
+    Value -= Rhs.Value;
+    NotMask |= Rhs.NotMask;
+    return *this;
+  }
+
+  IntWithNotMask &operator|=(const IntWithNotMask &Rhs) {
+    Value &= ~Rhs.NotMask;
+    Value |= Rhs.Value;
+    NotMask |= Rhs.NotMask;
+    return *this;
+  }
+
+  IntWithNotMask &operator&=(const IntWithNotMask &Rhs) {
+    Value &= ~Rhs.NotMask;
+    Value &= Rhs.Value;
+    NotMask |= Rhs.NotMask;
+    return *this;
+  }
+
+  IntWithNotMask operator-() const { return {-Value, NotMask}; }
+  IntWithNotMask operator~() const { return {~Value, 0}; }
+
+  friend raw_ostream &operator<<(raw_ostream &OS, const IntWithNotMask &Int) {
+    return OS << Int.Value;
+  }
+};
+
 // A class holding a name - either an integer or a reference to the string.
 class IntOrString {
 private:
   union Data {
     RCInt Int;
     StringRef String;
     Data(RCInt Value) : Int(Value) {}
     Data(const StringRef Value) : String(Value) {}

 // Ref: msdn.microsoft.com/en-us/library/windows/desktop/aa381002(v=vs.85).aspx
 
 // Single control definition.
 class Control {
 public:
   StringRef Type;
   IntOrString Title;
   uint32_t ID, X, Y, Width, Height;
-  Optional<uint32_t> Style, ExtStyle, HelpID;
+  Optional<IntWithNotMask> Style;
+  Optional<uint32_t> ExtStyle, HelpID;
   IntOrString Class;
 
   // Control classes as described in DLGITEMTEMPLATEEX documentation.
   //
   // Ref: msdn.microsoft.com/en-us/library/windows/desktop/ms645389.aspx
   enum CtlClasses {
     ClsButton = 0x80,
     ClsEdit = 0x81,
     ClsStatic = 0x82,
     ClsListBox = 0x83,
     ClsScrollBar = 0x84,
     ClsComboBox = 0x85
   };
 
   // Simple information about a single control type.
   struct CtlInfo {
     uint32_t Style;
     uint16_t CtlClass;
     bool HasTitle;
   };
 
   Control(StringRef CtlType, IntOrString CtlTitle, uint32_t CtlID,
           uint32_t PosX, uint32_t PosY, uint32_t ItemWidth, uint32_t ItemHeight,
-          Optional<uint32_t> ItemStyle, Optional<uint32_t> ExtItemStyle,
+          Optional<IntWithNotMask> ItemStyle, Optional<uint32_t> ExtItemStyle,
           Optional<uint32_t> CtlHelpID, IntOrString CtlClass)
       : Type(CtlType), Title(CtlTitle), ID(CtlID), X(PosX), Y(PosY),
         Width(ItemWidth), Height(ItemHeight), Style(ItemStyle),
         ExtStyle(ExtItemStyle), HelpID(CtlHelpID), Class(CtlClass) {}
 
   static const StringMap<CtlInfo> SupportedCtls;
 
   raw_ostream &log(raw_ostream &) const;

tools/llvm-rc/ResourceScriptStmt.cpp

const StringMap<Control::CtlInfo> Control::SupportedCtls = {
     {"CONTROL", CtlInfo{0x50000000, 0, true}},
 };
 
 raw_ostream &Control::log(raw_ostream &OS) const {
   OS << "  Control (" << ID << "): " << Type << ", title: " << Title
      << ", loc: (" << X << ", " << Y << "), size: [" << Width << ", " << Height
      << "]";
   if (Style)
-    OS << ", style: " << *Style;
+    OS << ", style: " << (*Style).getValue();
   if (ExtStyle)
     OS << ", ext. style: " << *ExtStyle;
   if (HelpID)
     OS << ", help ID: " << *HelpID;
   return OS << "\n";
 }
 
 raw_ostream &DialogResource::log(raw_ostream &OS) const {