Skip to content

Commit

Permalink
protoc: support inf, -inf, nan, and -nan in option values (#15017)
Browse files Browse the repository at this point in the history 
The parser already supports these values for the special "default" field option. But, inconsistently, does not support them for other options whose type is float or double. This addresses that inconsistency.

Fixes #15010.

Closes #15017

COPYBARA_INTEGRATE_REVIEW=#15017 from jhump:jh/inf-and-nan-in-option-values 1f8217c
PiperOrigin-RevId: 627399259
  • Loading branch information
@jhump @copybara-github
jhump authored and copybara-github committed Apr 23, 2024
1 parent 8f2da78 commit 3c03e93
Show file tree
Hide file tree
Showing 4 changed files with 146 additions and 4 deletions.
17 changes: 13 additions & 4 deletions src/google/protobuf/compiler/parser.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1623,12 +1623,21 @@ bool Parser::ParseOption(Message* options,
case io::Tokenizer::TYPE_IDENTIFIER: {
value_location.AddPath(
UninterpretedOption::kIdentifierValueFieldNumber);
if (is_negative) {
RecordError("Invalid '-' symbol before identifier.");
return false;
}
std::string value;
DO(ConsumeIdentifier(&value, "Expected identifier."));
if (is_negative) {
if (value == "inf") {
uninterpreted_option->set_double_value(
-std::numeric_limits<double>::infinity());
} else if (value == "nan") {
uninterpreted_option->set_double_value(
std::numeric_limits<double>::quiet_NaN());
} else {
RecordError("Identifier after '-' symbol must be inf or nan.");
return false;
}
break;
}
uninterpreted_option->set_identifier_value(value);
break;
}
Expand Down
105 changes: 105 additions & 0 deletions src/google/protobuf/compiler/parser_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "google/protobuf/compiler/parser.h"

#include <algorithm>
#include <cmath>
#include <memory>
#include <string>
#include <utility>
Expand Down Expand Up @@ -460,6 +461,7 @@ TEST_F(ParseMessageTest, FieldDefaults) {
" required double foo = 1 [default= inf ];\n"
" required double foo = 1 [default=-inf ];\n"
" required double foo = 1 [default= nan ];\n"
" required double foo = 1 [default= -nan ];\n"
" required string foo = 1 [default='13\\001'];\n"
" required string foo = 1 [default='a' \"b\" \n \"c\"];\n"
" required bytes foo = 1 [default='14\\002'];\n"
Expand Down Expand Up @@ -509,6 +511,8 @@ TEST_F(ParseMessageTest, FieldDefaults) {
" }"
" field { type:TYPE_DOUBLE default_value:\"nan\" " ETC
" }"
" field { type:TYPE_DOUBLE default_value:\"-nan\" " ETC
" }"
" field { type:TYPE_STRING default_value:\"13\\001\" " ETC
" }"
" field { type:TYPE_STRING default_value:\"abc\" " ETC
Expand Down Expand Up @@ -658,6 +662,65 @@ TEST_F(ParseMessageTest, FieldOptionsSupportLargeDecimalLiteral) {
"}");
}

TEST_F(ParseMessageTest, FieldOptionsSupportInfAndNan) {
ExpectParsesTo(
"import \"google/protobuf/descriptor.proto\";\n"
"extend google.protobuf.FieldOptions {\n"
" optional double f = 10101;\n"
"}\n"
"message TestMessage {\n"
" optional double a = 1 [(f) = inf];\n"
" optional double b = 2 [(f) = -inf];\n"
" optional double c = 3 [(f) = nan];\n"
" optional double d = 4 [(f) = -nan];\n"
"}\n",

"dependency: \"google/protobuf/descriptor.proto\""
"extension {"
" name: \"f\" label: LABEL_OPTIONAL type: TYPE_DOUBLE number: 10101"
" extendee: \"google.protobuf.FieldOptions\""
"}"
"message_type {"
" name: \"TestMessage\""
" field {"
" name: \"a\" label: LABEL_OPTIONAL type: TYPE_DOUBLE number: 1"
" options{"
" uninterpreted_option{"
" name{ name_part: \"f\" is_extension: true }"
" identifier_value: \"inf\""
" }"
" }"
" }"
" field {"
" name: \"b\" label: LABEL_OPTIONAL type: TYPE_DOUBLE number: 2"
" options{"
" uninterpreted_option{"
" name{ name_part: \"f\" is_extension: true }"
" double_value: -infinity"
" }"
" }"
" }"
" field {"
" name: \"c\" label: LABEL_OPTIONAL type: TYPE_DOUBLE number: 3"
" options{"
" uninterpreted_option{"
" name{ name_part: \"f\" is_extension: true }"
" identifier_value: \"nan\""
" }"
" }"
" }"
" field {"
" name: \"d\" label: LABEL_OPTIONAL type: TYPE_DOUBLE number: 4"
" options{"
" uninterpreted_option{"
" name{ name_part: \"f\" is_extension: true }"
" double_value: nan"
" }"
" }"
" }"
"}");
}

TEST_F(ParseMessageTest, Oneof) {
ExpectParsesTo(
"message TestMessage {\n"
Expand Down Expand Up @@ -1396,6 +1459,48 @@ TEST_F(ParseMiscTest, ParseFileOptions) {
"}");
}

TEST_F(ParseMiscTest, InterpretedOptions) {
// Since we're importing the generated code from parsing/compiling
// unittest_custom_options.proto, we can just look at the option
// values from that file's descriptor in the generated code.
{
const MessageOptions& options =
protobuf_unittest::SettingRealsFromInf ::descriptor()->options();
float float_val = options.GetExtension(protobuf_unittest::float_opt);
ASSERT_TRUE(std::isinf(float_val));
ASSERT_GT(float_val, 0);
double double_val = options.GetExtension(protobuf_unittest::double_opt);
ASSERT_TRUE(std::isinf(double_val));
ASSERT_GT(double_val, 0);
}
{
const MessageOptions& options =
protobuf_unittest::SettingRealsFromNegativeInf ::descriptor()->options();
float float_val = options.GetExtension(protobuf_unittest::float_opt);
ASSERT_TRUE(std::isinf(float_val));
ASSERT_LT(float_val, 0);
double double_val = options.GetExtension(protobuf_unittest::double_opt);
ASSERT_TRUE(std::isinf(double_val));
ASSERT_LT(double_val, 0);
}
{
const MessageOptions& options =
protobuf_unittest::SettingRealsFromNan ::descriptor()->options();
float float_val = options.GetExtension(protobuf_unittest::float_opt);
ASSERT_TRUE(std::isnan(float_val));
double double_val = options.GetExtension(protobuf_unittest::double_opt);
ASSERT_TRUE(std::isnan(double_val));
}
{
const MessageOptions& options =
protobuf_unittest::SettingRealsFromNegativeNan ::descriptor()->options();
float float_val = options.GetExtension(protobuf_unittest::float_opt);
ASSERT_TRUE(std::isnan(float_val));
double double_val = options.GetExtension(protobuf_unittest::double_opt);
ASSERT_TRUE(std::isnan(double_val));
}
}

// ===================================================================
// Error tests
//
Expand Down
8 changes: 8 additions & 0 deletions src/google/protobuf/descriptor.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9149,6 +9149,10 @@ bool DescriptorBuilder::OptionInterpreter::SetOptionValue(
value = uninterpreted_option_->positive_int_value();
} else if (uninterpreted_option_->has_negative_int_value()) {
value = uninterpreted_option_->negative_int_value();
} else if (uninterpreted_option_->identifier_value() == "inf") {
value = std::numeric_limits<float>::infinity();
} else if (uninterpreted_option_->identifier_value() == "nan") {
value = std::numeric_limits<float>::quiet_NaN();
} else {
return AddValueError([&] {
return absl::StrCat("Value must be number for float option \"",
Expand All @@ -9168,6 +9172,10 @@ bool DescriptorBuilder::OptionInterpreter::SetOptionValue(
value = uninterpreted_option_->positive_int_value();
} else if (uninterpreted_option_->has_negative_int_value()) {
value = uninterpreted_option_->negative_int_value();
} else if (uninterpreted_option_->identifier_value() == "inf") {
value = std::numeric_limits<double>::infinity();
} else if (uninterpreted_option_->identifier_value() == "nan") {
value = std::numeric_limits<double>::quiet_NaN();
} else {
return AddValueError([&] {
return absl::StrCat("Value must be number for double option \"",
Expand Down
20 changes: 20 additions & 0 deletions src/google/protobuf/unittest_custom_options.proto
View file
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,26 @@ message SettingRealsFromNegativeInts {
option (double_opt) = -154;
}

message SettingRealsFromInf {
option (float_opt) = inf;
option (double_opt) = inf;
}

message SettingRealsFromNegativeInf {
option (float_opt) = -inf;
option (double_opt) = -inf;
}

message SettingRealsFromNan {
option (float_opt) = nan;
option (double_opt) = nan;
}

message SettingRealsFromNegativeNan {
option (float_opt) = -nan;
option (double_opt) = -nan;
}

// Options of complex message types, themselves combined and extended in
// various ways.

Expand Down

0 comments on commit 3c03e93

Please sign in to comment.