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.

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

with clang, [[nodiscard]] is emitted also for -std=c++11 #9181

Closed
botovq opened this issue Nov 2, 2021 · 7 comments
Closed

with clang, [[nodiscard]] is emitted also for -std=c++11 #9181

botovq opened this issue Nov 2, 2021 · 7 comments
Assignees
Labels

Comments

@botovq
Copy link

botovq commented Nov 2, 2021

What version of protobuf and what language are you using?
Version: v3.19.1
Language: C++

What operating system (Linux, Windows, ...) and version?

OpenBSD -current

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

clang 11.1.0

What did you do?

Build protobuf 3.19.1, then try to build protozero 1.7.0.

What did you expect to see

protozero 1.7.0 builds

What did you see instead?

The build failed because protozero builds with -pedantic -std=c++11 -Werror. On the one hand, this is enough to have PROTOBUF_NODISCARD defined as [[nodiscard]]. On the other hand, this then trips over -Werror due to -Wc++17-extensions:

[79/90] /tmp/pobj/protozero-1.7.0/bin/c++  -I/tmp/pobj/protozero-1.7.0/protozero-1.7.0/include -I/tmp/pobj/protozero-1.7.0/protozero-1.7.0/test/include -isystem /tmp/pobj/protozero-1.7.0/protozero-1.7.0/test/catch -isystem /usr/local/include -isystem test -O2 -pipe  -std=c++17 -DNDEBUG   -std=c++11 -Wall -Wextra -pedantic -Wsign-compare -Wunused-parameter -Wno-float-equal -Wno-covered-switch-default -Werror -pthread -MD -MT test/CMakeFiles/writer_tests.dir/t/bytes/bytes_testcase.pb.cc.o -MF test/CMakeFiles/writer_tests.dir/t/bytes/bytes_testcase.pb.cc.o.d -o test/CMakeFiles/writer_tests.dir/t/bytes/bytes_testcase.pb.cc.o -c test/t/bytes/bytes_testcase.pb.cc
FAILED: test/CMakeFiles/writer_tests.dir/t/bytes/bytes_testcase.pb.cc.o /tmp/pobj/protozero-1.7.0/bin/c++  -I/tmp/pobj/protozero-1.7.0/protozero-1.7.0/include -I/tmp/pobj/protozero-1.7.0/protozero-1.7.0/test/include -isystem /tmp/pobj/protozero-1.7.0/protozero-1.7.0/test/catch -isystem /usr/local/include -isystem test -O2 -pipe  -std=c++17 -DNDEBUG   -std=c++11 -Wall -Wextra -pedantic -Wsign-compare -Wunused-parameter -Wno-float-equal -Wno-covered-switch-default -Werror -pthread -MD -MT test/CMakeFiles/writer_tests.dir/t/bytes/bytes_testcase.pb.cc.o -MF test/CMakeFiles/writer_tests.dir/t/bytes/bytes_testcase.pb.cc.o.d -o test/CMakeFiles/writer_tests.dir/t/bytes/bytes_testcase.pb.cc.o -c test/t/bytes/bytes_testcase.pb.cc
In file included from test/t/bytes/bytes_testcase.pb.cc:4:
test/t/bytes/bytes_testcase.pb.h:190:3: error: use of the 'nodiscard' attribute is a C++17 extension [-Werror,-Wc++17-extensions]
  PROTOBUF_NODISCARD std::string* release_s();
  ^
/usr/local/include/google/protobuf/port_def.inc:463:30: note: expanded from macro 'PROTOBUF_NODISCARD'
#define PROTOBUF_NODISCARD [[nodiscard]]
                             ^
1 error generated.
ninja: build stopped: subcommand failed.

Anything else we should know about your project / environment

It seems #if __has_cpp_attribute(nodiscard) is insufficient to determine whether [[nodiscard]] is actually allowed. A similar issue was also reported here: nlohmann/json#1535

This could be avoided if PROTOBUF_NODISCARD were only defined to [[nodiscard]] with

#if __has_cpp_attribute(nodiscard) && PROTOBUF_CPLUSPLUS_MIN(201703L)
#define PROTOBUF_NODISCARD [[nodiscard]]
#elif
...

or similar.

@elharo elharo added the c++ label Nov 3, 2021
@thomas-olszamowski
Copy link

Confirming same problem with:
Apple clang version 13.0.0
and protobuf 3.19.2

@ezzieyguywuf
Copy link

To add some more context: it appears that 624d29d added these lines to port_def.inc, which is where the error is originating from.

I think @botovq 's suggestion to extend the logic in the preprocessor check should help to mitigate this.

ezzieyguywuf added a commit to ezzieyguywuf/mosh that referenced this issue Jan 29, 2022
ezzieyguywuf added a commit to ezzieyguywuf/mosh that referenced this issue Jan 29, 2022
ezzieyguywuf added a commit to ezzieyguywuf/mosh that referenced this issue Jan 29, 2022
ezzieyguywuf added a commit to ezzieyguywuf/mosh that referenced this issue Jan 29, 2022
@ezzieyguywuf
Copy link

Here's how you can reproduce the error with a minimal example:

protoc --version # must be >= 3.18.1
# test.proto
syntax = "proto2";

package Nothing;

message Message {
  optional string something = 1;
}
// main.cpp
#include "test.pb.h"

int main(){}
# first, generate the protobuf code
protoc -I=. --cpp_out=. test.proto

# this works
clang++ -std=c++17 -pedantic -Werror main.cpp

# this does not
clang++ -std=c++14 -pedantic -Werror main.cpp

ezzieyguywuf added a commit to ezzieyguywuf/mosh that referenced this issue Jan 30, 2022
ezzieyguywuf added a commit to ezzieyguywuf/mosh that referenced this issue Jan 30, 2022
ezzieyguywuf added a commit to ezzieyguywuf/mosh that referenced this issue Jan 30, 2022
ezzieyguywuf added a commit to ezzieyguywuf/mosh that referenced this issue Jan 30, 2022
achernya pushed a commit to mobile-shell/mosh that referenced this issue Feb 4, 2022
@ezzieyguywuf
Copy link

This bug appears to be resolved by ab4585a

See this line

@oxoocoffee
Copy link

oxoocoffee commented Mar 13, 2022

Confirming the same problem with:
Apple clang version 13.0.0 (clang-1300.0.29.30)
protoc --version
libprotoc 3.19.4

Building code with c++11

Temp fix:
#pragma clang diagnostic ignored "-Wc++17-extensions"

@ezzieyguywuf
Copy link

Confirming the same problem with:
Apple clang version 13.0.0 (clang-1300.0.29.30)
protoc --version
libprotoc 3.19.4

I think this should be resolved in the next release.

You could try compiling protoc from the latest source to confirm

@botovq
Copy link
Author

botovq commented Apr 15, 2022

As pointed out by @ezzieyguywuf this issue is resolved in protobuf 3.20 and later.

@botovq botovq closed this as completed Apr 15, 2022
jlangston pushed a commit to jlangston/mosh that referenced this issue May 2, 2022
ezzieyguywuf added a commit to ezzieyguywuf/mosh that referenced this issue Jul 12, 2022
…otobuf#9181"

This reverts commit dbe419d.

Since protocolbuffers/protobuf#9181 hase been
resolved, this workaround should no longer be needed.
clrpackages pushed a commit to clearlinux-pkgs/mosh that referenced this issue Jan 30, 2023
Alex Chernyakhovsky (22):
      Add support for generating coverage reports
      Ignore generated protobufs for coverage
      Add code coverage instructions to README.md
      Add fuzzing infrastructure
      Add fuzzer for the terminal
      Revert "Remove redundant malloc/free"
      Use OpenSSL native OCB-AES implementation
      Add nettle to the CI matrix
      Stop using deprecated Nettle functions
      Correct memory leak in ocb-aes test
      Fixes for distcheck
      Add release action on Linux
      Add macOS release steps to Github Actions
      Remove obsolete Travis CI configuration
      Add -Wno-unused-parameter
      Switch macOS multi-arch to x86_64+arm64
      Tag mosh 1.4.0 Release Candidate
      Fetch forcibly while fetching tags
      Actually evaluate the github actions contains() expression for prerelease
      Bump release candidate to 1.4.0-rc1
      Switch 1.4.0-rc1 version to 1.3.2.95rc1
      Disable emulation-attributes-bce on tmux 3.3a

Alex Cornejo (1):
      support osc 52 clipboard copy integration.

Anders Kaseorg (12):
      Remove unused Action::operator==
      mosh.pl: Fix the error message if getaddrinfo is missing
      mosh.pl: Allow shell expansion of --server with --local
      Type Select::got_signal as volatile sig_atomic_t
      configure.ac: Fix underquoted AC_HELP_STRING call
      configure: Add --enable-static-LIBRARY options for selective static linking
      configure: Another flag for macOS distcheck: -Wno-error=nested-anon-types
      configure: Set language to C++ globally
      configure: Remove unused tests
      configure: Fix forkpty test code indentation
      configure: Fix FD_ISSET test under -Werror
      configure: Add test for whether protoc matches protobuf

Benjamin Barenblat (18):
      Separate OpenSSL-based OCB implementation from others
      Go back to internal OCB implementation
      Put -lnettle back on the link line when using Nettle
      Delete unused ROUNDS macro
      OCB: Make primitive AES API explicit
      OCB: Heap-allocate keys
      OCB: Use OpenSSL EVP instead of deprecated AES
      Audit and fix up format strings
      Bump release candidate to 1.3.2.95rc2
      Start updating Debian packaging for 1.3.2.95rc2
      Update to Debhelper 12
      Update debian/watch to version 4
      Update Standards-Version
      Add Rules-Requires-Root
      Support nocheck profile
      Release for Debian experimental
      Release for Debian experimental, take 2
      Bump version to 1.4.0

Harry Sintonen (2):
      Use CLOCK_MONOTONIC_RAW when available
      Only use CLOCK_MONOTONIC_RAW with __APPLE__ systems.

John Hood (67):
      Reformat printed strings in source
      Don't do prediction on large pastes into mosh-client.
      Allow non-inserting prediction.
      --predict-overwrite was still inserting one column sometimes.  Fix.
      Rename and document --predict-overwrite.
      Fix utempter #ifdefs.
      Add missing shared::make_shared<T>()
      Remove redundant new/delete
      remove unneeded new/delete in Compressor
      Remove redundant malloc/free
      Convert strdup/free to string
      Convert new/delete to shared_ptr.
      Deref a shared pointer in a per-byte loop
      Use shared_ptr and references for Actions.
      Simplify some conditionals in terminaloverlay.cc.
      Improve targets for existing static checkers.
      Add support for OCLint static checker.
      Split a long printf format string.
      Collapse nested conditionals.
      Eliminate unnecessary, trailing else conditional blocks.
      Prefer early exit/break to large conditional blocks.
      Remove excessive parentheses
      Various switch statement fixes.
      Remove private static class methods.
      Do not set function parameters.
      Remove an unnecessary conditional.
      Fix Debian GCC7 FTBFS: remove default constructors
      Remove various assert(constant) calls
      Safeguard tmux sanity check.
      Restore asserts and error handling
      Always use non-blocking sockets for recvmsg()
      Fix a minor uninitialized-variable warning.
      Add Perl compile
      Perl compile on Appveyor/Cygwin requires perl_pods package
      Check tmux version for truecolor test.
      Make Terminal::Renditions smaller, and its members private.
      Minor SGR printf type signedness fixes.
      Overlays were getting set to the wrong colors.  Fix.
      Make Renditions::sgr() more compact in both code and output.
      Fix issue with incorrect true-color background erase colors.
      Extend true color test to include background color erase.
      Revert "Extend true color test to include background color erase."
      Add a separate test for BCE
      Construct socket name correctly for tmux_check()
      Switch to MacOS 10.12 for builds and deployment target.
      Fix gcc8 snprintf truncation warning.
      Fix/workaround Homebrew failure on Travis MacOS XCode 9.1 image.
      Require C++11 if protobuf version >= 3.6.0 is installed
      Fix Homebrew failure on Travis.
      Allow Travis to use its preferred MacOS/XCode image.
      mosh-server: improve error logging
      Ignore select() errors on Travis/MacOS.
      Display CPU count on Travis/MacOS.
      C++03 bound functions are not available in C++17; remove
      src/statesync/completeterminal.cc: fix bad iterator type
      Fix more inappropriate const_iterator usage.
      Fix bind(2) being misinterpreted as std::bind() with libc++7 on FreeBSD.
      Always use std::min, std::max.
      Remove "using namespace std;".
      Some more namespace hygiene for "using decl;".
      unicode-later-combining.test: Document slightly.
      Restrict cppcheck to src/ directory to avoid Git worktrees.
      Fix Appveyor build.
      "Fix" Travis OS X builds.
      Use Travis Homebrew addon.
      Move generated includes to their own directory to avoid conflicts.
      If exec()ing the remote command fails, pause briefly

Kalle Samuels (1):
      Don't sometimes hang just after launching ssh

Kang.Jianbin (2):
      Add true color support.
      Add emulation-attributes test for true color.

Keith Winstein (4):
      debian/control: update standards version
      terminalframebuffer.cc: ignore unknown renditions
      debian/changelog: update for Debian 1.3.2-2 release
      debian/control: update standards version

Michael Jarvis (1):
      Use HAVE_UTEMPTER instead of HAVE_UPTEMPTER

Naïm Favier (1):
      Add tmux and alacritty to title_term_types

Peter Edwards (1):
      Apply latest consecutive resize, not earliest.

Tom Judge (1):
      Add syslog logging of connections

Wolfgang E. Sanyer (3):
      Add github action for CI build
      Replace Travis-CI badge with Github Actions badge.
      Disable clang warning in order to mitigate protocolbuffers/protobuf#9181

black_desk (2):
      .gitignore: add compile_commands.json
      .gitignore: fix path

buZz (1):
      fixed the irc channel link in README
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants