Hello Dodji,
I found time this morning to run your changes through our system. I patched our gcc-4.8.1 with your latest change, and ran it through our folly testsuite. One thing that I immediately noticed was that this increased the preprocessed size substantially. When preprocessing my favorite .cpp file, its .ii grew from 137k lines to 145k, a 5% increase. All the folly code compiled and ran successfully under the changes. I looked at some of the preprocessed output. I was pleased to see that consecutive macros that expanded entirely to system tokens did not insert unnecessary line directives between them. I did, however, notice that __LINE__ was treated as belonging to the calling file, even when its token appears in the system file. That is to say: CODE: // system macro #define FOO() sys_token __LINE__ sys_token // non-system callsite FOO() // preprocessed output # 3 "test.cpp" 3 4 sys_token # 3 "test.cpp" 3 # 3 "test.cpp" 3 4 sys_token :CODE This seems to generalize to other builtin macros, like __FILE__. Otherwise, the code looks fine. There is only one thing I noticed: > + if (do_line_adjustments > + && !in_pragma > + && !line_marker_emitted > + && print.prev_was_system_token != !!in_system_header_at(loc)) > + /* The system-ness of this token is different from the one > + of the previous token. Let's emit a line change to > + mark the new system-ness before we emit the token. */ > + line_marker_emitted = do_line_change (pfile, token, loc, false); This line_marker_emitted assignment is immediately overwritten, two lines below. However, from a maintainability perspective, this is probably a good assignment to keep. > cpp_output_token (token, print.outf); > + line_marker_emitted = false; > } Thanks for this diff! Cheers, Nicholas
