Skip to content

Commit

Permalink
Migrate std::operator+ to Abseil helpers in Objective-c compiler dire…
Browse files Browse the repository at this point in the history
…ctory.

This also opportunistically migrates many C-style and STL strings to string_view in touched code.

PiperOrigin-RevId: 502774859
  • Loading branch information
mkruskal-google authored and copybara-github committed Jan 18, 2023
1 parent 5184022 commit 324f0b5
Show file tree
Hide file tree
Showing 16 changed files with 192 additions and 182 deletions.
6 changes: 4 additions & 2 deletions src/google/protobuf/compiler/objectivec/enum_field.cc
Original file line number Diff line number Diff line change
Expand Up @@ -147,8 +147,10 @@ RepeatedEnumFieldGenerator::RepeatedEnumFieldGenerator(

void RepeatedEnumFieldGenerator::FinishInitialization() {
RepeatedFieldGenerator::FinishInitialization();
variables_["array_comment"] = "// |" + variables_["name"] + "| contains |" +
variables_["storage_type"] + "|\n";
std::string name = variables_["name"];
std::string storage_type = variables_["storage_type"];
variables_["array_comment"] =
absl::StrCat("// |", name, "| contains |", storage_type, "|\n");
}

// NOTE: RepeatedEnumFieldGenerator::DetermineForwardDeclarations isn't needed
Expand Down
9 changes: 5 additions & 4 deletions src/google/protobuf/compiler/objectivec/extension.cc
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,11 @@ namespace protobuf {
namespace compiler {
namespace objectivec {

ExtensionGenerator::ExtensionGenerator(const std::string& root_class_name,
ExtensionGenerator::ExtensionGenerator(absl::string_view root_class_name,
const FieldDescriptor* descriptor)
: method_name_(ExtensionMethodName(descriptor)),
root_class_and_method_name_(root_class_name + "_" + method_name_),
root_class_and_method_name_(
absl::StrCat(root_class_name, "_", method_name_)),
descriptor_(descriptor) {
if (descriptor->is_map()) {
// NOTE: src/google/protobuf/compiler/plugin.cc makes use of cerr for some
Expand Down Expand Up @@ -120,11 +121,11 @@ void ExtensionGenerator::GenerateStaticVariablesInitialization(
vars["default"] = DefaultValue(descriptor_);
}
std::string type = GetCapitalizedType(descriptor_);
vars["extension_type"] = std::string("GPBDataType") + type;
vars["extension_type"] = absl::StrCat("GPBDataType", type);

if (objc_type == OBJECTIVECTYPE_ENUM) {
vars["enum_desc_func_name"] =
EnumName(descriptor_->enum_type()) + "_EnumDescriptor";
absl::StrCat(EnumName(descriptor_->enum_type()), "_EnumDescriptor");
} else {
vars["enum_desc_func_name"] = "NULL";
}
Expand Down
2 changes: 1 addition & 1 deletion src/google/protobuf/compiler/objectivec/extension.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ namespace objectivec {

class ExtensionGenerator {
public:
ExtensionGenerator(const std::string& root_class_name,
ExtensionGenerator(absl::string_view root_class_name,
const FieldDescriptor* descriptor);
~ExtensionGenerator() = default;

Expand Down
6 changes: 3 additions & 3 deletions src/google/protobuf/compiler/objectivec/field.cc
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ void SetCommonFieldVariables(
(*variables)["capitalized_name"] = capitalized_name;
(*variables)["raw_field_name"] = raw_field_name;
(*variables)["field_number_name"] =
classname + "_FieldNumber_" + capitalized_name;
absl::StrCat(classname, "_FieldNumber_", capitalized_name);
(*variables)["field_number"] = absl::StrCat(descriptor->number());
(*variables)["field_type"] = GetCapitalizedType(descriptor);
(*variables)["deprecated_attribute"] =
Expand Down Expand Up @@ -120,8 +120,8 @@ void SetCommonFieldVariables(
(*variables)["dataTypeSpecific_name"] = "clazz";
(*variables)["dataTypeSpecific_value"] = "Nil";

(*variables)["storage_offset_value"] = "(uint32_t)offsetof(" + classname +
"__storage_, " + camel_case_name + ")";
(*variables)["storage_offset_value"] = absl::StrCat(
"(uint32_t)offsetof(", classname, "__storage_, ", camel_case_name, ")");
(*variables)["storage_offset_comment"] = "";

// Clear some common things so they can be set just when needed.
Expand Down
32 changes: 19 additions & 13 deletions src/google/protobuf/compiler/objectivec/generator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -145,8 +145,9 @@ bool ObjectiveCGenerator::GenerateAll(
// Default is "no".
if (!StringToBool(options[i].second,
&validation_options.prefixes_must_be_registered)) {
*error = "error: Unknown value for prefixes_must_be_registered: " +
options[i].second;
*error = absl::StrCat(
"error: Unknown value for prefixes_must_be_registered: ",
options[i].second);
return false;
}
} else if (options[i].first == "require_prefixes") {
Expand All @@ -158,8 +159,8 @@ bool ObjectiveCGenerator::GenerateAll(
// Default is "no".
if (!StringToBool(options[i].second,
&validation_options.require_prefixes)) {
*error =
"error: Unknown value for require_prefixes: " + options[i].second;
*error = absl::StrCat("error: Unknown value for require_prefixes: ",
options[i].second);
return false;
}
} else if (options[i].first == "generate_for_named_framework") {
Expand Down Expand Up @@ -229,7 +230,8 @@ bool ObjectiveCGenerator::GenerateAll(
if (StringToBool(options[i].second, &value)) {
SetUseProtoPackageAsDefaultPrefix(value);
} else {
*error = "error: Unknown use_package_as_prefix: " + options[i].second;
*error = absl::StrCat("error: Unknown use_package_as_prefix: ",
options[i].second);
return false;
}
} else if (options[i].first == "proto_package_prefix_exceptions_path") {
Expand All @@ -251,8 +253,9 @@ bool ObjectiveCGenerator::GenerateAll(
} else if (options[i].first == "headers_use_forward_declarations") {
if (!StringToBool(options[i].second,
&generation_options.headers_use_forward_declarations)) {
*error = "error: Unknown value for headers_use_forward_declarations: " +
options[i].second;
*error = absl::StrCat(
"error: Unknown value for headers_use_forward_declarations: ",
options[i].second);
return false;
}
} else if (options[i].first == "experimental_multi_source_generation") {
Expand All @@ -267,13 +270,14 @@ bool ObjectiveCGenerator::GenerateAll(
if (!StringToBool(
options[i].second,
&generation_options.experimental_multi_source_generation)) {
*error =
"error: Unknown value for experimental_multi_source_generation: " +
options[i].second;
*error = absl::StrCat(
"error: Unknown value for experimental_multi_source_generation: ",
options[i].second);
return false;
}
} else {
*error = "error: Unknown generator option: " + options[i].first;
*error =
absl::StrCat("error: Unknown generator option: ", options[i].first);
return false;
}
}
Expand Down Expand Up @@ -312,7 +316,8 @@ bool ObjectiveCGenerator::GenerateAll(

// Generate header.
{
auto output = absl::WrapUnique(context->Open(filepath + ".pbobjc.h"));
auto output =
absl::WrapUnique(context->Open(absl::StrCat(filepath, ".pbobjc.h")));
io::Printer printer(output.get());
file_generator.GenerateHeader(&printer);
if (printer.failed()) {
Expand Down Expand Up @@ -367,7 +372,8 @@ bool ObjectiveCGenerator::GenerateAll(
}
}
} else {
auto output = absl::WrapUnique(context->Open(filepath + ".pbobjc.m"));
auto output = absl::WrapUnique(
context->Open(absl::StrCat(filepath, ".pbobjc.m")));
io::Printer printer(output.get());
file_generator.GenerateSource(&printer);
if (printer.failed()) {
Expand Down
47 changes: 25 additions & 22 deletions src/google/protobuf/compiler/objectivec/helpers.cc
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
#include "absl/strings/match.h"
#include "absl/strings/str_replace.h"
#include "absl/strings/str_split.h"
#include "absl/strings/string_view.h"
#include "google/protobuf/compiler/objectivec/names.h"
#include "google/protobuf/io/strtod.h"
#include "google/protobuf/stubs/common.h"
Expand Down Expand Up @@ -97,7 +98,7 @@ std::string HandleExtremeFloatingPoint(std::string val, bool add_float_suffix) {
if (add_float_suffix &&
(absl::StrContains(val, '.') || absl::StrContains(val, 'e') ||
absl::StrContains(val, 'E'))) {
val += "f";
return absl::StrCat(val, "f");
}
return val;
}
Expand Down Expand Up @@ -255,15 +256,15 @@ std::string DefaultValue(const FieldDescriptor* field) {
}
return absl::StrCat(field->default_value_int32());
case FieldDescriptor::CPPTYPE_UINT32:
return absl::StrCat(field->default_value_uint32()) + "U";
return absl::StrCat(field->default_value_uint32(), "U");
case FieldDescriptor::CPPTYPE_INT64:
// gcc and llvm reject the decimal form of kint32min and kint64min.
if (field->default_value_int64() == LLONG_MIN) {
return "-0x8000000000000000LL";
}
return absl::StrCat(field->default_value_int64()) + "LL";
return absl::StrCat(field->default_value_int64(), "LL");
case FieldDescriptor::CPPTYPE_UINT64:
return absl::StrCat(field->default_value_uint64()) + "ULL";
return absl::StrCat(field->default_value_uint64(), "ULL");
case FieldDescriptor::CPPTYPE_DOUBLE:
return HandleExtremeFloatingPoint(
io::SimpleDtoa(field->default_value_double()), false);
Expand All @@ -274,7 +275,7 @@ std::string DefaultValue(const FieldDescriptor* field) {
return field->default_value_bool() ? "YES" : "NO";
case FieldDescriptor::CPPTYPE_STRING: {
const bool has_default_value = field->has_default_value();
const std::string& default_string = field->default_value_string();
absl::string_view default_string = field->default_value_string();
if (!has_default_value || default_string.length() == 0) {
// If the field is defined as being the empty string,
// then we will just assign to nil, as the empty string is the
Expand All @@ -292,10 +293,12 @@ std::string DefaultValue(const FieldDescriptor* field) {
// a cstring.
uint32_t length = ghtonl(default_string.length());
std::string bytes((const char*)&length, sizeof(length));
bytes.append(default_string);
return "(NSData*)\"" + EscapeTrigraphs(absl::CEscape(bytes)) + "\"";
absl::StrAppend(&bytes, default_string);
return absl::StrCat("(NSData*)\"",
EscapeTrigraphs(absl::CEscape(bytes)), "\"");
} else {
return "@\"" + EscapeTrigraphs(absl::CEscape(default_string)) + "\"";
return absl::StrCat(
"@\"", EscapeTrigraphs(absl::CEscape(default_string)), "\"");
}
}
case FieldDescriptor::CPPTYPE_ENUM:
Expand All @@ -317,7 +320,8 @@ std::string BuildFlagsString(FlagType flag_type,
} else if (strings.size() == 1) {
return strings[0];
}
std::string string("(" + GetEnumNameForFlagType(flag_type) + ")(");
std::string string =
absl::StrCat("(", GetEnumNameForFlagType(flag_type), ")(");
for (size_t i = 0; i != strings.size(); ++i) {
if (i > 0) {
string.append(" | ");
Expand All @@ -328,20 +332,20 @@ std::string BuildFlagsString(FlagType flag_type,
return string;
}

std::string ObjCClass(const std::string& class_name) {
return std::string("GPBObjCClass(") + class_name + ")";
std::string ObjCClass(absl::string_view class_name) {
return absl::StrCat("GPBObjCClass(", class_name, ")");
}

std::string ObjCClassDeclaration(const std::string& class_name) {
return std::string("GPBObjCClassDeclaration(") + class_name + ");";
std::string ObjCClassDeclaration(absl::string_view class_name) {
return absl::StrCat("GPBObjCClassDeclaration(", class_name, ");");
}

std::string BuildCommentsString(const SourceLocation& location,
bool prefer_single_line) {
const std::string& comments = location.leading_comments.empty()
? location.trailing_comments
: location.leading_comments;
std::vector<std::string> lines;
absl::string_view comments = location.leading_comments.empty()
? location.trailing_comments
: location.leading_comments;
std::vector<absl::string_view> lines;
lines = absl::StrSplit(comments, '\n', absl::AllowEmpty());
while (!lines.empty() && lines.back().empty()) {
lines.pop_back();
Expand All @@ -364,7 +368,7 @@ std::string BuildCommentsString(const SourceLocation& location,
} else {
prefix = "* ";
suffix = "\n";
final_comments += "/**\n";
absl::StrAppend(&final_comments, "/**\n");
epilogue = " **/\n";
add_leading_space = true;
}
Expand All @@ -382,11 +386,10 @@ std::string BuildCommentsString(const SourceLocation& location,
absl::StripAsciiWhitespace(&line);
// If not a one line, need to add the first space before *, as
// absl::StripAsciiWhitespace would have removed it.
line = (add_leading_space ? " " : "") + line;
final_comments += line + suffix;
line = absl::StrCat(add_leading_space ? " " : "", line);
absl::StrAppend(&final_comments, line, suffix);
}
final_comments += epilogue;
return final_comments;
return absl::StrCat(final_comments, epilogue);
}

} // namespace objectivec
Expand Down
20 changes: 7 additions & 13 deletions src/google/protobuf/compiler/objectivec/helpers.h
Original file line number Diff line number Diff line change
Expand Up @@ -105,11 +105,11 @@ std::string BuildFlagsString(FlagType type,

// Returns a symbol that can be used in C code to refer to an Objective C
// class without initializing the class.
std::string ObjCClass(const std::string& class_name);
std::string ObjCClass(absl::string_view class_name);

// Declares an Objective C class without initializing the class so that it can
// be refrerred to by ObjCClass.
std::string ObjCClassDeclaration(const std::string& class_name);
std::string ObjCClassDeclaration(absl::string_view class_name);

// Builds HeaderDoc/appledoc style comments out of the comments in the .proto
// file.
Expand All @@ -134,20 +134,14 @@ std::string GetOptionalDeprecatedAttribute(const TDescriptor* descriptor,
std::string message;
const FileDescriptor* sourceFile = descriptor->file();
if (isFileLevelDeprecation) {
message = sourceFile->name() + " is deprecated.";
message = absl::StrCat(sourceFile->name(), " is deprecated.");
} else {
message = descriptor->full_name() + " is deprecated (see " +
sourceFile->name() + ").";
message = absl::StrCat(descriptor->full_name(), " is deprecated (see ",
sourceFile->name(), ").");
}

std::string result = std::string("GPB_DEPRECATED_MSG(\"") + message + "\")";
if (preSpace) {
result.insert(0, " ");
}
if (postNewline) {
result.append("\n");
}
return result;
return absl::StrCat(preSpace ? " " : "", "GPB_DEPRECATED_MSG(\"", message,
"\")", postNewline ? "\n" : "");
} else {
return "";
}
Expand Down
15 changes: 7 additions & 8 deletions src/google/protobuf/compiler/objectivec/import_writer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -134,9 +134,8 @@ void ImportWriter::AddFile(const FileDescriptor* file,
// in other cases, they get skipped because the generated code already
// import GPBProtocolBuffers.h and hence proves them.
if (for_bundled_proto_) {
const std::string header_name =
"GPB" + FilePathBasename(file) + header_extension;
protobuf_imports_.push_back(header_name);
protobuf_imports_.emplace_back(
absl::StrCat("GPB", FilePathBasename(file), header_extension));
}
return;
}
Expand All @@ -148,15 +147,15 @@ void ImportWriter::AddFile(const FileDescriptor* file,

auto proto_lookup = proto_file_to_framework_name_.find(file->name());
if (proto_lookup != proto_file_to_framework_name_.end()) {
other_framework_imports_.push_back(
proto_lookup->second + "/" + FilePathBasename(file) + header_extension);
other_framework_imports_.emplace_back(absl::StrCat(
proto_lookup->second, "/", FilePathBasename(file), header_extension));
return;
}

if (!generate_for_named_framework_.empty()) {
other_framework_imports_.push_back(generate_for_named_framework_ + "/" +
FilePathBasename(file) +
header_extension);
other_framework_imports_.push_back(
absl::StrCat(generate_for_named_framework_, "/", FilePathBasename(file),
header_extension));
return;
}

Expand Down
6 changes: 3 additions & 3 deletions src/google/protobuf/compiler/objectivec/line_consumer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -161,11 +161,11 @@ bool Parser::Finish(std::string* out_error) {

} // namespace

bool ParseSimpleFile(const std::string& path, LineConsumer* line_consumer,
bool ParseSimpleFile(absl::string_view path, LineConsumer* line_consumer,
std::string* out_error) {
int fd;
do {
fd = posix::open(path.c_str(), O_RDONLY);
fd = posix::open(std::string(path).c_str(), O_RDONLY);
} while (fd < 0 && errno == EINTR);
if (fd < 0) {
*out_error =
Expand All @@ -179,7 +179,7 @@ bool ParseSimpleFile(const std::string& path, LineConsumer* line_consumer,
}

bool ParseSimpleStream(io::ZeroCopyInputStream& input_stream,
const std::string& stream_name,
absl::string_view stream_name,
LineConsumer* line_consumer, std::string* out_error) {
std::string local_error;
Parser parser(line_consumer);
Expand Down
4 changes: 2 additions & 2 deletions src/google/protobuf/compiler/objectivec/line_consumer.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,12 @@ class PROTOC_EXPORT LineConsumer {
virtual bool ConsumeLine(absl::string_view line, std::string* out_error) = 0;
};

bool PROTOC_EXPORT ParseSimpleFile(const std::string& path,
bool PROTOC_EXPORT ParseSimpleFile(absl::string_view path,
LineConsumer* line_consumer,
std::string* out_error);

bool PROTOC_EXPORT ParseSimpleStream(io::ZeroCopyInputStream& input_stream,
const std::string& stream_name,
absl::string_view stream_name,
LineConsumer* line_consumer,
std::string* out_error);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ class TestLineCollector : public LineConsumer {
bool ConsumeLine(absl::string_view line, std::string* out_error) override {
if (reject_ && *reject_ == line) {
if (!skip_msg_) {
*out_error = std::string("Rejected '") + *reject_ + "'";
*out_error = absl::StrCat("Rejected '", *reject_, "'");
}
return false;
}
Expand Down
Loading

0 comments on commit 324f0b5

Please sign in to comment.