This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] PR preprocessor/60723 - missing system-ness marks for macro


Hello Nicholas,

Nicholas Ormrod <nicholas.ormrod@hotmail.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.

Thanks!

> 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.

Cool!

> 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

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__.

Indeed.

>
> 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.

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!

Cheers.

-- 
		Dodji


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]