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] SYSROOT subdir support


gp@qnx.com wrote:
Here is the new SYSROOT subdir patch.  This results from a discussion
with Daniel Jacobowitz and Alexandre Oliva on the gcc mailing list,
starting at:


Richard and Alex argued over who should approve it, but neither did, so I will go ahead and review it. The concept looks good. It is similar to stuff we already have such as machine_suffix. I have only minor issues with it.

There is a duplicate comment as someone else already mentioned.

You have added two new macros, SYSROOT_SUBDIR_SPEC and SYSROOT_HEADERS_SUBDIR_SPEC, but you didn't add documentation for them. They should be documented in the doc/gcc.texi file near the other SPEC strings.

I think all of the code that checks for directory separators is superfluous. It is a gcc convention that the user specifies the directory separator if he/she wants it. This is for instance why one has to use -Bstage1/ instead of -Bstage1. This allows a little more flexibility, since one could have for instance either a stage1-cc1 or a stage1/cc1 depending on which you prefer. Thus I'd suggest getting rid of all the code that tries to add directory separators. It is a user error if they were needed and the user didn't specify them. This would make the patch a bit shorter. We could perhaps rename the SPECs to use SUFFIX instead of SUBDIR to make this clearer.

The code to initialize sysroot_{,headers}_subdir_spec looks reasonable. It looks a little funny in that if the spec expands to more than one argument, you silently ignore all but the first. That doesn't seem quite right. Maybe give an error here if argbuf_index isn't 0 or 1?

Jim


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