[PATCH] D63508: make -frewrite-includes also rewrite conditions in #if/#elif

Sun Jun 30 07:57:47 PDT 2019

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