From eb0b3822ef7c9f0bc92b76c72be6dd5f30e35f00 Mon Sep 17 00:00:00 2001 From: Jeremy Nimmer Date: Mon, 30 Jan 2023 08:32:48 -0800 Subject: [PATCH] [common] Provide drake_export and opt-in to Werror=attributes (#18677) As we start to be more careful with linker visibility, it's becoming ever more likely that -Wattributes warnings will leak to the console. Promote them to errors so that we see them and fix the problem. --- common/BUILD.bazel | 6 ++++++ common/drake_export.h | 39 ++++++++++++++++++++++++++++++++++++++ geometry/BUILD.bazel | 1 + geometry/meshcat.cc | 7 +++---- tools/skylark/drake_cc.bzl | 1 + 5 files changed, 50 insertions(+), 4 deletions(-) create mode 100644 common/drake_export.h diff --git a/common/BUILD.bazel b/common/BUILD.bazel index 3b62daf1d457..ad0c09cb3244 100644 --- a/common/BUILD.bazel +++ b/common/BUILD.bazel @@ -30,6 +30,7 @@ drake_cc_package_library( ":diagnostic_policy", ":double", ":drake_bool", + ":drake_export", ":drake_path", ":dummy_value", ":essential", @@ -190,6 +191,11 @@ drake_cc_library( # Miscellaneous utilities. +drake_cc_library( + name = "drake_export", + hdrs = ["drake_export.h"], +) + drake_cc_library( name = "identifier", srcs = ["identifier.cc"], diff --git a/common/drake_export.h b/common/drake_export.h new file mode 100644 index 000000000000..d42cc9c2a36e --- /dev/null +++ b/common/drake_export.h @@ -0,0 +1,39 @@ +#pragma once + +/** DRAKE_NO_EXPORT sets C++ code to use hidden linker visibility. + +Hidden visibility is appropriate for code that will be completely invisible to +users, e.g., for header files that are bazel-private, not installed, and only +used as implementation_deps. + +This macro is most useful when Drake code includes externals that themselves +have hidden linker visibility and compilers complain about mismatched +visibility attributes. + +For example, to un-export all classes and functions in a namespace: +
+namespace internal DRAKE_NO_EXPORT {
+class Foo {
+  // ...
+};
+}  // namespace internal
+
+ +To un-export just one class: +
+namespace internal {
+class DRAKE_NO_EXPORT Foo {
+  // ...
+};
+}  // namespace internal
+
+ +To un-export just one function: +
+DRAKE_NO_EXPORT void CalcFoo(double arg) { ... }
+
+ +For the related CMake module, see: +https://cmake.org/cmake/help/latest/module/GenerateExportHeader.html +*/ +#define DRAKE_NO_EXPORT __attribute__((visibility("hidden"))) diff --git a/geometry/BUILD.bazel b/geometry/BUILD.bazel index 03545843bdbc..8befa9cea120 100644 --- a/geometry/BUILD.bazel +++ b/geometry/BUILD.bazel @@ -466,6 +466,7 @@ drake_cc_library( "//perception:point_cloud", ], deps = [ + "//common:drake_export", "//common:find_resource", "//common:scope_exit", "//common:unused", diff --git a/geometry/meshcat.cc b/geometry/meshcat.cc index 570acd1ae78b..a9e0154acc9f 100644 --- a/geometry/meshcat.cc +++ b/geometry/meshcat.cc @@ -24,6 +24,7 @@ #include #include +#include "drake/common/drake_export.h" #include "drake/common/drake_throw.h" #include "drake/common/find_resource.h" #include "drake/common/never_destroyed.h" @@ -1088,10 +1089,8 @@ class Meshcat::Impl { }); } - // This function is public via the PIMPL. We'll set linker visibility to - // avoid a warning about our std::visit's lambda capture of a msgpack object. - __attribute__((visibility("hidden"))) - void SetAnimation(const MeshcatAnimation& animation) { + // This function is public via the PIMPL. + DRAKE_NO_EXPORT void SetAnimation(const MeshcatAnimation& animation) { DRAKE_DEMAND(IsThread(main_thread_id_)); std::stringstream message_stream; diff --git a/tools/skylark/drake_cc.bzl b/tools/skylark/drake_cc.bzl index f2be352f0bda..213a68b67ffc 100644 --- a/tools/skylark/drake_cc.bzl +++ b/tools/skylark/drake_cc.bzl @@ -7,6 +7,7 @@ load("//tools/skylark:kwargs.bzl", "incorporate_num_threads") # building with any compiler. CXX_FLAGS = [ "-Werror=all", + "-Werror=attributes", "-Werror=cpp", "-Werror=deprecated", "-Werror=deprecated-declarations",