> + for (elem = other_queue; elem ; elem = elem->next) > + { Note that your indentation is off in places.
> + /* We are parsing DEFINE_SUBST_ATTR, which could cause generation > + of DEFINE_ATTR for introduced DEFINE_SUBST. It doesn't happen > + if such DEFINE_ATTR has already been introduced. > + If this did happen, we need to return TRUE to process newly > + introduced DEFINE_ATTR. */ This comment is less than clear. Unclear enough that I don't even know how to suggest re-wording it. We're returning true so that the new define_subst_attr is processed, or that some define_subst is processed? > + while ((start = strchr (end, '<')) && (end = strchr (end, '>'))) > + { > + if (start && end > + && (end - start - 1 > 0) > + && (end - start - 1 < (int)sizeof (tmpstr))) That doesn't look right. (1) end search should be from start, not from the previous end. (2) you know start and end are non-null inside the loop; why are you checking for it again. The bulk of the patch is looking pretty decent now. We may find problems with it when we start to use it, but at the moment it's fairly hard to evaluate completely. --- Looking at all this, I'm wondering if we shouldn't split out all of this macro/include processing to a separate pass. Perform the preprocessing once, early, leaving the processed result in the build directory. Then run the original/traditional rtl reader on that when running the other rtl manipulation passes. The reason being that it's going to become increasingly hard to figure out if the reason for an error is in the macro processing or in the source md file. Being able to see the macro expansion is going to be important. Of course, another way to get this macro expansion is to leave the macro processing where it is and have another gen program that merely dumps the processed rtl. It wouldn't be run normally, but a makefile target used for debugging would be sufficient. --- Which brings us to the question of what to do with the patch for 4.8. It's true that you made the deadline for stage1 closure. But there will be no users of this feature, so it begs the question of why we should apply it now. Have you a convincing reason? RM's do you have an opinion here? r~