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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
