[PATCH] D42983: [clang-tidy] Add readability-simd-intrinsics check.

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.


================
Comment at: clang-tidy/readability/SIMDIntrinsicsCheck.cpp:77
+void SIMDIntrinsicsCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(
+      callExpr(callee(functionDecl(matchesName("^::(_mm_|_mm256_|_mm512_|vec_)"))),
----------------
Eugene.Zelenko wrote:
> MaskRay wrote:
> > Eugene.Zelenko wrote:
> > > You should enable this check only when compiling in appropriate C++ version. See getLangOpts() usage in other checks.
> > Thx, I didn't know getLangOpts() before.
> > 
> > I think even in `-x c` mode, `::` is still the prefix of an identifier.
> > 
> >       matchesName("^(_mm_|_mm256_|_mm512_|vec_)")
> > 
> > doesn't match anything but 
> > 
> >       matchesName("^::(_mm_|_mm256_|_mm512_|vec_)")
> > 
> > matches.
> > 
> > verified via `clang-tidy -checks='-*,readability-simd-intrinsics' a.c -- -xc`
> > 
> Check should be enabled only for C++ version which has std::experimental::simd implementation, so this is why you need to use getLangOpts().
I think this is still not done, 
```
        Experimental(Options.getLocalOrGlobal("Experimental", 1) != 0)
```
means it will still be on by default for C++11


================
Comment at: clang-tidy/readability/SIMDIntrinsicsCheck.cpp:106
+  } else
+    return;
+
----------------
This logic should be in the beginning of `registerMatchers()`, so this wouldn't even be registered/called if it won't do anything.


================
Comment at: clang-tidy/readability/SIMDIntrinsicsCheck.h:27
+      : ClangTidyCheck(Name, Context),
+        Experimental(Options.getLocalOrGlobal("Experimental", 1) != 0) {}
+
----------------
I //think// usually this is in `.cpp` file


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D42983