[clang-tidy] Don't complain about const pass_object_size params in declarations

[george.burgess.iv]

Summary

This patch seems trivial, but I've never touched clang-tidy before, so I'm just making sure I didn't miss something obvious. :)

---

Clang has a parameter attribute called pass_object_size, which requires that its parameter's type be const. This restriction only applies at function definitions. e.g.

void foo(void *p __attribute__((pass_object_size(0))); // this is a decl; const isn't required.
void foo(void *const p __attribute__((pass_object_size(0))) {} // const is required; this is a def.

readability-avoid-const-params-in-decls will complain if you put const in the decl. Given that clang gives you an error unless you use const in the def, I don't think it's clearly a bad thing to use const in the decl. This patch makes said checker not complain about const params with the pass_object_size attribute.

[alexfh]

There's a principal difference between top-level const on parameters in definitions and in declarations: in definitions const has an effect, as it makes the variable constant, but in declarations it has absolutely no effect. Since declarations usually represent the function's interface, and top-level const on parameters doesn't change the function's signature, it's considered useless and misleading when used on declarations. Long story short, I don't see any value in this change.

[alexfh]

To clarify: the pass_object_size attribute is not the only reason to mark parameter const in the definition. However, there's no reason to also mark them const in declarations, since that const is not a part of the function's interface.

[george.burgess.iv]

There's a principal difference between top-level const on parameters in definitions and in declarations: in definitions const has an effect, as it makes the variable constant, but in declarations it has absolutely no effect [...] it's considered useless and misleading when used on declarations

Yup! I didn't realize it was considered misleading, though -- good to know.

Long story short, I don't see any value in this change.

Yeah, the more I think about it, the more I agree that this doesn't clearly add value, so I'll drop it. Thanks for the review :)

clang-tidy/readability/AvoidConstParamsInDecls.cpp

SourceRange getTypeRange(const ParmVarDecl &Param) {
   return Param.getSourceRange();
 }
 
 } // namespace
 
 
 void AvoidConstParamsInDecls::registerMatchers(MatchFinder *Finder) {
   const auto ConstParamDecl =
-      parmVarDecl(hasType(qualType(isConstQualified()))).bind("param");
+      parmVarDecl(hasType(qualType(isConstQualified())),
+                  unless(hasAttr(clang::attr::PassObjectSize))).bind("param");
   Finder->addMatcher(functionDecl(unless(isDefinition()),
                                   has(typeLoc(forEach(ConstParamDecl))))
                          .bind("func"),
                      this);
 }
 
 void AvoidConstParamsInDecls::check(const MatchFinder::MatchResult &Result) {
   const auto *Func = Result.Nodes.getNodeAs<FunctionDecl>("func");

test/clang-tidy/readability-avoid-const-params-in-decls.cpp

 void NF1(const int i) {}
 void NF2(const int *const i) {}
 void NF3(int const i) {}
 void NF4(alias_type const i) {}
 void NF5(const int) {}
 void NF6(const int *const) {}
 void NF7(int, const int) {}
 void NF8(const int, const int) {}
+// pass_object_size requires a const pointer param in definitions.
+void NF9(const char *const __attribute__((pass_object_size(0)))) {}
 
 // Do not match on other stuff
 void NF(const alias_type& i);
 void NF(const int &i);
 void NF(const int *i);
 void NF(alias_const_type i);
 void NF(const alias_type&);
 void NF(const int&);
 void NF(const int*);
 void NF(const int* const*);
 void NF(alias_const_type);
+// While pass_object_size only requires a const pointer param in definitions,
+// it's probably best to not complain at users for using it in a decl.
+void NF(const char *const __attribute__((pass_object_size(0))));