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

Alexander Kornienko via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Apr 17 12:41:51 PDT 2019

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.

  rG LLVM Github Monorepo



More information about the cfe-commits mailing list