[PATCH] D24933: Enable configuration files in clang

[Serge Pavlov via cfe-commits]

2017-08-07 1:46 GMT+07:00 Richard Smith <richard at metafoo.co.uk>:

> On 6 August 2017 at 11:15, Serge Pavlov via cfe-commits <
> cfe-commits at lists.llvm.org> wrote:
>
>> 2017-08-06 6:43 GMT+07:00 Hal Finkel <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.
>>
>>>
>>> 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.
>>
>
> Do you know of actual software that depends on the fallback working this
> way? That seems very fragile to me, since a command line that uses @foo to
> name the file ./@foo would change meaning if a file named foo were created.
> Perhaps we should consider the fallback to be a mistake, and require files
> whose name starts with @ to be named as ./@filename, just like we do for
> files whose name starts with a hyphen.
>

Most likely if `@foo` is specified and `foo` is not found this is an error.
And indeed, it just `@foo` is needed, it still can be specified using
slightly modified file name.


>
> 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`.
>>
>
> If we go this way, I think we should also deprecate the @file -> "open
> file with name ./@file" (warn on it for now, with the intent to remove it
> in a future version).
>

 This is https://reviews.llvm.org/D36420

But... I think the concern about @ vs --config is principally around having
> two different file formats, not about having two different command-line
> syntaxes to specify a file, so this may be addressing a non-issue. And I
> think the different use cases provide a decent argument for using different
> search paths (compiler configs should live with the compiler, @-files are
> expected to be generated by the user or the build system so should be found
> relative to the current directory). Keeping the two separate but with a
> unified format and internal mechanism seems like a good approach to me.
>

Format of both files is very simple. Features like comments and line
concatenation are not specific to config files and can be extended for all
@-file, as you proposed earlier (potential implementaion is in
https://reviews.llvm.org/D36045). In this case the only difference is
nested `@file`, in which `file` is resolved relative to containing config
file, not current directory.And yes, implementation is the same.

> 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.
>>
>
> I can see reasonable arguments both ways here. My first impression is that
> keeping --config and --target separate seems like the simpler model. If we
> combine them, it seems like it could be difficult to set up cases like "use
> the configuration for target X-Y-Z, but tweak the precise target to
> actually be X-Y-Q".
>

It is a strong argument against changing treatment of `--target`.

If I understand the feedback correctly, --config now wins over @file?

Thanks,
--Serge

https://reviews.llvm.org/D24933
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170808/6cdffcbf/attachment-0001.html>