[PATCH] D105457: [clang] Refactor AST printing tests to share more infrastructure

nridge added a comment. @dyung I appreciate the help tracking this down! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105457/new/ https://reviews.llvm.org/D105457 ___ cfe-commits mailing list

[PATCH] D105457: [clang] Refactor AST printing tests to share more infrastructure

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG9cfec72ffeec: [clang] Refactor AST printing tests to share more infrastructure (authored by nridge). Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D105457: [clang] Refactor AST printing tests to share more infrastructure

dyung accepted this revision. dyung added a comment. In D105457#2878352 , @nridge wrote: > In D105457#2878302 , @dyung wrote: > >> Interesting. If you can get me an updated patch, I can give it a try on my >>

[PATCH] D105457: [clang] Refactor AST printing tests to share more infrastructure

nridge added a comment. In D105457#2878302 , @dyung wrote: > Interesting. If you can get me an updated patch, I can give it a try on my > machine with 7.5 to verify if you like. I posted an updated patch, if you can give that a whirl that would be

[PATCH] D105457: [clang] Refactor AST printing tests to share more infrastructure

nridge updated this revision to Diff 358750. nridge added a comment. Work around a gcc 7 bug Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105457/new/ https://reviews.llvm.org/D105457 Files: clang/unittests/AST/ASTPrint.h

[PATCH] D105457: [clang] Refactor AST printing tests to share more infrastructure

dyung added a comment. In D105457#2878292 , @nridge wrote: > Making the default argument a non-lambda seems to be sufficient to avoid the > error: > > template > class function { > public: > template > function(F) {} > }; > > void

[PATCH] D105457: [clang] Refactor AST printing tests to share more infrastructure

nridge added a comment. Making the default argument a non-lambda seems to be sufficient to avoid the error: template class function { public: template function(F) {} }; void DefaultFunc(); template void Foo(M, function = DefaultFunc); void Bar() {

[PATCH] D105457: [clang] Refactor AST printing tests to share more infrastructure

nridge added a comment. Thanks! I reduced it further to: template class function { public: template function(F) {} }; template void Foo(M, function = [](){}); void Bar() { Foo(42); Foo(42.0); } with gcc 7, this gives: /tmp/cccKjL8O.s: Assembler

[PATCH] D105457: [clang] Refactor AST printing tests to share more infrastructure

dyung added a comment. In case it is useful, here is the full preprocessed file. F17936275: StmtPrinterTest.preproc.cpp.orig.gz Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105457/new/

[PATCH] D105457: [clang] Refactor AST printing tests to share more infrastructure

dyung added a comment. In D105457#2877971 , @dblaikie wrote: > In D105457#2876511 , @dyung wrote: > >> If it helps, I have so far been able to reduce the file to this which still >> shows the failure when

[PATCH] D105457: [clang] Refactor AST printing tests to share more infrastructure

dblaikie added a comment. In D105457#2876511 , @dyung wrote: > If it helps, I have so far been able to reduce the file to this which still > shows the failure when compiled with gcc 7.5: Could you provide the preprocessed input file - maybe that run

[PATCH] D105457: [clang] Refactor AST printing tests to share more infrastructure

dyung added a comment. If it helps, I have so far been able to reduce the file to this which still shows the failure when compiled with gcc 7.5: #include "ASTPrint.h" #include "clang/AST/ASTContext.h" #include "clang/ASTMatchers/ASTMatchFinder.h" #include "clang/Tooling/Tooling.h"

[PATCH] D105457: [clang] Refactor AST printing tests to share more infrastructure

dyung added a comment. In D105457#2875672 , @dblaikie wrote: > In D105457#2874783 , @nridge wrote: > >> To be honest, I don't really understand this error. It seems to come from a >> compile step (not a link

[PATCH] D105457: [clang] Refactor AST printing tests to share more infrastructure

dblaikie added a comment. In D105457#2874783 , @nridge wrote: > To be honest, I don't really understand this error. It seems to come from a > compile step (not a link step), and it comes from the assembler and says > "symbol ... is already defined"? I

[PATCH] D105457: [clang] Refactor AST printing tests to share more infrastructure

dyung added a comment. In D105457#2875058 , @glaubitz wrote: > In D105457#2874516 , @dblaikie > wrote: > >> Any ideas what version of the standard library these buildbots are using? >> This /looks/ a bit like a

[PATCH] D105457: [clang] Refactor AST printing tests to share more infrastructure

glaubitz added a comment. In D105457#2874516 , @dblaikie wrote: > Any ideas what version of the standard library these buildbots are using? > This /looks/ a bit like a bug in the standard library in use, perhaps? (also > because it's not showing up in

[PATCH] D105457: [clang] Refactor AST printing tests to share more infrastructure

nridge added a comment. To be honest, I don't really understand this error. It seems to come from a compile step (not a link step), and it comes from the assembler and says "symbol ... is already defined"? I don't think I've seen an error like that before. (Typically, errors about duplicate

[PATCH] D105457: [clang] Refactor AST printing tests to share more infrastructure

dblaikie added a comment. In D105457#2873838 , @glaubitz wrote: > In D105457#2873294 , @dyung wrote: > >> This change I suspect is causing a build failure on a build bot. >> >>

[PATCH] D105457: [clang] Refactor AST printing tests to share more infrastructure

glaubitz added a comment. In D105457#2873294 , @dyung wrote: > This change I suspect is causing a build failure on a build bot. > > https://lab.llvm.org/buildbot/#/builders/110/builds/4827 > > FAILED: /usr/bin/c++ -DCLANG_ROUND_TRIP_CC1_ARGS=ON

[PATCH] D105457: [clang] Refactor AST printing tests to share more infrastructure

dyung added a comment. This change I suspect is causing a build failure on a build bot. https://lab.llvm.org/buildbot/#/builders/110/builds/4827 FAILED: /usr/bin/c++ -DCLANG_ROUND_TRIP_CC1_ARGS=ON -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS

[PATCH] D105457: [clang] Refactor AST printing tests to share more infrastructure

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG20176bc7dd3f: [clang] Refactor AST printing tests to share more infrastructure (authored by nridge). Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D105457: [clang] Refactor AST printing tests to share more infrastructure

dblaikie accepted this revision. dblaikie added a comment. This revision is now accepted and ready to land. Looks reasonable to me, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105457/new/ https://reviews.llvm.org/D105457

[PATCH] D105457: [clang] Refactor AST printing tests to share more infrastructure

nridge added inline comments. Comment at: clang/unittests/AST/DeclPrinterTest.cpp:59 + return PrintedNodeMatches(Code, Args, NodeMatch, ExpectedPrinted, + FileName, PrinterType{PrintDecl}, + PolicyModifier,

[PATCH] D105457: [clang] Refactor AST printing tests to share more infrastructure

nridge updated this revision to Diff 357826. nridge marked 3 inline comments as done. nridge added a comment. Address review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105457/new/ https://reviews.llvm.org/D105457 Files:

[PATCH] D105457: [clang] Refactor AST printing tests to share more infrastructure

dblaikie added a comment. Looks generally good - few things might be able to be tidied up/simplified, I think? Comment at: clang/unittests/AST/ASTPrint.h:61-62 +private: + // Can be specialized for specific node types. + bool shouldIgnoreNode(const NodeType *N) { return

[PATCH] D105457: [clang] Refactor AST printing tests to share more infrastructure

nridge updated this revision to Diff 356605. nridge added a comment. Move a piece of Decl-specific code into DeclPrinterTest.cpp Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105457/new/ https://reviews.llvm.org/D105457 Files:

[PATCH] D105457: [clang] Refactor AST printing tests to share more infrastructure

nridge added a comment. This is the refactor patch discussed in https://reviews.llvm.org/D104619#2846262. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105457/new/ https://reviews.llvm.org/D105457 ___

[PATCH] D105457: [clang] Refactor AST printing tests to share more infrastructure

nridge created this revision. nridge added a reviewer: dblaikie. nridge requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D105457 Files: clang/unittests/AST/ASTPrint.h