[PATCH] D25450: [clang-tidy] Fix identifier naming in macro args.
jhen created this revision.
jhen added a reviewer: alexfh.
jhen added a subscriber: cfe-commits.
Herald added a subscriber: jlebar.
clang-tidy should fix identifier naming even when the identifier is
referenced inside a macro expansion, provided that the identifier enters
the macro expansion completely within a macro argument.
For example, this will allow fixes to the naming of the identifier
'global' when it is declared and used as follows:
int global;
#define USE_IN_MACRO(m) auto use_##m = m
USE_IN_MACRO(global);
https://reviews.llvm.org/D25450
Files:
clang-tidy/readability/IdentifierNamingCheck.cpp
test/clang-tidy/readability-identifier-naming.cpp
Index: test/clang-tidy/readability-identifier-naming.cpp
===
--- test/clang-tidy/readability-identifier-naming.cpp
+++ test/clang-tidy/readability-identifier-naming.cpp
@@ -95,15 +95,35 @@
USER_MACRO(var2);
// NO warnings or fixes expected as var2 is declared in a macro expansion
-int global;
-#define USE_IN_MACRO(m) auto use_##m = m
-USE_IN_MACRO(global);
-// NO warnings or fixes expected as global is used in a macro expansion
-
#define BLA int FOO_bar
BLA;
// NO warnings or fixes expected as FOO_bar is from macro expansion
+int global0;
+#define USE_NUMBERED_GLOBAL(number) auto use_global##number = global##number
+USE_NUMBERED_GLOBAL(0);
+// NO warnings or fixes expected as global0 is pieced together in a macro
+// expansion.
+
+int global1;
+#define USE_NUMBERED_BAL(prefix, number) \
+ auto use_##prefix##bal##number = prefix##bal##number
+USE_NUMBERED_BAL(glo, 1);
+// NO warnings or fixes expected as global1 is pieced together in a macro
+// expansion.
+
+int global2;
+#define USE_RECONSTRUCTED(glo, bal) auto use_##glo##bal = glo##bal
+USE_RECONSTRUCTED(glo, bal2);
+// NO warnings or fixes expected as global2 is pieced together in a macro
+// expansion.
+
+int global;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: invalid case style for global variable 'global'
+// CHECK-FIXES: {{^}}int g_global;{{$}}
+#define USE_IN_MACRO(m) auto use_##m = m
+USE_IN_MACRO(global);
+
enum my_enumeration {
// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: invalid case style for enum 'my_enumeration'
// CHECK-FIXES: {{^}}enum EMyEnumeration {{{$}}
Index: clang-tidy/readability/IdentifierNamingCheck.cpp
===
--- clang-tidy/readability/IdentifierNamingCheck.cpp
+++ clang-tidy/readability/IdentifierNamingCheck.cpp
@@ -612,7 +612,7 @@
static void addUsage(IdentifierNamingCheck::NamingCheckFailureMap &Failures,
const IdentifierNamingCheck::NamingCheckId &Decl,
- SourceRange Range) {
+ SourceRange Range, SourceManager *SourceMgr = nullptr) {
// Do nothing if the provided range is invalid.
if (Range.getBegin().isInvalid() || Range.getEnd().isInvalid())
return;
@@ -623,16 +623,39 @@
if (!Failure.RawUsageLocs.insert(Range.getBegin().getRawEncoding()).second)
return;
- Failure.ShouldFix = Failure.ShouldFix && !Range.getBegin().isMacroID() &&
- !Range.getEnd().isMacroID();
+ // Check if the range is entirely contained within a macro argument.
+ SourceLocation MacroArgExpansionStartForRangeBegin;
+ SourceLocation MacroArgExpansionStartForRangeEnd;
+ bool RangeIsEntirelyWithinMacroArgument =
+ SourceMgr &&
+ SourceMgr->isMacroArgExpansion(Range.getBegin(),
+ &MacroArgExpansionStartForRangeBegin) &&
+ SourceMgr->isMacroArgExpansion(Range.getEnd(),
+ &MacroArgExpansionStartForRangeEnd) &&
+ MacroArgExpansionStartForRangeBegin == MacroArgExpansionStartForRangeEnd;
+
+ // Check if the range contains any locations from a macro expansion.
+ bool RangeContainsMacroExpansion = RangeIsEntirelyWithinMacroArgument;
+ if (!RangeIsEntirelyWithinMacroArgument)
+for (SourceLocation Loc = Range.getBegin(); Loc.isValid();
+ Loc = Loc.getLocWithOffset(1)) {
+ RangeContainsMacroExpansion =
+ RangeContainsMacroExpansion || Loc.isMacroID();
+ if (Loc == Range.getEnd())
+break;
+}
+
+ bool RangeCanBeFixed =
+ RangeIsEntirelyWithinMacroArgument || !RangeContainsMacroExpansion;
+ Failure.ShouldFix = Failure.ShouldFix && RangeCanBeFixed;
}
/// Convenience method when the usage to be added is a NamedDecl
static void addUsage(IdentifierNamingCheck::NamingCheckFailureMap &Failures,
- const NamedDecl *Decl, SourceRange Range) {
+ const NamedDecl *Decl, SourceRange Range, SourceManager *SourceMgr = nullptr) {
return addUsage(Failures, IdentifierNamingCheck::NamingCheckId(
Decl->getLocation(), Decl->getNameAsString()),
- Range);
+ Range, SourceMgr);
}
void IdentifierNamingCheck::check(const MatchFinder::
[PATCH] D25450: [clang-tidy] Fix identifier naming in macro args.
jhen added a comment. alexfh, sorry if you are not the right person to review this change. I based my choice on this history of this file. https://reviews.llvm.org/D25450 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D25450: [clang-tidy] Fix identifier naming in macro args.
jhen updated this revision to Diff 74181.
jhen added a comment.
- Return to original checking for macro in range
https://reviews.llvm.org/D25450
Files:
clang-tidy/readability/IdentifierNamingCheck.cpp
test/clang-tidy/readability-identifier-naming.cpp
Index: test/clang-tidy/readability-identifier-naming.cpp
===
--- test/clang-tidy/readability-identifier-naming.cpp
+++ test/clang-tidy/readability-identifier-naming.cpp
@@ -95,15 +95,35 @@
USER_MACRO(var2);
// NO warnings or fixes expected as var2 is declared in a macro expansion
-int global;
-#define USE_IN_MACRO(m) auto use_##m = m
-USE_IN_MACRO(global);
-// NO warnings or fixes expected as global is used in a macro expansion
-
#define BLA int FOO_bar
BLA;
// NO warnings or fixes expected as FOO_bar is from macro expansion
+int global0;
+#define USE_NUMBERED_GLOBAL(number) auto use_global##number = global##number
+USE_NUMBERED_GLOBAL(0);
+// NO warnings or fixes expected as global0 is pieced together in a macro
+// expansion.
+
+int global1;
+#define USE_NUMBERED_BAL(prefix, number) \
+ auto use_##prefix##bal##number = prefix##bal##number
+USE_NUMBERED_BAL(glo, 1);
+// NO warnings or fixes expected as global1 is pieced together in a macro
+// expansion.
+
+int global2;
+#define USE_RECONSTRUCTED(glo, bal) auto use_##glo##bal = glo##bal
+USE_RECONSTRUCTED(glo, bal2);
+// NO warnings or fixes expected as global2 is pieced together in a macro
+// expansion.
+
+int global;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: invalid case style for global variable 'global'
+// CHECK-FIXES: {{^}}int g_global;{{$}}
+#define USE_IN_MACRO(m) auto use_##m = m
+USE_IN_MACRO(global);
+
enum my_enumeration {
// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: invalid case style for enum 'my_enumeration'
// CHECK-FIXES: {{^}}enum EMyEnumeration {{{$}}
Index: clang-tidy/readability/IdentifierNamingCheck.cpp
===
--- clang-tidy/readability/IdentifierNamingCheck.cpp
+++ clang-tidy/readability/IdentifierNamingCheck.cpp
@@ -612,7 +612,7 @@
static void addUsage(IdentifierNamingCheck::NamingCheckFailureMap &Failures,
const IdentifierNamingCheck::NamingCheckId &Decl,
- SourceRange Range) {
+ SourceRange Range, SourceManager *SourceMgr = nullptr) {
// Do nothing if the provided range is invalid.
if (Range.getBegin().isInvalid() || Range.getEnd().isInvalid())
return;
@@ -623,16 +623,33 @@
if (!Failure.RawUsageLocs.insert(Range.getBegin().getRawEncoding()).second)
return;
- Failure.ShouldFix = Failure.ShouldFix && !Range.getBegin().isMacroID() &&
- !Range.getEnd().isMacroID();
+ // Check if the range is entirely contained within a macro argument.
+ SourceLocation MacroArgExpansionStartForRangeBegin;
+ SourceLocation MacroArgExpansionStartForRangeEnd;
+ bool RangeIsEntirelyWithinMacroArgument =
+ SourceMgr &&
+ SourceMgr->isMacroArgExpansion(Range.getBegin(),
+ &MacroArgExpansionStartForRangeBegin) &&
+ SourceMgr->isMacroArgExpansion(Range.getEnd(),
+ &MacroArgExpansionStartForRangeEnd) &&
+ MacroArgExpansionStartForRangeBegin == MacroArgExpansionStartForRangeEnd;
+
+ // Check if the range contains any locations from a macro expansion.
+ bool RangeContainsMacroExpansion = RangeIsEntirelyWithinMacroArgument ||
+ Range.getBegin().isMacroID() ||
+ Range.getEnd().isMacroID();
+
+ bool RangeCanBeFixed =
+ RangeIsEntirelyWithinMacroArgument || !RangeContainsMacroExpansion;
+ Failure.ShouldFix = Failure.ShouldFix && RangeCanBeFixed;
}
/// Convenience method when the usage to be added is a NamedDecl
static void addUsage(IdentifierNamingCheck::NamingCheckFailureMap &Failures,
- const NamedDecl *Decl, SourceRange Range) {
+ const NamedDecl *Decl, SourceRange Range, SourceManager *SourceMgr = nullptr) {
return addUsage(Failures, IdentifierNamingCheck::NamingCheckId(
Decl->getLocation(), Decl->getNameAsString()),
- Range);
+ Range, SourceMgr);
}
void IdentifierNamingCheck::check(const MatchFinder::MatchResult &Result) {
@@ -719,7 +736,7 @@
if (const auto *DeclRef = Result.Nodes.getNodeAs("declRef")) {
SourceRange Range = DeclRef->getNameInfo().getSourceRange();
-addUsage(NamingCheckFailures, DeclRef->getDecl(), Range);
+addUsage(NamingCheckFailures, DeclRef->getDecl(), Range, Result.SourceManager);
return;
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D25450: [clang-tidy] Fix identifier naming in macro args.
jhen added a comment. I found a bug in my first patch that I have fixed now. I was trying to iterate over the source range by using `SourceLocation::getLocWithOffset`, but I realized that doesn't work, so I removed it and went back to the original method of checking `SourceRange.getBegin().isMacroID()` and `SourceRange.getEnd().isMacroID()`. https://reviews.llvm.org/D25450 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D25450: [clang-tidy] Fix identifier naming in macro args.
jhen updated this revision to Diff 74184.
jhen added a comment.
- Prevent multiple fixes for macro expansion
https://reviews.llvm.org/D25450
Files:
clang-tidy/readability/IdentifierNamingCheck.cpp
test/clang-tidy/readability-identifier-naming.cpp
Index: test/clang-tidy/readability-identifier-naming.cpp
===
--- test/clang-tidy/readability-identifier-naming.cpp
+++ test/clang-tidy/readability-identifier-naming.cpp
@@ -95,14 +95,41 @@
USER_MACRO(var2);
// NO warnings or fixes expected as var2 is declared in a macro expansion
+#define BLA int FOO_bar
+BLA;
+// NO warnings or fixes expected as FOO_bar is from macro expansion
+
+int global0;
+#define USE_NUMBERED_GLOBAL(number) auto use_global##number = global##number
+USE_NUMBERED_GLOBAL(0);
+// NO warnings or fixes expected as global0 is pieced together in a macro
+// expansion.
+
+int global1;
+#define USE_NUMBERED_BAL(prefix, number) \
+ auto use_##prefix##bal##number = prefix##bal##number
+USE_NUMBERED_BAL(glo, 1);
+// NO warnings or fixes expected as global1 is pieced together in a macro
+// expansion.
+
+int global2;
+#define USE_RECONSTRUCTED(glo, bal) auto use_##glo##bal = glo##bal
+USE_RECONSTRUCTED(glo, bal2);
+// NO warnings or fixes expected as global2 is pieced together in a macro
+// expansion.
+
int global;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: invalid case style for global variable 'global'
+// CHECK-FIXES: {{^}}int g_global;{{$}}
#define USE_IN_MACRO(m) auto use_##m = m
USE_IN_MACRO(global);
-// NO warnings or fixes expected as global is used in a macro expansion
-#define BLA int FOO_bar
-BLA;
-// NO warnings or fixes expected as FOO_bar is from macro expansion
+int global3;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: invalid case style for global variable 'global3'
+// CHECK-FIXES: {{^}}int g_global3;{{$}}
+#define ADD_TO_SELF(m) (m) + (m)
+int g_twice_global3 = ADD_TO_SELF(global3);
+// CHECK-FIXES: {{^}}int g_twice_global3 = ADD_TO_SELF(g_global3);{{$}}
enum my_enumeration {
// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: invalid case style for enum 'my_enumeration'
Index: clang-tidy/readability/IdentifierNamingCheck.cpp
===
--- clang-tidy/readability/IdentifierNamingCheck.cpp
+++ clang-tidy/readability/IdentifierNamingCheck.cpp
@@ -612,27 +612,54 @@
static void addUsage(IdentifierNamingCheck::NamingCheckFailureMap &Failures,
const IdentifierNamingCheck::NamingCheckId &Decl,
- SourceRange Range) {
+ SourceRange Range, SourceManager *SourceMgr = nullptr) {
// Do nothing if the provided range is invalid.
if (Range.getBegin().isInvalid() || Range.getEnd().isInvalid())
return;
+ // If we have a source manager, use it to convert to the spelling location for
+ // performing the fix. This is necessary because macros can map the same
+ // spelling location to different source locations, and we only want to fix
+ // the token once, before it is expanded by the macro.
+ SourceLocation FixLocation = Range.getBegin();
+ if (SourceMgr)
+FixLocation = SourceMgr->getSpellingLoc(FixLocation);
+ if (FixLocation.isInvalid())
+return;
+
// Try to insert the identifier location in the Usages map, and bail out if it
// is already in there
auto &Failure = Failures[Decl];
- if (!Failure.RawUsageLocs.insert(Range.getBegin().getRawEncoding()).second)
+ if (!Failure.RawUsageLocs.insert(FixLocation.getRawEncoding()).second)
return;
- Failure.ShouldFix = Failure.ShouldFix && !Range.getBegin().isMacroID() &&
- !Range.getEnd().isMacroID();
+ // Check if the range is entirely contained within a macro argument.
+ SourceLocation MacroArgExpansionStartForRangeBegin;
+ SourceLocation MacroArgExpansionStartForRangeEnd;
+ bool RangeIsEntirelyWithinMacroArgument =
+ SourceMgr &&
+ SourceMgr->isMacroArgExpansion(Range.getBegin(),
+ &MacroArgExpansionStartForRangeBegin) &&
+ SourceMgr->isMacroArgExpansion(Range.getEnd(),
+ &MacroArgExpansionStartForRangeEnd) &&
+ MacroArgExpansionStartForRangeBegin == MacroArgExpansionStartForRangeEnd;
+
+ // Check if the range contains any locations from a macro expansion.
+ bool RangeContainsMacroExpansion = RangeIsEntirelyWithinMacroArgument ||
+ Range.getBegin().isMacroID() ||
+ Range.getEnd().isMacroID();
+
+ bool RangeCanBeFixed =
+ RangeIsEntirelyWithinMacroArgument || !RangeContainsMacroExpansion;
+ Failure.ShouldFix = Failure.ShouldFix && RangeCanBeFixed;
}
/// Convenience method when the usage to be added is a NamedDecl
static void addUsage(IdentifierNamingCheck::NamingCheckFailureMap &Failures,
- const NamedDecl *Decl, SourceRange Range) {
+
[PATCH] D25450: [clang-tidy] Fix identifier naming in macro args.
jhen added a comment. I just found and fixed another bug in this patch. Before, I wasn't using the spelling location for the fixit hint. This meant that a macro argument that was expanded to two locations, for example, would have the same fixit hint applied to it twice. My new test case verifies that this does not happen anymore. https://reviews.llvm.org/D25450 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r288448 - [CUDA] "Support" ASAN arguments in CudaToolChain
Author: jhen
Date: Thu Dec 1 19:42:54 2016
New Revision: 288448
URL: http://llvm.org/viewvc/llvm-project?rev=288448&view=rev
Log:
[CUDA] "Support" ASAN arguments in CudaToolChain
This fixes a bug that was introduced in rL287285. The bug made it
illegal to pass -fsanitize=address during CUDA compilation because the
CudaToolChain class was switched from deriving from the Linux toolchain
class to deriving directly from the ToolChain toolchain class. When
CudaToolChain derived from Linux, it used Linux's getSupportedSanitizers
method, and that method allowed ASAN, but when it switched to deriving
directly from ToolChain, it inherited a getSupportedSanitizers method
that didn't allow for ASAN.
This patch fixes that bug by creating a getSupportedSanitizers method
for CudaToolChain that supports ASAN.
This patch also fixes the test that checks that -fsanitize=address is
passed correctly for CUDA builds. That test didn't used to notice if an
error message was emitted, and that's why it didn't catch this bug when
it was first introduced. With the fix from this patch, that test will
now catch any similar bug in the future.
Modified:
cfe/trunk/lib/Driver/ToolChains.cpp
cfe/trunk/lib/Driver/ToolChains.h
cfe/trunk/test/Driver/cuda-no-sanitizers.cu
Modified: cfe/trunk/lib/Driver/ToolChains.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains.cpp?rev=288448&r1=288447&r2=288448&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains.cpp Thu Dec 1 19:42:54 2016
@@ -4973,6 +4973,15 @@ void CudaToolChain::AddIAMCUIncludeArgs(
HostTC.AddIAMCUIncludeArgs(Args, CC1Args);
}
+SanitizerMask CudaToolChain::getSupportedSanitizers() const {
+ // The CudaToolChain only supports address sanitization in the sense that it
+ // allows ASAN arguments on the command line. It must not error out on these
+ // command line arguments because the host code compilation supports them.
+ // However, it doesn't actually do any address sanitization for device code;
+ // instead, it just ignores any ASAN command line arguments it sees.
+ return SanitizerKind::Address;
+}
+
/// XCore tool chain
XCoreToolChain::XCoreToolChain(const Driver &D, const llvm::Triple &Triple,
const ArgList &Args)
Modified: cfe/trunk/lib/Driver/ToolChains.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains.h?rev=288448&r1=288447&r2=288448&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains.h (original)
+++ cfe/trunk/lib/Driver/ToolChains.h Thu Dec 1 19:42:54 2016
@@ -912,6 +912,8 @@ public:
void AddIAMCUIncludeArgs(const llvm::opt::ArgList &DriverArgs,
llvm::opt::ArgStringList &CC1Args) const override;
+ SanitizerMask getSupportedSanitizers() const override;
+
const ToolChain &HostTC;
CudaInstallationDetector CudaInstallation;
Modified: cfe/trunk/test/Driver/cuda-no-sanitizers.cu
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/cuda-no-sanitizers.cu?rev=288448&r1=288447&r2=288448&view=diff
==
--- cfe/trunk/test/Driver/cuda-no-sanitizers.cu (original)
+++ cfe/trunk/test/Driver/cuda-no-sanitizers.cu Thu Dec 1 19:42:54 2016
@@ -6,6 +6,7 @@
// RUN: %clang -### -target x86_64-linux-gnu -c --cuda-gpu-arch=sm_20
-fsanitize=address %s 2>&1 | \
// RUN: FileCheck %s
+// CHECK-NOT: error:
// CHECK-DAG: "-fcuda-is-device"
// CHECK-NOT: "-fsanitize=address"
// CHECK-DAG: "-triple" "x86_64--linux-gnu"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r288453 - [CUDA] Fix faulty test from rL288448
Author: jhen Date: Thu Dec 1 20:04:43 2016 New Revision: 288453 URL: http://llvm.org/viewvc/llvm-project?rev=288453&view=rev Log: [CUDA] Fix faulty test from rL288448 Summary: The test introduced by rL288448 is currently failing because unimportant but unexpected errors appear as output from a test compile line. This patch looks for a more specific error message, in order to avoid false positives. Reviewers: jlebar Subscribers: cfe-commits Differential Revision: https://reviews.llvm.org/D27328 Switch to more specific error Modified: cfe/trunk/test/Driver/cuda-no-sanitizers.cu Modified: cfe/trunk/test/Driver/cuda-no-sanitizers.cu URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/cuda-no-sanitizers.cu?rev=288453&r1=288452&r2=288453&view=diff == --- cfe/trunk/test/Driver/cuda-no-sanitizers.cu (original) +++ cfe/trunk/test/Driver/cuda-no-sanitizers.cu Thu Dec 1 20:04:43 2016 @@ -6,7 +6,7 @@ // RUN: %clang -### -target x86_64-linux-gnu -c --cuda-gpu-arch=sm_20 -fsanitize=address %s 2>&1 | \ // RUN: FileCheck %s -// CHECK-NOT: error: +// CHECK-NOT: error: unsupported option '-fsanitize=address' // CHECK-DAG: "-fcuda-is-device" // CHECK-NOT: "-fsanitize=address" // CHECK-DAG: "-triple" "x86_64--linux-gnu" ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r288448 - [CUDA] "Support" ASAN arguments in CudaToolChain
Hi Hal,
I don't understand why only ASAN is enabled. What about TSAN, etc.?
Shouldn't you query whatever the host toolchain is?
I think you're right, I should just implement it as
SanitizerMask CudaToolChain::getSupportedSanitizers() const { return
HostTC.getSupportedSanitizers(); }
I will get a patch up later today to do that.
Thanks for the suggestion!
-Jason
On Fri, Dec 2, 2016 at 4:31 AM Hal Finkel wrote:
> - Original Message -
> > From: "Jason Henline via cfe-commits"
> > To: cfe-commits@lists.llvm.org
> > Sent: Thursday, December 1, 2016 7:42:54 PM
> > Subject: r288448 - [CUDA] "Support" ASAN arguments in CudaToolChain
> >
> > Author: jhen
> > Date: Thu Dec 1 19:42:54 2016
> > New Revision: 288448
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=288448&view=rev
> > Log:
> > [CUDA] "Support" ASAN arguments in CudaToolChain
> >
> > This fixes a bug that was introduced in rL287285. The bug made it
> > illegal to pass -fsanitize=address during CUDA compilation because
> > the
> > CudaToolChain class was switched from deriving from the Linux
> > toolchain
> > class to deriving directly from the ToolChain toolchain class. When
> > CudaToolChain derived from Linux, it used Linux's
> > getSupportedSanitizers
> > method, and that method allowed ASAN, but when it switched to
> > deriving
> > directly from ToolChain, it inherited a getSupportedSanitizers method
> > that didn't allow for ASAN.
> >
> > This patch fixes that bug by creating a getSupportedSanitizers method
> > for CudaToolChain that supports ASAN.
> >
> > This patch also fixes the test that checks that -fsanitize=address is
> > passed correctly for CUDA builds. That test didn't used to notice if
> > an
> > error message was emitted, and that's why it didn't catch this bug
> > when
> > it was first introduced. With the fix from this patch, that test will
> > now catch any similar bug in the future.
> >
> > Modified:
> > cfe/trunk/lib/Driver/ToolChains.cpp
> > cfe/trunk/lib/Driver/ToolChains.h
> > cfe/trunk/test/Driver/cuda-no-sanitizers.cu
> >
> > Modified: cfe/trunk/lib/Driver/ToolChains.cpp
> > URL:
> >
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains.cpp?rev=288448&r1=288447&r2=288448&view=diff
> >
> ==
> > --- cfe/trunk/lib/Driver/ToolChains.cpp (original)
> > +++ cfe/trunk/lib/Driver/ToolChains.cpp Thu Dec 1 19:42:54 2016
> > @@ -4973,6 +4973,15 @@ void CudaToolChain::AddIAMCUIncludeArgs(
> >HostTC.AddIAMCUIncludeArgs(Args, CC1Args);
> > }
> >
> > +SanitizerMask CudaToolChain::getSupportedSanitizers() const {
> > + // The CudaToolChain only supports address sanitization in the
> > sense that it
> > + // allows ASAN arguments on the command line. It must not error
> > out on these
> > + // command line arguments because the host code compilation
> > supports them.
> > + // However, it doesn't actually do any address sanitization for
> > device code;
> > + // instead, it just ignores any ASAN command line arguments it
> > sees.
> > + return SanitizerKind::Address;
> > +}
>
> I don't understand why only ASAN is enabled. What about TSAN, etc.?
> Shouldn't you query whatever the host toolchain is?
>
> Thanks again,
> Hal
>
> > +
> > /// XCore tool chain
> > XCoreToolChain::XCoreToolChain(const Driver &D, const llvm::Triple
> > &Triple,
> > const ArgList &Args)
> >
> > Modified: cfe/trunk/lib/Driver/ToolChains.h
> > URL:
> >
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains.h?rev=288448&r1=288447&r2=288448&view=diff
> >
> ==
> > --- cfe/trunk/lib/Driver/ToolChains.h (original)
> > +++ cfe/trunk/lib/Driver/ToolChains.h Thu Dec 1 19:42:54 2016
> > @@ -912,6 +912,8 @@ public:
> >void AddIAMCUIncludeArgs(const llvm::opt::ArgList &DriverArgs,
> > llvm::opt::ArgStringList &CC1Args) const
> > override;
> >
> > + SanitizerMask getSupportedSanitizers() const override;
> > +
> >const ToolChain &HostTC;
> >CudaInstallationDetector CudaInstallation;
> >
> >
> > Modified: cfe/trunk/test/D
r288512 - [CUDA] Forward sanitizer support to host toolchain
Author: jhen
Date: Fri Dec 2 11:32:18 2016
New Revision: 288512
URL: http://llvm.org/viewvc/llvm-project?rev=288512&view=rev
Log:
[CUDA] Forward sanitizer support to host toolchain
Summary:
This is an improvement on rL288448 where address sanitization was listed
as supported for the CudaToolChain. Since the intent is for the
CudaToolChain not to reject any flags supported by the host compiler,
this patch switches to forwarding the CudaToolChain sanitizer support to
the host toolchain rather than explicitly whitelisting address
sanitization.
Thanks to hfinkel for this suggestion.
Reviewers: jlebar
Subscribers: hfinkel, cfe-commits
Differential Revision: https://reviews.llvm.org/D27351
Modified:
cfe/trunk/lib/Driver/ToolChains.cpp
Modified: cfe/trunk/lib/Driver/ToolChains.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains.cpp?rev=288512&r1=288511&r2=288512&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains.cpp Fri Dec 2 11:32:18 2016
@@ -4974,12 +4974,16 @@ void CudaToolChain::AddIAMCUIncludeArgs(
}
SanitizerMask CudaToolChain::getSupportedSanitizers() const {
- // The CudaToolChain only supports address sanitization in the sense that it
- // allows ASAN arguments on the command line. It must not error out on these
- // command line arguments because the host code compilation supports them.
- // However, it doesn't actually do any address sanitization for device code;
- // instead, it just ignores any ASAN command line arguments it sees.
- return SanitizerKind::Address;
+ // The CudaToolChain only supports sanitizers in the sense that it allows
+ // sanitizer arguments on the command line if they are supported by the host
+ // toolchain. The CudaToolChain will actually ignore any command line
+ // arguments for any of these "supported" sanitizers. That means that no
+ // sanitization of device code is actually supported at this time.
+ //
+ // This behavior is necessary because the host and device toolchains
+ // invocations often share the command line, so the device toolchain must
+ // tolerate flags meant only for the host toolchain.
+ return HostTC.getSupportedSanitizers();
}
/// XCore tool chain
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D25450: [clang-tidy] Fix identifier naming in macro args.
jhen added a comment. Adding arron.ballman as a reviewer as alexfh seems to be on leave for a few weeks. https://reviews.llvm.org/D25450 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D25450: [clang-tidy] Fix identifier naming in macro args.
jhen updated this revision to Diff 75430.
jhen added a comment.
- Early exit if not Failure.ShouldFix
https://reviews.llvm.org/D25450
Files:
clang-tidy/readability/IdentifierNamingCheck.cpp
test/clang-tidy/readability-identifier-naming.cpp
Index: test/clang-tidy/readability-identifier-naming.cpp
===
--- test/clang-tidy/readability-identifier-naming.cpp
+++ test/clang-tidy/readability-identifier-naming.cpp
@@ -95,14 +95,41 @@
USER_MACRO(var2);
// NO warnings or fixes expected as var2 is declared in a macro expansion
+#define BLA int FOO_bar
+BLA;
+// NO warnings or fixes expected as FOO_bar is from macro expansion
+
+int global0;
+#define USE_NUMBERED_GLOBAL(number) auto use_global##number = global##number
+USE_NUMBERED_GLOBAL(0);
+// NO warnings or fixes expected as global0 is pieced together in a macro
+// expansion.
+
+int global1;
+#define USE_NUMBERED_BAL(prefix, number) \
+ auto use_##prefix##bal##number = prefix##bal##number
+USE_NUMBERED_BAL(glo, 1);
+// NO warnings or fixes expected as global1 is pieced together in a macro
+// expansion.
+
+int global2;
+#define USE_RECONSTRUCTED(glo, bal) auto use_##glo##bal = glo##bal
+USE_RECONSTRUCTED(glo, bal2);
+// NO warnings or fixes expected as global2 is pieced together in a macro
+// expansion.
+
int global;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: invalid case style for global variable 'global'
+// CHECK-FIXES: {{^}}int g_global;{{$}}
#define USE_IN_MACRO(m) auto use_##m = m
USE_IN_MACRO(global);
-// NO warnings or fixes expected as global is used in a macro expansion
-#define BLA int FOO_bar
-BLA;
-// NO warnings or fixes expected as FOO_bar is from macro expansion
+int global3;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: invalid case style for global variable 'global3'
+// CHECK-FIXES: {{^}}int g_global3;{{$}}
+#define ADD_TO_SELF(m) (m) + (m)
+int g_twice_global3 = ADD_TO_SELF(global3);
+// CHECK-FIXES: {{^}}int g_twice_global3 = ADD_TO_SELF(g_global3);{{$}}
enum my_enumeration {
// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: invalid case style for enum 'my_enumeration'
Index: clang-tidy/readability/IdentifierNamingCheck.cpp
===
--- clang-tidy/readability/IdentifierNamingCheck.cpp
+++ clang-tidy/readability/IdentifierNamingCheck.cpp
@@ -612,27 +612,57 @@
static void addUsage(IdentifierNamingCheck::NamingCheckFailureMap &Failures,
const IdentifierNamingCheck::NamingCheckId &Decl,
- SourceRange Range) {
+ SourceRange Range, SourceManager *SourceMgr = nullptr) {
// Do nothing if the provided range is invalid.
if (Range.getBegin().isInvalid() || Range.getEnd().isInvalid())
return;
+ // If we have a source manager, use it to convert to the spelling location for
+ // performing the fix. This is necessary because macros can map the same
+ // spelling location to different source locations, and we only want to fix
+ // the token once, before it is expanded by the macro.
+ SourceLocation FixLocation = Range.getBegin();
+ if (SourceMgr)
+FixLocation = SourceMgr->getSpellingLoc(FixLocation);
+ if (FixLocation.isInvalid())
+return;
+
// Try to insert the identifier location in the Usages map, and bail out if it
// is already in there
auto &Failure = Failures[Decl];
- if (!Failure.RawUsageLocs.insert(Range.getBegin().getRawEncoding()).second)
+ if (!Failure.RawUsageLocs.insert(FixLocation.getRawEncoding()).second)
+return;
+
+ if (!Failure.ShouldFix)
return;
- Failure.ShouldFix = Failure.ShouldFix && !Range.getBegin().isMacroID() &&
- !Range.getEnd().isMacroID();
+ // Check if the range is entirely contained within a macro argument.
+ SourceLocation MacroArgExpansionStartForRangeBegin;
+ SourceLocation MacroArgExpansionStartForRangeEnd;
+ bool RangeIsEntirelyWithinMacroArgument =
+ SourceMgr &&
+ SourceMgr->isMacroArgExpansion(Range.getBegin(),
+ &MacroArgExpansionStartForRangeBegin) &&
+ SourceMgr->isMacroArgExpansion(Range.getEnd(),
+ &MacroArgExpansionStartForRangeEnd) &&
+ MacroArgExpansionStartForRangeBegin == MacroArgExpansionStartForRangeEnd;
+
+ // Check if the range contains any locations from a macro expansion.
+ bool RangeContainsMacroExpansion = RangeIsEntirelyWithinMacroArgument ||
+ Range.getBegin().isMacroID() ||
+ Range.getEnd().isMacroID();
+
+ bool RangeCanBeFixed =
+ RangeIsEntirelyWithinMacroArgument || !RangeContainsMacroExpansion;
+ Failure.ShouldFix = RangeCanBeFixed;
}
/// Convenience method when the usage to be added is a NamedDecl
static void addUsage(IdentifierNamingCheck::NamingCheckFailureMap &Failures,
- const NamedDecl *Decl, SourceRange Range) {
+
[PATCH] D25450: [clang-tidy] Fix identifier naming in macro args.
jhen marked an inline comment as done. jhen added inline comments. Comment at: clang-tidy/readability/IdentifierNamingCheck.cpp:654 + RangeIsEntirelyWithinMacroArgument || !RangeContainsMacroExpansion; + Failure.ShouldFix = Failure.ShouldFix && RangeCanBeFixed; } aaron.ballman wrote: > We could do an early return if `ShouldFix` is already false and simplify this > expression with `ShouldFix = RangeCanBeFixed;`, can't we? Thanks for pointing that out. The early exit is much more efficient. https://reviews.llvm.org/D25450 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r284992 - [clang-tidy] Fix identifier naming in macro args.
Author: jhen
Date: Mon Oct 24 12:20:32 2016
New Revision: 284992
URL: http://llvm.org/viewvc/llvm-project?rev=284992&view=rev
Log:
[clang-tidy] Fix identifier naming in macro args.
Summary:
clang-tidy should fix identifier naming even when the identifier is
referenced inside a macro expansion, provided that the identifier enters
the macro expansion completely within a macro argument.
For example, this will allow fixes to the naming of the identifier
'global' when it is declared and used as follows:
int global;
#define USE_IN_MACRO(m) auto use_##m = m
USE_IN_MACRO(global);
Reviewers: alexfh
Subscribers: jlebar, cfe-commits
Differential Revision: https://reviews.llvm.org/D25450
Modified:
clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp
clang-tools-extra/trunk/test/clang-tidy/readability-identifier-naming.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp?rev=284992&r1=284991&r2=284992&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp
(original)
+++ clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp
Mon Oct 24 12:20:32 2016
@@ -612,27 +612,57 @@ static StyleKind findStyleKind(
static void addUsage(IdentifierNamingCheck::NamingCheckFailureMap &Failures,
const IdentifierNamingCheck::NamingCheckId &Decl,
- SourceRange Range) {
+ SourceRange Range, SourceManager *SourceMgr = nullptr) {
// Do nothing if the provided range is invalid.
if (Range.getBegin().isInvalid() || Range.getEnd().isInvalid())
return;
+ // If we have a source manager, use it to convert to the spelling location
for
+ // performing the fix. This is necessary because macros can map the same
+ // spelling location to different source locations, and we only want to fix
+ // the token once, before it is expanded by the macro.
+ SourceLocation FixLocation = Range.getBegin();
+ if (SourceMgr)
+FixLocation = SourceMgr->getSpellingLoc(FixLocation);
+ if (FixLocation.isInvalid())
+return;
+
// Try to insert the identifier location in the Usages map, and bail out if
it
// is already in there
auto &Failure = Failures[Decl];
- if (!Failure.RawUsageLocs.insert(Range.getBegin().getRawEncoding()).second)
+ if (!Failure.RawUsageLocs.insert(FixLocation.getRawEncoding()).second)
+return;
+
+ if (!Failure.ShouldFix)
return;
- Failure.ShouldFix = Failure.ShouldFix && !Range.getBegin().isMacroID() &&
- !Range.getEnd().isMacroID();
+ // Check if the range is entirely contained within a macro argument.
+ SourceLocation MacroArgExpansionStartForRangeBegin;
+ SourceLocation MacroArgExpansionStartForRangeEnd;
+ bool RangeIsEntirelyWithinMacroArgument =
+ SourceMgr &&
+ SourceMgr->isMacroArgExpansion(Range.getBegin(),
+ &MacroArgExpansionStartForRangeBegin) &&
+ SourceMgr->isMacroArgExpansion(Range.getEnd(),
+ &MacroArgExpansionStartForRangeEnd) &&
+ MacroArgExpansionStartForRangeBegin == MacroArgExpansionStartForRangeEnd;
+
+ // Check if the range contains any locations from a macro expansion.
+ bool RangeContainsMacroExpansion = RangeIsEntirelyWithinMacroArgument ||
+ Range.getBegin().isMacroID() ||
+ Range.getEnd().isMacroID();
+
+ bool RangeCanBeFixed =
+ RangeIsEntirelyWithinMacroArgument || !RangeContainsMacroExpansion;
+ Failure.ShouldFix = RangeCanBeFixed;
}
/// Convenience method when the usage to be added is a NamedDecl
static void addUsage(IdentifierNamingCheck::NamingCheckFailureMap &Failures,
- const NamedDecl *Decl, SourceRange Range) {
+ const NamedDecl *Decl, SourceRange Range, SourceManager
*SourceMgr = nullptr) {
return addUsage(Failures, IdentifierNamingCheck::NamingCheckId(
Decl->getLocation(), Decl->getNameAsString()),
- Range);
+ Range, SourceMgr);
}
void IdentifierNamingCheck::check(const MatchFinder::MatchResult &Result) {
@@ -719,7 +749,7 @@ void IdentifierNamingCheck::check(const
if (const auto *DeclRef = Result.Nodes.getNodeAs("declRef")) {
SourceRange Range = DeclRef->getNameInfo().getSourceRange();
-addUsage(NamingCheckFailures, DeclRef->getDecl(), Range);
+addUsage(NamingCheckFailures, DeclRef->getDecl(), Range,
Result.SourceManager);
return;
}
Modified:
clang-tools-extra/trunk/test/clang-tidy/readability-identifier-naming.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/readability-identifier-naming.cpp?rev=284992&
[PATCH] D25450: [clang-tidy] Fix identifier naming in macro args.
jhen added a comment. Thanks for the review! Repository: rL LLVM https://reviews.llvm.org/D25450 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D25450: [clang-tidy] Fix identifier naming in macro args.
This revision was automatically updated to reflect the committed changes.
jhen marked an inline comment as done.
Closed by commit rL284992: [clang-tidy] Fix identifier naming in macro args.
(authored by jhen).
Changed prior to commit:
https://reviews.llvm.org/D25450?vs=75430&id=75610#toc
Repository:
rL LLVM
https://reviews.llvm.org/D25450
Files:
clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp
clang-tools-extra/trunk/test/clang-tidy/readability-identifier-naming.cpp
Index: clang-tools-extra/trunk/test/clang-tidy/readability-identifier-naming.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/readability-identifier-naming.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/readability-identifier-naming.cpp
@@ -95,14 +95,41 @@
USER_MACRO(var2);
// NO warnings or fixes expected as var2 is declared in a macro expansion
+#define BLA int FOO_bar
+BLA;
+// NO warnings or fixes expected as FOO_bar is from macro expansion
+
+int global0;
+#define USE_NUMBERED_GLOBAL(number) auto use_global##number = global##number
+USE_NUMBERED_GLOBAL(0);
+// NO warnings or fixes expected as global0 is pieced together in a macro
+// expansion.
+
+int global1;
+#define USE_NUMBERED_BAL(prefix, number) \
+ auto use_##prefix##bal##number = prefix##bal##number
+USE_NUMBERED_BAL(glo, 1);
+// NO warnings or fixes expected as global1 is pieced together in a macro
+// expansion.
+
+int global2;
+#define USE_RECONSTRUCTED(glo, bal) auto use_##glo##bal = glo##bal
+USE_RECONSTRUCTED(glo, bal2);
+// NO warnings or fixes expected as global2 is pieced together in a macro
+// expansion.
+
int global;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: invalid case style for global variable 'global'
+// CHECK-FIXES: {{^}}int g_global;{{$}}
#define USE_IN_MACRO(m) auto use_##m = m
USE_IN_MACRO(global);
-// NO warnings or fixes expected as global is used in a macro expansion
-#define BLA int FOO_bar
-BLA;
-// NO warnings or fixes expected as FOO_bar is from macro expansion
+int global3;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: invalid case style for global variable 'global3'
+// CHECK-FIXES: {{^}}int g_global3;{{$}}
+#define ADD_TO_SELF(m) (m) + (m)
+int g_twice_global3 = ADD_TO_SELF(global3);
+// CHECK-FIXES: {{^}}int g_twice_global3 = ADD_TO_SELF(g_global3);{{$}}
enum my_enumeration {
// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: invalid case style for enum 'my_enumeration'
Index: clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp
@@ -612,27 +612,57 @@
static void addUsage(IdentifierNamingCheck::NamingCheckFailureMap &Failures,
const IdentifierNamingCheck::NamingCheckId &Decl,
- SourceRange Range) {
+ SourceRange Range, SourceManager *SourceMgr = nullptr) {
// Do nothing if the provided range is invalid.
if (Range.getBegin().isInvalid() || Range.getEnd().isInvalid())
return;
+ // If we have a source manager, use it to convert to the spelling location for
+ // performing the fix. This is necessary because macros can map the same
+ // spelling location to different source locations, and we only want to fix
+ // the token once, before it is expanded by the macro.
+ SourceLocation FixLocation = Range.getBegin();
+ if (SourceMgr)
+FixLocation = SourceMgr->getSpellingLoc(FixLocation);
+ if (FixLocation.isInvalid())
+return;
+
// Try to insert the identifier location in the Usages map, and bail out if it
// is already in there
auto &Failure = Failures[Decl];
- if (!Failure.RawUsageLocs.insert(Range.getBegin().getRawEncoding()).second)
+ if (!Failure.RawUsageLocs.insert(FixLocation.getRawEncoding()).second)
+return;
+
+ if (!Failure.ShouldFix)
return;
- Failure.ShouldFix = Failure.ShouldFix && !Range.getBegin().isMacroID() &&
- !Range.getEnd().isMacroID();
+ // Check if the range is entirely contained within a macro argument.
+ SourceLocation MacroArgExpansionStartForRangeBegin;
+ SourceLocation MacroArgExpansionStartForRangeEnd;
+ bool RangeIsEntirelyWithinMacroArgument =
+ SourceMgr &&
+ SourceMgr->isMacroArgExpansion(Range.getBegin(),
+ &MacroArgExpansionStartForRangeBegin) &&
+ SourceMgr->isMacroArgExpansion(Range.getEnd(),
+ &MacroArgExpansionStartForRangeEnd) &&
+ MacroArgExpansionStartForRangeBegin == MacroArgExpansionStartForRangeEnd;
+
+ // Check if the range contains any locations from a macro expansion.
+ bool RangeContainsMacroExpansion = RangeIsEntirelyWithinMacroArgument ||
+ Range.getBegin().isMacroID() ||
+
[PATCH] D15858: Warn undeclared identifiers in CUDA kernel calls
jhen created this revision.
jhen added reviewers: tra, jlebar.
jhen added a subscriber: cfe-commits.
Value, type, and instantiation dependence were not being handled
correctly for CUDAKernelCallExpr AST nodes. As a result, if an undeclared
identifier was used in the triple-angle-bracket kernel call configuration,
there would be no error during parsing, and there would be a crash during code
gen. This patch makes sure that an error will be issued during parsing in this
case, just as there would be for any other use of an undeclared identifier in
C++.
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,15 @@
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);
+setValueDependent(isValueDependent() || E->isValueDependent());
+setTypeDependent(isTypeDependent() || E->isTypeDependent());
+setInstantiationDependent(isInstantiationDependent() ||
+ E->isInstantiationDependent());
+setContainsUnexpandedParameterPack(containsUnexpandedParameterPack() ||
+ E->containsUnexpandedParameterPack());
+ }
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,15 @@
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);
+setValueDependent(isValueDependent() || E->isValueDependent());
+setTypeDependent(isTypeDependent() || E->isTypeDependent());
+setInstantiationDependent(isInstantiationDependent() ||
+ E->isInstantiationDependent());
+setContainsUnexpandedParameterPack(containsUnexpandedParameterPack() ||
+ E->containsUnexpandedParameterPack());
+ }
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 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
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
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
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
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
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] D15534: [CUDA] renamed cuda_runtime.h wrapper to __clang_cuda_runtime_wrapper.h
jhen added a subscriber: jhen. Comment at: lib/Headers/__clang_cuda_runtime_wrapper.h:95 @@ -78,3 +94,3 @@ #define __CUDACC__ #include_next "cuda_runtime.h" Now that the name of this header has been changed, would it be appropriate to change this #include_next to a simple #include? http://reviews.llvm.org/D15534 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
