Stephen Kelly [Wed, 5 Dec 2018 21:12:39 +0000 (21:12 +0000)]
NFC: Extract TextNodeDumper class
Summary:
Start by moving some utilities to it. It will eventually house dumping
of individual nodes (after indentation etc has already been accounted
for).
Stephen Kelly [Wed, 5 Dec 2018 20:34:07 +0000 (20:34 +0000)]
NFC: Inline handling of DependentSizedArrayType
Summary:
Re-order handling of getElementType and getBracketsRange. It is
necessary to perform all printing before any traversal to child nodes.
This causes no change in the output of ast-dump-array.cpp due to the way
child nodes are printed with a delay. This new order of the code is
also the order that produces the expected output anyway.
Aaron Ballman [Wed, 5 Dec 2018 18:56:57 +0000 (18:56 +0000)]
Do not check for parameters shadowing fields in function declarations.
We would issue a false-positive diagnostic for parameters in function declarations shadowing fields; we now only issue the diagnostic on a function definition instead.
Adrian Prantl [Wed, 5 Dec 2018 18:37:44 +0000 (18:37 +0000)]
Honor -fdebug-prefix-map when creating function names for the debug info.
This adds a callback to PrintingPolicy to allow CGDebugInfo to remap
file paths according to -fdebug-prefix-map. Otherwise the debug info
(particularly function names for C++ lambdas) may contain paths that
should have been remapped in the debug info.
Bruno Ricci [Wed, 5 Dec 2018 17:16:55 +0000 (17:16 +0000)]
[Basic] Cleanups in IdentifierInfo following the removal of PTH
The Entry pointer in IdentifierInfo was only null for IdentifierInfo
created from a PTH. Now that PTH support has been removed we can remove
some PTH specific code in IdentifierInfo::getLength and
IdentifierInfo::getNameStart.
Also make the constructor of IdentifierInfo private to make sure that
they are only created by IdentifierTable, and move it to the header so
that it can be inlined in IdentifierTable::get and IdentifierTable::getOwn.
Kristina Brooks [Wed, 5 Dec 2018 15:05:06 +0000 (15:05 +0000)]
[Haiku] Support __float128 for x86 and x86_64
This patch addresses a compilation error with clang when
running in Haiku being unable to compile code using
float128 (throws compilation error such as 'float128 is
not supported on this target').
Ilya Biryukov [Wed, 5 Dec 2018 14:24:14 +0000 (14:24 +0000)]
Move detection of libc++ include dirs to Driver on MacOS
Summary:
The intention is to make the tools replaying compilations from 'compile_commands.json'
(clang-tidy, clangd, etc.) find the same standard library as the original compiler
specified in 'compile_commands.json'.
Previously, the library detection logic was in the frontend (InitHeaderSearch.cpp) and relied
on the value of resource dir as an approximation of the compiler install dir. The new logic
uses the actual compiler install dir and is performed in the driver. This is consistent with
the C++ standard library detection on other platforms and allows to override the resource dir
in the tools using the compile_commands.json without altering the
standard library detection mechanism. The tools have to override the resource dir to make sure
they use a consistent version of the builtin headers.
There is still logic in InitHeaderSearch that attemps to add the absolute includes for the
the C++ standard library, so we keep passing the -stdlib=libc++ from the driver to the frontend
via cc1 args to avoid breaking that. In the long run, we should move this logic to the driver too,
but it could potentially break the library detection on other systems, so we don't tackle it in this
patch to keep its scope manageable.
This is a second attempt to fix the issue, first one was commited in r346652 and reverted in r346675.
The original fix relied on an ad-hoc propagation (bypassing the cc1 flags) of the install dir from the
driver to the frontend's HeaderSearchOptions. Unsurpisingly, the propagation was incomplete, it broke
the libc++ detection in clang itself, which caused LLDB tests to break.
George Rimar [Wed, 5 Dec 2018 11:09:10 +0000 (11:09 +0000)]
[clang] - Simplify tools::SplitDebugName.
This is an updated version of the D54576, which was reverted.
Problem was that SplitDebugName calls the InputInfo::getFilename
which asserts if InputInfo given is not of type Filename:
const char *getFilename() const {
assert(isFilename() && "Invalid accessor.");
return Data.Filename;
}
At the same time at that point, it can be of type Nothing and
we need to use getBaseInput(), like original code did.
Erik Pilkington [Wed, 5 Dec 2018 00:43:11 +0000 (00:43 +0000)]
[Sema] Remove some conditions of a failing assert
We should have been checking that this state is consistent, but its
possible for it to be filled later, so it isn't really sound to check
it here anyways.
Ilya Biryukov [Tue, 4 Dec 2018 16:30:45 +0000 (16:30 +0000)]
Revert "Avoid emitting redundant or unusable directories in DIFile metadata entries."
This reverts commit r348154 and follow-up commits r348211 and r3248213.
Reason: the original commit broke compiler-rt tests and a follow-up fix
(r348203) broke our integrate and was reverted.
Alexey Bataev [Tue, 4 Dec 2018 15:25:01 +0000 (15:25 +0000)]
[OPENMP][NVPTX]Fixed emission of the critical region.
Critical regions in NVPTX are the constructs, which, generally speaking,
are not supported by the NVPTX target. Instead we're using special
technique to handle the critical regions. Currently they are supported
only within the loop and all the threads in the loop must execute the
same critical region.
Inside of this special regions the regions still must be emitted as
critical, to avoid possible data races between the teams +
synchronization must use __kmpc_barrier functions.
Alexey Bataev [Tue, 4 Dec 2018 15:03:25 +0000 (15:03 +0000)]
[OPENMP][NVPTX]Mark __kmpc_barrier functions as convergent.
__kmpc_barrier runtime functions must be marked as convergent to prevent
some dangerous optimizations. Also, for NVPTX target all barriers must
be emitted as simple barriers.
Erich Keane [Tue, 4 Dec 2018 14:34:09 +0000 (14:34 +0000)]
PTH-- Remove feature entirely-
When debugging a boost build with a modified
version of Clang, I discovered that the PTH implementation
stores TokenKind in 8 bits. However, we currently have 368
TokenKinds.
The result is that the value gets truncated and the wrong token
gets picked up when including PTH files. It seems that this will
go wrong every time someone uses a token that uses the 9th bit.
Upon asking on IRC, it was brought up that this was a highly
experimental features that was considered a failure. I discovered
via googling that BoostBuild (mostly Boost.Math) is the only user of
this
feature, using the CC1 flag directly. I believe that this can be
transferred over to normal PCH with minimal effort:
https://github.com/boostorg/build/issues/367
Based on advice on IRC and research showing that this is a nearly
completely unused feature, this patch removes it entirely.
Note: I considered leaving the build-flags in place and making them
emit an error/warning, however since I've basically identified and
warned the only user, it seemed better to just remove them.
Ulrich Weigand [Tue, 4 Dec 2018 10:51:36 +0000 (10:51 +0000)]
[SystemZ] Do not support __float128
As of rev. 268898, clang supports __float128 on SystemZ. This seems to
have been in error. GCC has never supported __float128 on SystemZ,
since the "long double" type on the platform is already IEEE-128. (GCC
only supports __float128 on platforms where "long double" is some other
data type.)
For compatibility reasons this patch removes __float128 on SystemZ
again. The test case is updated accordingly.
Adam Balogh [Tue, 4 Dec 2018 10:27:27 +0000 (10:27 +0000)]
[Analyzer] Iterator Checker - Forbid decrements past the begin() and increments past the end() of containers
Previously, the iterator range checker only warned upon dereferencing of
iterators outside their valid range as well as increments and decrements of
out-of-range iterators where the result remains out-of-range. However, the C++
standard is more strict than this: decrementing begin() or incrementing end()
results in undefined behaviour even if the iterator is not dereferenced
afterwards. Coming back to the range once out-of-range is also undefined.
This patch corrects the behaviour of the iterator range checker: warnings are
given for any operation whose result is ahead of begin() or past the end()
(which is the past-end iterator itself, thus now we are speaking of past
past-the-end).
Adam Balogh [Tue, 4 Dec 2018 10:22:28 +0000 (10:22 +0000)]
[Analyzer] Iterator Checkers - Use the region of the topmost base class for iterators stored in a region
If an iterator is represented by a derived C++ class but its comparison operator
is for its base the iterator checkers cannot recognize the iterators compared.
This results in false positives in very straightforward cases (range error when
dereferencing an iterator after disclosing that it is equal to the past-the-end
iterator).
To overcome this problem we always use the region of the topmost base class for
iterators stored in a region. A new method called getMostDerivedObjectRegion()
was added to the MemRegion class to get this region.
Clement Courbet [Tue, 4 Dec 2018 07:59:57 +0000 (07:59 +0000)]
[WIP][Sema] Improve static_assert diagnostics for type traits.
Summary:
In our codebase, `static_assert(std::some_type_trait<Ts...>::value, "msg")`
(where `some_type_trait` is an std type_trait and `Ts...` is the
appropriate template parameters) account for 11.2% of the `static_assert`s.
In these cases, the `Ts` are typically not spelled out explicitly, e.g.
`static_assert(std::is_same<SomeT::TypeT, typename SomeDependentT::value_type>::value, "message");`
The diagnostic when the assert fails is typically not very useful, e.g.
`static_assert failed due to requirement 'std::is_same<SomeT::TypeT, typename SomeDependentT::value_type>::value' "message"`
This change makes the diagnostic spell out the types explicitly , e.g.
`static_assert failed due to requirement 'std::is_same<int, float>::value' "message"`
See tests for more examples.
After this is submitted, I intend to handle
`static_assert(!std::some_type_trait<Ts...>::value, "msg")`,
which is another 6.6% of static_asserts.
Petr Hosek [Tue, 4 Dec 2018 03:25:25 +0000 (03:25 +0000)]
[Sema] Provide -fvisibility-global-new-delete-hidden option
When the global new and delete operators aren't declared, Clang
provides and implicit declaration, but this declaration currently
always uses the default visibility. This is a problem when the
C++ library itself is being built with non-default visibility because
the implicit declaration will force the new and delete operators to
have the default visibility unlike the rest of the library.
The existing workaround is to use assembly to enforce the visiblity:
https://fuchsia.googlesource.com/zircon/+/master/system/ulib/zxcpp/new.cpp#108
but that solution is not always available, e.g. in the case of of
libFuzzer which is using an internal version of libc++ that's also built
with -fvisibility=hidden where the existing behavior is causing issues.
This change introduces a new option -fvisibility-global-new-delete-hidden
which makes the implicit declaration of the global new and delete
operators hidden.
Richard Smith [Tue, 4 Dec 2018 02:45:28 +0000 (02:45 +0000)]
Fix -Wmismatched-tags to not warn on redeclarations of structs in system
headers.
Previously, we would only check whether the new declaration is in a
system header, but that requires the user to be able to correctly guess
whether a declaration in a system header is declared as a struct or a
class when specializing standard library traits templates.
We now entirely ignore declarations for which the warning was disabled
when determining whether to warn on a tag mismatch.
Also extend the diagnostic message to clarify that
a) code containing such a tag mismatch is in fact valid and correct,
and
b) the (non-coding-style) reason to emit such a warning is that the
Microsoft C++ ABI is broken and includes the tag kind in decorated
names,
as it seems a lot of users are confused by our diagnostic here (either
not understanding why we produce it, or believing that it represents an
actual language rule).
Artem Dergachev [Tue, 4 Dec 2018 02:00:29 +0000 (02:00 +0000)]
[analyzer] MoveChecker: Improve warning and note messages.
The warning piece traditionally describes the bug itself, i.e.
"The bug is a _____", eg. "Attempt to delete released memory",
"Resource leak", "Method call on a moved-from object".
Event pieces produced by the visitor are usually in a present tense, i.e.
"At this moment _____": "Memory is released", "File is closed",
"Object is moved".
Additionally, type information is added into the event pieces for STL objects
(in order to highlight that it is in fact an STL object), and the respective
event piece now mentions that the object is left in an unspecified state
after it was moved, which is a vital piece of information to understand the bug.
Erik Pilkington [Tue, 4 Dec 2018 00:31:31 +0000 (00:31 +0000)]
NFC: Make this test kinder on downstream forks
Downstream forks that have their own attributes often run into this
test failing when a new attribute is added to clang because the
number of supported attributes no longer match. This is redundant
information for this test, so we can get by without it.
Artem Dergachev [Mon, 3 Dec 2018 23:06:07 +0000 (23:06 +0000)]
[analyzer] MoveChecker: Restrict to locals and std:: objects.
In general case there use-after-move is not a bug. It depends on how the
move-constructor or move-assignment is implemented.
In STL, the convention that applies to most classes is that the move-constructor
(-assignment) leaves an object in a "valid but unspecified" state. Using such
object without resetting it to a known state first is likely a bug. Objects
Local value-type variables are special because due to their automatic lifetime
there is no intention to reuse space. If you want a fresh object, you might
as well make a new variable, no need to move from a variable and than re-use it.
Therefore, it is not always a bug, but it is obviously easy to suppress when it
isn't, and in most cases it indeed is - as there's no valid intention behind
the intentional use of a local after move.
This applies not only to local variables but also to parameter variables,
not only of value type but also of rvalue reference type (but not to lvalue
references).
Artem Dergachev [Mon, 3 Dec 2018 22:44:16 +0000 (22:44 +0000)]
[analyzer] MoveChecker: NFC: Remove the workaround for the "zombie symbols" bug.
The checker had extra code to clean up memory regions that were sticking around
in the checker without ever being cleaned up due to the bug that was fixed in
r347953. Because of that, if a region was moved from, then became dead,
and then reincarnated, there were false positives.
Why regions are even allowed to reincarnate is a separate story. Luckily, this
only happens for local regions that don't produce symbols when loaded from.
No functional change intended. The newly added test demonstrates that even
though no cleanup is necessary upon destructor calls, the early return
cannot be removed. It was not failing before the patch.
Artem Dergachev [Mon, 3 Dec 2018 22:23:21 +0000 (22:23 +0000)]
[analyzer] Dump stable identifiers for objects under construction.
This continues the work that was started in r342313, which now gets applied to
object-under-construction tracking in C++. Makes it possible to debug
temporaries by dumping exploded graphs again.
Artem Dergachev [Mon, 3 Dec 2018 22:15:34 +0000 (22:15 +0000)]
[AST] Generate unique identifiers for CXXCtorInitializer objects.
This continues the work started in r342309 and r342315 to provide identifiers
to AST objects that are shorter and easier to read and remember than pointers.
This patch looks at any common prefix between the compilation
directory and the (absolute) file path and strips the redundant
part. More importantly it leaves the compilation directory empty if
the two paths have no common prefix.
After this patch the above entry is (assuming a compilation dir of "/Volumes/Data/llvm/_build"):
Bruno Ricci [Mon, 3 Dec 2018 16:17:45 +0000 (16:17 +0000)]
[Serialization][NFC] Remove pointless "+ 0" in ASTReader
Remove the pointless "+ 0" which I added for some reason when
modifying these statement/expression classes since it looks
like this is a typo. Following the suggestion of aaron.ballman
in D54902. NFC.
Bruno Ricci [Mon, 3 Dec 2018 14:54:03 +0000 (14:54 +0000)]
[AST][Sema] Remove CallExpr::setNumArgs
CallExpr::setNumArgs is the only thing that prevents storing the arguments
in a trailing array. There is only 3 places in Sema where setNumArgs is called.
D54900 dealt with one of them.
This patch remove the other two calls to setNumArgs in ConvertArgumentsForCall.
To do this we do the following changes:
1.) Replace the first call to setNumArgs by an assertion since we are moving the
responsability to allocate enough space for the arguments from
Sema::ConvertArgumentsForCall to its callers
(which are Sema::BuildCallToMemberFunction, and Sema::BuildResolvedCallExpr).
2.) Add a new member function CallExpr::shrinkNumArgs, which can only be used
to drop arguments and then replace the second call to setNumArgs by
shrinkNumArgs.
3.) Add a new defaulted parameter MinNumArgs to CallExpr and its derived
classes which specifies a minimum number of argument slots to allocate.
The actual number of arguments slots allocated will be
max(number of args, MinNumArgs) with the extra args nulled. Note that
after the creation of the call expression all of the arguments will be
non-null. It is just during the creation of the call expression that some of
the last arguments can be temporarily null, until filled by default arguments.
4.) Update Sema::BuildCallToMemberFunction by passing the number of parameters
in the function prototype to the constructor of CXXMemberCallExpr. Here the
change is pretty straightforward.
5.) Update Sema::BuildResolvedCallExpr. Here the change is more complicated
since the type-checking for the function type was done after the creation of
the call expression. We need to move this before the creation of the call
expression, and then pass the number of parameters in the function prototype
(if any) to the constructor of the call expression.
6.) Update the deserialization of CallExpr and its derived classes.
Pablo Barrio [Mon, 3 Dec 2018 14:40:37 +0000 (14:40 +0000)]
[AArch64] Add command-line option for SSBS
Summary:
SSBS (Speculative Store Bypass Safe) is only mandatory from 8.5
onwards but is optional from Armv8.0-A. This patch adds testing for
the ssbs command line option, added to allow enabling the feature
in previous Armv8-A architectures to 8.5.
Bruno Ricci [Mon, 3 Dec 2018 13:23:56 +0000 (13:23 +0000)]
[Sema] Avoid CallExpr::setNumArgs in Sema::BuildCallToObjectOfClassType
CallExpr::setNumArgs is the only thing that prevents storing the arguments
of a call expression in a trailing array since it might resize the argument
array. setNumArgs is only called in 3 places in Sema, and for all of them it
is possible to avoid it.
This deals with the call to setNumArgs in BuildCallToObjectOfClassType.
Instead of constructing the CXXOperatorCallExpr first and later calling
setNumArgs if we have default arguments, we first construct a large
enough SmallVector, do the promotion/check of the arguments, and
then construct the CXXOperatorCallExpr.
Incidentally this also avoid reallocating the arguments when the
call operator has default arguments but this is not the primary goal.
Marco Antognini [Mon, 3 Dec 2018 10:58:56 +0000 (10:58 +0000)]
[OpenCL][Sema] Improve BuildResolvedCallExpr handling of builtins
Summary:
This is a follow-up on https://reviews.llvm.org/D52879, addressing a few issues.
This:
- adds a FIXME for later improvement for specific builtins: I previously have only checked OpenCL ones and ensured tests cover those.
- fixed the CallExpr type.
Stefan Granitz [Mon, 3 Dec 2018 10:34:25 +0000 (10:34 +0000)]
[CMake] Store path to vendor-specific headers in clang-headers target property
Summary:
LLDB.framework wants a copy these headers. With this change LLDB can easily glob for the list of files:
```
get_target_property(clang_include_dir clang-headers RUNTIME_OUTPUT_DIRECTORY)
file(GLOB_RECURSE clang_vendor_headers RELATIVE ${clang_include_dir} "${clang_include_dir}/*")
```
By default `RUNTIME_OUTPUT_DIRECTORY` is unset for custom targets like `clang-headers`.
Matt Arsenault [Sat, 1 Dec 2018 21:56:10 +0000 (21:56 +0000)]
OpenCL: Extend argument promotion rules to vector types
The spec is ambiguous on whether vector types are allowed to be
implicitly converted. The only legal context I think this can
be used for OpenCL is printf, where it seems necessary.
Fangrui Song [Sat, 1 Dec 2018 01:43:05 +0000 (01:43 +0000)]
[Basic] Move DiagnosticsEngine::dump from .h to .cpp
The two LLVM_DUMP_METHOD methods have a undefined reference on clang::DiagnosticsEngine::DiagStateMap::dump.
tools/clang/tools/extra/clangd/benchmarks/IndexBenchmark links in
clangDaemon but does not link in clangBasic explicitly, which causes a
linker error "undefined symbol" in !NDEBUG + -DBUILD_SHARED_LIBS=on builds.
Move LLVM_DUMP_METHOD methods to .cpp to fix IndexBenchmark. They should
be unconditionally defined as they are also used by non-dump-method #pragma clang __debug diag_mapping
Adrian Prantl [Sat, 1 Dec 2018 00:24:27 +0000 (00:24 +0000)]
Honor -fdebug-prefix-map when creating function names for the debug info.
This adds a callback to PrintingPolicy to allow CGDebugInfo to remap
file paths according to -fdebug-prefix-map. Otherwise the debug info
(particularly function names for C++ lambdas) may contain paths that
should have been remapped in the debug info.
Fangrui Song [Fri, 30 Nov 2018 21:26:09 +0000 (21:26 +0000)]
Revert r347417 "Re-Reinstate 347294 with a fix for the failures."
Kept the "indirect_builtin_constant_p" test case in test/SemaCXX/constant-expression-cxx1y.cpp
while we are investigating why the following snippet fails:
extern char extern_var;
struct { int a; } a = {__builtin_constant_p(extern_var)};
Fangrui Song [Fri, 30 Nov 2018 21:15:41 +0000 (21:15 +0000)]
[ExprConstant] Try fixing __builtin_constant_p after D54355 (rC347417)
Summary:
Reinstate the original behavior (Success(false, E)) before D54355 when this branch is
taken. This fixes spurious error of the following snippet:
extern char extern_var;
struct { int a; } a = {__builtin_constant_p(extern_var)};