Re: [PATCH] D16259: Add clang-tidy readability-redundant-return check

2016-01-17 Thread Richard via cfe-commits
LegalizeAdulthood updated this revision to Diff 45110.
LegalizeAdulthood added a comment.

Extend this check to handle redundant `continue` statements in loop bodies.

With this extension, I'm wondering if the check should be renamed 
redundant-control-flow instead of redundant-return.

Comments?


http://reviews.llvm.org/D16259

Files:
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tidy/readability/RedundantReturnCheck.cpp
  clang-tidy/readability/RedundantReturnCheck.h
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-redundant-return.rst
  test/clang-tidy/readability-redundant-return.cpp

Index: test/clang-tidy/readability-redundant-return.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-redundant-return.cpp
@@ -0,0 +1,68 @@
+// RUN: %check_clang_tidy %s readability-redundant-return %t
+
+void g(int i);
+void j();
+
+void f() {
+  return;
+}
+// CHECK-MESSAGES: :[[@LINE-2]]:3: warning: redundant return statement at the end of void function [readability-redundant-return]
+// CHECK-FIXES: {{^}}void f() {{{$}}
+// CHECK-FIXES-NEXT: {{^ *}$}}
+
+void g() {
+  f();
+  return;
+}
+// CHECK-MESSAGES: :[[@LINE-2]]:3: warning: redundant return statement
+// CHECK-FIXES: {{^  }}f();{{$}}
+// CHECK-FIXES-NEXT: {{^ *}$}}
+
+void g(int i) {
+  if (i < 0) {
+return;
+  }
+  if (i < 10) {
+f();
+  }
+}
+
+int h() {
+  return 1;
+}
+
+void j() {
+}
+
+void k() {
+  for (int i = 0; i < 10; ++i) {
+continue;
+  }
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:5: warning: redundant continue statement
+// CHECK-FIXES: {{^}}  for (int i = 0; i < 10; ++i) {{{$}}
+// CHECK-FIXES-NEXT: {{^ *}$}}
+
+void m() {
+  int i = 0;
+  do {
+++i;
+continue;
+  } while (i < 10);
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:5: warning: redundant continue statement
+// CHECK-FIXES: {{^  do {$}}
+// CHECK-FIXES-NEXT: {{^}}++i;{{$}}
+// CHECK-FIXES-NEXT: {{^ *}}} while (i < 10);{{$}}
+
+void p() {
+  int i = 0;
+  while (i < 10) {
+++i;
+continue;
+  }
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:5: warning: redundant continue statement
+// CHECK-FIXES: {{^}}  while (i < 10) {{{$}}
+// CHECK-FIXES-NEXT: {{^}}++i;{{$}}
+// CHECK-FIXES-NEXT: {{^ *}$}}
Index: docs/clang-tidy/checks/readability-redundant-return.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/readability-redundant-return.rst
@@ -0,0 +1,10 @@
+.. title:: clang-tidy - readability-redundant-return
+
+readability-redundant-return
+
+
+This check looks for procedures (functions returning no value) with `return`
+statements at the end of the function.  Such `return` statements are redundant.
+
+Loop statements (`for`, `while`, `do while`) are checked for redundant
+`continue` statements at the end of the loop body.
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -76,6 +76,7 @@
readability-implicit-bool-cast
readability-inconsistent-declaration-parameter-name
readability-named-parameter
+   readability-redundant-return
readability-redundant-smartptr-get
readability-redundant-string-cstr
readability-simplify-boolean-expr
Index: clang-tidy/readability/RedundantReturnCheck.h
===
--- /dev/null
+++ clang-tidy/readability/RedundantReturnCheck.h
@@ -0,0 +1,47 @@
+//===--- RedundantReturnCheck.h - clang-tidy-*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REDUNDANT_RETURN_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REDUNDANT_RETURN_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+/// Eliminates redundant `return` statements at the end of a function that
+/// returns `void`.
+///
+/// Eliminates redundant `continue` statements at the end of a loop body.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/readability-redundant-return.html
+class RedundantReturnCheck : public ClangTidyCheck {
+public:
+  RedundantReturnCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+  void
+  checkRedundantReturn(const ast_matchers::MatchFinder::MatchResult &Result,
+   const FunctionDecl *Fn);
+
+  void
+  checkRedundantCont

Re: [PATCH] D16259: Add clang-tidy readability-redundant-return check

2016-01-17 Thread Richard via cfe-commits
LegalizeAdulthood added a comment.

I didn't know about the bug reports.  I created this check because I have 
encountered such redundant control flow in real code bases.

This would close 21984 , but not 
22416 .  22416 feels more like a 
check to eliminate redundant parenthesis, whereas this check is concerned with 
the entire control flow statement being redundant.


http://reviews.llvm.org/D16259



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


Re: [PATCH] D15305: [CUDA] Do not allow dynamic initialization of global device side variables.

2016-01-17 Thread Justin Lebar via cfe-commits
jlebar added a comment.

tra asked me to check for coverage.  Looks pretty good in that respect.



Comment at: include/clang/Basic/DiagnosticSemaKinds.td:6419
@@ +6418,3 @@
+"dynamic initialization is not supported for "
+"__device__, __constant__ and __shared__ variables.">;
+def err_shared_var_init : Error<

Nit, but, since we're all language nerds here, suggest adding an Oxford comma.


Comment at: lib/Sema/SemaCUDA.cpp:436
@@ +435,3 @@
+  if (CD->isTrivial())
+return true;
+

The test passes if I comment out this if statement.  I'm not sure if that's 
expected; this may or may not be entirely covered below.


Comment at: lib/Sema/SemaCUDA.cpp:442
@@ +441,3 @@
+  // and the function body is an empty compound statement.
+  if (!(CD->isDefined() && CD->getNumParams() == 0 && CD->hasTrivialBody()))
+return false;

Tests pass if I comment out the isDefined check.


Comment at: lib/Sema/SemaDecl.cpp:10183
@@ +10182,3 @@
+  // 7.5).  We also allow constant initializers for __constant__ and
+  // __device__ variables.
+  const Expr *Init = VD->getInit();

> We also allow constant initializers for __constant__ and __device__ variables.

Consider rephrasing this -- it sounds like this is a clang extension, but I 
just checked and it does not appear to be.


Comment at: lib/Sema/SemaDecl.cpp:10186
@@ +10185,3 @@
+  const bool IsGlobal = VD->hasGlobalStorage() && !VD->isStaticLocal();
+  if (Init && IsGlobal && getLangOpts().CUDA && getLangOpts().CUDAIsDevice &&
+  (VD->hasAttr() || VD->hasAttr() ||

Test passes if I comment out IsGlobal or CUDAIsDevice.  (I'm not sure if you 
care to test the latter, but the former seems important.)


http://reviews.llvm.org/D15305



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


Re: [PATCH] D16044: getVariableName() for MemRegion

2016-01-17 Thread Alexander Droste via cfe-commits
Alexander_Droste marked an inline comment as done.


Comment at: tools/clang/lib/StaticAnalyzer/Core/MemRegion.cpp:653
@@ +652,3 @@
+llvm::SmallString<2> intValAsString;
+IndexInArray.toString(intValAsString);
+std::string idx{intValAsString.begin(), intValAsString.end()};

xazax.hun wrote:
> What will be the content of IndexInArray in case the index is not a concrete 
> int? Will the above code work at all in that case? In case the index is a 
> variable do we want to include the name of the variable in the index?
Right, `getAs<..::ConcreteInt>()` might return a `nullptr`. 
What do you think of including the number if it is a `nonloc::ConcreteInt`,
the name if it is a `nonloc::SymbolValKind`  
(`getAs().getSymbol()`)
and else keeping the brackets empty?



Comment at: tools/clang/lib/StaticAnalyzer/Core/MemRegion.cpp:656
@@ +655,3 @@
+
+ArrayIndices = "[" + idx + "]" + ArrayIndices;
+

xazax.hun wrote:
> I do not really like this part. You could either use Twine to avoid some 
> temporal values (and you could also eliminate the string copy this way) or 
> you could construct the indices using the reverse of idx and than reverse the 
> whole string after the loop.
So would be invoking `push_back` within and `reverse` after the loop sufficient?


http://reviews.llvm.org/D16044



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


Re: [PATCH] D12761: MPI-Checker patch for Clang Static Analyzer

2016-01-17 Thread Alexander Droste via cfe-commits
Alexander_Droste added inline comments.


Comment at: 
tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPICheckerPathSensitive.cpp:107
@@ +106,3 @@
+  State = State->remove(Req.first);
+}
+  }

zaks.anna wrote:
> Alexander_Droste wrote:
> > I need `addTransition()` later to take removed requests into account.
> What you have now, is generating a fork - two successors from the initial 
> predecessor. You need to use the addTransition method that takes a 
> predecessor and pass the ErrorNode there. You should also use the State from 
> that node, not the initial state.
Isn't this covered by the first case? 
If there's an `ErrorNode` pass it to `addTransition()` else just update the 
state regarding removed requests.


http://reviews.llvm.org/D12761



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


[PATCH] D16279: [OpenMP] Parsing + sema for "target exit data" directive.

2016-01-17 Thread Arpith Jacob via cfe-commits
arpith-jacob created this revision.
arpith-jacob added reviewers: kkwli0, sfantao, hfinkel, carlo.bertolli, ABataev.
arpith-jacob added subscribers: cfe-commits, caomhin, fraggamuffin.

Created a distinct patch for the 'target exit data' directive.

http://reviews.llvm.org/D16279

Files:
  include/clang-c/Index.h
  include/clang/AST/RecursiveASTVisitor.h
  include/clang/AST/StmtOpenMP.h
  include/clang/Basic/OpenMPKinds.def
  include/clang/Basic/StmtNodes.td
  include/clang/Sema/Sema.h
  include/clang/Serialization/ASTBitCodes.h
  lib/AST/StmtOpenMP.cpp
  lib/AST/StmtPrinter.cpp
  lib/AST/StmtProfile.cpp
  lib/Basic/OpenMPKinds.cpp
  lib/CodeGen/CGStmt.cpp
  lib/CodeGen/CGStmtOpenMP.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/Parse/ParseOpenMP.cpp
  lib/Sema/SemaOpenMP.cpp
  lib/Sema/TreeTransform.h
  lib/Serialization/ASTReaderStmt.cpp
  lib/Serialization/ASTWriterStmt.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  test/OpenMP/nesting_of_regions.cpp
  test/OpenMP/target_exit_data_ast_print.cpp
  test/OpenMP/target_exit_data_device_messages.cpp
  test/OpenMP/target_exit_data_if_messages.cpp
  test/OpenMP/target_exit_data_map_messages.c
  tools/libclang/CIndex.cpp
  tools/libclang/CXCursor.cpp

Index: tools/libclang/CXCursor.cpp
===
--- tools/libclang/CXCursor.cpp
+++ tools/libclang/CXCursor.cpp
@@ -603,6 +603,9 @@
   case Stmt::OMPTargetEnterDataDirectiveClass:
 K = CXCursor_OMPTargetEnterDataDirective;
 break;
+  case Stmt::OMPTargetExitDataDirectiveClass:
+K = CXCursor_OMPTargetExitDataDirective;
+break;
   case Stmt::OMPTeamsDirectiveClass:
 K = CXCursor_OMPTeamsDirective;
 break;
Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -1954,6 +1954,7 @@
   void VisitOMPTargetDirective(const OMPTargetDirective *D);
   void VisitOMPTargetDataDirective(const OMPTargetDataDirective *D);
   void VisitOMPTargetEnterDataDirective(const OMPTargetEnterDataDirective *D);
+  void VisitOMPTargetExitDataDirective(const OMPTargetExitDataDirective *D);
   void VisitOMPTeamsDirective(const OMPTeamsDirective *D);
   void VisitOMPTaskLoopDirective(const OMPTaskLoopDirective *D);
   void VisitOMPTaskLoopSimdDirective(const OMPTaskLoopSimdDirective *D);
@@ -2634,6 +2635,11 @@
   VisitOMPExecutableDirective(D);
 }
 
+void EnqueueVisitor::VisitOMPTargetExitDataDirective(
+const OMPTargetExitDataDirective *D) {
+  VisitOMPExecutableDirective(D);
+}
+
 void EnqueueVisitor::VisitOMPTeamsDirective(const OMPTeamsDirective *D) {
   VisitOMPExecutableDirective(D);
 }
@@ -4518,6 +4524,8 @@
 return cxstring::createRef("OMPTargetDataDirective");
   case CXCursor_OMPTargetEnterDataDirective:
 return cxstring::createRef("OMPTargetEnterDataDirective");
+  case CXCursor_OMPTargetExitDataDirective:
+return cxstring::createRef("OMPTargetExitDataDirective");
   case CXCursor_OMPTeamsDirective:
 return cxstring::createRef("OMPTeamsDirective");
   case CXCursor_OMPCancellationPointDirective:
Index: test/OpenMP/target_exit_data_map_messages.c
===
--- /dev/null
+++ test/OpenMP/target_exit_data_map_messages.c
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0 -verify -fopenmp -ferror-limit 100 -o - %s
+
+int main(int argc, char **argv) {
+
+  int r;
+  #pragma omp target exit data // expected-error {{expected at least one map clause for '#pragma omp target exit data'}}
+
+  #pragma omp target exit data map(tofrom: r) // expected-error {{map type 'tofrom' is not allowed for '#pragma omp target exit data'}}
+
+  #pragma omp target exit data map(always, from: r)
+  #pragma omp target exit data map(delete: r)
+  #pragma omp target exit data map(release: r)
+  #pragma omp target exit data map(always, alloc: r) // expected-error {{map type 'alloc' is not allowed for '#pragma omp target exit data'}}
+  #pragma omp target exit data map(to: r) // expected-error {{map type 'to' is not allowed for '#pragma omp target exit data'}}
+
+  return 0;
+}
Index: test/OpenMP/target_exit_data_if_messages.cpp
===
--- /dev/null
+++ test/OpenMP/target_exit_data_if_messages.cpp
@@ -0,0 +1,35 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0 -verify -fopenmp -ferror-limit 100 -o - %s
+
+void foo() {
+}
+
+bool foobool(int argc) {
+  return argc;
+}
+
+struct S1; // expected-note {{declared here}}
+
+int main(int argc, char **argv) {
+  int i;
+  #pragma omp target exit data map(from: i) if // expected-error {{expected '(' after 'if'}}
+  #pragma omp target exit data map(from: i) if ( // expected-error {{expected expression}} expected-error {{expected ')'}} expected-note {{to match this '('}}
+  #pragma omp target exit data map(from: i) if () // expected-error {{expected expression}}
+  #pragma omp target exit d

[PATCH] D16280: [OpenMP] Detect implicit map type to report unspecified map type for target enter/exit data directives.

2016-01-17 Thread Arpith Jacob via cfe-commits
arpith-jacob created this revision.
arpith-jacob added reviewers: kkwli0, ABataev, sfantao, hfinkel, carlo.bertolli.
arpith-jacob added subscribers: cfe-commits, caomhin, fraggamuffin.

Support for the following OpenMP 4.5 restriction on 'target enter data' and 
'target exit data':
- A map-type must be specified in all map clauses.

I have to save 'IsMapTypeImplicit' when parsing a map clause to support this 
constraint and for more informative error messages. This helps me support the 
following case:

#pragma omp target enter data map(r) // expected-error {{map type must be 
specified for '#pragma omp target enter data'}}

and distinguish it from:

#pragma omp target enter data map(tofrom: r) // expected-error {{map type 
'tofrom' is not allowed for '#pragma omp target enter data'}}


http://reviews.llvm.org/D16280

Files:
  include/clang/AST/OpenMPClause.h
  include/clang/Sema/Sema.h
  lib/AST/OpenMPClause.cpp
  lib/Parse/ParseOpenMP.cpp
  lib/Sema/SemaOpenMP.cpp
  lib/Sema/TreeTransform.h
  test/OpenMP/target_enter_data_map_messages.c
  test/OpenMP/target_exit_data_map_messages.c

Index: test/OpenMP/target_exit_data_map_messages.c
===
--- test/OpenMP/target_exit_data_map_messages.c
+++ test/OpenMP/target_exit_data_map_messages.c
@@ -5,6 +5,7 @@
   int r;
   #pragma omp target exit data // expected-error {{expected at least one map clause for '#pragma omp target exit data'}}
 
+  #pragma omp target exit data map(r) // expected-error {{map type must be specified for '#pragma omp target exit data'}}
   #pragma omp target exit data map(tofrom: r) // expected-error {{map type 'tofrom' is not allowed for '#pragma omp target exit data'}}
 
   #pragma omp target exit data map(always, from: r)
Index: test/OpenMP/target_enter_data_map_messages.c
===
--- test/OpenMP/target_enter_data_map_messages.c
+++ test/OpenMP/target_enter_data_map_messages.c
@@ -5,6 +5,7 @@
   int r;
   #pragma omp target enter data // expected-error {{expected at least one map clause for '#pragma omp target enter data'}}
 
+  #pragma omp target enter data map(r) // expected-error {{map type must be specified for '#pragma omp target enter data'}}
   #pragma omp target enter data map(tofrom: r) // expected-error {{map type 'tofrom' is not allowed for '#pragma omp target enter data'}}
 
   #pragma omp target enter data map(always, to: r)
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -1660,10 +1660,11 @@
   /// Subclasses may override this routine to provide different behavior.
   OMPClause *RebuildOMPMapClause(
   OpenMPMapClauseKind MapTypeModifier, OpenMPMapClauseKind MapType,
-  SourceLocation MapLoc, SourceLocation ColonLoc, ArrayRef VarList,
-  SourceLocation StartLoc, SourceLocation LParenLoc,
-  SourceLocation EndLoc) {
-return getSema().ActOnOpenMPMapClause(MapTypeModifier, MapType, MapLoc,
+  bool IsMapTypeImplicit, SourceLocation MapLoc, SourceLocation ColonLoc,
+  ArrayRef VarList, SourceLocation StartLoc,
+  SourceLocation LParenLoc, SourceLocation EndLoc) {
+return getSema().ActOnOpenMPMapClause(MapTypeModifier, MapType,
+  IsMapTypeImplicit, MapLoc,
   ColonLoc, VarList,StartLoc,
   LParenLoc, EndLoc);
   }
@@ -7847,9 +7848,9 @@
 Vars.push_back(EVar.get());
   }
   return getDerived().RebuildOMPMapClause(
-  C->getMapTypeModifier(), C->getMapType(), C->getMapLoc(),
-  C->getColonLoc(), Vars, C->getLocStart(), C->getLParenLoc(),
-  C->getLocEnd());
+  C->getMapTypeModifier(), C->getMapType(), C->isImplicitMapType(),
+  C->getMapLoc(), C->getColonLoc(), Vars, C->getLocStart(),
+  C->getLParenLoc(), C->getLocEnd());
 }
 
 template 
Index: lib/Sema/SemaOpenMP.cpp
===
--- lib/Sema/SemaOpenMP.cpp
+++ lib/Sema/SemaOpenMP.cpp
@@ -6504,7 +6504,8 @@
 SourceLocation EndLoc, CXXScopeSpec &ReductionIdScopeSpec,
 const DeclarationNameInfo &ReductionId, OpenMPDependClauseKind DepKind,
 OpenMPLinearClauseKind LinKind, OpenMPMapClauseKind MapTypeModifier, 
-OpenMPMapClauseKind MapType, SourceLocation DepLinMapLoc) {
+OpenMPMapClauseKind MapType, bool IsMapTypeImplicit,
+SourceLocation DepLinMapLoc) {
   OMPClause *Res = nullptr;
   switch (Kind) {
   case OMPC_private:
@@ -6545,8 +6546,9 @@
   StartLoc, LParenLoc, EndLoc);
 break;
   case OMPC_map:
-Res = ActOnOpenMPMapClause(MapTypeModifier, MapType, DepLinMapLoc, ColonLoc,
-   VarList, StartLoc, LParenLoc, EndLoc);
+Res = ActOnOpenMPMapClause(MapTypeModifier, MapType, IsMapTypeImplicit,
+   

Re: [PATCH] D12761: MPI-Checker patch for Clang Static Analyzer

2016-01-17 Thread Alexander Droste via cfe-commits
Alexander_Droste updated this revision to Diff 45126.
Alexander_Droste added a comment.

- removed AST related functionality from patch, as AST-based checks will be 
integrated into clang tidy
- fix `checkMissingWaits()`
- moved `getVariableName()` from patch (currently under review here: 
http://reviews.llvm.org/D16044)
- moved `is_contained()` from patch (currently under review here: 
http://reviews.llvm.org/D16053)


http://reviews.llvm.org/D12761

Files:
  tools/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
  tools/clang/lib/StaticAnalyzer/Checkers/Checkers.td
  tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIBugReporter.cpp
  tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIBugReporter.h
  tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.cpp
  
tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPICheckerPathSensitive.cpp
  tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPICheckerPathSensitive.h
  tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIFunctionClassifier.cpp
  tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIFunctionClassifier.h
  tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPITypes.h
  tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/Utility.cpp
  tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/Utility.h
  tools/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
  tools/clang/test/Analysis/MPIChecker.cpp
  tools/clang/unittests/StaticAnalyzer/CMakeLists.txt

Index: tools/clang/unittests/StaticAnalyzer/CMakeLists.txt
===
--- tools/clang/unittests/StaticAnalyzer/CMakeLists.txt
+++ tools/clang/unittests/StaticAnalyzer/CMakeLists.txt
@@ -9,5 +9,5 @@
 target_link_libraries(StaticAnalysisTests
   clangBasic
   clangAnalysis
-  clangStaticAnalyzerCore 
+  clangStaticAnalyzerCore
   )
Index: tools/clang/test/Analysis/MPIChecker.cpp
===
--- tools/clang/test/Analysis/MPIChecker.cpp
+++ tools/clang/test/Analysis/MPIChecker.cpp
@@ -0,0 +1,267 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=mpi.MPI-Checker -verify %s
+
+// MPI-Checker makes no assumptions about details of an MPI implementation.
+// Typedefs and constants are compared as strings.
+
+#define NULL 0
+
+// mock types
+typedef int MPI_Datatype;
+typedef int MPI_Comm;
+typedef int MPI_Request;
+typedef int MPI_Status;
+typedef int MPI_Op;
+typedef int int8_t;
+typedef int uint8_t;
+typedef int uint16_t;
+typedef int int64_t;
+namespace std { template struct complex { T real; T imag; }; }
+
+// mock constants
+#define MPI_DATATYPE_NULL 0
+#define MPI_CHAR 0
+#define MPI_BYTE 0
+#define MPI_INT 0
+#define MPI_LONG 0
+#define MPI_LONG_DOUBLE 0
+#define MPI_UNSIGNED 0
+#define MPI_INT8_T 0
+#define MPI_UINT8_T 0
+#define MPI_UINT16_T 0
+#define MPI_C_LONG_DOUBLE_COMPLEX 0
+#define MPI_FLOAT 0
+#define MPI_DOUBLE 0
+#define MPI_CXX_BOOL 0
+#define MPI_CXX_FLOAT_COMPLEX 0
+#define MPI_CXX_DOUBLE_COMPLEX 0
+#define MPI_CXX_LONG_DOUBLE_COMPLEX 0
+#define MPI_IN_PLACE 0
+#define MPI_COMM_WORLD 0
+#define MPI_STATUS_IGNORE 0
+#define MPI_STATUSES_IGNORE 0
+#define MPI_SUM 0
+
+int MPI_Comm_size(MPI_Comm, int *);
+int MPI_Comm_rank(MPI_Comm, int *);
+int MPI_Send(const void *, int, MPI_Datatype, int, int, MPI_Comm);
+int MPI_Recv(void *, int, MPI_Datatype, int, int, MPI_Comm, MPI_Status *);
+int MPI_Isend(const void *, int, MPI_Datatype, int, int, MPI_Comm,
+MPI_Request *);
+int MPI_Irecv(void *, int, MPI_Datatype, int, int, MPI_Comm, MPI_Request *);
+int MPI_Wait(MPI_Request *, MPI_Status *);
+int MPI_Waitall(int, MPI_Request[], MPI_Status[]);
+int MPI_Reduce(const void *, void *, int, MPI_Datatype, MPI_Op, int, MPI_Comm);
+int MPI_Ireduce(const void *, void *, int, MPI_Datatype, MPI_Op, int, MPI_Comm,
+MPI_Request *);
+int MPI_Bcast(void *, int count, MPI_Datatype, int, MPI_Comm);
+
+//integration tests---
+
+void matchedWait1() {
+  int rank = 0;
+  double buf = 0;
+  MPI_Comm_rank(MPI_COMM_WORLD, &rank);
+  if (rank >= 0) {
+MPI_Request sendReq1, recvReq1;
+MPI_Isend(&buf, 1, MPI_DOUBLE, rank + 1, 3, MPI_COMM_WORLD, &sendReq1);
+MPI_Irecv(&buf, 1, MPI_DOUBLE, rank - 1, 3, MPI_COMM_WORLD, &recvReq1);
+
+MPI_Wait(&sendReq1, MPI_STATUS_IGNORE);
+MPI_Wait(&recvReq1, MPI_STATUS_IGNORE);
+  }
+} // no error
+
+void matchedWait2() {
+  int rank = 0;
+  double buf = 0;
+  MPI_Comm_rank(MPI_COMM_WORLD, &rank);
+  if (rank >= 0) {
+MPI_Request sendReq1, recvReq1;
+MPI_Isend(&buf, 1, MPI_DOUBLE, rank + 1, 4, MPI_COMM_WORLD, &sendReq1);
+MPI_Irecv(&buf, 1, MPI_DOUBLE, rank - 1, 4, MPI_COMM_WORLD, &recvReq1);
+MPI_Wait(&sendReq1, MPI_STATUS_IGNORE);
+MPI_Wait(&recvReq1, MPI_STATUS_IGNORE);
+  }
+} // no error
+
+void matchedWait3() {
+  int rank = 0;
+  double buf = 0;
+  MPI_Comm_rank(MPI_COMM_WORLD, &rank);
+  if (rank >= 0) {
+MPI_Request sendReq1, recvReq1;
+MPI_Isend(&

Re: [PATCH] D12761: MPI-Checker patch for Clang Static Analyzer

2016-01-17 Thread Alexander Droste via cfe-commits
Alexander_Droste updated this revision to Diff 45129.
Alexander_Droste added a comment.

- removed files not being part of this patch anymore


http://reviews.llvm.org/D12761

Files:
  tools/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
  tools/clang/lib/StaticAnalyzer/Checkers/Checkers.td
  tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIBugReporter.cpp
  tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIBugReporter.h
  tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.cpp
  
tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPICheckerPathSensitive.cpp
  tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPICheckerPathSensitive.h
  tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIFunctionClassifier.cpp
  tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIFunctionClassifier.h
  tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPITypes.h
  tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/Utility.cpp
  tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/Utility.h
  tools/clang/test/Analysis/MPIChecker.cpp

Index: tools/clang/test/Analysis/MPIChecker.cpp
===
--- tools/clang/test/Analysis/MPIChecker.cpp
+++ tools/clang/test/Analysis/MPIChecker.cpp
@@ -0,0 +1,267 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=mpi.MPI-Checker -verify %s
+
+// MPI-Checker makes no assumptions about details of an MPI implementation.
+// Typedefs and constants are compared as strings.
+
+#define NULL 0
+
+// mock types
+typedef int MPI_Datatype;
+typedef int MPI_Comm;
+typedef int MPI_Request;
+typedef int MPI_Status;
+typedef int MPI_Op;
+typedef int int8_t;
+typedef int uint8_t;
+typedef int uint16_t;
+typedef int int64_t;
+namespace std { template struct complex { T real; T imag; }; }
+
+// mock constants
+#define MPI_DATATYPE_NULL 0
+#define MPI_CHAR 0
+#define MPI_BYTE 0
+#define MPI_INT 0
+#define MPI_LONG 0
+#define MPI_LONG_DOUBLE 0
+#define MPI_UNSIGNED 0
+#define MPI_INT8_T 0
+#define MPI_UINT8_T 0
+#define MPI_UINT16_T 0
+#define MPI_C_LONG_DOUBLE_COMPLEX 0
+#define MPI_FLOAT 0
+#define MPI_DOUBLE 0
+#define MPI_CXX_BOOL 0
+#define MPI_CXX_FLOAT_COMPLEX 0
+#define MPI_CXX_DOUBLE_COMPLEX 0
+#define MPI_CXX_LONG_DOUBLE_COMPLEX 0
+#define MPI_IN_PLACE 0
+#define MPI_COMM_WORLD 0
+#define MPI_STATUS_IGNORE 0
+#define MPI_STATUSES_IGNORE 0
+#define MPI_SUM 0
+
+int MPI_Comm_size(MPI_Comm, int *);
+int MPI_Comm_rank(MPI_Comm, int *);
+int MPI_Send(const void *, int, MPI_Datatype, int, int, MPI_Comm);
+int MPI_Recv(void *, int, MPI_Datatype, int, int, MPI_Comm, MPI_Status *);
+int MPI_Isend(const void *, int, MPI_Datatype, int, int, MPI_Comm,
+MPI_Request *);
+int MPI_Irecv(void *, int, MPI_Datatype, int, int, MPI_Comm, MPI_Request *);
+int MPI_Wait(MPI_Request *, MPI_Status *);
+int MPI_Waitall(int, MPI_Request[], MPI_Status[]);
+int MPI_Reduce(const void *, void *, int, MPI_Datatype, MPI_Op, int, MPI_Comm);
+int MPI_Ireduce(const void *, void *, int, MPI_Datatype, MPI_Op, int, MPI_Comm,
+MPI_Request *);
+int MPI_Bcast(void *, int count, MPI_Datatype, int, MPI_Comm);
+
+//integration tests---
+
+void matchedWait1() {
+  int rank = 0;
+  double buf = 0;
+  MPI_Comm_rank(MPI_COMM_WORLD, &rank);
+  if (rank >= 0) {
+MPI_Request sendReq1, recvReq1;
+MPI_Isend(&buf, 1, MPI_DOUBLE, rank + 1, 3, MPI_COMM_WORLD, &sendReq1);
+MPI_Irecv(&buf, 1, MPI_DOUBLE, rank - 1, 3, MPI_COMM_WORLD, &recvReq1);
+
+MPI_Wait(&sendReq1, MPI_STATUS_IGNORE);
+MPI_Wait(&recvReq1, MPI_STATUS_IGNORE);
+  }
+} // no error
+
+void matchedWait2() {
+  int rank = 0;
+  double buf = 0;
+  MPI_Comm_rank(MPI_COMM_WORLD, &rank);
+  if (rank >= 0) {
+MPI_Request sendReq1, recvReq1;
+MPI_Isend(&buf, 1, MPI_DOUBLE, rank + 1, 4, MPI_COMM_WORLD, &sendReq1);
+MPI_Irecv(&buf, 1, MPI_DOUBLE, rank - 1, 4, MPI_COMM_WORLD, &recvReq1);
+MPI_Wait(&sendReq1, MPI_STATUS_IGNORE);
+MPI_Wait(&recvReq1, MPI_STATUS_IGNORE);
+  }
+} // no error
+
+void matchedWait3() {
+  int rank = 0;
+  double buf = 0;
+  MPI_Comm_rank(MPI_COMM_WORLD, &rank);
+  if (rank >= 0) {
+MPI_Request sendReq1, recvReq1;
+MPI_Isend(&buf, 1, MPI_DOUBLE, rank + 1, 5, MPI_COMM_WORLD, &sendReq1);
+MPI_Irecv(&buf, 1, MPI_DOUBLE, rank - 1, 5, MPI_COMM_WORLD, &recvReq1);
+
+if (rank > 1000) {
+  MPI_Wait(&sendReq1, MPI_STATUS_IGNORE);
+  MPI_Wait(&recvReq1, MPI_STATUS_IGNORE);
+} else {
+  MPI_Wait(&sendReq1, MPI_STATUS_IGNORE);
+  MPI_Wait(&recvReq1, MPI_STATUS_IGNORE);
+}
+  }
+} // no error
+
+void missingWait1() { // Check missing wait for dead region.
+  int rank = 0;
+  double buf = 0;
+  MPI_Request sendReq1;
+  MPI_Comm_rank(MPI_COMM_WORLD, &rank);
+  MPI_Ireduce(MPI_IN_PLACE, &buf, 1, MPI_DOUBLE, MPI_SUM, 0, MPI_COMM_WORLD,
+  &sendReq1); // Request is previously used by nonblocking call here.
+} // expected-warning{{Request 'sendReq1' has no match

Re: [PATCH] D16259: Add clang-tidy readability-redundant-return check

2016-01-17 Thread Eugene Zelenko via cfe-commits
Eugene.Zelenko added a comment.

In http://reviews.llvm.org/D16259#328834, @LegalizeAdulthood wrote:

> I didn't know about the bug reports.  I created this check because I have 
> encountered such redundant control flow in real code bases.


Bug report was reflection on my code base, but I also spotted similar patterns 
in LLVM.

There are a lot of Clang-tidy enhancement ideas in Bugzilla, so may be you 
could find some interesting enough to be implemented?


http://reviews.llvm.org/D16259



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


Re: [PATCH] D16259: Add clang-tidy readability-redundant-return check

2016-01-17 Thread Richard via cfe-commits
LegalizeAdulthood added a comment.

I'm starting with stuff that I see in code bases where I work; chances are that 
other people see them too.  In the future, I'll look in the bug database for 
stuff that is similar or identical to what I'm doing.


http://reviews.llvm.org/D16259



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


Re: [PATCH] D15989: [OpenMP] Parsing + sema for "target enter data" and "target exit data" directives.

2016-01-17 Thread Alexey Bataev via cfe-commits
ABataev added inline comments.


Comment at: include/clang/Sema/Sema.h:7767-7768
@@ -7766,2 +7766,4 @@
 bool StrictlyPositive = true);
+  /// \brief Check for existence of a map clause
+  bool HasMapClause(ArrayRef Clauses);
 

I don't think it must be exposed as a member of Sema. It is enough just to make 
it static in SemaOpenMP.cpp


Comment at: lib/Parse/ParseOpenMP.cpp:175
@@ -164,3 +174,3 @@
 /// 'taskgroup' | 'teams' | 'taskloop' | 'taskloop simd' {clause} |
-/// 'distribute'
+/// 'distribute' | 'target enter data' | 'target exit data'
 /// annot_pragma_openmp_end

'target exit data' is not supported in this patch


Comment at: lib/Sema/SemaOpenMP.cpp:5929-5931
@@ +5928,5 @@
+   I != E; ++I) {
+if (*I) {
+  OMPClause *Clause = *I;
+  if (Clause->getClauseKind() == OMPC_map)
+return true;

just if(*I != nullptr && (*I)->getClauseKind() == OMPC_map)


Comment at: lib/Sema/SemaOpenMP.cpp:8525
@@ +8524,3 @@
+  // TODO: Need to determine if map type is implicitly determined
+  (0) <<
+  getOpenMPSimpleClauseTypeName(OMPC_map, MapType) <<

Remove parens around 0


http://reviews.llvm.org/D15989



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


Re: [PATCH] D16280: [OpenMP] Detect implicit map type to report unspecified map type for target enter/exit data directives.

2016-01-17 Thread Alexey Bataev via cfe-commits
ABataev added inline comments.


Comment at: lib/Sema/SemaOpenMP.cpp:8587
@@ -8583,4 +8586,3 @@
   Diag(StartLoc, diag::err_omp_invalid_map_type_for_directive) <<
-  // TODO: Need to determine if map type is implicitly determined
-  (0) <<
+  static_cast(IsMapTypeImplicit) <<
   getOpenMPSimpleClauseTypeName(OMPC_map, MapType) <<

Do not cast bool to unsigned, use (IsMapTypeImplicit ? 1 : 0) instead


Comment at: lib/Sema/SemaOpenMP.cpp:8603
@@ -8600,4 +8602,3 @@
   Diag(StartLoc, diag::err_omp_invalid_map_type_for_directive) <<
-  // TODO: Need to determine if map type is implicitly determined
-  (0) <<
+  static_cast(IsMapTypeImplicit) <<
   getOpenMPSimpleClauseTypeName(OMPC_map, MapType) <<

The same


http://reviews.llvm.org/D16280



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


Re: [PATCH] D11182: [OPENMP 4.0] Initial support for 'omp declare reduction' construct.

2016-01-17 Thread Alexey Bataev via cfe-commits
ABataev updated this revision to Diff 45149.
ABataev added a comment.

Simplified handling of private copies in C/C++. Now all outpup parameters of 
UDR must be passed by pointer, not by reference (better compatibility with C)


http://reviews.llvm.org/D11182

Files:
  include/clang/AST/DeclBase.h
  include/clang/AST/DeclCXX.h
  include/clang/AST/DeclOpenMP.h
  include/clang/AST/RecursiveASTVisitor.h
  include/clang/Basic/DeclNodes.td
  include/clang/Basic/DiagnosticParseKinds.td
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Basic/OpenMPKinds.def
  include/clang/Parse/Parser.h
  include/clang/Sema/ScopeInfo.h
  include/clang/Sema/Sema.h
  include/clang/Serialization/ASTBitCodes.h
  lib/AST/CXXInheritance.cpp
  lib/AST/Decl.cpp
  lib/AST/DeclBase.cpp
  lib/AST/DeclOpenMP.cpp
  lib/AST/DeclPrinter.cpp
  lib/AST/ItaniumMangle.cpp
  lib/AST/MicrosoftMangle.cpp
  lib/Basic/OpenMPKinds.cpp
  lib/CodeGen/CGDecl.cpp
  lib/CodeGen/CodeGenModule.cpp
  lib/CodeGen/CodeGenModule.h
  lib/Parse/ParseDecl.cpp
  lib/Parse/ParseDeclCXX.cpp
  lib/Parse/ParseOpenMP.cpp
  lib/Parse/Parser.cpp
  lib/Sema/ScopeInfo.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaLookup.cpp
  lib/Sema/SemaOpenMP.cpp
  lib/Sema/SemaTemplateInstantiateDecl.cpp
  lib/Serialization/ASTCommon.cpp
  lib/Serialization/ASTReaderDecl.cpp
  lib/Serialization/ASTWriterDecl.cpp
  test/OpenMP/declare_reduction_ast_print.c
  test/OpenMP/declare_reduction_ast_print.cpp
  test/OpenMP/declare_reduction_messages.c
  test/OpenMP/declare_reduction_messages.cpp
  tools/libclang/CIndex.cpp

Index: lib/Serialization/ASTWriterDecl.cpp
===
--- lib/Serialization/ASTWriterDecl.cpp
+++ lib/Serialization/ASTWriterDecl.cpp
@@ -131,6 +131,7 @@
 void VisitObjCPropertyDecl(ObjCPropertyDecl *D);
 void VisitObjCPropertyImplDecl(ObjCPropertyImplDecl *D);
 void VisitOMPThreadPrivateDecl(OMPThreadPrivateDecl *D);
+void VisitOMPDeclareReductionDecl(OMPDeclareReductionDecl *D);
 
 /// Add an Objective-C type parameter list to the given record.
 void AddObjCTypeParamList(ObjCTypeParamList *typeParams) {
@@ -1628,6 +1629,16 @@
   Code = serialization::DECL_OMP_THREADPRIVATE;
 }
 
+void ASTDeclWriter::VisitOMPDeclareReductionDecl(OMPDeclareReductionDecl *D) {
+  VisitNamedDecl(D);
+  Writer.AddSourceLocation(D->getLocStart(), Record);
+  Writer.AddStmt(D->getCombiner());
+  Writer.AddStmt(D->getInitializer());
+  Writer.AddDeclRef(D->getPrevDeclInScope(), Record);
+  Writer.AddTypeRef(D->getType(), Record);
+  Code = serialization::DECL_OMP_DECLARE_REDUCTION;
+}
+
 //===--===//
 // ASTWriter Implementation
 //===--===//
Index: lib/Serialization/ASTReaderDecl.cpp
===
--- lib/Serialization/ASTReaderDecl.cpp
+++ lib/Serialization/ASTReaderDecl.cpp
@@ -350,6 +350,7 @@
 void VisitObjCPropertyDecl(ObjCPropertyDecl *D);
 void VisitObjCPropertyImplDecl(ObjCPropertyImplDecl *D);
 void VisitOMPThreadPrivateDecl(OMPThreadPrivateDecl *D);
+void VisitOMPDeclareReductionDecl(OMPDeclareReductionDecl *D);
 
 /// We've merged the definition \p MergedDef into the existing definition
 /// \p Def. Ensure that \p Def is made visible whenever \p MergedDef is made
@@ -2360,6 +2361,15 @@
   D->setVars(Vars);
 }
 
+void ASTDeclReader::VisitOMPDeclareReductionDecl(OMPDeclareReductionDecl *D) {
+  VisitNamedDecl(D);
+  D->setLocation(Reader.ReadSourceLocation(F, Record, Idx));
+  D->setCombiner(Reader.ReadExpr(F));
+  D->setInitializer(Reader.ReadExpr(F));
+  D->PrevDeclInScope = Reader.ReadDeclID(F, Record, Idx);
+  D->setType(Reader.readType(F, Record, Idx));
+}
+
 //===--===//
 // Attribute Reading
 //===--===//
@@ -2409,7 +2419,8 @@
   isa(D) || 
   isa(D) ||
   isa(D) ||
-  isa(D))
+  isa(D) ||
+  isa(D))
 return true;
   if (VarDecl *Var = dyn_cast(D))
 return Var->isFileVarDecl() &&
@@ -3305,6 +3316,9 @@
   case DECL_OMP_THREADPRIVATE:
 D = OMPThreadPrivateDecl::CreateDeserialized(Context, ID, Record[Idx++]);
 break;
+  case DECL_OMP_DECLARE_REDUCTION:
+D = OMPDeclareReductionDecl::CreateDeserialized(Context, ID);
+break;
   case DECL_EMPTY:
 D = EmptyDecl::CreateDeserialized(Context, ID);
 break;
Index: lib/Serialization/ASTCommon.cpp
===
--- lib/Serialization/ASTCommon.cpp
+++ lib/Serialization/ASTCommon.cpp
@@ -329,6 +329,7 @@
   case Decl::ClassScopeFunctionSpecialization:
   case Decl::Import:
   case Decl::OMPThreadPrivate:
+  case Decl::OMPDeclareReduction:
   case Decl::BuiltinTemplate:
 return false;
 
I