This is the mail archive of the
mailing list for the GCC project.
Re: [PATCH] PR preprocessor/60723 - missing system-ness marks for macro
- From: Dodji Seketeli <dodji at redhat dot com>
- To: Nicholas Ormrod <nicholas dot ormrod at hotmail dot com>
- Cc: Jason Merrill <jason at redhat dot com>, GCC Patches <gcc-patches at gcc dot gnu dot org>, "Joseph S. Myers" <joseph at codesourcery dot com>, Manuel LÃpez-IbÃÃez <lopezibanez at gmail dot com>, "christophe dot lyon\ at st dot com" <christophe dot lyon at st dot com>
- Date: Fri, 04 Jul 2014 14:13:38 +0200
- Subject: Re: [PATCH] PR preprocessor/60723 - missing system-ness marks for macro
- Authentication-results: sourceware.org; auth=none
- References: <87d2du4xsu dot fsf at redhat dot com> <53ADD6C2 dot 9030200 at redhat dot com> <877g3uo6cq dot fsf at redhat dot com> <COL129-W8177087E5B88848923A685F5010 at phx dot gbl>
Nicholas Ormrod <email@example.com> writes:
> 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.
Yeah, the growth is expected. It's interesting to have actual numbers,
thank you for that.
> 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:
> // system macro
> #define FOO() sys_token __LINE__ sys_token
> // non-system callsite
> // preprocessed output
> # 3 "test.cpp" 3 4
> # 3 "test.cpp"
> # 3 "test.cpp" 3 4
Yeah. For Built-in tokens that are expanded like that we only do track
their the location of their expansion, not their spelling location. So
this behaviour is expected as well.
> 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
Yeah, maintainability is why I kept it. But if the maintainers feel
strongly about it I can, certainly just remove that assignment.
Thank you for your time on this!