mikecrowe added inline comments.

================
Comment at: clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.cpp:46
+void UseStdFormatCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(
+      callExpr(argumentCountAtLeast(1), hasArgument(0, stringLiteral()),
----------------
This matcher also matches the `operator+` call in:
```
std::string A(const std::string &in)                                            
                                                                                
                
{                                                                               
                                                                                
                
    return "_" + in;                                                            
                                                                                
                
}                                                                               
                                                                                
                
```
which causes an assertion failure:
```
clang-tidy: /home/mac/git/llvm-project/clang/include/clang/AST/Decl.h:275: 
llvm::StringRef clang::NamedDecl::getName() const: Assertion 
`Name.isIdentifier() && "Name is not a simple identifier"' failed.
```
when the `StrFormatLikeFunctions` option is set to an unqualified name:
```
-config="{CheckOptions: [{key: modernize-use-std-format.StrictMode, value: 
false}, {key: modernize-use-std-format.StrFormatLikeFunctions, value: 
'strprintf'}]}"
```

`MatchesAnyListedNameMatcher::NameMatcher::match` calls `NamedDecl.getName()` 
which presumably raises the assertion due to the `operator+` not having a name 
(that's mentioned in the source anyway.)

I'm unsure whether I should be narrowing the matcher here so that it guaranteed 
to not try calling `matchesAnyListedName` on something that lacks a name, or 
whether `MatchesAnyListedNameMatcher` ought to be more tolerant of being called 
in such situations.

I note that `HasNameMatcher` has rather more elaborate code for generating the 
name than `MatchesAnyListedNameMatcher` does.

This problem also affects `modernize-use-std-print`, but due to the need for 
there to be no return value in that check it requires somewhat-unlikely code 
like:
```
void A(const std::string &in)
{
  "_" + in;
}
```

Do you have any advice? Given that this problem affects a check that has 
already landed should I open a bug?


================
Comment at: clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.cpp:47
+  Finder->addMatcher(
+      callExpr(argumentCountAtLeast(1), hasArgument(0, stringLiteral()),
+               callee(functionDecl(matchers::matchesAnyListedName(
----------------
I need to add an `isOrdinary()` call here like `modernize-use-std-print` has.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154287

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to