JonChesterfield accepted this revision. JonChesterfield added a comment. This revision is now accepted and ready to land.
This looks good to me. I can't claim to have read the test case in detail. Some style suggestions inline around the vector, but they're not blocking. ================ Comment at: clang/include/clang/Sema/Sema.h:10042 void ActOnFinishedFunctionDefinitionInOpenMPDeclareVariantScope( - FunctionDecl *FD, FunctionDecl *BaseFD); + Decl *D, SmallVectorImpl<FunctionDecl *> &Bases); ---------------- SmallVectorImpl => SmallVector? ================ Comment at: clang/lib/Sema/SemaOpenMP.cpp:5826 -FunctionDecl * -Sema::ActOnStartOfFunctionDefinitionInOpenMPDeclareVariantScope(Scope *S, - Declarator &D) { +void Sema::ActOnStartOfFunctionDefinitionInOpenMPDeclareVariantScope( + Scope *S, Declarator &D, MultiTemplateParamsArg TemplateParamLists, ---------------- Return SmallVector<FunctionDecl> by value instead of via the reference? Makes it clear that we aren't adding to an existing vector ================ Comment at: clang/lib/Sema/SemaOpenMP.cpp:5875 + // TODO: Verify types for templates eventually. + if (!UDeclTy->isDependentType()) { + QualType NewType = Context.mergeFunctionTypes( ---------------- JonChesterfield wrote: > tabs! This turned out to be how phabricator represents changes in indent. Unfortunate that the symbol is the one I usually see used for tabs. ================ Comment at: clang/test/AST/ast-dump-openmp-begin-declare-variant_template_2.cpp:57 + +// CHECK: |-FunctionTemplateDecl [[ADDR_0:0x[a-z0-9]*]] <{{.*}}, line:7:1> line:5:5 also_before +// CHECK-NEXT: | |-TemplateTypeParmDecl [[ADDR_1:0x[a-z0-9]*]] <line:4:11, col:20> col:20 referenced typename depth 0 index 0 T ---------------- I'd like a different way for us to test this stuff. Realistically impossible to spot errors in 200 lines of tree. Don't have a suggestion at this time. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85735/new/ https://reviews.llvm.org/D85735 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits