Re: [PATCH] D17142: SystemZ: Check required features when handling builtins

[Jonas Paulsson via cfe-commits]

jonpa updated this revision to Diff 52026.
jonpa added a comment.

Tests updated to run with -verify and now work as expected.


http://reviews.llvm.org/D17142

Files:
  include/clang/Basic/BuiltinsSystemZ.def
  lib/Basic/Targets.cpp
  test/CodeGen/builtins-systemz-generic0.c
  test/CodeGen/builtins-systemz-generic1.c
  test/CodeGen/builtins-systemz-z13.c

Index: test/CodeGen/builtins-systemz-z13.c
===
--- /dev/null
+++ test/CodeGen/builtins-systemz-z13.c
@@ -0,0 +1,23 @@
+// REQUIRES: systemz-registered-target
+// RUN: %clang_cc1 -target-cpu z13 -triple s390x-ibm-linux -S -emit-llvm %s \
+// RUN: -o - 2>&1 | FileCheck --check-prefix=CHECK-Z13 %s
+
+// Check that vector and transactional-memory intrinsics are handled
+// as expected for z13.
+
+typedef __attribute__((vector_size(16))) char v16i8;
+
+v16i8 f0(v16i8 a, v16i8 b) {
+// CHECK-LABEL: f0
+// CHECK-Z13: call <16 x i8> @llvm.s390.vaq(<16 x i8> %0, <16 x i8> %1)
+// CHECK-Z13-NOT: error: '__builtin_s390_vaq' needs target feature vector
+  v16i8 tmp = __builtin_s390_vaq(a, b);
+  return tmp;
+}
+
+void f1() {
+// CHECK-LABEL: f1
+// CHECK-Z13: call i32 @llvm.s390.tbegin(i8* null, i32 65292)
+// CHECK-Z13-NOT: error: '__builtin_tbegin' needs target feature transactional-execution
+  __builtin_tbegin ((void *)0);
+}
Index: test/CodeGen/builtins-systemz-generic1.c
===
--- /dev/null
+++ test/CodeGen/builtins-systemz-generic1.c
@@ -0,0 +1,9 @@
+// REQUIRES: systemz-registered-target
+// RUN: %clang_cc1 -triple s390x-ibm-linux -S -emit-llvm %s -verify -o -
+
+// Check that using a transactional-memory intrinsic for the generic
+// subtarget (non-z13) aborts with an error message.
+void f1() {
+  __builtin_tbegin ((void *)0); // expected-error {{'__builtin_tbegin' needs target feature transactional-execution}}
+}
+
Index: test/CodeGen/builtins-systemz-generic0.c
===
--- /dev/null
+++ test/CodeGen/builtins-systemz-generic0.c
@@ -0,0 +1,13 @@
+// REQUIRES: systemz-registered-target
+// RUN: %clang_cc1 -triple s390x-ibm-linux -S -emit-llvm %s -verify -o -
+
+// Check that using a vector intrinsic for the generic subtarget
+// (non-z13) aborts with an error message.
+
+typedef __attribute__((vector_size(16))) char v16i8;
+
+v16i8 f0(v16i8 a, v16i8 b) {
+  v16i8 tmp = __builtin_s390_vaq(a, b); // expected-error {{'__builtin_s390_vaq' needs target feature vector}}
+  return tmp;
+}
+
Index: lib/Basic/Targets.cpp
===
--- lib/Basic/Targets.cpp
+++ lib/Basic/Targets.cpp
@@ -6383,6 +6383,8 @@
 const Builtin::Info SystemZTargetInfo::BuiltinInfo[] = {
 #define BUILTIN(ID, TYPE, ATTRS)   \
   { #ID, TYPE, ATTRS, nullptr, ALL_LANGUAGES, nullptr },
+#define TARGET_BUILTIN(ID, TYPE, ATTRS, FEATURE)   \
+  { #ID, TYPE, ATTRS, nullptr, ALL_LANGUAGES, FEATURE },
 #include "clang/Basic/BuiltinsSystemZ.def"
 };
 
Index: include/clang/Basic/BuiltinsSystemZ.def
===
--- include/clang/Basic/BuiltinsSystemZ.def
+++ include/clang/Basic/BuiltinsSystemZ.def
@@ -14,239 +14,244 @@
 
 // The format of this database matches clang/Basic/Builtins.def.
 
+#if defined(BUILTIN) && !defined(TARGET_BUILTIN)
+#   define TARGET_BUILTIN(ID, TYPE, ATTRS, FEATURE) BUILTIN(ID, TYPE, ATTRS)
+#endif
+
 // Transactional-memory intrinsics
-BUILTIN(__builtin_tbegin, "iv*", "j")
-BUILTIN(__builtin_tbegin_nofloat, "iv*", "j")
-BUILTIN(__builtin_tbeginc, "v", "nj")
-BUILTIN(__builtin_tabort, "vi", "r")
-BUILTIN(__builtin_tend, "i", "n")
-BUILTIN(__builtin_tx_nesting_depth, "i", "nc")
-BUILTIN(__builtin_tx_assist, "vi", "n")
-BUILTIN(__builtin_non_tx_store, "vULi*ULi", "")
+TARGET_BUILTIN(__builtin_tbegin, "iv*", "j", "transactional-execution")
+TARGET_BUILTIN(__builtin_tbegin_nofloat, "iv*", "j", "transactional-execution")
+TARGET_BUILTIN(__builtin_tbeginc, "v", "nj", "transactional-execution")
+TARGET_BUILTIN(__builtin_tabort, "vi", "r", "transactional-execution")
+TARGET_BUILTIN(__builtin_tend, "i", "n", "transactional-execution")
+TARGET_BUILTIN(__builtin_tx_nesting_depth, "i", "nc", "transactional-execution")
+TARGET_BUILTIN(__builtin_tx_assist, "vi", "n", "transactional-execution")
+TARGET_BUILTIN(__builtin_non_tx_store, "vULi*ULi", "", "transactional-execution")
 
 // Vector intrinsics.
 // These all map directly to z instructions, except that some variants ending
 // in "s" have a final "int *" that receives the post-instruction CC value.
 
 // Vector support instructions (chapter 21 of the PoP)
-BUILTIN(__builtin_s390_lcbb, "UivC*Ii", "nc")
-BUILTIN(__builtin_s390_vlbb, "V16ScvC*Ii", "")
-BUILTIN(__builtin_s390_vll, "V16ScUivC*", "")
-BUILTIN(__builtin_s390_vstl, "vV16ScUiv*", "")
-BUILTIN(__builtin_s390_vperm, "V16Uc

Re: [PATCH] D18565: Implement an "I'm dtrace, please retain all debug types" option.

[Joerg Sonnenberger via cfe-commits]

On Tue, Mar 29, 2016 at 12:03:46PM -0700, Adrian Prantl via cfe-commits wrote:
> 
> > On Mar 29, 2016, at 12:00 PM, Joerg Sonnenberger  
> > wrote:
> > 
> > On Tue, Mar 29, 2016 at 06:47:24PM +, Adrian Prantl via cfe-commits 
> > wrote:
> >> This code in this patch listens to the driver option -gfull, and lowers it 
> >> to the new cc1 option -debug-retain-types (1).
> >> When -debug-retain-types is present, CGDebugInfo will retain every(2) type 
> >> it creates.
> > 
> > Is there a good reason for calling it -gfull? I would find something
> > -gall-types or -gretain-all-types to make a lot more sense. This should
> > be orthogonal to other options like providing only line tables?
> 
> My thinking was this:
> The driver already supports -gfull, but it doesn’t do anything.
> This patch can be considered a first step towards making -gfull behave as 
> expected.
> Eventually it should emit debug info for *all* types.

-gfull can still be an option group, including -gall-types or whatever.

> > PS: Slightly related side question, do we have any tools for extracting
> > a given list of types for retaining? Either by name or global variable
> > expression.
> 
> Extract them from where? Can you give an example?

Let's take a kernel as example. I want to include certain types for
dtrace-like use with /dev/kmem, but not all the 100KB+ of boring types.
Enough for doing some basic post-mortem debugging even on embedded
systems.

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

Re: [PATCH] D18035: [GCC] PR23529 Mangler part of attrbute abi_tag support

[Dmitry Polukhin via cfe-commits]

DmitryPolukhin added a comment.

Friendly ping, please take a look!


http://reviews.llvm.org/D18035



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

Re: [PATCH] D13126: New static analyzer checker for loss of sign/precision

[Daniel Marjamäki via cfe-commits]

danielmarjamaki updated this revision to Diff 52029.
danielmarjamaki marked 2 inline comments as done.
danielmarjamaki added a comment.

Minor fixes of review comments. Renamed functions to "isNegative" and 
"isGreaterEqual" and return bool. Updated comment. Added testcases for false 
positives I have seen.


http://reviews.llvm.org/D13126

Files:
  lib/StaticAnalyzer/Checkers/CMakeLists.txt
  lib/StaticAnalyzer/Checkers/Checkers.td
  lib/StaticAnalyzer/Checkers/ConversionChecker.cpp
  test/Analysis/conversion.c

Index: test/Analysis/conversion.c
===
--- test/Analysis/conversion.c
+++ test/Analysis/conversion.c
@@ -0,0 +1,125 @@
+// RUN: %clang_cc1 -Wno-conversion -analyze -analyzer-checker=core,alpha.core.Conversion -verify %s
+
+unsigned char U8;
+signed char S8;
+
+void assign(unsigned U, signed S) {
+  if (S < -10)
+U8 = S; // expected-warning {{Loss of sign in implicit conversion}}
+  if (U > 300)
+S8 = U; // expected-warning {{Loss of precision in implicit conversion}}
+  if (S > 10)
+U8 = S;
+  if (U < 200)
+S8 = U;
+}
+
+void init1() {
+  long long A = 1LL << 60;
+  short X = A; // expected-warning {{Loss of precision in implicit conversion}}
+}
+
+void relational(unsigned U, signed S) {
+  if (S > 10) {
+if (U < S) {
+}
+  }
+  if (S < -10) {
+if (U < S) { // expected-warning {{Loss of sign in implicit conversion}}
+}
+  }
+}
+
+void multiplication(unsigned U, signed S) {
+  if (S > 5)
+S = U * S;
+  if (S < -10)
+S = U * S; // expected-warning {{Loss of sign}}
+}
+
+void division(unsigned U, signed S) {
+  if (S > 5)
+S = U / S;
+  if (S < -10)
+S = U / S; // expected-warning {{Loss of sign}}
+}
+
+void dontwarn1(unsigned U, signed S) {
+  U8 = S; // It might be known that S is always 0x00-0xff.
+  S8 = U; // It might be known that U is always 0x00-0xff.
+
+  U8 = -1;  // Explicit conversion.
+  S8 = ~0U; // Explicit conversion.
+  if (U > 300)
+U8 &= U; // No loss of precision since there is &=.
+}
+
+void dontwarn2(unsigned int U) {
+  if (U <= 4294967295) {
+  }
+  if (U <= (2147483647 * 2U + 1U)) {
+  }
+}
+
+void dontwarn3(int X) {
+  S8 = X ? 'a' : 'b';
+}
+
+// don't warn for macros
+#define DOSTUFF ({ unsigned X = 1000; U8 = X; })
+void dontwarn4() {
+  DOSTUFF;
+}
+
+// don't warn for calculations
+// seen some fp. For instance:  c2 = (c2 >= 'A' && c2 <= 'Z') ? c2 - 'A' + 'a' : c2;
+// there is a todo in the checker to handle calculations
+void dontwarn5() {
+  signed S = -32;
+  U8 = S + 10;
+}
+
+
+// false positives..
+
+int isascii(int c);
+void falsePositive1() {
+  char kb2[5];
+  int X = 1000;
+  if (isascii(X)) {
+// FIXME: should not warn here:
+kb2[0] = X; // expected-warning {{Loss of precision}}
+  }
+}
+
+
+typedef struct FILE {} FILE; int getc(FILE *stream);
+# define EOF (-1)
+char reply_string[8192];
+FILE *cin;
+extern int dostuff (void);
+int falsePositive2() {
+  int c, n;
+  int dig;
+  char *cp = reply_string;
+  int pflag = 0;
+  int code;
+
+  for (;;) {
+dig = n = code = 0;
+while ((c = getc(cin)) != '\n') {
+  if (dig < 4 && dostuff())
+code = code * 10 + (c - '0');
+  if (!pflag && code == 227)
+pflag = 1;
+  if (n == 0)
+n = c;
+  if (c == EOF)
+return(4);
+  if (cp < &reply_string[sizeof(reply_string) - 1])
+// FIXME: should not warn here:
+*cp++ = c; // expected-warning {{Loss of precision}}
+}
+  }
+}
+
Index: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/ConversionChecker.cpp
+++ lib/StaticAnalyzer/Checkers/ConversionChecker.cpp
@@ -0,0 +1,183 @@
+//=== ConversionChecker.cpp -*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// Check that there is not loss of sign/precision in assignments, comparisons
+// and multiplications.
+//
+// ConversionChecker uses path sensitive analysis to determine possible values
+// of expressions. A warning is reported when:
+// * a negative value is implicitly converted to an unsigned value in an
+//   assignment, comparison or multiplication.
+// * assignment / initialization when source value is greater than the max
+//   value of target
+//
+// Many compilers and tools have similar checks that are based on semantic
+// analysis. Those checks are sound but has poor precision. ConversionChecker
+// is an alternative to those checks.
+//
+//===--===//
+#include "ClangSACheckers.h"
+#include "clang/AST/ParentMap.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
+#include "clan

Re: [PATCH] D13126: New static analyzer checker for loss of sign/precision

[Daniel Marjamäki via cfe-commits]

danielmarjamaki added a comment.

In http://reviews.llvm.org/D13126#381772, @zaks.anna wrote:

> Could you add the reduced false positives to the tests file?
>
> > As far as I see the diagnostics are showing the proper path now..
>
>
> What do you mean? Does this refer to supplying more information the the path 
> about why the error occurs?


Sorry.. I was too quick. I changed back to "bool".



Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:13
@@ +12,3 @@
+//
+// ConversionChecker uses path sensitive analysis to determine possible values
+// of expressions. A warning is reported when:

ok, hope this is good.


Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:85
@@ +84,3 @@
+
+void ConversionChecker::reportBug(CheckerContext &C, const char Msg[]) const {
+  // Generate an error node.

I renamed to "isNegative" and "isGreaterEqual"


http://reviews.llvm.org/D13126



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

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

[Alexander Droste via cfe-commits]

Alexander_Droste marked 3 inline comments as done.


Comment at: lib/StaticAnalyzer/Checkers/MPI-Checker/MPIBugReporter.cpp:95
@@ +94,3 @@
+  if (Optional SP = N->getLocation().getAs()) {
+if (const CallExpr *CE = clang::dyn_cast(SP->getStmt())) {
+

zaks.anna wrote:
> Would it be possible to identify the "interesting node" by just looking at 
> how the state you track changes. This function will be called when we walk 
> the path upward from the error node. For example, you can notice that in the 
> previous state, the Request did not exist and in this state it is 
> Nonblocking. That would allow you to conclude that between those 2 states a 
> function was called. (This is the trick we use in the other checkers (ex: 
> malloc); it simplifies implementation and makes it more robust - we do not 
> need to pattern match as much.)
I think this should work but would slightly change the way diagnostics are 
presented in a specific case.  If more than 2 nonblocking calls are using a 
request in sequence, the expected note (`Request is previously used by 
nonblocking call here.`) would not point to the previous but to the first 
nonblocking call of that sequence. What do you think; should the `VisitNode` 
function be changed regardless or should it stay as it is because of this 
specific case?


Comment at: lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.cpp:86
@@ +85,3 @@
+  // A wait has no matching nonblocking call.
+  BReporter->reportUnmatchedWait(PreCallEvent, ReqRegion, ErrorNode);
+}

zaks.anna wrote:
> Can you add a test cases for this report when there are multiple ReqRegions? 
> Looks like all of the test cases with MPI_Waitall succeed. (I test a case 
> where one of the regions has a match in Waitall and one does not as well as 
> when several/all do not have a match.)
Sure, I'll add those tests. The case where none of the requests is matched 
should be covered by `missingNonBlockingMultiple`.


Comment at: test/Analysis/MPIChecker.cpp:98
@@ +97,3 @@
+
+  MPI_Request req;
+  MPI_Ireduce(MPI_IN_PLACE, &buf, 1, MPI_DOUBLE, MPI_SUM, 0, MPI_COMM_WORLD, 
&req); // expected-note{{Request is previously used by nonblocking call here.}}

zaks.anna wrote:
> Do you see the notes in the report? I think you should pass 
> "-analyzer-output=text" to see the notes.
If I pass `-analyzer-output=text` I see the notes in the report. But adding 
that flag also produces a lot of 'undesired' output like: `Loop condition is 
true. Entering loop body.`


Comment at: test/Analysis/MPIChecker.cpp:113
@@ +112,3 @@
+  MPI_Ireduce(MPI_IN_PLACE, &buf, 1, MPI_DOUBLE, MPI_SUM, 0, MPI_COMM_WORLD, 
&rs.req); // expected-note{{Request is previously used by nonblocking call 
here.}}
+  MPI_Ireduce(MPI_IN_PLACE, &buf, 1, MPI_DOUBLE, MPI_SUM, 0, MPI_COMM_WORLD, 
&rs.req); // expected-warning{{Double nonblocking on request 'rs.req'.}}
+  MPI_Wait(&rs.req, MPI_STATUS_IGNORE);

zaks.anna wrote:
> Are we allowed to call reduce -> wait -> reduce with the same req? If so, 
> let's test for that.
> 
> Also, can calls to reduce and wait occur in different functions? If so, we 
> need to test cases where they occur in different functions that are all in 
> the same translation unit - all of the implementations are visible to the 
> analyzer. And also test the cases where we pass the request to a function, 
> whose body is not visible to the analyzer, which is the case when the 
> function is defined in another translation unit.
> 
> Are we allowed to call reduce -> wait -> reduce with the same req? If so, 
> let's test for that.
Yes, that's allowed. I'll add a test.

> Also, can calls to reduce and wait occur in different functions?
This is also possible.

> And also test the cases where we pass the request to a function, whose body 
> is not visible to the analyzer, which is the case when the function is 
> defined in another translation unit.
I would then simply create a new pair of `.cpp` and `.h` files in the test 
folder where I define those functions so that the MPI-Checker tests can use 
them. 


http://reviews.llvm.org/D12761



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

Re: [PATCH] D18265: [clang-tidy] New: checker misc-assign-operator-return

[Alexander Kornienko via cfe-commits]

alexfh added a comment.

Adding the original author in case he has something to say here.



Comment at: clang-tidy/misc/AssignOperatorCheck.h:24
@@ +23,3 @@
+///   * Works with move-assign and assign by value.
+///   * Private and deleted operators are ignored.
+class AssignOperatorCheck : public ClangTidyCheck {

Please add:

  /// For the user-facing documentation see:
  /// http://clang.llvm.org/extra/clang-tidy/checks/misc-assign-operator.html


Comment at: clang-tidy/misc/MiscTidyModule.cpp:54
@@ -55,1 +53,3 @@
+CheckFactories.registerCheck(
+"misc-assign-operator");
 CheckFactories.registerCheck(

Check names usually contain hints at the class of problems they detect (e.g. 
"misc-inaccurate-erase" or "misc-macro-repeated-side-effects"), while 
`misc-assign-operator` doesn't serve this purpose and is overall a bit too 
vague (there's nothing wrong with assign operators per se).

I wonder whether a more specific name would be more helpful to the users. I'm 
thinking of something like `misc-assign-operator-conventions`. WDYT?


Comment at: docs/clang-tidy/checks/misc-assign-operator.rst:13
@@ +12,2 @@
+  * Private and deleted operators are ignored.
+  * The operator must always return ``*this``

nit: Please add a trailing period.


http://reviews.llvm.org/D18265



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

Re: [PATCH] D18210: [analyzer] Fix an assertion fail in hash generation.

[Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL264851: [analyzer] Fix an assertion fail in hash generation. 
(authored by xazax).

Changed prior to commit:
  http://reviews.llvm.org/D18210?vs=50819&id=52033#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D18210

Files:
  cfe/trunk/lib/StaticAnalyzer/Core/IssueHash.cpp

Index: cfe/trunk/lib/StaticAnalyzer/Core/IssueHash.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/IssueHash.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/IssueHash.cpp
@@ -132,8 +132,11 @@
 
   StringRef Str = GetNthLineOfFile(SM.getBuffer(L.getFileID(), L),
L.getExpansionLineNumber());
-  unsigned col = Str.find_first_not_of(Whitespaces);
-  col++;
+  StringRef::size_type col = Str.find_first_not_of(Whitespaces);
+  if (col == StringRef::npos)
+col = 1; // The line only contains whitespace.
+  else
+col++;
   SourceLocation StartOfLine =
   SM.translateLineCol(SM.getFileID(L), L.getExpansionLineNumber(), col);
   llvm::MemoryBuffer *Buffer =


Index: cfe/trunk/lib/StaticAnalyzer/Core/IssueHash.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/IssueHash.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/IssueHash.cpp
@@ -132,8 +132,11 @@
 
   StringRef Str = GetNthLineOfFile(SM.getBuffer(L.getFileID(), L),
L.getExpansionLineNumber());
-  unsigned col = Str.find_first_not_of(Whitespaces);
-  col++;
+  StringRef::size_type col = Str.find_first_not_of(Whitespaces);
+  if (col == StringRef::npos)
+col = 1; // The line only contains whitespace.
+  else
+col++;
   SourceLocation StartOfLine =
   SM.translateLineCol(SM.getFileID(L), L.getExpansionLineNumber(), col);
   llvm::MemoryBuffer *Buffer =
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r264851 - [analyzer] Fix an assertion fail in hash generation.

[Gabor Horvath via cfe-commits]

Author: xazax
Date: Wed Mar 30 05:08:59 2016
New Revision: 264851

URL: http://llvm.org/viewvc/llvm-project?rev=264851&view=rev
Log:
[analyzer] Fix an assertion fail in hash generation.

In case the (uniqueing) location of the diagnostic is in a line that only
contains whitespaces there was an assertion fail during issue hash generation.

Unfortunately I am unable to reproduce this error with the built in checkers, 
so no there is no failing test case with this patch. It would be possible to
write a debug checker for that purpuse but it does not worth the effort.

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

Modified:
cfe/trunk/lib/StaticAnalyzer/Core/IssueHash.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Core/IssueHash.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/IssueHash.cpp?rev=264851&r1=264850&r2=264851&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/IssueHash.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/IssueHash.cpp Wed Mar 30 05:08:59 2016
@@ -132,8 +132,11 @@ static std::string NormalizeLine(const S
 
   StringRef Str = GetNthLineOfFile(SM.getBuffer(L.getFileID(), L),
L.getExpansionLineNumber());
-  unsigned col = Str.find_first_not_of(Whitespaces);
-  col++;
+  StringRef::size_type col = Str.find_first_not_of(Whitespaces);
+  if (col == StringRef::npos)
+col = 1; // The line only contains whitespace.
+  else
+col++;
   SourceLocation StartOfLine =
   SM.translateLineCol(SM.getFileID(L), L.getExpansionLineNumber(), col);
   llvm::MemoryBuffer *Buffer =


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

Re: [PATCH] D18243: [ASTMatchers] Existing matcher hasAnyArgument fixed

[Balogh , Ádám via cfe-commits]

baloghadamsoftware added a comment.

Are there any other comments please?


http://reviews.llvm.org/D18243



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

Re: [PATCH] D18243: [ASTMatchers] Existing matcher hasAnyArgument fixed

[Alexander Kornienko via cfe-commits]

alexfh accepted this revision.
alexfh added a reviewer: alexfh.
alexfh added a comment.

No, thank you for the fix!

LG


http://reviews.llvm.org/D18243



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

r264853 - [OPENMP 4.0] Initial support for '#pragma omp declare simd' directive.

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Wed Mar 30 05:43:55 2016
New Revision: 264853

URL: http://llvm.org/viewvc/llvm-project?rev=264853&view=rev
Log:
[OPENMP 4.0] Initial support for '#pragma omp declare simd' directive.

Initial parsing/sema/serialization/deserialization support for '#pragma
omp declare simd' directive.
The 'declare simd' construct can be applied to a function to enable the
creation of one or more versions that can process multiple arguments
using SIMD instructions from a single invocation from a SIMD loop.
If the function has any declarations, then the declare simd construct
for any declaration that has one must be equivalent to the one specified
 for the definition. Otherwise, the result is unspecified.
This pragma can be applied many times to the same declaration.
Internally this pragma is represented as an attribute. But we need special 
processing for this pragma because it must be used before function declaration, 
this directive is applied to.
Differential Revision: http://reviews.llvm.org/D10599

Added:
cfe/trunk/test/OpenMP/declare_simd_ast_print.c
cfe/trunk/test/OpenMP/declare_simd_ast_print.cpp
cfe/trunk/test/OpenMP/declare_simd_messages.cpp
Modified:
cfe/trunk/include/clang/Basic/Attr.td
cfe/trunk/include/clang/Basic/AttrDocs.td
cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/include/clang/Basic/OpenMPKinds.def
cfe/trunk/include/clang/Parse/Parser.h
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Basic/OpenMPKinds.cpp
cfe/trunk/lib/Parse/ParseDecl.cpp
cfe/trunk/lib/Parse/ParseDeclCXX.cpp
cfe/trunk/lib/Parse/ParseOpenMP.cpp
cfe/trunk/lib/Parse/Parser.cpp
cfe/trunk/lib/Sema/SemaOpenMP.cpp

Modified: cfe/trunk/include/clang/Basic/Attr.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=264853&r1=264852&r2=264853&view=diff
==
--- cfe/trunk/include/clang/Basic/Attr.td (original)
+++ cfe/trunk/include/clang/Basic/Attr.td Wed Mar 30 05:43:55 2016
@@ -2244,6 +2244,17 @@ def OMPCaptureNoInit : InheritableAttr {
   let Documentation = [Undocumented];
 }
 
+def OMPDeclareSimdDecl : Attr {
+  let Spellings = [Pragma<"omp", "declare simd">];
+  let Subjects = SubjectList<[Function]>;
+  let SemaHandler = 0;
+  let HasCustomParsing = 1;
+  let Documentation = [OMPDeclareSimdDocs];
+  let AdditionalMembers = [{
+  void printPrettyPragma(raw_ostream &OS, const PrintingPolicy &Policy) const 
{}
+  }];
+}
+
 def InternalLinkage : InheritableAttr {
   let Spellings = [GNU<"internal_linkage">, CXX11<"clang", 
"internal_linkage">];
   let Subjects = SubjectList<[Var, Function, CXXRecord]>;

Modified: cfe/trunk/include/clang/Basic/AttrDocs.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/AttrDocs.td?rev=264853&r1=264852&r2=264853&view=diff
==
--- cfe/trunk/include/clang/Basic/AttrDocs.td (original)
+++ cfe/trunk/include/clang/Basic/AttrDocs.td Wed Mar 30 05:43:55 2016
@@ -1888,6 +1888,41 @@ arguments, with arbitrary offsets.
   }];
 }
 
+def OMPDeclareSimdDocs : Documentation {
+  let Category = DocCatStmt;
+  let Heading = "#pragma omp declare simd";
+  let Content = [{
+The `declare simd` construct can be applied to a function to enable the 
creation
+of one or more versions that can process multiple arguments using SIMD
+instructions from a single invocation in a SIMD loop. The `declare simd`
+directive is a declarative directive. There may be multiple `declare simd`
+directives for a function. The use of a `declare simd` construct on a function
+enables the creation of SIMD versions of the associated function that can be
+used to process multiple arguments from a single invocation from a SIMD loop
+concurrently.
+The syntax of the `declare simd` construct is as follows:
+
+  .. code-block:: c
+
+  #pragma omp declare simd [clause[[,] clause] ...] new-line
+  [#pragma omp declare simd [clause[[,] clause] ...] new-line]
+  [...]
+  function definition or declaration
+
+where clause is one of the following:
+
+  .. code-block:: c
+
+  simdlen(length)
+  linear(argument-list[:constant-linear-step])
+  aligned(argument-list[:alignment])
+  uniform(argument-list)
+  inbranch
+  notinbranch
+
+  }];
+}
+
 def NotTailCalledDocs : Documentation {
   let Category = DocCatFunction;
   let Content = [{

Modified: cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td?rev=264853&r1=264852&r2=264853&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td Wed Mar 30 05:43:55 
2016
@@ -953,6 +953,8 @@ def err_omp_expected_identifier_

Re: [PATCH] D10599: [OPENMP 4.0] Initial support for '#pragma omp declare simd' directive.

[Alexey Bataev via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL264853: [OPENMP 4.0] Initial support for '#pragma omp 
declare simd' directive. (authored by ABataev).

Changed prior to commit:
  http://reviews.llvm.org/D10599?vs=44833&id=52034#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D10599

Files:
  cfe/trunk/include/clang/Basic/Attr.td
  cfe/trunk/include/clang/Basic/AttrDocs.td
  cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
  cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
  cfe/trunk/include/clang/Basic/OpenMPKinds.def
  cfe/trunk/include/clang/Parse/Parser.h
  cfe/trunk/include/clang/Sema/Sema.h
  cfe/trunk/lib/Basic/OpenMPKinds.cpp
  cfe/trunk/lib/Parse/ParseDecl.cpp
  cfe/trunk/lib/Parse/ParseDeclCXX.cpp
  cfe/trunk/lib/Parse/ParseOpenMP.cpp
  cfe/trunk/lib/Parse/Parser.cpp
  cfe/trunk/lib/Sema/SemaOpenMP.cpp
  cfe/trunk/test/OpenMP/declare_simd_ast_print.c
  cfe/trunk/test/OpenMP/declare_simd_ast_print.cpp
  cfe/trunk/test/OpenMP/declare_simd_messages.cpp

Index: cfe/trunk/lib/Parse/ParseDecl.cpp
===
--- cfe/trunk/lib/Parse/ParseDecl.cpp
+++ cfe/trunk/lib/Parse/ParseDecl.cpp
@@ -3654,8 +3654,10 @@
 }
 
 if (Tok.is(tok::annot_pragma_openmp)) {
-  // There may be declared reduction operator inside structure/union.
-  (void)ParseOpenMPDeclarativeDirective(AS_public);
+  // Result can be ignored, because it must be always empty.
+  AccessSpecifier AS = AS_none;
+  ParsedAttributesWithRange Attrs(AttrFactory);
+  (void)ParseOpenMPDeclarativeDirectiveWithExtDecl(AS, Attrs);
   continue;
 }
 
Index: cfe/trunk/lib/Parse/Parser.cpp
===
--- cfe/trunk/lib/Parse/Parser.cpp
+++ cfe/trunk/lib/Parse/Parser.cpp
@@ -659,8 +659,10 @@
   case tok::annot_pragma_opencl_extension:
 HandlePragmaOpenCLExtension();
 return nullptr;
-  case tok::annot_pragma_openmp:
-return ParseOpenMPDeclarativeDirective(/*AS=*/AS_none);
+  case tok::annot_pragma_openmp: {
+AccessSpecifier AS = AS_none;
+return ParseOpenMPDeclarativeDirectiveWithExtDecl(AS, attrs);
+  }
   case tok::annot_pragma_ms_pointers_to_members:
 HandlePragmaMSPointersToMembers();
 return nullptr;
Index: cfe/trunk/lib/Parse/ParseOpenMP.cpp
===
--- cfe/trunk/lib/Parse/ParseOpenMP.cpp
+++ cfe/trunk/lib/Parse/ParseOpenMP.cpp
@@ -65,6 +65,7 @@
   static const unsigned F[][3] = {
 { OMPD_cancellation, OMPD_point, OMPD_cancellation_point },
 { OMPD_declare, OMPD_reduction, OMPD_declare_reduction },
+{ OMPD_declare, OMPD_simd, OMPD_declare_simd },
 { OMPD_target, OMPD_data, OMPD_target_data },
 { OMPD_target, OMPD_enter, OMPD_target_enter },
 { OMPD_target, OMPD_exit, OMPD_target_exit },
@@ -332,8 +333,14 @@
 ///annot_pragma_openmp 'declare' 'reduction' [...]
 ///annot_pragma_openmp_end
 ///
-Parser::DeclGroupPtrTy
-Parser::ParseOpenMPDeclarativeDirective(AccessSpecifier AS) {
+///   declare-simd-directive:
+/// annot_pragma_openmp 'declare simd' { [,]}
+/// annot_pragma_openmp_end
+/// 
+///
+Parser::DeclGroupPtrTy Parser::ParseOpenMPDeclarativeDirectiveWithExtDecl(
+AccessSpecifier &AS, ParsedAttributesWithRange &Attrs,
+DeclSpec::TST TagType, Decl *Tag) {
   assert(Tok.is(tok::annot_pragma_openmp) && "Not an OpenMP directive!");
   ParenBraceBracketBalancer BalancerRAIIObj(*this);
 
@@ -373,6 +380,47 @@
   return Res;
 }
 break;
+  case OMPD_declare_simd: {
+// The syntax is:
+// { #pragma omp declare simd }
+// 
+//
+
+ConsumeToken();
+// The last seen token is annot_pragma_openmp_end - need to check for
+// extra tokens.
+if (Tok.isNot(tok::annot_pragma_openmp_end)) {
+  Diag(Tok, diag::warn_omp_extra_tokens_at_eol)
+  << getOpenMPDirectiveName(OMPD_declare_simd);
+  while (Tok.isNot(tok::annot_pragma_openmp_end))
+ConsumeAnyToken();
+}
+// Skip the last annot_pragma_openmp_end.
+ConsumeToken();
+
+DeclGroupPtrTy Ptr;
+if (Tok.is(tok::annot_pragma_openmp)) {
+  Ptr = ParseOpenMPDeclarativeDirectiveWithExtDecl(AS, Attrs, TagType, Tag);
+} else if (Tok.isNot(tok::r_brace) && !isEofOrEom()) {
+  // Here we expect to see some function declaration.
+  if (AS == AS_none) {
+assert(TagType == DeclSpec::TST_unspecified);
+MaybeParseCXX11Attributes(Attrs);
+MaybeParseMicrosoftAttributes(Attrs);
+ParsingDeclSpec PDS(*this);
+Ptr = ParseExternalDeclaration(Attrs, &PDS);
+  } else {
+Ptr =
+ParseCXXClassMemberDeclarationWithPragmas(AS, Attrs, TagType, Tag);
+  }
+}
+if (!Ptr) {
+  Diag(Loc, diag::err_omp_decl_in_declare_simd);
+  return DeclGroupPtrTy();
+}
+
+return Actions.ActOnOpenMPDeclareSimdDirectiv

Re: [PATCH] D18265: [clang-tidy] New: checker misc-assign-operator-return

[Balogh , Ádám via cfe-commits]

baloghadamsoftware marked an inline comment as done.


Comment at: clang-tidy/misc/MiscTidyModule.cpp:54
@@ -55,1 +53,3 @@
+CheckFactories.registerCheck(
+"misc-assign-operator");
 CheckFactories.registerCheck(

alexfh wrote:
> Check names usually contain hints at the class of problems they detect (e.g. 
> "misc-inaccurate-erase" or "misc-macro-repeated-side-effects"), while 
> `misc-assign-operator` doesn't serve this purpose and is overall a bit too 
> vague (there's nothing wrong with assign operators per se).
> 
> I wonder whether a more specific name would be more helpful to the users. I'm 
> thinking of something like `misc-assign-operator-conventions`. WDYT?
I have no better idea either. Should I rename it or let us wait for other 
potential reviewers proposing another name?


http://reviews.llvm.org/D18265



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

Re: [PATCH] D17861: [OpenCL] Accept __attribute__((nosvm))

[Anastasia Stulova via cfe-commits]

Anastasia accepted this revision.
Anastasia added a comment.

LGTM, except for one tiny comment that can be addressed before committing.



Comment at: test/SemaOpenCL/nosvm.cl:11
@@ +10,3 @@
+// expected-warning@-4 {{'nosvm' attribute is deprecated and ignored in OpenCL 
version 2.0}}
+#endif
+__attribute__((nosvm)) void g(); // expected-warning {{'nosvm' attribute only 
applies to variables}}

Separate with an empty line here.


http://reviews.llvm.org/D17861



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

Re: [PATCH] D18265: [clang-tidy] New: checker misc-assign-operator-return

[Gábor Horváth via cfe-commits]

xazax.hun added a comment.

What about NonIdiomaticAddignOperator or UnchainableAssignOperator?


http://reviews.llvm.org/D18265



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

Re: [PATCH] D18265: [clang-tidy] New: checker misc-assign-operator-return

[Balogh , Ádám via cfe-commits]

baloghadamsoftware added a comment.

Unchainable is not enough: the original checker (which was extended) also 
checks for parameters and other qualifiers such as const or virtual.


http://reviews.llvm.org/D18265



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

Re: [PATCH] D17142: SystemZ: Check required features when handling builtins

[Ulrich Weigand via cfe-commits]

uweigand added a comment.

I don't think we actually need the builtins-systemz-z13.c test, it only 
duplicates tests already done by builtins-systemz.c and 
builtins-systemz-vector.c.  (Note that those tests implicitly verify that there 
is no compiler error by checking the compiler exit code.)

Also, you should be able to combine the two -generic tests into a single one 
(the compiler doesn't abort at errors, so you can check for multiple errors 
with a single -verify run).  Maybe the combined test should be named 
builtins-systemz-error.c ?

LGTM with those changes.


http://reviews.llvm.org/D17142



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

Re: [PATCH] D18265: [clang-tidy] New: checker misc-assign-operator-return

[Alexander Kornienko via cfe-commits]

alexfh added a comment.

> What about NonIdiomaticAddignOperator or UnchainableAssignOperator?


Yep, "unchainable" doesn't cover all problems the check detects. 
`misc-non-idiomatic-assign-operator` seems good though. I'd wait for the 
original author to chime in before making the change.


http://reviews.llvm.org/D18265



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

r264855 - [ASTMatchers] Existing matcher hasAnyArgument fixed

[Gabor Horvath via cfe-commits]

Author: xazax
Date: Wed Mar 30 06:22:14 2016
New Revision: 264855

URL: http://llvm.org/viewvc/llvm-project?rev=264855&view=rev
Log:
[ASTMatchers] Existing matcher hasAnyArgument fixed

Summary: A checker (will be uploaded after this patch) needs to check implicit 
casts. The checker needs matcher hasAnyArgument but it ignores implicit casts 
and parenthesized expressions which disables checking of implicit casts for 
arguments in the checker. However the documentation of the matcher contains a 
FIXME that this should be removed once separate matchers for ignoring implicit 
casts and parenthesized expressions are ready. Since these matchers were 
already there the fix could be executed. Only one Clang checker was affected 
which was also fixed (ignoreParenImpCasts added) and is separately uploaded. 
Third party checkers (not in the Clang repository) may be affected by this fix 
so the fix must be emphasized in the release notes.

Reviewers: klimek, sbenza, alexfh

Subscribers: alexfh, klimek, xazax.hun, cfe-commits

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

Modified:
cfe/trunk/docs/LibASTMatchersReference.html
cfe/trunk/docs/ReleaseNotes.rst
cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.cpp

Modified: cfe/trunk/docs/LibASTMatchersReference.html
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/LibASTMatchersReference.html?rev=264855&r1=264854&r2=264855&view=diff
==
--- cfe/trunk/docs/LibASTMatchersReference.html (original)
+++ cfe/trunk/docs/LibASTMatchersReference.html Wed Mar 30 06:22:14 2016
@@ -3636,11 +3636,6 @@ callExpr(hasAnyArgument(declRefExpr()))
   matches x(1, y, 42)
 with hasAnyArgument(...)
   matching y
-
-FIXME: Currently this will ignore parentheses and implicit casts on
-the argument before applying the inner matcher. We'll want to remove
-this to allow for greater control by the user once ignoreImplicit()
-has been implemented.
 
 
 
@@ -3907,11 +3902,6 @@ callExpr(hasAnyArgument(declRefExpr()))
   matches x(1, y, 42)
 with hasAnyArgument(...)
   matching y
-
-FIXME: Currently this will ignore parentheses and implicit casts on
-the argument before applying the inner matcher. We'll want to remove
-this to allow for greater control by the user once ignoreImplicit()
-has been implemented.
 
 
 

Modified: cfe/trunk/docs/ReleaseNotes.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ReleaseNotes.rst?rev=264855&r1=264854&r2=264855&view=diff
==
--- cfe/trunk/docs/ReleaseNotes.rst (original)
+++ cfe/trunk/docs/ReleaseNotes.rst Wed Mar 30 06:22:14 2016
@@ -120,6 +120,12 @@ this section should help get you past th
 AST Matchers
 
 
+- hasAnyArgument: Matcher no longer ignores parentheses and implicit casts on
+  the argument before applying the inner matcher. The fix was done to allow for
+  greater control by the user. In all existing checkers that use this matcher
+  all instances of code ``hasAnyArgument()`` must be changed to
+  ``hasAnyArgument(ignoringParenImpCasts())``.
+
 ...
 
 libclang

Modified: cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h?rev=264855&r1=264854&r2=264855&view=diff
==
--- cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h (original)
+++ cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h Wed Mar 30 06:22:14 2016
@@ -2989,18 +2989,13 @@ AST_MATCHER(CXXCtorInitializer, isMember
 ///   matches x(1, y, 42)
 /// with hasAnyArgument(...)
 ///   matching y
-///
-/// FIXME: Currently this will ignore parentheses and implicit casts on
-/// the argument before applying the inner matcher. We'll want to remove
-/// this to allow for greater control by the user once \c ignoreImplicit()
-/// has been implemented.
 AST_POLYMORPHIC_MATCHER_P(hasAnyArgument,
   AST_POLYMORPHIC_SUPPORTED_TYPES(CallExpr,
   CXXConstructExpr),
   internal::Matcher, InnerMatcher) {
   for (const Expr *Arg : Node.arguments()) {
 BoundNodesTreeBuilder Result(*Builder);
-if (InnerMatcher.matches(*Arg->IgnoreParenImpCasts(), Finder, &Result)) {
+if (InnerMatcher.matches(*Arg, Finder, &Result)) {
   *Builder = std::move(Result);
   return true;
 }

Modified: cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.cpp?rev=264855&r1=264854&r2=264855&view=diff
==
--- cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.cpp (original)
+++ cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.cpp Wed Mar 30 06:22:14 201

Re: [PATCH] D18408: readability check for const params in declarations

[Alexander Kornienko via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL264856: [clang-tidy] readability check for const params in 
declarations (authored by alexfh).

Changed prior to commit:
  http://reviews.llvm.org/D18408?vs=51954&id=52037#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D18408

Files:
  clang-tools-extra/trunk/clang-tidy/readability/AvoidConstParamsInDecls.cpp
  clang-tools-extra/trunk/clang-tidy/readability/AvoidConstParamsInDecls.h
  clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt
  clang-tools-extra/trunk/clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/trunk/docs/clang-tidy/checks/readability-avoid-const-params-in-decls.rst
  
clang-tools-extra/trunk/test/clang-tidy/readability-avoid-const-params-in-decls.cpp

Index: clang-tools-extra/trunk/clang-tidy/readability/ReadabilityTidyModule.cpp
===
--- clang-tools-extra/trunk/clang-tidy/readability/ReadabilityTidyModule.cpp
+++ clang-tools-extra/trunk/clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -10,6 +10,7 @@
 #include "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
+#include "AvoidConstParamsInDecls.h"
 #include "BracesAroundStatementsCheck.h"
 #include "ContainerSizeEmptyCheck.h"
 #include "ElseAfterReturnCheck.h"
@@ -32,6 +33,8 @@
 class ReadabilityModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
+CheckFactories.registerCheck(
+"readability-avoid-const-params-in-decls");
 CheckFactories.registerCheck(
 "readability-braces-around-statements");
 CheckFactories.registerCheck(
Index: clang-tools-extra/trunk/clang-tidy/readability/AvoidConstParamsInDecls.h
===
--- clang-tools-extra/trunk/clang-tidy/readability/AvoidConstParamsInDecls.h
+++ clang-tools-extra/trunk/clang-tidy/readability/AvoidConstParamsInDecls.h
@@ -0,0 +1,33 @@
+//===--- AvoidConstParamsInDecls.h - clang-tidy===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_AVOID_CONST_PARAMS_IN_DECLS_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_AVOID_CONST_PARAMS_IN_DECLS_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+// Detect function declarations that have const value parameters and discourage
+// them.
+class AvoidConstParamsInDecls : public ClangTidyCheck {
+public:
+  using ClangTidyCheck::ClangTidyCheck;
+
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_AVOID_CONST_PARAMS_IN_DECLS_H
Index: clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt
===
--- clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt
+++ clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt
@@ -1,6 +1,7 @@
 set(LLVM_LINK_COMPONENTS support)
 
 add_clang_library(clangTidyReadabilityModule
+  AvoidConstParamsInDecls.cpp
   BracesAroundStatementsCheck.cpp
   ContainerSizeEmptyCheck.cpp
   ElseAfterReturnCheck.cpp
Index: clang-tools-extra/trunk/clang-tidy/readability/AvoidConstParamsInDecls.cpp
===
--- clang-tools-extra/trunk/clang-tidy/readability/AvoidConstParamsInDecls.cpp
+++ clang-tools-extra/trunk/clang-tidy/readability/AvoidConstParamsInDecls.cpp
@@ -0,0 +1,71 @@
+//===--- AvoidConstParamsInDecls.cpp - clang-tidy--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "AvoidConstParamsInDecls.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace readability {
+namespace {
+
+SourceRange getTypeRange(const ParmVarDecl &Param) {
+  if (Param.getIdentifier() != nullptr)
+return SourceRange(Param.getLocStart(),
+   Param.getLocEnd().getLocWithOffset(-1));
+  return Param.getSourceRange();
+}
+
+} // namespace
+
+
+void AvoidConstParamsInDecls::regist

[clang-tools-extra] r264856 - [clang-tidy] readability check for const params in declarations

[Alexander Kornienko via cfe-commits]

Author: alexfh
Date: Wed Mar 30 06:31:33 2016
New Revision: 264856

URL: http://llvm.org/viewvc/llvm-project?rev=264856&view=rev
Log:
[clang-tidy] readability check for const params in declarations

Summary: Adds a clang-tidy warning for top-level consts in function 
declarations.

Reviewers: hokein, sbenza, alexfh

Subscribers: cfe-commits

Patch by Matt Kulukundis!

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

Added:
clang-tools-extra/trunk/clang-tidy/readability/AvoidConstParamsInDecls.cpp
clang-tools-extra/trunk/clang-tidy/readability/AvoidConstParamsInDecls.h

clang-tools-extra/trunk/docs/clang-tidy/checks/readability-avoid-const-params-in-decls.rst

clang-tools-extra/trunk/test/clang-tidy/readability-avoid-const-params-in-decls.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/readability/ReadabilityTidyModule.cpp
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst

Added: 
clang-tools-extra/trunk/clang-tidy/readability/AvoidConstParamsInDecls.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/AvoidConstParamsInDecls.cpp?rev=264856&view=auto
==
--- clang-tools-extra/trunk/clang-tidy/readability/AvoidConstParamsInDecls.cpp 
(added)
+++ clang-tools-extra/trunk/clang-tidy/readability/AvoidConstParamsInDecls.cpp 
Wed Mar 30 06:31:33 2016
@@ -0,0 +1,71 @@
+//===--- AvoidConstParamsInDecls.cpp - 
clang-tidy--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "AvoidConstParamsInDecls.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace readability {
+namespace {
+
+SourceRange getTypeRange(const ParmVarDecl &Param) {
+  if (Param.getIdentifier() != nullptr)
+return SourceRange(Param.getLocStart(),
+   Param.getLocEnd().getLocWithOffset(-1));
+  return Param.getSourceRange();
+}
+
+} // namespace
+
+
+void AvoidConstParamsInDecls::registerMatchers(MatchFinder *Finder) {
+  const auto ConstParamDecl =
+  parmVarDecl(hasType(qualType(isConstQualified(.bind("param");
+  Finder->addMatcher(functionDecl(unless(isDefinition()),
+  has(typeLoc(forEach(ConstParamDecl
+ .bind("func"),
+ this);
+}
+
+void AvoidConstParamsInDecls::check(const MatchFinder::MatchResult &Result) {
+  const auto *Func = Result.Nodes.getNodeAs("func");
+  const auto *Param = Result.Nodes.getNodeAs("param");
+
+  QualType Type = Param->getType();
+  if (!Type.isLocalConstQualified())
+return;
+
+  Type.removeLocalConst();
+
+  auto Diag = diag(Param->getLocStart(),
+   "parameter %0 is const-qualified in the function "
+   "declaration; const-qualification of parameters only has an 
"
+   "effect in function definitions");
+  if (Param->getName().empty()) {
+for (unsigned int i = 0; i < Func->getNumParams(); ++i) {
+  if (Param == Func->getParamDecl(i)) {
+Diag << (i + 1);
+break;
+  }
+}
+  } else {
+Diag << Param;
+  }
+  Diag << FixItHint::CreateReplacement(getTypeRange(*Param),
+   Type.getAsString());
+}
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang

Added: clang-tools-extra/trunk/clang-tidy/readability/AvoidConstParamsInDecls.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/AvoidConstParamsInDecls.h?rev=264856&view=auto
==
--- clang-tools-extra/trunk/clang-tidy/readability/AvoidConstParamsInDecls.h 
(added)
+++ clang-tools-extra/trunk/clang-tidy/readability/AvoidConstParamsInDecls.h 
Wed Mar 30 06:31:33 2016
@@ -0,0 +1,33 @@
+//===--- AvoidConstParamsInDecls.h - 
clang-tidy===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef 
LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_AVOID_CONST_PARAMS_IN_DECLS_H
+#define 
LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_AVOID_CONST_PARAMS_IN_DECLS_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+// Detect function declarations that have const value parameters and discourage
+// them.
+class AvoidC

Re: [PATCH] D18408: readability check for const params in declarations

[Alexander Kornienko via cfe-commits]

alexfh added a comment.

Actually, we missed one thing: may I ask you to update docs/ReleaseNotes.rst 
with a short description of the new check?


Repository:
  rL LLVM

http://reviews.llvm.org/D18408



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

[clang-tools-extra] r264858 - [docs] Added 3.8 clang-tidy release notes, fixed formatting.

[Alexander Kornienko via cfe-commits]

Author: alexfh
Date: Wed Mar 30 07:05:33 2016
New Revision: 264858

URL: http://llvm.org/viewvc/llvm-project?rev=264858&view=rev
Log:
[docs] Added 3.8 clang-tidy release notes, fixed formatting.

Modified:
clang-tools-extra/trunk/docs/ReleaseNotes.rst

Modified: clang-tools-extra/trunk/docs/ReleaseNotes.rst
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/ReleaseNotes.rst?rev=264858&r1=264857&r2=264858&view=diff
==
--- clang-tools-extra/trunk/docs/ReleaseNotes.rst (original)
+++ clang-tools-extra/trunk/docs/ReleaseNotes.rst Wed Mar 30 07:05:33 2016
@@ -4,7 +4,7 @@ Extra Clang Tools 3.9 (In-Progress) Rele
 
 .. contents::
:local:
-   :depth: 2
+   :depth: 3
 
 Written by the `LLVM Team `_
 
@@ -46,20 +46,20 @@ Major New Features
 
 - Feature1...
 
-Improvements to ``clang-query``
-^^^
+Improvements to clang-query
+---
 
 The improvements are...
 
-Improvements to ``clang-rename``
-
+Improvements to clang-rename
+
 
 The improvements are...
 
-Improvements to ``clang-tidy``
-^^
+Improvements to clang-tidy
+--
 
-``clang-tidy``'s checks are constantly being improved to catch more issues,
+:program:`clang-tidy`'s checks are constantly being improved to catch more 
issues,
 explain them more clearly, and provide more accurate fix-its for the issues
 identified.  The improvements since the 3.8 release include:
 
@@ -68,7 +68,99 @@ identified.  The improvements since the
   This check selectively replaces string literals containing escaped
   characters with raw string literals.
 
-Improvements to ``modularize``
-^^
+Clang-tidy changes from 3.7 to 3.8
+^^
+
+The 3.8 release didn't include release notes for :program:`clang-tidy`. In the
+3.8 release many new checks have been added to :program:`clang-tidy`:
+
+- Checks enforcing certain rules of the `CERT Secure Coding Standards
+  
`_:
+
+  * `cert-dcl03-c
+
`_
+(an alias to the pre-existing check `misc-static-assert
+
`_)
+  * `cert-dcl50-cpp
+
`_
+  * `cert-err52-cpp
+
`_
+  * `cert-err58-cpp
+
`_
+  * `cert-err60-cpp
+
`_
+  * `cert-err61-cpp
+
`_
+  * `cert-fio38-c
+
`_
+(an alias to the pre-existing check `misc-non-copyable-objects
+
`_)
+  * `cert-oop11-cpp
+
`_
+(an alias to the pre-existing check `misc-move-constructor-init
+
`_)
+
+- Checks supporting the `C++ Core Guidelines
+  
`_:
+
+  * `cppcoreguidelines-pro-bounds-array-to-pointer-decay
+
`_
+  * `cppcoreguidelines-pro-bounds-constant-array-index
+
`_
+  * `cppcoreguidelines-pro-bounds-pointer-arithmetic
+
`_
+  * `cppcoreguidelines-pro-type-const-cast
+
`_
+  * `cppcoreguidelines-pro-type-cstyle-cast
+
`_
+  * `cppcoreguidelines-pro-type-reinterpret-cast
+

Re: [clang-tools-extra] r264856 - [clang-tidy] readability check for const params in declarations

[Nico Weber via cfe-commits]

This doesn't build on Windows:

http://lab.llvm.org:8011/builders/clang-x64-ninja-win7/builds/11018/steps/build%20stage%201/logs/stdio

d:\buildslave\clang-x64-ninja-win7\llvm\tools\clang\tools\extra\clang-tidy\readability\../ClangTidyModule.h(61)
: error C2660:
'clang::tidy::readability::AvoidConstParamsInDecls::AvoidConstParamsInDecls'
: function does not take 2 arguments

D:\buildslave\clang-x64-ninja-win7\llvm\tools\clang\tools\extra\clang-tidy\readability\ReadabilityTidyModule.cpp(37)
: see reference to function template instantiation 'void
clang::tidy::ClangTidyCheckFactories::registerCheck(llvm::StringRef)'
being compiled

Can you take a look?


On Wed, Mar 30, 2016 at 4:31 AM, Alexander Kornienko via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: alexfh
> Date: Wed Mar 30 06:31:33 2016
> New Revision: 264856
>
> URL: http://llvm.org/viewvc/llvm-project?rev=264856&view=rev
> Log:
> [clang-tidy] readability check for const params in declarations
>
> Summary: Adds a clang-tidy warning for top-level consts in function
> declarations.
>
> Reviewers: hokein, sbenza, alexfh
>
> Subscribers: cfe-commits
>
> Patch by Matt Kulukundis!
>
> Differential Revision: http://reviews.llvm.org/D18408
>
> Added:
>
> clang-tools-extra/trunk/clang-tidy/readability/AvoidConstParamsInDecls.cpp
>
> clang-tools-extra/trunk/clang-tidy/readability/AvoidConstParamsInDecls.h
>
> clang-tools-extra/trunk/docs/clang-tidy/checks/readability-avoid-const-params-in-decls.rst
>
> clang-tools-extra/trunk/test/clang-tidy/readability-avoid-const-params-in-decls.cpp
> Modified:
> clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt
>
> clang-tools-extra/trunk/clang-tidy/readability/ReadabilityTidyModule.cpp
> clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
>
> Added:
> clang-tools-extra/trunk/clang-tidy/readability/AvoidConstParamsInDecls.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/AvoidConstParamsInDecls.cpp?rev=264856&view=auto
>
> ==
> ---
> clang-tools-extra/trunk/clang-tidy/readability/AvoidConstParamsInDecls.cpp
> (added)
> +++
> clang-tools-extra/trunk/clang-tidy/readability/AvoidConstParamsInDecls.cpp
> Wed Mar 30 06:31:33 2016
> @@ -0,0 +1,71 @@
> +//===--- AvoidConstParamsInDecls.cpp -
> clang-tidy--===//
> +//
> +// The LLVM Compiler Infrastructure
> +//
> +// This file is distributed under the University of Illinois Open Source
> +// License. See LICENSE.TXT for details.
> +//
>
> +//===--===//
> +
> +#include "AvoidConstParamsInDecls.h"
> +#include "clang/ASTMatchers/ASTMatchFinder.h"
> +#include "clang/ASTMatchers/ASTMatchers.h"
> +#include "clang/Lex/Lexer.h"
> +
> +using namespace clang::ast_matchers;
> +
> +namespace clang {
> +namespace tidy {
> +namespace readability {
> +namespace {
> +
> +SourceRange getTypeRange(const ParmVarDecl &Param) {
> +  if (Param.getIdentifier() != nullptr)
> +return SourceRange(Param.getLocStart(),
> +   Param.getLocEnd().getLocWithOffset(-1));
> +  return Param.getSourceRange();
> +}
> +
> +} // namespace
> +
> +
> +void AvoidConstParamsInDecls::registerMatchers(MatchFinder *Finder) {
> +  const auto ConstParamDecl =
> +  parmVarDecl(hasType(qualType(isConstQualified(.bind("param");
> +  Finder->addMatcher(functionDecl(unless(isDefinition()),
> +  has(typeLoc(forEach(ConstParamDecl
> + .bind("func"),
> + this);
> +}
> +
> +void AvoidConstParamsInDecls::check(const MatchFinder::MatchResult
> &Result) {
> +  const auto *Func = Result.Nodes.getNodeAs("func");
> +  const auto *Param = Result.Nodes.getNodeAs("param");
> +
> +  QualType Type = Param->getType();
> +  if (!Type.isLocalConstQualified())
> +return;
> +
> +  Type.removeLocalConst();
> +
> +  auto Diag = diag(Param->getLocStart(),
> +   "parameter %0 is const-qualified in the function "
> +   "declaration; const-qualification of parameters only
> has an "
> +   "effect in function definitions");
> +  if (Param->getName().empty()) {
> +for (unsigned int i = 0; i < Func->getNumParams(); ++i) {
> +  if (Param == Func->getParamDecl(i)) {
> +Diag << (i + 1);
> +break;
> +  }
> +}
> +  } else {
> +Diag << Param;
> +  }
> +  Diag << FixItHint::CreateReplacement(getTypeRange(*Param),
> +   Type.getAsString());
> +}
> +
> +} // namespace readability
> +} // namespace tidy
> +} // namespace clang
>
> Added:
> clang-tools-extra/trunk/clang-tidy/readability/AvoidConstParamsInDecls.h
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/AvoidConstParamsInDecls.h?rev=264856&view=auto
>
> =

[clang-tools-extra] r264859 - [clang-tidy] Adjust dangling references check to ASTMatcher changes.

[Gabor Horvath via cfe-commits]

Author: xazax
Date: Wed Mar 30 07:16:09 2016
New Revision: 264859

URL: http://llvm.org/viewvc/llvm-project?rev=264859&view=rev
Log:
[clang-tidy] Adjust dangling references check to ASTMatcher changes.

Modified:
clang-tools-extra/trunk/clang-tidy/misc/DanglingHandleCheck.cpp

Modified: clang-tools-extra/trunk/clang-tidy/misc/DanglingHandleCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/DanglingHandleCheck.cpp?rev=264859&r1=264858&r2=264859&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/misc/DanglingHandleCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/misc/DanglingHandleCheck.cpp Wed Mar 30 
07:16:09 2016
@@ -33,9 +33,9 @@ std::vector parseClasses(St
   return Result;
 }
 
-ast_matchers::internal::BindableMatcher handleFrom(
-ast_matchers::internal::Matcher IsAHandle,
-ast_matchers::internal::Matcher Arg) {
+ast_matchers::internal::BindableMatcher
+handleFrom(ast_matchers::internal::Matcher IsAHandle,
+   ast_matchers::internal::Matcher Arg) {
   return cxxConstructExpr(hasDeclaration(cxxMethodDecl(ofClass(IsAHandle))),
   hasArgument(0, Arg));
 }
@@ -76,7 +76,8 @@ makeContainerMatcher(ast_matchers::inter
   //  - map's insert: This requires detecting that the pair conversion triggers
   //  the bug. A little more complicated than what we have now.
   return callExpr(
-  hasAnyArgument(handleFromTemporaryValue(IsAHandle)),
+  hasAnyArgument(
+  ignoringParenImpCasts(handleFromTemporaryValue(IsAHandle))),
   anyOf(
   // For sequences: assign, push_back, resize.
   cxxMemberCallExpr(
@@ -91,7 +92,7 @@ makeContainerMatcher(ast_matchers::inter
   hasOverloadedOperatorName("[]";
 }
 
-}  // anonymous namespace
+} // anonymous namespace
 
 DanglingHandleCheck::DanglingHandleCheck(StringRef Name,
  ClangTidyContext *Context)
@@ -109,7 +110,7 @@ void DanglingHandleCheck::storeOptions(C
HandleClassesDelimiter));
 }
 
-void DanglingHandleCheck::registerMatchersForVariables(MatchFinder* Finder) {
+void DanglingHandleCheck::registerMatchersForVariables(MatchFinder *Finder) {
   const auto ConvertedHandle = handleFromTemporaryValue(IsAHandle);
 
   // Find 'Handle foo(ReturnsAValue());'
@@ -138,7 +139,7 @@ void DanglingHandleCheck::registerMatche
   Finder->addMatcher(makeContainerMatcher(IsAHandle).bind("bad_stmt"), this);
 }
 
-void DanglingHandleCheck::registerMatchersForReturn(MatchFinder* Finder) {
+void DanglingHandleCheck::registerMatchersForReturn(MatchFinder *Finder) {
   // Return a local.
   Finder->addMatcher(
   returnStmt(
@@ -168,12 +169,12 @@ void DanglingHandleCheck::registerMatche
   this);
 }
 
-void DanglingHandleCheck::registerMatchers(MatchFinder* Finder) {
+void DanglingHandleCheck::registerMatchers(MatchFinder *Finder) {
   registerMatchersForVariables(Finder);
   registerMatchersForReturn(Finder);
 }
 
-void DanglingHandleCheck::check(const MatchFinder::MatchResult& Result) {
+void DanglingHandleCheck::check(const MatchFinder::MatchResult &Result) {
   auto *Handle = Result.Nodes.getNodeAs("handle");
   diag(Result.Nodes.getNodeAs("bad_stmt")->getLocStart(),
"%0 outlives its value")


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

Re: [PATCH] D18582: [Clang-tidy] Update release notes with list of checks added since 3.8

[Alexander Kornienko via cfe-commits]

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

Looks good with a nit. Thank you!



Comment at: docs/ReleaseNotes.rst:129
@@ +128,3 @@
+
+  This check looks for procedures (functions returning no value) with `return`
+  statements at the end of the function.  Such `return` statements are

nit: Double backquotes should be used for inline code snippets.


Repository:
  rL LLVM

http://reviews.llvm.org/D18582



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

Re: r264855 - [ASTMatchers] Existing matcher hasAnyArgument fixed

[Nico Weber via cfe-commits]

Looks like this broke clang-tidy/misc-dangling-handle.cpp (
http://lab.llvm.org:8011/builders/clang-bpf-build/builds/9704). Can you
take a look?

On Wed, Mar 30, 2016 at 4:22 AM, Gabor Horvath via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: xazax
> Date: Wed Mar 30 06:22:14 2016
> New Revision: 264855
>
> URL: http://llvm.org/viewvc/llvm-project?rev=264855&view=rev
> Log:
> [ASTMatchers] Existing matcher hasAnyArgument fixed
>
> Summary: A checker (will be uploaded after this patch) needs to check
> implicit casts. The checker needs matcher hasAnyArgument but it ignores
> implicit casts and parenthesized expressions which disables checking of
> implicit casts for arguments in the checker. However the documentation of
> the matcher contains a FIXME that this should be removed once separate
> matchers for ignoring implicit casts and parenthesized expressions are
> ready. Since these matchers were already there the fix could be executed.
> Only one Clang checker was affected which was also fixed
> (ignoreParenImpCasts added) and is separately uploaded. Third party
> checkers (not in the Clang repository) may be affected by this fix so the
> fix must be emphasized in the release notes.
>
> Reviewers: klimek, sbenza, alexfh
>
> Subscribers: alexfh, klimek, xazax.hun, cfe-commits
>
> Differential Revision: http://reviews.llvm.org/D18243
>
> Modified:
> cfe/trunk/docs/LibASTMatchersReference.html
> cfe/trunk/docs/ReleaseNotes.rst
> cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
> cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.cpp
>
> Modified: cfe/trunk/docs/LibASTMatchersReference.html
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/LibASTMatchersReference.html?rev=264855&r1=264854&r2=264855&view=diff
>
> ==
> --- cfe/trunk/docs/LibASTMatchersReference.html (original)
> +++ cfe/trunk/docs/LibASTMatchersReference.html Wed Mar 30 06:22:14 2016
> @@ -3636,11 +3636,6 @@ callExpr(hasAnyArgument(declRefExpr()))
>matches x(1, y, 42)
>  with hasAnyArgument(...)
>matching y
> -
> -FIXME: Currently this will ignore parentheses and implicit casts on
> -the argument before applying the inner matcher. We'll want to remove
> -this to allow for greater control by the user once ignoreImplicit()
> -has been implemented.
>  
>
>
> @@ -3907,11 +3902,6 @@ callExpr(hasAnyArgument(declRefExpr()))
>matches x(1, y, 42)
>  with hasAnyArgument(...)
>matching y
> -
> -FIXME: Currently this will ignore parentheses and implicit casts on
> -the argument before applying the inner matcher. We'll want to remove
> -this to allow for greater control by the user once ignoreImplicit()
> -has been implemented.
>  
>
>
>
> Modified: cfe/trunk/docs/ReleaseNotes.rst
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ReleaseNotes.rst?rev=264855&r1=264854&r2=264855&view=diff
>
> ==
> --- cfe/trunk/docs/ReleaseNotes.rst (original)
> +++ cfe/trunk/docs/ReleaseNotes.rst Wed Mar 30 06:22:14 2016
> @@ -120,6 +120,12 @@ this section should help get you past th
>  AST Matchers
>  
>
> +- hasAnyArgument: Matcher no longer ignores parentheses and implicit
> casts on
> +  the argument before applying the inner matcher. The fix was done to
> allow for
> +  greater control by the user. In all existing checkers that use this
> matcher
> +  all instances of code ``hasAnyArgument()`` must be
> changed to
> +  ``hasAnyArgument(ignoringParenImpCasts())``.
> +
>  ...
>
>  libclang
>
> Modified: cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h?rev=264855&r1=264854&r2=264855&view=diff
>
> ==
> --- cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h (original)
> +++ cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h Wed Mar 30 06:22:14
> 2016
> @@ -2989,18 +2989,13 @@ AST_MATCHER(CXXCtorInitializer, isMember
>  ///   matches x(1, y, 42)
>  /// with hasAnyArgument(...)
>  ///   matching y
> -///
> -/// FIXME: Currently this will ignore parentheses and implicit casts on
> -/// the argument before applying the inner matcher. We'll want to remove
> -/// this to allow for greater control by the user once \c ignoreImplicit()
> -/// has been implemented.
>  AST_POLYMORPHIC_MATCHER_P(hasAnyArgument,
>AST_POLYMORPHIC_SUPPORTED_TYPES(CallExpr,
>
>  CXXConstructExpr),
>internal::Matcher, InnerMatcher) {
>for (const Expr *Arg : Node.arguments()) {
>  BoundNodesTreeBuilder Result(*Builder);
> -if (InnerMatcher.matches(*Arg->IgnoreParenImpCasts(), Finder,
> &Result)) {
> +if (InnerMatcher.matches(*Arg, Finder, &Result)) {
>*Builder = std::move(Result);
>return true;
>  }
>
> Modified: cfe/trunk/unitt

Re: r264855 - [ASTMatchers] Existing matcher hasAnyArgument fixed

[Gábor Horváth via cfe-commits]

Sure, this should be fixed in http://reviews.llvm.org/rL264859

On 30 March 2016 at 14:24, Nico Weber  wrote:

> Looks like this broke clang-tidy/misc-dangling-handle.cpp (
> http://lab.llvm.org:8011/builders/clang-bpf-build/builds/9704). Can you
> take a look?
>
> On Wed, Mar 30, 2016 at 4:22 AM, Gabor Horvath via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: xazax
>> Date: Wed Mar 30 06:22:14 2016
>> New Revision: 264855
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=264855&view=rev
>> Log:
>> [ASTMatchers] Existing matcher hasAnyArgument fixed
>>
>> Summary: A checker (will be uploaded after this patch) needs to check
>> implicit casts. The checker needs matcher hasAnyArgument but it ignores
>> implicit casts and parenthesized expressions which disables checking of
>> implicit casts for arguments in the checker. However the documentation of
>> the matcher contains a FIXME that this should be removed once separate
>> matchers for ignoring implicit casts and parenthesized expressions are
>> ready. Since these matchers were already there the fix could be executed.
>> Only one Clang checker was affected which was also fixed
>> (ignoreParenImpCasts added) and is separately uploaded. Third party
>> checkers (not in the Clang repository) may be affected by this fix so the
>> fix must be emphasized in the release notes.
>>
>> Reviewers: klimek, sbenza, alexfh
>>
>> Subscribers: alexfh, klimek, xazax.hun, cfe-commits
>>
>> Differential Revision: http://reviews.llvm.org/D18243
>>
>> Modified:
>> cfe/trunk/docs/LibASTMatchersReference.html
>> cfe/trunk/docs/ReleaseNotes.rst
>> cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
>> cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.cpp
>>
>> Modified: cfe/trunk/docs/LibASTMatchersReference.html
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/LibASTMatchersReference.html?rev=264855&r1=264854&r2=264855&view=diff
>>
>> ==
>> --- cfe/trunk/docs/LibASTMatchersReference.html (original)
>> +++ cfe/trunk/docs/LibASTMatchersReference.html Wed Mar 30 06:22:14 2016
>> @@ -3636,11 +3636,6 @@ callExpr(hasAnyArgument(declRefExpr()))
>>matches x(1, y, 42)
>>  with hasAnyArgument(...)
>>matching y
>> -
>> -FIXME: Currently this will ignore parentheses and implicit casts on
>> -the argument before applying the inner matcher. We'll want to remove
>> -this to allow for greater control by the user once ignoreImplicit()
>> -has been implemented.
>>  
>>
>>
>> @@ -3907,11 +3902,6 @@ callExpr(hasAnyArgument(declRefExpr()))
>>matches x(1, y, 42)
>>  with hasAnyArgument(...)
>>matching y
>> -
>> -FIXME: Currently this will ignore parentheses and implicit casts on
>> -the argument before applying the inner matcher. We'll want to remove
>> -this to allow for greater control by the user once ignoreImplicit()
>> -has been implemented.
>>  
>>
>>
>>
>> Modified: cfe/trunk/docs/ReleaseNotes.rst
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ReleaseNotes.rst?rev=264855&r1=264854&r2=264855&view=diff
>>
>> ==
>> --- cfe/trunk/docs/ReleaseNotes.rst (original)
>> +++ cfe/trunk/docs/ReleaseNotes.rst Wed Mar 30 06:22:14 2016
>> @@ -120,6 +120,12 @@ this section should help get you past th
>>  AST Matchers
>>  
>>
>> +- hasAnyArgument: Matcher no longer ignores parentheses and implicit
>> casts on
>> +  the argument before applying the inner matcher. The fix was done to
>> allow for
>> +  greater control by the user. In all existing checkers that use this
>> matcher
>> +  all instances of code ``hasAnyArgument()`` must be
>> changed to
>> +  ``hasAnyArgument(ignoringParenImpCasts())``.
>> +
>>  ...
>>
>>  libclang
>>
>> Modified: cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h?rev=264855&r1=264854&r2=264855&view=diff
>>
>> ==
>> --- cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h (original)
>> +++ cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h Wed Mar 30 06:22:14
>> 2016
>> @@ -2989,18 +2989,13 @@ AST_MATCHER(CXXCtorInitializer, isMember
>>  ///   matches x(1, y, 42)
>>  /// with hasAnyArgument(...)
>>  ///   matching y
>> -///
>> -/// FIXME: Currently this will ignore parentheses and implicit casts on
>> -/// the argument before applying the inner matcher. We'll want to remove
>> -/// this to allow for greater control by the user once \c
>> ignoreImplicit()
>> -/// has been implemented.
>>  AST_POLYMORPHIC_MATCHER_P(hasAnyArgument,
>>AST_POLYMORPHIC_SUPPORTED_TYPES(CallExpr,
>>
>>  CXXConstructExpr),
>>internal::Matcher, InnerMatcher) {
>>for (const Expr *Arg : Node.arguments()) {
>>  BoundNodesTreeBuilder Result(*Builder);
>

Re: [PATCH] D18582: [Clang-tidy] Update release notes with list of checks added since 3.8

[Alexander Kornienko via cfe-commits]

alexfh added a comment.

A few more nits.



Comment at: docs/ReleaseNotes.rst:68
@@ +67,3 @@
+
+  This check flags calls to ``system()``, ``popen()``, and ``_popen()``, which
+  execute a command processor.

I'd remove "This check" at the start of all check descriptions.


Comment at: docs/ReleaseNotes.rst:85
@@ +84,3 @@
+  This check detects dangling references in value handlers like
+  `std::experimental::string_view`.
+

nit: Double backquotes should be used for inline code snippets.


Comment at: docs/ReleaseNotes.rst:112
@@ +111,3 @@
+
+  Optimize calls to std::string::find() and friends when the needle passed is a
+  single character string literal.

nit: Double backquotes should be used for inline code snippets.


Comment at: docs/ReleaseNotes.rst:117
@@ +116,3 @@
+
+  This check warns about range-based loop with loop variable of const ref type
+  where the type of the variable does not match the one returned by the

nit: "with a loop variable"


Comment at: docs/ReleaseNotes.rst:139
@@ +138,3 @@
+
+  Crash when ``clang-tidy`` runs on compile database with relative source files
+  paths.

nit: Should be :program:clang-tidy


Repository:
  rL LLVM

http://reviews.llvm.org/D18582



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

[clang-tools-extra] r264862 - [clang-tidy] Fix MSVC build.

[Alexander Kornienko via cfe-commits]

Author: alexfh
Date: Wed Mar 30 07:35:05 2016
New Revision: 264862

URL: http://llvm.org/viewvc/llvm-project?rev=264862&view=rev
Log:
[clang-tidy] Fix MSVC build.

Modified:
clang-tools-extra/trunk/clang-tidy/readability/AvoidConstParamsInDecls.h

Modified: 
clang-tools-extra/trunk/clang-tidy/readability/AvoidConstParamsInDecls.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/AvoidConstParamsInDecls.h?rev=264862&r1=264861&r2=264862&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/readability/AvoidConstParamsInDecls.h 
(original)
+++ clang-tools-extra/trunk/clang-tidy/readability/AvoidConstParamsInDecls.h 
Wed Mar 30 07:35:05 2016
@@ -20,7 +20,8 @@ namespace readability {
 // them.
 class AvoidConstParamsInDecls : public ClangTidyCheck {
 public:
-  using ClangTidyCheck::ClangTidyCheck;
+  AvoidConstParamsInDecls(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
 
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;


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

Re: [clang-tools-extra] r264856 - [clang-tidy] readability check for const params in declarations

[Alexander Kornienko via cfe-commits]

Thanks for letting me know! Should be fixed in r264862.

On Wed, Mar 30, 2016 at 2:19 PM, Nico Weber  wrote:

> This doesn't build on Windows:
>
>
> http://lab.llvm.org:8011/builders/clang-x64-ninja-win7/builds/11018/steps/build%20stage%201/logs/stdio
>
> d:\buildslave\clang-x64-ninja-win7\llvm\tools\clang\tools\extra\clang-tidy\readability\../ClangTidyModule.h(61)
> : error C2660:
> 'clang::tidy::readability::AvoidConstParamsInDecls::AvoidConstParamsInDecls'
> : function does not take 2 arguments
>
> D:\buildslave\clang-x64-ninja-win7\llvm\tools\clang\tools\extra\clang-tidy\readability\ReadabilityTidyModule.cpp(37)
> : see reference to function template instantiation 'void
> clang::tidy::ClangTidyCheckFactories::registerCheck(llvm::StringRef)'
> being compiled
>
> Can you take a look?
>
>
> On Wed, Mar 30, 2016 at 4:31 AM, Alexander Kornienko via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: alexfh
>> Date: Wed Mar 30 06:31:33 2016
>> New Revision: 264856
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=264856&view=rev
>> Log:
>> [clang-tidy] readability check for const params in declarations
>>
>> Summary: Adds a clang-tidy warning for top-level consts in function
>> declarations.
>>
>> Reviewers: hokein, sbenza, alexfh
>>
>> Subscribers: cfe-commits
>>
>> Patch by Matt Kulukundis!
>>
>> Differential Revision: http://reviews.llvm.org/D18408
>>
>> Added:
>>
>> clang-tools-extra/trunk/clang-tidy/readability/AvoidConstParamsInDecls.cpp
>>
>> clang-tools-extra/trunk/clang-tidy/readability/AvoidConstParamsInDecls.h
>>
>> clang-tools-extra/trunk/docs/clang-tidy/checks/readability-avoid-const-params-in-decls.rst
>>
>> clang-tools-extra/trunk/test/clang-tidy/readability-avoid-const-params-in-decls.cpp
>> Modified:
>> clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt
>>
>> clang-tools-extra/trunk/clang-tidy/readability/ReadabilityTidyModule.cpp
>> clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
>>
>> Added:
>> clang-tools-extra/trunk/clang-tidy/readability/AvoidConstParamsInDecls.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/AvoidConstParamsInDecls.cpp?rev=264856&view=auto
>>
>> ==
>> ---
>> clang-tools-extra/trunk/clang-tidy/readability/AvoidConstParamsInDecls.cpp
>> (added)
>> +++
>> clang-tools-extra/trunk/clang-tidy/readability/AvoidConstParamsInDecls.cpp
>> Wed Mar 30 06:31:33 2016
>> @@ -0,0 +1,71 @@
>> +//===--- AvoidConstParamsInDecls.cpp -
>> clang-tidy--===//
>> +//
>> +// The LLVM Compiler Infrastructure
>> +//
>> +// This file is distributed under the University of Illinois Open Source
>> +// License. See LICENSE.TXT for details.
>> +//
>>
>> +//===--===//
>> +
>> +#include "AvoidConstParamsInDecls.h"
>> +#include "clang/ASTMatchers/ASTMatchFinder.h"
>> +#include "clang/ASTMatchers/ASTMatchers.h"
>> +#include "clang/Lex/Lexer.h"
>> +
>> +using namespace clang::ast_matchers;
>> +
>> +namespace clang {
>> +namespace tidy {
>> +namespace readability {
>> +namespace {
>> +
>> +SourceRange getTypeRange(const ParmVarDecl &Param) {
>> +  if (Param.getIdentifier() != nullptr)
>> +return SourceRange(Param.getLocStart(),
>> +   Param.getLocEnd().getLocWithOffset(-1));
>> +  return Param.getSourceRange();
>> +}
>> +
>> +} // namespace
>> +
>> +
>> +void AvoidConstParamsInDecls::registerMatchers(MatchFinder *Finder) {
>> +  const auto ConstParamDecl =
>> +  parmVarDecl(hasType(qualType(isConstQualified(.bind("param");
>> +  Finder->addMatcher(functionDecl(unless(isDefinition()),
>> +  has(typeLoc(forEach(ConstParamDecl
>> + .bind("func"),
>> + this);
>> +}
>> +
>> +void AvoidConstParamsInDecls::check(const MatchFinder::MatchResult
>> &Result) {
>> +  const auto *Func = Result.Nodes.getNodeAs("func");
>> +  const auto *Param = Result.Nodes.getNodeAs("param");
>> +
>> +  QualType Type = Param->getType();
>> +  if (!Type.isLocalConstQualified())
>> +return;
>> +
>> +  Type.removeLocalConst();
>> +
>> +  auto Diag = diag(Param->getLocStart(),
>> +   "parameter %0 is const-qualified in the function "
>> +   "declaration; const-qualification of parameters only
>> has an "
>> +   "effect in function definitions");
>> +  if (Param->getName().empty()) {
>> +for (unsigned int i = 0; i < Func->getNumParams(); ++i) {
>> +  if (Param == Func->getParamDecl(i)) {
>> +Diag << (i + 1);
>> +break;
>> +  }
>> +}
>> +  } else {
>> +Diag << Param;
>> +  }
>> +  Diag << FixItHint::CreateReplacement(getTypeRange(*Param),
>> +   Type.getAsString());
>> +}
>> +
>> +} // namespace readability
>> +} // namespace tidy
>> +} //

[PATCH] D18584: Complete support for C++ Core Guidelines Type.6: Always initialize a member variable.

[Michael Miller via cfe-commits]

michael_miller created this revision.
michael_miller added reviewers: flx, alexfh.
michael_miller added a subscriber: cfe-commits.

Added the remaining features needed to satisfy C++ Core Guideline Type.6: 
Always initialize a member variable to cppcoreguidelines-pro-type-member-init. 
The check now flags all default-constructed uses of record types without 
user-provided default constructors that would leave their memory in an 
undefined state. The check suggests value initializing them instead.

http://reviews.llvm.org/D18584

Files:
  clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
  clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.h
  docs/clang-tidy/checks/cppcoreguidelines-pro-type-member-init.rst
  test/clang-tidy/cppcoreguidelines-pro-type-member-init-cxx98.cpp
  test/clang-tidy/cppcoreguidelines-pro-type-member-init-delayed.cpp
  test/clang-tidy/cppcoreguidelines-pro-type-member-init.cpp

Index: test/clang-tidy/cppcoreguidelines-pro-type-member-init.cpp
===
--- test/clang-tidy/cppcoreguidelines-pro-type-member-init.cpp
+++ test/clang-tidy/cppcoreguidelines-pro-type-member-init.cpp
@@ -4,13 +4,13 @@
   int F;
   // CHECK-FIXES: int F{};
   PositiveFieldBeforeConstructor() {}
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize these built-in/pointer fields: F
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize these fields: F
   // CHECK-FIXES: PositiveFieldBeforeConstructor() {}
 };
 
 struct PositiveFieldAfterConstructor {
   PositiveFieldAfterConstructor() {}
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize these built-in/pointer fields: F, G
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize these fields: F, G
   // CHECK-FIXES: PositiveFieldAfterConstructor() {}
   int F;
   // CHECK-FIXES: int F{};
@@ -26,12 +26,12 @@
 };
 
 PositiveSeparateDefinition::PositiveSeparateDefinition() {}
-// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: constructor does not initialize these built-in/pointer fields: F
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: constructor does not initialize these fields: F
 // CHECK-FIXES: PositiveSeparateDefinition::PositiveSeparateDefinition() {}
 
 struct PositiveMixedFieldOrder {
   PositiveMixedFieldOrder() : J(0) {}
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize these built-in/pointer fields: I, K
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize these fields: I, K
   // CHECK-FIXES: PositiveMixedFieldOrder() : J(0) {}
   int I;
   // CHECK-FIXES: int I{};
@@ -43,7 +43,7 @@
 template 
 struct Template {
   Template() {}
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize these built-in/pointer fields: F
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize these fields: F
   int F;
   // CHECK-FIXES: int F{};
   T T1;
@@ -95,17 +95,226 @@
 // CHECK-FIXES: int FIELD;
 
 UNINITIALIZED_FIELD_IN_MACRO_BODY(F);
-// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: constructor does not initialize these built-in/pointer fields: F
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: constructor does not initialize these fields: F
 UNINITIALIZED_FIELD_IN_MACRO_BODY(G);
-// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: constructor does not initialize these built-in/pointer fields: G
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: constructor does not initialize these fields: G
 
 #define UNINITIALIZED_FIELD_IN_MACRO_ARGUMENT(ARGUMENT) \
   ARGUMENT		\
 
 UNINITIALIZED_FIELD_IN_MACRO_ARGUMENT(struct UninitializedFieldInMacroArg {
   UninitializedFieldInMacroArg() {}
   int Field;
 });
-// CHECK-MESSAGES: :[[@LINE-3]]:3: warning: constructor does not initialize these built-in/pointer fields: Field
+// CHECK-MESSAGES: :[[@LINE-3]]:3: warning: constructor does not initialize these fields: Field
 // Ensure FIELD is not initialized since fixes inside of macros are disabled.
 // CHECK-FIXES: int Field;
+
+struct NegativeAggregateType {
+  int X;
+  int Y;
+  int Z;
+};
+
+struct PositiveTrivialType {
+  PositiveTrivialType() { }
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize these fields: F
+
+  NegativeAggregateType F;
+  // CHECK-FIXES: NegativeAggregateType F{};
+};
+
+struct NegativeNonTrivialType {
+  PositiveTrivialType F;
+};
+
+static void PositiveUninitializedTrivialType() {
+  NegativeAggregateType X;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: uninitialized record type: X
+  // CHECK-FIXES: NegativeAggregateType X{};
+
+  NegativeAggregateType A[10];
+  // Don't warn because this isn't an object type
+}
+
+static void NegativeInitializedTrivialType() {
+  NegativeAggregateType X{};
+  NegativeAggregateType Y = {};
+  NegativeAggregateType Z = NegativeAggregateType();
+  NegativeAggregateType A[10]{};
+  NegativeAggregateType B[10] = {};
+  int C; // no need to initialize this

Re: [PATCH] D18265: [clang-tidy] New: checker misc-assign-operator-return

[Aaron Ballman via cfe-commits]

aaron.ballman added a comment.

In http://reviews.llvm.org/D18265#386676, @alexfh wrote:

> > What about NonIdiomaticAddignOperator or UnchainableAssignOperator?
>
>
> Yep, "unchainable" doesn't cover all problems the check detects. 
> `misc-non-idiomatic-assign-operator` seems good though. I'd wait for the 
> original author to chime in before making the change.


This doesn't check for idiomatic assignment, unfortunately. For instance, it 
allows `T &operator=(T)` which is a copy assignment, but not generally 
considered an idiomatic one. (Similar for allowing `volatile`-qualified 
parameters.) If we want to go with such a check, I would not be opposed to it, 
but we should definitely discuss what "idiomatic" means.


http://reviews.llvm.org/D18265



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

Re: [PATCH] D18265: [clang-tidy] New: checker misc-assign-operator-return

[Alexander Kornienko via cfe-commits]

alexfh added a comment.

In http://reviews.llvm.org/D18265#386717, @aaron.ballman wrote:

> In http://reviews.llvm.org/D18265#386676, @alexfh wrote:
>
> > > What about NonIdiomaticAddignOperator or UnchainableAssignOperator?
> >
> >
> > Yep, "unchainable" doesn't cover all problems the check detects. 
> > `misc-non-idiomatic-assign-operator` seems good though. I'd wait for the 
> > original author to chime in before making the change.
>
>
> This doesn't check for idiomatic assignment, unfortunately. For instance, it 
> allows `T &operator=(T)` which is a copy assignment, but not generally 
> considered an idiomatic one. (Similar for allowing `volatile`-qualified 
> parameters.) If we want to go with such a check, I would not be opposed to 
> it, but we should definitely discuss what "idiomatic" means.


Maybe you like `misc-assign-operator-conventions` more? ;)


http://reviews.llvm.org/D18265



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

Re: [PATCH] D18265: [clang-tidy] New: checker misc-assign-operator-return

[Gábor Horváth via cfe-commits]

xazax.hun added a comment.

In http://reviews.llvm.org/D18265#386717, @aaron.ballman wrote:

> In http://reviews.llvm.org/D18265#386676, @alexfh wrote:
>
> > > What about NonIdiomaticAddignOperator or UnchainableAssignOperator?
> >
> >
> > Yep, "unchainable" doesn't cover all problems the check detects. 
> > `misc-non-idiomatic-assign-operator` seems good though. I'd wait for the 
> > original author to chime in before making the change.
>
>
> This doesn't check for idiomatic assignment, unfortunately. For instance, it 
> allows `T &operator=(T)` which is a copy assignment, but not generally 
> considered an idiomatic one. (Similar for allowing `volatile`-qualified 
> parameters.) If we want to go with such a check, I would not be opposed to 
> it, but we should definitely discuss what "idiomatic" means.


IMHO `T &operator=(T)` can be idiomatic e.g. when one uses copy and swap idiom 
or `T &operator=(S)` where S is a type that can be copied efficiently.


http://reviews.llvm.org/D18265



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

Re: [PATCH] D18265: [clang-tidy] New: checker misc-assign-operator-return

[Balogh , Ádám via cfe-commits]

baloghadamsoftware added a comment.

Actually, there was nothing wrong with assign operator signatures per se either 
although the original name of the checker was AssignOperatorSignature. The only 
change here is that it does not check the signature only anymore, but also the 
body (if present).


http://reviews.llvm.org/D18265



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

Re: [PATCH] D15729: Load compiler plugins in ASTUnit, too

[Stefan Kemnitz via cfe-commits]

realincubus added a comment.

@bkramer I can verify that after building this with clang 3.9 in release mode 
it does not work anymore like expected. everything is fine in debug mode

if i look at the output generated from

  llvm::sys::DynamicLibrary::LoadLibraryPermanently

in 1761 CompilerInstance::loadPlugins()

i get errors like

  /home/incubus/llvm_patch_test/build/lib/ClangPlugin.so 
/home/incubus/llvm_patch_test/build/lib/ClangPlugin.so: undefined symbol: 
_ZN5clang17ASTFrontendAction13ExecuteActionEv

In my original patch http://reviews.llvm.org/D5611 I added a list of symbols 
that had to be exported in order to make libclang work with plugins like 
expected.
After adding the symbols from the original patch and 2 additional ones it is ok 
now with clang version 3.9 in release mode.

Symbols needed:

  _ZN5clang15PluginASTAction6anchorEv
  _ZN4llvm11raw_ostream5writeEPKcm
  _ZN4llvm4errsEv
  _ZN5clang11ASTConsumer33HandleTopLevelDeclInObjCContainerENS_12DeclGroupRefE
  _ZN5clang14FrontendActionD2Ev
  _ZN5clang13DiagnosticIDs15getCustomDiagIDENS0_5LevelEN4llvm9StringRefE
  _ZN5clang17ASTFrontendAction13ExecuteActionEv
  _ZN5clang11ASTConsumer24HandleImplicitImportDeclEPNS_10ImportDeclE
  _ZN5clang14FrontendAction22shouldEraseOutputFilesEv
  _ZNK5clang15DeclarationName11getAsStringEv
  _ZN5clang17DiagnosticsEngine21EmitCurrentDiagnosticEb
  _ZN5clang11ASTConsumer21HandleInterestingDeclENS_12DeclGroupRefE
  _ZN5clang14FrontendActionC2Ev
  
_ZN4llvm8RegistryIN5clang15PluginASTActionENS_14RegistryTraitsIS2_EEE4nodeC1ERKNS_19SimpleRegistryEntryIS2_EE
  _ZTVN5clang15PluginASTActionE
  _ZTVN5clang17ASTFrontendActionE
  _ZTVN5clang11ASTConsumerE
  _ZN4llvm8RegistryIN5clang15PluginASTActionEE4HeadE
  _ZN4llvm8RegistryIN5clang15PluginASTActionEE4TailE

Please tell me if I am wrong, but "tools/libclang/libclang.exports" is kind of 
a filter.
Everything that is not listed in this file is not exported as a symbol in the 
resulting libclang.so file.

Is there a way to automatically add the needed symbols to the exports file ?


http://reviews.llvm.org/D15729



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

Re: [PATCH] D18265: [clang-tidy] New: checker misc-assign-operator-return

[Aaron Ballman via cfe-commits]

On Wed, Mar 30, 2016 at 8:56 AM, Alexander Kornienko  wrote:
> alexfh added a comment.
>
> In http://reviews.llvm.org/D18265#386717, @aaron.ballman wrote:
>
>> In http://reviews.llvm.org/D18265#386676, @alexfh wrote:
>>
>> > > What about NonIdiomaticAddignOperator or UnchainableAssignOperator?
>> >
>> >
>> > Yep, "unchainable" doesn't cover all problems the check detects. 
>> > `misc-non-idiomatic-assign-operator` seems good though. I'd wait for the 
>> > original author to chime in before making the change.
>>
>>
>> This doesn't check for idiomatic assignment, unfortunately. For instance, it 
>> allows `T &operator=(T)` which is a copy assignment, but not generally 
>> considered an idiomatic one. (Similar for allowing `volatile`-qualified 
>> parameters.) If we want to go with such a check, I would not be opposed to 
>> it, but we should definitely discuss what "idiomatic" means.
>
>
> Maybe you like `misc-assign-operator-conventions` more? ;)

Certainly more than claiming it's idiomatic. :-) Personally, I would
prefer this to remain split into a signature and body check, but that
can be simulated with config options.

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

Re: [PATCH] D18265: [clang-tidy] New: checker misc-assign-operator-return

[Aaron Ballman via cfe-commits]

On Wed, Mar 30, 2016 at 8:52 AM, Gábor Horváth  wrote:
> xazax.hun added a comment.
>
> In http://reviews.llvm.org/D18265#386717, @aaron.ballman wrote:
>
>> In http://reviews.llvm.org/D18265#386676, @alexfh wrote:
>>
>> > > What about NonIdiomaticAddignOperator or UnchainableAssignOperator?
>> >
>> >
>> > Yep, "unchainable" doesn't cover all problems the check detects. 
>> > `misc-non-idiomatic-assign-operator` seems good though. I'd wait for the 
>> > original author to chime in before making the change.
>>
>>
>> This doesn't check for idiomatic assignment, unfortunately. For instance, it 
>> allows `T &operator=(T)` which is a copy assignment, but not generally 
>> considered an idiomatic one. (Similar for allowing `volatile`-qualified 
>> parameters.) If we want to go with such a check, I would not be opposed to 
>> it, but we should definitely discuss what "idiomatic" means.
>
>
> IMHO `T &operator=(T)` can be idiomatic e.g. when one uses copy and swap 
> idiom or `T &operator=(S)` where S is a type that can be copied efficiently.

For reference, between the Kona and Jacksonville WG21 meetings, I
started work on a checker for non-idiomatic copy and move assignment
operations because of confusion regarding copy assignment as the core
language sees it and the CopyAssignable library concept. "Idiomatic",
to me, means what the library cares about, not what the core language
does. A checker that tells the user "this is an idiomatic copy
assignment operator" is not very useful when you can't use that type
with the STL in well-defined ways. So if we want "idiomatic" in the
name of the check, we should probably review the patch behavior with
that in mind.

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

Re: [PATCH] D18265: [clang-tidy] New: checker misc-assign-operator-return

[Alexander Kornienko via cfe-commits]

alexfh added a comment.

In http://reviews.llvm.org/D18265#386720, @baloghadamsoftware wrote:

> Actually, there was nothing wrong with assign operator signatures per se 
> either although the original name of the checker was AssignOperatorSignature. 
> The only change here is that it does not check the signature only anymore, 
> but also the body (if present).


Maybe the old check name should have been 
`misc-unconventional-assign-operator-signature` or something like that, but 
even the old name limited the scope enough to make it easy to guess about the 
possible issues it flags. However, further expanding the scope makes the name 
even less informative. There are just too many things that could be wrong with 
assignment operators. If/when we add a check for another bug-prone pattern 
related to assignment operators, the name `misc-assign-operator` could be broad 
enough to cover this hypothetical new check as well. This will inevitably lead 
to confusion.


http://reviews.llvm.org/D18265



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

Re: [PATCH] D15729: Load compiler plugins in ASTUnit, too

[Benjamin Kramer via cfe-commits]

bkramer added a comment.

If we really want to go down this route we should drop the exports file, 
putting random symbols in there is not going to fly. I'm not sure what that 
would break for libclang users though.

Also make sure that your building without BUILD_SHARED_LIBS, otherwise you'll 
get a very distorted view of how libclang works.


http://reviews.llvm.org/D15729



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

Re: [PATCH] D18265: [clang-tidy] New: checker misc-assign-operator-return

[Balogh , Ádám via cfe-commits]

baloghadamsoftware added a comment.

misc-unconventional-assign-operator?


http://reviews.llvm.org/D18265



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

Re: [PATCH] D18338: AMDGPU: Add frexp_exp builtins

[Tom Stellard via cfe-commits]

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

LGTM.


http://reviews.llvm.org/D18338



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

Re: [PATCH] D18265: [clang-tidy] New: checker misc-assign-operator-return

[Aaron Ballman via cfe-commits]

On Wed, Mar 30, 2016 at 9:23 AM, Alexander Kornienko  wrote:
> alexfh added a comment.
>
> In http://reviews.llvm.org/D18265#386720, @baloghadamsoftware wrote:
>
>> Actually, there was nothing wrong with assign operator signatures per se 
>> either although the original name of the checker was 
>> AssignOperatorSignature. The only change here is that it does not check the 
>> signature only anymore, but also the body (if present).
>
>
> Maybe the old check name should have been 
> `misc-unconventional-assign-operator-signature` or something like that, but 
> even the old name limited the scope enough to make it easy to guess about the 
> possible issues it flags. However, further expanding the scope makes the name 
> even less informative. There are just too many things that could be wrong 
> with assignment operators. If/when we add a check for another bug-prone 
> pattern related to assignment operators, the name `misc-assign-operator` 
> could be broad enough to cover this hypothetical new check as well. This will 
> inevitably lead to confusion.

I agree with your point; that's why my slight preference is for
leaving them split into multiple checks. Either we want this to be the
catch-all for operator assignment checks (and plan to use config
options to control behavior for additional checks), at which point
misc-operator-assign is a reasonable enough name, or we want a clear
name for a check that checks two separate-but-related things (one
checks the signature, the other checks the return expression value).
I'm not certain we'll get a particularly *clear* name for a check that
diagnoses fairly separate issues.

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

Re: [PATCH] D18575: [clang-tidy] New checker to replace deprecated throw() specifications

[don hinton via cfe-commits]

hintonda added a comment.

Will do, but that will have to be done in it's own patch.


http://reviews.llvm.org/D18575



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

[PATCH] D18595: MPI-Checker test helper

[Alexander Droste via cfe-commits]

Alexander_Droste created this revision.
Alexander_Droste added a reviewer: zaks.anna.
Alexander_Droste added subscribers: dcoughlin, xazax.hun, cfe-commits.

This file contains definitions that help to test MPI-Checker functionality,
on functions not visible to the analyzer, as they are defined in a distinct
translation unit. The main MPI-Checker testfile is MPIChecker.cpp.


http://reviews.llvm.org/D18595

Files:
  test/Analysis/MPICheckerHelper.cpp

Index: test/Analysis/MPICheckerHelper.cpp
===
--- test/Analysis/MPICheckerHelper.cpp
+++ test/Analysis/MPICheckerHelper.cpp
@@ -0,0 +1,15 @@
+// This file contains definitions that help to test MPI-Checker functionality,
+// on functions not visible to the analyzer, as they are defined in a distinct
+// translation unit. The main MPI-Checker testfile is MPIChecker.cpp.
+
+#include "MPIMock.h"
+
+void callNonblockingExtern(MPI_Request *req) {
+  static double buf = 0;
+  MPI_Ireduce(MPI_IN_PLACE, &buf, 1, MPI_DOUBLE, MPI_SUM, 0, MPI_COMM_WORLD,
+  req);
+}
+
+void callWaitExtern(MPI_Request *req) {
+  MPI_Wait(req, MPI_STATUS_IGNORE);
+}


Index: test/Analysis/MPICheckerHelper.cpp
===
--- test/Analysis/MPICheckerHelper.cpp
+++ test/Analysis/MPICheckerHelper.cpp
@@ -0,0 +1,15 @@
+// This file contains definitions that help to test MPI-Checker functionality,
+// on functions not visible to the analyzer, as they are defined in a distinct
+// translation unit. The main MPI-Checker testfile is MPIChecker.cpp.
+
+#include "MPIMock.h"
+
+void callNonblockingExtern(MPI_Request *req) {
+  static double buf = 0;
+  MPI_Ireduce(MPI_IN_PLACE, &buf, 1, MPI_DOUBLE, MPI_SUM, 0, MPI_COMM_WORLD,
+  req);
+}
+
+void callWaitExtern(MPI_Request *req) {
+  MPI_Wait(req, MPI_STATUS_IGNORE);
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D18408: readability check for const params in declarations

[Matthew Fowles Kulukundis via cfe-commits]

My attempts to do this end with:

$ arc diff
Linting...
No lint engine configured for this project.
Running unit tests...
No unit test engine is configured for this project.
 SKIP STAGING  Unable to determine repository for this change.
Exception
ERR_CLOSED: This revision has already been closed.
(Run with `--trace` for a full exception trace.)


On Wed, Mar 30, 2016 at 7:39 AM, Alexander Kornienko 
wrote:

> alexfh added a comment.
>
> Actually, we missed one thing: may I ask you to update
> docs/ReleaseNotes.rst with a short description of the new check?
>
>
> Repository:
>   rL LLVM
>
> http://reviews.llvm.org/D18408
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r264868 - Prepare tests for change to emit Module SourceFileName to LLVM assembly

[Teresa Johnson via cfe-commits]

Author: tejohnson
Date: Wed Mar 30 08:59:49 2016
New Revision: 264868

URL: http://llvm.org/viewvc/llvm-project?rev=264868&view=rev
Log:
Prepare tests for change to emit Module SourceFileName to LLVM assembly

Modify these tests to ignore the source file name when looking for the
expected string. It was already catching the source file name once via
the ModuleID, and will catch it another time with an impending change to
LLVM to serialize out the module's SourceFileName.

Modified:
cfe/trunk/test/CodeGen/sret.c
cfe/trunk/test/CodeGen/sret2.c

Modified: cfe/trunk/test/CodeGen/sret.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/sret.c?rev=264868&r1=264867&r2=264868&view=diff
==
--- cfe/trunk/test/CodeGen/sret.c (original)
+++ cfe/trunk/test/CodeGen/sret.c Wed Mar 30 08:59:49 2016
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 %s -emit-llvm -o - | grep sret | count 5
+// RUN: %clang_cc1 %s -emit-llvm -o - | grep sret | grep -v 'sret.c' | count 4
 
 struct abc {
  long a;

Modified: cfe/trunk/test/CodeGen/sret2.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/sret2.c?rev=264868&r1=264867&r2=264868&view=diff
==
--- cfe/trunk/test/CodeGen/sret2.c (original)
+++ cfe/trunk/test/CodeGen/sret2.c Wed Mar 30 08:59:49 2016
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 %s -emit-llvm -o - | grep sret | count 2
+// RUN: %clang_cc1 %s -emit-llvm -o - | grep sret | grep -v 'sret2.c' | count 1
 
 struct abc {
  long a;


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

Re: [PATCH] D18408: readability check for const params in declarations

[Matt Kulukundis via cfe-commits]

fowles added a subscriber: fowles.
fowles added a comment.

My attempts to do this end with:

$ arc diff
Linting...
No lint engine configured for this project.
Running unit tests...
No unit test engine is configured for this project.
 SKIP STAGING  Unable to determine repository for this change.
Exception
ERR_CLOSED: This revision has already been closed.
(Run with `--trace` for a full exception trace.)


Repository:
  rL LLVM

http://reviews.llvm.org/D18408



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

Re: [PATCH] D18575: [clang-tidy] New checker to replace deprecated throw() specifications

[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 *really* like this as a modernize check, especially since (throwing) dynamic 
exception specifications may be removed in C++1z. However, I think this check 
doesn't do enough yet. If dynamic exception specifications are removed in the 
next standard, `throw()` is likely to remain for backwards compatibility since 
it does map directly to `noexcept`. I think this check really needs to convert 
`throw(type)` and `throw(...)` into `noexcept(false)` as well. Further, I 
prefer not to touch `noexcept(true)` as part of this check, since that really 
isn't modernizing anything.

Thank you for working on this!



Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:24
@@ +23,3 @@
+   char delimiter) {
+  SmallVector Candidates;
+  AllStrings.split(Candidates, ',');

Why 5?


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:33
@@ +32,3 @@
+
+using namespace lexer_utils;
+

This should not be at file scope; if it really clarifies the code, it should be 
at function scope where needed.


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:65
@@ +64,3 @@
+
+  if (const FunctionDecl *FuncDecl =
+  Result.Nodes.getNodeAs("functionDecl")) {

Instead of indenting a considerable amount of code, I think an early return may 
be a bit more clear.


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:83
@@ +82,3 @@
+BeforeThanCompare isBefore(SM);
+while (isBefore(BeginLoc, CurrentLoc)) {
+  SourceLocation Loc = Tok.getLocation();

This while loop could use some comments to explain what it is trying to do. As 
best I can tell, this appears to be looking purely at the text the user wrote 
to try to determine whether there is a `throw()` or a `noexcept(true)`, but 
that can be done more clearly with  FunctionType::getExceptionSpecType().


Comment at: clang-tidy/modernize/UseNoexceptCheck.h:21
@@ +20,3 @@
+
+/// \brief Replace noexcept specifications, or macros, with noexcept or a user 
defined macro. 
+/// \code

I think you mean "Replace dynamic exception specifications" instead of 
"noexcept specifications".


http://reviews.llvm.org/D18575



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

Re: [PATCH] D18575: [clang-tidy] New checker to replace deprecated throw() specifications

[Eugene Zelenko via cfe-commits]

Eugene.Zelenko added a comment.

It's much better to add information in release notes in same patch as check 
itself. See also http://reviews.llvm.org/D18509 for related add_new_check.py 
improvement.


http://reviews.llvm.org/D18575



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

Re: [PATCH] D18110: [OpenMP] Fix SEMA bug in the capture of global variables in template functions.

[Samuel Antao via cfe-commits]

sfantao added a comment.

Hi Alexey,

Thanks for the review.



Comment at: lib/Sema/SemaOpenMP.cpp:816-822
@@ -801,6 +815,9 @@
+
+  // A DSA refers to this captured region if the parent contexts match.
+  auto *ParentContext = RSI->TheCapturedDecl->getParent();
   for (auto I = Stack.rbegin(), EE = Stack.rend(); I != EE; ++I)
-if (I->CurScope == S)
+if (I->ParentDeclContext == ParentContext)
   return I->Directive;
   return OMPD_unknown;
 }
 

ABataev wrote:
> Actually, I think, we need to rework the whole IsOpenMPCapturedByRef() 
> function. I'm not sure that it is even required. It looks like some 
> optimization hack in the frontend. I'm against any not-necessary 
> optimizations in frontend. If scalar value is a firstprivate, it must be 
> handled in codegen, not by handling it by copy.
'IsOpenMPCapturedByRef' goal is to change the signature of the outlined 
function. We don't want to have reference types in target region arguments 
unless they are really required. We have seen performance being greatly 
affected just because of that. Of course a consequence of this is to have 
variables that become first private.

The current implementation of OpenMP firstprivate doesn't change the the 
function signatures, only the codegen inside the region is affected. So, this 
is not a complete overlap.

With this, I am not saying that this cannot be done in codegen. The reason I am 
doing it here is your initial guideline that we should attempt to fix most of 
the things in Sema and avoid complicating codegen which is a more sensitive 
part.

Doing this in Codegen would require changing the implementation of 
`GenerateOpenMPCapturedVars` and `GenerateOpenMPCapturedStmtFunction`. We 
wouldn't need the tracking of the context anymore (checking the directive would 
be sufficient), but we would still need to see what't in the map clauses.

So, do you think we should change this?


http://reviews.llvm.org/D18110



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

Re: [PATCH] D18596: [MSVC] PR27132: Proper mangling for __unaligned qualifier

[Aaron Ballman via cfe-commits]

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

I'd like to see some Sema tests for sanity checking; like applying __unaligned 
to a non-pointer type, to a declaration, when -fno-ms-extensions is enabled, 
etc.



Comment at: include/clang/Basic/Attr.td:2080
@@ -2079,2 +2079,3 @@
   let Spellings = [Keyword<"__unaligned">];
+  let Documentation = [UnalignedDocs];
 }

This is possibly missing `let LangOpts = [MicrosoftExt];`


Comment at: include/clang/Basic/AttrDocs.td:1964
@@ +1963,3 @@
+  let Content = [{
+The ``__unaligned`` modifier declares a pointer with unaligned address.
+It is available under the ``-fms-extensions`` flag for MSVC compatibility.

"an" unaligned address.


Comment at: lib/Sema/SemaType.cpp:
@@ -5552,1 +5554,3 @@
+  // anything, so this is an exception.
+  if (!isa(Desugared) && (Kind != AttributeList::AT_Unaligned)) {
 if (Type->isMemberPointerType())

This appears to be untested, but really should have a test case to ensure we 
don't regress later.


http://reviews.llvm.org/D18596



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

[PATCH] D18596: [MSVC] PR27132: Proper mangling for __unaligned qualifier

[Andrey Bokhanko via cfe-commits]

andreybokhanko created this revision.
andreybokhanko added reviewers: rnk, majnemer.
andreybokhanko added a subscriber: cfe-commits.

This patch implements proper mangling for MS-specific "__unaligned" qualifier. 
The mangling part itself is tiny; most of the patch is required to convert 
"__unaligned" from an ignored qualifier to a non-ignored one.

See PR27132 for an example of code that mangles incorrectly at the moment.

Yours,
Andrey
=
Software Engineer
Intel Compiler Team

http://reviews.llvm.org/D18596

Files:
  include/clang/AST/Type.h
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td
  lib/AST/MicrosoftMangle.cpp
  lib/AST/Type.cpp
  lib/AST/TypePrinter.cpp
  lib/Sema/SemaType.cpp
  test/CodeGenCXX/mangle-ms-cxx11.cpp

Index: lib/AST/MicrosoftMangle.cpp
===
--- lib/AST/MicrosoftMangle.cpp
+++ lib/AST/MicrosoftMangle.cpp
@@ -1413,6 +1413,12 @@
 
   if (HasRestrict)
 Out << 'I';
+
+  if (!PointeeType.isNull()) {
+auto AT = PointeeType->getAs();
+if ((AT != nullptr) && (AT->getAttrKind() == AttributedType::attr_unaligned))
+  Out << 'F';
+  }
 }
 
 void MicrosoftCXXNameMangler::manglePointerCVQualifiers(Qualifiers Quals) {
Index: lib/AST/TypePrinter.cpp
===
--- lib/AST/TypePrinter.cpp
+++ lib/AST/TypePrinter.cpp
@@ -1156,6 +1156,7 @@
 case AttributedType::attr_ptr64: OS << " __ptr64"; break;
 case AttributedType::attr_sptr: OS << " __sptr"; break;
 case AttributedType::attr_uptr: OS << " __uptr"; break;
+case AttributedType::attr_unaligned: OS << " __unaligned"; break;
 }
 spaceBeforePlaceHolder(OS);
   }
Index: lib/AST/Type.cpp
===
--- lib/AST/Type.cpp
+++ lib/AST/Type.cpp
@@ -3003,6 +3003,7 @@
   case AttributedType::attr_sptr:
   case AttributedType::attr_uptr:
   case AttributedType::attr_objc_kindof:
+  case AttributedType::attr_unaligned:
 return false;
   }
   llvm_unreachable("bad attributed type kind");
@@ -3015,6 +3016,7 @@
   case attr_ptr64:
   case attr_sptr:
   case attr_uptr:
+  case attr_unaligned:
 return true;
   }
   llvm_unreachable("invalid attr kind");
@@ -3039,6 +3041,7 @@
   case attr_nullable:
   case attr_null_unspecified:
   case attr_objc_kindof:
+  case attr_unaligned:
 return false;
 
   case attr_pcs:
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -120,7 +120,8 @@
 case AttributeList::AT_Ptr32: \
 case AttributeList::AT_Ptr64: \
 case AttributeList::AT_SPtr: \
-case AttributeList::AT_UPtr
+case AttributeList::AT_UPtr: \
+case AttributeList::AT_Unaligned
 
 // Nullability qualifiers.
 #define NULLABILITY_TYPE_ATTRS_CASELIST \
@@ -4538,6 +4539,8 @@
 return AttributeList::AT_TypeNullUnspecified;
   case AttributedType::attr_objc_kindof:
 return AttributeList::AT_ObjCKindOf;
+  case AttributedType::attr_unaligned:
+return AttributeList::AT_Unaligned;
   }
   llvm_unreachable("unexpected attribute kind!");
 }
@@ -5547,8 +5550,9 @@
   }
 
   // Pointer type qualifiers can only operate on pointer types, but not
-  // pointer-to-member types.
-  if (!isa(Desugared)) {
+  // pointer-to-member types. "__unaligned" qualifier can be applied to
+  // anything, so this is an exception.
+  if (!isa(Desugared) && (Kind != AttributeList::AT_Unaligned)) {
 if (Type->isMemberPointerType())
   S.Diag(Attr.getLoc(), diag::err_attribute_no_member_pointers)
   << Attr.getName();
@@ -5565,6 +5569,7 @@
   case AttributeList::AT_Ptr64: TAK = AttributedType::attr_ptr64; break;
   case AttributeList::AT_SPtr: TAK = AttributedType::attr_sptr; break;
   case AttributeList::AT_UPtr: TAK = AttributedType::attr_uptr; break;
+  case AttributeList::AT_Unaligned: TAK = AttributedType::attr_unaligned; break;
   }
 
   Type = S.Context.getAttributedType(TAK, Type, Type);
Index: include/clang/AST/Type.h
===
--- include/clang/AST/Type.h
+++ include/clang/AST/Type.h
@@ -3639,6 +3639,7 @@
 attr_null_unspecified,
 attr_objc_kindof,
 attr_objc_inert_unsafe_unretained,
+attr_unaligned,
   };
 
 private:
Index: include/clang/Basic/AttrDocs.td
===
--- include/clang/Basic/AttrDocs.td
+++ include/clang/Basic/AttrDocs.td
@@ -1957,3 +1957,19 @@
   The system will crash if the wrong handler is used.
   }];
 }
+
+def UnalignedDocs : Documentation {
+  let Category = DocCatVariable;
+  let Content = [{
+The ``__unaligned`` modifier declares a pointer with unaligned address.
+It is available under the ``-fms-extensions`` flag for MSVC compatibility.
+See the documentation for `__unaligned`_ on MSDN.
+
+.. _`__unaligned`: https://msdn.microsoft.com/en-us/library/ms

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

[Alexander Droste via cfe-commits]

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

- fix typo


http://reviews.llvm.org/D12761

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

Index: test/Analysis/MPIChecker.cpp
===
--- test/Analysis/MPIChecker.cpp
+++ test/Analysis/MPIChecker.cpp
@@ -0,0 +1,337 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=optin.mpi.MPI-Checker -verify %s
+
+// MPI-Checker makes no assumptions about details of an MPI implementation.
+// Typedefs and constants are compared as strings.
+
+#include "MPIMock.h"
+
+//integration tests
+
+void matchedWait1() {
+  int rank = 0;
+  double buf = 0;
+  MPI_Comm_rank(MPI_COMM_WORLD, &rank);
+  if (rank >= 0) {
+MPI_Request sendReq1, recvReq1;
+MPI_Isend(&buf, 1, MPI_DOUBLE, rank + 1, 0, MPI_COMM_WORLD, &sendReq1);
+MPI_Irecv(&buf, 1, MPI_DOUBLE, rank - 1, 0, MPI_COMM_WORLD, &recvReq1);
+
+MPI_Wait(&sendReq1, MPI_STATUS_IGNORE);
+MPI_Wait(&recvReq1, MPI_STATUS_IGNORE);
+  }
+} // no error
+
+void matchedWait2() {
+  int rank = 0;
+  double buf = 0;
+  MPI_Comm_rank(MPI_COMM_WORLD, &rank);
+  if (rank >= 0) {
+MPI_Request sendReq1, recvReq1;
+MPI_Isend(&buf, 1, MPI_DOUBLE, rank + 1, 0, MPI_COMM_WORLD, &sendReq1);
+MPI_Irecv(&buf, 1, MPI_DOUBLE, rank - 1, 0, MPI_COMM_WORLD, &recvReq1);
+MPI_Wait(&sendReq1, MPI_STATUS_IGNORE);
+MPI_Wait(&recvReq1, MPI_STATUS_IGNORE);
+  }
+} // no error
+
+void matchedWait3() {
+  int rank = 0;
+  double buf = 0;
+  MPI_Comm_rank(MPI_COMM_WORLD, &rank);
+  if (rank >= 0) {
+MPI_Request sendReq1, recvReq1;
+MPI_Isend(&buf, 1, MPI_DOUBLE, rank + 1, 0, MPI_COMM_WORLD, &sendReq1);
+MPI_Irecv(&buf, 1, MPI_DOUBLE, rank - 1, 0, MPI_COMM_WORLD, &recvReq1);
+
+if (rank > 1000) {
+  MPI_Wait(&sendReq1, MPI_STATUS_IGNORE);
+  MPI_Wait(&recvReq1, MPI_STATUS_IGNORE);
+} else {
+  MPI_Wait(&sendReq1, MPI_STATUS_IGNORE);
+  MPI_Wait(&recvReq1, MPI_STATUS_IGNORE);
+}
+  }
+} // no error
+
+void missingWait1() { // Check missing wait for dead region.
+  double buf = 0;
+  MPI_Request sendReq1;
+  MPI_Ireduce(MPI_IN_PLACE, &buf, 1, MPI_DOUBLE, MPI_SUM, 0, MPI_COMM_WORLD, &sendReq1); // expected-note{{Request is previously used by nonblocking call here.}}
+} // expected-warning{{Request 'sendReq1' has no matching wait.}}
+
+void missingWait2() {
+  int rank = 0;
+  double buf = 0;
+  MPI_Comm_rank(MPI_COMM_WORLD, &rank);
+  if (rank == 0) {
+  } else {
+MPI_Request sendReq1, recvReq1;
+
+MPI_Isend(&buf, 1, MPI_DOUBLE, rank + 1, 0, MPI_COMM_WORLD, &sendReq1);
+MPI_Irecv(&buf, 1, MPI_DOUBLE, rank - 1, 0, MPI_COMM_WORLD, &recvReq1); // expected-warning{{Request 'sendReq1' has no matching wait.}}
+MPI_Wait(&recvReq1, MPI_STATUS_IGNORE);
+  }
+}
+
+void doubleNonblocking() {
+  int rank = 0;
+  double buf = 0;
+  MPI_Comm_rank(MPI_COMM_WORLD, &rank);
+  if (rank == 1) {
+  } else {
+MPI_Request sendReq1;
+
+MPI_Isend(&buf, 1, MPI_DOUBLE, rank + 1, 0, MPI_COMM_WORLD, &sendReq1); // expected-note{{Request is previously used by nonblocking call here. }}
+MPI_Irecv(&buf, 1, MPI_DOUBLE, rank - 1, 0, MPI_COMM_WORLD, &sendReq1); // expected-warning{{Double nonblocking on request 'sendReq1'.}}
+MPI_Wait(&sendReq1, MPI_STATUS_IGNORE);
+  }
+}
+
+void doubleNonblocking2() {
+  int rank = 0;
+  double buf = 0;
+  MPI_Comm_rank(MPI_COMM_WORLD, &rank);
+
+  MPI_Request req;
+  MPI_Ireduce(MPI_IN_PLACE, &buf, 1, MPI_DOUBLE, MPI_SUM, 0, MPI_COMM_WORLD, &req); // expected-note{{Request is previously used by nonblocking call here.}}
+  MPI_Ireduce(MPI_IN_PLACE, &buf, 1, MPI_DOUBLE, MPI_SUM, 0, MPI_COMM_WORLD, &req); // expected-warning{{Double nonblocking on request 'req'.}}
+  MPI_Wait(&req, MPI_STATUS_IGNORE);
+}
+
+void doubleNonblocking3() {
+  typedef struct { MPI_Request req; } ReqStruct;
+
+  ReqStruct rs;
+  int rank = 0;
+  double buf = 0;
+  MPI_Comm_rank(MPI_COMM_WORLD, &rank);
+
+  MPI_Ireduce(MPI_IN_PLACE, &buf, 1, MPI_DOUBLE, MPI_SUM, 0, MPI_COMM_WORLD, &rs.req); // expected-note{{Request is previously used by nonblocking call here.}}
+  MPI_Ireduce(MPI_IN_PLACE, &buf, 1, MPI_DOUBLE, MPI_SUM, 0, MPI_COMM_WORLD, &rs.req); // expected-warning{{Double nonblocking on request 'rs.req'.}}
+  MPI_Wait(&rs.req, MPI_STATUS_IGNORE);
+}
+
+void doubleNonblocking4() {
+  int rank = 0;
+  double buf = 0;
+  MPI_Comm_rank(MPI_COMM_WORLD, &rank);
+
+  MPI_Request req;
+  for (int i = 0; i < 

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

[Alexander Droste via cfe-commits]

Alexander_Droste updated the summary for this revision.
Alexander_Droste updated this revision to Diff 52056.
Alexander_Droste added a comment.

- added test that uses wrapper functions around MPI functions
- added test to check behavior in case MPI functions are used in other 
translation units
- added more `MPI_Waitall` tests -> `missingNonBlockingWaitall*`


http://reviews.llvm.org/D12761

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

Index: test/Analysis/MPIChecker.cpp
===
--- test/Analysis/MPIChecker.cpp
+++ test/Analysis/MPIChecker.cpp
@@ -0,0 +1,337 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=optin.mpi.MPI-Checker -verify %s
+
+// MPI-Checker makes no assumptions about details of an MPI implementation.
+// Typedefs and constants are compared as strings.
+
+#include "MPIMock.h"
+
+//integration tests
+
+void matchedWait1() {
+  int rank = 0;
+  double buf = 0;
+  MPI_Comm_rank(MPI_COMM_WORLD, &rank);
+  if (rank >= 0) {
+MPI_Request sendReq1, recvReq1;
+MPI_Isend(&buf, 1, MPI_DOUBLE, rank + 1, 0, MPI_COMM_WORLD, &sendReq1);
+MPI_Irecv(&buf, 1, MPI_DOUBLE, rank - 1, 0, MPI_COMM_WORLD, &recvReq1);
+
+MPI_Wait(&sendReq1, MPI_STATUS_IGNORE);
+MPI_Wait(&recvReq1, MPI_STATUS_IGNORE);
+  }
+} // no error
+
+void matchedWait2() {
+  int rank = 0;
+  double buf = 0;
+  MPI_Comm_rank(MPI_COMM_WORLD, &rank);
+  if (rank >= 0) {
+MPI_Request sendReq1, recvReq1;
+MPI_Isend(&buf, 1, MPI_DOUBLE, rank + 1, 0, MPI_COMM_WORLD, &sendReq1);
+MPI_Irecv(&buf, 1, MPI_DOUBLE, rank - 1, 0, MPI_COMM_WORLD, &recvReq1);
+MPI_Wait(&sendReq1, MPI_STATUS_IGNORE);
+MPI_Wait(&recvReq1, MPI_STATUS_IGNORE);
+  }
+} // no error
+
+void matchedWait3() {
+  int rank = 0;
+  double buf = 0;
+  MPI_Comm_rank(MPI_COMM_WORLD, &rank);
+  if (rank >= 0) {
+MPI_Request sendReq1, recvReq1;
+MPI_Isend(&buf, 1, MPI_DOUBLE, rank + 1, 0, MPI_COMM_WORLD, &sendReq1);
+MPI_Irecv(&buf, 1, MPI_DOUBLE, rank - 1, 0, MPI_COMM_WORLD, &recvReq1);
+
+if (rank > 1000) {
+  MPI_Wait(&sendReq1, MPI_STATUS_IGNORE);
+  MPI_Wait(&recvReq1, MPI_STATUS_IGNORE);
+} else {
+  MPI_Wait(&sendReq1, MPI_STATUS_IGNORE);
+  MPI_Wait(&recvReq1, MPI_STATUS_IGNORE);
+}
+  }
+} // no error
+
+void missingWait1() { // Check missing wait for dead region.
+  double buf = 0;
+  MPI_Request sendReq1;
+  MPI_Ireduce(MPI_IN_PLACE, &buf, 1, MPI_DOUBLE, MPI_SUM, 0, MPI_COMM_WORLD, &sendReq1); // expected-note{{Request is previously used by nonblocking call here.}}
+} // expected-warning{{Request 'sendReq1' has no matching wait.}}
+
+void missingWait2() {
+  int rank = 0;
+  double buf = 0;
+  MPI_Comm_rank(MPI_COMM_WORLD, &rank);
+  if (rank == 0) {
+  } else {
+MPI_Request sendReq1, recvReq1;
+
+MPI_Isend(&buf, 1, MPI_DOUBLE, rank + 1, 0, MPI_COMM_WORLD, &sendReq1);
+MPI_Irecv(&buf, 1, MPI_DOUBLE, rank - 1, 0, MPI_COMM_WORLD, &recvReq1); // expected-warning{{Request 'sendReq1' has no matching wait.}}
+MPI_Wait(&recvReq1, MPI_STATUS_IGNORE);
+  }
+}
+
+void doubleNonblocking() {
+  int rank = 0;
+  double buf = 0;
+  MPI_Comm_rank(MPI_COMM_WORLD, &rank);
+  if (rank == 1) {
+  } else {
+MPI_Request sendReq1;
+
+MPI_Isend(&buf, 1, MPI_DOUBLE, rank + 1, 0, MPI_COMM_WORLD, &sendReq1); // expected-note{{Request is previously used by nonblocking call here. }}
+MPI_Irecv(&buf, 1, MPI_DOUBLE, rank - 1, 0, MPI_COMM_WORLD, &sendReq1); // expected-warning{{Double nonblocking on request 'sendReq1'.}}
+MPI_Wait(&sendReq1, MPI_STATUS_IGNORE);
+  }
+}
+
+void doubleNonblocking2() {
+  int rank = 0;
+  double buf = 0;
+  MPI_Comm_rank(MPI_COMM_WORLD, &rank);
+
+  MPI_Request req;
+  MPI_Ireduce(MPI_IN_PLACE, &buf, 1, MPI_DOUBLE, MPI_SUM, 0, MPI_COMM_WORLD, &req); // expected-note{{Request is previously used by nonblocking call here.}}
+  MPI_Ireduce(MPI_IN_PLACE, &buf, 1, MPI_DOUBLE, MPI_SUM, 0, MPI_COMM_WORLD, &req); // expected-warning{{Double nonblocking on request 'req'.}}
+  MPI_Wait(&req, MPI_STATUS_IGNORE);
+}
+
+void doubleNonblocking3() {
+  typedef struct { MPI_Request req; } ReqStruct;
+
+  ReqStruct rs;
+  int rank = 0;
+  double buf = 0;
+  MPI_Comm_rank(MPI_COMM_WORLD, &rank);
+
+  MPI_Ireduce(MPI_IN_PLACE, &buf, 1, MPI_DOUBLE, MPI_SUM, 0, MPI_COMM_WORLD, &rs.req); // expected-note{{Request is previously used by nonblocking call here.}}
+  MPI_Ireduce(MPI_IN_PLACE, &buf, 1, MPI_DOUBLE, MPI_SUM, 0, MPI_COMM_WORLD, &rs.req)

Re: [PATCH] D17412: PR19957: [OpenCL] incorrectly accepts implicit address space conversion with ternary operator

[Yaxun Liu via cfe-commits]

yaxunl marked 6 inline comments as done.


Comment at: lib/AST/ASTContext.cpp:7613
@@ +7612,3 @@
+if (getLangOpts().OpenCL) {
+  if (LHS.getUnqualifiedType() != RHS.getUnqualifiedType() ||
+  LQuals.getCVRQualifiers() != RQuals.getCVRQualifiers())

Anastasia wrote:
> > Here if unqualified types are different 
> 
> I think this check is redundant considering that we make check of canonical 
> types equivalence in line 7605. Also it doesn't really have anything to do 
> with any OpenCL specific rule. Therefore I would remove this check and just 
> merge with lines 7623 - 7624 as much as possible.
> 
> > or CVS qualifiers are different, the two types cannot be merged
> 
> The same is already being checked in line 7623. Could we merge with that code?
> 
> 
The check for unqualified type is not redundant.

Let's say global int and generic float gets here. If we don't check unqualified 
type, we will get a non-null merged type, which is not correct.

It seems to be cleaner to keep the OpenCL logic separate from line 7623.


http://reviews.llvm.org/D17412



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

Re: [PATCH] D18542: [OPENMP] Parsing and Sema support for 'omp declare target' directive

[Dmitry Polukhin via cfe-commits]

DmitryPolukhin updated this revision to Diff 52070.
DmitryPolukhin added a comment.

- Added test for templates in declare target region


http://reviews.llvm.org/D18542

Files:
  include/clang/AST/ASTMutationListener.h
  include/clang/AST/Attr.h
  include/clang/Basic/Attr.td
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticParseKinds.td
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Basic/OpenMPKinds.def
  include/clang/Sema/Sema.h
  include/clang/Serialization/ASTWriter.h
  lib/AST/ASTContext.cpp
  lib/AST/DeclPrinter.cpp
  lib/Basic/OpenMPKinds.cpp
  lib/Frontend/MultiplexConsumer.cpp
  lib/Parse/ParseOpenMP.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaOpenMP.cpp
  lib/Serialization/ASTCommon.h
  lib/Serialization/ASTReaderDecl.cpp
  lib/Serialization/ASTWriter.cpp
  lib/Serialization/ASTWriterDecl.cpp
  test/OpenMP/declare_target_ast_print.cpp
  test/OpenMP/declare_target_messages.cpp
  utils/TableGen/ClangAttrEmitter.cpp

Index: utils/TableGen/ClangAttrEmitter.cpp
===
--- utils/TableGen/ClangAttrEmitter.cpp
+++ utils/TableGen/ClangAttrEmitter.cpp
@@ -30,28 +30,32 @@
 
 namespace {
 class FlattenedSpelling {
-  std::string V, N, NS;
+  std::string V, N, NS, E;
   bool K;
 
 public:
   FlattenedSpelling(const std::string &Variety, const std::string &Name,
-const std::string &Namespace, bool KnownToGCC) :
-V(Variety), N(Name), NS(Namespace), K(KnownToGCC) {}
+const std::string &Namespace, const std::string &Ending,
+bool KnownToGCC) :
+V(Variety), N(Name), NS(Namespace), E(Ending), K(KnownToGCC) {}
   explicit FlattenedSpelling(const Record &Spelling) :
 V(Spelling.getValueAsString("Variety")),
 N(Spelling.getValueAsString("Name")) {
 
 assert(V != "GCC" && "Given a GCC spelling, which means this hasn't been"
"flattened!");
 if (V == "CXX11" || V == "Pragma")
   NS = Spelling.getValueAsString("Namespace");
+if (V == "Pragma")
+  E = Spelling.getValueAsString("Ending");
 bool Unset;
 K = Spelling.getValueAsBitOrUnset("KnownToGCC", Unset);
   }
 
   const std::string &variety() const { return V; }
   const std::string &name() const { return N; }
   const std::string &nameSpace() const { return NS; }
+  const std::string &ending() const { return E; }
   bool knownToGCC() const { return K; }
 };
 } // end anonymous namespace
@@ -64,8 +68,8 @@
   for (const auto &Spelling : Spellings) {
 if (Spelling->getValueAsString("Variety") == "GCC") {
   // Gin up two new spelling objects to add into the list.
-  Ret.emplace_back("GNU", Spelling->getValueAsString("Name"), "", true);
-  Ret.emplace_back("CXX11", Spelling->getValueAsString("Name"), "gnu",
+  Ret.emplace_back("GNU", Spelling->getValueAsString("Name"), "", "", true);
+  Ret.emplace_back("CXX11", Spelling->getValueAsString("Name"), "gnu", "",
true);
 } else
   Ret.push_back(FlattenedSpelling(*Spelling));
@@ -1259,6 +1263,63 @@
   OS << "}\n\n";
 }
 
+// Determines if an attribute has a Pragma spelling.
+static bool AttrHasPragmaSpelling(const Record *R) {
+  std::vector Spellings = GetFlattenedSpellings(*R);
+  return std::find_if(Spellings.begin(), Spellings.end(),
+  [](const FlattenedSpelling &S) {
+   return S.variety() == "Pragma";
+ }) != Spellings.end();
+}
+
+static void
+writePrettyPrintEndFunction(Record &R,
+const std::vector> &Args,
+raw_ostream &OS) {
+  std::vector Spellings = GetFlattenedSpellings(R);
+
+  OS << "void " << R.getName() << "Attr::printPrettyEnd("
+<< "raw_ostream &OS, const PrintingPolicy &Policy) const {\n";
+
+  if (!AttrHasPragmaSpelling(&R)) {
+OS << "}\n\n";
+return;
+  }
+
+  OS <<
+"  switch (SpellingListIndex) {\n"
+"  default:\n"
+"llvm_unreachable(\"Unknown attribute spelling!\");\n"
+"break;\n";
+
+  for (unsigned I = 0; I < Spellings.size(); ++ I) {
+if (Spellings[I].variety() != "Pragma")
+  continue;
+
+OS << "  case " << I << " : {\n";
+std::string Ending = Spellings[I].ending();
+if (!Ending.empty()) {
+  llvm::SmallString<64> Spelling;
+  std::string Namespace = Spellings[I].nameSpace();
+  if (!Namespace.empty()) {
+Spelling += Namespace;
+Spelling += " ";
+  }
+  Spelling += Ending;
+
+  OS << "OS << \"#pragma "  << Spelling << "\\n\";\n";
+
+}
+OS << "break;\n";
+OS << "  }\n";
+  }
+
+  // End of the switch statement.
+  OS << "}\n";
+  // End of the print function.
+  OS << "}\n\n";
+}
+
 /// \brief Return the index of a spelling in a spelling list.
 static unsigned
 getSpellingListIndex(const std::vector &SpellingList,
@@ -1642,6 +1703,8 @@
 OS << "  " << R.getName() << "Attr *clone(ASTContext &C) const;\n"

Re: [PATCH] D18457: [clang-tidy] Add a new checker to detect missing comma in initializer list.

[Alexander Kornienko via cfe-commits]

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

Looks good with a few comments. Further functionality improvements can be done 
in a follow up, I think.

Please update docs/ReleaseNotes.rst.



Comment at: clang-tidy/misc/SuspiciousMissingCommaCheck.cpp:72
@@ +71,3 @@
+  // The current threshold is set to less than 1/5 of the string literals.
+  if ((RatioThreshold * Count) / Size > 1) return;
+

I'd prefer this to be `double(Count) / Size > RatioThreshold` and 
`RatioThreshold` to be a double.


Comment at: clang-tidy/misc/SuspiciousMissingCommaCheck.cpp:75
@@ +74,3 @@
+  diag(ConcatenatedLiteral->getLocStart(),
+   "suspicious string literal, probably missing a comma");
+}

We need to add a recommendation on how to silence this warning (use 
parentheses, for example). Fine for a follow up.


http://reviews.llvm.org/D18457



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

r264873 - [SystemZ] Specify required features for builtins.

[Jonas Paulsson via cfe-commits]

Author: jonpa
Date: Wed Mar 30 10:51:24 2016
New Revision: 264873

URL: http://llvm.org/viewvc/llvm-project?rev=264873&view=rev
Log:
[SystemZ] Specify required features for builtins.

BuiltinsSystemZ.def is extended to include the required processor
features per intrinsic.

New test test/CodeGen/builtins-systemz-error2.c that checks for
expected errors when instrinsics are used with a subtarget that does
not support the required feature (e.g. vector support).

Reviewed by Ulrich Weigand.

Added:
cfe/trunk/test/CodeGen/builtins-systemz-error2.c
Modified:
cfe/trunk/include/clang/Basic/BuiltinsSystemZ.def
cfe/trunk/lib/Basic/Targets.cpp

Modified: cfe/trunk/include/clang/Basic/BuiltinsSystemZ.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsSystemZ.def?rev=264873&r1=264872&r2=264873&view=diff
==
--- cfe/trunk/include/clang/Basic/BuiltinsSystemZ.def (original)
+++ cfe/trunk/include/clang/Basic/BuiltinsSystemZ.def Wed Mar 30 10:51:24 2016
@@ -14,239 +14,244 @@
 
 // The format of this database matches clang/Basic/Builtins.def.
 
+#if defined(BUILTIN) && !defined(TARGET_BUILTIN)
+#   define TARGET_BUILTIN(ID, TYPE, ATTRS, FEATURE) BUILTIN(ID, TYPE, ATTRS)
+#endif
+
 // Transactional-memory intrinsics
-BUILTIN(__builtin_tbegin, "iv*", "j")
-BUILTIN(__builtin_tbegin_nofloat, "iv*", "j")
-BUILTIN(__builtin_tbeginc, "v", "nj")
-BUILTIN(__builtin_tabort, "vi", "r")
-BUILTIN(__builtin_tend, "i", "n")
-BUILTIN(__builtin_tx_nesting_depth, "i", "nc")
-BUILTIN(__builtin_tx_assist, "vi", "n")
-BUILTIN(__builtin_non_tx_store, "vULi*ULi", "")
+TARGET_BUILTIN(__builtin_tbegin, "iv*", "j", "transactional-execution")
+TARGET_BUILTIN(__builtin_tbegin_nofloat, "iv*", "j", "transactional-execution")
+TARGET_BUILTIN(__builtin_tbeginc, "v", "nj", "transactional-execution")
+TARGET_BUILTIN(__builtin_tabort, "vi", "r", "transactional-execution")
+TARGET_BUILTIN(__builtin_tend, "i", "n", "transactional-execution")
+TARGET_BUILTIN(__builtin_tx_nesting_depth, "i", "nc", 
"transactional-execution")
+TARGET_BUILTIN(__builtin_tx_assist, "vi", "n", "transactional-execution")
+TARGET_BUILTIN(__builtin_non_tx_store, "vULi*ULi", "", 
"transactional-execution")
 
 // Vector intrinsics.
 // These all map directly to z instructions, except that some variants ending
 // in "s" have a final "int *" that receives the post-instruction CC value.
 
 // Vector support instructions (chapter 21 of the PoP)
-BUILTIN(__builtin_s390_lcbb, "UivC*Ii", "nc")
-BUILTIN(__builtin_s390_vlbb, "V16ScvC*Ii", "")
-BUILTIN(__builtin_s390_vll, "V16ScUivC*", "")
-BUILTIN(__builtin_s390_vstl, "vV16ScUiv*", "")
-BUILTIN(__builtin_s390_vperm, "V16UcV16UcV16UcV16Uc", "nc")
-BUILTIN(__builtin_s390_vpdi, "V2ULLiV2ULLiV2ULLiIi", "nc")
-BUILTIN(__builtin_s390_vpksh, "V16ScV8SsV8Ss", "nc")
-BUILTIN(__builtin_s390_vpkshs, "V16ScV8SsV8Ssi*", "nc")
-BUILTIN(__builtin_s390_vpksf, "V8SsV4SiV4Si", "nc")
-BUILTIN(__builtin_s390_vpksfs, "V8SsV4SiV4Sii*", "nc")
-BUILTIN(__builtin_s390_vpksg, "V4SiV2SLLiV2SLLi", "nc")
-BUILTIN(__builtin_s390_vpksgs, "V4SiV2SLLiV2SLLii*", "nc")
-BUILTIN(__builtin_s390_vpklsh, "V16UcV8UsV8Us", "nc")
-BUILTIN(__builtin_s390_vpklshs, "V16UcV8UsV8Usi*", "nc")
-BUILTIN(__builtin_s390_vpklsf, "V8UsV4UiV4Ui", "nc")
-BUILTIN(__builtin_s390_vpklsfs, "V8UsV4UiV4Uii*", "nc")
-BUILTIN(__builtin_s390_vpklsg, "V4UiV2ULLiV2ULLi", "nc")
-BUILTIN(__builtin_s390_vpklsgs, "V4UiV2ULLiV2ULLii*", "nc")
-BUILTIN(__builtin_s390_vuphb, "V8SsV16Sc", "nc")
-BUILTIN(__builtin_s390_vuphh, "V4SiV8Ss", "nc")
-BUILTIN(__builtin_s390_vuphf, "V2SLLiV4Si", "nc")
-BUILTIN(__builtin_s390_vuplb, "V8SsV16Sc", "nc")
-BUILTIN(__builtin_s390_vuplhw, "V4SiV8Ss", "nc")
-BUILTIN(__builtin_s390_vuplf, "V2SLLiV4Si", "nc")
-BUILTIN(__builtin_s390_vuplhb, "V8UsV16Uc", "nc")
-BUILTIN(__builtin_s390_vuplhh, "V4UiV8Us", "nc")
-BUILTIN(__builtin_s390_vuplhf, "V2ULLiV4Ui", "nc")
-BUILTIN(__builtin_s390_vupllb, "V8UsV16Uc", "nc")
-BUILTIN(__builtin_s390_vupllh, "V4UiV8Us", "nc")
-BUILTIN(__builtin_s390_vupllf, "V2ULLiV4Ui", "nc")
+TARGET_BUILTIN(__builtin_s390_lcbb, "UivC*Ii", "nc", "vector")
+TARGET_BUILTIN(__builtin_s390_vlbb, "V16ScvC*Ii", "", "vector")
+TARGET_BUILTIN(__builtin_s390_vll, "V16ScUivC*", "", "vector")
+TARGET_BUILTIN(__builtin_s390_vstl, "vV16ScUiv*", "", "vector")
+TARGET_BUILTIN(__builtin_s390_vperm, "V16UcV16UcV16UcV16Uc", "nc", "vector")
+TARGET_BUILTIN(__builtin_s390_vpdi, "V2ULLiV2ULLiV2ULLiIi", "nc", "vector")
+TARGET_BUILTIN(__builtin_s390_vpksh, "V16ScV8SsV8Ss", "nc", "vector")
+TARGET_BUILTIN(__builtin_s390_vpkshs, "V16ScV8SsV8Ssi*", "nc", "vector")
+TARGET_BUILTIN(__builtin_s390_vpksf, "V8SsV4SiV4Si", "nc", "vector")
+TARGET_BUILTIN(__builtin_s390_vpksfs, "V8SsV4SiV4Sii*", "nc", "vector")
+TARGET_BUILTIN(__builtin_s390_vpksg, "V4SiV2SLLiV2SLLi", "nc", "vector")
+TARGET_BUILTIN(__builtin_s390_vpksgs, "V4SiV2SLLiV2SLLii*", "nc", "vector")
+TARGET_BUILTIN(__builtin_s

Re: [PATCH] D17412: PR19957: [OpenCL] incorrectly accepts implicit address space conversion with ternary operator

[Yaxun Liu via cfe-commits]

yaxunl added inline comments.


Comment at: test/SemaOpenCL/address-spaces-conversions-cl2.0.cl:276
@@ +275,3 @@
+  constant char *arg_const_ch;
+  var_void_const = 0 ? var_cond : arg_const_ch;
+#ifndef CONSTANT

Anastasia wrote:
> btw, what happens if we assign into non void* var? Do we get another error?
No error, since void pointer can be casted to non-void pointer implicitly in C.


http://reviews.llvm.org/D17412



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

Re: [PATCH] D17412: PR19957: [OpenCL] incorrectly accepts implicit address space conversion with ternary operator

[Yaxun Liu via cfe-commits]

yaxunl updated this revision to Diff 52073.
yaxunl marked 6 inline comments as done.
yaxunl added a comment.

Revised by Anastasia's comments.

Added comments to code for cases 1a-c and 2a-c.
Added codegen test for missing cases of 1a-b and 2a-b.
Rename variables in sema test.


http://reviews.llvm.org/D17412

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/AST/ASTContext.cpp
  lib/Sema/SemaExpr.cpp
  test/CodeGenOpenCL/address-spaces-conversions.cl
  test/SemaOpenCL/address-spaces-conversions-cl2.0.cl

Index: test/SemaOpenCL/address-spaces-conversions-cl2.0.cl
===
--- test/SemaOpenCL/address-spaces-conversions-cl2.0.cl
+++ test/SemaOpenCL/address-spaces-conversions-cl2.0.cl
@@ -225,3 +225,69 @@
 // expected-error@-2{{passing '__constant int *' to parameter of type '__generic int *' changes address space of pointer}}
 #endif
 }
+
+void test_ternary() {
+  AS int *var_cond;
+  generic int *var_gen;
+  global int *var_glob;
+  var_gen = 0 ? var_cond : var_glob;
+#ifdef CONSTANT
+// expected-error@-2{{conditional operator with the second and third operands of type  ('__constant int *' and '__global int *') which are pointers to non-overlapping address spaces}}
+#endif
+
+  local int *var_loc;
+  var_gen = 0 ? var_cond : var_loc;
+#ifndef GENERIC
+// expected-error-re@-2{{conditional operator with the second and third operands of type  ('__{{global|constant}} int *' and '__local int *') which are pointers to non-overlapping address spaces}}
+#endif
+
+  constant int *var_const;
+  var_cond = 0 ? var_cond : var_const;
+#ifndef CONSTANT
+// expected-error-re@-2{{conditional operator with the second and third operands of type  ('__{{global|generic}} int *' and '__constant int *') which are pointers to non-overlapping address spaces}}
+#endif
+
+  private int *var_priv;
+  var_gen = 0 ? var_cond : var_priv;
+#ifndef GENERIC
+// expected-error-re@-2{{conditional operator with the second and third operands of type  ('__{{global|constant}} int *' and 'int *') which are pointers to non-overlapping address spaces}}
+#endif
+
+  var_gen = 0 ? var_cond : var_gen;
+#ifdef CONSTANT
+// expected-error@-2{{conditional operator with the second and third operands of type  ('__constant int *' and '__generic int *') which are pointers to non-overlapping address spaces}}
+#endif
+
+  void *var_void_gen;
+  global char *var_glob_ch;
+  var_void_gen = 0 ? var_cond : var_glob_ch;
+#ifdef CONSTANT
+// expected-error@-2{{conditional operator with the second and third operands of type  ('__constant int *' and '__global char *') which are pointers to non-overlapping address spaces}}
+#endif
+
+  local char *var_loc_ch;
+  var_void_gen = 0 ? var_cond : var_loc_ch;
+#ifndef GENERIC
+// expected-error-re@-2{{conditional operator with the second and third operands of type  ('__{{global|constant}} int *' and '__local char *') which are pointers to non-overlapping address spaces}}
+#endif
+
+  constant void *var_void_const;
+  constant char *var_const_ch;
+  var_void_const = 0 ? var_cond : var_const_ch;
+#ifndef CONSTANT
+// expected-error-re@-2{{conditional operator with the second and third operands of type  ('__{{global|generic}} int *' and '__constant char *') which are pointers to non-overlapping address spaces}}
+#endif
+
+  private char *var_priv_ch;
+  var_void_gen = 0 ? var_cond : var_priv_ch;
+#ifndef GENERIC
+// expected-error-re@-2{{conditional operator with the second and third operands of type  ('__{{global|constant}} int *' and 'char *') which are pointers to non-overlapping address spaces}}
+#endif
+
+  generic char *var_gen_ch;
+  var_void_gen = 0 ? var_cond : var_gen_ch;
+#ifdef CONSTANT
+// expected-error@-2{{conditional operator with the second and third operands of type  ('__constant int *' and '__generic char *') which are pointers to non-overlapping address spaces}}
+#endif
+}
+
Index: test/CodeGenOpenCL/address-spaces-conversions.cl
===
--- test/CodeGenOpenCL/address-spaces-conversions.cl
+++ test/CodeGenOpenCL/address-spaces-conversions.cl
@@ -3,20 +3,65 @@
 // test that we generate address space casts everywhere we need conversions of
 // pointers to different address spaces
 
+// CHECK: define void @test
 void test(global int *arg_glob, generic int *arg_gen) {
   int var_priv;
+  
   arg_gen = arg_glob; // implicit cast global -> generic
   // CHECK: %{{[0-9]+}} = addrspacecast i32 addrspace(1)* %{{[0-9]+}} to i32 addrspace(4)*
+  
   arg_gen = &var_priv; // implicit cast with obtaining adr, private -> generic
   // CHECK: %{{[0-9]+}} = addrspacecast i32* %var_priv to i32 addrspace(4)*
+  
   arg_glob = (global int *)arg_gen; // explicit cast
   // CHECK: %{{[0-9]+}} = addrspacecast i32 addrspace(4)* %{{[0-9]+}} to i32 addrspace(1)*
+  
   global int *var_glob =
   (global int *)arg_glob; // explicit cast in the same address space
   // CHECK-NOT: %{{[0-9]+}} = addrspacecast i

Re: [PATCH] D18575: [clang-tidy] New checker to replace deprecated throw() specifications

[don hinton via cfe-commits]

hintonda added a comment.

Sounds like a good idea. I'll add the additional transformations you mentioned 
and remove s/noexcept(true)/noexcept/.



Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:24
@@ +23,3 @@
+   char delimiter) {
+  SmallVector Candidates;
+  AllStrings.split(Candidates, ',');

aaron.ballman wrote:
> Why 5?
No particular reason -- copied basic implementation from 
utils::parseHeaderFileExtensions() which did something similar.


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:33
@@ +32,3 @@
+
+using namespace lexer_utils;
+

aaron.ballman wrote:
> This should not be at file scope; if it really clarifies the code, it should 
> be at function scope where needed.
Will remove/refactor -- was just following the examples I found in other 
checkers.


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:83
@@ +82,3 @@
+BeforeThanCompare isBefore(SM);
+while (isBefore(BeginLoc, CurrentLoc)) {
+  SourceLocation Loc = Tok.getLocation();

aaron.ballman wrote:
> This while loop could use some comments to explain what it is trying to do. 
> As best I can tell, this appears to be looking purely at the text the user 
> wrote to try to determine whether there is a `throw()` or a `noexcept(true)`, 
> but that can be done more clearly with  FunctionType::getExceptionSpecType().
Ah, that helps a lot.  I'll use getExceptionSpecType(), but will still need to 
get the location of the end of the token sequence for replacement purposes, 
e.g., throw() is 3 tokens.


http://reviews.llvm.org/D18575



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

Re: [PATCH] D17142: SystemZ: Check required features when handling builtins

[Jonas Paulsson via cfe-commits]

jonpa added a comment.

Tests updated per instructions, and commited as r264873.


http://reviews.llvm.org/D17142



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

Re: [PATCH] D18457: [clang-tidy] Add a new checker to detect missing comma in initializer list.

[Gábor Horváth via cfe-commits]

xazax.hun added inline comments.


Comment at: clang-tidy/misc/SuspiciousMissingCommaCheck.cpp:75
@@ +74,3 @@
+  diag(ConcatenatedLiteral->getLocStart(),
+   "suspicious string literal, probably missing a comma");
+}

alexfh wrote:
> We need to add a recommendation on how to silence this warning (use 
> parentheses, for example). Fine for a follow up.
A possible way to silence this warning would be to implement a heuristic that 
is available in szdominik's implementation, i.e.: when the size of the array is 
explicitly given by the user and it matches the length of the initializer list 
do not warn.


http://reviews.llvm.org/D18457



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

r264874 - [c-index-test] Delete dead function, NFC

[Vedant Kumar via cfe-commits]

Author: vedantk
Date: Wed Mar 30 11:03:02 2016
New Revision: 264874

URL: http://llvm.org/viewvc/llvm-project?rev=264874&view=rev
Log:
[c-index-test] Delete dead function, NFC

Modified:
cfe/trunk/tools/c-index-test/c-index-test.c

Modified: cfe/trunk/tools/c-index-test/c-index-test.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/c-index-test/c-index-test.c?rev=264874&r1=264873&r2=264874&view=diff
==
--- cfe/trunk/tools/c-index-test/c-index-test.c (original)
+++ cfe/trunk/tools/c-index-test/c-index-test.c Wed Mar 30 11:03:02 2016
@@ -2134,25 +2134,6 @@ void print_completion_contexts(unsigned
   }
 }
 
-int my_stricmp(const char *s1, const char *s2) {
-  while (*s1 && *s2) {
-int c1 = tolower((unsigned char)*s1), c2 = tolower((unsigned char)*s2);
-if (c1 < c2)
-  return -1;
-else if (c1 > c2)
-  return 1;
-
-++s1;
-++s2;
-  }
-  
-  if (*s1)
-return 1;
-  else if (*s2)
-return -1;
-  return 0;
-}
-
 int perform_code_completion(int argc, const char **argv, int timing_only) {
   const char *input = argv[1];
   char *filename = 0;


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

Re: [PATCH] D18596: [MSVC] PR27132: Proper mangling for __unaligned qualifier

[Reid Kleckner via cfe-commits]

rnk added inline comments.


Comment at: include/clang/Basic/AttrDocs.td:1970
@@ +1969,3 @@
+
+This modifier makes sense for IPF targets only; Clang supports proper mangling
+of the variables with ``unaligned`` modifier, but it doesn't affect generated

I don't think "IPF targets" will be understood by readers to mean "Itanium 
targets". I'd just drop everything before the semicolon. At some point, we 
might want to add support `__unaligned __m128*`. Today we'll copy that with 
movaps.


Comment at: lib/Sema/SemaType.cpp:
@@ -5552,1 +5554,3 @@
+  // anything, so this is an exception.
+  if (!isa(Desugared) && (Kind != AttributeList::AT_Unaligned)) {
 if (Type->isMemberPointerType())

aaron.ballman wrote:
> This appears to be untested, but really should have a test case to ensure we 
> don't regress later.
+1, although I recall that someone else from Intel wanted to make sure this 
test case works:
  struct A { void g() __unaligned {} };
I could take it or leave it. I don't think that rejecting this program will 
break very much real code.


http://reviews.llvm.org/D18596



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

Re: [PATCH] D18457: [clang-tidy] Add a new checker to detect missing comma in initializer list.

[Alexander Kornienko via cfe-commits]

alexfh added inline comments.


Comment at: clang-tidy/misc/SuspiciousMissingCommaCheck.cpp:75
@@ +74,3 @@
+  diag(ConcatenatedLiteral->getLocStart(),
+   "suspicious string literal, probably missing a comma");
+}

xazax.hun wrote:
> alexfh wrote:
> > We need to add a recommendation on how to silence this warning (use 
> > parentheses, for example). Fine for a follow up.
> A possible way to silence this warning would be to implement a heuristic that 
> is available in szdominik's implementation, i.e.: when the size of the array 
> is explicitly given by the user and it matches the length of the initializer 
> list do not warn.
The check shouldn't complain in this case, but I wouldn't recommend this as a 
fix, since it makes the code more strict in an inconvenient way.


http://reviews.llvm.org/D18457



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

Re: [PATCH] D18567: Block: Fix a crash when we have type attributes or qualifiers with omitted return type.

[Anastasia Stulova via cfe-commits]

Anastasia added inline comments.


Comment at: test/SemaOpenCL/invalid-block.cl:9
@@ -8,3 +8,3 @@
   int (^const bl2)(); // expected-error{{invalid block variable declaration - 
must be initialized}}
-  int (^const bl3)() = ^const(){
+  int (^const bl3)() = ^(){ // expected-error{{incompatible block pointer 
types initializing 'int (^const)()' with an expression of type 'void 
(^)(void)'}}
   };

I think this test had a bug initially (multiple actually!). It was meant to 
initialize with int(^)() expression. Could you please add an int return so that 
this line has no error. 


Comment at: test/SemaOpenCL/invalid-block.cl:9
@@ -8,3 +8,3 @@
   int (^const bl2)(); // expected-error{{invalid block variable declaration - 
must be initialized}}
-  int (^const bl3)() = ^const(){
+  int (^const bl3)() = ^(){ // expected-error{{incompatible block pointer 
types initializing 'int (^const)()' with an expression of type 'void 
(^)(void)'}}
   };

Anastasia wrote:
> I think this test had a bug initially (multiple actually!). It was meant to 
> initialize with int(^)() expression. Could you please add an int return so 
> that this line has no error. 
I am not getting why this error didn't appear before your change. How does your 
change trigger this error now, as you seem to be only changing the attributes 
and not the deduced block type...


Comment at: test/SemaOpenCL/invalid-block.cl:14
@@ -13,3 +13,3 @@
 // A block with extern storage class is not allowed.
-extern int (^const bl)() = ^const(){}; // expected-error{{invalid block 
variable declaration - using 'extern' storage class is disallowed}}
+extern int (^const bl)() = ^(){}; // expected-error{{invalid block variable 
declaration - using 'extern' storage class is disallowed}}
 void f2() {

The same here, line 7 and 16. Could we return int please.


Comment at: test/SemaOpenCL/invalid-block.cl:32
@@ -31,3 +31,3 @@
 // A block with variadic argument is not allowed.
 int (^const bl)(int, ...) = ^const int(int I, ...) { // expected-error 
{{invalid block prototype, variadic arguments are not allowed in OpenCL}}
   return 0;

could you remove second const please?


Comment at: test/SemaOpenCL/invalid-block.cl:39
@@ -38,3 +38,3 @@
 void f5(int i) {
-  bl1_t bl1 = ^const(int i) {return 1;};
-  bl1_t bl2 = ^const(int i) {return 2;};
+  bl1_t bl1 = ^(int i) {return 1;};
+  bl1_t bl2 = ^(int i) {return 2;};

So the return type is deduced to be int for this block expression here?


http://reviews.llvm.org/D18567



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

Re: [PATCH] D17861: [OpenCL] Accept __attribute__((nosvm))

[Yaxun Liu via cfe-commits]

yaxunl updated this revision to Diff 52077.
yaxunl marked an inline comment as done.
yaxunl added a comment.

Insert an empty line to the test for clarity.


http://reviews.llvm.org/D17861

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaDeclAttr.cpp
  lib/Sema/SemaStmtAttr.cpp
  test/SemaOpenCL/nosvm.cl

Index: test/SemaOpenCL/nosvm.cl
===
--- /dev/null
+++ test/SemaOpenCL/nosvm.cl
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -verify %s
+// RUN: %clang_cc1 -verify -cl-std=CL2.0 -D CL20 %s
+// RUN: %clang_cc1 -verify -x c -D NOCL %s
+
+#ifndef NOCL
+kernel void f(__attribute__((nosvm)) global int* a);
+#ifndef CL20
+// expected-error@-2 {{'nosvm' attribute requires OpenCL version 2.0}}
+#else
+// expected-warning@-4 {{'nosvm' attribute is deprecated and ignored in OpenCL version 2.0}}
+#endif
+
+__attribute__((nosvm)) void g(); // expected-warning {{'nosvm' attribute only applies to variables}}
+
+#else
+void f(__attribute__((nosvm)) int* a); // expected-warning {{'nosvm' attribute ignored}}
+#endif
Index: lib/Sema/SemaStmtAttr.cpp
===
--- lib/Sema/SemaStmtAttr.cpp
+++ lib/Sema/SemaStmtAttr.cpp
@@ -221,7 +221,7 @@
 
   if (S.getLangOpts().OpenCLVersion < 200) {
 S.Diag(A.getLoc(), diag::err_attribute_requires_opencl_version)
-<< A.getName() << "2.0";
+<< A.getName() << "2.0" << 1;
 return nullptr;
   }
 
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -5206,6 +5206,15 @@
 D->addAttr(Internal);
 }
 
+static void handleOpenCLNoSVMAttr(Sema &S, Decl *D, const AttributeList &Attr) {
+  if (S.LangOpts.OpenCLVersion != 200)
+S.Diag(Attr.getLoc(), diag::err_attribute_requires_opencl_version)
+<< Attr.getName() << "2.0" << 0;
+  else
+S.Diag(Attr.getLoc(), diag::warn_opencl_attr_deprecated_ignored)
+<< Attr.getName() << "2.0";
+}
+
 /// Handles semantic checking for features that are common to all attributes,
 /// such as checking whether a parameter was properly specified, or the correct
 /// number of arguments were passed, etc.
@@ -5698,6 +5707,9 @@
   case AttributeList::AT_SwiftIndirectResult:
 handleParameterABIAttr(S, D, Attr, ParameterABI::SwiftIndirectResult);
 break;
+  case AttributeList::AT_OpenCLNoSVM:
+handleOpenCLNoSVMAttr(S, D, Attr);
+break;
   case AttributeList::AT_InternalLinkage:
 handleInternalLinkageAttr(S, D, Attr);
 break;
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -2129,7 +2129,7 @@
 def err_attribute_requires_positive_integer : Error<
   "%0 attribute requires a positive integral compile time constant expression">;
 def err_attribute_requires_opencl_version : Error<
-  "%0 attribute requires OpenCL version %1 or above">;
+  "%0 attribute requires OpenCL version %1%select{| or above}2">;
 def warn_unsupported_target_attribute
 : Warning<"Ignoring unsupported '%0' in the target attribute string">,
 InGroup;
@@ -7769,6 +7769,9 @@
   "pointer to type %0 is invalid in OpenCL">;
 def err_opencl_type_can_only_be_used_as_function_parameter : Error <
   "type %0 can only be used as a function parameter in OpenCL">;
+def warn_opencl_attr_deprecated_ignored : Warning <
+  "%0 attribute is deprecated and ignored in OpenCL version %1">,
+  InGroup;
 
 // OpenCL v2.0 s6.13.6 -- Builtin Pipe Functions
 def err_opencl_builtin_pipe_first_arg : Error<
Index: include/clang/Basic/AttrDocs.td
===
--- include/clang/Basic/AttrDocs.td
+++ include/clang/Basic/AttrDocs.td
@@ -1745,6 +1745,17 @@
   }];
 }
 
+def OpenCLNoSVMDocs : Documentation {
+  let Category = DocCatVariable;
+  let Content = [{
+OpenCL 2.0 supports the optional ``__attribute__((nosvm))`` qualifier for
+pointer variable. It informs the compiler that the pointer does not refer
+to a shared virtual memory region. See OpenCL v2.0 s6.7.2 for details.
+
+Since it is not widely used and has been removed from OpenCL 2.1, it is ignored
+by Clang.
+  }];
+}
 def NullabilityDocs : DocumentationCategory<"Nullability Attributes"> {
   let Content = [{
 Whether a particular pointer may be "null" is an important concern when working with pointers in the C family of languages. The various nullability attributes indicate whether a particular pointer can be null or not, which makes APIs more expressive and can help static analysis tools identify bugs involving null pointers. Clang supports several kinds of nullability attributes: the ``nonnull`` and ``returns_nonnull`` attributes indicate which function or method parameters and result

Re: [PATCH] D17412: PR19957: [OpenCL] incorrectly accepts implicit address space conversion with ternary operator

[Anastasia Stulova via cfe-commits]

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

LGTM, please correct two small issues commented here!



Comment at: lib/Sema/SemaExpr.cpp:169
@@ -168,4 +168,3 @@
 break;
- 
   bool Warn = !D->getAttr()->isInherited();
   // Objective-C method declarations in categories are not modelled as

Why removing line here?


Comment at: lib/Sema/SemaExpr.cpp:6192
@@ -6181,2 +6191,3 @@
 
+  // Cases 1c, 2a, 2b, and 2c.
   if (CompositeTy.isNull()) {

add OpenCL


http://reviews.llvm.org/D17412



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

Re: [PATCH] D17811: [clang-tidy] Add check to detect dangling references in value handlers.

[Richard via cfe-commits]

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

Can you add a synopsis of this new check to `docs/ReleaseNotes.rst` please?


Repository:
  rL LLVM

http://reviews.llvm.org/D17811



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

Re: [PATCH] D18408: readability check for const params in declarations

[Richard via cfe-commits]

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

You can submit the release notes changes in a new patch.


Repository:
  rL LLVM

http://reviews.llvm.org/D18408



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

Re: [PATCH] D18265: [clang-tidy] New: checker misc-assign-operator-return

[Richard via cfe-commits]

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

Please summarize this check in `docs/ReleaseNotes.rst`.

There is a pending review that will stub this out automatically, but it hasn't 
been approved yet


http://reviews.llvm.org/D18265



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

Re: [PATCH] D18408: readability check for const params in declarations

[Matt Kulukundis via cfe-commits]

fowles added a comment.

that is what I was trying to do.  I can't seem to make arc play nice.


Repository:
  rL LLVM

http://reviews.llvm.org/D18408



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

Re: [PATCH] D18396: Clang-tidy:modernize-use-override. Fix for __declspec attributes and const=0 without spacse

[Richard via cfe-commits]

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

Please summarize the improvements in `docs/ReleaseNotes.rst` in the clang tidy 
section.


http://reviews.llvm.org/D18396



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

Re: [PATCH] D18136: boost-use-to-string check

[Richard via cfe-commits]

LegalizeAdulthood added a comment.

Please summarize this check in `docs/ReleaseNotes.rst` in the clang-tidy 
section.


http://reviews.llvm.org/D18136



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

Re: [PATCH] D17412: PR19957: [OpenCL] incorrectly accepts implicit address space conversion with ternary operator

[Yaxun Liu via cfe-commits]

yaxunl updated this revision to Diff 52080.
yaxunl marked 3 inline comments as done.
yaxunl added a comment.

Add back a blank line deleted by accident.
Add OpenCL to a comment.


http://reviews.llvm.org/D17412

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/AST/ASTContext.cpp
  lib/Sema/SemaExpr.cpp
  test/CodeGenOpenCL/address-spaces-conversions.cl
  test/SemaOpenCL/address-spaces-conversions-cl2.0.cl

Index: test/SemaOpenCL/address-spaces-conversions-cl2.0.cl
===
--- test/SemaOpenCL/address-spaces-conversions-cl2.0.cl
+++ test/SemaOpenCL/address-spaces-conversions-cl2.0.cl
@@ -225,3 +225,69 @@
 // expected-error@-2{{passing '__constant int *' to parameter of type '__generic int *' changes address space of pointer}}
 #endif
 }
+
+void test_ternary() {
+  AS int *var_cond;
+  generic int *var_gen;
+  global int *var_glob;
+  var_gen = 0 ? var_cond : var_glob;
+#ifdef CONSTANT
+// expected-error@-2{{conditional operator with the second and third operands of type  ('__constant int *' and '__global int *') which are pointers to non-overlapping address spaces}}
+#endif
+
+  local int *var_loc;
+  var_gen = 0 ? var_cond : var_loc;
+#ifndef GENERIC
+// expected-error-re@-2{{conditional operator with the second and third operands of type  ('__{{global|constant}} int *' and '__local int *') which are pointers to non-overlapping address spaces}}
+#endif
+
+  constant int *var_const;
+  var_cond = 0 ? var_cond : var_const;
+#ifndef CONSTANT
+// expected-error-re@-2{{conditional operator with the second and third operands of type  ('__{{global|generic}} int *' and '__constant int *') which are pointers to non-overlapping address spaces}}
+#endif
+
+  private int *var_priv;
+  var_gen = 0 ? var_cond : var_priv;
+#ifndef GENERIC
+// expected-error-re@-2{{conditional operator with the second and third operands of type  ('__{{global|constant}} int *' and 'int *') which are pointers to non-overlapping address spaces}}
+#endif
+
+  var_gen = 0 ? var_cond : var_gen;
+#ifdef CONSTANT
+// expected-error@-2{{conditional operator with the second and third operands of type  ('__constant int *' and '__generic int *') which are pointers to non-overlapping address spaces}}
+#endif
+
+  void *var_void_gen;
+  global char *var_glob_ch;
+  var_void_gen = 0 ? var_cond : var_glob_ch;
+#ifdef CONSTANT
+// expected-error@-2{{conditional operator with the second and third operands of type  ('__constant int *' and '__global char *') which are pointers to non-overlapping address spaces}}
+#endif
+
+  local char *var_loc_ch;
+  var_void_gen = 0 ? var_cond : var_loc_ch;
+#ifndef GENERIC
+// expected-error-re@-2{{conditional operator with the second and third operands of type  ('__{{global|constant}} int *' and '__local char *') which are pointers to non-overlapping address spaces}}
+#endif
+
+  constant void *var_void_const;
+  constant char *var_const_ch;
+  var_void_const = 0 ? var_cond : var_const_ch;
+#ifndef CONSTANT
+// expected-error-re@-2{{conditional operator with the second and third operands of type  ('__{{global|generic}} int *' and '__constant char *') which are pointers to non-overlapping address spaces}}
+#endif
+
+  private char *var_priv_ch;
+  var_void_gen = 0 ? var_cond : var_priv_ch;
+#ifndef GENERIC
+// expected-error-re@-2{{conditional operator with the second and third operands of type  ('__{{global|constant}} int *' and 'char *') which are pointers to non-overlapping address spaces}}
+#endif
+
+  generic char *var_gen_ch;
+  var_void_gen = 0 ? var_cond : var_gen_ch;
+#ifdef CONSTANT
+// expected-error@-2{{conditional operator with the second and third operands of type  ('__constant int *' and '__generic char *') which are pointers to non-overlapping address spaces}}
+#endif
+}
+
Index: test/CodeGenOpenCL/address-spaces-conversions.cl
===
--- test/CodeGenOpenCL/address-spaces-conversions.cl
+++ test/CodeGenOpenCL/address-spaces-conversions.cl
@@ -3,20 +3,65 @@
 // test that we generate address space casts everywhere we need conversions of
 // pointers to different address spaces
 
+// CHECK: define void @test
 void test(global int *arg_glob, generic int *arg_gen) {
   int var_priv;
+  
   arg_gen = arg_glob; // implicit cast global -> generic
   // CHECK: %{{[0-9]+}} = addrspacecast i32 addrspace(1)* %{{[0-9]+}} to i32 addrspace(4)*
+  
   arg_gen = &var_priv; // implicit cast with obtaining adr, private -> generic
   // CHECK: %{{[0-9]+}} = addrspacecast i32* %var_priv to i32 addrspace(4)*
+  
   arg_glob = (global int *)arg_gen; // explicit cast
   // CHECK: %{{[0-9]+}} = addrspacecast i32 addrspace(4)* %{{[0-9]+}} to i32 addrspace(1)*
+  
   global int *var_glob =
   (global int *)arg_glob; // explicit cast in the same address space
   // CHECK-NOT: %{{[0-9]+}} = addrspacecast i32 addrspace(1)* %{{[0-9]+}} to i32 addrspace(1)*
+  
   var_priv = arg_gen - arg_glob; // arithmeti

Re: [PATCH] D18408: readability check for const params in declarations

[Richard via cfe-commits]

LegalizeAdulthood added a comment.

Maybe you need to rebase first?  I haven't used arc.


Repository:
  rL LLVM

http://reviews.llvm.org/D18408



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

Re: [PATCH] D18457: [clang-tidy] Add a new checker to detect missing comma in initializer list.

[Richard via cfe-commits]

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

Can you summarize this check in `docs/ReleaseNotes.rst` in the clang-tidy 
section, please?


http://reviews.llvm.org/D18457



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

Re: [PATCH] D18475: [clang-tidy] Add more detection rules for redundant c_str calls.

[Richard via cfe-commits]

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

Can you summarize the improvements in `docs/ReleaseNotes.rst` please?


http://reviews.llvm.org/D18475



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

Re: [PATCH] D18582: [Clang-tidy] Update release notes with list of checks added since 3.8

[Richard via cfe-commits]

LegalizeAdulthood added inline comments.


Comment at: docs/ReleaseNotes.rst:143
@@ -71,3 +142,3 @@
 Improvements to ``modularize``
 ^^
 

The formatting here has been changed on trunk.  Please rebase with a full file 
context so we can see the whole file in phabricator.


Repository:
  rL LLVM

http://reviews.llvm.org/D18582



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

Re: [PATCH] D18584: Complete support for C++ Core Guidelines Type.6: Always initialize a member variable.

[Richard via cfe-commits]

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

Can you summarize the improvements in `docs/ReleaseNotes.rst`, please?


http://reviews.llvm.org/D18584



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

[PATCH] D18608: note for top-level consts in function decls tidy

[Matt Kulukundis via cfe-commits]

fowles created this revision.
fowles added a reviewer: alexfh.
fowles added a subscriber: cfe-commits.

Add missing release note

http://reviews.llvm.org/D18608

Files:
  docs/ReleaseNotes.rst

Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -150,6 +150,8 @@
 
 - New checks flagging various readability-related issues:
 
+  * `readability-avoid-const-params-in-decls
+
`_
   * `readability-identifier-naming
 
`_
   * `readability-implicit-bool-cast


Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -150,6 +150,8 @@
 
 - New checks flagging various readability-related issues:
 
+  * `readability-avoid-const-params-in-decls
+`_
   * `readability-identifier-naming
 `_
   * `readability-implicit-bool-cast
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D18408: readability check for const params in declarations

[Matthew Fowles Kulukundis via cfe-commits]

Helped by sbenza@ http://reviews.llvm.org/D18608

On Wed, Mar 30, 2016 at 12:54 PM, Richard  wrote:

> LegalizeAdulthood added a comment.
>
> Maybe you need to rebase first?  I haven't used arc.
>
>
> Repository:
>   rL LLVM
>
> http://reviews.llvm.org/D18408
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D18408: readability check for const params in declarations

[Matt Kulukundis via cfe-commits]

fowles added a comment.

Helped by sbenza@ http://reviews.llvm.org/D18608


Repository:
  rL LLVM

http://reviews.llvm.org/D18408



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

Re: [PATCH] D18596: [MSVC] PR27132: Proper mangling for __unaligned qualifier

[David Majnemer via cfe-commits]

majnemer added a comment.

I didn't implement a mangling for `__unaligned` because our implementation of 
it is broken.
It should not be modeled as an attribute, it should be modeled as a qualifier 
because it is possible to overload on it.


http://reviews.llvm.org/D18596



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

Re: [PATCH] D18565: Implement an "I'm dtrace, please retain all debug types" option.

[Adrian Prantl via cfe-commits]

> On Mar 29, 2016, at 10:06 PM, David Blaikie  wrote:
> 
> 
> 
> On Tue, Mar 29, 2016 at 12:03 PM, Adrian Prantl via cfe-commits 
> mailto:cfe-commits@lists.llvm.org>> wrote:
> 
> > On Mar 29, 2016, at 12:00 PM, Joerg Sonnenberger  > > wrote:
> >
> > On Tue, Mar 29, 2016 at 06:47:24PM +, Adrian Prantl via cfe-commits 
> > wrote:
> >> This code in this patch listens to the driver option -gfull, and lowers it 
> >> to the new cc1 option -debug-retain-types (1).
> >> When -debug-retain-types is present, CGDebugInfo will retain every(2) type 
> >> it creates.
> >
> > Is there a good reason for calling it -gfull? I would find something
> > -gall-types or -gretain-all-types to make a lot more sense. This should
> > be orthogonal to other options like providing only line tables?
> 
> My thinking was this:
> The driver already supports -gfull, but it doesn’t do anything.
> This patch can be considered a first step towards making -gfull behave as 
> expected.
> Eventually it should emit debug info for *all* types.
> 
> Seems somewhat problematic to half implement it, though. (admittedly we're 
> just silently ignoring it right now)

I don’t think this is problematic at all. This is incremental development.

> 
> & is 'real' -gfull what dtrace really wants? (seems it isn't - since clang's 
> never really implemented it?)

Admitted, ‘real' -gfull is probably more than it absolutely needs.

> Emitting all types referenced by used (even if later optimized away) code 
> seems like the thing? -greferenced? or maybe a -f flag? Not sure.

I don’t see a compelling case for adding another driver option to the already 
confusing zoo of existing driver options. Note that we currently also accept a 
-gused option which according to the driver code is supposed to be the opposite 
of -gfull. Adding a -greferenced option IMO will only make this more confusion 
instead of helping.
My suggestion is to have -gfull (also) activate -debug-retain-types. In the 
somewhat hypothetical scenario that someone implements a more comprehensive 
version of -gfull we should revisit this and analyze whether the resulting 
debug information is really too large to be practical, and if we conclude that 
this is a problem, we can still decide to expose -debug-retain-types to the 
driver with a new separate option.

-- adrian

>  
> 
> > Joerg
> >
> > PS: Slightly related side question, do we have any tools for extracting
> > a given list of types for retaining? Either by name or global variable
> > expression.
> 
> Extract them from where? Can you give an example?
> 
> -- adrian
> ___
> 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] D16044: getDescriptiveName() for MemRegion

[Alexander Droste via cfe-commits]

Alexander_Droste added inline comments.


Comment at: lib/StaticAnalyzer/Core/MemRegion.cpp:653
@@ +652,3 @@
+// name by calling 'getDescriptiveName' recursively.
+else {
+  std::string Idx = ER->getDescriptiveName(false);

I wasn't able to build a test case yet for which the analyzer could not 
determine the constant value. Is there a way to trick the analyzer so that the 
else case is used ? Then I could test for something like `'sendReq1[a][7][b]'`.


http://reviews.llvm.org/D16044



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

Re: [PATCH] D18565: Implement an "I'm dtrace, please retain all debug types" option.

[Adrian Prantl via cfe-commits]

> On Mar 30, 2016, at 12:50 AM, Joerg Sonnenberger via cfe-commits 
>  wrote:
> 
> On Tue, Mar 29, 2016 at 12:03:46PM -0700, Adrian Prantl via cfe-commits wrote:
>> 
>>> On Mar 29, 2016, at 12:00 PM, Joerg Sonnenberger  
>>> wrote:
>>> 
>>> On Tue, Mar 29, 2016 at 06:47:24PM +, Adrian Prantl via cfe-commits 
>>> wrote:
 This code in this patch listens to the driver option -gfull, and lowers it 
 to the new cc1 option -debug-retain-types (1).
 When -debug-retain-types is present, CGDebugInfo will retain every(2) type 
 it creates.
>>> 
>>> Is there a good reason for calling it -gfull? I would find something
>>> -gall-types or -gretain-all-types to make a lot more sense. This should
>>> be orthogonal to other options like providing only line tables?
>> 
>> My thinking was this:
>> The driver already supports -gfull, but it doesn’t do anything.
>> This patch can be considered a first step towards making -gfull behave as 
>> expected.
>> Eventually it should emit debug info for *all* types.
> 
> -gfull can still be an option group, including -gall-types or whatever.
> 
>>> PS: Slightly related side question, do we have any tools for extracting
>>> a given list of types for retaining? Either by name or global variable
>>> expression.
>> 
>> Extract them from where? Can you give an example?
> 
> Let's take a kernel as example. I want to include certain types for
> dtrace-like use with /dev/kmem, but not all the 100KB+ of boring types.
> Enough for doing some basic post-mortem debugging even on embedded
> systems.

If I understand this correctly, you want to pass a list of “interesting” types 
to clang that are guaranteed to make it into the debug info? At the moment this 
is not directly possible. What you could do is collect all the interesting 
types in a header file and then build a Clang module (using -gmodules) from 
that header. The resulting .pcm will be and object file that has DWARF for all 
types in the module.

-- adrian

> 
> Joerg
> ___
> 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] D16044: getDescriptiveName() for MemRegion

[Gábor Horváth via cfe-commits]

xazax.hun added inline comments.


Comment at: lib/StaticAnalyzer/Core/MemRegion.cpp:653
@@ +652,3 @@
+// name by calling 'getDescriptiveName' recursively.
+else {
+  std::string Idx = ER->getDescriptiveName(false);

Alexander_Droste wrote:
> I wasn't able to build a test case yet for which the analyzer could not 
> determine the constant value. Is there a way to trick the analyzer so that 
> the else case is used ? Then I could test for something like 
> `'sendReq1[a][7][b]'`.
You can try use a value returned from a function that has an unknown body. E.g.:

int getUnknown();

void f() {
  int a = getUnKnown();
}


http://reviews.llvm.org/D16044



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

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

[Anna Zaks via cfe-commits]

zaks.anna added inline comments.


Comment at: lib/StaticAnalyzer/Core/MemRegion.cpp:653
@@ +652,3 @@
+// name by calling 'getDescriptiveName' recursively.
+else {
+  std::string Idx = ER->getDescriptiveName(false);

xazax.hun wrote:
> Alexander_Droste wrote:
> > I wasn't able to build a test case yet for which the analyzer could not 
> > determine the constant value. Is there a way to trick the analyzer so that 
> > the else case is used ? Then I could test for something like 
> > `'sendReq1[a][7][b]'`.
> You can try use a value returned from a function that has an unknown body. 
> E.g.:
> 
> int getUnknown();
> 
> void f() {
>   int a = getUnKnown();
> }
What happens when you try 'sendReq1[a][7][b]'? Does it know the values for "a" 
and "b" for some reason? If 'a' would be an input parameter and the analyzer 
did not see a call site, it won't know the value of 'a'.


Comment at: test/Analysis/MemRegion.cpp:3
@@ +2,3 @@
+
+#include "MPIMock.h"
+

Alexander_Droste wrote:
> The problem about these tests is that they introduce a cyclic commit 
> dependency. MPI-Checker depends on `getDescriptiveName`. `getDescriptiveName` 
> depends on MPI-Checker because of the tests.
> 
> Further, MPI-Checker depends on this function:
> 
> ```
> SourceRange MemRegion::sourceRange() const {
>   const VarRegion *const VR = dyn_cast(this->getBaseRegion());
>   const FieldRegion *const FR = dyn_cast(this);
>   const CXXBaseObjectRegion*const COR = dyn_cast(this);
> 
>   // Check for more specific regions first.
>   // FieldRegion
>   if (FR) {
> return FR->getDecl()->getSourceRange();
>   }
>   // CXXBaseObjectRegion
>   else if (COR) {
> return COR->getDecl()->getSourceRange();
>   }
>   // VarRegion
>   else if (VR) {
> return VR->getDecl()->getSourceRange();
>   }
>   // Return invalid source range (can be checked by client).
>   else {
> return SourceRange{};
>   }
> }
> ```
> Initially, my idea was to submit the `sourceRange` patch after 
> `getDescriptiveName`. Maybe it would be most convenient, to include the 
> `sourceRange` function into this patch and finally commit all connected 
> patches in one go.
> getDescriptiveName depends on MPI-Checker because of the tests.

Would committing the tests separately fix the  cyclic commit dependency 
problem? (Though, I'd still prefer to review them along with this patch.)


http://reviews.llvm.org/D16044



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

Re: [PATCH] D15031: CFG: Add CFGElement for automatic variables that leave the scope

[Richard Smith via cfe-commits]

rsmith added a comment.

Please add a test that builds and dumps a CFG and checks it has the right 
shape. It's generally not sufficient for the only tests for one LLVM project to 
exist in a different LLVM project. See test/Analysis/cfg.cpp for an example of 
how to do this.



Comment at: include/clang/Analysis/CFG.h:170
@@ -168,1 +169,3 @@
 
+class CFGAutomaticObjLeavesScope : public CFGElement {
+public:

What is this intended to model? There seem to be a few options here:

 1) The point at which the destructor would run if the object had a non-trivial 
destructor
 2) The point at which the storage duration for the underlying storage ends
 3) The point at which the lifetime of the object ends

Note that these are all different -- for an object with a non-trivial 
destructor, (1) and (3) are the same and for any other object (2) and (3) are 
the same; (2) occurs after all destructors for the scope have run.

This matters for cases like:

  void f() {
struct A { int *p; ~A() { *p = 0; } } a;
int n;
a.p = &n;
  }

Here, the lifetime of `n` would end before the lifetime of `a` if `n` had a 
non-trivial destructor. If `n`s lifetime actually ended there, this code would 
have undefined behavior. But because the destruction of `n` is trivial, the 
lifetime of `n` instead ends when its storage duration ends, which is *after* 
`A`'s destructor runs, so the code is valid.

Please add a comment to this class to describe what it means.


Comment at: include/clang/Analysis/CFG.h:728
@@ +727,3 @@
+
+  // Scope leaving must be performed in reversed order. So insertion is in two
+  // steps. First we prepare space for some number of elements, then we insert

Why must reverse order be used? It's not possible for the effects of these 
operations to interact with each other, is it? At least according to the C++ 
standard, the "end of storage duration" effects all occur simultaneously.


Comment at: lib/Analysis/CFG.cpp:1233
@@ +1232,3 @@
+
+/// addAutomaticObjLeavesScope - Add to current block automatic objects thats 
leave the scope.
+void CFGBuilder::addAutomaticObjLeavesScope(LocalScope::const_iterator B,

thats -> that


Comment at: lib/Analysis/CFG.cpp:1447
@@ -1389,1 +1446,3 @@
+  }
+return Scope;
   }

This will sometimes bail out without adding a scope; if both 
`AddAutomaticObjLeavesScope` and `AddImplicitDtors` are set, this seems like 
the wrong behavior. You should check for `AddAutomaticObjLeavesScope` first and 
unconditionally ensure there is a scope for that case.


Comment at: lib/Analysis/CFG.cpp:1490-1491
@@ +1489,4 @@
+/// destructors call site.
+/// FIXME: This mechanism for adding automatic destructors doesn't handle
+/// no-return destructors properly.
+void CFGBuilder::prependAutomaticObjLeavesScopeWithTerminator(CFGBlock *Blk,

This is not a mechanism for adding automatic destructors. This FIXME seems 
unnecessary.


Comment at: lib/Analysis/CFG.cpp:1501
@@ +1500,3 @@
+InsertPos = Blk->insertAutomaticObjLeavesScope(InsertPos, *I,
+Blk->getTerminator());
+}

Bad indentation


http://reviews.llvm.org/D15031



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

Re: [PATCH] D18582: [Clang-tidy] Update release notes with list of checks added since 3.8

[Eugene Zelenko via cfe-commits]

Eugene.Zelenko added a comment.

Which style I should use? Suggested by Richard or just provide links to 
documentation as in updated version?


Repository:
  rL LLVM

http://reviews.llvm.org/D18582



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

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

[Alexander Droste via cfe-commits]

Alexander_Droste added inline comments.


Comment at: lib/StaticAnalyzer/Core/MemRegion.cpp:653
@@ +652,3 @@
+// name by calling 'getDescriptiveName' recursively.
+else {
+  std::string Idx = ER->getDescriptiveName(false);

zaks.anna wrote:
> xazax.hun wrote:
> > Alexander_Droste wrote:
> > > I wasn't able to build a test case yet for which the analyzer could not 
> > > determine the constant value. Is there a way to trick the analyzer so 
> > > that the else case is used ? Then I could test for something like 
> > > `'sendReq1[a][7][b]'`.
> > You can try use a value returned from a function that has an unknown body. 
> > E.g.:
> > 
> > int getUnknown();
> > 
> > void f() {
> >   int a = getUnKnown();
> > }
> What happens when you try 'sendReq1[a][7][b]'? Does it know the values for 
> "a" and "b" for some reason? If 'a' would be an input parameter and the 
> analyzer did not see a call site, it won't know the value of 'a'.
If the return value of a function is used for which the body is not known Clang 
crashes.

```
  int getUnknown(void);
  int idxA = getUnknown();
  MPI_Request sendReq1[10][10][10];
  MPI_Wait(&sendReq1[1][idxA][9], MPI_STATUS_IGNORE); // 
expected-warning{{Request 'sendReq1[1][7][9]' has no matching nonblocking 
call.}}
```
Clang also crashes if the index-variable is not initialized.

```
  int idxA;
  MPI_Request sendReq1[10][10][10];
  MPI_Wait(&sendReq1[1][idxA][9], MPI_STATUS_IGNORE); // 
expected-warning{{Request 'sendReq1[1][7][9]' has no matching nonblocking 
call.}}
```
In case the variable is initialized with a constant, the `ConcreteInt` is 
determined.


Comment at: test/Analysis/MemRegion.cpp:3
@@ +2,3 @@
+
+#include "MPIMock.h"
+

zaks.anna wrote:
> Alexander_Droste wrote:
> > The problem about these tests is that they introduce a cyclic commit 
> > dependency. MPI-Checker depends on `getDescriptiveName`. 
> > `getDescriptiveName` depends on MPI-Checker because of the tests.
> > 
> > Further, MPI-Checker depends on this function:
> > 
> > ```
> > SourceRange MemRegion::sourceRange() const {
> >   const VarRegion *const VR = dyn_cast(this->getBaseRegion());
> >   const FieldRegion *const FR = dyn_cast(this);
> >   const CXXBaseObjectRegion*const COR = dyn_cast(this);
> > 
> >   // Check for more specific regions first.
> >   // FieldRegion
> >   if (FR) {
> > return FR->getDecl()->getSourceRange();
> >   }
> >   // CXXBaseObjectRegion
> >   else if (COR) {
> > return COR->getDecl()->getSourceRange();
> >   }
> >   // VarRegion
> >   else if (VR) {
> > return VR->getDecl()->getSourceRange();
> >   }
> >   // Return invalid source range (can be checked by client).
> >   else {
> > return SourceRange{};
> >   }
> > }
> > ```
> > Initially, my idea was to submit the `sourceRange` patch after 
> > `getDescriptiveName`. Maybe it would be most convenient, to include the 
> > `sourceRange` function into this patch and finally commit all connected 
> > patches in one go.
> > getDescriptiveName depends on MPI-Checker because of the tests.
> 
> Would committing the tests separately fix the  cyclic commit dependency 
> problem? (Though, I'd still prefer to review them along with this patch.)
Yes, that would fix the cyclic commit dependency.


http://reviews.llvm.org/D16044



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

Re: [PATCH] D18557: [Clang][ARM] __va_list declaration is not saved in ASTContext causing compilation error or crash

[Oleg Ranevskyy via cfe-commits]

iid_iunknown updated this revision to Diff 52092.
iid_iunknown added a comment.

Test case added.


Repository:
  rL LLVM

http://reviews.llvm.org/D18557

Files:
  lib/AST/ASTContext.cpp
  test/PCH/Inputs/arm-__va_list_tag.h
  test/PCH/arm-__va_list_tag.c

Index: test/PCH/arm-__va_list_tag.c
===
--- test/PCH/arm-__va_list_tag.c
+++ test/PCH/arm-__va_list_tag.c
@@ -0,0 +1,16 @@
+// REQUIRES: arm-registered-target
+
+// This test checks the patch for the compilation error / crash described in 
the D18557.
+
+// Test as a C source
+// RUN: %clang_cc1 -triple=armv7-linux-gnueabihf -emit-pch -x c-header -o %t 
%S/Inputs/arm-__va_list_tag.h
+// RUN: %clang_cc1 -triple=armv7-linux-gnueabihf -fsyntax-only -include-pch %t 
%s
+
+// Test as a C++ source
+// RUN: %clang_cc1 -triple=armv7-linux-gnueabihf -emit-pch -x c++-header -o %t 
%S/Inputs/arm-__va_list_tag.h
+// RUN: %clang_cc1 -triple=armv7-linux-gnueabihf -x c++ -fsyntax-only 
-include-pch %t %s
+
+// expected-no-diagnostics
+
+typedef __builtin_va_list va_list_2;
+void test(const char* format, ...) { va_list args; va_start( args, format ); }
Index: test/PCH/Inputs/arm-__va_list_tag.h
===
--- test/PCH/Inputs/arm-__va_list_tag.h
+++ test/PCH/Inputs/arm-__va_list_tag.h
@@ -0,0 +1,4 @@
+// Header for PCH test arm-__va_list_tag.c
+
+#include 
+typedef va_list va_list_1;
Index: lib/AST/ASTContext.cpp
===
--- lib/AST/ASTContext.cpp
+++ lib/AST/ASTContext.cpp
@@ -6389,6 +6389,7 @@
 
   // };
   VaListDecl->completeDefinition();
+  Context->VaListTagDecl = VaListDecl;
 
   // typedef struct __va_list __builtin_va_list;
   QualType T = Context->getRecordType(VaListDecl);


Index: test/PCH/arm-__va_list_tag.c
===
--- test/PCH/arm-__va_list_tag.c
+++ test/PCH/arm-__va_list_tag.c
@@ -0,0 +1,16 @@
+// REQUIRES: arm-registered-target
+
+// This test checks the patch for the compilation error / crash described in the D18557.
+
+// Test as a C source
+// RUN: %clang_cc1 -triple=armv7-linux-gnueabihf -emit-pch -x c-header -o %t %S/Inputs/arm-__va_list_tag.h
+// RUN: %clang_cc1 -triple=armv7-linux-gnueabihf -fsyntax-only -include-pch %t %s
+
+// Test as a C++ source
+// RUN: %clang_cc1 -triple=armv7-linux-gnueabihf -emit-pch -x c++-header -o %t %S/Inputs/arm-__va_list_tag.h
+// RUN: %clang_cc1 -triple=armv7-linux-gnueabihf -x c++ -fsyntax-only -include-pch %t %s
+
+// expected-no-diagnostics
+
+typedef __builtin_va_list va_list_2;
+void test(const char* format, ...) { va_list args; va_start( args, format ); }
Index: test/PCH/Inputs/arm-__va_list_tag.h
===
--- test/PCH/Inputs/arm-__va_list_tag.h
+++ test/PCH/Inputs/arm-__va_list_tag.h
@@ -0,0 +1,4 @@
+// Header for PCH test arm-__va_list_tag.c
+
+#include 
+typedef va_list va_list_1;
Index: lib/AST/ASTContext.cpp
===
--- lib/AST/ASTContext.cpp
+++ lib/AST/ASTContext.cpp
@@ -6389,6 +6389,7 @@
 
   // };
   VaListDecl->completeDefinition();
+  Context->VaListTagDecl = VaListDecl;
 
   // typedef struct __va_list __builtin_va_list;
   QualType T = Context->getRecordType(VaListDecl);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D18557: [Clang][ARM] __va_list declaration is not saved in ASTContext causing compilation error or crash

[Oleg Ranevskyy via cfe-commits]

iid_iunknown added a comment.

Thank you for your remark, Richard.
I added the test. Could you check if the patch is good now, please?


Repository:
  rL LLVM

http://reviews.llvm.org/D18557



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

[PATCH] D18613: [PassManager] Make PassManagerBuilder::addExtension take an std::function, rather than a function pointer.

[Justin Lebar via cfe-commits]

jlebar created this revision.
jlebar added a reviewer: chandlerc.
jlebar added a subscriber: cfe-commits.
Herald added a subscriber: joker.eph.

This gives callers flexibility to pass lambdas with captures, which lets
callers avoid the C-style void*-ptr closure style.  (Currently, callers
in clang store state in the PassManagerBuilderBase arg.)

No functional change, and the new API is backwards-compatible.

http://reviews.llvm.org/D18613

Files:
  include/llvm/Transforms/IPO/PassManagerBuilder.h
  lib/Transforms/IPO/PassManagerBuilder.cpp

Index: lib/Transforms/IPO/PassManagerBuilder.cpp
===
--- lib/Transforms/IPO/PassManagerBuilder.cpp
+++ lib/Transforms/IPO/PassManagerBuilder.cpp
@@ -158,11 +158,11 @@
 void PassManagerBuilder::addGlobalExtension(
 PassManagerBuilder::ExtensionPointTy Ty,
 PassManagerBuilder::ExtensionFn Fn) {
-  GlobalExtensions->push_back(std::make_pair(Ty, Fn));
+  GlobalExtensions->push_back(std::make_pair(Ty, std::move(Fn)));
 }
 
 void PassManagerBuilder::addExtension(ExtensionPointTy Ty, ExtensionFn Fn) {
-  Extensions.push_back(std::make_pair(Ty, Fn));
+  Extensions.push_back(std::make_pair(Ty, std::move(Fn)));
 }
 
 void PassManagerBuilder::addExtensionsToPM(ExtensionPointTy ETy,
Index: include/llvm/Transforms/IPO/PassManagerBuilder.h
===
--- include/llvm/Transforms/IPO/PassManagerBuilder.h
+++ include/llvm/Transforms/IPO/PassManagerBuilder.h
@@ -59,8 +59,9 @@
 public:
   /// Extensions are passed the builder itself (so they can see how it is
   /// configured) as well as the pass manager to add stuff to.
-  typedef void (*ExtensionFn)(const PassManagerBuilder &Builder,
-  legacy::PassManagerBase &PM);
+  typedef std::function
+  ExtensionFn;
   enum ExtensionPointTy {
 /// EP_EarlyAsPossible - This extension point allows adding passes before
 /// any other transformations, allowing them to see the code as it is 
coming
@@ -143,7 +144,7 @@
 
 private:
   /// ExtensionList - This is list of all of the extensions that are 
registered.
-  std::vector > Extensions;
+  std::vector> Extensions;
 
 public:
   PassManagerBuilder();
@@ -184,7 +185,7 @@
 struct RegisterStandardPasses {
   RegisterStandardPasses(PassManagerBuilder::ExtensionPointTy Ty,
  PassManagerBuilder::ExtensionFn Fn) {
-PassManagerBuilder::addGlobalExtension(Ty, Fn);
+PassManagerBuilder::addGlobalExtension(Ty, std::move(Fn));
   }
 };
 


Index: lib/Transforms/IPO/PassManagerBuilder.cpp
===
--- lib/Transforms/IPO/PassManagerBuilder.cpp
+++ lib/Transforms/IPO/PassManagerBuilder.cpp
@@ -158,11 +158,11 @@
 void PassManagerBuilder::addGlobalExtension(
 PassManagerBuilder::ExtensionPointTy Ty,
 PassManagerBuilder::ExtensionFn Fn) {
-  GlobalExtensions->push_back(std::make_pair(Ty, Fn));
+  GlobalExtensions->push_back(std::make_pair(Ty, std::move(Fn)));
 }
 
 void PassManagerBuilder::addExtension(ExtensionPointTy Ty, ExtensionFn Fn) {
-  Extensions.push_back(std::make_pair(Ty, Fn));
+  Extensions.push_back(std::make_pair(Ty, std::move(Fn)));
 }
 
 void PassManagerBuilder::addExtensionsToPM(ExtensionPointTy ETy,
Index: include/llvm/Transforms/IPO/PassManagerBuilder.h
===
--- include/llvm/Transforms/IPO/PassManagerBuilder.h
+++ include/llvm/Transforms/IPO/PassManagerBuilder.h
@@ -59,8 +59,9 @@
 public:
   /// Extensions are passed the builder itself (so they can see how it is
   /// configured) as well as the pass manager to add stuff to.
-  typedef void (*ExtensionFn)(const PassManagerBuilder &Builder,
-  legacy::PassManagerBase &PM);
+  typedef std::function
+  ExtensionFn;
   enum ExtensionPointTy {
 /// EP_EarlyAsPossible - This extension point allows adding passes before
 /// any other transformations, allowing them to see the code as it is coming
@@ -143,7 +144,7 @@
 
 private:
   /// ExtensionList - This is list of all of the extensions that are registered.
-  std::vector > Extensions;
+  std::vector> Extensions;
 
 public:
   PassManagerBuilder();
@@ -184,7 +185,7 @@
 struct RegisterStandardPasses {
   RegisterStandardPasses(PassManagerBuilder::ExtensionPointTy Ty,
  PassManagerBuilder::ExtensionFn Fn) {
-PassManagerBuilder::addGlobalExtension(Ty, Fn);
+PassManagerBuilder::addGlobalExtension(Ty, std::move(Fn));
   }
 };
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D18380: [CUDA] Make unattributed constexpr functions (usually) implicitly host+device.

[Justin Lebar via cfe-commits]

jlebar added a comment.

(Just to be clear, I'm waiting on Richard's review here, even though he lg'ed 
an version of this patch.)


http://reviews.llvm.org/D18380



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