[PATCH] D60629: [clang-tidy] Change the namespace for llvm checkers from 'llvm' to 'llvm_check'

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/add_new_check.py:382
+  if module == 'llvm' or module == 'clang':
+    namespace = module + '_checker'
+  else:
----------------
hintonda wrote:
> aaron.ballman wrote:
> > I thought we were going with `llvm_check`?
> Just typo on my part.  I the options mentioned before where things like `llvm_project` or `llvm_proj`, etc.  I just settled on `llvm_check` because it sounded better -- then actually typed `checker`.  
> 
> I'll fix it this afternoon.
I have another suggestion re: the color of this bike shed, namely: `llvm_checks`. But I don't feel strongly about this.


================
Comment at: clang-tools-extra/clang-tidy/add_new_check.py:381
+
+  # Don't allow 'clang' or 'llvm' namespaces
+  if module == 'llvm':
----------------
I'd add an explanation: `# Map module names to namespace names that don't conflict with widely used top-level namespaces.`

And again, no need to mention `clang` here.


================
Comment at: clang-tools-extra/clang-tidy/rename_check.py:218
   header_guard_variants = [
+      (args.old_check_name.replace('-', '_') + '_Check').upper(),
+      (old_module + '_' + check_name_camel).upper(),
----------------
s/_Check/_CHECK/, maybe?


================
Comment at: clang-tools-extra/clang-tidy/rename_check.py:267
 
-  if old_module != new_module:
+  if old_module != new_module or new_module == 'llvm':
+    if new_module == 'llvm':
----------------
I believe, we should first construct the new namespace name (using exactly the same code as in add_new_check.py) and then check if it differs from the old one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60629