<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">2017-08-07 1:46 GMT+07:00 Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>></span>:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span>On 6 August 2017 at 11:15, Serge Pavlov via cfe-commits <span dir="ltr"><<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span>2017-08-06 6:43 GMT+07:00 Hal Finkel <span dir="ltr"><<a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a>></span>:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div bgcolor="#FFFFFF"><span>
<p>On 07/24/2017 10:18 AM, Serge Pavlov
wrote:<br></p>
<blockquote type="cite">
<div dir="ltr">
<div>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:</div>
<div><span style="white-space:pre-wrap"> </span>- It is a convenient
way to switch between supported targets,</div>
<div><span style="white-space:pre-wrap"> </span>- SDK producer can
ship compiler with a set of appropriate options or prepare
them during installation.</div>
<div>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.</div>
<div><br>
</div>
<div>This solution has obvious drawbacks:</div>
<div><span style="white-space:pre-wrap"> </span>- User cannot specify
config file in command line in the same way as he can choose a
target: `clang --target <target>`,</div>
<div><span style="white-space:pre-wrap"> </span>- On Windows symlinks
are implemented as file copy, the solution looks awkward.</div>
<div>So more or less complete solution needs to allow specifying
config file in command line.</div>
</div>
</blockquote>
<br></span>
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?<span><br></span></div></blockquote><div><br></div></span><div>The only intent was to facilitate review process. </div><span><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div bgcolor="#FFFFFF"><span>
<blockquote type="cite">
<div dir="ltr">
<div><br>
</div>
<div>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.</div>
</div>
</blockquote>
<br></span>
I see no reason why we can't unify the processing but have different
search-path rules for @file vs. --config file.</div></blockquote><div><br></div></span><div>Now I think we can use @file without breaking compatibility.</div><div><br></div><div>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.</div></div></div></div></blockquote><div><br></div></span><div>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.</div></div></div></div></blockquote><div><br></div><div>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.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>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`.</div></div></div></div></blockquote><div><br></div></span><div>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).</div></div></div></div></blockquote><div><br></div><div> This is <a href="https://reviews.llvm.org/D36420" target="_blank">https://reviews.llvm.org/<wbr>D36420</a></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>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.<br></div></div></div></div></blockquote><div><br></div><div>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 <a href="https://reviews.llvm.org/D36045">https://reviews.llvm.org/D36045</a>). 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.</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div></div><span><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div bgcolor="#FFFFFF"><span><blockquote type="cite"><div dir="ltr"><div>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.<br></div>
</div>
</blockquote>
<br></span>
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.<br></div></blockquote><div><br></div></span><div>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.</div></div></div></div></blockquote><div><br></div></span><div>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".</div></div></div></div></blockquote><div><br></div><div>It is a strong argument against changing treatment of `--target`.</div><div><br></div><div>If I understand the feedback correctly, --config now wins over @file?</div><div><br></div><div>Thanks,</div><div>--Serge</div><div> </div><div><a href="https://reviews.llvm.org/D24933" rel="noreferrer" target="_blank">https://reviews.llvm.org/D2493<wbr>3</a></div></div><br></div></div>