This revision was automatically updated to reflect the committed changes.
Closed by commit rC351849: [ASTImporter] Fix importing OperatorDelete from
CXXConstructorDecl (authored by teemperor, committed by ).
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D56651/new/
martong accepted this revision.
martong added a comment.
The updated version looks good to me! Thank you!
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D56651/new/
https://reviews.llvm.org/D56651
___
cfe-commits mailing list
a_sidorin accepted this revision.
a_sidorin added a comment.
This revision is now accepted and ready to land.
LGTM, thank you!
I'm sorry if the change I requested doesn't look good, but I think the
correctness is our priority.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D56651/new/
teemperor updated this revision to Diff 182718.
teemperor added a comment.
- Moved code into the VisitFunctionDecl function.
I don't mind if we clean up that function in a later commit. Thanks for the
feedback!
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D56651/new/
a_sidorin added a comment.
Hi Shafik,
Please find my answers inline.
Comment at: lib/AST/ASTImporter.cpp:3243
+
+ if (R) {
+CXXDestructorDecl *ToDtor = cast(*R);
shafik wrote:
> a_sidorin wrote:
> > It's better to move this code to VisitFunctionDecl to
shafik added a comment.
Thank you, this looks good but perhaps some refactoring would help improve the
change.
Comment at: lib/AST/ASTImporter.cpp:3243
+
+ if (R) {
+CXXDestructorDecl *ToDtor = cast(*R);
a_sidorin wrote:
> It's better to move this code
a_sidorin added a comment.
Hi Rafael,
The change looks mostly fine but I have some comments inline.
Comment at: lib/AST/ASTImporter.cpp:3243
+
+ if (R) {
+CXXDestructorDecl *ToDtor = cast(*R);
It's better to move this code to VisitFunctionDecl to keep
teemperor added a comment.
Didn't really upstream any non-trivial ASTImporter patches yet, so please point
out any style errors.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D56651/new/
https://reviews.llvm.org/D56651
teemperor created this revision.
teemperor added reviewers: shafik, martong, a_sidorin.
Herald added subscribers: cfe-commits, rnkovacs.
Herald added a reviewer: a.sidorin.
Shafik found out that importing a CXXConstructorDecl will create a translation
unit that
causes Clang's CodeGen to crash.
