protobuf icon indicating copy to clipboard operation
protobuf copied to clipboard

Deprecating 'enum' field inside a 'message' causes compiler warning 'deprecated-declarations' in generated C++ header

Open patrickdepinguin opened this issue 1 year ago • 2 comments

What version of protobuf and what language are you using? Version: 3.21.12, but same occurs on latest 28.0 Language: C++

What operating system (Linux, Windows, ...) and version? Linux (e.g. Debian 12 bookworm)

What runtime / compiler are you using (e.g., python version or gcc version) gcc version 12.2.0 (Debian 12.2.0-14)

What did you do? Using following foobar.proto file:

syntax = "proto3";

message Result {
  enum Status {
    UNSET = 0;
    OK = 1 [deprecated = true];
    FAILED = 2;
  }
}

Generate C++ files with: protoc --cpp_out . foobar.proto Compile generated source file: g++ -c foobar.pb.cc

What did you expect to see

Compilation without any warnings.

What did you see instead?

In file included from foobar.pb.cc:4:
foobar.pb.h:191:5: warning: ‘Result_Status_OK’ is deprecated [-Wdeprecated-declarations]
  191 |     Result_Status_OK;
      |     ^~~~~~~~~~~~~~~~
foobar.pb.h:58:3: note: declared here
   58 |   Result_Status_OK PROTOBUF_DEPRECATED_ENUM = 1,
      |   ^~~~~~~~~~~~~~~~

In the generated foobar.pb.h header, the declaration of the enum is fine: (PROTOBUF_DEPRECATED_ENUM becomes 'attribute((deprecated))', and in latest 28.0 the code just generates [[deprecated]] )

enum Result_Status : int {
  Result_Status_UNSET = 0,
  Result_Status_OK PROTOBUF_DEPRECATED_ENUM = 1,
  Result_Status_FAILED = 2,
  Result_Status_Result_Status_INT_MIN_SENTINEL_DO_NOT_USE_ = std::numeric_limits<int32_t>::min(),
  Result_Status_Result_Status_INT_MAX_SENTINEL_DO_NOT_USE_ = std::numeric_limits<int32_t>::max()
};

However, later in the file, the deprecated 'OK' field is used, which causes the compiler warning:

  // nested types ----------------------------------------------------

  typedef Result_Status Status;
  static constexpr Status UNSET = 
    Result_Status_UNSET;
  PROTOBUF_DEPRECATED_ENUM static constexpr Status OK = 
    Result_Status_OK;   // <<<<<<<<<<<<<< Compiler warning here <<<<<<<<<<<<<<<
  static constexpr Status FAILED = 
    Result_Status_FAILED;

This is a problem when compiling the code with -Werror, because every warning is then treated as an error (intentionally).

Even when the user is not using the deprecated type in their own code, the generated code still uses it, outside of the user's control. The intention of deprecating a field is to highlight any real uses, not the internal use in code generated by protoc itself.

patrickdepinguin avatar Sep 10 '24 10:09 patrickdepinguin

Thanks for reaching out Patrick. This is definitely unintended on our part.

We recently added the compiler tag [[ deprecated ]] to enums -- prior to this marking enum fields as [deprecated = true] was a no-op.

I agree that generating warnings in code that user has no control over is a regression.

The best way to fix this would probably be to use the PROTOBUF_IGNORE_DEPRECATION_START and PROTOBUF_IGNORE_DEPRECATION_STOP macros to wrap the generated code. The macros are defined here: https://github.com/protocolbuffers/protobuf/blob/main/src/google/protobuf/port_def.inc#L258-L273

If you feel comfortable modifying the protoc source code, please feel free to send us a PR for this.

tonyliaoss avatar Sep 11 '24 16:09 tonyliaoss

Thanks for your guidance. I have a solution locally, will create a PR soon.

patrickdepinguin avatar Sep 13 '24 14:09 patrickdepinguin

I'm not able create a PR to this repo due to the CLA this project requires. Essentially, applying the solution you proposed in EnumGenerator::GenerateSymbolImports() solves this issue.

patrickdepinguin avatar Sep 24 '24 09:09 patrickdepinguin

Also mentioned in https://github.com/protocolbuffers/protobuf/issues/19568

tonyliaoss avatar Dec 09 '24 17:12 tonyliaoss