Re: [PATCH] D13497: Silence warning from lit about missing clang-interpreter

[Richard Barton via cfe-commits]

richard.barton.arm abandoned this revision.
richard.barton.arm added a comment.

Adandoning and starting again with cfe-commits as subscriber.


http://reviews.llvm.org/D13497



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

Re: [PATCH] D9888: [OPENMP] Driver support for OpenMP offloading

[Jonas Hahnfeld via cfe-commits]

Hahnfeld added a comment.

Currently trying to test, but

1. Offloading to the same target isn't supported (`x86_64-unknown-linux-gnu` as 
host and device) - this was working with `clang-omp`

The produced IR isn't showing any calls to the target library and on linkage it 
complains:

  undefined reference to `.omp_offloading.img_start.x86_64-unknown-linux-gnu'
  undefined reference to `.omp_offloading.img_end.x86_64-unknown-linux-gnu'
  undefined reference to `.omp_offloading.entries_begin'
  undefined reference to `.omp_offloading.entries_end'
  undefined reference to `.omp_offloading.entries_begin'
  undefined reference to `.omp_offloading.entries_end'

(btw: `clang-offload-bundler` saves the IR file to `$TMP` with `-S -emit-llvm`, 
this seems to be a bug - I had to use `--save-temps`)

2. I can't seem to figure out the target triple for NVIDIA GPUs. It should be 
`nvptx[64]-nvidia-cuda` which gives me

  include/llvm/Option/Option.h:101: const llvm::opt::Option 
llvm::opt::Option::getAlias() const: Assertion `Info && "Must have a valid 
info!"' failed.

In `clang-omp` it was `nvptxsm_35-nvidia-cuda` but this is now invalid...


http://reviews.llvm.org/D9888



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

[PATCH] D13498: Silence spurious warning from lit about missing clang-interpreter

[Richard Barton via cfe-commits]

richard.barton.arm created this revision.
richard.barton.arm added reviewers: ddunbar, rafael.
richard.barton.arm added a subscriber: cfe-commits.

Clang's lit.cfg checks for the presence of a few test binaries, including the 
clang-interpreter. This binary is only built if building the clang examples, so 
the check should also only fire if the clang examples are built.

http://reviews.llvm.org/D13498

Files:
  test/lit.cfg

Index: test/lit.cfg
===
--- test/lit.cfg
+++ test/lit.cfg
@@ -308,17 +308,21 @@
 NoPostHyphenDot = r"(?!(-|\.))"
 NoPostBar = r"(?!(/|\\))"
 
-for pattern in [r"\bFileCheck\b",
-r"\bc-index-test\b",
-NoPreHyphenDot + r"\bclang-check\b" + NoPostHyphenDot,
-NoPreHyphenDot + r"\bclang-format\b" + NoPostHyphenDot,
-NoPreHyphenDot + r"\bclang-interpreter\b" + NoPostHyphenDot,
-# FIXME: Some clang test uses opt?
-NoPreHyphenDot + r"\bopt\b" + NoPostBar + NoPostHyphenDot,
-# Handle these specially as they are strings searched
-# for during testing.
-r"\| \bcount\b",
-r"\| \bnot\b"]:
+tool_patterns = [r"\bFileCheck\b",
+ r"\bc-index-test\b",
+ NoPreHyphenDot + r"\bclang-check\b" + NoPostHyphenDot,
+ NoPreHyphenDot + r"\bclang-format\b" + NoPostHyphenDot,
+ # FIXME: Some clang test uses opt?
+ NoPreHyphenDot + r"\bopt\b" + NoPostBar + NoPostHyphenDot,
+ # Handle these specially as they are strings searched
+ # for during testing.
+ r"\| \bcount\b",
+ r"\| \bnot\b"]
+
+if config.clang_examples:
+tool_patterns.append(NoPreHyphenDot + r"\bclang-interpreter\b" + 
NoPostHyphenDot)
+
+for pattern in tool_patterns:
 # Extract the tool name from the pattern.  This relies on the tool
 # name being surrounded by \b word match operators.  If the
 # pattern starts with "| ", include it in the string to be


Index: test/lit.cfg
===
--- test/lit.cfg
+++ test/lit.cfg
@@ -308,17 +308,21 @@
 NoPostHyphenDot = r"(?!(-|\.))"
 NoPostBar = r"(?!(/|\\))"
 
-for pattern in [r"\bFileCheck\b",
-r"\bc-index-test\b",
-NoPreHyphenDot + r"\bclang-check\b" + NoPostHyphenDot,
-NoPreHyphenDot + r"\bclang-format\b" + NoPostHyphenDot,
-NoPreHyphenDot + r"\bclang-interpreter\b" + NoPostHyphenDot,
-# FIXME: Some clang test uses opt?
-NoPreHyphenDot + r"\bopt\b" + NoPostBar + NoPostHyphenDot,
-# Handle these specially as they are strings searched
-# for during testing.
-r"\| \bcount\b",
-r"\| \bnot\b"]:
+tool_patterns = [r"\bFileCheck\b",
+ r"\bc-index-test\b",
+ NoPreHyphenDot + r"\bclang-check\b" + NoPostHyphenDot,
+ NoPreHyphenDot + r"\bclang-format\b" + NoPostHyphenDot,
+ # FIXME: Some clang test uses opt?
+ NoPreHyphenDot + r"\bopt\b" + NoPostBar + NoPostHyphenDot,
+ # Handle these specially as they are strings searched
+ # for during testing.
+ r"\| \bcount\b",
+ r"\| \bnot\b"]
+
+if config.clang_examples:
+tool_patterns.append(NoPreHyphenDot + r"\bclang-interpreter\b" + NoPostHyphenDot)
+
+for pattern in tool_patterns:
 # Extract the tool name from the pattern.  This relies on the tool
 # name being surrounded by \b word match operators.  If the
 # pattern starts with "| ", include it in the string to be
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D13499: Convert test to take input from stdin rather than reading a file

[Richard Barton via cfe-commits]

richard.barton.arm created this revision.
richard.barton.arm added a reviewer: ABataev.
richard.barton.arm added a subscriber: cfe-commits.
Herald added a subscriber: aemerson.

This test takes input from a file rather than from stdin and then goes on to 
check for a label "bar". This could match something on the file path (guess 
what my uname at ARM is...) causing the test to fail.

This patch converts the test to take from stdin instead.

http://reviews.llvm.org/D13499

Files:
  test/OpenMP/for_simd_codegen.cpp

Index: test/OpenMP/for_simd_codegen.cpp
===
--- test/OpenMP/for_simd_codegen.cpp
+++ test/OpenMP/for_simd_codegen.cpp
@@ -1,7 +1,7 @@
-// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple x86_64-unknown-unknown 
-emit-llvm %s -fexceptions -fcxx-exceptions -o - | FileCheck %s
-// RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -triple x86_64-unknown-unknown 
-fexceptions -fcxx-exceptions -emit-pch -o %t %s
-// RUN: %clang_cc1 -fopenmp -x c++ -triple x86_64-unknown-unknown -fexceptions 
-fcxx-exceptions -g -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | 
FileCheck %s
-// RUN: %clang_cc1 -verify -triple x86_64-apple-darwin10 -fopenmp -fexceptions 
-fcxx-exceptions -gline-tables-only -x c++ -emit-llvm %s -o - | FileCheck %s 
--check-prefix=TERM_DEBUG
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple x86_64-unknown-unknown 
-emit-llvm -fexceptions -fcxx-exceptions -o - < %s | FileCheck %s
+// RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -triple x86_64-unknown-unknown 
-fexceptions -fcxx-exceptions -emit-pch -o %t < %s
+// RUN: %clang_cc1 -fopenmp -x c++ -triple x86_64-unknown-unknown -fexceptions 
-fcxx-exceptions -g -std=c++11 -include-pch %t -verify -emit-llvm -o - < %s | 
FileCheck %s
+// RUN: %clang_cc1 -verify -triple x86_64-apple-darwin10 -fopenmp -fexceptions 
-fcxx-exceptions -gline-tables-only -x c++ -emit-llvm -o - < %s | FileCheck %s 
--check-prefix=TERM_DEBUG
 // REQUIRES: x86-registered-target
 // expected-no-diagnostics
 #ifndef HEADER


Index: test/OpenMP/for_simd_codegen.cpp
===
--- test/OpenMP/for_simd_codegen.cpp
+++ test/OpenMP/for_simd_codegen.cpp
@@ -1,7 +1,7 @@
-// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple x86_64-unknown-unknown -emit-llvm %s -fexceptions -fcxx-exceptions -o - | FileCheck %s
-// RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -triple x86_64-unknown-unknown -fexceptions -fcxx-exceptions -emit-pch -o %t %s
-// RUN: %clang_cc1 -fopenmp -x c++ -triple x86_64-unknown-unknown -fexceptions -fcxx-exceptions -g -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s
-// RUN: %clang_cc1 -verify -triple x86_64-apple-darwin10 -fopenmp -fexceptions -fcxx-exceptions -gline-tables-only -x c++ -emit-llvm %s -o - | FileCheck %s --check-prefix=TERM_DEBUG
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple x86_64-unknown-unknown -emit-llvm -fexceptions -fcxx-exceptions -o - < %s | FileCheck %s
+// RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -triple x86_64-unknown-unknown -fexceptions -fcxx-exceptions -emit-pch -o %t < %s
+// RUN: %clang_cc1 -fopenmp -x c++ -triple x86_64-unknown-unknown -fexceptions -fcxx-exceptions -g -std=c++11 -include-pch %t -verify -emit-llvm -o - < %s | FileCheck %s
+// RUN: %clang_cc1 -verify -triple x86_64-apple-darwin10 -fopenmp -fexceptions -fcxx-exceptions -gline-tables-only -x c++ -emit-llvm -o - < %s | FileCheck %s --check-prefix=TERM_DEBUG
 // REQUIRES: x86-registered-target
 // expected-no-diagnostics
 #ifndef HEADER
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D13496: Convert test to take input from stdin rather than reading a file

[Richard Barton via cfe-commits]

richard.barton.arm abandoned this revision.
richard.barton.arm added a comment.

Abandoning as I forgot to add cfe-commits.


http://reviews.llvm.org/D13496



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

[PATCH] D13500: Improve helpfulness of assert message when number of diagnostics overflows number of available enum values.

[Richard Barton via cfe-commits]

richard.barton.arm created this revision.
richard.barton.arm added a reviewer: rafael.
richard.barton.arm added a subscriber: cfe-commits.

There are two asserts that can trigger if the number of diagnostic messages in a
kind overflows the number of available enums values. The one that fires if you
exceed this limit by one is helpful. The one that fires if you exceed it by 
more than
one is not as helpful. Combining the two makes sense I think.

http://reviews.llvm.org/D13500

Files:
  lib/Basic/DiagnosticIDs.cpp

Index: lib/Basic/DiagnosticIDs.cpp
===
--- lib/Basic/DiagnosticIDs.cpp
+++ lib/Basic/DiagnosticIDs.cpp
@@ -101,12 +101,9 @@
   static bool IsFirst = true; // So the check is only performed on first call.
   if (IsFirst) {
 for (unsigned i = 1; i != StaticDiagInfoSize; ++i) {
-  assert(StaticDiagInfo[i-1].DiagID != StaticDiagInfo[i].DiagID &&
+  assert(StaticDiagInfo[i-1].DiagID < StaticDiagInfo[i].DiagID &&
  "Diag ID conflict, the enums at the start of clang::diag (in "
  "DiagnosticIDs.h) probably need to be increased");
-
-  assert(StaticDiagInfo[i-1] < StaticDiagInfo[i] &&
- "Improperly sorted diag info");
 }
 IsFirst = false;
   }


Index: lib/Basic/DiagnosticIDs.cpp
===
--- lib/Basic/DiagnosticIDs.cpp
+++ lib/Basic/DiagnosticIDs.cpp
@@ -101,12 +101,9 @@
   static bool IsFirst = true; // So the check is only performed on first call.
   if (IsFirst) {
 for (unsigned i = 1; i != StaticDiagInfoSize; ++i) {
-  assert(StaticDiagInfo[i-1].DiagID != StaticDiagInfo[i].DiagID &&
+  assert(StaticDiagInfo[i-1].DiagID < StaticDiagInfo[i].DiagID &&
  "Diag ID conflict, the enums at the start of clang::diag (in "
  "DiagnosticIDs.h) probably need to be increased");
-
-  assert(StaticDiagInfo[i-1] < StaticDiagInfo[i] &&
- "Improperly sorted diag info");
 }
 IsFirst = false;
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D13495: Improve the helpfulness of an assert message

[Richard Barton via cfe-commits]

richard.barton.arm abandoned this revision.
richard.barton.arm added a comment.

Abandoning as cfe-commits was not on subscribers


http://reviews.llvm.org/D13495



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

r249525 - [VFS] Also drop '.' when adding files to an in-memory FS.

[Benjamin Kramer via cfe-commits]

Author: d0k
Date: Wed Oct  7 03:32:50 2015
New Revision: 249525

URL: http://llvm.org/viewvc/llvm-project?rev=249525&view=rev
Log:
[VFS] Also drop '.' when adding files to an in-memory FS.

Otherwise we won't be able to find them later.

Modified:
cfe/trunk/lib/Basic/VirtualFileSystem.cpp
cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp

Modified: cfe/trunk/lib/Basic/VirtualFileSystem.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/VirtualFileSystem.cpp?rev=249525&r1=249524&r2=249525&view=diff
==
--- cfe/trunk/lib/Basic/VirtualFileSystem.cpp (original)
+++ cfe/trunk/lib/Basic/VirtualFileSystem.cpp Wed Oct  7 03:32:50 2015
@@ -495,6 +495,13 @@ void InMemoryFileSystem::addFile(const T
   auto I = llvm::sys::path::begin(Path), E = llvm::sys::path::end(Path);
   while (true) {
 StringRef Name = *I;
+// Skip over ".".
+// FIXME: Also handle "..".
+if (Name == ".") {
+  ++I;
+  continue;
+}
+
 detail::InMemoryNode *Node = Dir->getChild(Name);
 ++I;
 if (!Node) {

Modified: cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp?rev=249525&r1=249524&r2=249525&view=diff
==
--- cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp (original)
+++ cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp Wed Oct  7 03:32:50 2015
@@ -568,6 +568,7 @@ TEST_F(InMemoryFileSystemTest, OverlayFi
 
 TEST_F(InMemoryFileSystemTest, OpenFileForRead) {
   FS.addFile("/a", 0, MemoryBuffer::getMemBuffer("a"));
+  FS.addFile("././c", 0, MemoryBuffer::getMemBuffer("c"));
   auto File = FS.openFileForRead("/a");
   ASSERT_EQ("a", (*(*File)->getBuffer("ignored"))->getBuffer());
   File = FS.openFileForRead("/a"); // Open again.
@@ -578,6 +579,8 @@ TEST_F(InMemoryFileSystemTest, OpenFileF
   ASSERT_EQ(File.getError(), errc::invalid_argument) << FS.toString();
   File = FS.openFileForRead("/b");
   ASSERT_EQ(File.getError(), errc::no_such_file_or_directory) << FS.toString();
+  File = FS.openFileForRead("./c");
+  ASSERT_EQ("c", (*(*File)->getBuffer("ignored"))->getBuffer());
 }
 
 TEST_F(InMemoryFileSystemTest, DirectoryIteration) {


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

[clang-tools-extra] r249526 - [VFS] Switch clang-tidy tests to use an in-memory fs.

[Benjamin Kramer via cfe-commits]

Author: d0k
Date: Wed Oct  7 03:35:23 2015
New Revision: 249526

URL: http://llvm.org/viewvc/llvm-project?rev=249526&view=rev
Log:
[VFS] Switch clang-tidy tests to use an in-memory fs.

Again, this is both cleaner and completely removes any depedency on the
host file system.

Modified:
clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h

Modified: clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h?rev=249526&r1=249525&r2=249526&view=diff
==
--- clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h (original)
+++ clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h Wed Oct  7 
03:35:23 2015
@@ -90,17 +90,21 @@ runCheckOnCode(StringRef Code, std::vect
   ArgCXX11.push_back(Filename.str());
 
   ast_matchers::MatchFinder Finder;
+  llvm::IntrusiveRefCntPtr InMemoryFileSystem(
+  new vfs::InMemoryFileSystem);
   llvm::IntrusiveRefCntPtr Files(
-  new FileManager(FileSystemOptions()));
+  new FileManager(FileSystemOptions(), InMemoryFileSystem));
 
   SmallVector, 1> Checks;
   CheckFactory::createChecks(&Context, Checks);
   tooling::ToolInvocation Invocation(
   ArgCXX11, new TestClangTidyAction(Checks, Finder, Context), Files.get());
-  Invocation.mapVirtualFile(Filename.str(), Code);
+  InMemoryFileSystem->addFile(Filename, 0,
+  llvm::MemoryBuffer::getMemBuffer(Code));
   for (const auto &FileContent : PathsToContent) {
-Invocation.mapVirtualFile(Twine("include/" + FileContent.first).str(),
-  FileContent.second);
+InMemoryFileSystem->addFile(
+Twine("include/") + FileContent.first, 0,
+llvm::MemoryBuffer::getMemBuffer(FileContent.second));
   }
   Invocation.setDiagnosticConsumer(&DiagConsumer);
   if (!Invocation.run()) {


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

[PATCH] D13504: Prevent modernize-use-auto from emitting a warning when 'auto' was already being used.

[Angel Garcia via cfe-commits]

angelgarcia created this revision.
angelgarcia added reviewers: klimek, bkramer.
angelgarcia added subscribers: alexfh, cfe-commits.

This fixes https://llvm.org/bugs/show_bug.cgi?id=25082 .

http://reviews.llvm.org/D13504

Files:
  clang-tidy/modernize/UseAutoCheck.cpp
  test/clang-tidy/modernize-use-auto-new.cpp

Index: test/clang-tidy/modernize-use-auto-new.cpp
===
--- test/clang-tidy/modernize-use-auto-new.cpp
+++ test/clang-tidy/modernize-use-auto-new.cpp
@@ -95,4 +95,7 @@
 // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use auto when initializing 
with new
 // CHECK-FIXES: auto g = new int*, h = new int_p;
   }
+
+  // Don't warn when 'auto' is already being used.
+  auto aut = new MyType();
 }
Index: clang-tidy/modernize/UseAutoCheck.cpp
===
--- clang-tidy/modernize/UseAutoCheck.cpp
+++ clang-tidy/modernize/UseAutoCheck.cpp
@@ -222,6 +222,8 @@
  has(varDecl()),
  unless(has(varDecl(anyOf(
  unless(hasInitializer(ignoringParenImpCasts(cxxNewExpr(,
+ // Skip declarations that are already using auto.
+ hasType(autoType()),
  // FIXME: TypeLoc information is not reliable where CV
  // qualifiers are concerned so these types can't be
  // handled for now.


Index: test/clang-tidy/modernize-use-auto-new.cpp
===
--- test/clang-tidy/modernize-use-auto-new.cpp
+++ test/clang-tidy/modernize-use-auto-new.cpp
@@ -95,4 +95,7 @@
 // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use auto when initializing with new
 // CHECK-FIXES: auto g = new int*, h = new int_p;
   }
+
+  // Don't warn when 'auto' is already being used.
+  auto aut = new MyType();
 }
Index: clang-tidy/modernize/UseAutoCheck.cpp
===
--- clang-tidy/modernize/UseAutoCheck.cpp
+++ clang-tidy/modernize/UseAutoCheck.cpp
@@ -222,6 +222,8 @@
  has(varDecl()),
  unless(has(varDecl(anyOf(
  unless(hasInitializer(ignoringParenImpCasts(cxxNewExpr(,
+ // Skip declarations that are already using auto.
+ hasType(autoType()),
  // FIXME: TypeLoc information is not reliable where CV
  // qualifiers are concerned so these types can't be
  // handled for now.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D13504: Prevent modernize-use-auto from emitting a warning when 'auto' was already being used.

[Alexander Kornienko via cfe-commits]

alexfh added inline comments.


Comment at: test/clang-tidy/modernize-use-auto-new.cpp:100
@@ -98,1 +99,3 @@
+  // Don't warn when 'auto' is already being used.
+  auto aut = new MyType();
 }

Please add tests for `auto *` and `const auto *`.


http://reviews.llvm.org/D13504



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

r249532 - [VFS] Refactor VFSFromYAML a bit.

[Benjamin Kramer via cfe-commits]

Author: d0k
Date: Wed Oct  7 05:05:44 2015
New Revision: 249532

URL: http://llvm.org/viewvc/llvm-project?rev=249532&view=rev
Log:
[VFS] Refactor VFSFromYAML a bit.

- Rename it to RedirectingFileSystem. This is what it does, YAML is just a
  serialization format for it.
- Consistently use unique_ptr for memory management.

No functional change intended.

Modified:
cfe/trunk/lib/Basic/VirtualFileSystem.cpp

Modified: cfe/trunk/lib/Basic/VirtualFileSystem.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/VirtualFileSystem.cpp?rev=249532&r1=249531&r2=249532&view=diff
==
--- cfe/trunk/lib/Basic/VirtualFileSystem.cpp (original)
+++ cfe/trunk/lib/Basic/VirtualFileSystem.cpp Wed Oct  7 05:05:44 2015
@@ -647,7 +647,7 @@ directory_iterator InMemoryFileSystem::d
 }
 
 
//===---===/
-// VFSFromYAML implementation
+// RedirectingFileSystem implementation
 
//===---===/
 
 namespace {
@@ -670,16 +670,16 @@ public:
 };
 
 class DirectoryEntry : public Entry {
-  std::vector Contents;
+  std::vector> Contents;
   Status S;
 
 public:
-  ~DirectoryEntry() override;
-  DirectoryEntry(StringRef Name, std::vector Contents, Status S)
+  DirectoryEntry(StringRef Name, std::vector> Contents,
+ Status S)
   : Entry(EK_Directory, Name), Contents(std::move(Contents)),
 S(std::move(S)) {}
   Status getStatus() { return S; }
-  typedef std::vector::iterator iterator;
+  typedef decltype(Contents)::iterator iterator;
   iterator contents_begin() { return Contents.begin(); }
   iterator contents_end() { return Contents.end(); }
   static bool classof(const Entry *E) { return E->getKind() == EK_Directory; }
@@ -708,14 +708,14 @@ public:
   static bool classof(const Entry *E) { return E->getKind() == EK_File; }
 };
 
-class VFSFromYAML;
+class RedirectingFileSystem;
 
 class VFSFromYamlDirIterImpl : public clang::vfs::detail::DirIterImpl {
   std::string Dir;
-  VFSFromYAML &FS;
+  RedirectingFileSystem &FS;
   DirectoryEntry::iterator Current, End;
 public:
-  VFSFromYamlDirIterImpl(const Twine &Path, VFSFromYAML &FS,
+  VFSFromYamlDirIterImpl(const Twine &Path, RedirectingFileSystem &FS,
  DirectoryEntry::iterator Begin,
  DirectoryEntry::iterator End, std::error_code &EC);
   std::error_code increment() override;
@@ -774,8 +774,9 @@ public:
 /// In both cases, the 'name' field may contain multiple path components (e.g.
 /// /path/to/file). However, any directory that contains more than one child
 /// must be uniquely represented by a directory entry.
-class VFSFromYAML : public vfs::FileSystem {
-  std::vector Roots; ///< The root(s) of the virtual file system.
+class RedirectingFileSystem : public vfs::FileSystem {
+  /// The root(s) of the virtual file system.
+  std::vector> Roots;
   /// \brief The file system to use for external references.
   IntrusiveRefCntPtr ExternalFS;
 
@@ -792,10 +793,10 @@ class VFSFromYAML : public vfs::FileSyst
   bool UseExternalNames;
   /// @}
 
-  friend class VFSFromYAMLParser;
+  friend class RedirectingFileSystemParser;
 
 private:
-  VFSFromYAML(IntrusiveRefCntPtr ExternalFS)
+  RedirectingFileSystem(IntrusiveRefCntPtr ExternalFS)
   : ExternalFS(ExternalFS), CaseSensitive(true), UseExternalNames(true) {}
 
   /// \brief Looks up \p Path in \c Roots.
@@ -810,14 +811,12 @@ private:
   ErrorOr status(const Twine &Path, Entry *E);
 
 public:
-  ~VFSFromYAML() override;
-
   /// \brief Parses \p Buffer, which is expected to be in YAML format and
   /// returns a virtual file system representing its contents.
-  static VFSFromYAML *create(std::unique_ptr Buffer,
- SourceMgr::DiagHandlerTy DiagHandler,
- void *DiagContext,
- IntrusiveRefCntPtr ExternalFS);
+  static RedirectingFileSystem *
+  create(std::unique_ptr Buffer,
+ SourceMgr::DiagHandlerTy DiagHandler, void *DiagContext,
+ IntrusiveRefCntPtr ExternalFS);
 
   ErrorOr status(const Twine &Path) override;
   ErrorOr> openFileForRead(const Twine &Path) override;
@@ -853,7 +852,7 @@ public:
 };
 
 /// \brief A helper class to hold the common YAML parsing state.
-class VFSFromYAMLParser {
+class RedirectingFileSystemParser {
   yaml::Stream &Stream;
 
   void error(yaml::Node *N, const Twine &Msg) {
@@ -929,7 +928,7 @@ class VFSFromYAMLParser {
 return true;
   }
 
-  Entry *parseEntry(yaml::Node *N) {
+  std::unique_ptr parseEntry(yaml::Node *N) {
 yaml::MappingNode *M = dyn_cast(N);
 if (!M) {
   error(N, "expected mapping node for file or directory entry");
@@ -948,7 +947,7 @@ class VFSFromYAMLParser {
 &Fields[0], Fields + sizeof(Fields)/sizeof(Fields[0]));
 
 bool HasContents = false; // external or otherwise

Re: [PATCH] D13504: Prevent modernize-use-auto from emitting a warning when 'auto' was already being used.

[Angel Garcia via cfe-commits]

angelgarcia updated this revision to Diff 36724.
angelgarcia added a comment.

Good point. Solved.


http://reviews.llvm.org/D13504

Files:
  clang-tidy/modernize/UseAutoCheck.cpp
  test/clang-tidy/modernize-use-auto-new.cpp

Index: test/clang-tidy/modernize-use-auto-new.cpp
===
--- test/clang-tidy/modernize-use-auto-new.cpp
+++ test/clang-tidy/modernize-use-auto-new.cpp
@@ -95,4 +95,9 @@
 // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use auto when initializing 
with new
 // CHECK-FIXES: auto g = new int*, h = new int_p;
   }
+
+  // Don't warn when 'auto' is already being used.
+  auto aut = new MyType();
+  auto *paut = new MyType();
+  const auto *pcaut = new MyType();
 }
Index: clang-tidy/modernize/UseAutoCheck.cpp
===
--- clang-tidy/modernize/UseAutoCheck.cpp
+++ clang-tidy/modernize/UseAutoCheck.cpp
@@ -222,6 +222,9 @@
  has(varDecl()),
  unless(has(varDecl(anyOf(
  unless(hasInitializer(ignoringParenImpCasts(cxxNewExpr(,
+ // Skip declarations that are already using auto.
+ anyOf(hasType(autoType()),
+   hasType(pointerType(pointee(autoType(),
  // FIXME: TypeLoc information is not reliable where CV
  // qualifiers are concerned so these types can't be
  // handled for now.


Index: test/clang-tidy/modernize-use-auto-new.cpp
===
--- test/clang-tidy/modernize-use-auto-new.cpp
+++ test/clang-tidy/modernize-use-auto-new.cpp
@@ -95,4 +95,9 @@
 // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use auto when initializing with new
 // CHECK-FIXES: auto g = new int*, h = new int_p;
   }
+
+  // Don't warn when 'auto' is already being used.
+  auto aut = new MyType();
+  auto *paut = new MyType();
+  const auto *pcaut = new MyType();
 }
Index: clang-tidy/modernize/UseAutoCheck.cpp
===
--- clang-tidy/modernize/UseAutoCheck.cpp
+++ clang-tidy/modernize/UseAutoCheck.cpp
@@ -222,6 +222,9 @@
  has(varDecl()),
  unless(has(varDecl(anyOf(
  unless(hasInitializer(ignoringParenImpCasts(cxxNewExpr(,
+ // Skip declarations that are already using auto.
+ anyOf(hasType(autoType()),
+   hasType(pointerType(pointee(autoType(),
  // FIXME: TypeLoc information is not reliable where CV
  // qualifiers are concerned so these types can't be
  // handled for now.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

cfe-commits@lists.llvm.org

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Wed Oct  7 05:22:08 2015
New Revision: 249534

URL: http://llvm.org/viewvc/llvm-project?rev=249534&view=rev
Log:
Fix crash in codegen on casting to `bool &`.
Currently codegen crashes trying to emit casting to bool &. It happens because 
bool type is converted to i1 and later then lvalue for reference is converted 
to i1*. But when codegen tries to load this lvalue it crashes trying to load 
value from this i1*.

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

Added:
cfe/trunk/test/CodeGenCXX/cast-to-ref-bool.cpp   (with props)
Modified:
cfe/trunk/lib/CodeGen/CGExprScalar.cpp

Modified: cfe/trunk/lib/CodeGen/CGExprScalar.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExprScalar.cpp?rev=249534&r1=249533&r2=249534&view=diff
==
--- cfe/trunk/lib/CodeGen/CGExprScalar.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExprScalar.cpp Wed Oct  7 05:22:08 2015
@@ -1383,7 +1383,7 @@ Value *ScalarExprEmitter::VisitCastExpr(
   case CK_LValueBitCast:
   case CK_ObjCObjectLValueCast: {
 Address Addr = EmitLValue(E).getAddress();
-Addr = Builder.CreateElementBitCast(Addr, ConvertType(DestTy));
+Addr = Builder.CreateElementBitCast(Addr, CGF.ConvertTypeForMem(DestTy));
 LValue LV = CGF.MakeAddrLValue(Addr, DestTy);
 return EmitLoadOfLValue(LV, CE->getExprLoc());
   }

Added: cfe/trunk/test/CodeGenCXX/cast-to-ref-bool.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/cast-to-ref-bool.cpp?rev=249534&view=auto
==
--- cfe/trunk/test/CodeGenCXX/cast-to-ref-bool.cpp (added)
+++ cfe/trunk/test/CodeGenCXX/cast-to-ref-bool.cpp Wed Oct  7 05:22:08 2015
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -emit-llvm -o - %s | FileCheck 
%s
+
+// CHECK-LABEL: main
+int main(int argc, char **argv) {
+  // CHECK: load i8, i8* %
+  // CHECK-NEXT: trunc i8 %{{.+}} to i1
+  bool b = (bool &)argv[argc][1];
+  return b;
+}

Propchange: cfe/trunk/test/CodeGenCXX/cast-to-ref-bool.cpp
--
svn:eol-style = native

Propchange: cfe/trunk/test/CodeGenCXX/cast-to-ref-bool.cpp
--
svn:keywords = Author Date Id Rev URL

Propchange: cfe/trunk/test/CodeGenCXX/cast-to-ref-bool.cpp
--
svn:mime-type = text/plain


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

cfe-commits@lists.llvm.org

[Alexey Bataev via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL249534: Fix crash in codegen on casting to `bool &`. 
(authored by ABataev).

Changed prior to commit:
  http://reviews.llvm.org/D13325?vs=36179&id=36725#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D13325

Files:
  cfe/trunk/lib/CodeGen/CGExprScalar.cpp
  cfe/trunk/test/CodeGenCXX/cast-to-ref-bool.cpp

Index: cfe/trunk/test/CodeGenCXX/cast-to-ref-bool.cpp
===
--- cfe/trunk/test/CodeGenCXX/cast-to-ref-bool.cpp
+++ cfe/trunk/test/CodeGenCXX/cast-to-ref-bool.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -emit-llvm -o - %s | FileCheck 
%s
+
+// CHECK-LABEL: main
+int main(int argc, char **argv) {
+  // CHECK: load i8, i8* %
+  // CHECK-NEXT: trunc i8 %{{.+}} to i1
+  bool b = (bool &)argv[argc][1];
+  return b;
+}
Index: cfe/trunk/lib/CodeGen/CGExprScalar.cpp
===
--- cfe/trunk/lib/CodeGen/CGExprScalar.cpp
+++ cfe/trunk/lib/CodeGen/CGExprScalar.cpp
@@ -1383,7 +1383,7 @@
   case CK_LValueBitCast:
   case CK_ObjCObjectLValueCast: {
 Address Addr = EmitLValue(E).getAddress();
-Addr = Builder.CreateElementBitCast(Addr, ConvertType(DestTy));
+Addr = Builder.CreateElementBitCast(Addr, CGF.ConvertTypeForMem(DestTy));
 LValue LV = CGF.MakeAddrLValue(Addr, DestTy);
 return EmitLoadOfLValue(LV, CE->getExprLoc());
   }


Index: cfe/trunk/test/CodeGenCXX/cast-to-ref-bool.cpp
===
--- cfe/trunk/test/CodeGenCXX/cast-to-ref-bool.cpp
+++ cfe/trunk/test/CodeGenCXX/cast-to-ref-bool.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -emit-llvm -o - %s | FileCheck %s
+
+// CHECK-LABEL: main
+int main(int argc, char **argv) {
+  // CHECK: load i8, i8* %
+  // CHECK-NEXT: trunc i8 %{{.+}} to i1
+  bool b = (bool &)argv[argc][1];
+  return b;
+}
Index: cfe/trunk/lib/CodeGen/CGExprScalar.cpp
===
--- cfe/trunk/lib/CodeGen/CGExprScalar.cpp
+++ cfe/trunk/lib/CodeGen/CGExprScalar.cpp
@@ -1383,7 +1383,7 @@
   case CK_LValueBitCast:
   case CK_ObjCObjectLValueCast: {
 Address Addr = EmitLValue(E).getAddress();
-Addr = Builder.CreateElementBitCast(Addr, ConvertType(DestTy));
+Addr = Builder.CreateElementBitCast(Addr, CGF.ConvertTypeForMem(DestTy));
 LValue LV = CGF.MakeAddrLValue(Addr, DestTy);
 return EmitLoadOfLValue(LV, CE->getExprLoc());
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r249535 - Make the test take input from stdin to prevent matching characters in a file path

[Richard Barton via cfe-commits]

Author: rbarton
Date: Wed Oct  7 05:33:36 2015
New Revision: 249535

URL: http://llvm.org/viewvc/llvm-project?rev=249535&view=rev
Log:
Make the test take input from stdin to prevent matching characters in a file 
path

Modified:
cfe/trunk/test/OpenMP/for_simd_codegen.cpp

Modified: cfe/trunk/test/OpenMP/for_simd_codegen.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/for_simd_codegen.cpp?rev=249535&r1=249534&r2=249535&view=diff
==
--- cfe/trunk/test/OpenMP/for_simd_codegen.cpp (original)
+++ cfe/trunk/test/OpenMP/for_simd_codegen.cpp Wed Oct  7 05:33:36 2015
@@ -1,7 +1,7 @@
-// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple x86_64-unknown-unknown 
-emit-llvm %s -fexceptions -fcxx-exceptions -o - | FileCheck %s
-// RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -triple x86_64-unknown-unknown 
-fexceptions -fcxx-exceptions -emit-pch -o %t %s
-// RUN: %clang_cc1 -fopenmp -x c++ -triple x86_64-unknown-unknown -fexceptions 
-fcxx-exceptions -g -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | 
FileCheck %s
-// RUN: %clang_cc1 -verify -triple x86_64-apple-darwin10 -fopenmp -fexceptions 
-fcxx-exceptions -gline-tables-only -x c++ -emit-llvm %s -o - | FileCheck %s 
--check-prefix=TERM_DEBUG
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple x86_64-unknown-unknown 
-emit-llvm -fexceptions -fcxx-exceptions -o - < %s | FileCheck %s
+// RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -triple x86_64-unknown-unknown 
-fexceptions -fcxx-exceptions -emit-pch -o %t < %s
+// RUN: %clang_cc1 -fopenmp -x c++ -triple x86_64-unknown-unknown -fexceptions 
-fcxx-exceptions -g -std=c++11 -include-pch %t -verify -emit-llvm -o - < %s | 
FileCheck %s
+// RUN: %clang_cc1 -verify -triple x86_64-apple-darwin10 -fopenmp -fexceptions 
-fcxx-exceptions -gline-tables-only -x c++ -emit-llvm -o - < %s | FileCheck %s 
--check-prefix=TERM_DEBUG
 // REQUIRES: x86-registered-target
 // expected-no-diagnostics
 #ifndef HEADER


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

Re: [PATCH] D13368: [clang-tidy] add check cppcoreguidelines-pro-type-static-cast-downcast

[Alexander Kornienko via cfe-commits]

I think, it would be more consistent to have clang-tidy insert a link to
the check documentation (on llvm.org) into each warning.
The documentation pages already contain links to the relevant rules. We
could even version documentation pages, if we think it's important (but
afaiu, we don't do this for clang, for example). This way we can control
link integrity and describe implementation-specific details, which can
sometimes be important.
On Tue, Oct 6, 2015 at 4:12 PM Aaron Ballman 
wrote:

> aaron.ballman added a comment.
>
> In http://reviews.llvm.org/D13368#260672, @klimek wrote:
>
> > In http://reviews.llvm.org/D13368#260669, @aaron.ballman wrote:
> >
> > > This wasn't a comment on the rule so much as a comment on the
> diagnostic not being very helpful.In this case, you're telling the user to
> not do something, but it is unclear how the user would structure their code
> to silence this diagnostic. Perhaps there is no way to word this to give
> the user a clue, but we should at least try. If I got this diagnostic as it
> is now, I would scratch my head and quickly decide to ignore it.
> >
> >
> > The cpp core guidelines are written in a way that they should be
> referenceable by links - do we want to add links to the relevant portions
> of the core guidelines from the clang-tidy checks?
>
>
> I'd be hesitant to do that. It would add a lot of verbiage to diagnostics
> that are likely to be chatty, and if the links ever go dead mid-release
> cycle for us, we're stuck looking bad with no way to fix it. CERT's
> guidelines are also linkable in the same fashion (as would be hypothetical
> checks for MISRA, JSF, etc), and I would have the same hesitation for those
> as well due to the potential dead link issue.
>
> I think that having the links within the user-facing documentation is a
> must-have though (and something we've been pretty good about thus far)
> because those can be updated live from ToT. So perhaps a permanent short
> link to our own documentation might be useful (if a bit obtuse since our
> docs mostly just point to other docs elsewhere)? I'd still be a bit worried
> about expired short links or something, but maybe we already host a service
> for this sort of thing?


I'll postulate that a) github will not go away anytime soon and b) github
will have a hard time changing their link structure so linking into
revision N in branch M doesn't work any more.
Thus, I think if we link into the github release of the core guildelines,
we'll be fine.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D13498: Silence spurious warning from lit about missing clang-interpreter

[Renato Golin via cfe-commits]

rengolin added a subscriber: rengolin.
rengolin accepted this revision.
rengolin added a reviewer: rengolin.
rengolin added a comment.
This revision is now accepted and ready to land.

Makes sense. LGTM. Thanks!


http://reviews.llvm.org/D13498



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

r249538 - Silence warning about not being able to find clang-interpreter

[Richard Barton via cfe-commits]

Author: rbarton
Date: Wed Oct  7 06:14:25 2015
New Revision: 249538

URL: http://llvm.org/viewvc/llvm-project?rev=249538&view=rev
Log:
Silence warning about not being able to find clang-interpreter

This binary is only built with the examples project, so only require it then.

Modified:
cfe/trunk/test/lit.cfg

Modified: cfe/trunk/test/lit.cfg
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/lit.cfg?rev=249538&r1=249537&r2=249538&view=diff
==
--- cfe/trunk/test/lit.cfg (original)
+++ cfe/trunk/test/lit.cfg Wed Oct  7 06:14:25 2015
@@ -308,17 +308,21 @@ NoPreHyphenDot = r"(?http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D13398: [clang-tidy] add check cppcoreguidelines-pro-type-const-cast

[Aaron Ballman via cfe-commits]

On Tue, Oct 6, 2015 at 11:07 PM, Alexander Kornienko  wrote:
> alexfh added a comment.
>
> It looks like potentially we're going to have tens of checks in this module. 
> I wonder whether we should start organizing the checks somehow right away. 
> For example, create a directory (and a namespace) for each profile. We'd need 
> to adapt the add_new_check.py script to support this then.

I wouldn't be opposed to that. Would this organization carry over to
other things as well, like the documentation and test directories?

~Aaron

>
> Just a thought. The patch LG.
>
>
> http://reviews.llvm.org/D13398
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r249540 - Add checker for the C++ Core Guidelines: cppcoreguidelines-pro-type-const-cast.

[Aaron Ballman via cfe-commits]

Author: aaronballman
Date: Wed Oct  7 07:24:57 2015
New Revision: 249540

URL: http://llvm.org/viewvc/llvm-project?rev=249540&view=rev
Log:
Add checker for the C++ Core Guidelines: cppcoreguidelines-pro-type-const-cast.

Patch by Matthias Gehre!

Added:

clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProTypeConstCastCheck.cpp
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProTypeConstCastCheck.h

clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-pro-type-const-cast.rst

clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-pro-type-const-cast.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CMakeLists.txt

clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst

Modified: clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CMakeLists.txt?rev=249540&r1=249539&r2=249540&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CMakeLists.txt 
(original)
+++ clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CMakeLists.txt Wed Oct 
 7 07:24:57 2015
@@ -2,6 +2,7 @@ set(LLVM_LINK_COMPONENTS support)
 
 add_clang_library(clangTidyCppCoreGuidelinesModule
   CppCoreGuidelinesTidyModule.cpp
+  ProTypeConstCastCheck.cpp
   ProTypeReinterpretCastCheck.cpp
 
   LINK_LIBS

Modified: 
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp?rev=249540&r1=249539&r2=249540&view=diff
==
--- 
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
 (original)
+++ 
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
 Wed Oct  7 07:24:57 2015
@@ -10,6 +10,7 @@
 #include "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
+#include "ProTypeConstCastCheck.h"
 #include "ProTypeReinterpretCastCheck.h"
 
 namespace clang {
@@ -19,6 +20,8 @@ namespace cppcoreguidelines {
 class CppCoreGuidelinesModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
+CheckFactories.registerCheck(
+"cppcoreguidelines-pro-type-const-cast");
 CheckFactories.registerCheck(
 "cppcoreguidelines-pro-type-reinterpret-cast");
   }

Added: 
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProTypeConstCastCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProTypeConstCastCheck.cpp?rev=249540&view=auto
==
--- 
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProTypeConstCastCheck.cpp 
(added)
+++ 
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProTypeConstCastCheck.cpp 
Wed Oct  7 07:24:57 2015
@@ -0,0 +1,33 @@
+//===--- ProTypeConstCastCheck.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 "ProTypeConstCastCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+
+void ProTypeConstCastCheck::registerMatchers(MatchFinder *Finder) {
+  if(!getLangOpts().CPlusPlus)
+return;
+
+  Finder->addMatcher(cxxConstCastExpr().bind("cast"), this);
+}
+
+void ProTypeConstCastCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *MatchedCast = Result.Nodes.getNodeAs("cast");
+  diag(MatchedCast->getOperatorLoc(), "do not use const_cast");
+}
+
+} // namespace tidy
+} // namespace clang
+

Added: 
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProTypeConstCastCheck.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProTypeConstCastCheck.h?rev=249540&view=auto
==
--- 
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProTypeConstCastCheck.h 
(added)
+++ 
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProTypeConstCastCheck.h 
Wed Oct  7 07:24:57 2015
@@ -0,0 +1,34 @@
+//===--- ProTypeConstCastCheck.h - clang-tidy*- C++ 
-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+

Re: [PATCH] D13398: [clang-tidy] add check cppcoreguidelines-pro-type-const-cast

[Aaron Ballman via cfe-commits]

aaron.ballman closed this revision.
aaron.ballman added a comment.

Thanks for the patch! I've commit in r249540.

Given the number of checkers that you're working on, I would recommend getting 
commit privileges if you'd like them.


http://reviews.llvm.org/D13398



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

Re: [PATCH] D13368: [clang-tidy] add check cppcoreguidelines-pro-type-static-cast-downcast

[Aaron Ballman via cfe-commits]

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

On Tue, Oct 6, 2015 at 5:46 PM, Matthias Gehre  wrote:

> mgehre marked an inline comment as done.

>  mgehre added a comment.

> 

> I cannot think of any way to improve the usefulness of the diagnostic in the 
> non-polymorphic case.

>  For now, this patch has a link to the CppCoreGuidelines document in the 
> docs/**/*.rst file.


Neither can I, so I guess it stands for right now.

> Also, this check is not enabled by default. One has to explicitly enable it, 
> and may at that

>  point reason about the usefulness of the CppCoreGuidelines and this check.


That is true, but enabling this check doesn't always require thoughtfulness on 
the part of the user. They can enable the check with 
-checks=corecppguidelines-* for instance. However, they can at least find the 
documentation from the check failures and hopefully go from there.

> 

>  Comment at: 
> test/clang-tidy/cppcoreguidelines-pro-type-static-cast-downcast.cpp:39

>  @@ +38,3 @@

>  +  auto PP0 = static_cast(new PolymorphicBase());

>  +  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: do not use static_cast to 
> downcast from a base to a derived class; use dynamic_cast instead 
> [cppcoreguidelines-pro-type-static-cast-downcast]

>  +  // CHECK-FIXES: auto PP0 = dynamic_cast(new 
> PolymorphicBase());

> 

>  

> 

> aaron.ballman wrote:

> 

> > No need to have the [cppcoreguidelines-pro-type-static-cast-downcast] part 
> > of the message on anything but the first diagnostic in the file. (This 
> > reduces clutter, but we test it one time just to make sure it's there).

> 

> 

> I can remove it on the remaining messages that end in "use dynamic_cast 
> instead". I need to keep it in the non-polymorphic case

>  to make sure that they are not followed by "use dynamic_cast instead".


Ah, that's an excellent point.

Patch LGTM! Since Samuel had comments as well, I will wait for his sign-off 
before committing.

~Aaron


http://reviews.llvm.org/D13368



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

Re: [PATCH] D13311: [clang-tidy] Add check cppcoreguidelines-pro-bounds-pointer-arithmetic

[Aaron Ballman via cfe-commits]

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

On Tue, Oct 6, 2015 at 5:31 PM, Matthias Gehre  wrote:

> mgehre marked 2 inline comments as done.

> 

> 

>  Comment at: 
> test/clang-tidy/cppcoreguidelines-pro-bounds-pointer-arithmetic.cpp:37

>  @@ +36,3 @@

>  +  q -= i;

>  +  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not use pointer arithmetic

>  +  q -= ENUM_LITERAL;

> 

>  

> 

> aaron.ballman wrote:

> 

> > I don't think this comment is "done" yet. I still don't know how this check 
> > is intended to handle code like that. Does it currently diagnose? Does it 
> > not diagnose? Should it diagnose?

> 

> 

> I had added your code, see line 55 of this file.


I am pretty sure I missed this by accidentally fiddling with a Phab knob, 
because this is definitely in there this morning, but wasn't last night. Sorry 
for the noise!

> It should diagnose, because an arithmetic operation (+) is used, which 
> results in a (possibly) changed pointer.

>  It does diagnose, as the CHECK-MESSAGE shows.


Great, thank you!

> 

>  Comment at: 
> test/clang-tidy/cppcoreguidelines-pro-bounds-pointer-arithmetic.cpp:51

>  @@ +50,3 @@

>  +

>  +  i = p[1];

>  +  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not use pointer arithmetic

> 

>  

> 

> aaron.ballman wrote:

> 

> > How well does this handle code like:

> 

> > 

> 

> >   void f(int i[], size_t s) {

> 

> > i[s - 1] = 0;

> 

> >   }

> 

> > 

> 

> > 

> 

> > Does it diagnose, and should it?

> 

> 

> Good point, I'll ask at https://github.com/isocpp/CppCoreGuidelines/issues/299


Thanks! The answer to this question should not hold up this patch, IMO.

LGTM!

~Aaron


http://reviews.llvm.org/D13311



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

Re: [PATCH] D13482: Revised Initial patch for PS4 toolchain

[pierre gousseau via cfe-commits]

pgousseau added a comment.

For some reasons the patch file I get from the "download raw diff link" does 
not contain the diff for the added .keep files? But the .keep files show up in 
phabricator, so I guess it is a phabricator issue?


Repository:
  rL LLVM

http://reviews.llvm.org/D13482



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

Re: r249392 - clang-format: Make IncludeCategories configurable in .clang-format file.

[Aaron Ballman via cfe-commits]

I am now getting sphinx warnings from this commit:

/opt/llvm/build-llvm/src/llvm/tools/clang/docs/ClangFormatStyleOptions.rst:159:
ERROR: Error in "code-block" directive:
maximum 1 argument(s) allowed, 3 supplied.

.. code-block:: c++
  someLongFunction(argument1,
   argument2);
/opt/llvm/build-llvm/src/llvm/tools/clang/docs/ClangFormatStyleOptions.rst:169:
ERROR: Error in "code-block" directive:
maximum 1 argument(s) allowed, 13 supplied.

.. code-block:: c++
  int  = 12;
  int b= 23;
  int ccc  = 23;
/opt/llvm/build-llvm/src/llvm/tools/clang/docs/ClangFormatStyleOptions.rst:180:
ERROR: Error in "code-block" directive:
maximum 1 argument(s) allowed, 13 supplied.

.. code-block:: c++
  int  = 12;
  float   b = 23;
  std::string ccc = 23;
/opt/llvm/build-llvm/src/llvm/tools/clang/docs/ClangFormatStyleOptions.rst:396:
ERROR: Error in "code-block" directive:
maximum 1 argument(s) allowed, 4 supplied.

.. code-block:: c++
  FOREACH(, ...)

/opt/llvm/build-llvm/src/llvm/tools/clang/docs/ClangFormatStyleOptions.rst:402:
ERROR: Error in "code-block" directive:
maximum 1 argument(s) allowed, 4 supplied.

.. code-block:: c++
  ForEachMacros: ['RANGES_FOR', 'FOREACH']
/opt/llvm/build-llvm/src/llvm/tools/clang/docs/ClangFormatStyleOptions.rst:424:
ERROR: Error in "code-block" directive:
maximum 1 argument(s) allowed, 17 supplied.

.. code-block:: c++
  IncludeCategories:
- Regex:   '^"(llvm|llvm-c|clang|clang-c)/'
  Priority:2
- Regex:   '^(<|"(gtest|isl|json)/)'
  Priority:3
- Regex:   '.\*'
  Priority:1

~Aaron

On Tue, Oct 6, 2015 at 7:54 AM, Daniel Jasper via cfe-commits
 wrote:
> Author: djasper
> Date: Tue Oct  6 06:54:18 2015
> New Revision: 249392
>
> URL: http://llvm.org/viewvc/llvm-project?rev=249392&view=rev
> Log:
> clang-format: Make IncludeCategories configurable in .clang-format file.
>
> This was made much easier by introducing an IncludeCategory struct to
> replace the previously used std::pair.
>
> Also, cleaned up documentation and added examples.
>
> Modified:
> cfe/trunk/docs/ClangFormatStyleOptions.rst
> cfe/trunk/docs/tools/dump_format_style.py
> cfe/trunk/include/clang/Format/Format.h
> cfe/trunk/lib/Format/Format.cpp
> cfe/trunk/unittests/Format/FormatTest.cpp
>
> Modified: cfe/trunk/docs/ClangFormatStyleOptions.rst
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ClangFormatStyleOptions.rst?rev=249392&r1=249391&r2=249392&view=diff
> ==
> --- cfe/trunk/docs/ClangFormatStyleOptions.rst (original)
> +++ cfe/trunk/docs/ClangFormatStyleOptions.rst Tue Oct  6 06:54:18 2015
> @@ -155,21 +155,29 @@ the configuration (without a prefix: ``A
>
>This applies to round brackets (parentheses), angle brackets and square
>brackets. This will result in formattings like
> -  \code
> -  someLongFunction(argument1,
> -  argument2);
> -  \endcode
> +  .. code-block:: c++
> +someLongFunction(argument1,
> + argument2);
>
>  **AlignConsecutiveAssignments** (``bool``)
>If ``true``, aligns consecutive assignments.
>
>This will align the assignment operators of consecutive lines. This
>will result in formattings like
> -  \code
> -  int  = 12;
> -  int b= 23;
> -  int ccc  = 23;
> -  \endcode
> +  .. code-block:: c++
> +int  = 12;
> +int b= 23;
> +int ccc  = 23;
> +
> +**AlignConsecutiveDeclarations** (``bool``)
> +  If ``true``, aligns consecutive declarations.
> +
> +  This will align the declaration names of consecutive lines. This
> +  will result in formattings like
> +  .. code-block:: c++
> +int  = 12;
> +float   b = 23;
> +std::string ccc = 23;
>
>  **AlignEscapedNewlinesLeft** (``bool``)
>If ``true``, aligns escaped newlines as far left as possible.
> @@ -381,14 +389,17 @@ the configuration (without a prefix: ``A
>instead of as function calls.
>
>These are expected to be macros of the form:
> -  \code
> -  FOREACH(, ...)
> -  
> -  \endcode
> +  .. code-block:: c++
> +FOREACH(, ...)
> +  
> +
> +  In the .clang-format configuration file, this can be configured like:
> +  .. code-block:: c++
> +ForEachMacros: ['RANGES_FOR', 'FOREACH']
>
>For example: BOOST_FOREACH.
>
> -**IncludeCategories** (``std::vector>``)
> +**IncludeCategories** (``std::vector``)
>Regular expressions denoting the different #include categories used
>for ordering #includes.
>
> @@ -403,6 +414,16 @@ the configuration (without a prefix: ``A
>so that it is kept at the beginning of the #includes
>(http://llvm.org/docs/CodingStandards.html#include-style).
>
> +  To configure this in the .clang-format file, use:
> +  .. code-block:: c++
> +IncludeCategories:
> +  - Regex:   '^"(llvm|llvm-c|clang|clang-c)/'
> +Priority:2
> +  - Regex:   '^(<|

r249542 - clang-format: Hopefully fix code blocks in docs.

[Daniel Jasper via cfe-commits]

Author: djasper
Date: Wed Oct  7 08:02:45 2015
New Revision: 249542

URL: http://llvm.org/viewvc/llvm-project?rev=249542&view=rev
Log:
clang-format: Hopefully fix code blocks in docs.

Otherwise I will have to install sphinx ;)..

Modified:
cfe/trunk/docs/ClangFormatStyleOptions.rst
cfe/trunk/docs/tools/dump_format_style.py

Modified: cfe/trunk/docs/ClangFormatStyleOptions.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ClangFormatStyleOptions.rst?rev=249542&r1=249541&r2=249542&view=diff
==
--- cfe/trunk/docs/ClangFormatStyleOptions.rst (original)
+++ cfe/trunk/docs/ClangFormatStyleOptions.rst Wed Oct  7 08:02:45 2015
@@ -157,6 +157,7 @@ the configuration (without a prefix: ``A
   brackets. This will result in formattings like
 
   .. code-block:: c++
+
 someLongFunction(argument1,
  argument2);
 
@@ -167,6 +168,7 @@ the configuration (without a prefix: ``A
   will result in formattings like
 
   .. code-block:: c++
+
 int  = 12;
 int b= 23;
 int ccc  = 23;
@@ -178,6 +180,7 @@ the configuration (without a prefix: ``A
   will result in formattings like
 
   .. code-block:: c++
+
 int  = 12;
 float   b = 23;
 std::string ccc = 23;
@@ -394,12 +397,14 @@ the configuration (without a prefix: ``A
   These are expected to be macros of the form:
 
   .. code-block:: c++
+
 FOREACH(, ...)
   
 
   In the .clang-format configuration file, this can be configured like:
 
   .. code-block:: c++
+
 ForEachMacros: ['RANGES_FOR', 'FOREACH']
 
   For example: BOOST_FOREACH.
@@ -422,6 +427,7 @@ the configuration (without a prefix: ``A
   To configure this in the .clang-format file, use:
 
   .. code-block:: c++
+
 IncludeCategories:
   - Regex:   '^"(llvm|llvm-c|clang|clang-c)/'
 Priority:2

Modified: cfe/trunk/docs/tools/dump_format_style.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/tools/dump_format_style.py?rev=249542&r1=249541&r2=249542&view=diff
==
--- cfe/trunk/docs/tools/dump_format_style.py (original)
+++ cfe/trunk/docs/tools/dump_format_style.py Wed Oct  7 08:02:45 2015
@@ -87,7 +87,7 @@ class EnumValue:
 
 def clean_comment_line(line):
   if line == '/// \\code':
-return '\n.. code-block:: c++\n'
+return '\n.. code-block:: c++\n\n'
   if line == '/// \\endcode':
 return ''
   return line[4:] + '\n'


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

Re: r249392 - clang-format: Make IncludeCategories configurable in .clang-format file.

[Daniel Jasper via cfe-commits]

Should be fixed now. Sorry about that.

On Wed, Oct 7, 2015 at 3:00 PM, Aaron Ballman 
wrote:

> I am now getting sphinx warnings from this commit:
>
>
> /opt/llvm/build-llvm/src/llvm/tools/clang/docs/ClangFormatStyleOptions.rst:159:
> ERROR: Error in "code-block" directive:
> maximum 1 argument(s) allowed, 3 supplied.
>
> .. code-block:: c++
>   someLongFunction(argument1,
>argument2);
>
> /opt/llvm/build-llvm/src/llvm/tools/clang/docs/ClangFormatStyleOptions.rst:169:
> ERROR: Error in "code-block" directive:
> maximum 1 argument(s) allowed, 13 supplied.
>
> .. code-block:: c++
>   int  = 12;
>   int b= 23;
>   int ccc  = 23;
>
> /opt/llvm/build-llvm/src/llvm/tools/clang/docs/ClangFormatStyleOptions.rst:180:
> ERROR: Error in "code-block" directive:
> maximum 1 argument(s) allowed, 13 supplied.
>
> .. code-block:: c++
>   int  = 12;
>   float   b = 23;
>   std::string ccc = 23;
>
> /opt/llvm/build-llvm/src/llvm/tools/clang/docs/ClangFormatStyleOptions.rst:396:
> ERROR: Error in "code-block" directive:
> maximum 1 argument(s) allowed, 4 supplied.
>
> .. code-block:: c++
>   FOREACH(, ...)
> 
>
> /opt/llvm/build-llvm/src/llvm/tools/clang/docs/ClangFormatStyleOptions.rst:402:
> ERROR: Error in "code-block" directive:
> maximum 1 argument(s) allowed, 4 supplied.
>
> .. code-block:: c++
>   ForEachMacros: ['RANGES_FOR', 'FOREACH']
>
> /opt/llvm/build-llvm/src/llvm/tools/clang/docs/ClangFormatStyleOptions.rst:424:
> ERROR: Error in "code-block" directive:
> maximum 1 argument(s) allowed, 17 supplied.
>
> .. code-block:: c++
>   IncludeCategories:
> - Regex:   '^"(llvm|llvm-c|clang|clang-c)/'
>   Priority:2
> - Regex:   '^(<|"(gtest|isl|json)/)'
>   Priority:3
> - Regex:   '.\*'
>   Priority:1
>
> ~Aaron
>
> On Tue, Oct 6, 2015 at 7:54 AM, Daniel Jasper via cfe-commits
>  wrote:
> > Author: djasper
> > Date: Tue Oct  6 06:54:18 2015
> > New Revision: 249392
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=249392&view=rev
> > Log:
> > clang-format: Make IncludeCategories configurable in .clang-format file.
> >
> > This was made much easier by introducing an IncludeCategory struct to
> > replace the previously used std::pair.
> >
> > Also, cleaned up documentation and added examples.
> >
> > Modified:
> > cfe/trunk/docs/ClangFormatStyleOptions.rst
> > cfe/trunk/docs/tools/dump_format_style.py
> > cfe/trunk/include/clang/Format/Format.h
> > cfe/trunk/lib/Format/Format.cpp
> > cfe/trunk/unittests/Format/FormatTest.cpp
> >
> > Modified: cfe/trunk/docs/ClangFormatStyleOptions.rst
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ClangFormatStyleOptions.rst?rev=249392&r1=249391&r2=249392&view=diff
> >
> ==
> > --- cfe/trunk/docs/ClangFormatStyleOptions.rst (original)
> > +++ cfe/trunk/docs/ClangFormatStyleOptions.rst Tue Oct  6 06:54:18 2015
> > @@ -155,21 +155,29 @@ the configuration (without a prefix: ``A
> >
> >This applies to round brackets (parentheses), angle brackets and
> square
> >brackets. This will result in formattings like
> > -  \code
> > -  someLongFunction(argument1,
> > -  argument2);
> > -  \endcode
> > +  .. code-block:: c++
> > +someLongFunction(argument1,
> > + argument2);
> >
> >  **AlignConsecutiveAssignments** (``bool``)
> >If ``true``, aligns consecutive assignments.
> >
> >This will align the assignment operators of consecutive lines. This
> >will result in formattings like
> > -  \code
> > -  int  = 12;
> > -  int b= 23;
> > -  int ccc  = 23;
> > -  \endcode
> > +  .. code-block:: c++
> > +int  = 12;
> > +int b= 23;
> > +int ccc  = 23;
> > +
> > +**AlignConsecutiveDeclarations** (``bool``)
> > +  If ``true``, aligns consecutive declarations.
> > +
> > +  This will align the declaration names of consecutive lines. This
> > +  will result in formattings like
> > +  .. code-block:: c++
> > +int  = 12;
> > +float   b = 23;
> > +std::string ccc = 23;
> >
> >  **AlignEscapedNewlinesLeft** (``bool``)
> >If ``true``, aligns escaped newlines as far left as possible.
> > @@ -381,14 +389,17 @@ the configuration (without a prefix: ``A
> >instead of as function calls.
> >
> >These are expected to be macros of the form:
> > -  \code
> > -  FOREACH(, ...)
> > -  
> > -  \endcode
> > +  .. code-block:: c++
> > +FOREACH(, ...)
> > +  
> > +
> > +  In the .clang-format configuration file, this can be configured like:
> > +  .. code-block:: c++
> > +ForEachMacros: ['RANGES_FOR', 'FOREACH']
> >
> >For example: BOOST_FOREACH.
> >
> > -**IncludeCategories** (``std::vector unsigned>>``)
> > +**IncludeCategories** (``std::vector``)
> >Regular expressions denoting the different #include categories used
> >for ordering #includes.
> >
> > @@ -403,6 +414,16 @

Re: r249392 - clang-format: Make IncludeCategories configurable in .clang-format file.

[Aaron Ballman via cfe-commits]

On Wed, Oct 7, 2015 at 9:07 AM, Daniel Jasper  wrote:
> Should be fixed now. Sorry about that.

No worries, thank you for the quick fix!

~Aaron

>
> On Wed, Oct 7, 2015 at 3:00 PM, Aaron Ballman 
> wrote:
>>
>> I am now getting sphinx warnings from this commit:
>>
>>
>> /opt/llvm/build-llvm/src/llvm/tools/clang/docs/ClangFormatStyleOptions.rst:159:
>> ERROR: Error in "code-block" directive:
>> maximum 1 argument(s) allowed, 3 supplied.
>>
>> .. code-block:: c++
>>   someLongFunction(argument1,
>>argument2);
>>
>> /opt/llvm/build-llvm/src/llvm/tools/clang/docs/ClangFormatStyleOptions.rst:169:
>> ERROR: Error in "code-block" directive:
>> maximum 1 argument(s) allowed, 13 supplied.
>>
>> .. code-block:: c++
>>   int  = 12;
>>   int b= 23;
>>   int ccc  = 23;
>>
>> /opt/llvm/build-llvm/src/llvm/tools/clang/docs/ClangFormatStyleOptions.rst:180:
>> ERROR: Error in "code-block" directive:
>> maximum 1 argument(s) allowed, 13 supplied.
>>
>> .. code-block:: c++
>>   int  = 12;
>>   float   b = 23;
>>   std::string ccc = 23;
>>
>> /opt/llvm/build-llvm/src/llvm/tools/clang/docs/ClangFormatStyleOptions.rst:396:
>> ERROR: Error in "code-block" directive:
>> maximum 1 argument(s) allowed, 4 supplied.
>>
>> .. code-block:: c++
>>   FOREACH(, ...)
>> 
>>
>> /opt/llvm/build-llvm/src/llvm/tools/clang/docs/ClangFormatStyleOptions.rst:402:
>> ERROR: Error in "code-block" directive:
>> maximum 1 argument(s) allowed, 4 supplied.
>>
>> .. code-block:: c++
>>   ForEachMacros: ['RANGES_FOR', 'FOREACH']
>>
>> /opt/llvm/build-llvm/src/llvm/tools/clang/docs/ClangFormatStyleOptions.rst:424:
>> ERROR: Error in "code-block" directive:
>> maximum 1 argument(s) allowed, 17 supplied.
>>
>> .. code-block:: c++
>>   IncludeCategories:
>> - Regex:   '^"(llvm|llvm-c|clang|clang-c)/'
>>   Priority:2
>> - Regex:   '^(<|"(gtest|isl|json)/)'
>>   Priority:3
>> - Regex:   '.\*'
>>   Priority:1
>>
>> ~Aaron
>>
>> On Tue, Oct 6, 2015 at 7:54 AM, Daniel Jasper via cfe-commits
>>  wrote:
>> > Author: djasper
>> > Date: Tue Oct  6 06:54:18 2015
>> > New Revision: 249392
>> >
>> > URL: http://llvm.org/viewvc/llvm-project?rev=249392&view=rev
>> > Log:
>> > clang-format: Make IncludeCategories configurable in .clang-format file.
>> >
>> > This was made much easier by introducing an IncludeCategory struct to
>> > replace the previously used std::pair.
>> >
>> > Also, cleaned up documentation and added examples.
>> >
>> > Modified:
>> > cfe/trunk/docs/ClangFormatStyleOptions.rst
>> > cfe/trunk/docs/tools/dump_format_style.py
>> > cfe/trunk/include/clang/Format/Format.h
>> > cfe/trunk/lib/Format/Format.cpp
>> > cfe/trunk/unittests/Format/FormatTest.cpp
>> >
>> > Modified: cfe/trunk/docs/ClangFormatStyleOptions.rst
>> > URL:
>> > http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ClangFormatStyleOptions.rst?rev=249392&r1=249391&r2=249392&view=diff
>> >
>> > ==
>> > --- cfe/trunk/docs/ClangFormatStyleOptions.rst (original)
>> > +++ cfe/trunk/docs/ClangFormatStyleOptions.rst Tue Oct  6 06:54:18 2015
>> > @@ -155,21 +155,29 @@ the configuration (without a prefix: ``A
>> >
>> >This applies to round brackets (parentheses), angle brackets and
>> > square
>> >brackets. This will result in formattings like
>> > -  \code
>> > -  someLongFunction(argument1,
>> > -  argument2);
>> > -  \endcode
>> > +  .. code-block:: c++
>> > +someLongFunction(argument1,
>> > + argument2);
>> >
>> >  **AlignConsecutiveAssignments** (``bool``)
>> >If ``true``, aligns consecutive assignments.
>> >
>> >This will align the assignment operators of consecutive lines. This
>> >will result in formattings like
>> > -  \code
>> > -  int  = 12;
>> > -  int b= 23;
>> > -  int ccc  = 23;
>> > -  \endcode
>> > +  .. code-block:: c++
>> > +int  = 12;
>> > +int b= 23;
>> > +int ccc  = 23;
>> > +
>> > +**AlignConsecutiveDeclarations** (``bool``)
>> > +  If ``true``, aligns consecutive declarations.
>> > +
>> > +  This will align the declaration names of consecutive lines. This
>> > +  will result in formattings like
>> > +  .. code-block:: c++
>> > +int  = 12;
>> > +float   b = 23;
>> > +std::string ccc = 23;
>> >
>> >  **AlignEscapedNewlinesLeft** (``bool``)
>> >If ``true``, aligns escaped newlines as far left as possible.
>> > @@ -381,14 +389,17 @@ the configuration (without a prefix: ``A
>> >instead of as function calls.
>> >
>> >These are expected to be macros of the form:
>> > -  \code
>> > -  FOREACH(, ...)
>> > -  
>> > -  \endcode
>> > +  .. code-block:: c++
>> > +FOREACH(, ...)
>> > +  
>> > +
>> > +  In the .clang-format configuration file, this can be configured like:
>> > +  .. code-block:: c++
>> > +ForEachMacros: ['RANGES_FOR', 'FOREACH']
>> >
>> >F

[PATCH] D13510: [PATCH] Support C++ Core Guidelines copy assignment restrictions

[Aaron Ballman via cfe-commits]

aaron.ballman created this revision.
aaron.ballman added reviewers: Eugene.Zelenko, alexfh, mgehre, sbenza.
aaron.ballman added a subscriber: cfe-commits.

Eugene pointed out to me that our existing misc-assign-operator-signature check 
was almost perfectly implementing the C++ Core Guideline guidance for Copy and 
move, found here: 
https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#-ccopy-copy-and-move.
 The only part that was missing was ensuring that the operator wasn't marked as 
virtual.

In this patch, I've implemented the check for the virtual qualifier, and 
registered the checker as a cppcoreguideline checker in addition to its usual 
place in misc.

~Aaron

http://reviews.llvm.org/D13510

Files:
  clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tidy/misc/AssignOperatorSignatureCheck.cpp
  test/clang-tidy/misc-assign-operator-signature.cpp

Index: test/clang-tidy/misc-assign-operator-signature.cpp
===
--- test/clang-tidy/misc-assign-operator-signature.cpp
+++ test/clang-tidy/misc-assign-operator-signature.cpp
@@ -49,3 +49,8 @@
   // Pre-C++11 way of disabling assignment.
   void operator=(const Private &);
 };
+
+struct Virtual {
+  virtual Virtual& operator=(const Virtual &);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: operator=() should not be marked 
'virtual'
+};
Index: clang-tidy/misc/AssignOperatorSignatureCheck.cpp
===
--- clang-tidy/misc/AssignOperatorSignatureCheck.cpp
+++ clang-tidy/misc/AssignOperatorSignatureCheck.cpp
@@ -51,11 +51,11 @@
   .bind("ArgumentType"),
   this);
 
-  Finder->addMatcher(cxxMethodDecl(IsSelfAssign, isConst()).bind("Const"),
- this);
+  Finder->addMatcher(
+  cxxMethodDecl(IsSelfAssign, anyOf(isConst(), isVirtual())).bind("cv"),
+  this);
 }
 
-
 void AssignOperatorSignatureCheck::check(
 const MatchFinder::MatchResult &Result) {
   const auto* Method = Result.Nodes.getNodeAs("method");
@@ -64,12 +64,13 @@
   static const char *Messages[][2] = {
   {"ReturnType", "operator=() should return '%0&'"},
   {"ArgumentType", "operator=() should take '%0 const&', '%0&&' or '%0'"},
-  {"Const", "operator=() should not be marked 'const'"},
+  {"cv", "operator=() should not be marked '%1'"}
   };
 
-  for (const auto& Message : Messages) {
+  for (const auto &Message : Messages) {
 if (Result.Nodes.getNodeAs(Message[0]))
-  diag(Method->getLocStart(), Message[1]) << Name;
+  diag(Method->getLocStart(), Message[1])
+  << Name << (Method->isConst() ? "const" : "virtual");
   }
 }
 
Index: clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
===
--- clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
+++ clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
@@ -10,6 +10,7 @@
 #include "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
+#include "../misc/AssignOperatorSignatureCheck.h"
 #include "ProTypeConstCastCheck.h"
 #include "ProTypeReinterpretCastCheck.h"
 
@@ -24,6 +25,8 @@
 "cppcoreguidelines-pro-type-const-cast");
 CheckFactories.registerCheck(
 "cppcoreguidelines-pro-type-reinterpret-cast");
+CheckFactories.registerCheck(
+"cppcoreguidelines-c-copy-assignment-signature");
   }
 };
 


Index: test/clang-tidy/misc-assign-operator-signature.cpp
===
--- test/clang-tidy/misc-assign-operator-signature.cpp
+++ test/clang-tidy/misc-assign-operator-signature.cpp
@@ -49,3 +49,8 @@
   // Pre-C++11 way of disabling assignment.
   void operator=(const Private &);
 };
+
+struct Virtual {
+  virtual Virtual& operator=(const Virtual &);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: operator=() should not be marked 'virtual'
+};
Index: clang-tidy/misc/AssignOperatorSignatureCheck.cpp
===
--- clang-tidy/misc/AssignOperatorSignatureCheck.cpp
+++ clang-tidy/misc/AssignOperatorSignatureCheck.cpp
@@ -51,11 +51,11 @@
   .bind("ArgumentType"),
   this);
 
-  Finder->addMatcher(cxxMethodDecl(IsSelfAssign, isConst()).bind("Const"),
- this);
+  Finder->addMatcher(
+  cxxMethodDecl(IsSelfAssign, anyOf(isConst(), isVirtual())).bind("cv"),
+  this);
 }
 
-
 void AssignOperatorSignatureCheck::check(
 const MatchFinder::MatchResult &Result) {
   const auto* Method = Result.Nodes.getNodeAs("method");
@@ -64,12 +64,13 @@
   static const char *Messages[][2] = {
   {"ReturnType", "operator=() should return '%0&'"},
   {"ArgumentType", "operator=() should take '%0 const&', '%0&&' or '%0'"},
-  {"Const", "operator=() should not be marked 'const'"},
+  {"cv", "operator=() should not be marked '%1'"}
   };
 

Re: [PATCH] D13304: Avoid inlining in throw statement

[Jun Bum Lim via cfe-commits]

junbuml added a comment.

Is there any comment on this change? Note that with change, I observed about 6% 
performance improvement in spec2006/xalancbmk. I will address any comment to 
get this in.


http://reviews.llvm.org/D13304



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

Re: [PATCH] D12358: [Analyzer] Handling constant bound loops

[Sean Eveson via cfe-commits]

seaneveson updated this revision to Diff 36739.
seaneveson added a comment.

Updated to latest revision.
Change patch to widen all loops which do not exit.
Changed to widen on block entrance so the guard condition is accounted for 
(thanks for the suggestion).
Updated tests to reflect changes.


http://reviews.llvm.org/D12358

Files:
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  include/clang/StaticAnalyzer/Core/PathSensitive/LoopWidening.h
  lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
  lib/StaticAnalyzer/Core/CMakeLists.txt
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  lib/StaticAnalyzer/Core/LoopWidening.cpp
  test/Analysis/analyzer-config.c
  test/Analysis/analyzer-config.cpp
  test/Analysis/loop-widening.c

Index: test/Analysis/loop-widening.c
===
--- test/Analysis/loop-widening.c
+++ test/Analysis/loop-widening.c
@@ -0,0 +1,159 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-store=region -analyzer-max-loop 4 -analyzer-config widen-loops=true -verify %s
+
+extern void clang_analyzer_eval(_Bool);
+extern void clang_analyzer_warnIfReached();
+
+typedef __typeof(sizeof(int)) size_t;
+void *malloc(size_t);
+void free(void *);
+
+void loop_which_iterates_limit_times_not_widened() {
+  int i;
+  int x = 1;
+  // Check loop isn't widened by checking x isn't invalidated
+  for (i = 0; i < 1; ++i) {}
+  clang_analyzer_eval(x == 1); // expected-warning {{TRUE}}
+  for (i = 0; i < 2; ++i) {}
+  clang_analyzer_eval(x == 1); // expected-warning {{TRUE}}
+  for (i = 0; i < 3; ++i) {}
+  clang_analyzer_eval(x == 1); // expected-warning {{TRUE}} // TODO?
+}
+
+int a_global;
+
+void loop_evaluated_before_widening() {
+  int i;
+  a_global = 1;
+  for (i = 0; i < 10; ++i) {
+if (i == 2) {
+  // True before widening then unknown after
+  clang_analyzer_eval(a_global == 1); // expected-warning{{TRUE}} expected-warning{{UNKNOWN}}
+}
+  }
+  clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void warnings_after_loop() {
+  int i;
+  for (i = 0; i < 10; ++i) {}
+  char *m = (char*)malloc(12);
+} // expected-warning {{Potential leak of memory pointed to by 'm'}}
+
+void for_loop_exits() {
+  int i;
+  for (i = 0; i < 10; ++i) {}
+  clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void while_loop_exits() {
+  int i = 0;
+  while (i < 10) {++i;}
+  clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void do_while_loop_exits() {
+  int i = 0;
+  do {++i;} while (i < 10);
+  clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void loop_body_is_widened() {
+  int i = 0;
+  while (i < 100) {
+if (i > 10) {
+  clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+++i;
+  }
+}
+
+void invariably_infinite_loop() {
+  int i = 0;
+  while (1) { ++i; }
+  clang_analyzer_warnIfReached(); // no-warning
+}
+
+void invariably_infinite_break_loop() {
+  int i = 0;
+  while (1) {
+++i;
+int x = 1;
+if (!x) break;
+  }
+  clang_analyzer_warnIfReached(); // no-warning
+}
+
+void reachable_break_loop() {
+  int i = 0;
+  while (1) {
+++i;
+if (i == 100) break;
+  }
+  clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void condition_constrained_true_in_loop() {
+  int i = 0;
+  while (i < 50) {
+if (i >= 50) {
+  clang_analyzer_warnIfReached(); // no-warning
+}
+++i;
+  }
+  clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void condition_constrained_false_after_loop() {
+  int i = 0;
+  while (i < 50) {
+++i;
+  }
+  if (i < 50) {
+clang_analyzer_warnIfReached(); // no-warning
+  }
+  clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void multiple_exit_test() {
+  int x = 0;
+  int i = 0;
+  while (i < 50) {
+if (x) {
+  i = 10;
+  break;
+}
+++i;
+  }
+  // Reachable by 'normal' exit
+  if (i == 50) clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  // Reachable by break point
+  if (i == 10) clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  // Not reachable
+  if (i < 10) clang_analyzer_warnIfReached(); // no-warning
+  if (i > 10 && i < 50) clang_analyzer_warnIfReached(); // no-warning
+}
+
+void pointer_doesnt_leak_from_loop() {
+  int *h_ptr = (int *) malloc(sizeof(int));
+  for (int i = 0; i < 2; ++i) {}
+  for (int i = 0; i < 10; ++i) {} // no-warning // TODO
+  free(h_ptr);
+}
+
+int g_global;
+
+void unknown_after_loop(int s_arg) {
+  g_global = 0;
+  s_arg = 1;
+  int s_local = 2;
+  int *h_ptr = malloc(sizeof(int));
+  *h_ptr = 3;
+  
+  for (int i = 0; i < 10; ++i) {}
+  
+  clang_analyzer_eval(g_global); // expected-warning {{UNKNOWN}}
+  clang_analyzer_eval(s_arg); // expected-warning {{UNKNOWN}}
+  clang_analyzer_eval(s_local); // expected-warning {{UNKNOWN}}
+  clang_analyzer_eval(h_ptr); // expected-warning {{UNKNOWN}}
+  free

[PATCH] D13513: [clang-format] Stop alignment sequences on open braces and parens when aligning assignments.

[Beren Minor via cfe-commits]

berenm created this revision.
berenm added a reviewer: djasper.
berenm added a subscriber: cfe-commits.
Herald added a subscriber: klimek.

This was done correctly when aligning the declarations, but not when aligning 
assignments.

FIXME: The code between assignments and declarations alignment is roughly 
duplicated and
would benefit from factorization.

Bug 25090: https://llvm.org/bugs/show_bug.cgi?id=25090

http://reviews.llvm.org/D13513

Files:
  lib/Format/WhitespaceManager.cpp
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -8649,6 +8649,19 @@
"someLongFunction();\n"
"int j = 2;",
Alignment);
+
+  verifyFormat("auto lambda = []() {\n"
+   "  auto i = 0;\n"
+   "  return 0;\n"
+   "};\n"
+   "int i  = 0;\n"
+   "auto v = type{\n"
+   "i = 1,   //\n"
+   "(i = 2), //\n"
+   "i = 3//\n"
+   "};",
+   Alignment);
+
   // FIXME: Should align all three assignments
   verifyFormat(
   "int i  = 1;\n"
@@ -8817,6 +8830,23 @@
"someLongFunction();\n"
"int j = 2;",
Alignment);
+
+  Alignment.AlignConsecutiveAssignments = true;
+  verifyFormat("auto lambda = []() {\n"
+   "  auto  ii = 0;\n"
+   "  float j  = 0;\n"
+   "  return 0;\n"
+   "};\n"
+   "int   i  = 0;\n"
+   "float i2 = 0;\n"
+   "auto  v  = type{\n"
+   "i = 1,   //\n"
+   "(i = 2), //\n"
+   "i = 3//\n"
+   "};",
+   Alignment);
+  Alignment.AlignConsecutiveAssignments = false;
+
   // FIXME: Should align all three declarations
   verifyFormat(
   "int  i = 1;\n"
Index: lib/Format/WhitespaceManager.cpp
===
--- lib/Format/WhitespaceManager.cpp
+++ lib/Format/WhitespaceManager.cpp
@@ -153,6 +153,9 @@
 // a "=" is found on a line, extend the current sequence. If the current line
 // cannot be part of a sequence, e.g. because there is an empty line before it
 // or it contains non-assignments, finalize the previous sequence.
+//
+// FIXME: The code between assignment and declaration alignment is mostly
+// duplicated and would benefit from factorization.
 void WhitespaceManager::alignConsecutiveAssignments() {
   if (!Style.AlignConsecutiveAssignments)
 return;
@@ -162,6 +165,7 @@
   unsigned StartOfSequence = 0;
   unsigned EndOfSequence = 0;
   bool FoundAssignmentOnLine = false;
+  bool FoundLeftBraceOnLine = false;
   bool FoundLeftParenOnLine = false;
 
   // Aligns a sequence of assignment tokens, on the MinColumn column.
@@ -181,18 +185,21 @@
   };
 
   for (unsigned i = 0, e = Changes.size(); i != e; ++i) {
-if (Changes[i].NewlinesBefore > 0) {
+if (Changes[i].NewlinesBefore != 0) {
   EndOfSequence = i;
-  // If there is a blank line or if the last line didn't contain any
-  // assignment, the sequence ends here.
-  if (Changes[i].NewlinesBefore > 1 || !FoundAssignmentOnLine) {
+  // If there is a blank line, if the last line didn't contain any
+  // assignment, or if we found an open brace or paren, the sequence ends
+  // here.
+  if (Changes[i].NewlinesBefore > 1 || !FoundAssignmentOnLine ||
+  FoundLeftBraceOnLine || FoundLeftParenOnLine) {
 // NB: In the latter case, the sequence should end at the beggining of
 // the previous line, but it doesn't really matter as there is no
 // assignment on it
 AlignSequence();
   }
 
   FoundAssignmentOnLine = false;
+  FoundLeftBraceOnLine = false;
   FoundLeftParenOnLine = false;
 }
 
@@ -202,14 +209,24 @@
 (FoundAssignmentOnLine || Changes[i].NewlinesBefore > 0 ||
  Changes[i + 1].NewlinesBefore > 0)) {
   AlignSequence();
-} else if (!FoundLeftParenOnLine && Changes[i].Kind == tok::r_paren) {
-  AlignSequence();
+} else if (Changes[i].Kind == tok::r_brace) {
+  if (!FoundLeftBraceOnLine)
+AlignSequence();
+  FoundLeftBraceOnLine = false;
+} else if (Changes[i].Kind == tok::l_brace) {
+  FoundLeftBraceOnLine = true;
+  if (!FoundAssignmentOnLine)
+AlignSequence();
+} else if (Changes[i].Kind == tok::r_paren) {
+  if (!FoundLeftParenOnLine)
+AlignSequence();
+  FoundLeftParenOnLine = false;
 } else if (Changes[i].Kind == tok::l_paren) {
   FoundLeftParenOnLine = true;
   if (!FoundAssignmentOnLine)
 AlignSequence();
-} else if (!FoundAssignmentOnLine && !FoundLeftParenOnLine &&
-   Changes[i].Kind == tok::eq

Re: [PATCH] D12358: [Analyzer] Handling constant bound loops

[Sean Eveson via cfe-commits]

seaneveson marked 4 inline comments as done.
seaneveson added a comment.

There are some issues which haven't been resolved yet:

- There is a loss of precision for loops that need to be executed exactly 
maxBlockVisitOnPath times, as the loop body is executed with the widened state 
**instead** of the last iteration.
- The invalidation still causes memory leak false positives (see failing test: 
pointer_doesnt_leak_from_loop).


http://reviews.llvm.org/D12358



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

Re: [PATCH] D13513: [clang-format] Stop alignment sequences on open braces and parens when aligning assignments.

[Daniel Jasper via cfe-commits]

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

Looks good, thank you.



Comment at: unittests/Format/FormatTest.cpp:8657
@@ +8656,3 @@
+   "};\n"
+   "int i  = 0;\n"
+   "auto v = type{\n"

So, it is kind of interesting that we (would) align with a lambda on the next 
line here, but not with the lambda declaration a few lines above.

I guess this makes sense, but I would also vager that somebody is going to file 
a bug report about it ;-).


http://reviews.llvm.org/D13513



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

r249552 - [clang-format] Stop alignment sequences on open braces and parens when

[Daniel Jasper via cfe-commits]

Author: djasper
Date: Wed Oct  7 10:03:26 2015
New Revision: 249552

URL: http://llvm.org/viewvc/llvm-project?rev=249552&view=rev
Log:
[clang-format] Stop alignment sequences on open braces and parens when
aligning assignments.

This was done correctly when aligning the declarations, but not when
aligning assignments.

FIXME: The code between assignments and declarations alignment is
roughly duplicated and
would benefit from factorization.

Bug 25090: https://llvm.org/bugs/show_bug.cgi?id=25090

Patch by Beren Minor. Thank you.

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

Modified: cfe/trunk/lib/Format/WhitespaceManager.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/WhitespaceManager.cpp?rev=249552&r1=249551&r2=249552&view=diff
==
--- cfe/trunk/lib/Format/WhitespaceManager.cpp (original)
+++ cfe/trunk/lib/Format/WhitespaceManager.cpp Wed Oct  7 10:03:26 2015
@@ -153,6 +153,9 @@ void WhitespaceManager::calculateLineBre
 // a "=" is found on a line, extend the current sequence. If the current line
 // cannot be part of a sequence, e.g. because there is an empty line before it
 // or it contains non-assignments, finalize the previous sequence.
+//
+// FIXME: The code between assignment and declaration alignment is mostly
+// duplicated and would benefit from factorization.
 void WhitespaceManager::alignConsecutiveAssignments() {
   if (!Style.AlignConsecutiveAssignments)
 return;
@@ -162,6 +165,7 @@ void WhitespaceManager::alignConsecutive
   unsigned StartOfSequence = 0;
   unsigned EndOfSequence = 0;
   bool FoundAssignmentOnLine = false;
+  bool FoundLeftBraceOnLine = false;
   bool FoundLeftParenOnLine = false;
 
   // Aligns a sequence of assignment tokens, on the MinColumn column.
@@ -181,11 +185,13 @@ void WhitespaceManager::alignConsecutive
   };
 
   for (unsigned i = 0, e = Changes.size(); i != e; ++i) {
-if (Changes[i].NewlinesBefore > 0) {
+if (Changes[i].NewlinesBefore != 0) {
   EndOfSequence = i;
-  // If there is a blank line or if the last line didn't contain any
-  // assignment, the sequence ends here.
-  if (Changes[i].NewlinesBefore > 1 || !FoundAssignmentOnLine) {
+  // If there is a blank line, if the last line didn't contain any
+  // assignment, or if we found an open brace or paren, the sequence ends
+  // here.
+  if (Changes[i].NewlinesBefore > 1 || !FoundAssignmentOnLine ||
+  FoundLeftBraceOnLine || FoundLeftParenOnLine) {
 // NB: In the latter case, the sequence should end at the beggining of
 // the previous line, but it doesn't really matter as there is no
 // assignment on it
@@ -193,6 +199,7 @@ void WhitespaceManager::alignConsecutive
   }
 
   FoundAssignmentOnLine = false;
+  FoundLeftBraceOnLine = false;
   FoundLeftParenOnLine = false;
 }
 
@@ -202,14 +209,24 @@ void WhitespaceManager::alignConsecutive
 (FoundAssignmentOnLine || Changes[i].NewlinesBefore > 0 ||
  Changes[i + 1].NewlinesBefore > 0)) {
   AlignSequence();
-} else if (!FoundLeftParenOnLine && Changes[i].Kind == tok::r_paren) {
-  AlignSequence();
+} else if (Changes[i].Kind == tok::r_brace) {
+  if (!FoundLeftBraceOnLine)
+AlignSequence();
+  FoundLeftBraceOnLine = false;
+} else if (Changes[i].Kind == tok::l_brace) {
+  FoundLeftBraceOnLine = true;
+  if (!FoundAssignmentOnLine)
+AlignSequence();
+} else if (Changes[i].Kind == tok::r_paren) {
+  if (!FoundLeftParenOnLine)
+AlignSequence();
+  FoundLeftParenOnLine = false;
 } else if (Changes[i].Kind == tok::l_paren) {
   FoundLeftParenOnLine = true;
   if (!FoundAssignmentOnLine)
 AlignSequence();
-} else if (!FoundAssignmentOnLine && !FoundLeftParenOnLine &&
-   Changes[i].Kind == tok::equal) {
+} else if (!FoundAssignmentOnLine && !FoundLeftBraceOnLine &&
+   !FoundLeftParenOnLine && Changes[i].Kind == tok::equal) {
   FoundAssignmentOnLine = true;
   if (StartOfSequence == 0)
 StartOfSequence = i;
@@ -267,6 +284,9 @@ void WhitespaceManager::alignConsecutive
 // current line cannot be part of a sequence, e.g. because there is an empty
 // line before it or it contains non-declarations, finalize the previous
 // sequence.
+//
+// FIXME: The code between assignment and declaration alignment is mostly
+// duplicated and would benefit from factorization.
 void WhitespaceManager::alignConsecutiveDeclarations() {
   if (!Style.AlignConsecutiveDeclarations)
 return;

Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=249552&r1=249551&r2=249552&view=diff
==
--- cfe/trunk/unittests/Format/FormatTe

Re: [PATCH] D13221: Make CompilerInvocation's use of the debug options more understandable.

[Douglas Katzman via cfe-commits]

dougk marked an inline comment as done.
dougk added a comment.

James, you're right, 'gdwarf-2' followed by 'line-tables-only' works, but the 
opposite order doesn't.
So it halfway works, which is better than not working at all.



Comment at: lib/Driver/Tools.cpp:2353
@@ +2352,3 @@
+  }
+  switch (DwarfVersion) {
+  case 2:

jyknight wrote:
> How about:
>   if (DwarfVersion > 0) CmdArgs.push_back("-dwarf-version=" + DwarfVersion);
minor point: the "+" syntax you've used is actually pointer arithmetic, but 
giving the benefit of doubt there, and writing it as to_string(DwarfVersion), 
the annoying thing is that CmdArgs wants a 'const char*'.
To get one, you have to call MakeArgString which is a method on Args, not 
CmdArgs, so you have to pass both.
I did that in the latest patch, see what you think.


http://reviews.llvm.org/D13221



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

Re: [PATCH] D13513: [clang-format] Stop alignment sequences on open braces and parens when aligning assignments.

[Daniel Jasper via cfe-commits]

djasper closed this revision.
djasper added a comment.

Submitted as r249552.


http://reviews.llvm.org/D13513



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

Re: [PATCH] D13513: [clang-format] Stop alignment sequences on open braces and parens when aligning assignments.

[Beren Minor via cfe-commits]

berenm added inline comments.


Comment at: unittests/Format/FormatTest.cpp:8657
@@ +8656,3 @@
+   "};\n"
+   "int i  = 0;\n"
+   "auto v = type{\n"

djasper wrote:
> So, it is kind of interesting that we (would) align with a lambda on the next 
> line here, but not with the lambda declaration a few lines above.
> 
> I guess this makes sense, but I would also vager that somebody is going to 
> file a bug report about it ;-).
Yes, this would require to handle nested levels of alignment sequences and push 
the current one when a new scope is detected. It's just not implemented yet :)


http://reviews.llvm.org/D13513



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

Re: [PATCH] D12489: [clang-format] Fixed missing space between Obj-C for/in and a typecast

[Daniel Jasper via cfe-commits]

djasper closed this revision.
djasper added a comment.

Submitted as r249553. Sorry for the delay.


http://reviews.llvm.org/D12489



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

[clang-tools-extra] r249555 - Loosening the restriction on variadic function definitions so that extern "C" function definitions are permissible.

[Aaron Ballman via cfe-commits]

Author: aaronballman
Date: Wed Oct  7 10:14:10 2015
New Revision: 249555

URL: http://llvm.org/viewvc/llvm-project?rev=249555&view=rev
Log:
Loosening the restriction on variadic function definitions so that extern "C" 
function definitions are permissible.

Modified:
clang-tools-extra/trunk/clang-tidy/cert/VariadicFunctionDefCheck.cpp
clang-tools-extra/trunk/test/clang-tidy/cert-variadic-function-def.cpp

Modified: clang-tools-extra/trunk/clang-tidy/cert/VariadicFunctionDefCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/cert/VariadicFunctionDefCheck.cpp?rev=249555&r1=249554&r2=249555&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/cert/VariadicFunctionDefCheck.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/cert/VariadicFunctionDefCheck.cpp Wed 
Oct  7 10:14:10 2015
@@ -20,9 +20,12 @@ void VariadicFunctionDefCheck::registerM
   if (!getLangOpts().CPlusPlus)
 return;
 
-  // We only care about function *definitions* that are variadic.
-  Finder->addMatcher(functionDecl(isDefinition(), isVariadic()).bind("func"),
- this);
+  // We only care about function *definitions* that are variadic, and do not
+  // have extern "C" language linkage.
+  Finder->addMatcher(
+  functionDecl(isDefinition(), isVariadic(), unless(isExternC()))
+  .bind("func"),
+  this);
 }
 
 void VariadicFunctionDefCheck::check(const MatchFinder::MatchResult &Result) {

Modified: clang-tools-extra/trunk/test/clang-tidy/cert-variadic-function-def.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/cert-variadic-function-def.cpp?rev=249555&r1=249554&r2=249555&view=diff
==
--- clang-tools-extra/trunk/test/clang-tidy/cert-variadic-function-def.cpp 
(original)
+++ clang-tools-extra/trunk/test/clang-tidy/cert-variadic-function-def.cpp Wed 
Oct  7 10:14:10 2015
@@ -16,3 +16,9 @@ struct S {
   void f1(int, ...) {}
   // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: do not define a C-style variadic 
function; consider using a function parameter pack or currying instead
 };
+
+// Function definitions that are extern "C" are good.
+extern "C" void f4(int, ...) {} // ok
+extern "C" {
+  void f5(int, ...) {} // ok
+}


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

Re: [PATCH] D13446: [PATCH] Add checker discouraging definition of variadic function definitions in C++

[Aaron Ballman via cfe-commits]

On Tue, Oct 6, 2015 at 4:51 PM, Aaron Ballman  wrote:
> On Tue, Oct 6, 2015 at 4:49 PM, Joerg Sonnenberger via cfe-commits
>  wrote:
>> On Tue, Oct 06, 2015 at 04:20:05PM -0400, Aaron Ballman via cfe-commits 
>> wrote:
>>> On Tue, Oct 6, 2015 at 4:12 PM, Joerg Sonnenberger via cfe-commits
>>>  wrote:
>>> > On Mon, Oct 05, 2015 at 07:36:08PM +, Aaron Ballman via cfe-commits 
>>> > wrote:
>>> >> C-style variadic functions (using an ellipsis) can be dangerous in C++
>>> >> due to the inherit lack of type safety with argument passing. Better
>>> >> alternatives exist, such as function currying (like STL stream objects
>>> >> use), or function parameter packs. This patch adds a checker to
>>> >> diagnose definitions of variadic functions in C++ code, but still
>>> >> allows variadic function declarations, as those can be safely used to
>>> >> good effect for SFINAE patterns.
>>> >
>>> > I would restrict this a bit to exclude function definitions with C
>>> > linkage. If you have such a ABI requirement, you normally can't replace
>>> > it with any of the alternatives.
>>>
>>> Under what circumstances would you run into this issue with C++ code?
>>> The only circumstance I can think of is if you have a C API that takes
>>> a function pointer to a varargs function as an argument. Is that the
>>> situation you are thinking of, or are there others?
>>
>> Consider a stdio implementation written in C++? Wouldn't the
>> implementation of e.g. vprintf trigger exactly this warning?
>
> It would, but it would also trigger a *lot* of warnings that aren't
> directed at implementers as well, such as anything about relying on
> implementation-defined or undefined behavior (like the implementation
> of offsetof(), as a trivial example). It's a good point to keep in
> mind though.

Thank you for the suggestion, Joerg -- I've implemented it in r249555,
and updated the CERT rule wording to match.

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

r249553 - clang-format: Fixed missing space between Obj-C for/in and a typecast.

[Daniel Jasper via cfe-commits]

Author: djasper
Date: Wed Oct  7 10:09:08 2015
New Revision: 249553

URL: http://llvm.org/viewvc/llvm-project?rev=249553&view=rev
Log:
clang-format: Fixed missing space between Obj-C for/in and a typecast.

Fixes this bug: https://llvm.org/bugs/show_bug.cgi?id=24504

TokenAnnotator::spaceRequiredBetween was handling TT_ForEachMacro but
not TT_ObjCForIn, so lines that look like:

  for (id nextObject in (NSArray *)myArray)

would incorrectly turn into:

  for (id nextObject in(NSArray *)myArray)

Patch by Kent Sutherland, thank you.

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

Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=249553&r1=249552&r2=249553&view=diff
==
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Wed Oct  7 10:09:08 2015
@@ -1902,7 +1902,8 @@ bool TokenAnnotator::spaceRequiredBetwee
 return Line.Type == LT_ObjCDecl || Left.is(tok::semi) ||
(Style.SpaceBeforeParens != FormatStyle::SBPO_Never &&
 (Left.isOneOf(tok::kw_if, tok::pp_elif, tok::kw_for, tok::kw_while,
-  tok::kw_switch, tok::kw_case, TT_ForEachMacro) ||
+  tok::kw_switch, tok::kw_case, TT_ForEachMacro,
+  TT_ObjCForIn) ||
  (Left.isOneOf(tok::kw_try, Keywords.kw___except, tok::kw_catch,
tok::kw_new, tok::kw_delete) &&
   (!Left.Previous || Left.Previous->isNot(tok::period) ||

Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=249553&r1=249552&r2=249553&view=diff
==
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Wed Oct  7 10:09:08 2015
@@ -7446,6 +7446,19 @@ TEST_F(FormatTest, ObjCSnippets) {
"@import baz;");
 }
 
+TEST_F(FormatTest, ObjCForIn) {
+  verifyFormat("- (void)test {\n"
+   "  for (NSString *n in arrayOfStrings) {\n"
+   "foo(n);\n"
+   "  }\n"
+   "}");
+  verifyFormat("- (void)test {\n"
+   "  for (NSString *n in (__bridge NSArray *)arrayOfStrings) {\n"
+   "foo(n);\n"
+   "  }\n"
+   "}");
+}
+
 TEST_F(FormatTest, ObjCLiterals) {
   verifyFormat("@\"String\"");
   verifyFormat("@1");


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

r249556 - [VFS] Port driver tool chains to VFS.

[Benjamin Kramer via cfe-commits]

Author: d0k
Date: Wed Oct  7 10:48:01 2015
New Revision: 249556

URL: http://llvm.org/viewvc/llvm-project?rev=249556&view=rev
Log:
[VFS] Port driver tool chains to VFS.

There are still some loose ends here but it's sufficient so we can detect
GCC headers that are inside of a VFS.

Added:
cfe/trunk/unittests/Driver/ToolChainTest.cpp
Modified:
cfe/trunk/include/clang/Basic/VirtualFileSystem.h
cfe/trunk/include/clang/Driver/Driver.h
cfe/trunk/include/clang/Driver/ToolChain.h
cfe/trunk/lib/Basic/VirtualFileSystem.cpp
cfe/trunk/lib/Driver/Driver.cpp
cfe/trunk/lib/Driver/ToolChain.cpp
cfe/trunk/lib/Driver/ToolChains.cpp
cfe/trunk/lib/Driver/ToolChains.h
cfe/trunk/lib/Driver/Tools.cpp
cfe/trunk/unittests/Driver/CMakeLists.txt

Modified: cfe/trunk/include/clang/Basic/VirtualFileSystem.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/VirtualFileSystem.h?rev=249556&r1=249555&r2=249556&view=diff
==
--- cfe/trunk/include/clang/Basic/VirtualFileSystem.h (original)
+++ cfe/trunk/include/clang/Basic/VirtualFileSystem.h Wed Oct  7 10:48:01 2015
@@ -206,6 +206,9 @@ public:
   /// Get the working directory of this file system.
   virtual llvm::ErrorOr getCurrentWorkingDirectory() const = 0;
 
+  /// Check whether a file exists. Provided for convenience.
+  bool exists(const Twine &Path);
+
   /// Make \a Path an absolute path.
   ///
   /// Makes \a Path absolute using the current directory if it is not already.

Modified: cfe/trunk/include/clang/Driver/Driver.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Driver.h?rev=249556&r1=249555&r2=249556&view=diff
==
--- cfe/trunk/include/clang/Driver/Driver.h (original)
+++ cfe/trunk/include/clang/Driver/Driver.h Wed Oct  7 10:48:01 2015
@@ -36,6 +36,11 @@ namespace opt {
 }
 
 namespace clang {
+
+namespace vfs {
+class FileSystem;
+}
+
 namespace driver {
 
   class Action;
@@ -54,6 +59,8 @@ class Driver {
 
   DiagnosticsEngine &Diags;
 
+  IntrusiveRefCntPtr VFS;
+
   enum DriverMode {
 GCCMode,
 GXXMode,
@@ -201,9 +208,9 @@ private:
  SmallVectorImpl &Names) const;
 
 public:
-  Driver(StringRef _ClangExecutable,
- StringRef _DefaultTargetTriple,
- DiagnosticsEngine &_Diags);
+  Driver(StringRef ClangExecutable, StringRef DefaultTargetTriple,
+ DiagnosticsEngine &Diags,
+ IntrusiveRefCntPtr VFS = nullptr);
   ~Driver();
 
   /// @name Accessors
@@ -216,6 +223,8 @@ public:
 
   const DiagnosticsEngine &getDiags() const { return Diags; }
 
+  vfs::FileSystem &getVFS() const { return *VFS; }
+
   bool getCheckInputsExist() const { return CheckInputsExist; }
 
   void setCheckInputsExist(bool Value) { CheckInputsExist = Value; }

Modified: cfe/trunk/include/clang/Driver/ToolChain.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/ToolChain.h?rev=249556&r1=249555&r2=249556&view=diff
==
--- cfe/trunk/include/clang/Driver/ToolChain.h (original)
+++ cfe/trunk/include/clang/Driver/ToolChain.h Wed Oct  7 10:48:01 2015
@@ -30,7 +30,10 @@ namespace opt {
 }
 
 namespace clang {
-  class ObjCRuntime;
+class ObjCRuntime;
+namespace vfs {
+class FileSystem;
+}
 
 namespace driver {
   class Compilation;
@@ -119,7 +122,8 @@ public:
 
   // Accessors
 
-  const Driver &getDriver() const;
+  const Driver &getDriver() const { return D; }
+  vfs::FileSystem &getVFS() const;
   const llvm::Triple &getTriple() const { return Triple; }
 
   llvm::Triple::ArchType getArch() const { return Triple.getArch(); }

Modified: cfe/trunk/lib/Basic/VirtualFileSystem.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/VirtualFileSystem.cpp?rev=249556&r1=249555&r2=249556&view=diff
==
--- cfe/trunk/lib/Basic/VirtualFileSystem.cpp (original)
+++ cfe/trunk/lib/Basic/VirtualFileSystem.cpp Wed Oct  7 10:48:01 2015
@@ -105,6 +105,11 @@ std::error_code FileSystem::makeAbsolute
   return llvm::sys::fs::make_absolute(WorkingDir.get(), Path);
 }
 
+bool FileSystem::exists(const Twine &Path) {
+  auto Status = status(Path);
+  return Status && Status->exists();
+}
+
 
//===---===/
 // RealFileSystem implementation
 
//===---===/

Modified: cfe/trunk/lib/Driver/Driver.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Driver.cpp?rev=249556&r1=249555&r2=249556&view=diff
==
--- cfe/trunk/lib/Driver/Driver.cpp (original)
+++ cfe/trunk/lib/Driver/Driver.cpp Wed Oct  7 10:48:01 2015
@@ -11,6 +11,7 @@
 #

Re: [clang-tools-extra] r249399 - Add a new module for the C++ Core Guidelines, and the first checker for those guidelines: cppcoreguidelines-pro-type-reinterpret-cast.

[Joerg Sonnenberger via cfe-commits]

On Tue, Oct 06, 2015 at 01:31:01PM -, Aaron Ballman via cfe-commits wrote:
> Log:
> Add a new module for the C++ Core Guidelines, and the first checker
> for those guidelines: cppcoreguidelines-pro-type-reinterpret-cast.

I wonder about the generality of this. Does it really make sense to flag
every reinterpret_cast of a void pointer? Such interface contracts
normally don't give you an alternative, so it seems a bit silly. This is
made worse by the concrete message used...

More importantly, can the checker detect all cases where
reinterpret_cast is used where a less generic cast applies?

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

Re: [PATCH] D13221: Make CompilerInvocation's use of the debug options more understandable.

[David Blaikie via cfe-commits]

dblaikie added a subscriber: dblaikie.
dblaikie accepted this revision.
dblaikie added a reviewer: dblaikie.
This revision is now accepted and ready to land.


Comment at: test/Driver/clang-g-opts.c:33
@@ -22,5 +32,3 @@
 
-// CHECK-WITHOUT-G-NOT: "-g"
-// CHECK-WITH-G: "-g"
-// CHECK-WITH-G-DWARF2: "-gdwarf-2"
-
+// CHECK-WITHOUT-G-NOT: -di-kind
+// CHECK-WITH-G: "-debug-info-kind=limited"

Should this be -debug-info-kind instead?


http://reviews.llvm.org/D13221



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

Re: [PATCH] D13510: [PATCH] Support C++ Core Guidelines copy assignment restrictions

[Eugene Zelenko via cfe-commits]

Eugene.Zelenko added a comment.

I think it'll be fine to rename check without leaving traces of misc. Same 
thing happened with modernize-shrink-to-fit.


http://reviews.llvm.org/D13510



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

Re: [PATCH] D13368: [clang-tidy] add check cppcoreguidelines-pro-type-static-cast-downcast

[Samuel Benzaquen via cfe-commits]

sbenza added inline comments.


Comment at: 
test/clang-tidy/cppcoreguidelines-pro-type-static-cast-downcast.cpp:28
@@ +27,3 @@
+
+  auto P0 = static_cast(new Base());
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: do not use static_cast to 
downcast from a base to a derived class 
[cppcoreguidelines-pro-type-static-cast-downcast]

I just noticed that we don't have any test with
  static_cast(...)  or static_cast
We should make sure that works.


http://reviews.llvm.org/D13368



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

r249567 - clang-format: Add include sorting capabilities to sublime, emacs and

[Daniel Jasper via cfe-commits]

Author: djasper
Date: Wed Oct  7 12:00:20 2015
New Revision: 249567

URL: http://llvm.org/viewvc/llvm-project?rev=249567&view=rev
Log:
clang-format: Add include sorting capabilities to sublime, emacs and
clang-format-diff.py.

Modified:
cfe/trunk/tools/clang-format/clang-format-diff.py
cfe/trunk/tools/clang-format/clang-format-sublime.py
cfe/trunk/tools/clang-format/clang-format.el

Modified: cfe/trunk/tools/clang-format/clang-format-diff.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/clang-format/clang-format-diff.py?rev=249567&r1=249566&r2=249567&view=diff
==
--- cfe/trunk/tools/clang-format/clang-format-diff.py (original)
+++ cfe/trunk/tools/clang-format/clang-format-diff.py Wed Oct  7 12:00:20 2015
@@ -52,6 +52,8 @@ def main():
   r'|protodevel|java)',
   help='custom pattern selecting file paths to reformat '
   '(case insensitive, overridden by -regex)')
+  parser.add_argument('-sort-includes', action='store_true', default=False,
+  help='let clang-format sort include blocks')
   parser.add_argument('-v', '--verbose', action='store_true',
   help='be more verbose, ineffective without -i')
   parser.add_argument(
@@ -96,6 +98,8 @@ def main():
 command = [binary, filename]
 if args.i:
   command.append('-i')
+if args.sort_includes:
+  command.append('-sort-includes')
 command.extend(lines)
 if args.style:
   command.extend(['-style', args.style])

Modified: cfe/trunk/tools/clang-format/clang-format-sublime.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/clang-format/clang-format-sublime.py?rev=249567&r1=249566&r2=249567&view=diff
==
--- cfe/trunk/tools/clang-format/clang-format-sublime.py (original)
+++ cfe/trunk/tools/clang-format/clang-format-sublime.py Wed Oct  7 12:00:20 
2015
@@ -32,7 +32,7 @@ class ClangFormatCommand(sublime_plugin.
 if encoding == 'Undefined':
   encoding = 'utf-8'
 regions = []
-command = [binary, '-style', style]
+command = [binary, '-sort-includes', '-style', style]
 for region in self.view.sel():
   regions.append(region)
   region_offset = min(region.a, region.b)

Modified: cfe/trunk/tools/clang-format/clang-format.el
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/clang-format/clang-format.el?rev=249567&r1=249566&r2=249567&view=diff
==
--- cfe/trunk/tools/clang-format/clang-format.el (original)
+++ cfe/trunk/tools/clang-format/clang-format.el Wed Oct  7 12:00:20 2015
@@ -126,6 +126,7 @@ is no active region.  If no style is giv
  nil `(,temp-buffer ,temp-file) nil
 
  "-output-replacements-xml"
+ "-sort-includes"
  "-assume-filename" (or (buffer-file-name) "")
  "-style" style
  "-offset" (number-to-string start)


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

[PATCH] D13516: Fix overlapping replacements in clang-tidy.

[Angel Garcia via cfe-commits]

angelgarcia created this revision.
angelgarcia added reviewers: klimek, bkramer.
angelgarcia added subscribers: alexfh, cfe-commits.

Prevent clang-tidy from applying fixes to errors that overlap with other 
errors' fixes, with one exception: if one fix is completely contained inside 
another one, then we can apply the big one.

http://reviews.llvm.org/D13516

Files:
  clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tidy/ClangTidyDiagnosticConsumer.h
  unittests/clang-tidy/OverlappingReplacementsTest.cpp

Index: unittests/clang-tidy/OverlappingReplacementsTest.cpp
===
--- unittests/clang-tidy/OverlappingReplacementsTest.cpp
+++ unittests/clang-tidy/OverlappingReplacementsTest.cpp
@@ -77,11 +77,12 @@
 auto *VD = Result.Nodes.getNodeAs(BoundDecl);
 std::string NewName = newName(VD->getName());
 
-auto Diag = diag(VD->getLocation(), "refactor")
+auto Diag = diag(VD->getLocation(), "refactor %0 into %1")
+<< VD->getName() << NewName
 << FixItHint::CreateReplacement(
-CharSourceRange::getTokenRange(VD->getLocation(),
-   VD->getLocation()),
-NewName);
+   CharSourceRange::getTokenRange(VD->getLocation(),
+  VD->getLocation()),
+   NewName);
 
 class UsageVisitor : public RecursiveASTVisitor {
 public:
@@ -281,7 +282,7 @@
 
   // Apply the UseCharCheck together with the IfFalseCheck.
   //
-  // The 'If' fix is bigger, so that is the one that has to be applied.
+  // The 'If' fix contains the other, so that is the one that has to be applied.
   // } else if (int a = 0) {
   //^^^ -> char
   //~ -> false
@@ -294,16 +295,16 @@
   }
 })";
   Res = runCheckOnCode(Code);
-  // FIXME: EXPECT_EQ(CharIfFix, Res);
+  EXPECT_EQ(CharIfFix, Res);
 
   // Apply the IfFalseCheck with the StartsWithPotaCheck.
   //
   // The 'If' replacement is bigger here.
   // if (char potato = 0) {
   //  ^^ -> tomato
   // ~~~ -> false
   //
-  // But the refactoring is bigger here:
+  // But the refactoring is the one that contains the other here:
   // char potato = 0;
   //  ^^ -> tomato
   // if (potato) potato;
@@ -318,60 +319,73 @@
   }
 })";
   Res = runCheckOnCode(Code);
-  // FIXME: EXPECT_EQ(IfStartsFix, Res);
-
-  // Silence warnings.
-  (void)CharIfFix;
-  (void)IfStartsFix;
+  EXPECT_EQ(IfStartsFix, Res);
 }
 
-TEST(OverlappingReplacementsTest, ApplyFullErrorOrNothingWhenOverlapping) {
-  std::string Res;
+TEST(OverlappingReplacements, TwoReplacementsInsideOne) {
   const char Code[] =
   R"(void f() {
-  int potato = 0;
-  potato += potato * potato;
-  if (char this_name_make_this_if_really_long = potato) potato;
+  if (int potato = 0) {
+int a = 0;
+  }
 })";
 
-  // StartsWithPotaCheck will try to refactor 'potato' into 'tomato',
-  // and EndsWithTatoCheck will try to use 'pomelo'. We have to apply
-  // either all conversions from one check, or all from the other.
-  const char StartsFix[] =
+  // The two smallest replacements should not be applied.
+  // if (int potato = 0) {
+  // ^^ -> tomato
+  // *** -> char
+  // ~~ -> false
+  // But other errors from the same checks should not be affected.
+  //   int a = 0;
+  //   *** -> char
+  const char Fix[] =
   R"(void f() {
-  int tomato = 0;
-  tomato += tomato * tomato;
-  if (char this_name_make_this_if_really_long = tomato) tomato;
+  if (false) {
+char a = 0;
+  }
 })";
-  const char EndsFix[] =
+  std::string Res =
+  runCheckOnCode(Code);
+  EXPECT_EQ(Fix, Res);
+}
+
+TEST(OverlappingReplacementsTest, DiscardBothChangesWhenPartialOverlapping) {
+  const char Code[] =
   R"(void f() {
-  int pomelo = 0;
-  pomelo += pomelo * pomelo;
-  if (char this_name_make_this_if_really_long = pomelo) pomelo;
+  if (int potato = 0) {
+int a = potato;
+  }
 })";
-  // In case of overlapping, we will prioritize the biggest fix. However, these
-  // two fixes have the same size and position, so we don't know yet which one
-  // will have preference.
-  Res = runCheckOnCode(Code);
-  // FIXME: EXPECT_TRUE(Res == StartsFix || Res == EndsFix);
 
-  // StartsWithPotaCheck will try to refactor 'potato' into 'tomato', but
-  // replacing the 'if' condition is a bigger change than all the refactoring
-  // changes together (48 vs 36), so this is the one that is going to be
-  // applied.
-  const char IfFix[] =
+  // These two replacements overlap, but none of them is completely contained
+  // inside the other. Both of them are discarded.
+  // if (int potato = 0) {
+  // ^^ -> tomato
+  // ~~ -> false
+  //   int a = potato;
+  //   ^^ -> tomato
+  std::string Res = runCheckOnCode(Code);
+  EXPECT_EQ(Code, Res);
+}
+
+TEST(

Re: [PATCH] D13351: [Power PC] add soft float support for ppc32

[Strahinja Petrovic via cfe-commits]

spetrovic updated this revision to Diff 36763.

http://reviews.llvm.org/D13351

Files:
  include/clang/Basic/TargetInfo.h
  lib/Basic/Targets.cpp
  lib/CodeGen/TargetInfo.cpp
  lib/Driver/Tools.cpp
  lib/Driver/Tools.h
  test/CodeGen/ppc-sfvarargs.c
  test/Driver/ppc-features.cpp

Index: test/Driver/ppc-features.cpp
===
--- test/Driver/ppc-features.cpp
+++ test/Driver/ppc-features.cpp
@@ -12,6 +12,10 @@
 // RUN: not %clang -target mips64-linux-gnu -faltivec -fsyntax-only %s 2>&1 | FileCheck %s
 // RUN: not %clang -target sparc-unknown-solaris -faltivec -fsyntax-only %s 2>&1 | FileCheck %s
 
+// check soft float option only for ppc32
+// RUN: %clang -target powerpc-unknown-linux-gnu %s -msoft-float -### -o %t.o 2>&1 | FileCheck --check-prefix=CHECK-SOFTFLOAT %s
+// CHECK-SOFTFLOAT: "-target-feature" "+soft-float"
+
 // CHECK: invalid argument '-faltivec' only allowed with 'ppc/ppc64/ppc64le'
 
 // Check that -fno-altivec and -mno-altivec correctly disable the altivec
Index: test/CodeGen/ppc-sfvarargs.c
===
--- test/CodeGen/ppc-sfvarargs.c
+++ test/CodeGen/ppc-sfvarargs.c
@@ -0,0 +1,17 @@
+// RUN: %clang -O0 --target=powerpc-unknown-linux-gnu -EB -msoft-float -S -emit-llvm %s -o - | FileCheck %s
+
+#include 
+void test(char *fmt, ...) {
+  va_list ap;
+  va_start(ap, fmt);
+  va_arg(ap, double);
+  va_end(ap);
+}
+
+void foo() {
+  double a;
+  test("test",a);
+}
+// CHECK: %{{[0-9]+}} = add i8 %numUsedRegs, 1
+// CHECK: %{{[0-9]+}} = and i8 %{{[0-9]+}}, -2
+// CHECK: %{{[0-9]+}} = mul i8 %{{[0-9]+}}, 4
\ No newline at end of file
Index: lib/Driver/Tools.h
===
--- lib/Driver/Tools.h
+++ lib/Driver/Tools.h
@@ -728,6 +728,10 @@
 
 FloatABI getARMFloatABI(const ToolChain &TC, const llvm::opt::ArgList &Args);
 }
+
+namespace ppc {
+   StringRef getPPCFloatABI(const llvm::opt::ArgList &Args);
+}
 namespace XCore {
 // For XCore, we do not need to instantiate tools for PreProcess, PreCompile and
 // Compile.
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -1344,15 +1344,44 @@
 // TODO: Change the LLVM backend option maybe?
 if (Name == "mfcrf")
   Name = "mfocrf";
-
+
 Features.push_back(Args.MakeArgString((IsNegative ? "-" : "+") + Name));
   }
 
+  StringRef FloatABI = ppc::getPPCFloatABI(Args);
+  if (FloatABI == "soft")
+Features.push_back("+soft-float");
+  
   // Altivec is a bit weird, allow overriding of the Altivec feature here.
   AddTargetFeature(Args, Features, options::OPT_faltivec,
options::OPT_fno_altivec, "altivec");
 }
 
+StringRef ppc::getPPCFloatABI(const ArgList &Args) {
+  StringRef FloatABI;
+  if (Arg *A = Args.getLastArg(options::OPT_msoft_float,
+  options::OPT_mhard_float,
+  options::OPT_mfloat_abi_EQ)) {
+if (A->getOption().matches(options::OPT_msoft_float))
+  FloatABI = "soft";
+else if (A->getOption().matches(options::OPT_mhard_float))
+  FloatABI = "hard";
+else {
+  FloatABI = A->getValue();
+  if (FloatABI != "soft" && FloatABI != "hard") {
+FloatABI = "hard";
+  }
+}
+  }
+
+  // If unspecified, choose the default based on the platform.
+  if (FloatABI.empty()) {
+FloatABI = "hard";
+  }
+
+  return FloatABI;
+}
+
 void Clang::AddPPCTargetArgs(const ArgList &Args,
  ArgStringList &CmdArgs) const {
   // Select the ABI to use.
@@ -1389,6 +1418,21 @@
 if (StringRef(A->getValue()) != "altivec")
   ABIName = A->getValue();
 
+  StringRef FloatABI = ppc::getPPCFloatABI(Args);
+
+  if (FloatABI == "soft") {
+// Floating point operations and argument passing are soft.
+CmdArgs.push_back("-msoft-float");
+CmdArgs.push_back("-mfloat-abi");
+CmdArgs.push_back("soft");
+  }
+  else {
+// Floating point operations and argument passing are hard.
+assert(FloatABI == "hard" && "Invalid float abi!");
+CmdArgs.push_back("-mfloat-abi");
+CmdArgs.push_back("hard");
+  }
+
   if (ABIName) {
 CmdArgs.push_back("-target-abi");
 CmdArgs.push_back(ABIName);
Index: lib/CodeGen/TargetInfo.cpp
===
--- lib/CodeGen/TargetInfo.cpp
+++ lib/CodeGen/TargetInfo.cpp
@@ -3422,6 +3422,8 @@
   bool isI64 = Ty->isIntegerType() && getContext().getTypeSize(Ty) == 64;
   bool isInt =
   Ty->isIntegerType() || Ty->isPointerType() || Ty->isAggregateType();
+  bool isF64 = Ty->isFloatingType() && getContext().getTypeSize(Ty) == 64;
+  bool isSF = getTarget().isSoftFloatABI();
 
   // All aggregates are passed indirectly?  That doesn't seem consistent
   // with the argument-lowering code.
@@ -3431,16 +3433,16 @@
 
   // The calling convention

Re: [clang-tools-extra] r249399 - Add a new module for the C++ Core Guidelines, and the first checker for those guidelines: cppcoreguidelines-pro-type-reinterpret-cast.

[Aaron Ballman via cfe-commits]

On Wed, Oct 7, 2015 at 12:05 PM, Joerg Sonnenberger via cfe-commits
 wrote:
> On Tue, Oct 06, 2015 at 01:31:01PM -, Aaron Ballman via cfe-commits wrote:
>> Log:
>> Add a new module for the C++ Core Guidelines, and the first checker
>> for those guidelines: cppcoreguidelines-pro-type-reinterpret-cast.
>
> I wonder about the generality of this. Does it really make sense to flag
> every reinterpret_cast of a void pointer? Such interface contracts
> normally don't give you an alternative, so it seems a bit silly. This is
> made worse by the concrete message used...
>
> More importantly, can the checker detect all cases where
> reinterpret_cast is used where a less generic cast applies?

This is a checker specific to the behaviors of the C++ Core
Guidelines. Personally, I share the same concerns about the viability
of these kind of checks (specifically, on existing code bases instead
of on newly-developed code). Especially when there's no guidance as to
how to correct the code, short of "just don't do that thing despite it
being well-formed code." However, the concept here is that we want
checkers to diagnose coding guideline violations as-written, and that
is why these checkers are being implemented under cppcoreguidelines-*.

~Aaron

>
> 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] D12793: Three new overflow builtins with generic argument types

[David Grayson via cfe-commits]

DavidEGrayson marked an inline comment as done.
DavidEGrayson added a comment.

Well, it has been another week.  John, I don't have commit access, so can you 
commit this?  Thanks!  Let me know if the patch doesn't apply cleanly or any 
tests fail.


http://reviews.llvm.org/D12793



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

Re: [PATCH] D13510: [PATCH] Support C++ Core Guidelines copy assignment restrictions

[Aaron Ballman via cfe-commits]

aaron.ballman added a comment.

In http://reviews.llvm.org/D13510#261825, @Eugene.Zelenko wrote:

> I think it'll be fine to rename check without leaving traces of misc. Same 
> thing happened with modernize-shrink-to-fit.


I think the difference here is that many C++ Core Guideline checks are... 
chatty, and so these checks are likely to not be enabled (especially on 
existing code bases). By leaving the check in misc-*, it is more likely to 
provide value to users that aren't able to use the cppcoreguidelines-* checks 
yet.


http://reviews.llvm.org/D13510



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

Re: [PATCH] D13221: Make CompilerInvocation's use of the debug options more understandable.

[Douglas Katzman via cfe-commits]

dougk marked an inline comment as done.
dougk added a comment.

http://reviews.llvm.org/D13221



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

[PATCH] D13523: ASTMatchers: Keep AllCallbacks in a set instead of a vector

[Daniel Jasper via cfe-commits]

djasper created this revision.
djasper added a reviewer: klimek.
djasper added a subscriber: cfe-commits.
Herald added a subscriber: klimek.

AllCallbacks is currently only used to call onStartOfTranslationUnit and 
onEndOfTranslationUnit on them. In this (and any other scenario I can come up 
with), it is important (or at least better) not to have duplicates in this 
container. E.g. currently onEndOfTranslationUnit is called repeatedly on the 
same callback for every matcher that is registered with it.

http://reviews.llvm.org/D13523

Files:
  include/clang/ASTMatchers/ASTMatchFinder.h
  lib/ASTMatchers/ASTMatchFinder.cpp

Index: lib/ASTMatchers/ASTMatchFinder.cpp
===
--- lib/ASTMatchers/ASTMatchFinder.cpp
+++ lib/ASTMatchers/ASTMatchFinder.cpp
@@ -913,37 +913,37 @@
 void MatchFinder::addMatcher(const DeclarationMatcher &NodeMatch,
  MatchCallback *Action) {
   Matchers.DeclOrStmt.emplace_back(NodeMatch, Action);
-  Matchers.AllCallbacks.push_back(Action);
+  Matchers.AllCallbacks.insert(Action);
 }
 
 void MatchFinder::addMatcher(const TypeMatcher &NodeMatch,
  MatchCallback *Action) {
   Matchers.Type.emplace_back(NodeMatch, Action);
-  Matchers.AllCallbacks.push_back(Action);
+  Matchers.AllCallbacks.insert(Action);
 }
 
 void MatchFinder::addMatcher(const StatementMatcher &NodeMatch,
  MatchCallback *Action) {
   Matchers.DeclOrStmt.emplace_back(NodeMatch, Action);
-  Matchers.AllCallbacks.push_back(Action);
+  Matchers.AllCallbacks.insert(Action);
 }
 
 void MatchFinder::addMatcher(const NestedNameSpecifierMatcher &NodeMatch,
  MatchCallback *Action) {
   Matchers.NestedNameSpecifier.emplace_back(NodeMatch, Action);
-  Matchers.AllCallbacks.push_back(Action);
+  Matchers.AllCallbacks.insert(Action);
 }
 
 void MatchFinder::addMatcher(const NestedNameSpecifierLocMatcher &NodeMatch,
  MatchCallback *Action) {
   Matchers.NestedNameSpecifierLoc.emplace_back(NodeMatch, Action);
-  Matchers.AllCallbacks.push_back(Action);
+  Matchers.AllCallbacks.insert(Action);
 }
 
 void MatchFinder::addMatcher(const TypeLocMatcher &NodeMatch,
  MatchCallback *Action) {
   Matchers.TypeLoc.emplace_back(NodeMatch, Action);
-  Matchers.AllCallbacks.push_back(Action);
+  Matchers.AllCallbacks.insert(Action);
 }
 
 bool MatchFinder::addDynamicMatcher(const internal::DynTypedMatcher &NodeMatch,
Index: include/clang/ASTMatchers/ASTMatchFinder.h
===
--- include/clang/ASTMatchers/ASTMatchFinder.h
+++ include/clang/ASTMatchers/ASTMatchFinder.h
@@ -42,6 +42,7 @@
 #define LLVM_CLANG_ASTMATCHERS_ASTMATCHFINDER_H
 
 #include "clang/ASTMatchers/ASTMatchers.h"
+#include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/Support/Timer.h"
 
@@ -208,7 +209,7 @@
 NestedNameSpecifierLoc;
 std::vector> TypeLoc;
 /// \brief All the callbacks in one container to simplify iteration.
-std::vector AllCallbacks;
+llvm::SmallPtrSet AllCallbacks;
   };
 
 private:


Index: lib/ASTMatchers/ASTMatchFinder.cpp
===
--- lib/ASTMatchers/ASTMatchFinder.cpp
+++ lib/ASTMatchers/ASTMatchFinder.cpp
@@ -913,37 +913,37 @@
 void MatchFinder::addMatcher(const DeclarationMatcher &NodeMatch,
  MatchCallback *Action) {
   Matchers.DeclOrStmt.emplace_back(NodeMatch, Action);
-  Matchers.AllCallbacks.push_back(Action);
+  Matchers.AllCallbacks.insert(Action);
 }
 
 void MatchFinder::addMatcher(const TypeMatcher &NodeMatch,
  MatchCallback *Action) {
   Matchers.Type.emplace_back(NodeMatch, Action);
-  Matchers.AllCallbacks.push_back(Action);
+  Matchers.AllCallbacks.insert(Action);
 }
 
 void MatchFinder::addMatcher(const StatementMatcher &NodeMatch,
  MatchCallback *Action) {
   Matchers.DeclOrStmt.emplace_back(NodeMatch, Action);
-  Matchers.AllCallbacks.push_back(Action);
+  Matchers.AllCallbacks.insert(Action);
 }
 
 void MatchFinder::addMatcher(const NestedNameSpecifierMatcher &NodeMatch,
  MatchCallback *Action) {
   Matchers.NestedNameSpecifier.emplace_back(NodeMatch, Action);
-  Matchers.AllCallbacks.push_back(Action);
+  Matchers.AllCallbacks.insert(Action);
 }
 
 void MatchFinder::addMatcher(const NestedNameSpecifierLocMatcher &NodeMatch,
  MatchCallback *Action) {
   Matchers.NestedNameSpecifierLoc.emplace_back(NodeMatch, Action);
-  Matchers.AllCallbacks.push_back(Action);
+  Matchers.AllCallbacks.insert(Action);
 }
 
 void MatchFinder::addMatcher(const TypeLocMatcher &NodeMatch,
  MatchCallback *Action) {
   Matchers.TypeLoc.emplace_back(NodeMatch, Action);
-  Matchers.AllCallbacks.push_back

[PATCH] D13525: [CodeGen] Attach function attributes to functions created in CGBlocks.cpp.

[Akira Hatanaka via cfe-commits]

ahatanak created this revision.
ahatanak added reviewers: dexonsmith, echristo.
ahatanak added a subscriber: cfe-commits.

This patch makes changes to attach function attributes to the following 
functions created in CGBlocks.cpp:

__copy_helper_block_
__destroy_helper_block_
__Block_byref_object_copy_
__Block_byref_object_dispose_

There are other places in clang where function attributes are not attached to 
functions. I plan to follow up with patches that fix those places too.

http://reviews.llvm.org/D13525

Files:
  lib/CodeGen/CGBlocks.cpp
  lib/CodeGen/CodeGenModule.cpp
  lib/CodeGen/TargetInfo.cpp
  test/CodeGenObjC/arc-blocks.m

Index: test/CodeGenObjC/arc-blocks.m
===
--- test/CodeGenObjC/arc-blocks.m
+++ test/CodeGenObjC/arc-blocks.m
@@ -43,7 +43,7 @@
   extern void test2_helper(id (^)(void));
   test2_helper(^{ return x; });
 
-// CHECK-LABEL:define internal void @__copy_helper_block_
+// CHECK-LABEL:define internal void @__copy_helper_block_(i8*, i8*) #{{[0-9]+}} {
 // CHECK:  [[T0:%.*]] = load i8*, i8**
 // CHECK-NEXT: [[SRC:%.*]] = bitcast i8* [[T0]] to [[BLOCK_T]]*
 // CHECK-NEXT: [[T0:%.*]] = load i8*, i8**
@@ -53,7 +53,7 @@
 // CHECK-NEXT: [[T2:%.*]] = call i8* @objc_retain(i8* [[T1]]) [[NUW]]
 // CHECK-NEXT: ret void
 
-// CHECK-LABEL:define internal void @__destroy_helper_block_
+// CHECK-LABEL:define internal void @__destroy_helper_block_(i8*) #{{[0-9]+}} {
 // CHECK:  [[T0:%.*]] = load i8*, i8**
 // CHECK-NEXT: [[T1:%.*]] = bitcast i8* [[T0]] to [[BLOCK_T]]*
 // CHECK-NEXT: [[T2:%.*]] = getelementptr inbounds [[BLOCK_T]], [[BLOCK_T]]* [[T1]], i32 0, i32 5
@@ -134,16 +134,16 @@
   // CHECK-NEXT: call void @objc_release(i8* [[T0]])
   // CHECK: ret void
 
-  // CHECK-LABEL:define internal void @__Block_byref_object_copy_
+  // CHECK-LABEL:define internal void @__Block_byref_object_copy_(i8*, i8*) #{{[0-9]+}} {
   // CHECK:  [[T0:%.*]] = getelementptr inbounds [[BYREF_T]], [[BYREF_T]]* {{%.*}}, i32 0, i32 6
   // CHECK-NEXT: load i8*, i8**
   // CHECK-NEXT: bitcast i8* {{%.*}} to [[BYREF_T]]*
   // CHECK-NEXT: [[T1:%.*]] = getelementptr inbounds [[BYREF_T]], [[BYREF_T]]* {{%.*}}, i32 0, i32 6
   // CHECK-NEXT: [[T2:%.*]] = load i8*, i8** [[T1]]
   // CHECK-NEXT: store i8* [[T2]], i8** [[T0]]
   // CHECK-NEXT: store i8* null, i8** [[T1]]
 
-  // CHECK-LABEL:define internal void @__Block_byref_object_dispose_
+  // CHECK-LABEL:define internal void @__Block_byref_object_dispose_(i8*) #{{[0-9]+}} {
   // CHECK:  [[T0:%.*]] = getelementptr inbounds [[BYREF_T]], [[BYREF_T]]* {{%.*}}, i32 0, i32 6
   // CHECK-NEXT: [[T1:%.*]] = load i8*, i8** [[T0]]
   // CHECK-NEXT: call void @objc_release(i8* [[T1]])
@@ -155,10 +155,10 @@
   // CHECK-NEXT: call void @objc_release(i8* [[T0]])
   // CHECK-NEXT: ret void
 
-  // CHECK-LABEL:define internal void @__copy_helper_block_
+  // CHECK-LABEL:define internal void @__copy_helper_block_.{{[0-9]+}}(i8*, i8*) #{{[0-9]+}} {
   // CHECK:  call void @_Block_object_assign(i8* {{%.*}}, i8* {{%.*}}, i32 8)
 
-  // CHECK-LABEL:define internal void @__destroy_helper_block_
+  // CHECK-LABEL:define internal void @__destroy_helper_block_.{{[0-9]+}}(i8*) #{{[0-9]+}} {
   // CHECK:  call void @_Block_object_dispose(i8* {{%.*}}, i32 8)
 }
 
@@ -221,27 +221,27 @@
   // CHECK-NEXT: call void @llvm.lifetime.end(i64 48, i8* [[VARPTR2]])
   // CHECK-NEXT: ret void
 
-  // CHECK-LABEL:define internal void @__Block_byref_object_copy_
+  // CHECK-LABEL:define internal void @__Block_byref_object_copy_.{{[0-9]+}}(i8*, i8*) #{{[0-9]+}} {
   // CHECK:  [[T0:%.*]] = getelementptr inbounds [[BYREF_T]], [[BYREF_T]]* {{%.*}}, i32 0, i32 6
   // CHECK-NEXT: load i8*, i8**
   // CHECK-NEXT: bitcast i8* {{%.*}} to [[BYREF_T]]*
   // CHECK-NEXT: [[T1:%.*]] = getelementptr inbounds [[BYREF_T]], [[BYREF_T]]* {{%.*}}, i32 0, i32 6
   // CHECK-NEXT: call void @objc_moveWeak(i8** [[T0]], i8** [[T1]])
 
-  // CHECK-LABEL:define internal void @__Block_byref_object_dispose_
+  // CHECK-LABEL:define internal void @__Block_byref_object_dispose_.{{[0-9]+}}(i8*) #{{[0-9]+}} {
   // CHECK:  [[T0:%.*]] = getelementptr inbounds [[BYREF_T]], [[BYREF_T]]* {{%.*}}, i32 0, i32 6
   // CHECK-NEXT: call void @objc_destroyWeak(i8** [[T0]])
 
   // CHECK-LABEL:define internal void @__test6_block_invoke
   // CHECK:  [[SLOT:%.*]] = getelementptr inbounds {{.*}}, i32 0, i32 6
   // CHECK-NEXT: call i8* @objc_storeWeak(i8** [[SLOT]], i8* null)
   // CHECK-NEXT: ret void
 
-  // CHECK-LABEL:define internal void @__copy_helper_block_
+  // CHECK-LABEL:define internal void @__copy_helper_block_.{{[0-9]+}}(i8*, i8*) #{{[0-9]+}} {
   // 0x8 - FIELD_IS_BYREF (no FIELD_IS_WEAK because clang in control)
   // CHECK:  call void @_Block_object_assign(i8* {{%.*}}, i8* {{%.*}}, i32 8)
 
-  // CHECK-LABEL:define internal void @__destroy_helper_bloc

Re: r248949 - Don't inherit availability information when implementing a protocol requirement.

[Hans Wennborg via cfe-commits]

Hi Doug,

On Wed, Sep 30, 2015 at 2:27 PM, Douglas Gregor via cfe-commits
 wrote:
> Author: dgregor
> Date: Wed Sep 30 16:27:42 2015
> New Revision: 248949
>
> URL: http://llvm.org/viewvc/llvm-project?rev=248949&view=rev
> Log:
> Don't inherit availability information when implementing a protocol 
> requirement.
>
> When an Objective-C method implements a protocol requirement, do not
> inherit any availability information from the protocol
> requirement. Rather, check that the implementation is not less
> available than the protocol requirement, as we do when overriding a
> method that has availability. Fixes rdar://problem/22734745.

This is causing new warnings to fire in Chromium, and I'm not sure
they're correct.

For example:

  $ cat | build/bin/clang -c -x objective-c++ -
  #import 
  @protocol Foo
  @end
  @interface Bar : NSTextView {
  }
  @end
  @implementation Bar
  - (void)setMarkedText:(id)aString selectedRange:(NSRange)selRange {
[super setMarkedText:aString selectedRange:selRange];
  }
  @end
  :9:10: warning: 'setMarkedText:selectedRange:' is deprecated:
first deprecated in OS X 10.6 [-Wdeprecated-declarations]
[super setMarkedText:aString selectedRange:selRange];
   ^
  /System/Library/Frameworks/AppKit.framework/Headers/NSInputManager.h:21:1:
note: 'setMarkedText:selectedRange:' has been explicitly marked
deprecated here
  - (void) setMarkedText:(id)aString selectedRange:(NSRange)selRange
NS_DEPRECATED_MAC(10_0, 10_6);
  ^

I don't really know Objective-C, but from what I understand,
NSTextView implements both NSTextInput and NSTextInputClient.
setMarkedText is deprecated in the former, but not the latter. So
warning here is probably wrong?

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

Re: [PATCH] D11328: [clang-tidy] new "throw-by-value-catch-by-reference" check for clang-tidy

[Aaron Ballman via cfe-commits]

aaron.ballman added a comment.

This patch is looking much closer! Thank you for continuing to work on it.

I found several mechanical changes that need to be made, like style, comments, 
formatting, isa<> followed by cast<>, etc. There is one logic issue regarding 
materialized temporaries that I think still needs to be addressed, but 
otherwise, this patch is getting close!



Comment at: clang-tidy/misc/ThrowByValueCatchByReferenceCheck.cpp:46
@@ +45,3 @@
+
+bool ThrowByValueCatchByReferenceCheck::isFunctionParameter(
+const DeclRefExpr *declRefExpr) {

Judging by the function name, I think the code should actually be:
```
return isa(declRefExpr->getDecl());
```


Comment at: clang-tidy/misc/ThrowByValueCatchByReferenceCheck.cpp:60
@@ +59,3 @@
+return false;
+  auto *varDecl = cast(valueDecl);
+  return varDecl->isExceptionVariable();

Instead of isa<> followed by cast<>, the more idiomatic approach is:
```
if (const auto *VD = dyn_cast(declRefExpr->getDecl()))
  return VD->isExceptionVariable();
```



Comment at: clang-tidy/misc/ThrowByValueCatchByReferenceCheck.cpp:66
@@ +65,3 @@
+const DeclRefExpr *declRefExpr)
+{
+ return isFunctionParameter(declRefExpr) || isCatchVariable(declRefExpr); 

Should run clang-format over this; the brace should go with the closing paren.


Comment at: clang-tidy/misc/ThrowByValueCatchByReferenceCheck.cpp:78
@@ +77,3 @@
+  auto qualType = subExpr->getType();
+  auto *type = qualType.getTypePtr();
+  if (type->isPointerType()) {

No need to call getTypePtr(); QualType objects are thin wrappers around a Type 
*. So you can do qualType->isPointerType(), for instance.


Comment at: clang-tidy/misc/ThrowByValueCatchByReferenceCheck.cpp:86
@@ +85,3 @@
+  return;
+//do not diagnose on catch variables
+if (isa(inner)) {

Space between the comment and the start of the sentence; also, comments should 
be full sentences (with capitalization and punctuation). Applies here and 
elsewhere in the patch.


Comment at: clang-tidy/misc/ThrowByValueCatchByReferenceCheck.cpp:88
@@ +87,3 @@
+if (isa(inner)) {
+  auto *declRef = cast(inner);
+  if (isCatchVariable(declRef))

Please use dyn_cast<> instead of isa<> followed by cast<>. Applies here and 
elsewhere in the patch.


Comment at: clang-tidy/misc/ThrowByValueCatchByReferenceCheck.cpp:93
@@ +92,3 @@
+diag(subExpr->getLocStart(), "throw expression throws a pointer; it should 
"
+ "throw a non pointer value instead");
+  }

non-pointer, please


Comment at: clang-tidy/misc/ThrowByValueCatchByReferenceCheck.cpp:111
@@ +110,3 @@
+auto *currentSubExpr = subExpr->IgnoreImpCasts();
+const DeclRefExpr *variableReference;
+if (isa(currentSubExpr))

This should be:
```
if (const auto *variableReference = dyn_cast(currentSubExpr)) {
} else if (const auto *constructorCall = 
dyn_cast(currentSubExpr)) {
}
```


Comment at: clang-tidy/misc/ThrowByValueCatchByReferenceCheck.cpp:129
@@ +128,3 @@
+// if it's a copy constructor it could copy from a lvalue
+else if (constructorCall &&
+ constructorCall->getConstructor()->isCopyOrMoveConstructor()) {

I think that what we need to model here is: 
constructExpr(hasDescendant(materializeTemporaryExpr())) and 
expr(hasType(hasCanonicalType(builtinType(

If the throw expression materializes a temporary, or it throws a builtin type, 
we are good. (Note, the checking for function parameter or catch variable is 
already done properly.)


Comment at: clang-tidy/misc/ThrowByValueCatchByReferenceCheck.cpp:156
@@ +155,3 @@
+return;
+  auto *type = caughtType.getTypePtr();
+  auto *varDecl = catchStmt->getExceptionDecl();

No need to call getTypePtr().


Comment at: clang-tidy/misc/ThrowByValueCatchByReferenceCheck.cpp:159
@@ +158,3 @@
+  if (type->isPointerType()) {
+auto canonical = type->getCanonicalTypeInternal().getTypePtr();
+// check if pointed-to-type is char, wchar_t, char16_t, char32_t

Same here.

Also, please do not use getCanonicalTypeInternal(), you should use 
getCanonicalType() instead.


Comment at: clang-tidy/misc/ThrowByValueCatchByReferenceCheck.cpp:162
@@ +161,3 @@
+auto *pointeeType =
+cast(canonical)->getPointeeType().getTypePtr();
+if (pointeeType->isAnyCharacterType() == false) {

A cleaner way to do this would be:
```
if (const auto *PT = getCanonicalType(caughtType)->getAs()) {
  if (!PT->getPointeeType()->isAnyCharacterType())
diag(...);
}

This also corrects direct comparison against a Boolean on the next line.


Comment at: clang-tidy/misc/ThrowByValueCatchByR

Re: [PATCH] D13440: [clang-tidy] Python script for easy check rename

[Matthias Gehre via cfe-commits]

mgehre added a subscriber: mgehre.


Comment at: clang-tidy/rename_check.py:39
@@ +38,3 @@
+  if len(sys.argv) != 4:
+print('Usage: rename_check.py  , e.g.\n')
+print('rename_check.py misc awesome-functions new-awesome-function\n')

Should be
```
Usage: rename_check.py   
```
?


Repository:
  rL LLVM

http://reviews.llvm.org/D13440



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

Re: [PATCH] D13440: [clang-tidy] Python script for easy check rename

[Alexander Kornienko via cfe-commits]

alexfh added inline comments.


Comment at: clang-tidy/rename_check.py:3
@@ +2,3 @@
+#
+#===- add_new_check.py - clang-tidy check generator --*- python 
-*--===#
+#

Please update the script name and description in the comment.


Comment at: clang-tidy/rename_check.py:18
@@ +17,3 @@
+def replaceInFile(fileName, sFrom, sTo):
+txt = None
+with open(fileName, "r") as f:

Please use 2 spaces for indentation.


Comment at: clang-tidy/rename_check.py:39
@@ +38,3 @@
+  if len(sys.argv) != 4:
+print('Usage: rename_check.py  , e.g.\n')
+print('rename_check.py misc awesome-functions new-awesome-function\n')

New check name is missing.


Comment at: clang-tidy/rename_check.py:52
@@ +51,3 @@
+  clang_tidy_path = os.path.dirname(sys.argv[0])
+  module_path = os.path.join(clang_tidy_path, module)
+

This should be updated similarly to the add_new_check.py change in 
http://reviews.llvm.org/D13313.


Comment at: clang-tidy/rename_check.py:64
@@ +63,3 @@
+  replaceInFile(filename, header_guard_old, header_guard_new)
+  replaceInFile(filename, check_name, check_name_new)
+  replaceInFile(filename, check_name_camel, check_name_new_camel)

Would be nice, if this script took care of updating the file comments in .h and 
.cpp files including it's padding with dashes, e.g.:

  //===--- ProTypeReinterpretCastCheck.cpp - 
clang-tidy--===//



Repository:
  rL LLVM

http://reviews.llvm.org/D13440



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

Re: [PATCH] D13523: ASTMatchers: Keep AllCallbacks in a set instead of a vector

[Manuel Klimek via cfe-commits]

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

That makes a lot of sense. LG


http://reviews.llvm.org/D13523



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

[libcxx] r249593 - While researching LWG#2244, I noticed we weren't testing that eofbit was being cleared. Now we are

[Marshall Clow via cfe-commits]

Author: marshall
Date: Wed Oct  7 14:41:24 2015
New Revision: 249593

URL: http://llvm.org/viewvc/llvm-project?rev=249593&view=rev
Log:
While researching LWG#2244, I noticed we weren't testing that eofbit was being 
cleared. Now we are

Modified:

libcxx/trunk/test/std/input.output/iostream.format/input.streams/istream.unformatted/seekg.pass.cpp

Modified: 
libcxx/trunk/test/std/input.output/iostream.format/input.streams/istream.unformatted/seekg.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/input.output/iostream.format/input.streams/istream.unformatted/seekg.pass.cpp?rev=249593&r1=249592&r2=249593&view=diff
==
--- 
libcxx/trunk/test/std/input.output/iostream.format/input.streams/istream.unformatted/seekg.pass.cpp
 (original)
+++ 
libcxx/trunk/test/std/input.output/iostream.format/input.streams/istream.unformatted/seekg.pass.cpp
 Wed Oct  7 14:41:24 2015
@@ -63,4 +63,13 @@ int main()
 is.seekg(-1);
 assert(is.fail());
 }
+{
+testbuf sb(" 123456789");
+std::istream is(&sb);
+is.setstate(std::ios_base::eofbit);
+assert(is.eof());
+is.seekg(5);
+assert(is.good());
+assert(!is.eof());
+}
 }


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

[libcxx] r249595 - Mark 2244 as 'Patch Ready', 2477 and 2487 as 'Complete'

[Marshall Clow via cfe-commits]

Author: marshall
Date: Wed Oct  7 14:45:14 2015
New Revision: 249595

URL: http://llvm.org/viewvc/llvm-project?rev=249595&view=rev
Log:
Mark 2244 as 'Patch Ready', 2477 and 2487 as 'Complete'

Modified:
libcxx/trunk/www/kona.html

Modified: libcxx/trunk/www/kona.html
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/www/kona.html?rev=249595&r1=249594&r2=249595&view=diff
==
--- libcxx/trunk/www/kona.html (original)
+++ libcxx/trunk/www/kona.html Wed Oct  7 14:45:14 2015
@@ -69,7 +69,7 @@
http://cplusplus.github.io/LWG/lwg-defects.html#2181";>2181Exceptions
 from seed sequence operationsKona
http://cplusplus.github.io/LWG/lwg-defects.html#2218";>2218Unclear
 how containers use 
allocator_traits::construct()Kona
http://cplusplus.github.io/LWG/lwg-defects.html#2219";>2219INVOKE-ing
 a pointer to member with a reference_wrapper as the object 
expressionKona
-   http://cplusplus.github.io/LWG/lwg-defects.html#2244";>2244Issue
 on basic_istream::seekgKona
+   http://cplusplus.github.io/LWG/lwg-defects.html#2244";>2244Issue
 on basic_istream::seekgKonaPatch Ready
http://cplusplus.github.io/LWG/lwg-defects.html#2250";>2250Follow-up
 On Library Issue 2207Kona
http://cplusplus.github.io/LWG/lwg-defects.html#2259";>2259Issues
 in 17.6.5.5 rules for member functionsKonaComplete
http://cplusplus.github.io/LWG/lwg-defects.html#2336";>2336is_trivially_constructible/is_trivially_assignable
 traits are always falseKona
@@ -85,12 +85,12 @@
http://cplusplus.github.io/LWG/lwg-defects.html#2469";>2469Wrong
 specification of Requires clause of operator[] for map and 
unordered_mapKona
http://cplusplus.github.io/LWG/lwg-defects.html#2473";>2473basic_filebuf's
 relation to C FILE semanticsKonaComplete
http://cplusplus.github.io/LWG/lwg-defects.html#2476";>2476scoped_allocator_adaptor
 is not assignableKonaPatch Ready
-   http://cplusplus.github.io/LWG/lwg-defects.html#2477";>2477Inconsistency
 of wordings in std::vector::erase() and 
std::deque::erase()Kona
+   http://cplusplus.github.io/LWG/lwg-defects.html#2477";>2477Inconsistency
 of wordings in std::vector::erase() and 
std::deque::erase()KonaComplete
http://cplusplus.github.io/LWG/lwg-defects.html#2483";>2483throw_with_nested()
 should use is_finalKonaComplete
http://cplusplus.github.io/LWG/lwg-defects.html#2484";>2484rethrow_if_nested()
 is doubly unimplementableKonaComplete
http://cplusplus.github.io/LWG/lwg-defects.html#2485";>2485get()
 should be overloaded for const 
tuple&&Kona
http://cplusplus.github.io/LWG/lwg-defects.html#2486";>2486mem_fn()
 should be required to use perfect forwardingKona
-   http://cplusplus.github.io/LWG/lwg-defects.html#2487";>2487bind()
 should be const-overloaded, not 
cv-overloadedKona
+   http://cplusplus.github.io/LWG/lwg-defects.html#2487";>2487bind()
 should be const-overloaded, not 
cv-overloadedKonaComplete
http://cplusplus.github.io/LWG/lwg-defects.html#2489";>2489mem_fn()
 should be noexceptKonaPatch Ready
http://cplusplus.github.io/LWG/lwg-defects.html#2492";>2492Clarify
 requirements for compKonaComplete
http://cplusplus.github.io/LWG/lwg-defects.html#2494";>2494[fund.ts.v2]
 ostream_joiner needs noexceptKona
@@ -114,7 +114,7 @@
 2181 - I suspect that this will not require any code changes, but will 
need to be read carefully.
 2218 - Shouldn't require any code changes.
 2219 - Sizable changes required. INVOKE needs 2 additional overloads and 
plenty of tests. I'm not going to back port this to C++03. (Eric)
-2244 - We don't do this; easy fix; think about how to test it.
+2244 - We don't do this; easy fix. Patch Available
 2250 - Looks like wording cleanup. Need to check more closely, but I think 
there's no code changes here.
 2259 - No code changes needed here.
 2336 - Check later
@@ -129,13 +129,13 @@
 2466 - Simple change; need a test. Patch Available
 2469 - This is a followon to 2464, which we're done. This restates a bunch 
of operations in terms of newer ops. Probably refactoring to do here, but tests 
shouldn't change.
 2473 - There are two changes here; one to filebuf::seekpos and 
the other to filebuf::sync. We do both of these already.
-2476 - Just needed tests. Patch Available
-2477 - Definitely wording cleanup, but check the tests.
+2476 - Just needed tests. Patch Available
+2477 - Wording cleanup.
 2483 - We already do this.
 2484 - We already do this.
 2485 - Ask Eric to do it. 
 2486 - Lots of code changes, all mechanical. Tests will be sizable.
-2487 - Don't know
+2487 - This is removing a requirement on bind(), which we don't 
currently satisfy. So, nothing to do.
 2489 - Looks easy. Just add some NOEXCEPT, and tests. Patch 
Available
 2492 - Wording cleanup; no code changes needed.
 2494 - My implementation of this (not checked in) already has these.

Re: [PATCH] D13523: ASTMatchers: Keep AllCallbacks in a set instead of a vector

[Daniel Jasper via cfe-commits]

djasper closed this revision.
djasper added a comment.

Submitted as r249598.


http://reviews.llvm.org/D13523



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

r249598 - ASTMatchers: Keep AllCallbacks in a set instead of a vector

[Daniel Jasper via cfe-commits]

Author: djasper
Date: Wed Oct  7 14:56:12 2015
New Revision: 249598

URL: http://llvm.org/viewvc/llvm-project?rev=249598&view=rev
Log:
ASTMatchers: Keep AllCallbacks in a set instead of a vector

AllCallbacks is currently only used to call onStartOfTranslationUnit and
onEndOfTranslationUnit on them. In this (and any other scenario I can
come up with), it is important (or at least better) not to have
duplicates in this container. E.g. currently onEndOfTranslationUnit is
called repeatedly on the same callback for every matcher that is
registered with it.

Modified:
cfe/trunk/include/clang/ASTMatchers/ASTMatchFinder.h
cfe/trunk/lib/ASTMatchers/ASTMatchFinder.cpp

Modified: cfe/trunk/include/clang/ASTMatchers/ASTMatchFinder.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/ASTMatchers/ASTMatchFinder.h?rev=249598&r1=249597&r2=249598&view=diff
==
--- cfe/trunk/include/clang/ASTMatchers/ASTMatchFinder.h (original)
+++ cfe/trunk/include/clang/ASTMatchers/ASTMatchFinder.h Wed Oct  7 14:56:12 
2015
@@ -42,6 +42,7 @@
 #define LLVM_CLANG_ASTMATCHERS_ASTMATCHFINDER_H
 
 #include "clang/ASTMatchers/ASTMatchers.h"
+#include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/Support/Timer.h"
 
@@ -208,7 +209,7 @@ public:
 NestedNameSpecifierLoc;
 std::vector> TypeLoc;
 /// \brief All the callbacks in one container to simplify iteration.
-std::vector AllCallbacks;
+llvm::SmallPtrSet AllCallbacks;
   };
 
 private:

Modified: cfe/trunk/lib/ASTMatchers/ASTMatchFinder.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/ASTMatchers/ASTMatchFinder.cpp?rev=249598&r1=249597&r2=249598&view=diff
==
--- cfe/trunk/lib/ASTMatchers/ASTMatchFinder.cpp (original)
+++ cfe/trunk/lib/ASTMatchers/ASTMatchFinder.cpp Wed Oct  7 14:56:12 2015
@@ -913,37 +913,37 @@ MatchFinder::~MatchFinder() {}
 void MatchFinder::addMatcher(const DeclarationMatcher &NodeMatch,
  MatchCallback *Action) {
   Matchers.DeclOrStmt.emplace_back(NodeMatch, Action);
-  Matchers.AllCallbacks.push_back(Action);
+  Matchers.AllCallbacks.insert(Action);
 }
 
 void MatchFinder::addMatcher(const TypeMatcher &NodeMatch,
  MatchCallback *Action) {
   Matchers.Type.emplace_back(NodeMatch, Action);
-  Matchers.AllCallbacks.push_back(Action);
+  Matchers.AllCallbacks.insert(Action);
 }
 
 void MatchFinder::addMatcher(const StatementMatcher &NodeMatch,
  MatchCallback *Action) {
   Matchers.DeclOrStmt.emplace_back(NodeMatch, Action);
-  Matchers.AllCallbacks.push_back(Action);
+  Matchers.AllCallbacks.insert(Action);
 }
 
 void MatchFinder::addMatcher(const NestedNameSpecifierMatcher &NodeMatch,
  MatchCallback *Action) {
   Matchers.NestedNameSpecifier.emplace_back(NodeMatch, Action);
-  Matchers.AllCallbacks.push_back(Action);
+  Matchers.AllCallbacks.insert(Action);
 }
 
 void MatchFinder::addMatcher(const NestedNameSpecifierLocMatcher &NodeMatch,
  MatchCallback *Action) {
   Matchers.NestedNameSpecifierLoc.emplace_back(NodeMatch, Action);
-  Matchers.AllCallbacks.push_back(Action);
+  Matchers.AllCallbacks.insert(Action);
 }
 
 void MatchFinder::addMatcher(const TypeLocMatcher &NodeMatch,
  MatchCallback *Action) {
   Matchers.TypeLoc.emplace_back(NodeMatch, Action);
-  Matchers.AllCallbacks.push_back(Action);
+  Matchers.AllCallbacks.insert(Action);
 }
 
 bool MatchFinder::addDynamicMatcher(const internal::DynTypedMatcher &NodeMatch,


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

Re: r249556 - [VFS] Port driver tool chains to VFS.

[Jan Vesely via cfe-commits]

This change breaks cmake build:
undefined reference to `clang::DiagnosticIDs::DiagnosticIDs()'
/home/jvesely/llvm/tools/clang/unittests/Driver/ToolChainTest.cpp:33:
undefined reference to
`clang::DiagnosticsEngine::DiagnosticsEngine(llvm::IntrusiveRefCntPtr
const&, clang::DiagnosticOptions*, clang::DiagnosticConsumer*, bool)'
/home/jvesely/llvm/tools/clang/unittests/Driver/ToolChainTest.cpp:35:
undefined reference to
`clang::vfs::InMemoryFileSystem::InMemoryFileSystem()'
/home/jvesely/llvm/tools/clang/unittests/Driver/ToolChainTest.cpp:59:
undefined reference to `clang::vfs::InMemoryFileSystem::addFile(llvm::Twine
const&, long, std::unique_ptr >)'
/home/jvesely/llvm/tools/clang/unittests/Driver/ToolChainTest.cpp:33:
undefined reference to `clang::DiagnosticsEngine::~DiagnosticsEngine()'
CMakeFiles/ClangDriverTests.dir/ToolChainTest.cpp.o: In function
`~TestDiagnosticConsumer':
/home/jvesely/llvm/tools/clang/unittests/Driver/ToolChainTest.cpp:32:
undefined reference to `clang::DiagnosticConsumer::~DiagnosticConsumer()'
CMakeFiles/ClangDriverTests.dir/ToolChainTest.cpp.o: In function
`clang::DiagnosticConsumer::DiagnosticConsumer()':
/home/jvesely/llvm/tools/clang/include/clang/Basic/Diagnostic.h:1313:
undefined reference to `vtable for clang::DiagnosticConsumer'
CMakeFiles/ClangDriverTests.dir/ToolChainTest.cpp.o: In function
`llvm::RefCountedBase::Release() const':
/home/jvesely/llvm/include/llvm/ADT/IntrusiveRefCntPtr.h:54: undefined
reference to `clang::DiagnosticIDs::~DiagnosticIDs()'
CMakeFiles/ClangDriverTests.dir/ToolChainTest.cpp.o:(.data.rel.ro+0x80):
undefined reference to
`clang::DiagnosticConsumer::IncludeInDiagnosticCounts() const'
CMakeFiles/ClangDriverTests.dir/ToolChainTest.cpp.o:(.data.rel.ro+0x88):
undefined reference to
`clang::DiagnosticConsumer::HandleDiagnostic(clang::DiagnosticsEngine::Level,
clang::Diagnostic const&)'
collect2: error: ld returned 1 exit status

using Diagnostics requires linking to libclangBasic. Please consider the
attached patch.

BR,
Jan



On Wed, Oct 7, 2015 at 10:48 AM, Benjamin Kramer via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: d0k
> Date: Wed Oct  7 10:48:01 2015
> New Revision: 249556
>
> URL: http://llvm.org/viewvc/llvm-project?rev=249556&view=rev
> Log:
> [VFS] Port driver tool chains to VFS.
>
> There are still some loose ends here but it's sufficient so we can detect
> GCC headers that are inside of a VFS.
>
> Added:
> cfe/trunk/unittests/Driver/ToolChainTest.cpp
> Modified:
> cfe/trunk/include/clang/Basic/VirtualFileSystem.h
> cfe/trunk/include/clang/Driver/Driver.h
> cfe/trunk/include/clang/Driver/ToolChain.h
> cfe/trunk/lib/Basic/VirtualFileSystem.cpp
> cfe/trunk/lib/Driver/Driver.cpp
> cfe/trunk/lib/Driver/ToolChain.cpp
> cfe/trunk/lib/Driver/ToolChains.cpp
> cfe/trunk/lib/Driver/ToolChains.h
> cfe/trunk/lib/Driver/Tools.cpp
> cfe/trunk/unittests/Driver/CMakeLists.txt
>
> Modified: cfe/trunk/include/clang/Basic/VirtualFileSystem.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/VirtualFileSystem.h?rev=249556&r1=249555&r2=249556&view=diff
>
> ==
> --- cfe/trunk/include/clang/Basic/VirtualFileSystem.h (original)
> +++ cfe/trunk/include/clang/Basic/VirtualFileSystem.h Wed Oct  7 10:48:01
> 2015
> @@ -206,6 +206,9 @@ public:
>/// Get the working directory of this file system.
>virtual llvm::ErrorOr getCurrentWorkingDirectory() const =
> 0;
>
> +  /// Check whether a file exists. Provided for convenience.
> +  bool exists(const Twine &Path);
> +
>/// Make \a Path an absolute path.
>///
>/// Makes \a Path absolute using the current directory if it is not
> already.
>
> Modified: cfe/trunk/include/clang/Driver/Driver.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Driver.h?rev=249556&r1=249555&r2=249556&view=diff
>
> ==
> --- cfe/trunk/include/clang/Driver/Driver.h (original)
> +++ cfe/trunk/include/clang/Driver/Driver.h Wed Oct  7 10:48:01 2015
> @@ -36,6 +36,11 @@ namespace opt {
>  }
>
>  namespace clang {
> +
> +namespace vfs {
> +class FileSystem;
> +}
> +
>  namespace driver {
>
>class Action;
> @@ -54,6 +59,8 @@ class Driver {
>
>DiagnosticsEngine &Diags;
>
> +  IntrusiveRefCntPtr VFS;
> +
>enum DriverMode {
>  GCCMode,
>  GXXMode,
> @@ -201,9 +208,9 @@ private:
>   SmallVectorImpl &Names)
> const;
>
>  public:
> -  Driver(StringRef _ClangExecutable,
> - StringRef _DefaultTargetTriple,
> - DiagnosticsEngine &_Diags);
> +  Driver(StringRef ClangExecutable, StringRef DefaultTargetTriple,
> + DiagnosticsEngine &Diags,
> + IntrusiveRefCntPtr VFS = nullptr);
>~Driver();
>
>/// @name Accessors
> @@ -216,6 +223,8 @@ public:
>
>const DiagnosticsEngine &getDiags() const { retur

Re: [PATCH] D13510: [PATCH] Support C++ Core Guidelines copy assignment restrictions

[Matthias Gehre via cfe-commits]

mgehre added a comment.

The user will see [cppcoreguidelines-c-copy-assignment-signature] in warnings, 
but there is no
docs/clang-tidy/checks/cppcoreguidelines-c-copy-assignment-signature.rst.

The user won't know that he needs to look for misc-assign-operator-signature.

Maybe create a link to 
docs/clang-tidy/checks/misc-assign-operator-signature.rst?
(and also add it to docs/clang-tidy/checks/list.rst)

Also, users reading the documentation will not see that 
cppcoreguidelines-c-copy-assignment-signature is available.


http://reviews.llvm.org/D13510



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

Re: [PATCH] D13304: Avoid inlining in throw statement

[Richard Smith via cfe-commits]

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

This seems like something that's much better handled by the inliner. We should 
presumably treat all the code that leads up to the throw as being cold, not 
just the invocation of the throw-expression itself, and it seems like it should 
be straightforward for the inliner to check whether a call invariably leads to 
an `unreachable` or an unwind.

Also, blindly marking the calls as noinline doesn't seem like a good idea at 
all. This should just factor into the inline cost heuristics, and shouldn't 
stop us inlining in cases where doing so is obviously going to cause a code 
size reduction.


http://reviews.llvm.org/D13304



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

Re: [PATCH] D13510: [PATCH] Support C++ Core Guidelines copy assignment restrictions

[Samuel Benzaquen via cfe-commits]

sbenza added a comment.

In http://reviews.llvm.org/D13510#261925, @aaron.ballman wrote:

> In http://reviews.llvm.org/D13510#261825, @Eugene.Zelenko wrote:
>
> > I think it'll be fine to rename check without leaving traces of misc. Same 
> > thing happened with modernize-shrink-to-fit.
>
>
> I think the difference here is that many C++ Core Guideline checks are... 
> chatty, and so these checks are likely to not be enabled (especially on 
> existing code bases). By leaving the check in misc-*, it is more likely to 
> provide value to users that aren't able to use the cppcoreguidelines-* checks 
> yet.


Now that we are registering checks with more than one name, it might be a good 
idea to add a dedup step to avoid redundant warnings and/or wasted resources.
Not on this change but something to consider.


http://reviews.llvm.org/D13510



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

r249608 - Fix a shared CMake build by linking with libclangBasic.

[Benjamin Kramer via cfe-commits]

Author: d0k
Date: Wed Oct  7 15:19:25 2015
New Revision: 249608

URL: http://llvm.org/viewvc/llvm-project?rev=249608&view=rev
Log:
Fix a shared CMake build by linking with libclangBasic.

Patch by Jan Vesely!

Modified:
cfe/trunk/unittests/Driver/CMakeLists.txt

Modified: cfe/trunk/unittests/Driver/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Driver/CMakeLists.txt?rev=249608&r1=249607&r2=249608&view=diff
==
--- cfe/trunk/unittests/Driver/CMakeLists.txt (original)
+++ cfe/trunk/unittests/Driver/CMakeLists.txt Wed Oct  7 15:19:25 2015
@@ -9,4 +9,5 @@ add_clang_unittest(ClangDriverTests
 
 target_link_libraries(ClangDriverTests
   clangDriver
+  clangBasic
   )


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

Re: r249556 - [VFS] Port driver tool chains to VFS.

[Benjamin Kramer via cfe-commits]

On Wed, Oct 7, 2015 at 10:09 PM, Jan Vesely  wrote:
>
> This change breaks cmake build:
> undefined reference to `clang::DiagnosticIDs::DiagnosticIDs()'
> /home/jvesely/llvm/tools/clang/unittests/Driver/ToolChainTest.cpp:33: 
> undefined reference to 
> `clang::DiagnosticsEngine::DiagnosticsEngine(llvm::IntrusiveRefCntPtr
>  const&, clang::DiagnosticOptions*, clang::DiagnosticConsumer*, bool)'
> /home/jvesely/llvm/tools/clang/unittests/Driver/ToolChainTest.cpp:35: 
> undefined reference to `clang::vfs::InMemoryFileSystem::InMemoryFileSystem()'
> /home/jvesely/llvm/tools/clang/unittests/Driver/ToolChainTest.cpp:59: 
> undefined reference to `clang::vfs::InMemoryFileSystem::addFile(llvm::Twine 
> const&, long, std::unique_ptr std::default_delete >)'
> /home/jvesely/llvm/tools/clang/unittests/Driver/ToolChainTest.cpp:33: 
> undefined reference to `clang::DiagnosticsEngine::~DiagnosticsEngine()'
> CMakeFiles/ClangDriverTests.dir/ToolChainTest.cpp.o: In function 
> `~TestDiagnosticConsumer':
> /home/jvesely/llvm/tools/clang/unittests/Driver/ToolChainTest.cpp:32: 
> undefined reference to `clang::DiagnosticConsumer::~DiagnosticConsumer()'
> CMakeFiles/ClangDriverTests.dir/ToolChainTest.cpp.o: In function 
> `clang::DiagnosticConsumer::DiagnosticConsumer()':
> /home/jvesely/llvm/tools/clang/include/clang/Basic/Diagnostic.h:1313: 
> undefined reference to `vtable for clang::DiagnosticConsumer'
> CMakeFiles/ClangDriverTests.dir/ToolChainTest.cpp.o: In function 
> `llvm::RefCountedBase::Release() const':
> /home/jvesely/llvm/include/llvm/ADT/IntrusiveRefCntPtr.h:54: undefined 
> reference to `clang::DiagnosticIDs::~DiagnosticIDs()'
> CMakeFiles/ClangDriverTests.dir/ToolChainTest.cpp.o:(.data.rel.ro+0x80): 
> undefined reference to 
> `clang::DiagnosticConsumer::IncludeInDiagnosticCounts() const'
> CMakeFiles/ClangDriverTests.dir/ToolChainTest.cpp.o:(.data.rel.ro+0x88): 
> undefined reference to 
> `clang::DiagnosticConsumer::HandleDiagnostic(clang::DiagnosticsEngine::Level, 
> clang::Diagnostic const&)'
> collect2: error: ld returned 1 exit status
>
> using Diagnostics requires linking to libclangBasic. Please consider the 
> attached patch.

r249608. Thanks!

- Ben

>
> On Wed, Oct 7, 2015 at 10:48 AM, Benjamin Kramer via cfe-commits 
>  wrote:
>>
>> Author: d0k
>> Date: Wed Oct  7 10:48:01 2015
>> New Revision: 249556
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=249556&view=rev
>> Log:
>> [VFS] Port driver tool chains to VFS.
>>
>> There are still some loose ends here but it's sufficient so we can detect
>> GCC headers that are inside of a VFS.
>>
>> Added:
>> cfe/trunk/unittests/Driver/ToolChainTest.cpp
>> Modified:
>> cfe/trunk/include/clang/Basic/VirtualFileSystem.h
>> cfe/trunk/include/clang/Driver/Driver.h
>> cfe/trunk/include/clang/Driver/ToolChain.h
>> cfe/trunk/lib/Basic/VirtualFileSystem.cpp
>> cfe/trunk/lib/Driver/Driver.cpp
>> cfe/trunk/lib/Driver/ToolChain.cpp
>> cfe/trunk/lib/Driver/ToolChains.cpp
>> cfe/trunk/lib/Driver/ToolChains.h
>> cfe/trunk/lib/Driver/Tools.cpp
>> cfe/trunk/unittests/Driver/CMakeLists.txt
>>
>> Modified: cfe/trunk/include/clang/Basic/VirtualFileSystem.h
>> URL: 
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/VirtualFileSystem.h?rev=249556&r1=249555&r2=249556&view=diff
>> ==
>> --- cfe/trunk/include/clang/Basic/VirtualFileSystem.h (original)
>> +++ cfe/trunk/include/clang/Basic/VirtualFileSystem.h Wed Oct  7 10:48:01 
>> 2015
>> @@ -206,6 +206,9 @@ public:
>>/// Get the working directory of this file system.
>>virtual llvm::ErrorOr getCurrentWorkingDirectory() const = 0;
>>
>> +  /// Check whether a file exists. Provided for convenience.
>> +  bool exists(const Twine &Path);
>> +
>>/// Make \a Path an absolute path.
>>///
>>/// Makes \a Path absolute using the current directory if it is not 
>> already.
>>
>> Modified: cfe/trunk/include/clang/Driver/Driver.h
>> URL: 
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Driver.h?rev=249556&r1=249555&r2=249556&view=diff
>> ==
>> --- cfe/trunk/include/clang/Driver/Driver.h (original)
>> +++ cfe/trunk/include/clang/Driver/Driver.h Wed Oct  7 10:48:01 2015
>> @@ -36,6 +36,11 @@ namespace opt {
>>  }
>>
>>  namespace clang {
>> +
>> +namespace vfs {
>> +class FileSystem;
>> +}
>> +
>>  namespace driver {
>>
>>class Action;
>> @@ -54,6 +59,8 @@ class Driver {
>>
>>DiagnosticsEngine &Diags;
>>
>> +  IntrusiveRefCntPtr VFS;
>> +
>>enum DriverMode {
>>  GCCMode,
>>  GXXMode,
>> @@ -201,9 +208,9 @@ private:
>>   SmallVectorImpl &Names) const;
>>
>>  public:
>> -  Driver(StringRef _ClangExecutable,
>> - StringRef _DefaultTargetTriple,
>> - DiagnosticsEngine &_Diags);
>> +  Driver(StringRef ClangExecutable, StringRef De

Re: [PATCH] D13510: [PATCH] Support C++ Core Guidelines copy assignment restrictions

[Aaron Ballman via cfe-commits]

On Wed, Oct 7, 2015 at 4:18 PM, Samuel Benzaquen  wrote:
> sbenza added a comment.
>
> In http://reviews.llvm.org/D13510#261925, @aaron.ballman wrote:
>
>> In http://reviews.llvm.org/D13510#261825, @Eugene.Zelenko wrote:
>>
>> > I think it'll be fine to rename check without leaving traces of misc. Same 
>> > thing happened with modernize-shrink-to-fit.
>>
>>
>> I think the difference here is that many C++ Core Guideline checks are... 
>> chatty, and so these checks are likely to not be enabled (especially on 
>> existing code bases). By leaving the check in misc-*, it is more likely to 
>> provide value to users that aren't able to use the cppcoreguidelines-* 
>> checks yet.
>
>
> Now that we are registering checks with more than one name, it might be a 
> good idea to add a dedup step to avoid redundant warnings and/or wasted 
> resources.
> Not on this change but something to consider.

They don't appear to be duplicated when you enable the check multiple
times; it seems to take the last registered checker. As an example:
http://pastebin.com/RyrNhKNL

However, I agree, registering checkers with multiple names does bring
up some problems regarding displaying the diagnostics as well as
documentation that likely requires a bit more thought.

~Aaron

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

Re: [PATCH] D13482: Revised Initial patch for PS4 toolchain

[Katya Romanova via cfe-commits]

kromanova added a comment.

Hi Pierre,
I noticed the same issue. When I downloaded a patch from  
http://reviews.llvm.org/D11279, I had to manually add .keep files to make tests 
to pass.

It's a Phabricator problem. The diff file that I uploaded to Phabricator 
contains .keep files, however the diff that is available for download doesn't 
mention these files. Most likely this happens because .keep files are empty.
Katya.


Repository:
  rL LLVM

http://reviews.llvm.org/D13482



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

[clang-tools-extra] r249612 - Fixing links and reformatting code; NFC.

[Aaron Ballman via cfe-commits]

Author: aaronballman
Date: Wed Oct  7 15:33:36 2015
New Revision: 249612

URL: http://llvm.org/viewvc/llvm-project?rev=249612&view=rev
Log:
Fixing links and reformatting code; NFC.

Patch by Marek Kurdej!

Modified:

clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProTypeConstCastCheck.cpp
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProTypeConstCastCheck.h

clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProTypeReinterpretCastCheck.cpp

clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProTypeReinterpretCastCheck.h

clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-pro-type-const-cast.cpp

clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-pro-type-reinterpret-cast.cpp

Modified: 
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProTypeConstCastCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProTypeConstCastCheck.cpp?rev=249612&r1=249611&r2=249612&view=diff
==
--- 
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProTypeConstCastCheck.cpp 
(original)
+++ 
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProTypeConstCastCheck.cpp 
Wed Oct  7 15:33:36 2015
@@ -17,7 +17,7 @@ namespace clang {
 namespace tidy {
 
 void ProTypeConstCastCheck::registerMatchers(MatchFinder *Finder) {
-  if(!getLangOpts().CPlusPlus)
+  if (!getLangOpts().CPlusPlus)
 return;
 
   Finder->addMatcher(cxxConstCastExpr().bind("cast"), this);
@@ -30,4 +30,3 @@ void ProTypeConstCastCheck::check(const
 
 } // namespace tidy
 } // namespace clang
-

Modified: 
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProTypeConstCastCheck.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProTypeConstCastCheck.h?rev=249612&r1=249611&r2=249612&view=diff
==
--- 
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProTypeConstCastCheck.h 
(original)
+++ 
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProTypeConstCastCheck.h 
Wed Oct  7 15:33:36 2015
@@ -31,4 +31,3 @@ public:
 } // namespace clang
 
 #endif // 
LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_PRO_TYPE_CONST_CAST_H
-

Modified: 
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProTypeReinterpretCastCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProTypeReinterpretCastCheck.cpp?rev=249612&r1=249611&r2=249612&view=diff
==
--- 
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProTypeReinterpretCastCheck.cpp
 (original)
+++ 
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProTypeReinterpretCastCheck.cpp
 Wed Oct  7 15:33:36 2015
@@ -1,4 +1,4 @@
-//===--- ProTypeReinterpretCastCheck.cpp - 
clang-tidy--===//
+//===--- ProTypeReinterpretCastCheck.cpp - 
clang-tidy--===//
 //
 // The LLVM Compiler Infrastructure
 //
@@ -23,11 +23,11 @@ void ProTypeReinterpretCastCheck::regist
   Finder->addMatcher(cxxReinterpretCastExpr().bind("cast"), this);
 }
 
-void ProTypeReinterpretCastCheck::check(const MatchFinder::MatchResult 
&Result) {
+void ProTypeReinterpretCastCheck::check(
+const MatchFinder::MatchResult &Result) {
   const auto *MatchedCast =
   Result.Nodes.getNodeAs("cast");
-  diag(MatchedCast->getOperatorLoc(),
-   "do not use reinterpret_cast");
+  diag(MatchedCast->getOperatorLoc(), "do not use reinterpret_cast");
 }
 
 } // namespace tidy

Modified: 
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProTypeReinterpretCastCheck.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProTypeReinterpretCastCheck.h?rev=249612&r1=249611&r2=249612&view=diff
==
--- 
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProTypeReinterpretCastCheck.h
 (original)
+++ 
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProTypeReinterpretCastCheck.h
 Wed Oct  7 15:33:36 2015
@@ -18,7 +18,7 @@ namespace tidy {
 /// Flags all occurrences of reinterpret_cast
 ///
 /// For the user-facing documentation see:
-/// http://clang.llvm.org/extra/clang-tidy/checks/misc-no-reinterpret-cast.html
+/// 
http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-pro-type-reinterpret-cast.html
 class ProTypeReinterpretCastCheck : public ClangTidyCheck {
 public:
   ProTypeReinterpretCastCheck(StringRef Name, ClangTidyContext *Context)

Modified: 
clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-pro-type-const-cast.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-pro-type-const-cast.cpp?rev=249612&r1=249611&r2=249612&view=diff

Re: [PATCH] D13525: [CodeGen] Attach function attributes to functions created in CGBlocks.cpp.

[Duncan P. N. Exon Smith via cfe-commits]

> On 2015-Oct-07, at 11:35, Akira Hatanaka  wrote:
> 
> ahatanak created this revision.
> ahatanak added reviewers: dexonsmith, echristo.
> ahatanak added a subscriber: cfe-commits.
> 
> This patch makes changes to attach function attributes to the following 
> functions created in CGBlocks.cpp:
> 
> __copy_helper_block_
> __destroy_helper_block_
> __Block_byref_object_copy_
> __Block_byref_object_dispose_
> 
> There are other places in clang where function attributes are not attached to 
> functions. I plan to follow up with patches that fix those places too.
> 
> http://reviews.llvm.org/D13525
> 
> Files:
>  lib/CodeGen/CGBlocks.cpp
>  lib/CodeGen/CodeGenModule.cpp
>  lib/CodeGen/TargetInfo.cpp
>  test/CodeGenObjC/arc-blocks.m
> 
> 

> Index: lib/CodeGen/CodeGenModule.cpp
> ===
> --- lib/CodeGen/CodeGenModule.cpp
> +++ lib/CodeGen/CodeGenModule.cpp
> @@ -786,29 +786,32 @@
>if (!hasUnwindExceptions(LangOpts))
>  B.addAttribute(llvm::Attribute::NoUnwind);
>  
> -  if (D->hasAttr()) {
> -// Naked implies noinline: we should not be inlining such functions.
> -B.addAttribute(llvm::Attribute::Naked);
> -B.addAttribute(llvm::Attribute::NoInline);
> -  } else if (D->hasAttr()) {
> -B.addAttribute(llvm::Attribute::NoDuplicate);
> -  } else if (D->hasAttr()) {
> -B.addAttribute(llvm::Attribute::NoInline);
> -  } else if (D->hasAttr() &&
> - 
> !F->getAttributes().hasAttribute(llvm::AttributeSet::FunctionIndex,
> -  llvm::Attribute::NoInline)) {
> -// (noinline wins over always_inline, and we can't specify both in IR)
> -B.addAttribute(llvm::Attribute::AlwaysInline);
> -  }
> -
> -  if (D->hasAttr()) {
> -if (!D->hasAttr())
> -  B.addAttribute(llvm::Attribute::OptimizeForSize);
> -B.addAttribute(llvm::Attribute::Cold);
> -  }
> -
> -  if (D->hasAttr())
> -B.addAttribute(llvm::Attribute::MinSize);
> +  if (D) {

I wonder, is there a way to reorder this function so that you can use an
early return here?  The extra indentation (and resulting noise) is
unfortunate.

Really, the way attributes are added should be refactored.  I spent a few
days trying to do that in the spring but (obviously) never got to
anything committable.  If you have any ideas for ways to clean this up
while you're in here, please "fix" it.

Anyway, LGTM after a prep NFC commit to avoid the extra indentation.

> +if (D->hasAttr()) {
> +  // Naked implies noinline: we should not be inlining such functions.
> +  B.addAttribute(llvm::Attribute::Naked);
> +  B.addAttribute(llvm::Attribute::NoInline);
> +} else if (D->hasAttr()) {
> +  B.addAttribute(llvm::Attribute::NoDuplicate);
> +} else if (D->hasAttr()) {
> +  B.addAttribute(llvm::Attribute::NoInline);
> +} else if (D->hasAttr() &&
> +   !F->getAttributes().hasAttribute(
> +   llvm::AttributeSet::FunctionIndex,
> +   llvm::Attribute::NoInline)) {
> +  // (noinline wins over always_inline, and we can't specify both in IR)
> +  B.addAttribute(llvm::Attribute::AlwaysInline);
> +}
> +
> +if (D->hasAttr()) {
> +  if (!D->hasAttr())
> +B.addAttribute(llvm::Attribute::OptimizeForSize);
> +  B.addAttribute(llvm::Attribute::Cold);
> +}
> +
> +if (D->hasAttr())
> +  B.addAttribute(llvm::Attribute::MinSize);
> +  }
>  
>if (LangOpts.getStackProtector() == LangOptions::SSPOn)
>  B.addAttribute(llvm::Attribute::StackProtect);
> @@ -821,6 +824,9 @@
> llvm::AttributeSet::get(
> F->getContext(), llvm::AttributeSet::FunctionIndex, 
> B));
>  
> +  if (!D)
> +return;
> +
>if (D->hasAttr()) {
>  // OptimizeNone implies noinline; we should not be inlining such 
> functions.
>  F->addFnAttr(llvm::Attribute::OptimizeNone);
> @@ -859,12 +865,12 @@
>  
>  void CodeGenModule::SetCommonAttributes(const Decl *D,
>  llvm::GlobalValue *GV) {
> -  if (const auto *ND = dyn_cast(D))
> +  if (const auto *ND = dyn_cast_or_null(D))
>  setGlobalVisibility(GV, ND);
>else
>  GV->setVisibility(llvm::GlobalValue::DefaultVisibility);
>  
> -  if (D->hasAttr())
> +  if (D && D->hasAttr())
>  addUsedGlobal(GV);
>  }
>  
> @@ -882,8 +888,9 @@
>llvm::GlobalObject *GO) {
>SetCommonAttributes(D, GO);
>  
> -  if (const SectionAttr *SA = D->getAttr())
> -GO->setSection(SA->getName());
> +  if (D)
> +if (const SectionAttr *SA = D->getAttr())
> +  GO->setSection(SA->getName());
>  
>getTargetCodeGenInfo().setTargetAttributes(D, GO, *this);
>  }
> Index: lib/CodeGen/CGBlocks.cpp
> ===
> --- lib/CodeGen/CGBlocks.cpp
> +++ lib/CodeGen/CGBlocks.cpp
> @@ -1345,6 +1345,9 @@
>

Re: [PATCH] D13399: [CMake] Bug 14109 - CMake build for compiler-rt should use just-built clang

[Chris Bieneman via cfe-commits]

beanz added a comment.

Ping?


http://reviews.llvm.org/D13399



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

Re: [PATCH] D11328: [clang-tidy] new "throw-by-value-catch-by-reference" check for clang-tidy

[Tobias Langner via cfe-commits]

randomcppprogrammer updated this revision to Diff 36795.
randomcppprogrammer marked 11 inline comments as done.
randomcppprogrammer added a comment.

Incorporated feedback from Aaron Ballmann

main changes:

- replaced isa followed by cast with dyn_cast
- reworked comments
- fixed typo in diagnosis message


http://reviews.llvm.org/D11328

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/MiscTidyModule.cpp
  clang-tidy/misc/ThrowByValueCatchByReferenceCheck.cpp
  clang-tidy/misc/ThrowByValueCatchByReferenceCheck.h
  test/clang-tidy/misc-throw-by-value-catch-by-reference.cpp

Index: test/clang-tidy/misc-throw-by-value-catch-by-reference.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-throw-by-value-catch-by-reference.cpp
@@ -0,0 +1,149 @@
+// RUN: %python %S/check_clang_tidy.py %s misc-throw-by-value-catch-by-reference %t
+
+
+class logic_error {
+public:
+  logic_error(const char *message) {}
+};
+
+typedef logic_error *logic_ptr;
+typedef logic_ptr logic_double_typedef;
+
+int lastException;
+
+template  struct remove_reference  {typedef T type;};
+template  struct remove_reference  {typedef T type;};
+template  struct remove_reference {typedef T type;};
+
+template 
+typename remove_reference::type&& move(T&& arg) {
+  return static_cast::type&&>(arg);
+}
+
+logic_error CreateException() { return logic_error("created"); }
+
+void testThrowFunc() {
+  throw new logic_error("by pointer");
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: throw expression throws a pointer; it should throw a non-pointer value instead [misc-throw-by-value-catch-by-reference]
+  logic_ptr tmp = new logic_error("by pointer");
+  throw tmp;
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: throw expression should throw anonymous temporary values instead [misc-throw-by-value-catch-by-reference]
+  // CHECK-MESSAGES: :[[@LINE-2]]:9: warning: throw expression throws a pointer; it should throw a non-pointer value instead [misc-throw-by-value-catch-by-reference]
+  throw logic_error("by value");
+  auto *literal = "test";
+  throw logic_error(literal);
+  throw "test string literal";
+  throw L"throw wide string literal";
+  const char *characters = 0;
+  throw characters;
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: throw expression should throw anonymous temporary values instead [misc-throw-by-value-catch-by-reference]
+  // CHECK-MESSAGES: :[[@LINE-2]]:9: warning: throw expression throws a pointer; it should throw a non-pointer value instead [misc-throw-by-value-catch-by-reference]
+   logic_error lvalue("lvalue");
+  throw lvalue;
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: throw expression should throw anonymous temporary values instead [misc-throw-by-value-catch-by-reference]
+  
+  // can be enabled once std::move can be included
+  throw move(lvalue);  
+  int &ex = lastException;
+  throw ex;
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: throw expression should throw anonymous temporary values instead [misc-throw-by-value-catch-by-reference]
+  throw CreateException();
+}
+
+void throwReferenceFunc(logic_error &ref) {
+  throw ref;
+}
+
+void catchByPointer() {
+  try {
+testThrowFunc();
+  } catch (logic_error *e) {
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: catch handler catches a pointer value; should throw a non-pointer value and catch by reference instead [misc-throw-by-value-catch-by-reference]
+  }
+}
+
+void catchByValue() {
+  try {
+testThrowFunc();
+  } catch (logic_error e) {
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: catch handler catches a pointer value; should throw a non-pointer value and catch by reference instead [misc-throw-by-value-catch-by-reference]
+  }
+}
+
+void catchByReference() {
+  try {
+testThrowFunc();
+  } catch (logic_error &e) {
+  }
+}
+
+void catchByConstReference() {
+  try {
+testThrowFunc();
+  } catch (const logic_error &e) {
+  }
+}
+
+void catchTypedef() {
+  try {
+testThrowFunc();
+  } catch (logic_ptr) {
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: catch handler catches a pointer value; should throw a non-pointer value and catch by reference instead [misc-throw-by-value-catch-by-reference]
+  }
+}
+
+void catchAll() {
+  try {
+testThrowFunc();
+  } catch (...) {
+  }
+}
+
+void catchLiteral() {
+  try {
+testThrowFunc();
+  } catch (const char *) {
+  } catch (const wchar_t *) {
+// disabled for now until it is clear
+// how to enable them in the test
+//} catch (const char16_t*) {
+//} catch (const char32_t*) {
+  }
+}
+
+// catching fundamentals should not warn
+void catchFundamental() {
+  try {
+testThrowFunc();
+  } catch (int) {
+  } catch (double) {
+  } catch (unsigned long) {
+  }
+}
+
+struct TrivialType {
+  double x;
+  double y;
+};
+
+void catchTrivial() {
+  try {
+testThrowFunc();
+  } catch (TrivialType) {
+  }
+}
+
+typedef logic_error& fine;
+void additionalTests() {
+  try {
+  } catch (int i) { // ok
+throw i;

r249616 - [WinEH] Don't use lifetime markers for MS catch parameters

[Reid Kleckner via cfe-commits]

Author: rnk
Date: Wed Oct  7 16:03:41 2015
New Revision: 249616

URL: http://llvm.org/viewvc/llvm-project?rev=249616&view=rev
Log:
[WinEH] Don't use lifetime markers for MS catch parameters

We don't have a good place to put them. Our previous spot was causing us
to optimize loads from the exception object to undef, because it was
after the catchpad instruction that models the write to the catch
object.

Modified:
cfe/trunk/lib/CodeGen/CGDecl.cpp
cfe/trunk/test/CodeGenCXX/microsoft-abi-eh-catch.cpp

Modified: cfe/trunk/lib/CodeGen/CGDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDecl.cpp?rev=249616&r1=249615&r2=249616&view=diff
==
--- cfe/trunk/lib/CodeGen/CGDecl.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDecl.cpp Wed Oct  7 16:03:41 2015
@@ -974,9 +974,15 @@ CodeGenFunction::EmitAutoVarAlloca(const
   address = CreateTempAlloca(allocaTy, allocaAlignment);
   address.getPointer()->setName(D.getName());
 
+  // Don't emit lifetime markers for MSVC catch parameters. The lifetime of
+  // the catch parameter starts in the catchpad instruction, and we can't
+  // insert code in those basic blocks.
+  bool IsMSCatchParam =
+  D.isExceptionVariable() && getTarget().getCXXABI().isMicrosoft();
+
   // Emit a lifetime intrinsic if meaningful.  There's no point
   // in doing this if we don't have a valid insertion point (?).
-  if (HaveInsertPoint()) {
+  if (HaveInsertPoint() && !IsMSCatchParam) {
 uint64_t size = CGM.getDataLayout().getTypeAllocSize(allocaTy);
 emission.SizeForLifetimeMarkers =
   EmitLifetimeStart(size, address.getPointer());

Modified: cfe/trunk/test/CodeGenCXX/microsoft-abi-eh-catch.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/microsoft-abi-eh-catch.cpp?rev=249616&r1=249615&r2=249616&view=diff
==
--- cfe/trunk/test/CodeGenCXX/microsoft-abi-eh-catch.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/microsoft-abi-eh-catch.cpp Wed Oct  7 16:03:41 
2015
@@ -1,5 +1,6 @@
 // RUN: %clang_cc1 -std=c++11 -emit-llvm %s -o - 
-triple=x86_64-pc-windows-msvc \
 // RUN: -mconstructor-aliases -fexceptions -fcxx-exceptions -fnew-ms-eh \
+// RUN: -O1 -disable-llvm-optzns \
 // RUN: | FileCheck -check-prefix WIN64 %s
 
 extern "C" void might_throw();
@@ -47,8 +48,17 @@ extern "C" void catch_int() {
 
 // WIN64-LABEL: define void @catch_int()
 // WIN64: catchpad [%rtti.TypeDescriptor2* @"\01??_R0H@8", i32 0, i32* 
%[[e_addr:[^\]]*]]]
+//
+// The catchpad instruction starts the lifetime of 'e'. Unfortunately, that
+// leaves us with nowhere to put lifetime.start, so we don't emit lifetime
+// markers for now.
+// WIN64-NOT: lifetime.start
+//
 // WIN64: %[[e_i8:[^ ]*]] = bitcast i32* %[[e_addr]] to i8*
-// WIN64: call void @handle_exception(i8* %[[e_i8]])
+// WIN64-NOT: lifetime.start
+// WIN64: call void @handle_exception
+// WIN64-SAME: (i8* %[[e_i8]])
+// WIN64-NOT: lifetime.end
 // WIN64: catchret
 
 extern "C" void catch_int_unnamed() {


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

Re: [PATCH] D13099: [Analyzer] Don’t invalidate CXXThis when conservatively evaluating const methods (PR 21606)

[Anna Zaks via cfe-commits]

zaks.anna added a comment.

> Again I'm not sure this is worth checking for, based on the assumption that a 
> reasonable const method won't modify the relevant object. What do people 
> think?


These are heuristics. I think dealing with this case should not be a 
pre-requisite for the patch. We can see how much this is an issue in practice 
through evaluation on existing codebases as well as getting user feedback once 
this lands.



Comment at: test/Analysis/PR21606.cpp:2
@@ +1,3 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core
+// PR21606
+

Can this be moved into some other test file?
You can have the PR name in a comment or a method name.


Comment at: test/Analysis/const-method-call.cpp:72
@@ +71,3 @@
+
+void checkThatConstMethodDoesInvalidateInheritedPointedAtMemory() {
+  int x = 1;

Maybe we should add a test case where the const method is inherited?


http://reviews.llvm.org/D13099



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

Fwd: [PATCH] D11328: [clang-tidy] new "throw-by-value-catch-by-reference" check for clang-tidy

[Tobias Langner via cfe-commits]

Hi,I incorporated most of your changes. There are 2 open issues- on one location I could not follow your advice, the compiler refused to compile the code. I chose a different approach and hope you like it.- the second thing is this MaterializeTemporary advice that you gave. I don’t fully understand it (possibly due to a lack of understanding the AST and a lack of documentation of the proposed methods). Could you please flesh out your idea and why you think it is necessary? Or at least give an example where the current code does not work correctly.RegardsTobiasBegin forwarded message:From: Tobias Langner Date: 8 October 2015 at 00:02:39 GMT+3To: randomcppprogram...@gmail.com, kli...@google.com, cfe-commits@lists.llvm.org, aaron.ball...@gmail.comCc: j...@jbcoe.netSubject: Re: [PATCH] D11328: [clang-tidy] new "throw-by-value-catch-by-reference" check for clang-tidyrandomcppprogrammer updated this revision to Diff 36795.randomcppprogrammer marked 11 inline comments as done.randomcppprogrammer added a comment.Incorporated feedback from Aaron Ballmannmain changes:- replaced isa followed by cast with dyn_cast- reworked comments- fixed typo in diagnosis messagehttp://reviews.llvm.org/D11328Files:  clang-tidy/misc/CMakeLists.txt  clang-tidy/misc/MiscTidyModule.cpp  clang-tidy/misc/ThrowByValueCatchByReferenceCheck.cpp  clang-tidy/misc/ThrowByValueCatchByReferenceCheck.h  test/clang-tidy/misc-throw-by-value-catch-by-reference.cppIndex: test/clang-tidy/misc-throw-by-value-catch-by-reference.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-throw-by-value-catch-by-reference.cpp
@@ -0,0 +1,149 @@
+// RUN: %python %S/check_clang_tidy.py %s misc-throw-by-value-catch-by-reference %t
+
+
+class logic_error {
+public:
+  logic_error(const char *message) {}
+};
+
+typedef logic_error *logic_ptr;
+typedef logic_ptr logic_double_typedef;
+
+int lastException;
+
+template  struct remove_reference  {typedef T type;};
+template  struct remove_reference  {typedef T type;};
+template  struct remove_reference {typedef T type;};
+
+template 
+typename remove_reference::type&& move(T&& arg) {
+  return static_cast::type&&>(arg);
+}
+
+logic_error CreateException() { return logic_error("created"); }
+
+void testThrowFunc() {
+  throw new logic_error("by pointer");
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: throw expression throws a pointer; it should throw a non-pointer value instead [misc-throw-by-value-catch-by-reference]
+  logic_ptr tmp = new logic_error("by pointer");
+  throw tmp;
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: throw expression should throw anonymous temporary values instead [misc-throw-by-value-catch-by-reference]
+  // CHECK-MESSAGES: :[[@LINE-2]]:9: warning: throw expression throws a pointer; it should throw a non-pointer value instead [misc-throw-by-value-catch-by-reference]
+  throw logic_error("by value");
+  auto *literal = "test";
+  throw logic_error(literal);
+  throw "test string literal";
+  throw L"throw wide string literal";
+  const char *characters = 0;
+  throw characters;
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: throw expression should throw anonymous temporary values instead [misc-throw-by-value-catch-by-reference]
+  // CHECK-MESSAGES: :[[@LINE-2]]:9: warning: throw expression throws a pointer; it should throw a non-pointer value instead [misc-throw-by-value-catch-by-reference]
+   logic_error lvalue("lvalue");
+  throw lvalue;
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: throw expression should throw anonymous temporary values instead [misc-throw-by-value-catch-by-reference]
+  
+  // can be enabled once std::move can be included
+  throw move(lvalue);  
+  int &ex = lastException;
+  throw ex;
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: throw expression should throw anonymous temporary values instead [misc-throw-by-value-catch-by-reference]
+  throw CreateException();
+}
+
+void throwReferenceFunc(logic_error &ref) {
+  throw ref;
+}
+
+void catchByPointer() {
+  try {
+testThrowFunc();
+  } catch (logic_error *e) {
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: catch handler catches a pointer value; should throw a non-pointer value and catch by reference instead [misc-throw-by-value-catch-by-reference]
+  }
+}
+
+void catchByValue() {
+  try {
+testThrowFunc();
+  } catch (logic_error e) {
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: catch handler catches a pointer value; should throw a non-pointer value and catch by reference instead [misc-throw-by-value-catch-by-reference]
+  }
+}
+
+void catchByReference() {
+  try {
+testThrowFunc();
+  } catch (logic_error &e) {
+  }
+}
+
+void catchByConstReference() {
+  try {
+testThrowFunc();
+  } catch (const logic_error &e) {
+  }
+}
+
+void catchTypedef() {
+  try {
+testThrowFunc();
+  } catch (logic_ptr) {
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: catch handler catches a pointer value; should throw a

Re: [PATCH] D13192: Fix incorrect parsing of arguments for nested functions

[Marshall Clow via cfe-commits]

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

This looks fine to me.  Thanks for the patch (and the test case).


http://reviews.llvm.org/D13192



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

Re: [PATCH] D12747: Implement [depr.c.headers]

[Richard Smith via cfe-commits]

Marshall: ping, does the below satisfy your concerns about the direction
here?

On Wed, Sep 16, 2015 at 2:04 PM, Richard Smith 
wrote:

> On Mon, Sep 14, 2015 at 7:07 AM, Marshall Clow 
> wrote:
>
>> mclow.lists added a comment.
>>
>> I have two concerns about this patch (w/o commenting on the actual code).
>>
>> 1. Until very recently, I was under the impression that C libraries
>> _either_ defined a macro, or had a function. I was quite surprised to find
>> that glibc did both.
>
>
> Yes, this is required by the C standard. C11 7.1.4/1 says:
>
> "Any function declared in a header may be additionally implemented as a
> function-like macro defined in the header [...]. Any macro definition of a
> function can be suppressed locally by enclosing the name of the function in
> parentheses, because the name is then not followed by the left parenthesis
> that indicates expansion of a macro function name. For the same syntactic
> reason, it is permitted to take the address of a library function even if
> it is also defined as a macro. [Footnote: This means that an implementation
> shall provide an actual function for each library function, even if it also
> provides a macro for that function.]"
>
> Have you checked other C libraries (Apple, FreeBSD, Android, Windows) to
>> see if they also define both?
>
>
> No, but libstdc++ does the same #undef thing, so any platform it supports
> must have a non-broken C standard library.
>
>
>> 2. This adds a lot of header files. Each header file slows down
>> compilation, and standard library header files get included *a lot*. We may
>> not be able to avoid this, but we should think about the costs here.
>
>
> I created a .cpp file that includes all of the <*.h> headers and does
> nothing else (which should maximize the performance difference), and built
> it with and without this change. I could not measure any difference (the
> average compile time with this change was slightly lower, but that is
> almost certainly noise).
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [libcxx] r249475 - Remove unnecessary inline functions capturing the contents of C library macros.

[Richard Smith via cfe-commits]

On Tue, Oct 6, 2015 at 3:31 PM, Jonathan Roelofs 
wrote:

> On 10/6/15 4:03 PM, Richard Smith via cfe-commits wrote:
>
>> Author: rsmith
>> Date: Tue Oct  6 17:03:22 2015
>> New Revision: 249475
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=249475&view=rev
>> Log:
>> Remove unnecessary inline functions capturing the contents of C library
>> macros.
>>
>> The C standard requires that these be provided as functions even if
>> they're
>> also provided as macros, and a strict reading of the C++ standard library
>> rules
>> suggests that (for instance) &::isdigit == &::std::isdigit, so these
>> wrappers
>> are technically non-conforming.
>>
>
> Mind adding testcases to reinforce those identities, quoting the relevant
> bit(s) of the standard?


Sure, sounds like a good idea (but it's slightly tricky to test since
[global.functions]/2's footnote allows an implementation to declare
additional overloads of these functions).
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D11328: [clang-tidy] new "throw-by-value-catch-by-reference" check for clang-tidy

[Aaron Ballman via cfe-commits]

aaron.ballman added a comment.

On Wed, Oct 7, 2015 at 5:08 PM, Tobias Langner  
wrote:

> Hi,

> 

> I incorporated most of your changes. There are 2 open issues


Thank you for your work on this! There are a few minor nitpicks still, but is 
definitely moving forward nicely.

> - on one location I could not follow your advice, the compiler refused to 
> compile the code. I chose a different approach and hope you like it.

> - the second thing is this MaterializeTemporary advice that you gave. I don’t 
> fully understand it (possibly due to a lack of understanding the AST and a 
> lack of documentation of the proposed methods). Could you please flesh out 
> your idea and why you think it is necessary? Or at least give an example 
> where the current code does not work correctly.


Consider code like:

  struct S {};
  
  S& g();
  S h();
  
  void f() {
throw g(); // Should diagnose, does not currently
throw h(); // Should not diagnose, does not currently
  }

"throw g();" isn't throwing a temporary because it's throwing from an lvalue 
reference object (so the user may have a catch clause expecting the caught 
object to be the same lvalue reference as what g() returns, except that's not 
actually the case). If you instead check to see whether the throw expression 
requires a temporary to be materialized, you'll find that g() will diagnose, 
while h() still will not.

Does that help?



Comment at: clang-tidy/misc/ThrowByValueCatchByReferenceCheck.cpp:55
@@ +54,3 @@
+  auto *varDecl = dyn_cast(valueDecl);
+  return varDecl ? varDecl->isExceptionVariable() : false;
+}

How about:
```
if (const auto *VD = dyn_cast(declRefExpr->getDecl()))
  return VD->isExceptionVariable();
return false;
```


Comment at: clang-tidy/misc/ThrowByValueCatchByReferenceCheck.cpp:72
@@ +71,3 @@
+  if (qualType->isPointerType()) {
+// the code is throwing a pointer
+// in case it is strng literal, it is safe and we return

Sentence capitalization and punctuation in the comment.


Comment at: clang-tidy/misc/ThrowByValueCatchByReferenceCheck.cpp:77
@@ +76,3 @@
+  return;
+// if it's a variable from a catch statement, we return as well
+auto *declRef = dyn_cast(inner);

Sentence capitalization and punctuation in the comment.


Comment at: clang-tidy/misc/ThrowByValueCatchByReferenceCheck.cpp:94
@@ +93,3 @@
+  // encounter a DeclRefExpr or a CXXConstructExpr.
+  // if it's a DeclRefExpr, we emit a message if the referenced variable is not
+  // a catch variable or function parameter

Sentence capitalization and punctuation in the comment.


Comment at: clang-tidy/misc/ThrowByValueCatchByReferenceCheck.cpp:121
@@ +120,3 @@
+  auto *currentSubExpr = (*argIter)->IgnoreImpCasts();
+  // if the subexpr is now a DeclRefExpr, it's a real variable and we emit 
a
+  // diagnosis message.

Sentence capitalization and punctuation in the comment.


Comment at: clang-tidy/misc/ThrowByValueCatchByReferenceCheck.cpp:143
@@ +142,3 @@
+  auto *varDecl = catchStmt->getExceptionDecl();
+  if (caughtType->isPointerType()) {
+// We do not diagnose when catching pointer to strings since we also allow

I think this would be better as:
```
if (const auto *PT = caughtType.getCanonicalType()->getAs()) {
  if (...)
diag(...);
} else if (!caughtType->isReferenceType() && !caughtType.isTrivialType(context))
  diag(...);
```
As-written, it currently checks whether the caught type is a pointer, but not 
whether the canonical caught type is a pointer.


Comment at: test/clang-tidy/misc-throw-by-value-catch-by-reference.cpp:45
@@ +44,3 @@
+  
+  // can be enabled once std::move can be included
+  throw move(lvalue);  

This comment is outdated.


http://reviews.llvm.org/D11328



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

Re: [PATCH] D13357: [Concepts] Add diagnostic; specializations of variable and function concepts

[Richard Smith via cfe-commits]

rsmith added inline comments.


Comment at: lib/Sema/SemaDecl.cpp:5902-5915
@@ -5901,1 +5901,16 @@
+
+  // C++ Concepts TS [dcl.spec.concept]p7: A program shall not declare 
[...]
+  //  an explicit specialization, or a partial specialization of a concept
+  // definition
+  if (IsVariableTemplateSpecialization && !IsPartialSpecialization) {
+Diag(NewVD->getLocation(), diag::err_concept_decl_specialized)
+  << 0 << 1;
+NewVD->setInvalidDecl(true);
+  }
+
+  if (IsVariableTemplateSpecialization && IsPartialSpecialization) {
+Diag(NewVD->getLocation(), diag::err_concept_decl_specialized)
+  << 0 << 2;
+NewVD->setInvalidDecl(true);
+  }
 }

Maybe combine these into a single check, and use `<< (IsPartialSpecialization ? 
2 : 1)` for the specialization kind.


Comment at: lib/Sema/SemaDecl.cpp:7562
@@ -7546,1 +7561,3 @@
 if (isConcept) {
+  // This is a function concept
+  NewFD->setConcept(true);

Missing period.


Comment at: lib/Sema/SemaDecl.cpp:7888
@@ -7863,1 +7887,3 @@
+
+  if (NewFD->isInvalidDecl() && !NewFD->isConcept()) {
 HasExplicitTemplateArgs = false;

I'm not convinced this is going to work: if the declaration is invalid because 
it's declared `concept` /and/ for another reason, we presumably want to use the 
same error recovery path as before.


Comment at: test/CXX/concepts-ts/dcl.dcl/dcl.spec/dcl.concept/p7.cpp:1
@@ +1,2 @@
+// RUN:  %clang_cc1 -std=c++14 -fconcepts-ts -x c++ -verify %s
+

This file is in the wrong directory; the relevant section is 
[dcl.spec.concept], not [dcl.concept].


Comment at: test/CXX/concepts-ts/dcl.dcl/dcl.spec/dcl.concept/p7.cpp:4
@@ +3,3 @@
+template concept bool VCEI { true };
+template concept bool VCEI; // expected-error {{variable concept cannot 
be explicitly instantiated}}
+

hubert.reinterpretcast wrote:
> This is not a declaration (never mind definition) of a function template or 
> variable template, but of a specialization. Presumably, this violates 
> [dcl.spec.concept]p1. Perhaps a test for p7 should omit the concept 
> specifier? The same logic may apply to the explicit specialization cases.
Right, there are two different ways things can go wrong here:

1) `concept` is specified on a non-template (such as in an explicit 
specialization or explicit instantiation declaration) or a variable template 
partial specialization
2) an explicit or partial specialization or an explicit instantiation declares 
(*without* the `concept` keyword) a specialization of a template that was 
declared as a concept.

This patch is only checking for the former case, whereas the rule in 
[dcl.spec.concept]p7 says that the latter case is ill-formed. So these tests 
are at least in the wrong file. To check for violations of p7, you'll need to 
look at whether the template's pattern is marked as being a concept.

I'm also not sure where we say that a partial specialization of a variable 
template cannot be declared `concept`; that seems to be a defect in the TS 
wording.


http://reviews.llvm.org/D13357



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

Re: [PATCH] D13357: [Concepts] Add diagnostic; specializations of variable and function concepts

[Richard Smith via cfe-commits]

On Wed, Oct 7, 2015 at 2:59 PM, Richard Smith via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> rsmith added inline comments.
>
> 
> Comment at: lib/Sema/SemaDecl.cpp:5902-5915
> @@ -5901,1 +5901,16 @@
> +
> +  // C++ Concepts TS [dcl.spec.concept]p7: A program shall not
> declare [...]
> +  //  an explicit specialization, or a partial specialization of a
> concept
> +  // definition
> +  if (IsVariableTemplateSpecialization && !IsPartialSpecialization) {
> +Diag(NewVD->getLocation(), diag::err_concept_decl_specialized)
> +  << 0 << 1;
> +NewVD->setInvalidDecl(true);
> +  }
> +
> +  if (IsVariableTemplateSpecialization && IsPartialSpecialization) {
> +Diag(NewVD->getLocation(), diag::err_concept_decl_specialized)
> +  << 0 << 2;
> +NewVD->setInvalidDecl(true);
> +  }
>  }
> 
> Maybe combine these into a single check, and use `<<
> (IsPartialSpecialization ? 2 : 1)` for the specialization kind.
>
> 
> Comment at: lib/Sema/SemaDecl.cpp:7562
> @@ -7546,1 +7561,3 @@
>  if (isConcept) {
> +  // This is a function concept
> +  NewFD->setConcept(true);
> 
> Missing period.
>
> 
> Comment at: lib/Sema/SemaDecl.cpp:7888
> @@ -7863,1 +7887,3 @@
> +
> +  if (NewFD->isInvalidDecl() && !NewFD->isConcept()) {
>  HasExplicitTemplateArgs = false;
> 
> I'm not convinced this is going to work: if the declaration is invalid
> because it's declared `concept` /and/ for another reason, we presumably
> want to use the same error recovery path as before.
>
> 
> Comment at: test/CXX/concepts-ts/dcl.dcl/dcl.spec/dcl.concept/p7.cpp:1
> @@ +1,2 @@
> +// RUN:  %clang_cc1 -std=c++14 -fconcepts-ts -x c++ -verify %s
> +
> 
> This file is in the wrong directory; the relevant section is
> [dcl.spec.concept], not [dcl.concept].
>
> 
> Comment at: test/CXX/concepts-ts/dcl.dcl/dcl.spec/dcl.concept/p7.cpp:4
> @@ +3,3 @@
> +template concept bool VCEI { true };
> +template concept bool VCEI; // expected-error {{variable concept
> cannot be explicitly instantiated}}
> +
> 
> hubert.reinterpretcast wrote:
> > This is not a declaration (never mind definition) of a function template
> or variable template, but of a specialization. Presumably, this violates
> [dcl.spec.concept]p1. Perhaps a test for p7 should omit the concept
> specifier? The same logic may apply to the explicit specialization cases.
> Right, there are two different ways things can go wrong here:
>
> 1) `concept` is specified on a non-template (such as in an explicit
> specialization or explicit instantiation declaration) or a variable
> template partial specialization
> 2) an explicit or partial specialization or an explicit instantiation
> declares (*without* the `concept` keyword) a specialization of a template
> that was declared as a concept.
>
> This patch is only checking for the former case, whereas the rule in
> [dcl.spec.concept]p7 says that the latter case is ill-formed. So these
> tests are at least in the wrong file. To check for violations of p7, you'll
> need to look at whether the template's pattern is marked as being a concept.
>
> I'm also not sure where we say that a partial specialization of a variable
> template cannot be declared `concept`; that seems to be a defect in the TS
> wording.


Hah, I see Hubert also found this and filed a DR last week :)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D13440: [clang-tidy] Python script for easy check rename

[Piotr Zegar via cfe-commits]

ClockMan marked 5 inline comments as done.


Comment at: clang-tidy/rename_check.py:53
@@ +52,3 @@
+  return newFileName
+
+def getListOfFiles(clang_tidy_path):

Not sure what you mean...


Repository:
  rL LLVM

http://reviews.llvm.org/D13440



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

Re: [PATCH] D13440: [clang-tidy] Python script for easy check rename

[Piotr Zegar via cfe-commits]

ClockMan updated this revision to Diff 36801.
ClockMan added a comment.

Corrected review issues.


Repository:
  rL LLVM

http://reviews.llvm.org/D13440

Files:
  clang-tidy/rename_check.py

Index: clang-tidy/rename_check.py
===
--- /dev/null
+++ clang-tidy/rename_check.py
@@ -0,0 +1,93 @@
+#!/usr/bin/env python
+#
+#===- rename_check.py - clang-tidy check renamer -*- python 
-*--===#
+#
+# The LLVM Compiler Infrastructure
+#
+# This file is distributed under the University of Illinois Open Source
+# License. See LICENSE.TXT for details.
+#
+#======#
+
+import os
+import re
+import sys
+import glob
+
+def replaceInFile(fileName, sFrom, sTo):
+  if sFrom == sTo:
+return
+  txt = None
+  with open(fileName, "r") as f:
+txt = f.read()
+
+  if sFrom not in txt:
+return
+
+  txt = txt.replace(sFrom, sTo)
+  print("Replace '%s' -> '%s' in '%s'" % (sFrom, sTo, fileName))
+  with open(fileName, "w") as f:
+f.write(txt)
+
+def generateCommentLineHeader(filename):
+  return ''.join(['//===--- ',
+  os.path.basename(filename),
+  ' - clang-tidy ',
+  '-' * max(0, 42 - len(os.path.basename(filename))),
+  '*- C++ -*-===//'])
+
+def generateCommentLineSource(filename):
+  return ''.join(['//===--- ',
+  os.path.basename(filename),
+  ' - clang-tidy',
+  '-' * max(0, 52 - len(os.path.basename(filename))),
+  '-===//'])
+
+def fileRename(fileName, sFrom, sTo):
+  if sFrom not in fileName:
+return fileName
+  newFileName = fileName.replace(sFrom, sTo)
+  print("Rename '%s' -> '%s'" % (fileName, newFileName))
+  os.rename(fileName, newFileName)
+  return newFileName
+
+def getListOfFiles(clang_tidy_path):
+  files =  glob.glob(os.path.join(clang_tidy_path,'*'))
+  for dirname in files:
+if os.path.isdir(dirname):
+  files += glob.glob(os.path.join(dirname,'*'))
+  files += glob.glob(os.path.join(clang_tidy_path,'..', 'test', 'clang-tidy', 
'*'))
+  files += glob.glob(os.path.join(clang_tidy_path,'..', 'docs', 'clang-tidy', 
'checks', '*'))
+  return [filename for filename in files if os.path.isfile(filename)]
+
+def main():
+  if len(sys.argv) != 4:
+print('Usage: rename_check.py   
\n')
+print('   example: rename_check.py misc awesome-functions 
new-awesome-function')
+return
+
+  module = sys.argv[1].lower()
+  check_name = sys.argv[2]
+  check_name_camel = ''.join(map(lambda elem: elem.capitalize(),
+ check_name.split('-'))) + 'Check'
+  check_name_new = sys.argv[3]
+  check_name_new_camel = ''.join(map(lambda elem: elem.capitalize(),
+ check_name_new.split('-'))) + 'Check'
+
+  clang_tidy_path = os.path.dirname(sys.argv[0])
+
+  header_guard_old = module.upper() + '_' + check_name.upper().replace('-', 
'_')
+  header_guard_new = module.upper() + '_' + 
check_name_new.upper().replace('-', '_')
+
+  for filename in getListOfFiles(clang_tidy_path):
+originalName = filename
+filename = fileRename(filename, check_name, check_name_new)
+filename = fileRename(filename, check_name_camel, check_name_new_camel)
+replaceInFile(filename, generateCommentLineHeader(originalName), 
generateCommentLineHeader(filename))
+replaceInFile(filename, generateCommentLineSource(originalName), 
generateCommentLineSource(filename))
+replaceInFile(filename, header_guard_old, header_guard_new)
+replaceInFile(filename, check_name, check_name_new)
+replaceInFile(filename, check_name_camel, check_name_new_camel)
+
+if __name__ == '__main__':
+  main()


Index: clang-tidy/rename_check.py
===
--- /dev/null
+++ clang-tidy/rename_check.py
@@ -0,0 +1,93 @@
+#!/usr/bin/env python
+#
+#===- rename_check.py - clang-tidy check renamer -*- python -*--===#
+#
+# The LLVM Compiler Infrastructure
+#
+# This file is distributed under the University of Illinois Open Source
+# License. See LICENSE.TXT for details.
+#
+#======#
+
+import os
+import re
+import sys
+import glob
+
+def replaceInFile(fileName, sFrom, sTo):
+  if sFrom == sTo:
+return
+  txt = None
+  with open(fileName, "r") as f:
+txt = f.read()
+
+  if sFrom not in txt:
+return
+
+  txt = txt.replace(sFrom, sTo)
+  print("Replace '%s' -> '%s' in '%s'" % (sFrom, sTo, fileName))
+  with open(fileName, "w") as f:
+f.write(txt)
+
+def generateCommentLineHeader(filename):
+  return ''.join(['//===--- ',
+  os.path.basename(filename),
+  ' - clang-tidy ',
+  '-' * max(0, 42 - len(os.path.basename(filename))),
+  '*- C++ -*-===//'])
+
+d

Re: [PATCH] D13525: [CodeGen] Attach function attributes to functions created in CGBlocks.cpp.

[Akira Hatanaka via cfe-commits]

On Wed, Oct 7, 2015 at 1:48 PM, Duncan P. N. Exon Smith <
dexonsm...@apple.com> wrote:

>
> > On 2015-Oct-07, at 11:35, Akira Hatanaka  wrote:
> >
> > ahatanak created this revision.
> > ahatanak added reviewers: dexonsmith, echristo.
> > ahatanak added a subscriber: cfe-commits.
> >
> > This patch makes changes to attach function attributes to the following
> functions created in CGBlocks.cpp:
> >
> > __copy_helper_block_
> > __destroy_helper_block_
> > __Block_byref_object_copy_
> > __Block_byref_object_dispose_
> >
> > There are other places in clang where function attributes are not
> attached to functions. I plan to follow up with patches that fix those
> places too.
> >
> > http://reviews.llvm.org/D13525
> >
> > Files:
> >  lib/CodeGen/CGBlocks.cpp
> >  lib/CodeGen/CodeGenModule.cpp
> >  lib/CodeGen/TargetInfo.cpp
> >  test/CodeGenObjC/arc-blocks.m
> >
> > 
>
> > Index: lib/CodeGen/CodeGenModule.cpp
> > ===
> > --- lib/CodeGen/CodeGenModule.cpp
> > +++ lib/CodeGen/CodeGenModule.cpp
> > @@ -786,29 +786,32 @@
> >if (!hasUnwindExceptions(LangOpts))
> >  B.addAttribute(llvm::Attribute::NoUnwind);
> >
> > -  if (D->hasAttr()) {
> > -// Naked implies noinline: we should not be inlining such functions.
> > -B.addAttribute(llvm::Attribute::Naked);
> > -B.addAttribute(llvm::Attribute::NoInline);
> > -  } else if (D->hasAttr()) {
> > -B.addAttribute(llvm::Attribute::NoDuplicate);
> > -  } else if (D->hasAttr()) {
> > -B.addAttribute(llvm::Attribute::NoInline);
> > -  } else if (D->hasAttr() &&
> > -
>  !F->getAttributes().hasAttribute(llvm::AttributeSet::FunctionIndex,
> > -
> llvm::Attribute::NoInline)) {
> > -// (noinline wins over always_inline, and we can't specify both in
> IR)
> > -B.addAttribute(llvm::Attribute::AlwaysInline);
> > -  }
> > -
> > -  if (D->hasAttr()) {
> > -if (!D->hasAttr())
> > -  B.addAttribute(llvm::Attribute::OptimizeForSize);
> > -B.addAttribute(llvm::Attribute::Cold);
> > -  }
> > -
> > -  if (D->hasAttr())
> > -B.addAttribute(llvm::Attribute::MinSize);
> > +  if (D) {
>
> I wonder, is there a way to reorder this function so that you can use an
> early return here?  The extra indentation (and resulting noise) is
> unfortunate.
>
>
I think we would need to call F->addAttributes() in two places (but not
twice) if we want to do early return. Would that look better?


> Really, the way attributes are added should be refactored.  I spent a few
> days trying to do that in the spring but (obviously) never got to
> anything committable.  If you have any ideas for ways to clean this up
> while you're in here, please "fix" it.
>
>
>From an efficiency point of view, I think we should stop calling
Function::addFnAttr and removeFnAttr to add or remove a single attribute.
Instead, AttrBuilder should be used and Function::addAttributes should be
called only when we the builder has a complete set of attributes. Also, I
think we need a member function of Function or AttributeSet that replaces
the attribute set at a certain index.


> Anyway, LGTM after a prep NFC commit to avoid the extra indentation.
>
> > +if (D->hasAttr()) {
> > +  // Naked implies noinline: we should not be inlining such
> functions.
> > +  B.addAttribute(llvm::Attribute::Naked);
> > +  B.addAttribute(llvm::Attribute::NoInline);
> > +} else if (D->hasAttr()) {
> > +  B.addAttribute(llvm::Attribute::NoDuplicate);
> > +} else if (D->hasAttr()) {
> > +  B.addAttribute(llvm::Attribute::NoInline);
> > +} else if (D->hasAttr() &&
> > +   !F->getAttributes().hasAttribute(
> > +   llvm::AttributeSet::FunctionIndex,
> > +   llvm::Attribute::NoInline)) {
> > +  // (noinline wins over always_inline, and we can't specify both
> in IR)
> > +  B.addAttribute(llvm::Attribute::AlwaysInline);
> > +}
> > +
> > +if (D->hasAttr()) {
> > +  if (!D->hasAttr())
> > +B.addAttribute(llvm::Attribute::OptimizeForSize);
> > +  B.addAttribute(llvm::Attribute::Cold);
> > +}
> > +
> > +if (D->hasAttr())
> > +  B.addAttribute(llvm::Attribute::MinSize);
> > +  }
> >
> >if (LangOpts.getStackProtector() == LangOptions::SSPOn)
> >  B.addAttribute(llvm::Attribute::StackProtect);
> > @@ -821,6 +824,9 @@
> > llvm::AttributeSet::get(
> > F->getContext(),
> llvm::AttributeSet::FunctionIndex, B));
> >
> > +  if (!D)
> > +return;
> > +
> >if (D->hasAttr()) {
> >  // OptimizeNone implies noinline; we should not be inlining such
> functions.
> >  F->addFnAttr(llvm::Attribute::OptimizeNone);
> > @@ -859,12 +865,12 @@
> >
> >  void CodeGenModule::SetCommonAttributes(const Decl *D,
> >  llvm::GlobalValue *GV) {
> > -  if (const auto *ND = dyn_cast(D))
> > +  if (const auto *ND = dyn_cast_or_null(D))
> >  setGlobalVisibility(GV, 

Re: [PATCH] D13304: Avoid inlining in throw statement

[Jun Bum Lim via cfe-commits]

junbuml added a comment.

Thanks Richard for the comment.

Initially, I intended to implement this in inliner by checking if a callsite is 
in exception handling regions. However, I decided not to implement this in 
inliner because this kind of check should be performed for all callsites if we 
implement it in inliner.

Instead of directly adding complexity in inliner, I implemented this in 
PruneEH.cpp (http://reviews.llvm.org/D12979) because this is very specific to 
exception handling regions. In this patch, I tried to mark all callsites 
invoked from throw statements as cold (noinline) by looking up users of 
__cxa_throw() and __cxa_allocate_exception(). We had many discussions and 
finally ended up to implement the same thing in clang to be more general and 
simpler as Hal suggested in http://reviews.llvm.org/D12979.

As you point out, it should be done by influencing inline cost heurisic, so I 
believe Attribute::Cold is the right attribute to be added here. However, as I 
FIXMEed, the current ColdThreshold is not tuned yet (r200898). So for now, I 
add both cold and noinline.

Regarding code size, I believe not inlining contractor calls in throw 
statements could be potentially more helpful for code size in many cases. If 
inlining very small callsites in throw statements could be issue, then we may 
be able to  check if callee is smaller than some threshold to avoid adding the 
attributes (cold and noinline).


http://reviews.llvm.org/D13304



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

Re: [PATCH] D13344: Keep the IfStmt node even if the condition is invalid

[Richard Smith via cfe-commits]

rsmith added inline comments.


Comment at: lib/Sema/SemaStmt.cpp:508-509
@@ -513,6 +507,4 @@
 
-  DiagnoseUnusedExprResult(elseStmt);
-
   return new (Context) IfStmt(Context, IfLoc, ConditionVar, ConditionExpr,
   thenStmt, ElseLoc, elseStmt);
 }

This will create an `IfStmt` with no `ConditionExpr`. That is not a valid AST 
construct, and it's not reasonable to expect all the downstream consumers of 
the AST to be able to cope with it. It's not hard to find parts of the codebase 
that will crash when presented with this:

  lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp:1133
  lib/Analysis/Consumed.cpp:1265
  lib/Sema/AnalysisBasedWarnings.cpp:740

... and so on.

As the comment above suggests, you could create some sort of placeholder 
expression node here instead (either use something like an `OpaqueValueExpr`, 
or add a new Expr subclass to represent an erroneous expression -- the latter 
would probably be useful in many cases where we hit parse errors but can still 
produce some useful information to tools).


http://reviews.llvm.org/D13344



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

Re: [PATCH] D12901: [Static Analyzer] Assertion "System is over constrained" after truncating 64 bits integers to 32 bits. (PR25078)

[Anna Zaks via cfe-commits]

zaks.anna added a comment.

I agree with Gabor. We should investigate how we can model the overflow on a 
cast correctly.


http://reviews.llvm.org/D12901



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

Re: [PATCH] D13304: Avoid inlining in throw statement

[Richard Smith via cfe-commits]

On Wed, Oct 7, 2015 at 3:10 PM, Jun Bum Lim via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> junbuml added a comment.
>
> Thanks Richard for the comment.
>
> Initially, I intended to implement this in inliner by checking if a
> callsite is in exception handling regions. However, I decided not to
> implement this in inliner because this kind of check should be performed
> for all callsites if we implement it in inliner.
>
> Instead of directly adding complexity in inliner, I implemented this in
> PruneEH.cpp (http://reviews.llvm.org/D12979) because this is very
> specific to exception handling regions. In this patch, I tried to mark all
> callsites invoked from throw statements as cold (noinline) by looking up
> users of __cxa_throw() and __cxa_allocate_exception(). We had many
> discussions and finally ended up to implement the same thing in clang to be
> more general and simpler as Hal suggested in
> http://reviews.llvm.org/D12979.
>

I think this actually makes it less general. You would presumably perform
different inlining for:

  throw f(x, y);

versus

  auto k = f(x, y);
  throw k;

which doesn't really seem defensible.


> As you point out, it should be done by influencing inline cost heurisic,
> so I believe Attribute::Cold is the right attribute to be added here.
> However, as I FIXMEed, the current ColdThreshold is not tuned yet
> (r200898). So for now, I add both cold and noinline.
>
> Regarding code size, I believe not inlining contractor calls in throw
> statements could be potentially more helpful for code size in many cases.
> If inlining very small callsites in throw statements could be issue, then
> we may be able to  check if callee is smaller than some threshold to avoid
> adding the attributes (cold and noinline).


That's not something we can really do from the frontend, because small
constructors often don't look small until they themselves have been
optimized.

Would it be sufficient to mark just the invocation of __cxa_throw (or its
equivalent) as cold? If the optimizer can't infer from that that the code
leading up to the call is also cold, we should fix that in the LLVM-side
analysis rather than working around it in the frontend.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D13453: Always generate cmake config files

[NAKAMURA Takumi via cfe-commits]

chapuni added a reviewer: jordan_rose.
chapuni added a comment.

LGTM, but I haven't tested. I'll wait for 3rd opinion for several hours. :)

I thought it may be moved to an arbitrary subdirectory.


http://reviews.llvm.org/D13453



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

Re: [PATCH] D13453: Always generate cmake config files

[Jordan Rose via cfe-commits]

jordan_rose resigned from this revision.
jordan_rose edited reviewers, added: beanz; removed: jordan_rose.
jordan_rose added a comment.

I just get things to build with CMake, and I'm not involved with Clang so much 
these days. This is more about controlling what gets installed, which should be 
covered by someone else. ChrisB is spearheading the effort to ditch autoconf, 
so adding him instead.


http://reviews.llvm.org/D13453



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

Re: [PATCH] D13453: Always generate cmake config files

[Chris Bieneman via cfe-commits]

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

LGTM.


http://reviews.llvm.org/D13453



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

Re: [PATCH] D13453: Always generate cmake config files

[NAKAMURA Takumi via cfe-commits]

chapuni added a comment.

I am testing this with cmake-2.8.12. I remember some issues would be there.


http://reviews.llvm.org/D13453



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

Re: [PATCH] D9888: [OPENMP] Driver support for OpenMP offloading

[Artem Belevich via cfe-commits]

tra added a comment.

In http://reviews.llvm.org/D9888#257904, @sfantao wrote:

> This diff refactors the original patch and is rebased on top of the latests 
> offloading changes inserted for CUDA.
>
> Here I don't touch the CUDA support. I tried, however, to have the 
> implementation modular enough so that it could eventually be combined with 
> the CUDA implementation. In my view OpenMP offloading is more general in the 
> sense that it does not refer to a given tool chain, instead it uses existing 
> toolchains to generate code for offloading devices. So, I believe that a tool 
> chain (which I did not include in this patch) targeting NVPTX will be able to 
> handle both CUDA and OpenMP offloading models.


What do you mean by "does not refer to a given toolchain"? Do you have the 
toolchain patch available?

Creating a separate toolchain for CUDA was a crutch that was available to craft 
appropriate cc1 command line for device-side compilation using existing 
toolchain. It works, but it's rather rigid arrangement. Creating a NVPTX 
toolchain which can be parameterized to produce CUDA or OpenMP would be an 
improvement.

Ideally toolchain tweaking should probably be done outside of the toolchain 
itself so that it can be used with any combination of {CUDA or OpenMP target 
tweaks}x{toolchains capable of generating target code}.

> b ) The building of the driver actions is unchanged.

> 

> I don't create device specific actions. Instead only the bundling/unbundling 
> are inserted as first or last action if the file type requires that.


Could you elaborate on that? The way I read it, the driver sees linear chain of 
compilation steps plus bundling/unbundling at the beginning/end and that each 
action would result in multiple compiler invocations, presumably per target.

If that's the case, then it may present a bit of a challenge in case one part 
of compilation depends on results of another. That's the case for CUDA where 
results of device-side compilation must be present for host-side compilation so 
we can generate additional code to initialize it at runtime.

> c) Add offloading kind to `ToolChain`

> 

> Offloading does not require a new toolchain to be created. Existent 
> toolchains are used and the offloading kind is used to drive specific 
> behavior in each toolchain so that valid device code is generated.

> 

> This is a major difference from what is currently done for CUDA. But I guess 
> the CUDA implementation easily fits this design and the Nvidia GPU toolchain 
> could be reused for both CUDA and OpenMP offloading.


Sounds good. I'd be happy to make necessary make CUDA support use it.

> d) Use Job results cache to easily use host results in device actions and 
> vice-versa.

> 

> An array of the results for each job is kept so that the device job can use 
> the result previously generated for the host and used it as input or 
> vice-versa.


Nice. That's something that will be handy for CUDA and may help to avoid 
passing bits of info about other jobs explicitly throughout the driver.

> The result cache can also be updated to keep the required information for the 
> CUDA implementation to decide host/device binaries combining  (injection is 
> the term used in the code). I don't have a concrete proposal for that 
> however, given that is not clear to me what are the plans for CUDA to support 
> separate compilation, I understand that the CUDA binary is inserted directly 
> in host IR (Art, can you shed some light on this?).


Currently CUDA depends on libcudart which assumes that GPU code and its 
initialization is done the way nvcc does it. Currently we do include PTX 
assembly (as in readable text)  generated on device side into host-side IR 
*and* generate some host data structures and init code to register GPU binaries 
with libcudart. I haven't figured out a way to compile host/device sides of 
CUDA without a host-side compilation depending on device results.

Long-term we're considering implementing CUDA runtime support based on plain 
driver interface which would give us more control over where we keep GPU code 
and how we initialize it. Then we could simplify things and, for example, 
incorporate GPU code via linker script.  Alas for the time being we're stuck 
with libcudart and sequential device and host compilation phases.

As for separate compilation -- compilation part is doable. It's using the 
results of such compilation that becomes tricky. CUDA's triple-bracket kernel 
launch syntax depends on libcudart and will not work, because we would not 
generate init code. You can still launch kernels manually using raw driver API, 
but it's quite a bit more convoluted.

--Artem



Comment at: include/clang/Driver/Driver.h:208
@@ +207,3 @@
+  /// CreateUnbundledOffloadingResult - Create a command to unbundle the input
+  /// and use the resulting input info. If there re inputs already cached in
+  /// OffloadingHostResults for that action use them instead.

r249639 - Make clang_Cursor_getMangling not mangle if the declaration isn't mangled

[Ehsan Akhgari via cfe-commits]

Author: ehsan
Date: Wed Oct  7 19:01:20 2015
New Revision: 249639

URL: http://llvm.org/viewvc/llvm-project?rev=249639&view=rev
Log:
Make clang_Cursor_getMangling not mangle if the declaration isn't mangled

Right now clang_Cursor_getMangling will attempt to mangle any
declaration, even if the declaration isn't mangled (extern C).  This
results in a partially mangled name which isn't useful for much. This
patch makes clang_Cursor_getMangling return an empty string if the
declaration isn't mangled.

Patch by Michael Wu .

Modified:
cfe/trunk/test/Index/print-mangled-name.cpp
cfe/trunk/tools/c-index-test/c-index-test.c
cfe/trunk/tools/libclang/CIndex.cpp

Modified: cfe/trunk/test/Index/print-mangled-name.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/print-mangled-name.cpp?rev=249639&r1=249638&r2=249639&view=diff
==
--- cfe/trunk/test/Index/print-mangled-name.cpp (original)
+++ cfe/trunk/test/Index/print-mangled-name.cpp Wed Oct  7 19:01:20 2015
@@ -29,3 +29,8 @@ int foo(S, S&);
 // ITANIUM: mangled=_Z3foo1SRS_
 // MACHO: mangled=__Z3foo1SRS_
 // MICROSOFT: mangled=?foo@@YAHUS
+
+extern "C" int foo(int);
+// ITANIUM: mangled=foo
+// MACHO: mangled=_foo
+// MICROSOFT: mangled=_foo

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=249639&r1=249638&r2=249639&view=diff
==
--- cfe/trunk/tools/c-index-test/c-index-test.c (original)
+++ cfe/trunk/tools/c-index-test/c-index-test.c Wed Oct  7 19:01:20 2015
@@ -1429,6 +1429,8 @@ static enum CXChildVisitResult PrintType
 
 static enum CXChildVisitResult PrintMangledName(CXCursor cursor, CXCursor p,
 CXClientData d) {
+  if (clang_isUnexposed(clang_getCursorKind(cursor)))
+return CXChildVisit_Recurse;
   CXString MangledName;
   PrintCursor(cursor, NULL);
   MangledName = clang_Cursor_getMangling(cursor);

Modified: cfe/trunk/tools/libclang/CIndex.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/libclang/CIndex.cpp?rev=249639&r1=249638&r2=249639&view=diff
==
--- cfe/trunk/tools/libclang/CIndex.cpp (original)
+++ cfe/trunk/tools/libclang/CIndex.cpp Wed Oct  7 19:01:20 2015
@@ -3890,7 +3890,11 @@ CXString clang_Cursor_getMangling(CXCurs
 
   std::string FrontendBuf;
   llvm::raw_string_ostream FrontendBufOS(FrontendBuf);
-  MC->mangleName(ND, FrontendBufOS);
+  if (MC->shouldMangleDeclName(ND)) {
+MC->mangleName(ND, FrontendBufOS);
+  } else {
+ND->printName(FrontendBufOS);
+  }
 
   // Now apply backend mangling.
   std::unique_ptr DL(


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