JonasToth added a comment.

thanks for your testing! i will look at the `__unaligned` issue, not sure if 
clang supports it, its an MSVC extension, is it?

Did the transformations produce issues and approximately how much code did you 
check? I would like to get a better feeling for its quality before enabling it 
by default.



================
Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.cpp:168
+            *Variable, *Result.Context, DeclSpec::TQ_const,
+            QualifierTarget::Value, QualifierPolicy::Right)) {
+      Diag << *Fix;
----------------
tiagoma wrote:
> The core guidelines suggests the use of west const. Could this be made the 
> default?
> https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rl-const
https://reviews.llvm.org/D69764


I was hoping for this feature to handle this instead.
It is extremely complicated to figure out where to insert the `const`, because 
pointer, pointee, value, templates, macros and so on and so forth.
I implemented it such that it is always correct (the right side) and hoped that 
clang-format can fix it up afterwards (`clang-tidy -fix -format`)


================
Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.h:31
+        WarnPointersAsValues(Options.get("WarnPointersAsValues", 0)),
+        TransformValues(Options.get("TransformValues", 1)),
+        TransformReferences(Options.get("TransformReferences", 1)),
----------------
tiagoma wrote:
> JonasToth wrote:
> > Deactivating the transformations by default, as they are not ready for 
> > production yet.
> Ok. This got me off-guard. I would rather have this on.
the transformations were buggy and required a lot of fix-up. This might not be 
the case, but i would rather have it non-destructive by default because i think 
many people will throw this check at their code-base.

if the transformations are solid already we can activate it again (or maybe 
references only or so).


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