There was a board consensus on the PR that added IS_USED and IS_ACTIVE that this is the route forward. (IIRC, only the implementation was a bit controversial. But the use of the approach had IIRC no opposition at all.)

@cgundogan, @smlng, @jia200x, @benpicco, @waehlisch: A few more ACKs would be nice here, even though this addition is pretty much the logical consequence of the discussion in #12626

Maybe some stronger wording would be nice: "The code should only contain #ifs and #ifdefs were solid technical arguments can justify their use."

Can we talk about consequences of merging this for a bit? Say there is an older PR with a few thousand lines of code containing a few #ifdefs. Do we really start, once this PR is merged (or even before), asking the contributor to apply this new rule even though it is logically the same code? While many cases can be automated, the exception to the rule make it harder to port to the new scheme (and puts the contributor on the stand to explain their reasoning for using #ifdefs even though they were not required before).

Do we really start, once this PR is merged (or even before), asking the contributor to apply this new rule even though it is logically the same code?

I'd say yes. Applying this only to a subset of PRs will only confuse everyone. And the motivation to clean things up after merging is often not as high as before merging ;-)

But I totally agree that a situation like that can be frustrating. However, better code maintainability can also avoid a lot of frustration later on.

Can we talk about consequences of merging this for a bit? Say there is an older PR with a few thousand lines of code containing a few #ifdefs. Do we really start, once this PR is merged (or even before), asking the contributor to apply this new rule even though it is logically the same code?

If those guidelines are getting interpreted as legal binding documents (which I see happening) I wonder if they end up doing more harm than good.

Before this turns into a game of Nomic, let's add a statement that those are guidelines and common sense / maintainer judgement should still apply.

But I totally agree that a situation like that can be frustrating. However, better code maintainability can also avoid a lot of frustration later on.

That's what I'm wondering: does the code really become more maintainable if I write

if (USED(GNRC_IPV6) {
    /* do something */
}

instead of

#ifdef MODULE_GNRC_IPV6
    /* do something */
#endif

?

Before this turns into a game of Nomic, let's add a statement that those are guidelines and common sense / maintainer judgement should still apply.

Doesn't help... The maintainer guidelines were also often misinterpreted as a bible in the past :-P.

If those guidelines are getting interpreted as legal binding documents (which I see happening) I wonder if they end up doing more harm than good.

The current wording is "In order to follow Linux's recommendation [...]". That is not that binding.

That's what I'm wondering: does the code really become more maintainable if I write

I don't think so. Especially considering that the macro cannot be used everywhere.

That's what I'm wondering: does the code really become more maintainable

In no cases it will get less maintainable, but in many cases it will become more maintainable. E.g. the compiler will verify the syntax of all code flavors and generate warnings for all code flavors if they are visible to the compiler. A #ifdef will make the compiler blind. Also: The preprocessor is pretty limited; some things can be expressed more concise in C.

But obviously not every #if and #ifdef can be replaced. Accessing struct members that are not present under some conditions will not work without, to just give one example. But where it can be used (without any solid technical reason against it), it will objectively not make the code less readable.

Summing up: It has some advantages and no disadvantages. That is a pretty easy decision to me.

The current wording is "In order to follow Linux's recommendation [...]". That is not that binding.

Yes but you write

Do we really start, once this PR is merged (or even before), asking the contributor to apply this new rule even though it is logically the same code?

I'd say yes. Applying this only to a subset of PRs will only confuse everyone.

I mean it doesn't hurt to suggest a change if it makes the code more readable, but this shouldn't be a requirement that is enforced no matter if it makes sense for that particular section of code or not.

So the wording of this PR is fine. It's an option that can be used to make the code more readable, just because this option exists doesn't mean we must stomp out all #ifdefs.

In no cases it will get less maintainable, but in many cases it will become more maintainable. […]

This is somewhat of a contradiction 😕 but I get what you mean

[…] E.g. the compiler will verify the syntax of all code flavors and generate warnings for all code flavors if they are visible to the compiler. A #ifdef will make the compiler blind. Also: The preprocessor is pretty limited; some things can be expressed more concise in C.

But obviously not every #if and #ifdef can be replaced. Accessing struct members that are not present under some conditions will not work without, to just give one example. But where it can be used (without any solid technical reason against it), it will objectively not make the code less readable.

Don't get me wrong: I'm always in favor of making the compiler spot things it usually wouldn't find! What I want to prevent is bugging contributors (especially those waiting patiently for ages to get their PR finally in) with such details especially if their tests might already cover the "blinded compiler" problem already (e.g. when I use #ifdefs I always make sure all compile paths are tested).

I mean it doesn't hurt to suggest a change if it makes the code more readable, but this shouldn't be a requirement that is enforced no matter if it makes sense for that particular section of code or not.

We'll, to me a coding convetion is not about providing tools, but consistency. E.g. I really like smart tabs for indent. I also prefer the Linux kernel coding convention in regard to typedefs and consider the rationale very convincing. Finally, I don't really care about C++ compatibility. Should I be allowed to just break the coding convention by using smart tabs, no typedefs and no C++ boilerplate? I think making the coding convention optional will only do harm.

But obviously, there are always cases when the coding convention cannot be reasonably applied. E.g. blindly enforcing the 80 chars limit everywhere is going to be headache. In some cases this just reduces the readability quite significantly. And the case of replacing #ifs and #ifdefs does not differ from the others to me qualitatively, only quantitatively: There will be clearly many exceptions.

How about adding an amendment for common sense in general: #12785. This could also be cited in discussions later on ;-)

But honestly: The time and effort required to update existing PRs to use IS_USED() where reasonable is less than complaining about it takes. And if a reviewer insists on getting rid of a specific #if or #ifdef, just ask the reviewer to suggest a better version. Either the reviewer will back off, or (even better!) provides a better solution ;-)

Again, I'm not arguing for softening the coding conventions. I just have a very bad feeling with enforcing a recently added feature via the coding convention.

I just have a very bad feeling with enforcing a recently added feature via the coding convention.

OK, count me convinced. How about just letting this unspecified and let the reviewer decide whether it would be beneficial and not to frustrating to apply it for old PR?

@miri64, as far as i understand you, your question is independent of the current PR.

In this case it is dependent of this PR, IMHO. This PR updates the coding conventions—which some maintainers enforce more rigorously than others—with a guideline to prefer a feature that was proposed and merged quite recently (#12626 was opened on Nov 1st and merged on Nov 11th) over the current state of the art (that is based on a more common C-based approach). But I agree that my concerned should not block this PR, just be taken into account when reviewing older PRs (that's why I didn't put a change request on this PR in the first place ;-)).

not sure about your argumentation @miri64. that a feature was merged recently, might lead to lower adaption compared to features that exist since ages. still, as long as usage is not enforced via the Coding Conventions, you cannot assume that developers use it. ... and again, your concerns don't relate specifically to this feature but to all changes that refer to recently introduced features.

anyhow, seems that we have consensus to merge and close, right?

not sure about your argumentation @miri64. that a feature was merged recently, might lead to lower adaption compared to features that exist since ages. still, as long as usage is not enforced via the Coding Conventions, you cannot assume that developers use it. ... and again, your concerns don't relate specifically to this feature but to all changes that refer to recently introduced features.

I am not against using this feature for newer PR and applying its usage for code already in master. I am however against bugging contributors of long open PRs about this (unless the PR stays open even longer). And yes, this is also related to potential other additions to the coding conventions, but was caused by this PR.

anyhow, seems that we have consensus to merge and close, right?

Yes.

I am not against using this feature for newer PR and applying its usage for code already in master. I am however against bugging contributors of long open PRs about this (unless the PR stays open even longer).

we already agreed on this ;).

Maybe some stronger wording would be nice: "The code should only contain #ifs and #ifdefs were solid technical arguments can justify their use."

I dissociate myself from my previous statement. If that gets interpreted as "documentation is needed for every #if and #ifdef", a lot of time better spent improving code is wasted. (Also: Comments like "a c-style conditional cannot be used to only conditionally add a member to a struct, thus the preprocessor has to be used here" will not really increase readability of the code.)

I'm still in favor to default to the c-style conditional, though.

For the record, I don't like this, I'm not gonna use this in code I write. And I'm not thrilled to see members of the community that don't write, read or review code to weigh in on decisions like this.

@kaspar030, i was asked to articulate my opinion. i did.

the PR is a low brainer, even for me.

Except you, @kaspar030, and me, i count four people who don't object the PR.

Ok, 3 ACKs and rough consensus. Let's get this one merged.