This revision was automatically updated to reflect the committed changes.
Closed by commit rG041080c24735: [AST] Fix a crash on invalid constexpr
Ctorinitializer when building… (authored by hokein).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
hokein updated this revision to Diff 254525.
hokein added a comment.
remove accident change.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D77041/new/
https://reviews.llvm.org/D77041
Files:
clang/lib/AST/ExprConstant.cpp
hokein updated this revision to Diff 254170.
hokein added a comment.
refine the fix based on the comment:
now constexpr ctor with error initializers is still a valid decl but not
constant-evaluate.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
sammccall added inline comments.
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:5005
+if (Member->getInit() && Member->getInit()->containsErrors())
+ Constructor->setInvalidDecl();
if (Member->isBaseInitializer())
hokein wrote:
> rsmith wrote:
> >
hokein marked 2 inline comments as done.
hokein added inline comments.
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:1688
CheckConstexprKind Kind) {
+ assert(!NewFD->isInvalidDecl());
const CXXMethodDecl *MD = dyn_cast(NewFD);
rsmith added inline comments.
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:5005
+if (Member->getInit() && Member->getInit()->containsErrors())
+ Constructor->setInvalidDecl();
if (Member->isBaseInitializer())
rsmith wrote:
> This is inappropriate.
rsmith added inline comments.
Comment at: clang/lib/Parse/ParseDeclCXX.cpp:3459-3461
+ if (MemInit.get()->getInit() &&
+ MemInit.get()->getInit()->containsErrors())
+AnyErrors = true;
The parser should not be inspecting properties of
hokein marked an inline comment as done.
hokein added inline comments.
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:5004
CXXCtorInitializer *Member = Initializers[i];
-
+if (Member->getInit() && Member->getInit()->containsErrors())
+ Constructor->setInvalidDecl();
sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:5004
CXXCtorInitializer *Member = Initializers[i];
-
+if (Member->getInit() &&
hokein updated this revision to Diff 253809.
hokein marked an inline comment as done.
hokein added a comment.
- mark CtorDecl as invalid when the Initializer init expr contains errors
- add a testcase that would crash the previous version of the patch
Repository:
rG LLVM Github Monorepo
hokein added a comment.
> This makes me nervous, marking the constructor as invalid seems much safer.
> Can you show tests it regresses?
Yeah, the first version of the patch doesn't seem to fix all crashes (X().Y
would lead another crash)...
so if we mark the CtorDecl as invalid
1. whenever
sammccall added a comment.
This makes me nervous, marking the constructor as invalid seems much safer. Can
you show tests it regresses?
Comment at: clang/lib/Sema/SemaDecl.cpp:14359
+!ErrorsInCtorInitializer &&
!CheckConstexprFunctionDefinition(FD,
hokein added inline comments.
Comment at: clang/lib/Sema/SemaDecl.cpp:14359
+!ErrorsInCtorInitializer &&
!CheckConstexprFunctionDefinition(FD, CheckConstexprKind::Diagnose))
FD->setInvalidDecl();
The crash is in
hokein created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
hokein edited the summary of this revision.
hokein added inline comments.
hokein added a reviewer: sammccall.
Comment at: clang/lib/Sema/SemaDecl.cpp:14359
+
14 matches
Mail list logo