[llvm] [llvm-debuginfo-analyzer] Use `LLVM_BUILD_DEBUG` in class definitions (PR #140265)
Carlos Alberto Enciso via llvm-commits
llvm-commits at lists.llvm.org
Mon May 19 23:17:12 PDT 2025
CarlosAlbertoEnciso wrote:
> > > I don't understand the motivation of this change. I realize that `NDEBUG` is a confusing macro, but this is not about a _debug_ build. `NDEBUG` indicates whether assertions are enabled or not.
> >
> >
> > Basically, when using `debuginfologicalview` as a library from a downstream project, `NDEBUG` may be defined if the downstream project is built in Release mode. This results in a different class layout (e.g. for `LVObject`), given that, by the time the `LVObject.h` is parsed in a downstream project, `NDEBUG` may be defined (or not) regardless of whether it was defined (or not) when LLVM was built.
> > Thus, `NDEBUG` cannot be relied for anything that affects class layout (at least on public headers). Looking at @dwblaikie and @compnerd comments, perhaps using `LLVM_ENABLE_ABI_BREAKING_CHECKS` (instead of introducing a new `LLVM_BUILD_DEBUG` PP macro) is the way to go 🤔 (?)
>
> Well, `dump` shouldn't be part of the public interface, so removal is a better option. Note the guard - `LLVM_ENABLE_DUMP` is an alternative spelling. Depending on `NDEBUG` is acceptable here. This doesn't change the object layout, but does change the API surface. If the downstream project is attempting to use the `dump` methods, it should be building LLVM with `LLVM_ENABLE_DUMP`.
May be some clarification:
```
virtual void print(raw_ostream &OS, bool Full = true) const;
#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
virtual void dump() const { print(dbgs()); }
#endif
```
`dump` should be called only for _debug_ builds.
`print` can be called for both builds (_debug_ and _non debug_).
```
- Debug build: print(dbgs())
- Non Debug Build: print(outs());
```
The downstream project should use `print`.
https://github.com/llvm/llvm-project/pull/140265
More information about the llvm-commits
mailing list