aaron.ballman added inline comments.

================
Comment at: 
clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp:430
+      AT->getKeyword() == AutoTypeKeyword::Auto &&
+      !hasAnyNestedLocalQualifiers(F->getDeclaredReturnType()))
+    return;
----------------
aaron.ballman wrote:
> bernhardmgruber wrote:
> > aaron.ballman wrote:
> > > Why do we need to check that there aren't any nested local qualifiers?
> > Because I would like the check to rewrite e.g. `const auto f();` into `auto 
> > f() -> const auto;`. If I omit the check for nested local qualifiers, then 
> > those kind of declarations would be skipped.
> I'm still wondering about this.
> Because I would like the check to rewrite e.g. const auto f(); into auto f() 
> -> const auto;. If I omit the check for nested local qualifiers, then those 
> kind of declarations would be skipped.

I don't think I understand why that's desirable though? What is it about the 
qualifier that makes it worthwhile to repeat the type like that?


================
Comment at: 
clang-tools-extra/test/clang-tidy/checkers/modernize-use-trailing-return-type.cpp:1
-// RUN: %check_clang_tidy -std=c++14,c++17 %s 
modernize-use-trailing-return-type %t -- -- -fdeclspec -fexceptions
+// RUN: %check_clang_tidy -std=c++14-or-later %s 
modernize-use-trailing-return-type %t -- -- -fdeclspec -fexceptions 
-DCOMMAND_LINE_INT=int
 // FIXME: Fix the checker to work in C++20 mode, it is performing a
----------------
bernhardmgruber wrote:
> aaron.ballman wrote:
> > The change to the language standard line makes me wonder if the fixme below 
> > it is now stale, or if the test will fail in C++20 mode.
> I just ran the tests again using `python .\check_clang_tidy.py -std=c++20 
> .\checkers\modernize-use-trailing-return-type.cpp 
> modernize-use-trailing-return-type aa -- -- -DCOMMAND_LINE_INT=int` and I did 
> not see mentioning of the use of an uninitialized variable. But I run on 
> Windows, maybe the issue just surfaces on another OS? Is there a way to 
> trigger the CI again?
> 
> I removed the FIXME in question in the hope the issue resolved itself.
> But I run on Windows, maybe the issue just surfaces on another OS? Is there a 
> way to trigger the CI again?

I also run on Windows so I can't easily test the behavior elsewhere for you. 
The CI will get triggered on new patch uploads, but I still don't always trust 
it. The bots are usually a more reliable source of CI truth but we have no way 
to speculatively trigger all the bots.


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

https://reviews.llvm.org/D80514

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

Reply via email to