JonasToth added a comment.

I believe there is some sort of memory leak in the `run-clang-tidy` or so. I 
had similar experience :)
Take this with a grain of salt, as I don't recall all details: 
`run-clang-tidy.py` just takes all output from clang-tidy and prints it. A lot 
is redundant due to the include-horror in c++. After I implemented 
deduplication (not in tree, but here https://reviews.llvm.org/D54141) things 
got better. You don't need to run it over the whole of LLVM, but can take a 
subset, too. Maybe `lib/` or `tools/clang/lib`. Note that `run-clang-tidy.py` 
reads in the compilation database and matches your path as regex against it. So 
`lib/` includes all libs (clang, llvm, ...).

> In the meantime I might need some help: I tried running the check on LLVM 
> last weekend using the run-clang-tidy.py file. The script eventually crashed 
> with a segmentation fault (after 1.5 days, running on CentOS 7, 30GiB RAM) 
> with no modifications of the source tree. I ran again and exported the fixes, 
> but again, python failed to merge the yaml files and crashed (probably 
> because it went out of memory). After manual merging, I ran 
> clang-apply-replacements and it took a while, but then I also had zero 
> modifications on my LLVM working copy. clang-apply-replacements reported a 
> few overlapping refactorings and missing files, but that's it. What am I 
> doing wrong?

Overlapping can not be resolved, but interesting it occurs. Maybe in headers? 
Each TU (~ .cpp-file) will emit transformations. If they are identical they are 
merged, but if not there is a collision which can not be resolved. Maybe there 
is something going on with macros, ... that make the same header included twice 
not equivalent or so?

Try to run it over a subset first and then think about the issues. Trying other 
projects is certainly a good idea as well.

> And btw, is there an easy way to get a compilation database on Windows?

No windows-user, but cmake creates one with `CMAKE_EXPORT_COMPILECOMMANDS=ON` 
that you need to pass in somewhere somehow :)



================
Comment at: clang-tidy/modernize/UseTrailingReturnCheck.cpp:162
+      functionDecl(unless(anyOf(hasTrailingReturn(), returns(voidType()),
+                                returns(autoType()), cxxConversionDecl(),
+                                cxxMethodDecl(isImplicit()))))
----------------
bernhardmgruber wrote:
> JonasToth wrote:
> > Shouldn't you include `returns(decltypeType())` as well?
> good question! i have a unit test of the form `decltype(auto) f();` and it 
> seems to be already excluded by `returns(autoType())`. but i could add your 
> suggestion as well to make it more explicit that (for now) we will not 
> rewrite functions returning `decltype(auto)`.
There should be a difference between `decltype(auto)` and 
`decltype(some_expression)`. I will check your tests and suggest something 
there.


================
Comment at: test/clang-tidy/modernize-use-trailing-return.cpp:269
+auto l1 = [](int arg) {};
+auto l2 = [](int arg) -> double {};
----------------
bernhardmgruber wrote:
> JonasToth wrote:
> > These tests are missing the great template fun :)
> > 
> > Lets start with those two examples:
> > 
> > ```
> > template <typename Container>
> > [[maybe_unused]] typename Container::value_type const volatile&& 
> > myFunnyFunction(Container& C) noexcept;
> > ```
> > 
> > and
> > 
> > ```
> > #define MAYBE_UNUSED_MACRO [[maybe_unused]]
> > template <typename Container>
> > MAYBE_UNUSED_MACRO typename Container::value_type const volatile** const 
> > myFunnyFunction(Container& C) noexcept;
> > ```
> > 
> > Its not necessarily nice code, but I am sure something like this is 
> > somewhere in boost for example ;)
> You remind me of Jason Turner at CppCon 2018 who said, we should pick a toy 
> project, for which we are sure we can handle it, because complexity will 
> manifest itself in the details. This is exactly what is happening now.
> 
> Thank you for input, I added it to my tests!
Templates made me sad often, after i "finished" my checks and ran them over 
projects ;)
Sometimes it is almost impossible to fix very weird (and uncommon) things. But 
its better to find such cases early and work around them.


================
Comment at: test/clang-tidy/modernize-use-trailing-return.cpp:116
+
+// TODO: not matched by the AST matcher
+//decltype(auto) e6();
----------------
Is this still the case? That should be doable with the matchers, if you have a 
specific question i can look at it and try to help.

Here would be a good place to test the different `decltype`s.

```
decltype(auto) foo0() { return 1 + 2; } // should be the same as ...
auto foo1() { return 1 + 2; } // this, BUT ...
decltype(1 + 2) foo2() { return 1 + 2; } // should be something else
```
The matching for all three of them should be done differently.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56160/new/

https://reviews.llvm.org/D56160



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to