cppparser: assorted preprocessor improvements:

* Fix function-like macro arguments being expanded even when they were participating in token expansion or stringification
* Fix __has_include with comma or closing parenthesis in angle-quoted filename
* Don't issue warning if macro is redefined with identical definition
* Fixes for extraneous spaces being added to expansions
* Assorted refactoring

This should resolve #1638.

dtool/src/cppparser/cppManifest.cxx

@@ -23,7 +23,11 @@ using std::string;
  */
 CPPManifest::ExpansionNode::
 ExpansionNode(int parm_number, bool stringify, bool paste) :
-  _parm_number(parm_number), _stringify(stringify), _paste(paste), _optional(false)
+  _parm_number(parm_number),
+  _expand(!stringify && !paste),
+  _stringify(stringify),
+  _paste(paste),
+  _optional(false)
 {
 }
 
@@ -32,7 +36,12 @@ ExpansionNode(int parm_number, bool stringify, bool paste) :
  */
 CPPManifest::ExpansionNode::
 ExpansionNode(const string &str, bool paste) :
-  _parm_number(-1), _stringify(false), _paste(paste), _optional(false), _str(str)
+  _parm_number(-1),
+  _expand(!paste),
+  _stringify(false),
+  _paste(paste),
+  _optional(false),
+  _str(str)
 {
 }
 
@@ -41,15 +50,35 @@ ExpansionNode(const string &str, bool paste) :
  */
 CPPManifest::ExpansionNode::
 ExpansionNode(Expansion nested, bool stringify, bool paste, bool optional) :
-  _parm_number(-1), _stringify(stringify), _paste(paste), _optional(optional), _nested(std::move(nested))
+  _parm_number(-1),
+  _expand(!stringify && !paste),
+  _stringify(stringify),
+  _paste(paste),
+  _optional(optional),
+  _nested(std::move(nested))
 {
 }
 
+/**
+ *
+ */
+bool CPPManifest::ExpansionNode::
+operator ==(const ExpansionNode &other) const {
+  return _parm_number == other._parm_number
+      && _expand == other._expand
+      && _stringify == other._stringify
+      && _paste == other._paste
+      && _optional == other._optional
+      && _str == other._str
+      && _nested == other._nested;
+}
+
 /**
  * Creates a manifest from a preprocessor definition.
  */
 CPPManifest::
-CPPManifest(const string &args, const cppyyltype &loc) :
+CPPManifest(const CPPPreprocessor &parser, const string &args, const cppyyltype &loc) :
+  _parser(parser),
   _variadic_param(-1),
   _loc(loc),
   _expr(nullptr),
@@ -93,7 +122,8 @@ CPPManifest(const string &args, const cppyyltype &loc) :
  * command-line -D option.
  */
 CPPManifest::
-CPPManifest(const string &macro, const string &definition) :
+CPPManifest(const CPPPreprocessor &parser, const string &macro, const string &definition) :
+  _parser(parser),
   _variadic_param(-1),
   _expr(nullptr),
   _vis(V_public)
@@ -190,12 +220,112 @@ stringify(const string &source) {
   return result;
 }
 
+/**
+ *
+ */
+void CPPManifest::
+extract_args(vector_string &args, const string &expr, size_t &p) const {
+  // Skip whitespace till paren.
+  while (p < expr.size() && isspace(expr[p])) {
+    p++;
+  }
+  if (p >= expr.size() || expr[p] != '(') {
+    // No paren, so we have only one arg.
+    size_t q = p;
+    while (p < expr.size() && (isalnum(expr[p]) || expr[p] == '_')) {
+      p++;
+    }
+    args.push_back(expr.substr(q, p - q));
+  }
+  else if (expr[p] == '"' || expr[p] == '\'') {
+    // Quoted string or character.
+    int quote_mark = expr[p];
+    p++;
+    while (p < expr.size() && expr[p] != quote_mark && expr[p] != '\n') {
+      if (expr[p] == '\\') {
+        p++;
+      }
+      if (p < expr.size()) {
+        p++;
+      }
+    }
+    p++;
+  }
+  else {
+    // Skip paren.
+    p++;
+    int paren_level = 1;
+    size_t q = p;
+    while (p < expr.size()) {
+      if (expr[p] == ',' && paren_level == 1) {
+        // Back up to strip any trailing whitespace.
+        size_t r = p;
+        while (r > q && isspace(expr[r - 1])) {
+          --r;
+        }
+        args.push_back(expr.substr(q, r - q));
+        q = p+1;
+      }
+      else if (expr[p] == '"' || expr[p] == '\'') {
+        // Quoted string or character.
+        int quote_mark = expr[p];
+        p++;
+        while (p < expr.size() && expr[p] != quote_mark && expr[p] != '\n') {
+          if (expr[p] == '\\') {
+            p++;
+          }
+          if (p < expr.size()) {
+            p++;
+          }
+        }
+      }
+      else if (expr[p] == '(') {
+        ++paren_level;
+      }
+      else if (expr[p] == ')') {
+        --paren_level;
+        if (paren_level == 0) {
+          break;
+        }
+      }
+      else if (isspace(expr[p])) {
+        // Skip whitespace at the beginning.
+        if (q == p) {
+          q++;
+        }
+      }
+      p++;
+    }
+    {
+      // Back up to strip any trailing whitespace.
+      size_t r = p;
+      while (r > q && isspace(expr[r - 1])) {
+        --r;
+      }
+      if (!args.empty() || r > q) {
+        args.push_back(expr.substr(q, r - q));
+      }
+    }
+
+    if (p < expr.size() && expr[p] == ')') {
+      p++;
+    }
+  }
+
+  if ((int)args.size() < _num_parameters) {
+    _parser.warning("Not enough arguments for manifest " + _name);
+  }
+  else if (_variadic_param < 0 && (int)args.size() > _num_parameters) {
+    _parser.warning("Too many arguments for manifest " + _name);
+  }
+}
+
 /**
  *
  */
 string CPPManifest::
-expand(const vector_string &args) const {
-  return r_expand(_expansion, args);
+expand(const vector_string &args, const Manifests &manifests, bool expand_undefined) const {
+  return r_expand(_expansion, args, manifests, expand_undefined);
 }
 
 /**
@@ -210,6 +340,32 @@ determine_type() const {
   return nullptr;
 }
 
+/**
+ * Returns true if the macro definitions are equal.
+ */
+bool CPPManifest::
+is_equal(const CPPManifest *other) const {
+  if (this == other) {
+    return true;
+  }
+  if (_name != other->_name) {
+    return false;
+  }
+  if (_has_parameters != other->_has_parameters) {
+    return false;
+  }
+  if (_num_parameters != other->_num_parameters) {
+    return false;
+  }
+  if (_variadic_param != other->_variadic_param) {
+    return false;
+  }
+  if (_expansion != other->_expansion) {
+    return false;
+  }
+  return true;
+}
+
 /**
  *
  */
@@ -409,6 +565,10 @@ save_expansion(Expansion &expansion, const string &exp, const vector_string &par
       if (p < exp.size() && exp[p] == '#') {
         // Woah, this is a token-pasting operator.
         paste = true;
+        if (!expansion.empty()) {
+          // The previous expansion shouldn't be expanded.
+          expansion.back()._expand = false;
+        }
         ++p;
       } else {
         // Mark that the next argument should be stringified.
@@ -439,7 +599,8 @@ save_expansion(Expansion &expansion, const string &exp, const vector_string &par
  *
  */
 string CPPManifest::
-r_expand(const Expansion &expansion, const vector_string &args) const {
+r_expand(const Expansion &expansion, const vector_string &args,
+         const Manifests &manifests, bool expand_undefined) const {
   std::string result;
 
   for (const ExpansionNode &node : expansion) {
@@ -458,7 +619,8 @@ r_expand(const Expansion &expansion, const vector_string &args) const {
         if (node._stringify) {
           subst = stringify(subst);
         }
-      } else if (i == _variadic_param && node._paste) {
+      }
+      else if (i == _variadic_param && node._paste) {
         // Special case GCC behavior: if __VA_ARGS__ is pasted to a comma and
         // no arguments are passed, the comma is removed.  MSVC does this
         // automatically.  Not sure if we should allow MSVC behavior as well.
@@ -467,8 +629,12 @@ r_expand(const Expansion &expansion, const vector_string &args) const {
         }
       }
 
+      if (node._expand) {
+        _parser.expand_manifests(subst, manifests, expand_undefined);
+      }
+
       if (!subst.empty()) {
-        if (result.empty() || node._paste) {
+        if (result.empty() || node._paste || result.back() == '(') {
           result += subst;
         } else {
           result += ' ';
@@ -477,7 +643,7 @@ r_expand(const Expansion &expansion, const vector_string &args) const {
       }
     }
     if (!node._str.empty()) {
-      if (result.empty() || node._paste) {
+      if (result.empty() || node._paste || node._str[0] == ',' || node._str[0] == ')') {
         result += node._str;
       } else {
         result += ' ';
@@ -487,7 +653,7 @@ r_expand(const Expansion &expansion, const vector_string &args) const {
     if (!node._nested.empty()) {
       string nested_result;
       if (node._optional && args.size() >= _num_parameters) {
-        nested_result = r_expand(node._nested, args);
+        nested_result = r_expand(node._nested, args, manifests, expand_undefined);
       }
       if (node._stringify) {
         nested_result = stringify(nested_result);

dtool/src/cppparser/cppManifest.h

@@ -30,17 +30,25 @@ class CPPType;
  */
 class CPPManifest {
 public:
-  CPPManifest(const std::string &args, const cppyyltype &loc);
-  CPPManifest(const std::string &macro, const std::string &definition);
+  typedef std::map<std::string, CPPManifest *> Manifests;
+
+  CPPManifest(const CPPPreprocessor &parser, const std::string &args, const cppyyltype &loc);
+  CPPManifest(const CPPPreprocessor &parser, const std::string &macro, const std::string &definition);
   ~CPPManifest();
 
   static std::string stringify(const std::string &source);
-  std::string expand(const vector_string &args = vector_string()) const;
+  void extract_args(vector_string &args, const std::string &expr, size_t &p) const;
+  std::string expand(const vector_string &args = vector_string(),
+                     const Manifests &manifests = Manifests(),
+                     bool expand_undefined = false) const;
+
 
   CPPType *determine_type() const;
 
+  bool is_equal(const CPPManifest *other) const;
   void output(std::ostream &out) const;
 
+  const CPPPreprocessor &_parser;
   std::string _name;
   bool _has_parameters;
   size_t _num_parameters;
@@ -59,7 +67,11 @@ private:
     ExpansionNode(int parm_number, bool stringify, bool paste);
     ExpansionNode(const std::string &str, bool paste = false);
     ExpansionNode(std::vector<ExpansionNode> nested, bool stringify = false, bool paste = false, bool optional = false);
+
+    bool operator ==(const ExpansionNode &other) const;
+
     int _parm_number;
+    bool _expand;
     bool _stringify;
     bool _paste;
     bool _optional;
@@ -73,8 +85,8 @@ private:
   void save_expansion(Expansion &expansion, const std::string &exp,
                       const vector_string &parameter_names);
 
-  std::string r_expand(const Expansion &expansion,
-                       const vector_string &args = vector_string()) const;
+  std::string r_expand(const Expansion &expansion, const vector_string &args,
+                       const Manifests &manifests, bool expand_undefined) const;
 
   Expansion _expansion;
 };

dtool/src/cppparser/cppPreprocessor.cxx

@@ -461,7 +461,7 @@ peek_next_token() {
  *
  */
 void CPPPreprocessor::
-warning(const string &message) {
+warning(const string &message) const {
   if (_verbose < 2) {
     return;
   }
@@ -480,7 +480,7 @@ warning(const string &message) {
  *
  */
 void CPPPreprocessor::
-warning(const string &message, const YYLTYPE &loc) {
+warning(const string &message, const YYLTYPE &loc) const {
   if (_verbose >= 2) {
     if (_verbose >= 3) {
       indent(cerr, _files.size() * 2);
@@ -507,7 +507,7 @@ warning(const string &message, const YYLTYPE &loc) {
  *
  */
 void CPPPreprocessor::
-error(const string &message) {
+error(const string &message) const {
   int line = get_line_number();
   int col = get_col_number();
   YYLTYPE loc;
@@ -523,7 +523,7 @@ error(const string &message) {
  *
  */
 void CPPPreprocessor::
-error(const string &message, const YYLTYPE &loc) {
+error(const string &message, const YYLTYPE &loc) const {
   if (_state == S_nested || _state == S_end_nested) {
     // Don't report or log errors in the nested state.  These will be reported
     // when the nesting level collapses.
@@ -550,20 +550,20 @@ error(const string &message, const YYLTYPE &loc) {
     show_line(loc);
 
     if (!_files.empty() && !loc.file.empty()) {
-      Files::reverse_iterator rit;
+      Files::const_reverse_iterator rit;
       for (rit = _files.rbegin();
            rit != _files.rend() && (*rit)._file == loc.file && (*rit)._manifest != nullptr;
            ++rit) {
       }
       if (rit != _files.rbegin() && rit != _files.rend()) {
         --rit;
-        InputFile &infile = *rit;
+        const InputFile &infile = *rit;
         if (_verbose >= 3) {
           cerr << "Expansion of " << infile._manifest->_name << ":\n";
           cerr << " -> " << trim_blanks(infile._input) << "\n";
           while (rit != _files.rbegin()) {
             --rit;
-            InputFile &infile = *rit;
+            const InputFile &infile = *rit;
             cerr << " -> " << trim_blanks(infile._input) << "\n";
           }
         }
@@ -589,7 +589,7 @@ error(const string &message, const YYLTYPE &loc) {
  * Shows the indicated line, useful for error messages.
  */
 void CPPPreprocessor::
-show_line(const YYLTYPE &loc) {
+show_line(const YYLTYPE &loc) const {
   if (loc.file._filename.empty()) {
     return;
   }
@@ -846,18 +846,98 @@ push_expansion(const string &input, const CPPManifest *manifest, const YYLTYPE &
 }
 
 /**
- * Given a string, expand all manifests within the string and return the new
- * string.
+ * Given a string, expand all manifests within the string.
  */
-string CPPPreprocessor::
-expand_manifests(const string &input_expr, bool expand_undefined,
-                 const YYLTYPE &loc) {
-  // Get a copy of the expression string we can modify.
-  string expr = input_expr;
+void CPPPreprocessor::
+expand_manifests(string &expr, const Manifests &manifests, bool expand_undefined) const {
+  size_t p = 0;
+  while (p < expr.size()) {
+    if (isalpha(expr[p]) || expr[p] == '_') {
+      size_t q = p;
+      while (p < expr.size() && (isalnum(expr[p]) || expr[p] == '_')) {
+        p++;
+      }
+      string ident = expr.substr(q, p - q);
+
+      // Here's an identifier.  Is it "defined"?
+      if (ident == "defined") {
+        expand_defined_function(expr, q, p);
+      }
+      else if (expand_undefined && ident == "__has_include") {
+        expand_has_include_function(expr, q, p);
+      }
+      else {
+        // Is it a manifest?
+        Manifests::const_iterator mi = manifests.find(ident);
+        if (mi != manifests.end()) {
+          const CPPManifest *manifest = (*mi).second;
+          vector_string args;
+          if (manifest->_has_parameters) {
+            // If it's not followed by a parenthesis, don't expand it.
+            while (p < expr.size() && isspace(expr[p])) {
+              p++;
+            }
+            if (p >= expr.size() || expr[p] != '(') {
+              continue;
+            }
+
+            manifest->extract_args(args, expr, p);
+          }
+
+          // Don't consider this manifest when expanding the arguments or
+          // result, to prevent recursion.
+          Manifests nested_manifests(manifests);
+          nested_manifests.erase((*mi).first);
+
+          string result = manifest->expand(args);
+          expand_manifests(result, nested_manifests, expand_undefined);
+
+          expr = expr.substr(0, q) + result + expr.substr(p);
+          p = q + result.size();
+        }
+        else if (ident == "__FILE__") {
+          // Special case: this is a dynamic definition.
+          CPPFile file = get_file();
+          string result = string("\"") + file._filename_as_referenced.get_fullpath() + "\"";
+          expr = expr.substr(0, q) + result + expr.substr(p);
+          p = q + result.size();
 
-  std::set<const CPPManifest *> expanded;
-  r_expand_manifests(expr, expand_undefined, loc, expanded);
-  return expr;
+        }
+        else if (ident == "__LINE__") {
+          // So is this.
+          string line = format_string(get_line_number());
+          expr = expr.substr(0, q) + line + expr.substr(p);
+          p = q + line.size();
+        }
+        else if (expand_undefined && ident != "true" && ident != "false") {
+          // It is not found.  Expand it to 0, but only if we are currently
+          // parsing an #if expression.
+          expr = expr.substr(0, q) + "0" + expr.substr(p);
+          p = q + 1;
+        }
+      }
+    }
+    else if (expr[p] == '\'' || expr[p] == '"') {
+      // Skip the next part until we find a closing quotation mark.
+      char quote = expr[p];
+      p++;
+      while (p < expr.size() && expr[p] != quote) {
+        if (expr[p] == '\\') {
+          // This might be an escaped quote.  Skip an extra char.
+          p++;
+        }
+        p++;
+      }
+      if (p >= expr.size()) {
+        // Unclosed string.
+        warning("missing terminating " + string(1, quote) + " character");
+      }
+      p++;
+    }
+    else {
+      p++;
+    }
+  }
 }
 
 /**
@@ -870,7 +950,8 @@ expand_manifests(const string &input_expr, bool expand_undefined,
 CPPExpression *CPPPreprocessor::
 parse_expr(const string &input_expr, CPPScope *current_scope,
            CPPScope *global_scope, const YYLTYPE &loc) {
-  string expr = expand_manifests(input_expr, false, loc);
+  string expr = input_expr;
+  expand_manifests(expr, _manifests, false);
 
   CPPExpressionParser ep(current_scope, global_scope);
   ep._verbose = 0;
@@ -1479,7 +1560,7 @@ handle_define_directive(const string &args, const YYLTYPE &loc) {
   if (args.empty()) {
     warning("Ignoring empty #define directive", loc);
   } else {
-    CPPManifest *manifest = new CPPManifest(args, loc);
+    CPPManifest *manifest = new CPPManifest(*this, args, loc);
     manifest->_vis = preprocessor_vis;
     if (!manifest->_has_parameters) {
       string expr_string = manifest->expand();
@@ -1494,8 +1575,10 @@ handle_define_directive(const string &args, const YYLTYPE &loc) {
     if (!result.second) {
       // There was already a macro with this name.  Delete the old.
       CPPManifest *other = result.first->second;
-      warning("redefinition of macro '" + manifest->_name + "'", loc);
-      warning("previous definition is here", other->_loc);
+      if (!manifest->is_equal(other)) {
+        warning("redefinition of macro '" + manifest->_name + "'", loc);
+        warning("previous definition is here", other->_loc);
+      }
       result.first->second = manifest;
     }
   }
@@ -1544,7 +1627,8 @@ handle_ifndef_directive(const string &args, const YYLTYPE &loc) {
 void CPPPreprocessor::
 handle_if_directive(const string &args, const YYLTYPE &loc) {
   // When expanding manifests, we should replace unknown macros with 0.
-  string expr = expand_manifests(args, true, loc);
+  string expr = args;
+  expand_manifests(expr, _manifests, true);
 
   int expression_result = 0;
   CPPExpressionParser ep(current_scope, global_scope);
@@ -1589,7 +1673,7 @@ handle_include_directive(const string &args, const YYLTYPE &loc) {
   // filter out quotes and angle brackets properly, we'll only expand
   // manifests if we don't begin with a quote or bracket.
   if (!expr.empty() && (expr[0] != '"' && expr[0] != '<')) {
-    expr = expand_manifests(expr, false, loc);
+    expand_manifests(expr, _manifests, false);
   }
 
   if (!expr.empty()) {
@@ -1767,7 +1851,7 @@ skip_false_if_block(bool consider_elifs) {
  * Returns true if the given manifest is defined.
  */
 bool CPPPreprocessor::
-is_manifest_defined(const string &manifest_name) {
+is_manifest_defined(const string &manifest_name) const {
   Manifests::const_iterator mi = _manifests.find(manifest_name);
   if (mi != _manifests.end()) {
     return true;
@@ -1787,7 +1871,7 @@ is_manifest_defined(const string &manifest_name) {
  * Locates the given filename.  Changes the first argument to the full path.
  */
 bool CPPPreprocessor::
-find_include(Filename &filename, bool angle_quotes, CPPFile::Source &source) {
+find_include(Filename &filename, bool angle_quotes, CPPFile::Source &source) const {
   // Now look for the filename.  If we didn't use angle quotes, look first in
   // the current directory.
   if (!angle_quotes && filename.exists()) {
@@ -2180,19 +2264,17 @@ expand_manifest(const CPPManifest *manifest, const YYLTYPE &loc) {
                           manifest->_variadic_param, args);
   }
 
-  // Perform expansion on the macro arguments.
-  for (string &arg : args) {
-    std::set<const CPPManifest *> expanded;
-    expanded.insert(manifest);
-    for (const InputFile &infile : _files) {
-      if (infile._ignore_manifest) {
-        expanded.insert(infile._manifest);
-      }
+  // Make a copy of the manifests, without the ones we're supposed to ignore.
+  Manifests manifests = _manifests;
+  manifests.erase(manifest->_name);
+
+  for (const InputFile &infile : _files) {
+    if (infile._ignore_manifest) {
+      manifests.erase(infile._manifest->_name);
     }
-    r_expand_manifests(arg, false, loc, expanded);
   }
 
-  string expanded = " " + manifest->expand(args) + " ";
+  string expanded = " " + manifest->expand(args, manifests, false) + " ";
   push_expansion(expanded, manifest, loc);
 
 #ifdef CPP_VERBOSE_LEX
@@ -2203,111 +2285,6 @@ expand_manifest(const CPPManifest *manifest, const YYLTYPE &loc) {
   return internal_get_next_token();
 }
 
-/**
- * Recursive implementation of expand_manifests().
- */
-void CPPPreprocessor::
-r_expand_manifests(string &expr, bool expand_undefined,
-                   const YYLTYPE &loc, std::set<const CPPManifest *> &expanded) {
-  size_t p = 0;
-  while (p < expr.size()) {
-    if (isalpha(expr[p]) || expr[p] == '_') {
-      size_t q = p;
-      while (p < expr.size() && (isalnum(expr[p]) || expr[p] == '_')) {
-        p++;
-      }
-      string ident = expr.substr(q, p - q);
-
-      // Here's an identifier.  Is it "defined"?
-      if (ident == "defined") {
-        expand_defined_function(expr, q, p);
-      }
-      else if (expand_undefined && ident == "__has_include") {
-        expand_has_include_function(expr, q, p, loc);
-      }
-      else {
-        // Is it a manifest?
-        Manifests::const_iterator mi = _manifests.find(ident);
-        if (mi != _manifests.end()) {
-          const CPPManifest *manifest = (*mi).second;
-          if (expanded.count(manifest) == 0) {
-            vector_string args;
-            if (manifest->_has_parameters) {
-              // If it's not followed by a parenthesis, don't expand it.
-              while (p < expr.size() && isspace(expr[p])) {
-                p++;
-              }
-              if (p >= expr.size() || expr[p] != '(') {
-                continue;
-              }
-
-              extract_manifest_args_inline(manifest->_name, manifest->_num_parameters,
-                                           manifest->_variadic_param, args, expr, p);
-            }
-
-            // Perform expansion on the macro arguments.
-            for (string &arg : args) {
-              std::set<const CPPManifest *> ignore = expanded;
-              ignore.insert(manifest);
-              r_expand_manifests(arg, expand_undefined, loc, ignore);
-            }
-
-            string result = manifest->expand(args);
-
-            // Recurse, but adding the manifest we just expanded to the list
-            // to be ignored for future expansion, to prevent recursion.
-            std::set<const CPPManifest *> ignore = expanded;
-            ignore.insert(manifest);
-            r_expand_manifests(result, expand_undefined, loc, ignore);
-
-            expr = expr.substr(0, q) + result + expr.substr(p);
-            p = q + result.size();
-          }
-        }
-        else if (ident == "__FILE__") {
-          // Special case: this is a dynamic definition.
-          string file = string("\"") + loc.file._filename_as_referenced.get_fullpath() + "\"";
-          expr = expr.substr(0, q) + file + expr.substr(p);
-          p = q + file.size();
-
-        }
-        else if (ident == "__LINE__") {
-          // So is this.
-          string line = format_string(loc.first_line);
-          expr = expr.substr(0, q) + line + expr.substr(p);
-          p = q + line.size();
-        }
-        else if (expand_undefined && ident != "true" && ident != "false") {
-          // It is not found.  Expand it to 0, but only if we are currently
-          // parsing an #if expression.
-          expr = expr.substr(0, q) + "0" + expr.substr(p);
-          p = q + 1;
-        }
-      }
-    }
-    else if (expr[p] == '\'' || expr[p] == '"') {
-      // Skip the next part until we find a closing quotation mark.
-      char quote = expr[p];
-      p++;
-      while (p < expr.size() && expr[p] != quote) {
-        if (expr[p] == '\\') {
-          // This might be an escaped quote.  Skip an extra char.
-          p++;
-        }
-        p++;
-      }
-      if (p >= expr.size()) {
-        // Unclosed string.
-        warning("missing terminating " + string(1, quote) + " character", loc);
-      }
-      p++;
-    }
-    else {
-      p++;
-    }
-  }
-}
-
 /**
  *
  */
@@ -2427,23 +2404,34 @@ extract_manifest_args(const string &name, int num_args, int va_arg,
  * whether the manifest exists.
  */
 void CPPPreprocessor::
-expand_defined_function(string &expr, size_t q, size_t &p) {
-  string result;
+expand_defined_function(string &expr, size_t q, size_t &p) const {
+  while (p < expr.size() && isspace(expr[p])) {
+    p++;
+  }
 
-  vector_string args;
-  extract_manifest_args_inline("defined", 1, -1, args, expr, p);
-  if (args.size() >= 1) {
-    if (is_manifest_defined(args[0])) {
-      // The macro is defined; the result is "1".
-      result = "1";
+  bool has_paren = false;
+  if (expr[p] == '(') {
+    has_paren = true;
+    p++;
+  }
+
+  size_t r = p;
+  while (p < expr.size() && (isalnum(expr[p]) || expr[p] == '_')) {
+    p++;
+  }
+
+  if (has_paren) {
+    if (expr[p] == ')') {
+      p++;
     } else {
-      // The macro is undefined; the result is "0".
-      result = "0";
+      error("missing ')' after 'defined'");
     }
   }
 
+  string name = expr.substr(r, p - r - 1);
+  char result = is_manifest_defined(name) ? '1' : '0';
   expr = expr.substr(0, q) + result + expr.substr(p);
-  p = q + result.size();
+  p = q + 1;
 }
 
 /**
@@ -2451,151 +2439,107 @@ expand_defined_function(string &expr, size_t q, size_t &p) {
  * whether the include file exists.
  */
 void CPPPreprocessor::
-expand_has_include_function(string &expr, size_t q, size_t &p, YYLTYPE loc) {
+expand_has_include_function(string &expr, size_t q, size_t &p) const {
   bool found_file = false;
 
   // Skip whitespace till paren.
   while (p < expr.size() && isspace(expr[p])) {
     p++;
   }
-  size_t args_begin = p + 1;
-
-  vector_string args;
-  extract_manifest_args_inline("__has_include", 1, -1, args, expr, p);
-
-  if (!args.empty() && args[0].size() >= 2) {
-    Filename filename;
-    bool angle_quotes = false;
-
-    string inc = args[0];
-
-    // Just to play things safe, since our manifest-expansion logic might not
-    // filter out quotes and angle brackets properly, we'll only expand
-    // manifests if we don't begin with a quote or bracket.
-    if (!inc.empty() && (inc[0] != '"' && inc[0] != '<')) {
-      inc = expand_manifests(inc, false, loc);
-    }
-
-    if (inc[0] == '"' && inc[inc.size() - 1] == '"') {
-      filename = inc.substr(1, inc.size() - 2);
-    } else if (inc[0] == '<' && inc[inc.size() - 1] == '>') {
-      filename = inc.substr(1, inc.size() - 2);
-      if (!_noangles) {
-        // If _noangles is true, we don't make a distinction between angle
-        // brackets and quote marks--all #inc statements are treated the
-        // same, as if they used quote marks.
-        angle_quotes = true;
-      }
-    } else {
-      loc.last_column += loc.first_column + p - 2;
-      loc.first_column += args_begin;
-      warning("invalid argument for __has_include() directive", loc);
-      expr = expr.substr(0, q) + "0" + expr.substr(p);
-      p = q + 1;
-      return;
-    }
 
-    filename.set_text();
-
-    CPPFile::Source source = CPPFile::S_none;
-    found_file = find_include(filename, angle_quotes, source);
-  } else {
-    loc.last_column += loc.first_column + p - 2;
-    loc.first_column += args_begin;
-    warning("invalid argument for __has_include() directive", loc);
+  if (expr[p] != '(') {
+    error("expected '(' after '__has_include'");
+    return;
   }
-
-  string result = found_file ? "1" : "0";
-  expr = expr.substr(0, q) + result + expr.substr(p);
-  p = q + result.size();
-}
-
-/**
- *
- */
-void CPPPreprocessor::
-extract_manifest_args_inline(const string &name, int num_args,
-                             int va_arg, vector_string &args,
-                             const string &expr, size_t &p) {
-  // Skip whitespace till paren.
+  p++;
   while (p < expr.size() && isspace(expr[p])) {
     p++;
   }
-  if (p >= expr.size() || expr[p] != '(') {
-    // No paren, so we have only one arg.
-    size_t q = p;
-    while (p < expr.size() && (isalnum(expr[p]) || expr[p] == '_')) {
-      p++;
-    }
-    args.push_back(expr.substr(q, p - q));
 
-  } else if (expr[p] == '"' || expr[p] == '\'') {
-    // Quoted string or character.
-    int quote_mark = expr[p];
-    p++;
-    while (p < expr.size() && expr[p] != quote_mark && expr[p] != '\n') {
-      if (expr[p] == '\\') {
-        p++;
+  int paren_level = 1;
+  bool needs_expansion = false;
+  size_t r = p;
+  while (p < expr.size()) {
+    if (expr[p] == '"' || expr[p] == '\'' || expr[p] == '<') {
+      // Quoted string or angle bracket.
+      int quote_mark = expr[p];
+      if (quote_mark == '<') {
+        quote_mark = '>';
       }
-      if (p < expr.size()) {
-        p++;
+      p++;
+      while (p < expr.size() && expr[p] != quote_mark && expr[p] != '\n') {
+        if (expr[p] == '\\') {
+          p++;
+        }
+        if (p < expr.size()) {
+          p++;
+        }
       }
     }
+    else if (expr[p] == '(') {
+      ++paren_level;
+    }
+    else if (expr[p] == ')') {
+      --paren_level;
+      if (paren_level == 0) {
+        break;
+      }
+    }
+    else if (isalnum(expr[p]) || expr[p] == '_') {
+      needs_expansion = true;
+    }
     p++;
+  }
 
-  } else {
-    // Skip paren.
-    p++;
-    int paren_level = 1;
-    size_t q = p;
-    while (p < expr.size()) {
-      if (expr[p] == ',' && paren_level == 1) {
-        args.push_back(trim_blanks(expr.substr(q, p - q)));
-        q = p+1;
-
-      } else if (expr[p] == '"' || expr[p] == '\'') {
-        // Quoted string or character.
-        int quote_mark = expr[p];
-        p++;
-        while (p < expr.size() && expr[p] != quote_mark && expr[p] != '\n') {
-          if (expr[p] == '\\') {
-            p++;
-          }
-          if (p < expr.size()) {
-            p++;
-          }
-        }
+  if (p >= expr.size() || expr[p] != ')') {
+    error("missing ')' after '__has_include'");
+    return;
+  }
 
-      } else if (expr[p] == '(') {
-        ++paren_level;
+  // Back up to strip trailing whitespace.
+  size_t t = p;
+  while (t > r && isspace(expr[t - 1])) {
+    --t;
+  }
+  string inc = expr.substr(r, t - r);
+  p++;
 
-      } else if (expr[p] == ')') {
-        --paren_level;
-        if (paren_level == 0) {
-          break;
-        }
+  // Only expand if we've encountered unquoted identifier-valid characters,
+  // to be on the safe side.
+  if (needs_expansion) {
+    expand_manifests(inc, _manifests, false);
+  }
 
-      } else if (isspace(expr[p])) {
-        // Skip whitespace at the beginning.
-        if (q == p) {
-          q++;
-        }
-      }
-      p++;
-    }
-    args.push_back(trim_blanks(expr.substr(q, p - q)));
+  Filename filename;
+  bool angle_quotes = false;
 
-    if (p < expr.size() && expr[p] == ')') {
-      p++;
+  if (!inc.empty() && inc[0] == '"' && inc[inc.size() - 1] == '"') {
+    filename = inc.substr(1, inc.size() - 2);
+  }
+  else if (!inc.empty() && inc[0] == '<' && inc[inc.size() - 1] == '>') {
+    filename = inc.substr(1, inc.size() - 2);
+    if (!_noangles) {
+      // If _noangles is true, we don't make a distinction between angle
+      // brackets and quote marks--all #inc statements are treated the
+      // same, as if they used quote marks.
+      angle_quotes = true;
     }
   }
+  else {
+    warning("invalid argument for __has_include() directive: " + inc);
+    expr = expr.substr(0, q) + "0" + expr.substr(p);
+    p = q + 1;
+    return;
+  }
+
+  filename.set_text();
 
-  if ((int)args.size() < num_args) {
-    warning("Not enough arguments for manifest " + name);
+  CPPFile::Source source = CPPFile::S_none;
+  found_file = find_include(filename, angle_quotes, source);
 
-  } else if (va_arg < 0 && (int)args.size() > num_args) {
-    warning("Too many arguments for manifest " + name);
-  }
+  string result = found_file ? "1" : "0";
+  expr = expr.substr(0, q) + result + expr.substr(p);
+  p = q + result.size();
 }
 
 /**

dtool/src/cppparser/cppPreprocessor.h

@@ -57,11 +57,11 @@ public:
   int _token_index;
 #endif
 
-  void warning(const std::string &message);
-  void warning(const std::string &message, const YYLTYPE &loc);
-  void error(const std::string &message);
-  void error(const std::string &message, const YYLTYPE &loc);
-  void show_line(const YYLTYPE &loc);
+  void warning(const std::string &message) const;
+  void warning(const std::string &message, const YYLTYPE &loc) const;
+  void error(const std::string &message) const;
+  void error(const std::string &message, const YYLTYPE &loc) const;
+  void show_line(const YYLTYPE &loc) const;
 
   CPPCommentBlock *get_comment_before(int line, CPPFile file);
   CPPCommentBlock *get_comment_on(int line, CPPFile file);
@@ -69,7 +69,7 @@ public:
   int get_warning_count() const;
   int get_error_count() const;
 
-  typedef std::map<std::string, CPPManifest *> Manifests;
+  typedef CPPManifest::Manifests Manifests;
   Manifests _manifests;
 
   typedef std::vector<CPPManifest *> ManifestStack;
@@ -116,8 +116,9 @@ protected:
   bool push_expansion(const std::string &input, const CPPManifest *manifest,
                       const YYLTYPE &loc);
 
-  std::string expand_manifests(const std::string &input_expr, bool expand_undefined,
-                               const YYLTYPE &loc);
+public:
+  void expand_manifests(std::string &expr, const Manifests &manifests,
+                        bool expand_undefined = false) const;
   CPPExpression *parse_expr(const std::string &expr, CPPScope *current_scope,
                             CPPScope *global_scope, const YYLTYPE &loc);
 
@@ -145,8 +146,8 @@ private:
   void handle_error_directive(const std::string &args, const YYLTYPE &loc);
 
   void skip_false_if_block(bool consider_elifs);
-  bool is_manifest_defined(const std::string &manifest_name);
-  bool find_include(Filename &filename, bool angle_quotes, CPPFile::Source &source);
+  bool is_manifest_defined(const std::string &manifest_name) const;
+  bool find_include(Filename &filename, bool angle_quotes, CPPFile::Source &source) const;
 
   CPPToken get_quoted_char(int c);
   CPPToken get_quoted_string(int c);
@@ -158,11 +159,8 @@ private:
                           const YYLTYPE &loc, std::set<const CPPManifest *> &expanded);
   void extract_manifest_args(const std::string &name, int num_args,
                              int va_arg, vector_string &args);
-  void expand_defined_function(std::string &expr, size_t q, size_t &p);
-  void expand_has_include_function(std::string &expr, size_t q, size_t &p, YYLTYPE loc);
-  void extract_manifest_args_inline(const std::string &name, int num_args,
-                                    int va_arg, vector_string &args,
-                                    const std::string &expr, size_t &p);
+  void expand_defined_function(std::string &expr, size_t q, size_t &p) const;
+  void expand_has_include_function(std::string &expr, size_t q, size_t &p) const;
 
   CPPToken get_number(int c);
   static int check_keyword(const std::string &name);
@@ -227,8 +225,8 @@ private:
 
   std::vector<CPPToken> _saved_tokens;
 
-  int _warning_count;
-  int _error_count;
+  mutable int _warning_count;
+  mutable int _error_count;
   bool _error_abort;
 };
 

dtool/src/interrogate/interrogate.cxx

@@ -302,7 +302,7 @@ predefine_macro(CPPParser& parser, const string& inoption) {
     macro_name = inoption;
   }
 
-  CPPManifest *macro = new CPPManifest(macro_name, macro_def);
+  CPPManifest *macro = new CPPManifest(parser, macro_name, macro_def);
   parser._manifests[macro->_name] = macro;
 }
 

dtool/src/interrogate/parse_file.cxx

@@ -45,7 +45,7 @@ predefine_macro(CPPParser &parser, const string &option) {
 
   cerr << "Predefining " << macro_name << " as " << macro_def << "\n";
 
-  CPPManifest *macro = new CPPManifest(macro_name, macro_def);
+  CPPManifest *macro = new CPPManifest(parser, macro_name, macro_def);
   parser._manifests[macro->_name] = macro;
 }