JonasToth marked an inline comment as done. JonasToth added inline comments.
================ Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.cpp:46 + +void ConstCorrectnessCheck::registerMatchers(MatchFinder *Finder) { + const auto ConstType = hasType(isConstQualified()); ---------------- aaron.ballman wrote: > JonasToth wrote: > > aaron.ballman wrote: > > > Should this check only fire in C++? I'm sort of on the fence. It's a C++ > > > core guideline, so it stands to reason it should be disabled for C code. > > > But const-correctness is a thing in C too. WDYT? > > I do not know all the subtle differences between C and C++ here. Judging > > from this: https://stackoverflow.com/questions/5248571/is-there-const-in-c > > the current form would not be correct for C for pointers. > > The assumptions of this check and especially the transformations are done > > for C++. I dont see a reason why the value-semantic would not apply for > > both. > > > > Maybe there is a way to make the code compatible for both languages. The > > easiest solution would probably to not do the 'pointer-as-value' > > transformation. This is not that relevant as a feature anyway. I expect not > > nearly as much usage of this option as for the others. > > > > In the end of the day i would like to support both C and C++. Right now it > > is untested and especially the transformation might break the code. It > > should run on both languages though, as there is no language checking. > > I will add some real world C code-bases for the transformation testing and > > see what happens :) > > Judging from this: > > https://stackoverflow.com/questions/5248571/is-there-const-in-c the current > > form would not be correct for C for pointers. > > Sure, there may be additional changes needed to support the other oddities of > C. I was asking more at the high level. > > > In the end of the day i would like to support both C and C++. Right now it > > is untested and especially the transformation might break the code. > > Another option is for us to restrict the check in its current form to just > C++ and then expose a C check from it in a follow-up (likely under a > different module than cppcodeguidelines). For instance, CERT has a > recommendation (not a rule) about this for C > (https://wiki.sei.cmu.edu/confluence/display/c/DCL00-C.+Const-qualify+immutable+objects) > and I would not be surprised to find it in other coding standards as well. Another const check is probably better. The clang-tidy part should be minimal effort. I think there isn't alot of duplication either. We could still think of proper configurability of this check and alias it into other modules with proper defaults for C. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54943/new/ https://reviews.llvm.org/D54943 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits