[PATCH] D133801: Extraction of a matcher for an unused value from an expression from the bugprone-unused-return-value check

[Bartłomiej Cieślar via Phabricator via cfe-commits]

barcisz added inline comments.



Comment at: clang-tools-extra/unittests/clang-tidy/MatchersTest.cpp:2
+#include "utils/Matchers.h"
+#include "../../clang/unittests/ASTMatchers/ASTMatchersTest.h"
+

njames93 wrote:
> kwk wrote:
> > njames93 wrote:
> > > @kwk I seem to recall you saying in another patch that this include will 
> > > break standalone builds, would stripping the leading `../../` work?
> > > @kwk I seem to recall you saying in another patch that this include will 
> > > break standalone builds, 
> > 
> > @njames93 that's correct. But I don't recall the patch ID :/
> > 
> > > would stripping the leading `../../` work?
> > 
> > I don't know if the file is found then but it would make standalone builds 
> > certainly easier.
> > 
> > I'm not sure if I calculate correctly but given:
> > 
> > `clang-tools-extra/unittests/clang-tidy/MatchersTest.cpp`
> > 
> > then `../../` means `clang-tools-extra/`, no? If so, that directory doesn't 
> > contain a `clang` directory here. It seems as if the `../../` is relative 
> > to some other directory but not this file. Or am I too tired to get it?
> > 
> I'll test when I get a few cycles. If not, may need to copy some of the 
> testing logic for this.
Yes, the tests do indeed work but it's probably because it's getting the 
include path from another directory. I was actually thinking of adding the 
directory for clang as an include path within cmake but thought this would be 
better. There is also an option of copying the testing setup from clang and 
pasting it here, or to just create testing checks and use the existing check 
testing utilities to run the matchers inside of those checks. What do you 
reckon is the best solution in that case to the problem of standalone builds?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133801/new/

https://reviews.llvm.org/D133801

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133942: Clang tidy utility to generate a fix hint for a subsequent expression to the existing one

[Bartłomiej Cieślar via Phabricator via cfe-commits]

barcisz updated this revision to Diff 461955.
barcisz added a comment.

Tests for the utility


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133942/new/

https://reviews.llvm.org/D133942

Files:
  clang-tools-extra/clang-tidy/utils/FixItHintUtils.cpp
  clang-tools-extra/clang-tidy/utils/FixItHintUtils.h
  clang-tools-extra/unittests/clang-tidy/CMakeLists.txt
  clang-tools-extra/unittests/clang-tidy/FixItHintUtilsTest.cpp

Index: clang-tools-extra/unittests/clang-tidy/FixItHintUtilsTest.cpp
===
--- /dev/null
+++ clang-tools-extra/unittests/clang-tidy/FixItHintUtilsTest.cpp
@@ -0,0 +1,101 @@
+#include "utils/FixItHintUtils.h"
+#include "ClangTidyCheck.h"
+#include "ClangTidyTest.h"
+#include "utils/LexerUtils.h"
+#include "gtest/gtest.h"
+
+#define REGISTER_TEST_MATCHER(TestSuiteName, MatcherType)  \
+  class TestSuiteName : public ClangTidyCheck {\
+  public:  \
+TestSuiteName(llvm::StringRef Name,\
+  clang::tidy::ClangTidyContext *Context)  \
+: ClangTidyCheck(Name, Context) {} \
+void registerMatchers(clang::ast_matchers::MatchFinder *Finder) override { \
+  Finder->addMatcher(getMatcher(), this);  \
+}  \
+void check(\
+const clang::ast_matchers::MatchFinder::MatchResult ) override; \
+   \
+  private: \
+clang::ast_matchers::internal::Matcher getMatcher();  \
+  };   \
+  clang::ast_matchers::internal::Matcher  \
+  TestSuiteName::getMatcher()
+
+#define CHECK_TEST_MATCHER(TestSuiteName)  \
+  void TestSuiteName::check(   \
+  const clang::ast_matchers::MatchFinder::MatchResult )
+
+#define RUN_TEST_MATCHER(TestSuiteName, TestName, SourceCode, TargetCode)  \
+  TEST(TestSuiteName, TestName) {  \
+EXPECT_EQ(TargetCode, runCheckOnCode(SourceCode));  \
+  }
+
+namespace clang {
+namespace tidy {
+namespace test {
+
+using namespace ast_matchers;
+
+REGISTER_TEST_MATCHER(AddSubsequentStatementUtil, Stmt) {
+  return stmt(forEach(callExpr().bind("call"))).bind("parent");
+}
+
+CHECK_TEST_MATCHER(AddSubsequentStatementUtil) {
+  const Stmt *const Node = Result.Nodes.getNodeAs("call");
+  const Stmt *const ParentNode = Result.Nodes.getNodeAs("parent");
+  EXPECT_TRUE(Node);
+  EXPECT_TRUE(ParentNode);
+  const auto NodeTerminator = utils::lexer::findNextTerminator(
+  Node->getEndLoc(), Result.Context->getSourceManager(),
+  Result.Context->getLangOpts());
+  auto Range = SourceRange(Node->getBeginLoc(), NodeTerminator);
+  diag(Node->getBeginLoc(), "") << utils::fixit::addSubsequentStatement(
+  Range, *ParentNode, "foo()", *Result.Context);
+}
+
+RUN_TEST_MATCHER(AddSubsequentStatementUtil, CompoundStatementParent,
+ "void foo() {\n  foo();\n}",
+ "void foo() {\n  foo();\n  foo();\n}")
+
+RUN_TEST_MATCHER(AddSubsequentStatementUtil,
+ CompoundStatementParentWithComments,
+ "void foo() {\n \tfoo(); //some /* comments */\n}",
+ "void foo() {\n \tfoo(); //some /* comments */\n \tfoo();\n}")
+
+RUN_TEST_MATCHER(AddSubsequentStatementUtil, IfStatementParent,
+ "void foo() {\n  if(true)\nfoo();\n}",
+ "void foo() {\n  if(true) {\nfoo();\nfoo();\n  }\n}")
+
+RUN_TEST_MATCHER(
+AddSubsequentStatementUtil, IfStatementParentWithComments,
+"void foo() {\n  if(true)\nfoo(); //some /* comments */\n}",
+"void foo() {\n  if(true) {\nfoo(); //some /* comments */\n"
+"foo();\n  }\n}")
+
+RUN_TEST_MATCHER(
+AddSubsequentStatementUtil, IfStatementSameLineParent,
+"void foo() {\n  if(true) foo();\n}",
+"void foo() {\n  if(true) { foo();\n foo();\n  }\n}")
+
+RUN_TEST_MATCHER(AddSubsequentStatementUtil,
+ IfStatementSameLineParentWithComments,
+ "void foo() {\n  if(true) foo(); //some /* comments */\n}",
+ "void foo() {\n  if(true) { foo(); //some /* comments */\n"
+ " foo();\n  }\n}")
+
+RUN_TEST_MATCHER(AddSubsequentStatementUtil, IfElseStatementParent,
+ "void foo() {\n  if(true)\nfoo();\n  else\nfoo();\n}",
+   

[PATCH] D133801: Extraction of a matcher for an unused value from an expression from the bugprone-unused-return-value check

[Bartłomiej Cieślar via Phabricator via cfe-commits]

barcisz updated this revision to Diff 461721.
barcisz added a comment.

Tests for the check


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133801/new/

https://reviews.llvm.org/D133801

Files:
  clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
  clang-tools-extra/clang-tidy/utils/Matchers.h
  clang-tools-extra/unittests/clang-tidy/CMakeLists.txt
  clang-tools-extra/unittests/clang-tidy/MatchersTest.cpp

Index: clang-tools-extra/unittests/clang-tidy/MatchersTest.cpp
===
--- /dev/null
+++ clang-tools-extra/unittests/clang-tidy/MatchersTest.cpp
@@ -0,0 +1,37 @@
+#include "utils/Matchers.h"
+#include "../../clang/unittests/ASTMatchers/ASTMatchersTest.h"
+
+namespace clang {
+namespace tidy {
+namespace test {
+
+using namespace ast_matchers;
+
+TEST_P(ASTMatchersTest, isValueUnused) {
+  auto Matcher = matchers::isValueUnused(integerLiteral(equals(42)));
+  std::string CodePrefix = "void bar()";
+
+  matches(CodePrefix + "{42;}", Matcher);
+  matches(CodePrefix + "{int x = ({42; 0;});}", Matcher);
+  matches(CodePrefix + "{if (true) 42;}", Matcher);
+  matches(CodePrefix + "{while(true) 42;}", Matcher);
+  matches(CodePrefix + "{do 42; while(true);}", Matcher);
+  matches(CodePrefix + "{for(;;) 42;}", Matcher);
+  matches(CodePrefix + "{for(42;;) bar();}", Matcher);
+  matches(CodePrefix + "{for(;;42) bar();}", Matcher);
+  matches(CodePrefix + "{int t[] = {1, 2, 3}; for(int x : t) 42;}", Matcher);
+  matches(CodePrefix + "{switch(1) {case 42:}", Matcher);
+
+  notMatches(CodePrefix + "{bar();}", Matcher);
+  notMatches(CodePrefix + "{int x = 42;}", Matcher);
+  notMatches(CodePrefix + "{int x = ({42;});}", Matcher);
+  notMatches(CodePrefix + "{if (42) bar();}", Matcher);
+  notMatches(CodePrefix + "{while(42) bar();}", Matcher);
+  notMatches(CodePrefix + "{do bar(); while(42);}", Matcher);
+  notMatches(CodePrefix + "{for(; 42; )} bar();", Matcher);
+  notMatches(CodePrefix + "switch(1) {default: bar();}", Matcher);
+}
+
+} // namespace test
+} // namespace tidy
+} // namespace clang
Index: clang-tools-extra/unittests/clang-tidy/CMakeLists.txt
===
--- clang-tools-extra/unittests/clang-tidy/CMakeLists.txt
+++ clang-tools-extra/unittests/clang-tidy/CMakeLists.txt
@@ -25,6 +25,7 @@
   GlobListTest.cpp
   GoogleModuleTest.cpp
   LLVMModuleTest.cpp
+  MatchersTest.cpp
   ModernizeModuleTest.cpp
   NamespaceAliaserTest.cpp
   ObjCModuleTest.cpp
@@ -43,6 +44,7 @@
   clangFrontend
   clangLex
   clangSerialization
+  clangTesting
   clangTooling
   clangToolingCore
   clangTransformer
Index: clang-tools-extra/clang-tidy/utils/Matchers.h
===
--- clang-tools-extra/clang-tidy/utils/Matchers.h
+++ clang-tools-extra/clang-tidy/utils/Matchers.h
@@ -49,6 +49,51 @@
   return pointerType(pointee(qualType(isConstQualified(;
 }
 
+// Matches the statements in a GNU statement-expression that are not returned
+// from it.
+AST_MATCHER_P(StmtExpr, hasUnreturning,
+  clang::ast_matchers::internal::Matcher, matcher) {
+  const auto compoundStmt = Node.getSubStmt();
+  assert(compoundStmt);
+
+  clang::ast_matchers::internal::BoundNodesTreeBuilder result;
+  bool matched = false;
+  for (auto stmt = compoundStmt->body_begin();
+   stmt + 1 < compoundStmt->body_end(); ++stmt) {
+clang::ast_matchers::internal::BoundNodesTreeBuilder builderInner(*Builder);
+assert(stmt && *stmt);
+if (matcher.matches(**stmt, Finder, )) {
+  result.addMatch(builderInner);
+  matched = true;
+}
+  }
+  *Builder = result;
+  return matched;
+}
+
+// Matches all of the nodes (simmilar to forEach) that match the matcher
+// and have return values not used in any statement.
+AST_MATCHER_FUNCTION_P(ast_matchers::StatementMatcher, isValueUnused,
+   ast_matchers::StatementMatcher, Matcher) {
+  using namespace ast_matchers;
+  const auto UnusedInCompoundStmt =
+  compoundStmt(forEach(Matcher), unless(hasParent(stmtExpr(;
+  const auto UnusedInGnuExprStmt = stmtExpr(hasUnreturning(Matcher));
+  const auto UnusedInIfStmt =
+  ifStmt(eachOf(hasThen(Matcher), hasElse(Matcher)));
+  const auto UnusedInWhileStmt = whileStmt(hasBody(Matcher));
+  const auto UnusedInDoStmt = doStmt(hasBody(Matcher));
+  const auto UnusedInForStmt = forStmt(
+  eachOf(hasLoopInit(Matcher), hasIncrement(Matcher), hasBody(Matcher)));
+  const auto UnusedInRangeForStmt = cxxForRangeStmt(hasBody(Matcher));
+  const auto UnusedInCaseStmt = switchCase(forEach(Matcher));
+  const auto Unused =
+  stmt(anyOf(UnusedInCompoundStmt, UnusedInGnuExprStmt, UnusedInIfStmt,
+ UnusedInWhileStmt, UnusedInDoStmt, UnusedInForStmt,
+ UnusedInRangeForStmt, UnusedInCaseStmt));
+  return stmt(eachOf(Unused, forEachDescendant(Unused)));
+}
+
 

[PATCH] D133801: Extraction of a matcher for an unused value from an expression from the bugprone-unused-return-value check

[Bartłomiej Cieślar via Phabricator via cfe-commits]

barcisz added a comment.

@njames93 What would be the best way to write up those tests here? I feel like 
writing a custom check to test the matcher each time we want to test one would 
be quite wasteful, but at the same time what would be the best way to invoke 
clang's frontend without using `tooling::Invocation`?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133801/new/

https://reviews.llvm.org/D133801

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133801: Extraction of a matcher for an unused value from an expression from the bugprone-unused-return-value check

[Bartłomiej Cieślar via Phabricator via cfe-commits]

barcisz updated this revision to Diff 461562.
barcisz added a comment.

Removed unused matcher definitions from bugprone-unused-return-value


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133801/new/

https://reviews.llvm.org/D133801

Files:
  clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
  clang-tools-extra/clang-tidy/utils/Matchers.h


Index: clang-tools-extra/clang-tidy/utils/Matchers.h
===
--- clang-tools-extra/clang-tidy/utils/Matchers.h
+++ clang-tools-extra/clang-tidy/utils/Matchers.h
@@ -49,6 +49,51 @@
   return pointerType(pointee(qualType(isConstQualified(;
 }
 
+// Matches the statements in a GNU statement-expression that are not returned
+// from it.
+AST_MATCHER_P(StmtExpr, hasUnreturning,
+  clang::ast_matchers::internal::Matcher, matcher) {
+  const auto compoundStmt = Node.getSubStmt();
+  assert(compoundStmt);
+
+  clang::ast_matchers::internal::BoundNodesTreeBuilder result;
+  bool matched = false;
+  for (auto stmt = compoundStmt->body_begin();
+   stmt + 1 < compoundStmt->body_end(); ++stmt) {
+clang::ast_matchers::internal::BoundNodesTreeBuilder 
builderInner(*Builder);
+assert(stmt && *stmt);
+if (matcher.matches(**stmt, Finder, )) {
+  result.addMatch(builderInner);
+  matched = true;
+}
+  }
+  *Builder = result;
+  return matched;
+}
+
+// Matches all of the nodes (simmilar to forEach) that match the matcher
+// and have return values not used in any statement.
+AST_MATCHER_FUNCTION_P(ast_matchers::StatementMatcher, isValueUnused,
+   ast_matchers::StatementMatcher, Matcher) {
+  using namespace ast_matchers;
+  const auto UnusedInCompoundStmt =
+  compoundStmt(forEach(Matcher), unless(hasParent(stmtExpr(;
+  const auto UnusedInGnuExprStmt = stmtExpr(hasUnreturning(Matcher));
+  const auto UnusedInIfStmt =
+  ifStmt(eachOf(hasThen(Matcher), hasElse(Matcher)));
+  const auto UnusedInWhileStmt = whileStmt(hasBody(Matcher));
+  const auto UnusedInDoStmt = doStmt(hasBody(Matcher));
+  const auto UnusedInForStmt = forStmt(
+  eachOf(hasLoopInit(Matcher), hasIncrement(Matcher), hasBody(Matcher)));
+  const auto UnusedInRangeForStmt = cxxForRangeStmt(hasBody(Matcher));
+  const auto UnusedInCaseStmt = switchCase(forEach(Matcher));
+  const auto Unused =
+  stmt(anyOf(UnusedInCompoundStmt, UnusedInGnuExprStmt, UnusedInIfStmt,
+ UnusedInWhileStmt, UnusedInDoStmt, UnusedInForStmt,
+ UnusedInRangeForStmt, UnusedInCaseStmt));
+  return stmt(eachOf(Unused, forEachDescendant(Unused)));
+}
+
 // A matcher implementation that matches a list of type name regular 
expressions
 // against a NamedDecl. If a regular expression contains the substring "::"
 // matching will occur against the qualified name, otherwise only the typename.
Index: clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
@@ -7,6 +7,7 @@
 
//===--===//
 
 #include "UnusedReturnValueCheck.h"
+#include "../utils/Matchers.h"
 #include "../utils/OptionsUtils.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
@@ -140,29 +141,8 @@
unless(returns(voidType())),
isInstantiatedFrom(hasAnyName(FunVec)
   .bind("match";
-
-  auto UnusedInCompoundStmt =
-  compoundStmt(forEach(MatchedCallExpr),
-   // The checker can't currently differentiate between the
-   // return statement and other statements inside GNU 
statement
-   // expressions, so disable the checker inside them to avoid
-   // false positives.
-   unless(hasParent(stmtExpr(;
-  auto UnusedInIfStmt =
-  ifStmt(eachOf(hasThen(MatchedCallExpr), hasElse(MatchedCallExpr)));
-  auto UnusedInWhileStmt = whileStmt(hasBody(MatchedCallExpr));
-  auto UnusedInDoStmt = doStmt(hasBody(MatchedCallExpr));
-  auto UnusedInForStmt =
-  forStmt(eachOf(hasLoopInit(MatchedCallExpr),
- hasIncrement(MatchedCallExpr), hasBody(MatchedCallExpr)));
-  auto UnusedInRangeForStmt = cxxForRangeStmt(hasBody(MatchedCallExpr));
-  auto UnusedInCaseStmt = switchCase(forEach(MatchedCallExpr));
-
   Finder->addMatcher(
-  stmt(anyOf(UnusedInCompoundStmt, UnusedInIfStmt, UnusedInWhileStmt,
- UnusedInDoStmt, UnusedInForStmt, UnusedInRangeForStmt,
- UnusedInCaseStmt)),
-  this);
+  functionDecl(hasBody(matchers::isValueUnused(MatchedCallExpr))), this);
 }
 
 void UnusedReturnValueCheck::check(const MatchFinder::MatchResult ) {


Index: 

[PATCH] D133436: Ground work for cuda-related checks in clang-tidy

[Bartłomiej Cieślar via Phabricator via cfe-commits]

barcisz updated this revision to Diff 461561.
barcisz added a comment.

Inlined definition of size_t


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133436/new/

https://reviews.llvm.org/D133436

Files:
  clang-tools-extra/clang-tidy/CMakeLists.txt
  clang-tools-extra/clang-tidy/ClangTidyForceLinker.h
  clang-tools-extra/clang-tidy/cuda/CMakeLists.txt
  clang-tools-extra/clang-tidy/cuda/CudaTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/index.rst
  clang-tools-extra/test/clang-tidy/check_clang_tidy.py
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda.h

Index: clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda.h
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda.h
@@ -0,0 +1,28 @@
+/* Minimal declarations for CUDA support.  Testing purposes only. */
+
+using size_t = decltype(sizeof(0));
+
+#define __constant__ __attribute__((constant))
+#define __device__ __attribute__((device))
+#define __global__ __attribute__((global))
+#define __host__ __attribute__((host))
+#define __shared__ __attribute__((shared))
+
+struct dim3 {
+  unsigned x, y, z;
+  __host__ __device__ dim3(unsigned x, unsigned y = 1, unsigned z = 1) : x(x), y(y), z(z) {}
+};
+
+typedef struct cudaStream *cudaStream_t;
+typedef enum cudaError {} cudaError_t;
+extern "C" int cudaConfigureCall(dim3 gridSize, dim3 blockSize,
+ size_t sharedSize = 0,
+ cudaStream_t stream = 0);
+extern "C" int __cudaPushCallConfiguration(dim3 gridSize, dim3 blockSize,
+   size_t sharedSize = 0,
+   cudaStream_t stream = 0);
+extern "C" cudaError_t cudaLaunchKernel(const void *func, dim3 gridDim,
+dim3 blockDim, void **args,
+size_t sharedMem, cudaStream_t stream);
+
+extern "C" __device__ int printf(const char*, ...);
Index: clang-tools-extra/test/clang-tidy/check_clang_tidy.py
===
--- clang-tools-extra/test/clang-tidy/check_clang_tidy.py
+++ clang-tools-extra/test/clang-tidy/check_clang_tidy.py
@@ -93,7 +93,7 @@
 
 file_name_with_extension = self.assume_file_name or self.input_file_name
 _, extension = os.path.splitext(file_name_with_extension)
-if extension not in ['.c', '.hpp', '.m', '.mm']:
+if extension not in ['.c', '.cu', '.hpp', '.m', '.mm']:
   extension = '.cpp'
 self.temp_file_name = self.temp_file_name + extension
 
@@ -115,9 +115,14 @@
   self.clang_extra_args = ['-fobjc-abi-version=2', '-fobjc-arc', '-fblocks'] + \
   self.clang_extra_args
 
-if extension in ['.cpp', '.hpp', '.mm']:
+if extension in ['.cpp', '.cu', '.hpp', '.mm']:
   self.clang_extra_args.append('-std=' + self.std)
 
+# Tests should not rely on a certain cuda headers and library version
+# being available on the machine
+if extension == '.cu':
+  self.clang_extra_args.extend(["--no-cuda-version-check", "-nocudalib", "-nocudainc"])
+
 # Tests should not rely on STL being available, and instead provide mock
 # implementations of relevant APIs.
 self.clang_extra_args.append('-nostdinc++')
Index: clang-tools-extra/docs/clang-tidy/index.rst
===
--- clang-tools-extra/docs/clang-tidy/index.rst
+++ clang-tools-extra/docs/clang-tidy/index.rst
@@ -67,6 +67,7 @@
 ``concurrency-``   Checks related to concurrent programming (including
threads, fibers, coroutines, etc.).
 ``cppcoreguidelines-`` Checks related to C++ Core Guidelines.
+``cuda-``  Checks related to CUDA best practices.
 ``darwin-``Checks related to Darwin coding conventions.
 ``fuchsia-``   Checks related to Fuchsia coding conventions.
 ``google-``Checks related to Google coding conventions.
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -16,6 +16,7 @@
clang-analyzer/*
concurrency/*
cppcoreguidelines/*
+   cuda/*
darwin/*
fuchsia/*
google/*
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -96,6 +96,8 @@
 Improvements to clang-tidy
 --
 
+- Introduce the cuda module for checks specific to CUDA code.
+
 New checks
 ^^
 
Index: 

[PATCH] D133436: Ground work for cuda-related checks in clang-tidy

[Bartłomiej Cieślar via Phabricator via cfe-commits]

barcisz added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/stddef.h:12
+
+using size_t = long long unsigned;
+

njames93 wrote:
> If this is all the file is used for, and it's only used for this one test 
> file, can probably remove this header and inline the definition.
> I also feel this definition may be fragile on certain platforms.
I wanted to add it in a separate header for future proofness; as for the 
definition, I cannot really use uint64_t here since I cannot include the real 
stddef.h


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133436/new/

https://reviews.llvm.org/D133436

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133942: Clang tidy utility to generate a fix hint for a subsequent expression to the existing one

[Bartłomiej Cieślar via Phabricator via cfe-commits]

barcisz added a comment.

In D133942#3797449 , @njames93 wrote:

> How does this handle pathological cases like the statement being the 
> iteration-expression of a for loop, or a init-statement in an 
> if/switch/range-for loop. The documentation looks like it tries to explain 
> that, but it doesn't do a great job IMHO.

It doesn't really, it's mostly meant for cases where we know that the statement 
will be in the body and it's only added in the utils because it's needed for 
D133956 , but I can just move it to inside 
the check if you believe that it won't be as useful for the general case. The 
reason I want to preserve the statement to which the comments relate to is 
because if I put the new statement/macro invocation right after the current 
statement then the formatted will move the comments to the new line with the 
new statement instead of keeping them in the same line as the current statement 
(and in case of macros I'm not sure if it will even move it to the next line at 
all)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133942/new/

https://reviews.llvm.org/D133942

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133942: Clang tidy utility to generate a fix hint for a subsequent expression to the existing one

[Bartłomiej Cieślar via Phabricator via cfe-commits]

barcisz added a comment.

In D133942#3797449 , @njames93 wrote:

> It would also be nice to add in some unittests to demonstrate that braces are 
> currently inserted etc.

Right sorry, forgot to port unit tests, will do that swiftly


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133942/new/

https://reviews.llvm.org/D133942

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133956: Cuda Check for ignored errors after calling a CUDA kernel

[Bartłomiej Cieślar via Phabricator via cfe-commits]

barcisz updated this revision to Diff 460743.
barcisz added a comment.
Herald added a subscriber: aheejin.

documentation for the check


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133956/new/

https://reviews.llvm.org/D133956

Files:
  .clang-tidy
  clang-tools-extra/clang-tidy/cuda/CMakeLists.txt
  clang-tools-extra/clang-tidy/cuda/CudaTidyModule.cpp
  clang-tools-extra/clang-tidy/cuda/UnsafeKernelCallCheck.cpp
  clang-tools-extra/clang-tidy/cuda/UnsafeKernelCallCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/cuda/unsafe-kernel-call.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda_runtime.h
  
clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-kernel-call-function-handler.cu
  
clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-kernel-call-macro-handler.cu
  test.cu

Index: test.cu
===
--- /dev/null
+++ test.cu
@@ -0,0 +1,8 @@
+#include "clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda_runtime.h"
+
+__global__
+void kernel();
+
+void foo() {
+  kernel<<<64, 128>>>();
+}
Index: clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-kernel-call-macro-handler.cu
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-kernel-call-macro-handler.cu
@@ -0,0 +1,116 @@
+// RUN: %check_clang_tidy %s cuda-unsafe-kernel-call %t -- \
+// RUN:   -config="{CheckOptions: \
+// RUN: [{key: cuda-unsafe-kernel-call.HandlerName, \
+// RUN:   value: 'CUDA_CHECK_KERNEL'}, \
+// RUN:   {key: cuda-unsafe-kernel-call.AcceptedHandlers, \
+// RUN:value: 'ALTERNATIVE_CUDA_CHECK_KERNEL, cudaCheckKernel, \
+// RUN:alternative::alternativeCudaCheckKernel, \
+// RUN:otherAlternativeCudaCheckKernel'}] \
+// RUN: }" \
+// RUN:   -- -isystem %clang_tidy_headers
+
+#include 
+
+#define CUDA_CHECK_KERNEL() do {} while(0)
+
+#define ALTERNATIVE_CUDA_CHECK_KERNEL() CUDA_CHECK_KERNEL()
+
+void cudaCheckKernel();
+
+namespace alternative {
+
+void alternativeCudaCheckKernel();
+void otherAlternativeCudaCheckKernel();
+
+}
+
+__global__
+void b();
+
+#define KERNEL_CALL() do {b<<<1, 2>>>();} while(0)
+
+void errorCheck() {
+  auto err = cudaGetLastError();
+}
+
+void bad() {
+  b<<<1, 2>>>(); // sample comment
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Possible unchecked error after a kernel launch.
+
+  KERNEL_CALL(); // sample comment
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Possible unchecked error after a kernel launch.
+  // There isn't supposed to be a fix here since it's a macro call
+
+  if(true)
+b<<<1, 2>>>()  ; // Brackets omitted purposefully, since they create an additional AST node
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Possible unchecked error after a kernel launch.
+  else {
+b<<<1, 2>>>();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Possible unchecked error after a kernel launch.
+b<<<1, 2>>>();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Possible unchecked error after a kernel launch.
+  }
+  auto err = cudaGetLastError();
+
+  b<<<1, 2>>>();
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Possible unchecked error after a kernel launch.
+  if (true)
+cudaGetLastError();
+
+  b<<<1, 2>>>();
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Possible unchecked error after a kernel launch.
+  for(;;)
+auto err2 = cudaGetLastError(); // Brackets omitted purposefully, since they create an additional AST node
+
+  b<<<1, 2>>>();
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Possible unchecked error after a kernel launch.
+  auto err3 = true ? 1 : cudaGetLastError();
+
+  b<<<1, 2>>>();
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Possible unchecked error after a kernel launch.
+  auto err4 = cudaDeviceReset() + cudaGetLastError();
+
+  b<<<1, 2>>>();
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Possible unchecked error after a kernel launch.
+  // Calling an error-checking function after a kernel is not considered safe.
+  errorCheck();
+}
+
+void good() {
+  b<<<1, 2>>>();; /* The semicolons are here because the
+detection of the macro is done with a lexer */ ;
+  CUDA_CHECK_KERNEL();
+
+  b<<<1, 2>>>();
+  ALTERNATIVE_CUDA_CHECK_KERNEL();
+
+  b<<<1, 2>>>();
+  alternative::alternativeCudaCheckKernel();
+
+  b<<<1, 2>>>();
+  alternative::otherAlternativeCudaCheckKernel();
+
+  b<<<1, 2>>>();
+  switch(1 + cudaGetLastError()) {
+default:;
+  }
+
+  b<<<1, 2>>>();
+  if(3 < cudaGetLastError()) {
+1;
+  } else {
+2;
+  }
+
+  b<<<1, 2>>>();
+  for(int i = cudaGetLastError();;);
+
+  b<<<1, 2>>>();
+  do {
+  do {
+  do {
+

[PATCH] D133956: Cuda Check for ignored errors after calling a CUDA kernel

[Bartłomiej Cieślar via Phabricator via cfe-commits]

barcisz added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-kernel-call-macro-handler.cu:53
+  }
+  auto err = cudaGetLastError();
+

tra wrote:
> barcisz wrote:
> > tra wrote:
> > > Just curious -- is it sufficient to just call `cudaGetLastError();` ? Or 
> > > does the checker require using its result, too? I.e. in practice this 
> > > particular check will not really do anything useful. The tests below look 
> > > somewhat inconsistent. 
> > Technically it does not require the user to actually use the value of 
> > `cudaGetLastError()`, but
> > 
> > 
> >   # If they are calling it then they most likely did not place this call 
> > there randomly and are using it to check for the error returned by the 
> > kernel
> >   # the check being introduced in D133804 can be used to check if the 
> > return value has been used, so checking it here as well would have been a 
> > duplication
> > 
> > 
> > 1. If they are calling it then they most likely did not place this call 
> > there randomly and are using it to check for the error returned by the 
> > kernel
> 
> If that's the case, then why kernel launches on lines 45 and 51 are reported 
> as possibly unchecked? Both are followed by the `cudaGetLastError()` call and 
> are, technically checked, if we're not analyzing the usage of the result of 
> the call. 
> 
> What am I missing?
The idea is that the call should happen directly after the kernel without any 
branching (because branching can often make things much harder to understand in 
case of things like for loop make the error not actually have 
`cudaGetLastError()` called after every kernel call



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-kernel-call-macro-handler.cu:75
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Possible unchecked 
error after a kernel launch.
+  // Calling an error-checking function after a kernel is not considered safe.
+  errorCheck();

tra wrote:
> barcisz wrote:
> > tra wrote:
> > > WDYM by "is not considered safe" here? How is that different from calling 
> > > `cudaGetLastError()` and checking its value?
> > As in the check does not do inter-procedural analysis short of adding the 
> > handler to AcceptedHandlers, so the check will flag such occurences
> Hmm.. Using a helper function to check for cuda errors is a fairly common 
> pattern. 
> Is there a way to annotate such a helper function as `checks 
> cudaGetLastError`?
There would be an easy way to do that, but it's much more common for projects 
to have those helper functions project-wide (or at least sub-project wide) 
which means they can be just specified explicitly for the project in the 
options for the check (the official documentation for the check will be 
uploaded tomorrow)



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-kernel-call-macro-handler.cu:80-82
+  b<<<1, 2>>>();; /* The semicolons are here because the
+detection of the macro is done with a lexer */ ;
+  CUDA_CHECK_KERNEL();

tra wrote:
> barcisz wrote:
> > tra wrote:
> > > What would happen with a single `;` as would be seen in the normal user 
> > > code?
> > Nothing, it would work just fine; it's rather that all other kernel calls 
> > in this test use a single `;` so I want to check this case here
> I still do not understand how it all fits together. What does a kernel call, 
> the extra `;`, the macro, and the checker code have to do with each other?
> 
> Is the idea that the checker should see though the empty statement between 
> the kernel call and the checker macro?
> If that's the case I'd make it a bit more prominent. E.g. something like this:
> 
> ```
> b<<<1, 2>>>();
> ; /* Make sure that we see through empty expressions in-between the call 
> and the checker. */ ;
>   CUDA_CHECK_KERNEL();
> ```
> 
> 
The reason we're checking for multiple `;`s here is that due to macros not 
being present in the AST they have to be located on the lexer stage, which 
makes it necessary to search for them based on tokens. The tokens used after 
the kernel call here (semicolons and a comment) are the only allowed token 
between the kernel call and the macro, since any other one would indicate 
another statement being present



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-kernel-call-macro-handler.cu:112
+  do {
+auto err2 = cudaGetLastError();
+  } while(0);

tra wrote:
> barcisz wrote:
> > tra wrote:
> > > Why does this case produce no warning, while a very similar case above 
> > > does? In both cases result of `cudaGetLastError()` is assigned to an 
> > > unused variable within the loop body.
> > > 
> > > ```
> > >   b<<<1, 2>>>();
> > >   // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Possible unchecked 
> > > error after a kernel launch.
> > >   for(;;)
> > > auto err2 = 

[PATCH] D133942: Clang tidy utility to generate a fix hint for a subsequent expression to the existing one

[Bartłomiej Cieślar via Phabricator via cfe-commits]

barcisz updated this revision to Diff 460551.
barcisz added a comment.

Misplaced diff


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133942/new/

https://reviews.llvm.org/D133942

Files:
  clang-tools-extra/clang-tidy/utils/FixItHintUtils.cpp
  clang-tools-extra/clang-tidy/utils/FixItHintUtils.h

Index: clang-tools-extra/clang-tidy/utils/FixItHintUtils.h
===
--- clang-tools-extra/clang-tidy/utils/FixItHintUtils.h
+++ clang-tools-extra/clang-tidy/utils/FixItHintUtils.h
@@ -11,7 +11,9 @@
 
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Decl.h"
+#include "clang/Basic/SourceManagerInternals.h"
 #include "clang/Sema/DeclSpec.h"
+#include "clang/Tooling/FixIt.h"
 
 namespace clang {
 namespace tidy {
@@ -46,6 +48,17 @@
   DeclSpec::TQ Qualifier,
   QualifierTarget CT = QualifierTarget::Pointee,
   QualifierPolicy CP = QualifierPolicy::Left);
+
+/// \brief Adds a statement to be executed right after this statement .
+/// Is designed for taking potential comments or statements in the same line
+/// into account. The statement should not be an expression that's part of
+/// another statement. The statement range should include the terminator
+/// (semicolon).
+llvm::SmallVector
+addSubsequentStatement(SourceRange stmtRangeWithTerminator,
+   const Stmt , llvm::StringRef nextStmt,
+   ASTContext );
+
 } // namespace fixit
 } // namespace utils
 } // namespace tidy
Index: clang-tools-extra/clang-tidy/utils/FixItHintUtils.cpp
===
--- clang-tools-extra/clang-tidy/utils/FixItHintUtils.cpp
+++ clang-tools-extra/clang-tidy/utils/FixItHintUtils.cpp
@@ -223,6 +223,154 @@
 
   return None;
 }
+
+static unsigned int getLineNumber(SourceLocation Loc, SourceManager& SM) {
+  FileID FID;
+  unsigned int Offset;
+  std::tie(FID, Offset) = SM.getDecomposedLoc(Loc);
+  return SM.getLineNumber(FID, Offset);
+}
+
+static std::string getIndent(SourceLocation sLoc, ASTContext& context) {
+  auto& SM = context.getSourceManager();
+
+  const auto sLocLineNo = getLineNumber(sLoc, SM);
+
+  auto indentation_template = tooling::fixit::internal::getText(
+  CharSourceRange::getCharRange(SourceRange(
+  SM.translateLineCol(SM.getFileID(sLoc), sLocLineNo, 1), sLoc)),
+  context);
+
+  std::string indentation;
+  indentation.reserve(indentation_template.size());
+  std::transform(
+  indentation_template.begin(),
+  indentation_template.end(),
+  std::back_inserter(indentation),
+  [](char c) { return isspace(c) ? c : ' '; });
+  return indentation;
+}
+
+llvm::SmallVector addSubsequentStatement(
+SourceRange stmtRangeWithTerminator,
+const Stmt& parentStmt,
+llvm::StringRef nextStmt,
+ASTContext& context) {
+  auto& SM = context.getSourceManager();
+  auto langOpts = context.getLangOpts();
+
+  const auto stmtEndLineNo =
+  getLineNumber(stmtRangeWithTerminator.getEnd(), SM);
+
+  // Find the first token's data for which the next token is
+  // either a line apart or is not a comment
+  SourceLocation lastTokenEndLoc =
+  stmtRangeWithTerminator.getEnd().getLocWithOffset(1);
+  auto lastTokenLine = stmtEndLineNo;
+  bool insertNewLine = true;
+  while (true) {
+llvm::Optional tokenOption = Lexer::findNextToken(
+lastTokenEndLoc.getLocWithOffset(-1), SM, langOpts, true);
+if (!tokenOption) {
+  return llvm::SmallVector();
+}
+if (tokenOption->is(tok::eof)) {
+  insertNewLine = false;
+  break;
+}
+const auto tokenBeginLineNo = getLineNumber(tokenOption->getLocation(), SM);
+
+if (tokenOption->isNot(tok::comment)) {
+  insertNewLine = tokenBeginLineNo != stmtEndLineNo;
+  break;
+}
+if (tokenBeginLineNo > lastTokenLine) {
+  break;
+}
+
+lastTokenEndLoc = tokenOption->getEndLoc();
+lastTokenLine = getLineNumber(tokenOption->getEndLoc(), SM);
+  }
+
+  bool isEnclosedWithBrackets =
+  parentStmt.getStmtClass() == Stmt::CompoundStmtClass;
+
+  // Generating the FixItHint
+  // There are 5 scenarios that we have to take into account:
+  // 1. The statement is enclosed in brackets but the next statement is
+  //in the same line - insert the new statement right after the previous one
+  // 2. The statement is not enclosed in brackets and the next statement is
+  //in the same line - same as 1. and enclose both statements in brackets
+  //on the same line
+  // 3. The statement is enclosed in brackets and the next statement is
+  //on subsequent lines - skip all the comments in this line
+  // 4. The statement is not enclosed in brackets but the next statement is on
+  //subsequent lines - same as 3. and enclose the statements with
+  //google-style multiline brackets (opening bracket right after the parent
+ 

[PATCH] D133956: Cuda Check for ignored errors after calling a CUDA kernel

[Bartłomiej Cieślar via Phabricator via cfe-commits]

barcisz updated this revision to Diff 460550.
barcisz added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133956/new/

https://reviews.llvm.org/D133956

Files:
  clang-tools-extra/clang-tidy/cuda/CMakeLists.txt
  clang-tools-extra/clang-tidy/cuda/CudaTidyModule.cpp
  clang-tools-extra/clang-tidy/cuda/UnsafeKernelCallCheck.cpp
  clang-tools-extra/clang-tidy/cuda/UnsafeKernelCallCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/cuda/unsafe-kernel-call.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda_runtime.h
  
clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-kernel-call-function-handler.cu
  
clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-kernel-call-macro-handler.cu

Index: clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-kernel-call-macro-handler.cu
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-kernel-call-macro-handler.cu
@@ -0,0 +1,116 @@
+// RUN: %check_clang_tidy %s cuda-unsafe-kernel-call %t -- \
+// RUN:   -config="{CheckOptions: \
+// RUN: [{key: cuda-unsafe-kernel-call.HandlerName, \
+// RUN:   value: 'CUDA_CHECK_KERNEL'}, \
+// RUN:   {key: cuda-unsafe-kernel-call.AcceptedHandlers, \
+// RUN:value: 'ALTERNATIVE_CUDA_CHECK_KERNEL, cudaCheckKernel, \
+// RUN:alternative::alternativeCudaCheckKernel, \
+// RUN:otherAlternativeCudaCheckKernel'}] \
+// RUN: }" \
+// RUN:   -- -isystem %clang_tidy_headers
+
+#include 
+
+#define CUDA_CHECK_KERNEL() do {} while(0)
+
+#define ALTERNATIVE_CUDA_CHECK_KERNEL() CUDA_CHECK_KERNEL()
+
+void cudaCheckKernel();
+
+namespace alternative {
+
+void alternativeCudaCheckKernel();
+void otherAlternativeCudaCheckKernel();
+
+}
+
+__global__
+void b();
+
+#define KERNEL_CALL() do {b<<<1, 2>>>();} while(0)
+
+void errorCheck() {
+  auto err = cudaGetLastError();
+}
+
+void bad() {
+  b<<<1, 2>>>(); // sample comment
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Possible unchecked error after a kernel launch.
+
+  KERNEL_CALL(); // sample comment
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Possible unchecked error after a kernel launch.
+  // There isn't supposed to be a fix here since it's a macro call
+
+  if(true)
+b<<<1, 2>>>()  ; // Brackets omitted purposefully, since they create an additional AST node
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Possible unchecked error after a kernel launch.
+  else {
+b<<<1, 2>>>();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Possible unchecked error after a kernel launch.
+b<<<1, 2>>>();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Possible unchecked error after a kernel launch.
+  }
+  auto err = cudaGetLastError();
+
+  b<<<1, 2>>>();
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Possible unchecked error after a kernel launch.
+  if (true)
+cudaGetLastError();
+
+  b<<<1, 2>>>();
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Possible unchecked error after a kernel launch.
+  for(;;)
+auto err2 = cudaGetLastError(); // Brackets omitted purposefully, since they create an additional AST node
+
+  b<<<1, 2>>>();
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Possible unchecked error after a kernel launch.
+  auto err3 = true ? 1 : cudaGetLastError();
+
+  b<<<1, 2>>>();
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Possible unchecked error after a kernel launch.
+  auto err4 = cudaDeviceReset() + cudaGetLastError();
+
+  b<<<1, 2>>>();
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Possible unchecked error after a kernel launch.
+  // Calling an error-checking function after a kernel is not considered safe.
+  errorCheck();
+}
+
+void good() {
+  b<<<1, 2>>>();; /* The semicolons are here because the
+detection of the macro is done with a lexer */ ;
+  CUDA_CHECK_KERNEL();
+
+  b<<<1, 2>>>();
+  ALTERNATIVE_CUDA_CHECK_KERNEL();
+
+  b<<<1, 2>>>();
+  alternative::alternativeCudaCheckKernel();
+
+  b<<<1, 2>>>();
+  alternative::otherAlternativeCudaCheckKernel();
+
+  b<<<1, 2>>>();
+  switch(1 + cudaGetLastError()) {
+default:;
+  }
+
+  b<<<1, 2>>>();
+  if(3 < cudaGetLastError()) {
+1;
+  } else {
+2;
+  }
+
+  b<<<1, 2>>>();
+  for(int i = cudaGetLastError();;);
+
+  b<<<1, 2>>>();
+  do {
+  do {
+  do {
+auto err2 = cudaGetLastError();
+  } while(0);
+  } while(0);
+  } while(0);
+}
Index: clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-kernel-call-function-handler.cu
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-kernel-call-function-handler.cu
@@ -0,0 +1,150 @@
+// RUN: 

[PATCH] D133942: Clang tidy utility to generate a fix hint for a subsequent expression to the existing one

[Bartłomiej Cieślar via Phabricator via cfe-commits]

barcisz added a comment.

In D133942#3793618 , @njames93 wrote:

> Would I be correct in assuming you have uploaded the wrong diff here?

Yes, sorry, got mixed up with D133956  (6 
diffs ain't easy to shuffle :-/)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133942/new/

https://reviews.llvm.org/D133942

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133942: Clang tidy utility to generate a fix hint for a subsequent expression to the existing one

[Bartłomiej Cieślar via Phabricator via cfe-commits]

barcisz updated this revision to Diff 460522.
barcisz added a comment.
Herald added a subscriber: mgorny.

rebase


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133942/new/

https://reviews.llvm.org/D133942

Files:
  clang-tools-extra/clang-tidy/cuda/CMakeLists.txt
  clang-tools-extra/clang-tidy/cuda/CudaTidyModule.cpp
  clang-tools-extra/clang-tidy/cuda/UnsafeKernelCallCheck.cpp
  clang-tools-extra/clang-tidy/cuda/UnsafeKernelCallCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/cuda/unsafe-kernel-call.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda_runtime.h
  
clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-kernel-call-function-handler.cu
  
clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-kernel-call-macro-handler.cu

Index: clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-kernel-call-macro-handler.cu
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-kernel-call-macro-handler.cu
@@ -0,0 +1,116 @@
+// RUN: %check_clang_tidy %s cuda-unsafe-kernel-call %t -- \
+// RUN:   -config="{CheckOptions: \
+// RUN: [{key: cuda-unsafe-kernel-call.HandlerName, \
+// RUN:   value: 'CUDA_CHECK_KERNEL'}, \
+// RUN:   {key: cuda-unsafe-kernel-call.AcceptedHandlers, \
+// RUN:value: 'ALTERNATIVE_CUDA_CHECK_KERNEL, cudaCheckKernel, \
+// RUN:alternative::alternativeCudaCheckKernel, \
+// RUN:otherAlternativeCudaCheckKernel'}] \
+// RUN: }" \
+// RUN:   -- -isystem %clang_tidy_headers
+
+#include 
+
+#define CUDA_CHECK_KERNEL() do {} while(0)
+
+#define ALTERNATIVE_CUDA_CHECK_KERNEL() CUDA_CHECK_KERNEL()
+
+void cudaCheckKernel();
+
+namespace alternative {
+
+void alternativeCudaCheckKernel();
+void otherAlternativeCudaCheckKernel();
+
+}
+
+__global__
+void b();
+
+#define KERNEL_CALL() do {b<<<1, 2>>>();} while(0)
+
+void errorCheck() {
+  auto err = cudaGetLastError();
+}
+
+void bad() {
+  b<<<1, 2>>>(); // sample comment
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Possible unchecked error after a kernel launch.
+
+  KERNEL_CALL(); // sample comment
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Possible unchecked error after a kernel launch.
+  // There isn't supposed to be a fix here since it's a macro call
+
+  if(true)
+b<<<1, 2>>>()  ; // Brackets omitted purposefully, since they create an additional AST node
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Possible unchecked error after a kernel launch.
+  else {
+b<<<1, 2>>>();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Possible unchecked error after a kernel launch.
+b<<<1, 2>>>();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Possible unchecked error after a kernel launch.
+  }
+  auto err = cudaGetLastError();
+
+  b<<<1, 2>>>();
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Possible unchecked error after a kernel launch.
+  if (true)
+cudaGetLastError();
+
+  b<<<1, 2>>>();
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Possible unchecked error after a kernel launch.
+  for(;;)
+auto err2 = cudaGetLastError(); // Brackets omitted purposefully, since they create an additional AST node
+
+  b<<<1, 2>>>();
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Possible unchecked error after a kernel launch.
+  auto err3 = true ? 1 : cudaGetLastError();
+
+  b<<<1, 2>>>();
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Possible unchecked error after a kernel launch.
+  auto err4 = cudaDeviceReset() + cudaGetLastError();
+
+  b<<<1, 2>>>();
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Possible unchecked error after a kernel launch.
+  // Calling an error-checking function after a kernel is not considered safe.
+  errorCheck();
+}
+
+void good() {
+  b<<<1, 2>>>();; /* The semicolons are here because the
+detection of the macro is done with a lexer */ ;
+  CUDA_CHECK_KERNEL();
+
+  b<<<1, 2>>>();
+  ALTERNATIVE_CUDA_CHECK_KERNEL();
+
+  b<<<1, 2>>>();
+  alternative::alternativeCudaCheckKernel();
+
+  b<<<1, 2>>>();
+  alternative::otherAlternativeCudaCheckKernel();
+
+  b<<<1, 2>>>();
+  switch(1 + cudaGetLastError()) {
+default:;
+  }
+
+  b<<<1, 2>>>();
+  if(3 < cudaGetLastError()) {
+1;
+  } else {
+2;
+  }
+
+  b<<<1, 2>>>();
+  for(int i = cudaGetLastError();;);
+
+  b<<<1, 2>>>();
+  do {
+  do {
+  do {
+auto err2 = cudaGetLastError();
+  } while(0);
+  } while(0);
+  } while(0);
+}
Index: clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-kernel-call-function-handler.cu
===
--- /dev/null
+++ 

[PATCH] D133436: Ground work for cuda-related checks in clang-tidy

[Bartłomiej Cieślar via Phabricator via cfe-commits]

barcisz updated this revision to Diff 460520.
barcisz added a comment.

moved size_t definition to an stddef.h stub


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133436/new/

https://reviews.llvm.org/D133436

Files:
  clang-tools-extra/clang-tidy/CMakeLists.txt
  clang-tools-extra/clang-tidy/ClangTidyForceLinker.h
  clang-tools-extra/clang-tidy/cuda/CMakeLists.txt
  clang-tools-extra/clang-tidy/cuda/CudaTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/index.rst
  clang-tools-extra/test/clang-tidy/check_clang_tidy.py
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda.h
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/stddef.h

Index: clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/stddef.h
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/stddef.h
@@ -0,0 +1,14 @@
+//===--- stddef.h - Stub header for tests ---*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef _STDDEF_H_
+#define _STDDEF_H_
+
+using size_t = long long unsigned;
+
+#endif // _STDDEF_H_
Index: clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda.h
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda.h
@@ -0,0 +1,28 @@
+/* Minimal declarations for CUDA support.  Testing purposes only. */
+
+#include "../stddef.h"
+
+#define __constant__ __attribute__((constant))
+#define __device__ __attribute__((device))
+#define __global__ __attribute__((global))
+#define __host__ __attribute__((host))
+#define __shared__ __attribute__((shared))
+
+struct dim3 {
+  unsigned x, y, z;
+  __host__ __device__ dim3(unsigned x, unsigned y = 1, unsigned z = 1) : x(x), y(y), z(z) {}
+};
+
+typedef struct cudaStream *cudaStream_t;
+typedef enum cudaError {} cudaError_t;
+extern "C" int cudaConfigureCall(dim3 gridSize, dim3 blockSize,
+ size_t sharedSize = 0,
+ cudaStream_t stream = 0);
+extern "C" int __cudaPushCallConfiguration(dim3 gridSize, dim3 blockSize,
+   size_t sharedSize = 0,
+   cudaStream_t stream = 0);
+extern "C" cudaError_t cudaLaunchKernel(const void *func, dim3 gridDim,
+dim3 blockDim, void **args,
+size_t sharedMem, cudaStream_t stream);
+
+extern "C" __device__ int printf(const char*, ...);
Index: clang-tools-extra/test/clang-tidy/check_clang_tidy.py
===
--- clang-tools-extra/test/clang-tidy/check_clang_tidy.py
+++ clang-tools-extra/test/clang-tidy/check_clang_tidy.py
@@ -93,7 +93,7 @@
 
 file_name_with_extension = self.assume_file_name or self.input_file_name
 _, extension = os.path.splitext(file_name_with_extension)
-if extension not in ['.c', '.hpp', '.m', '.mm']:
+if extension not in ['.c', '.cu', '.hpp', '.m', '.mm']:
   extension = '.cpp'
 self.temp_file_name = self.temp_file_name + extension
 
@@ -115,9 +115,14 @@
   self.clang_extra_args = ['-fobjc-abi-version=2', '-fobjc-arc', '-fblocks'] + \
   self.clang_extra_args
 
-if extension in ['.cpp', '.hpp', '.mm']:
+if extension in ['.cpp', '.cu', '.hpp', '.mm']:
   self.clang_extra_args.append('-std=' + self.std)
 
+# Tests should not rely on a certain cuda headers and library version
+# being available on the machine
+if extension == '.cu':
+  self.clang_extra_args.extend(["--no-cuda-version-check", "-nocudalib", "-nocudainc"])
+
 # Tests should not rely on STL being available, and instead provide mock
 # implementations of relevant APIs.
 self.clang_extra_args.append('-nostdinc++')
Index: clang-tools-extra/docs/clang-tidy/index.rst
===
--- clang-tools-extra/docs/clang-tidy/index.rst
+++ clang-tools-extra/docs/clang-tidy/index.rst
@@ -67,6 +67,7 @@
 ``concurrency-``   Checks related to concurrent programming (including
threads, fibers, coroutines, etc.).
 ``cppcoreguidelines-`` Checks related to C++ Core Guidelines.
+``cuda-``  Checks related to CUDA best practices.
 ``darwin-``Checks related to Darwin coding conventions.
 ``fuchsia-``   Checks related to Fuchsia coding conventions.
 ``google-``Checks related to Google coding 

[PATCH] D133436: Ground work for cuda-related checks in clang-tidy

[Bartłomiej Cieślar via Phabricator via cfe-commits]

barcisz updated this revision to Diff 460519.
barcisz added a comment.

Dummy definition for size_t


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133436/new/

https://reviews.llvm.org/D133436

Files:
  clang-tools-extra/clang-tidy/CMakeLists.txt
  clang-tools-extra/clang-tidy/ClangTidyForceLinker.h
  clang-tools-extra/clang-tidy/cuda/CMakeLists.txt
  clang-tools-extra/clang-tidy/cuda/CudaTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/index.rst
  clang-tools-extra/test/clang-tidy/check_clang_tidy.py
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda.h

Index: clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda.h
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda.h
@@ -0,0 +1,28 @@
+/* Minimal declarations for CUDA support.  Testing purposes only. */
+
+using size_t = long long unsigned;
+
+#define __constant__ __attribute__((constant))
+#define __device__ __attribute__((device))
+#define __global__ __attribute__((global))
+#define __host__ __attribute__((host))
+#define __shared__ __attribute__((shared))
+
+struct dim3 {
+  unsigned x, y, z;
+  __host__ __device__ dim3(unsigned x, unsigned y = 1, unsigned z = 1) : x(x), y(y), z(z) {}
+};
+
+typedef struct cudaStream *cudaStream_t;
+typedef enum cudaError {} cudaError_t;
+extern "C" int cudaConfigureCall(dim3 gridSize, dim3 blockSize,
+ size_t sharedSize = 0,
+ cudaStream_t stream = 0);
+extern "C" int __cudaPushCallConfiguration(dim3 gridSize, dim3 blockSize,
+   size_t sharedSize = 0,
+   cudaStream_t stream = 0);
+extern "C" cudaError_t cudaLaunchKernel(const void *func, dim3 gridDim,
+dim3 blockDim, void **args,
+size_t sharedMem, cudaStream_t stream);
+
+extern "C" __device__ int printf(const char*, ...);
Index: clang-tools-extra/test/clang-tidy/check_clang_tidy.py
===
--- clang-tools-extra/test/clang-tidy/check_clang_tidy.py
+++ clang-tools-extra/test/clang-tidy/check_clang_tidy.py
@@ -93,7 +93,7 @@
 
 file_name_with_extension = self.assume_file_name or self.input_file_name
 _, extension = os.path.splitext(file_name_with_extension)
-if extension not in ['.c', '.hpp', '.m', '.mm']:
+if extension not in ['.c', '.cu', '.hpp', '.m', '.mm']:
   extension = '.cpp'
 self.temp_file_name = self.temp_file_name + extension
 
@@ -115,9 +115,14 @@
   self.clang_extra_args = ['-fobjc-abi-version=2', '-fobjc-arc', '-fblocks'] + \
   self.clang_extra_args
 
-if extension in ['.cpp', '.hpp', '.mm']:
+if extension in ['.cpp', '.cu', '.hpp', '.mm']:
   self.clang_extra_args.append('-std=' + self.std)
 
+# Tests should not rely on a certain cuda headers and library version
+# being available on the machine
+if extension == '.cu':
+  self.clang_extra_args.extend(["--no-cuda-version-check", "-nocudalib", "-nocudainc"])
+
 # Tests should not rely on STL being available, and instead provide mock
 # implementations of relevant APIs.
 self.clang_extra_args.append('-nostdinc++')
Index: clang-tools-extra/docs/clang-tidy/index.rst
===
--- clang-tools-extra/docs/clang-tidy/index.rst
+++ clang-tools-extra/docs/clang-tidy/index.rst
@@ -67,6 +67,7 @@
 ``concurrency-``   Checks related to concurrent programming (including
threads, fibers, coroutines, etc.).
 ``cppcoreguidelines-`` Checks related to C++ Core Guidelines.
+``cuda-``  Checks related to CUDA best practices.
 ``darwin-``Checks related to Darwin coding conventions.
 ``fuchsia-``   Checks related to Fuchsia coding conventions.
 ``google-``Checks related to Google coding conventions.
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -16,6 +16,7 @@
clang-analyzer/*
concurrency/*
cppcoreguidelines/*
+   cuda/*
darwin/*
fuchsia/*
google/*
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -96,6 +96,8 @@
 Improvements to clang-tidy
 --
 
+- Introduce the cuda module for checks specific to CUDA code.
+
 New checks
 ^^
 
Index: 

[PATCH] D133725: Searching for tokens including comments

[Bartłomiej Cieślar via Phabricator via cfe-commits]

barcisz added a comment.

In D133725#3792787 , @aaron.ballman 
wrote:

> Can you help me understand the expected use for these change?

Changed the description to answer your question - it's a prerequisite to 
D133942  and D133956 



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133725/new/

https://reviews.llvm.org/D133725

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133956: git push Cuda Check for ignored errors after calling a CUDA kernel

[Bartłomiej Cieślar via Phabricator via cfe-commits]

barcisz added a comment.

In D133956#3793022 , @tra wrote:

> I think ultimately the checker should be generalized to flag all unchecked 
> CUDA runtime calls. The problem is that that is going to be exceedingly noisy 
> in practice as a lot of real code does not bother to check for the errors 
> consistently. Limiting the checks to kernel launches may be a reasonable 
> starting point as it would give us the ability to zero in on the culprit 
> kernel by running the app with "CUDA_LAUNCH_BLOCKING".

By that do you mean that the way the check is now it is acceptable or that it 
should be improved to handle intra-procedural analysis? The intention with this 
check is to work a lot in tandem with the one in D133804 
, which therefore prevents most such cases. 
The practice that the check checks for is also commonly used in ML frameworks 
which heavily rely on CUDA, so not catching such cases might still be helpful 
for them just for the sake of preserving code consistency and catching such 
errors (since if there is an error then that means that some part of the code 
was broken anyways). Thus, the check is optimized for lowering false positives 
during static checking and for a practice lowering the number of false 
negatives within the CUDA code.




Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-kernel-call-macro-handler.cu:53
+  }
+  auto err = cudaGetLastError();
+

tra wrote:
> Just curious -- is it sufficient to just call `cudaGetLastError();` ? Or does 
> the checker require using its result, too? I.e. in practice this particular 
> check will not really do anything useful. The tests below look somewhat 
> inconsistent. 
Technically it does not require the user to actually use the value of 
`cudaGetLastError()`, but


  # If they are calling it then they most likely did not place this call there 
randomly and are using it to check for the error returned by the kernel
  # the check being introduced in D133804 can be used to check if the return 
value has been used, so checking it here as well would have been a duplication





Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-kernel-call-macro-handler.cu:75
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Possible unchecked 
error after a kernel launch.
+  // Calling an error-checking function after a kernel is not considered safe.
+  errorCheck();

tra wrote:
> WDYM by "is not considered safe" here? How is that different from calling 
> `cudaGetLastError()` and checking its value?
As in the check does not do inter-procedural analysis short of adding the 
handler to AcceptedHandlers, so the check will flag such occurences



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-kernel-call-macro-handler.cu:80-82
+  b<<<1, 2>>>();; /* The semicolons are here because the
+detection of the macro is done with a lexer */ ;
+  CUDA_CHECK_KERNEL();

tra wrote:
> What would happen with a single `;` as would be seen in the normal user code?
Nothing, it would work just fine; it's rather that all other kernel calls in 
this test use a single `;` so I want to check this case here



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-kernel-call-macro-handler.cu:112
+  do {
+auto err2 = cudaGetLastError();
+  } while(0);

tra wrote:
> Why does this case produce no warning, while a very similar case above does? 
> In both cases result of `cudaGetLastError()` is assigned to an unused 
> variable within the loop body.
> 
> ```
>   b<<<1, 2>>>();
>   // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Possible unchecked 
> error after a kernel launch.
>   for(;;)
> auto err2 = cudaGetLastError(); // Brackets omitted purposefully, since 
> they create an additional AST node
> 
> ```
Because often a macro will wrap its error handling code in a do {...} while(0) 
loop and that's why we check this case and simmilar ones with CFG analysis


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133956/new/

https://reviews.llvm.org/D133956

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133956: git push Cuda Check for ignored errors after calling a CUDA kernel

[Bartłomiej Cieślar via Phabricator via cfe-commits]

barcisz created this revision.
Herald added subscribers: mattd, carlosgalvezp, yaxunl, mgorny.
Herald added a project: All.
barcisz requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Add cuda-unchecked-kernel-call check


Motivation
--

Calls to CUDA kernels can yield errors after their invocation. These errors can 
be obtained by calling `cudaGetLastError()`, which also resets CUDA’s error 
state. There is a non error-resetting version of this function called 
`cudaPeekAtLastError()`, but the lint check does not accept this (see 
below). A limited set of errors can block a kernel from launching including 
driver malfunctions, **trying to allocate too much shared memory**, or using 
too many threads or blocks. Since those errors can cause unexpected behavior 
that blocks subsequent computation, they should be caught as close 
to the launch point as possible. The lint check enforces this by requiring that 
every kernel be immediately followed by an error check.

Behavior


The **cuda-unchecked-kernel-call** checks whether there is a call to 
`cudaGetLastError()` directly after each kernel call. To be precise, there can 
be no side-effecting or branching code between the kernel call and the call to 
`cudaGetLastError()`, such as branching due to the `?:` operator or due 
to a call to a function. This is because a more complicated behavior is likely 
to be harder for humans to read and would would be significantly slower to 
automatically check. We want to encourage well-designed, multi-line macros that 
check for errors, so we explicitly allow macros whose content is 
`do { /* error check */ } while(false)`, since this is the recommended way 

 of making multi-line macros.
The check does also accept the handler it was provided as a valid way to handle 
the error, even if the handler does not comply with the rule above (or is a 
function which cannot be easily and quickly checked). However, it is still 
encouraged to call `cudaGetLastError()` early in the handler’s code 
for the code to be readable.

Automatic fixes
---

The lint check can be configured to automatically fix the issue by adding an 
error handling macro right after the kernel launch. You can specify the error 
handler for your project by setting the **HandlerName** option for the 
**cuda-unchecked-kernel-call**. Here is an example of how this fix can 
transform unhandled code from:

  void foo(bool b) {
if (b)
  kernel<<>>();
  }

to

  void foo(bool b) {
if(b)
  {kernel<<>>(); `C10_CUDA_KERNEL_LAUNCH_CHECK`();}
  }

The specific handler used for this example is taken from PyTorch and its 
definition can be found here 
.

Known Limitations
-

Using cudaPeekAtLastError()
---

`cudaPeekAtLastError()` can also be used to check for CUDA kernel launch 
errors. However, there are several reasons why this is not and will most likely 
not be considered as a valid way to check for errors after kernel invocations. 
This all has to do with the purpose of the function, which is to 
not reset the internal error variable:

- Subsequent kernel calls, even if they don’t produce any errors, will seem as 
if they produced an error due to the error not being reset. This behavior is 
easy to overlook and may cause he significant difficulty in debugging.
- Our linter cannot easily check whether the error was reset before subsequent 
kernel calls. It might even be impossible to do so due to the error leaking 
inter-procedurally from functions whose code we can’t access.

Checking for errors that occurred while a kernel was running


Our linter does not check whether errors occurred while a kernel was running. 
The linter only enforces checks that a kernel launched correctly. 
`cudaDeviceSynchronize()` and similar API calls can be used to see that a 
kernel’s computation was successful, but these are blocking calls, so we are 
not 
able to suggest where they should go automatically.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133956

Files:
  clang-tools-extra/clang-tidy/cuda/CMakeLists.txt
  clang-tools-extra/clang-tidy/cuda/CudaTidyModule.cpp
  clang-tools-extra/clang-tidy/cuda/UnsafeKernelCallCheck.cpp
  clang-tools-extra/clang-tidy/cuda/UnsafeKernelCallCheck.h
  clang-tools-extra/clang-tidy/utils/FixItHintUtils.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/cuda/unsafe-kernel-call.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda.h
  

[PATCH] D133804: Cuda Check for ignored return errors from api calls to cuda

[Bartłomiej Cieślar via Phabricator via cfe-commits]

barcisz updated this revision to Diff 460410.
barcisz added a comment.

Use header guards instead of pragma


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133804/new/

https://reviews.llvm.org/D133804

Files:
  clang-tools-extra/-
  clang-tools-extra/clang-tidy/cuda/CMakeLists.txt
  clang-tools-extra/clang-tidy/cuda/CudaTidyModule.cpp
  clang-tools-extra/clang-tidy/cuda/UnsafeApiCallCheck.cpp
  clang-tools-extra/clang-tidy/cuda/UnsafeApiCallCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/cuda/unsafe-api-call.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda.h
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda_runtime.h
  
clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-function-handler.cu
  
clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-macro-handler.cu

Index: clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-macro-handler.cu
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-macro-handler.cu
@@ -0,0 +1,96 @@
+// RUN: %check_clang_tidy %s cuda-unsafe-api-call %t -- \
+// RUN:   -config="{CheckOptions: \
+// RUN: [{key: cuda-unsafe-api-call.HandlerName, \
+// RUN:   value: 'CUDA_HANDLER'}] \
+// RUN: }" \
+// RUN:   -- -isystem %clang_tidy_headers -std=c++14
+#include 
+
+class DummyContainer {
+ public:
+  int* begin();
+  int* end();
+};
+
+#define DUMMY_CUDA_HANDLER(stmt) stmt
+#define CUDA_HANDLER(stmt) do {auto err = stmt;} while(0)
+#define API_CALL() do {cudaDeviceReset();} while(0)
+
+void errorCheck();
+void errorCheck(cudaError_t error);
+
+void bad() {
+  API_CALL();
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+  // There isn't supposed to be a fix here since it's a macro call
+
+  cudaDeviceReset();
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+  // CHECK-FIXES:  {{^}}  CUDA_HANDLER(cudaDeviceReset());{{$}}
+  errorCheck();
+
+  if (true)
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+
+  while (true)
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+
+  do
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+  while(false);
+
+  switch (0) {
+case 0:
+  cudaDeviceReset();
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+  // CHECK-FIXES:  {{^}}  CUDA_HANDLER(cudaDeviceReset());{{$}}
+  }
+
+  for(
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+;
+cudaDeviceReset()
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset()){{$}}
+  ) cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}  ) CUDA_HANDLER(cudaDeviceReset());{{$}}
+
+  for(int i : DummyContainer())
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+
+  auto x = ({
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+true;
+  });
+}
+
+int good() {
+  DUMMY_CUDA_HANDLER(cudaDeviceReset());
+
+  if (cudaDeviceReset()) {
+return 0;
+  }
+
+  switch (cudaDeviceReset()) {
+case cudaErrorInvalidValue: return 1;
+case cudaErrorMemoryAllocation: return 2;
+default: return 3;
+  }
+
+  auto err = ({cudaDeviceReset();});
+  // NOTE: We don't check that `errorCheck()` actually handles the error; we just assume it does.
+  errorCheck(cudaDeviceReset());
+}
Index: clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-function-handler.cu
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-function-handler.cu
@@ -0,0 +1,71 @@
+// RUN: %check_clang_tidy %s cuda-unsafe-api-call %t -- \
+// RUN:   -config="{CheckOptions: \
+// RUN: [{key: cuda-unsafe-api-call.HandlerName, \
+// RUN:   value: 'cudaHandler'}, \
+// RUN:  {key: cuda-unsafe-api-call.AcceptedHandlers, \
+// RUN:   

[PATCH] D133942: Clang tidy utility to generate a fix hint for a subsequent expression to the existing one

[Bartłomiej Cieślar via Phabricator via cfe-commits]

barcisz updated this revision to Diff 460405.
barcisz added a comment.

Use the new Lexer option to include comments while finding next token


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133942/new/

https://reviews.llvm.org/D133942

Files:
  clang-tools-extra/clang-tidy/utils/FixItHintUtils.cpp
  clang-tools-extra/clang-tidy/utils/FixItHintUtils.h

Index: clang-tools-extra/clang-tidy/utils/FixItHintUtils.h
===
--- clang-tools-extra/clang-tidy/utils/FixItHintUtils.h
+++ clang-tools-extra/clang-tidy/utils/FixItHintUtils.h
@@ -46,6 +46,17 @@
   DeclSpec::TQ Qualifier,
   QualifierTarget CT = QualifierTarget::Pointee,
   QualifierPolicy CP = QualifierPolicy::Left);
+
+/// \brief Adds a statement to be executed right after this statement .
+/// Is designed for taking potential comments or statements in the same line
+/// into account. The statement should not be an expression that's part of
+/// another statement. The statement range should include the terminator
+/// (semicolon).
+llvm::SmallVector
+addSubsequentStatement(SourceRange stmtRangeWithTerminator,
+   const Stmt , llvm::StringRef nextStmt,
+   ASTContext );
+
 } // namespace fixit
 } // namespace utils
 } // namespace tidy
Index: clang-tools-extra/clang-tidy/utils/FixItHintUtils.cpp
===
--- clang-tools-extra/clang-tidy/utils/FixItHintUtils.cpp
+++ clang-tools-extra/clang-tidy/utils/FixItHintUtils.cpp
@@ -223,6 +223,147 @@
 
   return None;
 }
+
+static std::string getIndent(SourceLocation sLoc, ASTContext& context) {
+  auto& SM = context.getSourceManager();
+
+  const auto sLocLineNo = getLineNumber(sLoc, SM);
+
+  auto indentation_template = tooling::fixit::internal::getText(
+  CharSourceRange::getCharRange(SourceRange(
+  SM.translateLineCol(SM.getFileID(sLoc), sLocLineNo, 1), sLoc)),
+  context);
+
+  std::string indentation;
+  indentation.reserve(indentation_template.size());
+  std::transform(
+  indentation_template.begin(),
+  indentation_template.end(),
+  std::back_inserter(indentation),
+  [](char c) { return isspace(c) ? c : ' '; });
+  return indentation;
+}
+
+static llvm::SmallVector addSubsequentStatement(
+SourceRange stmtRangeWithTerminator,
+const Stmt& parentStmt,
+llvm::StringRef nextStmt,
+ASTContext& context) {
+  auto& SM = context.getSourceManager();
+  auto langOpts = context.getLangOpts();
+
+  const auto stmtEndLineNo =
+  getLineNumber(stmtRangeWithTerminator.getEnd(), SM);
+
+  // Find the first token's data for which the next token is
+  // either a line apart or is not a comment
+  SourceLocation lastTokenEndLoc =
+  stmtRangeWithTerminator.getEnd().getLocWithOffset(1);
+  auto lastTokenLine = stmtEndLineNo;
+  bool insertNewLine = true;
+  while (true) {
+llvm::Optional tokenOption = Lexer::findNextToken(
+lastTokenEndLoc.getLocWithOffset(-1), SM, langOpts, true);
+if (!tokenOption) {
+  return llvm::SmallVector();
+}
+if (tokenOption->is(tok::eof)) {
+  insertNewLine = false;
+  break;
+}
+const auto tokenBeginLineNo = getLineNumber(tokenOption->getLocation(), SM);
+
+if (tokenOption->isNot(tok::comment)) {
+  insertNewLine = tokenBeginLineNo != stmtEndLineNo;
+  break;
+}
+if (tokenBeginLineNo > lastTokenLine) {
+  break;
+}
+
+lastTokenEndLoc = tokenOption->getEndLoc();
+lastTokenLine = getLineNumber(tokenOption->getEndLoc(), SM);
+  }
+
+  bool isEnclosedWithBrackets =
+  parentStmt.getStmtClass() == Stmt::CompoundStmtClass;
+
+  // Generating the FixItHint
+  // There are 5 scenarios that we have to take into account:
+  // 1. The statement is enclosed in brackets but the next statement is
+  //in the same line - insert the new statement right after the previous one
+  // 2. The statement is not enclosed in brackets and the next statement is
+  //in the same line - same as 1. and enclose both statements in brackets
+  //on the same line
+  // 3. The statement is enclosed in brackets and the next statement is
+  //on subsequent lines - skip all the comments in this line
+  // 4. The statement is not enclosed in brackets but the next statement is on
+  //subsequent lines - same as 3. and enclose the statements with
+  //google-style multiline brackets (opening bracket right after the parent
+  //statement and closing bracket on a new line after the new statement).
+  // 5. The statement is not enclosed in brackets but the next statement is on
+  //subsequent lines and the main statement is before an else token - same
+  //as 4. but the closing bracket is put on the same line as the else
+  //statement
+  if (!insertNewLine) {
+if (isEnclosedWithBrackets) {

[PATCH] D133942: Clang tidy utility to generate a fix hint for a subsequent expression to the existing one

[Bartłomiej Cieślar via Phabricator via cfe-commits]

barcisz created this revision.
Herald added a subscriber: carlosgalvezp.
Herald added a project: All.
barcisz requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133942

Files:
  clang-tools-extra/clang-tidy/utils/FixItHintUtils.cpp
  clang-tools-extra/clang-tidy/utils/FixItHintUtils.h

Index: clang-tools-extra/clang-tidy/utils/FixItHintUtils.h
===
--- clang-tools-extra/clang-tidy/utils/FixItHintUtils.h
+++ clang-tools-extra/clang-tidy/utils/FixItHintUtils.h
@@ -46,6 +46,17 @@
   DeclSpec::TQ Qualifier,
   QualifierTarget CT = QualifierTarget::Pointee,
   QualifierPolicy CP = QualifierPolicy::Left);
+
+/// \brief Adds a statement to be executed right after this statement .
+/// Is designed for taking potential comments or statements in the same line
+/// into account. The statement should not be an expression that's part of
+/// another statement. The statement range should include the terminator
+/// (semicolon).
+llvm::SmallVector
+addSubsequentStatement(SourceRange stmtRangeWithTerminator,
+   const Stmt , llvm::StringRef nextStmt,
+   ASTContext );
+
 } // namespace fixit
 } // namespace utils
 } // namespace tidy
Index: clang-tools-extra/clang-tidy/utils/FixItHintUtils.cpp
===
--- clang-tools-extra/clang-tidy/utils/FixItHintUtils.cpp
+++ clang-tools-extra/clang-tidy/utils/FixItHintUtils.cpp
@@ -223,6 +223,147 @@
 
   return None;
 }
+
+static std::string getIndent(SourceLocation sLoc, ASTContext& context) {
+  auto& SM = context.getSourceManager();
+
+  const auto sLocLineNo = getLineNumber(sLoc, SM);
+
+  auto indentation_template = tooling::fixit::internal::getText(
+  CharSourceRange::getCharRange(SourceRange(
+  SM.translateLineCol(SM.getFileID(sLoc), sLocLineNo, 1), sLoc)),
+  context);
+
+  std::string indentation;
+  indentation.reserve(indentation_template.size());
+  std::transform(
+  indentation_template.begin(),
+  indentation_template.end(),
+  std::back_inserter(indentation),
+  [](char c) { return isspace(c) ? c : ' '; });
+  return indentation;
+}
+
+static llvm::SmallVector addSubsequentStatement(
+SourceRange stmtRangeWithTerminator,
+const Stmt& parentStmt,
+llvm::StringRef nextStmt,
+ASTContext& context) {
+  auto& SM = context.getSourceManager();
+  auto langOpts = context.getLangOpts();
+
+  const auto stmtEndLineNo =
+  getLineNumber(stmtRangeWithTerminator.getEnd(), SM);
+
+  // Find the first token's data for which the next token is
+  // either a line apart or is not a comment
+  SourceLocation lastTokenEndLoc =
+  stmtRangeWithTerminator.getEnd().getLocWithOffset(1);
+  auto lastTokenLine = stmtEndLineNo;
+  bool insertNewLine = true;
+  while (true) {
+llvm::Optional tokenOption = findNextTokenIncludingComments(
+lastTokenEndLoc.getLocWithOffset(-1), SM, langOpts);
+if (!tokenOption) {
+  return llvm::SmallVector();
+}
+if (tokenOption->is(tok::eof)) {
+  insertNewLine = false;
+  break;
+}
+const auto tokenBeginLineNo = getLineNumber(tokenOption->getLocation(), SM);
+
+if (tokenOption->isNot(tok::comment)) {
+  insertNewLine = tokenBeginLineNo != stmtEndLineNo;
+  break;
+}
+if (tokenBeginLineNo > lastTokenLine) {
+  break;
+}
+
+lastTokenEndLoc = tokenOption->getEndLoc();
+lastTokenLine = getLineNumber(tokenOption->getEndLoc(), SM);
+  }
+
+  bool isEnclosedWithBrackets =
+  parentStmt.getStmtClass() == Stmt::CompoundStmtClass;
+
+  // Generating the FixItHint
+  // There are 5 scenarios that we have to take into account:
+  // 1. The statement is enclosed in brackets but the next statement is
+  //in the same line - insert the new statement right after the previous one
+  // 2. The statement is not enclosed in brackets and the next statement is
+  //in the same line - same as 1. and enclose both statements in brackets
+  //on the same line
+  // 3. The statement is enclosed in brackets and the next statement is
+  //on subsequent lines - skip all the comments in this line
+  // 4. The statement is not enclosed in brackets but the next statement is on
+  //subsequent lines - same as 3. and enclose the statements with
+  //google-style multiline brackets (opening bracket right after the parent
+  //statement and closing bracket on a new line after the new statement).
+  // 5. The statement is not enclosed in brackets but the next statement is on
+  //subsequent lines and the main statement is before an else token - same
+  //as 4. but the closing bracket is put on the same line as the else
+  //statement
+  if (!insertNewLine) {
+if 

[PATCH] D133804: Cuda Check for ignored return errors from api calls to cuda

[Bartłomiej Cieślar via Phabricator via cfe-commits]

barcisz updated this revision to Diff 460376.
barcisz added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133804/new/

https://reviews.llvm.org/D133804

Files:
  clang-tools-extra/-
  clang-tools-extra/clang-tidy/cuda/CMakeLists.txt
  clang-tools-extra/clang-tidy/cuda/CudaTidyModule.cpp
  clang-tools-extra/clang-tidy/cuda/UnsafeApiCallCheck.cpp
  clang-tools-extra/clang-tidy/cuda/UnsafeApiCallCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/cuda/unsafe-api-call.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda.h
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda_runtime.h
  
clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-function-handler.cu
  
clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-macro-handler.cu

Index: clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-macro-handler.cu
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-macro-handler.cu
@@ -0,0 +1,96 @@
+// RUN: %check_clang_tidy %s cuda-unsafe-api-call %t -- \
+// RUN:   -config="{CheckOptions: \
+// RUN: [{key: cuda-unsafe-api-call.HandlerName, \
+// RUN:   value: 'CUDA_HANDLER'}] \
+// RUN: }" \
+// RUN:   -- -isystem %clang_tidy_headers -std=c++14
+#include 
+
+class DummyContainer {
+ public:
+  int* begin();
+  int* end();
+};
+
+#define DUMMY_CUDA_HANDLER(stmt) stmt
+#define CUDA_HANDLER(stmt) do {auto err = stmt;} while(0)
+#define API_CALL() do {cudaDeviceReset();} while(0)
+
+void errorCheck();
+void errorCheck(cudaError_t error);
+
+void bad() {
+  API_CALL();
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+  // There isn't supposed to be a fix here since it's a macro call
+
+  cudaDeviceReset();
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+  // CHECK-FIXES:  {{^}}  CUDA_HANDLER(cudaDeviceReset());{{$}}
+  errorCheck();
+
+  if (true)
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+
+  while (true)
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+
+  do
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+  while(false);
+
+  switch (0) {
+case 0:
+  cudaDeviceReset();
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+  // CHECK-FIXES:  {{^}}  CUDA_HANDLER(cudaDeviceReset());{{$}}
+  }
+
+  for(
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+;
+cudaDeviceReset()
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset()){{$}}
+  ) cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}  ) CUDA_HANDLER(cudaDeviceReset());{{$}}
+
+  for(int i : DummyContainer())
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+
+  auto x = ({
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+true;
+  });
+}
+
+int good() {
+  DUMMY_CUDA_HANDLER(cudaDeviceReset());
+
+  if (cudaDeviceReset()) {
+return 0;
+  }
+
+  switch (cudaDeviceReset()) {
+case cudaErrorInvalidValue: return 1;
+case cudaErrorMemoryAllocation: return 2;
+default: return 3;
+  }
+
+  auto err = ({cudaDeviceReset();});
+  // NOTE: We don't check that `errorCheck()` actually handles the error; we just assume it does.
+  errorCheck(cudaDeviceReset());
+}
Index: clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-function-handler.cu
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-function-handler.cu
@@ -0,0 +1,71 @@
+// RUN: %check_clang_tidy %s cuda-unsafe-api-call %t -- \
+// RUN:   -config="{CheckOptions: \
+// RUN: [{key: cuda-unsafe-api-call.HandlerName, \
+// RUN:   value: 'cudaHandler'}, \
+// RUN:  {key: cuda-unsafe-api-call.AcceptedHandlers, \
+// RUN:   value: 'CUDA_HANDLER, 

[PATCH] D133436: Ground work for cuda-related checks in clang-tidy

[Bartłomiej Cieślar via Phabricator via cfe-commits]

barcisz updated this revision to Diff 460374.
barcisz marked 2 inline comments as done.
barcisz added a comment.

Changes suggested by njames93


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133436/new/

https://reviews.llvm.org/D133436

Files:
  clang-tools-extra/clang-tidy/CMakeLists.txt
  clang-tools-extra/clang-tidy/ClangTidyForceLinker.h
  clang-tools-extra/clang-tidy/cuda/CMakeLists.txt
  clang-tools-extra/clang-tidy/cuda/CudaTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/index.rst
  clang-tools-extra/test/clang-tidy/check_clang_tidy.py
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda.h

Index: clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda.h
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda.h
@@ -0,0 +1,26 @@
+/* Minimal declarations for CUDA support.  Testing purposes only. */
+
+#define __constant__ __attribute__((constant))
+#define __device__ __attribute__((device))
+#define __global__ __attribute__((global))
+#define __host__ __attribute__((host))
+#define __shared__ __attribute__((shared))
+
+struct dim3 {
+  unsigned x, y, z;
+  __host__ __device__ dim3(unsigned x, unsigned y = 1, unsigned z = 1) : x(x), y(y), z(z) {}
+};
+
+typedef struct cudaStream *cudaStream_t;
+typedef enum cudaError {} cudaError_t;
+extern "C" int cudaConfigureCall(dim3 gridSize, dim3 blockSize,
+ size_t sharedSize = 0,
+ cudaStream_t stream = 0);
+extern "C" int __cudaPushCallConfiguration(dim3 gridSize, dim3 blockSize,
+   size_t sharedSize = 0,
+   cudaStream_t stream = 0);
+extern "C" cudaError_t cudaLaunchKernel(const void *func, dim3 gridDim,
+dim3 blockDim, void **args,
+size_t sharedMem, cudaStream_t stream);
+
+extern "C" __device__ int printf(const char*, ...);
Index: clang-tools-extra/test/clang-tidy/check_clang_tidy.py
===
--- clang-tools-extra/test/clang-tidy/check_clang_tidy.py
+++ clang-tools-extra/test/clang-tidy/check_clang_tidy.py
@@ -93,7 +93,7 @@
 
 file_name_with_extension = self.assume_file_name or self.input_file_name
 _, extension = os.path.splitext(file_name_with_extension)
-if extension not in ['.c', '.hpp', '.m', '.mm']:
+if extension not in ['.c', '.cu', '.hpp', '.m', '.mm']:
   extension = '.cpp'
 self.temp_file_name = self.temp_file_name + extension
 
@@ -115,9 +115,14 @@
   self.clang_extra_args = ['-fobjc-abi-version=2', '-fobjc-arc', '-fblocks'] + \
   self.clang_extra_args
 
-if extension in ['.cpp', '.hpp', '.mm']:
+if extension in ['.cpp', '.cu', '.hpp', '.mm']:
   self.clang_extra_args.append('-std=' + self.std)
 
+# Tests should not rely on a certain cuda headers and library version
+# being available on the machine
+if extension == '.cu':
+  self.clang_extra_args.extend(["--no-cuda-version-check", "-nocudalib", "-nocudainc"])
+
 # Tests should not rely on STL being available, and instead provide mock
 # implementations of relevant APIs.
 self.clang_extra_args.append('-nostdinc++')
Index: clang-tools-extra/docs/clang-tidy/index.rst
===
--- clang-tools-extra/docs/clang-tidy/index.rst
+++ clang-tools-extra/docs/clang-tidy/index.rst
@@ -67,6 +67,7 @@
 ``concurrency-``   Checks related to concurrent programming (including
threads, fibers, coroutines, etc.).
 ``cppcoreguidelines-`` Checks related to C++ Core Guidelines.
+``cuda-``  Checks related to CUDA best practices.
 ``darwin-``Checks related to Darwin coding conventions.
 ``fuchsia-``   Checks related to Fuchsia coding conventions.
 ``google-``Checks related to Google coding conventions.
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -16,6 +16,7 @@
clang-analyzer/*
concurrency/*
cppcoreguidelines/*
+   cuda/*
darwin/*
fuchsia/*
google/*
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -96,6 +96,8 @@
 Improvements to clang-tidy
 --
 
+- Introduce the cuda module for checks specific to CUDA code.
+
 New checks
 ^^
 
Index: 

[PATCH] D133804: Cuda Check for ignored return errors from api calls to cuda

[Bartłomiej Cieślar via Phabricator via cfe-commits]

barcisz updated this revision to Diff 460261.
barcisz added a comment.

Removed unneeded headers from the check


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133804/new/

https://reviews.llvm.org/D133804

Files:
  clang-tools-extra/-
  clang-tools-extra/clang-tidy/cuda/CMakeLists.txt
  clang-tools-extra/clang-tidy/cuda/CudaTidyModule.cpp
  clang-tools-extra/clang-tidy/cuda/UnsafeApiCallCheck.cpp
  clang-tools-extra/clang-tidy/cuda/UnsafeApiCallCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/cuda/.keep
  clang-tools-extra/docs/clang-tidy/checks/cuda/unsafe-api-call.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda.h
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda_runtime.h
  clang-tools-extra/test/clang-tidy/checkers/cuda/.keep
  
clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-function-handler.cu
  
clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-macro-handler.cu

Index: clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-macro-handler.cu
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-macro-handler.cu
@@ -0,0 +1,96 @@
+// RUN: %check_clang_tidy %s cuda-unsafe-api-call %t -- \
+// RUN:   -config="{CheckOptions: \
+// RUN: [{key: cuda-unsafe-api-call.HandlerName, \
+// RUN:   value: 'CUDA_HANDLER'}] \
+// RUN: }" \
+// RUN:   -- -isystem %clang_tidy_headers -std=c++14
+#include 
+
+class DummyContainer {
+ public:
+  int* begin();
+  int* end();
+};
+
+#define DUMMY_CUDA_HANDLER(stmt) stmt
+#define CUDA_HANDLER(stmt) do {auto err = stmt;} while(0)
+#define API_CALL() do {cudaDeviceReset();} while(0)
+
+void errorCheck();
+void errorCheck(cudaError_t error);
+
+void bad() {
+  API_CALL();
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+  // There isn't supposed to be a fix here since it's a macro call
+
+  cudaDeviceReset();
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+  // CHECK-FIXES:  {{^}}  CUDA_HANDLER(cudaDeviceReset());{{$}}
+  errorCheck();
+
+  if (true)
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+
+  while (true)
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+
+  do
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+  while(false);
+
+  switch (0) {
+case 0:
+  cudaDeviceReset();
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+  // CHECK-FIXES:  {{^}}  CUDA_HANDLER(cudaDeviceReset());{{$}}
+  }
+
+  for(
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+;
+cudaDeviceReset()
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset()){{$}}
+  ) cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}  ) CUDA_HANDLER(cudaDeviceReset());{{$}}
+
+  for(int i : DummyContainer())
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+
+  auto x = ({
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+true;
+  });
+}
+
+int good() {
+  DUMMY_CUDA_HANDLER(cudaDeviceReset());
+
+  if (cudaDeviceReset()) {
+return 0;
+  }
+
+  switch (cudaDeviceReset()) {
+case cudaErrorInvalidValue: return 1;
+case cudaErrorMemoryAllocation: return 2;
+default: return 3;
+  }
+
+  auto err = ({cudaDeviceReset();});
+  // NOTE: We don't check that `errorCheck()` actually handles the error; we just assume it does.
+  errorCheck(cudaDeviceReset());
+}
Index: clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-function-handler.cu
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-function-handler.cu
@@ -0,0 +1,71 @@
+// RUN: %check_clang_tidy %s cuda-unsafe-api-call %t -- \
+// RUN:   -config="{CheckOptions: \
+// RUN: [{key: cuda-unsafe-api-call.HandlerName, \
+// RUN:   

[PATCH] D133804: Cuda Check for ignored return errors from api calls to cuda

[Bartłomiej Cieślar via Phabricator via cfe-commits]

barcisz updated this revision to Diff 460258.
barcisz added a comment.

Removed unneeded headers from the check


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133804/new/

https://reviews.llvm.org/D133804

Files:
  clang-tools-extra/-
  clang-tools-extra/clang-tidy/cuda/CMakeLists.txt
  clang-tools-extra/clang-tidy/cuda/CudaTidyModule.cpp
  clang-tools-extra/clang-tidy/cuda/UnsafeApiCallCheck.cpp
  clang-tools-extra/clang-tidy/cuda/UnsafeApiCallCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/cuda/.keep
  clang-tools-extra/docs/clang-tidy/checks/cuda/unsafe-api-call.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda.h
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda_runtime.h
  clang-tools-extra/test/clang-tidy/checkers/cuda/.keep
  
clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-function-handler.cu
  
clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-macro-handler.cu

Index: clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-macro-handler.cu
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-macro-handler.cu
@@ -0,0 +1,96 @@
+// RUN: %check_clang_tidy %s cuda-unsafe-api-call %t -- \
+// RUN:   -config="{CheckOptions: \
+// RUN: [{key: cuda-unsafe-api-call.HandlerName, \
+// RUN:   value: 'CUDA_HANDLER'}] \
+// RUN: }" \
+// RUN:   -- -isystem %clang_tidy_headers -std=c++14
+#include 
+
+class DummyContainer {
+ public:
+  int* begin();
+  int* end();
+};
+
+#define DUMMY_CUDA_HANDLER(stmt) stmt
+#define CUDA_HANDLER(stmt) do {auto err = stmt;} while(0)
+#define API_CALL() do {cudaDeviceReset();} while(0)
+
+void errorCheck();
+void errorCheck(cudaError_t error);
+
+void bad() {
+  API_CALL();
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+  // There isn't supposed to be a fix here since it's a macro call
+
+  cudaDeviceReset();
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+  // CHECK-FIXES:  {{^}}  CUDA_HANDLER(cudaDeviceReset());{{$}}
+  errorCheck();
+
+  if (true)
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+
+  while (true)
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+
+  do
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+  while(false);
+
+  switch (0) {
+case 0:
+  cudaDeviceReset();
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+  // CHECK-FIXES:  {{^}}  CUDA_HANDLER(cudaDeviceReset());{{$}}
+  }
+
+  for(
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+;
+cudaDeviceReset()
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset()){{$}}
+  ) cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}  ) CUDA_HANDLER(cudaDeviceReset());{{$}}
+
+  for(int i : DummyContainer())
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+
+  auto x = ({
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+true;
+  });
+}
+
+int good() {
+  DUMMY_CUDA_HANDLER(cudaDeviceReset());
+
+  if (cudaDeviceReset()) {
+return 0;
+  }
+
+  switch (cudaDeviceReset()) {
+case cudaErrorInvalidValue: return 1;
+case cudaErrorMemoryAllocation: return 2;
+default: return 3;
+  }
+
+  auto err = ({cudaDeviceReset();});
+  // NOTE: We don't check that `errorCheck()` actually handles the error; we just assume it does.
+  errorCheck(cudaDeviceReset());
+}
Index: clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-function-handler.cu
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-function-handler.cu
@@ -0,0 +1,71 @@
+// RUN: %check_clang_tidy %s cuda-unsafe-api-call %t -- \
+// RUN:   -config="{CheckOptions: \
+// RUN: [{key: cuda-unsafe-api-call.HandlerName, \
+// RUN:   

[PATCH] D133804: Cuda Check for ignored return errors from api calls to cuda

[Bartłomiej Cieślar via Phabricator via cfe-commits]

barcisz updated this revision to Diff 460256.
barcisz added a comment.

Brought back different message prefix for when AcceptedHandlers is set


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133804/new/

https://reviews.llvm.org/D133804

Files:
  clang-tools-extra/-
  clang-tools-extra/clang-tidy/cuda/CMakeLists.txt
  clang-tools-extra/clang-tidy/cuda/CudaTidyModule.cpp
  clang-tools-extra/clang-tidy/cuda/UnsafeApiCallCheck.cpp
  clang-tools-extra/clang-tidy/cuda/UnsafeApiCallCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/cuda/.keep
  clang-tools-extra/docs/clang-tidy/checks/cuda/unsafe-api-call.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda.h
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda_runtime.h
  clang-tools-extra/test/clang-tidy/checkers/cuda/.keep
  
clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-function-handler.cu
  
clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-macro-handler.cu

Index: clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-macro-handler.cu
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-macro-handler.cu
@@ -0,0 +1,96 @@
+// RUN: %check_clang_tidy %s cuda-unsafe-api-call %t -- \
+// RUN:   -config="{CheckOptions: \
+// RUN: [{key: cuda-unsafe-api-call.HandlerName, \
+// RUN:   value: 'CUDA_HANDLER'}] \
+// RUN: }" \
+// RUN:   -- -isystem %clang_tidy_headers -std=c++14
+#include 
+
+class DummyContainer {
+ public:
+  int* begin();
+  int* end();
+};
+
+#define DUMMY_CUDA_HANDLER(stmt) stmt
+#define CUDA_HANDLER(stmt) do {auto err = stmt;} while(0)
+#define API_CALL() do {cudaDeviceReset();} while(0)
+
+void errorCheck();
+void errorCheck(cudaError_t error);
+
+void bad() {
+  API_CALL();
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+  // There isn't supposed to be a fix here since it's a macro call
+
+  cudaDeviceReset();
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+  // CHECK-FIXES:  {{^}}  CUDA_HANDLER(cudaDeviceReset());{{$}}
+  errorCheck();
+
+  if (true)
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+
+  while (true)
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+
+  do
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+  while(false);
+
+  switch (0) {
+case 0:
+  cudaDeviceReset();
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+  // CHECK-FIXES:  {{^}}  CUDA_HANDLER(cudaDeviceReset());{{$}}
+  }
+
+  for(
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+;
+cudaDeviceReset()
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset()){{$}}
+  ) cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}  ) CUDA_HANDLER(cudaDeviceReset());{{$}}
+
+  for(int i : DummyContainer())
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+
+  auto x = ({
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+true;
+  });
+}
+
+int good() {
+  DUMMY_CUDA_HANDLER(cudaDeviceReset());
+
+  if (cudaDeviceReset()) {
+return 0;
+  }
+
+  switch (cudaDeviceReset()) {
+case cudaErrorInvalidValue: return 1;
+case cudaErrorMemoryAllocation: return 2;
+default: return 3;
+  }
+
+  auto err = ({cudaDeviceReset();});
+  // NOTE: We don't check that `errorCheck()` actually handles the error; we just assume it does.
+  errorCheck(cudaDeviceReset());
+}
Index: clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-function-handler.cu
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-function-handler.cu
@@ -0,0 +1,71 @@
+// RUN: %check_clang_tidy %s cuda-unsafe-api-call %t -- \
+// RUN:   -config="{CheckOptions: \
+// RUN: [{key: 

[PATCH] D133804: Cuda Check for ignored return errors from api calls to cuda

[Bartłomiej Cieślar via Phabricator via cfe-commits]

barcisz updated this revision to Diff 460184.
barcisz added a comment.

Documentation and small check-message-related bugfixes


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133804/new/

https://reviews.llvm.org/D133804

Files:
  clang-tools-extra/-
  clang-tools-extra/clang-tidy/cuda/CMakeLists.txt
  clang-tools-extra/clang-tidy/cuda/CudaTidyModule.cpp
  clang-tools-extra/clang-tidy/cuda/UnsafeApiCallCheck.cpp
  clang-tools-extra/clang-tidy/cuda/UnsafeApiCallCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/cuda/.keep
  clang-tools-extra/docs/clang-tidy/checks/cuda/unsafe-api-call.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda.h
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda_runtime.h
  clang-tools-extra/test/clang-tidy/checkers/cuda/.keep
  
clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-function-handler.cu
  
clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-macro-handler.cu

Index: clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-macro-handler.cu
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-macro-handler.cu
@@ -0,0 +1,96 @@
+// RUN: %check_clang_tidy %s cuda-unsafe-api-call %t -- \
+// RUN:   -config="{CheckOptions: \
+// RUN: [{key: cuda-unsafe-api-call.HandlerName, \
+// RUN:   value: 'CUDA_HANDLER'}] \
+// RUN: }" \
+// RUN:   -- -isystem %clang_tidy_headers -std=c++14
+#include 
+
+class DummyContainer {
+ public:
+  int* begin();
+  int* end();
+};
+
+#define DUMMY_CUDA_HANDLER(stmt) stmt
+#define CUDA_HANDLER(stmt) do {auto err = stmt;} while(0)
+#define API_CALL() do {cudaDeviceReset();} while(0)
+
+void errorCheck();
+void errorCheck(cudaError_t error);
+
+void bad() {
+  API_CALL();
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+  // There isn't supposed to be a fix here since it's a macro call
+
+  cudaDeviceReset();
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+  // CHECK-FIXES:  {{^}}  CUDA_HANDLER(cudaDeviceReset());{{$}}
+  errorCheck();
+
+  if (true)
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+
+  while (true)
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+
+  do
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+  while(false);
+
+  switch (0) {
+case 0:
+  cudaDeviceReset();
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+  // CHECK-FIXES:  {{^}}  CUDA_HANDLER(cudaDeviceReset());{{$}}
+  }
+
+  for(
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+;
+cudaDeviceReset()
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset()){{$}}
+  ) cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}  ) CUDA_HANDLER(cudaDeviceReset());{{$}}
+
+  for(int i : DummyContainer())
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+
+  auto x = ({
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+true;
+  });
+}
+
+int good() {
+  DUMMY_CUDA_HANDLER(cudaDeviceReset());
+
+  if (cudaDeviceReset()) {
+return 0;
+  }
+
+  switch (cudaDeviceReset()) {
+case cudaErrorInvalidValue: return 1;
+case cudaErrorMemoryAllocation: return 2;
+default: return 3;
+  }
+
+  auto err = ({cudaDeviceReset();});
+  // NOTE: We don't check that `errorCheck()` actually handles the error; we just assume it does.
+  errorCheck(cudaDeviceReset());
+}
Index: clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-function-handler.cu
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-function-handler.cu
@@ -0,0 +1,71 @@
+// RUN: %check_clang_tidy %s cuda-unsafe-api-call %t -- \
+// RUN:   -config="{CheckOptions: \
+// RUN: [{key: cuda-unsafe-api-call.HandlerName, \
+// RUN:   

[PATCH] D133436: Ground work for cuda-related checks in clang-tidy

[Bartłomiej Cieślar via Phabricator via cfe-commits]

barcisz updated this revision to Diff 460070.
barcisz added a comment.
Herald added a subscriber: arphaman.

Common documentation for cuda checks


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133436/new/

https://reviews.llvm.org/D133436

Files:
  clang-tools-extra/clang-tidy/CMakeLists.txt
  clang-tools-extra/clang-tidy/ClangTidyForceLinker.h
  clang-tools-extra/clang-tidy/cuda/CMakeLists.txt
  clang-tools-extra/clang-tidy/cuda/CudaTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/cuda/.keep
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/index.rst
  clang-tools-extra/test/clang-tidy/check_clang_tidy.py
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda-initializers.h
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda.h
  clang-tools-extra/test/clang-tidy/checkers/cuda/.keep

Index: clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda.h
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda.h
@@ -0,0 +1,28 @@
+/* Minimal declarations for CUDA support.  Testing purposes only. */
+
+#include 
+
+#define __constant__ __attribute__((constant))
+#define __device__ __attribute__((device))
+#define __global__ __attribute__((global))
+#define __host__ __attribute__((host))
+#define __shared__ __attribute__((shared))
+
+struct dim3 {
+  unsigned x, y, z;
+  __host__ __device__ dim3(unsigned x, unsigned y = 1, unsigned z = 1) : x(x), y(y), z(z) {}
+};
+
+typedef struct cudaStream *cudaStream_t;
+typedef enum cudaError {} cudaError_t;
+extern "C" int cudaConfigureCall(dim3 gridSize, dim3 blockSize,
+ size_t sharedSize = 0,
+ cudaStream_t stream = 0);
+extern "C" int __cudaPushCallConfiguration(dim3 gridSize, dim3 blockSize,
+   size_t sharedSize = 0,
+   cudaStream_t stream = 0);
+extern "C" cudaError_t cudaLaunchKernel(const void *func, dim3 gridDim,
+dim3 blockDim, void **args,
+size_t sharedMem, cudaStream_t stream);
+
+extern "C" __device__ int printf(const char*, ...);
Index: clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda-initializers.h
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda-initializers.h
@@ -0,0 +1,145 @@
+// CUDA struct types with interesting initialization properties.
+// Keep in sync with clang/test/SemaCUDA/Inputs/cuda-initializers.h.
+
+// Base classes with different initializer variants.
+
+// trivial constructor -- allowed
+struct T {
+  int t;
+};
+
+// empty constructor
+struct EC {
+  int ec;
+  __device__ EC() {} // -- allowed
+  __device__ EC(int) {}  // -- not allowed
+};
+
+// empty destructor
+struct ED {
+  __device__ ~ED() {} // -- allowed
+};
+
+struct ECD {
+  __device__ ECD() {} // -- allowed
+  __device__ ~ECD() {}// -- allowed
+};
+
+// empty templated constructor -- allowed with no arguments
+struct ETC {
+  template  __device__ ETC(T...) {}
+};
+
+// undefined constructor -- not allowed
+struct UC {
+  int uc;
+  __device__ UC();
+};
+
+// undefined destructor -- not allowed
+struct UD {
+  int ud;
+  __device__ ~UD();
+};
+
+// empty constructor w/ initializer list -- not allowed
+struct ECI {
+  int eci;
+  __device__ ECI() : eci(1) {}
+};
+
+// non-empty constructor -- not allowed
+struct NEC {
+  int nec;
+  __device__ NEC() { nec = 1; }
+};
+
+// non-empty destructor -- not allowed
+struct NED {
+  int ned;
+  __device__ ~NED() { ned = 1; }
+};
+
+// no-constructor,  virtual method -- not allowed
+struct NCV {
+  int ncv;
+  __device__ virtual void vm() {}
+};
+
+// virtual destructor -- not allowed.
+struct VD {
+  __device__ virtual ~VD() {}
+};
+
+// dynamic in-class field initializer -- not allowed
+__device__ int f();
+struct NCF {
+  int ncf = f();
+};
+
+// static in-class field initializer.  NVCC does not allow it, but
+// clang generates static initializer for this, so we'll accept it.
+// We still can't use it on __shared__ vars as they don't allow *any*
+// initializers.
+struct NCFS {
+  int ncfs = 3;
+};
+
+// undefined templated constructor -- not allowed
+struct UTC {
+  template  __device__ UTC(T...);
+};
+
+// non-empty templated constructor -- not allowed
+struct NETC {
+  int netc;
+  template  __device__ NETC(T...) { netc = 1; }
+};
+
+// Regular base class -- allowed
+struct T_B_T : T {};
+
+// Incapsulated object of allowed class -- allowed
+struct T_F_T {
+  T t;
+};
+
+// array of allowed objects -- allowed
+struct T_FA_T {
+  T t[2];
+};
+
+
+// Calling empty base class initializer is OK
+struct 

[PATCH] D133804: Cuda Check for ignored return errors from api calls to cuda

[Bartłomiej Cieślar via Phabricator via cfe-commits]

barcisz updated this revision to Diff 460055.
barcisz added a comment.

Update base to D133801 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133804/new/

https://reviews.llvm.org/D133804

Files:
  clang-tools-extra/-
  clang-tools-extra/clang-tidy/cuda/CMakeLists.txt
  clang-tools-extra/clang-tidy/cuda/CudaTidyModule.cpp
  clang-tools-extra/clang-tidy/cuda/UnsafeApiCallCheck.cpp
  clang-tools-extra/clang-tidy/cuda/UnsafeApiCallCheck.h
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda.h
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda_runtime.h
  clang-tools-extra/test/clang-tidy/checkers/cuda/.keep
  
clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-function-handler.cu
  
clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-macro-handler.cu

Index: clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-macro-handler.cu
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-macro-handler.cu
@@ -0,0 +1,96 @@
+// RUN: %check_clang_tidy %s cuda-unsafe-api-call %t -- \
+// RUN:   -config="{CheckOptions: \
+// RUN: [{key: cuda-unsafe-api-call.HandlerName, \
+// RUN:   value: 'CUDA_HANDLER'}] \
+// RUN: }" \
+// RUN:   -- -isystem %clang_tidy_headers -std=c++14
+#include 
+
+class DummyContainer {
+ public:
+  int* begin();
+  int* end();
+};
+
+#define DUMMY_CUDA_HANDLER(stmt) stmt
+#define CUDA_HANDLER(stmt) do {auto err = stmt;} while(0)
+#define API_CALL() do {cudaDeviceReset();} while(0)
+
+void errorCheck();
+void errorCheck(cudaError_t error);
+
+void bad() {
+  API_CALL();
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+  // There isn't supposed to be a fix here since it's a macro call
+
+  cudaDeviceReset();
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+  // CHECK-FIXES:  {{^}}  CUDA_HANDLER(cudaDeviceReset());{{$}}
+  errorCheck();
+
+  if (true)
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+
+  while (true)
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+
+  do
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+  while(false);
+
+  switch (0) {
+case 0:
+  cudaDeviceReset();
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+  // CHECK-FIXES:  {{^}}  CUDA_HANDLER(cudaDeviceReset());{{$}}
+  }
+
+  for(
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+;
+cudaDeviceReset()
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset()){{$}}
+  ) cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}  ) CUDA_HANDLER(cudaDeviceReset());{{$}}
+
+  for(int i : DummyContainer())
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+
+  auto x = ({
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+true;
+  });
+}
+
+int good() {
+  DUMMY_CUDA_HANDLER(cudaDeviceReset());
+
+  if (cudaDeviceReset()) {
+return 0;
+  }
+
+  switch (cudaDeviceReset()) {
+case cudaErrorInvalidValue: return 1;
+case cudaErrorMemoryAllocation: return 2;
+default: return 3;
+  }
+
+  auto err = ({cudaDeviceReset();});
+  // NOTE: We don't check that `errorCheck()` actually handles the error; we just assume it does.
+  errorCheck(cudaDeviceReset());
+}
Index: clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-function-handler.cu
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-function-handler.cu
@@ -0,0 +1,71 @@
+// RUN: %check_clang_tidy %s cuda-unsafe-api-call %t -- \
+// RUN:   -config="{CheckOptions: \
+// RUN: [{key: cuda-unsafe-api-call.HandlerName, \
+// RUN:   value: 'cudaHandler'}, \
+// RUN:  {key: cuda-unsafe-api-call.AcceptedHandlers, \
+// RUN:   value: 'CUDA_HANDLER, DUMMY_CUDA_HANDLER, \
+// RUN:   

[PATCH] D133804: Cuda Check for ignored return errors from api calls to cuda

[Bartłomiej Cieślar via Phabricator via cfe-commits]

barcisz added inline comments.



Comment at: clang-tools-extra/test/clang-tidy/check_clang_tidy.py:121
 
+# Tests should not rely on a certain cuda device being available on the 
machine,
+# or a certain version of it

tra wrote:
> The comment is incorrect. These flags have nothing to do with GPU 
> availability, but rather with CUDA SDK which is normally expected to provide 
> the 'standard' set of CUDA headers and libdevice bitcode.
Yes by CUDA I meant cuda library here but I'll change it accordingly



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-function-handler.cu:1
+// (c) Meta Platforms, Inc. and affiliates. Confidential and proprietary.
+

tra wrote:
> This does not look right. 
Fixed


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133804/new/

https://reviews.llvm.org/D133804

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133804: Cuda Check for ignored return errors from api calls to cuda

[Bartłomiej Cieślar via Phabricator via cfe-commits]

barcisz updated this revision to Diff 460034.
barcisz added a comment.

Removed unnecessary cuda-related compilation flags from tests


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133804/new/

https://reviews.llvm.org/D133804

Files:
  clang-tools-extra/-
  clang-tools-extra/clang-tidy/cuda/CMakeLists.txt
  clang-tools-extra/clang-tidy/cuda/CudaTidyModule.cpp
  clang-tools-extra/clang-tidy/cuda/UnsafeApiCallCheck.cpp
  clang-tools-extra/clang-tidy/cuda/UnsafeApiCallCheck.h
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda.h
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda_runtime.h
  clang-tools-extra/test/clang-tidy/checkers/cuda/.keep
  
clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-function-handler.cu
  
clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-macro-handler.cu

Index: clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-macro-handler.cu
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-macro-handler.cu
@@ -0,0 +1,96 @@
+// RUN: %check_clang_tidy %s cuda-unsafe-api-call %t -- \
+// RUN:   -config="{CheckOptions: \
+// RUN: [{key: cuda-unsafe-api-call.HandlerName, \
+// RUN:   value: 'CUDA_HANDLER'}] \
+// RUN: }" \
+// RUN:   -- -isystem %clang_tidy_headers -std=c++14
+#include 
+
+class DummyContainer {
+ public:
+  int* begin();
+  int* end();
+};
+
+#define DUMMY_CUDA_HANDLER(stmt) stmt
+#define CUDA_HANDLER(stmt) do {auto err = stmt;} while(0)
+#define API_CALL() do {cudaDeviceReset();} while(0)
+
+void errorCheck();
+void errorCheck(cudaError_t error);
+
+void bad() {
+  API_CALL();
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+  // There isn't supposed to be a fix here since it's a macro call
+
+  cudaDeviceReset();
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+  // CHECK-FIXES:  {{^}}  CUDA_HANDLER(cudaDeviceReset());{{$}}
+  errorCheck();
+
+  if (true)
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+
+  while (true)
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+
+  do
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+  while(false);
+
+  switch (0) {
+case 0:
+  cudaDeviceReset();
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+  // CHECK-FIXES:  {{^}}  CUDA_HANDLER(cudaDeviceReset());{{$}}
+  }
+
+  for(
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+;
+cudaDeviceReset()
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset()){{$}}
+  ) cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}  ) CUDA_HANDLER(cudaDeviceReset());{{$}}
+
+  for(int i : DummyContainer())
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+
+  auto x = ({
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+true;
+  });
+}
+
+int good() {
+  DUMMY_CUDA_HANDLER(cudaDeviceReset());
+
+  if (cudaDeviceReset()) {
+return 0;
+  }
+
+  switch (cudaDeviceReset()) {
+case cudaErrorInvalidValue: return 1;
+case cudaErrorMemoryAllocation: return 2;
+default: return 3;
+  }
+
+  auto err = ({cudaDeviceReset();});
+  // NOTE: We don't check that `errorCheck()` actually handles the error; we just assume it does.
+  errorCheck(cudaDeviceReset());
+}
Index: clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-function-handler.cu
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-function-handler.cu
@@ -0,0 +1,71 @@
+// RUN: %check_clang_tidy %s cuda-unsafe-api-call %t -- \
+// RUN:   -config="{CheckOptions: \
+// RUN: [{key: cuda-unsafe-api-call.HandlerName, \
+// RUN:   value: 'cudaHandler'}, \
+// RUN:  {key: cuda-unsafe-api-call.AcceptedHandlers, \
+// RUN:   value: 'CUDA_HANDLER, DUMMY_CUDA_HANDLER, \
+// RUN:   

[PATCH] D133436: Ground work for cuda-related checks in clang-tidy

[Bartłomiej Cieślar via Phabricator via cfe-commits]

barcisz updated this revision to Diff 460027.
barcisz added a comment.

Better explanation of cuda-related flags in tests


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133436/new/

https://reviews.llvm.org/D133436

Files:
  clang-tools-extra/clang-tidy/CMakeLists.txt
  clang-tools-extra/clang-tidy/ClangTidyForceLinker.h
  clang-tools-extra/clang-tidy/cuda/CMakeLists.txt
  clang-tools-extra/clang-tidy/cuda/CudaTidyModule.cpp
  clang-tools-extra/test/clang-tidy/check_clang_tidy.py
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda-initializers.h
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda.h
  clang-tools-extra/test/clang-tidy/checkers/cuda/.keep

Index: clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda.h
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda.h
@@ -0,0 +1,28 @@
+/* Minimal declarations for CUDA support.  Testing purposes only. */
+
+#include 
+
+#define __constant__ __attribute__((constant))
+#define __device__ __attribute__((device))
+#define __global__ __attribute__((global))
+#define __host__ __attribute__((host))
+#define __shared__ __attribute__((shared))
+
+struct dim3 {
+  unsigned x, y, z;
+  __host__ __device__ dim3(unsigned x, unsigned y = 1, unsigned z = 1) : x(x), y(y), z(z) {}
+};
+
+typedef struct cudaStream *cudaStream_t;
+typedef enum cudaError {} cudaError_t;
+extern "C" int cudaConfigureCall(dim3 gridSize, dim3 blockSize,
+ size_t sharedSize = 0,
+ cudaStream_t stream = 0);
+extern "C" int __cudaPushCallConfiguration(dim3 gridSize, dim3 blockSize,
+   size_t sharedSize = 0,
+   cudaStream_t stream = 0);
+extern "C" cudaError_t cudaLaunchKernel(const void *func, dim3 gridDim,
+dim3 blockDim, void **args,
+size_t sharedMem, cudaStream_t stream);
+
+extern "C" __device__ int printf(const char*, ...);
Index: clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda-initializers.h
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda-initializers.h
@@ -0,0 +1,145 @@
+// CUDA struct types with interesting initialization properties.
+// Keep in sync with clang/test/SemaCUDA/Inputs/cuda-initializers.h.
+
+// Base classes with different initializer variants.
+
+// trivial constructor -- allowed
+struct T {
+  int t;
+};
+
+// empty constructor
+struct EC {
+  int ec;
+  __device__ EC() {} // -- allowed
+  __device__ EC(int) {}  // -- not allowed
+};
+
+// empty destructor
+struct ED {
+  __device__ ~ED() {} // -- allowed
+};
+
+struct ECD {
+  __device__ ECD() {} // -- allowed
+  __device__ ~ECD() {}// -- allowed
+};
+
+// empty templated constructor -- allowed with no arguments
+struct ETC {
+  template  __device__ ETC(T...) {}
+};
+
+// undefined constructor -- not allowed
+struct UC {
+  int uc;
+  __device__ UC();
+};
+
+// undefined destructor -- not allowed
+struct UD {
+  int ud;
+  __device__ ~UD();
+};
+
+// empty constructor w/ initializer list -- not allowed
+struct ECI {
+  int eci;
+  __device__ ECI() : eci(1) {}
+};
+
+// non-empty constructor -- not allowed
+struct NEC {
+  int nec;
+  __device__ NEC() { nec = 1; }
+};
+
+// non-empty destructor -- not allowed
+struct NED {
+  int ned;
+  __device__ ~NED() { ned = 1; }
+};
+
+// no-constructor,  virtual method -- not allowed
+struct NCV {
+  int ncv;
+  __device__ virtual void vm() {}
+};
+
+// virtual destructor -- not allowed.
+struct VD {
+  __device__ virtual ~VD() {}
+};
+
+// dynamic in-class field initializer -- not allowed
+__device__ int f();
+struct NCF {
+  int ncf = f();
+};
+
+// static in-class field initializer.  NVCC does not allow it, but
+// clang generates static initializer for this, so we'll accept it.
+// We still can't use it on __shared__ vars as they don't allow *any*
+// initializers.
+struct NCFS {
+  int ncfs = 3;
+};
+
+// undefined templated constructor -- not allowed
+struct UTC {
+  template  __device__ UTC(T...);
+};
+
+// non-empty templated constructor -- not allowed
+struct NETC {
+  int netc;
+  template  __device__ NETC(T...) { netc = 1; }
+};
+
+// Regular base class -- allowed
+struct T_B_T : T {};
+
+// Incapsulated object of allowed class -- allowed
+struct T_F_T {
+  T t;
+};
+
+// array of allowed objects -- allowed
+struct T_FA_T {
+  T t[2];
+};
+
+
+// Calling empty base class initializer is OK
+struct EC_I_EC : EC {
+  __device__ EC_I_EC() : EC() {}
+};
+
+// .. though passing arguments is not allowed.
+struct EC_I_EC1 : EC {
+  __device__ EC_I_EC1() : EC(1) {}
+};
+
+// Virtual base class -- not allowed
+struct T_V_T : 

[PATCH] D133804: Cuda Check for ignored return errors from api calls to cuda

[Bartłomiej Cieślar via Phabricator via cfe-commits]

barcisz updated this revision to Diff 460025.
barcisz added a comment.

Rebase and better comments for cuda-related decisions in the check


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133804/new/

https://reviews.llvm.org/D133804

Files:
  clang-tools-extra/-
  clang-tools-extra/clang-tidy/cuda/CMakeLists.txt
  clang-tools-extra/clang-tidy/cuda/CudaTidyModule.cpp
  clang-tools-extra/clang-tidy/cuda/UnsafeApiCallCheck.cpp
  clang-tools-extra/clang-tidy/cuda/UnsafeApiCallCheck.h
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda.h
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda_runtime.h
  clang-tools-extra/test/clang-tidy/checkers/cuda/.keep
  
clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-function-handler.cu
  
clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-macro-handler.cu

Index: clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-macro-handler.cu
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-macro-handler.cu
@@ -0,0 +1,96 @@
+// RUN: %check_clang_tidy %s cuda-unsafe-api-call %t -- \
+// RUN:   -config="{CheckOptions: \
+// RUN: [{key: cuda-unsafe-api-call.HandlerName, \
+// RUN:   value: 'CUDA_HANDLER'}] \
+// RUN: }" \
+// RUN:   -- -isystem %clang_tidy_headers -nocudalib -nocudainc -std=c++14
+#include 
+
+class DummyContainer {
+ public:
+  int* begin();
+  int* end();
+};
+
+#define DUMMY_CUDA_HANDLER(stmt) stmt
+#define CUDA_HANDLER(stmt) do {auto err = stmt;} while(0)
+#define API_CALL() do {cudaDeviceReset();} while(0)
+
+void errorCheck();
+void errorCheck(cudaError_t error);
+
+void bad() {
+  API_CALL();
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+  // There isn't supposed to be a fix here since it's a macro call
+
+  cudaDeviceReset();
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+  // CHECK-FIXES:  {{^}}  CUDA_HANDLER(cudaDeviceReset());{{$}}
+  errorCheck();
+
+  if (true)
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+
+  while (true)
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+
+  do
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+  while(false);
+
+  switch (0) {
+case 0:
+  cudaDeviceReset();
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+  // CHECK-FIXES:  {{^}}  CUDA_HANDLER(cudaDeviceReset());{{$}}
+  }
+
+  for(
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+;
+cudaDeviceReset()
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset()){{$}}
+  ) cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}  ) CUDA_HANDLER(cudaDeviceReset());{{$}}
+
+  for(int i : DummyContainer())
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+
+  auto x = ({
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+true;
+  });
+}
+
+int good() {
+  DUMMY_CUDA_HANDLER(cudaDeviceReset());
+
+  if (cudaDeviceReset()) {
+return 0;
+  }
+
+  switch (cudaDeviceReset()) {
+case cudaErrorInvalidValue: return 1;
+case cudaErrorMemoryAllocation: return 2;
+default: return 3;
+  }
+
+  auto err = ({cudaDeviceReset();});
+  // NOTE: We don't check that `errorCheck()` actually handles the error; we just assume it does.
+  errorCheck(cudaDeviceReset());
+}
Index: clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-function-handler.cu
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-function-handler.cu
@@ -0,0 +1,71 @@
+// RUN: %check_clang_tidy %s cuda-unsafe-api-call %t -- \
+// RUN:   -config="{CheckOptions: \
+// RUN: [{key: cuda-unsafe-api-call.HandlerName, \
+// RUN:   value: 'cudaHandler'}, \
+// RUN:  {key: cuda-unsafe-api-call.AcceptedHandlers, \
+// RUN:   value: 'CUDA_HANDLER, DUMMY_CUDA_HANDLER, \
+// RUN: 

[PATCH] D133804: Cuda Check for ignored return errors from api calls to cuda

[Bartłomiej Cieślar via Phabricator via cfe-commits]

barcisz updated this revision to Diff 460019.
barcisz added a comment.

More explanation comments for the check's code


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133804/new/

https://reviews.llvm.org/D133804

Files:
  clang-tools-extra/clang-tidy/cuda/CMakeLists.txt
  clang-tools-extra/clang-tidy/cuda/CudaTidyModule.cpp
  clang-tools-extra/clang-tidy/cuda/UnsafeApiCallCheck.cpp
  clang-tools-extra/clang-tidy/cuda/UnsafeApiCallCheck.h
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda.h
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda_runtime.h
  clang-tools-extra/test/clang-tidy/checkers/cuda/.keep
  
clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-function-handler.cu
  
clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-macro-handler.cu

Index: clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-macro-handler.cu
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-macro-handler.cu
@@ -0,0 +1,104 @@
+//===--- SlicingCheck.cpp - clang-tidy-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+// RUN: %check_clang_tidy %s cuda-unsafe-api-call %t -- \
+// RUN:   -config="{CheckOptions: \
+// RUN: [{key: cuda-unsafe-api-call.HandlerName, \
+// RUN:   value: 'CUDA_HANDLER'}] \
+// RUN: }" \
+// RUN:   -- -isystem %clang_tidy_headers -nocudalib -nocudainc -std=c++14
+#include 
+
+class DummyContainer {
+ public:
+  int* begin();
+  int* end();
+};
+
+#define DUMMY_CUDA_HANDLER(stmt) stmt
+#define CUDA_HANDLER(stmt) do {auto err = stmt;} while(0)
+#define API_CALL() do {cudaDeviceReset();} while(0)
+
+void errorCheck();
+void errorCheck(cudaError_t error);
+
+void bad() {
+  API_CALL();
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+  // There isn't supposed to be a fix here since it's a macro call
+
+  cudaDeviceReset();
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+  // CHECK-FIXES:  {{^}}  CUDA_HANDLER(cudaDeviceReset());{{$}}
+  errorCheck();
+
+  if (true)
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+
+  while (true)
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+
+  do
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+  while(false);
+
+  switch (0) {
+case 0:
+  cudaDeviceReset();
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+  // CHECK-FIXES:  {{^}}  CUDA_HANDLER(cudaDeviceReset());{{$}}
+  }
+
+  for(
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+;
+cudaDeviceReset()
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset()){{$}}
+  ) cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}  ) CUDA_HANDLER(cudaDeviceReset());{{$}}
+
+  for(int i : DummyContainer())
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+
+  auto x = ({
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+true;
+  });
+}
+
+int good() {
+  DUMMY_CUDA_HANDLER(cudaDeviceReset());
+
+  if (cudaDeviceReset()) {
+return 0;
+  }
+
+  switch (cudaDeviceReset()) {
+case cudaErrorInvalidValue: return 1;
+case cudaErrorMemoryAllocation: return 2;
+default: return 3;
+  }
+
+  auto err = ({cudaDeviceReset();});
+  // NOTE: We don't check that `errorCheck()` actually handles the error; we just assume it does.
+  errorCheck(cudaDeviceReset());
+}
Index: clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-function-handler.cu
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-function-handler.cu
@@ -0,0 +1,73 @@
+// (c) Meta 

[PATCH] D133436: Ground work for cuda-related checks in clang-tidy

[Bartłomiej Cieślar via Phabricator via cfe-commits]

barcisz added a comment.

@tschuett does it look alright now?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133436/new/

https://reviews.llvm.org/D133436

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133804: Cuda Check for ignored return errors from api calls to cuda

[Bartłomiej Cieślar via Phabricator via cfe-commits]

barcisz updated this revision to Diff 459870.
barcisz added a comment.

Added some explanation comments


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133804/new/

https://reviews.llvm.org/D133804

Files:
  clang-tools-extra/clang-tidy/CMakeLists.txt
  clang-tools-extra/clang-tidy/ClangTidyForceLinker.h
  clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
  clang-tools-extra/clang-tidy/cuda/CMakeLists.txt
  clang-tools-extra/clang-tidy/cuda/CudaTidyModule.cpp
  clang-tools-extra/clang-tidy/cuda/UnsafeApiCallCheck.cpp
  clang-tools-extra/clang-tidy/cuda/UnsafeApiCallCheck.h
  clang-tools-extra/clang-tidy/utils/Matchers.h
  clang-tools-extra/test/clang-tidy/check_clang_tidy.py
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda-initializers.h
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda.h
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda_runtime.h
  
clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-function-handler.cu
  
clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-macro-handler.cu
  clang-tools-extra/test/lit.cfg.py

Index: clang-tools-extra/test/lit.cfg.py
===
--- clang-tools-extra/test/lit.cfg.py
+++ clang-tools-extra/test/lit.cfg.py
@@ -16,7 +16,7 @@
 config.test_format = lit.formats.ShTest(not llvm_config.use_lit_shell)
 
 # suffixes: A list of file extensions to treat as test files.
-config.suffixes = ['.c', '.cpp', '.hpp', '.m', '.mm', '.cu', '.ll', '.cl', '.s',
+config.suffixes = ['.c', '.cpp', '.cu', '.hpp', '.m', '.mm', '.cu', '.ll', '.cl', '.s',
   '.modularize', '.module-map-checker', '.test']
 
 # Test-time dependencies located in directories called 'Inputs' are excluded
Index: clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-macro-handler.cu
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-macro-handler.cu
@@ -0,0 +1,104 @@
+//===--- SlicingCheck.cpp - clang-tidy-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+// RUN: %check_clang_tidy %s cuda-unsafe-api-call %t -- \
+// RUN:   -config="{CheckOptions: \
+// RUN: [{key: cuda-unsafe-api-call.HandlerName, \
+// RUN:   value: 'CUDA_HANDLER'}] \
+// RUN: }" \
+// RUN:   -- -isystem %clang_tidy_headers -nocudalib -nocudainc -std=c++14
+#include 
+
+class DummyContainer {
+ public:
+  int* begin();
+  int* end();
+};
+
+#define DUMMY_CUDA_HANDLER(stmt) stmt
+#define CUDA_HANDLER(stmt) do {auto err = stmt;} while(0)
+#define API_CALL() do {cudaDeviceReset();} while(0)
+
+void errorCheck();
+void errorCheck(cudaError_t error);
+
+void bad() {
+  API_CALL();
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+  // There isn't supposed to be a fix here since it's a macro call
+
+  cudaDeviceReset();
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+  // CHECK-FIXES:  {{^}}  CUDA_HANDLER(cudaDeviceReset());{{$}}
+  errorCheck();
+
+  if (true)
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+
+  while (true)
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+
+  do
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+  while(false);
+
+  switch (0) {
+case 0:
+  cudaDeviceReset();
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+  // CHECK-FIXES:  {{^}}  CUDA_HANDLER(cudaDeviceReset());{{$}}
+  }
+
+  for(
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+;
+cudaDeviceReset()
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset()){{$}}
+  ) cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}  ) CUDA_HANDLER(cudaDeviceReset());{{$}}
+
+  for(int i : DummyContainer())
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}

[PATCH] D133804: Cuda Check for ignored return errors from api calls to cuda

[Bartłomiej Cieślar via Phabricator via cfe-commits]

barcisz created this revision.
Herald added subscribers: mattd, carlosgalvezp, yaxunl, mgorny.
Herald added a project: All.
barcisz requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133804

Files:
  clang-tools-extra/clang-tidy/cuda/CMakeLists.txt
  clang-tools-extra/clang-tidy/cuda/CudaTidyModule.cpp
  clang-tools-extra/clang-tidy/cuda/UnsafeApiCallCheck.cpp
  clang-tools-extra/clang-tidy/cuda/UnsafeApiCallCheck.h
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda.h
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda_runtime.h
  clang-tools-extra/test/clang-tidy/checkers/cuda/.keep
  
clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-function-handler.cu
  
clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-macro-handler.cu

Index: clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-macro-handler.cu
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-macro-handler.cu
@@ -0,0 +1,104 @@
+//===--- SlicingCheck.cpp - clang-tidy-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+// RUN: %check_clang_tidy %s cuda-unsafe-api-call %t -- \
+// RUN:   -config="{CheckOptions: \
+// RUN: [{key: cuda-unsafe-api-call.HandlerName, \
+// RUN:   value: 'CUDA_HANDLER'}] \
+// RUN: }" \
+// RUN:   -- -isystem %clang_tidy_headers -nocudalib -nocudainc -std=c++14
+#include 
+
+class DummyContainer {
+ public:
+  int* begin();
+  int* end();
+};
+
+#define DUMMY_CUDA_HANDLER(stmt) stmt
+#define CUDA_HANDLER(stmt) do {auto err = stmt;} while(0)
+#define API_CALL() do {cudaDeviceReset();} while(0)
+
+void errorCheck();
+void errorCheck(cudaError_t error);
+
+void bad() {
+  API_CALL();
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+  // There isn't supposed to be a fix here since it's a macro call
+
+  cudaDeviceReset();
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+  // CHECK-FIXES:  {{^}}  CUDA_HANDLER(cudaDeviceReset());{{$}}
+  errorCheck();
+
+  if (true)
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+
+  while (true)
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+
+  do
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+  while(false);
+
+  switch (0) {
+case 0:
+  cudaDeviceReset();
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+  // CHECK-FIXES:  {{^}}  CUDA_HANDLER(cudaDeviceReset());{{$}}
+  }
+
+  for(
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+;
+cudaDeviceReset()
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset()){{$}}
+  ) cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}  ) CUDA_HANDLER(cudaDeviceReset());{{$}}
+
+  for(int i : DummyContainer())
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+
+  auto x = ({
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+true;
+  });
+}
+
+int good() {
+  DUMMY_CUDA_HANDLER(cudaDeviceReset());
+
+  if (cudaDeviceReset()) {
+return 0;
+  }
+
+  switch (cudaDeviceReset()) {
+case cudaErrorInvalidValue: return 1;
+case cudaErrorMemoryAllocation: return 2;
+default: return 3;
+  }
+
+  auto err = ({cudaDeviceReset();});
+  // NOTE: We don't check that `errorCheck()` actually handles the error; we just assume it does.
+  errorCheck(cudaDeviceReset());
+}
Index: clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-function-handler.cu
===
--- /dev/null
+++ 

[PATCH] D133801: Extraction of a matcher for an unused value from an expression from the bugprone-unused-return-value check

[Bartłomiej Cieślar via Phabricator via cfe-commits]

barcisz created this revision.
Herald added a subscriber: carlosgalvezp.
Herald added a project: All.
barcisz requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

This diff extracts the matcher for an unused expression result from 
bugprone-unused-return-value into a utility. This diff is a prerequisite to the 
diff that introduces 
the cuda-unsafe-api-call-check


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133801

Files:
  clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
  clang-tools-extra/clang-tidy/utils/Matchers.h


Index: clang-tools-extra/clang-tidy/utils/Matchers.h
===
--- clang-tools-extra/clang-tidy/utils/Matchers.h
+++ clang-tools-extra/clang-tidy/utils/Matchers.h
@@ -49,6 +49,51 @@
   return pointerType(pointee(qualType(isConstQualified(;
 }
 
+// Matches the statements in a GNU statement-expression that are not returned
+// from it.
+AST_MATCHER_P(StmtExpr, hasUnreturning,
+  clang::ast_matchers::internal::Matcher, matcher) {
+  const auto compoundStmt = Node.getSubStmt();
+  assert(compoundStmt);
+
+  clang::ast_matchers::internal::BoundNodesTreeBuilder result;
+  bool matched = false;
+  for (auto stmt = compoundStmt->body_begin();
+   stmt + 1 < compoundStmt->body_end(); ++stmt) {
+clang::ast_matchers::internal::BoundNodesTreeBuilder 
builderInner(*Builder);
+assert(stmt && *stmt);
+if (matcher.matches(**stmt, Finder, )) {
+  result.addMatch(builderInner);
+  matched = true;
+}
+  }
+  *Builder = result;
+  return matched;
+}
+
+// Matches all of the nodes (simmilar to forEach) that match the matcher
+// and have return values not used in any statement.
+AST_MATCHER_FUNCTION_P(ast_matchers::StatementMatcher, isValueUnused,
+   ast_matchers::StatementMatcher, Matcher) {
+  using namespace ast_matchers;
+  const auto UnusedInCompoundStmt =
+  compoundStmt(forEach(Matcher), unless(hasParent(stmtExpr(;
+  const auto UnusedInGnuExprStmt = stmtExpr(hasUnreturning(Matcher));
+  const auto UnusedInIfStmt =
+  ifStmt(eachOf(hasThen(Matcher), hasElse(Matcher)));
+  const auto UnusedInWhileStmt = whileStmt(hasBody(Matcher));
+  const auto UnusedInDoStmt = doStmt(hasBody(Matcher));
+  const auto UnusedInForStmt = forStmt(
+  eachOf(hasLoopInit(Matcher), hasIncrement(Matcher), hasBody(Matcher)));
+  const auto UnusedInRangeForStmt = cxxForRangeStmt(hasBody(Matcher));
+  const auto UnusedInCaseStmt = switchCase(forEach(Matcher));
+  const auto Unused =
+  stmt(anyOf(UnusedInCompoundStmt, UnusedInGnuExprStmt, UnusedInIfStmt,
+ UnusedInWhileStmt, UnusedInDoStmt, UnusedInForStmt,
+ UnusedInRangeForStmt, UnusedInCaseStmt));
+  return stmt(eachOf(Unused, forEachDescendant(Unused)));
+}
+
 // A matcher implementation that matches a list of type name regular 
expressions
 // against a NamedDecl. If a regular expression contains the substring "::"
 // matching will occur against the qualified name, otherwise only the typename.
Index: clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
@@ -7,6 +7,7 @@
 
//===--===//
 
 #include "UnusedReturnValueCheck.h"
+#include "../utils/Matchers.h"
 #include "../utils/OptionsUtils.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
@@ -159,10 +160,7 @@
   auto UnusedInCaseStmt = switchCase(forEach(MatchedCallExpr));
 
   Finder->addMatcher(
-  stmt(anyOf(UnusedInCompoundStmt, UnusedInIfStmt, UnusedInWhileStmt,
- UnusedInDoStmt, UnusedInForStmt, UnusedInRangeForStmt,
- UnusedInCaseStmt)),
-  this);
+  functionDecl(hasBody(matchers::isValueUnused(MatchedCallExpr))), this);
 }
 
 void UnusedReturnValueCheck::check(const MatchFinder::MatchResult ) {


Index: clang-tools-extra/clang-tidy/utils/Matchers.h
===
--- clang-tools-extra/clang-tidy/utils/Matchers.h
+++ clang-tools-extra/clang-tidy/utils/Matchers.h
@@ -49,6 +49,51 @@
   return pointerType(pointee(qualType(isConstQualified(;
 }
 
+// Matches the statements in a GNU statement-expression that are not returned
+// from it.
+AST_MATCHER_P(StmtExpr, hasUnreturning,
+  clang::ast_matchers::internal::Matcher, matcher) {
+  const auto compoundStmt = Node.getSubStmt();
+  assert(compoundStmt);
+
+  clang::ast_matchers::internal::BoundNodesTreeBuilder result;
+  bool matched = false;
+  for (auto stmt = compoundStmt->body_begin();
+   stmt + 1 < compoundStmt->body_end(); ++stmt) {
+

[PATCH] D133725: Searching for tokens including comments

[Bartłomiej Cieślar via Phabricator via cfe-commits]

barcisz created this revision.
Herald added a project: All.
barcisz requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This is a small diff that adds an optional argument to the Lexer::findNextToken 
that allows for searching for the token while including comments (instead of 
having to 
instantiate the lexer separately)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133725

Files:
  clang/include/clang/Lex/Lexer.h
  clang/lib/Lex/Lexer.cpp
  clang/unittests/Lex/LexerTest.cpp


Index: clang/unittests/Lex/LexerTest.cpp
===
--- clang/unittests/Lex/LexerTest.cpp
+++ clang/unittests/Lex/LexerTest.cpp
@@ -622,6 +622,24 @@
 "xyz", "=", "abcd", ";"));
 }
 
+TEST_F(LexerTest, FindNextTokenWithComments) {
+  Lex("int /* type */ abcd = 0; // vardecl\n"
+  "int /*other type*/ xyz = abcd; //other vardecl \n");
+  std::vector GeneratedByNextToken;
+  SourceLocation Loc =
+  SourceMgr.getLocForStartOfFile(SourceMgr.getMainFileID());
+  while (true) {
+auto T = Lexer::findNextToken(Loc, SourceMgr, LangOpts, true);
+ASSERT_TRUE(T);
+if (T->is(tok::eof))
+  break;
+GeneratedByNextToken.push_back(getSourceText(*T, *T));
+Loc = T->getLocation();
+  }
+  EXPECT_THAT(GeneratedByNextToken, ElementsAre("/* type */", "abcd", "=", 
"0", ";", "// vardecl", "int", "/*other type*/",
+"xyz", "=", "abcd", ";", 
"//other vardecl "));
+}
+
 TEST_F(LexerTest, CreatedFIDCountForPredefinedBuffer) {
   TrivialModuleLoader ModLoader;
   auto PP = CreatePP("", ModLoader);
Index: clang/lib/Lex/Lexer.cpp
===
--- clang/lib/Lex/Lexer.cpp
+++ clang/lib/Lex/Lexer.cpp
@@ -1258,7 +1258,8 @@
 
 Optional Lexer::findNextToken(SourceLocation Loc,
  const SourceManager ,
- const LangOptions ) {
+ const LangOptions ,
+ bool IncludeComments) {
   if (Loc.isMacroID()) {
 if (!Lexer::isAtEndOfMacroExpansion(Loc, SM, LangOpts, ))
   return None;
@@ -1274,11 +1275,13 @@
   if (InvalidTemp)
 return None;
 
-  const char *TokenBegin = File.data() + LocInfo.second;
+  const char *const TokenBegin = File.data() + LocInfo.second;
 
   // Lex from the start of the given location.
   Lexer lexer(SM.getLocForStartOfFile(LocInfo.first), LangOpts, File.begin(),
   TokenBegin, File.end());
+  lexer.SetCommentRetentionState(IncludeComments);
+
   // Find the token.
   Token Tok;
   lexer.LexFromRawLexer(Tok);
Index: clang/include/clang/Lex/Lexer.h
===
--- clang/include/clang/Lex/Lexer.h
+++ clang/include/clang/Lex/Lexer.h
@@ -554,7 +554,8 @@
   /// Returns the next token, or none if the location is inside a macro.
   static Optional findNextToken(SourceLocation Loc,
const SourceManager ,
-   const LangOptions );
+   const LangOptions ,
+   bool IncludeComments = false);
 
   /// Checks that the given token is the first token that occurs after
   /// the given location (this excludes comments and whitespace). Returns the


Index: clang/unittests/Lex/LexerTest.cpp
===
--- clang/unittests/Lex/LexerTest.cpp
+++ clang/unittests/Lex/LexerTest.cpp
@@ -622,6 +622,24 @@
 "xyz", "=", "abcd", ";"));
 }
 
+TEST_F(LexerTest, FindNextTokenWithComments) {
+  Lex("int /* type */ abcd = 0; // vardecl\n"
+  "int /*other type*/ xyz = abcd; //other vardecl \n");
+  std::vector GeneratedByNextToken;
+  SourceLocation Loc =
+  SourceMgr.getLocForStartOfFile(SourceMgr.getMainFileID());
+  while (true) {
+auto T = Lexer::findNextToken(Loc, SourceMgr, LangOpts, true);
+ASSERT_TRUE(T);
+if (T->is(tok::eof))
+  break;
+GeneratedByNextToken.push_back(getSourceText(*T, *T));
+Loc = T->getLocation();
+  }
+  EXPECT_THAT(GeneratedByNextToken, ElementsAre("/* type */", "abcd", "=", "0", ";", "// vardecl", "int", "/*other type*/",
+"xyz", "=", "abcd", ";", "//other vardecl "));
+}
+
 TEST_F(LexerTest, CreatedFIDCountForPredefinedBuffer) {
   TrivialModuleLoader ModLoader;
   auto PP = CreatePP("", ModLoader);
Index: clang/lib/Lex/Lexer.cpp
===
--- clang/lib/Lex/Lexer.cpp
+++ clang/lib/Lex/Lexer.cpp
@@ -1258,7 +1258,8 @@
 
 Optional Lexer::findNextToken(SourceLocation Loc,
  const SourceManager ,
- 

[PATCH] D133436: Ground work for cuda-related checks in clang-tidy

[Bartłomiej Cieślar via Phabricator via cfe-commits]

barcisz added inline comments.



Comment at: clang-tools-extra/clang-tidy/cuda/CudaTidyModule.cpp:25
+
+// Register the GoogleTidyModule using this statically initialized variable.
+static ClangTidyModuleRegistry::Add

tschuett wrote:
> Is Google a copy and paste error?
Done



Comment at: clang-tools-extra/clang-tidy/cuda/CudaTidyModule.cpp:32
+// This anchor is used to force the linker to link in the generated object file
+// and thus register the GoogleModule.
+volatile int CudaModuleAnchorSource = 0;

tschuett wrote:
> Is Google a copy and paste error?
Done



Comment at: clang-tools-extra/test/lit.cfg.py:19
 # suffixes: A list of file extensions to treat as test files.
-config.suffixes = ['.c', '.cpp', '.hpp', '.m', '.mm', '.cu', '.ll', '.cl', 
'.s',
+config.suffixes = ['.c', '.cpp', '.cu', '.hpp', '.m', '.mm', '.cu', '.ll', 
'.cl', '.s',
   '.modularize', '.module-map-checker', '.test']

ivanmurashko wrote:
> `.cu` is specified several times there. It looks like the change is not 
> necessary at the line
Done


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133436/new/

https://reviews.llvm.org/D133436

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133436: Ground work for cuda-related checks in clang-tidy

[Bartłomiej Cieślar via Phabricator via cfe-commits]

barcisz updated this revision to Diff 458504.
barcisz added a comment.

removed duplication of '.cu' in the lit config for clang-tidy tests


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133436/new/

https://reviews.llvm.org/D133436

Files:
  clang-tools-extra/clang-tidy/CMakeLists.txt
  clang-tools-extra/clang-tidy/ClangTidyForceLinker.h
  clang-tools-extra/clang-tidy/cuda/CMakeLists.txt
  clang-tools-extra/clang-tidy/cuda/CudaTidyModule.cpp
  clang-tools-extra/test/clang-tidy/check_clang_tidy.py
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda-initializers.h
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda.h
  clang-tools-extra/test/clang-tidy/checkers/cuda/.keep

Index: clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda.h
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda.h
@@ -0,0 +1,28 @@
+/* Minimal declarations for CUDA support.  Testing purposes only. */
+
+#include 
+
+#define __constant__ __attribute__((constant))
+#define __device__ __attribute__((device))
+#define __global__ __attribute__((global))
+#define __host__ __attribute__((host))
+#define __shared__ __attribute__((shared))
+
+struct dim3 {
+  unsigned x, y, z;
+  __host__ __device__ dim3(unsigned x, unsigned y = 1, unsigned z = 1) : x(x), y(y), z(z) {}
+};
+
+typedef struct cudaStream *cudaStream_t;
+typedef enum cudaError {} cudaError_t;
+extern "C" int cudaConfigureCall(dim3 gridSize, dim3 blockSize,
+ size_t sharedSize = 0,
+ cudaStream_t stream = 0);
+extern "C" int __cudaPushCallConfiguration(dim3 gridSize, dim3 blockSize,
+   size_t sharedSize = 0,
+   cudaStream_t stream = 0);
+extern "C" cudaError_t cudaLaunchKernel(const void *func, dim3 gridDim,
+dim3 blockDim, void **args,
+size_t sharedMem, cudaStream_t stream);
+
+extern "C" __device__ int printf(const char*, ...);
Index: clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda-initializers.h
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda-initializers.h
@@ -0,0 +1,145 @@
+// CUDA struct types with interesting initialization properties.
+// Keep in sync with clang/test/SemaCUDA/Inputs/cuda-initializers.h.
+
+// Base classes with different initializer variants.
+
+// trivial constructor -- allowed
+struct T {
+  int t;
+};
+
+// empty constructor
+struct EC {
+  int ec;
+  __device__ EC() {} // -- allowed
+  __device__ EC(int) {}  // -- not allowed
+};
+
+// empty destructor
+struct ED {
+  __device__ ~ED() {} // -- allowed
+};
+
+struct ECD {
+  __device__ ECD() {} // -- allowed
+  __device__ ~ECD() {}// -- allowed
+};
+
+// empty templated constructor -- allowed with no arguments
+struct ETC {
+  template  __device__ ETC(T...) {}
+};
+
+// undefined constructor -- not allowed
+struct UC {
+  int uc;
+  __device__ UC();
+};
+
+// undefined destructor -- not allowed
+struct UD {
+  int ud;
+  __device__ ~UD();
+};
+
+// empty constructor w/ initializer list -- not allowed
+struct ECI {
+  int eci;
+  __device__ ECI() : eci(1) {}
+};
+
+// non-empty constructor -- not allowed
+struct NEC {
+  int nec;
+  __device__ NEC() { nec = 1; }
+};
+
+// non-empty destructor -- not allowed
+struct NED {
+  int ned;
+  __device__ ~NED() { ned = 1; }
+};
+
+// no-constructor,  virtual method -- not allowed
+struct NCV {
+  int ncv;
+  __device__ virtual void vm() {}
+};
+
+// virtual destructor -- not allowed.
+struct VD {
+  __device__ virtual ~VD() {}
+};
+
+// dynamic in-class field initializer -- not allowed
+__device__ int f();
+struct NCF {
+  int ncf = f();
+};
+
+// static in-class field initializer.  NVCC does not allow it, but
+// clang generates static initializer for this, so we'll accept it.
+// We still can't use it on __shared__ vars as they don't allow *any*
+// initializers.
+struct NCFS {
+  int ncfs = 3;
+};
+
+// undefined templated constructor -- not allowed
+struct UTC {
+  template  __device__ UTC(T...);
+};
+
+// non-empty templated constructor -- not allowed
+struct NETC {
+  int netc;
+  template  __device__ NETC(T...) { netc = 1; }
+};
+
+// Regular base class -- allowed
+struct T_B_T : T {};
+
+// Incapsulated object of allowed class -- allowed
+struct T_F_T {
+  T t;
+};
+
+// array of allowed objects -- allowed
+struct T_FA_T {
+  T t[2];
+};
+
+
+// Calling empty base class initializer is OK
+struct EC_I_EC : EC {
+  __device__ EC_I_EC() : EC() {}
+};
+
+// .. though passing arguments is not allowed.
+struct EC_I_EC1 : EC {
+  __device__ EC_I_EC1() : EC(1) {}
+};
+
+// Virtual base class -- not 

[PATCH] D133436: Ground work for cuda-related checks in clang-tidy

[Bartłomiej Cieślar via Phabricator via cfe-commits]

barcisz updated this revision to Diff 458502.
barcisz added a comment.

Fixed a copy-paste error with Google->Cuda in CudaTidyModule.cpp


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133436/new/

https://reviews.llvm.org/D133436

Files:
  clang-tools-extra/clang-tidy/CMakeLists.txt
  clang-tools-extra/clang-tidy/ClangTidyForceLinker.h
  clang-tools-extra/clang-tidy/cuda/CMakeLists.txt
  clang-tools-extra/clang-tidy/cuda/CudaTidyModule.cpp
  clang-tools-extra/test/clang-tidy/check_clang_tidy.py
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda-initializers.h
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda.h
  clang-tools-extra/test/clang-tidy/checkers/cuda/.keep
  clang-tools-extra/test/lit.cfg.py

Index: clang-tools-extra/test/lit.cfg.py
===
--- clang-tools-extra/test/lit.cfg.py
+++ clang-tools-extra/test/lit.cfg.py
@@ -16,7 +16,7 @@
 config.test_format = lit.formats.ShTest(not llvm_config.use_lit_shell)
 
 # suffixes: A list of file extensions to treat as test files.
-config.suffixes = ['.c', '.cpp', '.hpp', '.m', '.mm', '.cu', '.ll', '.cl', '.s',
+config.suffixes = ['.c', '.cpp', '.cu', '.hpp', '.m', '.mm', '.cu', '.ll', '.cl', '.s',
   '.modularize', '.module-map-checker', '.test']
 
 # Test-time dependencies located in directories called 'Inputs' are excluded
Index: clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda.h
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda.h
@@ -0,0 +1,28 @@
+/* Minimal declarations for CUDA support.  Testing purposes only. */
+
+#include 
+
+#define __constant__ __attribute__((constant))
+#define __device__ __attribute__((device))
+#define __global__ __attribute__((global))
+#define __host__ __attribute__((host))
+#define __shared__ __attribute__((shared))
+
+struct dim3 {
+  unsigned x, y, z;
+  __host__ __device__ dim3(unsigned x, unsigned y = 1, unsigned z = 1) : x(x), y(y), z(z) {}
+};
+
+typedef struct cudaStream *cudaStream_t;
+typedef enum cudaError {} cudaError_t;
+extern "C" int cudaConfigureCall(dim3 gridSize, dim3 blockSize,
+ size_t sharedSize = 0,
+ cudaStream_t stream = 0);
+extern "C" int __cudaPushCallConfiguration(dim3 gridSize, dim3 blockSize,
+   size_t sharedSize = 0,
+   cudaStream_t stream = 0);
+extern "C" cudaError_t cudaLaunchKernel(const void *func, dim3 gridDim,
+dim3 blockDim, void **args,
+size_t sharedMem, cudaStream_t stream);
+
+extern "C" __device__ int printf(const char*, ...);
Index: clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda-initializers.h
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda-initializers.h
@@ -0,0 +1,145 @@
+// CUDA struct types with interesting initialization properties.
+// Keep in sync with clang/test/SemaCUDA/Inputs/cuda-initializers.h.
+
+// Base classes with different initializer variants.
+
+// trivial constructor -- allowed
+struct T {
+  int t;
+};
+
+// empty constructor
+struct EC {
+  int ec;
+  __device__ EC() {} // -- allowed
+  __device__ EC(int) {}  // -- not allowed
+};
+
+// empty destructor
+struct ED {
+  __device__ ~ED() {} // -- allowed
+};
+
+struct ECD {
+  __device__ ECD() {} // -- allowed
+  __device__ ~ECD() {}// -- allowed
+};
+
+// empty templated constructor -- allowed with no arguments
+struct ETC {
+  template  __device__ ETC(T...) {}
+};
+
+// undefined constructor -- not allowed
+struct UC {
+  int uc;
+  __device__ UC();
+};
+
+// undefined destructor -- not allowed
+struct UD {
+  int ud;
+  __device__ ~UD();
+};
+
+// empty constructor w/ initializer list -- not allowed
+struct ECI {
+  int eci;
+  __device__ ECI() : eci(1) {}
+};
+
+// non-empty constructor -- not allowed
+struct NEC {
+  int nec;
+  __device__ NEC() { nec = 1; }
+};
+
+// non-empty destructor -- not allowed
+struct NED {
+  int ned;
+  __device__ ~NED() { ned = 1; }
+};
+
+// no-constructor,  virtual method -- not allowed
+struct NCV {
+  int ncv;
+  __device__ virtual void vm() {}
+};
+
+// virtual destructor -- not allowed.
+struct VD {
+  __device__ virtual ~VD() {}
+};
+
+// dynamic in-class field initializer -- not allowed
+__device__ int f();
+struct NCF {
+  int ncf = f();
+};
+
+// static in-class field initializer.  NVCC does not allow it, but
+// clang generates static initializer for this, so we'll accept it.
+// We still can't use it on __shared__ vars as they don't allow *any*
+// initializers.
+struct NCFS {
+  int ncfs = 3;
+};
+
+// undefined templated constructor -- 

[PATCH] D133436: Ground work for cuda-related checks in clang-tidy

[Bartłomiej Cieślar via Phabricator via cfe-commits]

barcisz created this revision.
Herald added subscribers: mattd, carlosgalvezp, yaxunl, mgorny.
Herald added a project: All.
barcisz requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Signed-off-by: bcieslar 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133436

Files:
  clang-tools-extra/clang-tidy/CMakeLists.txt
  clang-tools-extra/clang-tidy/ClangTidyForceLinker.h
  clang-tools-extra/clang-tidy/cuda/CMakeLists.txt
  clang-tools-extra/clang-tidy/cuda/CudaTidyModule.cpp
  clang-tools-extra/test/clang-tidy/check_clang_tidy.py
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda-initializers.h
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda.h
  clang-tools-extra/test/clang-tidy/checkers/cuda/.keep
  clang-tools-extra/test/lit.cfg.py

Index: clang-tools-extra/test/lit.cfg.py
===
--- clang-tools-extra/test/lit.cfg.py
+++ clang-tools-extra/test/lit.cfg.py
@@ -16,7 +16,7 @@
 config.test_format = lit.formats.ShTest(not llvm_config.use_lit_shell)
 
 # suffixes: A list of file extensions to treat as test files.
-config.suffixes = ['.c', '.cpp', '.hpp', '.m', '.mm', '.cu', '.ll', '.cl', '.s',
+config.suffixes = ['.c', '.cpp', '.cu', '.hpp', '.m', '.mm', '.cu', '.ll', '.cl', '.s',
   '.modularize', '.module-map-checker', '.test']
 
 # Test-time dependencies located in directories called 'Inputs' are excluded
Index: clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda.h
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda.h
@@ -0,0 +1,28 @@
+/* Minimal declarations for CUDA support.  Testing purposes only. */
+
+#include 
+
+#define __constant__ __attribute__((constant))
+#define __device__ __attribute__((device))
+#define __global__ __attribute__((global))
+#define __host__ __attribute__((host))
+#define __shared__ __attribute__((shared))
+
+struct dim3 {
+  unsigned x, y, z;
+  __host__ __device__ dim3(unsigned x, unsigned y = 1, unsigned z = 1) : x(x), y(y), z(z) {}
+};
+
+typedef struct cudaStream *cudaStream_t;
+typedef enum cudaError {} cudaError_t;
+extern "C" int cudaConfigureCall(dim3 gridSize, dim3 blockSize,
+ size_t sharedSize = 0,
+ cudaStream_t stream = 0);
+extern "C" int __cudaPushCallConfiguration(dim3 gridSize, dim3 blockSize,
+   size_t sharedSize = 0,
+   cudaStream_t stream = 0);
+extern "C" cudaError_t cudaLaunchKernel(const void *func, dim3 gridDim,
+dim3 blockDim, void **args,
+size_t sharedMem, cudaStream_t stream);
+
+extern "C" __device__ int printf(const char*, ...);
Index: clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda-initializers.h
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda-initializers.h
@@ -0,0 +1,145 @@
+// CUDA struct types with interesting initialization properties.
+// Keep in sync with clang/test/SemaCUDA/Inputs/cuda-initializers.h.
+
+// Base classes with different initializer variants.
+
+// trivial constructor -- allowed
+struct T {
+  int t;
+};
+
+// empty constructor
+struct EC {
+  int ec;
+  __device__ EC() {} // -- allowed
+  __device__ EC(int) {}  // -- not allowed
+};
+
+// empty destructor
+struct ED {
+  __device__ ~ED() {} // -- allowed
+};
+
+struct ECD {
+  __device__ ECD() {} // -- allowed
+  __device__ ~ECD() {}// -- allowed
+};
+
+// empty templated constructor -- allowed with no arguments
+struct ETC {
+  template  __device__ ETC(T...) {}
+};
+
+// undefined constructor -- not allowed
+struct UC {
+  int uc;
+  __device__ UC();
+};
+
+// undefined destructor -- not allowed
+struct UD {
+  int ud;
+  __device__ ~UD();
+};
+
+// empty constructor w/ initializer list -- not allowed
+struct ECI {
+  int eci;
+  __device__ ECI() : eci(1) {}
+};
+
+// non-empty constructor -- not allowed
+struct NEC {
+  int nec;
+  __device__ NEC() { nec = 1; }
+};
+
+// non-empty destructor -- not allowed
+struct NED {
+  int ned;
+  __device__ ~NED() { ned = 1; }
+};
+
+// no-constructor,  virtual method -- not allowed
+struct NCV {
+  int ncv;
+  __device__ virtual void vm() {}
+};
+
+// virtual destructor -- not allowed.
+struct VD {
+  __device__ virtual ~VD() {}
+};
+
+// dynamic in-class field initializer -- not allowed
+__device__ int f();
+struct NCF {
+  int ncf = f();
+};
+
+// static in-class field initializer.  NVCC does not allow it, but
+// clang generates static initializer for this, so we'll accept it.
+// We still can't use it on __shared__ vars as they don't allow *any*
+// initializers.
+struct