Re: [PATCH] Symbol Separation - Take 2

[Neil Booth <neil at daikokuya dot co dot uk>]

Devang Patel wrote:-

> This patch is same as previous patch. Only difference is in
> documentation.
> 
> OK to commit ?

I can't judge on the appropriateness of the patch or not, though
I'm naturally sceptical about these things, but...

>         * cppss.c : New file.

This file name is no good.

>         * cppfiles.c (struct include_file): New member - suppress_dbg.
>         (open_file_pch): Find and validate .cinfo.
>         (stack_include_file): If valid symbol repository is found then
>         suppress debug info by clearing write_symbols.
>         (suppress_dbg_info): New function.
>         (cpp_check_symbol_repository): New function.
>         * cpphash.h (enum cpp_cinfo_state): New.
>         (struct cpp_reader): New member - cinfo_state.
>         * cpplib.c (_cpp_pop_buffer): If debug info. was suppressed for
>         the buffer being popped then restore original status.
>         * cpplib.h (struct cpp_options): New members - make_pch, use_ss
>         and warn_invalid_sr.
>         * cpppch.c (count_defs): Do not count builtins for symbol
>         separation.
>         (write_defs): Do not write builtins for symbol separation.
>         (cpp_valid_state): Issue appropriate warning, when -Winvalid-sr 
> is set.

Similarly "-Winvalid-sr" is no good; practically no other switch is this
non-self-documenting.  Similarly use_ss and any others I've missed.

Without my having to figure out all of your patch, could you briefly
explain _all_ functionality you require of CPP that isn't already present?
Why do you require so many callbacks?

> /* Use PCH technique in future */
> static const char context_ident[8] = "cpp-cntx";
> 
> /* FIX ME : Add in struct include_file */
> /* Hold checksum for the include file. It is included with 
>    BINCL and EINCL stabs.  */
> static unsigned long stabs_checksum;

I thought we agreed cpplib doesn't have statics?  The const might
be an exception, but I can't see what that variable is for.

b.t.w. your method of posting patches makes replies very difficult,
so I'm just going to say it.  We don't want two options that mean
one thing going forwards.  And please do your -g option in common.opt;
all the -g options are going there when I get the time.

Neil.