On 6 August 2017 at 11:15, Serge Pavlov via cfe-commits < cfe-commits@lists.llvm.org> wrote:
> 2017-08-06 6:43 GMT+07:00 Hal Finkel <hfin...@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. 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). 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. > 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". > 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 <sepavl...@gmail.com>: >> >>> 2017-05-10 3:46 GMT+07:00 Richard Smith <rich...@metafoo.co.uk>: >>> >>>> On 1 March 2017 at 02:50, Serge Pavlov via Phabricator < >>>> revi...@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 >>>>> >>>>> >>>>> >>>>> >>>> >>> >> >> -- >> Hal Finkel >> Lead, Compiler Technology and Programming Languages >> Leadership Computing Facility >> Argonne National Laboratory >> >> > > _______________________________________________ > cfe-commits mailing list > cfe-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
