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: Update pass manager to handle generate_summary/transform functions


>>>>> "Rafael" == Rafael Espindola <espindola@google.com> writes:

Thanks for writing this.

Rafael> *) in_system_headers and location are not kept in perfect sync. In
Rafael> particular, the function "warning" should probably take a location_t
Rafael> argument informing where the warning happened.

I didn't realize this.  This is pretty gross.

I was hoping that the symbol 'in_system_header' could completely
vanish, but I see why you chose not to do this.

I am curious how you found the places where input_location and
in_system_header are out of sync.  If it was from running the test
suite, then my worry is that this is insufficient -- this seems like
something we would not test exhaustively.

Rafael> With this change it is no longer possible to change just the
Rafael> in_system_header of a decl. We could create a global
Rafael> dummy_system_header_location and set the location to that, but
Rafael> I don't think we need to.

I agree.  IMO the system-header-ness of a decl should not be mutable.
It isn't as if they can move around.

Rafael> OK for trunk if bootstraps and tests are OK?

I'd like to understand the above before signing off on this; but the
content of the patch is basically ok.

Some minor things below.

Rafael> diff --git a/gcc/c-parser.c b/gcc/c-parser.c
Rafael> index 45aff80..185df29 100644
Rafael> --- a/gcc/c-parser.c
Rafael> +++ b/gcc/c-parser.c
Rafael> @@ -258,7 +258,7 @@ typedef struct c_token GTY (())
Rafael>       was seen.  Otherwise it is PRAGMA_NONE.  */
Rafael>    ENUM_BITFIELD (pragma_kind) pragma_kind : 7;
Rafael>    /* True if this token is from a system header.  */
Rafael> -  BOOL_BITFIELD in_system_header : 1;
Rafael> +  BOOL_BITFIELD is_in_system_header : 1;

Is this field actually used any more?
I think it is probably just assigned to.

Probably this change could be made to the C++ parser, too; you'd just
have to change the one use to check the token's location instead.

Rafael>  #define DECL_IN_SYSTEM_HEADER(NODE) \
Rafael> -  (DECL_WITH_VIS_CHECK (NODE)->decl_with_vis.in_system_header_flag)
Rafael> +  (LOCATION_SYSP(DECL_SOURCE_LOCATION (NODE)) != 0)

Need a space between 'LOCATION_SYSP' and '('.

Rafael>   /* Don't belong to VAR_DECL exclusively.  */
Rafael> - unsigned in_system_header_flag : 1;

There is a comment a few lines below this that documents the number of
unused bits.  This needs an update.

Tom


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