sammccall added a comment.

In D138583#3949604 <https://reviews.llvm.org/D138583#3949604>, @ankineri wrote:

> I do agree that having it as part of `-Wunused-variable` is better on many 
> aspects, but I have a few concerns about it.
> The proposed check can be applied to any type including STL ones such as 
> `std::string`, all as per codebase owner's choice. Adding this attribute to 
> STL classes will probably be really hard as it will break many builds.
> Having `warn_unused absl::Status` (and a couple of others) is probably a good 
> idea anyway, but is it feasible to apply it to STL?

My guess is that libc++ and libstdc++ would accept such a patch *where it's 
safe*, since clang and gcc respectively support the attribute, but you'd have 
to check with them.

---

Since the standard library is mostly templates, one issue you're going to run 
into with say `std::vector<int>` is that it's only `warn_unused` because `int` 
is `warn_unused`, and this is hard to express with attributes.
`std::string` is actually the same since it's an alias for 
`std::basic_string<char, ...>`. While maybe nobody uses 
`std::basic_string<std::mutex>`, it's a bit of a grey area.

Fundamentally this is all stuff that such a clang-tidy check would have to 
address as well, and IMO the tools you have to address it with clang + 
attributes are much better than in a clang-tidy check.
(If it solves std::string and std::vector, then that might even be enough to 
provide some hacky `warn_unused_recursive` attribute that is equivalent to 
`warn_unused` if all type template parameters are warn-unused).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138583/new/

https://reviews.llvm.org/D138583

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

Reply via email to