Re: [RFC PATCH] pre-process: add __VA_OPT__ support
Al Viro <[email protected]> Wed, 25 Feb 2026 22:18:51 +0000
| Newsgroups | org.kernel.vger.linux-sparse |
|---|---|
| Message-ID | <20260225221851.GE1762976@ZenIV> |
On Wed, Feb 25, 2026 at 12:14:12AM -0800, Eric Zhang wrote: > For # __VA_OPT__(), a stringify flag on TOKEN_VA_OPT_START could > signal substitute() to stringify or produce "" depending on whether > varargs are empty. IMO it's better to turn that into TOKEN_VA_OPT[<token-list>] and TOKEN_QUOTED_VA_OPT[<token-list>] with list hanging off the cannibalized token. Interpreter (substitute()) can easily keep track of where it is. FWIW, the way they patched __VA_OPT__ into 10.5.1 is unfortunate - I can understand wanting to keep the changes localized, but it ends up very convoluted ;-/ In part it's due to the way ## and # evaluation order is left unspecified, but... ouch. Basically, #__VA_OPT__(<token-list>) is treated the following way: it's "" if va-opt is suppressed, otherwise we * do argument substitution in <token-list> * [unspecified, but everyone does that] process # and ## in the token-list. * do *NOT* remove placemaker tokens * do *NOT* rescan * stringify the resulting token list, same way we would if that token list had been passed as an argument and we were processing #<that argument> (as per 6.10.5.2[3]). ## vs. __VA_OPT__ is similar; the tricky part is placemaker treatment. For normal arguments it's either a non-empty list or a solitary placemaker; here we might have placemakers with non-empty list. #define F1(X, Y, ...) Y ## __VA_OPT__(X X) ## Y #define F2(X, Y, ...) Y ## __VA_OPT__(X) ## Y F1(,a,_) F2(,a,_) We get a ## placemaker placemaker ## a (i.e. a a) or a ## placemaker ## a (i.e. aa) respectively. Approach without explicit ## tokens at expansion time is easy to adapt to that - we just interpret the translated token-list hanging off __VA_OPT__, then return to the rest of the body; state is updated as usual. Quoted __VA_OPT__ == run the interpreter (starting from Normal) on the token-list, then feed that to stringify() and use the result as if it came from quoted argument (note that concatenation with previous token *is* possible - L ## #__VA_OPT__(something) is not invalid). Since __VA_OPT__ can't nest, it's easy to save the body->next into a local variable, set body to body->va_opt_list and, after the main loop check if that local variable is non-NULL. In that case we just set body to that, clear that variable and go back to the beginning of the loop. Quoted ones get a recursive call... NOTE: substitute() is the second hottest loop in the entire thing; only tokenizer is hotter. And gcc is too enthusiastic about the inlining around that function, ending up with bad register spills, along with a bunch of stalls. Worse, decisions are sensitive to minor changes in places textually far away, making it a real bitch to deal with. Makes for fun reordering the commits in local queue... ;-/ > > Another problem is that having no __VA_ARGS__ in the body should > > *not* be treated as "vararg is empty" > > Missed in the test case, but it seems like it can work in the current > version. AFAICS you won't even try to expand it at expand_arguments(), so the reference will remain NULL.