rsmith added a comment. Patch generally looks good; just a minor concern about the output format.
================ Comment at: clang/lib/Frontend/Rewrite/InclusionRewriter.cpp:487-490 + OS << " - evaluated by -frewrite-includes */" << MainEOL; + OS << (elif ? "#elif" : "#if"); + OS << " (" << (isTrue ? "1" : "0") + << ") /* evaluated by -frewrite-includes */"; ---------------- Adding an extra line here is going to mess up presumed line numbering. Also, we don't need parentheses around the constant `0` or `1`. Perhaps instead we could prepend a `#if 0 /*` or `#if 1 /*` to the directive: ``` #if FOO(x) == \ BAZ #ifndef BAR ``` -> ``` #if 1 /*#if FOO(x) == \ BAZ*/ #if 0 /*#ifndef BAR*/ ``` I don't think we really need the "evaluated by -frewrite-includes" part, but I have no objection to including it. The best place is probably between the `/*` and the `#`: ``` #if 1 /*evaluated by -frewrite-includes: #if FOO(x) == \ BAZ*/ #if 0 /*evaluated by -frewrite-includes: #ifndef BAR*/ ``` 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