From 5e905d44631ef385894cce0e24d6f2feb2f9754c Mon Sep 17 00:00:00 2001 From: rdb Date: Sun, 4 Jan 2015 20:30:24 +0100 Subject: [PATCH] Fix various parser bugs, improve error reporting (to show the included file chain -vv is now needed) --- dtool/src/cppparser/cppExtensionType.cxx | 23 +++++ dtool/src/cppparser/cppExtensionType.h | 7 +- dtool/src/cppparser/cppPreprocessor.cxx | 19 +++-- dtool/src/cppparser/cppScope.cxx | 83 +++++++++++++++---- dtool/src/cppparser/cppScope.h | 6 +- dtool/src/cppparser/cppTemplateScope.cxx | 4 +- dtool/src/cppparser/cppTemplateScope.h | 3 +- .../interfaceMakerPythonNative.cxx | 4 +- dtool/src/interrogate/typeManager.cxx | 32 ++++--- dtool/src/interrogate/typeManager.h | 2 +- dtool/src/parser-inc/cg.h | 13 +-- panda/src/express/zStreamBuf.cxx | 4 +- 12 files changed, 148 insertions(+), 52 deletions(-) diff --git a/dtool/src/cppparser/cppExtensionType.cxx b/dtool/src/cppparser/cppExtensionType.cxx index 92924f345a..26022cf8bf 100644 --- a/dtool/src/cppparser/cppExtensionType.cxx +++ b/dtool/src/cppparser/cppExtensionType.cxx @@ -130,6 +130,29 @@ substitute_decl(CPPDeclaration::SubstDecl &subst, return rep; } +//////////////////////////////////////////////////////////////////// +// Function: CPPExtensionType::resolve_type +// Access: Public, Virtual +// Description: If this CPPType object is a forward reference or +// other nonspecified reference to a type that might now +// be known a real type, returns the real type. +// Otherwise returns the type itself. +//////////////////////////////////////////////////////////////////// +CPPType *CPPExtensionType:: +resolve_type(CPPScope *current_scope, CPPScope *global_scope) { + if (_ident == NULL) { + // We can't resolve anonymous types. But that's OK, since they + // can't be forward declared anyway. + return this; + } + + // Maybe it has been defined by now. + CPPType *type = _ident->find_type(current_scope, global_scope); + if (type != NULL) { + return type; + } + return this; +} //////////////////////////////////////////////////////////////////// // Function: CPPExtensionType::is_equivalent_type diff --git a/dtool/src/cppparser/cppExtensionType.h b/dtool/src/cppparser/cppExtensionType.h index f8771f70ea..d1d810209c 100644 --- a/dtool/src/cppparser/cppExtensionType.h +++ b/dtool/src/cppparser/cppExtensionType.h @@ -25,7 +25,9 @@ class CPPIdentifier; /////////////////////////////////////////////////////////////////// // Class : CPPExtensionType -// Description : +// Description : Base class of enum, class, struct, and union types. +// An instance of the base class (instead of one of +// the specializations) is used for forward references. //////////////////////////////////////////////////////////////////// class CPPExtensionType : public CPPType { public: @@ -50,6 +52,9 @@ public: CPPScope *current_scope, CPPScope *global_scope); + virtual CPPType *resolve_type(CPPScope *current_scope, + CPPScope *global_scope); + virtual bool is_equivalent(const CPPType &other) const; diff --git a/dtool/src/cppparser/cppPreprocessor.cxx b/dtool/src/cppparser/cppPreprocessor.cxx index 226aabc132..42a786e729 100644 --- a/dtool/src/cppparser/cppPreprocessor.cxx +++ b/dtool/src/cppparser/cppPreprocessor.cxx @@ -432,10 +432,14 @@ warning(const string &message, int line, int col, CPPFile file) { if (file.empty()) { file = get_file(); } - indent(cerr, _files.size() * 2) + int indent_level = 0; + if (_verbose >= 3) { + indent_level = _files.size() * 2; + } + indent(cerr, indent_level) << "*** Warning in " << file << " near line " << line << ", column " << col << ":\n"; - indent(cerr, _files.size() * 2) + indent(cerr, indent_level) << message << "\n"; } _warning_count++; @@ -462,10 +466,14 @@ error(const string &message, int line, int col, CPPFile file) { if (file.empty()) { file = get_file(); } - indent(cerr, _files.size() * 2) + int indent_level = 0; + if (_verbose >= 3) { + indent_level = _files.size() * 2; + } + indent(cerr, indent_level) << "*** Error in " << file << " near line " << line << ", column " << col << ":\n"; - indent(cerr, _files.size() * 2) + indent(cerr, indent_level) << message << "\n"; if (_error_abort) { @@ -608,7 +616,7 @@ init_type(const string &type) { //////////////////////////////////////////////////////////////////// bool CPPPreprocessor:: push_file(const CPPFile &file) { - if (_verbose >= 2) { + if (_verbose >= 3) { indent(cerr, _files.size() * 2) << "Reading " << file << "\n"; } @@ -2294,7 +2302,6 @@ unget(int c) { _unget = c; } - //////////////////////////////////////////////////////////////////// // Function: CPPPreprocessor::nested_parse_template_instantiation // Access: Private diff --git a/dtool/src/cppparser/cppScope.cxx b/dtool/src/cppparser/cppScope.cxx index 39fdb5f89a..d32661e1f7 100644 --- a/dtool/src/cppparser/cppScope.cxx +++ b/dtool/src/cppparser/cppScope.cxx @@ -136,7 +136,7 @@ add_declaration(CPPDeclaration *decl, CPPScope *global_scope, _declarations.push_back(decl); - handle_declaration(decl, global_scope); + handle_declaration(decl, global_scope, preprocessor); } //////////////////////////////////////////////////////////////////// @@ -185,9 +185,12 @@ add_enum_value(CPPInstance *inst, CPPPreprocessor *preprocessor, // Description: //////////////////////////////////////////////////////////////////// void CPPScope:: -define_extension_type(CPPExtensionType *type) { +define_extension_type(CPPExtensionType *type, CPPPreprocessor *error_sink) { assert(type != NULL); - string name = type->get_simple_name(); + string name = type->get_local_name(this); + if (name.empty()) { + return; + } switch (type->_type) { case CPPExtensionType::T_class: @@ -204,6 +207,7 @@ define_extension_type(CPPExtensionType *type) { case CPPExtensionType::T_enum: _enums[name] = type; + break; } // Create an implicit typedef for the extension. @@ -213,21 +217,45 @@ define_extension_type(CPPExtensionType *type) { if (!result.second) { // There's already a typedef for this extension. This one - // overrides if it has template parameters and the other one - // doesn't. + // overrides it only if the other is a forward declaration. CPPType *other_type = (*result.first).second; - if (type->is_template() && !other_type->is_template()) { + + if (other_type->get_subtype() == CPPDeclaration::ST_extension) { + CPPExtensionType *other_ext = other_type->as_extension_type(); + + 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()); + } + } (*result.first).second = type; - // Or if the other one is a forward reference. - } else if (other_type->get_subtype() == CPPDeclaration::ST_extension) { - (*result.first).second = type; + } else { + CPPTypedefType *other_td = other_type->as_typedef_type(); + + // Error out if the declaration is different than the previous one. + if (other_type != type && + (other_td == NULL || other_td->_type != type)) { + + if (error_sink != NULL) { + ostringstream errstr; + type->output(errstr, 0, NULL, false); + errstr << " has conflicting declaration as "; + other_type->output(errstr, 0, NULL, true); + error_sink->error(errstr.str()); + } + } } } if (type->is_template()) { + string simple_name = type->get_simple_name(); + pair result = - _templates.insert(Templates::value_type(name, type)); + _templates.insert(Templates::value_type(simple_name, type)); if (!result.second) { // The template was not inserted because we already had a @@ -277,7 +305,7 @@ add_using(CPPUsing *using_decl, CPPScope *global_scope, } else { CPPDeclaration *decl = using_decl->_ident->find_symbol(this, global_scope); if (decl != NULL) { - handle_declaration(decl, global_scope); + handle_declaration(decl, global_scope, error_sink); } else { if (error_sink != NULL) { error_sink->warning("Attempt to use unknown symbol: " + using_decl->_ident->get_fully_scoped_name()); @@ -1047,16 +1075,39 @@ copy_substitute_decl(CPPScope *to_scope, CPPDeclaration::SubstDecl &subst, // functions, or whatever. //////////////////////////////////////////////////////////////////// void CPPScope:: -handle_declaration(CPPDeclaration *decl, CPPScope *global_scope) { +handle_declaration(CPPDeclaration *decl, CPPScope *global_scope, + CPPPreprocessor *error_sink) { CPPTypedefType *def = decl->as_typedef_type(); if (def != NULL) { string name = def->get_simple_name(); - _types[name] = def; + pair result = + _types.insert(Types::value_type(name, def)); + + if (!result.second) { + CPPType *other_type = result.first->second; + CPPTypedefType *other_td = other_type->as_typedef_type(); + + // We don't do redefinitions of typedefs. But we don't complain + // as long as this is actually a typedef to the previous definition. + if (other_type != def->_type && + (other_td == NULL || other_td->_type != def->_type)) { + + if (error_sink != NULL) { + ostringstream errstr; + def->output(errstr, 0, NULL, false); + errstr << " has conflicting declaration as "; + other_type->output(errstr, 0, NULL, true); + error_sink->error(errstr.str()); + } + } + } else { + _types[name] = def; + } CPPExtensionType *et = def->_type->as_extension_type(); if (et != NULL) { - define_extension_type(et); + define_extension_type(et, error_sink); } return; @@ -1066,7 +1117,7 @@ handle_declaration(CPPDeclaration *decl, CPPScope *global_scope) { if (typedecl != (CPPTypeDeclaration *)NULL) { CPPExtensionType *et = typedecl->_type->as_extension_type(); if (et != NULL) { - define_extension_type(et); + define_extension_type(et, error_sink); } return; } @@ -1121,6 +1172,6 @@ handle_declaration(CPPDeclaration *decl, CPPScope *global_scope) { CPPExtensionType *et = decl->as_extension_type(); if (et != NULL) { - define_extension_type(et); + define_extension_type(et, error_sink); } } diff --git a/dtool/src/cppparser/cppScope.h b/dtool/src/cppparser/cppScope.h index bbcd57bd86..e7bccbd7fe 100644 --- a/dtool/src/cppparser/cppScope.h +++ b/dtool/src/cppparser/cppScope.h @@ -66,7 +66,8 @@ public: virtual void add_enum_value(CPPInstance *inst, CPPPreprocessor *preprocessor, const cppyyltype &pos); - virtual void define_extension_type(CPPExtensionType *type); + virtual void define_extension_type(CPPExtensionType *type, + CPPPreprocessor *error_sink = NULL); virtual void define_namespace(CPPNamespace *scope); virtual void add_using(CPPUsing *using_decl, CPPScope *global_scope, CPPPreprocessor *error_sink = NULL); @@ -113,7 +114,8 @@ private: copy_substitute_decl(CPPScope *to_scope, CPPDeclaration::SubstDecl &subst, CPPScope *global_scope) const; - void handle_declaration(CPPDeclaration *decl, CPPScope *global_scope); + void handle_declaration(CPPDeclaration *decl, CPPScope *global_scope, + CPPPreprocessor *error_sink = NULL); public: typedef vector Declarations; diff --git a/dtool/src/cppparser/cppTemplateScope.cxx b/dtool/src/cppparser/cppTemplateScope.cxx index 520d8ba429..7f5fdbb089 100644 --- a/dtool/src/cppparser/cppTemplateScope.cxx +++ b/dtool/src/cppparser/cppTemplateScope.cxx @@ -64,10 +64,10 @@ add_enum_value(CPPInstance *inst, CPPPreprocessor *preprocessor, // Description: //////////////////////////////////////////////////////////////////// void CPPTemplateScope:: -define_extension_type(CPPExtensionType *type) { +define_extension_type(CPPExtensionType *type, CPPPreprocessor *error_sink) { type->_template_scope = this; assert(_parent_scope != NULL); - _parent_scope->define_extension_type(type); + _parent_scope->define_extension_type(type, error_sink); } //////////////////////////////////////////////////////////////////// diff --git a/dtool/src/cppparser/cppTemplateScope.h b/dtool/src/cppparser/cppTemplateScope.h index 40a7feb74c..b8fa313e23 100644 --- a/dtool/src/cppparser/cppTemplateScope.h +++ b/dtool/src/cppparser/cppTemplateScope.h @@ -39,7 +39,8 @@ public: virtual void add_enum_value(CPPInstance *inst, CPPPreprocessor *preprocessor, const cppyyltype &pos); - virtual void define_extension_type(CPPExtensionType *type); + virtual void define_extension_type(CPPExtensionType *type, + CPPPreprocessor *error_sink = NULL); virtual void define_namespace(CPPNamespace *scope); virtual void add_using(CPPUsing *using_decl, CPPScope *global_scope, CPPPreprocessor *error_sink = NULL); diff --git a/dtool/src/interrogate/interfaceMakerPythonNative.cxx b/dtool/src/interrogate/interfaceMakerPythonNative.cxx index a8a4d666b9..b6ea426020 100644 --- a/dtool/src/interrogate/interfaceMakerPythonNative.cxx +++ b/dtool/src/interrogate/interfaceMakerPythonNative.cxx @@ -4774,7 +4774,7 @@ is_cpp_type_legal(CPPType *in_ctype) { return true; } else if (TypeManager::is_pointer_to_simple(type)) { return true; - } else if (TypeManager::IsExported(type)) { + } else if (TypeManager::is_exported(type)) { return true; } else if (TypeManager::is_pointer_to_PyObject(in_ctype)) { return true; @@ -4797,7 +4797,7 @@ isExportThisRun(CPPType *ctype) { return true; } - if (!TypeManager::IsExported(ctype)) { + if (!TypeManager::is_exported(ctype)) { return false; } diff --git a/dtool/src/interrogate/typeManager.cxx b/dtool/src/interrogate/typeManager.cxx index c2e51f49e6..54174e651c 100644 --- a/dtool/src/interrogate/typeManager.cxx +++ b/dtool/src/interrogate/typeManager.cxx @@ -53,6 +53,10 @@ resolve_type(CPPType *type, CPPScope *scope) { return type; } + // I think I fixed the bug; no need for the below hack any more. + return type; + +/* CPPType *new_type = parser.parse_type(name); if (new_type == (CPPType *)NULL) { nout << "Type \"" << name << "\" (from " << *orig_type << ") is unknown to parser.\n"; @@ -61,6 +65,7 @@ resolve_type(CPPType *type, CPPScope *scope) { } return type; +*/ } //////////////////////////////////////////////////////////////////// @@ -2355,20 +2360,17 @@ has_protected_destructor(CPPType *type) { return false; } //////////////////////////////////////////////////////////////////// -// Function: TypeManager::has_protected_destructor +// Function: TypeManager::is_exported // Access: Public, Static -// Description: Returns true if the destructor for the given class or -// struct is protected or private, or false if the -// destructor is public or absent. +// Description: //////////////////////////////////////////////////////////////////// -bool TypeManager::IsExported(CPPType *in_type) { +bool TypeManager:: +is_exported(CPPType *in_type) { string name = in_type->get_local_name(&parser); if (name.empty()) { - return false; + return false; } - //return true; - // this question is about the base type CPPType *base_type = resolve_type(unwrap(in_type)); //CPPType *base_type = in_type; @@ -2415,7 +2417,7 @@ bool TypeManager::IsExported(CPPType *in_type) { CPPTypedefType *tdef = base_type->as_typedef_type(); if (tdef->_type->get_subtype() == CPPDeclaration::ST_struct) { CPPStructType *struct_type =tdef->_type->resolve_type(&parser, &parser)->as_struct_type(); - return IsExported(struct_type); + return is_exported(struct_type); } } else if (base_type->get_subtype() == CPPDeclaration::ST_type_declaration) { @@ -2423,12 +2425,13 @@ bool TypeManager::IsExported(CPPType *in_type) { if (type->get_subtype() == CPPDeclaration::ST_struct) { CPPStructType *struct_type =type->as_type()->resolve_type(&parser, &parser)->as_struct_type(); //CPPScope *scope = struct_type->_scope; - return IsExported(struct_type); + return is_exported(struct_type); } else if (type->get_subtype() == CPPDeclaration::ST_enum) { //CPPEnumType *enum_type = type->as_type()->resolve_type(&parser, &parser)->as_enum_type(); - if (type->_vis <= min_vis) + if (type->_vis <= min_vis) { return true; + } } } @@ -2464,8 +2467,11 @@ is_local(CPPType *source_type) { return false; default: - if (source_type->_file._source == CPPFile::S_local && !source_type->is_incomplete()) { - return true; + { + CPPType *resolved_type = resolve_type(source_type); + if (resolved_type->_file._source == CPPFile::S_local && !resolved_type->is_incomplete()) { + return true; + } } } diff --git a/dtool/src/interrogate/typeManager.h b/dtool/src/interrogate/typeManager.h index 8ee38490ee..9c3c1530c9 100644 --- a/dtool/src/interrogate/typeManager.h +++ b/dtool/src/interrogate/typeManager.h @@ -143,7 +143,7 @@ public: static bool has_protected_destructor(CPPType *type); - static bool IsExported(CPPType *type); + static bool is_exported(CPPType *type); static bool is_local(CPPType *type); }; diff --git a/dtool/src/parser-inc/cg.h b/dtool/src/parser-inc/cg.h index 5af0889578..a0adab732b 100755 --- a/dtool/src/parser-inc/cg.h +++ b/dtool/src/parser-inc/cg.h @@ -20,11 +20,12 @@ #ifndef CG_H #define CG_H -typedef int CGcontext; -typedef int CGprogram; -typedef int CGparameter; -typedef int CGprofile; -typedef int CGerror; +typedef struct _CGcontext *CGcontext; +typedef struct _CGcontext *CGcontext; +typedef struct _CGprogram *CGprogram; +typedef struct _CGparameter *CGparameter; + +typedef enum {} CGprofile; +typedef enum {} CGerror; #endif - diff --git a/panda/src/express/zStreamBuf.cxx b/panda/src/express/zStreamBuf.cxx index 8a27ab5a0d..1ea01fe889 100644 --- a/panda/src/express/zStreamBuf.cxx +++ b/panda/src/express/zStreamBuf.cxx @@ -19,13 +19,13 @@ #include "pnotify.h" #include "config_express.h" -#if !defined(USE_MEMORY_NOWRAPPERS) +#if !defined(USE_MEMORY_NOWRAPPERS) && !defined(CPPPARSER) // Define functions that hook zlib into panda's memory allocation system. static void * do_zlib_alloc(voidpf opaque, uInt items, uInt size) { return PANDA_MALLOC_ARRAY(items * size); } -static void +static void do_zlib_free(voidpf opaque, voidpf address) { PANDA_FREE_ARRAY(address); }