[PATCH] D102779: [clang-tidy] cppcoreguidelines-explicit-constructor-and-conversion: new alias

Marco Gartmann via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jun 14 05:47:17 PDT 2021


mgartmann added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/google/ExplicitConstructorCheck.cpp:28-30
+  std::string FullOperatorName =
+      Node.getParent()->getNameAsString().append("::").append(
+          Node.getNameAsString());
----------------
aaron.ballman wrote:
> Rather than trying to do this by hand, does `Decl::print()` give you the functionality you need? For example, this will likely not work well for classes defined within a namespace (it may ignore the wrong class due to not checking the namespace). Another thing to consider are templates and how to handle those. e.g.,
> ```
> struct Foo {
>   template <typename Ty>
>   operator Ty() const; // How to silence the diagnostic here?
> };
> ```
> Thankfully, specializations can't differ in their explicitness, so you don't have to also worry about:
> ```
> struct Foo {
>   template <typename Ty>
>   explicit operator Ty() const; // This one's explicit
> };
> 
> template <>
> Foo::operator int() const; // Thankfully, this inherits the explicit from the primary template.
> ```
Thanks for your comment, @aaron.ballman!

I was able to use `printQualifiedName` to get a `Node`'s qualified name, including its namespace. 

Templated operators are not represented by their name visible in the source code (e.g., `Ty`) in the AST. Instead, their name in the AST is something like `type-parameter-0-0`. As it is now, templated operators have to be ignored with the latter name, which is also displayed in the check's diagnostic message.
This was described in the documentation accordingly.

I was not able to find a feasible way to enable users to exclude templated operators by their original name (e.g., `Ty`). Does anyone have an idea how this could be done?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102779



More information about the cfe-commits mailing list