From 3c03e9351c57081d0dffae120ed37497017f105c Mon Sep 17 00:00:00 2001 From: Joshua Humphries <2035234+jhump@users.noreply.github.com> Date: Tue, 23 Apr 2024 08:38:54 -0700 Subject: [PATCH] protoc: support inf, -inf, nan, and -nan in option values (#15017) 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=https://github.com/protocolbuffers/protobuf/pull/15017 from jhump:jh/inf-and-nan-in-option-values 1f8217c4bd0706b145a716c3db4a026b04f5a7ba PiperOrigin-RevId: 627399259 --- src/google/protobuf/compiler/parser.cc | 17 ++- .../protobuf/compiler/parser_unittest.cc | 105 ++++++++++++++++++ src/google/protobuf/descriptor.cc | 8 ++ .../protobuf/unittest_custom_options.proto | 20 ++++ 4 files changed, 146 insertions(+), 4 deletions(-) diff --git a/src/google/protobuf/compiler/parser.cc b/src/google/protobuf/compiler/parser.cc index f6f7d1d50975..129ea2f3f2fb 100644 --- a/src/google/protobuf/compiler/parser.cc +++ b/src/google/protobuf/compiler/parser.cc @@ -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::infinity()); + } else if (value == "nan") { + uninterpreted_option->set_double_value( + std::numeric_limits::quiet_NaN()); + } else { + RecordError("Identifier after '-' symbol must be inf or nan."); + return false; + } + break; + } uninterpreted_option->set_identifier_value(value); break; } diff --git a/src/google/protobuf/compiler/parser_unittest.cc b/src/google/protobuf/compiler/parser_unittest.cc index 46c1e2e5ff87..f9b8f3231765 100644 --- a/src/google/protobuf/compiler/parser_unittest.cc +++ b/src/google/protobuf/compiler/parser_unittest.cc @@ -12,6 +12,7 @@ #include "google/protobuf/compiler/parser.h" #include +#include #include #include #include @@ -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" @@ -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 @@ -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" @@ -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 // diff --git a/src/google/protobuf/descriptor.cc b/src/google/protobuf/descriptor.cc index db45ca90ca24..50d5a0a7754b 100644 --- a/src/google/protobuf/descriptor.cc +++ b/src/google/protobuf/descriptor.cc @@ -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::infinity(); + } else if (uninterpreted_option_->identifier_value() == "nan") { + value = std::numeric_limits::quiet_NaN(); } else { return AddValueError([&] { return absl::StrCat("Value must be number for float option \"", @@ -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::infinity(); + } else if (uninterpreted_option_->identifier_value() == "nan") { + value = std::numeric_limits::quiet_NaN(); } else { return AddValueError([&] { return absl::StrCat("Value must be number for double option \"", diff --git a/src/google/protobuf/unittest_custom_options.proto b/src/google/protobuf/unittest_custom_options.proto index ca6bec7a60c1..7075012778c5 100644 --- a/src/google/protobuf/unittest_custom_options.proto +++ b/src/google/protobuf/unittest_custom_options.proto @@ -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.