Vastly improve quality of Interrogate error messages, also fix erroneous location indications

This change also causes Interrogate to safely ignore "register" keywords in function parameters

dtool/src/cppparser/cppBison.yxx

@@ -60,14 +60,19 @@ yyerror(const string &msg) {
   current_lexer->error(msg);
 }
 
+static void
+yyerror(YYLTYPE *loc, const string &msg) {
+  current_lexer->error(msg, *loc);
+}
+
 static void
 yyerror(const string &msg, YYLTYPE &loc) {
-  current_lexer->error(msg, loc.first_line, loc.first_column);
+  current_lexer->error(msg, loc);
 }
 
 static void
 yywarning(const string &msg, YYLTYPE &loc) {
-  current_lexer->warning(msg, loc.first_line, loc.first_column);
+  current_lexer->warning(msg, loc);
 }
 
 static int
@@ -197,6 +202,12 @@ pop_struct() {
    pointers to the current yylval and yylloc. */
 %pure_parser
 
+/* Ideally we'd define this instead of the above, which gives us better
+   tracking of error locations, but we are still using a very old
+   version of Bison on Windows. */
+//%define api.pure full
+//%locations
+
 %token <u.real> REAL
 %token <u.integer> INTEGER
 %token <u.integer> CHAR_TOK
@@ -822,7 +833,7 @@ function_prototype:
     yyerror("Invalid destructor name: ~" + $2->get_fully_scoped_name(), @2);
   } else {
     CPPIdentifier *ident =
-      new CPPIdentifier("~" + $2->get_simple_name(), @2.file);
+      new CPPIdentifier("~" + $2->get_simple_name(), @2);
     delete $2;
 
     CPPType *type;
@@ -897,7 +908,7 @@ function_prototype:
   string name = "operator typecast " + $2->get_simple_name();
   CPPIdentifier *ident = $1;
   if (ident == NULL) {
-    ident = new CPPIdentifier(name, @1.file);
+    ident = new CPPIdentifier(name, @2);
   } else {
     ident->add_name(name);
   }
@@ -918,7 +929,7 @@ function_prototype:
 
   CPPIdentifier *ident = $1;
   if (ident == NULL) {
-    ident = new CPPIdentifier("operator typecast", @1.file);
+    ident = new CPPIdentifier("operator typecast", @4);
   } else {
     ident->add_name("operator typecast");
   }
@@ -1223,7 +1234,7 @@ instance_identifier:
   // like a regular function.
   CPPIdentifier *ident = $1;
   if (ident == NULL) {
-    ident = new CPPIdentifier("operator "+$2, @2.file);
+    ident = new CPPIdentifier("operator "+$2, @2);
   } else {
     ident->_names.push_back("operator "+$2);
   }
@@ -1414,6 +1425,16 @@ formal_parameter:
   $3->add_modifier(IIT_const);
   $$ = new CPPInstance($2, $3, 0, @3.file);
   $$->set_initializer($4);
+}
+        | KW_CONST KW_REGISTER type formal_parameter_identifier maybe_initialize
+{
+  $4->add_modifier(IIT_const);
+  $$ = new CPPInstance($3, $4, 0, @3.file);
+  $$->set_initializer($5);
+}
+        | KW_REGISTER formal_parameter
+{
+  $$ = $2;
 }
         | formal_const_expr
 {

dtool/src/cppparser/cppIdentifier.cxx

@@ -28,9 +28,14 @@
 //  Description:
 ////////////////////////////////////////////////////////////////////
 CPPIdentifier::
-CPPIdentifier(const string &name, const CPPFile &file) : _file(file) {
+CPPIdentifier(const string &name, const CPPFile &file) {
   _names.push_back(CPPNameComponent(name));
   _native_scope = (CPPScope *)NULL;
+  _loc.first_line = 0;
+  _loc.first_column = 0;
+  _loc.last_line = 0;
+  _loc.last_column = 0;
+  _loc.file = file;
 }
 
 ////////////////////////////////////////////////////////////////////
@@ -39,7 +44,34 @@ CPPIdentifier(const string &name, const CPPFile &file) : _file(file) {
 //  Description:
 ////////////////////////////////////////////////////////////////////
 CPPIdentifier::
-CPPIdentifier(const CPPNameComponent &name, const CPPFile &file) : _file(file) {
+CPPIdentifier(const CPPNameComponent &name, const CPPFile &file) {
+  _names.push_back(name);
+  _native_scope = (CPPScope *)NULL;
+  _loc.first_line = 0;
+  _loc.first_column = 0;
+  _loc.last_line = 0;
+  _loc.last_column = 0;
+  _loc.file = file;
+}
+
+////////////////////////////////////////////////////////////////////
+//     Function: CPPIdentifier::Constructor
+//       Access: Public
+//  Description:
+////////////////////////////////////////////////////////////////////
+CPPIdentifier::
+CPPIdentifier(const string &name, const cppyyltype &loc) : _loc(loc) {
+  _names.push_back(CPPNameComponent(name));
+  _native_scope = (CPPScope *)NULL;
+}
+
+////////////////////////////////////////////////////////////////////
+//     Function: CPPIdentifier::Constructor
+//       Access: Public
+//  Description:
+////////////////////////////////////////////////////////////////////
+CPPIdentifier::
+CPPIdentifier(const CPPNameComponent &name, const cppyyltype &loc) : _loc(loc) {
   _names.push_back(name);
   _native_scope = (CPPScope *)NULL;
 }
@@ -256,7 +288,8 @@ get_scope(CPPScope *current_scope, CPPScope *global_scope,
       if (error_sink != NULL) {
         error_sink->error("Symbol " + _names[i].get_name() +
                           " is not a known scope in " +
-                          scope->get_fully_scoped_name());
+                          scope->get_fully_scoped_name(),
+                          _loc);
       }
       return (CPPScope *)NULL;
     }
@@ -302,7 +335,8 @@ get_scope(CPPScope *current_scope, CPPScope *global_scope,
       if (error_sink != NULL) {
         error_sink->error("Symbol " + _names[i].get_name() +
                           " is not a known scope in " +
-                          scope->get_fully_scoped_name());
+                          scope->get_fully_scoped_name(),
+                          _loc);
       }
       return (CPPScope *)NULL;
     }

dtool/src/cppparser/cppIdentifier.h

@@ -20,6 +20,7 @@
 #include "cppDeclaration.h"
 #include "cppNameComponent.h"
 #include "cppFile.h"
+#include "cppBisonDefs.h"
 
 #include <string>
 #include <vector>
@@ -37,6 +38,8 @@ class CPPIdentifier {
 public:
   CPPIdentifier(const string &name, const CPPFile &file = CPPFile());
   CPPIdentifier(const CPPNameComponent &name, const CPPFile &file = CPPFile());
+  CPPIdentifier(const string &name, const cppyyltype &loc);
+  CPPIdentifier(const CPPNameComponent &name, const cppyyltype &loc);
   void add_name(const string &name);
   void add_name(const CPPNameComponent &name);
 
@@ -91,7 +94,7 @@ public:
   typedef vector<CPPNameComponent> Names;
   Names _names;
   CPPScope *_native_scope;
-  CPPFile _file;
+  cppyyltype _loc;
 };
 
 inline ostream &operator << (ostream &out, const CPPIdentifier &identifier) {

dtool/src/cppparser/cppInstance.h

@@ -19,7 +19,6 @@
 
 #include "cppDeclaration.h"
 #include "cppType.h"
-#include "cppIdentifier.h"
 #include "cppTemplateParameterList.h"
 
 class CPPInstanceIdentifier;

dtool/src/cppparser/cppNameComponent.h

@@ -16,6 +16,7 @@
 #define CPPNAMECOMPONENT_H
 
 #include "dtoolbase.h"
+#include "cppBisonDefs.h"
 
 #include <string>
 

dtool/src/cppparser/cppPreprocessor.h

@@ -53,15 +53,17 @@ public:
   int get_col_number() const;
 
   CPPToken get_next_token();
+  CPPToken peek_next_token();
 #ifdef CPP_VERBOSE_LEX
   CPPToken get_next_token0();
   int _token_index;
 #endif
 
-  void warning(const string &message, int line = 0, int col = 0,
-               CPPFile file = CPPFile());
-  void error(const string &message, int line = 0, int col = 0,
-             CPPFile file = CPPFile());
+  void warning(const string &message);
+  void warning(const string &message, const YYLTYPE &loc);
+  void error(const string &message);
+  void error(const string &message, const YYLTYPE &loc);
+  void show_line(const YYLTYPE &loc);
 
   CPPCommentBlock *get_comment_before(int line, CPPFile file);
   CPPCommentBlock *get_comment_on(int line, CPPFile file);
@@ -112,6 +114,8 @@ protected:
 
 private:
   CPPToken internal_get_next_token();
+  int check_digraph(int c);
+  int check_trigraph(int c);
   int skip_whitespace(int c);
   int skip_comment(int c);
   int skip_c_comment(int c);
@@ -121,22 +125,14 @@ private:
   int get_preprocessor_command(int c, string &command);
   int get_preprocessor_args(int c, string &args);
 
-  void handle_define_directive(const string &args, int first_line,
-                               int first_col, const CPPFile &first_file);
-  void handle_undef_directive(const string &args, int first_line,
-                              int first_col, const CPPFile &first_file);
-  void handle_ifdef_directive(const string &args, int first_line,
-                              int first_col, const CPPFile &first_file);
-  void handle_ifndef_directive(const string &args, int first_line,
-                               int first_col, const CPPFile &first_file);
-  void handle_if_directive(const string &args, int first_line,
-                           int first_col, const CPPFile &first_file);
-  void handle_include_directive(const string &args, int first_line,
-                                int first_col, const CPPFile &first_file);
-  void handle_pragma_directive(const string &args, int first_line,
-                               int first_col, const CPPFile &first_file);
-  void handle_error_directive(const string &args, int first_line,
-                              int first_col, const CPPFile &first_file);
+  void handle_define_directive(const string &args, const YYLTYPE &loc);
+  void handle_undef_directive(const string &args, const YYLTYPE &loc);
+  void handle_ifdef_directive(const string &args, const YYLTYPE &loc);
+  void handle_ifndef_directive(const string &args, const YYLTYPE &loc);
+  void handle_if_directive(const string &args, const YYLTYPE &loc);
+  void handle_include_directive(const string &args, const YYLTYPE &loc);
+  void handle_pragma_directive(const string &args, const YYLTYPE &loc);
+  void handle_error_directive(const string &args, const YYLTYPE &loc);
 
   void skip_false_if_block(bool consider_elifs);
 
@@ -153,7 +149,7 @@ private:
                                     int va_arg, vector_string &args,
                                     const string &expr, size_t &p);
 
-  CPPToken get_number(int c, int c2 = 0);
+  CPPToken get_number(int c);
   static int check_keyword(const string &name);
   int scan_escape_sequence(int c);
   string scan_quoted(int c);
@@ -162,6 +158,7 @@ private:
   bool should_ignore_preprocessor() const;
 
   int get();
+  int peek();
   void unget(int c);
 
   CPPTemplateParameterList *
@@ -177,6 +174,7 @@ private:
     bool open(const CPPFile &file);
     bool connect_input(const string &input);
     int get();
+    int peek();
 
     const CPPManifest *_ignore_manifest;
     CPPFile _file;
@@ -184,6 +182,8 @@ private:
     istream *_in;
     int _line_number;
     int _col_number;
+    int _next_line_number;
+    int _next_col_number;
     bool _lock_position;
     int _prev_last_c;
   };

dtool/src/cppparser/cppScope.cxx

@@ -226,9 +226,14 @@ define_extension_type(CPPExtensionType *type, CPPPreprocessor *error_sink) {
       if (other_ext->_type != type->_type) {
         if (error_sink != NULL) {
           ostringstream errstr;
-          errstr << other_ext->_type << " " << type->get_fully_scoped_name()
-                 << " was previously declared as " << other_ext->_type << "\n";
-          error_sink->error(errstr.str());
+          errstr << type->_type << " " << type->get_fully_scoped_name()
+                 << " was previously declared as " << other_ext->_type;
+          error_sink->error(errstr.str(), type->_ident->_loc);
+
+          if (other_ext->_ident != NULL) {
+            error_sink->error("previous declaration is here",
+                              other_ext->_ident->_loc);
+          }
         }
       }
       (*result.first).second = type;
@@ -242,10 +247,19 @@ define_extension_type(CPPExtensionType *type, CPPPreprocessor *error_sink) {
 
         if (error_sink != NULL) {
           ostringstream errstr;
+          if (!cppparser_output_class_keyword) {
+            errstr << type->_type << " ";
+          }
           type->output(errstr, 0, NULL, false);
-          errstr << " has conflicting declaration as ";
+          errstr << " has conflicting definition as ";
           other_type->output(errstr, 0, NULL, true);
-          error_sink->error(errstr.str());
+          error_sink->error(errstr.str(), type->_ident->_loc);
+
+          CPPExtensionType *other_ext = other_type->as_extension_type();
+          if (other_ext != NULL && other_ext->_ident != NULL) {
+            error_sink->error("previous definition is here",
+                              other_ext->_ident->_loc);
+          }
         }
       }
     }
@@ -1107,7 +1121,9 @@ handle_declaration(CPPDeclaration *decl, CPPScope *global_scope,
           def->output(errstr, 0, NULL, false);
           errstr << " has conflicting declaration as ";
           other_type->output(errstr, 0, NULL, true);
-          error_sink->error(errstr.str());
+          error_sink->error(errstr.str(), def->_ident->_loc);
+          error_sink->error("previous definition is here",
+                            other_td->_ident->_loc);
         }
       }
     } else {

dtool/src/cppparser/cppToken.cxx

@@ -38,6 +38,18 @@ CPPToken(int token, int line_number, int col_number,
   _lloc.file = file;
 }
 
+////////////////////////////////////////////////////////////////////
+//     Function: CPPToken::Constructor
+//       Access: Public
+//  Description:
+////////////////////////////////////////////////////////////////////
+CPPToken::
+CPPToken(int token, const YYLTYPE &loc, const string &str, const YYSTYPE &val) :
+  _token(token), _lloc(loc), _lval(val)
+{
+  _lval.str = str;
+}
+
 ////////////////////////////////////////////////////////////////////
 //     Function: CPPToken::Copy Constructor
 //       Access: Public

dtool/src/cppparser/cppToken.h

@@ -29,6 +29,9 @@ public:
            const CPPFile &file = CPPFile(""),
            const string &str = string(),
            const YYSTYPE &lval = YYSTYPE());
+  CPPToken(int token, const YYLTYPE &loc,
+           const string &str = string(),
+           const YYSTYPE &lval = YYSTYPE());
   CPPToken(const CPPToken &copy);
   void operator = (const CPPToken &copy);
 

dtool/src/interrogate/typeManager.cxx

@@ -20,6 +20,7 @@
 #include "cppEnumType.h"
 #include "cppFunctionGroup.h"
 #include "cppFunctionType.h"
+#include "cppIdentifier.h"
 #include "cppParameterList.h"
 #include "cppPointerType.h"
 #include "cppReferenceType.h"