aaron.ballman 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());
----------------
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.
```
================
Comment at: clang-tools-extra/clang-tidy/google/ExplicitConstructorCheck.cpp:33
+ return llvm::any_of(IgnoredConversionOps,
+ [FullOperatorName](std::string NameInOptions) {
+ return NameInOptions == FullOperatorName;
----------------
================
Comment at: clang-tools-extra/clang-tidy/google/ExplicitConstructorCheck.cpp:102
if (const auto *Conversion =
- Result.Nodes.getNodeAs<CXXConversionDecl>("conversion")) {
+ Result.Nodes.getNodeAs<CXXConversionDecl>("conversion")) {
if (Conversion->isOutOfLine())
----------------
mgartmann wrote:
> aaron.ballman wrote:
> > Unintended formatting change?
> This formatting change was done by `clang-format` when `git clang-format
> HEAD~1` was run. Therefore, I assumed that it is now correctly formatted.
>
> Am I wrong?
I've never used the integrated git clang-format before, but I sort of wonder if
it is getting extra context lines and that's why this one changed? I usually do
`git diff -U0 --no-color HEAD^ | clang-format-diff.py -i -p1` and then I know
exactly what range of lines are being touched.
================
Comment at:
clang-tools-extra/docs/clang-tidy/checks/google-explicit-constructor.rst:74-75
+ ignored and will not trigger a warning. The list to ignore conversion
+ operator `operator B()` in class ``A`` would look as follows:
+ ``"A::operator B"``. The default list is empty.
----------------
We should probably document other identifiers that contribute to the name of
the operator, like namespaces, template ids, etc. Using code examples for
anything you think is tricky would be helpful.
================
Comment at:
clang-tools-extra/test/clang-tidy/checkers/google-explicit-constructor-ignoredconversionoperators-option.cpp:84
+
+ operator A() const;
+ // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:3: warning: 'operator A' must be
marked explicit to avoid unintentional implicit conversions
[google-explicit-constructor]
----------------
Can you also add an example that an explicit conversion operator does not
diagnose?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D102779/new/
https://reviews.llvm.org/D102779
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
