Make it possible to overwrite debug macros locally.

[asynts]

This should solve the compile-time problem when toggling debug options. #5104 (comment)

Now, it is possible to locally overwrite debug macros by defining them before including AK/Debug.h or Kernel/Debug.h:

// file: Example.cpp

#define FOO_DEBUG 1

#include <AK/Debug.h>

#if FOO_DEBUG
// Do something.
#endif

The problem with this approach is that the locally defined macro always has precedence:

// file: Example.cpp

// Oops. Even if we build with -DFOO_DEBUG, we disable this debug option.
#define FOO_DEBUG 0

#include <AK/Debug.h>

#if FOO_DEBUG
// Do something.
#endif

This is why I held this back for a while, but I can't think of a better way.

---

In theory I could solve this by changing AK/Debug.h to something like this:

#ifdef DEBUG_FOO
# if !DEBUG_FOO
#  undef DEBUG_FOO
#  cmakedefine01 DEBUG_FOO
# endif
#else
#  cmakedefine01 DEBUG_FOO
#endif

But that's pretty spam-y and in some situations it might be helpful to disable debugging in one source file.

[awesomekling]

Hmm, I don't think ENABLE_ALL_THE_DEBUG_MACROS is actually needed anymore.
Its purpose was to prevent bitrot in #if FOO_DEBUG blocks, but that's now covered by making those blocks if constexpr (FOO_DEBUG) instead.

[asynts]

The problem is that template code inside of if constexpr is not fully checked by the compiler:

#include <type_traits>

template<typename T>
void bar() {
    static_assert(std::is_same<T, void>::value);
}

void foo() {
    if constexpr (false) {
        // This will cause the static assertion in bar() to fail, but we don't get an
        // error because it's not evaluated for some reason.
        bar<int>();
    }
}

https://godbolt.org/z/a86cdP

If the if constexpr is replaced with a runtime if, then this static assertion is evaluated and fails.

---

In other words, code rot is possible with this new approach too. It might be possible to use a runtime if and then rely on the optimizer to remove the code though. (Not sure if that would be good for compile time, because if constexpr allows the compiler to throw that code away early.)

Also, there are still a few cases left where #if FOO_DEBUG is used, for example in Thread.cpp (it conditionally has a member variable to keep track of locks being held.)

---

This might just be a compiler bug though, so maybe I should check in the standard if the compiler actually behaves correctly here. (I am a bit scared of doing that, but maybe I'll spend an hour or two tomorrow and try to make sense of this.)

[tomuta]

I think the whole point of if constexpr is that the compiler does not fully check the code if the expression is false. E.g. it should be possible to call non-existing methods etc in there that only compile successfully if the if constexpr evaluates to true.

[awesomekling]

I think the whole point of if constexpr is that the compiler does not fully check the code if the expression is false. E.g. it should be possible to call non-existing methods etc in there that only compile successfully if the if constexpr evaluates to true.

Uh, that's a good point. So this doesn't actually cover bitrot beyond basic parsing I guess. :/

[asynts]

I've tried actually looking in the standard but was only able to find this in N4868:

8.5.2.2

If the if statement is of the form if constexpr, the value of the condition shall be a contextually converted constant expression of type bool (7.7); this form is called a constexpr if statement. If the value of the converted condition is false, the first substatement is a discarded statement, otherwise the second substatement, if present, is a discarded statement. During the instantiation of an enclosing templated entity (13.1), if the condition is not value-dependent after its instantiation, the discarded substatement (if any) is not instantiated.

[Note 1: Odr-uses (6.3) in a discarded statement do not require an entity to be defined.— end note]

I couldn't really find any proper definition of a "discarded statement", but this really sounds like the compiler is behaving correctly here.