Hi!
Segher Boessenkool <seg...@kernel.crashing.org> writes: > On Mon, Mar 21, 2022 at 02:14:08PM -0400, David Edelsohn wrote: >> On Mon, Mar 21, 2022 at 5:13 AM Jiufu Guo <guoji...@linux.ibm.com> wrote: >> > There is a rare corner case: where __vector is followed only with ";" >> > and near the end of the file. > >> This is okay. Maybe a tweak to the comment, see below. > > This whole function could use some restructuring / rewriting to make > clearer what it actually does. See the function comment: > > /* Called to decide whether a conditional macro should be expanded. > Since we have exactly one such macro (i.e, 'vector'), we do not > need to examine the 'tok' parameter. */ > > ... followed by 17 uses of "tok". Yes, some of those overwrite the > function argument, but that doesn't make it any better! :-P > > Some factoring would help, too, perhaps. Thanks for your review! I am also confused about it when I check this function for the first time. In the function, 'tok' is used directly at the beginning, and then it is overwritten by cpp_peek_token. >From the history of this function, the first version of this function contains this 'inconsistency' between comments and implementations. :-P With check related code, it seems this function is used to predicate if a conditional macro should be expanded by peeking two or more tokens. The context-sensitive macros are vector/bool/pixel. Correponding keywords __vector/__bool/__pixel are unconditional. Based on those related codes, the behavior of function rs6000_macro_to_expand would be checking if the 'vector' token is followed by bool/__bool or pixel/__pixel. To do this the 'tok' has to be 'examined'. >From this understanding, we may just update the comment. While the patch does not cover this. BR, Jiufu > > > Segher
