lebedev.ri added inline comments.
================
Comment at: clang-tidy/readability/SIMDIntrinsicsCheck.cpp:26
+
+ static const llvm::StringMap<std::string> Mapping{
+ // [simd.alg]
----------------
You probably want to move `Mapping` out of the function.
================
Comment at: clang-tidy/readability/SIMDIntrinsicsCheck.cpp:34
+ {"sub", "operator- on std::experimental::simd objects"},
+ {"mul", "operator* on std::experimental::simd objects"},
+ };
----------------
To point the obvious, this is not exhaustive list, at least `div` is missing.
================
Comment at: clang-tidy/readability/SIMDIntrinsicsCheck.cpp:55
+ // [simd.binary]
+ if (Name.startswith("add_"))
+ return "operator+ on std::experimental::simd objects";
----------------
This function was not updated to use the `Mapping` map.
================
Comment at: clang-tidy/readability/SIMDIntrinsicsCheck.cpp:75
+ // libcxx implementation of std::experimental::simd requires at least C++11.
+ if (!Result.Context->getLangOpts().CPlusPlus11)
+ return;
----------------
Is it reasonable to suggest to use `<experimental/*>`?
I would guess it should be `CPlusPlus2a`
================
Comment at: clang-tidy/readability/SIMDIntrinsicsCheck.cpp:84
+ // SIMD intrinsic call.
+ const FunctionDecl *Callee = Call->getDirectCallee();
+ if (!Callee)
----------------
I would refactor this as astmatcher, at least a local one, e.g. something like
```
AST_MATCHER(CallExpr, hasDirectCallee) {
return Node.getDirectCallee();
}
```
+
```
void SIMDIntrinsicsCheck::registerMatchers(MatchFinder *Finder) {
Finder->addMatcher(
callExpr(callee(
+ allOf(
functionDecl(matchesName("^::(_mm_|_mm256_|_mm512_|vec_)")
+ , hasDirectCallee()
+ )
)),
unless(isExpansionInSystemHeader()))
.bind("call"),
this);
}
```
Unless of course there is already a narrower that does that
================
Comment at: clang-tidy/readability/SIMDIntrinsicsCheck.cpp:87
+ return;
+ bool IsVector = Callee->getReturnType()->isVectorType();
+ for (const ParmVarDecl *Parm : Callee->parameters()) {
----------------
Same here, i *think* something like this would be better?
```
AST_MATCHER(FunctionDecl, isVectorFunction) {
bool IsVector = Node.getReturnType()->isVectorType();
for (const ParmVarDecl *Parm : Node.parameters()) {
QualType Type = Parm->getType();
if (Type->isPointerType())
Type = Type->getPointeeType();
if (Type->isVectorType())
IsVector = true;
return IsVector;
}
```
+
```
void SIMDIntrinsicsCheck::registerMatchers(MatchFinder *Finder) {
Finder->addMatcher(
callExpr(callee(
allOf(
functionDecl(
+ allOf(
matchesName("^::(_mm_|_mm256_|_mm512_|vec_)")
+ , isVectorFunction()
+ )
, hasDirectCallee()
)
)),
unless(isExpansionInSystemHeader()))
.bind("call"),
this);
}
```
================
Comment at: docs/clang-tidy/checks/readability-simd-intrinsics.rst:1
+.. title:: clang-tidy - simd-readability-intrinsics
+
----------------
This should be
```
.. title:: clang-tidy - readability-simd-intrinsics
```
================
Comment at: docs/clang-tidy/checks/readability-simd-intrinsics.rst:3
+
+simd-readability-intrinsics
+===========================
----------------
Here too
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D42983
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
