This looks fairly odd, as we could have kept the category extension
on the previous line.
Category extensions are a single item, so they are generally very
short compared to protocol lists. We should prefer breaking after the
opening `<` of the protocol list over breaking after the opening `(`
of the category extension.
With this diff, we now avoid breaking after the category extension's
open paren, which causes us to break after the protocol list's
open angle bracket:
Test Plan: New test added. Confirmed test failed before diff and
passed after diff by running:
% make -j16 FormatTests && ./tools/clang/unittests/Format/FormatTests
Ben Hamilton [Thu, 12 Apr 2018 15:11:53 +0000 (15:11 +0000)]
[clang-format] Improve ObjC guessing heuristic by supporting all @keywords
Summary:
This diff improves the Objective-C guessing heuristic by
replacing the hard-coded list of a subset of Objective-C @keywords
with a general check which supports all @keywords.
I also added a few more Foundation keywords which were missing from
the heuristic.
Test Plan: Unit tests updated. Ran tests with:
% make -j16 FormatTests && ./tools/clang/unittests/Format/FormatTests
Ben Hamilton [Thu, 12 Apr 2018 15:11:51 +0000 (15:11 +0000)]
[clang-format] Don't insert space between ObjC class and lightweight generic
Summary:
In D45185, I added clang-format parser support for Objective-C
generics. However, I didn't touch the whitespace logic, so they
got the same space logic as Objective-C protocol lists.
In every example in the Apple SDK and in the documentation,
there is no space between the class name and the opening `<`
for the lightweight generic specification, so this diff
removes the space and updates the tests.
Test Plan: Tests updated. Ran tests with:
% make -j16 FormatTests && ./tools/clang/unittests/Format/FormatTests
Summary:
Currently, indentation of Objective-C method names which are wrapped
onto the next line due to a long return type is controlled by the
style option `IndentWrappedFunctionNames`.
This diff changes the behavior so we always indent wrapped Objective-C
selector names.
NOTE: I partially reverted https://github.com/llvm-mirror/clang/commit/6159c0fbd1876c7f5f984b4830c664cc78f16e2e / rL242484, as it was causing wrapped selectors to be double-indented. Its tests in FormatTestObjC.cpp still pass.
Test Plan: Tests updated. Ran tests with:
% make -j12 FormatTests && ./tools/clang/unittests/Format/FormatTests
Diagnose cases of "return x" that should be "return std::move(x)" for efficiency
Summary:
This patch adds two new diagnostics, which are off by default:
**-Wreturn-std-move**
This diagnostic is enabled by `-Wreturn-std-move`, `-Wmove`, or `-Wall`.
Diagnose cases of `return x` or `throw x`, where `x` is the name of a local variable or parameter, in which a copy operation is performed when a move operation would have been available. The user probably expected a move, but they're not getting a move, perhaps because the type of "x" is different from the return type of the function.
A place where this comes up in the wild is `stdext::inplace_function<Sig, N>` which implements conversion via a conversion operator rather than a converting constructor; see https://github.com/WG21-SG14/SG14/issues/125#issue-297201412
Another place where this has come up in the wild, but where the fix ended up being different, was
where the appropriate fix in that case was to replace `throw ex;` with `throw;`, and incidentally to catch by reference instead of by value. (But one could contrive a scenario where the slicing was intentional, in which case throw-by-move would have been the appropriate fix after all.)
Another example (intentional slicing to a base class) is dissected in https://github.com/accuBayArea/Slides/blob/master/slides/2018-03-07.pdf
**-Wreturn-std-move-in-c++11**
This diagnostic is enabled only by the exact spelling `-Wreturn-std-move-in-c++11`.
Diagnose cases of "return x;" or "throw x;" which in this version of Clang *do* produce moves, but which prior to Clang 3.9 / GCC 5.1 produced copies instead. This is useful in codebases which care about portability to those older compilers.
The name "-in-c++11" is not technically correct; what caused the version-to-version change in behavior here was actually CWG 1579, not C++14. I think it's likely that codebases that need portability to GCC 4.9-and-earlier may understand "C++11" as a colloquialism for "older compilers." The wording of this diagnostic is based on feedback from @rsmith.
**Discussion**
Notice that this patch is kind of a negative-space version of Richard Trieu's `-Wpessimizing-move`. That diagnostic warns about cases of `return std::move(x)` that should be `return x` for speed. These diagnostics warn about cases of `return x` that should be `return std::move(x)` for speed. (The two diagnostics' bailiwicks do not overlap: we don't have to worry about a `return` statement flipping between the two states indefinitely.)
I propose to write a paper for San Diego that would relax the implicit-move rules so that in C++2a the user //would// see the moves they expect, and the diagnostic could be re-worded in a later version of Clang to suggest explicit `std::move` only "in C++17 and earlier." But in the meantime (and/or forever if that proposal is not well received), this diagnostic will be useful to detect accidental copy operations.
David Chisnall [Thu, 12 Apr 2018 06:46:15 +0000 (06:46 +0000)]
ObjCGNU: Fix empty v3 protocols being emitted two fields short
Summary:
Protocols that were being referenced but could not be fully realized were being emitted without `properties`/`optional_properties`. Since all v3 protocols must be 9 processor words wide, the lack of these fields is catastrophic for the runtime.
As an example, the runtime cannot know [here](https://github.com/gnustep/libobjc2/blob/master/protocol.c#L73) that `properties` and `optional_properties` are invalid.
[Sema][ObjC] Ensure that the return type of an ObjC method is a complete
type.
Copy the code in ActOnStartOfFunctionDef that checks a function's return
type to ActOnStartOfObjCMethodDef. This fixes an assertion failure in
IRGen caused by an uninstantiated return type.
Lex: make `clang::Preprocessor::macros` work on MSVC
The order of argument construction is reversed on MS ABI on Windows.
When `macros` was invoked, the `end` call is made prior to `begin`. In
such a case, the DenseMap (`ModuleMap`) is populated after the `end`
iterator is constructed. This reversal results in the invalidation of
the end iterator, resulting in a failure at runtime (assertion failure
in `DenseMap<T>::operator!=` that "handles are not in sync!"). Ensure
that the end iterator is constructed after the begin iterator. This
fixes the use of `macros(bool)`, which symptomized as an assertion
failure in the swift compiler in the clang importer.
The WBNOINVD instruction writes back all modified
cache lines in the processor’s internal cache to main memory
but does not invalidate (flush) the internal caches.
When we enter a __finally block, the CGF's CurCodeDecl will be null
(because CodeGenFunction::StartFunction is given an empty GlobalDecl for
a __finally block), and so the dyn_cast here will result in an assertion
failure. Change it to dyn_cast_or_null to handle this case.
[NVPTX, CUDA] Improved feature constraints on NVPTX target builtins.
When NVPTX TARGET_BUILTIN specifies sm_XX or ptxYY as required feature,
consider those features available if we're compiling for GPU >= sm_XX or have
enabled PTX version >= ptxYY.
Summary:
After a remark on a FreeBSD mailing list that the clang man page did
not have any list of possible values for the `-std=` flag, I have now
attempted to exhaustively list those, for each available language.
I also documented the default standard for each language, if there was
more than one choice.
Yonghong Song [Wed, 11 Apr 2018 16:08:00 +0000 (16:08 +0000)]
bpf: accept all asm register names
Sometimes when people compile bpf programs with
"clang ... -target bpf ...", the kernel header
files may contain host arch inline assembly codes
as in the patch https://patchwork.kernel.org/patch/10119683/
by Arnaldo Carvaldo de Melo.
The current workaround in the above patch
is to guard the inline assembly with "#ifndef __BPF__"
marco. So when __BPF__ is defined, these macros will
have no use.
Such a method is not extensible. As a matter of fact,
most of these inline assembly codes will be thrown away
at the end of clang compilation.
So for bpf target, this patch accepts all asm register
names in clang AST stage. The name will be checked
again during llc code generation if the inline assembly
code is indeed for bpf programs.
With this patch, the above "#ifndef __BPF__" is not needed
any more in https://patchwork.kernel.org/patch/10119683/.
Signed-off-by: Yonghong Song <yhs@fb.com>
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@329823 91177308-0d34-0410-b5e6-96231b3b80d8
Manuel Klimek [Wed, 11 Apr 2018 14:51:54 +0000 (14:51 +0000)]
Fix bugs around handling C++11 attributes.
Previously, we would format:
int a() { ... }
[[unused]] int b() { ... }
as...
int a() {} [[unused] int b() {}
Now we correctly format each on its own line.
Similarly, we would detect:
[[unused]] int b() { return 42; }
As a lambda and leave it on a single line, even if that was disallowed
by the format style.
"For every pair (T, VQ), where T is an arithmetic type other than bool, and VQ is either volatile or empty, there exist candidate operator functions of the form
VQ T& operator--(VQ T&);
T operator--(VQ T&, int);
"
The bool type is in position LastPromotedIntegralType in BuiltinOperatorOverloadBuilder::getArithmeticType::ArithmeticTypes, but addPlusPlusMinusMinusArithmeticOverloads() was expecting it at position 0.
[Tooling] Optimize memory usage in InMemoryToolResults.
Avoid storing duplicated "std::string"s.
clangd's global-symbol-builder takes 20+GB memory running across LLVM
repository. With this patch, the used memory is ~10GB (running on 48
threads, most of meory are AST-related).
Adam Balogh [Wed, 11 Apr 2018 06:21:12 +0000 (06:21 +0000)]
[Analyzer] SValBuilder Comparison Rearrangement (with Restrictions and Analyzer Option)
Since the range-based constraint manager (default) is weak in handling comparisons where symbols are on both sides it is wise to rearrange them to have symbols only on the left side. Thus e.g. A + n >= B + m becomes A - B >= m - n which enables the constraint manager to store a range m - n .. MAX_VALUE for the symbolic expression A - B. This can be used later to check whether e.g. A + k == B + l can be true, which is also rearranged to A - B == l - k so the constraint manager can check whether l - k is in the range (thus greater than or equal to m - n).
The restriction in this version is the the rearrangement happens only if both the symbols and the concrete integers are within the range [min/4 .. max/4] where min and max are the minimal and maximal values of their type.
The rearrangement is not enabled by default. It has to be enabled by using -analyzer-config aggressive-relational-comparison-simplification=true.
[XRay][clang+compiler-rt] Support build-time mode selection
Summary:
This patch implements the `-fxray-modes=` flag which allows users
building with XRay instrumentation to decide which modes to pre-package
into the binary being linked. The default is the status quo, which will
link all the available modes.
For this to work we're also breaking apart the mode implementations
(xray-fdr and xray-basic) from the main xray runtime. This gives more
granular control of which modes are pre-packaged, and picked from
clang's invocation.
This fixes llvm.org/PR37066.
Note that in the future, we may change the default for clang to only
contain the profiling implementation under development in D44620, when
that implementation is ready.
Introduce a new builtin, __builtin_dump_struct, that is useful for dumping structure contents at runtime in circumstances where debuggers may not be easily available (such as in kernel work).
Petr Hosek [Tue, 10 Apr 2018 19:55:55 +0000 (19:55 +0000)]
[Driver] Allow drivers to add multiple libc++ include paths
This allows toolchain drivers to add multiple libc++ include paths akin
to libstdc++. This is useful in multiarch setup when some headers might
be in target specific include directory. There should be no functional
change.
This test ensures the popfd instruction in MS inline assembly can properly find a clobber name for the dirflag register. Previously the register was named 'DF', but it needs to be named 'dirflag' to match the name in the GCC register name list.
Currently we always include PTX into the fatbin along
with the GPU code.It about doubles the size of the GPU binary
we need to carry in the executable. These options allow control
inclusion of PTX into GPU binary.
This patch does not change the defaults, though we may consider
making no-PTX the default in the future.
[Parser] Fix assertion-on-invalid for unexpected typename.
In `ParseDeclarationSpecifiers` for the code
class A typename A;
we were able to annotate token `kw_typename` because it refers to
existing type. But later during processing token `annot_typename` we
failed to `SetTypeSpecType` and exited switch statement leaving
annotation token unconsumed. The code after the switch statement failed
because it didn't expect a special token.
The fix is not to assume that switch statement consumes all special
tokens and consume any token, not just non-special.
-ftime-report switch support in Clang.
The current support of the feature produces only 2 lines in report:
-Some general Code Generation Time;
-Total time of Backend Consumer actions.
This patch extends Clang time report with new lines related to Preprocessor, Include Filea Search, Parsing, etc.
Differential Revision: https://reviews.llvm.org/D43578
[ExprConstant] Use an AST node and a version number as a key to create
an APValue and retrieve it from map Temporaries.
The version number is needed when a single AST node is visited multiple
times and is used to create APValues that are required to be distinct
from each other (for example, MaterializeTemporaryExprs in default
arguments and VarDecls in loops).
[AST] Attempt to fix buildbot warnings + appease MSVC; NFCI
GCC 4.8.4 on a bot was warning about `ArgPassingKind` not fitting in
`ArgPassingRestrictions`, which appears to be incorrect, since
`ArgPassingKind` only has three potential values:
"warning: 'clang::RecordDecl::ArgPassingRestrictions' is too small to
hold all values of 'enum clang::RecordDecl::ArgPassingKind'"
Additionally, I remember hearing (though my knowledge may be outdated)
that MSVC won't merge adjacent bitfields if their types are different.
Try to fix both issues by turning these into `uint8_t`s.
[ObjC++] Never pass structs that transitively contain __weak fields in
registers.
This patch fixes a bug in r328731 that caused structs transitively
containing __weak fields to be passed in registers. The patch replaces
the flag RecordDecl::CanPassInRegisters with a 2-bit enum that indicates
whether the struct or structs containing the struct are forced to be
passed indirectly.
This reapplies r329617. r329617 didn't specify the underlying type for
enum ArgPassingKind, which caused regression tests to fail on a windows
bot.
[ObjC++] Never pass structs that transitively contain __weak fields in
registers.
This patch fixes a bug in r328731 that caused structs transitively
containing __weak fields to be passed in registers. The patch replaces
the flag RecordDecl::CanPassInRegisters with a 2-bit enum that indicates
whether the struct or structs containing the struct are forced to be
passed indirectly.
asan: kernel: make no_sanitize("address") attribute work with -fsanitize=kernel-address
Summary:
Right now to disable -fsanitize=kernel-address instrumentation, one needs to use no_sanitize("kernel-address"). Make either no_sanitize("address") or no_sanitize("kernel-address") disable both ASan and KASan instrumentation. Also remove redundant test.
I believe all the pieces are now in place in the backend to make this work correctly. We can either mask the input to 32 bits for pmuludg or shl/ashr for pmuldq and use a regular mul instruction. The backend should combine this to PMULUDQ/PMULDQ and then SimplifyDemandedBits will remove the and/shifts.
Sam McCall [Mon, 9 Apr 2018 14:12:51 +0000 (14:12 +0000)]
[Index] Return SourceLocation to consumers, not FileID/Offset pair.
Summary:
The FileID/Offset conversion is lossy. The code takes the fileLoc, which loses
e.g. the spelling location in some macro cases.
Instead, pass the original SourceLocation which preserves all information, and
update consumers to match current behavior.
This allows us to fix two bugs in clangd that need the spelling location.
[XRay][llvm+clang] Consolidate attribute list files
Summary:
This change consolidates the always/never lists that may be provided to
clang to externally control which functions should be XRay instrumented
by imbuing attributes. The files follow the same format as defined in
https://clang.llvm.org/docs/SanitizerSpecialCaseList.html for the
sanitizer blacklist.
We also deprecate the existing `-fxray-instrument-always=` and
`-fxray-instrument-never=` flags, in favour of `-fxray-attr-list=`.
Summary:
Currently clang doesn't do qualified lookup when building indirect field decl references. This causes ambiguity when the field is in a base class to which there are multiple valid paths even though a qualified name is used.
For example:
```
class B {
protected:
int i;
union { int j; };
};
class X : public B { };
class Y : public B { };
class Z : public X, public Y {
int a() { return X::i; } // works
int b() { return X::j; } // fails
};
```
Summary:
Currently clang doesn't do qualified lookup when building indirect field decl references. This causes ambiguity when the field is in a base class to which there are multiple valid paths even though a qualified name is used.
For example:
```
class B {
protected:
int i;
union { int j; };
};
class X : public B { };
class Y : public B { };
class Z : public X, public Y {
int a() { return X::i; } // works
int b() { return X::j; } // fails
};
```
Eric Fiselier [Sun, 8 Apr 2018 05:12:55 +0000 (05:12 +0000)]
[Sema] Remove dead code in BuildAnonymousStructUnionMemberReference. NFCI
Summary:
This patch cleans up a bunch of dead or unused code in BuildAnonymousStructUnionMemberReference.
The dead code was a branch that built a new CXXThisExpr when we weren't given a base object expression or base variable.
However, BuildAnonymousFoo has only two callers. One of which always builds a base object expression first, the second only calls when the IndirectFieldDecl is not a C++ class member. Even within C this branch seems entirely unused.
I tried diligently to write a test which hit it with no success.
This patch removes the branch and replaces it with an assertion that we were given either a base object expression or a base variable.
Fangrui Song [Sat, 7 Apr 2018 20:50:35 +0000 (20:50 +0000)]
[libclang] Add clang_File_tryGetRealPathName
Summary:
clang_getFileName() may return a path relative to WorkingDir.
On Arch Linux, during clang_indexTranslationUnit(), clang_getFileName() on
CXIdxIncludedIncludedFileInfo::file may return
"/../lib64/gcc/x86_64-pc-linux-gnu/7.3.0/../../../../include/c++/7.3.0/string",
for `#include <string>`.
I presume WorkingDir is somehow changed to /usr/lib or /usr/include and
clang_getFileName() returns a path relative to WorkingDir.
clang_File_tryGetRealPathName() returns "/usr/include/c++/7.3.0/string"
which is more useful for the indexer in this case.
Roman Lebedev [Sat, 7 Apr 2018 10:39:21 +0000 (10:39 +0000)]
[Sema] Extend -Wself-assign and -Wself-assign-field to warn on overloaded self-assignment (classes)
Summary:
This has just bit me, so i though it would be nice to avoid that next time :)
Motivational case:
https://godbolt.org/g/cq9UNk
Basically, it's likely to happen if you don't like shadowing issues,
and use `-Wshadow` and friends. And it won't be diagnosed by clang.
The reason is, these self-assign diagnostics only work for builtin assignment
operators. Which makes sense, one could have a very special operator=,
that does something unusual in case of self-assignment,
so it may make sense to not warn on that.
But while it may be intentional in some cases, it may be a bug in other cases,
so it would be really great to have some diagnostic about it...
Alex Lorenz [Sat, 7 Apr 2018 00:03:27 +0000 (00:03 +0000)]
Recommit r329442: Generate Libclang invocation reproducers using a new
-cc1gen-reproducer driver option
The recommit fixes:
- An MSAN failure (CCPrintOptions wasn't initialized in the Driver)
- Ensures that the strings in the libclang invocation files are escaped
Original message:
This commit is a follow up to the previous work that recorded Libclang invocations
into temporary files: r319702.
It adds a new -cc1 mode to clang: -cc1gen-reproducer. The goal of this mode is to generate
Clang reproducer files for Libclang tool invocation. The JSON format in the invocation
files is not really intended to be stable, so Libclang and Clang should be of the same version
when generating reproducers.
The new mode emits the information about the temporary files and Libclang-specific information
to stdout using JSON.
Alex Lorenz [Fri, 6 Apr 2018 18:30:14 +0000 (18:30 +0000)]
Generate Libclang invocation reproducers using a new -cc1gen-reproducer
driver option
This commit is a follow up to the previous work that recorded Libclang invocations
into temporary files: r319702.
It adds a new -cc1 mode to clang: -cc1gen-reproducer. The goal of this mode is to generate
Clang reproducer files for Libclang tool invocation. The JSON format in the invocation
files is not really intended to be stable, so Libclang and Clang should be of the same version
when generating reproducers.
The new mode emits the information about the temporary files and Libclang-specific information
to stdout using JSON.
Allow the creation of human-friendly ASTDumper to arbitrary output stream
Summary:
`ASTPrinter` allows setting the ouput to any O-Stream, but that printer creates source-code-like syntax (and is also marked with a `FIXME`). The nice, colourful, mostly human-readable `ASTDumper` only works on the standard output, which is not feasible in case a user wants to see the AST of a file through a code navigation/comprehension tool.
This small addition of an overload solves generating a nice colourful AST block for the users of a tool I'm working on, [[ http://github.com/Ericsson/CodeCompass | CodeCompass ]], as opposed to having to duplicate the behaviour of definitions that only exist in the anonymous namespace of implementation TUs related to this module.
[XRay][clang] Add a flag to enable/disable linking XRay deps explicitly
Summary:
This change introduces `-fxray-link-deps` and `-fnoxray-link-deps`. The
`-fnoxray-link-deps` allows for directly controlling which specific XRay
runtime to link. The default is for clang to link the XRay runtime that
is shipped with the compiler (if there are any), but users may want to
explicitly add the XRay dependencies from other locations or other
means.
[XRay][clang] Consolidate runtime and link-time flag processing (NFC)
Summary:
This change fixes http://llvm.org/PR36985 to define a single place in
CommonArgs.{h,cpp} where XRay runtime flags and link-time dependencies
are processed for all toolchains that support XRay instrumentation. This
is a refactoring of the same functionality spread across multiple
toolchain definitions.