[clang-tidy] prevent readability-identifier-naming from triggering on implicit VarDecls in coroutines

[0x1eaf]

Summary

We have an internal bug report about readability-identifier-naming
warnings on implicit __coro_gro and __promise variables
in coroutines.

Turns out they haven't been marked as implicit in SemaCoroutine.
This diff resolves that and adds a couroutine test for the check.

Tested with:

> cd build/Debug/bin
> ./llvm-lit -sv --param build_mode=Debug \
             ../../tools/clang/tools/extra/test \
             --filter readability-identifier-naming.cpp

[aaron.ballman]

LGTM aside from some small nits, thank you for the fix! Do you need me to commit on your behalf? (If so, are you okay with using Emma Blink <[e-mail]> for patch attribution?)

clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp

619

clang/lib/Sema/SemaCoroutine.cpp

547

1581

[0x1eaf]

Updated to address review comments.

[0x1eaf]

Do you need me to commit on your behalf?

Yes, I don't have commit access. The attribution line looks good to me. Thank you! 🙂

[aaron.ballman]

In D93402#2458591, @0x1eaf wrote:

Do you need me to commit on your behalf?

Yes, I don't have commit access. The attribution line looks good to me. Thank you! 🙂

I've commit on your behalf in 7685d818ef329cd3f6ef121af1208be409eb59db, thank you for the patch!

clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/system/coroutines.h

+#pragma once
+
+namespace std {
+namespace experimental {
+
+template <typename ret_t, typename... args_t>
+struct coroutine_traits {
+  using promise_type = typename ret_t::promise_type;
+};
+
+template <class promise_t>
+struct coroutine_handle {
+  static constexpr coroutine_handle from_address(void *addr) noexcept { return {}; };
+};
+
+} // namespace experimental
+} // namespace std
+
+struct never_suspend {
+  bool await_ready() noexcept { return false; }
+  template <typename coro_t>
+  void await_suspend(coro_t handle) noexcept {}
+  void await_resume() noexcept {}
+};
+
+struct task {
+  struct promise_type {
+    task get_return_object() noexcept { return {}; }
+    never_suspend initial_suspend() noexcept { return {}; }
+    never_suspend final_suspend() noexcept { return {}; }
+    void return_void() {}
+    void unhandled_exception() {}
+  };
+};

clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp

 // RUN:     {key: readability-identifier-naming.PointerParameterCase, value: lower_case}, \
 // RUN:     {key: readability-identifier-naming.PointerParameterPrefix, value: 'p_'}, \
 // RUN:     {key: readability-identifier-naming.ConstantPointerParameterCase, value: CamelCase}, \
 // RUN:     {key: readability-identifier-naming.ConstantPointerParameterPrefix, value: 'cp_'}, \
 // RUN:     {key: readability-identifier-naming.LocalPointerCase, value: CamelCase}, \
 // RUN:     {key: readability-identifier-naming.LocalPointerPrefix, value: 'l_'}, \
 // RUN:     {key: readability-identifier-naming.LocalConstantPointerCase, value: CamelCase}, \
 // RUN:     {key: readability-identifier-naming.LocalConstantPointerPrefix, value: 'lc_'}, \
-// RUN:   ]}' -- -fno-delayed-template-parsing -Dbad_macro \
+// RUN:   ]}' -- -fno-delayed-template-parsing -Dbad_macro -std=c++17 -fcoroutines-ts \
 // RUN:   -I%S/Inputs/readability-identifier-naming \
 // RUN:   -isystem %S/Inputs/readability-identifier-naming/system
 
 // clang-format off
 
 #include <system-header.h>
+#include <coroutines.h>
 #include "user-header.h"
 // NO warnings or fixes expected from declarations within header files without
 // the -header-filter= option
 
 namespace FOO_NS {
 // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: invalid case style for namespace 'FOO_NS' [readability-identifier-naming]
 // CHECK-FIXES: {{^}}namespace foo_ns {{{$}}
 inline namespace InlineNamespace {
public:
   // CHECK-FIXES: {{^}}  virtual void v_Bad_Base_Method() = 0;
 };
 
 class COverriding : public AOverridden {
 public:
   // Overriding a badly-named base isn't a new violation.
   void BadBaseMethod() override {}
   // CHECK-FIXES: {{^}}  void v_Bad_Base_Method() override {}
-  
   void foo() {
     BadBaseMethod();
     // CHECK-FIXES: {{^}}    v_Bad_Base_Method();
     this->BadBaseMethod();
     // CHECK-FIXES: {{^}}    this->v_Bad_Base_Method();
     AOverridden::BadBaseMethod();
     // CHECK-FIXES: {{^}}    AOverridden::v_Bad_Base_Method();
     COverriding::BadBaseMethod();

 #undef M1
 #undef DUP
 } // namespace scratchspace
 
 template<typename type_t>
 auto GetRes(type_t& Param) -> decltype(Param.res());
 // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: invalid case style for parameter 'Param'
 // CHECK-FIXES: auto GetRes(type_t& a_param) -> decltype(a_param.res());
+
+// Check implicit declarations in coroutines
+
+struct async_obj {
+public:
+  never_suspend operator co_await() const noexcept;
+};
+
+task ImplicitDeclTest(async_obj &a_object) {
+  co_await a_object;  // CHECK-MESSAGES-NOT: warning: invalid case style for local variable
+}

clang/lib/Sema/SemaCoroutine.cpp

QualType T = FD->getType()->isDependentType() || IsThisDependentType
                    ? Context.DependentTy
                    : lookupPromiseType(*this, FD, Loc);
   if (T.isNull())
     return nullptr;
 
   auto *VD = VarDecl::Create(Context, FD, FD->getLocation(), FD->getLocation(),
                              &PP.getIdentifierTable().get("__promise"), T,
                              Context.getTrivialTypeSourceInfo(T, Loc), SC_None);
+  VD->setImplicit();
   CheckVariableDeclarationType(VD);
   if (VD->isInvalidDecl())
     return nullptr;
 
   auto *ScopeInfo = getCurFunction();
 
   // Build a list of arguments, based on the coroutine function's arguments,
   // that if present will be passed to the promise type's constructor.
if (GroType->isVoidType()) {
     noteMemberDeclaredHere(S, ReturnValue, Fn);
     return false;
   }
 
   auto *GroDecl = VarDecl::Create(
       S.Context, &FD, FD.getLocation(), FD.getLocation(),
       &S.PP.getIdentifierTable().get("__coro_gro"), GroType,
       S.Context.getTrivialTypeSourceInfo(GroType, Loc), SC_None);
+  GroDecl->setImplicit();
 
   S.CheckVariableDeclarationType(GroDecl);
   if (GroDecl->isInvalidDecl())
     return false;
 
   InitializedEntity Entity = InitializedEntity::InitializeVariable(GroDecl);
   ExprResult Res = S.PerformMoveOrCopyInitialization(Entity, nullptr, GroType,
                                                      this->ReturnValue);