bernhardmgruber added a comment.
Thank you @JonasToth for providing more feedback! I will add a few more tests
with templates. Maybe I should even try to run the check on Boost and see what
happens.
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?
And btw, is there an easy way to get a compilation database on Windows?
Many thanks!
================
Comment at: clang-tidy/modernize/UseTrailingReturnCheck.cpp:118
+ if (Info.hasMacroDefinition()) {
+ diag(F.getLocation(), Message); // CV qualifiers in macros
+ return {};
----------------
JonasToth wrote:
> Please improve that comment and here I would prefer a non-trailing comment,
> too.
> Especially formulate whats with CV and macros, the meaning has to be guessed
> (and can be guessed wrong).
replaced by "The CV qualifiers of the return type are inside macros"
================
Comment at: clang-tidy/modernize/UseTrailingReturnCheck.cpp:133
+ bool ExtendedLeft = false;
+ for (size_t i = 0; i < Tokens.size(); i++) {
+ // if we found the beginning of the return type, include const and volatile
----------------
JonasToth wrote:
> please use uppercase `i` and `j` names for consistency.
i considered this with respect to the style guide, but it just looked far to
unfamiliar to me. done.
================
Comment at: clang-tidy/modernize/UseTrailingReturnCheck.cpp:162
+ functionDecl(unless(anyOf(hasTrailingReturn(), returns(voidType()),
+ returns(autoType()), cxxConversionDecl(),
+ cxxMethodDecl(isImplicit()))))
----------------
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)`.
================
Comment at: test/clang-tidy/modernize-use-trailing-return.cpp:269
+auto l1 = [](int arg) {};
+auto l2 = [](int arg) -> double {};
----------------
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!
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
