Ben Hamilton [Tue, 3 Apr 2018 14:07:11 +0000 (14:07 +0000)]
[clang-format/ObjC] Do not detect "[]" as ObjC method expression
Summary:
The following C++ code was being detected by
`guessLanguage()` as Objective-C:
#define FOO(...) auto bar = [] __VA_ARGS__;
This was because `[] __VA_ARGS__` is not currently detected as a C++
lambda expression (it has no parens or braces), so
`TokenAnnotator::parseSquare()` incorrectly treats the opening square
as an ObjC method expression.
We have two options to fix this:
1. Parse `[] __VA_ARGS__` explicitly as a C++ lambda
2. Make it so `[]` is never parsed as an Objective-C method expression
This diff implements option 2, which causes the `[` to be parsed
as `TT_ArraySubscriptLSquare` instead of `TT_ObjCMethodExpr`.
Note that when I fixed this, it caused one change in formatting
behavior, where the following was implicitly relying on the `[`
being parsed as `TT_ObjCMethodExpr`:
A<int * []> a;
becomes:
A<int *[]> a;
with `Style.PointerAlignment = Middle`.
I don't really know what the desired format is for this syntax; the
test was added by Janusz Sobczak and integrated by @djasper in
https://github.com/llvm-mirror/clang/commit/b511fe9818829d7ece0cc0b2ce1fbe04a1f0739a
.
I went ahead and changed the test for now.
Test Plan: New tests added. Ran tests with:
% make -j12 FormatTests && ./tools/clang/unittests/Format/FormatTests
Ben Hamilton [Tue, 3 Apr 2018 14:07:09 +0000 (14:07 +0000)]
[clang-format/ObjC] Do not insert space after opening brace of ObjC dict literal
Summary:
D44816 attempted to fix a few cases where `clang-format` incorrectly
inserted a space before the closing brace of an Objective-C dictionary
literal.
This revealed there were still a few cases where we inserted a space
after the opening brace of an Objective-C dictionary literal.
This fixes the formatting to be consistent and adds more tests.
Test Plan: New tests added. Confirmed tests failed before
diff and passed after diff.
Ran tests with:
% make -j12 FormatTests && ./tools/clang/unittests/Format/FormatTests
Petr Hosek [Mon, 2 Apr 2018 23:36:14 +0000 (23:36 +0000)]
[Driver] Wire up the -f[no-]rtlib-add-rpath flag and tests
D30700 added the -f[no-]rtlib-add-rpath flag, but that flag was never
wired up in the driver and tests were updated to check whether it
actually does anything. This patch wires up the flag and updates test.
Microsoft has reserved 'U' for the PreserveMostCC which is used in the
swift runtime. Add support for this. This allows the swift runtime to
be built for Windows again.
[MS] Emit vftable thunks for functions with incomplete prototypes
Summary:
The following class hierarchy requires that we be able to emit a
this-adjusting thunk for B::foo in C's vftable:
struct Incomplete;
struct A {
virtual A* foo(Incomplete p) = 0;
};
struct B : virtual A {
void foo(Incomplete p) override;
};
struct C : B { int c; };
This TU is valid, but lacks a definition of 'Incomplete', which makes it
hard to build a thunk for the final overrider, B::foo.
Before this change, Clang gives up attempting to emit the thunk, because
it assumes that if the parameter types are incomplete, it must be
emitting the thunk for optimization purposes. This is untrue for the MS
ABI, where the implementation of B::foo has no idea what thunks C's
vftable may require. Clang needs to emit the thunk without necessarily
having access to the complete prototype of foo.
This change makes Clang emit a musttail variadic call when it needs such
a thunk. I call these "unprototyped" thunks, because they only prototype
the "this" parameter, which must always come first in the MS C++ ABI.
These thunks work, but they create ugly LLVM IR. If the call to the
thunk is devirtualized, it will be a call to a bitcast of a function
pointer. Today, LLVM cannot inline through such a call, but I want to
address that soon, because we also use this pattern for virtual member
pointer thunks.
This change also implements an old FIXME in the code about reusing the
thunk's computed CGFunctionInfo as much as possible. Now we don't end up
computing the thunk's mangled name and arranging it's prototype up to
around three times.
Fix some DenseMap use-after-rehash bugs and hoist MethodVFTableLocation
This re-lands r328845 with fixes for crbug.com/827810.
The initial motiviation was to hoist MethodVFTableLocation to global
scope so it could be forward declared.
In this patch, I noticed that MicrosoftVTableContext uses some risky
patterns. It has methods that return references to data stored in
DenseMaps. I've made some of them return by value for trivial structs
and I've moved some things into separate allocations.
A recent addition to Coroutines TS (https://wg21.link/p0913) adds a pre-defined
coroutine noop_coroutine that does nothing. To implement this feature, we implemented
an llvm.coro.noop intrinsic that returns a coroutine handle to a coroutine that
does nothing when resumed or destroyed.
This patch adds a builtin __builtin_coro_noop() that maps to llvm.coro.noop intrinsic.
Related llvm change: https://reviews.llvm.org/D45114
Brian Gesiak [Sun, 1 Apr 2018 23:55:21 +0000 (23:55 +0000)]
[Coroutines] Schedule coro-split before asan
Summary:
The docs for the LLVM coroutines intrinsic `@llvm.coro.id` state that
"The second argument, if not null, designates a particular alloca instruction
to be a coroutine promise."
However, if the address sanitizer pass is run before the `@llvm.coro.id`
intrinsic is lowered, the `alloca` instruction passed to the intrinsic as its
second argument is converted, as per the
https://github.com/google/sanitizers/wiki/AddressSanitizerAlgorithm docs, to
an `inttoptr` instruction that accesses the address of the promise.
On optimization levels `-O1` and above, the `-asan` pass is run after
`-coro-early`, `-coro-split`, and `-coro-elide`, and before
`-coro-cleanup`, and so there is no issue. At `-O0`, however, `-asan`
is run in between `-coro-early` and `-coro-split`, which causes an
assertion to be hit when the `inttoptr` instruction is forcibly cast to
an `alloca`.
Rearrange the passes such that the coroutine passes are registered
before the sanitizer passes.
Test Plan:
Compile a simple C++ program that uses coroutines in `-O0` with
`-fsanitize-address`, and confirm no assertion is hit:
`clang++ coro-example.cpp -fcoroutines-ts -g -fsanitize=address -fno-omit-frame-pointer`.
Brian Gesiak [Sun, 1 Apr 2018 22:59:22 +0000 (22:59 +0000)]
[Coroutines] Find custom allocators in class scope
Summary:
https://reviews.llvm.org/rL325291 implemented Coroutines TS N4723
section [dcl.fct.def.coroutine]/7, but it performed lookup of allocator
functions within both the global and class scope, whereas the specified
behavior is to perform lookup for custom allocators within just the
class scope.
To fix, add parameters to the `Sema::FindAllocationFunctions` function
such that it can be used to lookup allocators in global scope,
class scope, or both (instead of just being able to look up in just global
scope or in both global and class scope). Then, use those parameters
from within the coroutine Sema.
This incorrect behavior had the unfortunate side-effect of causing the
bug https://bugs.llvm.org/show_bug.cgi?id=36578 (or at least the reports
of that bug in C++ programs). That bug would occur for any C++ user with
a coroutine frame that took a single pointer argument, since it would
then find the global placement form `operator new`, described in the
C++ standard 18.6.1.3.1. This patch prevents Clang from generating code
that triggers the LLVM assert described in that bug report.
John McCall [Sun, 1 Apr 2018 21:04:30 +0000 (21:04 +0000)]
Fix a major swiftcall ABI bug with trivial C++ class types.
The problem with the previous logic was that there might not be any
explicit copy/move constructor declarations, e.g. if the type is
trivial and we've never type-checked a copy of it. Relying on Sema's
computation seems much more reliable.
Also, I believe Richard's recommendation is exactly the rule we use
now on the Itanium ABI, modulo the trivial_abi attribute (which this
change of course fixes our handling of in Swift).
This does mean that we have a less portable rule for deciding
indirectness for swiftcall. I would prefer it if we just applied the
Itanium rule universally under swiftcall, but in the meantime, I need
to fix this bug.
This only arises when defining functions with class-type arguments
in C++, as we do in the Swift runtime. It doesn't affect normal Swift
operation because we don't import code as C++.
Henry Wong [Sat, 31 Mar 2018 12:46:46 +0000 (12:46 +0000)]
[analyzer] Unroll the loop when it has a unsigned counter.
Summary:
The original implementation in the `LoopUnrolling.cpp` didn't consider the case where the counter is unsigned. This case is only handled in `simpleCondition()`, but this is not enough, we also need to deal with the unsinged counter with the counter initialization.
Since `IntegerLiteral` is `signed`, there is a `ImplicitCastExpr<IntegralCast>` in `unsigned counter = IntergerLiteral`. This patch add the `ignoringParenImpCasts()` in the `IntegerLiteral` matcher.
George Karpenkov [Sat, 31 Mar 2018 01:20:06 +0000 (01:20 +0000)]
[analyzer] Fix liveness calculation for C++17 structured bindings
C++ structured bindings for non-tuple-types are defined in a peculiar
way, where the resulting declaration is not a VarDecl, but a
BindingDecl.
That means a lot of existing machinery stops working.
Peter Szecsi [Fri, 30 Mar 2018 22:03:29 +0000 (22:03 +0000)]
[ASTImporter] Add test helper Fixture
Add a helper test Fixture, so we can add tests which can check internal
attributes of AST nodes like getPreviousDecl(), isVirtual(), etc.
This enables us to check if a redeclaration chain is correctly built during
import, if the virtual flag is preserved during import, etc. We cannot check
such attributes with the existing testImport.
Also, this fixture makes it possible to import from several "From" contexts.
We also added several test cases here, some of them are disabled.
We plan to pass the disabled tests in other patches.
Artem Dergachev [Fri, 30 Mar 2018 19:27:42 +0000 (19:27 +0000)]
[analyzer] Track null or undef values through pointer arithmetic.
Pointer arithmetic on null or undefined pointers results in null or undefined
pointers. This is obvious for undefined pointers; for null pointers it follows
from our incorrect-but-somehow-working approach that declares that 0 (Loc)
doesn't necessarily represent a pointer of numeric address value 0, but instead
it represents any pointer that will cause a valid "null pointer dereference"
issue when dereferenced.
For now we've been seeing through pointer arithmetic at the original dereference
expression, i.e. in bugreporter::getDerefExpr(), but not during further
investigation of the value's origins in bugreporter::trackNullOrUndefValue().
The patch fixes it.
Artem Dergachev [Fri, 30 Mar 2018 19:25:39 +0000 (19:25 +0000)]
[CFG] [analyzer] Work around a disappearing CXXBindTemporaryExpr.
Sometimes template instantiation causes CXXBindTemporaryExpr to be missing in
its usual spot. In CFG, temporary destructors work by relying on
CXXBindTemporaryExprs, so they won't work in this case.
Avoid the crash and notify the clients that we've encountered an unsupported AST
by failing to provide the ill-formed construction context for the temporary.
Artem Dergachev [Fri, 30 Mar 2018 19:21:18 +0000 (19:21 +0000)]
[CFG] [analyzer] Avoid modeling C++17 constructors that aren't fully supported.
Not enough work has been done so far to ensure correctness of construction
contexts in the CFG when C++17 copy elision is in effect, so for now we
should drop construction contexts in the CFG and in the analyzer when
they seem different from what we support anyway.
This includes initializations with conditional operators and return values
across multiple stack frames.
Alexey Bataev [Fri, 30 Mar 2018 18:31:07 +0000 (18:31 +0000)]
[OPENMP] Added emission of offloading data sections for declare target
variables.
Added emission of the offloading data sections for the variables within
declare target regions + fixes emission of the declare target variables
marked as declare target not within the declare target region.
Ben Hamilton [Fri, 30 Mar 2018 15:38:45 +0000 (15:38 +0000)]
[clang-format] Ensure wrapped ObjC selectors with 1 arg obey IndentWrappedFunctionNames
Summary:
In D43121, @Typz introduced logic to avoid indenting 2-or-more
argument ObjC selectors too far to the right if the first component
of the selector was longer than the others.
This had a small side effect of causing wrapped ObjC selectors with
exactly 1 argument to not obey IndentWrappedFunctionNames:
Test Plan: New tests added. Test failed before change, passed
after change. Ran tests with:
% make -j12 FormatTests && ./tools/clang/unittests/Format/FormatTests
Reid Kleckner [Thu, 29 Mar 2018 22:42:24 +0000 (22:42 +0000)]
Hoist MethodVFTableLocation out of MicrosoftVTableContext, NFC
This allows forward declaring it so that we can add it to
MicrosoftMangleContext::mangleVirtualMemPtrThunk without including
VTableBuilder.h. That saves a hashtable lookup when emitting virtual
member pointer functions.
It also shortens a really long type name. This struct has "VFtable" in
the name, so it seems pretty unlikely that someone will assume it is
generally useful for non-MS C++ ABI stuff.
Manoj Gupta [Thu, 29 Mar 2018 21:11:15 +0000 (21:11 +0000)]
[AArch64]: Add support for parsing rN registers.
Summary:
Allow rN registers to be simply parsed as correspoing xN registers.
The "register ... asm("rN")" is an command to the
compiler's register allocator, not an operand to any individual assembly
instruction. GCC documents this syntax as "...the name of the register
that should be used."
This is needed to support the changes in Linux kernel (see
https://lkml.org/lkml/2018/3/1/268 )
Note: This will add support only for the limited use case of
register ... asm("rN"). Any other uses that make rN leak into assembly
are not supported.
Volodymyr Sapsai [Thu, 29 Mar 2018 17:34:09 +0000 (17:34 +0000)]
[Sema] Make deprecation fix-it replace all multi-parameter ObjC method slots.
Deprecation replacement can be any text but if it looks like a name of
ObjC method and has the same number of arguments as original method,
replace all slot names so after applying a fix-it you have valid code.
[Hexagon] Aid bit-reverse load intrinsics lowering with bitcode
The conversion of operatios to bitcode helps to eliminate an additional
store in certain cases. We used to lower these load intrinsics in DAG to
DAG conversion by which time, the "Dead Store Elimination" pass is
already run. There is an associated LLVM patch.
Sylvestre Ledru [Thu, 29 Mar 2018 10:05:46 +0000 (10:05 +0000)]
Rename clang link from clang-X.Y to clang-X
Summary:
As we are only doing X.0.Z releases (not using the minor version), there is no need to keep -X.Y in the version.
So, instead, I propose the following:
Instead of having clang-7.0 in bin/, we will have clang-7
Richard Trieu [Thu, 29 Mar 2018 05:14:17 +0000 (05:14 +0000)]
Refactor some code for a warning. NFC.
Use range-based for-loops instead of iterators to walk over vectors.
Switch the key of the DenseMap so a custom key handler is no longer needed.
Remove unncessary adds to the DenseMap.
Use unique_ptr instead of manual memory management.
Note how in the instantiated version ImplicitValueInitExpr unexpectedly appears.
While objects are auto-initialized under ARC, it does not make sense to
have an initializer for a for-loop variable, and it makes even less
sense to have such a different AST for instantiated and non-instantiated
version.
Digging deeper, I have found that there are two separate Sema* files for
dealing with templates and for dealing with non-templatized code.
In a non-templatized version, an initialization was performed only for
variables which are not loop variables for an Objective-C loop and not
variables for a C++ for-in loop:
Akira Hatanaka [Wed, 28 Mar 2018 21:13:14 +0000 (21:13 +0000)]
[ObjC++] Make parameter passing and function return compatible with ObjC
ObjC and ObjC++ pass non-trivial structs in a way that is incompatible
with each other. For example:
typedef struct {
id f0;
__weak id f1;
} S;
// this code is compiled in c++.
extern "C" {
void foo(S s);
}
void caller() {
// the caller passes the parameter indirectly and destructs it.
foo(S());
}
// this function is compiled in c.
// 'a' is passed directly and is destructed in the callee.
void foo(S a) {
}
This patch fixes the incompatibility by passing and returning structs
with __strong or weak fields using the C ABI in C++ mode. __strong and
__weak fields in a struct do not cause the struct to be destructed in
the caller and __strong fields do not cause the struct to be passed
indirectly.
Also, this patch fixes the microsoft ABI bug mentioned here:
[Hexagon] Add support for "new" circular buffer intrinsics
These instructions have been around for a long time, but we
haven't supported intrinsics for them. The "new" vesrions use
the CSx register for the start of the buffer instead of the K
field in the Mx register.
Reid Kleckner [Wed, 28 Mar 2018 18:23:35 +0000 (18:23 +0000)]
[MS] Fix bug in method vfptr location code
We were assuming that vbtable indices were assigned in layout order in
our comparison, which is not the case. When a virtual method, such as
the destructor, appears in multiple vftables, the vftable that appears
first in object layout order is the one that points to the main
implementation of that method. The later vftables use thunks.
In this layout, we adjusted "this" in the main implementation by the
amount that is appropriate for 'B' instead of 'A', even though the main
implementation is found in D's vftable for A:
struct A {
virtual ~A() {}
};
struct B {
virtual ~B() {}
};
struct C : virtual B {};
struct D : virtual A, C {};
D's layout looks like:
0 D subobject (empty)
0 C base suboject
8 A base subobject
16 B base subobject
With this fix, we correctly adjust by -8 in D's deleting destructor
instead of -16.
Matt Davis [Wed, 28 Mar 2018 16:05:05 +0000 (16:05 +0000)]
[Diag] Avoid emitting a redefinition note if no location is available.
Summary:
The "previous definition is here" note is not helpful if there is no location information. The note will reference nothing in such a case. This patch first checks to see if there is location data, and if so the note diagnostic is emitted.
This fixes PR15409. The issue in the first comment seems to already be resolved. This patch addresses the second example.
Alexey Bataev [Wed, 28 Mar 2018 14:28:54 +0000 (14:28 +0000)]
[OPENMP] Codegen for ctor|dtor of declare target variables.
When the declare target variables are emitted for the device,
constructors|destructors for these variables must emitted and registered
by the runtime in the offloading sections.
Richard Trieu [Wed, 28 Mar 2018 04:16:13 +0000 (04:16 +0000)]
Fix some handling of AST nodes with diagnostics.
The diagnostic system for Clang can already handle many AST nodes. Instead
of converting them to strings first, just hand the AST node directly to
the diagnostic system and let it handle the output. Minor changes in some
diagnostic output.
Akira Hatanaka [Wed, 28 Mar 2018 00:12:08 +0000 (00:12 +0000)]
[ObjC] Make C++ triviality type traits available to non-trivial C
structs.
r326307 and r327870 made changes that allowed using non-trivial C
structs with fields qualified with __strong or __weak. This commit makes
the following C++ triviality type traits available to non-trivial C
structs:
Shoaib Meenai [Tue, 27 Mar 2018 18:58:28 +0000 (18:58 +0000)]
[Sema] Avoid crash for category implementation without interface
When we have a category implementation without a corresponding interface
(which is an error by itself), semantic checks for property accesses
will attempt to access a null interface declaration and then segfault.
Error out in such cases instead.
[clang] Change std::sort to llvm::sort in response to r327219
r327219 added wrappers to std::sort which randomly shuffle the container before
sorting. This will help in uncovering non-determinism caused due to undefined
sorting order of objects having the same key.
To make use of that infrastructure we need to invoke llvm::sort instead of
std::sort.
Ben Hamilton [Tue, 27 Mar 2018 15:01:21 +0000 (15:01 +0000)]
[clang-format] Refine ObjC guesser to handle child lines of child lines
Summary:
This fixes an issue brought up by djasper@ in his review of D44790. We
handled top-level child lines, but if those child lines themselves
had child lines, we didn't handle them.
Rather than use recursion (which could blow out the stack), I use a
DenseSet to hold the set of lines we haven't yet checked (since order
doesn't matter), and update the set to add the children of each
line as we check it.
Test Plan: New tests added. Confirmed tests failed before fix
and passed after fix.
Ben Hamilton [Tue, 27 Mar 2018 15:01:17 +0000 (15:01 +0000)]
[clang-format] Do not insert space before closing brace in ObjC dict literal
Summary:
Previously, `clang-format` would sometimes insert a space
before the closing brace in an Objective-C dictionary literal.
Unlike array literals (which obey `Style.SpacesInContainerLiterals`
to add a space after `[` and before `]`), Objective-C dictionary
literals currently are not meant to insert a space after `{` and before
`}`, regardless of `Style.SpacesInContainerLiterals`.
However, some constructs like `@{foo : @(bar)}` caused `clang-format`
to insert a space between `)` and `}`.
This fixes the issue and adds tests. (I understand the behavior is
not consistent between array literals and dictionary literals, but
that's existing behavior that's a much larger change.)
Test Plan: New tests added. Ran tests with:
% make -j12 FormatTests && ./tools/clang/unittests/Format/FormatTests
Peter Szecsi [Tue, 27 Mar 2018 12:16:56 +0000 (12:16 +0000)]
[analyzer] LoopUnrolling: update the matched assignment operators
Extended the matched assignment operators when checking for bound changes in a body of the loop by using the freshly added isAssignmentOperator matcher.
This covers all the (current) possible assignments, tests added as well.
Peter Szecsi [Tue, 27 Mar 2018 12:11:46 +0000 (12:11 +0000)]
[ASTMatchers] Add isAssignmentOperator matcher
Adding a matcher for BinaryOperator and cxxOperatorCallExpr to be able to
decide whether it is any kind of assignment operator or not. This would be
useful since allows us to easily detect assignments via matchers for static
analysis (Tidy, SA) purposes.
Eric Fiselier [Tue, 27 Mar 2018 03:15:46 +0000 (03:15 +0000)]
[coroutines] Fix invalid source range in co_await call expressions.
Summary:
Currently an invalid source range is generated for the member call expressions of `co_await`. The end location of the call expression is the `co_await` token loc, while the start is the location of the operand. This causes crashes when the source range is used to produce diagnostics.
This patch fixes the issues by using the expression location instead of the token location when building the member calls.
Eric Fiselier [Tue, 27 Mar 2018 00:58:16 +0000 (00:58 +0000)]
Fix unused expression warning in co_await.
Previously, anytime the result of the resume expression in
operator co_await was unused, a warning was generated. This
patch fixes the issue by only generating the unused result warning
if calling `await_resume()` would also generate a warning.
Reid Kleckner [Mon, 26 Mar 2018 18:22:47 +0000 (18:22 +0000)]
[MS] Fix late-parsed template infinite loop in eager instantiation
Summary:
This fixes PR33561 and PR34185.
Don't store pending template instantiations for late-parsed templates in
the normal PendingInstantiations queue. Instead, use a separate list
that will only be parsed and instantiated at end of TU when late
template parsing actually works and doesn't infinite loop.
Alexey Bataev [Mon, 26 Mar 2018 16:40:55 +0000 (16:40 +0000)]
[OPENMP] Codegen for declare target with link clause.
If the link clause is used on the declare target directive, the object
should be linked on target or target data directives, not during the
codegen. Patch adds support for this clause.
Summary:
Add support for wildcard expansion in command line arguments on Windows.
See https://docs.microsoft.com/en-us/cpp/c-language/expanding-wildcard-arguments
projects / clang / log