<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">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:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span class="">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:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div bgcolor="#FFFFFF" text="#000000"><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 class=""><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div bgcolor="#FFFFFF" text="#000000"><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><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><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;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><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><br></div><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.</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span class=""><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div bgcolor="#FFFFFF" text="#000000"><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><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><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>Thanks,</div><div>--Serge</div><div><div class="h5"><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div bgcolor="#FFFFFF" text="#000000">
<br>
Thanks again,<br>
Hal<span><br>
<br>
<blockquote type="cite">
<div dir="ltr">
<div><br>
</div>
<div>Using special option for config files does not bring risk
of compatibility breakage and does not change meaning of
existing options.</div>
<div><br>
</div>
</div>
<div class="gmail_extra"><br clear="all">
<div>
<div class="m_-1612313898778346008m_5554482854794619793m_-4846745411548960104gmail_signature" data-smartmail="gmail_signature">Thanks,<br>
--Serge<br>
</div>
</div>
<br>
<div class="gmail_quote">2017-05-10 11:25 GMT+07:00 Serge Pavlov
<span dir="ltr"><<a href="mailto:sepavloff@gmail.com" target="_blank">sepavloff@gmail.com</a>></span>:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote"><span>2017-05-10 3: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>
</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 class="m_-1612313898778346008m_5554482854794619793m_-4846745411548960104m_-4899742039534227284gmail-"><span>On 1 March 2017 at 02:50, Serge
Pavlov via Phabricator <span dir="ltr"><<a href="mailto:reviews@reviews.llvm.org" target="_blank">reviews@reviews.llvm.org</a>></span>
wrote:<br>
</span><span>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><br>
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:<br>
<br>
- it is searched for in well-known
places rather than in current directory,<br>
</blockquote>
<div><br>
</div>
</span></span><span>
<div>This (and suppressing unused-argument
warnings) might well be sufficient to
justify a different command-line syntax
rather than @file...</div>
</span></div>
</div>
</div>
</blockquote>
<div><br>
</div>
<div>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.</div>
<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"><span class="m_-1612313898778346008m_5554482854794619793m_-4846745411548960104m_-4899742039534227284gmail-">
<div> </div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
- it may contain comments, long options
may be split between lines using
trailing backslashes,<br>
- other files may be included
by `@file` and they will be resolved
relative to the including file,<br>
</blockquote>
<div><br>
</div>
</span>
<div>... 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.</div>
</div>
</div>
</div>
</blockquote>
<div><br>
</div>
</span>
<div>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:</div>
<div><br>
</div>
<div> clang --config a.cfg -opt1 -opt2 file1.cpp</div>
<div> clang -opt1 -opt2 file1.cpp --config a.cfg</div>
<div><br>
</div>
<div>are equivalent, but variants with `@file` can
have different effect.</div>
<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"><span class="m_-1612313898778346008m_5554482854794619793m_-4846745411548960104m_-4899742039534227284gmail-">
<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">
- the file may be encoded in executable
name,<br>
- unused options from configuration file
do not produce warnings.<br>
<br>
<br>
<a href="https://reviews.llvm.org/D24933" rel="noreferrer" target="_blank">https://reviews.llvm.org/D2493<wbr>3</a><br>
<br>
<br>
<br>
</blockquote>
</span></div>
<br>
</div>
</div>
</blockquote>
</span></div>
<br>
</div>
</div>
</blockquote>
</div>
<br>
</div>
</blockquote>
<br>
</span><span class="m_-1612313898778346008m_5554482854794619793HOEnZb"><font color="#888888"><pre class="m_-1612313898778346008m_5554482854794619793m_-4846745411548960104moz-signature" cols="72">--
Hal Finkel
Lead, Compiler Technology and Programming Languages
Leadership Computing Facility
Argonne National Laboratory</pre>
</font></span></div>
</blockquote></div></div></div><br></div></div>
<br>______________________________<wbr>_________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/cfe-commits</a><br>
<br></blockquote></div><br></div></div>