Re: [PATCH] D15858: Warn undeclared identifiers in CUDA kernel calls
jlebar added inline comments. Comment at: test/SemaCUDA/kernel-call.cu:27 @@ -26,1 +26,3 @@ + + g1<<>>(42); // expected-error {{use of undeclared identifier 'undeclared'}} } We set four things in setConfig -- does this test fail if any one of them is commented out? If not, is it hard to add additional tests that will cover the changes? http://reviews.llvm.org/D15858 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15858: Warn undeclared identifiers in CUDA kernel calls
jhen updated this revision to Diff 44131. jhen added a reviewer: jlebar. jhen removed a subscriber: jlebar. jhen added a comment. - Correct dependence info in CUDA kernel call AST This patch removes the propagation of type and value dependence and the propagation of information on unexpanded parameter packs from the CUDA kernel configuration function call expression to its parent CUDA kernel call expression AST node. It does, however, maintain the propagation of instantiation dependence between those nodes, as introduced in the previous revision of this patch. The last patch should not have propagated value and type dependence from the CUDA kernel config function to the entire CUDA kernel call expression AST node. The reason is that the CUDA kernel call expression has a void value, so it's value cannot depend on template types or values, it is always simply void. However, the CUDA kernel call expression node can contain template arguments, so it can be instantiation dependent. That means that the instantiation dependence should be propagated from the config call to the kernel call node. The instantiation dependence propagation is also sufficient to fix the crashing bug that results from using an undeclared identifier as a config argument. As for tracking unexpanded parameter packs, it is not yet clear how the CUDA triple-angle-bracket syntax will interoperate with variadic templates, so I will leave that propagation out of this patch and it can be dealt with later. http://reviews.llvm.org/D15858 Files: include/clang/AST/ExprCXX.h test/SemaCUDA/kernel-call.cu Index: test/SemaCUDA/kernel-call.cu === --- test/SemaCUDA/kernel-call.cu +++ test/SemaCUDA/kernel-call.cu @@ -23,4 +23,6 @@ int (*fp)(int) = h2; fp<<<1, 1>>>(42); // expected-error {{must have void return type}} + + g1<<>>(42); // expected-error {{use of undeclared identifier 'undeclared'}} } Index: include/clang/AST/ExprCXX.h === --- include/clang/AST/ExprCXX.h +++ include/clang/AST/ExprCXX.h @@ -171,7 +171,11 @@ return cast_or_null(getPreArg(CONFIG)); } CallExpr *getConfig() { return cast_or_null(getPreArg(CONFIG)); } - void setConfig(CallExpr *E) { setPreArg(CONFIG, E); } + void setConfig(CallExpr *E) { +setPreArg(CONFIG, E); +setInstantiationDependent(isInstantiationDependent() || + E->isInstantiationDependent()); + } static bool classof(const Stmt *T) { return T->getStmtClass() == CUDAKernelCallExprClass; Index: test/SemaCUDA/kernel-call.cu === --- test/SemaCUDA/kernel-call.cu +++ test/SemaCUDA/kernel-call.cu @@ -23,4 +23,6 @@ int (*fp)(int) = h2; fp<<<1, 1>>>(42); // expected-error {{must have void return type}} + + g1<<>>(42); // expected-error {{use of undeclared identifier 'undeclared'}} } Index: include/clang/AST/ExprCXX.h === --- include/clang/AST/ExprCXX.h +++ include/clang/AST/ExprCXX.h @@ -171,7 +171,11 @@ return cast_or_null(getPreArg(CONFIG)); } CallExpr *getConfig() { return cast_or_null(getPreArg(CONFIG)); } - void setConfig(CallExpr *E) { setPreArg(CONFIG, E); } + void setConfig(CallExpr *E) { +setPreArg(CONFIG, E); +setInstantiationDependent(isInstantiationDependent() || + E->isInstantiationDependent()); + } static bool classof(const Stmt *T) { return T->getStmtClass() == CUDAKernelCallExprClass; ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15858: Warn undeclared identifiers in CUDA kernel calls
jlebar accepted this revision. jlebar added a comment. This revision is now accepted and ready to land. Looks sane to me, although I have no idea what I'm doing here; you should probably get someone else's approval. http://reviews.llvm.org/D15858 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15858: Warn undeclared identifiers in CUDA kernel calls
jhen added inline comments. Comment at: test/SemaCUDA/kernel-call.cu:27 @@ -26,1 +26,3 @@ + + g1<<>>(42); // expected-error {{use of undeclared identifier 'undeclared'}} } Thanks for bringing this up. While trying to find tests that dealt with each dependence individually, I came to realize that value and type dependence should not be set for the CUDAKernelCallExpr node because it's value is always void. So, I removed the propagation of those two dependencies. Then, while looking for a test that could handle the parameter pack information, I realized that it was opening up a whole new can of worms and that the triple-angle-bracket syntax does not currently support variadic templates. I decided that parameter packs should be handled as a separate bug, so I removed them from this patch. The instantiation dependence propagation is still valid, though, because it just represents whether a template parameter is present anywhere in the expression, so I left it in. Correctly tracking instantiation dependence in enough to fix the bug this patch was meant to fix, so I think it is the only change that should be made in this patch. http://reviews.llvm.org/D15858 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15858: Warn undeclared identifiers in CUDA kernel calls
rsmith added a subscriber: rsmith. Comment at: include/clang/AST/ExprCXX.h:175 @@ +174,3 @@ + void setConfig(CallExpr *E) { +setPreArg(CONFIG, E); +setInstantiationDependent(isInstantiationDependent() || Can you assert that the argument is only set once here? (If we set it to something instantiation-dependent and then to something that isn't instantiation-dependent, we'd compute the wrong instantiation-dependence flag.) This function should only be called by the normal constructor and by people who called the `EmptyShell` constructor. Comment at: test/SemaCUDA/kernel-call.cu:27 @@ -26,1 +26,3 @@ + + g1<<>>(42); // expected-error {{use of undeclared identifier 'undeclared'}} } jhen wrote: > Thanks for bringing this up. While trying to find tests that dealt with each > dependence individually, I came to realize that value and type dependence > should not be set for the CUDAKernelCallExpr node because it's value is > always void. So, I removed the propagation of those two dependencies. > > Then, while looking for a test that could handle the parameter pack > information, I realized that it was opening up a whole new can of worms and > that the triple-angle-bracket syntax does not currently support variadic > templates. I decided that parameter packs should be handled as a separate > bug, so I removed them from this patch. > > The instantiation dependence propagation is still valid, though, because it > just represents whether a template parameter is present anywhere in the > expression, so I left it in. Correctly tracking instantiation dependence in > enough to fix the bug this patch was meant to fix, so I think it is the only > change that should be made in this patch. What happens if an unexpanded pack is used within the kernel arguments of a CUDA kernel call? Do we already reject that? Are there tests for that somewhere? http://reviews.llvm.org/D15858 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15858: Warn undeclared identifiers in CUDA kernel calls
jhen marked 2 inline comments as done. jhen added a comment. http://reviews.llvm.org/D15858 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15858: Warn undeclared identifiers in CUDA kernel calls
jhen updated this revision to Diff 44143. jhen added a comment. - Assert setConfig only called once http://reviews.llvm.org/D15858 Files: include/clang/AST/ExprCXX.h test/SemaCUDA/kernel-call.cu Index: test/SemaCUDA/kernel-call.cu === --- test/SemaCUDA/kernel-call.cu +++ test/SemaCUDA/kernel-call.cu @@ -23,4 +23,6 @@ int (*fp)(int) = h2; fp<<<1, 1>>>(42); // expected-error {{must have void return type}} + + g1<<>>(42); // expected-error {{use of undeclared identifier 'undeclared'}} } Index: include/clang/AST/ExprCXX.h === --- include/clang/AST/ExprCXX.h +++ include/clang/AST/ExprCXX.h @@ -155,6 +155,7 @@ class CUDAKernelCallExpr : public CallExpr { private: enum { CONFIG, END_PREARG }; + bool IsConfigSet = false; public: CUDAKernelCallExpr(ASTContext &C, Expr *fn, CallExpr *Config, @@ -171,7 +172,19 @@ return cast_or_null(getPreArg(CONFIG)); } CallExpr *getConfig() { return cast_or_null(getPreArg(CONFIG)); } - void setConfig(CallExpr *E) { setPreArg(CONFIG, E); } + + /// \brief Sets the kernel configuration expression. + /// + /// Note that this method can only be called once per class instance. + void setConfig(CallExpr *E) { +assert( +!IsConfigSet && +"CUDAKernelCallExpr.setConfig can only be called once per instance."); +setPreArg(CONFIG, E); +setInstantiationDependent(isInstantiationDependent() || + E->isInstantiationDependent()); +IsConfigSet = true; + } static bool classof(const Stmt *T) { return T->getStmtClass() == CUDAKernelCallExprClass; Index: test/SemaCUDA/kernel-call.cu === --- test/SemaCUDA/kernel-call.cu +++ test/SemaCUDA/kernel-call.cu @@ -23,4 +23,6 @@ int (*fp)(int) = h2; fp<<<1, 1>>>(42); // expected-error {{must have void return type}} + + g1<<>>(42); // expected-error {{use of undeclared identifier 'undeclared'}} } Index: include/clang/AST/ExprCXX.h === --- include/clang/AST/ExprCXX.h +++ include/clang/AST/ExprCXX.h @@ -155,6 +155,7 @@ class CUDAKernelCallExpr : public CallExpr { private: enum { CONFIG, END_PREARG }; + bool IsConfigSet = false; public: CUDAKernelCallExpr(ASTContext &C, Expr *fn, CallExpr *Config, @@ -171,7 +172,19 @@ return cast_or_null(getPreArg(CONFIG)); } CallExpr *getConfig() { return cast_or_null(getPreArg(CONFIG)); } - void setConfig(CallExpr *E) { setPreArg(CONFIG, E); } + + /// \brief Sets the kernel configuration expression. + /// + /// Note that this method can only be called once per class instance. + void setConfig(CallExpr *E) { +assert( +!IsConfigSet && +"CUDAKernelCallExpr.setConfig can only be called once per instance."); +setPreArg(CONFIG, E); +setInstantiationDependent(isInstantiationDependent() || + E->isInstantiationDependent()); +IsConfigSet = true; + } static bool classof(const Stmt *T) { return T->getStmtClass() == CUDAKernelCallExprClass; ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15858: Warn undeclared identifiers in CUDA kernel calls
jhen marked an inline comment as done. Comment at: test/SemaCUDA/kernel-call.cu:27 @@ -26,1 +26,3 @@ + + g1<<>>(42); // expected-error {{use of undeclared identifier 'undeclared'}} } rsmith wrote: > jhen wrote: > > Thanks for bringing this up. While trying to find tests that dealt with > > each dependence individually, I came to realize that value and type > > dependence should not be set for the CUDAKernelCallExpr node because it's > > value is always void. So, I removed the propagation of those two > > dependencies. > > > > Then, while looking for a test that could handle the parameter pack > > information, I realized that it was opening up a whole new can of worms and > > that the triple-angle-bracket syntax does not currently support variadic > > templates. I decided that parameter packs should be handled as a separate > > bug, so I removed them from this patch. > > > > The instantiation dependence propagation is still valid, though, because it > > just represents whether a template parameter is present anywhere in the > > expression, so I left it in. Correctly tracking instantiation dependence in > > enough to fix the bug this patch was meant to fix, so I think it is the > > only change that should be made in this patch. > What happens if an unexpanded pack is used within the kernel arguments of a > CUDA kernel call? Do we already reject that? Are there tests for that > somewhere? There don't seem to be any tests currently that handle this case. The case I had in mind for an unexpanded parameter pack was something like the following: __global__ void kernel() {} template kernel_wrapper() { kernel<<>>(); } This currently leads to a warning at the time of parsing that says the closing ">>>" is not found. I believe the cause is that the argument list is parsed as a simple argument list, so it doesn't handle the ellipsis correctly. I experimented with using standard (non-simple) parsing for the argument list, but that led to failures in other unit tests where ">>" wasn't being warned correctly in C++98 mode. I'm planning to file a bug for this (at least to fix the warning if not to allow the construction) and deal with it in a later patch. Does that sound reasonable? http://reviews.llvm.org/D15858 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15858: Warn undeclared identifiers in CUDA kernel calls
rsmith added inline comments. Comment at: include/clang/AST/ExprCXX.h:181 @@ +180,3 @@ +assert( +!IsConfigSet && +"CUDAKernelCallExpr.setConfig can only be called once per instance."); Perhaps `assert(!getPreArg(CONFIG))` instead of storing a separate flag? Comment at: test/SemaCUDA/kernel-call.cu:27 @@ -26,1 +26,3 @@ + + g1<<>>(42); // expected-error {{use of undeclared identifier 'undeclared'}} } Your approach for that testcase seems fine, but it's not a test for the right thing as it doesn't have an unexpanded pack within the kernel call args. Here's a testcase for that scenario: template void kernel_wrapper() { void (*fs[])() = { []{ kernel<<>>(); } ... }; } http://reviews.llvm.org/D15858 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15858: Warn undeclared identifiers in CUDA kernel calls
jhen added inline comments. Comment at: include/clang/AST/ExprCXX.h:181 @@ +180,3 @@ +assert( +!IsConfigSet && +"CUDAKernelCallExpr.setConfig can only be called once per instance."); rsmith wrote: > Perhaps `assert(!getPreArg(CONFIG))` instead of storing a separate flag? Yes, I think that is much nicer. But I am worried that the config pre-arg might not be initialized before I check it, so in my next revision of this patch I've also added code in the CallExpr constructors to make sure that pre-args are always initialized to zero. Unfortunately, the simple way to do this leads to the entire array of function argument pointers being initialized for every function call expression construction (not just CUDA calls). Do you think I should try to avoid this overhead by only initializing the pre-arguments (either in the CallExpr code for everyone to share, or just in the CUDA code)? Comment at: test/SemaCUDA/kernel-call.cu:27 @@ -26,1 +26,3 @@ + + g1<<>>(42); // expected-error {{use of undeclared identifier 'undeclared'}} } rsmith wrote: > Your approach for that testcase seems fine, but it's not a test for the right > thing as it doesn't have an unexpanded pack within the kernel call args. > Here's a testcase for that scenario: > > template void kernel_wrapper() { > void (*fs[])() = { > []{ kernel<<>>(); } ... > }; > } Excellent! Thank you for that test case. I suspected that I wasn't really exercising the "unexpanded" parameter pack with my code. http://reviews.llvm.org/D15858 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15858: Warn undeclared identifiers in CUDA kernel calls
jhen updated this revision to Diff 44167. jhen added a comment. - Use config ptr itself rather than boolean flag http://reviews.llvm.org/D15858 Files: include/clang/AST/ExprCXX.h lib/AST/Expr.cpp test/SemaCUDA/kernel-call.cu Index: test/SemaCUDA/kernel-call.cu === --- test/SemaCUDA/kernel-call.cu +++ test/SemaCUDA/kernel-call.cu @@ -23,4 +23,6 @@ int (*fp)(int) = h2; fp<<<1, 1>>>(42); // expected-error {{must have void return type}} + + g1<<>>(42); // expected-error {{use of undeclared identifier 'undeclared'}} } Index: lib/AST/Expr.cpp === --- lib/AST/Expr.cpp +++ lib/AST/Expr.cpp @@ -1148,7 +1148,7 @@ fn->containsUnexpandedParameterPack()), NumArgs(args.size()) { - SubExprs = new (C) Stmt*[args.size()+PREARGS_START+NumPreArgs]; + SubExprs = new (C) Stmt*[args.size()+PREARGS_START+NumPreArgs](); SubExprs[FN] = fn; for (unsigned i = 0; i != args.size(); ++i) { if (args[i]->isTypeDependent()) @@ -1179,7 +1179,7 @@ EmptyShell Empty) : Expr(SC, Empty), SubExprs(nullptr), NumArgs(0) { // FIXME: Why do we allocate this? - SubExprs = new (C) Stmt*[PREARGS_START+NumPreArgs]; + SubExprs = new (C) Stmt*[PREARGS_START+NumPreArgs](); CallExprBits.NumPreArgs = NumPreArgs; } Index: include/clang/AST/ExprCXX.h === --- include/clang/AST/ExprCXX.h +++ include/clang/AST/ExprCXX.h @@ -171,7 +171,18 @@ return cast_or_null(getPreArg(CONFIG)); } CallExpr *getConfig() { return cast_or_null(getPreArg(CONFIG)); } - void setConfig(CallExpr *E) { setPreArg(CONFIG, E); } + + /// \brief Sets the kernel configuration expression. + /// + /// Note that this method can only be called once per class instance. + void setConfig(CallExpr *E) { +assert( +!getConfig() && +"CUDAKernelCallExpr.setConfig can only be called once per instance."); +setPreArg(CONFIG, E); +setInstantiationDependent(isInstantiationDependent() || + E->isInstantiationDependent()); + } static bool classof(const Stmt *T) { return T->getStmtClass() == CUDAKernelCallExprClass; Index: test/SemaCUDA/kernel-call.cu === --- test/SemaCUDA/kernel-call.cu +++ test/SemaCUDA/kernel-call.cu @@ -23,4 +23,6 @@ int (*fp)(int) = h2; fp<<<1, 1>>>(42); // expected-error {{must have void return type}} + + g1<<>>(42); // expected-error {{use of undeclared identifier 'undeclared'}} } Index: lib/AST/Expr.cpp === --- lib/AST/Expr.cpp +++ lib/AST/Expr.cpp @@ -1148,7 +1148,7 @@ fn->containsUnexpandedParameterPack()), NumArgs(args.size()) { - SubExprs = new (C) Stmt*[args.size()+PREARGS_START+NumPreArgs]; + SubExprs = new (C) Stmt*[args.size()+PREARGS_START+NumPreArgs](); SubExprs[FN] = fn; for (unsigned i = 0; i != args.size(); ++i) { if (args[i]->isTypeDependent()) @@ -1179,7 +1179,7 @@ EmptyShell Empty) : Expr(SC, Empty), SubExprs(nullptr), NumArgs(0) { // FIXME: Why do we allocate this? - SubExprs = new (C) Stmt*[PREARGS_START+NumPreArgs]; + SubExprs = new (C) Stmt*[PREARGS_START+NumPreArgs](); CallExprBits.NumPreArgs = NumPreArgs; } Index: include/clang/AST/ExprCXX.h === --- include/clang/AST/ExprCXX.h +++ include/clang/AST/ExprCXX.h @@ -171,7 +171,18 @@ return cast_or_null(getPreArg(CONFIG)); } CallExpr *getConfig() { return cast_or_null(getPreArg(CONFIG)); } - void setConfig(CallExpr *E) { setPreArg(CONFIG, E); } + + /// \brief Sets the kernel configuration expression. + /// + /// Note that this method can only be called once per class instance. + void setConfig(CallExpr *E) { +assert( +!getConfig() && +"CUDAKernelCallExpr.setConfig can only be called once per instance."); +setPreArg(CONFIG, E); +setInstantiationDependent(isInstantiationDependent() || + E->isInstantiationDependent()); + } static bool classof(const Stmt *T) { return T->getStmtClass() == CUDAKernelCallExprClass; ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15858: Warn undeclared identifiers in CUDA kernel calls
rsmith added inline comments. Comment at: include/clang/AST/ExprCXX.h:180 @@ +179,3 @@ +assert( +!getConfig() && +"CUDAKernelCallExpr.setConfig can only be called once per instance."); My preference would be to pass the `CallExpr` constructor an `ArrayRef` for the preargs and have it initialize them itself. http://reviews.llvm.org/D15858 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15858: Warn undeclared identifiers in CUDA kernel calls
jhen updated this revision to Diff 44538. jhen added a comment. - Handle unexpanded parameter packs The changes for instantiation dependence also fix a bug with unexpanded parameter packs, so add a unit test for unexpanded parameter packs as well. http://reviews.llvm.org/D15858 Files: include/clang/AST/Expr.h include/clang/AST/ExprCXX.h lib/AST/Expr.cpp test/SemaCUDA/cxx11-kernel-call.cu test/SemaCUDA/kernel-call.cu Index: test/SemaCUDA/kernel-call.cu === --- test/SemaCUDA/kernel-call.cu +++ test/SemaCUDA/kernel-call.cu @@ -23,4 +23,6 @@ int (*fp)(int) = h2; fp<<<1, 1>>>(42); // expected-error {{must have void return type}} + + g1<<>>(42); // expected-error {{use of undeclared identifier 'undeclared'}} } Index: test/SemaCUDA/cxx11-kernel-call.cu === --- /dev/null +++ test/SemaCUDA/cxx11-kernel-call.cu @@ -0,0 +1,9 @@ +// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s + +#include "Inputs/cuda.h" + +__global__ void k1() {} + +template void k1Wrapper() { + void (*f)() = [] { k1<<>>(); }; // expected-error {{initializer contains unexpanded parameter pack 'Dimensions'}} +} Index: lib/AST/Expr.cpp === --- lib/AST/Expr.cpp +++ lib/AST/Expr.cpp @@ -1138,38 +1138,38 @@ // Postfix Operators. //===--===// -CallExpr::CallExpr(const ASTContext& C, StmtClass SC, Expr *fn, - unsigned NumPreArgs, ArrayRef args, QualType t, +CallExpr::CallExpr(const ASTContext &C, StmtClass SC, Expr *fn, + ArrayRef preargs, ArrayRef args, QualType t, ExprValueKind VK, SourceLocation rparenloc) - : Expr(SC, t, VK, OK_Ordinary, - fn->isTypeDependent(), - fn->isValueDependent(), - fn->isInstantiationDependent(), - fn->containsUnexpandedParameterPack()), -NumArgs(args.size()) { - - SubExprs = new (C) Stmt*[args.size()+PREARGS_START+NumPreArgs]; +: Expr(SC, t, VK, OK_Ordinary, fn->isTypeDependent(), + fn->isValueDependent(), fn->isInstantiationDependent(), + fn->containsUnexpandedParameterPack()), + NumArgs(args.size()) { + + unsigned NumPreArgs = preargs.size(); + SubExprs = new (C) Stmt *[args.size()+PREARGS_START+NumPreArgs]; SubExprs[FN] = fn; + for (unsigned i = 0; i != NumPreArgs; ++i) { +updateDependenciesFromArg(preargs[i]); +SubExprs[i+PREARGS_START] = preargs[i]; + } for (unsigned i = 0; i != args.size(); ++i) { -if (args[i]->isTypeDependent()) - ExprBits.TypeDependent = true; -if (args[i]->isValueDependent()) - ExprBits.ValueDependent = true; -if (args[i]->isInstantiationDependent()) - ExprBits.InstantiationDependent = true; -if (args[i]->containsUnexpandedParameterPack()) - ExprBits.ContainsUnexpandedParameterPack = true; - +updateDependenciesFromArg(args[i]); SubExprs[i+PREARGS_START+NumPreArgs] = args[i]; } CallExprBits.NumPreArgs = NumPreArgs; RParenLoc = rparenloc; } +CallExpr::CallExpr(const ASTContext &C, StmtClass SC, Expr *fn, + ArrayRef args, QualType t, ExprValueKind VK, + SourceLocation rparenloc) +: CallExpr(C, SC, fn, ArrayRef(), args, t, VK, rparenloc) {} + CallExpr::CallExpr(const ASTContext &C, Expr *fn, ArrayRef args, QualType t, ExprValueKind VK, SourceLocation rparenloc) -: CallExpr(C, CallExprClass, fn, /*NumPreArgs=*/0, args, t, VK, rparenloc) { +: CallExpr(C, CallExprClass, fn, ArrayRef(), args, t, VK, rparenloc) { } CallExpr::CallExpr(const ASTContext &C, StmtClass SC, EmptyShell Empty) @@ -1179,10 +1179,21 @@ EmptyShell Empty) : Expr(SC, Empty), SubExprs(nullptr), NumArgs(0) { // FIXME: Why do we allocate this? - SubExprs = new (C) Stmt*[PREARGS_START+NumPreArgs]; + SubExprs = new (C) Stmt*[PREARGS_START+NumPreArgs](); CallExprBits.NumPreArgs = NumPreArgs; } +void CallExpr::updateDependenciesFromArg(Expr *Arg) { + if (Arg->isTypeDependent()) +ExprBits.TypeDependent = true; + if (Arg->isValueDependent()) +ExprBits.ValueDependent = true; + if (Arg->isInstantiationDependent()) +ExprBits.InstantiationDependent = true; + if (Arg->containsUnexpandedParameterPack()) +ExprBits.ContainsUnexpandedParameterPack = true; +} + Decl *CallExpr::getCalleeDecl() { Expr *CEE = getCallee()->IgnoreParenImpCasts(); Index: include/clang/AST/ExprCXX.h === --- include/clang/AST/ExprCXX.h +++ include/clang/AST/ExprCXX.h @@ -66,8 +66,7 @@ CXXOperatorCallExpr(ASTContext& C, OverloadedOperatorKind Op, Expr *fn, ArrayRef args, QualType t, ExprValueKind VK, SourceLocation operatorloc, bool fpContractable)
Re: [PATCH] D15858: Warn undeclared identifiers in CUDA kernel calls
jhen added a comment. rsmith, I think the patch is ready to be committed. Please take a look if you have a moment. Thanks for your help. http://reviews.llvm.org/D15858 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15858: Warn undeclared identifiers in CUDA kernel calls
rsmith accepted this revision. rsmith added a reviewer: rsmith. rsmith added a comment. LGTM, thanks! Comment at: test/SemaCUDA/cxx11-kernel-call.cu:8 @@ +7,3 @@ +template void k1Wrapper() { + void (*f)() = [] { k1<<>>(); }; // expected-error {{initializer contains unexpanded parameter pack 'Dimensions'}} +} Perhaps also add another test: void (*g[])() = { [] { k1<<>>(); } ... }; // ok http://reviews.llvm.org/D15858 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15858: Warn undeclared identifiers in CUDA kernel calls
jhen updated this revision to Diff 44933. jhen added a comment. - Add extra test for OK parameter pack http://reviews.llvm.org/D15858 Files: include/clang/AST/Expr.h include/clang/AST/ExprCXX.h lib/AST/Expr.cpp test/SemaCUDA/cxx11-kernel-call.cu test/SemaCUDA/kernel-call.cu Index: test/SemaCUDA/kernel-call.cu === --- test/SemaCUDA/kernel-call.cu +++ test/SemaCUDA/kernel-call.cu @@ -23,4 +23,6 @@ int (*fp)(int) = h2; fp<<<1, 1>>>(42); // expected-error {{must have void return type}} + + g1<<>>(42); // expected-error {{use of undeclared identifier 'undeclared'}} } Index: test/SemaCUDA/cxx11-kernel-call.cu === --- /dev/null +++ test/SemaCUDA/cxx11-kernel-call.cu @@ -0,0 +1,10 @@ +// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s + +#include "Inputs/cuda.h" + +__global__ void k1() {} + +template void k1Wrapper() { + void (*f)() = [] { k1<<>>(); }; // expected-error {{initializer contains unexpanded parameter pack 'Dimensions'}} + void (*g[])() = { [] { k1<<>>(); } ... }; // ok +} Index: lib/AST/Expr.cpp === --- lib/AST/Expr.cpp +++ lib/AST/Expr.cpp @@ -1138,38 +1138,38 @@ // Postfix Operators. //===--===// -CallExpr::CallExpr(const ASTContext& C, StmtClass SC, Expr *fn, - unsigned NumPreArgs, ArrayRef args, QualType t, +CallExpr::CallExpr(const ASTContext &C, StmtClass SC, Expr *fn, + ArrayRef preargs, ArrayRef args, QualType t, ExprValueKind VK, SourceLocation rparenloc) - : Expr(SC, t, VK, OK_Ordinary, - fn->isTypeDependent(), - fn->isValueDependent(), - fn->isInstantiationDependent(), - fn->containsUnexpandedParameterPack()), -NumArgs(args.size()) { - - SubExprs = new (C) Stmt*[args.size()+PREARGS_START+NumPreArgs]; +: Expr(SC, t, VK, OK_Ordinary, fn->isTypeDependent(), + fn->isValueDependent(), fn->isInstantiationDependent(), + fn->containsUnexpandedParameterPack()), + NumArgs(args.size()) { + + unsigned NumPreArgs = preargs.size(); + SubExprs = new (C) Stmt *[args.size()+PREARGS_START+NumPreArgs]; SubExprs[FN] = fn; + for (unsigned i = 0; i != NumPreArgs; ++i) { +updateDependenciesFromArg(preargs[i]); +SubExprs[i+PREARGS_START] = preargs[i]; + } for (unsigned i = 0; i != args.size(); ++i) { -if (args[i]->isTypeDependent()) - ExprBits.TypeDependent = true; -if (args[i]->isValueDependent()) - ExprBits.ValueDependent = true; -if (args[i]->isInstantiationDependent()) - ExprBits.InstantiationDependent = true; -if (args[i]->containsUnexpandedParameterPack()) - ExprBits.ContainsUnexpandedParameterPack = true; - +updateDependenciesFromArg(args[i]); SubExprs[i+PREARGS_START+NumPreArgs] = args[i]; } CallExprBits.NumPreArgs = NumPreArgs; RParenLoc = rparenloc; } +CallExpr::CallExpr(const ASTContext &C, StmtClass SC, Expr *fn, + ArrayRef args, QualType t, ExprValueKind VK, + SourceLocation rparenloc) +: CallExpr(C, SC, fn, ArrayRef(), args, t, VK, rparenloc) {} + CallExpr::CallExpr(const ASTContext &C, Expr *fn, ArrayRef args, QualType t, ExprValueKind VK, SourceLocation rparenloc) -: CallExpr(C, CallExprClass, fn, /*NumPreArgs=*/0, args, t, VK, rparenloc) { +: CallExpr(C, CallExprClass, fn, ArrayRef(), args, t, VK, rparenloc) { } CallExpr::CallExpr(const ASTContext &C, StmtClass SC, EmptyShell Empty) @@ -1179,10 +1179,21 @@ EmptyShell Empty) : Expr(SC, Empty), SubExprs(nullptr), NumArgs(0) { // FIXME: Why do we allocate this? - SubExprs = new (C) Stmt*[PREARGS_START+NumPreArgs]; + SubExprs = new (C) Stmt*[PREARGS_START+NumPreArgs](); CallExprBits.NumPreArgs = NumPreArgs; } +void CallExpr::updateDependenciesFromArg(Expr *Arg) { + if (Arg->isTypeDependent()) +ExprBits.TypeDependent = true; + if (Arg->isValueDependent()) +ExprBits.ValueDependent = true; + if (Arg->isInstantiationDependent()) +ExprBits.InstantiationDependent = true; + if (Arg->containsUnexpandedParameterPack()) +ExprBits.ContainsUnexpandedParameterPack = true; +} + Decl *CallExpr::getCalleeDecl() { Expr *CEE = getCallee()->IgnoreParenImpCasts(); Index: include/clang/AST/ExprCXX.h === --- include/clang/AST/ExprCXX.h +++ include/clang/AST/ExprCXX.h @@ -66,8 +66,7 @@ CXXOperatorCallExpr(ASTContext& C, OverloadedOperatorKind Op, Expr *fn, ArrayRef args, QualType t, ExprValueKind VK, SourceLocation operatorloc, bool fpContractable) -: CallExpr(C, CXXOperatorCallExprClass, fn, 0, args, t, VK, - operatorloc), +
Re: [PATCH] D15858: Warn undeclared identifiers in CUDA kernel calls
jhen marked an inline comment as done. jhen added a comment. Thanks for the review rsmith! http://reviews.llvm.org/D15858 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15858: Warn undeclared identifiers in CUDA kernel calls
This revision was automatically updated to reflect the committed changes. Closed by commit rL257839: [CUDA] Warn undeclared identifiers in CUDA kernel calls (authored by jlebar). Changed prior to commit: http://reviews.llvm.org/D15858?vs=44933&id=44938#toc Repository: rL LLVM http://reviews.llvm.org/D15858 Files: cfe/trunk/include/clang/AST/Expr.h cfe/trunk/include/clang/AST/ExprCXX.h cfe/trunk/lib/AST/Expr.cpp cfe/trunk/test/SemaCUDA/cxx11-kernel-call.cu cfe/trunk/test/SemaCUDA/kernel-call.cu Index: cfe/trunk/test/SemaCUDA/cxx11-kernel-call.cu === --- cfe/trunk/test/SemaCUDA/cxx11-kernel-call.cu +++ cfe/trunk/test/SemaCUDA/cxx11-kernel-call.cu @@ -0,0 +1,10 @@ +// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s + +#include "Inputs/cuda.h" + +__global__ void k1() {} + +template void k1Wrapper() { + void (*f)() = [] { k1<<>>(); }; // expected-error {{initializer contains unexpanded parameter pack 'Dimensions'}} + void (*g[])() = { [] { k1<<>>(); } ... }; // ok +} Index: cfe/trunk/test/SemaCUDA/kernel-call.cu === --- cfe/trunk/test/SemaCUDA/kernel-call.cu +++ cfe/trunk/test/SemaCUDA/kernel-call.cu @@ -23,4 +23,6 @@ int (*fp)(int) = h2; fp<<<1, 1>>>(42); // expected-error {{must have void return type}} + + g1<<>>(42); // expected-error {{use of undeclared identifier 'undeclared'}} } Index: cfe/trunk/lib/AST/Expr.cpp === --- cfe/trunk/lib/AST/Expr.cpp +++ cfe/trunk/lib/AST/Expr.cpp @@ -1138,38 +1138,38 @@ // Postfix Operators. //===--===// -CallExpr::CallExpr(const ASTContext& C, StmtClass SC, Expr *fn, - unsigned NumPreArgs, ArrayRef args, QualType t, +CallExpr::CallExpr(const ASTContext &C, StmtClass SC, Expr *fn, + ArrayRef preargs, ArrayRef args, QualType t, ExprValueKind VK, SourceLocation rparenloc) - : Expr(SC, t, VK, OK_Ordinary, - fn->isTypeDependent(), - fn->isValueDependent(), - fn->isInstantiationDependent(), - fn->containsUnexpandedParameterPack()), -NumArgs(args.size()) { +: Expr(SC, t, VK, OK_Ordinary, fn->isTypeDependent(), + fn->isValueDependent(), fn->isInstantiationDependent(), + fn->containsUnexpandedParameterPack()), + NumArgs(args.size()) { - SubExprs = new (C) Stmt*[args.size()+PREARGS_START+NumPreArgs]; + unsigned NumPreArgs = preargs.size(); + SubExprs = new (C) Stmt *[args.size()+PREARGS_START+NumPreArgs]; SubExprs[FN] = fn; + for (unsigned i = 0; i != NumPreArgs; ++i) { +updateDependenciesFromArg(preargs[i]); +SubExprs[i+PREARGS_START] = preargs[i]; + } for (unsigned i = 0; i != args.size(); ++i) { -if (args[i]->isTypeDependent()) - ExprBits.TypeDependent = true; -if (args[i]->isValueDependent()) - ExprBits.ValueDependent = true; -if (args[i]->isInstantiationDependent()) - ExprBits.InstantiationDependent = true; -if (args[i]->containsUnexpandedParameterPack()) - ExprBits.ContainsUnexpandedParameterPack = true; - +updateDependenciesFromArg(args[i]); SubExprs[i+PREARGS_START+NumPreArgs] = args[i]; } CallExprBits.NumPreArgs = NumPreArgs; RParenLoc = rparenloc; } +CallExpr::CallExpr(const ASTContext &C, StmtClass SC, Expr *fn, + ArrayRef args, QualType t, ExprValueKind VK, + SourceLocation rparenloc) +: CallExpr(C, SC, fn, ArrayRef(), args, t, VK, rparenloc) {} + CallExpr::CallExpr(const ASTContext &C, Expr *fn, ArrayRef args, QualType t, ExprValueKind VK, SourceLocation rparenloc) -: CallExpr(C, CallExprClass, fn, /*NumPreArgs=*/0, args, t, VK, rparenloc) { +: CallExpr(C, CallExprClass, fn, ArrayRef(), args, t, VK, rparenloc) { } CallExpr::CallExpr(const ASTContext &C, StmtClass SC, EmptyShell Empty) @@ -1179,10 +1179,21 @@ EmptyShell Empty) : Expr(SC, Empty), SubExprs(nullptr), NumArgs(0) { // FIXME: Why do we allocate this? - SubExprs = new (C) Stmt*[PREARGS_START+NumPreArgs]; + SubExprs = new (C) Stmt*[PREARGS_START+NumPreArgs](); CallExprBits.NumPreArgs = NumPreArgs; } +void CallExpr::updateDependenciesFromArg(Expr *Arg) { + if (Arg->isTypeDependent()) +ExprBits.TypeDependent = true; + if (Arg->isValueDependent()) +ExprBits.ValueDependent = true; + if (Arg->isInstantiationDependent()) +ExprBits.InstantiationDependent = true; + if (Arg->containsUnexpandedParameterPack()) +ExprBits.ContainsUnexpandedParameterPack = true; +} + Decl *CallExpr::getCalleeDecl() { Expr *CEE = getCallee()->IgnoreParenImpCasts(); Index: cfe/trunk/include/clang/AST/Expr.h === --- cfe/trunk/include/clang/AST/Expr.h +++ c