Skip to content

[VL][MINOR] Replace NULL with nullptr - #10958

Merged
FelixYBW merged 2 commits into
apache:mainfrom
beliefer:replace-NULL-with-nullptr
Oct 31, 2025
Merged

[VL][MINOR] Replace NULL with nullptr#10958
FelixYBW merged 2 commits into
apache:mainfrom
beliefer:replace-NULL-with-nullptr

Conversation

@beliefer

Copy link
Copy Markdown
Contributor

What changes are proposed in this pull request?

This PR proposes to replace NULL with nullptr.
In C++, nullptr should always be used instead of NULL because:

  • Provide better type security
  • Avoid overloading and resolving ambiguity
  • Better performance in template programming
  • The code intent is clearer
  • It is a standard practice in modern C++

Only consider using NULL when maintaining very old code repositories or interacting with C interfaces that only accept NULL.

How was this patch tested?

GA tests.

@github-actions github-actions Bot added the VELOX label Oct 28, 2025
@wForget

wForget commented Oct 28, 2025

Copy link
Copy Markdown
Member

The original code is referenced from https://github.com/jemalloc/jemalloc/wiki/Use-Case%3A-Heap-Profiling. If NULL and nullptr are equivalent, this change looks good to me. @philo-he Could you please take a look?

@wForget
wForget requested a review from philo-he October 28, 2025 09:21
@beliefer

Copy link
Copy Markdown
Contributor Author

@wForget Thanks ! I think https://github.com/jemalloc/jemalloc/wiki/Use-Case%3A-Heap-Profiling just give us an use case.

@FelixYBW

Copy link
Copy Markdown
Contributor

@philo-he philo-he left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good. Just verified in my local build with enable_jemalloc_stats set to ON.

@FelixYBW, it seems clang-format doesn't support the check for using nullptr. @beliefer, could you add the following sentence in Constant section of CppCodingStyle.md to make developer aware of this convention?

Always use `nullptr` if you need a constant that represents a null pointer (`T*` for some `T`); use `0` otherwise for a zero value.

Reference:
https://github.com/facebookincubator/velox/blob/main/CODING_STYLE.md#constants

@github-actions github-actions Bot added the DOCS label Oct 30, 2025
@FelixYBW

Copy link
Copy Markdown
Contributor

@FelixYBW, it seems clang-format doesn't support the check for using nullptr.

I mean to add a sed like sed -i 's/NULL/nullptr/g'

@philo-he

Copy link
Copy Markdown
Member

@FelixYBW, I feel that using sed command may cause unintentional replacement issue.

I just investigated Velox. It uses clang-tidy to check code style based on a configuration file, where we can set to replace NULL with nullptr, see https://github.com/facebookincubator/velox/blob/8a5a520fba06ab1fc10f124f3eb000bab886cd5d/.clang-tidy#L102.

If this sounds good to you, I can help enable clang-tidy in another PR. It can also cover some other code styles checks.

@FelixYBW

Copy link
Copy Markdown
Contributor

If this sounds good to you, I can help enable clang-tidy in another PR. It can also cover some other code styles checks.

Thank you, @philo-he good idea.

@FelixYBW
FelixYBW merged commit 2f7b138 into apache:main Oct 31, 2025
98 of 99 checks passed
@beliefer

Copy link
Copy Markdown
Contributor Author

@philo-he @FelixYBW @wForget Thank you all

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants