[PATCH] D45898: [SemaCXX] Mark destructor as referenced

orivej added a comment. I have noticed that this change breaks seemingly valid code: class A { protected: ~A(); }; struct B : A {}; B f() { return B(); } B g() { return {}; } `f` compiles, but `g` fails with `temporary of type 'A' has protected destructor`. (g++ 8.2 compiles this file.)

r348713 - Fix InitListExpr test

Author: steveire Date: Sun Dec 9 05:13:41 2018 New Revision: 348713 URL: http://llvm.org/viewvc/llvm-project?rev=348713&view=rev Log: Fix InitListExpr test Wrong case of Check meant this has no effect. Modified: cfe/trunk/test/AST/ast-dump-stmt.cpp Modified: cfe/trunk/test/AST/ast-dump-stm

r348714 - Re-order content from InitListExpr

Author: steveire Date: Sun Dec 9 05:15:18 2018 New Revision: 348714 URL: http://llvm.org/viewvc/llvm-project?rev=348714&view=rev Log: Re-order content from InitListExpr Summary: This causes no change in the output of ast-dump-stmt.cpp due to the way child nodes are printed with a delay. Reviewe

[PATCH] D55398: Re-order content from InitListExpr

This revision was automatically updated to reflect the committed changes. Closed by commit rL348714: Re-order content from InitListExpr (authored by steveire, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D5539

[PATCH] D55393: Re-order content of template parameter dumps

steveire updated this revision to Diff 177412. steveire added a comment. New approach Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55393/new/ https://reviews.llvm.org/D55393 Files: lib/AST/ASTDumper.cpp test/AST/ast-dump-decl.cpp Index: test/AST/ast-dump-d

r348715 - Inline dumpFullComment into callers

Author: steveire Date: Sun Dec 9 05:18:55 2018 New Revision: 348715 URL: http://llvm.org/viewvc/llvm-project?rev=348715&view=rev Log: Inline dumpFullComment into callers It causes confusion over whether it or dumpComment is the more important. It is easier to refactor with fewer utility methods.

r348716 - Inline hasNodes into only caller

Author: steveire Date: Sun Dec 9 05:20:43 2018 New Revision: 348716 URL: http://llvm.org/viewvc/llvm-project?rev=348716&view=rev Log: Inline hasNodes into only caller It is easier to refactor with fewer utility methods. Modified: cfe/trunk/lib/AST/ASTDumper.cpp Modified: cfe/trunk/lib/AST/

r348717 - Introduce optional labels to dumpStmt

Author: steveire Date: Sun Dec 9 05:23:07 2018 New Revision: 348717 URL: http://llvm.org/viewvc/llvm-project?rev=348717&view=rev Log: Introduce optional labels to dumpStmt If the label is present, it is added as a child, with the statement a child of the label. This preserves behavior of the In

r348718 - Revert "Introduce optional labels to dumpStmt"

Author: steveire Date: Sun Dec 9 05:24:40 2018 New Revision: 348718 URL: http://llvm.org/viewvc/llvm-project?rev=348718&view=rev Log: Revert "Introduce optional labels to dumpStmt" This reverts commit 933402caa09963792058198578522a95f013c69c. Modified: cfe/trunk/lib/AST/ASTDumper.cpp Modif

[PATCH] D55488: Add utility for dumping a label with child nodes

steveire created this revision. steveire added a reviewer: aaron.ballman. Herald added a subscriber: cfe-commits. Use it to add optional label nodes to Stmt dumps. This preserves behavior of InitExprList dump: // CHECK-NEXT: `-InitListExpr {{.+}} 'U [3]' // CHECK-NEXT: |-array filler // CHECK

[PATCH] D55190: Move dump of individual comment nodes to NodeDumper

This revision was automatically updated to reflect the committed changes. Closed by commit rL348719: NFC: Move dump of individual comment nodes to NodeDumper (authored by steveire, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D55190?

r348719 - NFC: Move dump of individual comment nodes to NodeDumper

Author: steveire Date: Sun Dec 9 05:30:17 2018 New Revision: 348719 URL: http://llvm.org/viewvc/llvm-project?rev=348719&view=rev Log: NFC: Move dump of individual comment nodes to NodeDumper Reviewers: aaron.ballman Subscribers: cfe-commits Differential Revision: https://reviews.llvm.org/D5519

r348720 - NFC: Rename TemplateDecl dump utilities

Author: steveire Date: Sun Dec 9 05:33:30 2018 New Revision: 348720 URL: http://llvm.org/viewvc/llvm-project?rev=348720&view=rev Log: NFC: Rename TemplateDecl dump utilities There is a clang::TemplateDecl AST type, so a method called VisitTemplateDecl looks like it should 'override' the method f

[PATCH] D55489: Implement dumpFunctionDeclParameters in NodeDumper

steveire created this revision. steveire added a reviewer: aaron.ballman. Herald added a subscriber: cfe-commits. Repository: rC Clang https://reviews.llvm.org/D55489 Files: include/clang/AST/TextNodeDumper.h lib/AST/ASTDumper.cpp lib/AST/TextNodeDumper.cpp Index: lib/AST/TextNodeDumpe

[PATCH] D55490: Add dumpMethodDeclOverrides to NodeDumper

steveire created this revision. steveire added a reviewer: aaron.ballman. Herald added a subscriber: cfe-commits. Repository: rC Clang https://reviews.llvm.org/D55490 Files: include/clang/AST/TextNodeDumper.h lib/AST/ASTDumper.cpp lib/AST/TextNodeDumper.cpp Index: lib/AST/TextNodeDumpe

[PATCH] D55491: Implement TemplateArgument dumping in terms of Visitor

steveire created this revision. steveire added a reviewer: aaron.ballman. Herald added a subscriber: cfe-commits. Split the output streaming from the traversal to other AST nodes. Repository: rC Clang https://reviews.llvm.org/D55491 Files: include/clang/AST/TemplateArgumentVisitor.h incl

[PATCH] D55492: Implement Attr dumping in terms of visitors

steveire created this revision. steveire added a reviewer: aaron.ballman. Herald added subscribers: cfe-commits, mgorny. Repository: rC Clang https://reviews.llvm.org/D55492 Files: include/clang/AST/AttrVisitor.h include/clang/AST/CMakeLists.txt include/clang/AST/TextNodeDumper.h lib/A

[PATCH] D55488: Add utility for dumping a label with child nodes

steveire updated this revision to Diff 177419. steveire added a comment. Use std::string Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55488/new/ https://reviews.llvm.org/D55488 Files: include/clang/AST/ASTDumperUtils.h lib/AST/ASTDumper.cpp Index: lib/AST

[PATCH] D55395: Re-order content in OMPDeclareReductionDecl dump

steveire updated this revision to Diff 177420. steveire added a comment. Use child node labels Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55395/new/ https://reviews.llvm.org/D55395 Files: lib/AST/ASTDumper.cpp test/AST/dump.cpp Index: test/AST/dump.cpp

[PATCH] D55475: Misc typos fixes in ./lib folder

teemperor accepted this revision. teemperor added a comment. This revision is now accepted and ready to land. This looks all good to me and from what see this was requested in a previous review (D44188 ). Do you need someone to commit this or did you receive comm

[PATCH] D55491: Implement TemplateArgument dumping in terms of Visitor

steveire updated this revision to Diff 177421. steveire added a comment. Update Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55491/new/ https://reviews.llvm.org/D55491 Files: include/clang/AST/TemplateArgumentVisitor.h include/clang/AST/TextNodeDumper.h l

[PATCH] D55388: [analyzer] MoveChecker Pt.8: Add checks for dereferencing a smart pointer after move.

MTC added a comment. In D55388#1322601 , @xazax.hun wrote: > Hm. I wonder if it would also make sense to model e.g. the get method to > return nullptr for moved from smart ptrs. This could help null dereference > checker and also aid false path prunning.

[PATCH] D55395: Re-order content in OMPDeclareReductionDecl dump

aaron.ballman added a comment. This is a novel approach that's not used anywhere else in the AST dumper and there are several ways we could handle this, including: - What's proposed (adding a new node to the tree that's not directly an AST node) - Making use of the pointer information. e.g., ht

[PATCH] D55395: Re-order content in OMPDeclareReductionDecl dump

steveire added a comment. In D55395#1324699 , @aaron.ballman wrote: > This is a novel approach that's not used anywhere else in the AST dumper Actually it's used by the InitListExpr dump. See https://reviews.llvm.org/D55488 Repository: rC Clang CHA

[PATCH] D55395: Re-order content in OMPDeclareReductionDecl dump

riccibruno added a comment. In D55395#1324699 , @aaron.ballman wrote: > This is a novel approach that's not used anywhere else in the AST dumper and > there are several ways we could handle this, including: > > - What's proposed (adding a new node to the

[PATCH] D55495: Change InitListExpr dump to label and pointer

steveire created this revision. steveire added a reviewer: aaron.ballman. Herald added a subscriber: cfe-commits. Don't add a child just for the label. Repository: rC Clang https://reviews.llvm.org/D55495 Files: lib/AST/ASTDumper.cpp test/AST/ast-dump-stmt.cpp Index: test/AST/ast-dump-

[PATCH] D55495: Change InitListExpr dump to label and pointer

steveire added a comment. My real preference is just to remove the `array filler` child node entirely and not add a pointer here. However, I'm just looking for something that has a chance of passing review. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55495/new

[PATCH] D55395: Re-order content in OMPDeclareReductionDecl dump

steveire added a comment. I would prefer *not* to do what is in this patch. I'm just trying to find a way forward. I like the approach of using the existing precedent of printing the pointers as a compromise between removing such labels entirely and not having things like that at all. I did tha

[PATCH] D55395: Re-order content in OMPDeclareReductionDecl dump

steveire updated this revision to Diff 177428. steveire added a comment. Use pointer approach Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55395/new/ https://reviews.llvm.org/D55395 Files: lib/AST/ASTDumper.cpp test/AST/dump.cpp Index: test/AST/dump.cpp =

[PATCH] D55387: [analyzer] MoveChecker Pt.7: NFC: Misc refactoring.

a_sidorin added a comment. Hi Artem, The overall idea is good but I have some remarks inline. Comment at: lib/StaticAnalyzer/Checkers/MoveChecker.cpp:265 +void MoveChecker::checkUse(ProgramStateRef State, const MemRegion *Region, + const CXXRecordDec

[PATCH] D44100: [ASTImporter] Reorder fields after structure import is finished

a_sidorin added a comment. Hi Gabor, Thank you for the investigation! I'll try to take a look. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D44100/new/ https://reviews.llvm.org/D44100 ___ cfe-commits mailing list cfe

[PATCH] D55395: Re-order content in OMPDeclareReductionDecl dump

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM with a small nit. @ABataev, are you okay with this approach? Comment at: lib/AST/ASTDumper.cpp:1060 + dumpStmt(D->getCombiner()); + if (auto *Initializer = D->getInitializer()) { dumpStmt(Initializ

Re: r348713 - Fix InitListExpr test

On Sun, Dec 9, 2018 at 8:16 AM Stephen Kelly via cfe-commits wrote: > > Author: steveire > Date: Sun Dec 9 05:13:41 2018 > New Revision: 348713 > > URL: http://llvm.org/viewvc/llvm-project?rev=348713&view=rev > Log: > Fix InitListExpr test > > Wrong case of Check meant this has no effect. Good c

[PATCH] D54109: [clang-query] continue querying even if files are skipped

Lekensteyn abandoned this revision. Lekensteyn added a comment. This was addressed by https://reviews.llvm.org/D51183 (without new tests though). CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54109/new/ https://reviews.llvm.org/D54109 ___

[PATCH] D55393: Re-order content of template parameter dumps

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from some minor nits Comment at: lib/AST/ASTDumper.cpp:106 + const Decl *From = nullptr, +

[PATCH] D55489: Implement dumpFunctionDeclParameters in NodeDumper

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55489/new/ https://reviews.llvm.org/D55489 ___

[PATCH] D55490: Add dumpMethodDeclOverrides to NodeDumper

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM, aside from a minor nit. Comment at: lib/AST/ASTDumper.cpp:921 if (const CXXMethodDecl *MD = dyn_cast(D)) { +dumpMethodDeclOverrides(MD);

[PATCH] D55495: Change InitListExpr dump to label and pointer

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from a small nit. Comment at: lib/AST/ASTDumper.cpp:1956-1959 + OS << " array_filler"; + NodeDumper.dumpPointer(Filler); + dumpStmt(

[PATCH] D55475: Misc typos fixes in ./lib folder

luzpaz added a comment. Please commit as I do not have access/privs. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55475/new/ https://reviews.llvm.org/D55475 ___ cfe-commits mailing list cfe-commits@lists.llvm.org ht

[PATCH] D55492: Implement Attr dumping in terms of visitors

aaron.ballman added inline comments. Comment at: include/clang/AST/AttrVisitor.h:21 + +namespace attrvisitor { + I don't think the namespace adds value. Comment at: include/clang/AST/AttrVisitor.h:23-24 + +template struct make_ptr { using type

[PATCH] D55492: Implement Attr dumping in terms of visitors

steveire marked 3 inline comments as done. steveire added inline comments. Comment at: include/clang/AST/AttrVisitor.h:21 + +namespace attrvisitor { + aaron.ballman wrote: > I don't think the namespace adds value. I think the point is to separate the implementati

[PATCH] D55492: Implement Attr dumping in terms of visitors

aaron.ballman added inline comments. Comment at: include/clang/AST/AttrVisitor.h:21 + +namespace attrvisitor { + steveire wrote: > aaron.ballman wrote: > > I don't think the namespace adds value. > I think the point is to separate the implementation detail. I don

[PATCH] D55270: [Sema] Further improvements to to static_assert diagnostics.

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Comment at: test/PCH/cxx-static_assert.cpp:17 -// expected-error@12 {{static_assert failed "N is not 2!"}} +// expected-error@12 {{static_assert failed

[PATCH] D45898: [SemaCXX] Mark destructor as referenced

rjmccall added a comment. In D45898#1324625 , @orivej wrote: > I have noticed that this change breaks seemingly valid code: > > class A { protected: ~A(); }; > struct B : A {}; > B f() { return B(); } > B g() { return {}; } > > > `f` compiles, but

r348730 - Move the make_const_ptr trait into STLExtras; use add_pointer where possible; NFC.

Author: aaronballman Date: Sun Dec 9 11:53:24 2018 New Revision: 348730 URL: http://llvm.org/viewvc/llvm-project?rev=348730&view=rev Log: Move the make_const_ptr trait into STLExtras; use add_pointer where possible; NFC. Modified: cfe/trunk/include/clang/AST/CommentVisitor.h cfe/trunk/i

[PATCH] D55492: Implement Attr dumping in terms of visitors

aaron.ballman added inline comments. Comment at: include/clang/AST/AttrVisitor.h:23-24 + +template struct make_ptr { using type = T *; }; +template struct make_const_ptr { using type = const T *; }; + aaron.ballman wrote: > steveire wrote: > > aaron.ballman wro

[PATCH] D55468: Use zip_longest for iterator range comparisons. NFC.

dblaikie added inline comments. Comment at: lib/Sema/SemaOverload.cpp:8979 +// has fewer enable_if attributes than Cand2, and vice versa. +if (std::get<0>(Pair) == None) return Comparison::Worse; I'd probably write this as "if (!std::get<0>(Pair))"

[PATCH] D55468: Use zip_longest for iterator range comparisons. NFC.

Meinersbur updated this revision to Diff 177444. Meinersbur marked 4 inline comments as done. Meinersbur added a comment. - Address @dblaikie's review Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55468/new/ https://reviews.llvm.org/D55468 Files: lib/Sema/Sema

[PATCH] D55468: Use zip_longest for iterator range comparisons. NFC.

Meinersbur added inline comments. Comment at: lib/Sema/SemaOverload.cpp:8979 +// has fewer enable_if attributes than Cand2, and vice versa. +if (std::get<0>(Pair) == None) return Comparison::Worse; dblaikie wrote: > I'd probably write this as "if (

[PATCH] D40218: [Clang] Add __builtin_launder

EricWF marked 22 inline comments as done. EricWF added inline comments. Comment at: lib/CodeGen/CGBuiltin.cpp:1425-1426 + + // FIXME: We either have an incomplete class type, or we have a class template + // whose instantiation has not been forced. Example: + // -

[PATCH] D55468: Use zip_longest for iterator range comparisons. NFC.

dblaikie added inline comments. Comment at: lib/Sema/SemaOverload.cpp:8979 +// has fewer enable_if attributes than Cand2, and vice versa. +if (std::get<0>(Pair) == None) return Comparison::Worse; Meinersbur wrote: > dblaikie wrote: > > I'd probably

[PATCH] D40218: [Clang] Add __builtin_launder

EricWF updated this revision to Diff 177445. EricWF marked 6 inline comments as done. EricWF added a comment. Address review comments. @rsmith I think this is good to go. Just need your input on one change. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D40218/new/ https://reviews.llvm.o

[PATCH] D47111: : Implement monotonic_buffer_resource.

Quuxplusone added a comment. @ericwf ping? This still needs someone to land it; I don't have commit privs. Thanks! Repository: rCXX libc++ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D47111/new/ https://reviews.llvm.org/D47111 ___ cfe-

[PATCH] D55468: Use zip_longest for iterator range comparisons. NFC.

Meinersbur updated this revision to Diff 177447. Meinersbur added a comment. - Fix xor - Store tuple elements in variables Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55468/new/ https://reviews.llvm.org/D55468 Files: lib/Sema/SemaOverload.cpp lib/Serializa

[PATCH] D55468: Use zip_longest for iterator range comparisons. NFC.

dblaikie accepted this revision. dblaikie added a comment. This revision is now accepted and ready to land. Overall I'm not sure this construct/pattern improves readability, but not too fussed either way. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55468/new/

[PATCH] D55468: Use zip_longest for iterator range comparisons. NFC.

Meinersbur marked 3 inline comments as done. Meinersbur added inline comments. Comment at: lib/Serialization/ASTReaderDecl.cpp:2922 +// Return false if the number of enable_if attributes is different. +if (std::get<0>(Pair).hasValue() != std::get<1>(Pair).hasValue()) +

[PATCH] D55468: Use zip_longest for iterator range comparisons. NFC.

dblaikie added inline comments. Comment at: lib/Serialization/ASTReaderDecl.cpp:2922 +// Return false if the number of enable_if attributes is different. +if (std::get<0>(Pair).hasValue() != std::get<1>(Pair).hasValue()) + return false; Meinersbur wr

[PATCH] D55500: [Builtins] Implement __builtin_is_constant_evaluated for use in C++2a

EricWF created this revision. EricWF added reviewers: rsmith, MaskRay, bruno, void. Herald added a subscriber: kristina. This patch implements `__builtin_is_constant_evaluated` as specifier by P0595R2 . It is built on the back of Bill Wendling's work for `__builtin_con

[PATCH] D47111: : Implement monotonic_buffer_resource.

EricWF requested changes to this revision. EricWF added a comment. This revision now requires changes to proceed. Some of the tests aren't passing with Trunk clang. Please fix these. Comment at: include/experimental/memory_resource:427 +{ +static const size_t __default_buff

[PATCH] D47344: LWG 2843 "Unclear behavior of std::pmr::memory_resource::do_allocate()"

EricWF requested changes to this revision. EricWF added a comment. This revision now requires changes to proceed. I don't think this patch is correct or desirable any longer. I think what we should do is throw an exception right away if `__is_overaligned_for_new(align)`, and not even try to allo

[PATCH] D55484: ComputeLineNumbers: delete SSE2 vectorization

bkramer accepted this revision. bkramer added a comment. This revision is now accepted and ready to land. The performance difference on preprocessing huge files was tiny back then, doesn't surprise me that it disappeared. What did you test this on? Dropping it is fine with me. Repository: rC

[PATCH] D55415: Revert removal of tidy plugin support from libclang

bkramer accepted this revision. bkramer added a comment. This revision is now accepted and ready to land. I think that's a fair point for bringing it back for now. It's not supported though and we will get rid of it eventually. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55415/new/ h

r348738 - [X86] Remove the addcarry builtins. Leaving only the addcarryx builtins since that matches gcc.

Author: ctopper Date: Sun Dec 9 22:07:59 2018 New Revision: 348738 URL: http://llvm.org/viewvc/llvm-project?rev=348738&view=rev Log: [X86] Remove the addcarry builtins. Leaving only the addcarryx builtins since that matches gcc. The addcarry and addcarryx builtins do the same thing. The only di