[PATCH] D49910: [clang-tidy] Recognize [[clang::reinitializes]] attribute in bugprone-use-after-move

[Martin Böhme via Phabricator via cfe-commits]

mboehme added a comment.

In https://reviews.llvm.org/D49910#1187809, @aaron.ballman wrote:

> In https://reviews.llvm.org/D49910#1187492, @mboehme wrote:
>
> > In https://reviews.llvm.org/D49910#1187455, @aaron.ballman wrote:
> >
> > > Are you going to propose adding this attribute to libc++, or is this expected to only work with UDTs?
> >
> >
> > I don't have any experience contributing to libc++, but I think this would make sense.
> >
> > The check currently hard-codes various member functions of classes in the "std" namespace that do reinitializations; I'm not sure though if those can be removed after the attribute has been added to libc++. We'd would also presumably have to add the attribute to libstdc++ -- does it accept Clang-only attributes? And what is the story for people using clang-tidy with MSVC projects? (I have to admit I'm very hazy on how that works...)
>
>
> I ask the question because it's novel to add an attribute to Clang that's used only by clang-tidy, and I'm wondering who is expected to use that attribute to help with this check. Is it expected to be an attribute users use with their own custom datatypes and it's not needed for standards-based APIs (because we handle those some other way), or something else? As it stands, I sort of feel like this is a very heavy approach to solve an edge case -- is there a lot of evidence for re-using a moved-from object after reinitializing it?


Ah, now I understand what you're getting at.

Yes, I see this attribute mainly as something that people would add to their own user-defined types -- typically, containers and smart pointers.

I've regularly been getting internal feature requests for this. One typical pattern in which this comes up is the following:

MyContainer<T> container;
T t;
while (GetNextT(&t)) {

  container.Add(t);
  if (SomeCondition()) {
    PassToConsumer(std::move(container));
    container.Clear();
  }

}

I.e. you're incrementally adding items to some data structure, and at some point you decide the data structure is now complete, so you hand it off to a consumer and clear it, ready to be filled with the next batch of items.

As clang-tidy doesn't understand that the "container.Clear()" reinitializes the container, it complains that the Clear() is a use-after-move.

Yes, it's unusual to add an attribute to Clang that is only (at least initially) intended for use by clang-tidy -- but I don't really see how to implement this other than with an attribute.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D49910