A part of the unit test CommandLineTest/BadResponseFile, added in
the commit fd3d7a9f8c (Handle errors in expansion of response files)
fails on AIX. The test checks if an error is issued when `file` in
`@file` is a path to directory. This change excludes that check on
AIX platform.
Differential Revision: https://reviews.llvm.org/D136090
Previously an error raised during an expansion of response files (including
configuration files) was ignored and only the fact of its presence was
reported to the user with generic error messages. This made it difficult to
analyze problems. For example, if a configuration file tried to read an
inexistent file, the error message said that 'configuration file cannot
be found', which is wrong and misleading.
This change enhances handling errors in the expansion so that users
could get more informative error messages.
Differential Revision: https://reviews.llvm.org/D136090
Previously an error raised during an expansion of response files (including
configuration files) was ignored and only the fact of its presence was
reported to the user with generic error messages. This made it difficult to
analyze problems. For example, if a configuration file tried to read an
inexistent file, the error message said that 'configuration file cannot
be found', which is wrong and misleading.
This change enhances handling errors in the expansion so that users
could get more informative error messages.
Differential Revision: https://reviews.llvm.org/D136090
Functions that implement expansion of response and config files depend
on many options, which are passes as arguments. Extending the expansion
requires new options, it in turn causes changing calls in various places
making them even more bulky.
This change introduces a class ExpansionContext, which represents set of
options that control the expansion. Its methods implements expansion of
responce files including config files. It makes extending the expansion
easier.
No functional changes.
Differential Revision: https://reviews.llvm.org/D132379
Functions that implement expansion of response and config files depend
on many options, which are passes as arguments. Extending the expansion
requires new options, it in turn causes changing calls in various places
making them even more bulky.
This change introduces a class ExpansionContext, which represents set of
options that control the expansion. Its methods implements expansion of
responce files including config files. It makes extending the expansion
easier.
No functional changes.
Differential Revision: https://reviews.llvm.org/D132379
Clang has support of virtual file system for the purpose of testing, but
treatment of config files did not use it. This change enables VFS in it
as well.
Differential Revision: https://reviews.llvm.org/D132867
LLVM contains a helpful function for getting the size of a C-style
array: `llvm::array_lengthof`. This is useful prior to C++17, but not as
helpful for C++17 or later: `std::size` already has support for C-style
arrays.
Change call sites to use `std::size` instead.
Differential Revision: https://reviews.llvm.org/D133429
Prefer using these accessors to access the special sub-commands
corresponding to the top-level (no subcommand) and all sub-commands.
This is a preparatory step towards removing the use of ManagedStatic:
with a subsequent change, these global instances will be moved to
be regular function-scope statics.
It is split up to give downstream projects a (albeit short) window in
which they can switch to using the accessors in a forward-compatible
way.
Differential Revision: https://reviews.llvm.org/D129118
Bugzilla #47579: if you invoke clang on Windows via a pathname in
which a quoted section closes just after a backslash, e.g.
"C:\Program Files\Whatever\"clang.exe
then cmd.exe and CreateProcess will correctly find the binary, because
when they parse the program name at the start of the command line,
they don't regard the \ before the " as having any kind of escaping
effect. This is different from the behaviour of the Windows standard C
library when it parses the rest of the command line, which would
consider that \" not to close the quoted string.
But this confuses windows::GetCommandLineArguments, because the
Windows API function GetCommandLineW() will return a command line
containing that \" sequence, and cl::TokenizeWindowsCommandLine will
tokenize the whole string according to the C library's rules. So it
will misidentify where the program name stops and the arguments start.
To fix this, I've introduced a new variant function
cl::TokenizeWindowsCommandLineFull(), intended to be applied to the
string returned from GetCommandLineW(). It parses the first word of
the command line according to CreateProcess's rules, considering \ to
never be an escaping character; thereafter, it switches over to the C
library rules for the rest of the command line.
Reviewed By: hans
Differential Revision: https://reviews.llvm.org/D122914
When cl::TokenizeWindowsCommandLine received a command line with an
unterminated double-quoted string at the end, it would discard the
text within that string. That doesn't match the behavior of the
standard Windows C library, which will return the text in the unclosed
quoted string as an argv word.
Fixed, and added extra unit tests in that area.
In some cases (specifically the one in Bugzilla #47579) this could
cause TokenizeWindowsCommandLine to return a zero-length list of
arguments, leading to an array overrun at the call site in
windows::GetCommandLineArguments. Added a check there, for extra
safety: now windows::GetCommandLineArguments will return an error code
instead of failing an assertion.
(This change was written as part of https://reviews.llvm.org/D122914,
but split into a separate commit at the last minute at the code
reviewer's suggestion, because it's fixing an unrelated bug in the
same area. The rest of D122914 will follow in the next commit.)
by invoking `SupportTests --gtest_shuffle=1`.
`HideUnrelatedOptions`/`HideUnrelatedOptionsMulti` failed due to other
tests calling `cl::ResetCommandLineParser()` which causes default
options to be removed.
`ExitOnError` would hang due to the threading environment. Renaming it
as `*Deathtest` is the recommended practice by GTest docs.
Early adoption of new technologies or adjusting certain code generation/IR optimization thresholds
is often available through some cl::opt options (which have unstable surfaces).
Specifying such an option twice will lead to an error.
```
% clang -c a.c -mllvm -disable-binop-extract-shuffle -mllvm -disable-binop-extract-shuffle
clang (LLVM option parsing): for the --disable-binop-extract-shuffle option: may only occur zero or one times!
% clang -c a.c -mllvm -hwasan-instrument-reads=0 -mllvm -hwasan-instrument-reads=0
clang (LLVM option parsing): for the --hwasan-instrument-reads option: may only occur zero or one times!
% clang -c a.c -mllvm --scalar-evolution-max-arith-depth=32 -mllvm --scalar-evolution-max-arith-depth=16
clang (LLVM option parsing): for the --scalar-evolution-max-arith-depth option: may only occur zero or one times!
```
The option is specified twice, because there is sometimes a global setting and
a specific file or project may need to override (or duplicately specify) the
value.
The error is contrary to the common practice of getopt/getopt_long command line
utilities that let the last option win and the `getLastArg` behavior used by
Clang driver options. I have seen such errors for several times. I think the
error just makes users inconvenient, while providing very little value on
discouraging production usage of unstable surfaces (this goal is itself
controversial, because developers might not want to commit to a stable surface
too early, or there is just some subtle codegen toggle which is infeasible to
have a driver option). Therefore, I suggest we drop the diagnostic, at least
before the diagnostic gets sufficiently better support for the overridding needs.
Removing the error is a degraded error checking experience. I think this error
checking behavior, if desirable, should be enabled explicitly by tools. Users
preferring the behavior can figure out a way to do so.
Reviewed By: jhenderson, rnk
Differential Revision: https://reviews.llvm.org/D120455
Current declaration of cl::opt is incoherent between class and non-class
specializations of the opt_storage template. There is an inconsistency
in the initialization of the Default field: for inClass instances
the default constructor is used - it sets the Optional Default field to
None; though for non-inClass instances the Default field is set to
the type's default value. For non-inClass instances it is impossible
to know if the option is defined with cl::init() initializer or not:
cl::opt<int> i1("option-i1");
cl::opt<int> i2("option-i2", cl::init(0));
cl::opt<std::string> s1("option-s1");
cl::opt<std::string> s2("option-s2", cl::init(""));
assert(s1.Default.hasValue() != s2.Default.hasValue()); // Ok
assert(i1.Default.hasValue() != i2.Default.hasValue()); // Fails
This patch changes constructor of the non-class specializations to keep
the Default field unset (that is None) rather than initialize it with
DataType().
Reviewed By: lattner
Differential Revision: https://reviews.llvm.org/D114645
opt::setDefaultImpl() is changed to set the option value to the option
type's default if the Default field is not set. This results in option
value reset by Option::reset() or ResetAllOptionOccurrences() even if
the cl::init() is not specified.
Example:
StackOption<std::string> Str("str"); // No cl::init().
Str = "some value";
cl::ResetAllOptionOccurrences();
EXPECT_EQ("", Str); // The Str is reset.
Reviewed By: lattner
Differential Revision: https://reviews.llvm.org/D115433
opt::setDefaultImpl() is changed to set the option value to the option
type's default if the Default field is not set. This results in option
value reset by Option::reset() or ResetAllOptionOccurrences() even if
the cl::init() is not specified.
Example:
StackOption<std::string> Str("str"); // No cl::init().
Str = "some value";
cl::ResetAllOptionOccurrences();
EXPECT_EQ("", Str); // The Str is reset.
Reviewed By: lattner
Differential Revision: https://reviews.llvm.org/D115433
This patch fixes two issues with clearing of the internal storage for cl::bits
1. The internal bits storage for cl::bits is uninitialized. This is a problem if a cl::bits option is not defined with static lifetime.
2. ResetAllOptionOccurrences does not reset cl::bits options.
The latter is also discussed in:
https://lists.llvm.org/pipermail/llvm-dev/2021-February/148299.html
Differential Revision: https://reviews.llvm.org/D119066
This reverts commit ef82063207.
- It conflicts with the existing llvm::size in STLExtras, which will now
never be called.
- Calling it without llvm:: breaks C++17 compat
The error messages in tests are far better when a test fails if the test
is written using ASSERT_/EXPECT_<operator>(A, B) rather than
ASSERT_/EXPECT_TRUE(A <operator> B).
This commit updates all of llvm/unittests/Support to use these macros
where possible.
This change has not been possible in:
- llvm/unittests/Support/FSUniqueIDTest.cpp - due to not overloading
operators beyond ==, != and <.
- llvm/unittests/Support/BranchProbabilityTest.cpp - where the unchanged
tests are of the operator overloads themselves.
There are other possibilities of this conversion not being valid, which
have not applied in these tests, as they do not use NULL (they use
nullptr), and they do not use const char* (they use std::string or
StringRef).
Reviewed By: mubashar_
Differential Revision: https://reviews.llvm.org/D117319
Extends response file expansion to recognize `<CFGDIR>` and expand to the
current file's directory. This makes it much easier to author clang config
files rooted in portable, potentially not-installed SDK directories.
A typical use case may be something like the following:
```
# sample_sdk.cfg
--target=sample
-isystem <CFGDIR>/include
-L <CFGDIR>/lib
-T <CFGDIR>/ldscripts/link.ld
```
Reviewed By: sepavloff
Differential Revision: https://reviews.llvm.org/D115604
This normalizes most paths (except ones input from the user as command
line arguments) into the preferred form, if `real_style()` evaluates to
`windows_forward`.
Differential Revision: https://reviews.llvm.org/D111880
Reset cl::Positional, cl::Sink and cl::ConsumeAfter options as well in cl::ResetCommandLineParser().
Reviewed By: rriddle, sammccall
Differential Revision: https://reviews.llvm.org/D103356
We can build it with -Werror=global-constructors now. This helps
in situation where libSupport is embedded as a shared library,
potential with dlopen/dlclose scenario, and when command-line
parsing or other facilities may not be involved. Avoiding the
implicit construction of these cl::opt can avoid double-registration
issues and other kind of behavior.
Reviewed By: lattner, jpienaar
Differential Revision: https://reviews.llvm.org/D105959
We can build it with -Werror=global-constructors now. This helps
in situation where libSupport is embedded as a shared library,
potential with dlopen/dlclose scenario, and when command-line
parsing or other facilities may not be involved. Avoiding the
implicit construction of these cl::opt can avoid double-registration
issues and other kind of behavior.
Reviewed By: lattner, jpienaar
Differential Revision: https://reviews.llvm.org/D105959
We can build it with -Werror=global-constructors now. This helps
in situation where libSupport is embedded as a shared library,
potential with dlopen/dlclose scenario, and when command-line
parsing or other facilities may not be involved. Avoiding the
implicit construction of these cl::opt can avoid double-registration
issues and other kind of behavior.
Reviewed By: lattner, jpienaar
Differential Revision: https://reviews.llvm.org/D105959
C++23 will make these conversions ambiguous - so fix them to make the
codebase forward-compatible with C++23 (& a follow-up change I've made
will make this ambiguous/invalid even in <C++23 so we don't regress
this & it generally improves the code anyway)
This is a mechanical change. This actually also renames the
similarly named methods in the SmallString class, however these
methods don't seem to be used outside of the llvm subproject, so
this doesn't break building of the rest of the monorepo.
CommandLine.h is indirectly included in ~50% of TUs when building
clang, and VirtualFileSystem.h is large.
(Already remarked by jhenderson on D70769.)
No behavior change.
Differential Revision: https://reviews.llvm.org/D100957
As noted in https://reviews.llvm.org/D93459, the formatting of
multi-line descriptions of clEnumValN and the likes is unfavorable.
Thus this patch adds support for correctly indenting these.
Reviewed By: serge-sans-paille
Differential Revision: https://reviews.llvm.org/D93494
Add unit tests for this behavior, since the integration test for
clang-cl did not catch these bugs.
Fixes PR47604
Differential Revision: https://reviews.llvm.org/D90866
Some LLVM unit tests forget to clean up temporary files and
directories. Introduce RAII classes for cleaning them up.
Refactor the tests to use those classes.
Differential Revision: https://reviews.llvm.org/D83228
This reapplies commit d4020ef7c4, reverted in ac0edc5588 because it
broke build of LLDB. This commit contains appropriate changes for LLDB.
The original commit message is below.
Documentation on CreateProcessW states that maximal size of command line
is 32767 characters including ternimation null character. In the
function llvm::sys::commandLineFitsWithinSystemLimits this limit was set
to 32768. As a result if command line was exactly 32768 characters long,
a response file was not created and CreateProcessW was called with
too long command line.
Differential Revision: https://reviews.llvm.org/D83772
Documentation on CreateProcessW states that maximal size of command line
is 32767 characters including ternimation null character. In the
function llvm::sys::commandLineFitsWithinSystemLimits this limit was set
to 32768. As a result if command line was exactly 32768 characters long,
a response file was not created and CreateProcessW was called with
too long command line.
Differential Revision: https://reviews.llvm.org/D83772
Skip 'really hidden' options when performing lookup of the nearest
option when invalid option was passed. Since these options aren't even
documented in --help-hidden, it seems inconsistent to suggest them
to users.
This fixes clang-tools-extra test failures due to unexpected suggestions
when linking the tools to LLVM dylib (that provides more options than
the subset of LLVM libraries linked directly).
Differential Revision: https://reviews.llvm.org/D82001
Patch by Neil Dhar <dhar@alumni.duke.edu>
Current state machine for parsing tokens from response files in Windows
does not correctly handle the case where the last token is "". The current
implementation handles the last token by only adding it if it is not empty,
however this does not cover the case where the last token is meant to be
the empty string. We can cover this case by checking whether the state
machine was last in the UNQUOTED state, which indicates that the last
character of the input was a non-whitespace character.
Differential Revision: https://reviews.llvm.org/D78346
Summary:
Currently, cl::ConsumeAfter only works for the case that has exactly one
positional argument. Without the fix, it skip fulfilling first positional
argument and put that additional positional argument in interpreter arguments.
Reviewers: bkramer, Mordante, rnk, lattner, beanz, craig.topper
Reviewed By: rnk
Subscribers: JosephTremoulet, hiraditya, llvm-commits
Tags: #llvm
Differential Revision: https://reviews.llvm.org/D77242
Kazu Hirata