erichkeane 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?
Sorry, this was a bit of a gag. Read the message to the end.
>
> > 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").
>
Yeah, I don't think `[[smart_pointer]]` is sufficient. Though we DO have a
handful of attributes that mean "this acts like a standard type in the ways
that are important for the purposes of... whatever", I'm not at all a fan of it
as it seems 'lazy' from a compiler design perspective.
> > -- 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".
Perhaps slightly more abstract? Now I DO wonder if a field attribute of
"set-to-zero-null-whatever-after-parent-type-moved-from" is what we're looking
for? Though the 'unsafe-after-move' operations are reasonable too...
>
> > 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?
>
I think that is a reasonable question as well.
> * Annotating the operations that _are_ safe is more straightforward,
> because the 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 mean that
> all unannotated operations can then 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 the attribute should be called
> `[[null_after_move]]`.
`null after move` is fine I guess? As a record-decl attribute it is a little
weird as it is not particularly specific what it means though in the case of
multiple fields. UNLESS we want it to mean something like
"memset-0-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
