dsanders updated this revision to Diff 214476.
dsanders added a comment.

- Give in on eliding the llvm:: for `using namespace llvm` inside a function 
They don't appear in the DeclContext and I've been unable to find a way to 
reasonably detect them Fortunately they don't occur in practice


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65919

Files:
  clang-tools-extra/clang-tidy/llvm/CMakeLists.txt
  clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
  clang-tools-extra/clang-tidy/llvm/PreferRegisterOverUnsignedCheck.cpp
  clang-tools-extra/clang-tidy/llvm/PreferRegisterOverUnsignedCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/docs/clang-tidy/checks/llvm-prefer-register-over-unsigned.rst
  clang-tools-extra/test/clang-tidy/llvm-prefer-register-over-unsigned.cpp
  clang-tools-extra/test/clang-tidy/llvm-prefer-register-over-unsigned2.cpp

Index: clang-tools-extra/test/clang-tidy/llvm-prefer-register-over-unsigned2.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/llvm-prefer-register-over-unsigned2.cpp
@@ -0,0 +1,25 @@
+// RUN: %check_clang_tidy %s llvm-prefer-register-over-unsigned %t
+
+namespace llvm {
+class Register {
+public:
+  operator unsigned();
+};
+} // end namespace llvm
+
+llvm::Register getReg();
+
+using namespace llvm;
+
+void apply_1() {
+  unsigned Reg = getReg();
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: variable 'Reg' declared as 'unsigned int'; use 'Register' instead [llvm-prefer-register-over-unsigned]
+  // CHECK-FIXES: apply_1()
+  // CHECK-FIXES-NEXT: Register Reg = getReg();
+}
+
+void done_1() {
+  llvm::Register Reg = getReg();
+  // CHECK-FIXES: done_1()
+  // CHECK-FIXES-NEXT: llvm::Register Reg = getReg();
+}
Index: clang-tools-extra/test/clang-tidy/llvm-prefer-register-over-unsigned.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/llvm-prefer-register-over-unsigned.cpp
@@ -0,0 +1,60 @@
+// RUN: %check_clang_tidy %s llvm-prefer-register-over-unsigned %t
+
+namespace llvm {
+class Register {
+public:
+  operator unsigned();
+};
+} // end namespace llvm
+
+llvm::Register getReg();
+
+void apply_1() {
+  unsigned Reg1 = getReg();
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: variable 'Reg1' declared as 'unsigned int'; use 'llvm::Register' instead [llvm-prefer-register-over-unsigned]
+  // CHECK-FIXES: apply_1()
+  // CHECK-FIXES-NEXT: llvm::Register Reg1 = getReg();
+}
+
+void apply_2() {
+  using namespace llvm;
+  unsigned Reg2 = getReg();
+  // FIXME: Function-scoped UsingDirectiveDecl's don't appear to be in the
+  //        DeclContext for the function so we can't elide the llvm:: in this
+  //        case. Fortunately, it doesn't actually occur in the LLVM code base.
+  // CHECK-MESSAGES: :[[@LINE-4]]:12: warning: variable 'Reg2' declared as 'unsigned int'; use 'llvm::Register' instead [llvm-prefer-register-over-unsigned]
+  // CHECK-FIXES: apply_2()
+  // CHECK-FIXES-NEXT: using namespace llvm;
+  // CHECK-FIXES-NEXT: llvm::Register Reg2 = getReg();
+}
+
+namespace llvm {
+void apply_3() {
+  unsigned Reg3 = getReg();
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: variable 'Reg3' declared as 'unsigned int'; use 'Register' instead [llvm-prefer-register-over-unsigned]
+  // CHECK-FIXES: apply_3()
+  // CHECK-FIXES-NEXT: Register Reg3 = getReg();
+}
+} // end namespace llvm
+
+void done_1() {
+  llvm::Register Reg1 = getReg();
+  // CHECK-FIXES: done_1()
+  // CHECK-FIXES-NEXT: llvm::Register Reg1 = getReg();
+}
+
+void done_2() {
+  using namespace llvm;
+  Register Reg2 = getReg();
+  // CHECK-FIXES: done_2()
+  // CHECK-FIXES-NEXT: using namespace llvm;
+  // CHECK-FIXES-NEXT: Register Reg2 = getReg();
+}
+
+namespace llvm {
+void done_3() {
+  Register Reg3 = getReg();
+  // CHECK-FIXES: done_3()
+  // CHECK-FIXES-NEXT: Register Reg3 = getReg();
+}
+} // end namespace llvm
Index: clang-tools-extra/docs/clang-tidy/checks/llvm-prefer-register-over-unsigned.rst
===================================================================
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/llvm-prefer-register-over-unsigned.rst
@@ -0,0 +1,27 @@
+.. title:: clang-tidy - llvm-prefer-register-over-unsigned
+
+llvm-prefer-register-over-unsigned
+==================================
+
+Finds historical use of ``unsigned`` to hold vregs and physregs and rewrites
+them to use ``Register``.
+
+Currently this works by finding all variables of unsigned integer type whose
+initialize begins with an implicit cast from ``Register`` to ``unsigned``.
+
+.. code-block:: c++
+
+  void example(MachineOperand &MO) {
+    unsigned Reg = MO.getReg();
+    ...
+  }
+
+becomes:
+
+.. code-block:: c++
+
+  void example(MachineOperand &MO) {
+    Register Reg = MO.getReg();
+    ...
+  }
+
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -274,6 +274,7 @@
    llvm-include-order
    llvm-namespace-comment
    llvm-prefer-isa-or-dyn-cast-in-conditionals
+   llvm-prefer-register-over-unsigned
    llvm-twine-local
    misc-definitions-in-headers
    misc-misplaced-const
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -79,6 +79,12 @@
   Finds uses of deprecated Googletest APIs with names containing ``case`` and
   replaces them with equivalent APIs with ``suite``.
 
+- New :doc:`llvm-prefer-register-over-unsigned
+  <clang-tidy/checks/llvm-prefer-register-over-unsigned>` check.
+
+  Finds historical use of ``unsigned`` to hold vregs and physregs and rewrites
+  them to use ``Register``
+
 
 Improvements to include-fixer
 -----------------------------
Index: clang-tools-extra/clang-tidy/llvm/PreferRegisterOverUnsignedCheck.h
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/llvm/PreferRegisterOverUnsignedCheck.h
@@ -0,0 +1,36 @@
+//===--- PreferRegisterOverUnsignedCheck.h - clang-tidy ---------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_PREFERREGISTEROVERUNSIGNEDCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_PREFERREGISTEROVERUNSIGNEDCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace llvm_check {
+
+/// Historically, LLVM has used `unsigned` to represent registers. Since then
+/// a `Register` object has been introduced for improved type-safety and make
+/// the code more explicit.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/llvm-prefer-register-over-unsigned.html
+class PreferRegisterOverUnsignedCheck : public ClangTidyCheck {
+public:
+  PreferRegisterOverUnsignedCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace llvm_check
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_PREFERREGISTEROVERUNSIGNEDCHECK_H
Index: clang-tools-extra/clang-tidy/llvm/PreferRegisterOverUnsignedCheck.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/llvm/PreferRegisterOverUnsignedCheck.cpp
@@ -0,0 +1,63 @@
+//===--- PreferRegisterOverUnsignedCheck.cpp - clang-tidy -----------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "PreferRegisterOverUnsignedCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace llvm_check {
+
+void PreferRegisterOverUnsignedCheck::registerMatchers(MatchFinder *Finder) {
+  auto RegisterClassMatch = hasType(
+      cxxRecordDecl(hasName("llvm::Register")).bind("registerClassDecl"));
+
+  Finder->addMatcher(
+      valueDecl(allOf(
+          hasType(qualType(isUnsignedInteger()).bind("varType")),
+          varDecl(
+              hasInitializer(exprWithCleanups(
+                  has(implicitCastExpr(has(cxxMemberCallExpr(allOf(
+                      on(RegisterClassMatch),
+                      has(memberExpr(hasDeclaration(
+                          cxxConversionDecl().bind("operatorDecl"))))))))))))
+              .bind("var"))),
+      this);
+}
+
+void PreferRegisterOverUnsignedCheck::check(
+    const MatchFinder::MatchResult &Result) {
+  const auto *VarType = Result.Nodes.getNodeAs<QualType>("varType");
+  const auto *UserVarDecl = Result.Nodes.getNodeAs<VarDecl>("var");
+
+  StringRef Replacement = "llvm::Register";
+  const DeclContext *Context = UserVarDecl->getDeclContext();
+  while (Context) {
+    if (const auto *Namespace = dyn_cast<NamespaceDecl>(Context))
+      if (Namespace->getQualifiedNameAsString() == "llvm")
+        Replacement = "Register";
+    for (const auto *UsingDirective : Context->using_directives())
+      if (UsingDirective->getNominatedNamespace()->getQualifiedNameAsString() ==
+          "llvm")
+        Replacement = "Register";
+    Context = Context->getParent();
+  }
+  diag(UserVarDecl->getLocation(),
+       "variable %0 declared as %1; use '%2' instead")
+      << UserVarDecl << *VarType << Replacement
+      << FixItHint::CreateReplacement(
+             UserVarDecl->getTypeSourceInfo()->getTypeLoc().getSourceRange(),
+             Replacement);
+}
+
+} // namespace llvm_check
+} // namespace tidy
+} // namespace clang
Index: clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
===================================================================
--- clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
+++ clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
@@ -13,6 +13,7 @@
 #include "HeaderGuardCheck.h"
 #include "IncludeOrderCheck.h"
 #include "PreferIsaOrDynCastInConditionalsCheck.h"
+#include "PreferRegisterOverUnsignedCheck.h"
 #include "TwineLocalCheck.h"
 
 namespace clang {
@@ -24,6 +25,8 @@
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.registerCheck<LLVMHeaderGuardCheck>("llvm-header-guard");
     CheckFactories.registerCheck<IncludeOrderCheck>("llvm-include-order");
+    CheckFactories.registerCheck<PreferRegisterOverUnsignedCheck>(
+        "llvm-prefer-register-over-unsigned");
     CheckFactories.registerCheck<readability::NamespaceCommentCheck>(
         "llvm-namespace-comment");
     CheckFactories.registerCheck<PreferIsaOrDynCastInConditionalsCheck>(
Index: clang-tools-extra/clang-tidy/llvm/CMakeLists.txt
===================================================================
--- clang-tools-extra/clang-tidy/llvm/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/llvm/CMakeLists.txt
@@ -5,6 +5,7 @@
   IncludeOrderCheck.cpp
   LLVMTidyModule.cpp
   PreferIsaOrDynCastInConditionalsCheck.cpp
+  PreferRegisterOverUnsignedCheck.cpp
   TwineLocalCheck.cpp
 
   LINK_LIBS
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to