aboutsummaryrefslogtreecommitdiffhomepage
path: root/src
diff options
context:
space:
mode:
authorGravatar temporal <temporal@630680e5-0e50-0410-840e-4b1c322b438d>2008-07-23 01:19:07 +0000
committerGravatar temporal <temporal@630680e5-0e50-0410-840e-4b1c322b438d>2008-07-23 01:19:07 +0000
commitf206351d1469970230caa5f61eaa54797194bee1 (patch)
tree3b1b9ea37171fdb231bca2fe6c74492eeea2abd7 /src
parentcc930432c2823c3d82e0b8dd2ae4f446c82f4fce (diff)
Sync code with Google-internal branch. Changes:
Protoc (parser) - Improved error message when an enum value's name conflicts with another symbol defined in the enum type's scope, e.g. if two enum types declared in the same scope have values with the same name. This is disallowed for compatibility with C++, but this wasn't clear from the error. C++ - Restored the set_foo(const char*) accessor for "bytes" type because some code inside Google depends on it. However, set_foo(const char*, int) is still there (and actually is changed to take const void*). - Fixed TokenizerTest when compiling with -DNDEBUG on Linux. - Other irrelevant tweaks. Java - Fixed UnknownFieldSet's parsing of varints larger than 32 bits. - Fixed TextFormat's parsing of "inf" and "nan". - Fixed TextFormat's parsing of comments. Python - Fixed text_format_test on Windows where floating-point exponents sometimes contain extra zeros.
Diffstat (limited to 'src')
-rw-r--r--src/google/protobuf/compiler/cpp/cpp_string_field.cc63
-rw-r--r--src/google/protobuf/compiler/parser.cc3
-rw-r--r--src/google/protobuf/descriptor.cc46
-rw-r--r--src/google/protobuf/descriptor.h14
-rw-r--r--src/google/protobuf/descriptor.pb.h8
-rw-r--r--src/google/protobuf/descriptor_unittest.cc27
-rw-r--r--src/google/protobuf/io/tokenizer_unittest.cc12
7 files changed, 120 insertions, 53 deletions
diff --git a/src/google/protobuf/compiler/cpp/cpp_string_field.cc b/src/google/protobuf/compiler/cpp/cpp_string_field.cc
index e74eb432..8590df7e 100644
--- a/src/google/protobuf/compiler/cpp/cpp_string_field.cc
+++ b/src/google/protobuf/compiler/cpp/cpp_string_field.cc
@@ -96,13 +96,11 @@ GenerateAccessorDeclarations(io::Printer* printer) const {
printer->Print(variables_,
"inline const ::std::string& $name$() const;\n"
- "inline void set_$name$(const ::std::string& value);\n");
+ "inline void set_$name$(const ::std::string& value);\n"
+ "inline void set_$name$(const char* value);\n");
if (descriptor_->type() == FieldDescriptor::TYPE_BYTES) {
printer->Print(variables_,
- "inline void set_$name$(const char* value, size_t size);\n");
- } else {
- printer->Print(variables_,
- "inline void set_$name$(const char* value);\n");
+ "inline void set_$name$(const void* value, size_t size);\n");
}
printer->Print(variables_,
@@ -127,25 +125,23 @@ GenerateInlineAccessorDefinitions(io::Printer* printer) const {
" $name$_ = new ::std::string;\n"
" }\n"
" $name$_->assign(value);\n"
+ "}\n"
+ "inline void $classname$::set_$name$(const char* value) {\n"
+ " _set_bit($index$);\n"
+ " if ($name$_ == &_default_$name$_) {\n"
+ " $name$_ = new ::std::string;\n"
+ " }\n"
+ " $name$_->assign(value);\n"
"}\n");
if (descriptor_->type() == FieldDescriptor::TYPE_BYTES) {
printer->Print(variables_,
- "inline void $classname$::set_$name$(const char* value, size_t size) {\n"
+ "inline void $classname$::set_$name$(const void* value, size_t size) {\n"
" _set_bit($index$);\n"
" if ($name$_ == &_default_$name$_) {\n"
" $name$_ = new ::std::string;\n"
" }\n"
- " $name$_->assign(value, size);\n"
- "}\n");
- } else {
- printer->Print(variables_,
- "inline void $classname$::set_$name$(const char* value) {\n"
- " _set_bit($index$);\n"
- " if ($name$_ == &_default_$name$_) {\n"
- " $name$_ = new ::std::string;\n"
- " }\n"
- " $name$_->assign(value);\n"
+ " $name$_->assign(reinterpret_cast<const char*>(value), size);\n"
"}\n");
}
@@ -265,17 +261,15 @@ GenerateAccessorDeclarations(io::Printer* printer) const {
"inline const ::std::string& $name$(int index) const;\n"
"inline ::std::string* mutable_$name$(int index);\n"
"inline void set_$name$(int index, const ::std::string& value);\n"
+ "inline void set_$name$(int index, const char* value);\n"
"inline ::std::string* add_$name$();\n"
- "inline void add_$name$(const ::std::string& value);\n");
+ "inline void add_$name$(const ::std::string& value);\n"
+ "inline void add_$name$(const char* value);\n");
if (descriptor_->type() == FieldDescriptor::TYPE_BYTES) {
printer->Print(variables_,
- "inline void set_$name$(int index, const char* value, size_t size);\n"
- "inline void add_$name$(const char* value, size_t size);\n");
- } else {
- printer->Print(variables_,
- "inline void set_$name$(int index, const char* value);\n"
- "inline void add_$name$(const char* value);\n");
+ "inline void set_$name$(int index, const void* value, size_t size);\n"
+ "inline void add_$name$(const void* value, size_t size);\n");
}
if (descriptor_->options().has_ctype()) {
@@ -305,29 +299,28 @@ GenerateInlineAccessorDefinitions(io::Printer* printer) const {
"inline void $classname$::set_$name$(int index, const ::std::string& value) {\n"
" $name$_.Mutable(index)->assign(value);\n"
"}\n"
+ "inline void $classname$::set_$name$(int index, const char* value) {\n"
+ " $name$_.Mutable(index)->assign(value);\n"
+ "}\n"
"inline ::std::string* $classname$::add_$name$() {\n"
" return $name$_.Add();\n"
"}\n"
"inline void $classname$::add_$name$(const ::std::string& value) {\n"
" $name$_.Add()->assign(value);\n"
+ "}\n"
+ "inline void $classname$::add_$name$(const char* value) {\n"
+ " $name$_.Add()->assign(value);\n"
"}\n");
if (descriptor_->type() == FieldDescriptor::TYPE_BYTES) {
printer->Print(variables_,
"inline void "
- "$classname$::set_$name$(int index, const char* value, size_t size) {\n"
- " $name$_.Mutable(index)->assign(value, size);\n"
- "}\n"
- "inline void $classname$::add_$name$(const char* value, size_t size) {\n"
- " $name$_.Add()->assign(value, size);\n"
- "}\n");
- } else {
- printer->Print(variables_,
- "inline void $classname$::set_$name$(int index, const char* value) {\n"
- " $name$_.Mutable(index)->assign(value);\n"
+ "$classname$::set_$name$(int index, const void* value, size_t size) {\n"
+ " $name$_.Mutable(index)->assign(\n"
+ " reinterpret_cast<const char*>(value), size);\n"
"}\n"
- "inline void $classname$::add_$name$(const char* value) {\n"
- " $name$_.Add()->assign(value);\n"
+ "inline void $classname$::add_$name$(const void* value, size_t size) {\n"
+ " $name$_.Add()->assign(reinterpret_cast<const char*>(value), size);\n"
"}\n");
}
}
diff --git a/src/google/protobuf/compiler/parser.cc b/src/google/protobuf/compiler/parser.cc
index 622895ff..3b73530b 100644
--- a/src/google/protobuf/compiler/parser.cc
+++ b/src/google/protobuf/compiler/parser.cc
@@ -1034,6 +1034,9 @@ bool Parser::ParseUserDefinedType(string* type_name) {
bool Parser::ParsePackage(FileDescriptorProto* file) {
if (file->has_package()) {
AddError("Multiple package definitions.");
+ // Don't append the new package to the old one. Just replace it. Not
+ // that it really matters since this is an error anyway.
+ file->clear_package();
}
DO(Consume("package"));
diff --git a/src/google/protobuf/descriptor.cc b/src/google/protobuf/descriptor.cc
index c75cf623..48db0cc2 100644
--- a/src/google/protobuf/descriptor.cc
+++ b/src/google/protobuf/descriptor.cc
@@ -1604,8 +1604,10 @@ class DescriptorBuilder {
// FindSymbol("foo.bar").
Symbol LookupSymbol(const string& name, const string& relative_to);
- // Calls tables_->AddSymbol() and records an error if it fails.
- void AddSymbol(const string& full_name,
+ // Calls tables_->AddSymbol() and records an error if it fails. Returns
+ // true if successful or false if failed, though most callers can ignore
+ // the return value since an error has already been recorded.
+ bool AddSymbol(const string& full_name,
const void* parent, const string& name,
const Message& proto, Symbol symbol);
@@ -1918,14 +1920,16 @@ Symbol DescriptorBuilder::LookupSymbol(
}
}
-void DescriptorBuilder::AddSymbol(
+bool DescriptorBuilder::AddSymbol(
const string& full_name, const void* parent, const string& name,
const Message& proto, Symbol symbol) {
// If the caller passed NULL for the parent, the symbol is at file scope.
// Use its file as the parent instead.
if (parent == NULL) parent = file_;
- if (!tables_->AddSymbol(full_name, parent, name, symbol)) {
+ if (tables_->AddSymbol(full_name, parent, name, symbol)) {
+ return true;
+ } else {
const FileDescriptor* other_file = tables_->FindSymbol(full_name).GetFile();
if (other_file == file_) {
string::size_type dot_pos = full_name.find_last_of('.');
@@ -1944,6 +1948,7 @@ void DescriptorBuilder::AddSymbol(
"\"" + full_name + "\" is already defined in file \"" +
other_file->name() + "\".");
}
+ return false;
}
}
@@ -2480,14 +2485,41 @@ void DescriptorBuilder::BuildEnumValue(const EnumValueDescriptorProto& proto,
// Again, enum values are weird because we makes them appear as siblings
// of the enum type instead of children of it. So, we use
// parent->containing_type() as the value's parent.
- AddSymbol(result->full_name(), parent->containing_type(), result->name(),
- proto, Symbol(result));
+ bool added_to_outer_scope =
+ AddSymbol(result->full_name(), parent->containing_type(), result->name(),
+ proto, Symbol(result));
// However, we also want to be able to search for values within a single
// enum type, so we add it as a child of the enum type itself, too.
// Note: This could fail, but if it does, the error has already been
// reported by the above AddSymbol() call, so we ignore the return code.
- tables_->AddAliasUnderParent(parent, result->name(), Symbol(result));
+ bool added_to_inner_scope =
+ tables_->AddAliasUnderParent(parent, result->name(), Symbol(result));
+
+ if (added_to_inner_scope && !added_to_outer_scope) {
+ // This value did not conflict with any values defined in the same enum,
+ // but it did conflict with some other symbol defined in the enum type's
+ // scope. Let's print an additional error to explain this.
+ string outer_scope;
+ if (parent->containing_type() == NULL) {
+ outer_scope = file_->package();
+ } else {
+ outer_scope = parent->containing_type()->full_name();
+ }
+
+ if (outer_scope.empty()) {
+ outer_scope = "the global scope";
+ } else {
+ outer_scope = "\"" + outer_scope + "\"";
+ }
+
+ AddError(result->full_name(), proto,
+ DescriptorPool::ErrorCollector::NAME,
+ "Note that enum values use C++ scoping rules, meaning that "
+ "enum values are siblings of their type, not children of it. "
+ "Therefore, \"" + result->name() + "\" must be unique within "
+ + outer_scope + ", not just within \"" + parent->name() + "\".");
+ }
// An enum is allowed to define two numbers that refer to the same value.
// FindValueByNumber() should return the first such value, so we simply
diff --git a/src/google/protobuf/descriptor.h b/src/google/protobuf/descriptor.h
index 2bba4c38..4203f496 100644
--- a/src/google/protobuf/descriptor.h
+++ b/src/google/protobuf/descriptor.h
@@ -128,6 +128,7 @@ class LIBPROTOBUF_EXPORT Descriptor {
// The number of fields in this message type.
int field_count() const;
// Gets a field by index, where 0 <= index < field_count().
+ // These are returned in the order they were defined in the .proto file.
const FieldDescriptor* field(int index) const;
// Looks up a field by declared tag number. Returns NULL if no such field
@@ -141,6 +142,7 @@ class LIBPROTOBUF_EXPORT Descriptor {
// The number of nested types in this message type.
int nested_type_count() const;
// Gets a nested type by index, where 0 <= index < nested_type_count().
+ // These are returned in the order they were defined in the .proto file.
const Descriptor* nested_type(int index) const;
// Looks up a nested type by name. Returns NULL if no such nested type
@@ -152,6 +154,7 @@ class LIBPROTOBUF_EXPORT Descriptor {
// The number of enum types in this message type.
int enum_type_count() const;
// Gets an enum type by index, where 0 <= index < enum_type_count().
+ // These are returned in the order they were defined in the .proto file.
const EnumDescriptor* enum_type(int index) const;
// Looks up an enum type by name. Returns NULL if no such enum type exists.
@@ -173,7 +176,8 @@ class LIBPROTOBUF_EXPORT Descriptor {
// The number of extension ranges in this message type.
int extension_range_count() const;
// Gets an extension range by index, where 0 <= index <
- // extension_range_count().
+ // extension_range_count(). These are returned in the order they were defined
+ // in the .proto file.
const ExtensionRange* extension_range(int index) const;
// Returns true if the number is in one of the extension ranges.
@@ -183,6 +187,7 @@ class LIBPROTOBUF_EXPORT Descriptor {
// defined nested within this message type's scope.
int extension_count() const;
// Get an extension by index, where 0 <= index < extension_count().
+ // These are returned in the order they were defined in the .proto file.
const FieldDescriptor* extension(int index) const;
// Looks up a named extension (which extends some *other* message type)
@@ -463,6 +468,7 @@ class LIBPROTOBUF_EXPORT EnumDescriptor {
// than zero.
int value_count() const;
// Gets a value by index, where 0 <= index < value_count().
+ // These are returned in the order they were defined in the .proto file.
const EnumValueDescriptor* value(int index) const;
// Looks up a value by name. Returns NULL if no such value exists.
@@ -583,6 +589,7 @@ class LIBPROTOBUF_EXPORT ServiceDescriptor {
// The number of methods this service defines.
int method_count() const;
// Gets a MethodDescriptor by index, where 0 <= index < method_count().
+ // These are returned in the order they were defined in the .proto file.
const MethodDescriptor* method(int index) const;
// Look up a MethodDescriptor by name.
@@ -683,29 +690,34 @@ class LIBPROTOBUF_EXPORT FileDescriptor {
// The number of files imported by this one.
int dependency_count() const;
// Gets an imported file by index, where 0 <= index < dependency_count().
+ // These are returned in the order they were defined in the .proto file.
const FileDescriptor* dependency(int index) const;
// Number of top-level message types defined in this file. (This does not
// include nested types.)
int message_type_count() const;
// Gets a top-level message type, where 0 <= index < message_type_count().
+ // These are returned in the order they were defined in the .proto file.
const Descriptor* message_type(int index) const;
// Number of top-level enum types defined in this file. (This does not
// include nested types.)
int enum_type_count() const;
// Gets a top-level enum type, where 0 <= index < enum_type_count().
+ // These are returned in the order they were defined in the .proto file.
const EnumDescriptor* enum_type(int index) const;
// Number of services defined in this file.
int service_count() const;
// Gets a service, where 0 <= index < service_count().
+ // These are returned in the order they were defined in the .proto file.
const ServiceDescriptor* service(int index) const;
// Number of extensions defined at file scope. (This does not include
// extensions nested within message types.)
int extension_count() const;
// Gets an extension's descriptor, where 0 <= index < extension_count().
+ // These are returned in the order they were defined in the .proto file.
const FieldDescriptor* extension(int index) const;
// Get options for this file. These are specified in the .proto
diff --git a/src/google/protobuf/descriptor.pb.h b/src/google/protobuf/descriptor.pb.h
index 27994dda..892f92d0 100644
--- a/src/google/protobuf/descriptor.pb.h
+++ b/src/google/protobuf/descriptor.pb.h
@@ -174,9 +174,9 @@ class LIBPROTOBUF_EXPORT FileDescriptorProto : public ::google::protobuf::Messag
inline const ::std::string& dependency(int index) const;
inline ::std::string* mutable_dependency(int index);
inline void set_dependency(int index, const ::std::string& value);
+ inline void set_dependency(int index, const char* value);
inline ::std::string* add_dependency();
inline void add_dependency(const ::std::string& value);
- inline void set_dependency(int index, const char* value);
inline void add_dependency(const char* value);
// repeated .google.protobuf.DescriptorProto message_type = 4;
@@ -1835,15 +1835,15 @@ inline ::std::string* FileDescriptorProto::mutable_dependency(int index) {
inline void FileDescriptorProto::set_dependency(int index, const ::std::string& value) {
dependency_.Mutable(index)->assign(value);
}
+inline void FileDescriptorProto::set_dependency(int index, const char* value) {
+ dependency_.Mutable(index)->assign(value);
+}
inline ::std::string* FileDescriptorProto::add_dependency() {
return dependency_.Add();
}
inline void FileDescriptorProto::add_dependency(const ::std::string& value) {
dependency_.Add()->assign(value);
}
-inline void FileDescriptorProto::set_dependency(int index, const char* value) {
- dependency_.Mutable(index)->assign(value);
-}
inline void FileDescriptorProto::add_dependency(const char* value) {
dependency_.Add()->assign(value);
}
diff --git a/src/google/protobuf/descriptor_unittest.cc b/src/google/protobuf/descriptor_unittest.cc
index 18397a66..25d6040d 100644
--- a/src/google/protobuf/descriptor_unittest.cc
+++ b/src/google/protobuf/descriptor_unittest.cc
@@ -1625,6 +1625,33 @@ TEST_F(ValidationErrorTest, PackageAlreadyDefined) {
"than a package) in file \"foo.proto\".\n");
}
+TEST_F(ValidationErrorTest, EnumValueAlreadyDefinedInParent) {
+ BuildFileWithErrors(
+ "name: \"foo.proto\" "
+ "enum_type { name: \"Foo\" value { name: \"FOO\" number: 1 } } "
+ "enum_type { name: \"Bar\" value { name: \"FOO\" number: 1 } } ",
+
+ "foo.proto: FOO: NAME: \"FOO\" is already defined.\n"
+ "foo.proto: FOO: NAME: Note that enum values use C++ scoping rules, "
+ "meaning that enum values are siblings of their type, not children of "
+ "it. Therefore, \"FOO\" must be unique within the global scope, not "
+ "just within \"Bar\".\n");
+}
+
+TEST_F(ValidationErrorTest, EnumValueAlreadyDefinedInParentNonGlobal) {
+ BuildFileWithErrors(
+ "name: \"foo.proto\" "
+ "package: \"pkg\" "
+ "enum_type { name: \"Foo\" value { name: \"FOO\" number: 1 } } "
+ "enum_type { name: \"Bar\" value { name: \"FOO\" number: 1 } } ",
+
+ "foo.proto: pkg.FOO: NAME: \"FOO\" is already defined in \"pkg\".\n"
+ "foo.proto: pkg.FOO: NAME: Note that enum values use C++ scoping rules, "
+ "meaning that enum values are siblings of their type, not children of "
+ "it. Therefore, \"FOO\" must be unique within \"pkg\", not just within "
+ "\"Bar\".\n");
+}
+
TEST_F(ValidationErrorTest, MissingName) {
BuildFileWithErrors(
"name: \"foo.proto\" "
diff --git a/src/google/protobuf/io/tokenizer_unittest.cc b/src/google/protobuf/io/tokenizer_unittest.cc
index e2cede7e..2171fcc3 100644
--- a/src/google/protobuf/io/tokenizer_unittest.cc
+++ b/src/google/protobuf/io/tokenizer_unittest.cc
@@ -477,22 +477,22 @@ TEST_F(TokenizerTest, ParseInteger) {
// Test invalid integers that may still be tokenized as integers.
EXPECT_EQ(0, ParseInteger("0x"));
+ uint64 i;
#ifdef GTEST_HAS_DEATH_TEST // death tests do not work on Windows yet
// Test invalid integers that will never be tokenized as integers.
- EXPECT_DEBUG_DEATH(ParseInteger("zxy"),
+ EXPECT_DEBUG_DEATH(Tokenizer::ParseInteger("zxy", kuint64max, &i),
"passed text that could not have been tokenized as an integer");
- EXPECT_DEBUG_DEATH(ParseInteger("1.2"),
+ EXPECT_DEBUG_DEATH(Tokenizer::ParseInteger("1.2", kuint64max, &i),
"passed text that could not have been tokenized as an integer");
- EXPECT_DEBUG_DEATH(ParseInteger("08"),
+ EXPECT_DEBUG_DEATH(Tokenizer::ParseInteger("08", kuint64max, &i),
"passed text that could not have been tokenized as an integer");
- EXPECT_DEBUG_DEATH(ParseInteger("0xg"),
+ EXPECT_DEBUG_DEATH(Tokenizer::ParseInteger("0xg", kuint64max, &i),
"passed text that could not have been tokenized as an integer");
- EXPECT_DEBUG_DEATH(ParseInteger("-1"),
+ EXPECT_DEBUG_DEATH(Tokenizer::ParseInteger("-1", kuint64max, &i),
"passed text that could not have been tokenized as an integer");
#endif // GTEST_HAS_DEATH_TEST
// Test overflows.
- uint64 i;
EXPECT_TRUE (Tokenizer::ParseInteger("0", 0, &i));
EXPECT_FALSE(Tokenizer::ParseInteger("1", 0, &i));
EXPECT_TRUE (Tokenizer::ParseInteger("1", 1, &i));