llunak added a comment. In D63508#1558390 <https://reviews.llvm.org/D63508#1558390>, @rsmith wrote:
> Patch generally looks good; just a minor concern about the output format. > Also, we don't need parentheses around the constant 0 or 1. That was consistent with what the current code does, but I don't mind changing that. > Adding an extra line here is going to mess up presumed line numbering. That's ok. The code already inserts extra lines and always fixes them up using line markers. And I think that's unavoidable due to reasons below. > Perhaps instead we could prepend a #if 0 /* or #if 1 /* to the directive: That's fragile. Do it with something like "#if value /*blah*/ " and you'll have a problem with comment nesting. That is why all the changes are inserted between the two extra #if 0 / #endif lines. IIRC I thought about this for a while when designing -frewrite-includes and this is the best I came up with. But you made me realize that the previous patch didn't handle this properly, so the current one has been fixed to be consistent there. > I don't think we really need the "evaluated by -frewrite-includes" part, but > I have no objection to including it. It's strictly speaking not necessary, but it's there to make it easier to identify those extra #if lines added by -frewrite-includes. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63508/new/ https://reviews.llvm.org/D63508 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
