LegalizeAdulthood marked 4 inline comments as done. LegalizeAdulthood added inline comments. Herald added a subscriber: jdoerfert.
================ Comment at: clang-tidy/readability/SimplifyBooleanExprCheck.cpp:386 - bool BoolValue = Bool->getValue(); + const bool BoolValue = Bool->getValue(); ---------------- JonasToth wrote: > aaron.ballman wrote: > > LegalizeAdulthood wrote: > > > JonasToth wrote: > > > > `const` on values is uncommon in clang-tidy code. Please keep that > > > > consistent. > > > I can change the code, but I don't understand the push back. > > > > > > "That's the way it's done elsewhere" just doesn't seem like good > > > reasoning. > > > > > > I write const on values to signify that they are computed once and only > > > once. What is gained by removing that statement of once-and-only-once? > > > "That's the way it's done elsewhere" just doesn't seem like good > > > reasoning. > > > > Keeping the code consistent with the vast majority of the remainder of the > > project is valid reasoning. I am echoing the request to drop the top-level > > const. We don't use this pattern for non-pointer/reference types and > > there's not a whole lot gained by using it inconsistently. > Plus we do have a clang-tidy check (in the pipeline) that could do that > automatically if the LLVM projects decides to go that route. So we won't > suffer in the future, if we add the const. You haven't addressed my point, which is that `const` is for values that don't change. Instead, you're just repeating "we haven't done it that way" instead of refuting the utility of `const`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56303/new/ https://reviews.llvm.org/D56303 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
