Skip to content

Commit

Permalink
[common] Provide drake_export and opt-in to Werror=attributes (RobotL…
Browse files Browse the repository at this point in the history
…ocomotion#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.
  • Loading branch information
jwnimmer-tri authored and xuchenhan-tri committed Feb 6, 2023
1 parent 635ac95 commit aa89941
Show file tree
Hide file tree
Showing 5 changed files with 50 additions and 4 deletions.
6 changes: 6 additions & 0 deletions common/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ drake_cc_package_library(
":diagnostic_policy",
":double",
":drake_bool",
":drake_export",
":drake_path",
":dummy_value",
":essential",
Expand Down Expand Up @@ -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"],
Expand Down
39 changes: 39 additions & 0 deletions common/drake_export.h
Original file line number Diff line number Diff line change
@@ -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:
<pre>
namespace internal DRAKE_NO_EXPORT {
class Foo {
// ...
};
} // namespace internal
</pre>
To un-export just one class:
<pre>
namespace internal {
class DRAKE_NO_EXPORT Foo {
// ...
};
} // namespace internal
</pre>
To un-export just one function:
<pre>
DRAKE_NO_EXPORT void CalcFoo(double arg) { ... }
</pre>
For the related CMake module, see:
https://cmake.org/cmake/help/latest/module/GenerateExportHeader.html
*/
#define DRAKE_NO_EXPORT __attribute__((visibility("hidden")))
1 change: 1 addition & 0 deletions geometry/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -466,6 +466,7 @@ drake_cc_library(
"//perception:point_cloud",
],
deps = [
"//common:drake_export",
"//common:find_resource",
"//common:scope_exit",
"//common:unused",
Expand Down
7 changes: 3 additions & 4 deletions geometry/meshcat.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include <drake_vendor/uuid.h>
#include <fmt/format.h>

#include "drake/common/drake_export.h"
#include "drake/common/drake_throw.h"
#include "drake/common/find_resource.h"
#include "drake/common/never_destroyed.h"
Expand Down Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions tools/skylark/drake_cc.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down

0 comments on commit aa89941

Please sign in to comment.