Skip to content

Conversation

@mabruzzo
Copy link
Collaborator

@mabruzzo mabruzzo commented Jan 5, 2026

A lightweight PR that introduces the GRIMPL_NS and GRIMPL_NAMESPACE_DECL macros.

Description

The basic idea is that we will gradually replace

  • every occurrence of grackle::impl that is used to open a namespace block with GRIMPL_NAMESPACE_DECL. After the replacement, a namespace block looks like:
    namespace GRIMPL_NAMESPACE_DECL {
    // ...
    }  // namespace GRIMPL_NAMESPACE_DECL
  • All other occurrences of grackle::impl will be replaced with GRIMPL_NS

As a part of this PR I demonstrated what this will look like in the ceiling_species.hpp file.

The impetus for doing this sooner rather than later is to start using the macros in newly created C++ files.

Motivation

These macros are inspired by similar macros used to define LLVM's implementation of the standard C library.

I provided really detailed docstrings explaining why we want this but it essentially boils down to

  1. widespread use of GRIMPL_NS lets us rename grackle::impl namespace. This affords a few benefits (with the benefit of hindsight, I slightly regret grackle::impl as a name, and it's probably important for distributing Grackle as Header-Only library). It's also goes hand-in-hand with the GRIMPL_NAMESPACE_DECL macro
  2. the GRIMPL_NAMESPACE_DECL macro is a relatively elegant way to control symbol visibility in shared libraries

Other thoughts

I generally think that introducing GRIMPL_NS is a good idea. But, if you aren't convinced, I could remake the PR without GRIMPL_NAMESPACE_DECL.

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

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

1 participant