aaron.ballman added inline comments.

================
Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.cpp:46
+
+void ConstCorrectnessCheck::registerMatchers(MatchFinder *Finder) {
+  const auto ConstType = hasType(isConstQualified());
----------------
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.


================
Comment at: clang-tools-extra/docs/ReleaseNotes.rst:121
+^^^^^^^^^^^^^^^^^
+>>>>>>> 8d940dfbccb... remove spurious formatting change
+
----------------
This looks unintentional to me.


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

Reply via email to