[PATCH] D24933: Enable configuration files in clang

Hal Finkel via cfe-commits cfe-commits at lists.llvm.org
Sun Aug 6 11:44:19 PDT 2017


On 08/06/2017 01:15 PM, Serge Pavlov wrote:
> 2017-08-06 6:43 GMT+07:00 Hal Finkel <hfinkel at anl.gov 
> <mailto:hfinkel at anl.gov>>:
>
>     On 07/24/2017 10:18 AM, Serge Pavlov wrote:
>
>>     I am thinking about reducing the patch further to leave only the
>>     ability to include config file when clang is called as
>>     `target-clang-drivermode`. It is still useful for cross
>>     compilation tasks because:
>>     - It is a convenient way to switch between supported targets,
>>     - SDK producer can ship compiler with a set of appropriate
>>     options or prepare them during installation.
>>     In this case if clang is called as `target-clang-drivermode`, it
>>     first tries to find file `target-drivermode.cfg` or `target.cfg`
>>     in a set of well-known directories, which in minimal case
>>     includes the directory where clang executable resides. If such
>>     file is found, options are  read from it, otherwise only option
>>     --target is added as clang does it now.
>>
>>     This solution has obvious drawbacks:
>>     - User cannot specify config file in command line in the same way
>>     as he can choose a target: `clang --target <target>`,
>>     - On Windows symlinks are implemented as file copy, the solution
>>     looks awkward.
>>     So more or less complete solution needs to allow specifying
>>     config file in command line.
>
>     I'd rather not reduce the patch in this way, and you didn't
>     describe why you're considering reducing the patch. Can you please
>     elaborate?
>
>
> The only intent was to facilitate review process.

As someone who's worked on reviewing the patches, I don't think this 
makes things any easier or harder. Once we decide on what we want to do, 
the rest of the review process should be straightforward.

>>
>>     Using `@file` has some problems. Config file is merely a set of
>>     options, just as file included by `@file`. Different include file
>>     search is only a convenience and could be sacrificed. Comments
>>     and unused option warning suppression could be extended for all
>>     files included with `@file`. The real problem is the search path.
>>     To be useful, config files must be searched for in well-known
>>     directories, so that meaning of `clang @config_fille` does not
>>     depend on the current directory. So clang must have some rule to
>>     distinguish between config file and traditional use of `@file`.
>>     For instance, if file name ends with `.cfg` and there is a file
>>     with this name in config search directories, this is a config
>>     file and it is interpreted a bit differently. Of course, the file
>>     may be specified with full path, but this way is inconvenient.
>
>     I see no reason why we can't unify the processing but have
>     different search-path rules for @file vs. --config file.
>
>
> Now I think we can use @file without breaking compatibility.
>
> libiberty resolves `file` in `@file` always relative to current 
> directory. If such file is not found, it tries to open file with name 
> `@file`. We must keep this behavior for the sake of compatibility. If 
> after these steps `file` is not found and `file` does not contain 
> directory separator, clang could try to treat `file` as config file 
> and search it using special search path. If such solution is 
> acceptable, we can get rid of `--config`.

I think that I'd prefer --config to this scheme. For one thing, it means 
that if I have a wrapper script that adds --config foo, this will break 
if the user happens to have a file named foo in their directory. I think 
that unifying the implementation of @foo and --config foo is a good 
idea, but combining them all into the same interface is not obviously 
optimal.

Thanks again,
Hal

>
>>     Another possible solution is to extend meaning of `--target` so
>>     that it fully matches with the use of `target-clang-drivermode`,
>>     that is the option `--target=hexagon` causes clang first to look
>>     for the file `hexagon.cfg` in well-known directories and use it
>>     if found. In this case treatment of `--target` is different if
>>     the option is specified in command line or in the content of
>>     config file (in the latter case it is processed as target name
>>     only), it may be confusing. Besides, use of config files is not
>>     restricted to the choice of target.
>
>     I think we should do this, so long as the implementation is
>     reasonable, and the special case doesn't bother me in this regard.
>     I don't view this as a replacement for '--config file', however,
>     because, as you mention, the config files need not be restricted
>     to target triples.
>
>
> Different treatment of  `--target` in config file and in command line 
> is still a concern, to do or not to do this depends on which is looks 
> more intuitive. I would try implementing it is a separate patch.
>
> Thanks,
> --Serge
>
>
>     Thanks again,
>     Hal
>
>>
>>     Using special option for config files does not bring risk of
>>     compatibility breakage and does not change meaning of existing
>>     options.
>>
>>
>>     Thanks,
>>     --Serge
>>
>>     2017-05-10 11:25 GMT+07:00 Serge Pavlov <sepavloff at gmail.com
>>     <mailto:sepavloff at gmail.com>>:
>>
>>         2017-05-10 3:46 GMT+07:00 Richard Smith
>>         <richard at metafoo.co.uk <mailto:richard at metafoo.co.uk>>:
>>
>>             On 1 March 2017 at 02:50, Serge Pavlov via Phabricator
>>             <reviews at reviews.llvm.org
>>             <mailto:reviews at reviews.llvm.org>> wrote:
>>
>>
>>                 Format of configuration file is similar to file used
>>                 in the construct `@file`, it is a set of options.
>>                 Configuration file have advantage over this construct:
>>
>>                 - it is searched for in well-known places rather than
>>                 in current directory,
>>
>>
>>             This (and suppressing unused-argument warnings) might
>>             well be sufficient to justify a different command-line
>>             syntax rather than @file...
>>
>>
>>         Construct `@file` in this implementation is used only to read
>>         parts of config file inside containing file. Driver knows
>>         that it processes config file and can adjust treatment of
>>         `@file`. On the other hand, driver might parse config files
>>         in a more complicated way, for instance, it could treat line
>>         `# include(file_name)` as a command to include another file.
>>
>>                 - it may contain comments, long options may be split
>>                 between lines using trailing backslashes,
>>                 - other files may be included by `@file` and they
>>                 will be resolved relative to the including file,
>>
>>
>>             ... but I think we should just add these extensions to
>>             our @file handling, and then use the exact same syntax
>>             and code to handle config files and @file files. That is,
>>             the difference between @ and --config would be that the
>>             latter looks in a different directory and suppresses
>>             "unused argument" warnings, but they would otherwise be
>>             identical.
>>
>>
>>         Changing treatment of `@file` can cause compatibility issues,
>>         in particular, both libiberty and cl resolves file name
>>         relative to current directory. So driver must deduce that
>>         `@file` is used to load config file rather than merely to
>>         organize arguments. Another difference is that `@file`
>>         inserts its content in the place where it occurs, while
>>         `--config` always puts arguments before user specified
>>         options. The following invocations:
>>
>>             clang --config a.cfg -opt1 -opt2 file1.cpp
>>             clang -opt1 -opt2 file1.cpp --config a.cfg
>>
>>         are equivalent, but variants with `@file` can have different
>>         effect.
>>
>>
>>                 - the file may be encoded in executable name,
>>                 - unused options from configuration file do not
>>                 produce warnings.
>>
>>
>>                 https://reviews.llvm.org/D24933
>>                 <https://reviews.llvm.org/D24933>
>>
>>
>>
>>
>>
>>
>
>     -- 
>     Hal Finkel
>     Lead, Compiler Technology and Programming Languages
>     Leadership Computing Facility
>     Argonne National Laboratory
>
>

-- 
Hal Finkel
Lead, Compiler Technology and Programming Languages
Leadership Computing Facility
Argonne National Laboratory

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170806/5b93a6ba/attachment-0001.html>


More information about the cfe-commits mailing list