[PATCH] D11833: s/NDEBUG/LLVM_NDEBUG/ in most places

Roger Jarrett via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Nov 14 15:04:37 PST 2019


rogerjarrett added a comment.

Yes I agree the example case I provided is not an  LLVM_ENABLE_ABI_BREAKING_CHECKS  modification

I agree with dexonsmith  on these points

1. That LLVM_ENABLE_ABI_BREAKING_CHECKS  addresses the issue

2. The use   LLVM_ENABLE_ABI_BREAKING_CHECKS should be better  documented in the llvm-project coding standards

2a.  And the use or non-use llvm_assert(),assert(), NDEBUG,.. should also be documented

3. That we need to think through how this model should apply to subprojects: clang, lld, mlir,...

How can I help enable this discussion of 1 and 2?

Rhetorical question:  What is the definition of when   LLVM_ENABLE_ABI_BREAKING_CHECKS should be used ?

Candidate answer:  Any change to the class/template/... that changes the size of class or template

Correct usage example from: llvm/include/llvm/Analysis/LoopInfo.h
...
template <class BlockT, class LoopT> class LoopBase {

  LoopT *ParentLoop;
  // Loops contained entirely within this one.
  std::vector<LoopT *> SubLoops;
  
  // The list of blocks in this loop. First entry is the header node.
  std::vector<BlockT *> Blocks;
  
  SmallPtrSet<const BlockT *, 8> DenseBlockSet;

#if LLVM_ENABLE_ABI_BREAKING_CHECKS

  /// Indicator that this loop is no longer a valid loop.
  bool IsInvalid = false;

#endif
...

header example that appears  to need LLVM_ENABLE_ABI_BREAKING_CHECKS

File:   llvm/include/llvm/CodeGen/LiveIntervalUnion.h

Line 101 in llvm-project 9.0.0
...
 // Print union, using TRI to translate register names

  void print(raw_ostream &OS, const TargetRegisterInfo *TRI) const;

#ifndef NDEBUG

  // Verify the live intervals in this union and add them to the visited set.
  void verify(LiveVirtRegBitSet& VisitedVRegs);

#endif
...

How do I confirm that the use of NDEBUG in a header file is correct?


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

https://reviews.llvm.org/D11833





More information about the llvm-commits mailing list