Skip to content

Commit

Permalink
place alignas() before lifetime specifiers (#11248)
Browse files Browse the repository at this point in the history
This patch fixes a compilation bug introduced in 821b073.

When the `constinit` keyword is available (such as in C++20), putting the `alignas` attribute after `ABSL_CONST_INIT` is an error:

```
ERROR: /home/widders/.cache/bazel/_bazel_widders/25f0d335ca0e01a2fd74c60e90175e8f/external/com_google_protobuf/src/google/protobuf/compiler/java/BUILD.bazel:44:11: Compiling src/google/protobuf/compiler/java/generator_factory.cc failed: (Exit 1): clang failed: error executing command /usr/lib/llvm-16/bin/clang -U_FORTIFY_SOURCE -fstack-protector -Wall -Wthread-safety -Wself-assign -Wunused-but-set-parameter -Wno-free-nonheap-object -fcolor-diagnostics -fno-omit-frame-pointer -g0 ... (remaining 64 arguments skipped)

Use --sandbox_debug to see verbose messages from the sandbox and retain the sandbox build root for debugging
In file included from external/com_google_protobuf/src/google/protobuf/compiler/java/generator_factory.cc:35:
In file included from bazel-out/k8-opt-exec-2B5CBBC6/bin/external/com_google_protobuf/src/google/protobuf/compiler/java/_virtual_includes/java/google/protobuf/compiler/java/context.h:38:
In file included from bazel-out/k8-opt-exec-2B5CBBC6/bin/external/com_google_protobuf/src/google/protobuf/compiler/java/_virtual_includes/names_internal/google/protobuf/compiler/java/helpers.h:45:
In file included from bazel-out/k8-opt-exec-2B5CBBC6/bin/external/com_google_protobuf/src/google/protobuf/_virtual_includes/protobuf_nowkt/google/protobuf/descriptor.pb.h:25:
In file included from bazel-out/k8-opt-exec-2B5CBBC6/bin/external/com_google_protobuf/src/google/protobuf/_virtual_includes/protobuf_lite/google/protobuf/arena.h:53:
bazel-out/k8-opt-exec-2B5CBBC6/bin/external/com_google_protobuf/src/google/protobuf/_virtual_includes/protobuf_lite/google/protobuf/arena_impl.h:584:19: error: an attribute list cannot appear here
  ABSL_CONST_INIT alignas(
                  ^~~~~~~~
1 error generated.
```

Therefore, it should be ordered as
1. always attribute-like (`alignas`)
2. sometimes attributes (`ABSL_CONST_INIT`, which may be a lifetime specifier or an attribute)
3. lifetime specifiers (`static`)
4. type etc.

Where it currently is it may be in the midst of lifetime specifiers, and if it is moved to the right it syntactically applies to the type rather than the whole declaration which is also invalid.

If this ordering could not be resolved for all cases it could also be moved to the end, after the name being declared, but we don't need to do that here.

Closes #11248

COPYBARA_INTEGRATE_REVIEW=#11248 from mumbleskates:alignas-fix 496af77
PiperOrigin-RevId: 495739359
  • Loading branch information
mumbleskates authored and copybara-github committed Dec 16, 2022
1 parent 0804621 commit 5712e1a
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 3 deletions.
2 changes: 1 addition & 1 deletion src/google/protobuf/arena.cc
Original file line number Diff line number Diff line change
Expand Up @@ -443,7 +443,7 @@ ThreadSafeArena::SerialArenaChunk* ThreadSafeArena::SentrySerialArenaChunk() {
}


ABSL_CONST_INIT alignas(kCacheAlignment)
alignas(kCacheAlignment) ABSL_CONST_INIT
std::atomic<ThreadSafeArena::LifecycleId> ThreadSafeArena::lifecycle_id_{0};
#if defined(PROTOBUF_NO_THREADLOCAL)
ThreadSafeArena::ThreadCache& ThreadSafeArena::thread_cache() {
Expand Down
4 changes: 2 additions & 2 deletions src/google/protobuf/thread_safe_arena.h
Original file line number Diff line number Diff line change
Expand Up @@ -269,8 +269,8 @@ class PROTOBUF_EXPORT ThreadSafeArena {
#pragma warning(disable : 4324)
#endif
using LifecycleId = uint64_t;
ABSL_CONST_INIT alignas(
kCacheAlignment) static std::atomic<LifecycleId> lifecycle_id_;
alignas(kCacheAlignment) ABSL_CONST_INIT
static std::atomic<LifecycleId> lifecycle_id_;
#if defined(PROTOBUF_NO_THREADLOCAL)
// iOS does not support __thread keyword so we use a custom thread local
// storage class we implemented.
Expand Down

0 comments on commit 5712e1a

Please sign in to comment.