Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove trivial methods from the codebase #8498

Closed
wants to merge 1 commit into from
Closed

Remove trivial methods from the codebase #8498

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
21 changes: 4 additions & 17 deletions src/google/protobuf/compiler/cpp/cpp_helpers.cc
View file
Open in desktop
Expand Up @@ -1011,29 +1011,16 @@ bool IsWellKnownMessage(const FileDescriptor* file) {
return well_known_files.find(file->name()) != well_known_files.end();
}

static bool FieldEnforceUtf8(const FieldDescriptor* field,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry but unfortunately we need to keep these functions around. Within our internal version of the codebase we have some additional code in these functions for legacy reasons, to allow the UTF-8 enforcement to be disabled for particular fields. Even though the functions look pointless, it helps us to have them around so that we can easily apply our internal changes.

If you just want to fix an unused-parameter warning, it would be fine to remove the parameter names (i.e. just make it (const FieldDescriptor*, const Options&)) or add lines like (void) field;.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not working on unused parameter in protoc itself, but rather in the generated code.

It is ok if these methods would not be removed, though this will make interfaces a bit clumsier.

Could you, please, take a look on the question below?

const Options& options) {
return true;
}

static bool FileUtf8Verification(const FileDescriptor* file,
const Options& options) {
return true;
}

// Which level of UTF-8 enforcemant is placed on this file.
Utf8CheckMode GetUtf8CheckMode(const FieldDescriptor* field,
const Options& options) {
if (field->file()->syntax() == FileDescriptor::SYNTAX_PROTO3 &&
FieldEnforceUtf8(field, options)) {
if (field->file()->syntax() == FileDescriptor::SYNTAX_PROTO3) {
return Utf8CheckMode::kStrict;
} else if (GetOptimizeFor(field->file(), options) !=
FileOptions::LITE_RUNTIME &&
FileUtf8Verification(field->file(), options)) {
}
if (GetOptimizeFor(field->file(), options) != FileOptions::LITE_RUNTIME) {
return Utf8CheckMode::kVerify;
} else {
return Utf8CheckMode::kNone;
}
return Utf8CheckMode::kNone;
}

static void GenerateUtf8CheckCode(const FieldDescriptor* field,
Expand Down