-
Notifications
You must be signed in to change notification settings - Fork 14.2k
[Clang] Added explanation why is_constructible
evaluated to false.
#143309
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
Changes from 1 commit
ed1f379
ef79556
cb89df9
bd3cefd
172cee8
bd3cfca
bd1e468
124e9f1
e18073b
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 | ||||
---|---|---|---|---|---|---|
|
@@ -21,6 +21,7 @@ | |||||
#include "clang/Sema/Overload.h" | ||||||
#include "clang/Sema/Sema.h" | ||||||
#include "clang/Sema/SemaHLSL.h" | ||||||
#include "llvm/Support/raw_ostream.h" | ||||||
|
||||||
using namespace clang; | ||||||
|
||||||
|
@@ -1941,6 +1942,7 @@ static std::optional<TypeTrait> StdNameToTypeTrait(StringRef Name) { | |||||
.Case("is_trivially_relocatable", | ||||||
TypeTrait::UTT_IsCppTriviallyRelocatable) | ||||||
.Case("is_trivially_copyable", TypeTrait::UTT_IsTriviallyCopyable) | ||||||
.Case("is_constructible", TypeTrait::TT_IsConstructible) | ||||||
.Default(std::nullopt); | ||||||
} | ||||||
|
||||||
|
@@ -1977,8 +1979,14 @@ static ExtractedTypeTraitInfo ExtractTypeTraitFromExpression(const Expr *E) { | |||||
Trait = StdNameToTypeTrait(Name); | ||||||
if (!Trait) | ||||||
return std::nullopt; | ||||||
for (const auto &Arg : VD->getTemplateArgs().asArray()) | ||||||
Args.push_back(Arg.getAsType()); | ||||||
for (const auto &Arg : VD->getTemplateArgs().asArray()) { | ||||||
if (Arg.getKind() == TemplateArgument::ArgKind::Pack) { | ||||||
for (const auto &InnerArg : Arg.pack_elements()) | ||||||
Args.push_back(InnerArg.getAsType()); | ||||||
} | ||||||
if (Arg.getKind() == TemplateArgument::ArgKind::Type) | ||||||
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.
Suggested change
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. Fixed |
||||||
Args.push_back(Arg.getAsType()); | ||||||
} | ||||||
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. If the kind is neither, we should probably do SOMETHING about it, even if it is just an assert. 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. Added assert |
||||||
return {{Trait.value(), std::move(Args)}}; | ||||||
} | ||||||
|
||||||
|
@@ -2159,6 +2167,26 @@ static void DiagnoseNonTriviallyCopyableReason(Sema &SemaRef, | |||||
} | ||||||
} | ||||||
|
||||||
static void DiagnoseNonConstructibleReason(Sema &SemaRef, SourceLocation Loc, | ||||||
QualType T) { | ||||||
SemaRef.Diag(Loc, diag::note_unsatisfied_trait) | ||||||
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. I think what would be useful here is to try to run overload resolution again on all the arguments, You can look at how I think that by calling We don't have a good way to turn the errors that would be produced into notes, though @AaronBallman @erichkeane @Sirraide - but maybe emitting errors is good enough as a first approach 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.
I’m working on something diagnostics-related right now that would allow nesting errors but it’s not finished yet. Because of how that works though it should be straight-forward to add the nesting later, so just emitting the errors for now should be fine imo. 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. Thanks for your advice @cor3ntin (and also can i ping you?). I have tried to use
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. Well, I figured it out! SFINAETrap was the problem. So now it works as expected. |
||||||
<< T << diag::TraitName::Constructible; | ||||||
|
||||||
if (T->isFunctionType()) | ||||||
SemaRef.Diag(Loc, diag::note_unsatisfied_trait_reason) | ||||||
<< diag::TraitNotSatisfiedReason::FunctionType; | ||||||
|
||||||
if (T->isVoidType()) | ||||||
SemaRef.Diag(Loc, diag::note_unsatisfied_trait_reason) | ||||||
<< diag::TraitNotSatisfiedReason::CVVoidType; | ||||||
|
||||||
const CXXRecordDecl *D = T->getAsCXXRecordDecl(); | ||||||
if (!D || D->isInvalidDecl() || !D->hasDefinition()) | ||||||
return; | ||||||
|
||||||
SemaRef.Diag(D->getLocation(), diag::note_defined_here) << D; | ||||||
} | ||||||
|
||||||
static void DiagnoseNonTriviallyCopyableReason(Sema &SemaRef, | ||||||
SourceLocation Loc, QualType T) { | ||||||
SemaRef.Diag(Loc, diag::note_unsatisfied_trait) | ||||||
|
@@ -2195,6 +2223,9 @@ void Sema::DiagnoseTypeTraitDetails(const Expr *E) { | |||||
case UTT_IsTriviallyCopyable: | ||||||
DiagnoseNonTriviallyCopyableReason(*this, E->getBeginLoc(), Args[0]); | ||||||
break; | ||||||
case TT_IsConstructible: | ||||||
DiagnoseNonConstructibleReason(*this, E->getBeginLoc(), Args[0]); | ||||||
break; | ||||||
default: | ||||||
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.
IS this a valid assumption here (that this is a type?). Presumably this could be an NTTP pack as well...
Though
getAsType
would assert for us?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.
getAsType would assert for us