Re: [PATCH] D16797: Update clang support on recent Haiku

2016-05-11 Thread Jérôme Duval via cfe-commits
korli added a comment.

Ping?


Repository:
  rL LLVM

http://reviews.llvm.org/D16797



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


[PATCH] D20150: clang-rename: fix renaming of field with implicit initializers

2016-05-11 Thread Miklos Vajna via cfe-commits
vmiklos created this revision.
vmiklos added reviewers: cfe-commits, klimek.

The last check failed as Cla::Cla() was rewritten to Cla::hector().

http://reviews.llvm.org/D20150

Files:
  clang-rename/USRLocFinder.cpp
  test/clang-rename/CtorInitializerTest.cpp

Index: test/clang-rename/CtorInitializerTest.cpp
===
--- /dev/null
+++ test/clang-rename/CtorInitializerTest.cpp
@@ -0,0 +1,20 @@
+// RUN: cat %s > %t.cpp
+// RUN: clang-rename -offset=162 -new-name=hector %t.cpp -i --
+// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
+class A
+{
+};
+
+class Cla
+{
+  A foo; // CHECK: hector;
+public:
+  Cla();
+};
+
+Cla::Cla() // CHECK: Cla::Cla()
+{
+}
+
+// Use grep -FUbo 'foo'  to get the correct offset of foo when changing
+// this file.
Index: clang-rename/USRLocFinder.cpp
===
--- clang-rename/USRLocFinder.cpp
+++ clang-rename/USRLocFinder.cpp
@@ -60,6 +60,11 @@
   bool VisitCXXConstructorDecl(clang::CXXConstructorDecl *ConstructorDecl) {
 for (clang::CXXConstructorDecl::init_const_iterator it = 
ConstructorDecl->init_begin(); it != ConstructorDecl->init_end(); ++it) {
   const clang::CXXCtorInitializer* Initializer = *it;
+  if (Initializer->getSourceOrder() == -1) {
+// Ignore implicit initializers.
+continue;
+  }
+
   if (const clang::FieldDecl *FieldDecl = Initializer->getAnyMember()) {
 if (getUSRForDecl(FieldDecl) == USR) {
   // The initializer refers to a field that is to be renamed.


Index: test/clang-rename/CtorInitializerTest.cpp
===
--- /dev/null
+++ test/clang-rename/CtorInitializerTest.cpp
@@ -0,0 +1,20 @@
+// RUN: cat %s > %t.cpp
+// RUN: clang-rename -offset=162 -new-name=hector %t.cpp -i --
+// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
+class A
+{
+};
+
+class Cla
+{
+  A foo; // CHECK: hector;
+public:
+  Cla();
+};
+
+Cla::Cla() // CHECK: Cla::Cla()
+{
+}
+
+// Use grep -FUbo 'foo'  to get the correct offset of foo when changing
+// this file.
Index: clang-rename/USRLocFinder.cpp
===
--- clang-rename/USRLocFinder.cpp
+++ clang-rename/USRLocFinder.cpp
@@ -60,6 +60,11 @@
   bool VisitCXXConstructorDecl(clang::CXXConstructorDecl *ConstructorDecl) {
 for (clang::CXXConstructorDecl::init_const_iterator it = ConstructorDecl->init_begin(); it != ConstructorDecl->init_end(); ++it) {
   const clang::CXXCtorInitializer* Initializer = *it;
+  if (Initializer->getSourceOrder() == -1) {
+// Ignore implicit initializers.
+continue;
+  }
+
   if (const clang::FieldDecl *FieldDecl = Initializer->getAnyMember()) {
 if (getUSRForDecl(FieldDecl) == USR) {
   // The initializer refers to a field that is to be renamed.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D20150: clang-rename: fix renaming of field with implicit initializers

2016-05-11 Thread Manuel Klimek via cfe-commits
klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.

LG



Comment at: clang-rename/USRLocFinder.cpp:64
@@ +63,3 @@
+  if (Initializer->getSourceOrder() == -1) {
+// Ignore implicit initializers.
+continue;

Add a comment like:
// The source location of implicit initializers is the constructor declaration.

Also: should we add a check that the token of the source location we find 
actually has the old name?


http://reviews.llvm.org/D20150



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


Re: [PATCH] D20094: [include-fixer] Add basic documentation.

2016-05-11 Thread Benjamin Kramer via cfe-commits
bkramer marked 2 inline comments as done.


Comment at: docs/include-fixer.rst:48
@@ +47,3 @@
+... wait as clang indexes the code base ...
+  $ ln -s $PWD/find_all_symbols_db.yaml path/to/llvm/source/ # Link database 
into the source tree.
+  $ cd path/to/llvm/source

hokein wrote:
> We should also create a link to `compile_commands.json` in 
> `path/to/llvm/source/`.
If the user followed the instructions at 
http://clang.llvm.org/docs/HowToSetupToolingForLLVM.html the link will be there 
already. I'll add it here with a clarifying comment.


http://reviews.llvm.org/D20094



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


Re: [PATCH] D20094: [include-fixer] Add basic documentation.

2016-05-11 Thread Benjamin Kramer via cfe-commits
bkramer updated this revision to Diff 56856.
bkramer added a comment.

Address review comments.


http://reviews.llvm.org/D20094

Files:
  docs/include-fixer.rst
  docs/index.rst

Index: docs/index.rst
===
--- docs/index.rst
+++ docs/index.rst
@@ -21,6 +21,7 @@
:maxdepth: 2
 
clang-tidy/index
+   include-fixer
modularize
pp-trace
 
Index: docs/include-fixer.rst
===
--- /dev/null
+++ docs/include-fixer.rst
@@ -0,0 +1,71 @@
+===
+Clang-Include-Fixer
+===
+
+.. contents::
+
+One of the major nuisances of C++ compared to other languages is the manual
+management of ``#include`` directives in any file.
+:program:`clang-include-fixer` addresses one aspect of this problem by 
providing
+an automated way of adding ``#include`` directives for missing symbols in one
+translation unit.
+
+Setup
+=
+
+To use :program:`clang-include-fixer` two databases are required, both can be
+generated with existing tools.
+
+- Compilation database. Contains the compiler commands for any given file in a
+  project and can be generated by CMake, see `How To Setup Tooling For LLVM`_.
+- Xrefs database. Contains all symbol information in a project to match a given
+  identifier to a header file.
+
+Ideally both databases (``compile_commands.json`` and
+``find_all_symbols_db.yaml``) are linked into the root of the source tree they
+correspond to. Then the :program:`clang-include-fixer` can automatically pick
+them up if called with a source file from that tree. Note that by default
+``compile_commands.json`` as generated by CMake does not include header files,
+so only implementation files can be handled by tools.
+
+.. _How To Setup Tooling For LLVM: 
http://clang.llvm.org/docs/HowToSetupToolingForLLVM.html
+
+Creating an Xrefs Database From a Compilation Database
+--
+
+The include fixer contains :program:`find-all-symbols`, a tool to create a
+symbol database in YAML format from a compilation database by parsing all
+source files listed in it. The following list of commands shows how to set up a
+database for LLVM, any project built by CMake should follow similar steps.
+
+.. code-block:: console
+
+  $ cd path/to/llvm-build
+  $ ls compile_commands.json # Make sure compile_commands.json exists.
+compile_commands.json
+  $ 
path/to/llvm/source/tools/clang/tools/extra/include-fixer/find-all-symbols/tool/run-find-all-symbols.py
+... wait as clang indexes the code base ...
+  $ ln -s $PWD/find_all_symbols_db.yaml path/to/llvm/source/ # Link database 
into the source tree.
+  $ ln -s $PWD/compile_commands.json path/to/llvm/source/ # Also link 
compilation database if it's not there already.
+  $ cd path/to/llvm/source
+  $ clang-include-fixer -db=yaml path/to/file/with/missing/include.cpp
+Added #include "foo.h"
+
+How it Works
+
+
+To get the most information out of clang at parse time,
+:program:`clang-include-fixer` runs in tandem with the parse and receives
+callbacks from Clang's semantic analysis. In particular it reuses the existing
+support for typo corrections. Whenever Clang tries to correct a potential typo
+it emits a callback to the include fixer which then looks for a corresponding
+file. At this point rich lookup information is still available, which is not
+available in the AST at a later stage.
+
+The identifier that should be typo corrected is then sent to the database, if a
+header file is returned it is added as an include directive at the top of the
+file.
+
+Currently :program:`clang-include-fixer` only inserts a single include at a
+time to avoid getting caught in follow-up errors. If multiple `#include`
+additions are desired the program can be rerun until a fix-point is reached.


Index: docs/index.rst
===
--- docs/index.rst
+++ docs/index.rst
@@ -21,6 +21,7 @@
:maxdepth: 2
 
clang-tidy/index
+   include-fixer
modularize
pp-trace
 
Index: docs/include-fixer.rst
===
--- /dev/null
+++ docs/include-fixer.rst
@@ -0,0 +1,71 @@
+===
+Clang-Include-Fixer
+===
+
+.. contents::
+
+One of the major nuisances of C++ compared to other languages is the manual
+management of ``#include`` directives in any file.
+:program:`clang-include-fixer` addresses one aspect of this problem by providing
+an automated way of adding ``#include`` directives for missing symbols in one
+translation unit.
+
+Setup
+=
+
+To use :program:`clang-include-fixer` two databases are required, both can be
+generated with existing tools.
+
+- Compilation database. Contains the compiler commands for any given file in a
+  project and can be generated by CMake, see `How To Setup Tooling For LLVM`_.
+- Xrefs database. Contains a

[clang-tools-extra] r269161 - clang-rename: fix renaming of field with implicit initializers

2016-05-11 Thread Miklos Vajna via cfe-commits
Author: vmiklos
Date: Wed May 11 03:08:07 2016
New Revision: 269161

URL: http://llvm.org/viewvc/llvm-project?rev=269161&view=rev
Log:
clang-rename: fix renaming of field with implicit initializers

The last check failed as Cla::Cla() was rewritten to Cla::hector().

Reviewers: cfe-commits, klimek

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

Added:
clang-tools-extra/trunk/test/clang-rename/CtorInitializerTest.cpp
Modified:
clang-tools-extra/trunk/clang-rename/USRLocFinder.cpp

Modified: clang-tools-extra/trunk/clang-rename/USRLocFinder.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-rename/USRLocFinder.cpp?rev=269161&r1=269160&r2=269161&view=diff
==
--- clang-tools-extra/trunk/clang-rename/USRLocFinder.cpp (original)
+++ clang-tools-extra/trunk/clang-rename/USRLocFinder.cpp Wed May 11 03:08:07 
2016
@@ -60,6 +60,11 @@ public:
   bool VisitCXXConstructorDecl(clang::CXXConstructorDecl *ConstructorDecl) {
 for (clang::CXXConstructorDecl::init_const_iterator it = 
ConstructorDecl->init_begin(); it != ConstructorDecl->init_end(); ++it) {
   const clang::CXXCtorInitializer* Initializer = *it;
+  if (Initializer->getSourceOrder() == -1) {
+// Ignore implicit initializers.
+continue;
+  }
+
   if (const clang::FieldDecl *FieldDecl = Initializer->getAnyMember()) {
 if (getUSRForDecl(FieldDecl) == USR) {
   // The initializer refers to a field that is to be renamed.

Added: clang-tools-extra/trunk/test/clang-rename/CtorInitializerTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-rename/CtorInitializerTest.cpp?rev=269161&view=auto
==
--- clang-tools-extra/trunk/test/clang-rename/CtorInitializerTest.cpp (added)
+++ clang-tools-extra/trunk/test/clang-rename/CtorInitializerTest.cpp Wed May 
11 03:08:07 2016
@@ -0,0 +1,20 @@
+// RUN: cat %s > %t.cpp
+// RUN: clang-rename -offset=162 -new-name=hector %t.cpp -i --
+// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
+class A
+{
+};
+
+class Cla
+{
+  A foo; // CHECK: hector;
+public:
+  Cla();
+};
+
+Cla::Cla() // CHECK: Cla::Cla()
+{
+}
+
+// Use grep -FUbo 'foo'  to get the correct offset of foo when changing
+// this file.


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


Re: [PATCH] D20150: clang-rename: fix renaming of field with implicit initializers

2016-05-11 Thread Miklos Vajna via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL269161: clang-rename: fix renaming of field with implicit 
initializers (authored by vmiklos).

Changed prior to commit:
  http://reviews.llvm.org/D20150?vs=56854&id=56859#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D20150

Files:
  clang-tools-extra/trunk/clang-rename/USRLocFinder.cpp
  clang-tools-extra/trunk/test/clang-rename/CtorInitializerTest.cpp

Index: clang-tools-extra/trunk/clang-rename/USRLocFinder.cpp
===
--- clang-tools-extra/trunk/clang-rename/USRLocFinder.cpp
+++ clang-tools-extra/trunk/clang-rename/USRLocFinder.cpp
@@ -60,6 +60,11 @@
   bool VisitCXXConstructorDecl(clang::CXXConstructorDecl *ConstructorDecl) {
 for (clang::CXXConstructorDecl::init_const_iterator it = 
ConstructorDecl->init_begin(); it != ConstructorDecl->init_end(); ++it) {
   const clang::CXXCtorInitializer* Initializer = *it;
+  if (Initializer->getSourceOrder() == -1) {
+// Ignore implicit initializers.
+continue;
+  }
+
   if (const clang::FieldDecl *FieldDecl = Initializer->getAnyMember()) {
 if (getUSRForDecl(FieldDecl) == USR) {
   // The initializer refers to a field that is to be renamed.
Index: clang-tools-extra/trunk/test/clang-rename/CtorInitializerTest.cpp
===
--- clang-tools-extra/trunk/test/clang-rename/CtorInitializerTest.cpp
+++ clang-tools-extra/trunk/test/clang-rename/CtorInitializerTest.cpp
@@ -0,0 +1,20 @@
+// RUN: cat %s > %t.cpp
+// RUN: clang-rename -offset=162 -new-name=hector %t.cpp -i --
+// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
+class A
+{
+};
+
+class Cla
+{
+  A foo; // CHECK: hector;
+public:
+  Cla();
+};
+
+Cla::Cla() // CHECK: Cla::Cla()
+{
+}
+
+// Use grep -FUbo 'foo'  to get the correct offset of foo when changing
+// this file.


Index: clang-tools-extra/trunk/clang-rename/USRLocFinder.cpp
===
--- clang-tools-extra/trunk/clang-rename/USRLocFinder.cpp
+++ clang-tools-extra/trunk/clang-rename/USRLocFinder.cpp
@@ -60,6 +60,11 @@
   bool VisitCXXConstructorDecl(clang::CXXConstructorDecl *ConstructorDecl) {
 for (clang::CXXConstructorDecl::init_const_iterator it = ConstructorDecl->init_begin(); it != ConstructorDecl->init_end(); ++it) {
   const clang::CXXCtorInitializer* Initializer = *it;
+  if (Initializer->getSourceOrder() == -1) {
+// Ignore implicit initializers.
+continue;
+  }
+
   if (const clang::FieldDecl *FieldDecl = Initializer->getAnyMember()) {
 if (getUSRForDecl(FieldDecl) == USR) {
   // The initializer refers to a field that is to be renamed.
Index: clang-tools-extra/trunk/test/clang-rename/CtorInitializerTest.cpp
===
--- clang-tools-extra/trunk/test/clang-rename/CtorInitializerTest.cpp
+++ clang-tools-extra/trunk/test/clang-rename/CtorInitializerTest.cpp
@@ -0,0 +1,20 @@
+// RUN: cat %s > %t.cpp
+// RUN: clang-rename -offset=162 -new-name=hector %t.cpp -i --
+// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
+class A
+{
+};
+
+class Cla
+{
+  A foo; // CHECK: hector;
+public:
+  Cla();
+};
+
+Cla::Cla() // CHECK: Cla::Cla()
+{
+}
+
+// Use grep -FUbo 'foo'  to get the correct offset of foo when changing
+// this file.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D20150: clang-rename: fix renaming of field with implicit initializers

2016-05-11 Thread Miklos Vajna via cfe-commits
vmiklos added inline comments.


Comment at: clang-rename/USRLocFinder.cpp:64
@@ +63,3 @@
+  if (Initializer->getSourceOrder() == -1) {
+// Ignore implicit initializers.
+continue;

klimek wrote:
> Add a comment like:
> // The source location of implicit initializers is the constructor 
> declaration.
> 
> Also: should we add a check that the token of the source location we find 
> actually has the old name?
I don't know off the top of my head a situation where this is needed, but sure, 
sounds like a useful safety check. I'll have a look.


Repository:
  rL LLVM

http://reviews.llvm.org/D20150



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


Re: [PATCH] D19941: [tooling] FixItHint Tooling refactoring

2016-05-11 Thread Alexander Kornienko via cfe-commits
alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.

LG. Thanks!



Comment at: unittests/Tooling/FixItTest.cpp:37
@@ +36,3 @@
+EXPECT_EQ("foo(x, y)",
+  tooling::fixit::getText(CE->getSourceRange(), *Context));
+

Let's either put the tests in the `clang::tooling::` namespace (not 
common for unit tests in this specific directory, but nevertheless reasonable, 
imo) or add using declarations for `tooling::fixit::getText` and other tested 
functions to reduce the amount of boilerplate.


http://reviews.llvm.org/D19941



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


Re: [PATCH] D19703: [clang-tidy] Improve misc-redundant-expression and decrease false-positive

2016-05-11 Thread Alexander Kornienko via cfe-commits
alexfh requested changes to this revision.
This revision now requires changes to proceed.


Comment at: clang-tidy/misc/RedundantExpressionCheck.cpp:27
@@ +26,3 @@
+
+static void ParseMacroNames(StringRef Option,
+std::vector *Result) {

Now that you have pulled an API for this, I'll wait until you update this patch.


http://reviews.llvm.org/D19703



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


Re: [PATCH] D20095: [find-all-symbols] Slim SymbolInfo.

2016-05-11 Thread Manuel Klimek via cfe-commits
klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.

lg


http://reviews.llvm.org/D20095



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


Re: [PATCH] D14326: ASTImporter: expressions, pt.2

2016-05-11 Thread Aleksei Sidorin via cfe-commits
a.sidorin updated this revision to Diff 56860.
a.sidorin added a comment.

Add error checks; replace `NULL` with `nullptr`.


http://reviews.llvm.org/D14326

Files:
  include/clang/AST/ASTImporter.h
  include/clang/AST/DeclFriend.h
  lib/AST/ASTImporter.cpp
  test/ASTMerge/Inputs/class3.cpp
  test/ASTMerge/Inputs/exprs3.cpp
  test/ASTMerge/class2.cpp
  test/ASTMerge/exprs.cpp
  unittests/AST/ASTImporterTest.cpp

Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -466,5 +466,48 @@
 }
 
 
+const internal::VariadicDynCastAllOfMatcher vaArgExpr;
+
+/// \brief Matches the decayed type, whose original type matches \c InnerMatcher
+AST_MATCHER_P(DecayedType, hasOriginalType, internal::Matcher,
+  InnerType) {
+  return InnerType.matches(Node.getOriginalType(), Finder, Builder);
+}
+
+TEST(ImportExpr, ImportVAArgExpr) {
+  MatchVerifier Verifier;
+  EXPECT_TRUE(
+testImport(
+  "void declToImport(__builtin_va_list list, ...) {"
+  "  (void)__builtin_va_arg(list, int); }",
+  Lang_CXX, "", Lang_CXX, Verifier,
+  functionDecl(
+hasParameter(0,
+ varDecl(
+   hasName("list"),
+   hasType(
+ decayedType(
+   hasOriginalType(
+ typedefType(
+   hasDeclaration(
+ typedefDecl(
+   hasName("__builtin_va_list"),
+   hasType(
+ constantArrayType(
+   hasSize(1),
+   hasElementType(
+ qualType(
+   recordType(),
+   asString("struct __va_list_tag"
+  ),
+hasBody(
+  compoundStmt(
+has(
+  cStyleCastExpr(
+hasSourceExpression(
+  vaArgExpr();
+}
+
+
 } // end namespace ast_matchers
 } // end namespace clang
Index: test/ASTMerge/exprs.cpp
===
--- /dev/null
+++ test/ASTMerge/exprs.cpp
@@ -0,0 +1,40 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++1z -fcxx-exceptions -emit-pch -o %t.1.ast %S/Inputs/exprs3.cpp
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++1z -fcxx-exceptions -ast-merge %t.1.ast -fsyntax-only -verify %s
+// expected-no-diagnostics
+
+static_assert(Ch1 == 'a');
+static_assert(Ch2 == 'b');
+static_assert(Ch3 == 'c');
+
+static_assert(Ch4 == L'd');
+static_assert(Ch5 == L'e');
+static_assert(Ch6 == L'f');
+
+static_assert(C1 == 12);
+static_assert(C2 == 13);
+
+static_assert(C3 == 12);
+static_assert(C4 == 13);
+
+static_assert(C5 == 22L);
+static_assert(C6 == 23L);
+
+static_assert(C7 == 66LL);
+static_assert(C8 == 67ULL);
+
+static_assert(bval1 == true);
+static_assert(bval2 == false);
+
+void testImport(int *x, const S1 &cs1, S1 &s1) {
+  testNewThrowDelete();
+  testArrayElement(nullptr, 12);
+  testTernaryOp(0, 1, 2);
+  testConstCast(cs1);
+  testStaticCast(s1);
+  testReinterpretCast(s1);
+  testDynamicCast(s1);
+  testScalarInit(42);
+  testOffsetOf();
+  testDefaultArg(12);
+  useTemplateType();
+}
Index: test/ASTMerge/class2.cpp
===
--- /dev/null
+++ test/ASTMerge/class2.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++1z -emit-pch -o %t.1.ast %S/Inputs/class3.cpp
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++1z -ast-merge %t.1.ast -fsyntax-only -verify %s
+// expected-no-diagnostics
+
+class C3 {
+  int method_1(C2 *x) {
+return x->x;
+  }
+};
Index: test/ASTMerge/Inputs/exprs3.cpp
===
--- /dev/null
+++ test/ASTMerge/Inputs/exprs3.cpp
@@ -0,0 +1,127 @@
+// Integer literals
+const char Ch1 = 'a';
+const signed char Ch2 = 'b';
+const unsigned char Ch3 = 'c';
+
+const wchar_t Ch4 = L'd';
+const signed wchar_t Ch5 = L'e';
+const unsigned wchar_t Ch6 = L'f';
+
+const short C1 = 12;
+const unsigned short C2 = 13;
+
+const int C3 = 12;
+const unsigned int C4 = 13;
+
+const long C5 = 22;
+const unsigned long C6 = 23;
+
+const long long C7 = 66;
+const unsigned long long C8 = 67;
+
+
+// String literals
+const char str1[] = "ABCD";
+const char str2[] = "ABCD" "0123";
+
+const wchar_t wstr1[] = L"DEF";
+const wchar_t wstr2[] = L"DEF" L"123";
+
+
+// Boolean literals
+const bool bval1 = true;
+const bool bval2 = false;
+
+// Floating Literals
+const float F1 = 1

Re: [PATCH] D14326: ASTImporter: expressions, pt.2

2016-05-11 Thread Aleksei Sidorin via cfe-commits
a.sidorin marked 4 inline comments as done.


Comment at: lib/AST/ASTImporter.cpp:5527
@@ -5353,3 +5526,3 @@
  FoundD,
- /*FIXME:TemplateArgs=*/nullptr);
+ /*TemplateArgs=*/ResInfo);
   if (E->hadMultipleCandidates())

Nice :)


http://reviews.llvm.org/D14326



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


Re: [PATCH] D17578: [OpenCL]Allowing explicit conversion of "0" to event_t type

2016-05-11 Thread Joey Gouly via cfe-commits
joey accepted this revision.
joey added a comment.
This revision is now accepted and ready to land.

LGTM!


http://reviews.llvm.org/D17578



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


Re: [PATCH] D20095: [find-all-symbols] Slim SymbolInfo.

2016-05-11 Thread Haojian Wu via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL269162: [find-all-symbols] Slim SymbolInfo. (authored by 
hokein).

Changed prior to commit:
  http://reviews.llvm.org/D20095?vs=56725&id=56862#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D20095

Files:
  clang-tools-extra/trunk/include-fixer/InMemoryXrefsDB.cpp
  clang-tools-extra/trunk/include-fixer/XrefsDBManager.cpp
  clang-tools-extra/trunk/include-fixer/YamlXrefsDB.cpp
  clang-tools-extra/trunk/include-fixer/find-all-symbols/FindAllSymbols.cpp
  clang-tools-extra/trunk/include-fixer/find-all-symbols/SymbolInfo.cpp
  clang-tools-extra/trunk/include-fixer/find-all-symbols/SymbolInfo.h
  
clang-tools-extra/trunk/unittests/include-fixer/find-all-symbols/FindAllSymbolsTests.cpp

Index: clang-tools-extra/trunk/unittests/include-fixer/find-all-symbols/FindAllSymbolsTests.cpp
===
--- clang-tools-extra/trunk/unittests/include-fixer/find-all-symbols/FindAllSymbolsTests.cpp
+++ clang-tools-extra/trunk/unittests/include-fixer/find-all-symbols/FindAllSymbolsTests.cpp
@@ -46,18 +46,6 @@
 return false;
   }
 
-  bool getSymbolExtraInfo(SymbolInfo *Symbol) {
-for (const auto &S : Symbols) {
-  if (S == *Symbol) {
-Symbol->FunctionInfos = S.FunctionInfos;
-Symbol->TypedefNameInfos = S.TypedefNameInfos;
-Symbol->VariableInfos = S.VariableInfos;
-return true;
-  }
-}
-return false;
-  }
-
 private:
   std::vector Symbols;
 };
@@ -68,10 +56,6 @@
 return Reporter.hasSymbol(Symbol);
   }
 
-  bool getSymbolExtraInfo(SymbolInfo &Symbol) {
-return Reporter.getSymbolExtraInfo(&Symbol);
-  }
-
   bool runFindAllSymbols(StringRef Code) {
 FindAllSymbols matcher(&Reporter);
 clang::ast_matchers::MatchFinder MatchFinder;
@@ -87,7 +71,7 @@
 clang::tooling::newFrontendActionFactory(&MatchFinder);
 tooling::ToolInvocation Invocation(
 {std::string("find_all_symbols"), std::string("-fsyntax-only"),
- FileName},
+ std::string("-std=c++11"), FileName},
 Factory->create(), Files.get(),
 std::make_shared());
 
@@ -105,17 +89,10 @@
   MockReporter Reporter;
 };
 
-SymbolInfo
-CreateSymbolInfo(StringRef Name, SymbolInfo::SymbolKind Type,
- const std::string FilePath, int LineNumber,
- const std::vector &Contexts) {
-  SymbolInfo Symbol;
-  Symbol.Name = Name;
-  Symbol.Type = Type;
-  Symbol.FilePath = FilePath;
-  Symbol.LineNumber = LineNumber;
-  Symbol.Contexts = Contexts;
-  return Symbol;
+SymbolInfo CreateSymbolInfo(StringRef Name, SymbolInfo::SymbolKind Type,
+StringRef FilePath, int LineNumber,
+const std::vector &Contexts) {
+  return SymbolInfo(Name, Type, FilePath, Contexts, LineNumber);
 }
 
 TEST_F(FindAllSymbolsTest, VariableSymbols) {
@@ -127,29 +104,18 @@
   })";
   runFindAllSymbols(Code);
 
-  {
-SymbolInfo Symbol =
-CreateSymbolInfo("xargc", SymbolInfo::Variable, HeaderName, 2, {});
-EXPECT_TRUE(hasSymbol(Symbol));
-getSymbolExtraInfo(Symbol);
-EXPECT_EQ("int", Symbol.VariableInfos.getValue().Type);
-  }
-  {
-SymbolInfo Symbol =
-CreateSymbolInfo("", SymbolInfo::Variable, HeaderName, 4,
- {{SymbolInfo::Namespace, "na"}});
-EXPECT_TRUE(hasSymbol(Symbol));
-getSymbolExtraInfo(Symbol);
-EXPECT_EQ("_Bool", Symbol.VariableInfos.getValue().Type);
-  }
-  {
-SymbolInfo Symbol = CreateSymbolInfo(
-"", SymbolInfo::Variable, HeaderName, 5,
-{{SymbolInfo::Namespace, "nb"}, {SymbolInfo::Namespace, "na"}});
-EXPECT_TRUE(hasSymbol(Symbol));
-getSymbolExtraInfo(Symbol);
-EXPECT_EQ("const long long *", Symbol.VariableInfos.getValue().Type);
-  }
+  SymbolInfo Symbol =
+  CreateSymbolInfo("xargc", SymbolInfo::Variable, HeaderName, 2, {});
+  EXPECT_TRUE(hasSymbol(Symbol));
+
+  Symbol = CreateSymbolInfo("", SymbolInfo::Variable, HeaderName, 4,
+{{SymbolInfo::Namespace, "na"}});
+  EXPECT_TRUE(hasSymbol(Symbol));
+
+  Symbol = CreateSymbolInfo(
+  "", SymbolInfo::Variable, HeaderName, 5,
+  {{SymbolInfo::Namespace, "nb"}, {SymbolInfo::Namespace, "na"}});
+  EXPECT_TRUE(hasSymbol(Symbol));
 }
 
 TEST_F(FindAllSymbolsTest, ExternCSymbols) {
@@ -162,19 +128,12 @@
   })";
   runFindAllSymbols(Code);
 
-  {
-SymbolInfo Symbol =
-CreateSymbolInfo("C_Func", SymbolInfo::Function, HeaderName, 3, {});
-EXPECT_TRUE(hasSymbol(Symbol));
-getSymbolExtraInfo(Symbol);
-EXPECT_EQ("int", Symbol.FunctionInfos.getValue().ReturnType);
-EXPECT_TRUE(Symbol.FunctionInfos.getValue().ParameterTypes.empty());
-  }
-  {
-SymbolInfo Symbol =
-CreateSymbolInfo("C_struct", SymbolInfo::Class, HeaderName, 4, {});
-EXPECT_TRUE(hasSymbol(Symbol));
-  }
+  SymbolInfo Symbol =
+  CreateSymb

[clang-tools-extra] r269162 - [find-all-symbols] Slim SymbolInfo.

2016-05-11 Thread Haojian Wu via cfe-commits
Author: hokein
Date: Wed May 11 03:38:21 2016
New Revision: 269162

URL: http://llvm.org/viewvc/llvm-project?rev=269162&view=rev
Log:
[find-all-symbols] Slim SymbolInfo.

Summary:
SymbolInfo has some optional fields, which is a bad-smell
implementation. For now, we

* remove the optional field since we don't need them (we can probably
  add them back if we actually need them in the future)
* make SymbolInfo to be a class.

By this change, the code is more simplified.

Reviewers: klimek

Subscribers: cfe-commits, ioeric, bkramer

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

Modified:
clang-tools-extra/trunk/include-fixer/InMemoryXrefsDB.cpp
clang-tools-extra/trunk/include-fixer/XrefsDBManager.cpp
clang-tools-extra/trunk/include-fixer/YamlXrefsDB.cpp
clang-tools-extra/trunk/include-fixer/find-all-symbols/FindAllSymbols.cpp
clang-tools-extra/trunk/include-fixer/find-all-symbols/SymbolInfo.cpp
clang-tools-extra/trunk/include-fixer/find-all-symbols/SymbolInfo.h

clang-tools-extra/trunk/unittests/include-fixer/find-all-symbols/FindAllSymbolsTests.cpp

Modified: clang-tools-extra/trunk/include-fixer/InMemoryXrefsDB.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/include-fixer/InMemoryXrefsDB.cpp?rev=269162&r1=269161&r2=269162&view=diff
==
--- clang-tools-extra/trunk/include-fixer/InMemoryXrefsDB.cpp (original)
+++ clang-tools-extra/trunk/include-fixer/InMemoryXrefsDB.cpp Wed May 11 
03:38:21 2016
@@ -21,17 +21,15 @@ InMemoryXrefsDB::InMemoryXrefsDB(
 llvm::SmallVector Names;
 Identifier.split(Names, "::");
 for (const auto &Header : Entry.second) {
-  // FIXME: create a complete instance with static member function when it
-  // is implemented.
-  SymbolInfo Info;
-  Info.Name = Names.back();
-  Info.FilePath = Header;
+  std::vector Contexts;
   for (auto IdentiferContext = Names.rbegin() + 1;
IdentiferContext != Names.rend(); ++IdentiferContext) {
-Info.Contexts.push_back(
-{SymbolInfo::ContextType::Namespace, *IdentiferContext});
+Contexts.emplace_back(SymbolInfo::ContextType::Namespace,
+  *IdentiferContext);
   }
-  this->LookupTable[Info.Name].push_back(Info);
+
+  SymbolInfo Symbol(Names.back(), SymbolInfo::Class, Header, Contexts, 1);
+  this->LookupTable[Symbol.getName()].push_back(Symbol);
 }
   }
 }

Modified: clang-tools-extra/trunk/include-fixer/XrefsDBManager.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/include-fixer/XrefsDBManager.cpp?rev=269162&r1=269161&r2=269162&view=diff
==
--- clang-tools-extra/trunk/include-fixer/XrefsDBManager.cpp (original)
+++ clang-tools-extra/trunk/include-fixer/XrefsDBManager.cpp Wed May 11 
03:38:21 2016
@@ -36,13 +36,13 @@ XrefsDBManager::search(llvm::StringRef I
   std::vector Results;
   for (const auto &Symbol : Symbols) {
 // Match the identifier name without qualifier.
-if (Symbol.Name == Names.back()) {
+if (Symbol.getName() == Names.back()) {
   bool IsMatched = true;
-  auto SymbolContext = Symbol.Contexts.begin();
+  auto SymbolContext = Symbol.getContexts().begin();
   // Match the remaining context names.
   for (auto IdentiferContext = Names.rbegin() + 1;
IdentiferContext != Names.rend() &&
-   SymbolContext != Symbol.Contexts.end();
+   SymbolContext != Symbol.getContexts().end();
++IdentiferContext, ++SymbolContext) {
 if (SymbolContext->second != *IdentiferContext) {
   IsMatched = false;
@@ -56,10 +56,10 @@ XrefsDBManager::search(llvm::StringRef I
 // need to be changed.
 // FIXME: if the file path is a system header name, we want to use 
angle
 // brackets.
-Results.push_back(
-(Symbol.FilePath[0] == '"' || Symbol.FilePath[0] == '<')
-? Symbol.FilePath
-: "\"" + Symbol.FilePath + "\"");
+std::string FilePath = Symbol.getFilePath().str();
+Results.push_back((FilePath[0] == '"' || FilePath[0] == '<')
+  ? FilePath
+  : "\"" + FilePath + "\"");
   }
 }
   }

Modified: clang-tools-extra/trunk/include-fixer/YamlXrefsDB.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/include-fixer/YamlXrefsDB.cpp?rev=269162&r1=269161&r2=269162&view=diff
==
--- clang-tools-extra/trunk/include-fixer/YamlXrefsDB.cpp (original)
+++ clang-tools-extra/trunk/include-fixer/YamlXrefsDB.cpp Wed May 11 03:38:21 
2016
@@ -50,7 +50,7 @@ YamlXrefsDB::createFromDirectory(llvm::S
 std::vector YamlXrefsDB::search(llvm::StringRef Identifier) {
   std::vector Results;
   for (const auto &Symbol : Sy

Re: [PATCH] D20110: [include-fixer] Improve include paths' minimization.

2016-05-11 Thread Benjamin Kramer via cfe-commits
bkramer added a comment.

Making one path relative to another part is a hard problem. We'd need to make 
this work on windows and possibly also have it working in presence of symlinks. 
I'd much prefer to make paths in the YAML file relative to the "directory" in 
the compilation database, which is always the working directory of any clang 
tool. Would that break something?


Repository:
  rL LLVM

http://reviews.llvm.org/D20110



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


Re: [PATCH] D20113: Fix mangled name of method with ns_consumed parameters.

2016-05-11 Thread Sylvain Defresne via cfe-commits
sdefresne added a comment.

In http://reviews.llvm.org/D20113#425984, @rjmccall wrote:

> This is a good catch, thanks!


Thank you for the quick reply.

Please excuse me if I misunderstood you or if my remark appear off the mark, 
this is my first time sending patches to clang. I'm not yet completely familiar 
with all clang concepts :-)

> As a slight adjustment, It's probably better to just ignore this attribute 
> when mangling the function type of an entity, the same way that we generally 
> don't mangle return types because they don't affect overloading.  That will 
> require an extra flag to be passed down in a few places, but that's pretty 
> reasonable.  This will generally allow NS_CONSUMED and NS_RETURNS_RETAINED to 
> be placed on existing APIs without changing their mangling unless the 
> attribute is used in a secondary position (such as the type of an argument).

> 

> Finally, you should give ns_returns_retained the same treatment, as well as 
> the parameter-ABI attributes.


I'm not really sure what you mean by "ignore this attribute when mangling the 
function type of an entity". I read this as "we should ensure that the 
ns_consumed attribute is only mangled if it is applied to a parameter". Is this 
correct?

If so, then there is nothing to do, as "ns_consumed" attibute is ignored if 
applied to a non-parameter type as far as I can tell (see below for compilation 
warning clang already emits regarding ignored attributes). So, my understanding 
is that the ns_consumed attribute is only used if applied to a parameter, and 
thus only when it is relevant to include it in the mangled name.

Regarding ns_returns_retained, it is ignored if not applied to the function 
itself.

  $ clang++ -fobjc-arc -o x.o -c x.mm
  x.mm:1:19: warning: 'ns_consumed' attribute only applies to parameters
[-Wignored-attributes]
  id __attribute__((ns_consumed)) f() { return 0; }
^
  x.mm:2:23: warning: 'ns_consumed' attribute only applies to parameters
[-Wignored-attributes]
  id g() __attribute__((ns_consumed)) { return 0; }
^
  x.mm:7:26: warning: 'ns_returns_retained' only applies to function types; type
here is 'id' [-Wignored-attributes]
  void k(id __attribute__((ns_returns_retained))) {}

So, could you elaborate a bit more on what additional changes I need to make to 
my patch?


http://reviews.llvm.org/D20113



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


Re: [PATCH] D19105: Changes in clang after running http://reviews.llvm.org/D18821

2016-05-11 Thread Alexander Kornienko via cfe-commits
alexfh added a comment.

Too much noise here as well. Particularly, `someBool == true` should not be 
changed to `someBool == 1`. Please fix the check and update the results.



Comment at: clang-tidy/readability/ImplicitBoolCastCheck.cpp:253
@@ -252,3 +252,3 @@
DestinationType->isMemberPointerType()) &&
-  BoolLiteralExpression->getValue() == false) {
+  BoolLiteralExpression->getValue() == 0) {
 return "0";

Seems like a bug. `CXXBoolLiteralExpr::getValue()` returns `bool`.


Comment at: lib/AsmParser/LLParser.cpp:4868
@@ -4867,3 +4867,3 @@
 ///
 int LLParser::ParseInstruction(Instruction *&Inst, BasicBlock *BB,
PerFunctionState &PFS) {

Looks like this should return `InstResult` and the return values should be 
replaced by proper enumerators.


Comment at: lib/CodeGen/SelectionDAG/ScheduleDAGRRList.cpp:1562
@@ -1563,1 +1561,3 @@
+IsBottomUp = 1,
+HasReadyFilter = 0
   };

Quuxplusone wrote:
> If I understand correctly, the idiomatic C++11 way to write this code would be
> 
> static constexpr bool IsBottomUp = true;
> static constexpr bool HasReadyFilter = false;
> 
> I think the proposed correction is worse than the original, because it makes 
> this look like an integer enumeration.
I agree that changing this to integers is a regression. However, we can't yet 
use `constexpr`, since VS2013 doesn't support it. Looks like this file just 
needs to be reverted.


Comment at: lib/IR/Core.cpp:227
@@ -226,3 +226,3 @@
 *ErrorMessage = strdup(EC.message().c_str());
-return true;
+return 1;
   }

Quuxplusone wrote:
> This correction is wrong; either `true` should be accepted as a valid value 
> for `LLVMBool`, or `LLVMBool` should be replaced with `bool`.
This function is a part of C API, so we can't use `bool` here, and `LLVMBool` 
is the right choice. However, it might make sense to add `LLVM_TRUE` and 
`LLVM_FALSE` constants/macros to `include/llvm-c/Types.h` to more clearly 
specify possible values for the `LLVMBool` type.


Comment at: lib/Lex/PPMacroExpansion.cpp:1689
@@ -1688,3 +1688,3 @@
 if (!II)
-  return false;
+  return 0;
 else if (II->getBuiltinID() != 0)

This actually seems like a useful replacement ;)


Comment at: lib/MC/MCContext.cpp:314
@@ -313,3 +313,3 @@
   MCSectionELF(I->getKey(), Type, Flags, SectionKind::getReadOnly(),
-   EntrySize, Group, true, nullptr, Associated);
+   EntrySize, Group, 1, nullptr, Associated);
 }

That one is interesting: passing `true` (or `1`) to `unsigned UniqueID` seems 
like a very weird thing to do. Please at least add a `/*UniqueID=*/` comment 
before the argument to make this weirdness more explicit.


Comment at: lib/Sema/SemaOpenMP.cpp:4487
@@ -4486,3 +4486,3 @@
   // OK to convert to signed, because new type has more bits than old.
-  QualType NewType = C.getIntTypeForBitwidth(Bits, /* Signed */ true);
+  QualType NewType = C.getIntTypeForBitwidth(Bits, /* Signed */ 1);
   return SemaRef.PerformImplicitConversion(E, NewType, Sema::AA_Converting,

Instead, the parameter type should be changed to bool.


Comment at: lib/Serialization/ASTReader.cpp:5453
@@ -5452,3 +5452,3 @@
 AutoTypeKeyword Keyword = (AutoTypeKeyword)Record[Idx++];
-bool IsDependent = Deduced.isNull() ? Record[Idx++] : false;
+bool IsDependent = Deduced.isNull() ? Record[Idx++] : 0;
 return Context.getAutoType(Deduced, Keyword, IsDependent);

That's wrong. Instead, `Record[Idx++]` should be changed to `Record[Idx++] != 
0`.


Comment at: lib/Target/Hexagon/MCTargetDesc/HexagonMCShuffler.cpp:194
@@ -193,3 +193,3 @@
 
-  if (doneShuffling == false) {
+  if (doneShuffling == 0) {
 HexagonMCShuffler MCS(MCII, STI, MCB);

This is wrong.


http://reviews.llvm.org/D19105



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


Re: [PATCH] D19105: Changes in clang after running http://reviews.llvm.org/D18821

2016-05-11 Thread Alexander Kornienko via cfe-commits
alexfh requested changes to this revision.
alexfh added a comment.
This revision now requires changes to proceed.

The bottom line is that the fixit in its current form is not particularly 
useful in most cases. We should try to detect some cases where the replacement 
is unlikely to be useful, e.g. the `enum { FollowRAUW = false };` pattern, 
returning a bool from a function that is declared to return a typedef to an 
integral type that contains `bool` in its name (e.g. `LLVMBool`), and maybe 
some other cases.


http://reviews.llvm.org/D19105



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


Re: [PATCH] D20094: [include-fixer] Add basic documentation.

2016-05-11 Thread Manuel Klimek via cfe-commits
klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.

lg



Comment at: docs/include-fixer.rst:16
@@ +15,3 @@
+
+To use :program:`clang-include-fixer` two databases are required, both can be
+generated with existing tools.

s/, b/. B/


http://reviews.llvm.org/D20094



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


[clang-tools-extra] r269167 - [include-fixer] Add basic documentation.

2016-05-11 Thread Benjamin Kramer via cfe-commits
Author: d0k
Date: Wed May 11 04:44:10 2016
New Revision: 269167

URL: http://llvm.org/viewvc/llvm-project?rev=269167&view=rev
Log:
[include-fixer] Add basic documentation.

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

Added:
clang-tools-extra/trunk/docs/include-fixer.rst
Modified:
clang-tools-extra/trunk/docs/index.rst

Added: clang-tools-extra/trunk/docs/include-fixer.rst
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/include-fixer.rst?rev=269167&view=auto
==
--- clang-tools-extra/trunk/docs/include-fixer.rst (added)
+++ clang-tools-extra/trunk/docs/include-fixer.rst Wed May 11 04:44:10 2016
@@ -0,0 +1,71 @@
+===
+Clang-Include-Fixer
+===
+
+.. contents::
+
+One of the major nuisances of C++ compared to other languages is the manual
+management of ``#include`` directives in any file.
+:program:`clang-include-fixer` addresses one aspect of this problem by 
providing
+an automated way of adding ``#include`` directives for missing symbols in one
+translation unit.
+
+Setup
+=
+
+To use :program:`clang-include-fixer` two databases are required. Both can be
+generated with existing tools.
+
+- Compilation database. Contains the compiler commands for any given file in a
+  project and can be generated by CMake, see `How To Setup Tooling For LLVM`_.
+- Xrefs database. Contains all symbol information in a project to match a given
+  identifier to a header file.
+
+Ideally both databases (``compile_commands.json`` and
+``find_all_symbols_db.yaml``) are linked into the root of the source tree they
+correspond to. Then the :program:`clang-include-fixer` can automatically pick
+them up if called with a source file from that tree. Note that by default
+``compile_commands.json`` as generated by CMake does not include header files,
+so only implementation files can be handled by tools.
+
+.. _How To Setup Tooling For LLVM: 
http://clang.llvm.org/docs/HowToSetupToolingForLLVM.html
+
+Creating an Xrefs Database From a Compilation Database
+--
+
+The include fixer contains :program:`find-all-symbols`, a tool to create a
+symbol database in YAML format from a compilation database by parsing all
+source files listed in it. The following list of commands shows how to set up a
+database for LLVM, any project built by CMake should follow similar steps.
+
+.. code-block:: console
+
+  $ cd path/to/llvm-build
+  $ ls compile_commands.json # Make sure compile_commands.json exists.
+compile_commands.json
+  $ 
path/to/llvm/source/tools/clang/tools/extra/include-fixer/find-all-symbols/tool/run-find-all-symbols.py
+... wait as clang indexes the code base ...
+  $ ln -s $PWD/find_all_symbols_db.yaml path/to/llvm/source/ # Link database 
into the source tree.
+  $ ln -s $PWD/compile_commands.json path/to/llvm/source/ # Also link 
compilation database if it's not there already.
+  $ cd path/to/llvm/source
+  $ clang-include-fixer -db=yaml path/to/file/with/missing/include.cpp
+Added #include "foo.h"
+
+How it Works
+
+
+To get the most information out of clang at parse time,
+:program:`clang-include-fixer` runs in tandem with the parse and receives
+callbacks from Clang's semantic analysis. In particular it reuses the existing
+support for typo corrections. Whenever Clang tries to correct a potential typo
+it emits a callback to the include fixer which then looks for a corresponding
+file. At this point rich lookup information is still available, which is not
+available in the AST at a later stage.
+
+The identifier that should be typo corrected is then sent to the database, if a
+header file is returned it is added as an include directive at the top of the
+file.
+
+Currently :program:`clang-include-fixer` only inserts a single include at a
+time to avoid getting caught in follow-up errors. If multiple `#include`
+additions are desired the program can be rerun until a fix-point is reached.

Modified: clang-tools-extra/trunk/docs/index.rst
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/index.rst?rev=269167&r1=269166&r2=269167&view=diff
==
--- clang-tools-extra/trunk/docs/index.rst (original)
+++ clang-tools-extra/trunk/docs/index.rst Wed May 11 04:44:10 2016
@@ -21,6 +21,7 @@ Contents
:maxdepth: 2
 
clang-tidy/index
+   include-fixer
modularize
pp-trace
 


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


Re: [PATCH] D20094: [include-fixer] Add basic documentation.

2016-05-11 Thread Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL269167: [include-fixer] Add basic documentation. (authored 
by d0k).

Changed prior to commit:
  http://reviews.llvm.org/D20094?vs=56856&id=56867#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D20094

Files:
  clang-tools-extra/trunk/docs/include-fixer.rst
  clang-tools-extra/trunk/docs/index.rst

Index: clang-tools-extra/trunk/docs/index.rst
===
--- clang-tools-extra/trunk/docs/index.rst
+++ clang-tools-extra/trunk/docs/index.rst
@@ -21,6 +21,7 @@
:maxdepth: 2
 
clang-tidy/index
+   include-fixer
modularize
pp-trace
 
Index: clang-tools-extra/trunk/docs/include-fixer.rst
===
--- clang-tools-extra/trunk/docs/include-fixer.rst
+++ clang-tools-extra/trunk/docs/include-fixer.rst
@@ -0,0 +1,71 @@
+===
+Clang-Include-Fixer
+===
+
+.. contents::
+
+One of the major nuisances of C++ compared to other languages is the manual
+management of ``#include`` directives in any file.
+:program:`clang-include-fixer` addresses one aspect of this problem by 
providing
+an automated way of adding ``#include`` directives for missing symbols in one
+translation unit.
+
+Setup
+=
+
+To use :program:`clang-include-fixer` two databases are required. Both can be
+generated with existing tools.
+
+- Compilation database. Contains the compiler commands for any given file in a
+  project and can be generated by CMake, see `How To Setup Tooling For LLVM`_.
+- Xrefs database. Contains all symbol information in a project to match a given
+  identifier to a header file.
+
+Ideally both databases (``compile_commands.json`` and
+``find_all_symbols_db.yaml``) are linked into the root of the source tree they
+correspond to. Then the :program:`clang-include-fixer` can automatically pick
+them up if called with a source file from that tree. Note that by default
+``compile_commands.json`` as generated by CMake does not include header files,
+so only implementation files can be handled by tools.
+
+.. _How To Setup Tooling For LLVM: 
http://clang.llvm.org/docs/HowToSetupToolingForLLVM.html
+
+Creating an Xrefs Database From a Compilation Database
+--
+
+The include fixer contains :program:`find-all-symbols`, a tool to create a
+symbol database in YAML format from a compilation database by parsing all
+source files listed in it. The following list of commands shows how to set up a
+database for LLVM, any project built by CMake should follow similar steps.
+
+.. code-block:: console
+
+  $ cd path/to/llvm-build
+  $ ls compile_commands.json # Make sure compile_commands.json exists.
+compile_commands.json
+  $ 
path/to/llvm/source/tools/clang/tools/extra/include-fixer/find-all-symbols/tool/run-find-all-symbols.py
+... wait as clang indexes the code base ...
+  $ ln -s $PWD/find_all_symbols_db.yaml path/to/llvm/source/ # Link database 
into the source tree.
+  $ ln -s $PWD/compile_commands.json path/to/llvm/source/ # Also link 
compilation database if it's not there already.
+  $ cd path/to/llvm/source
+  $ clang-include-fixer -db=yaml path/to/file/with/missing/include.cpp
+Added #include "foo.h"
+
+How it Works
+
+
+To get the most information out of clang at parse time,
+:program:`clang-include-fixer` runs in tandem with the parse and receives
+callbacks from Clang's semantic analysis. In particular it reuses the existing
+support for typo corrections. Whenever Clang tries to correct a potential typo
+it emits a callback to the include fixer which then looks for a corresponding
+file. At this point rich lookup information is still available, which is not
+available in the AST at a later stage.
+
+The identifier that should be typo corrected is then sent to the database, if a
+header file is returned it is added as an include directive at the top of the
+file.
+
+Currently :program:`clang-include-fixer` only inserts a single include at a
+time to avoid getting caught in follow-up errors. If multiple `#include`
+additions are desired the program can be rerun until a fix-point is reached.


Index: clang-tools-extra/trunk/docs/index.rst
===
--- clang-tools-extra/trunk/docs/index.rst
+++ clang-tools-extra/trunk/docs/index.rst
@@ -21,6 +21,7 @@
:maxdepth: 2
 
clang-tidy/index
+   include-fixer
modularize
pp-trace
 
Index: clang-tools-extra/trunk/docs/include-fixer.rst
===
--- clang-tools-extra/trunk/docs/include-fixer.rst
+++ clang-tools-extra/trunk/docs/include-fixer.rst
@@ -0,0 +1,71 @@
+===
+Clang-Include-Fixer
+===
+
+.. contents::
+
+One of the major nuisances of C++ compared to other languages is the manual
+management of ``#inc

Re: [PATCH] D19816: [find-all-symbols] Add IWYU private pragma support.

2016-05-11 Thread Haojian Wu via cfe-commits
hokein updated this revision to Diff 56872.
hokein marked an inline comment as done.
hokein added a comment.

Rebase to master.


http://reviews.llvm.org/D19816

Files:
  include-fixer/find-all-symbols/CMakeLists.txt
  include-fixer/find-all-symbols/FindAllSymbols.cpp
  include-fixer/find-all-symbols/FindAllSymbols.h
  include-fixer/find-all-symbols/HeaderMapCollector.h
  include-fixer/find-all-symbols/PragmaCommentHandler.cpp
  include-fixer/find-all-symbols/PragmaCommentHandler.h
  include-fixer/find-all-symbols/tool/FindAllSymbolsMain.cpp
  unittests/include-fixer/find-all-symbols/FindAllSymbolsTests.cpp

Index: unittests/include-fixer/find-all-symbols/FindAllSymbolsTests.cpp
===
--- unittests/include-fixer/find-all-symbols/FindAllSymbolsTests.cpp
+++ unittests/include-fixer/find-all-symbols/FindAllSymbolsTests.cpp
@@ -8,11 +8,14 @@
 //===--===//
 
 #include "FindAllSymbols.h"
+#include "HeaderMapCollector.h"
+#include "PragmaCommentHandler.h"
 #include "SymbolInfo.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/FileSystemOptions.h"
 #include "clang/Basic/VirtualFileSystem.h"
+#include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/PCHContainerOperations.h"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
@@ -50,25 +53,58 @@
   std::vector Symbols;
 };
 
+class TestFindAllSymbolsAction : public clang::ASTFrontendAction {
+public:
+  TestFindAllSymbolsAction(FindAllSymbols::ResultReporter *Reporter)
+  : MatchFinder(), Collector(), Handler(&Collector),
+Matcher(Reporter, &Collector) {
+Matcher.registerMatchers(&MatchFinder);
+  }
+
+  std::unique_ptr
+  CreateASTConsumer(clang::CompilerInstance &Compiler,
+StringRef InFile) override {
+Compiler.getPreprocessor().addCommentHandler(&Handler);
+return MatchFinder.newASTConsumer();
+  }
+
+private:
+  ast_matchers::MatchFinder MatchFinder;
+  HeaderMapCollector Collector;
+  PragmaCommentHandler Handler;
+  FindAllSymbols Matcher;
+};
+
+class TestFindAllSymbolsActionFactory
+: public clang::tooling::FrontendActionFactory {
+public:
+  TestFindAllSymbolsActionFactory(MockReporter *Reporter)
+  : Reporter(Reporter) {}
+  clang::FrontendAction *create() override {
+return new TestFindAllSymbolsAction(Reporter);
+  }
+
+private:
+  MockReporter *const Reporter;
+};
+
 class FindAllSymbolsTest : public ::testing::Test {
 public:
   bool hasSymbol(const SymbolInfo &Symbol) {
 return Reporter.hasSymbol(Symbol);
   }
 
   bool runFindAllSymbols(StringRef Code) {
-FindAllSymbols matcher(&Reporter);
-clang::ast_matchers::MatchFinder MatchFinder;
-matcher.registerMatchers(&MatchFinder);
-
 llvm::IntrusiveRefCntPtr InMemoryFileSystem(
 new vfs::InMemoryFileSystem);
 llvm::IntrusiveRefCntPtr Files(
 new FileManager(FileSystemOptions(), InMemoryFileSystem));
 
 std::string FileName = "symbol.cc";
-std::unique_ptr Factory =
-clang::tooling::newFrontendActionFactory(&MatchFinder);
+
+std::unique_ptr Factory(
+new TestFindAllSymbolsActionFactory(&Reporter));
+
 tooling::ToolInvocation Invocation(
 {std::string("find_all_symbols"), std::string("-fsyntax-only"),
  std::string("-std=c++11"), FileName},
@@ -273,5 +309,18 @@
   EXPECT_TRUE(hasSymbol(Symbol));
 }
 
+TEST_F(FindAllSymbolsTest, IWYUPrivatePragmaTest) {
+  static const char Code[] = R"(
+// IWYU pragma: private, include "bar.h"
+struct Bar {
+};
+  )";
+  runFindAllSymbols(Code);
+
+  SymbolInfo Symbol =
+  CreateSymbolInfo("Bar", SymbolInfo::Class, "bar.h", 3, {});
+  EXPECT_TRUE(hasSymbol(Symbol));
+}
+
 } // namespace find_all_symbols
 } // namespace clang
Index: include-fixer/find-all-symbols/tool/FindAllSymbolsMain.cpp
===
--- include-fixer/find-all-symbols/tool/FindAllSymbolsMain.cpp
+++ include-fixer/find-all-symbols/tool/FindAllSymbolsMain.cpp
@@ -8,8 +8,14 @@
 //===--===//
 
 #include "FindAllSymbols.h"
+#include "HeaderMapCollector.h"
+#include "PragmaCommentHandler.h"
 #include "SymbolInfo.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/FrontendActions.h"
+#include "clang/Lex/Preprocessor.h"
 #include "clang/Tooling/CommonOptionsParser.h"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/ArrayRef.h"
@@ -82,6 +88,31 @@
   std::map> Symbols;
 };
 
+class FindAllSymbolsAction : public clang::ASTFrontendAction {
+public:
+  FindAllSymbolsAction()
+  : Reporter(), MatchFinder(), Collector(), Handler(&Collector),
+Matcher(&Reporter, &Collector) {
+Match

Re: [PATCH] D19816: [find-all-symbols] Add IWYU private pragma support.

2016-05-11 Thread Haojian Wu via cfe-commits
hokein marked an inline comment as done.
hokein added a comment.

http://reviews.llvm.org/D19816



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


[PATCH] D20157: Add test for D20156

2016-05-11 Thread Diana Picus via cfe-commits
rovka created this revision.
rovka added reviewers: rengolin, resistor, mcrosier, echristo, qcolombet, 
bogner.
rovka added subscribers: llvm-commits, cfe-commits.
Herald added a subscriber: joker.eph.

This adds a test for PR24071. Its purpose is to make sure that we do not assert 
after identifying an error in inline asm.

It has to be added in Clang because llc exits after identifying the error, and 
therefore never reaches the assert. The situation is similar to [1].

http://reviews.llvm.org/D20156

[1] https://www.mail-archive.com/cfe-commits@cs.uiuc.edu/msg70985.html

http://reviews.llvm.org/D20157

Files:
  test/CodeGen/arm64-asm-diag.c

Index: test/CodeGen/arm64-asm-diag.c
===
--- /dev/null
+++ test/CodeGen/arm64-asm-diag.c
@@ -0,0 +1,24 @@
+// RUN: not %clang_cc1 -triple arm64 %s -S -o /dev/null 2>&1 | FileCheck %s
+
+// This test makes sure that we correctly recover after an error is identified
+// in inline asm during instruction selection.
+// It needs to be in Clang because the diagnostic handler that is used by llc
+// in LLVM's CodeGen tests exits after identifying an error, whereas the
+// diagnostic handler used by Clang keeps running and may trigger assertions
+// (see PR24071).
+// This test as well as arm-asm-diag.c can be moved back into LLVM if llc is
+// updated to use a different diagnostic handler, that tries to catch as many
+// errors as possible without exiting.
+int foo(long int a, long int b){
+const long int c = 32 - b;
+
+long int r;
+asm("lsl  %[o],%[s],%[a]"
+:   [o]"=r"  (r)
+:   [s]"r"   (a),
+[a]"n"   (c));
+
+return r;
+}
+
+// CHECK: error: invalid operand for inline asm constraint 'n'


Index: test/CodeGen/arm64-asm-diag.c
===
--- /dev/null
+++ test/CodeGen/arm64-asm-diag.c
@@ -0,0 +1,24 @@
+// RUN: not %clang_cc1 -triple arm64 %s -S -o /dev/null 2>&1 | FileCheck %s
+
+// This test makes sure that we correctly recover after an error is identified
+// in inline asm during instruction selection.
+// It needs to be in Clang because the diagnostic handler that is used by llc
+// in LLVM's CodeGen tests exits after identifying an error, whereas the
+// diagnostic handler used by Clang keeps running and may trigger assertions
+// (see PR24071).
+// This test as well as arm-asm-diag.c can be moved back into LLVM if llc is
+// updated to use a different diagnostic handler, that tries to catch as many
+// errors as possible without exiting.
+int foo(long int a, long int b){
+const long int c = 32 - b;
+
+long int r;
+asm("lsl  %[o],%[s],%[a]"
+:   [o]"=r"  (r)
+:   [s]"r"   (a),
+[a]"n"   (c));
+
+return r;
+}
+
+// CHECK: error: invalid operand for inline asm constraint 'n'
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D20103: PR27132: Proper mangling for __unaligned qualifier (now with both PR27367 and PR27666 fixed)

2016-05-11 Thread Andrey Bokhanko via cfe-commits
andreybokhanko updated this revision to Diff 56874.
andreybokhanko added a comment.

Patch updated to address David's comments.

Yours,
Andrey


http://reviews.llvm.org/D20103

Files:
  include/clang/AST/Type.h
  include/clang/Basic/AddressSpaces.h
  include/clang/Basic/Attr.td
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/DeclSpec.h
  include/clang/Sema/Sema.h
  lib/AST/MicrosoftMangle.cpp
  lib/AST/TypePrinter.cpp
  lib/Parse/ParseDecl.cpp
  lib/Parse/ParseTentative.cpp
  lib/Sema/DeclSpec.cpp
  lib/Sema/SemaCodeComplete.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaDeclObjC.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaOverload.cpp
  lib/Sema/SemaType.cpp
  test/CodeGenCXX/mangle-ms-cxx11.cpp
  test/CodeGenCXX/mangle-ms-cxx14.cpp
  test/Sema/MicrosoftExtensions.c
  test/Sema/address_spaces.c
  test/Sema/invalid-assignment-constant-address-space.c
  test/SemaCXX/MicrosoftExtensions.cpp

Index: lib/Parse/ParseDecl.cpp
===
--- lib/Parse/ParseDecl.cpp
+++ lib/Parse/ParseDecl.cpp
@@ -609,7 +609,6 @@
 case tok::kw___ptr64:
 case tok::kw___w64:
 case tok::kw___ptr32:
-case tok::kw___unaligned:
 case tok::kw___sptr:
 case tok::kw___uptr: {
   IdentifierInfo *AttrName = Tok.getIdentifierInfo();
@@ -3087,6 +3086,11 @@
   break;
 }
 
+case tok::kw___unaligned:
+  isInvalid = DS.SetTypeQual(DeclSpec::TQ_unaligned, Loc, PrevSpec, DiagID,
+ getLangOpts());
+  break;
+
 case tok::kw___sptr:
 case tok::kw___uptr:
 case tok::kw___ptr64:
@@ -3097,7 +3101,6 @@
 case tok::kw___fastcall:
 case tok::kw___thiscall:
 case tok::kw___vectorcall:
-case tok::kw___unaligned:
   ParseMicrosoftTypeAttributes(DS.getAttributes());
   continue;
 
@@ -4798,6 +4801,10 @@
   ParseOpenCLQualifiers(DS.getAttributes());
   break;
 
+case tok::kw___unaligned:
+  isInvalid = DS.SetTypeQual(DeclSpec::TQ_unaligned, Loc, PrevSpec, DiagID,
+ getLangOpts());
+  break;
 case tok::kw___uptr:
   // GNU libc headers in C mode use '__uptr' as an identifer which conflicts
   // with the MS modifier keyword.
@@ -4815,7 +4822,6 @@
 case tok::kw___fastcall:
 case tok::kw___thiscall:
 case tok::kw___vectorcall:
-case tok::kw___unaligned:
   if (AttrReqs & AR_DeclspecAttributesParsed) {
 ParseMicrosoftTypeAttributes(DS.getAttributes());
 continue;
@@ -5038,7 +5044,8 @@
 DS.getConstSpecLoc(),
 DS.getVolatileSpecLoc(),
 DS.getRestrictSpecLoc(),
-DS.getAtomicSpecLoc()),
+DS.getAtomicSpecLoc(),
+DS.getUnalignedSpecLoc()),
 DS.getAttributes(),
 SourceLocation());
 else
Index: lib/Parse/ParseTentative.cpp
===
--- lib/Parse/ParseTentative.cpp
+++ lib/Parse/ParseTentative.cpp
@@ -833,7 +833,7 @@
   // '(' abstract-declarator ')'
   if (Tok.isOneOf(tok::kw___attribute, tok::kw___declspec, tok::kw___cdecl,
   tok::kw___stdcall, tok::kw___fastcall, tok::kw___thiscall,
-  tok::kw___vectorcall, tok::kw___unaligned))
+  tok::kw___vectorcall))
 return TPResult::True; // attributes indicate declaration
   TPResult TPR = TryParseDeclarator(mayBeAbstract, mayHaveIdentifier);
   if (TPR != TPResult::Ambiguous)
Index: lib/AST/MicrosoftMangle.cpp
===
--- lib/AST/MicrosoftMangle.cpp
+++ lib/AST/MicrosoftMangle.cpp
@@ -1446,6 +1446,9 @@
 
   if (HasRestrict)
 Out << 'I';
+
+  if (!PointeeType.isNull() && PointeeType.getLocalQualifiers().hasUnaligned())
+Out << 'F';
 }
 
 void MicrosoftCXXNameMangler::manglePointerCVQualifiers(Qualifiers Quals) {
@@ -1577,6 +1580,8 @@
 }
 break;
   case QMM_Result:
+// Presence of __unaligned qualifier shouldn't affect mangling here.
+Quals.removeUnaligned();
 if ((!IsPointer && Quals) || isa(T)) {
   Out << '?';
   mangleQualifiers(Quals, false);
Index: lib/AST/TypePrinter.cpp
===
--- lib/AST/TypePrinter.cpp
+++ lib/AST/TypePrinter.cpp
@@ -1593,6 +1593,12 @@
 AppendTypeQualList(OS, quals, Policy.LangOpts.C99);
 addSpace = true;
   }
+  if (hasUnaligned()) {
+if (addSpace)
+  OS << ' ';
+OS << "__unaligned";
+addSpace = true;
+  }
   if (unsigned addrspace = getAddressSpace()) {
 if (addSpace)
   OS << ' ';
Index: lib/Sema/SemaExpr.cpp

Re: [PATCH] D20103: PR27132: Proper mangling for __unaligned qualifier (now with both PR27367 and PR27666 fixed)

2016-05-11 Thread Andrey Bokhanko via cfe-commits
andreybokhanko marked 3 inline comments as done.
andreybokhanko added a comment.

In http://reviews.llvm.org/D20103#425855, @majnemer wrote:

> Can we test pointers to data members? Is it possible to have `__unaligned int 
> *S::*` or `int *S::* __unaligned` or even `__unaligned int *S::* __unaligned` 
> ?


I added these three cases to MicrosoftExtensions.cpp. We behave exactly like MS 
compiler: allow all three usages, disallow assignment of an unaligned pointer 
to a non-unaligned one.



Comment at: include/clang/AST/Type.h:446
@@ -437,1 +445,3 @@
+   // U qualifier may superset.
+   (!other.hasUnaligned() || hasUnaligned());
   }

Fixed


Comment at: include/clang/AST/Type.h:5393-5399
@@ -5379,3 +5392,9 @@
 inline bool QualType::isAtLeastAsQualifiedAs(QualType other) const {
-  return getQualifiers().compatiblyIncludes(other.getQualifiers());
+  Qualifiers OtherQuals = other.getQualifiers();
+
+  // Ignore __unaligned qualifier if this type is a void.
+  if (getUnqualifiedType()->isVoidType())
+OtherQuals.removeUnaligned();
+
+  return getQualifiers().compatiblyIncludes(OtherQuals);
 }

Fixed


Comment at: lib/Sema/SemaType.cpp:2680-2681
@@ -2674,4 +2679,4 @@
 
   // Build a string naming the redundant qualifiers.
-  for (unsigned I = 0; I != 4; ++I) {
-if (Quals & QualKinds[I].Mask) {
+  for (auto &E : QualKinds) {
+if (Quals & E.Mask) {

Fixed


http://reviews.llvm.org/D20103



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


r269170 - [clang][AVX512] completing missing intrinsics for [vpermt2d|vptestm] instruction set.

2016-05-11 Thread Michael Zuckerman via cfe-commits
Author: mzuckerm
Date: Wed May 11 06:21:18 2016
New Revision: 269170

URL: http://llvm.org/viewvc/llvm-project?rev=269170&view=rev
Log:
[clang][AVX512] completing missing intrinsics for [vpermt2d|vptestm] 
instruction set.

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


Modified:
cfe/trunk/lib/Headers/avx512fintrin.h
cfe/trunk/test/CodeGen/avx512f-builtins.c

Modified: cfe/trunk/lib/Headers/avx512fintrin.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/avx512fintrin.h?rev=269170&r1=269169&r2=269170&view=diff
==
--- cfe/trunk/lib/Headers/avx512fintrin.h (original)
+++ cfe/trunk/lib/Headers/avx512fintrin.h Wed May 11 06:21:18 2016
@@ -2830,6 +2830,29 @@ _mm512_permutex2var_epi32(__m512i __A, _
(__v16si) __B,
(__mmask16) -1);
 }
+
+static __inline__ __m512i __DEFAULT_FN_ATTRS
+_mm512_mask_permutex2var_epi32 (__m512i __A, __mmask16 __U,
+__m512i __I, __m512i __B)
+{
+  return (__m512i) __builtin_ia32_vpermt2vard512_mask ((__v16si) __I
+/* idx */ ,
+(__v16si) __A,
+(__v16si) __B,
+(__mmask16) __U);
+}
+
+static __inline__ __m512i __DEFAULT_FN_ATTRS
+_mm512_maskz_permutex2var_epi32 (__mmask16 __U, __m512i __A,
+ __m512i __I, __m512i __B)
+{
+  return (__m512i) __builtin_ia32_vpermt2vard512_maskz ((__v16si) __I
+/* idx */ ,
+(__v16si) __A,
+(__v16si) __B,
+(__mmask16) __U);
+}
+
 static __inline __m512i __DEFAULT_FN_ATTRS
 _mm512_permutex2var_epi64(__m512i __A, __m512i __I, __m512i __B)
 {
@@ -2840,23 +2863,27 @@ _mm512_permutex2var_epi64(__m512i __A, _
(__mmask8) -1);
 }
 
-static __inline __m512d __DEFAULT_FN_ATTRS
-_mm512_permutex2var_pd(__m512d __A, __m512i __I, __m512d __B)
+static __inline__ __m512i __DEFAULT_FN_ATTRS
+_mm512_mask_permutex2var_epi64 (__m512i __A, __mmask8 __U, __m512i __I,
+__m512i __B)
 {
-  return (__m512d) __builtin_ia32_vpermt2varpd512_mask ((__v8di) __I
-/* idx */ ,
-(__v8df) __A,
-(__v8df) __B,
-(__mmask8) -1);
+  return (__m512i) __builtin_ia32_vpermt2varq512_mask ((__v8di) __I
+   /* idx */ ,
+   (__v8di) __A,
+   (__v8di) __B,
+   (__mmask8) __U);
 }
-static __inline __m512 __DEFAULT_FN_ATTRS
-_mm512_permutex2var_ps(__m512 __A, __m512i __I, __m512 __B)
+
+
+static __inline__ __m512i __DEFAULT_FN_ATTRS
+_mm512_maskz_permutex2var_epi64 (__mmask8 __U, __m512i __A,
+ __m512i __I, __m512i __B)
 {
-  return (__m512) __builtin_ia32_vpermt2varps512_mask ((__v16si) __I
-   /* idx */ ,
-   (__v16sf) __A,
-   (__v16sf) __B,
-   (__mmask16) -1);
+  return (__m512i) __builtin_ia32_vpermt2varq512_maskz ((__v8di) __I
+/* idx */ ,
+(__v8di) __A,
+(__v8di) __B,
+(__mmask8) __U);
 }
 
 #define _mm512_alignr_epi64(A, B, I) __extension__ ({ \
@@ -3466,6 +3493,13 @@ _mm512_test_epi32_mask(__m512i __A, __m5
 (__mmask16) -1);
 }
 
+static __inline__ __mmask16 __DEFAULT_FN_ATTRS
+_mm512_mask_test_epi32_mask (__mmask16 __U, __m512i __A, __m512i __B)
+{
+  return (__mmask16) __builtin_ia32_ptestmd512 ((__v16si) __A,
+ (__v16si) __B, __U);
+}
+
 static __inline __mmask8 __DEFAULT_FN_ATTRS
 _mm512_test_epi64_mask(__m512i __A, __m512i __B)
 {
@@ -3474,6 +3508,13 @@ _mm512_test_epi64_mask(__m512i __A, __m5
  (__mmask8) -1);
 }
 
+static __inline__ __mmask8 __DEFAULT_FN_ATTRS
+_mm512_mask_test_epi64_mask (__mmask8 __U, __m512i __A, __m512i __B)
+{
+  return (__mmask8) __builtin_ia32_ptestmq512 ((__v8di) __A,

Re: [PATCH] D20118: Add support for injected class names and constructor initializers in C++

2016-05-11 Thread Aleksei Sidorin via cfe-commits
a.sidorin added a comment.

I' d like to have some tests for this. CXXCtorInitializers can be tested with 
D14224 -like stuff or with ASTMatchers 
(ASTImporterTest.cpp).
Some code samples may be found in CXX/Sema tests, you just need to 
port/simplify them.



Comment at: lib/AST/ASTImporter.cpp:3034
@@ +3033,3 @@
+SmallVector CtorInitializers;
+for (const CXXCtorInitializer *I : FromConstructor->inits()) {
+  CXXCtorInitializer *ToI =cast_or_null(

In my latest patch, I have introduced a function named 
`ImportContainerChecked()`. I think it could make this code a bit more clean. 
What do you think?


Comment at: lib/AST/ASTImporter.cpp:3035
@@ +3034,3 @@
+for (const CXXCtorInitializer *I : FromConstructor->inits()) {
+  CXXCtorInitializer *ToI =cast_or_null(
+  Importer.Import(I));

Space after '='


Comment at: lib/AST/ASTImporter.cpp:3041
@@ +3040,3 @@
+}
+if (unsigned NumInitializers = CtorInitializers.size()) {
+  CXXCtorInitializer **Memory = new (Importer.getToContext())

If we move this condition (I suggest to use 
`FromConstructor->getNumCtorIintializers()` instead) upper to cover the 
importing loop, we may skip this loop as well.


Comment at: lib/AST/ASTImporter.cpp:3045
@@ +3044,3 @@
+  std::copy(CtorInitializers.begin(), CtorInitializers.end(), Memory);
+  llvm::cast(ToFunction)->setCtorInitializers(Memory);
+  llvm::cast(ToFunction)->setNumCtorInitializers(

As I can see, LLVM code style avoids qualified `cast`s.


Comment at: lib/AST/ASTImporter.cpp:6384
@@ +6383,3 @@
+  return nullptr;
+
+return new (ToContext)

Trailing whitespace.


Comment at: lib/AST/ASTImporter.cpp:6389
@@ +6388,3 @@
+ Import(From->getRParenLoc()),
+ From->isPackExpansion() ?
+ Import(From->getEllipsisLoc()) : SourceLocation());

The indentation for `?:` here is a bit confusing.


Comment at: lib/AST/ASTImporter.cpp:6396
@@ +6395,3 @@
+  return nullptr;
+
+return new (ToContext)

Training whitespace. It's pretty difficult to find them with Phabricator, could 
you check your patch for them?


Comment at: lib/AST/ASTImporter.cpp:6423
@@ +6422,3 @@
+  } else {
+return nullptr;
+  }

It seems like a case with indexed initializer is missed here. You can find my 
implementation on 
https://github.com/haoNoQ/clang/blob/summary-ipa-draft/lib/AST/ASTImporter.cpp#L2456-L2464.
A sample that requires it is `auto parens4 = [p4(1)] {};` 
(test/CXX/dcl.dcl/dcl.spec/dcl.type/dcl.spec.auto/p4.cpp)



Repository:
  rL LLVM

http://reviews.llvm.org/D20118



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


Re: [PATCH] D20110: [include-fixer] Improve include paths' minimization.

2016-05-11 Thread Haojian Wu via cfe-commits
hokein abandoned this revision.
hokein added a comment.

Abandon this, in favor of http://reviews.llvm.org/D20158.


Repository:
  rL LLVM

http://reviews.llvm.org/D20110



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


Re: [PATCH] D20158: [find-all-symbols] Save relative file path for each symbol.

2016-05-11 Thread Haojian Wu via cfe-commits
hokein added inline comments.


Comment at: include-fixer/find-all-symbols/SymbolInfo.h:81
@@ -81,1 +80,3 @@
+  /// \brief The file path where the symbol comes from. It's a relative file
+  /// path based on the build directory.
   std::string FilePath;

One question is that whether should we add a `BuildDir` in `SymbolInfo`. With 
`BuildDir`, we can check the build directory in include-fixer make sure they 
are the same :).


Repository:
  rL LLVM

http://reviews.llvm.org/D20158



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


Re: [PATCH] D20158: [find-all-symbols] Save relative file path for each symbol.

2016-05-11 Thread Benjamin Kramer via cfe-commits
bkramer accepted this revision.
bkramer added a comment.
This revision is now accepted and ready to land.

LG!


Repository:
  rL LLVM

http://reviews.llvm.org/D20158



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


Re: [PATCH] D20158: [find-all-symbols] Save relative file path for each symbol.

2016-05-11 Thread Benjamin Kramer via cfe-commits
bkramer added inline comments.


Comment at: include-fixer/find-all-symbols/SymbolInfo.h:81
@@ -81,1 +80,3 @@
+  /// \brief The file path where the symbol comes from. It's a relative file
+  /// path based on the build directory.
   std::string FilePath;

hokein wrote:
> One question is that whether should we add a `BuildDir` in `SymbolInfo`. With 
> `BuildDir`, we can check the build directory in include-fixer make sure they 
> are the same :).
Hmm. If both the symbol finder and include fixer run on the same compilation 
database this can never happen, right?


Repository:
  rL LLVM

http://reviews.llvm.org/D20158



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


r269172 - [clang][AVX512] completing missing set intrinsics

2016-05-11 Thread Michael Zuckerman via cfe-commits
Author: mzuckerm
Date: Wed May 11 06:41:29 2016
New Revision: 269172

URL: http://llvm.org/viewvc/llvm-project?rev=269172&view=rev
Log:
[clang][AVX512] completing missing set intrinsics

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


Modified:
cfe/trunk/lib/Headers/avx512fintrin.h
cfe/trunk/test/CodeGen/avx512f-builtins.c

Modified: cfe/trunk/lib/Headers/avx512fintrin.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/avx512fintrin.h?rev=269172&r1=269171&r2=269172&view=diff
==
--- cfe/trunk/lib/Headers/avx512fintrin.h (original)
+++ cfe/trunk/lib/Headers/avx512fintrin.h Wed May 11 06:41:29 2016
@@ -8959,6 +8959,48 @@ _mm_cvtu64_ss (__m128 __A, unsigned long
 _MM_FROUND_CUR_DIRECTION);
 }
 
+static __inline__ __m512i __DEFAULT_FN_ATTRS
+_mm512_mask_set1_epi32 (__m512i __O, __mmask16 __M, int __A)
+{
+  return (__m512i) __builtin_ia32_pbroadcastd512_gpr_mask (__A, (__v16si) __O,
+ __M);
+}
+
+static __inline__ __m512i __DEFAULT_FN_ATTRS
+_mm512_mask_set1_epi64 (__m512i __O, __mmask8 __M, long long __A)
+{
+  return (__m512i) __builtin_ia32_pbroadcastq512_gpr_mask (__A, (__v8di) __O,
+ __M);
+}
+
+static __inline__ __m512i __DEFAULT_FN_ATTRS
+_mm512_set_epi64 (long long __A, long long __B, long long __C,
+ long long __D, long long __E, long long __F,
+ long long __G, long long __H)
+{
+  return __extension__ (__m512i) (__v8di)
+  { __H, __G, __F, __E, __D, __C, __B, __A };
+}
+
+static __inline__ __m512d __DEFAULT_FN_ATTRS
+_mm512_set_pd (double __A, double __B, double __C, double __D,
+double __E, double __F, double __G, double __H)
+{
+  return __extension__ (__m512d)
+  { __H, __G, __F, __E, __D, __C, __B, __A };
+}
+
+static __inline__ __m512 __DEFAULT_FN_ATTRS
+_mm512_set_ps (float __A, float __B, float __C, float __D,
+float __E, float __F, float __G, float __H,
+float __I, float __J, float __K, float __L,
+float __M, float __N, float __O, float __P)
+{
+  return __extension__ (__m512)
+  { __P, __O, __N, __M, __L, __K, __J, __I,
+__H, __G, __F, __E, __D, __C, __B, __A };
+}
+
 #undef __DEFAULT_FN_ATTRS
 
 #endif // __AVX512FINTRIN_H

Modified: cfe/trunk/test/CodeGen/avx512f-builtins.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/avx512f-builtins.c?rev=269172&r1=269171&r2=269172&view=diff
==
--- cfe/trunk/test/CodeGen/avx512f-builtins.c (original)
+++ cfe/trunk/test/CodeGen/avx512f-builtins.c Wed May 11 06:41:29 2016
@@ -6382,3 +6382,75 @@ __m512i test_mm512_maskz_min_epu64 (__mm
   // CHECK: @llvm.x86.avx512.mask.pminu.q.512
   return _mm512_maskz_min_epu64 (__M,__A,__B);
 }
+
+__m512i test_mm512_mask_set1_epi32 (__m512i __O, __mmask16 __M, int __A)
+{
+//CHECK-LABLE: @test_mm512_mask_set1_epi32
+//CHECK: @llvm.x86.avx512.mask.pbroadcast.d.gpr.512
+  return _mm512_mask_set1_epi32 ( __O, __M, __A);
+}
+
+__m512i test_mm512_mask_set1_epi64 (__m512i __O, __mmask8 __M, long long __A)
+{
+//CHECK-LABLE: @test_mm512_mask_set1_epi64
+//CHECK: @llvm.x86.avx512.mask.pbroadcast.q.gpr.512
+  return _mm512_mask_set1_epi64 (__O, __M, __A);
+}
+
+__m512i test_mm512_set_epi64 (long long __A, long long __B, long long __C,
+  long long __D, long long __E, long long __F,
+  long long __G, long long __H)
+{
+//CHECK-LABLE: @test_mm512_set_epi64
+//CHECK: insertelement{{.*}}i32 0
+//CHECK: insertelement{{.*}}i32 1
+//CHECK: insertelement{{.*}}i32 2
+//CHECK: insertelement{{.*}}i32 3
+//CHECK: insertelement{{.*}}i32 4
+//CHECK: insertelement{{.*}}i32 5
+//CHECK: insertelement{{.*}}i32 6
+//CHECK: insertelement{{.*}}i32 7
+  return _mm512_set_epi64(__A, __B, __C, __D, __E, __F, __G, __H );
+}
+
+__m512d test_mm512_set_pd (double __A, double __B, double __C, double __D,
+   double __E, double __F, double __G, double __H)
+{
+//CHECK-LABLE: @test_mm512_set_pd
+//CHECK: insertelement{{.*}}i32 0
+//CHECK: insertelement{{.*}}i32 1
+//CHECK: insertelement{{.*}}i32 2
+//CHECK: insertelement{{.*}}i32 3
+//CHECK: insertelement{{.*}}i32 4
+//CHECK: insertelement{{.*}}i32 5
+//CHECK: insertelement{{.*}}i32 6
+//CHECK: insertelement{{.*}}i32 7
+  return _mm512_set_pd( __A, __B, __C, __D, __E, __F, __G, __H);
+}
+
+__m512 test_mm512_set_ps (float __A, float __B, float __C, float __D,
+  float __E, float __F, float __G, float __H,
+  float __I, float __J, float __K, float __L,
+  float __M, float __N, float __O, float __P)
+{
+//CHECK-LABLE: @test_mm512_set_ps
+//CHECK: insertelement{{.*}}i32 0
+//CHECK: insertelement{{.*}}i32 1
+//CHECK: insertelement{{.*}}i32 2
+//CHECK: insertelement{{.*}}

Re: [PATCH] D20158: [find-all-symbols] Save relative file path for each symbol.

2016-05-11 Thread Haojian Wu via cfe-commits
hokein added inline comments.


Comment at: include-fixer/find-all-symbols/SymbolInfo.h:81
@@ -81,1 +80,3 @@
+  /// \brief The file path where the symbol comes from. It's a relative file
+  /// path based on the build directory.
   std::string FilePath;

bkramer wrote:
> hokein wrote:
> > One question is that whether should we add a `BuildDir` in `SymbolInfo`. 
> > With `BuildDir`, we can check the build directory in include-fixer make 
> > sure they are the same :).
> Hmm. If both the symbol finder and include fixer run on the same compilation 
> database this can never happen, right?
Yeah. So don't do it now.


Repository:
  rL LLVM

http://reviews.llvm.org/D20158



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


Re: [PATCH] D20158: [find-all-symbols] Save relative file path for each symbol.

2016-05-11 Thread Haojian Wu via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL269173: [find-all-symbols] Save relative file path for each 
symbol. (authored by hokein).

Changed prior to commit:
  http://reviews.llvm.org/D20158?vs=56879&id=56881#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D20158

Files:
  clang-tools-extra/trunk/include-fixer/find-all-symbols/FindAllSymbols.cpp
  clang-tools-extra/trunk/include-fixer/find-all-symbols/SymbolInfo.h

Index: clang-tools-extra/trunk/include-fixer/find-all-symbols/FindAllSymbols.cpp
===
--- clang-tools-extra/trunk/include-fixer/find-all-symbols/FindAllSymbols.cpp
+++ clang-tools-extra/trunk/include-fixer/find-all-symbols/FindAllSymbols.cpp
@@ -17,7 +17,6 @@
 #include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/Support/FileSystem.h"
-#include "llvm/Support/Path.h"
 
 using namespace clang::ast_matchers;
 
@@ -76,21 +75,7 @@
   if (FilePath.empty())
 return llvm::None;
 
-  llvm::SmallString<128> AbsolutePath;
-  if (llvm::sys::path::is_absolute(FilePath)) {
-AbsolutePath = FilePath;
-  } else {
-auto WorkingDir = SM.getFileManager()
-  .getVirtualFileSystem()
-  ->getCurrentWorkingDirectory();
-if (!WorkingDir)
-  return llvm::None;
-AbsolutePath = *WorkingDir;
-llvm::sys::path::append(AbsolutePath, FilePath);
-  }
-
-  llvm::sys::path::remove_dots(AbsolutePath, true);
-  return SymbolInfo(ND->getNameAsString(), Type, AbsolutePath.str(),
+  return SymbolInfo(ND->getNameAsString(), Type, FilePath.str(),
 GetContexts(ND), SM.getExpansionLineNumber(Loc));
 }
 
Index: clang-tools-extra/trunk/include-fixer/find-all-symbols/SymbolInfo.h
===
--- clang-tools-extra/trunk/include-fixer/find-all-symbols/SymbolInfo.h
+++ clang-tools-extra/trunk/include-fixer/find-all-symbols/SymbolInfo.h
@@ -55,7 +55,7 @@
   /// \brief Get symbol type.
   SymbolKind getSymbolKind() const;
 
-  /// \brief Get the file path where symbol comes from
+  /// \brief Get a relative file path where symbol comes from.
   llvm::StringRef getFilePath() const;
 
   /// \brief Get symbol contexts.
@@ -77,7 +77,8 @@
   /// \brief Symbol type.
   SymbolKind Type;
 
-  /// \brief The file path where the symbol comes from.
+  /// \brief The file path where the symbol comes from. It's a relative file
+  /// path based on the build directory.
   std::string FilePath;
 
   /// \brief Contains information about symbol contexts. Context information is


Index: clang-tools-extra/trunk/include-fixer/find-all-symbols/FindAllSymbols.cpp
===
--- clang-tools-extra/trunk/include-fixer/find-all-symbols/FindAllSymbols.cpp
+++ clang-tools-extra/trunk/include-fixer/find-all-symbols/FindAllSymbols.cpp
@@ -17,7 +17,6 @@
 #include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/Support/FileSystem.h"
-#include "llvm/Support/Path.h"
 
 using namespace clang::ast_matchers;
 
@@ -76,21 +75,7 @@
   if (FilePath.empty())
 return llvm::None;
 
-  llvm::SmallString<128> AbsolutePath;
-  if (llvm::sys::path::is_absolute(FilePath)) {
-AbsolutePath = FilePath;
-  } else {
-auto WorkingDir = SM.getFileManager()
-  .getVirtualFileSystem()
-  ->getCurrentWorkingDirectory();
-if (!WorkingDir)
-  return llvm::None;
-AbsolutePath = *WorkingDir;
-llvm::sys::path::append(AbsolutePath, FilePath);
-  }
-
-  llvm::sys::path::remove_dots(AbsolutePath, true);
-  return SymbolInfo(ND->getNameAsString(), Type, AbsolutePath.str(),
+  return SymbolInfo(ND->getNameAsString(), Type, FilePath.str(),
 GetContexts(ND), SM.getExpansionLineNumber(Loc));
 }
 
Index: clang-tools-extra/trunk/include-fixer/find-all-symbols/SymbolInfo.h
===
--- clang-tools-extra/trunk/include-fixer/find-all-symbols/SymbolInfo.h
+++ clang-tools-extra/trunk/include-fixer/find-all-symbols/SymbolInfo.h
@@ -55,7 +55,7 @@
   /// \brief Get symbol type.
   SymbolKind getSymbolKind() const;
 
-  /// \brief Get the file path where symbol comes from
+  /// \brief Get a relative file path where symbol comes from.
   llvm::StringRef getFilePath() const;
 
   /// \brief Get symbol contexts.
@@ -77,7 +77,8 @@
   /// \brief Symbol type.
   SymbolKind Type;
 
-  /// \brief The file path where the symbol comes from.
+  /// \brief The file path where the symbol comes from. It's a relative file
+  /// path based on the build directory.
   std::string FilePath;
 
   /// \brief Contains information about symbol contexts. Context information is
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo

[clang-tools-extra] r269173 - [find-all-symbols] Save relative file path for each symbol.

2016-05-11 Thread Haojian Wu via cfe-commits
Author: hokein
Date: Wed May 11 06:50:08 2016
New Revision: 269173

URL: http://llvm.org/viewvc/llvm-project?rev=269173&view=rev
Log:
[find-all-symbols] Save relative file path for each symbol.

Summary:
The HeaderSearch::suggestPathToFileForDiagnostics used in include-fixer
doesn't work well with absolute file path, it assumes a relative file
path based on header searching path.

So saving relative file path to make include-fixer get the most
appropriate header path.

Reviewers: bkramer

Subscribers: cfe-commits

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

Modified:
clang-tools-extra/trunk/include-fixer/find-all-symbols/FindAllSymbols.cpp
clang-tools-extra/trunk/include-fixer/find-all-symbols/SymbolInfo.h

Modified: 
clang-tools-extra/trunk/include-fixer/find-all-symbols/FindAllSymbols.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/include-fixer/find-all-symbols/FindAllSymbols.cpp?rev=269173&r1=269172&r2=269173&view=diff
==
--- clang-tools-extra/trunk/include-fixer/find-all-symbols/FindAllSymbols.cpp 
(original)
+++ clang-tools-extra/trunk/include-fixer/find-all-symbols/FindAllSymbols.cpp 
Wed May 11 06:50:08 2016
@@ -17,7 +17,6 @@
 #include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/Support/FileSystem.h"
-#include "llvm/Support/Path.h"
 
 using namespace clang::ast_matchers;
 
@@ -76,21 +75,7 @@ llvm::Optional CreateSymbolI
   if (FilePath.empty())
 return llvm::None;
 
-  llvm::SmallString<128> AbsolutePath;
-  if (llvm::sys::path::is_absolute(FilePath)) {
-AbsolutePath = FilePath;
-  } else {
-auto WorkingDir = SM.getFileManager()
-  .getVirtualFileSystem()
-  ->getCurrentWorkingDirectory();
-if (!WorkingDir)
-  return llvm::None;
-AbsolutePath = *WorkingDir;
-llvm::sys::path::append(AbsolutePath, FilePath);
-  }
-
-  llvm::sys::path::remove_dots(AbsolutePath, true);
-  return SymbolInfo(ND->getNameAsString(), Type, AbsolutePath.str(),
+  return SymbolInfo(ND->getNameAsString(), Type, FilePath.str(),
 GetContexts(ND), SM.getExpansionLineNumber(Loc));
 }
 

Modified: clang-tools-extra/trunk/include-fixer/find-all-symbols/SymbolInfo.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/include-fixer/find-all-symbols/SymbolInfo.h?rev=269173&r1=269172&r2=269173&view=diff
==
--- clang-tools-extra/trunk/include-fixer/find-all-symbols/SymbolInfo.h 
(original)
+++ clang-tools-extra/trunk/include-fixer/find-all-symbols/SymbolInfo.h Wed May 
11 06:50:08 2016
@@ -55,7 +55,7 @@ public:
   /// \brief Get symbol type.
   SymbolKind getSymbolKind() const;
 
-  /// \brief Get the file path where symbol comes from
+  /// \brief Get a relative file path where symbol comes from.
   llvm::StringRef getFilePath() const;
 
   /// \brief Get symbol contexts.
@@ -77,7 +77,8 @@ private:
   /// \brief Symbol type.
   SymbolKind Type;
 
-  /// \brief The file path where the symbol comes from.
+  /// \brief The file path where the symbol comes from. It's a relative file
+  /// path based on the build directory.
   std::string FilePath;
 
   /// \brief Contains information about symbol contexts. Context information is


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


Re: [PATCH] D19105: Changes in clang after running http://reviews.llvm.org/D18821

2016-05-11 Thread Piotr Padlewski via cfe-commits
Prazek added a comment.

In http://reviews.llvm.org/D19105#426903, @alexfh wrote:

> returning a bool from a function that is declared to return a typedef to an 
> integral type that contains `bool` in its name (e.g. `LLVMBool`), and maybe 
> some other cases.


Isn't it LLVMBool issue?

I won't have enough time to fix it, so I would prefer to comment fixit and only 
push warnings.


http://reviews.llvm.org/D19105



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


Re: [PATCH] D20159: [include-fixer] Add lit-test for relative include path.

2016-05-11 Thread Benjamin Kramer via cfe-commits
bkramer accepted this revision.
bkramer added a comment.
This revision is now accepted and ready to land.

Make sure to watch the buildbots when this goes in, there's a lot of 
potentially system-dependent shell complexity in the test.


Repository:
  rL LLVM

http://reviews.llvm.org/D20159



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


Re: [PATCH] D20136: Get default -fms-compatibility-version from cl.exe's version

2016-05-11 Thread Nico Weber via cfe-commits
thakis added inline comments.


Comment at: lib/Driver/MSVCToolChain.cpp:478
@@ +477,3 @@
+
+  const DWORD VersionSize = ::GetFileVersionInfoSizeW(ClExeWide.c_str(),
+  nullptr);

amccarth wrote:
> Yes, it looks in the executable (which I tried to emphasize with the method 
> name).
> 
> I don't think this is very expensive given that Explorer often makes zillions 
> of such calls, but I'm open to other suggestions.
> 
> I know that you can't use a library that's newer than the compiler (because 
> it may use new language features), but I don't know if that applies in the 
> other direction or how we would safely and reliably map directory names to 
> library versions and therefore to compiler versions.
I agree that figuring out the right value for fmsc-version automatically 
somehow is definitely something we should do.

I forgot that `getVisualStudioBinariesFolder` already works by looking for 
cl.exe in PATH, so cl.exe's metadata is already warmed up in the disk cache. 
However, GetFileVersionInfoW() probably opens cl.exe itself and does some PE 
parsing to get at the version, and that probably is in cold cache territory. 
(https://msdn.microsoft.com/en-us/library/windows/desktop/ms647003(v=vs.85).aspx
 suggests that this function might open several files).

`getVisualStudioBinariesFolder` checks:

1. getenv("VCINSTALLDIR")
2. cl.exe in getenv("PATH")
3. registry (via getVisualStudioInstallDir)

The common cases are 1 and 3. For 1, for default installs, the version number 
is part of the directory name (for default installs, what most people have). 
For 3, the version number is in the registry key we query. So in most cases we 
shouldn't have to look at cl.exe itself. And for the cases where we would have 
to look, maybe it's ok to require an explicit fmsc-version flag.


http://reviews.llvm.org/D20136



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


Re: [PATCH] D20159: [include-fixer] Add lit-test for relative include path.

2016-05-11 Thread Haojian Wu via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL269177: [include-fixer] Add lit-test for relative include 
path. (authored by hokein).

Changed prior to commit:
  http://reviews.llvm.org/D20159?vs=56884&id=56886#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D20159

Files:
  clang-tools-extra/trunk/test/include-fixer/Inputs/database_template.json
  clang-tools-extra/trunk/test/include-fixer/Inputs/fake_yaml_db.yaml
  clang-tools-extra/trunk/test/include-fixer/include_path.cpp

Index: clang-tools-extra/trunk/test/include-fixer/Inputs/database_template.json
===
--- clang-tools-extra/trunk/test/include-fixer/Inputs/database_template.json
+++ clang-tools-extra/trunk/test/include-fixer/Inputs/database_template.json
@@ -0,0 +1,7 @@
+[
+{
+  "directory": "test_dir/build",
+  "command": "clang++ -I../include -o bar.o test_dir/src/bar.cpp",
+  "file": "test_dir/src/bar.cpp"
+}
+]
Index: clang-tools-extra/trunk/test/include-fixer/Inputs/fake_yaml_db.yaml
===
--- clang-tools-extra/trunk/test/include-fixer/Inputs/fake_yaml_db.yaml
+++ clang-tools-extra/trunk/test/include-fixer/Inputs/fake_yaml_db.yaml
@@ -9,3 +9,14 @@
 LineNumber:  1
 Type:Class
 ...
+---
+Name:   bar
+Contexts:
+  - ContextType: Namespace
+ContextName: a
+  - ContextType: Namespace
+ContextName: b
+FilePath:../include/bar.h
+LineNumber:  1
+Type:Class
+...
Index: clang-tools-extra/trunk/test/include-fixer/include_path.cpp
===
--- clang-tools-extra/trunk/test/include-fixer/include_path.cpp
+++ clang-tools-extra/trunk/test/include-fixer/include_path.cpp
@@ -0,0 +1,14 @@
+// REQUIRES: shell
+// RUN: mkdir -p %T/include-fixer/include
+// RUN: mkdir -p %T/include-fixer/build
+// RUN: mkdir -p %T/include-fixer/src
+// RUN: sed 's|test_dir|%T/include-fixer|g' %S/Inputs/database_template.json > 
%T/include-fixer/build/compile_commands.json
+// RUN: cp %S/Inputs/fake_yaml_db.yaml %T/include-fixer/build/fake_yaml_db.yaml
+// RUN: echo 'b::a::bar f;' > %T/include-fixer/src/bar.cpp
+// RUN: touch %T/include-fixer/include/bar.h
+// RUN: cd %T/include-fixer/build
+// RUN: clang-include-fixer -db=yaml -input=fake_yaml_db.yaml -p=. 
%T/include-fixer/src/bar.cpp
+// RUN: FileCheck -input-file=%T/include-fixer/src/bar.cpp %s
+
+// CHECK: #include "bar.h"
+// CHECK: b::a::bar f;


Index: clang-tools-extra/trunk/test/include-fixer/Inputs/database_template.json
===
--- clang-tools-extra/trunk/test/include-fixer/Inputs/database_template.json
+++ clang-tools-extra/trunk/test/include-fixer/Inputs/database_template.json
@@ -0,0 +1,7 @@
+[
+{
+  "directory": "test_dir/build",
+  "command": "clang++ -I../include -o bar.o test_dir/src/bar.cpp",
+  "file": "test_dir/src/bar.cpp"
+}
+]
Index: clang-tools-extra/trunk/test/include-fixer/Inputs/fake_yaml_db.yaml
===
--- clang-tools-extra/trunk/test/include-fixer/Inputs/fake_yaml_db.yaml
+++ clang-tools-extra/trunk/test/include-fixer/Inputs/fake_yaml_db.yaml
@@ -9,3 +9,14 @@
 LineNumber:  1
 Type:Class
 ...
+---
+Name:   bar
+Contexts:
+  - ContextType: Namespace
+ContextName: a
+  - ContextType: Namespace
+ContextName: b
+FilePath:../include/bar.h
+LineNumber:  1
+Type:Class
+...
Index: clang-tools-extra/trunk/test/include-fixer/include_path.cpp
===
--- clang-tools-extra/trunk/test/include-fixer/include_path.cpp
+++ clang-tools-extra/trunk/test/include-fixer/include_path.cpp
@@ -0,0 +1,14 @@
+// REQUIRES: shell
+// RUN: mkdir -p %T/include-fixer/include
+// RUN: mkdir -p %T/include-fixer/build
+// RUN: mkdir -p %T/include-fixer/src
+// RUN: sed 's|test_dir|%T/include-fixer|g' %S/Inputs/database_template.json > %T/include-fixer/build/compile_commands.json
+// RUN: cp %S/Inputs/fake_yaml_db.yaml %T/include-fixer/build/fake_yaml_db.yaml
+// RUN: echo 'b::a::bar f;' > %T/include-fixer/src/bar.cpp
+// RUN: touch %T/include-fixer/include/bar.h
+// RUN: cd %T/include-fixer/build
+// RUN: clang-include-fixer -db=yaml -input=fake_yaml_db.yaml -p=. %T/include-fixer/src/bar.cpp
+// RUN: FileCheck -input-file=%T/include-fixer/src/bar.cpp %s
+
+// CHECK: #include "bar.h"
+// CHECK: b::a::bar f;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] r269177 - [include-fixer] Add lit-test for relative include path.

2016-05-11 Thread Haojian Wu via cfe-commits
Author: hokein
Date: Wed May 11 07:30:45 2016
New Revision: 269177

URL: http://llvm.org/viewvc/llvm-project?rev=269177&view=rev
Log:
[include-fixer] Add lit-test for relative include path.

Reviewers: bkramer

Subscribers: cfe-commits

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

Added:
clang-tools-extra/trunk/test/include-fixer/Inputs/database_template.json
clang-tools-extra/trunk/test/include-fixer/include_path.cpp
Modified:
clang-tools-extra/trunk/test/include-fixer/Inputs/fake_yaml_db.yaml

Added: clang-tools-extra/trunk/test/include-fixer/Inputs/database_template.json
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/include-fixer/Inputs/database_template.json?rev=269177&view=auto
==
--- clang-tools-extra/trunk/test/include-fixer/Inputs/database_template.json 
(added)
+++ clang-tools-extra/trunk/test/include-fixer/Inputs/database_template.json 
Wed May 11 07:30:45 2016
@@ -0,0 +1,7 @@
+[
+{
+  "directory": "test_dir/build",
+  "command": "clang++ -I../include -o bar.o test_dir/src/bar.cpp",
+  "file": "test_dir/src/bar.cpp"
+}
+]

Modified: clang-tools-extra/trunk/test/include-fixer/Inputs/fake_yaml_db.yaml
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/include-fixer/Inputs/fake_yaml_db.yaml?rev=269177&r1=269176&r2=269177&view=diff
==
--- clang-tools-extra/trunk/test/include-fixer/Inputs/fake_yaml_db.yaml 
(original)
+++ clang-tools-extra/trunk/test/include-fixer/Inputs/fake_yaml_db.yaml Wed May 
11 07:30:45 2016
@@ -9,3 +9,14 @@ FilePath:foo.h
 LineNumber:  1
 Type:Class
 ...
+---
+Name:   bar
+Contexts:
+  - ContextType: Namespace
+ContextName: a
+  - ContextType: Namespace
+ContextName: b
+FilePath:../include/bar.h
+LineNumber:  1
+Type:Class
+...

Added: clang-tools-extra/trunk/test/include-fixer/include_path.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/include-fixer/include_path.cpp?rev=269177&view=auto
==
--- clang-tools-extra/trunk/test/include-fixer/include_path.cpp (added)
+++ clang-tools-extra/trunk/test/include-fixer/include_path.cpp Wed May 11 
07:30:45 2016
@@ -0,0 +1,14 @@
+// REQUIRES: shell
+// RUN: mkdir -p %T/include-fixer/include
+// RUN: mkdir -p %T/include-fixer/build
+// RUN: mkdir -p %T/include-fixer/src
+// RUN: sed 's|test_dir|%T/include-fixer|g' %S/Inputs/database_template.json > 
%T/include-fixer/build/compile_commands.json
+// RUN: cp %S/Inputs/fake_yaml_db.yaml %T/include-fixer/build/fake_yaml_db.yaml
+// RUN: echo 'b::a::bar f;' > %T/include-fixer/src/bar.cpp
+// RUN: touch %T/include-fixer/include/bar.h
+// RUN: cd %T/include-fixer/build
+// RUN: clang-include-fixer -db=yaml -input=fake_yaml_db.yaml -p=. 
%T/include-fixer/src/bar.cpp
+// RUN: FileCheck -input-file=%T/include-fixer/src/bar.cpp %s
+
+// CHECK: #include "bar.h"
+// CHECK: b::a::bar f;


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


[clang-tools-extra] r269171 - [clang-tidy] Adds modernize-use-bool-literals check.

2016-05-11 Thread Jakub Staron via cfe-commits
Author: staronj
Date: Wed May 11 06:33:16 2016
New Revision: 269171

URL: http://llvm.org/viewvc/llvm-project?rev=269171&view=rev
Log:
[clang-tidy] Adds modernize-use-bool-literals check.

Review link: http://reviews.llvm.org/D18745

Added:
clang-tools-extra/trunk/clang-tidy/modernize/UseBoolLiteralsCheck.cpp
clang-tools-extra/trunk/clang-tidy/modernize/UseBoolLiteralsCheck.h

clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-use-bool-literals.rst
clang-tools-extra/trunk/test/clang-tidy/modernize-use-bool-literals.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/modernize/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/modernize/ModernizeTidyModule.cpp
clang-tools-extra/trunk/docs/ReleaseNotes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst

Modified: clang-tools-extra/trunk/clang-tidy/modernize/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/CMakeLists.txt?rev=269171&r1=269170&r2=269171&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/modernize/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/modernize/CMakeLists.txt Wed May 11 
06:33:16 2016
@@ -14,6 +14,7 @@ add_clang_library(clangTidyModernizeModu
   ReplaceAutoPtrCheck.cpp
   ShrinkToFitCheck.cpp
   UseAutoCheck.cpp
+  UseBoolLiteralsCheck.cpp
   UseDefaultCheck.cpp
   UseNullptrCheck.cpp
   UseOverrideCheck.cpp

Modified: clang-tools-extra/trunk/clang-tidy/modernize/ModernizeTidyModule.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/ModernizeTidyModule.cpp?rev=269171&r1=269170&r2=269171&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/modernize/ModernizeTidyModule.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/modernize/ModernizeTidyModule.cpp Wed 
May 11 06:33:16 2016
@@ -20,6 +20,7 @@
 #include "ReplaceAutoPtrCheck.h"
 #include "ShrinkToFitCheck.h"
 #include "UseAutoCheck.h"
+#include "UseBoolLiteralsCheck.h"
 #include "UseDefaultCheck.h"
 #include "UseNullptrCheck.h"
 #include "UseOverrideCheck.h"
@@ -47,6 +48,8 @@ public:
 "modernize-replace-auto-ptr");
 CheckFactories.registerCheck("modernize-shrink-to-fit");
 CheckFactories.registerCheck("modernize-use-auto");
+CheckFactories.registerCheck(
+"modernize-use-bool-literals");
 CheckFactories.registerCheck("modernize-use-default");
 CheckFactories.registerCheck("modernize-use-nullptr");
 CheckFactories.registerCheck("modernize-use-override");

Added: clang-tools-extra/trunk/clang-tidy/modernize/UseBoolLiteralsCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/UseBoolLiteralsCheck.cpp?rev=269171&view=auto
==
--- clang-tools-extra/trunk/clang-tidy/modernize/UseBoolLiteralsCheck.cpp 
(added)
+++ clang-tools-extra/trunk/clang-tidy/modernize/UseBoolLiteralsCheck.cpp Wed 
May 11 06:33:16 2016
@@ -0,0 +1,55 @@
+//===--- UseBoolLiteralsCheck.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 "UseBoolLiteralsCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+void UseBoolLiteralsCheck::registerMatchers(MatchFinder *Finder) {
+  if (!getLangOpts().CPlusPlus)
+return;
+
+  Finder->addMatcher(
+  implicitCastExpr(
+  has(integerLiteral().bind("literal")),
+  hasImplicitDestinationType(qualType(booleanType())),
+  unless(isInTemplateInstantiation()),
+  anyOf(hasParent(explicitCastExpr().bind("cast")), anything())),
+  this);
+}
+
+void UseBoolLiteralsCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *Literal = Result.Nodes.getNodeAs("literal");
+  const auto *Cast = Result.Nodes.getNodeAs("cast");
+  bool LiteralBooleanValue = Literal->getValue().getBoolValue();
+
+  if (Literal->isInstantiationDependent())
+return;
+
+  const Expr *Expression = Cast ? Cast : Literal;
+
+  auto Diag =
+  diag(Expression->getExprLoc(),
+   "converting integer literal to bool, use bool literal instead");
+
+  if (!Expression->getLocStart().isMacroID())
+Diag << FixItHint::CreateReplacement(
+Expression->getSourceRange(), LiteralBooleanValue ? "true" : "false");
+}
+
+} // namespace modernize
+} // namespace tidy
+} // namespace clang

Added: clang-tools-extra/trunk/clang-tidy/modernize/UseBoolLiteralsCheck.h
U

Re: [PATCH] D17983: Eliminate many benign instances of "potentially uninitialized local variable" warnings

2016-05-11 Thread Hannah von Reth via cfe-commits
TheOneRing added a subscriber: TheOneRing.
TheOneRing added a comment.

Hi is there any update on this issue, which is blocking 
https://llvm.org/bugs/show_bug.cgi?id=11446 ?


http://reviews.llvm.org/D17983



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


Re: [PATCH] D20136: Get default -fms-compatibility-version from cl.exe's version

2016-05-11 Thread Aaron Ballman via cfe-commits
On Wed, May 11, 2016 at 8:29 AM, Nico Weber via cfe-commits
 wrote:
> thakis added inline comments.
>
> 
> Comment at: lib/Driver/MSVCToolChain.cpp:478
> @@ +477,3 @@
> +
> +  const DWORD VersionSize = ::GetFileVersionInfoSizeW(ClExeWide.c_str(),
> +  nullptr);
> 
> amccarth wrote:
>> Yes, it looks in the executable (which I tried to emphasize with the method 
>> name).
>>
>> I don't think this is very expensive given that Explorer often makes 
>> zillions of such calls, but I'm open to other suggestions.
>>
>> I know that you can't use a library that's newer than the compiler (because 
>> it may use new language features), but I don't know if that applies in the 
>> other direction or how we would safely and reliably map directory names to 
>> library versions and therefore to compiler versions.
> I agree that figuring out the right value for fmsc-version automatically 
> somehow is definitely something we should do.
>
> I forgot that `getVisualStudioBinariesFolder` already works by looking for 
> cl.exe in PATH, so cl.exe's metadata is already warmed up in the disk cache. 
> However, GetFileVersionInfoW() probably opens cl.exe itself and does some PE 
> parsing to get at the version, and that probably is in cold cache territory. 
> (https://msdn.microsoft.com/en-us/library/windows/desktop/ms647003(v=vs.85).aspx
>  suggests that this function might open several files).
>
> `getVisualStudioBinariesFolder` checks:
>
> 1. getenv("VCINSTALLDIR")
> 2. cl.exe in getenv("PATH")
> 3. registry (via getVisualStudioInstallDir)
>
> The common cases are 1 and 3. For 1, for default installs, the version number 
> is part of the directory name (for default installs, what most people have). 
> For 3, the version number is in the registry key we query. So in most cases 
> we shouldn't have to look at cl.exe itself. And for the cases where we would 
> have to look, maybe it's ok to require an explicit fmsc-version flag.

I agree that in most cases we shouldn't have to look at cl.exe itself,
but I think it's better for the users in the other case that we just
open cl.exe instead of force them to specify a flag that we could just
query ourselves. Yes, it's a performance hit (though I don't know that
I've seen any measurements to suggest it's a bad perf hit in
practice). However, it's also a usability gain and would be consistent
behavior with default installs.

~Aaron

>
>
> http://reviews.llvm.org/D20136
>
>
>
> ___
> 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] D19780: Output OpenCL version in Clang diagnostics

2016-05-11 Thread Vedran Miletić via cfe-commits
rivanvx added a comment.

Please, can anyone push this?


http://reviews.llvm.org/D19780



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


Re: [PATCH] D20125: [libclang] Added clang_getRealSpellingLocation to compensate for clang_getSpellingLocation returning the expansion location

2016-05-11 Thread Cameron via cfe-commits
cameron314 added a comment.

Ah, I was wondering where the tests were. I found this in the CMake file:

  # FIXME: libclang unit tests are disabled on Windows due
  # to failures, mostly in libclang.VirtualFileOverlay_*.
  if(NOT WIN32 AND CLANG_TOOL_LIBCLANG_BUILD) 
add_subdirectory(libclang)
  endif()

I'll ignore the VFO failures for now, and add a new test for 
clang_getRealSpellingLocation :-)


http://reviews.llvm.org/D20125



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


[PATCH] D20168: [CodeGen] Handle structs directly in AMDGPUABIInfo

2016-05-11 Thread Vedran Miletić via cfe-commits
rivanvx created this revision.
rivanvx added reviewers: arsenm, tstellarAMD.
rivanvx added a subscriber: cfe-commits.
Herald added a subscriber: kzhuravl.

Structs are currently handled as pointer + byval, which makes AMDGPU LLVM 
backend generate incorrect code when structs are used. This patch changes 
struct argument to be handled directly and without flattening, which Clover 
(Mesa 3D Gallium OpenCL state tracker) will be able to handle. Flattening would 
expand the struct to individual elements and pass each as a separate argument, 
which Clover can not handle. Furthermore, such expansion does not fit the 
OpenCL programming model which requires to explicitely specify each argument 
index, size and memory location.

This patch is a modification of a patch provided by Matt Arsenault.

http://reviews.llvm.org/D20168

Files:
  lib/CodeGen/TargetInfo.cpp

Index: lib/CodeGen/TargetInfo.cpp
===
--- lib/CodeGen/TargetInfo.cpp
+++ lib/CodeGen/TargetInfo.cpp
@@ -6808,10 +6808,41 @@
 
 namespace {
 
+class AMDGPUABIInfo final : public DefaultABIInfo {
+public:
+  explicit AMDGPUABIInfo(CodeGen::CodeGenTypes &CGT) : DefaultABIInfo(CGT) {}
+
+private:
+  ABIArgInfo classifyArgumentType(QualType Ty) const;
+
+  void computeInfo(CGFunctionInfo &FI) const override;
+};
+
+void AMDGPUABIInfo::computeInfo(CGFunctionInfo &FI) const {
+  if (!getCXXABI().classifyReturnType(FI))
+FI.getReturnInfo() = classifyReturnType(FI.getReturnType());
+
+  for (auto &Arg : FI.arguments())
+Arg.info = classifyArgumentType(Arg.type);
+}
+
+/// \brief Classify argument of given type \p Ty.
+ABIArgInfo AMDGPUABIInfo::classifyArgumentType(QualType Ty) const {
+  llvm::StructType *StrTy = dyn_cast(CGT.ConvertType(Ty));
+  if (!StrTy) {
+return DefaultABIInfo::classifyArgumentType(Ty);
+  }
+
+  // If we set CanBeFlattened to true, CodeGen will expand the struct to its
+  // individual elements, which confuses the Clover OpenCL backend; therefore 
we
+  // have to set it to false here. Other args of getDirect() are just defaults.
+  return ABIArgInfo::getDirect(nullptr, 0, nullptr, false);
+}
+
 class AMDGPUTargetCodeGenInfo : public TargetCodeGenInfo {
 public:
   AMDGPUTargetCodeGenInfo(CodeGenTypes &CGT)
-: TargetCodeGenInfo(new DefaultABIInfo(CGT)) {}
+: TargetCodeGenInfo(new AMDGPUABIInfo(CGT)) {}
   void setTargetAttributes(const Decl *D, llvm::GlobalValue *GV,
CodeGen::CodeGenModule &M) const override;
 };


Index: lib/CodeGen/TargetInfo.cpp
===
--- lib/CodeGen/TargetInfo.cpp
+++ lib/CodeGen/TargetInfo.cpp
@@ -6808,10 +6808,41 @@
 
 namespace {
 
+class AMDGPUABIInfo final : public DefaultABIInfo {
+public:
+  explicit AMDGPUABIInfo(CodeGen::CodeGenTypes &CGT) : DefaultABIInfo(CGT) {}
+
+private:
+  ABIArgInfo classifyArgumentType(QualType Ty) const;
+
+  void computeInfo(CGFunctionInfo &FI) const override;
+};
+
+void AMDGPUABIInfo::computeInfo(CGFunctionInfo &FI) const {
+  if (!getCXXABI().classifyReturnType(FI))
+FI.getReturnInfo() = classifyReturnType(FI.getReturnType());
+
+  for (auto &Arg : FI.arguments())
+Arg.info = classifyArgumentType(Arg.type);
+}
+
+/// \brief Classify argument of given type \p Ty.
+ABIArgInfo AMDGPUABIInfo::classifyArgumentType(QualType Ty) const {
+  llvm::StructType *StrTy = dyn_cast(CGT.ConvertType(Ty));
+  if (!StrTy) {
+return DefaultABIInfo::classifyArgumentType(Ty);
+  }
+
+  // If we set CanBeFlattened to true, CodeGen will expand the struct to its
+  // individual elements, which confuses the Clover OpenCL backend; therefore we
+  // have to set it to false here. Other args of getDirect() are just defaults.
+  return ABIArgInfo::getDirect(nullptr, 0, nullptr, false);
+}
+
 class AMDGPUTargetCodeGenInfo : public TargetCodeGenInfo {
 public:
   AMDGPUTargetCodeGenInfo(CodeGenTypes &CGT)
-: TargetCodeGenInfo(new DefaultABIInfo(CGT)) {}
+: TargetCodeGenInfo(new AMDGPUABIInfo(CGT)) {}
   void setTargetAttributes(const Decl *D, llvm::GlobalValue *GV,
CodeGen::CodeGenModule &M) const override;
 };
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D19274: Compilation for Intel MCU (Part 2/3)

2016-05-11 Thread Andrey Turetskiy via cfe-commits
aturetsk added a comment.

Ping.


http://reviews.llvm.org/D19274



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


Re: [PATCH] D19941: [tooling] FixItHint Tooling refactoring

2016-05-11 Thread Etienne Bergeron via cfe-commits
etienneb updated this revision to Diff 56907.
etienneb marked an inline comment as done.
etienneb added a comment.

fix nits


http://reviews.llvm.org/D19941

Files:
  include/clang/Tooling/FixIt.h
  lib/Tooling/CMakeLists.txt
  lib/Tooling/FixIt.cpp
  unittests/Tooling/CMakeLists.txt
  unittests/Tooling/FixItTest.cpp

Index: unittests/Tooling/FixItTest.cpp
===
--- /dev/null
+++ unittests/Tooling/FixItTest.cpp
@@ -0,0 +1,232 @@
+//===- unittest/Tooling/FixitTest.cpp ===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "TestVisitor.h"
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Tooling/FixIt.h"
+
+using namespace clang;
+
+using tooling::fixit::getText;
+using tooling::fixit::createRemoval;
+using tooling::fixit::createReplacement;
+
+namespace {
+
+struct CallsVisitor : TestVisitor {
+  bool VisitCallExpr(CallExpr *Expr) {
+OnCall(Expr, Context);
+return true;
+  }
+
+  std::function OnCall;
+};
+
+std::string LocationToString(SourceLocation Loc, ASTContext *Context) {
+  return Loc.printToString(Context->getSourceManager());
+}
+
+TEST(FixItTest, getText) {
+  CallsVisitor Visitor;
+
+  Visitor.OnCall = [](CallExpr *CE, ASTContext *Context) {
+EXPECT_EQ("foo(x, y)", getText(*CE, *Context));
+EXPECT_EQ("foo(x, y)", getText(CE->getSourceRange(), *Context));
+
+Expr *P0 = CE->getArg(0);
+Expr *P1 = CE->getArg(1);
+EXPECT_EQ("x", getText(*P0, *Context));
+EXPECT_EQ("y", getText(*P1, *Context));
+  };
+  Visitor.runOver("void foo(int x, int y) { foo(x, y); }");
+
+  Visitor.OnCall = [](CallExpr *CE, ASTContext *Context) {
+EXPECT_EQ("APPLY(foo, x, y)", getText(*CE, *Context));
+  };
+  Visitor.runOver("#define APPLY(f, x, y) f(x, y)\n"
+  "void foo(int x, int y) { APPLY(foo, x, y); }");
+}
+
+TEST(FixItTest, getTextWithMacro) {
+  CallsVisitor Visitor;
+
+  Visitor.OnCall = [](CallExpr *CE, ASTContext *Context) {
+EXPECT_EQ("F OO", getText(*CE, *Context));
+Expr *P0 = CE->getArg(0);
+Expr *P1 = CE->getArg(1);
+EXPECT_EQ("", getText(*P0, *Context));
+EXPECT_EQ("", getText(*P1, *Context));
+  };
+  Visitor.runOver("#define F foo(\n"
+  "#define OO x, y)\n"
+  "void foo(int x, int y) { F OO ; }");
+
+  Visitor.OnCall = [](CallExpr *CE, ASTContext *Context) {
+EXPECT_EQ("", getText(*CE, *Context));
+Expr *P0 = CE->getArg(0);
+Expr *P1 = CE->getArg(1);
+EXPECT_EQ("x", getText(*P0, *Context));
+EXPECT_EQ("y", getText(*P1, *Context));
+  };
+  Visitor.runOver("#define FOO(x, y) (void)x; (void)y; foo(x, y);\n"
+  "void foo(int x, int y) { FOO(x,y) }");
+}
+
+TEST(FixItTest, createRemoval) {
+  CallsVisitor Visitor;
+
+  Visitor.OnCall = [](CallExpr *CE, ASTContext *Context) {
+FixItHint Hint = createRemoval(*CE);
+EXPECT_EQ("foo(x, y)", getText(Hint.RemoveRange.getAsRange(), *Context));
+EXPECT_TRUE(Hint.InsertFromRange.isInvalid());
+EXPECT_TRUE(Hint.CodeToInsert.empty());
+
+Expr *P0 = CE->getArg(0);
+FixItHint Hint0 = createRemoval(*P0);
+EXPECT_EQ("x", getText(Hint0.RemoveRange.getAsRange(), *Context));
+EXPECT_TRUE(Hint0.InsertFromRange.isInvalid());
+EXPECT_TRUE(Hint0.CodeToInsert.empty());
+
+Expr *P1 = CE->getArg(1);
+FixItHint Hint1 = createRemoval(*P1);
+EXPECT_EQ("y", getText(Hint1.RemoveRange.getAsRange(), *Context));
+EXPECT_TRUE(Hint1.InsertFromRange.isInvalid());
+EXPECT_TRUE(Hint1.CodeToInsert.empty());
+  };
+  Visitor.runOver("void foo(int x, int y) { foo(x, y); }");
+
+  Visitor.OnCall = [](CallExpr *CE, ASTContext *Context) {
+Expr *P0 = CE->getArg(0);
+FixItHint Hint0 = createRemoval(*P0);
+EXPECT_EQ("x + y", getText(Hint0.RemoveRange.getAsRange(), *Context));
+
+Expr *P1 = CE->getArg(1);
+FixItHint Hint1 = createRemoval(*P1);
+EXPECT_EQ("y + x", getText(Hint1.RemoveRange.getAsRange(), *Context));
+  };
+  Visitor.runOver("void foo(int x, int y) { foo(x + y, y + x); }");
+}
+
+TEST(FixItTest, createRemovalWithMacro) {
+  CallsVisitor Visitor;
+
+  Visitor.OnCall = [](CallExpr *CE, ASTContext *Context) {
+FixItHint Hint = createRemoval(*CE);
+EXPECT_EQ("FOO", getText(Hint.RemoveRange.getAsRange(), *Context));
+EXPECT_TRUE(Hint.InsertFromRange.isInvalid());
+EXPECT_TRUE(Hint.CodeToInsert.empty());
+
+Expr *P0 = CE->getArg(0);
+FixItHint Hint0 = createRemoval(*P0);
+EXPECT_EQ("input.cc:2:26 ",
+  LocationToString(Hint0.RemoveRange.getBegin(), Context));
+EXPECT_EQ("input.cc:2:26 ",
+  LocationToString(Hint0.RemoveRange.getEnd(), Context));
+EXPECT_TRUE(Hint0.InsertFromRange.isInvalid());
+ 

Re: [PATCH] D19941: [tooling] FixItHint Tooling refactoring

2016-05-11 Thread Etienne Bergeron via cfe-commits
etienneb added a comment.

I added the "using statements".
landing.



Comment at: include/clang/Tooling/Fixit.h:35
@@ +34,3 @@
+///used by the following template abstractions.
+inline SourceRange getSourceRange(const SourceRange &Range) { return Range; }
+

alexfh wrote:
> Do we want to expose `getSourceRange` as an API? If no, it should also be in 
> the `namespace internal`.
Good question.
I moved it into the internal namespace.

If it's become needed, we can re-expose it.


Comment at: include/clang/Tooling/Fixit.h:55
@@ +54,3 @@
+// \brief Returns a FixItHint to remove \p Node.
+template  FixItHint createRemoval(const T &Node) {
+  return FixItHint::CreateRemoval(getSourceRange(Node));

alexfh wrote:
> I wonder whether we should later extend this to remove comments attached to 
> nodes like Stmt and FunctionDecl? It's out of this patches scope, but maybe 
> add a FIXME to sketch the further development plans.
I like the idea. I didn't think about comments and other syntactical elements 
that may be related to a Node. 
But, I believe we will have a other set of functions for Node and comments.

An other concept that I believe it's missing is to provide helper to append 
expression (operators) and stay semantically correct with operator precedence. 
Some cases needed to add parentheses.




Comment at: unittests/Tooling/FixitTest.cpp:53
@@ +52,3 @@
+
+TEST(FixitTest, getTextWithMacro) {
+  CallsVisitor Visitor;

alexfh wrote:
> Could you add a test where getText returns an empty string?
see line 68, already there!


Comment at: unittests/Tooling/FixitTest.cpp:119
@@ +118,3 @@
+  LocationToString(Hint0.RemoveRange.getBegin(), Context));
+EXPECT_EQ("input.cc:2:26 ",
+  LocationToString(Hint0.RemoveRange.getEnd(), Context));

alexfh wrote:
> One character range is boring. Can you make the parameter longer and also 
> verify whether the range is a token range or a character range?
In this case, it's not one character... it's one token.
As it's a "token range", the whole token is used.

But, I can use an expression too.
Not a bad idea: added but in 'createRemoval'

There is already a reand at line 138:
  EXPECT_EQ("input.cc:2:26 ",
  EXPECT_EQ("input.cc:2:26 ",


http://reviews.llvm.org/D19941



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


Re: [PATCH] D19703: [clang-tidy] Improve misc-redundant-expression and decrease false-positive

2016-05-11 Thread Etienne Bergeron via cfe-commits
etienneb updated this revision to Diff 56911.
etienneb marked an inline comment as done.
etienneb added a comment.
Herald added a subscriber: klimek.

use new API


http://reviews.llvm.org/D19703

Files:
  include/clang/Tooling/FixIt.h
  lib/Tooling/CMakeLists.txt
  lib/Tooling/FixIt.cpp
  unittests/Tooling/CMakeLists.txt
  unittests/Tooling/FixItTest.cpp

Index: unittests/Tooling/FixItTest.cpp
===
--- /dev/null
+++ unittests/Tooling/FixItTest.cpp
@@ -0,0 +1,232 @@
+//===- unittest/Tooling/FixitTest.cpp ===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "TestVisitor.h"
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Tooling/FixIt.h"
+
+using namespace clang;
+
+using tooling::fixit::getText;
+using tooling::fixit::createRemoval;
+using tooling::fixit::createReplacement;
+
+namespace {
+
+struct CallsVisitor : TestVisitor {
+  bool VisitCallExpr(CallExpr *Expr) {
+OnCall(Expr, Context);
+return true;
+  }
+
+  std::function OnCall;
+};
+
+std::string LocationToString(SourceLocation Loc, ASTContext *Context) {
+  return Loc.printToString(Context->getSourceManager());
+}
+
+TEST(FixItTest, getText) {
+  CallsVisitor Visitor;
+
+  Visitor.OnCall = [](CallExpr *CE, ASTContext *Context) {
+EXPECT_EQ("foo(x, y)", getText(*CE, *Context));
+EXPECT_EQ("foo(x, y)", getText(CE->getSourceRange(), *Context));
+
+Expr *P0 = CE->getArg(0);
+Expr *P1 = CE->getArg(1);
+EXPECT_EQ("x", getText(*P0, *Context));
+EXPECT_EQ("y", getText(*P1, *Context));
+  };
+  Visitor.runOver("void foo(int x, int y) { foo(x, y); }");
+
+  Visitor.OnCall = [](CallExpr *CE, ASTContext *Context) {
+EXPECT_EQ("APPLY(foo, x, y)", getText(*CE, *Context));
+  };
+  Visitor.runOver("#define APPLY(f, x, y) f(x, y)\n"
+  "void foo(int x, int y) { APPLY(foo, x, y); }");
+}
+
+TEST(FixItTest, getTextWithMacro) {
+  CallsVisitor Visitor;
+
+  Visitor.OnCall = [](CallExpr *CE, ASTContext *Context) {
+EXPECT_EQ("F OO", getText(*CE, *Context));
+Expr *P0 = CE->getArg(0);
+Expr *P1 = CE->getArg(1);
+EXPECT_EQ("", getText(*P0, *Context));
+EXPECT_EQ("", getText(*P1, *Context));
+  };
+  Visitor.runOver("#define F foo(\n"
+  "#define OO x, y)\n"
+  "void foo(int x, int y) { F OO ; }");
+
+  Visitor.OnCall = [](CallExpr *CE, ASTContext *Context) {
+EXPECT_EQ("", getText(*CE, *Context));
+Expr *P0 = CE->getArg(0);
+Expr *P1 = CE->getArg(1);
+EXPECT_EQ("x", getText(*P0, *Context));
+EXPECT_EQ("y", getText(*P1, *Context));
+  };
+  Visitor.runOver("#define FOO(x, y) (void)x; (void)y; foo(x, y);\n"
+  "void foo(int x, int y) { FOO(x,y) }");
+}
+
+TEST(FixItTest, createRemoval) {
+  CallsVisitor Visitor;
+
+  Visitor.OnCall = [](CallExpr *CE, ASTContext *Context) {
+FixItHint Hint = createRemoval(*CE);
+EXPECT_EQ("foo(x, y)", getText(Hint.RemoveRange.getAsRange(), *Context));
+EXPECT_TRUE(Hint.InsertFromRange.isInvalid());
+EXPECT_TRUE(Hint.CodeToInsert.empty());
+
+Expr *P0 = CE->getArg(0);
+FixItHint Hint0 = createRemoval(*P0);
+EXPECT_EQ("x", getText(Hint0.RemoveRange.getAsRange(), *Context));
+EXPECT_TRUE(Hint0.InsertFromRange.isInvalid());
+EXPECT_TRUE(Hint0.CodeToInsert.empty());
+
+Expr *P1 = CE->getArg(1);
+FixItHint Hint1 = createRemoval(*P1);
+EXPECT_EQ("y", getText(Hint1.RemoveRange.getAsRange(), *Context));
+EXPECT_TRUE(Hint1.InsertFromRange.isInvalid());
+EXPECT_TRUE(Hint1.CodeToInsert.empty());
+  };
+  Visitor.runOver("void foo(int x, int y) { foo(x, y); }");
+
+  Visitor.OnCall = [](CallExpr *CE, ASTContext *Context) {
+Expr *P0 = CE->getArg(0);
+FixItHint Hint0 = createRemoval(*P0);
+EXPECT_EQ("x + y", getText(Hint0.RemoveRange.getAsRange(), *Context));
+
+Expr *P1 = CE->getArg(1);
+FixItHint Hint1 = createRemoval(*P1);
+EXPECT_EQ("y + x", getText(Hint1.RemoveRange.getAsRange(), *Context));
+  };
+  Visitor.runOver("void foo(int x, int y) { foo(x + y, y + x); }");
+}
+
+TEST(FixItTest, createRemovalWithMacro) {
+  CallsVisitor Visitor;
+
+  Visitor.OnCall = [](CallExpr *CE, ASTContext *Context) {
+FixItHint Hint = createRemoval(*CE);
+EXPECT_EQ("FOO", getText(Hint.RemoveRange.getAsRange(), *Context));
+EXPECT_TRUE(Hint.InsertFromRange.isInvalid());
+EXPECT_TRUE(Hint.CodeToInsert.empty());
+
+Expr *P0 = CE->getArg(0);
+FixItHint Hint0 = createRemoval(*P0);
+EXPECT_EQ("input.cc:2:26 ",
+  LocationToString(Hint0.RemoveRange.getBegin(), Context));
+EXPECT_EQ("input.cc:2:26 ",
+  LocationToString(Hint0.RemoveRange.getEnd(), Context));
+EXPECT_TRUE(

Re: [PATCH] D19703: [clang-tidy] Improve misc-redundant-expression and decrease false-positive

2016-05-11 Thread Etienne Bergeron via cfe-commits
etienneb updated this revision to Diff 56912.
etienneb added a comment.

fix bad upload


http://reviews.llvm.org/D19703

Files:
  clang-tidy/misc/RedundantExpressionCheck.cpp
  docs/clang-tidy/checks/misc-redundant-expression.rst
  test/clang-tidy/misc-redundant-expression.cpp

Index: test/clang-tidy/misc-redundant-expression.cpp
===
--- test/clang-tidy/misc-redundant-expression.cpp
+++ test/clang-tidy/misc-redundant-expression.cpp
@@ -68,14 +68,14 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: both side of operator are equivalent
 
   if (foo(0) - 2 < foo(0) - 2) return 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: both side of operator are equivalent  
+  // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: both side of operator are equivalent
   if (foo(bar(0)) < (foo(bar((0) return 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: both side of operator are equivalent  
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: both side of operator are equivalent
 
   if (P1.x < P2.x && P1.x < P2.x) return 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: both side of operator are equivalent  
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: both side of operator are equivalent
   if (P2.a[P1.x + 2] < P2.x && P2.a[(P1.x) + (2)] < (P2.x)) return 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: both side of operator are equivalent  
+  // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: both side of operator are equivalent
 
   return 0;
 }
@@ -100,13 +100,36 @@
   return 0;
 }
 
+int TestConditional(int x, int y) {
+  int k = 0;
+  k += (y < 0) ? x : x;
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: 'true' and 'false' expression are equivalent
+  k += (y < 0) ? x + 1 : x + 1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: 'true' and 'false' expression are equivalent
+  return k;
+}
+
+struct MyStruct {
+  int x;
+} Q;
+bool operator==(const MyStruct& lhs, const MyStruct& rhs) { return lhs.x == rhs.x; }
+bool TestOperator(const MyStruct& S) {
+  if (S == Q) return false;
+  return S == S;
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: both side of overloaded operator are equivalent
+}
+
 #define LT(x, y) (void)((x) < (y))
+#define COND(x, y, z) ((x)?(y):(z))
+#define EQUALS(x, y) (x) == (y)
 
 int TestMacro(int X, int Y) {
   LT(0, 0);
   LT(1, 0);
   LT(X, X);
   LT(X+1, X + 1);
+  COND(X < Y, X, X);
+  EQUALS(Q, Q);
 }
 
 int TestFalsePositive(int* A, int X, float F) {
@@ -118,3 +141,22 @@
   if (F != F && F == F) return 1;
   return 0;
 }
+
+int TestBannedMacros() {
+#define EAGAIN 3
+#define NOT_EAGAIN 3
+  if (EAGAIN == 0 | EAGAIN == 0) return 0;
+  if (NOT_EAGAIN == 0 | NOT_EAGAIN == 0) return 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: both side of operator are equivalent
+}
+
+struct MyClass {
+static const int Value = 42;
+};
+template 
+void TemplateCheck() {
+  static_assert(T::Value == U::Value, "should be identical");
+  static_assert(T::Value == T::Value, "should be identical");
+  // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: both side of overloaded operator are equivalent
+}
+void TestTemplate() { TemplateCheck(); }
Index: docs/clang-tidy/checks/misc-redundant-expression.rst
===
--- docs/clang-tidy/checks/misc-redundant-expression.rst
+++ docs/clang-tidy/checks/misc-redundant-expression.rst
@@ -12,6 +12,7 @@
   * always be a constant (zero or one)
 
 Example:
+
 .. code:: c++
 
   ((x+1) | (x+1)) // (x+1) is redundant
Index: clang-tidy/misc/RedundantExpressionCheck.cpp
===
--- clang-tidy/misc/RedundantExpressionCheck.cpp
+++ clang-tidy/misc/RedundantExpressionCheck.cpp
@@ -9,16 +9,30 @@
 
 #include "RedundantExpressionCheck.h"
 #include "../utils/Matchers.h"
+#include "../utils/OptionsUtils.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
 
 using namespace clang::ast_matchers;
 
 namespace clang {
 namespace tidy {
 namespace misc {
 
-static bool AreIdenticalExpr(const Expr *Left, const Expr *Right) {
+static const char KnownBannedMacroNames[] =
+"EAGAIN;EWOULDBLOCK;SIGCLD;SIGCHLD;";
+
+
+static bool AreEquivalentNameSpecifier(const NestedNameSpecifier *Left,
+   const NestedNameSpecifier *Right) {
+  llvm::FoldingSetNodeID LeftID, RightID;
+  Left->Profile(LeftID);
+  Right->Profile(RightID);
+  return LeftID == RightID;
+}
+
+static bool AreEquivalentExpr(const Expr *Left, const Expr *Right) {
   if (!Left || !Right)
 return !Left && !Right;
 
@@ -33,8 +47,8 @@
   Expr::const_child_iterator LeftIter = Left->child_begin();
   Expr::const_child_iterator RightIter = Right->child_begin();
   while (LeftIter != Left->child_end() && RightIter != Right->child_end()) {
-if (!AreIdenticalExpr(dyn_cast(*LeftIter),
-  dyn_cast(*RightIter)))
+if (!AreEquiva

r269188 - [tooling] FixItHint Tooling refactoring

2016-05-11 Thread Etienne Bergeron via cfe-commits
Author: etienneb
Date: Wed May 11 09:31:39 2016
New Revision: 269188

URL: http://llvm.org/viewvc/llvm-project?rev=269188&view=rev
Log:
[tooling] FixItHint Tooling refactoring

Summary:
This is the refactoring to lift some FixItHint into tooling.
used by: http://reviews.llvm.org/D19807

Reviewers: klimek, alexfh

Subscribers: cfe-commits

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

Added:
cfe/trunk/include/clang/Tooling/FixIt.h
cfe/trunk/lib/Tooling/FixIt.cpp
cfe/trunk/unittests/Tooling/FixItTest.cpp
Modified:
cfe/trunk/lib/Tooling/CMakeLists.txt
cfe/trunk/unittests/Tooling/CMakeLists.txt

Added: cfe/trunk/include/clang/Tooling/FixIt.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/FixIt.h?rev=269188&view=auto
==
--- cfe/trunk/include/clang/Tooling/FixIt.h (added)
+++ cfe/trunk/include/clang/Tooling/FixIt.h Wed May 11 09:31:39 2016
@@ -0,0 +1,72 @@
+//===--- FixIt.h - FixIt Hint utilities -*- C++ 
-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+//  This file implements functions to ease source rewriting from AST-nodes.
+//
+//  Example swapping A and B expressions:
+//
+//Expr *A, *B;
+//tooling::fixit::createReplacement(*A, *B);
+//tooling::fixit::createReplacement(*B, *A);
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLING_FIXIT_H
+#define LLVM_CLANG_TOOLING_FIXIT_H
+
+#include "clang/AST/ASTContext.h"
+
+namespace clang {
+namespace tooling {
+namespace fixit {
+
+namespace internal {
+StringRef getText(SourceRange Range, const ASTContext &Context);
+
+/// \brief Returns the SourceRange of a SourceRange. This identity function is
+///used by the following template abstractions.
+inline SourceRange getSourceRange(const SourceRange &Range) { return Range; }
+
+/// \brief Returns the SourceRange of the token at Location \p Loc.
+inline SourceRange getSourceRange(const SourceLocation &Loc) {
+  return SourceRange(Loc);
+}
+
+/// \brief Returns the SourceRange of an given Node. \p Node is typically a
+///'Stmt', 'Expr' or a 'Decl'.
+template  SourceRange getSourceRange(const T &Node) {
+  return Node.getSourceRange();
+}
+} // end namespace internal
+
+// \brief Returns a textual representation of \p Node.
+template 
+StringRef getText(const T &Node, const ASTContext &Context) {
+  return internal::getText(internal::getSourceRange(Node), Context);
+}
+
+// \brief Returns a FixItHint to remove \p Node.
+// TODO: Add support for related syntactical elements (i.e. comments, ...).
+template  FixItHint createRemoval(const T &Node) {
+  return FixItHint::CreateRemoval(internal::getSourceRange(Node));
+}
+
+// \brief Returns a FixItHint to replace \p Destination by \p Source.
+template 
+FixItHint createReplacement(const D &Destination, const S &Source,
+const ASTContext &Context) {
+  return FixItHint::CreateReplacement(internal::getSourceRange(Destination),
+  getText(Source, Context));
+}
+
+} // end namespace fixit
+} // end namespace tooling
+} // end namespace clang
+
+#endif // LLVM_CLANG_TOOLING_FIXINT_H

Modified: cfe/trunk/lib/Tooling/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/CMakeLists.txt?rev=269188&r1=269187&r2=269188&view=diff
==
--- cfe/trunk/lib/Tooling/CMakeLists.txt (original)
+++ cfe/trunk/lib/Tooling/CMakeLists.txt Wed May 11 09:31:39 2016
@@ -7,6 +7,7 @@ add_clang_library(clangTooling
   CommonOptionsParser.cpp
   CompilationDatabase.cpp
   FileMatchTrie.cpp
+  FixIt.cpp
   JSONCompilationDatabase.cpp
   Refactoring.cpp
   RefactoringCallbacks.cpp

Added: cfe/trunk/lib/Tooling/FixIt.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/FixIt.cpp?rev=269188&view=auto
==
--- cfe/trunk/lib/Tooling/FixIt.cpp (added)
+++ cfe/trunk/lib/Tooling/FixIt.cpp Wed May 11 09:31:39 2016
@@ -0,0 +1,31 @@
+//===--- FixIt.cpp - FixIt Hint utilities ---*- C++ 
-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// This file contains implementations of utitilies to ease source code 
rewriting
+// by providing helper functions related to FixItHint.
+//
+//===--===//
+

[PATCH] D20170: [clang-tidy] TypeTraits - Type is not expensive to copy when it has a deleted copy constructor.

2016-05-11 Thread Felix Berger via cfe-commits
flx created this revision.
flx added reviewers: alexfh, sbenza.
flx added a subscriber: cfe-commits.
flx set the repository for this revision to rL LLVM.

Repository:
  rL LLVM

http://reviews.llvm.org/D20170

Files:
  clang-tidy/utils/TypeTraits.cpp
  test/clang-tidy/performance-unnecessary-value-param.cpp

Index: test/clang-tidy/performance-unnecessary-value-param.cpp
===
--- test/clang-tidy/performance-unnecessary-value-param.cpp
+++ test/clang-tidy/performance-unnecessary-value-param.cpp
@@ -23,6 +23,12 @@
   SomewhatTrivial& operator=(const SomewhatTrivial&);
 };
 
+struct MoveOnlyType {
+  MoveOnlyType(const MoveOnlyType&) = delete;
+  ~MoveOnlyType();
+  void constMethod() const;
+};
+
 void positiveExpensiveConstValue(const ExpensiveToCopyType Obj);
 // CHECK-FIXES: void positiveExpensiveConstValue(const ExpensiveToCopyType& 
Obj);
 void positiveExpensiveConstValue(const ExpensiveToCopyType Obj) {
@@ -169,3 +175,7 @@
   NegativeDeletedMethod& operator=(NegativeDeletedMethod N) = delete;
   // CHECK-FIXES: NegativeDeletedMethod& operator=(NegativeDeletedMethod N) = 
delete;
 };
+
+void NegativeMoveOnlyTypePassedByValue(MoveOnlyType M) {
+  M.constMethod();
+}
Index: clang-tidy/utils/TypeTraits.cpp
===
--- clang-tidy/utils/TypeTraits.cpp
+++ clang-tidy/utils/TypeTraits.cpp
@@ -10,26 +10,42 @@
 #include "TypeTraits.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/DeclCXX.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
 
 namespace clang {
 namespace tidy {
 namespace utils {
 namespace type_traits {
 
+using namespace ::clang::ast_matchers;
+
 namespace {
 bool classHasTrivialCopyAndDestroy(QualType Type) {
   auto *Record = Type->getAsCXXRecordDecl();
   return Record && Record->hasDefinition() &&
  !Record->hasNonTrivialCopyConstructor() &&
  !Record->hasNonTrivialDestructor();
 }
+
+bool hasDeletedCopyConstructor(QualType Type, ASTContext &Context) {
+  auto *Record = Type->getAsCXXRecordDecl();
+  if (Record == nullptr || !Record->hasDefinition())
+return false;
+  auto Matches = match(cxxRecordDecl(hasMethod(
+   cxxConstructorDecl(isCopyConstructor(), isDeleted())
+   .bind("constructor"))),
+   *Record, Context);
+  return !Matches.empty();
+}
+
 } // namespace
 
 llvm::Optional isExpensiveToCopy(QualType Type, ASTContext &Context) {
   if (Type->isDependentType())
 return llvm::None;
   return !Type.isTriviallyCopyableType(Context) &&
- !classHasTrivialCopyAndDestroy(Type);
+ !classHasTrivialCopyAndDestroy(Type) &&
+ !hasDeletedCopyConstructor(Type, Context);
 }
 
 bool recordIsTriviallyDefaultConstructible(const RecordDecl &RecordDecl,


Index: test/clang-tidy/performance-unnecessary-value-param.cpp
===
--- test/clang-tidy/performance-unnecessary-value-param.cpp
+++ test/clang-tidy/performance-unnecessary-value-param.cpp
@@ -23,6 +23,12 @@
   SomewhatTrivial& operator=(const SomewhatTrivial&);
 };
 
+struct MoveOnlyType {
+  MoveOnlyType(const MoveOnlyType&) = delete;
+  ~MoveOnlyType();
+  void constMethod() const;
+};
+
 void positiveExpensiveConstValue(const ExpensiveToCopyType Obj);
 // CHECK-FIXES: void positiveExpensiveConstValue(const ExpensiveToCopyType& Obj);
 void positiveExpensiveConstValue(const ExpensiveToCopyType Obj) {
@@ -169,3 +175,7 @@
   NegativeDeletedMethod& operator=(NegativeDeletedMethod N) = delete;
   // CHECK-FIXES: NegativeDeletedMethod& operator=(NegativeDeletedMethod N) = delete;
 };
+
+void NegativeMoveOnlyTypePassedByValue(MoveOnlyType M) {
+  M.constMethod();
+}
Index: clang-tidy/utils/TypeTraits.cpp
===
--- clang-tidy/utils/TypeTraits.cpp
+++ clang-tidy/utils/TypeTraits.cpp
@@ -10,26 +10,42 @@
 #include "TypeTraits.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/DeclCXX.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
 
 namespace clang {
 namespace tidy {
 namespace utils {
 namespace type_traits {
 
+using namespace ::clang::ast_matchers;
+
 namespace {
 bool classHasTrivialCopyAndDestroy(QualType Type) {
   auto *Record = Type->getAsCXXRecordDecl();
   return Record && Record->hasDefinition() &&
  !Record->hasNonTrivialCopyConstructor() &&
  !Record->hasNonTrivialDestructor();
 }
+
+bool hasDeletedCopyConstructor(QualType Type, ASTContext &Context) {
+  auto *Record = Type->getAsCXXRecordDecl();
+  if (Record == nullptr || !Record->hasDefinition())
+return false;
+  auto Matches = match(cxxRecordDecl(hasMethod(
+   cxxConstructorDecl(isCopyConstructor(), isDeleted())
+   .bind("constructor"))),
+   *Record, Context);
+  return !Matches.empty();
+}
+
 } // namespace
 
 llvm::

Re: [PATCH] D19932: [OpenCL] Add to_{global|local|private} builtin functions.

2016-05-11 Thread Yaxun Liu via cfe-commits
yaxunl marked 8 inline comments as done.


Comment at: lib/Sema/SemaChecking.cpp:480
@@ +479,3 @@
+  .getQualifiers().getAddressSpace() == LangAS::opencl_constant) {
+S.Diag(Call->getLocStart(), diag::err_opencl_builtin_to_addr_invalid_arg)
+<< Call->getArg(0) << Call->getDirectCallee() << 
Call->getSourceRange();

Anastasia wrote:
> I just think you should simply set the function return type in line 487 based 
> on the passed argument type (RT variable declared in line 477).
> 
> The situation I would like us to avoid here is:
> 
>   int *gptr = ..;
>   local char *lptr = to_local(gptr);
> 
> So basically by calling to_local here, we allow conversion of not only an 
> address space but also an underlaying type silently. 
> 
> Could we add a test to make sure it's not happening.
> 
> Similarly, in C such situations trigger a warning:
>   int *ptr1 = ...;
>   char *ptr2 = ptr1; //expected-warning{{incompatible pointer types 
> initializing 'char *' with an expression of type 'int *'}}
I added the above test. The current implementation is able to diagnose it.

Your suggested approach also works, so I will remove dynamic generation of 
to_addr declarations since it seems to be an overkill.


Comment at: test/SemaOpenCL/to_addr_builtin.cl:25
@@ +24,3 @@
+  glob = to_global(con);
+#if __OPENCL_C_VERSION__ < CL_VERSION_2_0
+  // expected-error@-2{{'to_global' needs OpenCL version 2.0 or above}}

pxli168 wrote:
> yaxunl wrote:
> > pxli168 wrote:
> > > But in Spec OpenCL V2.0 s6.13.9 the description for to_global is:
> > > 
> > > > Returns a pointer that points to a region in the global address space 
> > > > if to_global can cast ptr to the global address space. Otherwise it 
> > > > returns NULL.
> > > 
> > I think this is only for valid call expression. For constant pointer, it is 
> > an invalid call expression due to s6.5.5, so an error of invalid argument 
> > should be emitted.
> > 
> > If we allow passing constant pointer to this function, we violates s6.5.5.
> >A pointer that points to the constant address space cannot be cast or 
> >implicitly converted to the generic address space.
>  
> So if we can not cast, we maybe should return NULL?
> @Anastasia What do you think about this?
> 
> 
What the function returns is implemented by builtin library and Clang does not 
care about that. Clang only does syntax checking, type checking, etc. Since 
this is still a function, it is subject to all these rules. Unless we want to 
exempt this function from the type-checking rules. However I don't think the 
spec implies that.


http://reviews.llvm.org/D19932



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


Re: [PATCH] D19932: [OpenCL] Add to_{global|local|private} builtin functions.

2016-05-11 Thread Yaxun Liu via cfe-commits
yaxunl updated the summary for this revision.
yaxunl updated this revision to Diff 56915.
yaxunl marked an inline comment as done.
yaxunl added a comment.

Removed dynamic generation of builtin decl. Added a test for mismatched 
returned pointee types.


http://reviews.llvm.org/D19932

Files:
  include/clang/Basic/Builtins.def
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/CodeGen/CGBuiltin.cpp
  lib/Sema/SemaChecking.cpp
  test/CodeGenOpenCL/to_addr_builtin.cl
  test/SemaOpenCL/to_addr_builtin.cl

Index: test/SemaOpenCL/to_addr_builtin.cl
===
--- /dev/null
+++ test/SemaOpenCL/to_addr_builtin.cl
@@ -0,0 +1,45 @@
+// RUN: %clang_cc1 -verify -fsyntax-only %s
+// RUN: %clang_cc1 -verify -fsyntax-only -cl-std=CL2.0 %s
+
+void test(void) {
+  global int *glob;
+  local int *loc;
+  constant int *con;
+
+  glob = to_global(glob, loc);
+#if __OPENCL_C_VERSION__ < CL_VERSION_2_0
+  // expected-error@-2{{'to_global' needs OpenCL version 2.0 or above}}
+#else
+  // expected-error@-4{{invalid number of arguments to function: 'to_global'}}
+#endif
+
+  int x;
+  glob = to_global(x);
+#if __OPENCL_C_VERSION__ < CL_VERSION_2_0
+  // expected-error@-2{{'to_global' needs OpenCL version 2.0 or above}}
+#else
+  // expected-error@-4{{invalid argument x to function: 'to_global', expecting a generic pointer argument}}
+#endif
+
+  glob = to_global(con);
+#if __OPENCL_C_VERSION__ < CL_VERSION_2_0
+  // expected-error@-2{{'to_global' needs OpenCL version 2.0 or above}}
+#else
+  // expected-error@-4{{invalid argument con to function: 'to_global', expecting a generic pointer argument}}
+#endif
+
+  loc = to_global(glob);
+#if __OPENCL_C_VERSION__ < CL_VERSION_2_0
+  // expected-error@-2{{'to_global' needs OpenCL version 2.0 or above}}
+#else
+  // expected-error@-4{{assigning '__global int *' to '__local int *' changes address space of pointer}}
+#endif
+
+  global char *glob_c = to_global(loc);
+#if __OPENCL_C_VERSION__ < CL_VERSION_2_0
+  // expected-error@-2{{'to_global' needs OpenCL version 2.0 or above}}
+#else
+  // expected-warning@-4{{incompatible pointer types initializing '__global char *' with an expression of type '__global int *'}}
+#endif
+
+}
Index: test/CodeGenOpenCL/to_addr_builtin.cl
===
--- /dev/null
+++ test/CodeGenOpenCL/to_addr_builtin.cl
@@ -0,0 +1,89 @@
+// RUN: %clang_cc1 -triple spir-unknown-unknown -emit-llvm -O0 -cl-std=CL2.0 -o - %s | FileCheck %s
+
+// CHECK: %[[A:.*]] = type { float, float, float }
+typedef struct {
+  float x,y,z;
+} A;
+typedef private A *PA;
+typedef global A *GA;
+
+void test(void) {
+  global int *glob;
+  local int *loc;
+  private int *priv;
+  generic int *gen;
+
+  //CHECK: %[[ARG:.*]] = addrspacecast i32 addrspace(1)* %{{.*}} to i8 addrspace(4)*
+  //CHECK: %[[RET:.*]] = call i8 addrspace(1)* @to_global(i8 addrspace(4)* %[[ARG]])
+  //CHECK: %{{.*}} = bitcast i8 addrspace(1)* %[[RET]] to i32 addrspace(1)*
+  glob = to_global(glob);
+  
+  //CHECK: %[[ARG:.*]] = addrspacecast i32 addrspace(3)* %{{.*}} to i8 addrspace(4)*
+  //CHECK: %[[RET:.*]] = call i8 addrspace(1)* @to_global(i8 addrspace(4)* %[[ARG]])
+  //CHECK: %{{.*}} = bitcast i8 addrspace(1)* %[[RET]] to i32 addrspace(1)*
+  glob = to_global(loc);
+ 
+  //CHECK: %[[ARG:.*]] = addrspacecast i32* %{{.*}} to i8 addrspace(4)*
+  //CHECK: %[[RET:.*]] = call i8 addrspace(1)* @to_global(i8 addrspace(4)* %[[ARG]])
+  //CHECK: %{{.*}} = bitcast i8 addrspace(1)* %[[RET]] to i32 addrspace(1)*
+  glob = to_global(priv);
+ 
+  //CHECK: %[[ARG:.*]] = bitcast i32 addrspace(4)* %{{.*}} to i8 addrspace(4)*
+  //CHECK: %[[RET:.*]] = call i8 addrspace(1)* @to_global(i8 addrspace(4)* %[[ARG]])
+  //CHECK: %{{.*}} = bitcast i8 addrspace(1)* %[[RET]] to i32 addrspace(1)*
+  glob = to_global(gen);
+  
+  //CHECK: %[[ARG:.*]] = addrspacecast i32 addrspace(1)* %{{.*}} to i8 addrspace(4)*
+  //CHECK: %[[RET:.*]] = call i8 addrspace(3)* @to_local(i8 addrspace(4)* %[[ARG]])
+  //CHECK: %{{.*}} = bitcast i8 addrspace(3)* %[[RET]] to i32 addrspace(3)*
+  loc = to_local(glob);
+
+  //CHECK: %[[ARG:.*]] = addrspacecast i32 addrspace(3)* %{{.*}} to i8 addrspace(4)*
+  //CHECK: %[[RET:.*]] = call i8 addrspace(3)* @to_local(i8 addrspace(4)* %[[ARG]])
+  //CHECK: %{{.*}} = bitcast i8 addrspace(3)* %[[RET]] to i32 addrspace(3)*
+  loc = to_local(loc);
+
+  //CHECK: %[[ARG:.*]] = addrspacecast i32* %{{.*}} to i8 addrspace(4)*
+  //CHECK: %[[RET:.*]] = call i8 addrspace(3)* @to_local(i8 addrspace(4)* %[[ARG]])
+  //CHECK: %{{.*}} = bitcast i8 addrspace(3)* %[[RET]] to i32 addrspace(3)*
+  loc = to_local(priv);
+
+  //CHECK: %[[ARG:.*]] = bitcast i32 addrspace(4)* %{{.*}} to i8 addrspace(4)*
+  //CHECK: %[[RET:.*]] = call i8 addrspace(3)* @to_local(i8 addrspace(4)* %[[ARG]])
+  //CHECK: %{{.*}} = bitcast i8 addrspace(3)* %[[RET]] to i32 addrspace(3)*
+  loc = to_local(gen);
+
+  //CHECK: %[[ARG:.*]] = addrspacecast i32 addr

Re: [PATCH] D20136: Get default -fms-compatibility-version from cl.exe's version

2016-05-11 Thread Adrian McCarthy via cfe-commits
amccarth added inline comments.


Comment at: lib/Driver/MSVCToolChain.cpp:478
@@ +477,3 @@
+
+  const DWORD VersionSize = ::GetFileVersionInfoSizeW(ClExeWide.c_str(),
+  nullptr);

thakis wrote:
> amccarth wrote:
> > Yes, it looks in the executable (which I tried to emphasize with the method 
> > name).
> > 
> > I don't think this is very expensive given that Explorer often makes 
> > zillions of such calls, but I'm open to other suggestions.
> > 
> > I know that you can't use a library that's newer than the compiler (because 
> > it may use new language features), but I don't know if that applies in the 
> > other direction or how we would safely and reliably map directory names to 
> > library versions and therefore to compiler versions.
> I agree that figuring out the right value for fmsc-version automatically 
> somehow is definitely something we should do.
> 
> I forgot that `getVisualStudioBinariesFolder` already works by looking for 
> cl.exe in PATH, so cl.exe's metadata is already warmed up in the disk cache. 
> However, GetFileVersionInfoW() probably opens cl.exe itself and does some PE 
> parsing to get at the version, and that probably is in cold cache territory. 
> (https://msdn.microsoft.com/en-us/library/windows/desktop/ms647003(v=vs.85).aspx
>  suggests that this function might open several files).
> 
> `getVisualStudioBinariesFolder` checks:
> 
> 1. getenv("VCINSTALLDIR")
> 2. cl.exe in getenv("PATH")
> 3. registry (via getVisualStudioInstallDir)
> 
> The common cases are 1 and 3. For 1, for default installs, the version number 
> is part of the directory name (for default installs, what most people have). 
> For 3, the version number is in the registry key we query. So in most cases 
> we shouldn't have to look at cl.exe itself. And for the cases where we would 
> have to look, maybe it's ok to require an explicit fmsc-version flag.
The version number in the directory name and the registry is the version number 
of Visual Studio not of the compiler.  Yes, we could do a mapping (VS 14 comes 
bundled with CL 19), assuming Microsoft continues to keep VS releases and 
compiler releases in sync, and it means this code will forever need updates to 
the mapping data.

The mapping would give just the major version number, which might be all that 
matters now, but if there's ever a CL 19.1 that has different compatibility 
requirements (and is maybe released out-of-band with Visual Studio), we'd be 
stuck.

Getting the actual version from the compiler seems the most accurate and 
future-proof way to check.  If that's too expensive, then maybe we should 
abandon the idea of detecting the default for compatibility.


http://reviews.llvm.org/D20136



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


Re: [PATCH] D17578: [OpenCL]Allowing explicit conversion of "0" to event_t type

2016-05-11 Thread Yaxun Liu via cfe-commits
yaxunl accepted this revision.
yaxunl added a comment.

LGTM. Thanks!


http://reviews.llvm.org/D17578



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


Re: [PATCH] D20125: [libclang] Added clang_getRealSpellingLocation to compensate for clang_getSpellingLocation returning the expansion location

2016-05-11 Thread Cameron via cfe-commits
cameron314 updated this revision to Diff 56916.
cameron314 added a comment.

I've added a unit test.


http://reviews.llvm.org/D20125

Files:
  include/clang-c/Index.h
  tools/libclang/CXSourceLocation.cpp
  unittests/libclang/LibclangTest.cpp

Index: unittests/libclang/LibclangTest.cpp
===
--- unittests/libclang/LibclangTest.cpp
+++ unittests/libclang/LibclangTest.cpp
@@ -15,6 +15,9 @@
 #include "gtest/gtest.h"
 #include 
 #include 
+#include 
+#include 
+#include 
 #define DEBUG_TYPE "libclang-test"
 
 TEST(libclang, clang_parseTranslationUnit2_InvalidArgs) {
@@ -349,21 +352,25 @@
   clang_ModuleMapDescriptor_dispose(MMD);
 }
 
-class LibclangReparseTest : public ::testing::Test {
+class LibclangParseTest : public ::testing::Test {
   std::set Files;
+  typedef std::unique_ptr fixed_addr_string;
+  std::map UnsavedFileContents;
 public:
   std::string TestDir;
   CXIndex Index;
   CXTranslationUnit ClangTU;
   unsigned TUFlags;
+  std::vector UnsavedFiles;
 
   void SetUp() override {
 llvm::SmallString<256> Dir;
 ASSERT_FALSE(llvm::sys::fs::createUniqueDirectory("libclang-test", Dir));
 TestDir = Dir.str();
 TUFlags = CXTranslationUnit_DetailedPreprocessingRecord |
-  clang_defaultEditingTranslationUnitOptions();
+  clang_defaultEditingTranslationUnitOptions();
 Index = clang_createIndex(0, 0);
+ClangTU = nullptr;
   }
   void TearDown() override {
 clang_disposeTranslationUnit(ClangTU);
@@ -384,6 +391,64 @@
 OS << Contents;
 assert(OS.good());
   }
+  void MapUnsavedFile(const char* name, const std::string &contents) {
+auto it = UnsavedFileContents.emplace(
+fixed_addr_string(new std::string(name)),
+fixed_addr_string(new std::string(contents)));
+UnsavedFiles.push_back({
+it.first->first->c_str(),   // filename
+it.first->second->c_str(),  // contents
+it.first->second->size()// length
+});
+  }
+  template
+  void Traverse(const F &TraversalFunctor) {
+CXCursor TuCursor = clang_getTranslationUnitCursor(ClangTU);
+std::reference_wrapper FunctorRef = std::cref(TraversalFunctor);
+clang_visitChildren(TuCursor,
+&TraverseStateless>,
+&FunctorRef);
+  }
+
+private:
+  template
+  static CXChildVisitResult TraverseStateless(CXCursor cx, CXCursor parent,
+  CXClientData data) {
+TState *State = static_cast(data);
+return State->get()(cx, parent);
+  }
+};
+
+TEST_F(LibclangParseTest, SpellingLocation) {
+  MapUnsavedFile("main.cpp",
+"#define BANANAS 4011\n"
+"int plu = BANANAS;");
+
+  ClangTU = clang_parseTranslationUnit(Index, "main.cpp", nullptr, 0,
+UnsavedFiles.data(), UnsavedFiles.size(), TUFlags);
+
+  bool sawInt;
+  Traverse([&](CXCursor cx, CXCursor) {
+if (cx.kind == CXCursor_IntegerLiteral) {
+  sawInt = true;
+  auto cxl = clang_getCursorLocation(cx);
+  CXFile expansionFile, spellingFile;
+  unsigned line, column, offset;
+  clang_getSpellingLocation(cxl, &expansionFile, &line, nullptr, nullptr);
+  EXPECT_EQ(2, line);  // getSpellingLocation returns expansion location
+  clang_getRealSpellingLocation(cxl, &spellingFile, &line, &column, &offset);
+  EXPECT_TRUE(clang_File_isEqual(expansionFile, spellingFile));
+  EXPECT_EQ(1, line);
+  EXPECT_EQ(17, column);
+  EXPECT_EQ(16, offset);
+}
+return CXChildVisit_Recurse;
+  });
+  EXPECT_TRUE(sawInt);
+}
+
+class LibclangReparseTest : public LibclangParseTest {
+public:
   void DisplayDiagnostics() {
 unsigned NumDiagnostics = clang_getNumDiagnostics(ClangTU);
 for (unsigned i = 0; i < NumDiagnostics; ++i) {
Index: tools/libclang/CXSourceLocation.cpp
===
--- tools/libclang/CXSourceLocation.cpp
+++ tools/libclang/CXSourceLocation.cpp
@@ -346,6 +346,42 @@
 *offset = FileOffset;
 }
 
+void clang_getRealSpellingLocation(CXSourceLocation location,
+   CXFile *file,
+   unsigned *line,
+   unsigned *column,
+   unsigned *offset) {
+
+  if (!isASTUnitSourceLocation(location)) {
+CXLoadedDiagnostic::decodeLocation(location, file, line, column, offset);
+return;
+  }
+
+  SourceLocation Loc = SourceLocation::getFromRawEncoding(location.int_data);
+
+  if (!location.ptr_data[0] || Loc.isInvalid())
+return createNullLocation(file, line, column, offset);
+
+  const SourceManager &SM =
+  *static_cast(location.ptr_data[0]);
+  SourceLocation SpellLoc = SM.getSpellingLoc(Loc);
+  std::pair LocInfo = SM.getDecomposedLoc(SpellLoc);
+  FileID FID = LocInfo.first;
+  unsigned FileOffset = LocInfo.second;
+
+  if (FID.isInvalid())
+return createNullLocation(file, line, column, offset);
+
+  if (file)
+*file = const_cast(SM.getFileEntryForID(FID));
+  if (line)
+*line

Re: [PATCH] D20170: [clang-tidy] TypeTraits - Type is not expensive to copy when it has a deleted copy constructor.

2016-05-11 Thread Aaron Ballman via cfe-commits
aaron.ballman added a subscriber: aaron.ballman.


Comment at: clang-tidy/utils/TypeTraits.cpp:20
@@ -18,1 +19,3 @@
 
+using namespace ::clang::ast_matchers;
+

I would prefer this be used locally instead of at namespace scope to avoid 
potential name collisions.


Comment at: clang-tidy/utils/TypeTraits.cpp:32
@@ +31,3 @@
+  auto *Record = Type->getAsCXXRecordDecl();
+  if (Record == nullptr || !Record->hasDefinition())
+return false;

`!Record` instead of explicit comparison.


Comment at: clang-tidy/utils/TypeTraits.cpp:34
@@ +33,3 @@
+return false;
+  auto Matches = match(cxxRecordDecl(hasMethod(
+   cxxConstructorDecl(isCopyConstructor(), isDeleted())

Why use the matcher at all? You have the record decl, it can give you the copy 
constructor, and you can directly inspect whether that is deleted or not.


Comment at: test/clang-tidy/performance-unnecessary-value-param.cpp:27
@@ +26,3 @@
+struct MoveOnlyType {
+  MoveOnlyType(const MoveOnlyType&) = delete;
+  ~MoveOnlyType();

This is not a move-only type. Because you have a user-declared copy constructor 
(or destructor), the move constructor will not be implicitly declared. See 
[class.copy]p9 for details.


Repository:
  rL LLVM

http://reviews.llvm.org/D20170



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


Re: [PATCH] D20136: Get default -fms-compatibility-version from cl.exe's version

2016-05-11 Thread Aaron Ballman via cfe-commits
On Wed, May 11, 2016 at 11:00 AM, Adrian McCarthy via cfe-commits
 wrote:
> amccarth added inline comments.
>
> 
> Comment at: lib/Driver/MSVCToolChain.cpp:478
> @@ +477,3 @@
> +
> +  const DWORD VersionSize = ::GetFileVersionInfoSizeW(ClExeWide.c_str(),
> +  nullptr);
> 
> thakis wrote:
>> amccarth wrote:
>> > Yes, it looks in the executable (which I tried to emphasize with the 
>> > method name).
>> >
>> > I don't think this is very expensive given that Explorer often makes 
>> > zillions of such calls, but I'm open to other suggestions.
>> >
>> > I know that you can't use a library that's newer than the compiler 
>> > (because it may use new language features), but I don't know if that 
>> > applies in the other direction or how we would safely and reliably map 
>> > directory names to library versions and therefore to compiler versions.
>> I agree that figuring out the right value for fmsc-version automatically 
>> somehow is definitely something we should do.
>>
>> I forgot that `getVisualStudioBinariesFolder` already works by looking for 
>> cl.exe in PATH, so cl.exe's metadata is already warmed up in the disk cache. 
>> However, GetFileVersionInfoW() probably opens cl.exe itself and does some PE 
>> parsing to get at the version, and that probably is in cold cache territory. 
>> (https://msdn.microsoft.com/en-us/library/windows/desktop/ms647003(v=vs.85).aspx
>>  suggests that this function might open several files).
>>
>> `getVisualStudioBinariesFolder` checks:
>>
>> 1. getenv("VCINSTALLDIR")
>> 2. cl.exe in getenv("PATH")
>> 3. registry (via getVisualStudioInstallDir)
>>
>> The common cases are 1 and 3. For 1, for default installs, the version 
>> number is part of the directory name (for default installs, what most people 
>> have). For 3, the version number is in the registry key we query. So in most 
>> cases we shouldn't have to look at cl.exe itself. And for the cases where we 
>> would have to look, maybe it's ok to require an explicit fmsc-version flag.
> The version number in the directory name and the registry is the version 
> number of Visual Studio not of the compiler.  Yes, we could do a mapping (VS 
> 14 comes bundled with CL 19), assuming Microsoft continues to keep VS 
> releases and compiler releases in sync, and it means this code will forever 
> need updates to the mapping data.
>
> The mapping would give just the major version number, which might be all that 
> matters now, but if there's ever a CL 19.1 that has different compatibility 
> requirements (and is maybe released out-of-band with Visual Studio), we'd be 
> stuck.

The Updates to MSVC will change the version number (but not the major
version), for instance.

> Getting the actual version from the compiler seems the most accurate and 
> future-proof way to check.  If that's too expensive, then maybe we should 
> abandon the idea of detecting the default for compatibility.

I think that abandoning the idea would be a shame. The discussion
about perf is a good one to have, but I don't think it's sufficient to
abandon the idea unless we have some actual measurements to provide
concrete data demonstrating that the perf hit is unacceptable.

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


Re: [PATCH] D20136: Get default -fms-compatibility-version from cl.exe's version

2016-05-11 Thread Adrian McCarthy via cfe-commits
amccarth added inline comments.


Comment at: lib/Driver/MSVCToolChain.cpp:478
@@ +477,3 @@
+
+  const DWORD VersionSize = ::GetFileVersionInfoSizeW(ClExeWide.c_str(),
+  nullptr);

amccarth wrote:
> thakis wrote:
> > amccarth wrote:
> > > Yes, it looks in the executable (which I tried to emphasize with the 
> > > method name).
> > > 
> > > I don't think this is very expensive given that Explorer often makes 
> > > zillions of such calls, but I'm open to other suggestions.
> > > 
> > > I know that you can't use a library that's newer than the compiler 
> > > (because it may use new language features), but I don't know if that 
> > > applies in the other direction or how we would safely and reliably map 
> > > directory names to library versions and therefore to compiler versions.
> > I agree that figuring out the right value for fmsc-version automatically 
> > somehow is definitely something we should do.
> > 
> > I forgot that `getVisualStudioBinariesFolder` already works by looking for 
> > cl.exe in PATH, so cl.exe's metadata is already warmed up in the disk 
> > cache. However, GetFileVersionInfoW() probably opens cl.exe itself and does 
> > some PE parsing to get at the version, and that probably is in cold cache 
> > territory. 
> > (https://msdn.microsoft.com/en-us/library/windows/desktop/ms647003(v=vs.85).aspx
> >  suggests that this function might open several files).
> > 
> > `getVisualStudioBinariesFolder` checks:
> > 
> > 1. getenv("VCINSTALLDIR")
> > 2. cl.exe in getenv("PATH")
> > 3. registry (via getVisualStudioInstallDir)
> > 
> > The common cases are 1 and 3. For 1, for default installs, the version 
> > number is part of the directory name (for default installs, what most 
> > people have). For 3, the version number is in the registry key we query. So 
> > in most cases we shouldn't have to look at cl.exe itself. And for the cases 
> > where we would have to look, maybe it's ok to require an explicit 
> > fmsc-version flag.
> The version number in the directory name and the registry is the version 
> number of Visual Studio not of the compiler.  Yes, we could do a mapping (VS 
> 14 comes bundled with CL 19), assuming Microsoft continues to keep VS 
> releases and compiler releases in sync, and it means this code will forever 
> need updates to the mapping data.
> 
> The mapping would give just the major version number, which might be all that 
> matters now, but if there's ever a CL 19.1 that has different compatibility 
> requirements (and is maybe released out-of-band with Visual Studio), we'd be 
> stuck.
> 
> Getting the actual version from the compiler seems the most accurate and 
> future-proof way to check.  If that's too expensive, then maybe we should 
> abandon the idea of detecting the default for compatibility.
I'll do some research to figure out the actual costs.  I suspect that walking 
the PATH for the executable may be far more expensive, but I'll get some 
numbers and report back.


http://reviews.llvm.org/D20136



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


Re: [PATCH] D19105: Changes in clang after running http://reviews.llvm.org/D18821

2016-05-11 Thread Piotr Padlewski via cfe-commits
Prazek added inline comments.


Comment at: lib/Target/Hexagon/MCTargetDesc/HexagonMCShuffler.cpp:194
@@ -193,3 +193,3 @@
 
-  if (doneShuffling == false) {
+  if (doneShuffling == 0) {
 HexagonMCShuffler MCS(MCII, STI, MCB);

alexfh wrote:
> This is wrong.
Of course it is, but the prevoius code was also creepy :P


http://reviews.llvm.org/D19105



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


Re: [PATCH] D19156: [ms][dll] #27212: Generating of implicit special members should take into account MSVC compatibility version

2016-05-11 Thread Reid Kleckner via cfe-commits
rnk added inline comments.


Comment at: lib/Sema/SemaDeclCXX.cpp:4813
@@ +4812,3 @@
+// and move constructor, so don't attempt to import/export them if
+// we have a definition.
+auto *CXXC = dyn_cast(MD);

Oh, so we were already doing this check. I don't see what's wrong with our 
current behavior, though. We export a few more symbols than MSVC 2013, but 
there's no ABI problem with that.


http://reviews.llvm.org/D19156



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


Re: [PATCH] D20125: [libclang] Added clang_getRealSpellingLocation to compensate for clang_getSpellingLocation returning the expansion location

2016-05-11 Thread Cameron via cfe-commits
cameron314 updated this revision to Diff 56917.

http://reviews.llvm.org/D20125

Files:
  include/clang-c/Index.h
  tools/libclang/CXSourceLocation.cpp
  unittests/libclang/LibclangTest.cpp

Index: unittests/libclang/LibclangTest.cpp
===
--- unittests/libclang/LibclangTest.cpp
+++ unittests/libclang/LibclangTest.cpp
@@ -15,6 +15,9 @@
 #include "gtest/gtest.h"
 #include 
 #include 
+#include 
+#include 
+#include 
 #define DEBUG_TYPE "libclang-test"
 
 TEST(libclang, clang_parseTranslationUnit2_InvalidArgs) {
@@ -349,21 +352,25 @@
   clang_ModuleMapDescriptor_dispose(MMD);
 }
 
-class LibclangReparseTest : public ::testing::Test {
+class LibclangParseTest : public ::testing::Test {
   std::set Files;
+  typedef std::unique_ptr fixed_addr_string;
+  std::map UnsavedFileContents;
 public:
   std::string TestDir;
   CXIndex Index;
   CXTranslationUnit ClangTU;
   unsigned TUFlags;
+  std::vector UnsavedFiles;
 
   void SetUp() override {
 llvm::SmallString<256> Dir;
 ASSERT_FALSE(llvm::sys::fs::createUniqueDirectory("libclang-test", Dir));
 TestDir = Dir.str();
 TUFlags = CXTranslationUnit_DetailedPreprocessingRecord |
-  clang_defaultEditingTranslationUnitOptions();
+  clang_defaultEditingTranslationUnitOptions();
 Index = clang_createIndex(0, 0);
+ClangTU = nullptr;
   }
   void TearDown() override {
 clang_disposeTranslationUnit(ClangTU);
@@ -384,6 +391,64 @@
 OS << Contents;
 assert(OS.good());
   }
+  void MapUnsavedFile(const char* name, const std::string &contents) {
+auto it = UnsavedFileContents.emplace(
+fixed_addr_string(new std::string(name)),
+fixed_addr_string(new std::string(contents)));
+UnsavedFiles.push_back({
+it.first->first->c_str(),   // filename
+it.first->second->c_str(),  // contents
+it.first->second->size()// length
+});
+  }
+  template
+  void Traverse(const F &TraversalFunctor) {
+CXCursor TuCursor = clang_getTranslationUnitCursor(ClangTU);
+std::reference_wrapper FunctorRef = std::cref(TraversalFunctor);
+clang_visitChildren(TuCursor,
+&TraverseStateless>,
+&FunctorRef);
+  }
+
+private:
+  template
+  static CXChildVisitResult TraverseStateless(CXCursor cx, CXCursor parent,
+  CXClientData data) {
+TState *State = static_cast(data);
+return State->get()(cx, parent);
+  }
+};
+
+TEST_F(LibclangParseTest, SpellingLocation) {
+  MapUnsavedFile("main.cpp",
+"#define BANANAS 4011\n"
+"int plu = BANANAS;");
+
+  ClangTU = clang_parseTranslationUnit(Index, "main.cpp", nullptr, 0,
+UnsavedFiles.data(), UnsavedFiles.size(), TUFlags);
+
+  bool sawInt = false;
+  Traverse([&](CXCursor cx, CXCursor) {
+if (cx.kind == CXCursor_IntegerLiteral) {
+  sawInt = true;
+  auto cxl = clang_getCursorLocation(cx);
+  CXFile expansionFile, spellingFile;
+  unsigned line, column, offset;
+  clang_getSpellingLocation(cxl, &expansionFile, &line, nullptr, nullptr);
+  EXPECT_EQ(2, line);  // getSpellingLocation returns expansion location
+  clang_getRealSpellingLocation(cxl, &spellingFile, &line, &column, &offset);
+  EXPECT_TRUE(clang_File_isEqual(expansionFile, spellingFile));
+  EXPECT_EQ(1, line);
+  EXPECT_EQ(17, column);
+  EXPECT_EQ(16, offset);
+}
+return CXChildVisit_Recurse;
+  });
+  EXPECT_TRUE(sawInt);
+}
+
+class LibclangReparseTest : public LibclangParseTest {
+public:
   void DisplayDiagnostics() {
 unsigned NumDiagnostics = clang_getNumDiagnostics(ClangTU);
 for (unsigned i = 0; i < NumDiagnostics; ++i) {
Index: tools/libclang/CXSourceLocation.cpp
===
--- tools/libclang/CXSourceLocation.cpp
+++ tools/libclang/CXSourceLocation.cpp
@@ -346,6 +346,42 @@
 *offset = FileOffset;
 }
 
+void clang_getRealSpellingLocation(CXSourceLocation location,
+   CXFile *file,
+   unsigned *line,
+   unsigned *column,
+   unsigned *offset) {
+
+  if (!isASTUnitSourceLocation(location)) {
+CXLoadedDiagnostic::decodeLocation(location, file, line, column, offset);
+return;
+  }
+
+  SourceLocation Loc = SourceLocation::getFromRawEncoding(location.int_data);
+
+  if (!location.ptr_data[0] || Loc.isInvalid())
+return createNullLocation(file, line, column, offset);
+
+  const SourceManager &SM =
+  *static_cast(location.ptr_data[0]);
+  SourceLocation SpellLoc = SM.getSpellingLoc(Loc);
+  std::pair LocInfo = SM.getDecomposedLoc(SpellLoc);
+  FileID FID = LocInfo.first;
+  unsigned FileOffset = LocInfo.second;
+
+  if (FID.isInvalid())
+return createNullLocation(file, line, column, offset);
+
+  if (file)
+*file = const_cast(SM.getFileEntryForID(FID));
+  if (line)
+*line = SM.getLineNumber(FID, FileOffset);
+  if (c

Re: [PATCH] D20136: Get default -fms-compatibility-version from cl.exe's version

2016-05-11 Thread Nico Weber via cfe-commits
thakis added inline comments.


Comment at: lib/Driver/MSVCToolChain.cpp:478
@@ +477,3 @@
+
+  const DWORD VersionSize = ::GetFileVersionInfoSizeW(ClExeWide.c_str(),
+  nullptr);

amccarth wrote:
> amccarth wrote:
> > thakis wrote:
> > > amccarth wrote:
> > > > Yes, it looks in the executable (which I tried to emphasize with the 
> > > > method name).
> > > > 
> > > > I don't think this is very expensive given that Explorer often makes 
> > > > zillions of such calls, but I'm open to other suggestions.
> > > > 
> > > > I know that you can't use a library that's newer than the compiler 
> > > > (because it may use new language features), but I don't know if that 
> > > > applies in the other direction or how we would safely and reliably map 
> > > > directory names to library versions and therefore to compiler versions.
> > > I agree that figuring out the right value for fmsc-version automatically 
> > > somehow is definitely something we should do.
> > > 
> > > I forgot that `getVisualStudioBinariesFolder` already works by looking 
> > > for cl.exe in PATH, so cl.exe's metadata is already warmed up in the disk 
> > > cache. However, GetFileVersionInfoW() probably opens cl.exe itself and 
> > > does some PE parsing to get at the version, and that probably is in cold 
> > > cache territory. 
> > > (https://msdn.microsoft.com/en-us/library/windows/desktop/ms647003(v=vs.85).aspx
> > >  suggests that this function might open several files).
> > > 
> > > `getVisualStudioBinariesFolder` checks:
> > > 
> > > 1. getenv("VCINSTALLDIR")
> > > 2. cl.exe in getenv("PATH")
> > > 3. registry (via getVisualStudioInstallDir)
> > > 
> > > The common cases are 1 and 3. For 1, for default installs, the version 
> > > number is part of the directory name (for default installs, what most 
> > > people have). For 3, the version number is in the registry key we query. 
> > > So in most cases we shouldn't have to look at cl.exe itself. And for the 
> > > cases where we would have to look, maybe it's ok to require an explicit 
> > > fmsc-version flag.
> > The version number in the directory name and the registry is the version 
> > number of Visual Studio not of the compiler.  Yes, we could do a mapping 
> > (VS 14 comes bundled with CL 19), assuming Microsoft continues to keep VS 
> > releases and compiler releases in sync, and it means this code will forever 
> > need updates to the mapping data.
> > 
> > The mapping would give just the major version number, which might be all 
> > that matters now, but if there's ever a CL 19.1 that has different 
> > compatibility requirements (and is maybe released out-of-band with Visual 
> > Studio), we'd be stuck.
> > 
> > Getting the actual version from the compiler seems the most accurate and 
> > future-proof way to check.  If that's too expensive, then maybe we should 
> > abandon the idea of detecting the default for compatibility.
> I'll do some research to figure out the actual costs.  I suspect that walking 
> the PATH for the executable may be far more expensive, but I'll get some 
> numbers and report back.
Compilers being released independently of VC versions and fractional compat 
numbers sounds like things we can worry about when they happen (probably not 
soon, right?).

We already walk PATH, so that wouldn't be an additional cost.

Be sure to measure cold disk cache perf impact (which is tricky on Windows 
since there's as far as I know no way to tell the OS to drop its caches). As 
far as I know file metadata is stored with the directory node on NTFS, so 
stating files doesn't warm up file content accesses.


http://reviews.llvm.org/D20136



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


[clang-tools-extra] r269195 - [include-fixer] Always ignore SFINAE contexts.

2016-05-11 Thread Benjamin Kramer via cfe-commits
Author: d0k
Date: Wed May 11 10:33:30 2016
New Revision: 269195

URL: http://llvm.org/viewvc/llvm-project?rev=269195&view=rev
Log:
[include-fixer] Always ignore SFINAE contexts.

This could lead to spurious includes being added for identifiers that
are supposed to be not available.

Modified:
clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp

Modified: clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp?rev=269195&r1=269194&r2=269195&view=diff
==
--- clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp (original)
+++ clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp Wed May 11 10:33:30 
2016
@@ -94,6 +94,10 @@ public:
   /// have the fully qualified name ready. Just query that.
   bool MaybeDiagnoseMissingCompleteType(clang::SourceLocation Loc,
 clang::QualType T) override {
+// Ignore spurious callbacks from SFINAE contexts.
+if (getCompilerInstance().getSema().isSFINAEContext())
+  return false;
+
 clang::ASTContext &context = getCompilerInstance().getASTContext();
 query(T.getUnqualifiedType().getAsString(context.getPrintingPolicy()), 
Loc);
 return false;
@@ -107,6 +111,10 @@ public:
 DeclContext *MemberContext,
 bool EnteringContext,
 const ObjCObjectPointerType *OPT) override 
{
+// Ignore spurious callbacks from SFINAE contexts.
+if (getCompilerInstance().getSema().isSFINAEContext())
+  return clang::TypoCorrection();
+
 /// If we have a scope specification, use that to get more precise results.
 std::string QueryString;
 if (SS && SS->getRange().isValid()) {


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


Re: [PATCH] D18745: [clang-tidy] Adds modernize-use-bool-literals check.

2016-05-11 Thread Piotr Padlewski via cfe-commits
Prazek closed this revision.
Prazek added a comment.

gg for your first check :)


http://reviews.llvm.org/D18745



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


[PATCH] D20171: Support for MSVS default calling convention options (/Gd, /Gz, /Gv, /Gr)

2016-05-11 Thread Alexander Makarov via cfe-commits
a.makarov created this revision.
a.makarov added a reviewer: rnk.
a.makarov added a subscriber: cfe-commits.

Patch for bug #27711

http://reviews.llvm.org/D20171

Files:
  include/clang/Basic/LangOptions.def
  include/clang/Basic/LangOptions.h
  include/clang/Driver/CC1Options.td
  include/clang/Driver/CLCompatOptions.td
  lib/AST/ASTContext.cpp
  lib/Driver/Tools.cpp
  lib/Frontend/CompilerInvocation.cpp
  lib/Sema/SemaDeclAttr.cpp
  test/CodeGenCXX/default_calling_conv.cpp

Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -6140,6 +6140,15 @@
   CmdArgs.push_back("-fms-memptr-rep=virtual");
   }
 
+  if (Args.getLastArg(options::OPT__SLASH_Gd))
+ CmdArgs.push_back("-fms-default-calling-conv=cdecl");
+  else if (Args.getLastArg(options::OPT__SLASH_Gr))
+ CmdArgs.push_back("-fms-default-calling-conv=fastcall");
+  else if (Args.getLastArg(options::OPT__SLASH_Gz))
+ CmdArgs.push_back("-fms-default-calling-conv=stdcall");
+  else if (Args.getLastArg(options::OPT__SLASH_Gv))
+ CmdArgs.push_back("-fms-default-calling-conv=vectorcall");
+
   if (Arg *A = Args.getLastArg(options::OPT_vtordisp_mode_EQ))
 A->render(Args, CmdArgs);
 
Index: lib/AST/ASTContext.cpp
===
--- lib/AST/ASTContext.cpp
+++ lib/AST/ASTContext.cpp
@@ -8607,7 +8607,25 @@
 
   if (LangOpts.MRTD && !IsVariadic) return CC_X86StdCall;
 
-  return Target->getDefaultCallingConv(TargetInfo::CCMT_Unknown);
+  if (LangOpts.MSVCCompat) {
+switch (LangOpts.getDefaultMSCallingConv()) {
+case LangOptions::DCC_None:
+  return Target->getDefaultCallingConv(TargetInfo::CCMT_Unknown);
+case LangOptions::DCC_CDecl:
+  return CC_C;
+case LangOptions::DCC_FastCall:
+  return CC_X86FastCall;
+case LangOptions::DCC_StdCall:
+  return CC_X86StdCall;
+case LangOptions::DCC_VectorCall:
+  // __vectorcall cannot be applied to variadic functions.
+  if (!IsVariadic)
+return CC_X86VectorCall;
+  else
+return Target->getDefaultCallingConv(TargetInfo::CCMT_Unknown);
+}
+  } else
+return Target->getDefaultCallingConv(TargetInfo::CCMT_Unknown);
 }
 
 bool ASTContext::isNearlyEmpty(const CXXRecordDecl *RD) const {
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -3895,11 +3895,12 @@
 
 // This convention is not valid for the target. Use the default function or
 // method calling convention.
-TargetInfo::CallingConvMethodType MT = TargetInfo::CCMT_Unknown;
-if (FD)
-  MT = FD->isCXXInstanceMember() ? TargetInfo::CCMT_Member : 
-TargetInfo::CCMT_NonMember;
-CC = TI.getDefaultCallingConv(MT);
+bool isCXXMethod = false, isVariadic = false;
+if (FD) {
+  isCXXMethod = FD->isCXXInstanceMember();
+  isVariadic = FD->isVariadic();
+}
+CC = Context.getDefaultCallingConvention(isVariadic, isCXXMethod);
   }
 
   attr.setProcessingCache((unsigned) CC);
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -1849,6 +1849,34 @@
 Opts.setMSPointerToMemberRepresentationMethod(InheritanceModel);
   }
 
+  // Check for MS default calling conventions being specified.
+  if (Arg *A = Args.getLastArg(OPT_fms_ms_default_calling_conv_EQ)) {
+LangOptions::DefaultMSCallingConvention DefaultCC =
+llvm::StringSwitch(
+A->getValue())
+.Case("cdecl", LangOptions::DCC_CDecl)
+.Case("fastcall", LangOptions::DCC_FastCall)
+.Case("stdcall", LangOptions::DCC_StdCall)
+.Case("vectorcall", LangOptions::DCC_VectorCall)
+.Default(LangOptions::DCC_None);
+if (DefaultCC == LangOptions::DCC_None)
+  Diags.Report(diag::err_drv_invalid_value)
+  << "-fms-default-calling-conv" << A->getValue();
+
+llvm::Triple T(TargetOpts.Triple);
+llvm::Triple::ArchType Arch = T.getArch();
+bool emitError = (DefaultCC == LangOptions::DCC_FastCall ||
+  DefaultCC == LangOptions::DCC_StdCall) &&
+ Arch != llvm::Triple::x86;
+emitError |= DefaultCC == LangOptions::DCC_VectorCall &&
+ !(Arch == llvm::Triple::x86 || Arch == llvm::Triple::x86_64);
+if (emitError)
+  Diags.Report(diag::err_drv_argument_not_allowed_with)
+  << A->getSpelling() << T.getTriple();
+else
+  Opts.setDefaultMSCallingConv(DefaultCC);
+  }
+
   // Check if -fopenmp is specified.
   Opts.OpenMP = Args.hasArg(options::OPT_fopenmp);
   Opts.OpenMPUseTLS =
Index: include/clang/Driver/CC1Options.td
===
--- include/clang/Driver/C

Re: [PATCH] D19547: [clang-tidy] Add FixIt for swapping arguments in string-constructor-checker.

2016-05-11 Thread Etienne Bergeron via cfe-commits
etienneb updated this revision to Diff 56919.
etienneb marked an inline comment as done.
etienneb added a comment.

use new API. code simplification


http://reviews.llvm.org/D19547

Files:
  clang-tidy/misc/StringConstructorCheck.cpp
  test/clang-tidy/misc-string-constructor.cpp

Index: test/clang-tidy/misc-string-constructor.cpp
===
--- test/clang-tidy/misc-string-constructor.cpp
+++ test/clang-tidy/misc-string-constructor.cpp
@@ -21,9 +21,11 @@
 
 void Test() {
   std::string str('x', 4);
-  // CHECK-MESSAGES: [[@LINE-1]]:15: warning: constructor parameters are 
probably swapped [misc-string-constructor]
+  // CHECK-MESSAGES: [[@LINE-1]]:15: warning: string constructor parameters 
are probably swapped; expecting string(count, character) 
[misc-string-constructor]
+  // CHECK-FIXES: std::string str(4, 'x');  
   std::wstring wstr(L'x', 4);
-  // CHECK-MESSAGES: [[@LINE-1]]:16: warning: constructor parameters are 
probably swapped
+  // CHECK-MESSAGES: [[@LINE-1]]:16: warning: string constructor parameters 
are probably swapped
+  // CHECK-FIXES: std::wstring wstr(4, L'x');
   std::string s0(0, 'x');
   // CHECK-MESSAGES: [[@LINE-1]]:15: warning: constructor creating an empty 
string
   std::string s1(-4, 'x');
Index: clang-tidy/misc/StringConstructorCheck.cpp
===
--- clang-tidy/misc/StringConstructorCheck.cpp
+++ clang-tidy/misc/StringConstructorCheck.cpp
@@ -10,6 +10,7 @@
 #include "StringConstructorCheck.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Tooling/FixIt.h"
 
 using namespace clang::ast_matchers;
 
@@ -54,7 +55,7 @@
   isDefinition(),
   hasType(pointerType(pointee(isAnyCharacter(), isConstQualified(,
   hasInitializer(ignoringParenImpCasts(BoundStringLiteral)));
-  auto ConstStrLiteral = expr(ignoringParenImpCasts(anyOf(
+  const auto ConstStrLiteral = expr(ignoringParenImpCasts(anyOf(
   BoundStringLiteral, declRefExpr(hasDeclaration(anyOf(
   ConstPtrStrLiteralDecl, 
ConstStrLiteralDecl));
 
@@ -88,7 +89,7 @@
   // Detect the expression: string("...", 0);
   hasArgument(1, ZeroExpr.bind("empty-string")),
   // Detect the expression: string("...", -4);
-  hasArgument(1, NegativeExpr.bind("negative-length")),
  
+  hasArgument(1, NegativeExpr.bind("negative-length")),
   // Detect the expression: string("lit", 0x1234567);
   hasArgument(1, LargeLengthExpr.bind("large-length")),
   // Detect the expression: string("lit", 5)
@@ -100,11 +101,18 @@
 }
 
 void StringConstructorCheck::check(const MatchFinder::MatchResult &Result) {
-  const auto *E = Result.Nodes.getNodeAs("constructor");
+  const ASTContext &Ctx = *Result.Context;
+  const auto *E = Result.Nodes.getNodeAs("constructor");
+  assert(E && "missing constructor expression");
   SourceLocation Loc = E->getLocStart();
 
   if (Result.Nodes.getNodeAs("swapped-parameter")) {
-diag(Loc, "constructor parameters are probably swapped");
+const Expr* P0 = E->getArg(0);
+const Expr* P1 = E->getArg(1);
+diag(Loc, "string constructor parameters are probably swapped;"
+  " expecting string(count, character)")
+<< tooling::fixit::createReplacement(*P0, *P1, Ctx)
+<< tooling::fixit::createReplacement(*P1, *P0, Ctx);
   } else if (Result.Nodes.getNodeAs("empty-string")) {
 diag(Loc, "constructor creating an empty string");
   } else if (Result.Nodes.getNodeAs("negative-length")) {


Index: test/clang-tidy/misc-string-constructor.cpp
===
--- test/clang-tidy/misc-string-constructor.cpp
+++ test/clang-tidy/misc-string-constructor.cpp
@@ -21,9 +21,11 @@
 
 void Test() {
   std::string str('x', 4);
-  // CHECK-MESSAGES: [[@LINE-1]]:15: warning: constructor parameters are probably swapped [misc-string-constructor]
+  // CHECK-MESSAGES: [[@LINE-1]]:15: warning: string constructor parameters are probably swapped; expecting string(count, character) [misc-string-constructor]
+  // CHECK-FIXES: std::string str(4, 'x');  
   std::wstring wstr(L'x', 4);
-  // CHECK-MESSAGES: [[@LINE-1]]:16: warning: constructor parameters are probably swapped
+  // CHECK-MESSAGES: [[@LINE-1]]:16: warning: string constructor parameters are probably swapped
+  // CHECK-FIXES: std::wstring wstr(4, L'x');
   std::string s0(0, 'x');
   // CHECK-MESSAGES: [[@LINE-1]]:15: warning: constructor creating an empty string
   std::string s1(-4, 'x');
Index: clang-tidy/misc/StringConstructorCheck.cpp
===
--- clang-tidy/misc/StringConstructorCheck.cpp
+++ clang-tidy/misc/StringConstructorCheck.cpp
@@ -10,6 +10,7 @@
 #include "StringConstructorCheck.h"
 #include "clang/AST/ASTCo

Re: [PATCH] D20136: Get default -fms-compatibility-version from cl.exe's version

2016-05-11 Thread Adrian McCarthy via cfe-commits
amccarth added inline comments.


Comment at: lib/Driver/MSVCToolChain.cpp:478
@@ +477,3 @@
+
+  const DWORD VersionSize = ::GetFileVersionInfoSizeW(ClExeWide.c_str(),
+  nullptr);

thakis wrote:
> amccarth wrote:
> > amccarth wrote:
> > > thakis wrote:
> > > > amccarth wrote:
> > > > > Yes, it looks in the executable (which I tried to emphasize with the 
> > > > > method name).
> > > > > 
> > > > > I don't think this is very expensive given that Explorer often makes 
> > > > > zillions of such calls, but I'm open to other suggestions.
> > > > > 
> > > > > I know that you can't use a library that's newer than the compiler 
> > > > > (because it may use new language features), but I don't know if that 
> > > > > applies in the other direction or how we would safely and reliably 
> > > > > map directory names to library versions and therefore to compiler 
> > > > > versions.
> > > > I agree that figuring out the right value for fmsc-version 
> > > > automatically somehow is definitely something we should do.
> > > > 
> > > > I forgot that `getVisualStudioBinariesFolder` already works by looking 
> > > > for cl.exe in PATH, so cl.exe's metadata is already warmed up in the 
> > > > disk cache. However, GetFileVersionInfoW() probably opens cl.exe itself 
> > > > and does some PE parsing to get at the version, and that probably is in 
> > > > cold cache territory. 
> > > > (https://msdn.microsoft.com/en-us/library/windows/desktop/ms647003(v=vs.85).aspx
> > > >  suggests that this function might open several files).
> > > > 
> > > > `getVisualStudioBinariesFolder` checks:
> > > > 
> > > > 1. getenv("VCINSTALLDIR")
> > > > 2. cl.exe in getenv("PATH")
> > > > 3. registry (via getVisualStudioInstallDir)
> > > > 
> > > > The common cases are 1 and 3. For 1, for default installs, the version 
> > > > number is part of the directory name (for default installs, what most 
> > > > people have). For 3, the version number is in the registry key we 
> > > > query. So in most cases we shouldn't have to look at cl.exe itself. And 
> > > > for the cases where we would have to look, maybe it's ok to require an 
> > > > explicit fmsc-version flag.
> > > The version number in the directory name and the registry is the version 
> > > number of Visual Studio not of the compiler.  Yes, we could do a mapping 
> > > (VS 14 comes bundled with CL 19), assuming Microsoft continues to keep VS 
> > > releases and compiler releases in sync, and it means this code will 
> > > forever need updates to the mapping data.
> > > 
> > > The mapping would give just the major version number, which might be all 
> > > that matters now, but if there's ever a CL 19.1 that has different 
> > > compatibility requirements (and is maybe released out-of-band with Visual 
> > > Studio), we'd be stuck.
> > > 
> > > Getting the actual version from the compiler seems the most accurate and 
> > > future-proof way to check.  If that's too expensive, then maybe we should 
> > > abandon the idea of detecting the default for compatibility.
> > I'll do some research to figure out the actual costs.  I suspect that 
> > walking the PATH for the executable may be far more expensive, but I'll get 
> > some numbers and report back.
> Compilers being released independently of VC versions and fractional compat 
> numbers sounds like things we can worry about when they happen (probably not 
> soon, right?).
> 
> We already walk PATH, so that wouldn't be an additional cost.
> 
> Be sure to measure cold disk cache perf impact (which is tricky on Windows 
> since there's as far as I know no way to tell the OS to drop its caches). As 
> far as I know file metadata is stored with the directory node on NTFS, so 
> stating files doesn't warm up file content accesses.
> Compilers being released independently of VC versions and fractional compat 
> numbers sounds like things we can worry about when they happen (probably not 
> soon, right?).

It already happens.  Herb Sutter talks about it in one of his blogs:  "Soon 
after VC++11 ships we have announced we will do out-of-band releases for 
additional C++11 conformance which will naturally also include more C11 
features that are in the C subset of C++11."  In this case, it's just the build 
number (of major.minor.build) that's updating, but it's for increasing 
conformance, which is exactly a compatibility issue.

> We already walk PATH, so that wouldn't be an additional cost.

I suspect we may be walking it more than once, which can be expensive even if 
the cache is all warmed up.  This is one of the things I'm checking.  If it's a 
problem, I'll propose a patch to cache the result from the first walk.

> stating files doesn't warm up file content accesses.

That is correct.


http://reviews.llvm.org/D20136



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

[clang-tools-extra] r269194 - [include-fixer] Also output the location where we found an unknown identifier.

2016-05-11 Thread Benjamin Kramer via cfe-commits
Author: d0k
Date: Wed May 11 10:33:28 2016
New Revision: 269194

URL: http://llvm.org/viewvc/llvm-project?rev=269194&view=rev
Log:
[include-fixer] Also output the location where we found an unknown identifier.

For debugging only, makes it a bit easier when there are queries coming
out of headers.

Modified:
clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp

Modified: clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp?rev=269194&r1=269193&r2=269194&view=diff
==
--- clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp (original)
+++ clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp Wed May 11 10:33:28 
2016
@@ -95,7 +95,7 @@ public:
   bool MaybeDiagnoseMissingCompleteType(clang::SourceLocation Loc,
 clang::QualType T) override {
 clang::ASTContext &context = getCompilerInstance().getASTContext();
-query(T.getUnqualifiedType().getAsString(context.getPrintingPolicy()));
+query(T.getUnqualifiedType().getAsString(context.getPrintingPolicy()), 
Loc);
 return false;
   }
 
@@ -141,7 +141,7 @@ public:
   QueryString = Typo.getAsString();
 }
 
-return query(QueryString);
+return query(QueryString, Typo.getLoc());
   }
 
   StringRef filename() const { return Filename; }
@@ -226,14 +226,16 @@ public:
 
 private:
   /// Query the database for a given identifier.
-  clang::TypoCorrection query(StringRef Query) {
+  clang::TypoCorrection query(StringRef Query, SourceLocation Loc) {
 assert(!Query.empty() && "Empty query!");
 
 // Save database lookups by not looking up identifiers multiple times.
 if (!SeenQueries.insert(Query).second)
   return clang::TypoCorrection();
 
-DEBUG(llvm::dbgs() << "Looking up " << Query << " ... ");
+DEBUG(llvm::dbgs() << "Looking up '" << Query << "' at ");
+DEBUG(Loc.print(llvm::dbgs(), getCompilerInstance().getSourceManager()));
+DEBUG(llvm::dbgs() << " ...");
 
 std::string error_text;
 auto SearchReply = XrefsDBMgr.search(Query);


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


Re: [PATCH] D20171: Support for MSVS default calling convention options (/Gd, /Gz, /Gv, /Gr)

2016-05-11 Thread Reid Kleckner via cfe-commits
rnk added inline comments.


Comment at: include/clang/Basic/LangOptions.def:220
@@ -218,3 +219,3 @@
 
 LANGOPT(MRTD , 1, 0, "-mrtd calling convention")
 BENIGN_LANGOPT(DelayedTemplateParsing , 1, 0, "delayed template parsing")

Let's get rid of this and have it use a single DefaultCallingConv LangOpt.


Comment at: include/clang/Driver/CC1Options.td:613
@@ -612,1 +612,3 @@
   HelpText<"Allow function arguments and returns of type half">;
+def fms_ms_default_calling_conv_EQ : Joined<["-"], 
"fms-default-calling-conv=">,
+  HelpText<"Set default MS calling convention">;

Let's unify the -cc1 layer with -mrtd, and just have a single 
-fdefault-calling-convention=stdcall/etc flag.


Comment at: lib/AST/ASTContext.cpp:8610
@@ -8609,2 +8609,3 @@
 
-  return Target->getDefaultCallingConv(TargetInfo::CCMT_Unknown);
+  if (LangOpts.MSVCCompat) {
+switch (LangOpts.getDefaultMSCallingConv()) {

I don't think we need to guard this on MSVCCompat, it'll be unset usually. It 
will also help us avoid a special path for MRTD.


Comment at: lib/AST/ASTContext.cpp:8616-8619
@@ +8615,6 @@
+  return CC_C;
+case LangOptions::DCC_FastCall:
+  return CC_X86FastCall;
+case LangOptions::DCC_StdCall:
+  return CC_X86StdCall;
+case LangOptions::DCC_VectorCall:

Neither fastcall nor stdcall can be applied to variadic functions.


http://reviews.llvm.org/D20171



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


Re: [PATCH] D19156: [ms][dll] #27212: Generating of implicit special members should take into account MSVC compatibility version

2016-05-11 Thread Reid Kleckner via cfe-commits
rnk added inline comments.


Comment at: lib/Sema/SemaDeclCXX.cpp:4816
@@ -4815,1 +4815,3 @@
+if ((MD->isMoveAssignmentOperator() ||
+ (CXXC && CXXC->isMoveConstructor())) &&
 !getLangOpts().isCompatibleWithMSVC(LangOptions::MSVC2015))

The move constructor part of this is definitely a good fix though.


http://reviews.llvm.org/D19156



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


Re: [PATCH] D20171: Support for MSVS default calling convention options (/Gd, /Gz, /Gv, /Gr)

2016-05-11 Thread David Majnemer via cfe-commits
majnemer added a subscriber: majnemer.


Comment at: lib/AST/ASTContext.cpp:8604-8606
@@ -8603,5 +8603,5 @@
 bool IsCXXMethod) const {
   // Pass through to the C++ ABI object
   if (IsCXXMethod)
 return ABI->getDefaultMethodCallConv(IsVariadic);
 

Do these flags not have an effect on methods?


http://reviews.llvm.org/D20171



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


Re: [PATCH] D20103: PR27132: Proper mangling for __unaligned qualifier (now with both PR27367 and PR27666 fixed)

2016-05-11 Thread David Majnemer via cfe-commits
majnemer accepted this revision.
majnemer added a comment.
This revision is now accepted and ready to land.

LGTM, thanks!


http://reviews.llvm.org/D20103



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


Re: [PATCH] D20171: Support for MSVS default calling convention options (/Gd, /Gz, /Gv, /Gr)

2016-05-11 Thread Reid Kleckner via cfe-commits
rnk added inline comments.


Comment at: lib/AST/ASTContext.cpp:8604-8606
@@ -8603,5 +8603,5 @@
 bool IsCXXMethod) const {
   // Pass through to the C++ ABI object
   if (IsCXXMethod)
 return ABI->getDefaultMethodCallConv(IsVariadic);
 

majnemer wrote:
> Do these flags not have an effect on methods?
Nope:
https://msdn.microsoft.com/en-us/library/46t77ak2.aspx
"for all functions except C++ member functions..."


http://reviews.llvm.org/D20171



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


Re: [PATCH] D20171: Support for MSVS default calling convention options (/Gd, /Gz, /Gv, /Gr)

2016-05-11 Thread David Majnemer via cfe-commits
majnemer added inline comments.


Comment at: lib/AST/ASTContext.cpp:8604-8606
@@ -8603,5 +8603,5 @@
 bool IsCXXMethod) const {
   // Pass through to the C++ ABI object
   if (IsCXXMethod)
 return ABI->getDefaultMethodCallConv(IsVariadic);
 

rnk wrote:
> majnemer wrote:
> > Do these flags not have an effect on methods?
> Nope:
> https://msdn.microsoft.com/en-us/library/46t77ak2.aspx
> "for all functions except C++ member functions..."
Yeah, that's what the documentation says.  But do they really have no effect? :)


http://reviews.llvm.org/D20171



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


Re: [Clang] Convergent Attribute

2016-05-11 Thread Ettore Speziale via cfe-commits
Hello,

>> CUDA? In any case, I don't see how the restriction helps users, and the 
>> attribute at the IR level has a well-defined meaning regardless. If a user 
>> were to have a use case, they'd simply find the restriction arbitrary and 
>> frustrating.
> 
> Yes, CUDA was already considered as well. I just think that compilers should 
> help to reduce amount of erroneous or meaningless use cases. That's one of 
> the reasons to have language options for the attributes. But I don't feel 
> strongly about this particular case anyways. So let's make it language 
> independent then. ;)

Is the patch OK now or you guys want to apply some other modifications?



convergent.diff
Description: Binary data


Thanks,
Ettore Speziale

--
Ettore Speziale — Compiler Engineer
speziale.ett...@gmail.com
espezi...@apple.com
--

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


r269200 - [Hexagon] Avoid spurious failures in test/Driver/hexagon-toolchain-elf.c

2016-05-11 Thread Krzysztof Parzyszek via cfe-commits
Author: kparzysz
Date: Wed May 11 11:11:22 2016
New Revision: 269200

URL: http://llvm.org/viewvc/llvm-project?rev=269200&view=rev
Log:
[Hexagon] Avoid spurious failures in test/Driver/hexagon-toolchain-elf.c

Modified:
cfe/trunk/test/Driver/hexagon-toolchain-elf.c

Modified: cfe/trunk/test/Driver/hexagon-toolchain-elf.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/hexagon-toolchain-elf.c?rev=269200&r1=269199&r2=269200&view=diff
==
--- cfe/trunk/test/Driver/hexagon-toolchain-elf.c (original)
+++ cfe/trunk/test/Driver/hexagon-toolchain-elf.c Wed May 11 11:11:22 2016
@@ -63,7 +63,7 @@
 // RUN:   %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHECK020 %s
 // CHECK020: "-cc1" {{.*}} "-target-cpu" "hexagonv4"
-// CHECK020: "hexagon-link" 
{{.*}}/Inputs/hexagon_tree/Tools/bin/../target/hexagon/lib/v4/crt0
+// CHECK020: 
hexagon-link{{.*}}/Inputs/hexagon_tree/Tools/bin/../target/hexagon/lib/v4/crt0
 
 // RUN: %clang -### -target hexagon-unknown-elf \
 // RUN:   -ccc-install-dir %S/Inputs/hexagon_tree/Tools/bin \
@@ -71,7 +71,7 @@
 // RUN:   %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHECK021 %s
 // CHECK021: "-cc1" {{.*}} "-target-cpu" "hexagonv5"
-// CHECK021: "hexagon-link" 
{{.*}}/Inputs/hexagon_tree/Tools/bin/../target/hexagon/lib/v5/crt0
+// CHECK021: 
hexagon-link{{.*}}/Inputs/hexagon_tree/Tools/bin/../target/hexagon/lib/v5/crt0
 
 // RUN: %clang -### -target hexagon-unknown-elf \
 // RUN:   -ccc-install-dir %S/Inputs/hexagon_tree/Tools/bin \
@@ -79,7 +79,7 @@
 // RUN:   %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHECK022 %s
 // CHECK022: "-cc1" {{.*}} "-target-cpu" "hexagonv55"
-// CHECK022: "hexagon-link" 
{{.*}}/Inputs/hexagon_tree/Tools/bin/../target/hexagon/lib/v55/crt0
+// CHECK022: 
hexagon-link{{.*}}/Inputs/hexagon_tree/Tools/bin/../target/hexagon/lib/v55/crt0
 
 // RUN: %clang -### -target hexagon-unknown-elf \
 // RUN:   -ccc-install-dir %S/Inputs/hexagon_tree/Tools/bin \
@@ -87,7 +87,7 @@
 // RUN:   %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHECK023 %s
 // CHECK023: "-cc1" {{.*}} "-target-cpu" "hexagonv60"
-// CHECK023: "hexagon-link" 
{{.*}}/Inputs/hexagon_tree/Tools/bin/../target/hexagon/lib/v60/crt0
+// CHECK023: 
hexagon-link{{.*}}/Inputs/hexagon_tree/Tools/bin/../target/hexagon/lib/v60/crt0
 
 // 
-
 // Test Linker related args


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


Re: [PATCH] D20168: [CodeGen] Handle structs directly in AMDGPUABIInfo

2016-05-11 Thread Matt Arsenault via cfe-commits
arsenm added a comment.

Needs tests


http://reviews.llvm.org/D20168



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


r269201 - Update clang support on recent Haiku

2016-05-11 Thread Reid Kleckner via cfe-commits
Author: rnk
Date: Wed May 11 11:19:05 2016
New Revision: 269201

URL: http://llvm.org/viewvc/llvm-project?rev=269201&view=rev
Log:
Update clang support on recent Haiku

[ Copied from https://llvm.org/bugs/show_bug.cgi?id=26404 ]

clang support on Haiku is lagging a bit, and missing on x86_64.

This patch updates support for x86 and add support for x86_64. It should
apply directly to trunk and it's harmless in the sense that it only
affects Haiku.

Reviewers: rnk, rsmith

Patch by Jérôme Duval

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

Modified:
cfe/trunk/lib/Basic/Targets.cpp
cfe/trunk/lib/Driver/Driver.cpp
cfe/trunk/lib/Driver/ToolChains.cpp
cfe/trunk/lib/Driver/ToolChains.h
cfe/trunk/lib/Frontend/InitHeaderSearch.cpp

Modified: cfe/trunk/lib/Basic/Targets.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets.cpp?rev=269201&r1=269200&r2=269201&view=diff
==
--- cfe/trunk/lib/Basic/Targets.cpp (original)
+++ cfe/trunk/lib/Basic/Targets.cpp Wed May 11 11:19:05 2016
@@ -382,6 +382,29 @@ public:
   : OSTargetInfo(Triple, Opts) {}
 };
 
+// Haiku Target
+template
+class HaikuTargetInfo : public OSTargetInfo {
+protected:
+  void getOSDefines(const LangOptions &Opts, const llvm::Triple &Triple,
+MacroBuilder &Builder) const override {
+// Haiku defines; list based off of gcc output
+Builder.defineMacro("__HAIKU__");
+Builder.defineMacro("__ELF__");
+DefineStd(Builder, "unix", Opts);
+  }
+public:
+  HaikuTargetInfo(const llvm::Triple &Triple, const TargetOptions &Opts)
+  : OSTargetInfo(Triple, Opts) {
+this->SizeType = TargetInfo::UnsignedLong;
+this->IntPtrType = TargetInfo::SignedLong;
+this->PtrDiffType = TargetInfo::SignedLong;
+this->ProcessIDType = TargetInfo::SignedLong;
+this->TLSSupported = false;
+
+  }
+};
+
 // Minix Target
 template
 class MinixTargetInfo : public OSTargetInfo {
@@ -4088,21 +4111,15 @@ public:
 };
 
 // x86-32 Haiku target
-class HaikuX86_32TargetInfo : public X86_32TargetInfo {
+class HaikuX86_32TargetInfo : public HaikuTargetInfo {
 public:
   HaikuX86_32TargetInfo(const llvm::Triple &Triple, const TargetOptions &Opts)
-  : X86_32TargetInfo(Triple, Opts) {
-SizeType = UnsignedLong;
-IntPtrType = SignedLong;
-PtrDiffType = SignedLong;
-ProcessIDType = SignedLong;
-this->TLSSupported = false;
+: HaikuTargetInfo(Triple, Opts) {
   }
   void getTargetDefines(const LangOptions &Opts,
 MacroBuilder &Builder) const override {
-X86_32TargetInfo::getTargetDefines(Opts, Builder);
+HaikuTargetInfo::getTargetDefines(Opts, Builder);
 Builder.defineMacro("__INTEL__");
-Builder.defineMacro("__HAIKU__");
   }
 };
 
@@ -8360,6 +8377,8 @@ static TargetInfo *AllocateTarget(const
 return new MicrosoftX86_64TargetInfo(Triple, Opts);
   }
 }
+case llvm::Triple::Haiku:
+  return new HaikuTargetInfo(Triple, Opts);
 case llvm::Triple::NaCl:
   return new NaClTargetInfo(Triple, Opts);
 case llvm::Triple::PS4:

Modified: cfe/trunk/lib/Driver/Driver.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Driver.cpp?rev=269201&r1=269200&r2=269201&view=diff
==
--- cfe/trunk/lib/Driver/Driver.cpp (original)
+++ cfe/trunk/lib/Driver/Driver.cpp Wed May 11 11:19:05 2016
@@ -2425,6 +2425,9 @@ const ToolChain &Driver::getToolChain(co
   ToolChain *&TC = ToolChains[Target.str()];
   if (!TC) {
 switch (Target.getOS()) {
+case llvm::Triple::Haiku:
+  TC = new toolchains::Haiku(*this, Target, Args);
+  break;
 case llvm::Triple::CloudABI:
   TC = new toolchains::CloudABI(*this, Target, Args);
   break;

Modified: cfe/trunk/lib/Driver/ToolChains.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains.cpp?rev=269201&r1=269200&r2=269201&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains.cpp Wed May 11 11:19:05 2016
@@ -3040,6 +3040,38 @@ SanitizerMask CloudABI::getDefaultSaniti
   return SanitizerKind::SafeStack;
 }
 
+/// Haiku - Haiku tool chain which can call as(1) and ld(1) directly.
+
+Haiku::Haiku(const Driver &D, const llvm::Triple& Triple, const ArgList &Args)
+  : Generic_ELF(D, Triple, Args) {
+
+}
+
+void Haiku::AddClangCXXStdlibIncludeArgs(const ArgList &DriverArgs,
+  ArgStringList &CC1Args) const {
+  if (DriverArgs.hasArg(options::OPT_nostdlibinc) ||
+  DriverArgs.hasArg(options::OPT_nostdincxx))
+return;
+
+  switch (GetCXXStdlibType(DriverArgs)) {
+  case ToolChain::CST_Libcxx:
+addSystemInclude(DriverArgs, CC1Args,
+ getDriver().SysRoot + "/system/develop/headers/c++/v1");
+break;

Re: [PATCH] D16797: Update clang support on recent Haiku

2016-05-11 Thread Reid Kleckner via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL269201: Update clang support on recent Haiku (authored by 
rnk).

Changed prior to commit:
  http://reviews.llvm.org/D16797?vs=53464&id=56929#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D16797

Files:
  cfe/trunk/lib/Basic/Targets.cpp
  cfe/trunk/lib/Driver/Driver.cpp
  cfe/trunk/lib/Driver/ToolChains.cpp
  cfe/trunk/lib/Driver/ToolChains.h
  cfe/trunk/lib/Frontend/InitHeaderSearch.cpp

Index: cfe/trunk/lib/Driver/ToolChains.cpp
===
--- cfe/trunk/lib/Driver/ToolChains.cpp
+++ cfe/trunk/lib/Driver/ToolChains.cpp
@@ -3040,6 +3040,38 @@
   return SanitizerKind::SafeStack;
 }
 
+/// Haiku - Haiku tool chain which can call as(1) and ld(1) directly.
+
+Haiku::Haiku(const Driver &D, const llvm::Triple& Triple, const ArgList &Args)
+  : Generic_ELF(D, Triple, Args) {
+
+}
+
+void Haiku::AddClangCXXStdlibIncludeArgs(const ArgList &DriverArgs,
+  ArgStringList &CC1Args) const {
+  if (DriverArgs.hasArg(options::OPT_nostdlibinc) ||
+  DriverArgs.hasArg(options::OPT_nostdincxx))
+return;
+
+  switch (GetCXXStdlibType(DriverArgs)) {
+  case ToolChain::CST_Libcxx:
+addSystemInclude(DriverArgs, CC1Args,
+ getDriver().SysRoot + "/system/develop/headers/c++/v1");
+break;
+  case ToolChain::CST_Libstdcxx:
+addSystemInclude(DriverArgs, CC1Args,
+ getDriver().SysRoot + "/system/develop/headers/c++");
+addSystemInclude(DriverArgs, CC1Args,
+ getDriver().SysRoot + "/system/develop/headers/c++/backward");
+
+StringRef Triple = getTriple().str();
+addSystemInclude(DriverArgs, CC1Args,
+ getDriver().SysRoot + "/system/develop/headers/c++/" +
+ Triple);
+break;
+  }
+}
+
 /// OpenBSD - OpenBSD tool chain which can call as(1) and ld(1) directly.
 
 OpenBSD::OpenBSD(const Driver &D, const llvm::Triple &Triple,
Index: cfe/trunk/lib/Driver/Driver.cpp
===
--- cfe/trunk/lib/Driver/Driver.cpp
+++ cfe/trunk/lib/Driver/Driver.cpp
@@ -2425,6 +2425,9 @@
   ToolChain *&TC = ToolChains[Target.str()];
   if (!TC) {
 switch (Target.getOS()) {
+case llvm::Triple::Haiku:
+  TC = new toolchains::Haiku(*this, Target, Args);
+  break;
 case llvm::Triple::CloudABI:
   TC = new toolchains::CloudABI(*this, Target, Args);
   break;
Index: cfe/trunk/lib/Driver/ToolChains.h
===
--- cfe/trunk/lib/Driver/ToolChains.h
+++ cfe/trunk/lib/Driver/ToolChains.h
@@ -680,6 +680,18 @@
   void findGccLibDir();
 };
 
+class LLVM_LIBRARY_VISIBILITY Haiku : public Generic_ELF {
+public:
+  Haiku(const Driver &D, const llvm::Triple &Triple,
+  const llvm::opt::ArgList &Args);
+
+  bool isPIEDefault() const override { return getTriple().getArch() == llvm::Triple::x86_64; }
+
+  void
+  AddClangCXXStdlibIncludeArgs(const llvm::opt::ArgList &DriverArgs,
+  llvm::opt::ArgStringList &CC1Args) const override;
+};
+
 class LLVM_LIBRARY_VISIBILITY OpenBSD : public Generic_ELF {
 public:
   OpenBSD(const Driver &D, const llvm::Triple &Triple,
Index: cfe/trunk/lib/Basic/Targets.cpp
===
--- cfe/trunk/lib/Basic/Targets.cpp
+++ cfe/trunk/lib/Basic/Targets.cpp
@@ -382,6 +382,29 @@
   : OSTargetInfo(Triple, Opts) {}
 };
 
+// Haiku Target
+template
+class HaikuTargetInfo : public OSTargetInfo {
+protected:
+  void getOSDefines(const LangOptions &Opts, const llvm::Triple &Triple,
+MacroBuilder &Builder) const override {
+// Haiku defines; list based off of gcc output
+Builder.defineMacro("__HAIKU__");
+Builder.defineMacro("__ELF__");
+DefineStd(Builder, "unix", Opts);
+  }
+public:
+  HaikuTargetInfo(const llvm::Triple &Triple, const TargetOptions &Opts)
+  : OSTargetInfo(Triple, Opts) {
+this->SizeType = TargetInfo::UnsignedLong;
+this->IntPtrType = TargetInfo::SignedLong;
+this->PtrDiffType = TargetInfo::SignedLong;
+this->ProcessIDType = TargetInfo::SignedLong;
+this->TLSSupported = false;
+
+  }
+};
+
 // Minix Target
 template
 class MinixTargetInfo : public OSTargetInfo {
@@ -4088,21 +4111,15 @@
 };
 
 // x86-32 Haiku target
-class HaikuX86_32TargetInfo : public X86_32TargetInfo {
+class HaikuX86_32TargetInfo : public HaikuTargetInfo {
 public:
   HaikuX86_32TargetInfo(const llvm::Triple &Triple, const TargetOptions &Opts)
-  : X86_32TargetInfo(Triple, Opts) {
-SizeType = UnsignedLong;
-IntPtrType = SignedLong;
-PtrDiffType = SignedLong;
-ProcessIDType = SignedLong;
-this->TLSSupported = false;
+: HaikuTargetInfo(Triple, Opts) {
   }
   void getTargetDefines(const LangOptions &Opts,
 

Re: [PATCH] D19703: [clang-tidy] Improve misc-redundant-expression and decrease false-positive

2016-05-11 Thread Alexander Kornienko via cfe-commits
alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.

LG.



Comment at: clang-tidy/misc/RedundantExpressionCheck.cpp:140
@@ +139,3 @@
+  const LangOptions &LO = Finder->getASTContext().getLangOpts();
+  std::set Names(NameRefs.begin(), NameRefs.end());
+  SourceLocation Loc = Node.getExprLoc();

Is there a way to not create a set on each call to the matcher? There also 
might be a more suitable container in llvm/ADT.


Comment at: clang-tidy/misc/RedundantExpressionCheck.cpp:189
@@ +188,3 @@
+  anyOf(
+  hasOverloadedOperatorName("-"), hasOverloadedOperatorName("/"),
+  hasOverloadedOperatorName("%"), hasOverloadedOperatorName("|"),

We should add a `hasAnyOverloadedOperatorName` matcher at some point.


http://reviews.llvm.org/D19703



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


r269202 - Embed bitcode in object file (clang cc1 part)

2016-05-11 Thread Steven Wu via cfe-commits
Author: steven_wu
Date: Wed May 11 11:26:03 2016
New Revision: 269202

URL: http://llvm.org/viewvc/llvm-project?rev=269202&view=rev
Log:
Embed bitcode in object file (clang cc1 part)

Summary:
Teach clang to embed bitcode inside bitcode. When -fembed-bitcode cc1
option is used, clang will embed both the input bitcode and cc1
commandline into the bitcode in special sections before compiling to
the object file.  Using -fembed-bitcode-marker will only introduce a
marker in both sections.

Depends on D17390

Reviewers: rsmith

Subscribers: yaron.keren, vsk, cfe-commits

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

Added:
cfe/trunk/test/Frontend/embed-bitcode.ll
Modified:
cfe/trunk/include/clang/CodeGen/BackendUtil.h
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/include/clang/Frontend/CodeGenOptions.def
cfe/trunk/include/clang/Frontend/CodeGenOptions.h
cfe/trunk/lib/CodeGen/BackendUtil.cpp
cfe/trunk/lib/CodeGen/CodeGenAction.cpp
cfe/trunk/lib/Driver/Driver.cpp
cfe/trunk/lib/Driver/Tools.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/test/Driver/embed-bitcode.c

Modified: cfe/trunk/include/clang/CodeGen/BackendUtil.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/CodeGen/BackendUtil.h?rev=269202&r1=269201&r2=269202&view=diff
==
--- cfe/trunk/include/clang/CodeGen/BackendUtil.h (original)
+++ cfe/trunk/include/clang/CodeGen/BackendUtil.h Wed May 11 11:26:03 2016
@@ -16,6 +16,7 @@
 
 namespace llvm {
   class Module;
+  class MemoryBufferRef;
 }
 
 namespace clang {
@@ -37,6 +38,9 @@ namespace clang {
  const TargetOptions &TOpts, const LangOptions &LOpts,
  const llvm::DataLayout &TDesc, llvm::Module *M,
  BackendAction Action, raw_pwrite_stream *OS);
+
+  void EmbedBitcode(llvm::Module *M, const CodeGenOptions &CGOpts,
+llvm::MemoryBufferRef Buf);
 }
 
 #endif

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=269202&r1=269201&r2=269202&view=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Wed May 11 11:26:03 2016
@@ -450,11 +450,14 @@ def fno_autolink : Flag <["-"], "fno-aut
   Flags<[DriverOption, CC1Option]>,
   HelpText<"Disable generation of linker directives for automatic library 
linking">;
 
+def fembed_bitcode_EQ : Joined<["-"], "fembed-bitcode=">,
+Group, Flags<[DriverOption, CC1Option]>, MetaVarName<"">,
+HelpText<"Embed LLVM bitcode (option: off, all, bitcode, marker)">;
 def fembed_bitcode : Flag<["-"], "fembed-bitcode">, Group,
-  Flags<[CC1Option, CC1AsOption]>,
+  Alias, AliasArgs<["all"]>,
   HelpText<"Embed LLVM IR bitcode as data">;
 def fembed_bitcode_marker : Flag<["-"], "fembed-bitcode-marker">,
-  Group, Flags<[CC1Option]>,
+  Alias, AliasArgs<["marker"]>,
   HelpText<"Embed placeholder LLVM IR data as a marker">;
 def fgnu_inline_asm : Flag<["-"], "fgnu-inline-asm">, Group, 
Flags<[DriverOption]>;
 def fno_gnu_inline_asm : Flag<["-"], "fno-gnu-inline-asm">, Group,

Modified: cfe/trunk/include/clang/Frontend/CodeGenOptions.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/CodeGenOptions.def?rev=269202&r1=269201&r2=269202&view=diff
==
--- cfe/trunk/include/clang/Frontend/CodeGenOptions.def (original)
+++ cfe/trunk/include/clang/Frontend/CodeGenOptions.def Wed May 11 11:26:03 2016
@@ -66,6 +66,8 @@ CODEGENOPT(EmitOpenCLArgMetadata , 1, 0)
 CODEGENOPT(EmulatedTLS   , 1, 0) ///< Set when -femulated-tls is enabled.
 /// \brief FP_CONTRACT mode (on/off/fast).
 ENUM_CODEGENOPT(FPContractMode, FPContractModeKind, 2, FPC_On)
+/// \brief Embed Bitcode mode (off/all/bitcode/marker).
+ENUM_CODEGENOPT(EmbedBitcode, EmbedBitcodeKind, 2, Embed_Off)
 CODEGENOPT(ForbidGuardVariables , 1, 0) ///< Issue errors if C++ guard 
variables
 ///< are required.
 CODEGENOPT(FunctionSections  , 1, 0) ///< Set when -ffunction-sections is 
enabled.

Modified: cfe/trunk/include/clang/Frontend/CodeGenOptions.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/CodeGenOptions.h?rev=269202&r1=269201&r2=269202&view=diff
==
--- cfe/trunk/include/clang/Frontend/CodeGenOptions.h (original)
+++ cfe/trunk/include/clang/Frontend/CodeGenOptions.h Wed May 11 11:26:03 2016
@@ -86,6 +86,13 @@ public:
 ProfileIRInstr,// IR level PGO instrumentation in LLVM.
   };
 
+  enum EmbedBitcodeKind {
+Embed_Off,  // No embedded bitcode.
+Embed_All,  // Embed both bitcode and commandline in 

Re: [PATCH] D17392: Embed bitcode in object file (clang cc1 part)

2016-05-11 Thread Steven Wu via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL269202: Embed bitcode in object file (clang cc1 part) 
(authored by steven_wu).

Changed prior to commit:
  http://reviews.llvm.org/D17392?vs=56371&id=56930#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D17392

Files:
  cfe/trunk/include/clang/CodeGen/BackendUtil.h
  cfe/trunk/include/clang/Driver/Options.td
  cfe/trunk/include/clang/Frontend/CodeGenOptions.def
  cfe/trunk/include/clang/Frontend/CodeGenOptions.h
  cfe/trunk/lib/CodeGen/BackendUtil.cpp
  cfe/trunk/lib/CodeGen/CodeGenAction.cpp
  cfe/trunk/lib/Driver/Driver.cpp
  cfe/trunk/lib/Driver/Tools.cpp
  cfe/trunk/lib/Frontend/CompilerInvocation.cpp
  cfe/trunk/test/Driver/embed-bitcode.c
  cfe/trunk/test/Frontend/embed-bitcode.ll

Index: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
===
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp
@@ -634,6 +634,45 @@
   }
 }
   }
+	// Handle -fembed-bitcode option.
+  if (Arg *A = Args.getLastArg(OPT_fembed_bitcode_EQ)) {
+StringRef Name = A->getValue();
+unsigned Model = llvm::StringSwitch(Name)
+.Case("off", CodeGenOptions::Embed_Off)
+.Case("all", CodeGenOptions::Embed_All)
+.Case("bitcode", CodeGenOptions::Embed_Bitcode)
+.Case("marker", CodeGenOptions::Embed_Marker)
+.Default(~0U);
+if (Model == ~0U) {
+  Diags.Report(diag::err_drv_invalid_value) << A->getAsString(Args) << Name;
+  Success = false;
+} else
+  Opts.setEmbedBitcode(
+  static_cast(Model));
+  }
+  // FIXME: For backend options that are not yet recorded as function
+  // attributes in the IR, keep track of them so we can embed them in a
+  // separate data section and use them when building the bitcode.
+  if (Opts.getEmbedBitcode() == CodeGenOptions::Embed_All) {
+for (const auto &A : Args) {
+  // Do not encode output and input.
+  if (A->getOption().getID() == options::OPT_o ||
+  A->getOption().getID() == options::OPT_INPUT ||
+  A->getOption().getID() == options::OPT_x ||
+  A->getOption().getID() == options::OPT_fembed_bitcode ||
+  (A->getOption().getGroup().isValid() &&
+   A->getOption().getGroup().getID() == options::OPT_W_Group))
+continue;
+  ArgStringList ASL;
+  A->render(Args, ASL);
+  for (const auto &arg : ASL) {
+StringRef ArgStr(arg);
+Opts.CmdArgs.insert(Opts.CmdArgs.end(), ArgStr.begin(), ArgStr.end());
+// using \00 to seperate each commandline options.
+Opts.CmdArgs.push_back('\0');
+  }
+}
+  }
 
   Opts.InstrumentFunctions = Args.hasArg(OPT_finstrument_functions);
   Opts.InstrumentForProfiling = Args.hasArg(OPT_pg);
Index: cfe/trunk/lib/CodeGen/CodeGenAction.cpp
===
--- cfe/trunk/lib/CodeGen/CodeGenAction.cpp
+++ cfe/trunk/lib/CodeGen/CodeGenAction.cpp
@@ -173,6 +173,8 @@
   return;
   }
 
+  EmbedBitcode(getModule(), CodeGenOpts, llvm::MemoryBufferRef());
+
   EmitBackendOutput(Diags, CodeGenOpts, TargetOpts, LangOpts,
 C.getTargetInfo().getDataLayout(),
 getModule(), Action, AsmOutStream);
@@ -831,9 +833,13 @@
   TheModule->setTargetTriple(TargetOpts.Triple);
 }
 
+EmbedBitcode(TheModule.get(), CI.getCodeGenOpts(),
+ MainFile->getMemBufferRef());
+
 LLVMContext &Ctx = TheModule->getContext();
 Ctx.setInlineAsmDiagnosticHandler(BitcodeInlineAsmDiagHandler,
   &CI.getDiagnostics());
+
 EmitBackendOutput(CI.getDiagnostics(), CI.getCodeGenOpts(), TargetOpts,
   CI.getLangOpts(), CI.getTarget().getDataLayout(),
   TheModule.get(), BA, OS);
Index: cfe/trunk/lib/CodeGen/BackendUtil.cpp
===
--- cfe/trunk/lib/CodeGen/BackendUtil.cpp
+++ cfe/trunk/lib/CodeGen/BackendUtil.cpp
@@ -16,9 +16,11 @@
 #include "clang/Frontend/Utils.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringSwitch.h"
+#include "llvm/ADT/Triple.h"
 #include "llvm/Analysis/TargetLibraryInfo.h"
 #include "llvm/Analysis/TargetTransformInfo.h"
 #include "llvm/Bitcode/BitcodeWriterPass.h"
+#include "llvm/Bitcode/ReaderWriter.h"
 #include "llvm/CodeGen/RegAllocRegistry.h"
 #include "llvm/CodeGen/SchedulerRegistry.h"
 #include "llvm/IR/DataLayout.h"
@@ -763,3 +765,91 @@
 }
   }
 }
+
+static const char* getSectionNameForBitcode(const Triple &T) {
+  switch (T.getObjectFormat()) {
+  case Triple::MachO:
+return "__LLVM,__bitcode";
+  case Triple::COFF:
+  case Triple::ELF:
+  case Triple::UnknownObjectFormat:
+return ".llvmbc";
+  }
+}
+
+static const char* getSectionNameForCommandline(const Triple &T) {
+  switch (T.getObjectFormat(

Re: [PATCH] D19804: Make clang-format cleaner remove redundant commas in list and redundant colon in constructor initializer.

2016-05-11 Thread Eric Liu via cfe-commits
ioeric added a comment.

PING


http://reviews.llvm.org/D19804



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


Re: r269116 - Add -Wcast-calling-convention to warn when casting away calling conventions

2016-05-11 Thread Reid Kleckner via cfe-commits
On Tue, May 10, 2016 at 5:55 PM, Hans Wennborg  wrote:
>> This warning is currently off by default while we study its usefulness.
>
> Turns out this wasn't true :-) For example, it's firing here:
> https://build.chromium.org/p/chromium.fyi/builders/ClangToTWin%28dbg%29/builds/5569/steps/compile/logs/stdio
>
> Adding DefaultIgnore in r269148.

Oops, thanks. I think I was going to run it over Chromium myself, and
then I changed my mind, and forgot to change it back.

---

Based on the two warning instances in that Mesa build, I think we
should relax this warning when casting from a non-standard convention
to a default convention. We could also consider relaxing the warning
when casting to a void function, since that's what people seem to use
to mean "generic function pointer". The code in question is doing
stuff like:

void __stdcall api_func0(int a) { ... }
void __stdcall api_func1(int a, int b) { ... }
...
typedef void (*func_ty)();
func_ty api_entries[] = {
  (func_ty) &api_func0,
  (func_ty) &api_func1,
  ...
};

Our fixit suggestions are also very silly, because the functions in
question already have explicit calling convention attributes:

..\..\third_party\mesa\src\chromium_gensrc\mesa\glapi_mapi_tmp_shared.h(12817,15):
 note: consider defining 'shared_dispatch_stub_2' with the 'cdecl'
calling convention
void APIENTRY shared_dispatch_stub_2(GLuint list)
  ^
  CDECL_NON_WVMPURE
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D20170: [clang-tidy] TypeTraits - Type is not expensive to copy when it has a deleted copy constructor.

2016-05-11 Thread Alexander Kornienko via cfe-commits
alexfh added inline comments.


Comment at: clang-tidy/utils/TypeTraits.cpp:20
@@ -18,1 +19,3 @@
 
+using namespace ::clang::ast_matchers;
+

aaron.ballman wrote:
> I would prefer this be used locally instead of at namespace scope to avoid 
> potential name collisions.
Note, that file-level `using namespace ast_matchers` is rather common in Clang 
tools, so I wouldn't bother here. That said, I'm fine either way.


Repository:
  rL LLVM

http://reviews.llvm.org/D20170



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


Re: [PATCH] D19274: Compilation for Intel MCU (Part 2/3)

2016-05-11 Thread Bruno Cardoso Lopes via cfe-commits
bruno added a comment.

Hi,



Comment at: include/clang/Basic/DiagnosticDriverKinds.td:154
@@ -153,1 +153,3 @@
+def err_drv_cxx_not_supported : Error<
+  "C++ is not supported for target '%0'">;
 

Are you sure there's no equivalente for this already? I'm surprised!


Comment at: lib/Driver/Tools.cpp:300
@@ -299,1 +299,3 @@
+  const bool IsIAMCU = getToolChain().getTriple().isOSIAMCU();
+
   CheckPreprocessingOptions(D, Args);

Tidy up this declaration with others, no need for two newlines here.


Comment at: lib/Driver/Tools.cpp:575
@@ -569,1 +574,3 @@
+  else
+getToolChain().AddIAMCUIncludeArgs(Args, CmdArgs);
 

Use braces for the "else" and move the comment somewhere inside the braces


http://reviews.llvm.org/D19274



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


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

2016-05-11 Thread Alexander Kornienko via cfe-commits
alexfh requested changes to this revision.
This revision now requires changes to proceed.


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:39
@@ +38,3 @@
+
+  const FunctionDecl *FuncDecl =
+  Result.Nodes.getNodeAs("functionDecl");

s/FunctionDecl/auto/


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

s/clang:://


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:49-50
@@ +48,4 @@
+  SourceLocation CurrentLoc = Range.getEnd();
+  SourceLocation ReplaceStart;
+  SourceLocation ReplaceEnd;
+  std::string Replacement = ReplacementStr;

These two seem to represent a `SourceRange`.


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:51
@@ +50,3 @@
+  SourceLocation ReplaceEnd;
+  std::string Replacement = ReplacementStr;
+  unsigned TokenLength = 0;

s/std::string/StringRef/


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:54-90
@@ +53,39 @@
+
+  SmallVector Tokens =
+  utils::lexer::ParseTokens(Context, SM, CharSourceRange(Range, true));
+  auto TokensEnd = Tokens.rend();
+  for (auto I = Tokens.rbegin(); I != TokensEnd; ++I) {
+SourceLocation Loc = I->getLocation();
+TokenLength = I->getLength();
+
+// Looking for throw(), throw([,...]), or throw(...).
+if (I->is(tok::r_paren)) {
+  if (++I == TokensEnd)
+return;
+  bool Empty = true;
+  // Found ')', now loop till we find '('.
+  while (I->isNot(tok::l_paren)) {
+Empty = false;
+if (++I == TokensEnd)
+  return;
+  }
+  if (++I == TokensEnd)
+return;
+  if (StringRef(SM.getCharacterData(I->getLocation()), I->getLength()) ==
+  "throw") {
+if (!Empty) {
+  // We only support macro replacement for "throw()".
+  if (Replacement != "noexcept")
+break;
+  Replacement = "noexcept(false)";
+}
+ReplaceEnd = Loc;
+ReplaceStart = I->getLocation();
+break;
+  }
+} else if (++I == TokensEnd) {
+  return;
+}
+CurrentLoc = I->getLocation();
+  }
+

I'd pull this to a separate function to make the `check()` method easier to 
read.


Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:93
@@ +92,3 @@
+  if (ReplaceStart.isValid() && ReplaceEnd.isValid()) {
+std::pair BeginInfo = SM.getDecomposedLoc(ReplaceStart);
+std::pair EndInfo = SM.getDecomposedLoc(ReplaceEnd);

`Lexer::makeFileCharRange` is a convenient way to ensure a range is a 
contiguous range in the same file.


http://reviews.llvm.org/D18575



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


Re: [PATCH] D19703: [clang-tidy] Improve misc-redundant-expression and decrease false-positive

2016-05-11 Thread Etienne Bergeron via cfe-commits
etienneb added a comment.



> We should add a hasAnyOverloadedOperatorName matcher at some point.


I proposed it to sbenza@ and he told me that the reason he did HasAnyName was 
for speed issue.
I can't tell if that one has speed issue too.

But, otherwise, I agree... for code readability... we should do it.


http://reviews.llvm.org/D19703



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


Re: [PATCH] D19547: [clang-tidy] Add FixIt for swapping arguments in string-constructor-checker.

2016-05-11 Thread Alexander Kornienko via cfe-commits
alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.

LG.


http://reviews.llvm.org/D19547



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


Re: [PATCH] D19703: [clang-tidy] Improve misc-redundant-expression and decrease false-positive

2016-05-11 Thread Etienne Bergeron via cfe-commits
etienneb updated this revision to Diff 56935.
etienneb marked an inline comment as done.
etienneb added a comment.

remove useless set creation


http://reviews.llvm.org/D19703

Files:
  clang-tidy/misc/RedundantExpressionCheck.cpp
  docs/clang-tidy/checks/misc-redundant-expression.rst
  test/clang-tidy/misc-redundant-expression.cpp

Index: test/clang-tidy/misc-redundant-expression.cpp
===
--- test/clang-tidy/misc-redundant-expression.cpp
+++ test/clang-tidy/misc-redundant-expression.cpp
@@ -68,14 +68,14 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: both side of operator are equivalent
 
   if (foo(0) - 2 < foo(0) - 2) return 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: both side of operator are equivalent  
+  // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: both side of operator are equivalent
   if (foo(bar(0)) < (foo(bar((0) return 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: both side of operator are equivalent  
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: both side of operator are equivalent
 
   if (P1.x < P2.x && P1.x < P2.x) return 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: both side of operator are equivalent  
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: both side of operator are equivalent
   if (P2.a[P1.x + 2] < P2.x && P2.a[(P1.x) + (2)] < (P2.x)) return 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: both side of operator are equivalent  
+  // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: both side of operator are equivalent
 
   return 0;
 }
@@ -100,13 +100,36 @@
   return 0;
 }
 
+int TestConditional(int x, int y) {
+  int k = 0;
+  k += (y < 0) ? x : x;
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: 'true' and 'false' expression are equivalent
+  k += (y < 0) ? x + 1 : x + 1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: 'true' and 'false' expression are equivalent
+  return k;
+}
+
+struct MyStruct {
+  int x;
+} Q;
+bool operator==(const MyStruct& lhs, const MyStruct& rhs) { return lhs.x == rhs.x; }
+bool TestOperator(const MyStruct& S) {
+  if (S == Q) return false;
+  return S == S;
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: both side of overloaded operator are equivalent
+}
+
 #define LT(x, y) (void)((x) < (y))
+#define COND(x, y, z) ((x)?(y):(z))
+#define EQUALS(x, y) (x) == (y)
 
 int TestMacro(int X, int Y) {
   LT(0, 0);
   LT(1, 0);
   LT(X, X);
   LT(X+1, X + 1);
+  COND(X < Y, X, X);
+  EQUALS(Q, Q);
 }
 
 int TestFalsePositive(int* A, int X, float F) {
@@ -118,3 +141,22 @@
   if (F != F && F == F) return 1;
   return 0;
 }
+
+int TestBannedMacros() {
+#define EAGAIN 3
+#define NOT_EAGAIN 3
+  if (EAGAIN == 0 | EAGAIN == 0) return 0;
+  if (NOT_EAGAIN == 0 | NOT_EAGAIN == 0) return 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: both side of operator are equivalent
+}
+
+struct MyClass {
+static const int Value = 42;
+};
+template 
+void TemplateCheck() {
+  static_assert(T::Value == U::Value, "should be identical");
+  static_assert(T::Value == T::Value, "should be identical");
+  // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: both side of overloaded operator are equivalent
+}
+void TestTemplate() { TemplateCheck(); }
Index: docs/clang-tidy/checks/misc-redundant-expression.rst
===
--- docs/clang-tidy/checks/misc-redundant-expression.rst
+++ docs/clang-tidy/checks/misc-redundant-expression.rst
@@ -12,6 +12,7 @@
   * always be a constant (zero or one)
 
 Example:
+
 .. code:: c++
 
   ((x+1) | (x+1)) // (x+1) is redundant
Index: clang-tidy/misc/RedundantExpressionCheck.cpp
===
--- clang-tidy/misc/RedundantExpressionCheck.cpp
+++ clang-tidy/misc/RedundantExpressionCheck.cpp
@@ -9,16 +9,29 @@
 
 #include "RedundantExpressionCheck.h"
 #include "../utils/Matchers.h"
+#include "../utils/OptionsUtils.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
 
 using namespace clang::ast_matchers;
 
 namespace clang {
 namespace tidy {
 namespace misc {
 
-static bool AreIdenticalExpr(const Expr *Left, const Expr *Right) {
+static const char KnownBannedMacroNames[] =
+"EAGAIN;EWOULDBLOCK;SIGCLD;SIGCHLD;";
+
+static bool AreEquivalentNameSpecifier(const NestedNameSpecifier *Left,
+   const NestedNameSpecifier *Right) {
+  llvm::FoldingSetNodeID LeftID, RightID;
+  Left->Profile(LeftID);
+  Right->Profile(RightID);
+  return LeftID == RightID;
+}
+
+static bool AreEquivalentExpr(const Expr *Left, const Expr *Right) {
   if (!Left || !Right)
 return !Left && !Right;
 
@@ -33,8 +46,8 @@
   Expr::const_child_iterator LeftIter = Left->child_begin();
   Expr::const_child_iterator RightIter = Right->child_begin();
   while (LeftIter != Left->child_end() && RightIter != Right->child_end()) {
-if (!AreIdenticalExpr(dyn_cast(*LeftIter),
-  

Patch submission for bug 27400

2016-05-11 Thread Mads Ravn via cfe-commits
Hi,

I would like to submit a patch for
https://llvm.org/bugs/show_bug.cgi?id=27400 .

Beside attaching the patch, is there anything I should be aware of? I have
not submitted a patch before.

You can find the patch attached to this mail.

Kind regards,
Mads Ravn
Index: clang-tidy/misc/MacroParenthesesCheck.cpp
===
--- clang-tidy/misc/MacroParenthesesCheck.cpp	(revision 268956)
+++ clang-tidy/misc/MacroParenthesesCheck.cpp	(working copy)
@@ -184,6 +184,9 @@
 Next.isOneOf(tok::comma, tok::greater))
   continue;
 
+if(Prev.is(tok::kw_namespace))
+continue;
+
 Check->diag(Tok.getLocation(), "macro argument should be enclosed in "
"parentheses")
 << FixItHint::CreateInsertion(Tok.getLocation(), "(")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D19703: [clang-tidy] Improve misc-redundant-expression and decrease false-positive

2016-05-11 Thread Etienne Bergeron via cfe-commits
etienneb updated this revision to Diff 56937.
etienneb added a comment.

fix naming nits


http://reviews.llvm.org/D19703

Files:
  clang-tidy/misc/RedundantExpressionCheck.cpp
  docs/clang-tidy/checks/misc-redundant-expression.rst
  test/clang-tidy/misc-redundant-expression.cpp

Index: test/clang-tidy/misc-redundant-expression.cpp
===
--- test/clang-tidy/misc-redundant-expression.cpp
+++ test/clang-tidy/misc-redundant-expression.cpp
@@ -68,14 +68,14 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: both side of operator are equivalent
 
   if (foo(0) - 2 < foo(0) - 2) return 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: both side of operator are equivalent  
+  // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: both side of operator are equivalent
   if (foo(bar(0)) < (foo(bar((0) return 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: both side of operator are equivalent  
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: both side of operator are equivalent
 
   if (P1.x < P2.x && P1.x < P2.x) return 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: both side of operator are equivalent  
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: both side of operator are equivalent
   if (P2.a[P1.x + 2] < P2.x && P2.a[(P1.x) + (2)] < (P2.x)) return 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: both side of operator are equivalent  
+  // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: both side of operator are equivalent
 
   return 0;
 }
@@ -100,13 +100,36 @@
   return 0;
 }
 
+int TestConditional(int x, int y) {
+  int k = 0;
+  k += (y < 0) ? x : x;
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: 'true' and 'false' expression are equivalent
+  k += (y < 0) ? x + 1 : x + 1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: 'true' and 'false' expression are equivalent
+  return k;
+}
+
+struct MyStruct {
+  int x;
+} Q;
+bool operator==(const MyStruct& lhs, const MyStruct& rhs) { return lhs.x == rhs.x; }
+bool TestOperator(const MyStruct& S) {
+  if (S == Q) return false;
+  return S == S;
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: both side of overloaded operator are equivalent
+}
+
 #define LT(x, y) (void)((x) < (y))
+#define COND(x, y, z) ((x)?(y):(z))
+#define EQUALS(x, y) (x) == (y)
 
 int TestMacro(int X, int Y) {
   LT(0, 0);
   LT(1, 0);
   LT(X, X);
   LT(X+1, X + 1);
+  COND(X < Y, X, X);
+  EQUALS(Q, Q);
 }
 
 int TestFalsePositive(int* A, int X, float F) {
@@ -118,3 +141,22 @@
   if (F != F && F == F) return 1;
   return 0;
 }
+
+int TestBannedMacros() {
+#define EAGAIN 3
+#define NOT_EAGAIN 3
+  if (EAGAIN == 0 | EAGAIN == 0) return 0;
+  if (NOT_EAGAIN == 0 | NOT_EAGAIN == 0) return 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: both side of operator are equivalent
+}
+
+struct MyClass {
+static const int Value = 42;
+};
+template 
+void TemplateCheck() {
+  static_assert(T::Value == U::Value, "should be identical");
+  static_assert(T::Value == T::Value, "should be identical");
+  // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: both side of overloaded operator are equivalent
+}
+void TestTemplate() { TemplateCheck(); }
Index: docs/clang-tidy/checks/misc-redundant-expression.rst
===
--- docs/clang-tidy/checks/misc-redundant-expression.rst
+++ docs/clang-tidy/checks/misc-redundant-expression.rst
@@ -12,6 +12,7 @@
   * always be a constant (zero or one)
 
 Example:
+
 .. code:: c++
 
   ((x+1) | (x+1)) // (x+1) is redundant
Index: clang-tidy/misc/RedundantExpressionCheck.cpp
===
--- clang-tidy/misc/RedundantExpressionCheck.cpp
+++ clang-tidy/misc/RedundantExpressionCheck.cpp
@@ -9,16 +9,29 @@
 
 #include "RedundantExpressionCheck.h"
 #include "../utils/Matchers.h"
+#include "../utils/OptionsUtils.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
 
 using namespace clang::ast_matchers;
 
 namespace clang {
 namespace tidy {
 namespace misc {
 
-static bool AreIdenticalExpr(const Expr *Left, const Expr *Right) {
+static const char KnownBannedMacroNames[] =
+"EAGAIN;EWOULDBLOCK;SIGCLD;SIGCHLD;";
+
+static bool areEquivalentNameSpecifier(const NestedNameSpecifier *Left,
+   const NestedNameSpecifier *Right) {
+  llvm::FoldingSetNodeID LeftID, RightID;
+  Left->Profile(LeftID);
+  Right->Profile(RightID);
+  return LeftID == RightID;
+}
+
+static bool areEquivalentExpr(const Expr *Left, const Expr *Right) {
   if (!Left || !Right)
 return !Left && !Right;
 
@@ -33,8 +46,8 @@
   Expr::const_child_iterator LeftIter = Left->child_begin();
   Expr::const_child_iterator RightIter = Right->child_begin();
   while (LeftIter != Left->child_end() && RightIter != Right->child_end()) {
-if (!AreIdenticalExpr(dyn_cast(*LeftIter),
-  dyn_cast(*RightIter)))
+if (!areEquival

Re: [PATCH] D19703: [clang-tidy] Improve misc-redundant-expression and decrease false-positive

2016-05-11 Thread Etienne Bergeron via cfe-commits
etienneb added a comment.

+ Removed copy of a vector to a set
+ Fixed some minor coding-style issues



Comment at: clang-tidy/misc/RedundantExpressionCheck.cpp:140
@@ +139,3 @@
+  const LangOptions &LO = Finder->getASTContext().getLangOpts();
+  std::set Names(NameRefs.begin(), NameRefs.end());
+  SourceLocation Loc = Node.getExprLoc();

alexfh wrote:
> Is there a way to not create a set on each call to the matcher? There also 
> might be a more suitable container in llvm/ADT.
Some ideas:
1) We may assume the vector is small and do the lookup.
2) We may assume a sorted vector and do a binary search.
3) The matcher could receive a set instead of a vector and lifting out the copy.

Implemented 3)


http://reviews.llvm.org/D19703



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


Re: [PATCH] D16962: clang-tidy: avoid std::bind

2016-05-11 Thread Alexander Kornienko via cfe-commits
alexfh requested changes to this revision.
This revision now requires changes to proceed.


Comment at: clang-tidy/modernize/AvoidBindCheck.cpp:27
@@ +26,3 @@
+  StringRef Tokens;
+  BindArgumentKind Kind = BK_Other;
+  size_t PlaceHolderIndex = 0;

I wonder whether VS2013 supports inline member initializers?


Comment at: clang-tidy/modernize/AvoidBindCheck.cpp:77
@@ +76,3 @@
+  for (size_t I = 1; I <= PlaceholderCount; ++I) {
+Stream << Delimiter << "auto &&"
+   << " arg" << I;

clang-format, please. Also, please join the two string literals here.


Comment at: clang-tidy/modernize/AvoidBindCheck.cpp:116
@@ +115,3 @@
+void AvoidBindCheck::check(const MatchFinder::MatchResult &Result) {
+  if (!getLangOpts().CPlusPlus14) // Need C++14 for generic lambdas.
+return;

This check should be done in `registerMatchers` to avoid unnecessary work.


Comment at: clang-tidy/modernize/AvoidBindCheck.cpp:161
@@ +160,3 @@
+  MatchedDecl->getLocStart(),
+  Lexer::getLocForEndOfToken(MatchedDecl->getLocEnd(), 0,
+ *Result.SourceManager,

I don't think you need `getLocForEndOfToken`, since 
`FixItHint::CreateReplacement(SourceRange)` treats the range as a token range.


Comment at: docs/clang-tidy/checks/modernize-avoid-bind.rst:26
@@ +25,3 @@
+  
+.. code:: C++
+  void f() {

`..code:: C++` directive should be followed by an empty line. Please check the 
documentation builds without warnings.


Comment at: docs/clang-tidy/checks/modernize-avoid-bind.rst:32
@@ +31,3 @@
+
+We created this check because ``std::bind`` can be hard to read and can result
+in larger object files and binaries due to type information that will not be

s/We created this check because //


Comment at: docs/clang-tidy/checks/readability-avoid-std-bind.rst:13
@@ +12,3 @@
+
+.. code:: C++
+  int add(int x, int y) { return x + y; }

alexfh wrote:
> `.. code::` should be followed by an empty line.
> 
> Please also verify the documentation can be built without errors. On Ubuntu 
> this boils down to:
> 
> 1. Install sphinx:
> 
> $ sudo apt-get install sphinx-common
> 
> 2. Enable `LLVM_BUILD_DOCS` and maybe some other options in cmake.
> 
> 3. `ninja docs-clang-tools-html` (or something similar, if you use make).
This comment is not addressed.


Comment at: test/clang-tidy/modernize-avoid-bind.cpp:25
@@ +24,3 @@
+  auto clj = std::bind(add, x, y);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind 
[modernize-avoid-bind]
+  // CHECK-FIXES: auto clj = [=] { return add(x, y); };

Remove `[modernize-avoid-bind]` from all CHECK-MESSAGES lines except for the 
first one.


http://reviews.llvm.org/D16962



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


  1   2   >