Skip to content

correct [[deprecated]] attribute placement on class#3142

Closed
EricX-Zhao wants to merge 1 commit into
ros2:rollingfrom
EricX-Zhao:fix/memory-strategy-deprecated-attribute-placement
Closed

correct [[deprecated]] attribute placement on class#3142
EricX-Zhao wants to merge 1 commit into
ros2:rollingfrom
EricX-Zhao:fix/memory-strategy-deprecated-attribute-placement

Conversation

@EricX-Zhao
Copy link
Copy Markdown

Description

The previous declaration placed the [[deprecated]] attribute between the class-key and the RCLCPP_PUBLIC visibility macro:

class RCLCPP_PUBLIC [[deprecated(\"...\")]] MemoryStrategy

This form fails to compile under GCC 11.2 because the parser cannot correctly handle a C++ standard attribute followed by a GNU attribute (which RCLCPP_PUBLIC expands to on Linux) in the class head, resulting in errors such as:

error: expected primary-expression before 'public'
error: expected '}' before 'public'

Move the [[deprecated]] attribute to the declaration position, immediately before the class-key. This form is parsed reliably by GCC, Clang, and MSVC, ensures the deprecation warning is actually emitted at use sites, and also propagates to derived classes.

Is this user-facing behavior change?

No

Did you use Generative AI?

Yes Claude Code Opus4.6

Additional Information

…lass

The previous declaration placed the [[deprecated]] attribute between the class-key and the RCLCPP_PUBLIC visibility macro:

    class RCLCPP_PUBLIC [[deprecated(\"...\")]] MemoryStrategy

This form fails to compile under GCC 11.2 because the parser cannot
correctly handle a C++ standard attribute followed by a GNU
__attribute__ (which RCLCPP_PUBLIC expands to on Linux) in the class
head, resulting in errors such as:

    error: expected primary-expression before 'public'
    error: expected '}' before 'public'

Move the [[deprecated]] attribute to the declaration position,
immediately before the class-key. This form is parsed reliably by
GCC, Clang, and MSVC, ensures the deprecation warning is actually
emitted at use sites, and also propagates to derived classes.

Signed-off-by: EricX-Zhao <ericzhao0325@163.com>
@ahcorde
Copy link
Copy Markdown
Contributor

ahcorde commented May 11, 2026

Pulls: #3142
Gist: https://gist.githubusercontent.com/ahcorde/1ac969ae68463008178a3daad2b8de0b/raw/d8dddef9628782f9715f890112e9af61a1ca5b5f/ros2.repos
BUILD args: --packages-above-and-dependencies rclcpp
TEST args: --packages-above rclcpp
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/19211

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows Build Status

Copy link
Copy Markdown
Collaborator

@jmachowinski jmachowinski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This removes the deprecation warning, as it is simply ignored.

I found no way to make it compile under GCC 12.1.

@jmachowinski
Copy link
Copy Markdown
Collaborator

@jmachowinski
Copy link
Copy Markdown
Collaborator

Closing, as the proposed patch is not working, and aiming at an unsupported platform (gcc 12.3 / ubuntu 22.04)

Copy link
Copy Markdown
Collaborator

@fujitatomoya fujitatomoya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

standard says an attribute-specifier-seq appertaining to a class declaration should appear right after the class keyword or before the class keyword as part of the decl-specifier-seq. so i think this is a right thing to do.

@fujitatomoya
Copy link
Copy Markdown
Collaborator

my reply was cached in the browser, so ignore the previous comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants