On Thu, May 23, 2019 at 6:13 PM Julius Werner jwerner@chromium.org wrote:
- Can we adopt on principle removing all cpp guards on protos?
Why would this be a good idea? What does "cpp guards" actually refer to?
Upon re-reading this I *think* what they mean is stuff like
#if CONFIG(SOME_OPTION) void function_prototype(void); #endif
yes. That's what the question meant: "should we remove cpp guards on protos", as it said. I guess "cpp guards" is just a term of art I grew up with, so did not realize it might confuse. OTOH it gets 2.5M hits.
I asked this question in the meeting because it came up in some reviews, and just for one example, https://review.coreboot.org/c/coreboot/+/32764/3/src/arch/x86/include/arch/a...
and in some of those reviews a number of comments were made that some of the guards on protos could be removed, even in cases where the functions in question were not used or defined. A function prototype essentially being a type declaration, this makes a sort of sense.
I also noticed this from https://review.coreboot.org/c/coreboot/+/30466/29/src/mainboard/sifive/hifiv... +++ b/src/mainboard/sifive/hifive-unleashed/flash.h @@ -0,0 +1,21 @@ +/* + * This file is part of the coreboot project. + * + * Copyright (C) 2018 HardenedLinux + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#ifndef __HIFIVE_UNLEASHED_FLASH_H__ +#define __HIFIVE_UNLEASHED_FLASH_H__ + +void flash_init(void); + +#endif /* __HIFIVE_UNLEASHED_FLASH_H__ */
This one file contains one prototype, i.e. one line, with three lines of cpp stuff to guard it. Is this sensible? I have no idea. But were we to adopt a policy of not guarding prototype declarations, we could, for a cpu or architecture, put all the protos in one file. That's what the Plan 9 authors (kernel written in C, by the inventors of Unix and C, for reference) elected to do. It was incredibly convenient -- all the protos you need were in one place. As weird as it sounds, it's very, very nice. I'm not recommending that for coreboot, just trying to point out what other projects have done.
But the overall observation was that we have guards around protos in some places, and in other places people have argued they can be removed, even when the function in question is not defined or used, and the question I got to wondering was: should we just remove all those guards, like Plan 9, or what?
There are situations where what would have been a compile time error is now a link time error. People might find this confusing.
The discussion evolved to a related question, around #pragma once. A few years back, on the Akaros project (kernel written in C, FWIW), a Linux kernel luminary convinced us to get rid of file guards and go to #pragma once. I am not sure it was worth the trouble but we did it. It *can* speed up compile time; cpp doesn't need to process a whole file and then conclude it did not have to process it; it can realize it can skip the open. A significant downside is that it's not in any standard -- just all the compilers out there, it seems, save romcc.
I did a simple test: apply #pragma once to coreboot. A coreboot build for watson opens 80K .h files today. #pragma once makes barely any difference; this says we are doing a good job in how we use our .h files.
From that point of view #pragma once hardly seems worth it. Either way, the build takes 35 seconds. For compilation speed, I see no advantage. And, again, #pragma once is not in any standard.
And that's it. I got kind of curious about usage of cpp guards, the meeting was on the schedule, I asked the question, and we discussed it. I value the opinions of the people on the call.
I'm sorry it elicited such a strong reaction.
As for the call: it's open. Anyone can call in. I'd be happy if more people did.
ron