martinboehme wrote:
> > Given that there are a bunch of design questions that comes to mind here,
> > notably whether the attribute is on the declaration of the class or the
> > constructor, the impact of move assignment operators, constraints etc and
> > the fact we need more illustration of usage, I would like to see an RFC for
> > that before moving forward with the PR
> > @AaronBallman @erichkeane
>
> This is frankly pretty specific, I'm not sure how useful an RFC would be?
I think the idea may be that people who care about additional attributes may
not necessarily be looking at every single PR that goes by but will see an RFC?
> That said: --Application: ANYTHING besides on the type decl is probably going
> to be really confusing. Having to separately attest (and perhaps DIFFERENTLY
> attest!) based on move construction or assignment seems nuts. While I agree
> that this is something potentially USEFUL (consider a case where a move
> assignment does a 'swap', like IIRC 'vector'), it seems very difficult to
> reason about.
Agree, I think this should go on the type decl.
> --The attachment to 'default constructed state' seems pretty ethereal and not
> very useful/possible to reason about either. For general user defined types
> (which, if this is an attribute, we need to make sure this is useful for!),
> this seems very difficult to figure out. IF all you care about is smart
> pointers, you should just teach your static analyzer what the smart pointer
> semantics are anyway if you're hard coding in their default constructed
> semantics.
Yes, the bugprone-use-after-move check (which is the motivation for adding this
attribute) does already understand the smart pointer semantics.
The purpose of this attribute is being able to mark a type as "this is a
user-library-defined smart pointer" (as opposed to the ones in the standard
library, which the check simply hardcodes).
So maybe the attribute should simply be called `[[smart_pointer]]`? The thing
I'm a bit hesitant about here is: Conceivably, someone _could_ define a smart
pointer type that doesn't guarantee that it's null when moved from. And more
broadly, the name `[[smart_pointer]]` doesn't clearly communicate what the
attribute actually signifies ("null when moved from").
> -- On that purpose: I think we need to step back and decide WHAT information
> is useful. Currently we already have the guarantee of moved-from as "valid
> but unspecified". WHAT sort of specification is useful?
I think the attribute should specify "this is a smart pointer that is null when
moved from".
> Actually... I wonder if the individual OPERATIONS on the type are what is
> valuable after move. Like, should the `unique_ptr`'s `operator*` be what we
> mark with `[[can-do-something-with-after-move]]` (or vice versa?)?
Actually, the `operator*` is one of the operations that _isn't_ safe after a
move.
But then the question becomes: Do we annotate the operations that _are_ safe,
or the ones that _aren't_ safe?
* Annotating the operations that _are_ safe is more straightforward, because
default (without annotation) is that all operations are unsafe. But as the
majority of operations on a moved-from smart pointer are safe, this means
annotating quite a lot of oeprations.
* Annotating the operations that _aren't_ safe requires fewer annotations in
total, but it's a bit strange in that the default is for an operation to be
unsafe after a move anyway. So what does the rule become -- if at least one
operation on a class is annotated unsafe-after-move, does that all unannotated
operations can be assumed to be safe-after-move? It seems a bit surprising that
an annotation on one operation would, in effect, flip the default for all other
operations.
In total, I think the most straightforward approach here is still to annotate
the class, and I'm starting to think that it should be called
`[[null_after_move]]`.
https://github.com/llvm/llvm-project/pull/178432
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
