-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[Clang] Add diagnostic reasoning for unsatisfied is_destructible trait #167291
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2028,6 +2028,7 @@ static std::optional<TypeTrait> StdNameToTypeTrait(StringRef Name) { | |
| .Case("is_constructible", TypeTrait::TT_IsConstructible) | ||
| .Case("is_final", TypeTrait::UTT_IsFinal) | ||
| .Case("is_abstract", TypeTrait::UTT_IsAbstract) | ||
| .Case("is_destructible", TypeTrait::UTT_IsDestructible) | ||
| .Default(std::nullopt); | ||
| } | ||
|
|
||
|
|
@@ -2399,6 +2400,66 @@ static void DiagnoseNonConstructibleReason( | |
| SemaRef.Diag(D->getLocation(), diag::note_defined_here) << D; | ||
| } | ||
|
|
||
| static void DiagnoseNonDestructibleReason(Sema &SemaRef, SourceLocation Loc, | ||
| QualType T) { | ||
|
|
||
| QualType CoreT = T.getCanonicalType(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What does
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It stands for the core or canonical type, obtained using getCanonicalType(). We use it to analyze the underlying type (without typedefs or qualifiers) when determining if a type is destructible.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We don't say a type is 'core' anyway, instead we use the word Are you using AI to answer questions?
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
No we don't? I don't believe we've ever used that before. I'm also suspecting AI at play here, which is not acceptable if so, unless you completely understand what it has done. Please answer questions/make patches that you completely understand. |
||
| if (const ArrayType *AT = SemaRef.Context.getAsArrayType(CoreT)) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please comment on this to tell why this is appropriate. |
||
| CoreT = AT->getElementType(); | ||
|
|
||
| SemaRef.Diag(Loc, diag::note_unsatisfied_trait) | ||
| << CoreT << diag::TraitName::Destructible; | ||
|
|
||
| if (CoreT->isFunctionType()) { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is so much repetition on these next handful of branches. It looks like some sort of work shoudl be done to only do the SemaRef.Diag 1x and store the TraitNotSatisifedReason to then diagnose it. |
||
| SemaRef.Diag(Loc, diag::note_unsatisfied_trait_reason) | ||
| << diag::TraitNotSatisfiedReason::FunctionType; | ||
| return; | ||
| } | ||
|
|
||
| if (CoreT->isVoidType()) { | ||
| SemaRef.Diag(Loc, diag::note_unsatisfied_trait_reason) | ||
| << diag::TraitNotSatisfiedReason::CVVoidType; | ||
| return; | ||
| } | ||
|
|
||
| if (CoreT->isIncompleteType()) { | ||
| SemaRef.Diag(Loc, diag::note_unsatisfied_trait_reason) | ||
| << diag::TraitNotSatisfiedReason::IncompleteType; | ||
| return; | ||
| } | ||
|
Comment on lines
+2425
to
+2429
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you test this branch? |
||
|
|
||
| const CXXRecordDecl *RD = CoreT->getAsCXXRecordDecl(); | ||
| if (!RD || RD->isInvalidDecl()) | ||
| return; | ||
|
|
||
| const CXXRecordDecl *Def = RD->getDefinition(); | ||
|
Comment on lines
+2434
to
+2435
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. DO you have any examples that hit this branch? You very much should not be able to. |
||
| if (!Def) { | ||
| SemaRef.Diag(Loc, diag::note_unsatisfied_trait_reason) | ||
| << diag::TraitNotSatisfiedReason::IncompleteType; | ||
| return; | ||
| } | ||
|
|
||
| CXXDestructorDecl *Dtor = Def->getDestructor(); | ||
| if (!Dtor) | ||
| return; | ||
|
|
||
| if (Dtor->isDeleted()) { | ||
| SemaRef.Diag(Loc, diag::note_unsatisfied_trait_reason) | ||
| << diag::TraitNotSatisfiedReason::DeletedDtr << 0 | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What does the 0 mean here? |
||
| << Dtor->getSourceRange(); | ||
| return; | ||
| } | ||
|
|
||
| AccessSpecifier AS = Dtor->getAccess(); | ||
| if (AS == AS_private || AS == AS_protected) { | ||
| unsigned Select = AS != AS_private; | ||
| SemaRef.Diag(Loc, diag::note_unsatisfied_trait_reason) | ||
| << diag::TraitNotSatisfiedReason::InaccessibleDtr << Select | ||
| << Dtor->getSourceRange(); | ||
| return; | ||
| } | ||
| } | ||
|
|
||
| static void DiagnoseNonTriviallyCopyableReason(Sema &SemaRef, | ||
| SourceLocation Loc, QualType T) { | ||
| SemaRef.Diag(Loc, diag::note_unsatisfied_trait) | ||
|
|
@@ -2889,6 +2950,9 @@ void Sema::DiagnoseTypeTraitDetails(const Expr *E) { | |
| case TT_IsConstructible: | ||
| DiagnoseNonConstructibleReason(*this, E->getBeginLoc(), Args); | ||
| break; | ||
| case UTT_IsDestructible: | ||
| DiagnoseNonDestructibleReason(*this, E->getBeginLoc(), Args[0]); | ||
| break; | ||
| case UTT_IsAggregate: | ||
| DiagnoseNonAggregateReason(*this, E->getBeginLoc(), Args[0]); | ||
| break; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a really odd change, why are you moving this? Looking at 'main' this hasn't moved, so this doesn't appear to be a bad merge thing.