[Clang] Add detailed notes explaining why is_aggregate evaluates to false #152488
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1965,6 +1965,7 @@ static std::optional<TypeTrait> StdNameToTypeTrait(StringRef Name) { | |
| .Case("is_assignable", TypeTrait::BTT_IsAssignable) | ||
| .Case("is_empty", TypeTrait::UTT_IsEmpty) | ||
| .Case("is_standard_layout", TypeTrait::UTT_IsStandardLayout) | ||
| .Case("is_aggregate", TypeTrait::UTT_IsAggregate) | ||
| .Case("is_constructible", TypeTrait::TT_IsConstructible) | ||
| .Default(std::nullopt); | ||
| } | ||
|
|
@@ -2595,6 +2596,105 @@ static void DiagnoseNonStandardLayoutReason(Sema &SemaRef, SourceLocation Loc, | |
| SemaRef.Diag(D->getLocation(), diag::note_defined_here) << D; | ||
| } | ||
|
|
||
| static void DiagnoseNonAggregateReason(Sema &SemaRef, SourceLocation Loc, | ||
| const CXXRecordDecl *D) { | ||
| for (const CXXConstructorDecl *Ctor : D->ctors()) { | ||
| if (Ctor->isUserProvided()) | ||
| SemaRef.Diag(Loc, diag::note_unsatisfied_trait_reason) | ||
| << diag::TraitNotSatisfiedReason::UserDeclaredCtr; | ||
| if (Ctor->isInheritingConstructor()) | ||
| SemaRef.Diag(Loc, diag::note_unsatisfied_trait_reason) | ||
| << diag::TraitNotSatisfiedReason::InheritedCtr; | ||
| } | ||
|
|
||
| bool HasInherited = false; | ||
| for (const Decl *Sub : D->decls()) { | ||
| if (auto *UD = dyn_cast<UsingDecl>(Sub)) { | ||
| for (auto I = UD->shadow_begin(), E = UD->shadow_end(); I != E; ++I) { | ||
| if (isa<ConstructorUsingShadowDecl>(*I)) { | ||
| HasInherited = true; | ||
| break; | ||
| } | ||
| } | ||
| if (HasInherited) | ||
| break; | ||
| } | ||
| if (isa<ConstructorUsingShadowDecl>(Sub)) { | ||
| HasInherited = true; | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| if (HasInherited) { | ||
| SemaRef.Diag(Loc, diag::note_unsatisfied_trait_reason) | ||
| << diag::TraitNotSatisfiedReason::InheritedCtr; | ||
| } | ||
|
|
||
| for (const FieldDecl *Field : D->fields()) { | ||
| switch (Field->getAccess()) { | ||
| case AS_private: | ||
| SemaRef.Diag(Loc, diag::note_unsatisfied_trait_reason) | ||
| << diag::TraitNotSatisfiedReason::PrivateDirectDataMember; | ||
| break; | ||
| case AS_protected: | ||
| SemaRef.Diag(Loc, diag::note_unsatisfied_trait_reason) | ||
| << diag::TraitNotSatisfiedReason::ProtectedDirectDataMember; | ||
| break; | ||
| default: | ||
| break; | ||
| } | ||
|
||
| } | ||
|
|
||
| for (const CXXBaseSpecifier &B : D->bases()) { | ||
|
Contributor
There was a problem hiding this comment. This is kind of a nit: I feel like base classes should go prior to fields. As an actual issue you may need to diagnose bases recursively as the reason for a type being a non-aggregate may not be direct. e.g. I'm not sure what diagnostic would be produced here for something like class A {};
class B: private A {};
class C: public B {};
require is_aggregate(C)
Contributor
Author
There was a problem hiding this comment. https://godbolt.org/z/fsoP3h4on |
||
| if (B.isVirtual()) { | ||
| SemaRef.Diag(Loc, diag::note_unsatisfied_trait_reason) | ||
| << diag::TraitNotSatisfiedReason::VBase << B.getType() | ||
| << B.getSourceRange(); | ||
| continue; | ||
| } | ||
| switch (B.getAccessSpecifier()) { | ||
| case AS_private: | ||
| SemaRef.Diag(Loc, diag::note_unsatisfied_trait_reason) | ||
| << diag::TraitNotSatisfiedReason::PrivateDirectBase; | ||
| break; | ||
| case AS_protected: | ||
| SemaRef.Diag(Loc, diag::note_unsatisfied_trait_reason) | ||
| << diag::TraitNotSatisfiedReason::ProtectedDirectBase; | ||
| break; | ||
| default: | ||
| break; | ||
|
Contributor
There was a problem hiding this comment. I think we could merge these two diagnostics |
||
| } | ||
| } | ||
|
|
||
| for (const CXXMethodDecl *Method : D->methods()) { | ||
|
Contributor
There was a problem hiding this comment. This might be better noted as the object being polymorphic, rather than enumerating the individual virtual methods
Contributor
Author
There was a problem hiding this comment. I can add a
Collaborator
There was a problem hiding this comment. I suspect a double-note here is even better. First one that says "because it is a polymorphic type", second of: "check out this method here, which is virtual".
Contributor
Author
There was a problem hiding this comment.
Added note about polymorphic type. |
||
| if (Method->isVirtual()) { | ||
| SemaRef.Diag(Loc, diag::note_unsatisfied_trait_reason) | ||
| << diag::TraitNotSatisfiedReason::VirtualFunction << Method | ||
| << Method->getSourceRange(); | ||
| } | ||
| } | ||
|
|
||
| SemaRef.Diag(D->getLocation(), diag::note_defined_here) << D; | ||
| } | ||
|
|
||
| static void DiagnoseNonAggregateReason(Sema &SemaRef, SourceLocation Loc, | ||
| QualType T) { | ||
| SemaRef.Diag(Loc, diag::note_unsatisfied_trait) | ||
| << T << diag::TraitName::Aggregate; | ||
|
|
||
| if (T->isVoidType()) | ||
| SemaRef.Diag(Loc, diag::note_unsatisfied_trait_reason) | ||
| << diag::TraitNotSatisfiedReason::CVVoidType; | ||
|
|
||
| T = T.getNonReferenceType(); | ||
| const CXXRecordDecl *D = T->getAsCXXRecordDecl(); | ||
| if (!D || D->isInvalidDecl()) | ||
| return; | ||
|
|
||
| if (D->hasDefinition()) | ||
| DiagnoseNonAggregateReason(SemaRef, Loc, D); | ||
| } | ||
|
|
||
| void Sema::DiagnoseTypeTraitDetails(const Expr *E) { | ||
| E = E->IgnoreParenImpCasts(); | ||
| if (E->containsErrors()) | ||
|
|
@@ -2627,6 +2727,9 @@ void Sema::DiagnoseTypeTraitDetails(const Expr *E) { | |
| case TT_IsConstructible: | ||
| DiagnoseNonConstructibleReason(*this, E->getBeginLoc(), Args); | ||
| break; | ||
| case UTT_IsAggregate: | ||
| DiagnoseNonAggregateReason(*this, E->getBeginLoc(), Args[0]); | ||
| break; | ||
| default: | ||
| break; | ||
| } | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.