https://github.com/vbvictor commented:
Looks good, thanks for your work; however, I think we should add some options
to tweak behavior before we can land it.
One major drawback is this pattern:
```diff
template <typename T> std::string getArcana(const T &N) {
return dump([&](TextNodeDumper &D) { D.Visit(N); });
}
- std::string getArcana(const NestedNameSpecifierLoc &NNS) { return ""; }
- std::string getArcana(const TemplateName &NNS) { return ""; }
- std::string getArcana(const CXXBaseSpecifier &CBS) { return ""; }
+ std::string_view getArcana(const NestedNameSpecifierLoc &NNS) { return ""; }
+ std::string_view getArcana(const TemplateName &NNS) { return ""; }
+ std::string_view getArcana(const CXXBaseSpecifier &CBS) { return ""; }
std::string getArcana(const TemplateArgumentLoc &TAL) {
return dump([&](TextNodeDumper &D) {
D.Visit(TAL.getArgument(), TAL.getSourceRange());
});
}
std::string getArcana(const TypeLoc &TL) {
return dump([&](TextNodeDumper &D) { D.Visit(TL.getType()); });
}
```
We have plenty of similar methods declared with `string` but some of them
appear to use `std::string_view`. I'd want to leave `string` even in trivial
cases for consistency.
So an option `AllowExplicitStringReturn` would allow manual `string` creation
like this:
```cpp
std::string getArcana(const TemplateName &NNS) {
return std::string{""}; // no warning
return std::string(""); // no warning
}
```
Instead of writing a bunch of `// NOLINT`
Note that `string("")` may conflict with `modernize-return-braced-init-list`
but `string{}` won't.
Another drawback is:
```diff
-std::string toString(InvalidName::Kind K) {
+std::string_view toString(InvalidName::Kind K) {
switch (K) {
case InvalidName::Keywords:
return "Keywords";
llvm_unreachable("unhandled InvalidName kind");
}
```
We probably want real `std::string` returned here, so I suggest the
`IgnoredFunctions` option to handle this case (could be empty by default, and
users would write their own functions like `to_string;toString`).
There are `IgnoredFunctions` in other options too.
https://github.com/llvm/llvm-project/pull/172170
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
