[libcxx-commits] [PATCH] D115368: [NFC][libcxxabi] Re-organized inheritance structure to remove CRTP in cxa_guard

[Daniel McIntosh via Phabricator via libcxx-commits]

DanielMcIntosh-IBM marked an inline comment as not done.
DanielMcIntosh-IBM added inline comments.


================
Comment at: libcxxabi/src/cxa_guard_impl.h:212-215
+  /// The init byte portion of cxa_guard_acquire. Returns true if
+  /// initialization has already been completed.
+  /// Note: On completion, we haven't 'acquired' ownership of anything mutex
+  /// related. The name is simply referring to __cxa_guard_acquire.
----------------
ldionne wrote:
> The duplication of these comments feels weird to me. Instead, I believe we should have a comment like:
> 
> ```
> //===----------------------------------------------------------------------===//
> //                    Initialization Byte Implementations
> //===----------------------------------------------------------------------===//
> //
> // Each initialization byte implementation supports the following method:
> //
> //    InitByte(uint8_t* _init_byte_address, uint32_t*)
> //        Here explain the purpose of these constructors.
> //
> //    bool acquire();
> //        Here document what this method does and you can have the note about ownership documented in exactly one place, here.
> //
> //    etc...
> //
> ```
> 
> Then, each implementation of an initialization byte can simply implement the required methods and document anything interesting that is specific to that implementation.
I left very small summary still, just for tools which recognize documentation comments to pick up.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115368/new/

https://reviews.llvm.org/D115368