[PATCH] D75340: [clang-tidy] Change checks to use new isLanguageVersionSupported restriction

Dmitri Gribenko via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Mar 3 08:06:07 PST 2020


gribozavr2 accepted this revision.
gribozavr2 added inline comments.
This revision is now accepted and ready to land.


================
Comment at: clang-tools-extra/clang-tidy/cert/DefaultOperatorNewAlignmentCheck.h:30
+  bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+    return LangOpts.CPlusPlus && !LangOpts.CPlusPlus17;
+  }
----------------
njames93 wrote:
> gribozavr2 wrote:
> > I think CPlusPlus17 implies CPlusPlus.
> This is saying it needs c++ but not c++17, so c++98->c++14 are Ok but not 17 or 20. Besides this is the current behaviour in the cpp file
Sorry, I misread and didn't notice the exclamation mark. Your edit makes sense to me.

I commented because I was comparing with the .cpp file which was not checking `CPlusPlus`:

```
void DefaultOperatorNewAlignmentCheck::registerMatchers(MatchFinder *Finder) {
  // Check not applicable in C++17 (or newer).
  if (getLangOpts().CPlusPlus17)
    return;
```

So there is a tiny change here: previously, this check was being registered in non-C++ language modes, now it is not. I think your change to only enable in C++ modes is good, but again I think it would be best done in a separate patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75340





More information about the cfe-commits mailing list