date:20160119

Re: [libcxx] r258107 - Fix PR#26175. Thanks to Josh Petrie for the report and the patch. Reviewed as http://reviews.llvm.org/D16262

2016-01-19 Thread Dimitry Andric via cfe-commits

On 19 Jan 2016, at 01:50, Marshall Clow via cfe-commits 
 wrote:
> 
> Author: marshall
> Date: Mon Jan 18 18:50:37 2016
> New Revision: 258107
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=258107&view=rev
> Log:
> Fix PR#26175. Thanks to Josh Petrie for the report and the patch. Reviewed as 
> http://reviews.llvm.org/D16262

This looks like a good candidate for the 3.8 branch, do you agree?

-Dimitry



signature.asc
Description: Message signed with OpenPGP using GPGMail
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D11035: trivial patch, improve constness

2016-01-19 Thread Daniel Marjamäki via cfe-commits

danielmarjamaki added a subscriber: rsmith.
danielmarjamaki added a comment.

ping.. can somebody review.


http://reviews.llvm.org/D11035



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

Re: [PATCH] D16215: ASTMatchers: enable hasBody() matcher for FunctionDecls

2016-01-19 Thread Aleksei Sidorin via cfe-commits

a.sidorin updated this revision to Diff 45232.
a.sidorin added a comment.

Match only FunctionDecls which are definitions and ignore redeclarations 
without bodies.


http://reviews.llvm.org/D16215

Files:
  docs/LibASTMatchersReference.html
  include/clang/ASTMatchers/ASTMatchers.h
  include/clang/ASTMatchers/ASTMatchersInternal.h
  lib/ASTMatchers/ASTMatchersInternal.cpp
  unittests/ASTMatchers/ASTMatchersTest.cpp
  unittests/ASTMatchers/Dynamic/ParserTest.cpp

Index: unittests/ASTMatchers/Dynamic/ParserTest.cpp
===
--- unittests/ASTMatchers/Dynamic/ParserTest.cpp
+++ unittests/ASTMatchers/Dynamic/ParserTest.cpp
@@ -263,7 +263,7 @@
 "1:1: Matcher does not support binding.",
 ParseWithError("isArrow().bind(\"foo\")"));
   EXPECT_EQ("Input value has unresolved overloaded type: "
-"Matcher",
+"Matcher",
 ParseMatcherWithError("hasBody(stmt())"));
 }
 
Index: unittests/ASTMatchers/ASTMatchersTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersTest.cpp
+++ unittests/ASTMatchers/ASTMatchersTest.cpp
@@ -2858,7 +2858,7 @@
   compoundStmt()));
 }
 
-TEST(HasBody, FindsBodyOfForWhileDoLoops) {
+TEST(HasBody, FindsBodyOfForWhileDoLoopsAndFunctions) {
   EXPECT_TRUE(matches("void f() { for(;;) {} }",
   forStmt(hasBody(compoundStmt();
   EXPECT_TRUE(notMatches("void f() { for(;;); }",
@@ -2869,6 +2869,10 @@
   doStmt(hasBody(compoundStmt();
   EXPECT_TRUE(matches("void f() { int p[2]; for (auto x : p) {} }",
   cxxForRangeStmt(hasBody(compoundStmt();
+  EXPECT_TRUE(matches("void f() {}", functionDecl(hasBody(compoundStmt();
+  EXPECT_TRUE(notMatches("void f();", functionDecl(hasBody(compoundStmt();
+  EXPECT_TRUE(matches("void f(); void f() {}",
+ functionDecl(hasBody(compoundStmt();
 }
 
 TEST(HasAnySubstatement, MatchesForTopLevelCompoundStatement) {
Index: lib/ASTMatchers/ASTMatchersInternal.cpp
===
--- lib/ASTMatchers/ASTMatchersInternal.cpp
+++ lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -339,6 +339,11 @@
   return matchesNodeFull(Node);
 }
 
+template <>
+const Stmt *GetBodyMatcher::get(const FunctionDecl &Node) {
+  return Node.isThisDeclarationADefinition() ? Node.getBody() : NULL;
+}
+
 } // end namespace internal
 } // end namespace ast_matchers
 } // end namespace clang
Index: include/clang/ASTMatchers/ASTMatchersInternal.h
===
--- include/clang/ASTMatchers/ASTMatchersInternal.h
+++ include/clang/ASTMatchers/ASTMatchersInternal.h
@@ -1584,6 +1584,14 @@
   ast_type_traits::DynTypedNode Node;
 };
 
+
+template 
+struct GetBodyMatcher {
+static const Stmt *get(const Ty &Node) {
+  return Node.getBody();
+}
+};
+
 } // end namespace internal
 } // end namespace ast_matchers
 } // end namespace clang
Index: include/clang/ASTMatchers/ASTMatchers.h
===
--- include/clang/ASTMatchers/ASTMatchers.h
+++ include/clang/ASTMatchers/ASTMatchers.h
@@ -3114,8 +3114,8 @@
   return false;
 }
 
-/// \brief Matches a 'for', 'while', or 'do while' statement that has
-/// a given body.
+/// \brief Matches a 'for', 'while', 'do while' statement or a function
+/// declaration that has a given body.
 ///
 /// Given
 /// \code
@@ -3128,9 +3128,10 @@
 AST_POLYMORPHIC_MATCHER_P(hasBody,
   AST_POLYMORPHIC_SUPPORTED_TYPES(DoStmt, ForStmt,
   WhileStmt,
-  CXXForRangeStmt),
+  CXXForRangeStmt,
+  FunctionDecl),
   internal::Matcher, InnerMatcher) {
-  const Stmt *const Statement = Node.getBody();
+  const Stmt *const Statement = internal::GetBodyMatcher::get(Node);
   return (Statement != nullptr &&
   InnerMatcher.matches(*Statement, Finder, Builder));
 }
Index: docs/LibASTMatchersReference.html
===
--- docs/LibASTMatchersReference.html
+++ docs/LibASTMatchersReference.html
@@ -3400,8 +3400,8 @@
 
 
 MatcherCXXForRangeStmt>hasBodyMatcherStmt> InnerMatcher
-Matches a 'for', 'while', or 'do while' statement that has
-a given body.
+Matches a 'for', 'while', 'do while' statement or a function
+declaration that has a given body.
 
 Given
   for (;;) {}
@@ -3825,8 +3825,8 @@
 
 
 MatcherDoStmt>hasBodyMatcher

Re: [PATCH] D15914: [OpenCL] Pipe builtin functions

2016-01-19 Thread Xiuli PAN via cfe-commits

pxli168 updated the summary for this revision.
pxli168 updated this revision to Diff 45234.
pxli168 marked 9 inline comments as done.
pxli168 added a comment.

Fix some small comments problems.


http://reviews.llvm.org/D15914

Files:
  include/clang/Basic/Builtins.def
  include/clang/Basic/Builtins.h
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Basic/Builtins.cpp
  lib/CodeGen/CGBuiltin.cpp
  lib/Sema/SemaChecking.cpp
  test/CodeGenOpenCL/pipe_builtin.cl
  test/SemaOpenCL/invalid-pipe-builtin-cl2.0.cl

Index: test/SemaOpenCL/invalid-pipe-builtin-cl2.0.cl
===
--- /dev/null
+++ test/SemaOpenCL/invalid-pipe-builtin-cl2.0.cl
@@ -0,0 +1,55 @@
+// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=CL2.0
+
+void test1(read_only pipe int p, global int* ptr){
+  int tmp;
+  reserve_id_t rid;
+
+  // read/write_pipe
+  read_pipe(tmp, p);// expected-error {{first argument to read_pipe must be a pipe type}}
+  read_pipe(p);   // expected-error {{invalid number of arguments to function: read_pipe}}
+  read_pipe(p, tmp, tmp, ptr);   // expected-error {{invalid argument type to function read_pipe (expecting 'reserve_id_t')}}
+  read_pipe(p, rid, rid, ptr);   // expected-error {{invalid argument type to function read_pipe (expecting 'unsigned int')}}
+  read_pipe(p, tmp);   // expected-error {{invalid argument type to function read_pipe (expecting 'int *')}}
+  write_pipe(p, ptr);// expected-error {{invalid pipe access modifier (expecting write_only)}}
+  write_pipe(p, rid, tmp, ptr);// expected-error {{invalid pipe access modifier (expecting write_only)}}
+
+  // reserve_read/write_pipe
+  reserve_read_pipe(p, ptr);// expected-error{{invalid argument type to function reserve_read_pipe (expecting 'unsigned int')}}
+  work_group_reserve_read_pipe(tmp, tmp);// expected-error{{first argument to work_group_reserve_read_pipe must be a pipe type}}
+  sub_group_reserve_write_pipe(p, tmp);// expected-error{{invalid pipe access modifier (expecting write_only)}}
+
+  // commit_read/write_pipe
+  commit_read_pipe(tmp, rid);// expected-error{{first argument to commit_read_pipe must be a pipe type}}
+  work_group_commit_read_pipe(p, tmp);// expected-error{{invalid argument type to function work_group_commit_read_pipe (expecting 'reserve_id_t')}}
+  sub_group_commit_write_pipe(p, tmp);// expected-error{{nvalid pipe access modifier (expecting write_only)}}
+}
+
+void test2(write_only pipe int p, global int* ptr){
+  int tmp;
+  reserve_id_t rid;
+
+  // read/write_pipe
+  write_pipe(tmp, p);// expected-error {{first argument to write_pipe must be a pipe type}}
+  write_pipe(p);   // expected-error {{invalid number of arguments to function: write_pipe}}
+  write_pipe(p, tmp, tmp, ptr);   // expected-error {{invalid argument type to function write_pipe (expecting 'reserve_id_t')}}
+  write_pipe(p, rid, rid, ptr);   // expected-error {{invalid argument type to function write_pipe (expecting 'unsigned int')}}
+  write_pipe(p, tmp);   // expected-error {{invalid argument type to function write_pipe (expecting 'int *')}}
+  read_pipe(p, ptr);// expected-error {{invalid pipe access modifier (expecting read_only)}}
+  read_pipe(p, rid, tmp, ptr);// expected-error {{invalid pipe access modifier (expecting read_only)}}
+
+  // reserve_read/write_pipe
+  reserve_write_pipe(p, ptr);// expected-error{{invalid argument type to function reserve_write_pipe (expecting 'unsigned int')}}
+  work_group_reserve_write_pipe(tmp, tmp);// expected-error{{first argument to work_group_reserve_write_pipe must be a pipe type}}
+  sub_group_reserve_read_pipe(p, tmp);// expected-error{{invalid pipe access modifier (expecting read_only)}}
+
+  // commit_read/write_pipe
+  commit_write_pipe(tmp, rid);// expected-error{{first argument to commit_write_pipe must be a pipe type}}
+  work_group_commit_write_pipe(p, tmp);// expected-error{{invalid argument type to function work_group_commit_write_pipe (expecting 'reserve_id_t')}}
+  sub_group_commit_read_pipe(p, tmp);// expected-error{{nvalid pipe access modifier (expecting read_only)}}
+}
+
+void test3(){
+  int tmp;
+  get_pipe_num_packets(tmp);// expected-error {{first argument to get_pipe_num_packets must be a pipe type}}
+  get_pipe_max_packets(tmp);// expected-error {{first argument to get_pipe_max_packets must be a pipe type}}
+}
Index: test/CodeGenOpenCL/pipe_builtin.cl
===
--- /dev/null
+++ test/CodeGenOpenCL/pipe_builtin.cl
@@ -0,0 +1,61 @@
+// RUN: %clang_cc1 -emit-llvm -O0 -cl-std=CL2.0 -o - %s | FileCheck %s
+
+// CHECK: %opencl.pipe_t = type opaque
+// CHECK: %opencl.reserve_id_t = type opaque
+
+void test1(read_only pipe int p, global int *ptr) {
+  // CHECK: call i32 @__read_pipe_2(%opencl.pipe_t* %{{.*}}, i8* %{{.*}})
+  read_pipe(p, ptr);
+  // CHECK: call %opencl.reserve_id_t* @__reserve

Re: [PATCH] D15914: [OpenCL] Pipe builtin functions

2016-01-19 Thread Xiuli PAN via cfe-commits

pxli168 added inline comments.


Comment at: lib/CodeGen/CGBuiltin.cpp:1974
@@ +1973,3 @@
+// Type of the generic packet parameter.
+unsigned GenericAS =
+getContext().getTargetAddressSpace(LangAS::opencl_generic);

Good idea! This can fit different target.


http://reviews.llvm.org/D15914



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

[PATCH] D16310: new clang-tidy checker misc-long-cast

2016-01-19 Thread Daniel Marjamäki via cfe-commits

danielmarjamaki created this revision.
danielmarjamaki added subscribers: cfe-commits, alexfh.

This is a new checker for clang-tidy.

This checker will warn when there is a explicit redundant cast of a calculation
result to a bigger type. If the intention of the cast is to avoid loss of
precision then the cast is misplaced, and there can be loss of precision.
Otherwise the cast is ineffective.

No warning is written when it is seen that the cast is ineffective.

I tested this on debian projects.. and I see quite little noise. in 1212 
projects I got 76 warnings. The cast is either ineffective or there is loss of 
precision in all these cases. I don't see warnings where I think the warning is 
wrong. But I don't want to warn when it can be determined that the cast is just 
ineffective, I think that the perfect checker would be better at determining 
this.


http://reviews.llvm.org/D16310

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/LongCastCheck.cpp
  clang-tidy/misc/LongCastCheck.h
  clang-tidy/misc/MiscTidyModule.cpp
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-long-cast.rst
  test/clang-tidy/misc-long-cast.cpp

Index: test/clang-tidy/misc-long-cast.cpp
===
--- test/clang-tidy/misc-long-cast.cpp
+++ test/clang-tidy/misc-long-cast.cpp
@@ -0,0 +1,48 @@
+// RUN: %check_clang_tidy %s misc-long-cast %t
+
+void assign(int a, int b) {
+  long l;
+
+  l = a * b;
+  l = (long)(a * b);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: either cast from 'int' to 'long' is ineffective, or there is loss of precision before the conversion
+  l = (long)a * b;
+
+  l = (long)(a << 8);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: either cast from 'int' to 'long'
+  l = (long)b << 8;
+}
+
+void init(unsigned int n) {
+  long l = (long)(n << 8);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: either cast from 'int' to 'long'
+}
+
+long ret(int a) {
+  return (long)(a * 1000);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: either cast from 'int' to 'long'
+}
+
+void dontwarn1(unsigned char a, int i, unsigned char *p) {
+  long l;
+  // the result is a 9 bit value, there is no truncation in the implicit cast
+  l = (long)(a + 15);
+  // the result is a 12 bit value, there is no truncation in the implicit cast
+  l = (long)(a << 4);
+  // the result is a 3 bit value, there is no truncation in the implicit cast
+  l = (long)((i%5)+1);
+  // the result is a 16 bit value, there is no truncation in the implicit cast
+  l = (long)(((*p)<<8) + *(p+1));
+}
+
+// Cast is not suspicious when casting macro
+#define A  (X<<2)
+long macro1(int X) {
+  return (long)A;
+}
+
+// Don't warn about cast in macro
+#define B(X,Y)   (long)(X*Y)
+long macro2(int x, int y) {
+  return B(x,y);
+}
Index: docs/clang-tidy/checks/misc-long-cast.rst
===
--- docs/clang-tidy/checks/misc-long-cast.rst
+++ docs/clang-tidy/checks/misc-long-cast.rst
@@ -0,0 +1,29 @@
+.. title:: clang-tidy - misc-long-cast
+
+misc-long-cast
+==
+
+This checker will warn when there is a explicit redundant cast of a calculation
+result to a bigger type. If the intention of the cast is to avoid loss of
+precision then the cast is misplaced, and there can be loss of precision.
+Otherwise the cast is ineffective.
+
+Example code::
+
+long f(int x) {
+return (long)(x*1000);
+}
+
+The result x*1000 is first calculated using int precision. If the result
+exceeds int precision there is loss of precision. Then the result is casted to
+long.
+
+If there is no loss of precision then the cast can be removed or you can
+explicitly cast to int instead.
+
+If you want to avoid loss of precision then put the cast in a proper location,
+for instance::
+
+long f(int x) {
+return (long)x * 1000;
+}
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -49,6 +49,7 @@
misc-definitions-in-headers
misc-inaccurate-erase
misc-inefficient-algorithm
+   misc-long-cast
misc-macro-parentheses
misc-macro-repeated-side-effects
misc-move-constructor-init
Index: clang-tidy/misc/MiscTidyModule.cpp
===
--- clang-tidy/misc/MiscTidyModule.cpp
+++ clang-tidy/misc/MiscTidyModule.cpp
@@ -17,6 +17,7 @@
 #include "DefinitionsInHeadersCheck.h"
 #include "InaccurateEraseCheck.h"
 #include "InefficientAlgorithmCheck.h"
+#include "LongCastCheck.h"
 #include "MacroParenthesesCheck.h"
 #include "MacroRepeatedSideEffectsCheck.h"
 #include "MoveConstantArgumentCheck.h"
@@ -56,6 +57,8 @@
 "misc-inaccurate-erase");
 CheckFactories.registerCheck(
 "misc-inefficient-algorithm");
+CheckFactories.registerCheck(
+"misc-long-cast");
 CheckFactories.registerCheck(
 "misc-macro-pare

Re: [PATCH] D15914: [OpenCL] Pipe builtin functions

2016-01-19 Thread Anastasia Stulova via cfe-commits

Anastasia accepted this revision.
Anastasia added a comment.
This revision is now accepted and ready to land.

LGTM! Could you please remove the line with printf before committing (see 
comment above)?



Comment at: lib/CodeGen/CGBuiltin.cpp:1976
@@ +1975,3 @@
+getContext().getTargetAddressSpace(LangAS::opencl_generic);
+if(GenericAS != 4U) printf("Generic AS is %d\n",GenericAS);
+llvm::Type *I8PTy = llvm::PointerType::get(

Could you remove this line with printf please?


http://reviews.llvm.org/D15914



___
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-19 Thread Alexander Droste via cfe-commits

Alexander_Droste updated this revision to Diff 45239.
Alexander_Droste added a comment.

- reduce copying
- differentiate ER->getIndex().getAs<> cases

How about this?


http://reviews.llvm.org/D16044

Files:
  tools/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
  tools/clang/lib/StaticAnalyzer/Core/MemRegion.cpp

Index: tools/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
===
--- tools/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
+++ tools/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
@@ -149,6 +149,14 @@
   template const RegionTy* getAs() const;
 
   virtual bool isBoundable() const { return false; }
+
+  /// Get variable name for memory region. The name is obtained from
+  /// the variable/field declaration retrieved from the memory region.
+  /// Regions that point to an element of an array are returned as: "arr[0]".
+  /// Regions that point to a struct are returned as: "st.var".
+  ///
+  /// \returns variable name for memory region
+  std::string getVariableName() const;
 };
 
 /// MemSpaceRegion - A memory region that represents a "memory space";
Index: tools/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
===
--- tools/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
+++ tools/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
@@ -639,6 +639,55 @@
   superRegion->printPrettyAsExpr(os);
 }
 
+std::string MemRegion::getVariableName() const {
+  std::string VariableName{""};
+  std::string ArrayIndices{""};
+
+  const clang::ento::MemRegion *R = this;
+  llvm::SmallString<50> buf;
+  llvm::raw_svector_ostream os(buf);
+
+  // Obtain array indices to add them to the variable name.
+  const clang::ento::ElementRegion *ER = nullptr;
+  while ((ER = R->getAs())) {
+
+std::string idx{""};
+if (auto CI = ER->getIndex().getAs()) {
+  llvm::SmallString<2> intValAsString;
+  CI->getValue().toString(intValAsString);
+  idx = {intValAsString.begin(), intValAsString.end()};
+}
+
+else if (auto SV =
+ER->getIndex().getAs()) {
+llvm::SmallString<8> buf;
+llvm::raw_svector_ostream os(buf);
+os << SV->getSymbol();
+idx = os.str();
+}
+
+ArrayIndices += "]" + idx + "[";
+R = ER->getSuperRegion();
+  }
+
+  std::reverse(ArrayIndices.begin(), ArrayIndices.end());
+
+  // Get variable name.
+  if (R && R->canPrintPretty()) {
+R->printPretty(os);
+VariableName += os.str();
+  }
+
+  // Combine variable name with indices.
+  if (VariableName.size() && VariableName.back() == '\'') {
+VariableName.insert(VariableName.size() - 1, ArrayIndices);
+  } else {
+VariableName += ArrayIndices;
+  }
+
+  return VariableName;
+}
+
 
//===--===//
 // MemRegionManager methods.
 
//===--===//


Index: tools/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
===
--- tools/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
+++ tools/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
@@ -149,6 +149,14 @@
   template const RegionTy* getAs() const;
 
   virtual bool isBoundable() const { return false; }
+
+  /// Get variable name for memory region. The name is obtained from
+  /// the variable/field declaration retrieved from the memory region.
+  /// Regions that point to an element of an array are returned as: "arr[0]".
+  /// Regions that point to a struct are returned as: "st.var".
+  ///
+  /// \returns variable name for memory region
+  std::string getVariableName() const;
 };
 
 /// MemSpaceRegion - A memory region that represents a "memory space";
Index: tools/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
===
--- tools/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
+++ tools/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
@@ -639,6 +639,55 @@
   superRegion->printPrettyAsExpr(os);
 }
 
+std::string MemRegion::getVariableName() const {
+  std::string VariableName{""};
+  std::string ArrayIndices{""};
+
+  const clang::ento::MemRegion *R = this;
+  llvm::SmallString<50> buf;
+  llvm::raw_svector_ostream os(buf);
+
+  // Obtain array indices to add them to the variable name.
+  const clang::ento::ElementRegion *ER = nullptr;
+  while ((ER = R->getAs())) {
+
+std::string idx{""};
+if (auto CI = ER->getIndex().getAs()) {
+  llvm::SmallString<2> intValAsString;
+  CI->getValue().toString(intValAsString);
+  idx = {intValAsString.begin(), intValAsString.end()};
+}
+
+else if (auto SV =
+ER->getIndex().getAs()) {
+llvm::SmallString<8> buf;
+llvm::raw_svector_ostream os(buf);
+os << SV->getSymbol();
+

Re: [PATCH] D16295: Change of UserLabelPrefix default value from "_" to ""

2016-01-19 Thread Andrey Bokhanko via cfe-commits

andreybokhanko added a comment.

@rafael, all these changes are driven by tests.

It seems you mean OS targeting, which is handled in other TargetInfo classes 
(LinuxTargetInfo in Linux case).



Comment at: lib/Basic/Targets.cpp:801
@@ -818,2 +800,3 @@
 LongDoubleFormat = &llvm::APFloat::PPCDoubleDouble;
+UserLabelPrefix = "_";
   }

rafael wrote:
> This looks wrong, we produce a "f:" not an "_f:" when targeting 
> powerpc-linux-gnu.
> 
Is this commented out, tools/clang/test/Preprocessor/init.c:5216 fails. As can 
be seen in the test, PPC603E target expects UserLabelPrefix to be equal to "_" 
in freestanding mode.

As for powerpc-linux-gnu target, UserLabelPrefix is set to "" at 
lib/Basic/Target.cpp:416 (LinuxTargetInfo constructor).


Comment at: lib/Basic/Targets.cpp:1617
@@ -1633,2 +1616,3 @@
 GPU = GK_SM20;
+UserLabelPrefix = "_";
   }

rafael wrote:
> This also looks wrong.
Same as above -- NVPTX target expects UserLabelPrefix to be "_" in freestanding 
mode (tools/clang/test/Preprocessor/init.c:4853). Linux target is covered in 
LinuxTargetInfo constructor.


http://reviews.llvm.org/D16295



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

Re: [PATCH] D13285: Fix for bug 24196: clang fails on assertion on complex doubles multiplication when EH is enabled

2016-01-19 Thread Denis Zobnin via cfe-commits

d.zobnin.bugzilla abandoned this revision.
d.zobnin.bugzilla added a comment.

This issue is fixed in http://reviews.llvm.org/rL253926.


http://reviews.llvm.org/D13285



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

Re: [PATCH] D16295: Change of UserLabelPrefix default value from "_" to ""

2016-01-19 Thread Rafael Espíndola via cfe-commits

I am pretty sure the cases in init.c are wrong as the assembly itself
doesn't use a '_'.

Having said that, it is probably a good thing to do this in two steps.
So this patch LGTM on the condition that you also open a bug to audit
the cases where we define __USER_LABEL_PREFIX__ to _ in init.c and CC
whoever added those.

Thanks,
Rafael


On 19 January 2016 at 05:58, Andrey Bokhanko  wrote:
> andreybokhanko added a comment.
>
> @rafael, all these changes are driven by tests.
>
> It seems you mean OS targeting, which is handled in other TargetInfo classes 
> (LinuxTargetInfo in Linux case).
>
>
> 
> Comment at: lib/Basic/Targets.cpp:801
> @@ -818,2 +800,3 @@
>  LongDoubleFormat = &llvm::APFloat::PPCDoubleDouble;
> +UserLabelPrefix = "_";
>}
> 
> rafael wrote:
>> This looks wrong, we produce a "f:" not an "_f:" when targeting 
>> powerpc-linux-gnu.
>>
> Is this commented out, tools/clang/test/Preprocessor/init.c:5216 fails. As 
> can be seen in the test, PPC603E target expects UserLabelPrefix to be equal 
> to "_" in freestanding mode.
>
> As for powerpc-linux-gnu target, UserLabelPrefix is set to "" at 
> lib/Basic/Target.cpp:416 (LinuxTargetInfo constructor).
>
> 
> Comment at: lib/Basic/Targets.cpp:1617
> @@ -1633,2 +1616,3 @@
>  GPU = GK_SM20;
> +UserLabelPrefix = "_";
>}
> 
> rafael wrote:
>> This also looks wrong.
> Same as above -- NVPTX target expects UserLabelPrefix to be "_" in 
> freestanding mode (tools/clang/test/Preprocessor/init.c:4853). Linux target 
> is covered in LinuxTargetInfo constructor.
>
>
> http://reviews.llvm.org/D16295
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r258123 - Fix formatting of fully qualified names in array subscripts.

2016-01-19 Thread Manuel Klimek via cfe-commits

Author: klimek
Date: Tue Jan 19 08:05:32 2016
New Revision: 258123

URL: http://llvm.org/viewvc/llvm-project?rev=258123&view=rev
Log:
Fix formatting of fully qualified names in array subscripts.

Before:
  a[ ::b::c];

After:
  a[::b::c];

Modified:
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/unittests/Format/FormatTest.cpp

Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=258123&r1=258122&r2=258123&view=diff
==
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Tue Jan 19 08:05:32 2016
@@ -2114,7 +2114,8 @@ bool TokenAnnotator::spaceRequiredBefore
   if (Right.is(tok::coloncolon) && Left.isNot(tok::l_brace))
 return (Left.is(TT_TemplateOpener) &&
 Style.Standard == FormatStyle::LS_Cpp03) ||
-   !(Left.isOneOf(tok::identifier, tok::l_paren, tok::r_paren) ||
+   !(Left.isOneOf(tok::identifier, tok::l_paren, tok::r_paren,
+  tok::l_square) ||
  Left.isOneOf(TT_TemplateCloser, TT_TemplateOpener));
   if ((Left.is(TT_TemplateOpener)) != (Right.is(TT_TemplateCloser)))
 return Style.SpacesInAngles;

Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=258123&r1=258122&r2=258123&view=diff
==
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Tue Jan 19 08:05:32 2016
@@ -6133,6 +6133,7 @@ TEST_F(FormatTest, FormatsArrays) {
   "aaa aaa = 
aa->a[0]\n"
   "  .aaa[0]\n"
   "  .aa();");
+  verifyFormat("a[::b::c];");
 
   verifyNoCrash("a[,Y?)]", getLLVMStyleWithColumns(10));
 


___
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-19 Thread Aaron Ballman via cfe-commits

aaron.ballman added a subscriber: aaron.ballman.
aaron.ballman added a comment.

I would like to see some additional tests to ensure that this does not turn 
dead code into live code with the fix. e.g.,

  void g();
  void f() {
return;
g(); // This would become live code if return were removed.
  }

A similar test for continue would be appreciated as well. Tests for range-based 
for loops also.



Comment at: clang-tidy/readability/RedundantReturnCheck.cpp:23
@@ +22,3 @@
+  Finder->addMatcher(
+  functionDecl(isDefinition(), returns(asString("void")),
+   has(compoundStmt(hasAnySubstatement(returnStmt()

Would be better written as: `returns(voidType())` instead of using `asString()`.


Comment at: clang-tidy/readability/RedundantReturnCheck.cpp:24
@@ +23,3 @@
+  functionDecl(isDefinition(), returns(asString("void")),
+   has(compoundStmt(hasAnySubstatement(returnStmt()
+  .bind("fn"),

Would be best to restrict this to a return statement that has no expression if 
we don't want to diagnose this:
```
void g();

void f() {
  return g();
}
```
Either way, it would be good to have a test that ensures this isn't mangled.


Comment at: clang-tidy/readability/RedundantReturnCheck.cpp:28
@@ +27,3 @@
+  auto CompoundContinue = 
has(compoundStmt(hasAnySubstatement(continueStmt(;
+  Finder->addMatcher(forStmt(CompoundContinue).bind("for"), this);
+  Finder->addMatcher(whileStmt(CompoundContinue).bind("while"), this);

I think you also want to match cxxForRangeStmt() in the same way.


Comment at: clang-tidy/readability/RedundantReturnCheck.cpp:34
@@ +33,3 @@
+void RedundantReturnCheck::check(const MatchFinder::MatchResult &Result) {
+  if (const auto *Fn = Result.Nodes.getNodeAs("fn")) {
+checkRedundantReturn(Result, Fn);

Elide braces for these if-else statements.


Comment at: clang-tidy/readability/RedundantReturnCheck.cpp:75
@@ +74,3 @@
+void RedundantReturnCheck::checkRedundantContinue(
+const ast_matchers::MatchFinder::MatchResult &Result,
+const CompoundStmt *Block) {

No need for `ast_matchers`.


Comment at: clang-tidy/readability/RedundantReturnCheck.cpp:77
@@ +76,3 @@
+const CompoundStmt *Block) {
+  if (Block) {
+CompoundStmt::const_reverse_body_iterator last = Block->body_rbegin();

Please consolidate this duplicated code.


Comment at: clang-tidy/readability/RedundantReturnCheck.h:26
@@ +25,3 @@
+/// 
http://clang.llvm.org/extra/clang-tidy/checks/readability-redundant-return.html
+class RedundantReturnCheck : public ClangTidyCheck {
+public:

Since this also handling continue, I think this would be 
SpuriousFlowControlCheck instead?


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] D16248: [Clang-tidy] rename misc-inefficient-algorithm to performance-inefficient-algorithm

2016-01-19 Thread Aaron Ballman via cfe-commits

aaron.ballman added inline comments.


Comment at: clang-tidy/misc/CMakeLists.txt:10
@@ -9,3 +9,2 @@
   InaccurateEraseCheck.cpp
-  InefficientAlgorithmCheck.cpp
   MacroParenthesesCheck.cpp

What about MoveConstantArgumentCheck.cpp?


Comment at: clang-tidy/misc/MiscTidyModule.cpp:58
@@ -57,3 +56,1 @@
-CheckFactories.registerCheck(
-"misc-inefficient-algorithm");
 CheckFactories.registerCheck(

This will break projects that enable the misc-inefficient-algorithm check 
(which clang 3.7 exposed). Is there a reason to not keep this check registered 
under this name?

(Perhaps a follow-up patch to allow deprecation of check names so that users 
are given guidance would make sense.)


http://reviews.llvm.org/D16248



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

Re: [PATCH] D16278: ASTMatcher for ParenExpr node.

2016-01-19 Thread Aaron Ballman via cfe-commits

aaron.ballman added a subscriber: aaron.ballman.
aaron.ballman added a reviewer: aaron.ballman.
aaron.ballman added a comment.

I have a few small nits that don't require additional review.



Comment at: include/clang/ASTMatchers/ASTMatchers.h:1052
@@ +1051,3 @@
+/// \brief Matches parentheses used in expressions.
+//
+// Example matches (foo() + 1)

Please use doxygen-style comments for the entire comment block.


Comment at: unittests/ASTMatchers/ASTMatchersTest.cpp:3570
@@ +3569,3 @@
+  EXPECT_FALSE(matches("int i = 3;", parenExpr()));
+  EXPECT_FALSE(matches("int foo() { return 1; }; int a = foo();", 
parenExpr()));
+}

Please change this to EXPECT_TRUE and notMatches instead (for consistency in 
the rest of the file).


http://reviews.llvm.org/D16278



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

[PATCH] D16317: [Analyzer] Fix for PR23790: bind real value returned from strcmp when modelling strcmp.

2016-01-19 Thread Антон Ярцев via cfe-commits

ayartsev created this revision.
ayartsev added a reviewer: zaks.anna.
ayartsev added a subscriber: cfe-commits.

The patch is a fix for [[ https://llvm.org/bugs/show_bug.cgi?id=23790 | PR23790 
]]. Call to StringRef::Compare() returning [1,0,-1] is replaced with the real 
call to strcmp to be more precise in modelling. This also may theoretically 
help to find defects if a tested code relays on a value returned from strcmp 
like
if (strcmp(x, y) == 1) { ... } 

Please review!

http://reviews.llvm.org/D16317

Files:
  lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  test/Analysis/string.c

Index: test/Analysis/string.c
===
--- test/Analysis/string.c
+++ test/Analysis/string.c
@@ -703,13 +703,13 @@
 void strcmp_1() {
   char *x = "234";
   char *y = "123";
-  clang_analyzer_eval(strcmp(x, y) == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(strcmp(x, y) > 0); // expected-warning{{TRUE}}
 }
 
 void strcmp_2() {
   char *x = "123";
   char *y = "234";
-  clang_analyzer_eval(strcmp(x, y) == -1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(strcmp(x, y) < 0); // expected-warning{{TRUE}}
 }
 
 void strcmp_null_0() {
@@ -727,25 +727,25 @@
 void strcmp_diff_length_0() {
   char *x = "12345";
   char *y = "234";
-  clang_analyzer_eval(strcmp(x, y) == -1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(strcmp(x, y) < 0); // expected-warning{{TRUE}}
 }
 
 void strcmp_diff_length_1() {
   char *x = "123";
   char *y = "23456";
-  clang_analyzer_eval(strcmp(x, y) == -1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(strcmp(x, y) < 0); // expected-warning{{TRUE}}
 }
 
 void strcmp_diff_length_2() {
   char *x = "12345";
   char *y = "123";
-  clang_analyzer_eval(strcmp(x, y) == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(strcmp(x, y) > 0); // expected-warning{{TRUE}}
 }
 
 void strcmp_diff_length_3() {
   char *x = "123";
   char *y = "12345";
-  clang_analyzer_eval(strcmp(x, y) == -1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(strcmp(x, y) < 0); // expected-warning{{TRUE}}
 }
 
 void strcmp_embedded_null () {
@@ -786,13 +786,13 @@
 void strncmp_1() {
   char *x = "234";
   char *y = "123";
-  clang_analyzer_eval(strncmp(x, y, 3) == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(strncmp(x, y, 3) > 0); // expected-warning{{TRUE}}
 }
 
 void strncmp_2() {
   char *x = "123";
   char *y = "234";
-  clang_analyzer_eval(strncmp(x, y, 3) == -1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(strncmp(x, y, 3) < 0); // expected-warning{{TRUE}}
 }
 
 void strncmp_null_0() {
@@ -810,25 +810,25 @@
 void strncmp_diff_length_0() {
   char *x = "12345";
   char *y = "234";
-  clang_analyzer_eval(strncmp(x, y, 5) == -1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(strncmp(x, y, 5) < 0); // expected-warning{{TRUE}}
 }
 
 void strncmp_diff_length_1() {
   char *x = "123";
   char *y = "23456";
-  clang_analyzer_eval(strncmp(x, y, 5) == -1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(strncmp(x, y, 5) < 0); // expected-warning{{TRUE}}
 }
 
 void strncmp_diff_length_2() {
   char *x = "12345";
   char *y = "123";
-  clang_analyzer_eval(strncmp(x, y, 5) == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(strncmp(x, y, 5) > 0); // expected-warning{{TRUE}}
 }
 
 void strncmp_diff_length_3() {
   char *x = "123";
   char *y = "12345";
-  clang_analyzer_eval(strncmp(x, y, 5) == -1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(strncmp(x, y, 5) < 0); // expected-warning{{TRUE}}
 }
 
 void strncmp_diff_length_4() {
@@ -840,13 +840,13 @@
 void strncmp_diff_length_5() {
   char *x = "012";
   char *y = "12345";
-  clang_analyzer_eval(strncmp(x, y, 3) == -1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(strncmp(x, y, 3) < 0); // expected-warning{{TRUE}}
 }
 
 void strncmp_diff_length_6() {
   char *x = "234";
   char *y = "12345";
-  clang_analyzer_eval(strncmp(x, y, 3) == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(strncmp(x, y, 3) > 0); // expected-warning{{TRUE}}
 }
 
 void strncmp_embedded_null () {
Index: lib/StaticAnalyzer/Checkers/CStringChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -1872,8 +1872,15 @@
 // Compare string 1 to string 2 the same way strcasecmp() does.
 result = s1StrRef.compare_lower(s2StrRef);
   } else {
-// Compare string 1 to string 2 the same way strcmp() does.
-result = s1StrRef.compare(s2StrRef);
+// Compare string 1 to string 2.
+char s1[s1StrRef.size() + 1];
+memcpy(s1, s1StrRef.data(), s1StrRef.size());
+s1[s1StrRef.size()] = '\0';
+char s2[s2StrRef.size() + 1];
+memcpy(s2, s2StrRef.data(), s2StrRef.size());
+s2[s2StrRef.size()] = '\0';
+
+result = strcmp(s1, s2);
   }
 
   // Build the SVal of the comparison and bin

Re: [PATCH] D16286: [clang-tidy] Readability check for redundant parenthesis in return expression.

2016-01-19 Thread Aaron Ballman via cfe-commits

aaron.ballman added a subscriber: aaron.ballman.
aaron.ballman added a comment.

I think this check is a bit too aggressive about suggesting removal of parens. 
For instance:

  return (foo && (bar || baz));

It seems reasonable that with more complex subexpressions involving operators 
of differing precedence that a user may want to keep the outer parens for 
clarity and that removing them actually reduces readability. Perhaps an option 
is in order (uncertain what to default its value to) to support that?

Also, how well does this interact with macro expansions, where parens are like 
candy?

  #define FOO(x) (x)
  
  void f() {
return FOO(12)
  }



Comment at: clang-tidy/readability/ReturnWithRedundantParensCheck.cpp:26
@@ +25,3 @@
+void ReturnWithRedundantParensCheck::check(
+const ast_matchers::MatchFinder::MatchResult &Result) {
+

No need for `ast_matchers`.


Comment at: test/clang-tidy/readability-return-with-redundant-parens.cpp:24
@@ +23,2 @@
+  // CHECK-FIXES: {{^  }}return 1;{{$}}
+}

> IMO, clang-tidy should be agnostic as to input formatting for what it can 
> handle.

Strongly agreed.


http://reviews.llvm.org/D16286



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

Re: [PATCH] D16248: [Clang-tidy] rename misc-inefficient-algorithm to performance-inefficient-algorithm

2016-01-19 Thread Alexander Kornienko via cfe-commits

alexfh added inline comments.


Comment at: clang-tidy/misc/MiscTidyModule.cpp:58
@@ -57,3 +56,1 @@
-CheckFactories.registerCheck(
-"misc-inefficient-algorithm");
 CheckFactories.registerCheck(

alexfh wrote:
> aaron.ballman wrote:
> > This will break projects that enable the misc-inefficient-algorithm check 
> > (which clang 3.7 exposed). Is there a reason to not keep this check 
> > registered under this name?
> > 
> > (Perhaps a follow-up patch to allow deprecation of check names so that 
> > users are given guidance would make sense.)
> I don't feel strongly, but I'm somewhat reluctant to keep old check names. 
> With every new clang-tidy version someone starts using on a project, they 
> need to carefully look at the list of checks and select relevant ones anyway. 
> I think, adding facilities for deprecating checks and keeping old names is 
> not going to help much, but will certainly add support burden for us.
But we certainly need to mention the rename in the release notes for 3.8.


http://reviews.llvm.org/D16248



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

Re: [PATCH] D8149: Add hasUnderlyingType narrowing matcher for TypedefDecls, functionProtoType matcher for FunctionProtoType nodes, extend parameterCountIs to FunctionProtoType nodes

2016-01-19 Thread Aaron Ballman via cfe-commits

aaron.ballman added a comment.

In http://reviews.llvm.org/D8149#329791, @LegalizeAdulthood wrote:

> Is there any reason we can't proceed with the patch as-is and then see if it 
> can be refactored into an overload of `hasType`?

The patch currently includes hasUnderlyingType() which we don't want because 
hasType() is the logically correct place to expose that functionality (it would 
be providing duplicate functionality with no benefit to expose 
hasUnderlyingType()). If you removed hasUnderlyingType() and split the 
hasType() changes into its own patch, we can easily proceed with the function 
prototype changes (AFAICT, there's only one outstanding test to add for that 
and it otherwise looks fine to me).

Comment at: unittests/ASTMatchers/ASTMatchersTest.cpp:1567
@@ -1566,1 +1566,3 @@
+  EXPECT_TRUE(matches("void f(...);", functionDecl(parameterCountIs(0;
+  EXPECT_TRUE(matches("void f(int, ...);", functionDecl(parameterCountIs(1;
 }

aaron.ballman wrote:
> What should be the outcome of this test (matches or not matches)?
> ```
> matchesC("void f();", functionDecl(parameterCountIs(0)));
> ```
> (Note, it's using C where this is variadic.) My guess is that it should 
> match, but I'd like to make sure (and have a test for it).
I still don't see a `matchesC()` test case for 
`functionDecl(parameterCountIs()` on a varargs function, so I don't believe 
this comment is Done yet. (I do see one for getting to it through 
functionProtoType()).

http://reviews.llvm.org/D8149

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

Re: [PATCH] D16248: [Clang-tidy] rename misc-inefficient-algorithm to performance-inefficient-algorithm

2016-01-19 Thread Aaron Ballman via cfe-commits

aaron.ballman added inline comments.

Comment at: clang-tidy/misc/MiscTidyModule.cpp:58
@@ -57,3 +56,1 @@
-CheckFactories.registerCheck(
-"misc-inefficient-algorithm");
 CheckFactories.registerCheck(

alexfh wrote:
> alexfh wrote:
> > aaron.ballman wrote:
> > > This will break projects that enable the misc-inefficient-algorithm check 
> > > (which clang 3.7 exposed). Is there a reason to not keep this check 
> > > registered under this name?
> > > 
> > > (Perhaps a follow-up patch to allow deprecation of check names so that 
> > > users are given guidance would make sense.)
> > I don't feel strongly, but I'm somewhat reluctant to keep old check names. 
> > With every new clang-tidy version someone starts using on a project, they 
> > need to carefully look at the list of checks and select relevant ones 
> > anyway. I think, adding facilities for deprecating checks and keeping old 
> > names is not going to help much, but will certainly add support burden for 
> > us.
> But we certainly need to mention the rename in the release notes for 3.8.
> I don't feel strongly, but I'm somewhat reluctant to keep old check names. 
> With every new clang-tidy version someone starts using on a project, they 
> need to carefully look at the list of checks and select relevant ones anyway. 
> I think, adding facilities for deprecating checks and keeping old names is 
> not going to help much, but will certainly add support burden for us.

I'm more worried about upgrading existing uses than initiating new uses on a 
project. If my build system enabled this check for my project, then upgrading 
clang-tidy will cause that build to break because of an unknown check name, 
won't it? I think it's reasonable to do that if there's compelling reason 
(e.g., we remove a check entirely because it's no longer useful for some 
reason), but I'd like to avoid gratuitously breaking changes because it adds a 
barrier to people's upgrade paths.

Oye. I just tested this out and the results were...surprisingly unhelpful.
```
e:\llvm\2015>clang-tidy -checks=misc-hahahaha-nope E:\Desktop\test.cpp --
e:\llvm\2015>
```
So it seems we don't currently diagnose providing unknown check names at all, 
which would make this a silently breaking change (existing uses will no longer 
trigger the check *and* they won't trigger any diagnostic mentioning that the 
check isn't known). :-(

http://reviews.llvm.org/D16248

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

Re: [PATCH] D16248: [Clang-tidy] rename misc-inefficient-algorithm to performance-inefficient-algorithm

2016-01-19 Thread Alexander Kornienko via cfe-commits

alexfh added inline comments.


Comment at: clang-tidy/misc/MiscTidyModule.cpp:58
@@ -57,3 +56,1 @@
-CheckFactories.registerCheck(
-"misc-inefficient-algorithm");
 CheckFactories.registerCheck(

aaron.ballman wrote:
> This will break projects that enable the misc-inefficient-algorithm check 
> (which clang 3.7 exposed). Is there a reason to not keep this check 
> registered under this name?
> 
> (Perhaps a follow-up patch to allow deprecation of check names so that users 
> are given guidance would make sense.)
I don't feel strongly, but I'm somewhat reluctant to keep old check names. With 
every new clang-tidy version someone starts using on a project, they need to 
carefully look at the list of checks and select relevant ones anyway. I think, 
adding facilities for deprecating checks and keeping old names is not going to 
help much, but will certainly add support burden for us.


Comment at: docs/clang-tidy/checks/misc-inefficient-algorithm.rst:4
@@ -5,1 +3,3 @@
+.. meta::
+   :http-equiv=refresh: 5;URL=performance-inefficient-algorithm.html
 

We need to change the add_new_check.py script to exclude obsolete check names 
from the list (it could exclude all files marked `:orphan:`). Tell me, if you 
need help with this.


Comment at: docs/clang-tidy/checks/performance-inefficient-algorithm.rst:3
@@ -2,3 +2,3 @@
 
-misc-inefficient-algorithm
+performance-inefficient-algorithm
 ==

After reading this check name a few times, I found it too generic (one may 
think that this is a generic algorithm-level code profiler ;). I think, we need 
to rename it to `performance-inefficient-lookup-algorithm` or 
`performance-inefficient-search-algorithm`, since we're changing the name 
anyway.


Comment at: docs/clang-tidy/checks/performance-inefficient-algorithm.rst:4
@@ -4,2 +3,3 @@
+performance-inefficient-algorithm
 ==
 

Please make the underlining the same length as the line above.


http://reviews.llvm.org/D16248



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

Re: [PATCH] D16286: [clang-tidy] Readability check for redundant parenthesis in return expression.

2016-01-19 Thread Alexander Kornienko via cfe-commits

alexfh added a comment.

A high-level comment: why do we want to limit this check to only remove 
parentheses from `return ()`? Anything wrong with removing unnecessary 
parentheses everywhere (except for macro bodies)?



Comment at: clang-tidy/readability/ReturnWithRedundantParensCheck.cpp:28
@@ +27,3 @@
+
+  const ParenExpr *ParenExpression =
+  Result.Nodes.getNodeAs("paren_expr");

Please use `const auto *`: repeating the type name doesn't make anything 
clearer.


Comment at: clang-tidy/readability/ReturnWithRedundantParensCheck.cpp:31
@@ +30,3 @@
+
+  const SourceLocation &LParenLoc = ParenExpression->getLParen();
+  const SourceLocation &RParenLoc = ParenExpression->getRParen();

SourceLocations are small and trivially copyable, so they should be passed by 
value.


Comment at: test/clang-tidy/readability-return-with-redundant-parens.cpp:1
@@ +1,2 @@
+// RUN: %check_clang_tidy %s readability-return-with-redundant-parens %t
+

Please add tests with macros:
  1. where superfluous parentheses are in the macro body (and they usually 
shouldn't be removed);
  2. where superfluous parentheses are in a macro argument (which is not itself 
located in a macro body (e.g. `EXPECT_EQ(x, (y + z));`) - I'd say that in this 
case we usually can remove parentheses;

Please add a test to ensure replacements are only applied once in templates 
with multiple instantiations.


http://reviews.llvm.org/D16286



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

r258128 - Add -Wexpansion-to-undefined: warn when using `defined` in a macro definition.

2016-01-19 Thread Nico Weber via cfe-commits

Author: nico
Date: Tue Jan 19 09:15:31 2016
New Revision: 258128

URL: http://llvm.org/viewvc/llvm-project?rev=258128&view=rev
Log:
Add -Wexpansion-to-undefined: warn when using `defined` in a macro definition.

[cpp.cond]p4:
  Prior to evaluation, macro invocations in the list of preprocessing
  tokens that will become the controlling constant expression are replaced
  (except for those macro names modified by the 'defined' unary operator),
  just as in normal text. If the token 'defined' is generated as a result
  of this replacement process or use of the 'defined' unary operator does
  not match one of the two specified forms prior to macro replacement, the
  behavior is undefined.

This isn't an idle threat, consider this program:
  #define FOO
  #define BAR defined(FOO)
  #if BAR
  ...
  #else
  ...
  #endif
clang and gcc will pick the #if branch while Visual Studio will take the
#else branch.  Emit a warning about this undefined behavior.

One problem is that this also applies to function-like macros. While the
example above can be written like

#if defined(FOO) && defined(BAR)
#defined HAVE_FOO 1
#else
#define HAVE_FOO 0
#endif

there is no easy way to rewrite a function-like macro like `#define FOO(x)
(defined __foo_##x && __foo_##x)`.  Function-like macros like this are used in
practice, and compilers seem to not have differing behavior in that case. So
this a default-on warning only for object-like macros. For function-like
macros, it is an extension warning that only shows up with `-pedantic`.
(But it's undefined behavior in both cases.)

Modified:
cfe/trunk/include/clang/Basic/DiagnosticGroups.td
cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td
cfe/trunk/lib/Lex/PPExpressions.cpp
cfe/trunk/test/Lexer/cxx-features.cpp
cfe/trunk/test/Preprocessor/expr_define_expansion.c

Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=258128&r1=258127&r2=258128&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Tue Jan 19 09:15:31 2016
@@ -204,6 +204,7 @@ def OverloadedShiftOpParentheses: DiagGr
 def DanglingElse: DiagGroup<"dangling-else">;
 def DanglingField : DiagGroup<"dangling-field">;
 def DistributedObjectModifiers : DiagGroup<"distributed-object-modifiers">;
+def ExpansionToUndefined : DiagGroup<"expansion-to-undefined">;
 def FlagEnum : DiagGroup<"flag-enum">;
 def IncrementBool : DiagGroup<"increment-bool", [DeprecatedIncrementBool]>;
 def InfiniteRecursion : DiagGroup<"infinite-recursion">;

Modified: cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td?rev=258128&r1=258127&r2=258128&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td Tue Jan 19 09:15:31 2016
@@ -658,6 +658,13 @@ def warn_header_guard : Warning<
 def note_header_guard : Note<
   "%0 is defined here; did you mean %1?">;
 
+def warn_defined_in_object_type_macro : Warning<
+  "macro expansion producing 'defined' has undefined behavior">,
+  InGroup;
+def warn_defined_in_function_type_macro : Extension<
+  "macro expansion producing 'defined' has undefined behavior">,
+  InGroup;
+
 let CategoryName = "Nullability Issue" in {
 
 def err_pp_assume_nonnull_syntax : Error<"expected 'begin' or 'end'">;

Modified: cfe/trunk/lib/Lex/PPExpressions.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPExpressions.cpp?rev=258128&r1=258127&r2=258128&view=diff
==
--- cfe/trunk/lib/Lex/PPExpressions.cpp (original)
+++ cfe/trunk/lib/Lex/PPExpressions.cpp Tue Jan 19 09:15:31 2016
@@ -140,6 +140,51 @@ static bool EvaluateDefined(PPValue &Res
 PP.LexNonComment(PeekTok);
   }
 
+  // [cpp.cond]p4:
+  //   Prior to evaluation, macro invocations in the list of preprocessing
+  //   tokens that will become the controlling constant expression are replaced
+  //   (except for those macro names modified by the 'defined' unary operator),
+  //   just as in normal text. If the token 'defined' is generated as a result
+  //   of this replacement process or use of the 'defined' unary operator does
+  //   not match one of the two specified forms prior to macro replacement, the
+  //   behavior is undefined.
+  // This isn't an idle threat, consider this program:
+  //   #define FOO
+  //   #define BAR defined(FOO)
+  //   #if BAR
+  //   ...
+  //   #else
+  //   ...
+  //   #endif
+  // clang and gcc will pick the #if branch while Visual Studio will take the
+  // #else branch.  Emit a warning about this undefined behavior.

Re: [PATCH] D15866: Warn when using `defined` in a macro definition.

2016-01-19 Thread Nico Weber via cfe-commits

thakis closed this revision.
thakis added a comment.

r258128


http://reviews.llvm.org/D15866



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

Re: [PATCH] D16248: [Clang-tidy] rename misc-inefficient-algorithm to performance-inefficient-algorithm

2016-01-19 Thread Alexander Kornienko via cfe-commits

alexfh added inline comments.


Comment at: clang-tidy/misc/MiscTidyModule.cpp:58
@@ -57,3 +56,1 @@
-CheckFactories.registerCheck(
-"misc-inefficient-algorithm");
 CheckFactories.registerCheck(

aaron.ballman wrote:
> alexfh wrote:
> > alexfh wrote:
> > > aaron.ballman wrote:
> > > > This will break projects that enable the misc-inefficient-algorithm 
> > > > check (which clang 3.7 exposed). Is there a reason to not keep this 
> > > > check registered under this name?
> > > > 
> > > > (Perhaps a follow-up patch to allow deprecation of check names so that 
> > > > users are given guidance would make sense.)
> > > I don't feel strongly, but I'm somewhat reluctant to keep old check 
> > > names. With every new clang-tidy version someone starts using on a 
> > > project, they need to carefully look at the list of checks and select 
> > > relevant ones anyway. I think, adding facilities for deprecating checks 
> > > and keeping old names is not going to help much, but will certainly add 
> > > support burden for us.
> > But we certainly need to mention the rename in the release notes for 3.8.
> > I don't feel strongly, but I'm somewhat reluctant to keep old check names. 
> > With every new clang-tidy version someone starts using on a project, they 
> > need to carefully look at the list of checks and select relevant ones 
> > anyway. I think, adding facilities for deprecating checks and keeping old 
> > names is not going to help much, but will certainly add support burden for 
> > us.
> 
> I'm more worried about upgrading existing uses than initiating new uses on a 
> project. If my build system enabled this check for my project, then upgrading 
> clang-tidy will cause that build to break because of an unknown check name, 
> won't it? I think it's reasonable to do that if there's compelling reason 
> (e.g., we remove a check entirely because it's no longer useful for some 
> reason), but I'd like to avoid gratuitously breaking changes because it adds 
> a barrier to people's upgrade paths.
> 
> Oye. I just tested this out and the results were...surprisingly unhelpful.
> ```
> e:\llvm\2015>clang-tidy -checks=misc-hahahaha-nope E:\Desktop\test.cpp --
> e:\llvm\2015>
> ```
> So it seems we don't currently diagnose providing unknown check names at all, 
> which would make this a silently breaking change (existing uses will no 
> longer trigger the check *and* they won't trigger any diagnostic mentioning 
> that the check isn't known). :-(
> If my build system enabled this check for my project, then upgrading 
> clang-tidy will cause that build to break because of an unknown check name, 
> won't it?

Only in one case: when you have just one check enabled. Clang-tidy's -checks= 
option is a **filter**, not a **list**, so it can't detect a presence of 
invalid check names there. We could add this detection, probably (e.g. if 
removal of a glob from the list doesn't change anything), and issue a warning, 
but there is no reason to fail hard, when the check filter contains invalid 
entries, IMO.


http://reviews.llvm.org/D16248



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

Re: [PATCH] D16248: [Clang-tidy] rename misc-inefficient-algorithm to performance-inefficient-algorithm

2016-01-19 Thread Alexander Kornienko via cfe-commits

alexfh added inline comments.


Comment at: clang-tidy/misc/MiscTidyModule.cpp:58
@@ -57,3 +56,1 @@
-CheckFactories.registerCheck(
-"misc-inefficient-algorithm");
 CheckFactories.registerCheck(

alexfh wrote:
> aaron.ballman wrote:
> > alexfh wrote:
> > > alexfh wrote:
> > > > aaron.ballman wrote:
> > > > > This will break projects that enable the misc-inefficient-algorithm 
> > > > > check (which clang 3.7 exposed). Is there a reason to not keep this 
> > > > > check registered under this name?
> > > > > 
> > > > > (Perhaps a follow-up patch to allow deprecation of check names so 
> > > > > that users are given guidance would make sense.)
> > > > I don't feel strongly, but I'm somewhat reluctant to keep old check 
> > > > names. With every new clang-tidy version someone starts using on a 
> > > > project, they need to carefully look at the list of checks and select 
> > > > relevant ones anyway. I think, adding facilities for deprecating checks 
> > > > and keeping old names is not going to help much, but will certainly add 
> > > > support burden for us.
> > > But we certainly need to mention the rename in the release notes for 3.8.
> > > I don't feel strongly, but I'm somewhat reluctant to keep old check 
> > > names. With every new clang-tidy version someone starts using on a 
> > > project, they need to carefully look at the list of checks and select 
> > > relevant ones anyway. I think, adding facilities for deprecating checks 
> > > and keeping old names is not going to help much, but will certainly add 
> > > support burden for us.
> > 
> > I'm more worried about upgrading existing uses than initiating new uses on 
> > a project. If my build system enabled this check for my project, then 
> > upgrading clang-tidy will cause that build to break because of an unknown 
> > check name, won't it? I think it's reasonable to do that if there's 
> > compelling reason (e.g., we remove a check entirely because it's no longer 
> > useful for some reason), but I'd like to avoid gratuitously breaking 
> > changes because it adds a barrier to people's upgrade paths.
> > 
> > Oye. I just tested this out and the results were...surprisingly unhelpful.
> > ```
> > e:\llvm\2015>clang-tidy -checks=misc-hahahaha-nope E:\Desktop\test.cpp --
> > e:\llvm\2015>
> > ```
> > So it seems we don't currently diagnose providing unknown check names at 
> > all, which would make this a silently breaking change (existing uses will 
> > no longer trigger the check *and* they won't trigger any diagnostic 
> > mentioning that the check isn't known). :-(
> > If my build system enabled this check for my project, then upgrading 
> > clang-tidy will cause that build to break because of an unknown check name, 
> > won't it?
> 
> Only in one case: when you have just one check enabled. Clang-tidy's -checks= 
> option is a **filter**, not a **list**, so it can't detect a presence of 
> invalid check names there. We could add this detection, probably (e.g. if 
> removal of a glob from the list doesn't change anything), and issue a 
> warning, but there is no reason to fail hard, when the check filter contains 
> invalid entries, IMO.
(volunteers to implement this? ;))


http://reviews.llvm.org/D16248



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

Re: [PATCH] D15914: [OpenCL] Pipe builtin functions

2016-01-19 Thread Xiuli PAN via cfe-commits

pxli168 updated this revision to Diff 45258.
pxli168 added a comment.

Remove some debug lines.


http://reviews.llvm.org/D15914

Files:
  include/clang/Basic/Builtins.def
  include/clang/Basic/Builtins.h
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Basic/Builtins.cpp
  lib/CodeGen/CGBuiltin.cpp
  lib/Sema/SemaChecking.cpp
  test/CodeGenOpenCL/pipe_builtin.cl
  test/SemaOpenCL/invalid-pipe-builtin-cl2.0.cl

Index: test/SemaOpenCL/invalid-pipe-builtin-cl2.0.cl
===
--- /dev/null
+++ test/SemaOpenCL/invalid-pipe-builtin-cl2.0.cl
@@ -0,0 +1,55 @@
+// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=CL2.0
+
+void test1(read_only pipe int p, global int* ptr){
+  int tmp;
+  reserve_id_t rid;
+
+  // read/write_pipe
+  read_pipe(tmp, p);// expected-error {{first argument to read_pipe must be a pipe type}}
+  read_pipe(p);   // expected-error {{invalid number of arguments to function: read_pipe}}
+  read_pipe(p, tmp, tmp, ptr);   // expected-error {{invalid argument type to function read_pipe (expecting 'reserve_id_t')}}
+  read_pipe(p, rid, rid, ptr);   // expected-error {{invalid argument type to function read_pipe (expecting 'unsigned int')}}
+  read_pipe(p, tmp);   // expected-error {{invalid argument type to function read_pipe (expecting 'int *')}}
+  write_pipe(p, ptr);// expected-error {{invalid pipe access modifier (expecting write_only)}}
+  write_pipe(p, rid, tmp, ptr);// expected-error {{invalid pipe access modifier (expecting write_only)}}
+
+  // reserve_read/write_pipe
+  reserve_read_pipe(p, ptr);// expected-error{{invalid argument type to function reserve_read_pipe (expecting 'unsigned int')}}
+  work_group_reserve_read_pipe(tmp, tmp);// expected-error{{first argument to work_group_reserve_read_pipe must be a pipe type}}
+  sub_group_reserve_write_pipe(p, tmp);// expected-error{{invalid pipe access modifier (expecting write_only)}}
+
+  // commit_read/write_pipe
+  commit_read_pipe(tmp, rid);// expected-error{{first argument to commit_read_pipe must be a pipe type}}
+  work_group_commit_read_pipe(p, tmp);// expected-error{{invalid argument type to function work_group_commit_read_pipe (expecting 'reserve_id_t')}}
+  sub_group_commit_write_pipe(p, tmp);// expected-error{{nvalid pipe access modifier (expecting write_only)}}
+}
+
+void test2(write_only pipe int p, global int* ptr){
+  int tmp;
+  reserve_id_t rid;
+
+  // read/write_pipe
+  write_pipe(tmp, p);// expected-error {{first argument to write_pipe must be a pipe type}}
+  write_pipe(p);   // expected-error {{invalid number of arguments to function: write_pipe}}
+  write_pipe(p, tmp, tmp, ptr);   // expected-error {{invalid argument type to function write_pipe (expecting 'reserve_id_t')}}
+  write_pipe(p, rid, rid, ptr);   // expected-error {{invalid argument type to function write_pipe (expecting 'unsigned int')}}
+  write_pipe(p, tmp);   // expected-error {{invalid argument type to function write_pipe (expecting 'int *')}}
+  read_pipe(p, ptr);// expected-error {{invalid pipe access modifier (expecting read_only)}}
+  read_pipe(p, rid, tmp, ptr);// expected-error {{invalid pipe access modifier (expecting read_only)}}
+
+  // reserve_read/write_pipe
+  reserve_write_pipe(p, ptr);// expected-error{{invalid argument type to function reserve_write_pipe (expecting 'unsigned int')}}
+  work_group_reserve_write_pipe(tmp, tmp);// expected-error{{first argument to work_group_reserve_write_pipe must be a pipe type}}
+  sub_group_reserve_read_pipe(p, tmp);// expected-error{{invalid pipe access modifier (expecting read_only)}}
+
+  // commit_read/write_pipe
+  commit_write_pipe(tmp, rid);// expected-error{{first argument to commit_write_pipe must be a pipe type}}
+  work_group_commit_write_pipe(p, tmp);// expected-error{{invalid argument type to function work_group_commit_write_pipe (expecting 'reserve_id_t')}}
+  sub_group_commit_read_pipe(p, tmp);// expected-error{{nvalid pipe access modifier (expecting read_only)}}
+}
+
+void test3(){
+  int tmp;
+  get_pipe_num_packets(tmp);// expected-error {{first argument to get_pipe_num_packets must be a pipe type}}
+  get_pipe_max_packets(tmp);// expected-error {{first argument to get_pipe_max_packets must be a pipe type}}
+}
Index: test/CodeGenOpenCL/pipe_builtin.cl
===
--- /dev/null
+++ test/CodeGenOpenCL/pipe_builtin.cl
@@ -0,0 +1,61 @@
+// RUN: %clang_cc1 -emit-llvm -O0 -cl-std=CL2.0 -o - %s | FileCheck %s
+
+// CHECK: %opencl.pipe_t = type opaque
+// CHECK: %opencl.reserve_id_t = type opaque
+
+void test1(read_only pipe int p, global int *ptr) {
+  // CHECK: call i32 @__read_pipe_2(%opencl.pipe_t* %{{.*}}, i8* %{{.*}})
+  read_pipe(p, ptr);
+  // CHECK: call %opencl.reserve_id_t* @__reserve_read_pipe(%opencl.pipe_t* %{{.*}}, i32 {{.*}})
+  reserve_id_t rid = reserve_read_pipe(p, 2);
+

Re: [PATCH] D16317: [Analyzer] Fix for PR23790: bind real value returned from strcmp when modelling strcmp.

2016-01-19 Thread Artem Dergachev via cfe-commits

NoQ added a subscriber: NoQ.
NoQ added a comment.

Hmm. If we want to catch bugs resulting from alternative `strcmp()` 
implementations, then probably a test case that demonstrates the improvement 
would be worth it, eg.:

  int x = strcmp("foo", "bar"));
  if (x == 1 || x == -1)
clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
  if (x > 1 || x < -1)
clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}

However, now we don't quite pass it yet, because the hardcoded implementation 
of `strcmp()` is still specific, just different depending on how the clang code 
was compiled (which may be similar to or different from the implementation on 
which the code under analysis relies).

In order to pass such test, we could conjure a symbol for return value of 
`strcmp()` and only enforce range on this symbol (such as `[INT_MIN, -1]` or 
`[1, INT_MAX]`), rather than returning a concrete value.


http://reviews.llvm.org/D16317



___
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-19 Thread Alexander Kornienko via cfe-commits

alexfh added inline comments.


Comment at: clang-tidy/readability/RedundantReturnCheck.h:26
@@ +25,3 @@
+/// 
http://clang.llvm.org/extra/clang-tidy/checks/readability-redundant-return.html
+class RedundantReturnCheck : public ClangTidyCheck {
+public:

aaron.ballman wrote:
> Since this also handling continue, I think this would be 
> SpuriousFlowControlCheck instead?
Maybe `RedundantControlFlowStatementsCheck`?


Comment at: docs/clang-tidy/checks/readability-redundant-return.rst:7
@@ +6,3 @@
+This check looks for procedures (functions returning no value) with `return`
+statements at the end of the function.  Such `return` statements are redundant.
+

Please add an example for `return` and one more for `continue`.


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] D16248: [Clang-tidy] rename misc-inefficient-algorithm to performance-inefficient-algorithm

2016-01-19 Thread Aaron Ballman via cfe-commits

On Tue, Jan 19, 2016 at 10:20 AM, Alexander Kornienko  wrote:
> alexfh added inline comments.
>
> 
> Comment at: clang-tidy/misc/MiscTidyModule.cpp:58
> @@ -57,3 +56,1 @@
> -CheckFactories.registerCheck(
> -"misc-inefficient-algorithm");
>  CheckFactories.registerCheck(
> 
> aaron.ballman wrote:
>> alexfh wrote:
>> > alexfh wrote:
>> > > aaron.ballman wrote:
>> > > > This will break projects that enable the misc-inefficient-algorithm 
>> > > > check (which clang 3.7 exposed). Is there a reason to not keep this 
>> > > > check registered under this name?
>> > > >
>> > > > (Perhaps a follow-up patch to allow deprecation of check names so that 
>> > > > users are given guidance would make sense.)
>> > > I don't feel strongly, but I'm somewhat reluctant to keep old check 
>> > > names. With every new clang-tidy version someone starts using on a 
>> > > project, they need to carefully look at the list of checks and select 
>> > > relevant ones anyway. I think, adding facilities for deprecating checks 
>> > > and keeping old names is not going to help much, but will certainly add 
>> > > support burden for us.
>> > But we certainly need to mention the rename in the release notes for 3.8.
>> > I don't feel strongly, but I'm somewhat reluctant to keep old check names. 
>> > With every new clang-tidy version someone starts using on a project, they 
>> > need to carefully look at the list of checks and select relevant ones 
>> > anyway. I think, adding facilities for deprecating checks and keeping old 
>> > names is not going to help much, but will certainly add support burden for 
>> > us.
>>
>> I'm more worried about upgrading existing uses than initiating new uses on a 
>> project. If my build system enabled this check for my project, then 
>> upgrading clang-tidy will cause that build to break because of an unknown 
>> check name, won't it? I think it's reasonable to do that if there's 
>> compelling reason (e.g., we remove a check entirely because it's no longer 
>> useful for some reason), but I'd like to avoid gratuitously breaking changes 
>> because it adds a barrier to people's upgrade paths.
>>
>> Oye. I just tested this out and the results were...surprisingly unhelpful.
>> ```
>> e:\llvm\2015>clang-tidy -checks=misc-hahahaha-nope E:\Desktop\test.cpp --
>> e:\llvm\2015>
>> ```
>> So it seems we don't currently diagnose providing unknown check names at 
>> all, which would make this a silently breaking change (existing uses will no 
>> longer trigger the check *and* they won't trigger any diagnostic mentioning 
>> that the check isn't known). :-(
>> If my build system enabled this check for my project, then upgrading 
>> clang-tidy will cause that build to break because of an unknown check name, 
>> won't it?
>
> Only in one case: when you have just one check enabled. Clang-tidy's -checks= 
> option is a **filter**, not a **list**, so it can't detect a presence of 
> invalid check names there. We could add this detection, probably (e.g. if 
> removal of a glob from the list doesn't change anything), and issue a 
> warning, but there is no reason to fail hard, when the check filter contains 
> invalid entries, IMO.

The user wrote something and likely assumed it had an effect when it
doesn't have one -- that doesn't seem like intuitive (or particularly
useful) behavior as far as the user is concerned. Typos are easy
mistakes to make ("is inefficient spelled 'inefficient' or
'inefficeint', that whole i before e thing is so tricky"), and the
problem here is that it can be hard for a user to detect when they've
messed up the filter because it's impossible to tell the difference
between "check never ran" and "my code is perfect."

~Aaron
___
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-19 Thread Aaron Ballman via cfe-commits

aaron.ballman added inline comments.


Comment at: clang-tidy/readability/RedundantReturnCheck.h:26
@@ +25,3 @@
+/// 
http://clang.llvm.org/extra/clang-tidy/checks/readability-redundant-return.html
+class RedundantReturnCheck : public ClangTidyCheck {
+public:

alexfh wrote:
> aaron.ballman wrote:
> > Since this also handling continue, I think this would be 
> > SpuriousFlowControlCheck instead?
> Maybe `RedundantControlFlowStatementsCheck`?
I like that name better. :-)


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] D16308: clang-tidy Enhance readability-simplify-boolean-expr check to handle implicit conversions of integral types to bool and member pointers

2016-01-19 Thread Aaron Ballman via cfe-commits

aaron.ballman added a subscriber: aaron.ballman.
aaron.ballman added a reviewer: aaron.ballman.


Comment at: clang-tidy/readability/SimplifyBooleanExprCheck.h:50
@@ -49,3 +49,3 @@
 ///  opposite condition.
 ///   4. Implicit conversions of pointer to `bool` are replaced with explicit
 ///  comparisons to `nullptr`.

Can you also update for member pointers?


Comment at: clang-tidy/readability/SimplifyBooleanExprCheck.h:77
@@ -74,3 +76,3 @@
 ///  implicit conversion of `i & 1` to `bool` and becomes
-///  `bool b = static_cast(i & 1);`.
+///  `bool b = i & 1 != 0;`.
 ///

To me, this does not improve readability -- I now have to think much harder 
about operator precedence. Including parens would help significantly, IMO, so 
that it becomes `(i & 1) != 0`.


Comment at: docs/clang-tidy/checks/readability-simplify-boolean-expr.rst:59
@@ -56,3 +58,3 @@
 
   4. The conditional return ``if (p) return true; return false;`` has an
  implicit conversion of a pointer to ``bool`` and becomes

Update for member pointers.


http://reviews.llvm.org/D16308



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

r258131 - Rename -Wexpansion-to-undefined to -Wexpansion-to-defined.

2016-01-19 Thread Nico Weber via cfe-commits

Author: nico
Date: Tue Jan 19 09:32:55 2016
New Revision: 258131

URL: http://llvm.org/viewvc/llvm-project?rev=258131&view=rev
Log:
Rename -Wexpansion-to-undefined to -Wexpansion-to-defined.

Modified:
cfe/trunk/include/clang/Basic/DiagnosticGroups.td
cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td

Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=258131&r1=258130&r2=258131&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Tue Jan 19 09:32:55 2016
@@ -204,7 +204,7 @@ def OverloadedShiftOpParentheses: DiagGr
 def DanglingElse: DiagGroup<"dangling-else">;
 def DanglingField : DiagGroup<"dangling-field">;
 def DistributedObjectModifiers : DiagGroup<"distributed-object-modifiers">;
-def ExpansionToUndefined : DiagGroup<"expansion-to-undefined">;
+def ExpansionToDefined : DiagGroup<"expansion-to-defined">;
 def FlagEnum : DiagGroup<"flag-enum">;
 def IncrementBool : DiagGroup<"increment-bool", [DeprecatedIncrementBool]>;
 def InfiniteRecursion : DiagGroup<"infinite-recursion">;

Modified: cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td?rev=258131&r1=258130&r2=258131&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td Tue Jan 19 09:32:55 2016
@@ -660,10 +660,10 @@ def note_header_guard : Note<
 
 def warn_defined_in_object_type_macro : Warning<
   "macro expansion producing 'defined' has undefined behavior">,
-  InGroup;
+  InGroup;
 def warn_defined_in_function_type_macro : Extension<
   "macro expansion producing 'defined' has undefined behavior">,
-  InGroup;
+  InGroup;
 
 let CategoryName = "Nullability Issue" in {
 


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

Re: [PATCH] D16267: Handle C++11 brace initializers in readability-braces-around-statements

2016-01-19 Thread Alexander Kornienko via cfe-commits

alexfh added inline comments.


Comment at: clang-tidy/readability/BracesAroundStatementsCheck.cpp:63-72
@@ -62,2 +62,12 @@
   tok::TokenKind TokKind = getTokenKind(Loc, SM, Context);
+  // If we are at "}", but the following token is ";", then we could be
+  // reading a statement like "x = A{};" and "}" does not mean we reached
+  // the end of a statement. Hence we look ahead.
+  SourceLocation LocOneAhead = 
forwardSkipWhitespaceAndComments(Loc.getLocWithOffset(1), SM, Context);
+  tok::TokenKind OneAheadTokKind;
+  if (LocOneAhead.isValid()) {
+OneAheadTokKind = getTokenKind(LocOneAhead, SM, Context);
+  } else {
+OneAheadTokKind = tok::NUM_TOKENS;
+  }
   if (TokKind == tok::NUM_TOKENS || TokKind == tok::semi ||

adek05 wrote:
> alexfh wrote:
> > adek05 wrote:
> > > This might not be the best approach. Maybe it will be better to try to 
> > > find the matching opening brace and see whether it is the opening brace 
> > > for 'then' block. Feel free to throw more suggestions.
> > Maybe just skip correct bracket sequences (a statement can also contain 
> > lambdas, subscript expressions and normal parentheses in various 
> > combinations)?
> Well, we could just remove `TokKind == tok::r_brace` in line 64. I don't 
> really understand why this check is there. The test suite passes if I remove 
> that check, including the new cases I have added. Maybe that's the way to go?
> 
> I tracked the initial commit which introduced `TokKind == tok::r_brace`: 
> D5395 and I don't see a clear reason why it needs to be there. Do you know 
> why we need it?
> Well, we could just remove TokKind == tok::r_brace in line 64.

You mean line 74, I guess. It should be easy to check if removing it helps by 
running tests and applying fixes to a large enough codebase, e.g. LLVM+Clang, 
and running its tests to ensure the correctness of changes + manual inspection 
of a few replacements.


http://reviews.llvm.org/D16267



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

Re: [PATCH] D16215: ASTMatchers: enable hasBody() matcher for FunctionDecls

2016-01-19 Thread Aaron Ballman via cfe-commits

aaron.ballman added inline comments.


Comment at: include/clang/ASTMatchers/ASTMatchers.h:3118
@@ -3119,1 +3117,3 @@
+/// \brief Matches a 'for', 'while', 'do while' statement or a function
+/// declaration that has a given body.
 ///

s/declaration/definition.


Comment at: include/clang/ASTMatchers/ASTMatchersInternal.h:1590
@@ +1589,3 @@
+struct GetBodyMatcher {
+static const Stmt *get(const Ty &Node) {
+  return Node.getBody();

Indentation here is incorrect, you should run clang-format over the patch.


Comment at: lib/ASTMatchers/ASTMatchersInternal.cpp:342
@@ -341,1 +341,3 @@
 
+template <>
+const Stmt *GetBodyMatcher::get(const FunctionDecl &Node) {

The specialization should live in the header file, not the source file.


Comment at: lib/ASTMatchers/ASTMatchersInternal.cpp:344
@@ +343,3 @@
+const Stmt *GetBodyMatcher::get(const FunctionDecl &Node) {
+  return Node.isThisDeclarationADefinition() ? Node.getBody() : NULL;
+}

I would use `Node.doesThisDeclarationHaveABody()` instead; otherwise this will 
match definitions that are explicitly deleted, which isn't particularly useful.


http://reviews.llvm.org/D16215



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

Re: r257868 - PR26111: segmentation fault with attribute((mode(QI))) on function declaration, by Denis Zobnin

2016-01-19 Thread Aaron Ballman via cfe-commits

On Fri, Jan 15, 2016 at 11:55 AM, Aaron Ballman  wrote:
> On Fri, Jan 15, 2016 at 11:52 AM, Hans Wennborg via cfe-commits
>  wrote:
>> Should this be merged to 3.8?
>
> I can't speak to whether it severe enough to warrant it, but I think
> that it is sufficiently safe to merge.

I lied. This caused some regressions in practice, so it should not be
merged into 3.8.

~Aaron

>
> ~Aaron
>
>>
>> On Thu, Jan 14, 2016 at 8:36 PM, Alexey Bataev via cfe-commits
>>  wrote:
>>> Author: abataev
>>> Date: Thu Jan 14 22:36:32 2016
>>> New Revision: 257868
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=257868&view=rev
>>> Log:
>>> PR26111: segmentation fault with __attribute__((mode(QI))) on function 
>>> declaration, by Denis Zobnin
>>> Allow "mode" attribute to be applied to VarDecl, not ValueDecl (which 
>>> includes FunctionDecl and EnumConstantDecl), emit an error if this 
>>> attribute is used with function declarations and enum constants.
>>> Differential Revision: http://reviews.llvm.org/D16112
>>>
>>> Modified:
>>> cfe/trunk/include/clang/AST/Decl.h
>>> cfe/trunk/include/clang/Basic/Attr.td
>>> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>>> cfe/trunk/lib/Sema/SemaDeclAttr.cpp
>>> cfe/trunk/test/Sema/attr-mode.c
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D16301: Allow mode attribute for member variables again

2016-01-19 Thread Aaron Ballman via cfe-commits

aaron.ballman added inline comments.


Comment at: include/clang/Basic/Attr.td:891
@@ -891,2 +890,3 @@
+  let Subjects = SubjectList<[Var, TypedefName, Field], ErrorDiag,
  "ExpectedVariableOrTypedef">;
   let Args = [IdentifierArgument<"Mode">];

This diagnostic is no longer correct because it doesn't talk about fields.


http://reviews.llvm.org/D16301



___
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-19 Thread Alexander Kornienko via cfe-commits

alexfh added inline comments.


Comment at: clang-tidy/readability/RedundantReturnCheck.cpp:36
@@ +35,3 @@
+checkRedundantReturn(Result, Fn);
+  } else if (const auto *For = Result.Nodes.getNodeAs("for")) {
+checkRedundantContinue(Result, dyn_cast(For->getBody()));

Maybe just bind the compound statement and get rid of these `if`s?


Comment at: test/clang-tidy/readability-redundant-return.cpp:1
@@ +1,2 @@
+// RUN: %check_clang_tidy %s readability-redundant-return %t
+

Please add tests with macros and templates that ensure:
  * no replacements in macro bodies
  * correct replacements in macro arguments (that are not inside another 
macro's body)
  * replacements in template bodies with multiple instantiations are applied 
only once (adding `unless(isInTemplateInstantiation())` filter to the matcher 
is a usual way to handle this).


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] D16262: [libc++] Treat trailing backslashes in a regex pattern as invalid.

2016-01-19 Thread Josh Petrie via cfe-commits

jpetrie added a comment.

In http://reviews.llvm.org/D16262#329328, @mclow.lists wrote:

> This looks good to me.  A quick search for `\\` in regex didn't find any 
> other obvious instances of this anti-pattern.  For the record, the bug is 
> https://llvm.org/bugs/show_bug.cgi?id=26175
>
> Can you commit this yourself, or would you rather I did it?


I don't have commit access; if you could commit it, that would be great.


http://reviews.llvm.org/D16262



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

Re: [PATCH] D16113: [clang-tdiy] Add header file extension configuration support.

2016-01-19 Thread Aaron Ballman via cfe-commits

aaron.ballman added inline comments.


Comment at: clang-tidy/google/GlobalNamesInHeadersCheck.h:26
@@ +25,3 @@
+///   - `HeaderFileExtensions`: a comma-separated list of filename extensions 
of
+/// header files (no need to includ "."). "h" by default.
+/// For extension-less header files, using an empty string or leaving an

s/includ/include

Is "." tolerated in the extension? It sounds like you can write: `h,.hpp,hxx` 
and it will work? Best to clarify the requirements (here and elsewhere). I 
don't have a strong opinion on whether the extension should include the full 
stop or not.


Comment at: clang-tidy/google/GlobalNamesInHeadersCheck.h:28
@@ -22,1 +27,3 @@
+/// For extension-less header files, using an empty string or leaving an
+/// empty string between "," if there are other file extensions.
 class GlobalNamesInHeadersCheck : public ClangTidyCheck {

What if you only want to match extensionless header files?


Comment at: clang-tidy/google/UnnamedNamespaceInHeaderCheck.h:25
@@ +24,3 @@
+///   - `HeaderFileExtensions`: a comma-separated list of filename extensions 
of
+/// header files (no need to includ "."). "h,hh,hpp,hxx" by default.
+/// For extension-less header files, using an empty string or leaving an

s/includ/include

Same comment about use of ".".


Comment at: clang-tidy/misc/DefinitionsInHeadersCheck.h:27
@@ +26,3 @@
+///   - `HeaderFileExtensions`: a comma-separated list of filename extensions 
of
+/// header files (no need to includ "."). ",h,hh,hpp,hxx" by default.
+/// For extension-less header files, using an empty string or leaving an

s/includ/include

Also, same comments about the full stop.


http://reviews.llvm.org/D16113



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

Re: [PATCH] D16248: [Clang-tidy] rename misc-inefficient-algorithm to performance-inefficient-algorithm

2016-01-19 Thread Alexander Kornienko via cfe-commits

On Tue, Jan 19, 2016 at 4:28 PM, Aaron Ballman 
wrote:

> On Tue, Jan 19, 2016 at 10:20 AM, Alexander Kornienko 
> wrote:
> > alexfh added inline comments.
> >
> > 
> > Comment at: clang-tidy/misc/MiscTidyModule.cpp:58
> > @@ -57,3 +56,1 @@
> > -CheckFactories.registerCheck(
> > -"misc-inefficient-algorithm");
> >  CheckFactories.registerCheck(
> > 
> > aaron.ballman wrote:
> >> alexfh wrote:
> >> > alexfh wrote:
> >> > > aaron.ballman wrote:
> >> > > > This will break projects that enable the
> misc-inefficient-algorithm check (which clang 3.7 exposed). Is there a
> reason to not keep this check registered under this name?
> >> > > >
> >> > > > (Perhaps a follow-up patch to allow deprecation of check names so
> that users are given guidance would make sense.)
> >> > > I don't feel strongly, but I'm somewhat reluctant to keep old check
> names. With every new clang-tidy version someone starts using on a project,
> they need to carefully look at the list of checks and select relevant ones
> anyway. I think, adding facilities for deprecating checks and keeping old
> names is not going to help much, but will certainly add support burden for
> us.
> >> > But we certainly need to mention the rename in the release notes for
> 3.8.
> >> > I don't feel strongly, but I'm somewhat reluctant to keep old check
> names. With every new clang-tidy version someone starts using on a project,
> they need to carefully look at the list of checks and select relevant ones
> anyway. I think, adding facilities for deprecating checks and keeping old
> names is not going to help much, but will certainly add support burden for
> us.
> >>
> >> I'm more worried about upgrading existing uses than initiating new uses
> on a project. If my build system enabled this check for my project, then
> upgrading clang-tidy will cause that build to break because of an unknown
> check name, won't it? I think it's reasonable to do that if there's
> compelling reason (e.g., we remove a check entirely because it's no longer
> useful for some reason), but I'd like to avoid gratuitously breaking
> changes because it adds a barrier to people's upgrade paths.
> >>
> >> Oye. I just tested this out and the results were...surprisingly
> unhelpful.
> >> ```
> >> e:\llvm\2015>clang-tidy -checks=misc-hahahaha-nope E:\Desktop\test.cpp
> --
> >> e:\llvm\2015>
> >> ```
> >> So it seems we don't currently diagnose providing unknown check names
> at all, which would make this a silently breaking change (existing uses
> will no longer trigger the check *and* they won't trigger any diagnostic
> mentioning that the check isn't known). :-(
> >> If my build system enabled this check for my project, then upgrading
> clang-tidy will cause that build to break because of an unknown check name,
> won't it?
> >
> > Only in one case: when you have just one check enabled. Clang-tidy's
> -checks= option is a **filter**, not a **list**, so it can't detect a
> presence of invalid check names there. We could add this detection,
> probably (e.g. if removal of a glob from the list doesn't change anything),
> and issue a warning, but there is no reason to fail hard, when the check
> filter contains invalid entries, IMO.
>
> The user wrote something and likely assumed it had an effect when it
> doesn't have one -- that doesn't seem like intuitive (or particularly
> useful) behavior as far as the user is concerned. Typos are easy
> mistakes to make ("is inefficient spelled 'inefficient' or
> 'inefficeint', that whole i before e thing is so tricky"), and the
> problem here is that it can be hard for a user to detect when they've
> messed up the filter because it's impossible to tell the difference
> between "check never ran" and "my code is perfect."
>

That's why I say clang-tidy could issue a warning, if a glob list fed to
-checks= has entries that have no effect. The only question is who is
bothered enough by this and has time to implement this safeguard. ;)


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

Re: [PATCH] D12192: Add clang support for AAP

2016-01-19 Thread Edward Jones via cfe-commits

edward-jones updated this revision to Diff 45262.
edward-jones added a comment.

This updates the Clang support of AAP to top of tree.


http://reviews.llvm.org/D12192

Files:
  lib/Basic/Targets.cpp
  lib/Driver/Driver.cpp
  lib/Driver/ToolChains.cpp
  lib/Driver/ToolChains.h
  lib/Driver/Tools.cpp
  lib/Driver/Tools.h
  lib/Headers/float.h

Index: lib/Headers/float.h
===
--- lib/Headers/float.h
+++ lib/Headers/float.h
@@ -74,7 +74,14 @@
 /* Characteristics of floating point types, C99 5.2.4.2.2 */
 
 #define FLT_EVAL_METHOD __FLT_EVAL_METHOD__
-#define FLT_ROUNDS (__builtin_flt_rounds())
+
+/* __builtin_flt_rounds is not supported by AAP, and the rounding mode cannot
+   be changed anyway so we just default to 'to nearest' */
+#ifdef __AAP__
+  #define FLT_ROUNDS 1
+#else
+  #define FLT_ROUNDS (__builtin_flt_rounds())
+#endif
 #define FLT_RADIX __FLT_RADIX__
 
 #define FLT_MANT_DIG __FLT_MANT_DIG__
Index: lib/Driver/Tools.h
===
--- lib/Driver/Tools.h
+++ lib/Driver/Tools.h
@@ -60,6 +60,8 @@
const InputInfoList &Inputs,
const ToolChain *AuxToolChain) const;
 
+  void AddAAPTargetArgs(const llvm::opt::ArgList &Args,
+llvm::opt::ArgStringList &CmdArgs) const;
   void AddAArch64TargetArgs(const llvm::opt::ArgList &Args,
 llvm::opt::ArgStringList &CmdArgs) const;
   void AddARMTargetArgs(const llvm::Triple &Triple,
@@ -938,6 +940,34 @@
 
 }  // end namespace NVPTX
 
+namespace AAP {
+  class LLVM_LIBRARY_VISIBILITY Assemble : public Tool {
+  public:
+Assemble(const ToolChain &TC) : Tool("AAP::Assemble", "aap-as", TC)
+{}
+
+bool hasIntegratedCPP() const override { return false; }
+void ConstructJob(Compilation &C, const JobAction &JA,
+  const InputInfo &Output,
+  const InputInfoList &Inputs,
+  const llvm::opt::ArgList &TCArgs,
+  const char *LinkingOutput) const override;
+  };
+  class LLVM_LIBRARY_VISIBILITY Link : public Tool {
+  public:
+Link(const ToolChain &TC) : Tool("AAP::Link", "aap-ld", TC)
+{}
+
+bool hasIntegratedCPP() const override { return false; }
+bool isLinkJob() const override { return true; }
+void ConstructJob(Compilation &C, const JobAction &JA,
+  const InputInfo &Output,
+  const InputInfoList &Inputs,
+  const llvm::opt::ArgList &TCArgs,
+  const char *LinkingOutput) const override;
+  };
+} // end namespace AAP
+
 } // end namespace tools
 } // end namespace driver
 } // end namespace clang
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -2069,6 +2069,11 @@
   CmdArgs.push_back("-machine-sink-split=0");
 }
 
+void Clang::AddAAPTargetArgs(const ArgList &Args,
+ ArgStringList &CmdArgs) const {
+  return;
+}
+
 void Clang::AddWebAssemblyTargetArgs(const ArgList &Args,
  ArgStringList &CmdArgs) const {
   // Default to "hidden" visibility.
@@ -2915,8 +2920,10 @@
   case llvm::Triple::xcore:
   case llvm::Triple::wasm32:
   case llvm::Triple::wasm64:
+  case llvm::Triple::aap:
 // XCore never wants frame pointers, regardless of OS.
 // WebAssembly never wants frame pointers.
+// AAP never wants frame pointers
 return false;
   default:
 break;
@@ -4028,6 +4035,10 @@
   default:
 break;
 
+  case llvm::Triple::aap:
+AddAAPTargetArgs(Args, CmdArgs);
+break;
+
   case llvm::Triple::arm:
   case llvm::Triple::armeb:
   case llvm::Triple::thumb:
@@ -10721,3 +10732,71 @@
   const char *Exec = Args.MakeArgString(TC.GetProgramPath("fatbinary"));
   C.addCommand(llvm::make_unique(JA, *this, Exec, CmdArgs, Inputs));
 }
+
+void AAP::Assemble::ConstructJob(Compilation &C, const JobAction &JA,
+ const InputInfo &Output,
+ const InputInfoList &Inputs,
+ const ArgList &Args,
+ const char *LinkingOutput) const {
+  ArgStringList CmdArgs;
+
+  // Add input assembly files to command line
+  for (InputInfoList::const_iterator it = Inputs.begin(), ie = Inputs.end();
+   it != ie;
+   ++it) {
+const InputInfo &II = *it;
+CmdArgs.push_back(II.getFilename());
+  }
+
+  const char *Exec =
+Args.MakeArgString(getToolChain().GetProgramPath("aap-as"));
+
+  C.addCommand(llvm::make_unique(JA, *this, Exec, CmdArgs, Inputs));
+}
+
+void AAP::Link::ConstructJob(Compilation &C, const JobAction &JA,
+ const InputInfo &Output,
+ const InputInfoList &Inputs,
+ const

Re: [PATCH] D16248: [Clang-tidy] rename misc-inefficient-algorithm to performance-inefficient-algorithm

2016-01-19 Thread Aaron Ballman via cfe-commits

On Tue, Jan 19, 2016 at 11:06 AM, Alexander Kornienko  wrote:
> On Tue, Jan 19, 2016 at 4:28 PM, Aaron Ballman 
> wrote:
>>
>> On Tue, Jan 19, 2016 at 10:20 AM, Alexander Kornienko 
>> wrote:
>> > alexfh added inline comments.
>> >
>> > 
>> > Comment at: clang-tidy/misc/MiscTidyModule.cpp:58
>> > @@ -57,3 +56,1 @@
>> > -CheckFactories.registerCheck(
>> > -"misc-inefficient-algorithm");
>> >  CheckFactories.registerCheck(
>> > 
>> > aaron.ballman wrote:
>> >> alexfh wrote:
>> >> > alexfh wrote:
>> >> > > aaron.ballman wrote:
>> >> > > > This will break projects that enable the
>> >> > > > misc-inefficient-algorithm check (which clang 3.7 exposed). Is 
>> >> > > > there a
>> >> > > > reason to not keep this check registered under this name?
>> >> > > >
>> >> > > > (Perhaps a follow-up patch to allow deprecation of check names so
>> >> > > > that users are given guidance would make sense.)
>> >> > > I don't feel strongly, but I'm somewhat reluctant to keep old check
>> >> > > names. With every new clang-tidy version someone starts using on a 
>> >> > > project,
>> >> > > they need to carefully look at the list of checks and select relevant 
>> >> > > ones
>> >> > > anyway. I think, adding facilities for deprecating checks and keeping 
>> >> > > old
>> >> > > names is not going to help much, but will certainly add support 
>> >> > > burden for
>> >> > > us.
>> >> > But we certainly need to mention the rename in the release notes for
>> >> > 3.8.
>> >> > I don't feel strongly, but I'm somewhat reluctant to keep old check
>> >> > names. With every new clang-tidy version someone starts using on a 
>> >> > project,
>> >> > they need to carefully look at the list of checks and select relevant 
>> >> > ones
>> >> > anyway. I think, adding facilities for deprecating checks and keeping 
>> >> > old
>> >> > names is not going to help much, but will certainly add support burden 
>> >> > for
>> >> > us.
>> >>
>> >> I'm more worried about upgrading existing uses than initiating new uses
>> >> on a project. If my build system enabled this check for my project, then
>> >> upgrading clang-tidy will cause that build to break because of an unknown
>> >> check name, won't it? I think it's reasonable to do that if there's
>> >> compelling reason (e.g., we remove a check entirely because it's no longer
>> >> useful for some reason), but I'd like to avoid gratuitously breaking 
>> >> changes
>> >> because it adds a barrier to people's upgrade paths.
>> >>
>> >> Oye. I just tested this out and the results were...surprisingly
>> >> unhelpful.
>> >> ```
>> >> e:\llvm\2015>clang-tidy -checks=misc-hahahaha-nope E:\Desktop\test.cpp
>> >> --
>> >> e:\llvm\2015>
>> >> ```
>> >> So it seems we don't currently diagnose providing unknown check names
>> >> at all, which would make this a silently breaking change (existing uses 
>> >> will
>> >> no longer trigger the check *and* they won't trigger any diagnostic
>> >> mentioning that the check isn't known). :-(
>> >> If my build system enabled this check for my project, then upgrading
>> >> clang-tidy will cause that build to break because of an unknown check 
>> >> name,
>> >> won't it?
>> >
>> > Only in one case: when you have just one check enabled. Clang-tidy's
>> > -checks= option is a **filter**, not a **list**, so it can't detect a
>> > presence of invalid check names there. We could add this detection, 
>> > probably
>> > (e.g. if removal of a glob from the list doesn't change anything), and 
>> > issue
>> > a warning, but there is no reason to fail hard, when the check filter
>> > contains invalid entries, IMO.
>>
>> The user wrote something and likely assumed it had an effect when it
>> doesn't have one -- that doesn't seem like intuitive (or particularly
>> useful) behavior as far as the user is concerned. Typos are easy
>> mistakes to make ("is inefficient spelled 'inefficient' or
>> 'inefficeint', that whole i before e thing is so tricky"), and the
>> problem here is that it can be hard for a user to detect when they've
>> messed up the filter because it's impossible to tell the difference
>> between "check never ran" and "my code is perfect."
>
>
> That's why I say clang-tidy could issue a warning, if a glob list fed to
> -checks= has entries that have no effect. The only question is who is
> bothered enough by this and has time to implement this safeguard. ;)

Hmm, if only we knew of someone, anyone at all, who was bothered by
this. Hmmm.. ;-)

I can tackle this when I get the chance. :-D

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

Re: [PATCH] D16248: [Clang-tidy] rename misc-inefficient-algorithm to performance-inefficient-algorithm

2016-01-19 Thread Aaron Ballman via cfe-commits

On Tue, Jan 19, 2016 at 11:07 AM, Aaron Ballman  wrote:
> On Tue, Jan 19, 2016 at 11:06 AM, Alexander Kornienko  
> wrote:
>> On Tue, Jan 19, 2016 at 4:28 PM, Aaron Ballman 
>> wrote:
>>>
>>> On Tue, Jan 19, 2016 at 10:20 AM, Alexander Kornienko 
>>> wrote:
>>> > alexfh added inline comments.
>>> >
>>> > 
>>> > Comment at: clang-tidy/misc/MiscTidyModule.cpp:58
>>> > @@ -57,3 +56,1 @@
>>> > -CheckFactories.registerCheck(
>>> > -"misc-inefficient-algorithm");
>>> >  CheckFactories.registerCheck(
>>> > 
>>> > aaron.ballman wrote:
>>> >> alexfh wrote:
>>> >> > alexfh wrote:
>>> >> > > aaron.ballman wrote:
>>> >> > > > This will break projects that enable the
>>> >> > > > misc-inefficient-algorithm check (which clang 3.7 exposed). Is 
>>> >> > > > there a
>>> >> > > > reason to not keep this check registered under this name?
>>> >> > > >
>>> >> > > > (Perhaps a follow-up patch to allow deprecation of check names so
>>> >> > > > that users are given guidance would make sense.)
>>> >> > > I don't feel strongly, but I'm somewhat reluctant to keep old check
>>> >> > > names. With every new clang-tidy version someone starts using on a 
>>> >> > > project,
>>> >> > > they need to carefully look at the list of checks and select 
>>> >> > > relevant ones
>>> >> > > anyway. I think, adding facilities for deprecating checks and 
>>> >> > > keeping old
>>> >> > > names is not going to help much, but will certainly add support 
>>> >> > > burden for
>>> >> > > us.
>>> >> > But we certainly need to mention the rename in the release notes for
>>> >> > 3.8.
>>> >> > I don't feel strongly, but I'm somewhat reluctant to keep old check
>>> >> > names. With every new clang-tidy version someone starts using on a 
>>> >> > project,
>>> >> > they need to carefully look at the list of checks and select relevant 
>>> >> > ones
>>> >> > anyway. I think, adding facilities for deprecating checks and keeping 
>>> >> > old
>>> >> > names is not going to help much, but will certainly add support burden 
>>> >> > for
>>> >> > us.
>>> >>
>>> >> I'm more worried about upgrading existing uses than initiating new uses
>>> >> on a project. If my build system enabled this check for my project, then
>>> >> upgrading clang-tidy will cause that build to break because of an unknown
>>> >> check name, won't it? I think it's reasonable to do that if there's
>>> >> compelling reason (e.g., we remove a check entirely because it's no 
>>> >> longer
>>> >> useful for some reason), but I'd like to avoid gratuitously breaking 
>>> >> changes
>>> >> because it adds a barrier to people's upgrade paths.
>>> >>
>>> >> Oye. I just tested this out and the results were...surprisingly
>>> >> unhelpful.
>>> >> ```
>>> >> e:\llvm\2015>clang-tidy -checks=misc-hahahaha-nope E:\Desktop\test.cpp
>>> >> --
>>> >> e:\llvm\2015>
>>> >> ```
>>> >> So it seems we don't currently diagnose providing unknown check names
>>> >> at all, which would make this a silently breaking change (existing uses 
>>> >> will
>>> >> no longer trigger the check *and* they won't trigger any diagnostic
>>> >> mentioning that the check isn't known). :-(
>>> >> If my build system enabled this check for my project, then upgrading
>>> >> clang-tidy will cause that build to break because of an unknown check 
>>> >> name,
>>> >> won't it?
>>> >
>>> > Only in one case: when you have just one check enabled. Clang-tidy's
>>> > -checks= option is a **filter**, not a **list**, so it can't detect a
>>> > presence of invalid check names there. We could add this detection, 
>>> > probably
>>> > (e.g. if removal of a glob from the list doesn't change anything), and 
>>> > issue
>>> > a warning, but there is no reason to fail hard, when the check filter
>>> > contains invalid entries, IMO.
>>>
>>> The user wrote something and likely assumed it had an effect when it
>>> doesn't have one -- that doesn't seem like intuitive (or particularly
>>> useful) behavior as far as the user is concerned. Typos are easy
>>> mistakes to make ("is inefficient spelled 'inefficient' or
>>> 'inefficeint', that whole i before e thing is so tricky"), and the
>>> problem here is that it can be hard for a user to detect when they've
>>> messed up the filter because it's impossible to tell the difference
>>> between "check never ran" and "my code is perfect."
>>
>>
>> That's why I say clang-tidy could issue a warning, if a glob list fed to
>> -checks= has entries that have no effect. The only question is who is
>> bothered enough by this and has time to implement this safeguard. ;)
>
> Hmm, if only we knew of someone, anyone at all, who was bothered by
> this. Hmmm.. ;-)
>
> I can tackle this when I get the chance. :-D

However, it still leave the question unanswered about what to do with
renaming this check. Right now it will cause silent behavior changes
for existing build systems that enable the check under the old name.
Someday it will hopefully bark about the name not being recognized as

[clang-tools-extra] r258133 - [clang-tidy] Python scripts shebang fixes

2016-01-19 Thread Alexander Kornienko via cfe-commits

Author: alexfh
Date: Tue Jan 19 10:10:39 2016
New Revision: 258133

URL: http://llvm.org/viewvc/llvm-project?rev=258133&view=rev
Log:
[clang-tidy] Python scripts shebang fixes

Summary:
This patch fixes shebang lines in Python script files.

Most Python scripts in LLVM & Clang are using this shebang line.

[[ https://mail.python.org/pipermail/tutor/2007-June/054816.html | Here]] is an 
explanaiton of why such line should be used instead of what is currently in 
these few files.

Reviewers: klimek, alexfh

Subscribers: cfe-commits

Patch by Kirill Bobyrev!

Differential Revision: http://reviews.llvm.org/D16270

Modified:
clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py
clang-tools-extra/trunk/docs/clang-tidy/tools/dump_check_docs.py
clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy.py

Modified: clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py?rev=258133&r1=258132&r2=258133&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py (original)
+++ clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py Tue Jan 19 
10:10:39 2016
@@ -1,4 +1,4 @@
-#!/usr/bin/python
+#!/usr/bin/env python
 #
 #===- clang-tidy-diff.py - ClangTidy Diff Checker *- python 
-*--===#
 #

Modified: clang-tools-extra/trunk/docs/clang-tidy/tools/dump_check_docs.py
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/tools/dump_check_docs.py?rev=258133&r1=258132&r2=258133&view=diff
==
--- clang-tools-extra/trunk/docs/clang-tidy/tools/dump_check_docs.py (original)
+++ clang-tools-extra/trunk/docs/clang-tidy/tools/dump_check_docs.py Tue Jan 19 
10:10:39 2016
@@ -1,4 +1,4 @@
-#!/usr/bin/python
+#!/usr/bin/env python
 
 r"""
 Create stubs for check documentation files.

Modified: clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy.py
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy.py?rev=258133&r1=258132&r2=258133&view=diff
==
--- clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy.py (original)
+++ clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy.py Tue Jan 19 
10:10:39 2016
@@ -1,4 +1,4 @@
-#!/usr/bin/python
+#!/usr/bin/env python
 #
 #===- check_clang_tidy.py - ClangTidy Test Helper *- python 
-*--===#
 #


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

Re: r258128 - Add -Wexpansion-to-undefined: warn when using `defined` in a macro definition.

2016-01-19 Thread Krzysztof Parzyszek via cfe-commits


This generates hundreds of warnings when doing check-all.

Here's the offending code:


utils/unittest/googletest/include/gtest/internal/gtest-port.h

// Cygwin 1.7 and below doesn't support ::std::wstring.
// Solaris' libc++ doesn't support it either.  Android has
// no support for it at least as recent as Froyo (2.2).
// Minix currently doesn't support it either.
# define GTEST_HAS_STD_WSTRING \
(!(GTEST_OS_LINUX_ANDROID || GTEST_OS_CYGWIN || GTEST_OS_SOLARIS || 
GTEST_OS_HAIKU || defined(_MINIX)))



-Krzysztof



On 1/19/2016 9:15 AM, Nico Weber via cfe-commits wrote:

Author: nico
Date: Tue Jan 19 09:15:31 2016
New Revision: 258128

URL: http://llvm.org/viewvc/llvm-project?rev=258128&view=rev
Log:
Add -Wexpansion-to-undefined: warn when using `defined` in a macro definition.

[cpp.cond]p4:
   Prior to evaluation, macro invocations in the list of preprocessing
   tokens that will become the controlling constant expression are replaced
   (except for those macro names modified by the 'defined' unary operator),
   just as in normal text. If the token 'defined' is generated as a result
   of this replacement process or use of the 'defined' unary operator does
   not match one of the two specified forms prior to macro replacement, the
   behavior is undefined.

This isn't an idle threat, consider this program:
   #define FOO
   #define BAR defined(FOO)
   #if BAR
   ...
   #else
   ...
   #endif
clang and gcc will pick the #if branch while Visual Studio will take the
#else branch.  Emit a warning about this undefined behavior.

One problem is that this also applies to function-like macros. While the
example above can be written like

 #if defined(FOO) && defined(BAR)
 #defined HAVE_FOO 1
 #else
 #define HAVE_FOO 0
 #endif

there is no easy way to rewrite a function-like macro like `#define FOO(x)
(defined __foo_##x && __foo_##x)`.  Function-like macros like this are used in
practice, and compilers seem to not have differing behavior in that case. So
this a default-on warning only for object-like macros. For function-like
macros, it is an extension warning that only shows up with `-pedantic`.
(But it's undefined behavior in both cases.)

Modified:
 cfe/trunk/include/clang/Basic/DiagnosticGroups.td
 cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td
 cfe/trunk/lib/Lex/PPExpressions.cpp
 cfe/trunk/test/Lexer/cxx-features.cpp
 cfe/trunk/test/Preprocessor/expr_define_expansion.c

Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=258128&r1=258127&r2=258128&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Tue Jan 19 09:15:31 2016
@@ -204,6 +204,7 @@ def OverloadedShiftOpParentheses: DiagGr
  def DanglingElse: DiagGroup<"dangling-else">;
  def DanglingField : DiagGroup<"dangling-field">;
  def DistributedObjectModifiers : DiagGroup<"distributed-object-modifiers">;
+def ExpansionToUndefined : DiagGroup<"expansion-to-undefined">;
  def FlagEnum : DiagGroup<"flag-enum">;
  def IncrementBool : DiagGroup<"increment-bool", [DeprecatedIncrementBool]>;
  def InfiniteRecursion : DiagGroup<"infinite-recursion">;

Modified: cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td?rev=258128&r1=258127&r2=258128&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td Tue Jan 19 09:15:31 2016
@@ -658,6 +658,13 @@ def warn_header_guard : Warning<
  def note_header_guard : Note<
"%0 is defined here; did you mean %1?">;

+def warn_defined_in_object_type_macro : Warning<
+  "macro expansion producing 'defined' has undefined behavior">,
+  InGroup;
+def warn_defined_in_function_type_macro : Extension<
+  "macro expansion producing 'defined' has undefined behavior">,
+  InGroup;
+
  let CategoryName = "Nullability Issue" in {

  def err_pp_assume_nonnull_syntax : Error<"expected 'begin' or 'end'">;

Modified: cfe/trunk/lib/Lex/PPExpressions.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPExpressions.cpp?rev=258128&r1=258127&r2=258128&view=diff
==
--- cfe/trunk/lib/Lex/PPExpressions.cpp (original)
+++ cfe/trunk/lib/Lex/PPExpressions.cpp Tue Jan 19 09:15:31 2016
@@ -140,6 +140,51 @@ static bool EvaluateDefined(PPValue &Res
  PP.LexNonComment(PeekTok);
}

+  // [cpp.cond]p4:
+  //   Prior to evaluation, macro invocations in the list of preprocessing
+  //   tokens that will become the controlling constant expression are replaced
+  //   (except for those macro names modified by the 'defi

Re: [PATCH] D16270: [clang-tidy] Python scripts shebang fixes

2016-01-19 Thread Alexander Kornienko via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL258133: [clang-tidy] Python scripts shebang fixes (authored 
by alexfh).

Changed prior to commit:
  http://reviews.llvm.org/D16270?vs=45106&id=45265#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D16270

Files:
  clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py
  clang-tools-extra/trunk/docs/clang-tidy/tools/dump_check_docs.py
  clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy.py

Index: clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy.py
===
--- clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy.py
+++ clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy.py
@@ -1,4 +1,4 @@
-#!/usr/bin/python
+#!/usr/bin/env python
 #
 #===- check_clang_tidy.py - ClangTidy Test Helper *- python 
-*--===#
 #
Index: clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py
===
--- clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py
+++ clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py
@@ -1,4 +1,4 @@
-#!/usr/bin/python
+#!/usr/bin/env python
 #
 #===- clang-tidy-diff.py - ClangTidy Diff Checker *- python 
-*--===#
 #
Index: clang-tools-extra/trunk/docs/clang-tidy/tools/dump_check_docs.py
===
--- clang-tools-extra/trunk/docs/clang-tidy/tools/dump_check_docs.py
+++ clang-tools-extra/trunk/docs/clang-tidy/tools/dump_check_docs.py
@@ -1,4 +1,4 @@
-#!/usr/bin/python
+#!/usr/bin/env python
 
 r"""
 Create stubs for check documentation files.


Index: clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy.py
===
--- clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy.py
+++ clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy.py
@@ -1,4 +1,4 @@
-#!/usr/bin/python
+#!/usr/bin/env python
 #
 #===- check_clang_tidy.py - ClangTidy Test Helper *- python -*--===#
 #
Index: clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py
===
--- clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py
+++ clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py
@@ -1,4 +1,4 @@
-#!/usr/bin/python
+#!/usr/bin/env python
 #
 #===- clang-tidy-diff.py - ClangTidy Diff Checker *- python -*--===#
 #
Index: clang-tools-extra/trunk/docs/clang-tidy/tools/dump_check_docs.py
===
--- clang-tools-extra/trunk/docs/clang-tidy/tools/dump_check_docs.py
+++ clang-tools-extra/trunk/docs/clang-tidy/tools/dump_check_docs.py
@@ -1,4 +1,4 @@
-#!/usr/bin/python
+#!/usr/bin/env python
 
 r"""
 Create stubs for check documentation files.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D15373: Fix for bug 25786 - Assertion "Chunk.Kind == DeclaratorChunk::Function" failed with regparm attribute.

2016-01-19 Thread Aaron Ballman via cfe-commits

aaron.ballman added a comment.

This looks correct to me, but my knowledge of type attributes isn't strong 
enough to officially sign off, so please wait for @rnk or @rsmith to take a 
look as well.


http://reviews.llvm.org/D15373



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

Re: [PATCH] D16215: ASTMatchers: enable hasBody() matcher for FunctionDecls

2016-01-19 Thread Aleksei Sidorin via cfe-commits

a.sidorin updated this revision to Diff 45267.
a.sidorin added a comment.

Fix issues pointed on review.


http://reviews.llvm.org/D16215

Files:
  docs/LibASTMatchersReference.html
  include/clang/ASTMatchers/ASTMatchers.h
  include/clang/ASTMatchers/ASTMatchersInternal.h
  unittests/ASTMatchers/ASTMatchersTest.cpp
  unittests/ASTMatchers/Dynamic/ParserTest.cpp

Index: unittests/ASTMatchers/Dynamic/ParserTest.cpp
===
--- unittests/ASTMatchers/Dynamic/ParserTest.cpp
+++ unittests/ASTMatchers/Dynamic/ParserTest.cpp
@@ -263,7 +263,7 @@
 "1:1: Matcher does not support binding.",
 ParseWithError("isArrow().bind(\"foo\")"));
   EXPECT_EQ("Input value has unresolved overloaded type: "
-"Matcher",
+"Matcher",
 ParseMatcherWithError("hasBody(stmt())"));
 }
 
Index: unittests/ASTMatchers/ASTMatchersTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersTest.cpp
+++ unittests/ASTMatchers/ASTMatchersTest.cpp
@@ -2869,6 +2869,10 @@
   doStmt(hasBody(compoundStmt();
   EXPECT_TRUE(matches("void f() { int p[2]; for (auto x : p) {} }",
   cxxForRangeStmt(hasBody(compoundStmt();
+  EXPECT_TRUE(matches("void f() {}", functionDecl(hasBody(compoundStmt();
+  EXPECT_TRUE(notMatches("void f();", functionDecl(hasBody(compoundStmt();
+  EXPECT_TRUE(matches("void f(); void f() {}",
+ functionDecl(hasBody(compoundStmt();
 }
 
 TEST(HasAnySubstatement, MatchesForTopLevelCompoundStatement) {
Index: include/clang/ASTMatchers/ASTMatchersInternal.h
===
--- include/clang/ASTMatchers/ASTMatchersInternal.h
+++ include/clang/ASTMatchers/ASTMatchersInternal.h
@@ -1584,6 +1584,18 @@
   ast_type_traits::DynTypedNode Node;
 };
 
+template 
+struct GetBodyMatcher {
+  static const Stmt *get(const Ty &Node) {
+return Node.getBody();
+  }
+};
+
+template <>
+inline const Stmt *GetBodyMatcher::get(const FunctionDecl &Node) {
+  return Node.doesThisDeclarationHaveABody() ? Node.getBody() : NULL;
+}
+
 } // end namespace internal
 } // end namespace ast_matchers
 } // end namespace clang
Index: include/clang/ASTMatchers/ASTMatchers.h
===
--- include/clang/ASTMatchers/ASTMatchers.h
+++ include/clang/ASTMatchers/ASTMatchers.h
@@ -3114,8 +3114,8 @@
   return false;
 }
 
-/// \brief Matches a 'for', 'while', or 'do while' statement that has
-/// a given body.
+/// \brief Matches a 'for', 'while', 'do while' statement or a function
+/// definition that has a given body.
 ///
 /// Given
 /// \code
@@ -3128,9 +3128,10 @@
 AST_POLYMORPHIC_MATCHER_P(hasBody,
   AST_POLYMORPHIC_SUPPORTED_TYPES(DoStmt, ForStmt,
   WhileStmt,
-  CXXForRangeStmt),
+  CXXForRangeStmt,
+  FunctionDecl),
   internal::Matcher, InnerMatcher) {
-  const Stmt *const Statement = Node.getBody();
+  const Stmt *const Statement = internal::GetBodyMatcher::get(Node);
   return (Statement != nullptr &&
   InnerMatcher.matches(*Statement, Finder, Builder));
 }
Index: docs/LibASTMatchersReference.html
===
--- docs/LibASTMatchersReference.html
+++ docs/LibASTMatchersReference.html
@@ -3400,8 +3400,8 @@
 
 
 MatcherCXXForRangeStmt>hasBodyMatcherStmt> InnerMatcher
-Matches a 'for', 'while', or 'do while' statement that has
-a given body.
+Matches a 'for', 'while', 'do while' statement or a function
+definition that has a given body.
 
 Given
   for (;;) {}
@@ -3825,8 +3825,8 @@
 
 
 MatcherDoStmt>hasBodyMatcherStmt> InnerMatcher
-Matches a 'for', 'while', or 'do while' statement that has
-a given body.
+Matches a 'for', 'while', 'do while' statement or a function
+definition that has a given body.
 
 Given
   for (;;) {}
@@ -4006,8 +4006,8 @@
 
 
 MatcherForStmt>hasBodyMatcherStmt> InnerMatcher
-Matches a 'for', 'while', or 'do while' statement that has
-a given body.
+Matches a 'for', 'while', 'do while' statement or a function
+definition that has a given body.
 
 Given
   for (;;) {}
@@ -4061,6 +4061,19 @@
 
 
 
+MatcherFunctionDecl>hasBodyMatcher

Re: [PATCH] D8149: Add hasUnderlyingType narrowing matcher for TypedefDecls, functionProtoType matcher for FunctionProtoType nodes, extend parameterCountIs to FunctionProtoType nodes

2016-01-19 Thread Samuel Benzaquen via cfe-commits

sbenza added inline comments.


Comment at: unittests/ASTMatchers/ASTMatchersTest.cpp:4994
@@ +4993,3 @@
+  EXPECT_TRUE(matches("typedef int hasUnderlyingTypeTest;",
+  typedefDecl(hasUnderlyingType(asString("int");
+  EXPECT_TRUE(matches("typedef const int T;",

LegalizeAdulthood wrote:
> sbenza wrote:
> > aaron.ballman wrote:
> > > LegalizeAdulthood wrote:
> > > > aaron.ballman wrote:
> > > > > LegalizeAdulthood wrote:
> > > > > > aaron.ballman wrote:
> > > > > > > Thank you for those examples! Given the following code:
> > > > > > > ```
> > > > > > > typedef int foo;
> > > > > > > typedef foo bar;
> > > > > > > 
> > > > > > > bar i;
> > > > > > > ```
> > > > > > > clang-query> match varDecl(hasType(asString("int")))
> > > > > > > 0 matches.
> > > > > > > clang-query> match varDecl(hasType(asString("foo")))
> > > > > > > 0 matches.
> > > > > > > clang-query> match varDecl(hasType(asString("bar")))
> > > > > > > 
> > > > > > > Match #1:
> > > > > > > 
> > > > > > > E:\Desktop\t.cpp:4:1: note: "root" binds here
> > > > > > > bar i;
> > > > > > > ^
> > > > > > > 1 match.
> > > > > > > 
> > > > > > > So hasType() looks at what the immediate type is for the 
> > > > > > > declaration (which we document, yay us!). Based on that, I don't 
> > > > > > > think hasUnderlyingType() makes sense -- you should modify 
> > > > > > > hasType() to work on a TypedefNameDecl (not just a TypedefDecl!) 
> > > > > > > so that it looks at the immediate type of the type definition. I 
> > > > > > > would expect your tests then to result in:
> > > > > > > ```
> > > > > > > 1: typedef void (fn)(void);
> > > > > > > 2: typedef fn foo;
> > > > > > > 3: typedef int bar;
> > > > > > > 4: typedef int (f);
> > > > > > > 5: typedef int (fn2)(int);
> > > > > > > clang-query> match typedefDecl(hasType(asString("int")))
> > > > > > > 
> > > > > > > Match #1:
> > > > > > > 
> > > > > > > /tmp/a.cpp:3:1: note: "root" binds here
> > > > > > > typedef int bar;
> > > > > > > ^~~
> > > > > > > 
> > > > > > > Match #2:
> > > > > > > 
> > > > > > > /tmp/a.cpp:4:1: note: "root" binds here
> > > > > > > typedef int (f);
> > > > > > > ^~~
> > > > > > > 2 matches.
> > > > > > > clang-query> match typedefDecl(hasType(typedefType()))
> > > > > > > 
> > > > > > > Match #1:
> > > > > > > 
> > > > > > > /tmp/a.cpp:2:1: note: "root" binds here
> > > > > > > typedef fn foo;
> > > > > > > ^~
> > > > > > > 1 match.
> > > > > > > clang-query> match typedefDecl(hasType(parenType()))
> > > > > > > 
> > > > > > > Match #1:
> > > > > > > 
> > > > > > > /tmp/a.cpp:1:1: note: "root" binds here
> > > > > > > typedef void (fn)(void);
> > > > > > > ^~~
> > > > > > > 
> > > > > > > Match #2:
> > > > > > > 
> > > > > > > /tmp/a.cpp:4:1: note: "root" binds here
> > > > > > > typedef int (f);
> > > > > > > ^~~
> > > > > > > 
> > > > > > > Match #3:
> > > > > > > 
> > > > > > > /tmp/a.cpp:5:1: note: "root" binds here
> > > > > > > typedef int (fn2)(int);
> > > > > > > ^~
> > > > > > > 3 matches.
> > > > > > > clang-query> match 
> > > > > > > typedefDecl(hasType(parenType(innerType(functionType()
> > > > > > > 
> > > > > > > Match #1:
> > > > > > > 
> > > > > > > /tmp/a.cpp:1:1: note: "root" binds here
> > > > > > > typedef void (fn)(void);
> > > > > > > ^~~
> > > > > > > 
> > > > > > > Match #2:
> > > > > > > 
> > > > > > > /tmp/a.cpp:5:1: note: "root" binds here
> > > > > > > typedef int (fn2)(int);
> > > > > > > ^~
> > > > > > > 2 matches.
> > > > > > > ```
> > > > > > > The end results are the same, so this is just changing the way 
> > > > > > > the information is surfaced to the user that is logically 
> > > > > > > consistent. ValueDecls have an immediate type, and so do 
> > > > > > > TypedefDecls. By using TypedefNameDecl instead of TypedefDecl, 
> > > > > > > this also covers the case where hasType() is useful for an 
> > > > > > > alias-declaration. (We don't expose the matcher for that yet, but 
> > > > > > > it seems quite reasonable to add in the future, and it would be 
> > > > > > > nice for hasType to automatically work with that.)
> > > > > > > 
> > > > > > > You can implement this with a helper function to handle 
> > > > > > > abstracting away the call to getType() vs getUnderlyingType(), 
> > > > > > > then updating the hasType() matchers to use it. Something like:
> > > > > > > ```
> > > > > > > template 
> > > > > > > struct UnderlyingTypeGetter {
> > > > > > >   static QualType get(const Ty &Node) {
> > > > > > > return Node.getType();
> > > > > > >   }
> > > > > > > };
> > > > > > > 
> > > > > > > template <>
> > > > > > > QualType UnderlyingTypeGetter::get(const 
> > > > > > > TypedefNameDecl &Node) {
> > > > > > >   return Node.getUnderlyingType();
> > > > > > > }
> > > > > > > ```
> > > > > > > (Somewhere in ASTMatchersInternal.h most likely.)
> > > > > >

Re: [PATCH] D16301: Allow mode attribute for member variables again

2016-01-19 Thread Stephan Bergmann via cfe-commits

sberg updated this revision to Diff 45268.
sberg added a comment.

updated the diagnostic message to mention fields in addition to variables and 
typedefs


http://reviews.llvm.org/D16301

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/AttributeList.h
  lib/Sema/SemaDeclAttr.cpp
  test/Sema/attr-mode.c

Index: test/Sema/attr-mode.c
===
--- test/Sema/attr-mode.c
+++ test/Sema/attr-mode.c
@@ -24,8 +24,8 @@
 
 int **__attribute((mode(QI)))* i32;  // expected-error{{mode attribute}}
 
-__attribute__((mode(QI))) int invalid_func() { return 1; } // 
expected-error{{'mode' attribute only applies to variables and typedefs}}
-enum invalid_enum { A1 __attribute__((mode(QI))) }; // expected-error{{'mode' 
attribute only applies to variables and typedefs}}
+__attribute__((mode(QI))) int invalid_func() { return 1; } // 
expected-error{{'mode' attribute only applies to variables, fields and 
typedefs}}
+enum invalid_enum { A1 __attribute__((mode(QI))) }; // expected-error{{'mode' 
attribute only applies to variables, fields and typedefs}}
 
 typedef _Complex double c32 __attribute((mode(SC)));
 int c32_test[sizeof(c32) == 8 ? 1 : -1];
@@ -76,3 +76,7 @@
 #else
 #error Unknown test architecture.
 #endif
+
+struct S {
+  int n __attribute((mode(HI)));
+};
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -3392,7 +3392,7 @@
   if (TypedefNameDecl *TD = dyn_cast(D))
 OldTy = TD->getUnderlyingType();
   else
-OldTy = cast(D)->getType();
+OldTy = cast(D)->getType();
 
   // Base type can also be a vector type (see PR17453).
   // Distinguish between base type and base element type.
@@ -3465,7 +3465,7 @@
   if (TypedefNameDecl *TD = dyn_cast(D))
 TD->setModedTypeSourceInfo(TD->getTypeSourceInfo(), NewTy);
   else
-cast(D)->setType(NewTy);
+cast(D)->setType(NewTy);
 
   D->addAttr(::new (S.Context)
  ModeAttr(Attr.getRange(), S.Context, Name,
Index: include/clang/Sema/AttributeList.h
===
--- include/clang/Sema/AttributeList.h
+++ include/clang/Sema/AttributeList.h
@@ -856,7 +856,8 @@
   ExpectedObjectiveCInterfaceOrProtocol,
   ExpectedKernelFunction,
   ExpectedFunctionWithProtoType,
-  ExpectedStructClassVariableFunctionMethodOrInlineNamespace
+  ExpectedStructClassVariableFunctionMethodOrInlineNamespace,
+  ExpectedVariableFieldOrTypedef
 };
 
 }  // end namespace clang
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -2442,7 +2442,8 @@
   "variables, functions and classes|Objective-C protocols|"
   "functions and global variables|structs, unions, and typedefs|structs and 
typedefs|"
   "interface or protocol declarations|kernel functions|non-K&R-style 
functions|"
-  "structs, classes, variables, functions, methods and inline namespaces}1">,
+  "structs, classes, variables, functions, methods and inline namespaces|"
+  "variables, fields and typedefs}1">,
   InGroup;
 def err_attribute_wrong_decl_type : Error;
 def warn_type_attribute_wrong_type : Warning<
Index: include/clang/Basic/Attr.td
===
--- include/clang/Basic/Attr.td
+++ include/clang/Basic/Attr.td
@@ -887,8 +887,8 @@
 
 def Mode : Attr {
   let Spellings = [GCC<"mode">];
-  let Subjects = SubjectList<[Var, TypedefName], ErrorDiag,
- "ExpectedVariableOrTypedef">;
+  let Subjects = SubjectList<[Var, TypedefName, Field], ErrorDiag,
+ "ExpectedVariableFieldOrTypedef">;
   let Args = [IdentifierArgument<"Mode">];
   let Documentation = [Undocumented];
 }


Index: test/Sema/attr-mode.c
===
--- test/Sema/attr-mode.c
+++ test/Sema/attr-mode.c
@@ -24,8 +24,8 @@
 
 int **__attribute((mode(QI)))* i32;  // expected-error{{mode attribute}}
 
-__attribute__((mode(QI))) int invalid_func() { return 1; } // expected-error{{'mode' attribute only applies to variables and typedefs}}
-enum invalid_enum { A1 __attribute__((mode(QI))) }; // expected-error{{'mode' attribute only applies to variables and typedefs}}
+__attribute__((mode(QI))) int invalid_func() { return 1; } // expected-error{{'mode' attribute only applies to variables, fields and typedefs}}
+enum invalid_enum { A1 __attribute__((mode(QI))) }; // expected-error{{'mode' attribute only applies to variables, fields and typedefs}}
 
 typedef _Complex double c32 __attribute((mode(SC)));
 int c32_test[sizeof(c32) == 8 ? 1 : -1];
@@ -76,3 +76,7 @@
 #else
 #error Unknown test architecture.
 #endif
+
+struct S {
+  int n __attribute((mode(HI)));
+};
Index: lib/S

Re: [PATCH] D16317: [Analyzer] Fix for PR23790: bind real value returned from strcmp when modelling strcmp.

2016-01-19 Thread Devin Coughlin via cfe-commits

dcoughlin added a subscriber: dcoughlin.
dcoughlin added a comment.

As Artem notes, you can't defer to the host strcmp() -- doing so is just as 
unsound as using StringRef::compare() less predictable under optimization of 
the analyzer. I think his suggested approach is the way to go: create a symbol 
and constrain its range based on the result of StringRef::compare.

For tests, I would suggest:

  int lessThanZero = strcmp("aaa", "nnn");
  clang_analyzer_eval(lessThanZero < 0); // expected-warning {{TRUE}}
  clang_analyzer_eval(lessThanZero >= 0); // expected-warning {{FALSE}}
  clang_analyzer_eval(lessThanZero < -13); // expected-warning {{UNKNOWN}}
  
  int greaterThanZero = strcmp("nnn", "aaa");
  clang_analyzer_eval(greaterThanZero > 0); // expected-warning {{TRUE}}
  clang_analyzer_eval(greaterThanZero <= 0); // expected-warning {{FALSE}}
  clang_analyzer_eval(greaterThanZero > 13); // expected-warning {{UNKNOWN}}
  
  int equalToZero = strcmp("aaa", "aaa");
  clang_analyzer_eval(equalToZero == 0); // expected-warning {{TRUE}}

These show that the analyzer does assume the strongest sound postcondition and 
spot checks that it doesn't assume either the stronger, 
StringRef-implementation-specific invariant (1/-1) or an invariant from a 
common unoptimized memcpy() implementation ('a' - 'n' is 13).



Comment at: lib/StaticAnalyzer/Checkers/CStringChecker.cpp:1873
@@ -1872,3 +1872,3 @@
 // Compare string 1 to string 2 the same way strcasecmp() does.
 result = s1StrRef.compare_lower(s2StrRef);
   } else {

Whatever changes you make for the case-sensitive compare should also be 
analogously applied to the case-insensitive compare.


http://reviews.llvm.org/D16317



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

r258140 - Activate OpenMP private clause for target construct and a regression test.

2016-01-19 Thread Carlo Bertolli via cfe-commits

Author: cbertol
Date: Tue Jan 19 10:53:55 2016
New Revision: 258140

URL: http://llvm.org/viewvc/llvm-project?rev=258140&view=rev
Log:
Activate OpenMP private clause for target construct and a regression test.

Added:
cfe/trunk/test/OpenMP/target_private_messages.cpp
Modified:
cfe/trunk/include/clang/Basic/OpenMPKinds.def
cfe/trunk/lib/Sema/SemaOpenMP.cpp

Modified: cfe/trunk/include/clang/Basic/OpenMPKinds.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/OpenMPKinds.def?rev=258140&r1=258139&r2=258140&view=diff
==
--- cfe/trunk/include/clang/Basic/OpenMPKinds.def (original)
+++ cfe/trunk/include/clang/Basic/OpenMPKinds.def Tue Jan 19 10:53:55 2016
@@ -346,6 +346,7 @@ OPENMP_ATOMIC_CLAUSE(seq_cst)
 OPENMP_TARGET_CLAUSE(if)
 OPENMP_TARGET_CLAUSE(device)
 OPENMP_TARGET_CLAUSE(map)
+OPENMP_TARGET_CLAUSE(private)
 
 // Clauses allowed for OpenMP directive 'target data'.
 // TODO More clauses for 'target data' directive.

Modified: cfe/trunk/lib/Sema/SemaOpenMP.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOpenMP.cpp?rev=258140&r1=258139&r2=258140&view=diff
==
--- cfe/trunk/lib/Sema/SemaOpenMP.cpp (original)
+++ cfe/trunk/lib/Sema/SemaOpenMP.cpp Tue Jan 19 10:53:55 2016
@@ -6475,7 +6475,7 @@ OMPClause *Sema::ActOnOpenMPPrivateClaus
 }
 
 SourceLocation ELoc = RefExpr->getExprLoc();
-// OpenMP [2.1, C/C++]
+// OpenMP [3.1, C/C++]
 //  A list item is a variable name.
 // OpenMP  [2.9.3.3, Restrictions, p.1]
 //  A variable that is part of another variable (as an array or

Added: cfe/trunk/test/OpenMP/target_private_messages.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/target_private_messages.cpp?rev=258140&view=auto
==
--- cfe/trunk/test/OpenMP/target_private_messages.cpp (added)
+++ cfe/trunk/test/OpenMP/target_private_messages.cpp Tue Jan 19 10:53:55 2016
@@ -0,0 +1,121 @@
+// RUN: %clang_cc1 -verify -fopenmp %s
+
+struct S1; // expected-note 2 {{declared here}} expected-note 2 {{forward 
declaration of 'S1'}}
+extern S1 a;
+class S2 {
+  mutable int a;
+
+public:
+  S2() : a(0) {}
+};
+const S2 b;
+const S2 ba[5];
+class S3 {
+  int a;
+
+public:
+  S3() : a(0) {}
+};
+const S3 ca[5];
+class S4 {
+  int a;
+  S4(); // expected-note {{implicitly declared private here}}
+
+public:
+  S4(int v) : a(v) {}
+};
+class S5 {
+  int a;
+  S5() : a(0) {} // expected-note {{implicitly declared private here}}
+
+public:
+  S5(int v) : a(v) {}
+};
+
+S3 h;
+#pragma omp threadprivate(h) // expected-note 2 {{defined as threadprivate or 
thread local}}
+
+template 
+int foomain(I argc, C **argv) {
+  I e(4);
+  I g(5);
+  int i;
+  int &j = i;
+#pragma omp target private // expected-error {{expected '(' after 'private'}}
+#pragma omp target private( // expected-error {{expected expression}} 
expected-error {{expected ')'}} expected-note {{to match this '('}}
+#pragma omp target private() // expected-error {{expected expression}}
+#pragma omp target private(argc // expected-error {{expected ')'}} 
expected-note {{to match this '('}}
+#pragma omp target private(argc, // expected-error {{expected expression}} 
expected-error {{expected ')'}} expected-note {{to match this '('}}
+#pragma omp target private(argc > 0 ? argv[1] : argv[2]) // expected-error 
{{expected variable name}}
+#pragma omp target private(argc)
+#pragma omp target private(S1) // expected-error {{'S1' does not refer to a 
value}}
+#pragma omp target private(a, b) // expected-error {{private variable with 
incomplete type 'S1'}}
+#pragma omp target private(argv[1]) // expected-error {{expected variable 
name}}
+#pragma omp target private(e, g)
+#pragma omp target private(h) // expected-error {{threadprivate or thread 
local variable cannot be private}}
+#pragma omp target shared(i) // expected-error {{unexpected OpenMP clause 
'shared' in directive '#pragma omp target'}}
+#pragma omp parallel
+  {
+int v = 0;
+int i;
+#pragma omp target private(i)
+{}
+  }
+#pragma omp parallel shared(i)
+#pragma omp parallel private(i)
+#pragma omp target private(j)
+#pragma omp target private(i)
+  {}
+  return 0;
+}
+
+void bar(S4 a[2]) {
+#pragma omp parallel
+#pragma omp target private(a)
+  {}
+}
+
+namespace A {
+double x;
+#pragma omp threadprivate(x) // expected-note {{defined as threadprivate or 
thread local}}
+}
+namespace B {
+using A::x;
+}
+
+int main(int argc, char **argv) {
+  S4 e(4);
+  S5 g(5);
+  int i;
+  int &j = i;
+#pragma omp target private // expected-error {{expected '(' after 'private'}}
+#pragma omp target private( // expected-error {{expected expression}} 
expected-error {{expected ')'}} expected-note {{to match this '('}}
+#pragma omp target private() // expected-error {{expected expression}}
+#pragma o

Re: [PATCH] D16317: [Analyzer] Fix for PR23790: bind real value returned from strcmp when modelling strcmp.

2016-01-19 Thread Devin Coughlin via cfe-commits

dcoughlin added a comment.

ayartsev:

> This also may theoretically help to find defects if a tested code relays on a 
> value returned from strcmp like

> if (strcmp(x, y) == 1) { ... }


I think it would be useful and not that difficult to write a checker that 
checks for this explicitly. I don't think an AST-based/clang-tidycheck would 
quite be enough because I suspect programmers often store the result of 
strcmp() in a local before using it. One approach would be to have a 
path-sensitive checker keep a set of symbolic result values from strcmp() and 
diagnose if any is ever explicitly checked for (dis)-equality with -1 or 1.


http://reviews.llvm.org/D16317



___
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-19 Thread Richard via cfe-commits

LegalizeAdulthood added inline comments.


Comment at: clang-tidy/readability/RedundantReturnCheck.cpp:24
@@ +23,3 @@
+  functionDecl(isDefinition(), returns(asString("void")),
+   has(compoundStmt(hasAnySubstatement(returnStmt()
+  .bind("fn"),

aaron.ballman wrote:
> Would be best to restrict this to a return statement that has no expression 
> if we don't want to diagnose this:
> ```
> void g();
> 
> void f() {
>   return g();
> }
> ```
> Either way, it would be good to have a test that ensures this isn't mangled.
How about transforming this odd looking duck into

```
void g();

void f() {
  g();
}
```

?


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] D16308: clang-tidy Enhance readability-simplify-boolean-expr check to handle implicit conversions of integral types to bool and member pointers

2016-01-19 Thread Richard via cfe-commits

LegalizeAdulthood added inline comments.


Comment at: clang-tidy/readability/SimplifyBooleanExprCheck.h:50
@@ -49,3 +49,3 @@
 ///  opposite condition.
 ///   4. Implicit conversions of pointer to `bool` are replaced with explicit
 ///  comparisons to `nullptr`.

aaron.ballman wrote:
> Can you also update for member pointers?
I can explicitly state that; to me "pointers" is a term that includes member 
pointers.


Comment at: clang-tidy/readability/SimplifyBooleanExprCheck.h:77
@@ -74,3 +76,3 @@
 ///  implicit conversion of `i & 1` to `bool` and becomes
-///  `bool b = static_cast(i & 1);`.
+///  `bool b = i & 1 != 0;`.
 ///

aaron.ballman wrote:
> To me, this does not improve readability -- I now have to think much harder 
> about operator precedence. Including parens would help significantly, IMO, so 
> that it becomes `(i & 1) != 0`.
To clarify: You'd like to see parens around the expression when it is a binary 
operator, correct?

When it is a variable, there's no need to add parentheses.


http://reviews.llvm.org/D16308



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

r258143 - [CMake] Properly respect the CLANG_APPEND_VC_REV option

2016-01-19 Thread Chris Bieneman via cfe-commits

Author: cbieneman
Date: Tue Jan 19 11:06:12 2016
New Revision: 258143

URL: http://llvm.org/viewvc/llvm-project?rev=258143&view=rev
Log:
[CMake] Properly respect the CLANG_APPEND_VC_REV option

Only set -DSVN_REVISION if CLANG_APPEND_VC_REV=On

Modified:
cfe/trunk/CMakeLists.txt

Modified: cfe/trunk/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/CMakeLists.txt?rev=258143&r1=258142&r2=258143&view=diff
==
--- cfe/trunk/CMakeLists.txt (original)
+++ cfe/trunk/CMakeLists.txt Tue Jan 19 11:06:12 2016
@@ -216,14 +216,15 @@ endif()
 
 option(CLANG_APPEND_VC_REV
   "Append the version control system revision id to clang version spew" OFF)
+if(CLANG_APPEND_VC_REV)
+  if(NOT SVN_REVISION)
+# This macro will set SVN_REVISION in the parent scope
+add_version_info_from_vcs(VERSION_VAR)
+  endif()
 
-if(NOT SVN_REVISION)
-  # This macro will set SVN_REVISION in the parent scope
-  add_version_info_from_vcs(VERSION_VAR)
-endif()
-
-if(SVN_REVISION)
-  add_definitions(-DSVN_REVISION="${SVN_REVISION}")
+  if(SVN_REVISION)
+add_definitions(-DSVN_REVISION="${SVN_REVISION}")
+  endif()
 endif()
 
 set(CLANG_VENDOR_UTI "org.llvm.clang" CACHE STRING


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

Re: r257827 - [CMake] Set SVN_REVISION if CLANG_APPEND_VC_REV=On

2016-01-19 Thread Chris Bieneman via cfe-commits

This was my bad. The two ifs were meant to be wrapped in 
if(CLANG_APPEND_VC_REV), I’ve added that in r258143.

-Chris

> On Jan 18, 2016, at 11:40 AM, Craig Topper  wrote:
> 
> CLANG_APPEND_VC_REV doesn't appear to be checked anywhere. Were the two ifs 
> supposed to check it instead of SVN_VERSION? The way it is now if the first 
> if body executes, the second if does too since add_version_info_from_vcs sets 
> SVN_VERSION and thus satisfies the second if.
> 
> On Mon, Jan 18, 2016 at 6:06 AM, Daniel Sanders via cfe-commits 
> mailto:cfe-commits@lists.llvm.org>> wrote:
> Thanks, that did the trick. I've removed the workaround from LLVMLinux.
> 
> > -Original Message-
> > From: cbiene...@apple.com  
> > [mailto:cbiene...@apple.com ] On Behalf Of
> > Chris Bieneman
> > Sent: 15 January 2016 17:55
> > To: Daniel Sanders
> > Cc: cfe-commits@lists.llvm.org 
> > Subject: Re: r257827 - [CMake] Set SVN_REVISION if
> > CLANG_APPEND_VC_REV=On
> >
> > Thanks for the heads up. It looks like that module is was excluded from the
> > LLVM install. I’ve changed that in LLVM r257909. That change should resolve
> > your build issue. Please let me know if you continue having problems.
> >
> > Thanks,
> > -Chris
> >
> > > On Jan 15, 2016, at 5:18 AM, Daniel Sanders  > > >
> > wrote:
> > >
> > > Hi Chris,
> > >
> > > This doesn't seem to work when building clang separately from llvm.
> > LLVMLinux fails to build clang with:
> > > CMake Error at CMakeLists.txt:104 (include):
> > >   include could not find load file:
> > >
> > > VersionFromVCS
> > >
> > > CMake Error at CMakeLists.txt:222 (add_version_info_from_vcs):
> > >   Unknown CMake command "add_version_info_from_vcs".
> > > See
> > http://buildbot.llvm.linuxfoundation.org/builders/13_malta/builds/383/step 
> > 
> > s/shell_3/logs/stdio for the full log.
> > >
> > > I've added a patch to llvmlinux to work around the problem for now
> > http://git.linuxfoundation.org/?p=llvmlinux.git;a=blob;f=toolchain/clang/pat
> >  
> > 
> > ches/clang/workaround-
> > versionfromvcsbug.patch;h=848a096df37b1255575650680a266234f5d4936e;h
> > b=e0c4c72c5a008006dc230db748ea69e0d1518daf.
> > > Should we make that change to clang or fix it another way?
> > >
> > >> -Original Message-
> > >> From: cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org 
> > >> ] On
> > Behalf
> > >> Of Chris Bieneman via cfe-commits
> > >> Sent: 14 January 2016 22:45
> > >> To: cfe-commits@lists.llvm.org 
> > >> Subject: r257827 - [CMake] Set SVN_REVISION if
> > >> CLANG_APPEND_VC_REV=On
> > >>
> > >> Author: cbieneman
> > >> Date: Thu Jan 14 16:45:12 2016
> > >> New Revision: 257827
> > >>
> > >> URL: http://llvm.org/viewvc/llvm-project?rev=257827&view=rev 
> > >> 
> > >> Log:
> > >> [CMake] Set SVN_REVISION if CLANG_APPEND_VC_REV=On
> > >>
> > >> This matches autoconf's ability to put clang revisions in the clang 
> > >> --version
> > >> spew.
> > >>
> > >> Modified:
> > >>cfe/trunk/CMakeLists.txt
> > >>
> > >> Modified: cfe/trunk/CMakeLists.txt
> > >> URL: http://llvm.org/viewvc/llvm- 
> > >>
> > project/cfe/trunk/CMakeLists.txt?rev=257827&r1=257826&r2=257827&view
> > >> =diff
> > >>
> > ==
> > >> 
> > >> --- cfe/trunk/CMakeLists.txt (original)
> > >> +++ cfe/trunk/CMakeLists.txt Thu Jan 14 16:45:12 2016
> > >> @@ -101,6 +101,7 @@ if( CMAKE_SOURCE_DIR STREQUAL CMAKE_CURR
> > >>   include(AddLLVM)
> > >>   include(TableGen)
> > >>   include(HandleLLVMOptions)
> > >> +  include(VersionFromVCS)
> > >>
> > >>   set(PACKAGE_VERSION "${LLVM_PACKAGE_VERSION}")
> > >>
> > >> @@ -213,6 +214,18 @@ if(CLANG_REPOSITORY_STRING)
> > >>   add_definitions(-
> > >> DCLANG_REPOSITORY_STRING="${CLANG_REPOSITORY_STRING}")
> > >> endif()
> > >>
> > >> +option(CLANG_APPEND_VC_REV
> > >> +  "Append the version control system revision id to clang version spew"
> > >> OFF)
> > >> +
> > >> +if(NOT SVN_REVISION)
> > >> +  # This macro will set SVN_REVISION in the parent scope
> > >> +  add_version_info_from_vcs(VERSION_VAR)
> > >> +endif()
> > >> +
> > >> +if(SVN_REVISION)
> > >> +  add_definitions(-DSVN_REVISION="${SVN_REVISION}")
> > >> +endif()
> > >> +
> > >> set(CLANG_VENDOR_UTI "org.llvm.clang" CACHE STRING
> > >>   "Vendor-specific uti.")
> > >>
> > >>
> > >>
> > >> ___
> > >> cfe-commits mailing list
> > >> cfe-commits@lists.llvm.org 
> > >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-comm

Re: [libcxx] r258107 - Fix PR#26175. Thanks to Josh Petrie for the report and the patch. Reviewed as http://reviews.llvm.org/D16262

2016-01-19 Thread Hans Wennborg via cfe-commits

On Tue, Jan 19, 2016 at 12:01 AM, Dimitry Andric  wrote:
> On 19 Jan 2016, at 01:50, Marshall Clow via cfe-commits 
>  wrote:
>>
>> Author: marshall
>> Date: Mon Jan 18 18:50:37 2016
>> New Revision: 258107
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=258107&view=rev
>> Log:
>> Fix PR#26175. Thanks to Josh Petrie for the report and the patch. Reviewed 
>> as http://reviews.llvm.org/D16262
>
> This looks like a good candidate for the 3.8 branch, do you agree?

Sounds good to me if Marshall agrees.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [libcxxabi] r256323 - Add new tests for throwing incomplete pointer types

2016-01-19 Thread Hans Wennborg via cfe-commits

Nico pointed out his revert should probably be merged to 3.8.

Has there been any follow-up here, e.g. fixes, that I should be aware of?

Thanks,
Hans

On Fri, Jan 15, 2016 at 7:48 AM, Nico Weber via cfe-commits
 wrote:
> I reverted this and 322 for now in 257896.
>
> On Fri, Jan 15, 2016 at 10:36 AM, Nico Weber  wrote:
>>
>> This test fails on OS X for me:
>>
>>
>> Testing: 0 .. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90..
>> FAIL: libc++abi :: incomplete_type.sh.cpp (29529 of 29545)
>>  TEST 'libc++abi :: incomplete_type.sh.cpp' FAILED
>> 
>> Script:
>> --
>>
>> /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/c++
>> -v -DLIBCXXABI_NO_TIMER -funwind-tables -std=c++1z -nostdinc++
>> -I/Users/thakis/src/chrome/src/third_party/llvm/projects/libcxx/include
>> -I/Users/thakis/src/chrome/src/third_party/llvm/projects/libcxxabi/include
>> -isysroot
>> /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.11.sdk
>> -c
>> /Users/thakis/src/chrome/src/third_party/llvm/projects/libcxxabi/test/incomplete_type.sh.cpp
>> -o
>> /Users/thakis/src/chrome/src/third_party/llvm-bootstrap/projects/libcxxabi/test/Output/incomplete_type.sh.cpp.tmp.one.o
>>
>> /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/c++
>> -v -DLIBCXXABI_NO_TIMER -funwind-tables -std=c++1z -nostdinc++
>> -I/Users/thakis/src/chrome/src/third_party/llvm/projects/libcxx/include
>> -I/Users/thakis/src/chrome/src/third_party/llvm/projects/libcxxabi/include
>> -isysroot
>> /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.11.sdk
>> -c
>> /Users/thakis/src/chrome/src/third_party/llvm/projects/libcxxabi/test/incomplete_type.sh.cpp
>> -o
>> /Users/thakis/src/chrome/src/third_party/llvm-bootstrap/projects/libcxxabi/test/Output/incomplete_type.sh.cpp.tmp.two.o
>> -DTU_ONE
>>
>> /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/c++
>> -v -nodefaultlibs
>> -L/Users/thakis/src/chrome/src/third_party/llvm-bootstrap/lib
>> -Wl,-rpath,/Users/thakis/src/chrome/src/third_party/llvm-bootstrap/lib -lc++
>> -lc++abi -lSystem -o
>> /Users/thakis/src/chrome/src/third_party/llvm-bootstrap/projects/libcxxabi/test/Output/incomplete_type.sh.cpp.tmp.exe
>> /Users/thakis/src/chrome/src/third_party/llvm-bootstrap/projects/libcxxabi/test/Output/incomplete_type.sh.cpp.tmp.one.o
>> /Users/thakis/src/chrome/src/third_party/llvm-bootstrap/projects/libcxxabi/test/Output/incomplete_type.sh.cpp.tmp.two.o
>>
>> /Users/thakis/src/chrome/src/third_party/llvm-bootstrap/projects/libcxxabi/test/Output/incomplete_type.sh.cpp.tmp.exe
>> --
>> Exit Code: 134
>>
>> Command Output (stderr):
>> --
>> Apple LLVM version 7.0.0 (clang-700.1.76)
>> Target: x86_64-apple-darwin15.0.0
>> Thread model: posix
>>
>> "/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang"
>> -cc1 -triple x86_64-apple-macosx10.11.0 -Wdeprecated-objc-isa-usage
>> -Werror=deprecated-objc-isa-usage -emit-obj -mrelax-all -disable-free
>> -disable-llvm-verifier -main-file-name incomplete_type.sh.cpp
>> -mrelocation-model pic -pic-level 2 -mthread-model posix -mdisable-fp-elim
>> -masm-verbose -munwind-tables -target-cpu core2 -target-linker-version 253.6
>> -v -dwarf-column-info -coverage-file
>> /Users/thakis/src/chrome/src/third_party/llvm-bootstrap/projects/libcxxabi/test/Output/incomplete_type.sh.cpp.tmp.one.o
>> -nostdinc++ -resource-dir
>> /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../lib/clang/7.0.0
>> -isysroot
>> /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.11.sdk
>> -D LIBCXXABI_NO_TIMER -I
>> /Users/thakis/src/chrome/src/third_party/llvm/projects/libcxx/include -I
>> /Users/thakis/src/chrome/src/third_party/llvm/projects/libcxxabi/include
>> -stdlib=libc++ -std=c++1z -fdeprecated-macro -fdebug-compilation-dir
>> /Users/thakis/src/chrome/src/third_party/llvm-bootstrap/projects/libcxxabi/test
>> -ferror-limit 19 -fmessage-length 0 -stack-protector 1 -mstackrealign
>> -fblocks -fobjc-runtime=macosx-10.11.0 -fencode-extended-block-signature
>> -fcxx-exceptions -fexceptions -fmax-type-align=16 -fdiagnostics-show-option
>> -o
>> /Users/thakis/src/chrome/src/third_party/llvm-bootstrap/projects/libcxxabi/test/Output/incomplete_type.sh.cpp.tmp.one.o
>> -x c++
>> /Users/thakis/src/chrome/src/third_party/llvm/projects/libcxxabi/test/incomplete_type.sh.cpp
>> clang -cc1 version 7.0.0 based upon LLVM 3.7.0svn default target
>> x86_64-apple-darwin15.0.0
>> ignoring nonexistent directory
>> "/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.11.sdk/usr/local/include"
>> ignoring nonexistent directory
>> "/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.11.sdk/Library/Frameworks"
>> #include "..."

Re: [libcxxabi] r256323 - Add new tests for throwing incomplete pointer types

2016-01-19 Thread Eric Fiselier via cfe-commits

Hi Hans,

Can I have today to work on this? If I can't come up with a fix we can
revert tommorow.
I'll ping you tommorow with the result.

Does that work?

/Eric

On Tue, Jan 19, 2016 at 10:30 AM, Hans Wennborg  wrote:

> Nico pointed out his revert should probably be merged to 3.8.
>
> Has there been any follow-up here, e.g. fixes, that I should be aware of?
>
> Thanks,
> Hans
>
> On Fri, Jan 15, 2016 at 7:48 AM, Nico Weber via cfe-commits
>  wrote:
> > I reverted this and 322 for now in 257896.
> >
> > On Fri, Jan 15, 2016 at 10:36 AM, Nico Weber 
> wrote:
> >>
> >> This test fails on OS X for me:
> >>
> >>
> >> Testing: 0 .. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90..
> >> FAIL: libc++abi :: incomplete_type.sh.cpp (29529 of 29545)
> >>  TEST 'libc++abi :: incomplete_type.sh.cpp' FAILED
> >> 
> >> Script:
> >> --
> >>
> >>
> /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/c++
> >> -v -DLIBCXXABI_NO_TIMER -funwind-tables -std=c++1z -nostdinc++
> >> -I/Users/thakis/src/chrome/src/third_party/llvm/projects/libcxx/include
> >>
> -I/Users/thakis/src/chrome/src/third_party/llvm/projects/libcxxabi/include
> >> -isysroot
> >>
> /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.11.sdk
> >> -c
> >>
> /Users/thakis/src/chrome/src/third_party/llvm/projects/libcxxabi/test/incomplete_type.sh.cpp
> >> -o
> >>
> /Users/thakis/src/chrome/src/third_party/llvm-bootstrap/projects/libcxxabi/test/Output/incomplete_type.sh.cpp.tmp.one.o
> >>
> >>
> /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/c++
> >> -v -DLIBCXXABI_NO_TIMER -funwind-tables -std=c++1z -nostdinc++
> >> -I/Users/thakis/src/chrome/src/third_party/llvm/projects/libcxx/include
> >>
> -I/Users/thakis/src/chrome/src/third_party/llvm/projects/libcxxabi/include
> >> -isysroot
> >>
> /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.11.sdk
> >> -c
> >>
> /Users/thakis/src/chrome/src/third_party/llvm/projects/libcxxabi/test/incomplete_type.sh.cpp
> >> -o
> >>
> /Users/thakis/src/chrome/src/third_party/llvm-bootstrap/projects/libcxxabi/test/Output/incomplete_type.sh.cpp.tmp.two.o
> >> -DTU_ONE
> >>
> >>
> /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/c++
> >> -v -nodefaultlibs
> >> -L/Users/thakis/src/chrome/src/third_party/llvm-bootstrap/lib
> >> -Wl,-rpath,/Users/thakis/src/chrome/src/third_party/llvm-bootstrap/lib
> -lc++
> >> -lc++abi -lSystem -o
> >>
> /Users/thakis/src/chrome/src/third_party/llvm-bootstrap/projects/libcxxabi/test/Output/incomplete_type.sh.cpp.tmp.exe
> >>
> /Users/thakis/src/chrome/src/third_party/llvm-bootstrap/projects/libcxxabi/test/Output/incomplete_type.sh.cpp.tmp.one.o
> >>
> /Users/thakis/src/chrome/src/third_party/llvm-bootstrap/projects/libcxxabi/test/Output/incomplete_type.sh.cpp.tmp.two.o
> >>
> >>
> /Users/thakis/src/chrome/src/third_party/llvm-bootstrap/projects/libcxxabi/test/Output/incomplete_type.sh.cpp.tmp.exe
> >> --
> >> Exit Code: 134
> >>
> >> Command Output (stderr):
> >> --
> >> Apple LLVM version 7.0.0 (clang-700.1.76)
> >> Target: x86_64-apple-darwin15.0.0
> >> Thread model: posix
> >>
> >>
> "/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang"
> >> -cc1 -triple x86_64-apple-macosx10.11.0 -Wdeprecated-objc-isa-usage
> >> -Werror=deprecated-objc-isa-usage -emit-obj -mrelax-all -disable-free
> >> -disable-llvm-verifier -main-file-name incomplete_type.sh.cpp
> >> -mrelocation-model pic -pic-level 2 -mthread-model posix
> -mdisable-fp-elim
> >> -masm-verbose -munwind-tables -target-cpu core2 -target-linker-version
> 253.6
> >> -v -dwarf-column-info -coverage-file
> >>
> /Users/thakis/src/chrome/src/third_party/llvm-bootstrap/projects/libcxxabi/test/Output/incomplete_type.sh.cpp.tmp.one.o
> >> -nostdinc++ -resource-dir
> >>
> /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../lib/clang/7.0.0
> >> -isysroot
> >>
> /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.11.sdk
> >> -D LIBCXXABI_NO_TIMER -I
> >> /Users/thakis/src/chrome/src/third_party/llvm/projects/libcxx/include -I
> >> /Users/thakis/src/chrome/src/third_party/llvm/projects/libcxxabi/include
> >> -stdlib=libc++ -std=c++1z -fdeprecated-macro -fdebug-compilation-dir
> >>
> /Users/thakis/src/chrome/src/third_party/llvm-bootstrap/projects/libcxxabi/test
> >> -ferror-limit 19 -fmessage-length 0 -stack-protector 1 -mstackrealign
> >> -fblocks -fobjc-runtime=macosx-10.11.0 -fencode-extended-block-signature
> >> -fcxx-exceptions -fexceptions -fmax-type-align=16
> -fdiagnostics-show-option
> >> -o
> >>
> /Users/thakis/src/chrome/src/third_party/llvm-bootstrap/projects/libcxxabi/test/Output/incomplete_type.sh.cpp.tmp.one.o
> >> -x c++
> >>
> /Users/thakis/src/chrome/src/third_party/llvm/projects/libcxxabi/test/inc

Re: [PATCH] D16158: [CMake] Add clang's targets to LLVM's export set when not building standalone

2016-01-19 Thread Chris Bieneman via cfe-commits

beanz abandoned this revision.
beanz added a comment.

@hintonda, your comment actually trigged something in my brain and I just 
realized the problem. It is a much simpler fix. If you set LLVM_ENABLE_PIC=Off 
libclang isn't built as a dylib, it is a static lib. If 
LLVM_INSTALL_TOOLCHAIN_ONLY=On, we need to exclude all static libraries from 
the Clang exports file.


http://reviews.llvm.org/D16158



___
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-19 Thread Aaron Ballman via cfe-commits

aaron.ballman added inline comments.


Comment at: clang-tidy/readability/RedundantReturnCheck.cpp:24
@@ +23,3 @@
+  functionDecl(isDefinition(), returns(asString("void")),
+   has(compoundStmt(hasAnySubstatement(returnStmt()
+  .bind("fn"),

LegalizeAdulthood wrote:
> aaron.ballman wrote:
> > Would be best to restrict this to a return statement that has no expression 
> > if we don't want to diagnose this:
> > ```
> > void g();
> > 
> > void f() {
> >   return g();
> > }
> > ```
> > Either way, it would be good to have a test that ensures this isn't mangled.
> How about transforming this odd looking duck into
> 
> ```
> void g();
> 
> void f() {
>   g();
> }
> ```
> 
> ?
I think in the context of this check, that would be fine.


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: [libcxxabi] r256323 - Add new tests for throwing incomplete pointer types

2016-01-19 Thread Hans Wennborg via cfe-commits

That sounds great. Thanks!

On Tue, Jan 19, 2016 at 9:33 AM, Eric Fiselier  wrote:
> Hi Hans,
>
> Can I have today to work on this? If I can't come up with a fix we can
> revert tommorow.
> I'll ping you tommorow with the result.
>
> Does that work?
>
> /Eric
>
> On Tue, Jan 19, 2016 at 10:30 AM, Hans Wennborg  wrote:
>>
>> Nico pointed out his revert should probably be merged to 3.8.
>>
>> Has there been any follow-up here, e.g. fixes, that I should be aware of?
>>
>> Thanks,
>> Hans
>>
>> On Fri, Jan 15, 2016 at 7:48 AM, Nico Weber via cfe-commits
>>  wrote:
>> > I reverted this and 322 for now in 257896.
>> >
>> > On Fri, Jan 15, 2016 at 10:36 AM, Nico Weber 
>> > wrote:
>> >>
>> >> This test fails on OS X for me:
>> >>
>> >>
>> >> Testing: 0 .. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90..
>> >> FAIL: libc++abi :: incomplete_type.sh.cpp (29529 of 29545)
>> >>  TEST 'libc++abi :: incomplete_type.sh.cpp' FAILED
>> >> 
>> >> Script:
>> >> --
>> >>
>> >>
>> >> /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/c++
>> >> -v -DLIBCXXABI_NO_TIMER -funwind-tables -std=c++1z -nostdinc++
>> >> -I/Users/thakis/src/chrome/src/third_party/llvm/projects/libcxx/include
>> >>
>> >> -I/Users/thakis/src/chrome/src/third_party/llvm/projects/libcxxabi/include
>> >> -isysroot
>> >>
>> >> /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.11.sdk
>> >> -c
>> >>
>> >> /Users/thakis/src/chrome/src/third_party/llvm/projects/libcxxabi/test/incomplete_type.sh.cpp
>> >> -o
>> >>
>> >> /Users/thakis/src/chrome/src/third_party/llvm-bootstrap/projects/libcxxabi/test/Output/incomplete_type.sh.cpp.tmp.one.o
>> >>
>> >>
>> >> /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/c++
>> >> -v -DLIBCXXABI_NO_TIMER -funwind-tables -std=c++1z -nostdinc++
>> >> -I/Users/thakis/src/chrome/src/third_party/llvm/projects/libcxx/include
>> >>
>> >> -I/Users/thakis/src/chrome/src/third_party/llvm/projects/libcxxabi/include
>> >> -isysroot
>> >>
>> >> /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.11.sdk
>> >> -c
>> >>
>> >> /Users/thakis/src/chrome/src/third_party/llvm/projects/libcxxabi/test/incomplete_type.sh.cpp
>> >> -o
>> >>
>> >> /Users/thakis/src/chrome/src/third_party/llvm-bootstrap/projects/libcxxabi/test/Output/incomplete_type.sh.cpp.tmp.two.o
>> >> -DTU_ONE
>> >>
>> >>
>> >> /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/c++
>> >> -v -nodefaultlibs
>> >> -L/Users/thakis/src/chrome/src/third_party/llvm-bootstrap/lib
>> >> -Wl,-rpath,/Users/thakis/src/chrome/src/third_party/llvm-bootstrap/lib
>> >> -lc++
>> >> -lc++abi -lSystem -o
>> >>
>> >> /Users/thakis/src/chrome/src/third_party/llvm-bootstrap/projects/libcxxabi/test/Output/incomplete_type.sh.cpp.tmp.exe
>> >>
>> >> /Users/thakis/src/chrome/src/third_party/llvm-bootstrap/projects/libcxxabi/test/Output/incomplete_type.sh.cpp.tmp.one.o
>> >>
>> >> /Users/thakis/src/chrome/src/third_party/llvm-bootstrap/projects/libcxxabi/test/Output/incomplete_type.sh.cpp.tmp.two.o
>> >>
>> >>
>> >> /Users/thakis/src/chrome/src/third_party/llvm-bootstrap/projects/libcxxabi/test/Output/incomplete_type.sh.cpp.tmp.exe
>> >> --
>> >> Exit Code: 134
>> >>
>> >> Command Output (stderr):
>> >> --
>> >> Apple LLVM version 7.0.0 (clang-700.1.76)
>> >> Target: x86_64-apple-darwin15.0.0
>> >> Thread model: posix
>> >>
>> >>
>> >> "/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang"
>> >> -cc1 -triple x86_64-apple-macosx10.11.0 -Wdeprecated-objc-isa-usage
>> >> -Werror=deprecated-objc-isa-usage -emit-obj -mrelax-all -disable-free
>> >> -disable-llvm-verifier -main-file-name incomplete_type.sh.cpp
>> >> -mrelocation-model pic -pic-level 2 -mthread-model posix
>> >> -mdisable-fp-elim
>> >> -masm-verbose -munwind-tables -target-cpu core2 -target-linker-version
>> >> 253.6
>> >> -v -dwarf-column-info -coverage-file
>> >>
>> >> /Users/thakis/src/chrome/src/third_party/llvm-bootstrap/projects/libcxxabi/test/Output/incomplete_type.sh.cpp.tmp.one.o
>> >> -nostdinc++ -resource-dir
>> >>
>> >> /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../lib/clang/7.0.0
>> >> -isysroot
>> >>
>> >> /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.11.sdk
>> >> -D LIBCXXABI_NO_TIMER -I
>> >> /Users/thakis/src/chrome/src/third_party/llvm/projects/libcxx/include
>> >> -I
>> >>
>> >> /Users/thakis/src/chrome/src/third_party/llvm/projects/libcxxabi/include
>> >> -stdlib=libc++ -std=c++1z -fdeprecated-macro -fdebug-compilation-dir
>> >>
>> >> /Users/thakis/src/chrome/src/third_party/llvm-bootstrap/projects/libcxxabi/test
>> >> -ferror-limit 19 -fmessage-length 0 -stack-protector 1 -mstackrealign
>> >> -fblocks -fobjc-runtime=macosx-10.11.0
>> >> -fencode-extended-block-signature
>> >> -fcx

Re: [PATCH] D16308: clang-tidy Enhance readability-simplify-boolean-expr check to handle implicit conversions of integral types to bool and member pointers

2016-01-19 Thread Aaron Ballman via cfe-commits

aaron.ballman added inline comments.


Comment at: clang-tidy/readability/SimplifyBooleanExprCheck.h:50
@@ -49,3 +49,3 @@
 ///  opposite condition.
 ///   4. Implicit conversions of pointer to `bool` are replaced with explicit
 ///  comparisons to `nullptr`.

LegalizeAdulthood wrote:
> aaron.ballman wrote:
> > Can you also update for member pointers?
> I can explicitly state that; to me "pointers" is a term that includes member 
> pointers.
Hmm, perhaps I am being too pedantic. I'm used to thinking of member pointers 
as different from pointers because it's that way in standardese.


Comment at: clang-tidy/readability/SimplifyBooleanExprCheck.h:77
@@ -74,3 +76,3 @@
 ///  implicit conversion of `i & 1` to `bool` and becomes
-///  `bool b = static_cast(i & 1);`.
+///  `bool b = i & 1 != 0;`.
 ///

LegalizeAdulthood wrote:
> aaron.ballman wrote:
> > To me, this does not improve readability -- I now have to think much harder 
> > about operator precedence. Including parens would help significantly, IMO, 
> > so that it becomes `(i & 1) != 0`.
> To clarify: You'd like to see parens around the expression when it is a 
> binary operator, correct?
> 
> When it is a variable, there's no need to add parentheses.
Correct; if it's just a DeclRefExpr or unary operator then parens aren't 
useful; but if it's a binary operator, I think parens may help clarify.


http://reviews.llvm.org/D16308



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

LLVM buildmaster will be restarted tonight

2016-01-19 Thread Galina Kistanova via cfe-commits

Hello everyone,

LLVM buildmaster will be updated and restarted after 7 PM Pacific time
today.

Thanks

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

Re: [PATCH] D16301: Allow mode attribute for member variables again

2016-01-19 Thread Aaron Ballman via cfe-commits

aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM, thank you for fixing this!


http://reviews.llvm.org/D16301



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

Re: [PATCH] D16215: ASTMatchers: enable hasBody() matcher for FunctionDecls

2016-01-19 Thread Aaron Ballman via cfe-commits

aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

With one small nit, LGTM, thanks!



Comment at: include/clang/ASTMatchers/ASTMatchersInternal.h:1596
@@ +1595,3 @@
+inline const Stmt *GetBodyMatcher::get(const FunctionDecl &Node) 
{
+  return Node.doesThisDeclarationHaveABody() ? Node.getBody() : NULL;
+}

s/NULL/nullptr


http://reviews.llvm.org/D16215



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

r258152 - Module Debugging: Defer the emission of anonymous tag decls

2016-01-19 Thread Adrian Prantl via cfe-commits

Author: adrian
Date: Tue Jan 19 12:02:47 2016
New Revision: 258152

URL: http://llvm.org/viewvc/llvm-project?rev=258152&view=rev
Log:
Module Debugging: Defer the emission of anonymous tag decls
until we are visiting their declcontext.

This fixes a regression introduced in r256962:
When building debug info for a typdef'd anonymous tag type, we would be
visiting the inner anonymous type first thus creating a "typedef changes
linkage of anonymous type, but linkage was already computed" error.

rdar://problem/24199640

Modified:
cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp
cfe/trunk/test/Modules/ExtDebugInfo.cpp
cfe/trunk/test/Modules/Inputs/DebugCXX.h
cfe/trunk/test/Modules/ModuleDebugInfo.cpp

Modified: cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp?rev=258152&r1=258151&r2=258152&view=diff
==
--- cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp (original)
+++ cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp Tue Jan 19 
12:02:47 2016
@@ -190,6 +190,10 @@ public:
 if (D->isFromASTFile())
   return;
 
+// Anonymous tag decls are deferred until we are building their 
declcontext.
+if (D->getName().empty())
+  return;
+
 DebugTypeVisitor DTV(*Builder->getModuleDebugInfo(), *Ctx, false);
 DTV.TraverseDecl(D);
 Builder->UpdateCompletedType(D);

Modified: cfe/trunk/test/Modules/ExtDebugInfo.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/ExtDebugInfo.cpp?rev=258152&r1=258151&r2=258152&view=diff
==
--- cfe/trunk/test/Modules/ExtDebugInfo.cpp (original)
+++ cfe/trunk/test/Modules/ExtDebugInfo.cpp Tue Jan 19 12:02:47 2016
@@ -35,6 +35,10 @@ enum {
 auto anon_enum = DebugCXX::e2;
 char _anchor = anon_enum + conflicting_uid;
 
+TypedefUnion tdu;
+TypedefEnum tde;
+TypedefStruct tds;
+
 // CHECK: ![[NS:.*]] = !DINamespace(name: "DebugCXX", scope: ![[MOD:[0-9]+]],
 // CHECK: ![[MOD]] = !DIModule(scope: null, name: {{.*}}DebugCXX
 
@@ -62,6 +66,13 @@ char _anchor = anon_enum + conflicting_u
 // CHECK-SAME: flags: DIFlagFwdDecl,
 // CHECK-SAME: identifier: 
"_ZTSN8DebugCXX8TemplateIfNS_6traitsIf")
 
+// CHECK: !DICompositeType(tag: DW_TAG_union_type,
+// CHECK-SAME: flags: DIFlagFwdDecl, identifier: 
"_ZTS12TypedefUnion")
+// CHECK: !DICompositeType(tag: DW_TAG_enumeration_type,
+// CHECK-SAME: flags: DIFlagFwdDecl, identifier: 
"_ZTS11TypedefEnum")
+// CHECK: !DICompositeType(tag: DW_TAG_structure_type,
+// CHECK-SAME: flags: DIFlagFwdDecl, identifier: 
"_ZTS13TypedefStruct")
+
 // CHECK: !DIDerivedType(tag: DW_TAG_member, name: "static_member",
 // CHECK-SAME:   scope: !"_ZTSN8DebugCXX6StructE"
 

Modified: cfe/trunk/test/Modules/Inputs/DebugCXX.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/DebugCXX.h?rev=258152&r1=258151&r2=258152&view=diff
==
--- cfe/trunk/test/Modules/Inputs/DebugCXX.h (original)
+++ cfe/trunk/test/Modules/Inputs/DebugCXX.h Tue Jan 19 12:02:47 2016
@@ -58,3 +58,7 @@ class FwdVirtual {
 };
 
 struct PureForwardDecl;
+
+typedef union { int i; } TypedefUnion;
+typedef enum { e0 = 0 } TypedefEnum;
+typedef struct { int i; } TypedefStruct;

Modified: cfe/trunk/test/Modules/ModuleDebugInfo.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/ModuleDebugInfo.cpp?rev=258152&r1=258151&r2=258152&view=diff
==
--- cfe/trunk/test/Modules/ModuleDebugInfo.cpp (original)
+++ cfe/trunk/test/Modules/ModuleDebugInfo.cpp Tue Jan 19 12:02:47 2016
@@ -28,6 +28,10 @@
 // CHECK-SAME: identifier: "_ZTSN8DebugCXX4EnumE")
 // CHECK: !DINamespace(name: "DebugCXX"
 
+// CHECK: !DICompositeType(tag: DW_TAG_enumeration_type,
+// CHECK-SAME-NOT: name:
+// CHECK-SAME: identifier: "_ZTS11TypedefEnum")
+
 // CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "Struct"
 // CHECK-SAME: identifier: "_ZTSN8DebugCXX6StructE")
 
@@ -47,6 +51,14 @@
 // CHECK-SAME: identifier: "_ZTS10FwdVirtual")
 // CHECK: !DIDerivedType(tag: DW_TAG_member, name: "_vptr$FwdVirtual"
 
+// CHECK: !DICompositeType(tag: DW_TAG_union_type,
+// CHECK-SAME-NOT: name:
+// CHECK-SAME: identifier: "_ZTS12TypedefUnion")
+
+// CHECK: !DICompositeType(tag: DW_TAG_structure_type,
+// CHECK-SAME-NOT: name:
+// CHECK-SAME: identifier: "_ZTS13TypedefStruct")
+
 // CHECK: !DIDerivedType(tag: DW_TAG_typedef, name: "FloatInstatiation"
 // no mangled name here yet.
 


___
cfe-commits mailing list
cfe-commits@list

Re: [PATCH] D8822: Proposed fix for PR23076 (conditional branch debug line info)

2016-01-19 Thread Adrian Prantl via cfe-commits

aprantl added a subscriber: aprantl.
aprantl added a comment.

Reposting dblaikie's example hoping that phabricator doesn't mangle it this 
time:

   1: int main() {
   2: if (
   3: tr()
   4: &&
   5: tr()
   6: )
   7: func();
   8: if (
   9: fa()
  10: &&
  11: tr()
  12: )
  13: func();
  14: }
  15:

G++-4.8:
3, 4, 5, 4, 2, 7
10, 11, 9, 15

Clang before the change:
3, 4, 5, 3, 7
10, 11, 15

Clang after the change:
3, 5, 7
10, 15


http://reviews.llvm.org/D8822



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

Re: [PATCH] D16307: [CUDA] Handle -O options (more) correctly.

2016-01-19 Thread Artem Belevich via cfe-commits

tra accepted this revision.
tra added a comment.
This revision is now accepted and ready to land.

LGTM.


http://reviews.llvm.org/D16307



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

Re: [PATCH] D16310: new clang-tidy checker misc-long-cast

2016-01-19 Thread Eugene Zelenko via cfe-commits

Eugene.Zelenko added a subscriber: Eugene.Zelenko.


Comment at: docs/clang-tidy/checks/misc-long-cast.rst:6
@@ +5,3 @@
+
+This checker will warn when there is a explicit redundant cast of a calculation
+result to a bigger type. If the intention of the cast is to avoid loss of

Please use //check// instead of //checker// for documentation consistency.


http://reviews.llvm.org/D16310



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

Re: [PATCH] D16158: [CMake] Add clang's targets to LLVM's export set when not building standalone

2016-01-19 Thread don hinton via cfe-commits

hintonda added a comment.

Ah, great.  That is simpler.


http://reviews.llvm.org/D16158



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

Re: [PATCH] D16248: [Clang-tidy] rename misc-inefficient-algorithm to performance-inefficient-algorithm

2016-01-19 Thread Eugene Zelenko via cfe-commits

Eugene.Zelenko added inline comments.


Comment at: docs/clang-tidy/checks/performance-inefficient-algorithm.rst:3
@@ -2,3 +2,3 @@
 
-misc-inefficient-algorithm
+performance-inefficient-algorithm
 ==

alexfh wrote:
> After reading this check name a few times, I found it too generic (one may 
> think that this is a generic algorithm-level code profiler ;). I think, we 
> need to rename it to `performance-inefficient-lookup-algorithm` or 
> `performance-inefficient-search-algorithm`, since we're changing the name 
> anyway.
I think will be better to keep generic name, since other algorithms could be 
added later.


http://reviews.llvm.org/D16248



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

Re: r258110 - Fix PR26134: When substituting into default template arguments, keep CurContext unchanged.

2016-01-19 Thread Dimitry Andric via cfe-commits

Hi Richard,

I am unsure if you are specifically the code owner of Sema, but can you please 
approve this change for the 3.8 branch?

-Dimitry

> On 19 Jan 2016, at 04:58, Faisal Vali via cfe-commits 
>  wrote:
> 
> Author: faisalv
> Date: Mon Jan 18 21:58:55 2016
> New Revision: 258110
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=258110&view=rev
> Log:
> Fix PR26134: When substituting into default template arguments, keep 
> CurContext unchanged.
> 
> Or, do not set Sema's CurContext to the template declaration's when 
> substituting into default template arguments of said template declaration.
> If we do push the template declaration context on to Sema, and the template 
> declaration is at namespace scope, Sema can get confused and try and do odr 
> analysis when substituting into default template arguments, even though the 
> substitution could be occurring within a dependent context.
> I'm not sure why this was being done, perhaps there was concern that if a 
> default template argument referred to a previous template parameter, it might 
> not be found during substitution - but all regression tests pass, and I can't 
> craft a test that would cause it to fails (if some one does, please inform 
> me, and i'll craft a different fix for the PR).
> 
> 
> This patch removes a single line of code, but unfortunately adds more than it 
> removes, because of the tests.  Some day I still hope to commit a patch that 
> removes far more lines than it adds, while leaving clang better for it ;)
> 
> Sorry that r253590 ("Change the expression evaluation context from 
> Unevaluated to ConstantEvaluated while substituting into non-type template 
> argument defaults") caused the PR!
> 
> 
> 
> 
> 
> Modified:
>cfe/trunk/lib/Sema/SemaTemplate.cpp
>cfe/trunk/test/SemaTemplate/default-arguments.cpp
> 
> Modified: cfe/trunk/lib/Sema/SemaTemplate.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplate.cpp?rev=258110&r1=258109&r2=258110&view=diff
> ==
> --- cfe/trunk/lib/Sema/SemaTemplate.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaTemplate.cpp Mon Jan 18 21:58:55 2016
> @@ -3281,7 +3281,6 @@ SubstDefaultTemplateArgument(Sema &SemaR
>   for (unsigned i = 0, e = Param->getDepth(); i != e; ++i)
> TemplateArgLists.addOuterTemplateArguments(None);
> 
> -  Sema::ContextRAII SavedContext(SemaRef, Template->getDeclContext());
>   EnterExpressionEvaluationContext ConstantEvaluated(SemaRef,
>  Sema::ConstantEvaluated);
>   return SemaRef.SubstExpr(Param->getDefaultArgument(), TemplateArgLists);
> 
> Modified: cfe/trunk/test/SemaTemplate/default-arguments.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaTemplate/default-arguments.cpp?rev=258110&r1=258109&r2=258110&view=diff
> ==
> --- cfe/trunk/test/SemaTemplate/default-arguments.cpp (original)
> +++ cfe/trunk/test/SemaTemplate/default-arguments.cpp Mon Jan 18 21:58:55 2016
> @@ -179,3 +179,31 @@ struct C {
>   C(T t = ); // expected-error {{expected expression}}
> };
> C obj;
> +
> +namespace PR26134 {
> +// Make sure when substituting default template arguments we do it in the 
> current context.
> +template
> +struct X {};
> +
> +template struct Y {
> +  void f() { X xy; }
> +  static const bool value = B;
> +};
> +
> +namespace ns1 {
> +template
> +struct X {
> +  template struct XInner { static const bool value = B; 
> };
> +};
> +template struct S { static const bool value = B; };
> +#if __cplusplus > 199711L
> +template struct Y {
> +  static constexpr bool f() { return typename X>::template 
> XInner<>{}.value; }
> +  static_assert(f() == B, "");
> +};
> +Y y;
> +Y y2;
> +#endif
> +
> +} // end ns1
> +} // end ns PR26134
> 
> 
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits



signature.asc
Description: Message signed with OpenPGP using GPGMail
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D16261: [CUDA] Only allow global on free functions and static member functions.

2016-01-19 Thread Artem Belevich via cfe-commits

tra added inline comments.


Comment at: include/clang/Basic/DiagnosticSemaKinds.td:6418
@@ +6417,3 @@
+def warn_nvcc_compat_kern_is_method : Warning<
+  "kernel function %0 is a member function; this may not be accepted by nvcc">,
+  InGroup;

There's an Extension<> tablegen class used to report various C and C++ 
extensions.
This looks like a good use case for it.


Comment at: include/clang/Basic/DiagnosticSemaKinds.td:6421
@@ +6420,3 @@
+def warn_nvcc_compat_kern_is_inlined : Warning<
+  "kernel function %0 is inlined; this may not be accepted by nvcc">,
+  InGroup;

Perhaps we should use the same message as nvcc here:
'inline qualifier ignored for "global" function'


http://reviews.llvm.org/D16261



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

Re: [PATCH] D16310: new clang-tidy checker misc-long-cast

2016-01-19 Thread Eugene Zelenko via cfe-commits

Eugene.Zelenko added a comment.

Clang-tidy has 6 cast related checks. May be this is good time to introduce 
dedicated category for them?


Repository:
  rL LLVM

http://reviews.llvm.org/D16310



___
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-19 Thread Kim Gräsman via cfe-commits

kimgr added a subscriber: kimgr.
kimgr added a comment.

Came up with another test case.



Comment at: test/clang-tidy/readability-redundant-return.cpp:21-24
@@ +20,6 @@
+
+void g(int i) {
+  if (i < 0) {
+return;
+  }
+  if (i < 10) {

What happens to guard clauses invoking void functions?

void h() {
}

void g(int i) {
  if(i < 0) {
return h();
  }
}



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: r258128 - Add -Wexpansion-to-undefined: warn when using `defined` in a macro definition.

2016-01-19 Thread Evgenii Stepanov via cfe-commits

This broke all WERROR bots. Sounds like this warning should be
disabled by default.

On Tue, Jan 19, 2016 at 8:15 AM, Krzysztof Parzyszek via cfe-commits
 wrote:
> This generates hundreds of warnings when doing check-all.
>
> Here's the offending code:
>
>
> utils/unittest/googletest/include/gtest/internal/gtest-port.h
>
> // Cygwin 1.7 and below doesn't support ::std::wstring.
> // Solaris' libc++ doesn't support it either.  Android has
> // no support for it at least as recent as Froyo (2.2).
> // Minix currently doesn't support it either.
> # define GTEST_HAS_STD_WSTRING \
> (!(GTEST_OS_LINUX_ANDROID || GTEST_OS_CYGWIN || GTEST_OS_SOLARIS ||
> GTEST_OS_HAIKU || defined(_MINIX)))
>
>
> -Krzysztof
>
>
>
>
> On 1/19/2016 9:15 AM, Nico Weber via cfe-commits wrote:
>>
>> Author: nico
>> Date: Tue Jan 19 09:15:31 2016
>> New Revision: 258128
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=258128&view=rev
>> Log:
>> Add -Wexpansion-to-undefined: warn when using `defined` in a macro
>> definition.
>>
>> [cpp.cond]p4:
>>Prior to evaluation, macro invocations in the list of preprocessing
>>tokens that will become the controlling constant expression are
>> replaced
>>(except for those macro names modified by the 'defined' unary
>> operator),
>>just as in normal text. If the token 'defined' is generated as a result
>>of this replacement process or use of the 'defined' unary operator does
>>not match one of the two specified forms prior to macro replacement,
>> the
>>behavior is undefined.
>>
>> This isn't an idle threat, consider this program:
>>#define FOO
>>#define BAR defined(FOO)
>>#if BAR
>>...
>>#else
>>...
>>#endif
>> clang and gcc will pick the #if branch while Visual Studio will take the
>> #else branch.  Emit a warning about this undefined behavior.
>>
>> One problem is that this also applies to function-like macros. While the
>> example above can be written like
>>
>>  #if defined(FOO) && defined(BAR)
>>  #defined HAVE_FOO 1
>>  #else
>>  #define HAVE_FOO 0
>>  #endif
>>
>> there is no easy way to rewrite a function-like macro like `#define FOO(x)
>> (defined __foo_##x && __foo_##x)`.  Function-like macros like this are
>> used in
>> practice, and compilers seem to not have differing behavior in that case.
>> So
>> this a default-on warning only for object-like macros. For function-like
>> macros, it is an extension warning that only shows up with `-pedantic`.
>> (But it's undefined behavior in both cases.)
>>
>> Modified:
>>  cfe/trunk/include/clang/Basic/DiagnosticGroups.td
>>  cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td
>>  cfe/trunk/lib/Lex/PPExpressions.cpp
>>  cfe/trunk/test/Lexer/cxx-features.cpp
>>  cfe/trunk/test/Preprocessor/expr_define_expansion.c
>>
>> Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=258128&r1=258127&r2=258128&view=diff
>>
>> ==
>> --- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)
>> +++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Tue Jan 19 09:15:31
>> 2016
>> @@ -204,6 +204,7 @@ def OverloadedShiftOpParentheses: DiagGr
>>   def DanglingElse: DiagGroup<"dangling-else">;
>>   def DanglingField : DiagGroup<"dangling-field">;
>>   def DistributedObjectModifiers :
>> DiagGroup<"distributed-object-modifiers">;
>> +def ExpansionToUndefined : DiagGroup<"expansion-to-undefined">;
>>   def FlagEnum : DiagGroup<"flag-enum">;
>>   def IncrementBool : DiagGroup<"increment-bool",
>> [DeprecatedIncrementBool]>;
>>   def InfiniteRecursion : DiagGroup<"infinite-recursion">;
>>
>> Modified: cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td?rev=258128&r1=258127&r2=258128&view=diff
>>
>> ==
>> --- cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td (original)
>> +++ cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td Tue Jan 19
>> 09:15:31 2016
>> @@ -658,6 +658,13 @@ def warn_header_guard : Warning<
>>   def note_header_guard : Note<
>> "%0 is defined here; did you mean %1?">;
>>
>> +def warn_defined_in_object_type_macro : Warning<
>> +  "macro expansion producing 'defined' has undefined behavior">,
>> +  InGroup;
>> +def warn_defined_in_function_type_macro : Extension<
>> +  "macro expansion producing 'defined' has undefined behavior">,
>> +  InGroup;
>> +
>>   let CategoryName = "Nullability Issue" in {
>>
>>   def err_pp_assume_nonnull_syntax : Error<"expected 'begin' or 'end'">;
>>
>> Modified: cfe/trunk/lib/Lex/PPExpressions.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPExpressions.cpp?rev=258128&r1=258127&r2=258128&view=diff
>>
>>

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

2016-01-19 Thread Richard via cfe-commits

LegalizeAdulthood added inline comments.


Comment at: test/clang-tidy/readability-redundant-return.cpp:21-24
@@ +20,6 @@
+
+void g(int i) {
+  if (i < 0) {
+return;
+  }
+  if (i < 10) {

kimgr wrote:
> What happens to guard clauses invoking void functions?
> 
> void h() {
> }
> 
> void g(int i) {
>   if(i < 0) {
> return h();
>   }
> }
> 
Nothing because the last statement of the `compoundStmt` that is the function 
body is an `if` statement and not a `return` statement.

That is exactly why lines 21-24 are in the test suite :).


http://reviews.llvm.org/D16259



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

r258165 - [OpenMP] Parsing + sema for "target enter data" directive.

2016-01-19 Thread Samuel Antao via cfe-commits

Author: sfantao
Date: Tue Jan 19 13:15:56 2016
New Revision: 258165

URL: http://llvm.org/viewvc/llvm-project?rev=258165&view=rev
Log:
[OpenMP] Parsing + sema for "target enter data" directive.

Patch by Arpith Jacob. Thanks!


Added:
cfe/trunk/test/OpenMP/target_enter_data_ast_print.cpp
cfe/trunk/test/OpenMP/target_enter_data_device_messages.cpp
cfe/trunk/test/OpenMP/target_enter_data_if_messages.cpp
cfe/trunk/test/OpenMP/target_enter_data_map_messages.c
Modified:
cfe/trunk/include/clang-c/Index.h
cfe/trunk/include/clang/AST/RecursiveASTVisitor.h
cfe/trunk/include/clang/AST/StmtOpenMP.h
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/include/clang/Basic/OpenMPKinds.def
cfe/trunk/include/clang/Basic/StmtNodes.td
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/include/clang/Serialization/ASTBitCodes.h
cfe/trunk/lib/AST/StmtOpenMP.cpp
cfe/trunk/lib/AST/StmtPrinter.cpp
cfe/trunk/lib/AST/StmtProfile.cpp
cfe/trunk/lib/Basic/OpenMPKinds.cpp
cfe/trunk/lib/CodeGen/CGStmt.cpp
cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp
cfe/trunk/lib/CodeGen/CodeGenFunction.h
cfe/trunk/lib/Parse/ParseOpenMP.cpp
cfe/trunk/lib/Sema/SemaOpenMP.cpp
cfe/trunk/lib/Sema/TreeTransform.h
cfe/trunk/lib/Serialization/ASTReaderStmt.cpp
cfe/trunk/lib/Serialization/ASTWriterStmt.cpp
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
cfe/trunk/test/OpenMP/nesting_of_regions.cpp
cfe/trunk/tools/libclang/CIndex.cpp
cfe/trunk/tools/libclang/CXCursor.cpp

Modified: cfe/trunk/include/clang-c/Index.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang-c/Index.h?rev=258165&r1=258164&r2=258165&view=diff
==
--- cfe/trunk/include/clang-c/Index.h (original)
+++ cfe/trunk/include/clang-c/Index.h Tue Jan 19 13:15:56 2016
@@ -2274,7 +2274,11 @@ enum CXCursorKind {
*/
   CXCursor_OMPDistributeDirective= 260,
 
-  CXCursor_LastStmt  = CXCursor_OMPDistributeDirective,
+  /** \brief OpenMP target enter data directive.
+   */
+  CXCursor_OMPTargetEnterDataDirective   = 261,
+
+  CXCursor_LastStmt  = 
CXCursor_OMPTargetEnterDataDirective,
 
   /**
* \brief Cursor that represents the translation unit itself.

Modified: cfe/trunk/include/clang/AST/RecursiveASTVisitor.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/RecursiveASTVisitor.h?rev=258165&r1=258164&r2=258165&view=diff
==
--- cfe/trunk/include/clang/AST/RecursiveASTVisitor.h (original)
+++ cfe/trunk/include/clang/AST/RecursiveASTVisitor.h Tue Jan 19 13:15:56 2016
@@ -2433,6 +2433,9 @@ DEF_TRAVERSE_STMT(OMPTargetDirective,
 DEF_TRAVERSE_STMT(OMPTargetDataDirective,
   { TRY_TO(TraverseOMPExecutableDirective(S)); })
 
+DEF_TRAVERSE_STMT(OMPTargetEnterDataDirective,
+  { TRY_TO(TraverseOMPExecutableDirective(S)); })
+
 DEF_TRAVERSE_STMT(OMPTeamsDirective,
   { TRY_TO(TraverseOMPExecutableDirective(S)); })
 

Modified: cfe/trunk/include/clang/AST/StmtOpenMP.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/StmtOpenMP.h?rev=258165&r1=258164&r2=258165&view=diff
==
--- cfe/trunk/include/clang/AST/StmtOpenMP.h (original)
+++ cfe/trunk/include/clang/AST/StmtOpenMP.h Tue Jan 19 13:15:56 2016
@@ -2038,6 +2038,66 @@ public:
   }
 };
 
+/// \brief This represents '#pragma omp target enter data' directive.
+///
+/// \code
+/// #pragma omp target enter data device(0) if(a) map(b[:])
+/// \endcode
+/// In this example directive '#pragma omp target enter data' has clauses
+/// 'device' with the value '0', 'if' with condition 'a' and 'map' with array
+/// section 'b[:]'.
+///
+class OMPTargetEnterDataDirective : public OMPExecutableDirective {
+  friend class ASTStmtReader;
+  /// \brief Build directive with the given start and end location.
+  ///
+  /// \param StartLoc Starting location of the directive kind.
+  /// \param EndLoc Ending Location of the directive.
+  /// \param NumClauses The number of clauses.
+  ///
+  OMPTargetEnterDataDirective(SourceLocation StartLoc, SourceLocation EndLoc,
+  unsigned NumClauses)
+  : OMPExecutableDirective(this, OMPTargetEnterDataDirectiveClass,
+   OMPD_target_enter_data, StartLoc, EndLoc,
+   NumClauses, /*NumChildren=*/0) {}
+
+  /// \brief Build an empty directive.
+  ///
+  /// \param NumClauses Number of clauses.
+  ///
+  explicit OMPTargetEnterDataDirective(unsigned NumClauses)
+  : OMPExecutableDirective(this, OMPTargetEnterDataDirectiveClass,
+   OMPD_target_enter_data, SourceLocation(),
+   SourceLocation(), NumClauses,
+

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

2016-01-19 Thread Samuel Antao via cfe-commits

sfantao closed this revision.
sfantao added a comment.

Committed revision 258165.

Thanks,
Samuel


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] D16259: Add clang-tidy readability-redundant-return check

2016-01-19 Thread Kim Gräsman via cfe-commits

kimgr added inline comments.


Comment at: test/clang-tidy/readability-redundant-return.cpp:21-24
@@ +20,6 @@
+
+void g(int i) {
+  if (i < 0) {
+return;
+  }
+  if (i < 10) {

LegalizeAdulthood wrote:
> kimgr wrote:
> > What happens to guard clauses invoking void functions?
> > 
> > void h() {
> > }
> > 
> > void g(int i) {
> >   if(i < 0) {
> > return h();
> >   }
> > }
> > 
> Nothing because the last statement of the `compoundStmt` that is the function 
> body is an `if` statement and not a `return` statement.
> 
> That is exactly why lines 21-24 are in the test suite :).
Ah, I hadn't understood the mechanics of the check. I read the docs, and now I 
do! Don't mind me :-)


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: r258128 - Add -Wexpansion-to-undefined: warn when using `defined` in a macro definition.

2016-01-19 Thread Diego Novillo via cfe-commits

Nico, this is producing tons of warnings on an LLVM build and is actually
breaking our internal builds (we build with -Werror).

I fixed one file that was producing this, but there are several that have
the same problem (e.g., gtest-port.h).  Could you fix them or rollback?


Thanks.  Diego.

On Tue, Jan 19, 2016 at 10:15 AM, Nico Weber via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: nico
> Date: Tue Jan 19 09:15:31 2016
> New Revision: 258128
>
> URL: http://llvm.org/viewvc/llvm-project?rev=258128&view=rev
> Log:
> Add -Wexpansion-to-undefined: warn when using `defined` in a macro
> definition.
>
> [cpp.cond]p4:
>   Prior to evaluation, macro invocations in the list of preprocessing
>   tokens that will become the controlling constant expression are replaced
>   (except for those macro names modified by the 'defined' unary operator),
>   just as in normal text. If the token 'defined' is generated as a result
>   of this replacement process or use of the 'defined' unary operator does
>   not match one of the two specified forms prior to macro replacement, the
>   behavior is undefined.
>
> This isn't an idle threat, consider this program:
>   #define FOO
>   #define BAR defined(FOO)
>   #if BAR
>   ...
>   #else
>   ...
>   #endif
> clang and gcc will pick the #if branch while Visual Studio will take the
> #else branch.  Emit a warning about this undefined behavior.
>
> One problem is that this also applies to function-like macros. While the
> example above can be written like
>
> #if defined(FOO) && defined(BAR)
> #defined HAVE_FOO 1
> #else
> #define HAVE_FOO 0
> #endif
>
> there is no easy way to rewrite a function-like macro like `#define FOO(x)
> (defined __foo_##x && __foo_##x)`.  Function-like macros like this are
> used in
> practice, and compilers seem to not have differing behavior in that case.
> So
> this a default-on warning only for object-like macros. For function-like
> macros, it is an extension warning that only shows up with `-pedantic`.
> (But it's undefined behavior in both cases.)
>
> Modified:
> cfe/trunk/include/clang/Basic/DiagnosticGroups.td
> cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td
> cfe/trunk/lib/Lex/PPExpressions.cpp
> cfe/trunk/test/Lexer/cxx-features.cpp
> cfe/trunk/test/Preprocessor/expr_define_expansion.c
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=258128&r1=258127&r2=258128&view=diff
>
> ==
> --- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Tue Jan 19 09:15:31
> 2016
> @@ -204,6 +204,7 @@ def OverloadedShiftOpParentheses: DiagGr
>  def DanglingElse: DiagGroup<"dangling-else">;
>  def DanglingField : DiagGroup<"dangling-field">;
>  def DistributedObjectModifiers :
> DiagGroup<"distributed-object-modifiers">;
> +def ExpansionToUndefined : DiagGroup<"expansion-to-undefined">;
>  def FlagEnum : DiagGroup<"flag-enum">;
>  def IncrementBool : DiagGroup<"increment-bool",
> [DeprecatedIncrementBool]>;
>  def InfiniteRecursion : DiagGroup<"infinite-recursion">;
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td?rev=258128&r1=258127&r2=258128&view=diff
>
> ==
> --- cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td Tue Jan 19
> 09:15:31 2016
> @@ -658,6 +658,13 @@ def warn_header_guard : Warning<
>  def note_header_guard : Note<
>"%0 is defined here; did you mean %1?">;
>
> +def warn_defined_in_object_type_macro : Warning<
> +  "macro expansion producing 'defined' has undefined behavior">,
> +  InGroup;
> +def warn_defined_in_function_type_macro : Extension<
> +  "macro expansion producing 'defined' has undefined behavior">,
> +  InGroup;
> +
>  let CategoryName = "Nullability Issue" in {
>
>  def err_pp_assume_nonnull_syntax : Error<"expected 'begin' or 'end'">;
>
> Modified: cfe/trunk/lib/Lex/PPExpressions.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPExpressions.cpp?rev=258128&r1=258127&r2=258128&view=diff
>
> ==
> --- cfe/trunk/lib/Lex/PPExpressions.cpp (original)
> +++ cfe/trunk/lib/Lex/PPExpressions.cpp Tue Jan 19 09:15:31 2016
> @@ -140,6 +140,51 @@ static bool EvaluateDefined(PPValue &Res
>  PP.LexNonComment(PeekTok);
>}
>
> +  // [cpp.cond]p4:
> +  //   Prior to evaluation, macro invocations in the list of preprocessing
> +  //   tokens that will become the controlling constant expression are
> replaced
> +  //   (except for those macro names modified by the 'defined' unary
> operato

Re: [PATCH] D16286: [clang-tidy] Readability check for redundant parenthesis in return expression.

2016-01-19 Thread Richard via cfe-commits

LegalizeAdulthood added a comment.

@alexfh: The check and the bug report 
 originate in the preferences of 
the google style guide.


http://reviews.llvm.org/D16286



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

Re: [PATCH] D16286: [clang-tidy] Readability check for redundant parenthesis in return expression.

2016-01-19 Thread Richard via cfe-commits

LegalizeAdulthood added a comment.

With readability checks, there are always people who disagree about what makes 
for more readable code.  I think I have seen this back-and-forth discussion 
with **every** readability check since I've been paying attention.  It simply 
isn't possible to make everyone happy and we shouldn't even try.

As I stated earlier on this review, IMO the way to deal with complex 
expressions is not to shotgun blast extra parentheses into the expression, but 
to extract variables or functions with intention revealing names that break out 
meaningful subexpressions.

For checks relating to readability or style, I think it is fair to simply 
assume that the results are subjective.  At the risk of sounding tautological, 
those who don't want the transformation shouldn't ask clang-tidy to perform the 
transformation.

There are already tons of transformations that clang-tidy performs that aren't 
to my personal taste, but are there because of a particular style guide (LLVM, 
google, etc.) or because the author of the check desires the transformation 
that is implemented.

I don't think such transformations should be denied from clang-tidy simply 
because of a difference of opinion.

One could argue that this check should be part of the google module.  The 
google style guide specifically forbids writing return with extra parenthesis.


http://reviews.llvm.org/D16286



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

Re: [PATCH] D16310: new clang-tidy checker misc-long-cast

2016-01-19 Thread Richard via cfe-commits

LegalizeAdulthood added a subscriber: LegalizeAdulthood.
LegalizeAdulthood added a comment.

Why not supply a fixit that removes the cast?


Repository:
  rL LLVM

http://reviews.llvm.org/D16310



___
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-19 Thread Richard via cfe-commits

LegalizeAdulthood added inline comments.

Comment at: test/clang-tidy/readability-redundant-return.cpp:21-24
@@ +20,6 @@
+
+void g(int i) {
+  if (i < 0) {
+return;
+  }
+  if (i < 10) {

kimgr wrote:
> LegalizeAdulthood wrote:
> > kimgr wrote:
> > > What happens to guard clauses invoking void functions?
> > > 
> > > void h() {
> > > }
> > > 
> > > void g(int i) {
> > >   if(i < 0) {
> > > return h();
> > >   }
> > > }
> > > 
> > Nothing because the last statement of the `compoundStmt` that is the 
> > function body is an `if` statement and not a `return` statement.
> > 
> > That is exactly why lines 21-24 are in the test suite :).
> Ah, I hadn't understood the mechanics of the check. I read the docs, and now 
> I do! Don't mind me :-)
I had thought about doing a deeper analysis of the control flow to look for 
such cases, but I will leave that for later.

For instance, the following code may plausibly appear in a code base:

```
void f() {
  do_stuff();
  {
lock l(mutex);
do_locked_stuff();
return;
  }
}
```

I haven't tried this on this patch, but I suspect it would do nothing; I will 
add some examples of these more complicated cases to the test suite to show 
that the implementation doesn't yet handle more advanced flow analysis.

In this case, the `return` is similarly redundant, as well as a `return` as the 
last statement of an `if` as you mentioned.  However, I wanted to start with 
something simple and get feedback on that before attempting to do more advanced 
cases.

http://reviews.llvm.org/D16259

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

r258174 - [CUDA] Handle -O options (more) correctly.

2016-01-19 Thread Justin Lebar via cfe-commits

Author: jlebar
Date: Tue Jan 19 13:52:21 2016
New Revision: 258174

URL: http://llvm.org/viewvc/llvm-project?rev=258174&view=rev
Log:
[CUDA] Handle -O options (more) correctly.

Summary:
Previously we'd crash the driver if you passed -O0.  Now we try to
handle all of clang's various optimization flags in a sane way.

Reviewers: tra

Subscribers: cfe-commits, echristo, jhen

Differential Revision: http://reviews.llvm.org/D16307

Modified:
cfe/trunk/lib/Driver/Tools.cpp
cfe/trunk/test/Driver/cuda-external-tools.cu

Modified: cfe/trunk/lib/Driver/Tools.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Tools.cpp?rev=258174&r1=258173&r2=258174&view=diff
==
--- cfe/trunk/lib/Driver/Tools.cpp (original)
+++ cfe/trunk/lib/Driver/Tools.cpp Tue Jan 19 13:52:21 2016
@@ -10663,10 +10663,35 @@ void NVPTX::Assembler::ConstructJob(Comp
   ArgStringList CmdArgs;
   CmdArgs.push_back(TC.getTriple().isArch64Bit() ? "-m64" : "-m32");
 
-  // Clang's default optimization level is -O0, but ptxas's default is -O3.
-  CmdArgs.push_back(Args.MakeArgString(
-  llvm::Twine("-O") +
-  Args.getLastArgValue(options::OPT_O_Group, "0").data()));
+  // Map the -O we received to -O{0,1,2,3}.
+  //
+  // TODO: Perhaps we should map host -O2 to ptxas -O3. -O3 is ptxas's default,
+  // so it may correspond more closely to the spirit of clang -O2.
+  if (Arg *A = Args.getLastArg(options::OPT_O_Group)) {
+// -O3 seems like the least-bad option when -Osomething is specified to
+// clang but it isn't handled below.
+StringRef OOpt = "3";
+if (A->getOption().matches(options::OPT_O4) ||
+A->getOption().matches(options::OPT_Ofast))
+  OOpt = "3";
+else if (A->getOption().matches(options::OPT_O0))
+  OOpt = "0";
+else if (A->getOption().matches(options::OPT_O)) {
+  // -Os, -Oz, and -O(anything else) map to -O2, for lack of better 
options.
+  OOpt = llvm::StringSwitch(A->getValue())
+ .Case("1", "1")
+ .Case("2", "2")
+ .Case("3", "3")
+ .Case("s", "2")
+ .Case("z", "2")
+ .Default("2");
+}
+CmdArgs.push_back(Args.MakeArgString(llvm::Twine("-O") + OOpt));
+  } else {
+// If no -O was passed, pass -O0 to ptxas -- no opt flag should correspond
+// to no optimizations, but ptxas's default is -O3.
+CmdArgs.push_back("-O0");
+  }
 
   // Don't bother passing -g to ptxas: It's enabled by default at -O0, and
   // not supported at other optimization levels.

Modified: cfe/trunk/test/Driver/cuda-external-tools.cu
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/cuda-external-tools.cu?rev=258174&r1=258173&r2=258174&view=diff
==
--- cfe/trunk/test/Driver/cuda-external-tools.cu (original)
+++ cfe/trunk/test/Driver/cuda-external-tools.cu Tue Jan 19 13:52:21 2016
@@ -4,14 +4,31 @@
 // REQUIRES: x86-registered-target
 // REQUIRES: nvptx-registered-target
 
-// Regular compile with -O2.
+// Regular compiles with -O{0,1,2,3,4,fast}.  -O4 and -Ofast map to ptxas O3.
+// RUN: %clang -### -target x86_64-linux-gnu -O0 -c %s 2>&1 \
+// RUN: | FileCheck -check-prefix ARCH64 -check-prefix SM20 -check-prefix OPT0 
%s
+// RUN: %clang -### -target x86_64-linux-gnu -O1 -c %s 2>&1 \
+// RUN: | FileCheck -check-prefix ARCH64 -check-prefix SM20 -check-prefix OPT1 
%s
 // RUN: %clang -### -target x86_64-linux-gnu -O2 -c %s 2>&1 \
 // RUN: | FileCheck -check-prefix ARCH64 -check-prefix SM20 -check-prefix OPT2 
%s
+// RUN: %clang -### -target x86_64-linux-gnu -O3 -c %s 2>&1 \
+// RUN: | FileCheck -check-prefix ARCH64 -check-prefix SM20 -check-prefix OPT3 
%s
+// RUN: %clang -### -target x86_64-linux-gnu -O4 -c %s 2>&1 \
+// RUN: | FileCheck -check-prefix ARCH64 -check-prefix SM20 -check-prefix OPT3 
%s
+// RUN: %clang -### -target x86_64-linux-gnu -Ofast -c %s 2>&1 \
+// RUN: | FileCheck -check-prefix ARCH64 -check-prefix SM20 -check-prefix OPT3 
%s
 
 // Regular compile without -O.  This should result in us passing -O0 to ptxas.
 // RUN: %clang -### -target x86_64-linux-gnu -c %s 2>&1 \
 // RUN: | FileCheck -check-prefix ARCH64 -check-prefix SM20 -check-prefix OPT0 
%s
 
+// Regular compiles with -Os and -Oz.  For lack of a better option, we map
+// these to ptxas -O3.
+// RUN: %clang -### -target x86_64-linux-gnu -Os -c %s 2>&1 \
+// RUN: | FileCheck -check-prefix ARCH64 -check-prefix SM20 -check-prefix OPT2 
%s
+// RUN: %clang -### -target x86_64-linux-gnu -Oz -c %s 2>&1 \
+// RUN: | FileCheck -check-prefix ARCH64 -check-prefix SM20 -check-prefix OPT2 
%s
+
 // Regular compile targeting sm_35.
 // RUN: %clang -### -target x86_64-linux-gnu --cuda-gpu-arch=sm_35 -c %s 2>&1 \
 // RUN: | FileCheck -check-prefix ARCH64 -check-prefix SM35 %s
@@ -42,7 +59,9 @@
 // ARCH64: "-m64"
 // ARCH32: "-m32"
 // OPT0: "-O0"
+// OPT1: "-O1

Re: [PATCH] D16286: [clang-tidy] Readability check for redundant parenthesis in return expression.

2016-01-19 Thread Aaron Ballman via cfe-commits

aaron.ballman added a comment.

In http://reviews.llvm.org/D16286#330360, @LegalizeAdulthood wrote:

> With readability checks, there are always people who disagree about what 
> makes for more readable code.  I think I have seen this back-and-forth 
> discussion with **every** readability check since I've been paying attention. 
>  It simply isn't possible to make everyone happy and we shouldn't even try.

It is perfectly reasonable for people to disagree about what makes code more or 
less readable. Some readability issues are more obvious than others, and the 
threshold for "readable" is going to change from check to check. However, that 
is not a reason to not attempt to make a check that makes everyone reasonably 
(un)happy.

> As I stated earlier on this review, IMO the way to deal with complex 
> expressions is not to shotgun blast extra parentheses into the expression, 
> but to extract variables or functions with intention revealing names that 
> break out meaningful subexpressions.

That seems reasonable to me, but this checker is *removing* parens, not adding 
them. The point I was making before is that when a subexpression is 
sufficiently complex, parens are a quick-and-dirty way to reduce cognitive load 
for understanding operator precedence. This is an approach you see relatively 
frequently -- a quick look at the LLVM source base shows that. For a check that 
claims it will make something more readable, removing code the user wrote had 
better have a pretty good chance of improving readability or else the check 
will quickly get disabled.

A good way to empirically test this is to run this check over the LLVM source 
base (and/or a few other large source bases) and see how often the diagnostic 
is triggered, and take a random sampling to see how often the removal of parens 
involves an expression with multiple operators of differing precedence to see 
if perhaps an option or design change is warranted or not. These sort of 
metrics are something we usually look for with any clang-tidy check that may 
have a high false-positive rate. For stylistic checks, I suppose just about 
anything could qualify as a false-positive (unless it's part of a style 
guideline that's being followed).

> For checks relating to readability or style, I think it is fair to simply 
> assume that the results are subjective.

Definitely agreed.

> At the risk of sounding tautological, those who don't want the transformation 
> shouldn't ask clang-tidy to perform the transformation.

That presumes the user knows what transformations this check will apply. If 
this was misc-remove-redundant-parens or 
some-style-guide-remove-redundant-parens, I would agree with your statement. 
However, with readability-* I think the user is likely to ask clang-tidy to 
perform the check and apply changes on a case-by-case basis. So we may want to 
start with the smallest set of cases where the check might increase readability 
and then expand from there to cover more cases as uses are requested.

> There are already tons of transformations that clang-tidy performs that 
> aren't to my personal taste, but are there because of a particular style 
> guide (LLVM, google, etc.) or because the author of the check desires the 
> transformation that is implemented.

> 

> I don't think such transformations should be denied from clang-tidy simply 
> because of a difference of opinion.

No one is recommending denying this transformation; just discussing the 
particulars of it to ensure we have reasonable semantics for the check.

> One could argue that this check should be part of the google module.  The 
> google style guide specifically forbids writing return with extra parenthesis.

If it is meant to work for a particular style guide, then it should absolutely 
go under that style guide. But for something more generally under the 
readability-* umbrella, I think it doesn't hurt to discuss where to draw the 
line, or what options may be valuable.

http://reviews.llvm.org/D16286

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

Re: [PATCH] D16307: [CUDA] Handle -O options (more) correctly.

2016-01-19 Thread Justin Lebar via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL258174: [CUDA] Handle -O options (more) correctly. (authored 
by jlebar).

Changed prior to commit:
  http://reviews.llvm.org/D16307?vs=45223&id=45290#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D16307

Files:
  cfe/trunk/lib/Driver/Tools.cpp
  cfe/trunk/test/Driver/cuda-external-tools.cu

Index: cfe/trunk/test/Driver/cuda-external-tools.cu
===
--- cfe/trunk/test/Driver/cuda-external-tools.cu
+++ cfe/trunk/test/Driver/cuda-external-tools.cu
@@ -4,14 +4,31 @@
 // REQUIRES: x86-registered-target
 // REQUIRES: nvptx-registered-target
 
-// Regular compile with -O2.
+// Regular compiles with -O{0,1,2,3,4,fast}.  -O4 and -Ofast map to ptxas O3.
+// RUN: %clang -### -target x86_64-linux-gnu -O0 -c %s 2>&1 \
+// RUN: | FileCheck -check-prefix ARCH64 -check-prefix SM20 -check-prefix OPT0 
%s
+// RUN: %clang -### -target x86_64-linux-gnu -O1 -c %s 2>&1 \
+// RUN: | FileCheck -check-prefix ARCH64 -check-prefix SM20 -check-prefix OPT1 
%s
 // RUN: %clang -### -target x86_64-linux-gnu -O2 -c %s 2>&1 \
 // RUN: | FileCheck -check-prefix ARCH64 -check-prefix SM20 -check-prefix OPT2 
%s
+// RUN: %clang -### -target x86_64-linux-gnu -O3 -c %s 2>&1 \
+// RUN: | FileCheck -check-prefix ARCH64 -check-prefix SM20 -check-prefix OPT3 
%s
+// RUN: %clang -### -target x86_64-linux-gnu -O4 -c %s 2>&1 \
+// RUN: | FileCheck -check-prefix ARCH64 -check-prefix SM20 -check-prefix OPT3 
%s
+// RUN: %clang -### -target x86_64-linux-gnu -Ofast -c %s 2>&1 \
+// RUN: | FileCheck -check-prefix ARCH64 -check-prefix SM20 -check-prefix OPT3 
%s
 
 // Regular compile without -O.  This should result in us passing -O0 to ptxas.
 // RUN: %clang -### -target x86_64-linux-gnu -c %s 2>&1 \
 // RUN: | FileCheck -check-prefix ARCH64 -check-prefix SM20 -check-prefix OPT0 
%s
 
+// Regular compiles with -Os and -Oz.  For lack of a better option, we map
+// these to ptxas -O3.
+// RUN: %clang -### -target x86_64-linux-gnu -Os -c %s 2>&1 \
+// RUN: | FileCheck -check-prefix ARCH64 -check-prefix SM20 -check-prefix OPT2 
%s
+// RUN: %clang -### -target x86_64-linux-gnu -Oz -c %s 2>&1 \
+// RUN: | FileCheck -check-prefix ARCH64 -check-prefix SM20 -check-prefix OPT2 
%s
+
 // Regular compile targeting sm_35.
 // RUN: %clang -### -target x86_64-linux-gnu --cuda-gpu-arch=sm_35 -c %s 2>&1 \
 // RUN: | FileCheck -check-prefix ARCH64 -check-prefix SM35 %s
@@ -42,7 +59,9 @@
 // ARCH64: "-m64"
 // ARCH32: "-m32"
 // OPT0: "-O0"
+// OPT1: "-O1"
 // OPT2: "-O2"
+// OPT3: "-O3"
 // SM20: "--gpu-name" "sm_20"
 // SM35: "--gpu-name" "sm_35"
 // SM20: "--output-file" "[[CUBINFILE:[^"]*]]"
Index: cfe/trunk/lib/Driver/Tools.cpp
===
--- cfe/trunk/lib/Driver/Tools.cpp
+++ cfe/trunk/lib/Driver/Tools.cpp
@@ -10663,10 +10663,35 @@
   ArgStringList CmdArgs;
   CmdArgs.push_back(TC.getTriple().isArch64Bit() ? "-m64" : "-m32");
 
-  // Clang's default optimization level is -O0, but ptxas's default is -O3.
-  CmdArgs.push_back(Args.MakeArgString(
-  llvm::Twine("-O") +
-  Args.getLastArgValue(options::OPT_O_Group, "0").data()));
+  // Map the -O we received to -O{0,1,2,3}.
+  //
+  // TODO: Perhaps we should map host -O2 to ptxas -O3. -O3 is ptxas's default,
+  // so it may correspond more closely to the spirit of clang -O2.
+  if (Arg *A = Args.getLastArg(options::OPT_O_Group)) {
+// -O3 seems like the least-bad option when -Osomething is specified to
+// clang but it isn't handled below.
+StringRef OOpt = "3";
+if (A->getOption().matches(options::OPT_O4) ||
+A->getOption().matches(options::OPT_Ofast))
+  OOpt = "3";
+else if (A->getOption().matches(options::OPT_O0))
+  OOpt = "0";
+else if (A->getOption().matches(options::OPT_O)) {
+  // -Os, -Oz, and -O(anything else) map to -O2, for lack of better 
options.
+  OOpt = llvm::StringSwitch(A->getValue())
+ .Case("1", "1")
+ .Case("2", "2")
+ .Case("3", "3")
+ .Case("s", "2")
+ .Case("z", "2")
+ .Default("2");
+}
+CmdArgs.push_back(Args.MakeArgString(llvm::Twine("-O") + OOpt));
+  } else {
+// If no -O was passed, pass -O0 to ptxas -- no opt flag should correspond
+// to no optimizations, but ptxas's default is -O3.
+CmdArgs.push_back("-O0");
+  }
 
   // Don't bother passing -g to ptxas: It's enabled by default at -O0, and
   // not supported at other optimization levels.


Index: cfe/trunk/test/Driver/cuda-external-tools.cu
===
--- cfe/trunk/test/Driver/cuda-external-tools.cu
+++ cfe/trunk/test/Driver/cuda-external-tools.cu
@@ -4,14 +4,31 @@
 // REQUIRES: x86-registered-target
 // REQUIRES: nvptx-registered-target
 
-// Regular compile with -O2.
+// Regular compiles with -O{0,

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

2016-01-19 Thread Aaron Ballman via cfe-commits

aaron.ballman added inline comments.


Comment at: test/clang-tidy/readability-redundant-return.cpp:21-24
@@ +20,6 @@
+
+void g(int i) {
+  if (i < 0) {
+return;
+  }
+  if (i < 10) {

LegalizeAdulthood wrote:
> kimgr wrote:
> > LegalizeAdulthood wrote:
> > > kimgr wrote:
> > > > What happens to guard clauses invoking void functions?
> > > > 
> > > > void h() {
> > > > }
> > > > 
> > > > void g(int i) {
> > > >   if(i < 0) {
> > > > return h();
> > > >   }
> > > > }
> > > > 
> > > Nothing because the last statement of the `compoundStmt` that is the 
> > > function body is an `if` statement and not a `return` statement.
> > > 
> > > That is exactly why lines 21-24 are in the test suite :).
> > Ah, I hadn't understood the mechanics of the check. I read the docs, and 
> > now I do! Don't mind me :-)
> I had thought about doing a deeper analysis of the control flow to look for 
> such cases, but I will leave that for later.
> 
> For instance, the following code may plausibly appear in a code base:
> 
> ```
> void f() {
>   do_stuff();
>   {
> lock l(mutex);
> do_locked_stuff();
> return;
>   }
> }
> ```
> 
> I haven't tried this on this patch, but I suspect it would do nothing; I will 
> add some examples of these more complicated cases to the test suite to show 
> that the implementation doesn't yet handle more advanced flow analysis.
> 
> In this case, the `return` is similarly redundant, as well as a `return` as 
> the last statement of an `if` as you mentioned.  However, I wanted to start 
> with something simple and get feedback on that before attempting to do more 
> advanced cases.
> 
I think that starting simple and expanding later is the right approach.


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] D15699: [cfi] Cross-DSO CFI diagnostic mode (clang part)

2016-01-19 Thread Evgeniy Stepanov via cfe-commits

eugenis updated this revision to Diff 45292.

Repository:
  rL LLVM

http://reviews.llvm.org/D15699

Files:
  lib/CodeGen/CGClass.cpp
  lib/CodeGen/CGExpr.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/CodeGen/CodeGenModule.cpp
  lib/Driver/Tools.cpp
  test/CodeGen/cfi-check-fail.c
  test/CodeGen/cfi-icall-cross-dso.c
  test/CodeGenCXX/cfi-cross-dso.cpp
  test/CodeGenCXX/cfi-vcall.cpp

Index: test/CodeGenCXX/cfi-vcall.cpp
===
--- test/CodeGenCXX/cfi-vcall.cpp
+++ test/CodeGenCXX/cfi-vcall.cpp
@@ -55,7 +55,7 @@
 
 // DIAG: @[[SRC:.*]] = private unnamed_addr constant [{{.*}} x i8] c"{{.*}}cfi-vcall.cpp\00", align 1
 // DIAG: @[[TYPE:.*]] = private unnamed_addr constant { i16, i16, [4 x i8] } { i16 -1, i16 0, [4 x i8] c"'A'\00" }
-// DIAG: @[[BADTYPESTATIC:.*]] = private unnamed_addr global { { [{{.*}} x i8]*, i32, i32 }, { i16, i16, [4 x i8] }*, i8 } { { [{{.*}} x i8]*, i32, i32 } { [{{.*}} x i8]* @[[SRC]], i32 [[@LINE+21]], i32 3 }, { i16, i16, [4 x i8] }* @[[TYPE]], i8 0 }
+// DIAG: @[[BADTYPESTATIC:.*]] = private unnamed_addr global { i8, { [{{.*}} x i8]*, i32, i32 }, { i16, i16, [4 x i8] }* } { i8 0, { [{{.*}} x i8]*, i32, i32 } { [{{.*}} x i8]* @[[SRC]], i32 [[@LINE+21]], i32 3 }, { i16, i16, [4 x i8] }* @[[TYPE]] }
 
 // ITANIUM: define void @_Z2afP1A
 // MS: define void @"\01?af@@YAXPEAUA@@@Z"
@@ -69,9 +69,9 @@
   // NDIAG-NEXT: call void @llvm.trap()
   // NDIAG-NEXT: unreachable
   // DIAG-NEXT: [[VTINT:%[^ ]*]] = ptrtoint i8* [[VT]] to i64
-  // DIAG-ABORT-NEXT: call void @__ubsan_handle_cfi_bad_type_abort(i8* bitcast ({{.*}} @[[BADTYPESTATIC]] to i8*), i64 [[VTINT]])
+  // DIAG-ABORT-NEXT: call void @__ubsan_handle_cfi_check_fail_abort(i8* getelementptr inbounds ({{.*}} @[[BADTYPESTATIC]], i32 0, i32 0), i64 [[VTINT]])
   // DIAG-ABORT-NEXT: unreachable
-  // DIAG-RECOVER-NEXT: call void @__ubsan_handle_cfi_bad_type(i8* bitcast ({{.*}} @[[BADTYPESTATIC]] to i8*), i64 [[VTINT]])
+  // DIAG-RECOVER-NEXT: call void @__ubsan_handle_cfi_check_fail(i8* getelementptr inbounds ({{.*}} @[[BADTYPESTATIC]], i32 0, i32 0), i64 [[VTINT]])
   // DIAG-RECOVER-NEXT: br label %[[CONTBB]]
 
   // CHECK: [[CONTBB]]
Index: test/CodeGenCXX/cfi-cross-dso.cpp
===
--- test/CodeGenCXX/cfi-cross-dso.cpp
+++ test/CodeGenCXX/cfi-cross-dso.cpp
@@ -34,8 +34,8 @@
 // MS:   %[[TEST:.*]] = call i1 @llvm.bitset.test(i8* %[[VT2]], metadata !"?AUA@@"), !nosanitize
 // CHECK:   br i1 %[[TEST]], label %[[CONT:.*]], label %[[SLOW:.*]], {{.*}} !nosanitize
 // CHECK: [[SLOW]]
-// ITANIUM:   call void @__cfi_slowpath(i64 7004155349499253778, i8* %[[VT2]]) {{.*}} !nosanitize
-// MS:   call void @__cfi_slowpath(i64 -8005289897957287421, i8* %[[VT2]]) {{.*}} !nosanitize
+// ITANIUM:   call void @__cfi_slowpath_diag(i64 7004155349499253778, i8* %[[VT2]], {{.*}}) {{.*}} !nosanitize
+// MS:   call void @__cfi_slowpath_diag(i64 -8005289897957287421, i8* %[[VT2]], {{.*}}) {{.*}} !nosanitize
 // CHECK:   br label %[[CONT]], !nosanitize
 // CHECK: [[CONT]]
 // CHECK:   call void %{{.*}}(%struct.A* %{{.*}})
Index: test/CodeGen/cfi-icall-cross-dso.c
===
--- test/CodeGen/cfi-icall-cross-dso.c
+++ test/CodeGen/cfi-icall-cross-dso.c
@@ -1,5 +1,30 @@
-// RUN: %clang_cc1 -triple x86_64-unknown-linux -O1 -fsanitize=cfi-icall -fsanitize-cfi-cross-dso -emit-llvm -o - %s | FileCheck --check-prefix=CHECK --check-prefix=ITANIUM %s
-// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc -O1 -fsanitize=cfi-icall  -fsanitize-cfi-cross-dso -emit-llvm -o - %s | FileCheck --check-prefix=CHECK --check-prefix=MS %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux -O1 \
+// RUN:   -fsanitize=cfi-icall -fsanitize-cfi-cross-dso \
+// RUN:   -emit-llvm -o - %s | FileCheck \
+// RUN:   --check-prefix=CHECK --check-prefix=CHECK-DIAG \
+// RUN:   --check-prefix=ITANIUM --check-prefix=ITANIUM-DIAG \
+// RUN:   %s
+
+// RUN: %clang_cc1 -triple x86_64-unknown-linux -O1 \
+// RUN:   -fsanitize=cfi-icall -fsanitize-cfi-cross-dso -fsanitize-trap=cfi-icall \
+// RUN:   -emit-llvm -o - %s | FileCheck \
+// RUN:   --check-prefix=CHECK \
+// RUN:   --check-prefix=ITANIUM --check-prefix=ITANIUM-TRAP \
+// RUN:   %s
+
+// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc -O1 \
+// RUN:   -fsanitize=cfi-icall -fsanitize-cfi-cross-dso \
+// RUN:   -emit-llvm -o - %s | FileCheck \
+// RUN:   --check-prefix=CHECK --check-prefix=CHECK-DIAG \
+// RUN:   --check-prefix=MS --check-prefix=MS-DIAG \
+// RUN:   %s
+
+// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc -O1 \
+// RUN:   -fsanitize=cfi-icall -fsanitize-cfi-cross-dso -fsanitize-trap=cfi-icall \
+// RUN:   -emit-llvm -o - %s | FileCheck \
+// RUN:   --check-prefix=CHECK \
+// RUN:   --check-prefix=MS --check-prefix=MS-TRAP \
+// RUN:   %s
 
 void caller(void (*f)()) {
   f();
@@ -19,11 +44,18 @@
 inline void foo() {

r258177 - [OpenMP] Parsing + sema for "target exit data" directive.

2016-01-19 Thread Samuel Antao via cfe-commits

Author: sfantao
Date: Tue Jan 19 14:04:50 2016
New Revision: 258177

URL: http://llvm.org/viewvc/llvm-project?rev=258177&view=rev
Log:
[OpenMP] Parsing + sema for "target exit data" directive.

Patch by Arpith Jacob. Thanks!

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

Modified: cfe/trunk/include/clang-c/Index.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang-c/Index.h?rev=258177&r1=258176&r2=258177&view=diff
==
--- cfe/trunk/include/clang-c/Index.h (original)
+++ cfe/trunk/include/clang-c/Index.h Tue Jan 19 14:04:50 2016
@@ -2278,7 +2278,11 @@ enum CXCursorKind {
*/
   CXCursor_OMPTargetEnterDataDirective   = 261,
 
-  CXCursor_LastStmt  = 
CXCursor_OMPTargetEnterDataDirective,
+  /** \brief OpenMP target exit data directive.
+   */
+  CXCursor_OMPTargetExitDataDirective= 262,
+
+  CXCursor_LastStmt  = CXCursor_OMPTargetExitDataDirective,
 
   /**
* \brief Cursor that represents the translation unit itself.

Modified: cfe/trunk/include/clang/AST/RecursiveASTVisitor.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/RecursiveASTVisitor.h?rev=258177&r1=258176&r2=258177&view=diff
==
--- cfe/trunk/include/clang/AST/RecursiveASTVisitor.h (original)
+++ cfe/trunk/include/clang/AST/RecursiveASTVisitor.h Tue Jan 19 14:04:50 2016
@@ -2436,6 +2436,9 @@ DEF_TRAVERSE_STMT(OMPTargetDataDirective
 DEF_TRAVERSE_STMT(OMPTargetEnterDataDirective,
   { TRY_TO(TraverseOMPExecutableDirective(S)); })
 
+DEF_TRAVERSE_STMT(OMPTargetExitDataDirective,
+  { TRY_TO(TraverseOMPExecutableDirective(S)); })
+
 DEF_TRAVERSE_STMT(OMPTeamsDirective,
   { TRY_TO(TraverseOMPExecutableDirective(S)); })
 

Modified: cfe/trunk/include/clang/AST/StmtOpenMP.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/StmtOpenMP.h?rev=258177&r1=258176&r2=258177&view=diff
==
--- cfe/trunk/include/clang/AST/StmtOpenMP.h (original)
+++ cfe/trunk/include/clang/AST/StmtOpenMP.h Tue Jan 19 14:04:50 2016
@@ -2098,6 +2098,66 @@ public:
   }
 };
 
+/// \brief This represents '#pragma omp target exit data' directive.
+///
+/// \code
+/// #pragma omp target exit data device(0) if(a) map(b[:])
+/// \endcode
+/// In this example directive '#pragma omp target exit data' has clauses
+/// 'device' with the value '0', 'if' with condition 'a' and 'map' with array
+/// section 'b[:]'.
+///
+class OMPTargetExitDataDirective : public OMPExecutableDirective {
+  friend class ASTStmtReader;
+  /// \brief Build directive with the given start and end location.
+  ///
+  /// \param StartLoc Starting location of the directive kind.
+  /// \param EndLoc Ending Location of the directive.
+  /// \param NumClauses The number of clauses.
+  ///
+  OMPTargetExitDataDirective(SourceLocation StartLoc, SourceLocation EndLoc,
+ unsigned NumClauses)
+  : OMPExecutableDirective(this, OMPTargetExitDataDirectiveClass,
+   OMPD_target_exit_data, StartLoc, EndLoc,
+   NumClauses, /*NumChildren=*/0) {}
+
+  /// \brief Build an empty directive.
+  ///
+  /// \param NumClauses Number of clauses.
+  ///
+  explicit OMPTargetExitDataDirective(unsigned NumClauses)
+  : OMPExecutableDirective(this, OMPTargetExitDataDirectiveClass,
+   OMPD_target_exit_data, SourceLocation(),
+   SourceLocation(), NumClauses,
+   /*NumChildren=*/0) {}
+
+public:
+  /// \bri

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

2016-01-19 Thread Samuel Antao via cfe-commits

sfantao closed this revision.
sfantao added a comment.

Committed revision 258177.

Thanks,
Samuel


http://reviews.llvm.org/D16279



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

Re: r258128 - Add -Wexpansion-to-undefined: warn when using `defined` in a macro definition.

2016-01-19 Thread Nico Weber via cfe-commits

I'll take a look. If it's urgent it's also possible to disable this warning.
On Jan 19, 2016 2:29 PM, "Diego Novillo via cfe-commits" <
cfe-commits@lists.llvm.org> wrote:

> Nico, this is producing tons of warnings on an LLVM build and is actually
> breaking our internal builds (we build with -Werror).
>
> I fixed one file that was producing this, but there are several that have
> the same problem (e.g., gtest-port.h).  Could you fix them or rollback?
>
>
> Thanks.  Diego.
>
> On Tue, Jan 19, 2016 at 10:15 AM, Nico Weber via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: nico
>> Date: Tue Jan 19 09:15:31 2016
>> New Revision: 258128
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=258128&view=rev
>> Log:
>> Add -Wexpansion-to-undefined: warn when using `defined` in a macro
>> definition.
>>
>> [cpp.cond]p4:
>>   Prior to evaluation, macro invocations in the list of preprocessing
>>   tokens that will become the controlling constant expression are replaced
>>   (except for those macro names modified by the 'defined' unary operator),
>>   just as in normal text. If the token 'defined' is generated as a result
>>   of this replacement process or use of the 'defined' unary operator does
>>   not match one of the two specified forms prior to macro replacement, the
>>   behavior is undefined.
>>
>> This isn't an idle threat, consider this program:
>>   #define FOO
>>   #define BAR defined(FOO)
>>   #if BAR
>>   ...
>>   #else
>>   ...
>>   #endif
>> clang and gcc will pick the #if branch while Visual Studio will take the
>> #else branch.  Emit a warning about this undefined behavior.
>>
>> One problem is that this also applies to function-like macros. While the
>> example above can be written like
>>
>> #if defined(FOO) && defined(BAR)
>> #defined HAVE_FOO 1
>> #else
>> #define HAVE_FOO 0
>> #endif
>>
>> there is no easy way to rewrite a function-like macro like `#define FOO(x)
>> (defined __foo_##x && __foo_##x)`.  Function-like macros like this are
>> used in
>> practice, and compilers seem to not have differing behavior in that case.
>> So
>> this a default-on warning only for object-like macros. For function-like
>> macros, it is an extension warning that only shows up with `-pedantic`.
>> (But it's undefined behavior in both cases.)
>>
>> Modified:
>> cfe/trunk/include/clang/Basic/DiagnosticGroups.td
>> cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td
>> cfe/trunk/lib/Lex/PPExpressions.cpp
>> cfe/trunk/test/Lexer/cxx-features.cpp
>> cfe/trunk/test/Preprocessor/expr_define_expansion.c
>>
>> Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=258128&r1=258127&r2=258128&view=diff
>>
>> ==
>> --- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)
>> +++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Tue Jan 19 09:15:31
>> 2016
>> @@ -204,6 +204,7 @@ def OverloadedShiftOpParentheses: DiagGr
>>  def DanglingElse: DiagGroup<"dangling-else">;
>>  def DanglingField : DiagGroup<"dangling-field">;
>>  def DistributedObjectModifiers :
>> DiagGroup<"distributed-object-modifiers">;
>> +def ExpansionToUndefined : DiagGroup<"expansion-to-undefined">;
>>  def FlagEnum : DiagGroup<"flag-enum">;
>>  def IncrementBool : DiagGroup<"increment-bool",
>> [DeprecatedIncrementBool]>;
>>  def InfiniteRecursion : DiagGroup<"infinite-recursion">;
>>
>> Modified: cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td?rev=258128&r1=258127&r2=258128&view=diff
>>
>> ==
>> --- cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td (original)
>> +++ cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td Tue Jan 19
>> 09:15:31 2016
>> @@ -658,6 +658,13 @@ def warn_header_guard : Warning<
>>  def note_header_guard : Note<
>>"%0 is defined here; did you mean %1?">;
>>
>> +def warn_defined_in_object_type_macro : Warning<
>> +  "macro expansion producing 'defined' has undefined behavior">,
>> +  InGroup;
>> +def warn_defined_in_function_type_macro : Extension<
>> +  "macro expansion producing 'defined' has undefined behavior">,
>> +  InGroup;
>> +
>>  let CategoryName = "Nullability Issue" in {
>>
>>  def err_pp_assume_nonnull_syntax : Error<"expected 'begin' or 'end'">;
>>
>> Modified: cfe/trunk/lib/Lex/PPExpressions.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPExpressions.cpp?rev=258128&r1=258127&r2=258128&view=diff
>>
>> ==
>> --- cfe/trunk/lib/Lex/PPExpressions.cpp (original)
>> +++ cfe/trunk/lib/Lex/PPExpressions.cpp Tue Jan 19 09:15:31 2016
>> @@ -140,6 +140,51 @@ static bool EvaluateDefined(PPValue &Res
>>  PP.Lex

Re: r258128 - Add -Wexpansion-to-undefined: warn when using `defined` in a macro definition.

2016-01-19 Thread Diego Novillo via cfe-commits

On Tue, Jan 19, 2016 at 3:30 PM, Nico Weber  wrote:

> I'll take a look. If it's urgent it's also possible to disable this
> warning.
>

Yeah.  I think disabling it by default may be the better choice.  Thanks.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r258128 - Add -Wexpansion-to-undefined: warn when using `defined` in a macro definition.

2016-01-19 Thread Nico Weber via cfe-commits

I mean you could pass a -Wno flag. It's undefined behavior that's actually
causing bugs in practice; it should probably be on by default.
On Jan 19, 2016 3:38 PM, "Diego Novillo"  wrote:

>
>
> On Tue, Jan 19, 2016 at 3:30 PM, Nico Weber  wrote:
>
>> I'll take a look. If it's urgent it's also possible to disable this
>> warning.
>>
>
> Yeah.  I think disabling it by default may be the better choice.  Thanks.
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r258128 - Add -Wexpansion-to-undefined: warn when using `defined` in a macro definition.

2016-01-19 Thread Diego Novillo via cfe-commits

On Tue, Jan 19, 2016 at 3:43 PM, Nico Weber  wrote:

> I mean you could pass a -Wno flag. It's undefined behavior that's actually
> causing bugs in practice; it should probably be on by default.
>

But then you need to fix all the warnings it's producing in an llvm build.
That is currently blocking several folks (us included).
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r258179 - [OpenMP] Detect implicit map type to report unspecified map type for target enter/exit data directives.

2016-01-19 Thread Samuel Antao via cfe-commits

Author: sfantao
Date: Tue Jan 19 14:40:49 2016
New Revision: 258179

URL: http://llvm.org/viewvc/llvm-project?rev=258179&view=rev
Log:
[OpenMP] Detect implicit map type to report unspecified map type for target 
enter/exit data directives.

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'}}

Patch by Arpith Jacob. Thanks!


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

Modified: cfe/trunk/include/clang/AST/OpenMPClause.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/OpenMPClause.h?rev=258179&r1=258178&r2=258179&view=diff
==
--- cfe/trunk/include/clang/AST/OpenMPClause.h (original)
+++ cfe/trunk/include/clang/AST/OpenMPClause.h Tue Jan 19 14:40:49 2016
@@ -2741,6 +2741,8 @@ class OMPMapClause final : public OMPVar
   OpenMPMapClauseKind MapTypeModifier;
   /// \brief Map type for the 'map' clause.
   OpenMPMapClauseKind MapType;
+  /// \brief Is this an implicit map type or not.
+  bool MapTypeIsImplicit;
   /// \brief Location of the map type.
   SourceLocation MapLoc;
   /// \brief Colon location.
@@ -2771,17 +2773,21 @@ class OMPMapClause final : public OMPVar
   ///
   /// \param MapTypeModifier Map type modifier.
   /// \param MapType Map type.
+  /// \param MapTypeIsImplicit Map type is inferred implicitly.
   /// \param MapLoc Location of the map type.
   /// \param StartLoc Starting location of the clause.
   /// \param EndLoc Ending location of the clause.
   /// \param N Number of the variables in the clause.
   ///
   explicit OMPMapClause(OpenMPMapClauseKind MapTypeModifier,
-OpenMPMapClauseKind MapType, SourceLocation MapLoc,
-SourceLocation StartLoc, SourceLocation LParenLoc,
-SourceLocation EndLoc, unsigned N)
-: OMPVarListClause(OMPC_map, StartLoc, LParenLoc, EndLoc, N),
-  MapTypeModifier(MapTypeModifier), MapType(MapType), MapLoc(MapLoc) {}
+OpenMPMapClauseKind MapType, bool MapTypeIsImplicit,
+SourceLocation MapLoc, SourceLocation StartLoc,
+SourceLocation LParenLoc, SourceLocation EndLoc,
+unsigned N)
+  : OMPVarListClause(OMPC_map, StartLoc, LParenLoc, EndLoc,
+   N),
+MapTypeModifier(MapTypeModifier), MapType(MapType),
+MapTypeIsImplicit(MapTypeIsImplicit), MapLoc(MapLoc) {}
 
   /// \brief Build an empty clause.
   ///
@@ -2790,7 +2796,8 @@ class OMPMapClause final : public OMPVar
   explicit OMPMapClause(unsigned N)
   : OMPVarListClause(OMPC_map, SourceLocation(),
SourceLocation(), SourceLocation(), N),
-MapTypeModifier(OMPC_MAP_unknown), MapType(OMPC_MAP_unknown), MapLoc() 
{}
+MapTypeModifier(OMPC_MAP_unknown), MapType(OMPC_MAP_unknown),
+MapTypeIsImplicit(false), MapLoc() {}
 
 public:
   /// \brief Creates clause with a list of variables \a VL.
@@ -2801,13 +2808,15 @@ public:
   /// \param VL List of references to the variables.
   /// \param TypeModifier Map type modifier.
   /// \param Type Map type.
+  /// \param TypeIsImplicit Map type is inferred implicitly.
   /// \param TypeLoc Location of the map type.
   ///
   static OMPMapClause *Create(const ASTContext &C, SourceLocation StartLoc,
-  SourceLocation LParenLoc,
-  SourceLocation EndLoc, ArrayRef VL,
+  SourceLocation LParenLoc, SourceLocation EndLoc,
+  ArrayRef VL,
   OpenMPMapClauseKind TypeModifier,
-  OpenMPMapClauseKind Type, SourceLocation 
TypeLoc);
+  OpenMPMapClauseKind Type, bool TypeIsImplicit,
+  SourceLocation TypeLoc);
   /// \brief Creates an empty clause with the place for \a N variables.
   ///
   /// \param C AST context.
@@ -2818,6 +2827,13 @@ public:
   /// \brief Fetches mapping kind for the clause.
   OpenMPMapC

1 2 >

1 - 100 of 199 matches

Mail list logo