JonasToth added a comment.
I did evaluate the performance of the check on LLVM with the script in the
attachement. F11185973: full_transformation.sh
<https://reviews.llvm.org/F11185973>
The result, after splitting all multi-decl statements with
`readability-isolate-declarations` (which produces no issues) the current
analysis for `const` is run for values, references and pointers (the pointer
itself, not the pointee) and the automated fixes are applied. This is done for
**each** TU in LLVM/{llvm,clang,clang-tools-exta,lld}, including unit-tests.
Simply each target that is in the compilation_commands.json.
The patch for that: F11185980:
0001-Refactor-automatically-declare-everything-const.patch
<https://reviews.llvm.org/F11185980>
That is `3697 files changed, 164'596 insertions(+), 164'596 deletions(-)`
This patch unfortunatly does not compile cleanly anymore. To fix LLVM up I had
to manually fix the following things: F11186007:
0002-fixup-wrong-transformations.patch <https://reviews.llvm.org/F11186007>
These are 34 places that create compilation failures, not all of them are to
blame on the analysis/transformation.
The only actual deficits in analysis seem to be:
- overloaded operators for templated classes that are unresolved
- calls to methods that are called through captured `this` im lambda expression
within templated function
These are ~4 instances in all of LLVM.
- `MyType *& my_var = foo(); *(my_var + 3) = 42;` the pointer would be
qualified const, because the pointee-analysis does not exist. By default
pointers will not be marked as `const`. References to pointers should probably
be excluded as well.
Other problems are:
- the class does not provide a userdefined default constructor, after applying
const there is a `MyType const my_var -->{}<--;` missing -> can be added with
clang-tidy as well
- ternary operator has a `const` type for `true`, and non-const for `false`
- `MyType const my_var; decltype(my_var);` stops working
- in one test a type-trait that included a `decltype` stopped working, probably
same problem as previous point
I do not consider the latter issues to actually be an issue in the
analysis/transformation. They are very seldom and for such cases `NOLINT` is ok
in my opinion.
This leads to LLVM compiling cleanly.
$ ninja check-all
>
> ********************
Testing Time: 442.99s
********************
Failing Tests (9):
LLVM-Unit ::
ExecutionEngine/Orc/./OrcJITTests/CoreAPIsStandardTest.TestBasicAliases
LLVM-Unit ::
ExecutionEngine/Orc/./OrcJITTests/CoreAPIsStandardTest.TestBasicReExports
LLVM-Unit ::
ExecutionEngine/Orc/./OrcJITTests/CoreAPIsStandardTest.TestChainedAliases
LLVM-Unit ::
ExecutionEngine/Orc/./OrcJITTests/CoreAPIsStandardTest.TestReexportsGenerator
LLVM-Unit ::
ExecutionEngine/Orc/./OrcJITTests/CoreAPIsStandardTest.TestThatReExportsDontUnnecessarilyMaterialize
LLVM :: ExecutionEngine/OrcLazy/common-symbols.ll
LLVM :: ExecutionEngine/OrcLazy/global-ctors-and-dtors.ll
LLVM :: ExecutionEngine/OrcLazy/global_aliases.ll
LLVM :: ExecutionEngine/OrcLazy/hidden-visibility.ll
Expected Passes : 54838
Expected Failures : 175
Unsupported Tests : 531
Unexpected Failures: 9
FAILED: CMakeFiles/check-all
:(
There is an assertion-failure for `Orc`-stuff.
--
lli:
/data/big/llvm-project/llvm/include/llvm/ExecutionEngine/Orc/SymbolStringPool.h:146:
llvm::orc::SymbolStringPool::~SymbolStringPool(): Assertion `Pool.empty() &&
"Dangling references at pool destruction time"' failed.
Stack dump:
0. Program arguments: /data/big/test-build/bin/lli -jit-kind=orc-lazy
-extra-module
/data/big/llvm-project/llvm/test/ExecutionEngine/OrcLazy/Inputs/hidden-definitions.ll
/data/big/llvm-pr
oject/llvm/test/ExecutionEngine/OrcLazy/hidden-visibility.ll
#0 0x00007f8626fbbd4f llvm::sys::PrintStackTrace(llvm::raw_ostream&)
/data/big/llvm-project/llvm/lib/Support/Unix/Signals.inc:548:13
#1 0x00007f8626fb9fd0 llvm::sys::RunSignalHandlers()
/data/big/llvm-project/llvm/lib/Support/Signals.cpp:69:18
#2 0x00007f8626fbc155 SignalHandler(int)
/data/big/llvm-project/llvm/lib/Support/Unix/Signals.inc:380:3
#3 0x00007f86282ee770 __restore_rt (/lib64/libpthread.so.0+0x15770)
#4 0x00007f86268cf3c1 raise (/lib64/libc.so.6+0x3a3c1)
#5 0x00007f86268b755b abort (/lib64/libc.so.6+0x2255b)
#6 0x00007f86268b742f (/lib64/libc.so.6+0x2242f)
#7 0x00007f86268c6a92 (/lib64/libc.so.6+0x31a92)
#8 0x00007f8628e34f72 std::mutex::lock()
/usr/lib/gcc/x86_64-pc-linux-gnu/9.2.0/include/g++-v9/bits/std_mutex.h:104:2
#9 0x00007f8628e34f72 std::lock_guard<std::mutex>::lock_guard(std::mutex&)
/usr/lib/gcc/x86_64-pc-linux-gnu/9.2.0/include/g++-v9/bits/std_mutex.h:159:19
#10 0x00007f8628e34f72 llvm::orc::SymbolStringPool::clearDeadEntries()
/data/big/llvm-project/llvm/include/llvm/ExecutionEngine/Orc/SymbolStringPool.h:159:31
#11 0x00007f8628e34f72 llvm::orc::SymbolStringPool::~SymbolStringPool()
/data/big/llvm-project/llvm/include/llvm/ExecutionEngine/Orc/SymbolStringPool.h:145:3
#12 0x00007f8628e34f72 void
__gnu_cxx::new_allocator<llvm::orc::SymbolStringPool>::destroy<llvm::orc::SymbolStringPool>(llvm::orc::SymbolStringPool*)
/usr/lib/gcc/x86_64-pc-linux-gnu/9.2.0/inc
lude/g++-v9/ext/new_allocator.h:153:10
#13 0x00007f8628e34f72 void
std::allocator_traits<std::allocator<llvm::orc::SymbolStringPool>
>::destroy<llvm::orc::SymbolStringPool>(std::allocator<llvm::orc::SymbolStringPool>&,
llvm::orc::S
ymbolStringPool*)
/usr/lib/gcc/x86_64-pc-linux-gnu/9.2.0/include/g++-v9/bits/alloc_traits.h:497:8
#14 0x00007f8628e34f72
std::_Sp_counted_ptr_inplace<llvm::orc::SymbolStringPool,
std::allocator<llvm::orc::SymbolStringPool>,
(__gnu_cxx::_Lock_policy)2>::_M_dispose() /usr/lib/gcc/x86_64-pc-l
inux-gnu/9.2.0/include/g++-v9/bits/shared_ptr_base.h:557:2
#15 0x00007f8628e587eb __gnu_cxx::__exchange_and_add_dispatch(int*, int)
/usr/lib/gcc/x86_64-pc-linux-gnu/9.2.0/include/g++-v9/ext/atomicity.h:81:9
#16 0x00007f8628e587eb
std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release()
/usr/lib/gcc/x86_64-pc-linux-gnu/9.2.0/include/g++-v9/bits/shared_ptr_base.h:167:10
#17 0x00007f8628e587eb
std::__shared_count<(__gnu_cxx::_Lock_policy)2>::~__shared_count()
/usr/lib/gcc/x86_64-pc-linux-gnu/9.2.0/include/g++-v9/bits/shared_ptr_base.h:730:11
#18 0x00007f8628e587eb std::__shared_ptr<llvm::orc::SymbolStringPool,
(__gnu_cxx::_Lock_policy)2>::~__shared_ptr()
/usr/lib/gcc/x86_64-pc-linux-gnu/9.2.0/include/g++-v9/bits/shared_ptr_base.h:
1169:31
#19 0x00007f8628e587eb llvm::orc::ExecutionSession::~ExecutionSession()
/data/big/llvm-project/llvm/include/llvm/ExecutionEngine/Orc/Core.h:1053:7
#20 0x00007f8628e54f96
std::default_delete<llvm::orc::ExecutionSession>::operator()(llvm::orc::ExecutionSession*)
const /usr/lib/gcc/x86_64-pc-linux-gnu/9.2.0/include/g++-v9/bits/unique_ptr.h:
81:2
#21 0x00007f8628e54f96 std::unique_ptr<llvm::orc::ExecutionSession,
std::default_delete<llvm::orc::ExecutionSession> >::~unique_ptr()
/usr/lib/gcc/x86_64-pc-linux-gnu/9.2.0/include/g++-v9/bits
/unique_ptr.h:284:4
#22 0x00007f8628e54f96 llvm::orc::LLJIT::~LLJIT()
/data/big/llvm-project/llvm/lib/ExecutionEngine/Orc/LLJIT.cpp:55:1
#23 0x000000000041c2f1
std::default_delete<llvm::orc::LLLazyJIT>::operator()(llvm::orc::LLLazyJIT*)
const
/usr/lib/gcc/x86_64-pc-linux-gnu/9.2.0/include/g++-v9/bits/unique_ptr.h:81:2
#24 0x000000000041c2f1 std::unique_ptr<llvm::orc::LLLazyJIT,
std::default_delete<llvm::orc::LLLazyJIT> >::~unique_ptr()
/usr/lib/gcc/x86_64-pc-linux-gnu/9.2.0/include/g++-v9/bits/unique_ptr.h:
But all failures seem to be this assertion.
Given this result (i mostly consider it a succes, even though the overloaded
operators not working confuse me) I want to start cleaning up this patch and
start committing e.g. formatting changes and everything that can be extracted
from this patch.
What is currently missing are isolated unit-tests in `unittests/Analysis` for
each new fixed code construct. They all live in the clang-tidy test which is
suboptimal.
Once the cleaning is successfull I want to address this.
@aaron.ballman and others: what do you think of the quality of the analysis?
Should this check be comittable functionality-wise?
IMHO it would really help to get some stuff into master
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D54943/new/
https://reviews.llvm.org/D54943
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
