Re: [PATCH] D18765: [clang-tidy] Don't complain about const pass_object_size params in declarations

[George Burgess IV via cfe-commits]

george.burgess.iv abandoned this revision.
george.burgess.iv added a comment.

> 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 :)


http://reviews.llvm.org/D18765



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D18765: [clang-tidy] Don't complain about const pass_object_size params in declarations

[Alexander Kornienko via cfe-commits]

alexfh added a comment.

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.


http://reviews.llvm.org/D18765



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D18765: [clang-tidy] Don't complain about const pass_object_size params in declarations

[Alexander Kornienko via cfe-commits]

alexfh requested changes to this revision.
alexfh added a comment.
This revision now requires changes to proceed.

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.


http://reviews.llvm.org/D18765



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[George Burgess IV via cfe-commits]

george.burgess.iv created this revision.
george.burgess.iv added a reviewer: alexfh.
george.burgess.iv added a subscriber: cfe-commits.

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.

http://reviews.llvm.org/D18765

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

Index: test/clang-tidy/readability-avoid-const-params-in-decls.cpp
===
--- test/clang-tidy/readability-avoid-const-params-in-decls.cpp
+++ test/clang-tidy/readability-avoid-const-params-in-decls.cpp
@@ -65,6 +65,8 @@
 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);
@@ -76,3 +78,6 @@
 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;
Index: clang-tidy/readability/AvoidConstParamsInDecls.cpp
===
--- clang-tidy/readability/AvoidConstParamsInDecls.cpp
+++ clang-tidy/readability/AvoidConstParamsInDecls.cpp
@@ -31,7 +31,8 @@
 
 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"),


Index: test/clang-tidy/readability-avoid-const-params-in-decls.cpp
===
--- test/clang-tidy/readability-avoid-const-params-in-decls.cpp
+++ test/clang-tidy/readability-avoid-const-params-in-decls.cpp
@@ -65,6 +65,8 @@
 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);
@@ -76,3 +78,6 @@
 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;
Index: clang-tidy/readability/AvoidConstParamsInDecls.cpp
===
--- clang-tidy/readability/AvoidConstParamsInDecls.cpp
+++ clang-tidy/readability/AvoidConstParamsInDecls.cpp
@@ -31,7 +31,8 @@
 
 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"),
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits