protocolbuffers / protobuf

Protocol Buffers - Google's data interchange format
http://protobuf.dev
Other
65.23k stars 15.45k forks source link

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

Open patrickdepinguin opened 1 week ago

patrickdepinguin commented 1 week ago

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.

tonyliaoss commented 1 week ago

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.

patrickdepinguin commented 6 days ago

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