From ba406d3a022967e6189249bd8e805f0eb9ac2921 Mon Sep 17 00:00:00 2001 From: Roger Meier Date: Mon, 15 Jul 2013 22:41:34 +0200 Subject: THRIFT-2089 Compiler ignores duplicate typenames Patch: Randy Abernethy --- compiler/cpp/src/parse/t_program.h | 97 +++++++++++++++++++++++++++ compiler/cpp/src/thrifty.yy | 8 +++ lib/cpp/test/TFileTransportTest.cpp | 5 +- lib/cpp/test/concurrency/ThreadFactoryTests.h | 2 +- lib/cpp/test/concurrency/ThreadManagerTests.h | 2 +- 5 files changed, 109 insertions(+), 5 deletions(-) mode change 100644 => 100755 lib/cpp/test/TFileTransportTest.cpp mode change 100644 => 100755 lib/cpp/test/concurrency/ThreadFactoryTests.h mode change 100644 => 100755 lib/cpp/test/concurrency/ThreadManagerTests.h diff --git a/compiler/cpp/src/parse/t_program.h b/compiler/cpp/src/parse/t_program.h index dfd9d43d2..f22da0737 100644 --- a/compiler/cpp/src/parse/t_program.h +++ b/compiler/cpp/src/parse/t_program.h @@ -123,6 +123,103 @@ class t_program : public t_doc { } } + // Typename collision detection + /** + * Search for typename collisions + * @param t the type to test for collisions + * @return true if a certain collision was found, otherwise false + */ + bool is_unique_typename(t_type * t) { + int occurances = program_typename_count(this, t); + for (std::vector::iterator it = includes_.begin(); + it != includes_.end(); ++it) { + occurances += program_typename_count(*it, t); + } + return 0 == occurances; + } + + /** + * Search all type collections for duplicate typenames + * @param prog the program to search + * @param t the type to test for collisions + * @return the number of certain typename collisions + */ + int program_typename_count(t_program * prog, t_type * t) { + int occurances = 0; + occurances += collection_typename_count(prog, prog->typedefs_, t); + occurances += collection_typename_count(prog, prog->enums_, t); + occurances += collection_typename_count(prog, prog->objects_, t); + occurances += collection_typename_count(prog, prog->services_, t); + return occurances; + } + + /** + * Search a type collection for duplicate typenames + * @param prog the program to search + * @param type_collection the type collection to search + * @param t the type to test for collisions + * @return the number of certain typename collisions + */ + template + int collection_typename_count(t_program * prog, T type_collection, t_type * t) { + int occurances = 0; + for (typename T::iterator it = type_collection.begin(); it != type_collection.end(); ++it) + if (t != *it && 0 == t->get_name().compare((*it)->get_name()) && is_common_namespace(prog, t)) + ++occurances; + return occurances; + } + + /** + * Determine whether identical typenames will collide based on namespaces. + * + * Because we do not know which languages the user will generate code for, + * collisions within programs (IDL files) having namespace declarations can be + * difficult to determine. Only guaranteed collisions return true (cause an error). + * Possible collisions involving explicit namespace declarations produce a warning. + * Other possible collisions go unreported. + * @param prog the program containing the preexisting typename + * @param t the type containing the typename match + * @return true if a collision within namespaces is found, otherwise false + */ + bool is_common_namespace(t_program * prog, t_type * t) { + //Case 1: Typenames are in the same program [collision] + if (prog == t->get_program()) { + pwarning(1, "Duplicate typename %s found in %s", + t->get_name().c_str(), t->get_program()->get_name().c_str()); + return true; + } + + //Case 2: Both programs have identical namespace scope/name declarations [collision] + bool match = true; + for (std::map::iterator it = prog->namespaces_.begin(); + it != prog->namespaces_.end(); ++it) { + if (0 == it->second.compare(t->get_program()->get_namespace(it->first))) { + pwarning(1, "Duplicate typename %s found in %s,%s,%s and %s,%s,%s [file,scope,ns]", + t->get_name().c_str(), + t->get_program()->get_name().c_str(), it->first.c_str(), it->second.c_str(), + prog->get_name().c_str(), it->first.c_str(), it->second.c_str()); + } else { + match = false; + } + } + for (std::map::iterator it = t->get_program()->namespaces_.begin(); + it != t->get_program()->namespaces_.end(); ++it) { + if (0 == it->second.compare(prog->get_namespace(it->first))) { + pwarning(1, "Duplicate typename %s found in %s,%s,%s and %s,%s,%s [file,scope,ns]", + t->get_name().c_str(), + t->get_program()->get_name().c_str(), it->first.c_str(), it->second.c_str(), + prog->get_name().c_str(), it->first.c_str(), it->second.c_str()); + } else { + match = false; + } + } + if (0 == prog->namespaces_.size() && 0 == t->get_program()->namespaces_.size()) { + pwarning(1, "Duplicate typename %s found in %s and %s", + t->get_name().c_str(), t->get_program()->get_name().c_str(), prog->get_name().c_str()); + } + return match; + } + // Scoping and namespacing void set_namespace(std::string name) { namespace_ = name; diff --git a/compiler/cpp/src/thrifty.yy b/compiler/cpp/src/thrifty.yy index 2d90f21ae..5b91a0a21 100644 --- a/compiler/cpp/src/thrifty.yy +++ b/compiler/cpp/src/thrifty.yy @@ -450,6 +450,10 @@ Definition: if (g_parent_scope != NULL) { g_parent_scope->add_type(g_parent_prefix + $1->get_name(), $1); } + if (! g_program->is_unique_typename($1)) { + yyerror("Type \"%s\" is already defined.", $1->get_name().c_str()); + exit(1); + } } $$ = $1; } @@ -462,6 +466,10 @@ Definition: g_parent_scope->add_service(g_parent_prefix + $1->get_name(), $1); } g_program->add_service($1); + if (! g_program->is_unique_typename($1)) { + yyerror("Type \"%s\" is already defined.", $1->get_name().c_str()); + exit(1); + } } $$ = $1; } diff --git a/lib/cpp/test/TFileTransportTest.cpp b/lib/cpp/test/TFileTransportTest.cpp old mode 100644 new mode 100755 index 7691c73fc..20c3b5c27 --- a/lib/cpp/test/TFileTransportTest.cpp +++ b/lib/cpp/test/TFileTransportTest.cpp @@ -20,9 +20,8 @@ #define _GNU_SOURCE // needed for getopt_long #endif -#ifdef HAVE_CONFIG_H -#include -#endif +#include + #ifdef HAVE_SYS_TIME_H #include #endif diff --git a/lib/cpp/test/concurrency/ThreadFactoryTests.h b/lib/cpp/test/concurrency/ThreadFactoryTests.h old mode 100644 new mode 100755 index b7e873ff1..fda6c9e6c --- a/lib/cpp/test/concurrency/ThreadFactoryTests.h +++ b/lib/cpp/test/concurrency/ThreadFactoryTests.h @@ -17,7 +17,7 @@ * under the License. */ -#include +#include #include #include #include diff --git a/lib/cpp/test/concurrency/ThreadManagerTests.h b/lib/cpp/test/concurrency/ThreadManagerTests.h old mode 100644 new mode 100755 index b734f7a0f..4e53a2d12 --- a/lib/cpp/test/concurrency/ThreadManagerTests.h +++ b/lib/cpp/test/concurrency/ThreadManagerTests.h @@ -17,7 +17,7 @@ * under the License. */ -#include +#include #include #include #include -- cgit v1.2.1