[PATCH] D65919: [clang-tidy] Add llvm-prefer-register-over-unsigned check and apply it to LLVM

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Aside from the few remaining nits, I think this is almost ready to go.



================
Comment at: clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp:32-33
+        "llvm-prefer-register-over-unsigned");
+    CheckFactories.registerCheck<readability::NamespaceCommentCheck>(
+        "llvm-namespace-comment");
     CheckFactories.registerCheck<TwineLocalCheck>("llvm-twine-local");
----------------
Spurious change that moved `llvm-namespace-comment` to be out of alphabetical order?


================
Comment at: clang-tools-extra/test/clang-tidy/llvm-prefer-register-over-unsigned.cpp:60
+}
+} // end namespace llvm
----------------
dsanders wrote:
> aaron.ballman wrote:
> > I'd also like to see some tests like:
> > ```
> > void func(unsigned Reg);
> > 
> > struct S {
> >   unsigned Reg;
> > 
> >   S(unsigned Reg) : Reg(Reg) {}
> > };
> > 
> > void other_func() {
> >   func(getReg());
> >   S s{getReg()};
> >   s.Reg = getReg();
> > }
> > ```
> Added tests for the following cases that do not make changes*
> 1. Similar interface but not called Register
> 2. Register class not inside llvm namespace
> 3. Calling a function that takes an llvm::Register with a llvm::Register argument
> 4. Calling a function that takes an unsigned and is given an llvm::Register argument
> 5. Initializing an llvm::Register using an llvm::Register argument
> 6. Initializing any other object using a constructor parameter that's unsigned and a llvm::Register argument
> 7. Assigning to a member of llvm::Register
> 8. Assigning to a unsigned member of any other object
> 
> *4, 6, and 8 should eventually but don't yet
Nice, thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65919