date:20191013

[PATCH] D68931: [clang] [clang-offload-bundler] Fix finding installed llvm-objcopy

2019-10-13 Thread Michał Górny via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf89e758d886a: [clang] [clang-offload-bundler] Fix finding 
installed llvm-objcopy (authored by mgorny).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68931/new/

https://reviews.llvm.org/D68931

Files:
  clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp


Index: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
===
--- clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
+++ clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
@@ -468,6 +468,8 @@
 // Find llvm-objcopy in order to create the bundle binary.
 ErrorOr Objcopy = sys::findProgramByName(
 "llvm-objcopy", sys::path::parent_path(BundlerExecutable));
+if (!Objcopy)
+  Objcopy = sys::findProgramByName("llvm-objcopy");
 if (!Objcopy) {
   errs() << "error: unable to find 'llvm-objcopy' in path.\n";
   return true;


Index: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
===
--- clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
+++ clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
@@ -468,6 +468,8 @@
 // Find llvm-objcopy in order to create the bundle binary.
 ErrorOr Objcopy = sys::findProgramByName(
 "llvm-objcopy", sys::path::parent_path(BundlerExecutable));
+if (!Objcopy)
+  Objcopy = sys::findProgramByName("llvm-objcopy");
 if (!Objcopy) {
   errs() << "error: unable to find 'llvm-objcopy' in path.\n";
   return true;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r374754 - [clang] [clang-offload-bundler] Fix finding installed llvm-objcopy

2019-10-13 Thread Michal Gorny via cfe-commits

Author: mgorny
Date: Sun Oct 13 22:33:23 2019
New Revision: 374754

URL: http://llvm.org/viewvc/llvm-project?rev=374754=rev
Log:
[clang] [clang-offload-bundler] Fix finding installed llvm-objcopy

Allow finding installed llvm-objcopy in PATH if it's not present
in the directory containing clang-offload-bundler.  This is the case
if clang is being built stand-alone, and llvm-objcopy is already
installed while the c-o-b tool is still present in build directory.

This is consistent with how e.g. llvm-symbolizer is found in LLVM.
However, most of similar searches in LLVM and Clang are performed
without special-casing the program directory.

Fixes r369955.

Differential Revision: https://reviews.llvm.org/D68931

Modified:
cfe/trunk/tools/clang-offload-bundler/ClangOffloadBundler.cpp

Modified: cfe/trunk/tools/clang-offload-bundler/ClangOffloadBundler.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/clang-offload-bundler/ClangOffloadBundler.cpp?rev=374754=374753=374754=diff
==
--- cfe/trunk/tools/clang-offload-bundler/ClangOffloadBundler.cpp (original)
+++ cfe/trunk/tools/clang-offload-bundler/ClangOffloadBundler.cpp Sun Oct 13 
22:33:23 2019
@@ -468,6 +468,8 @@ public:
 // Find llvm-objcopy in order to create the bundle binary.
 ErrorOr Objcopy = sys::findProgramByName(
 "llvm-objcopy", sys::path::parent_path(BundlerExecutable));
+if (!Objcopy)
+  Objcopy = sys::findProgramByName("llvm-objcopy");
 if (!Objcopy) {
   errs() << "error: unable to find 'llvm-objcopy' in path.\n";
   return true;


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

[clang-tools-extra] r374753 - clangd tests: use extended regex with sed

2019-10-13 Thread Nico Weber via cfe-commits

Author: nico
Date: Sun Oct 13 20:44:47 2019
New Revision: 374753

URL: http://llvm.org/viewvc/llvm-project?rev=374753=rev
Log:
clangd tests: use extended regex with sed

The escaped parens seem to confuse the combination of lit, cygwin
quoting, and cygwin's sed. unxutils sed in cmd.exe is fine with both
forms, so use the extended regex form that doesn't need an escaped
paren.

Modified:
clang-tools-extra/trunk/clangd/test/compile-commands-path-in-initialize.test
clang-tools-extra/trunk/clangd/test/system-include-extractor.test
clang-tools-extra/trunk/clangd/test/target_info.test

Modified: 
clang-tools-extra/trunk/clangd/test/compile-commands-path-in-initialize.test
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/test/compile-commands-path-in-initialize.test?rev=374753=374752=374753=diff
==
--- 
clang-tools-extra/trunk/clangd/test/compile-commands-path-in-initialize.test 
(original)
+++ 
clang-tools-extra/trunk/clangd/test/compile-commands-path-in-initialize.test 
Sun Oct 13 20:44:47 2019
@@ -10,7 +10,7 @@
 
 # On Windows, we need the URI in didOpen to look like "uri":"file:///C:/..."
 # (with the extra slash in the front), so we add it here.
-# RUN: sed -e "s|file://\([A-Z]\):/|file:///\1:/|g" %t.test.1 > %t.test
+# RUN: sed -E -e "s|file://([A-Z]):/|file:///\1:/|g" %t.test.1 > %t.test
 
 # RUN: clangd -lit-test < %t.test | FileCheck -strict-whitespace %t.test
 

Modified: clang-tools-extra/trunk/clangd/test/system-include-extractor.test
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/test/system-include-extractor.test?rev=374753=374752=374753=diff
==
--- clang-tools-extra/trunk/clangd/test/system-include-extractor.test (original)
+++ clang-tools-extra/trunk/clangd/test/system-include-extractor.test Sun Oct 
13 20:44:47 2019
@@ -27,7 +27,7 @@
 # RUN: sed -e "s|INPUT_DIR|%/t.dir|g" %s > %t.test.1
 # On Windows, we need the URI in didOpen to look like "uri":"file:///C:/..."
 # (with the extra slash in the front), so we add it here.
-# RUN: sed -e "s|file://\([A-Z]\):/|file:///\1:/|g" %t.test.1 > %t.test
+# RUN: sed -E -e "s|file://([A-Z]):/|file:///\1:/|g" %t.test.1 > %t.test
 
 # Bless the mock driver we've just created so that clangd can execute it.
 # RUN: clangd -lit-test -query-driver="**.test,**.sh" < %t.test | FileCheck 
-strict-whitespace %t.test

Modified: clang-tools-extra/trunk/clangd/test/target_info.test
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/test/target_info.test?rev=374753=374752=374753=diff
==
--- clang-tools-extra/trunk/clangd/test/target_info.test (original)
+++ clang-tools-extra/trunk/clangd/test/target_info.test Sun Oct 13 20:44:47 
2019
@@ -9,7 +9,7 @@
 # RUN: sed -e "s|INPUT_DIR|%/t.dir|g" %s > %t.test.1
 # On Windows, we need the URI in didOpen to look like "uri":"file:///C:/..."
 # (with the extra slash in the front), so we add it here.
-# RUN: sed -e "s|file://\([A-Z]\):/|file:///\1:/|g" %t.test.1 > %t.test
+# RUN: sed -E -e "s|file://([A-Z]):/|file:///\1:/|g" %t.test.1 > %t.test
 
 # RUN: clangd -lit-test < %t.test 2>&1 | FileCheck -strict-whitespace %t.test
 {"jsonrpc":"2.0","id":0,"method":"initialize","params":{}}


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

[PATCH] D68931: [clang] [clang-offload-bundler] Fix finding installed llvm-objcopy

2019-10-13 Thread Sergey Dmitriev via Phabricator via cfe-commits

sdmitriev accepted this revision.
sdmitriev added a comment.

Looks good.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68931/new/

https://reviews.llvm.org/D68931



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

[clang-tools-extra] r374752 - convert another test to unix line endings

2019-10-13 Thread Nico Weber via cfe-commits

Author: nico
Date: Sun Oct 13 19:21:12 2019
New Revision: 374752

URL: http://llvm.org/viewvc/llvm-project?rev=374752=rev
Log:
convert another test to unix line endings

Modified:
clang-tools-extra/trunk/clangd/test/symbol-info.test

Modified: clang-tools-extra/trunk/clangd/test/symbol-info.test
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/test/symbol-info.test?rev=374752=374751=374752=diff
==
--- clang-tools-extra/trunk/clangd/test/symbol-info.test (original)
+++ clang-tools-extra/trunk/clangd/test/symbol-info.test Sun Oct 13 19:21:12 
2019
@@ -1,14 +1,14 @@
-# RUN: clangd -lit-test < %s | FileCheck %s
-{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}

-{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///simple.cpp","languageId":"cpp","version":1,"text":"void
 foo(); int main() { foo(); }\n"}}}

-{"jsonrpc":"2.0","id":1,"method":"textDocument/symbolInfo","params":{"textDocument":{"uri":"test:///simple.cpp"},"position":{"line":0,"character":27}}}
-#  CHECK:"containerName": null,
-# CHECK-NEXT:"id": "CA2EBE44A1D76D2A",
-# CHECK-NEXT:"name": "foo",
-# CHECK-NEXT:"usr": "c:@F@foo#"

-{"jsonrpc":"2.0","id":3,"method":"shutdown"}

-{"jsonrpc":"2.0","method":"exit"}
+# RUN: clangd -lit-test < %s | FileCheck %s
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
+---
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///simple.cpp","languageId":"cpp","version":1,"text":"void
 foo(); int main() { foo(); }\n"}}}
+---
+{"jsonrpc":"2.0","id":1,"method":"textDocument/symbolInfo","params":{"textDocument":{"uri":"test:///simple.cpp"},"position":{"line":0,"character":27}}}
+#  CHECK:"containerName": null,
+# CHECK-NEXT:"id": "CA2EBE44A1D76D2A",
+# CHECK-NEXT:"name": "foo",
+# CHECK-NEXT:"usr": "c:@F@foo#"
+---
+{"jsonrpc":"2.0","id":3,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}


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

[clang-tools-extra] r374751 - convert a test to unix line endings

2019-10-13 Thread Nico Weber via cfe-commits

Author: nico
Date: Sun Oct 13 19:14:18 2019
New Revision: 374751

URL: http://llvm.org/viewvc/llvm-project?rev=374751=rev
Log:
convert a test to unix line endings

Modified:
clang-tools-extra/trunk/clangd/test/symbols.test

Modified: clang-tools-extra/trunk/clangd/test/symbols.test
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/test/symbols.test?rev=374751=374750=374751=diff
==
--- clang-tools-extra/trunk/clangd/test/symbols.test (original)
+++ clang-tools-extra/trunk/clangd/test/symbols.test Sun Oct 13 19:14:18 2019
@@ -1,84 +1,84 @@
-# RUN: clangd --index-file=%S/Inputs/symbols.test.yaml -lit-test < %s | 
FileCheck %s
-{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{"workspace":{"symbol":{"symbolKind":{"valueSet":
 
[1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26],"trace":"off"}}

-{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,"text":"void
 foo(); int main() { foo(); }\n"}}}

-{"jsonrpc":"2.0","id":1,"method":"workspace/symbol","params":{"query":"vector"}}
-#  CHECK:  "id": 1,
-# CHECK-NEXT:  "jsonrpc": "2.0",
-# CHECK-NEXT:"result": [
-# CHECK-NEXT:  {
-# CHECK-NEXT:"containerName": "std",
-# CHECK-NEXT:"kind": 5,
-# CHECK-NEXT:"location": {
-# CHECK-NEXT:  "range": {
-# CHECK-NEXT:"end": {
-# CHECK-NEXT:  "character": {{.*}},
-# CHECK-NEXT:  "line": {{.*}}
-# CHECK-NEXT:},
-# CHECK-NEXT:"start": {
-# CHECK-NEXT:  "character": {{.*}},
-# CHECK-NEXT:  "line": {{.*}}
-# CHECK-NEXT:}
-# CHECK-NEXT:  },
-# CHECK-NEXT:  "uri": "file://{{.*}}/vector.h"
-# CHECK-NEXT:},
-# CHECK-NEXT:"name": "vector"
-# CHECK-NEXT:  }
-# CHECK-NEXT:]
-# CHECK-NEXT:}

-{"jsonrpc":"2.0","id":2,"method":"textDocument/documentSymbol","params":{"textDocument":{"uri":"test:///main.cpp"}}}
-#  CHECK:  "id": 2,
-# CHECK-NEXT:  "jsonrpc": "2.0",
-# CHECK-NEXT:"result": [
-# CHECK-NEXT:  {
-# CHECK-NEXT:"containerName": "",
-# CHECK-NEXT:"kind": 12,
-# CHECK-NEXT:"location": {
-# CHECK-NEXT:  "range": {
-# CHECK-NEXT:"end": {
-# CHECK-NEXT:  "character": {{.*}},
-# CHECK-NEXT:  "line": {{.*}}
-# CHECK-NEXT:},
-# CHECK-NEXT:"start": {
-# CHECK-NEXT:  "character": {{.*}},
-# CHECK-NEXT:  "line": {{.*}}
-# CHECK-NEXT:}
-# CHECK-NEXT:  },
-# CHECK-NEXT:  "uri": "file://{{.*}}/main.cpp"
-# CHECK-NEXT:},
-# CHECK-NEXT:"name": "foo"
-# CHECK-NEXT:  }
-# CHECK-NEXT:  {
-# CHECK-NEXT:"containerName": "",
-# CHECK-NEXT:"kind": 12,
-# CHECK-NEXT:"location": {
-# CHECK-NEXT:  "range": {
-# CHECK-NEXT:"end": {
-# CHECK-NEXT:  "character": {{.*}},
-# CHECK-NEXT:  "line": {{.*}}
-# CHECK-NEXT:},
-# CHECK-NEXT:"start": {
-# CHECK-NEXT:  "character": {{.*}},
-# CHECK-NEXT:  "line": {{.*}}
-# CHECK-NEXT:}
-# CHECK-NEXT:  },
-# CHECK-NEXT:  "uri": "file://{{.*}}/main.cpp"
-# CHECK-NEXT:},
-# CHECK-NEXT:"name": "main"
-# CHECK-NEXT:  }
-# CHECK-NEXT:]
-# CHECK-NEXT:}

-{"jsonrpc":"2.0","id":3,"method":"textDocument/documentSymbol","params":{"textDocument":{"uri":"test:///foo.cpp"}}}
-#  CHECK:  "error": {
-# CHECK-NEXT:"code": -32602,
-# CHECK-NEXT:"message": "trying to get AST for non-added document"
-# CHECK-NEXT:  },
-# CHECK-NEXT:  "id": 3,
-# CHECK-NEXT:  "jsonrpc": "2.0"

-{"jsonrpc":"2.0","id":3,"method":"shutdown"}

-{"jsonrpc":"2.0","method":"exit"}
+# RUN: clangd --index-file=%S/Inputs/symbols.test.yaml -lit-test < %s | 
FileCheck %s
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{"workspace":{"symbol":{"symbolKind":{"valueSet":
 
[1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26],"trace":"off"}}
+---
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,"text":"void
 foo(); int main() { foo(); }\n"}}}
+---
+{"jsonrpc":"2.0","id":1,"method":"workspace/symbol","params":{"query":"vector"}}
+#  CHECK:  "id": 1,
+# CHECK-NEXT:  "jsonrpc": "2.0",
+# CHECK-NEXT:"result": [
+# CHECK-NEXT:  {
+# CHECK-NEXT:"containerName": "std",
+# CHECK-NEXT:"kind": 5,
+# CHECK-NEXT:"location": {
+# CHECK-NEXT:  "range": {
+# CHECK-NEXT:"end": {
+# CHECK-NEXT:  "character": {{.*}},
+# CHECK-NEXT:  "line":

[PATCH] D68117: [DWARF-5] Support for C++11 defaulted, deleted member functions.

2019-10-13 Thread David Blaikie via Phabricator via cfe-commits

dblaikie added a comment.

In D68117#1707578 , @SouraVX wrote:

> In D68117#1702595 , @probinson wrote:
>
> > We really do want to pack the four mutually exclusive cases into two bits.  
> > I have tried to give more explicit comments inline to explain how you would 
> > do this.  It really should work fine, recognizing that the "not defaulted" 
> > case is not explicitly represented in the textual IR because it uses a zero 
> > value in the defaulted/deleted subfield of SPFlags.
>
>
> Thanks Paul, for suggesting this. Your approach works fine. But as I was 
> working on some lvm-dwarfdump test cases. We seems to miss one corner case --
>  Consider this test case;
>  class foo{
>
>   foo() = default;
>   ~foo() = default;
>void not_special() {}
>
> };
>  void not_a_member_of_foo(){}
>
> Now I'm getting DW_AT_defaulted getting emitted with value DW_DEFAULTED_no, 
> for functions "not_special" and "not_a_member_of_foo". This behavior is 
> undesirable since, DW_AT_defaulted attributes is only valid for C++ special 
> member functions{Constructors/Destructors, ...}.
>
> Please correct me if I'm wrong -- Now This attributes to- implicitly defined 
> "0" NotDefaulted bit.  which is getting checked{that's fine as long as we 
> have a dedicated bits for distinguishing} and true for every subprogram or 
> function in a CU.
>  void DwarfUnit::applySubprogramAttributes( ...
>  ...
>  else if (SP->isNotDefaulted())
>
>   addUInt(SPDie, dwarf::DW_AT_defaulted, dwarf::DW_FORM_data1,
>   dwarf::DW_DEFAULTED_no);
>
> ...


Perhaps we should only emit DEFAULTED_yes, and assume anything that's not 
DEFAULTED_yes, is... not defaulted?

Also: What features is anyone planning to build with this information? I'm sort 
of inclined not to implement features without some use-case in mind/planned.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68117/new/

https://reviews.llvm.org/D68117



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

[clang-tools-extra] r374750 - fix typo in 374747

2019-10-13 Thread Nico Weber via cfe-commits

Author: nico
Date: Sun Oct 13 18:44:29 2019
New Revision: 374750

URL: http://llvm.org/viewvc/llvm-project?rev=374750=rev
Log:
fix typo in 374747

Modified:
clang-tools-extra/trunk/clangd/test/symbols.test

Modified: clang-tools-extra/trunk/clangd/test/symbols.test
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/test/symbols.test?rev=374750=374749=374750=diff
==
--- clang-tools-extra/trunk/clangd/test/symbols.test (original)
+++ clang-tools-extra/trunk/clangd/test/symbols.test Sun Oct 13 18:44:29 2019
@@ -21,7 +21,7 @@
 # CHECK-NEXT:  "line": {{.*}}
 # CHECK-NEXT:}
 # CHECK-NEXT:  },
-# CHECK-NEXT:  "uri": "file:/{{.*}}//vector.h"
+# CHECK-NEXT:  "uri": "file://{{.*}}/vector.h"
 # CHECK-NEXT:},
 # CHECK-NEXT:"name": "vector"
 # CHECK-NEXT:  }


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

[clang-tools-extra] r374749 - Prefer 'env not' over 'not env' in tests.

2019-10-13 Thread Nico Weber via cfe-commits

Author: nico
Date: Sun Oct 13 18:41:56 2019
New Revision: 374749

URL: http://llvm.org/viewvc/llvm-project?rev=374749=rev
Log:
Prefer 'env not' over 'not env' in tests.

That way, lit's builtin 'env' command can be used for the 'env' bit.

Also it's clearer that way that the 'not' shouldn't cover 'env'
failures.

Modified:
clang-tools-extra/trunk/clangd/test/log.test

Modified: clang-tools-extra/trunk/clangd/test/log.test
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/test/log.test?rev=374749=374748=374749=diff
==
--- clang-tools-extra/trunk/clangd/test/log.test (original)
+++ clang-tools-extra/trunk/clangd/test/log.test Sun Oct 13 18:41:56 2019
@@ -1,4 +1,4 @@
-# RUN: not env CLANGD_FLAGS=-index-file=no-such-index clangd -lit-test 
&1 >/dev/null | FileCheck %s
+# RUN: env CLANGD_FLAGS=-index-file=no-such-index not clangd -lit-test 
&1 >/dev/null | FileCheck %s
 CHECK: I[{{.*}}] clangd version {{.*}}
 CHECK: Working directory: {{.*}}
 CHECK: argv[0]: clangd


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

r374749 - Prefer 'env not' over 'not env' in tests.

2019-10-13 Thread Nico Weber via cfe-commits

Author: nico
Date: Sun Oct 13 18:41:56 2019
New Revision: 374749

URL: http://llvm.org/viewvc/llvm-project?rev=374749=rev
Log:
Prefer 'env not' over 'not env' in tests.

That way, lit's builtin 'env' command can be used for the 'env' bit.

Also it's clearer that way that the 'not' shouldn't cover 'env'
failures.

Modified:
cfe/trunk/test/Driver/crash-report-crashfile.m
cfe/trunk/test/Driver/crash-report-null.test
cfe/trunk/test/Modules/crash-vfs-headermaps.m
cfe/trunk/test/Modules/crash-vfs-include-pch.m
cfe/trunk/test/Modules/crash-vfs-ivfsoverlay.m
cfe/trunk/test/Modules/crash-vfs-path-emptydir-entries.m
cfe/trunk/test/Modules/crash-vfs-path-symlink-component.m
cfe/trunk/test/Modules/crash-vfs-path-symlink-topheader.m
cfe/trunk/test/Modules/crash-vfs-path-traversal.m
cfe/trunk/test/Modules/crash-vfs-relative-incdir.m
cfe/trunk/test/Modules/crash-vfs-relative-overlay.m
cfe/trunk/test/Modules/crash-vfs-run-reproducer.m
cfe/trunk/test/Modules/crash-vfs-umbrella-frameworks.m
cfe/trunk/test/Tooling/auto-detect-from-source-parent-of-cwd.cpp
cfe/trunk/test/Tooling/clang-check-pwd.cpp

Modified: cfe/trunk/test/Driver/crash-report-crashfile.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/crash-report-crashfile.m?rev=374749=374748=374749=diff
==
--- cfe/trunk/test/Driver/crash-report-crashfile.m (original)
+++ cfe/trunk/test/Driver/crash-report-crashfile.m Sun Oct 13 18:41:56 2019
@@ -2,14 +2,14 @@
 // RUN: rm -rf %t
 // RUN: mkdir -p %t/i %t/m %t
 
-// RUN: not env FORCE_CLANG_DIAGNOSTICS_CRASH= TMPDIR=%t TEMP=%t TMP=%t \
-// RUN: %clang -fsyntax-only %s \
+// RUN: env FORCE_CLANG_DIAGNOSTICS_CRASH= TMPDIR=%t TEMP=%t TMP=%t \
+// RUN: not %clang -fsyntax-only %s \
 // RUN:   -I %S/Inputs/module -isysroot %/t/i/ \
 // RUN:   -fmodules -fmodules-cache-path=%t/m/ -DFOO=BAR 2>&1 | \
 // RUN:   FileCheck -check-prefix=CRASH_ENV %s
 
-// RUN: not env TMPDIR=%t TEMP=%t TMP=%t \
-// RUN: %clang -gen-reproducer -fsyntax-only %s \
+// RUN: env TMPDIR=%t TEMP=%t TMP=%t \
+// RUN: not %clang -gen-reproducer -fsyntax-only %s \
 // RUN:   -I %S/Inputs/module -isysroot %/t/i/ \
 // RUN:   -fmodules -fmodules-cache-path=%t/m/ -DFOO=BAR 2>&1 | \
 // RUN:   FileCheck -check-prefix=CRASH_FLAG %s

Modified: cfe/trunk/test/Driver/crash-report-null.test
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/crash-report-null.test?rev=374749=374748=374749=diff
==
--- cfe/trunk/test/Driver/crash-report-null.test (original)
+++ cfe/trunk/test/Driver/crash-report-null.test Sun Oct 13 18:41:56 2019
@@ -1,4 +1,4 @@
-// RUN: not env FORCE_CLANG_DIAGNOSTICS_CRASH=1 %clang -fsyntax-only -x c 
/dev/null -lstdc++ 2>&1 | FileCheck %s
+// RUN: env FORCE_CLANG_DIAGNOSTICS_CRASH=1 not %clang -fsyntax-only -x c 
/dev/null -lstdc++ 2>&1 | FileCheck %s
 
 // FIXME: Investigating. "fatal error: file 'nul' modified since it was first 
processed"
 // XFAIL: windows-gnu

Modified: cfe/trunk/test/Modules/crash-vfs-headermaps.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/crash-vfs-headermaps.m?rev=374749=374748=374749=diff
==
--- cfe/trunk/test/Modules/crash-vfs-headermaps.m (original)
+++ cfe/trunk/test/Modules/crash-vfs-headermaps.m Sun Oct 13 18:41:56 2019
@@ -5,8 +5,8 @@
 // RUN: echo '// Foo.h' > %t/i/Foo.framework/Headers/Foo.h
 // RUN: %hmaptool write %S/../Preprocessor/Inputs/headermap-rel/foo.hmap.json 
%t/i/foo.hmap
 
-// RUN: not env FORCE_CLANG_DIAGNOSTICS_CRASH= TMPDIR=%t TEMP=%t TMP=%t \
-// RUN: %clang -fsyntax-only -fmodules -fmodules-cache-path=%t/m %s \
+// RUN: env FORCE_CLANG_DIAGNOSTICS_CRASH= TMPDIR=%t TEMP=%t TMP=%t \
+// RUN: not %clang -fsyntax-only -fmodules -fmodules-cache-path=%t/m %s \
 // RUN: -I %t/i/foo.hmap -F %t/i 2>&1 | FileCheck %s
 
 // RUN: FileCheck --check-prefix=CHECKSH %s -input-file %t/crash-vfs-*.sh

Modified: cfe/trunk/test/Modules/crash-vfs-include-pch.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/crash-vfs-include-pch.m?rev=374749=374748=374749=diff
==
--- cfe/trunk/test/Modules/crash-vfs-include-pch.m (original)
+++ cfe/trunk/test/Modules/crash-vfs-include-pch.m Sun Oct 13 18:41:56 2019
@@ -8,8 +8,8 @@
 // RUN: -fmodules-cache-path=%t/cache -O0 \
 // RUN: -isystem %S/Inputs/System/usr/include
 
-// RUN: not env FORCE_CLANG_DIAGNOSTICS_CRASH= TMPDIR=%t TEMP=%t TMP=%t \
-// RUN: %clang %s -E -include-pch %t/out/pch-used.h.pch -fmodules -nostdlibinc 
\
+// RUN: env FORCE_CLANG_DIAGNOSTICS_CRASH= TMPDIR=%t TEMP=%t TMP=%t \
+// RUN: not %clang %s -E -include-pch %t/out/pch-used.h.pch -fmodules 
-nostdlibinc \
 // RUN: -fimplicit-module-maps -fmodules-cache-path=%t/cache -O0

[clang-tools-extra] r374747 - Make symbols.test pass on Windows.

2019-10-13 Thread Nico Weber via cfe-commits

Author: nico
Date: Sun Oct 13 18:19:53 2019
New Revision: 374747

URL: http://llvm.org/viewvc/llvm-project?rev=374747=rev
Log:
Make symbols.test pass on Windows.

See commit message of r374746 for details.

Hopefully final bit of PR43592.

Modified:
clang-tools-extra/trunk/clangd/test/Inputs/symbols.test.yaml
clang-tools-extra/trunk/clangd/test/symbols.test

Modified: clang-tools-extra/trunk/clangd/test/Inputs/symbols.test.yaml
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/test/Inputs/symbols.test.yaml?rev=374747=374746=374747=diff
==
--- clang-tools-extra/trunk/clangd/test/Inputs/symbols.test.yaml (original)
+++ clang-tools-extra/trunk/clangd/test/Inputs/symbols.test.yaml Sun Oct 13 
18:19:53 2019
@@ -7,7 +7,7 @@ SymInfo:
   Kind:Class
   Lang:Cpp
 CanonicalDeclaration: 
-  FileURI: 'file:///vector.h'
+  FileURI: 'test:///vector.h'
   Start:   
 Line:215
 Column:  10

Modified: clang-tools-extra/trunk/clangd/test/symbols.test
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/test/symbols.test?rev=374747=374746=374747=diff
==
--- clang-tools-extra/trunk/clangd/test/symbols.test (original)
+++ clang-tools-extra/trunk/clangd/test/symbols.test Sun Oct 13 18:19:53 2019
@@ -21,7 +21,7 @@
 # CHECK-NEXT:  "line": {{.*}}
 # CHECK-NEXT:}
 # CHECK-NEXT:  },
-# CHECK-NEXT:  "uri": "file:///vector.h"
+# CHECK-NEXT:  "uri": "file:/{{.*}}//vector.h"
 # CHECK-NEXT:},
 # CHECK-NEXT:"name": "vector"
 # CHECK-NEXT:  }


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

[PATCH] D66564: [clang-tidy] new performance struct pack align check

2019-10-13 Thread Frank Derry Wanye via Phabricator via cfe-commits

ffrankies added a comment.

As per the previous discussion, the check has been moved into the `performance` 
module.

In D66564#1670640 , @lebedev.ri wrote:

> I, too, don't believe this is FPGA specific; it should likely go into `misc-` 
> or even `performance-`.
>  The wording of the diags seems weird to me, it would be good to 1. add more 
> explanation to the docs and 2. reword the diags.

Implemented the requested code refactoring changes, and reworded the diags to 
now say `accessing the fields in struct 'name' is inefficient due to 
padding/poor alignment;`.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66564/new/

https://reviews.llvm.org/D66564

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

[PATCH] D66564: [clang-tidy] new performance struct pack align check

2019-10-13 Thread Frank Derry Wanye via Phabricator via cfe-commits

ffrankies updated this revision to Diff 224803.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66564/new/

https://reviews.llvm.org/D66564

Files:
  clang-tidy/performance/CMakeLists.txt
  clang-tidy/performance/PerformanceTidyModule.cpp
  clang-tidy/performance/StructPackAlignCheck.cpp
  clang-tidy/performance/StructPackAlignCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/performance-struct-pack-align.rst
  test/clang-tidy/performance-struct-pack-align.cpp

Index: test/clang-tidy/performance-struct-pack-align.cpp
===
--- /dev/null
+++ test/clang-tidy/performance-struct-pack-align.cpp
@@ -0,0 +1,73 @@
+// RUN: %check_clang_tidy %s performance-struct-pack-align %t -- -header-filter=.* "--" --include opencl-c.h -cl-std=CL1.2 -c
+
+// Struct needs both alignment and packing
+struct error {
+  char a;
+  double b;
+  char c;
+};
+// CHECK-MESSAGES: :[[@LINE-5]]:8: warning: accessing fields in struct 'error' is inefficient due to padding; only needs 10 bytes but is using 24 bytes, use "__attribute((packed))" [performance-struct-pack-align]
+// CHECK-MESSAGES: :[[@LINE-6]]:8: warning: accessing fields in struct 'error' is inefficient due to poor alignment; currently aligned to 8 bytes, but size 10 bytes is large enough to benefit from "__attribute((aligned(16)))" [performance-struct-pack-align]
+
+// Struct is explicitly packed, but needs alignment
+struct error_packed {
+  char a;
+  double b;
+  char c;
+} __attribute((packed));
+// CHECK-MESSAGES: :[[@LINE-5]]:8: warning: accessing fields in struct 'error_packed' is inefficient due to poor alignment; currently aligned to 1 bytes, but size 10 bytes is large enough to benefit from "__attribute((aligned(16)))" [performance-struct-pack-align]
+
+// Struct is properly packed, but needs alignment
+struct align_only {
+  char a;
+  char b;
+  char c;
+  char d;
+  int e;
+  double f;
+};
+// CHECK-MESSAGES: :[[@LINE-8]]:8: warning: accessing fields in struct 'align_only' is inefficient due to poor alignment; currently aligned to 8 bytes, but size 16 bytes is large enough to benefit from "__attribute((aligned(16)))" [performance-struct-pack-align]
+
+// Struct is perfectly packed but wrongly aligned
+struct bad_align {
+  char a;
+  double b;
+  char c;
+} __attribute((packed)) __attribute((aligned(8)));
+// CHECK-MESSAGES: :[[@LINE-5]]:8: warning: accessing fields in struct 'bad_align' is inefficient due to poor alignment; currently aligned to 8 bytes, but size 10 bytes is large enough to benefit from "__attribute((aligned(16)))" [performance-struct-pack-align]
+
+struct bad_align2 {
+  char a;
+  double b;
+  char c;
+} __attribute((packed)) __attribute((aligned(32)));
+// CHECK-MESSAGES: :[[@LINE-5]]:8: warning: accessing fields in struct 'bad_align2' is inefficient due to poor alignment; currently aligned to 32 bytes, but size 10 bytes is large enough to benefit from "__attribute((aligned(16)))" [performance-struct-pack-align]
+
+struct bad_align3 {
+  char a;
+  double b;
+  char c;
+} __attribute((packed)) __attribute((aligned(4)));
+// CHECK-MESSAGES: :[[@LINE-5]]:8: warning: accessing fields in struct 'bad_align3' is inefficient due to poor alignment; currently aligned to 4 bytes, but size 10 bytes is large enough to benefit from "__attribute((aligned(16)))" [performance-struct-pack-align]
+
+// Struct is both perfectly packed and aligned
+struct success {
+  char a;
+  double b;
+  char c;
+} __attribute((packed)) __attribute((aligned(16)));
+//Should take 10 bytes and be aligned to 16 bytes
+
+// Struct is properly packed, and explicitly aligned
+struct success2 {
+  int a;
+  int b;
+  int c;
+} __attribute((aligned(16)));
+
+// If struct is properly aligned, packing not needed
+struct error_aligned {
+  char a;
+  double b;
+  char c;
+} __attribute((aligned(16)));
Index: docs/clang-tidy/checks/performance-struct-pack-align.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/performance-struct-pack-align.rst
@@ -0,0 +1,51 @@
+.. title:: clang-tidy - performance-struct-pack-align
+
+performance-struct-pack-align
+==
+
+Finds structs that are inefficiently packed or aligned, and recommends
+packing and/or aligning of said structs as needed. 
+
+Structs that are not packed take up more space than they should, and accessing 
+structs that are not well aligned is inefficient.
+
+Based on the `Altera SDK for OpenCL: Best Practices Guide 
+`_.
+
+.. code-block:: c++
+
+  // The following struct is originally aligned to 4 bytes, and thus takes up
+  // 12 bytes of memory instead of 10. Packing the struct will make it use
+  // only 10 bytes of memory, and aligning it to 16 bytes will make it 
+  // efficient to access. 
+  struct example {
+char a;// 1 byte
+double b;  //

[PATCH] D66564: [clang-tidy] new performance struct pack align check

2019-10-13 Thread Frank Derry Wanye via Phabricator via cfe-commits

ffrankies updated this revision to Diff 224802.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66564/new/

https://reviews.llvm.org/D66564

Files:
  clang-tidy/performance/CMakeLists.txt
  clang-tidy/performance/PerformanceTidyModule.cpp
  clang-tidy/performance/StructPackAlignCheck.cpp
  clang-tidy/performance/StructPackAlignCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/performance-struct-pack-align.rst
  test/clang-tidy/performance-struct-pack-align.cpp

Index: test/clang-tidy/performance-struct-pack-align.cpp
===
--- /dev/null
+++ test/clang-tidy/performance-struct-pack-align.cpp
@@ -0,0 +1,73 @@
+// RUN: %check_clang_tidy %s performance-struct-pack-align %t -- -header-filter=.* "--" --include opencl-c.h -cl-std=CL1.2 -c
+
+// Struct needs both alignment and packing
+struct error {
+  char a;
+  double b;
+  char c;
+};
+// CHECK-MESSAGES: :[[@LINE-5]]:8: warning: accessing fields in struct 'error' is inefficient due to padding; only needs 10 bytes but is using 24 bytes, use "__attribute((packed))" [performance-struct-pack-align]
+// CHECK-MESSAGES: :[[@LINE-6]]:8: warning: accessing fields in struct 'error' is inefficient due to poor alignment; currently aligned to 8 bytes, but size 10 bytes is large enough to benefit from "__attribute((aligned(16)))" [performance-struct-pack-align]
+
+// Struct is explicitly packed, but needs alignment
+struct error_packed {
+  char a;
+  double b;
+  char c;
+} __attribute((packed));
+// CHECK-MESSAGES: :[[@LINE-5]]:8: warning: accessing fields in struct 'error_packed' is inefficient due to poor alignment; currently aligned to 1 bytes, but size 10 bytes is large enough to benefit from "__attribute((aligned(16)))" [performance-struct-pack-align]
+
+// Struct is properly packed, but needs alignment
+struct align_only {
+  char a;
+  char b;
+  char c;
+  char d;
+  int e;
+  double f;
+};
+// CHECK-MESSAGES: :[[@LINE-8]]:8: warning: accessing fields in struct 'align_only' is inefficient due to poor alignment; currently aligned to 8 bytes, but size 16 bytes is large enough to benefit from "__attribute((aligned(16)))" [performance-struct-pack-align]
+
+// Struct is perfectly packed but wrongly aligned
+struct bad_align {
+  char a;
+  double b;
+  char c;
+} __attribute((packed)) __attribute((aligned(8)));
+// CHECK-MESSAGES: :[[@LINE-5]]:8: warning: accessing fields in struct 'bad_align' is inefficient due to poor alignment; currently aligned to 8 bytes, but size 10 bytes is large enough to benefit from "__attribute((aligned(16)))" [performance-struct-pack-align]
+
+struct bad_align2 {
+  char a;
+  double b;
+  char c;
+} __attribute((packed)) __attribute((aligned(32)));
+// CHECK-MESSAGES: :[[@LINE-5]]:8: warning: accessing fields in struct 'bad_align2' is inefficient due to poor alignment; currently aligned to 32 bytes, but size 10 bytes is large enough to benefit from "__attribute((aligned(16)))" [performance-struct-pack-align]
+
+struct bad_align3 {
+  char a;
+  double b;
+  char c;
+} __attribute((packed)) __attribute((aligned(4)));
+// CHECK-MESSAGES: :[[@LINE-5]]:8: warning: accessing fields in struct 'bad_align3' is inefficient due to poor alignment; currently aligned to 4 bytes, but size 10 bytes is large enough to benefit from "__attribute((aligned(16)))" [performance-struct-pack-align]
+
+// Struct is both perfectly packed and aligned
+struct success {
+  char a;
+  double b;
+  char c;
+} __attribute((packed)) __attribute((aligned(16)));
+//Should take 10 bytes and be aligned to 16 bytes
+
+// Struct is properly packed, and explicitly aligned
+struct success2 {
+  int a;
+  int b;
+  int c;
+} __attribute((aligned(16)));
+
+// If struct is properly aligned, packing not needed
+struct error_aligned {
+  char a;
+  double b;
+  char c;
+} __attribute((aligned(16)));
Index: docs/clang-tidy/checks/performance-struct-pack-align.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/performance-struct-pack-align.rst
@@ -0,0 +1,51 @@
+.. title:: clang-tidy - performance-struct-pack-align
+
+performance-struct-pack-align
+==
+
+Finds structs that are inefficiently packed or aligned, and recommends
+packing and/or aligning of said structs as needed. 
+
+Structs that are not packed take up more space than they should, and accessing 
+structs that are not well aligned is inefficient.
+
+Based on the `Altera SDK for OpenCL: Best Practices Guide 
+`_.
+
+.. code-block:: c++
+
+  // The following struct is originally aligned to 4 bytes, and thus takes up
+  // 12 bytes of memory instead of 10. Packing the struct will make it use
+  // only 10 bytes of memory, and aligning it to 16 bytes will make it 
+  // efficient to access. 
+  struct example {
+char a;// 1 byte
+double b;  //

[PATCH] D62855: [clangd] Implementation of auto type expansion.

2019-10-13 Thread Nico Weber via Phabricator via cfe-commits

thakis added inline comments.
Herald added a subscriber: usaxena95.



Comment at: clang-tools-extra/trunk/clangd/test/code-action-request.test:54
+---
+{"jsonrpc":"2.0","id":4,"method":"workspace/executeCommand","params":{"command":"clangd.applyTweak","arguments":[{"file":"file:///clangd-test/main.cpp","selection":{"end":{"character":4,"line":0},"start":{"character":0,"line":0}},"tweakID":"ExpandAutoType"}]}}
+#  CHECK:"newText": "int",

FYI, referring to a file opened as test:///foo.cpp as 
file:///clangd-test/file.cpp is wrong on Windows. I fixed this in rL374746.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62855/new/

https://reviews.llvm.org/D62855



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

[PATCH] D65387: [clangd] Add a callback mechanism for handling responses from client.

2019-10-13 Thread Nico Weber via Phabricator via cfe-commits

thakis added inline comments.
Herald added a subscriber: usaxena95.



Comment at: clang-tools-extra/trunk/clangd/test/request-reply.test:6
+---
+{"jsonrpc":"2.0","id":4,"method":"workspace/executeCommand","params":{"command":"clangd.applyTweak","arguments":[{"file":"file:///clangd-test/main.cpp","selection":{"end":{"character":4,"line":0},"start":{"character":0,"line":0}},"tweakID":"ExpandAutoType"}]}}
+#  CHECK:  "id": 0,

FYI, referring to a file opened as test:///foo.cpp as 
file:///clangd-test/file.cpp is wrong on Windows. I fixed this in rL374746.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65387/new/

https://reviews.llvm.org/D65387



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

[clang-tools-extra] r374746 - Make code-action-request.test and request-reply.test pass on Windows.

2019-10-13 Thread Nico Weber via cfe-commits

Author: nico
Date: Sun Oct 13 18:00:33 2019
New Revision: 374746

URL: http://llvm.org/viewvc/llvm-project?rev=374746=rev
Log:
Make code-action-request.test and request-reply.test pass on Windows.

clangd's test:// scheme expands to /clangd-test on non-Win and to
C:/clang-test on Win, so it can't be mixed freely with
file:///clangd-test since that's wrong on Windows. This makes both
tests consistenly use the test:// scheme. (Alternatively, we could use
the //INPUT_DIR pattern used in a few other tests.)

Part of PR43592.

Modified:
clang-tools-extra/trunk/clangd/test/code-action-request.test
clang-tools-extra/trunk/clangd/test/request-reply.test

Modified: clang-tools-extra/trunk/clangd/test/code-action-request.test
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/test/code-action-request.test?rev=374746=374745=374746=diff
==
--- clang-tools-extra/trunk/clangd/test/code-action-request.test (original)
+++ clang-tools-extra/trunk/clangd/test/code-action-request.test Sun Oct 13 
18:00:33 2019
@@ -32,7 +32,7 @@
 # CHECK-NEXT:{
 # CHECK-NEXT:  "arguments": [
 # CHECK-NEXT:{
-# CHECK-NEXT:  "file": "file:///clangd-test/main.cpp",
+# CHECK-NEXT:  "file": "file://{{.*}}/clangd-test/main.cpp",
 # CHECK-NEXT:  "selection": {
 # CHECK-NEXT:"end": {
 # CHECK-NEXT:  "character": 4,
@@ -51,7 +51,7 @@
 # CHECK-NEXT:}
 # CHECK-NEXT:  ]
 ---
-{"jsonrpc":"2.0","id":4,"method":"workspace/executeCommand","params":{"command":"clangd.applyTweak","arguments":[{"file":"file:///clangd-test/main.cpp","selection":{"end":{"character":4,"line":0},"start":{"character":0,"line":0}},"tweakID":"ExpandAutoType"}]}}
+{"jsonrpc":"2.0","id":4,"method":"workspace/executeCommand","params":{"command":"clangd.applyTweak","arguments":[{"file":"test:///main.cpp","selection":{"end":{"character":4,"line":0},"start":{"character":0,"line":0}},"tweakID":"ExpandAutoType"}]}}
 #  CHECK:"newText": "int",
 # CHECK-NEXT:"range": {
 # CHECK-NEXT:  "end": {
@@ -67,4 +67,4 @@
 {"jsonrpc":"2.0","id":4,"method":"shutdown"}
 ---
 {"jsonrpc":"2.0","method":"exit"}

\ No newline at end of file
+---

Modified: clang-tools-extra/trunk/clangd/test/request-reply.test
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/test/request-reply.test?rev=374746=374745=374746=diff
==
--- clang-tools-extra/trunk/clangd/test/request-reply.test (original)
+++ clang-tools-extra/trunk/clangd/test/request-reply.test Sun Oct 13 18:00:33 
2019
@@ -3,7 +3,7 @@
 ---
 
{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,"text":"auto
 i = 0;"}}}
 ---
-{"jsonrpc":"2.0","id":4,"method":"workspace/executeCommand","params":{"command":"clangd.applyTweak","arguments":[{"file":"file:///clangd-test/main.cpp","selection":{"end":{"character":4,"line":0},"start":{"character":0,"line":0}},"tweakID":"ExpandAutoType"}]}}
+{"jsonrpc":"2.0","id":4,"method":"workspace/executeCommand","params":{"command":"clangd.applyTweak","arguments":[{"file":"test:///main.cpp","selection":{"end":{"character":4,"line":0},"start":{"character":0,"line":0}},"tweakID":"ExpandAutoType"}]}}
 #  CHECK:  "id": 0,
 #  CHECK:  "method": "workspace/applyEdit",
 #  CHECK:  "newText": "int",
@@ -25,7 +25,7 @@
 # CHECK-NEXT:  },
 # CHECK-NEXT:  "id": 4,
 ---
-{"jsonrpc":"2.0","id":5,"method":"workspace/executeCommand","params":{"command":"clangd.applyTweak","arguments":[{"file":"file:///clangd-test/main.cpp","selection":{"end":{"character":4,"line":0},"start":{"character":0,"line":0}},"tweakID":"ExpandAutoType"}]}}
+{"jsonrpc":"2.0","id":5,"method":"workspace/executeCommand","params":{"command":"clangd.applyTweak","arguments":[{"file":"test:///main.cpp","selection":{"end":{"character":4,"line":0},"start":{"character":0,"line":0}},"tweakID":"ExpandAutoType"}]}}
 #  CHECK:  "id": 1,
 #  CHECK:  "method": "workspace/applyEdit",
 ---


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

[clang-tools-extra] r374745 - Don't run background-index.test on Windows.

2019-10-13 Thread Nico Weber via cfe-commits

Author: nico
Date: Sun Oct 13 17:45:02 2019
New Revision: 374745

URL: http://llvm.org/viewvc/llvm-project?rev=374745=rev
Log:
Don't run background-index.test on Windows.

The test had a "UNSUPPORTED: win32" line, but the spelling of that
changed in r339307 a year ago. Finally update this test too.

Part of PR43592.

Modified:
clang-tools-extra/trunk/clangd/test/background-index.test

Modified: clang-tools-extra/trunk/clangd/test/background-index.test
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/test/background-index.test?rev=374745=374744=374745=diff
==
--- clang-tools-extra/trunk/clangd/test/background-index.test (original)
+++ clang-tools-extra/trunk/clangd/test/background-index.test Sun Oct 13 
17:45:02 2019
@@ -1,5 +1,5 @@
 # We need to splice paths into file:// URIs for this test.
-# UNSUPPORTED: win32
+# UNSUPPORTED: windows-msvc
 
 # Use a copy of inputs, as we'll mutate it (as will the background index).
 # RUN: rm -rf %t


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

[PATCH] D66564: [clang-tidy] new performance struct pack align check

2019-10-13 Thread Frank Derry Wanye via Phabricator via cfe-commits

ffrankies updated this revision to Diff 224799.
ffrankies retitled this revision from "[clang-tidy] new FPGA struct pack align 
check" to "[clang-tidy] new performance struct pack align check".
ffrankies edited the summary of this revision.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66564/new/

https://reviews.llvm.org/D66564

Files:
  clang-tidy/performance/CMakeLists.txt
  clang-tidy/performance/PerformanceTidyModule.cpp
  clang-tidy/performance/StructPackAlignCheck.cpp
  clang-tidy/performance/StructPackAlignCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/performance-struct-pack-align.rst
  test/clang-tidy/performance-struct-pack-align.cpp

Index: test/clang-tidy/performance-struct-pack-align.cpp
===
--- /dev/null
+++ test/clang-tidy/performance-struct-pack-align.cpp
@@ -0,0 +1,73 @@
+// RUN: %check_clang_tidy %s performance-struct-pack-align %t -- -header-filter=.* "--" --include opencl-c.h -cl-std=CL1.2 -c
+
+// Struct needs both alignment and packing
+struct error {
+  char a;
+  double b;
+  char c;
+};
+// CHECK-MESSAGES: :[[@LINE-5]]:8: warning: accessing fields in struct 'error' is inefficient access due to padding; only needs 10 bytes but is using 24 bytes, use "__attribute((packed))" [performance-struct-pack-align]
+// CHECK-MESSAGES: :[[@LINE-6]]:8: warning: accessing fields in struct 'error' is inefficient access due to poor alignment; currently aligned to 8 bytes, but size 10 bytes is large enough to benefit from "__attribute((aligned(16)))" [performance-struct-pack-align]
+
+// Struct is explicitly packed, but needs alignment
+struct error_packed {
+  char a;
+  double b;
+  char c;
+} __attribute((packed));
+// CHECK-MESSAGES: :[[@LINE-5]]:8: warning: accessing fields in struct 'error_packed' is inefficient access due to poor alignment; currently aligned to 1 bytes, but size 10 bytes is large enough to benefit from "__attribute((aligned(16)))" [performance-struct-pack-align]
+
+// Struct is properly packed, but needs alignment
+struct align_only {
+  char a;
+  char b;
+  char c;
+  char d;
+  int e;
+  double f;
+};
+// CHECK-MESSAGES: :[[@LINE-8]]:8: warning: accessing fields in struct 'align_only' is inefficient access due to poor alignment; currently aligned to 8 bytes, but size 16 bytes is large enough to benefit from "__attribute((aligned(16)))" [performance-struct-pack-align]
+
+// Struct is perfectly packed but wrongly aligned
+struct bad_align {
+  char a;
+  double b;
+  char c;
+} __attribute((packed)) __attribute((aligned(8)));
+// CHECK-MESSAGES: :[[@LINE-5]]:8: warning: accessing fields in struct 'bad_align' is inefficient access due to poor alignment; currently aligned to 8 bytes, but size 10 bytes is large enough to benefit from "__attribute((aligned(16)))" [performance-struct-pack-align]
+
+struct bad_align2 {
+  char a;
+  double b;
+  char c;
+} __attribute((packed)) __attribute((aligned(32)));
+// CHECK-MESSAGES: :[[@LINE-5]]:8: warning: accessing fields in struct 'bad_align2' is inefficient access due to poor alignment; currently aligned to 32 bytes, but size 10 bytes is large enough to benefit from "__attribute((aligned(16)))" [performance-struct-pack-align]
+
+struct bad_align3 {
+  char a;
+  double b;
+  char c;
+} __attribute((packed)) __attribute((aligned(4)));
+// CHECK-MESSAGES: :[[@LINE-5]]:8: warning: accessing fields in struct 'bad_align3' is inefficient access due to poor alignment; currently aligned to 4 bytes, but size 10 bytes is large enough to benefit from "__attribute((aligned(16)))" [performance-struct-pack-align]
+
+// Struct is both perfectly packed and aligned
+struct success {
+  char a;
+  double b;
+  char c;
+} __attribute((packed)) __attribute((aligned(16)));
+//Should take 10 bytes and be aligned to 16 bytes
+
+// Struct is properly packed, and explicitly aligned
+struct success2 {
+  int a;
+  int b;
+  int c;
+} __attribute((aligned(16)));
+
+// If struct is properly aligned, packing not needed
+struct error_aligned {
+  char a;
+  double b;
+  char c;
+} __attribute((aligned(16)));
Index: docs/clang-tidy/checks/performance-struct-pack-align.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/performance-struct-pack-align.rst
@@ -0,0 +1,51 @@
+.. title:: clang-tidy - performance-struct-pack-align
+
+performance-struct-pack-align
+==
+
+Finds structs that are inefficiently packed or aligned, and recommends
+packing and/or aligning of said structs as needed. 
+
+Structs that are not packed take up more space than they should, and accessing 
+structs that are not well aligned is inefficient.
+
+Based on the `Altera SDK for OpenCL: Best Practices Guide 
+`_.
+
+.. code-block:: c++
+
+  // The following struct is originally aligned to 4 bytes, and thus takes up

[PATCH] D68391: [RISCV] Improve sysroot computation if no GCC install detected

2019-10-13 Thread Luís Marques via Phabricator via cfe-commits

luismarques added a comment.

This is indeed an issue that would be nice to fix, I've often been annoyed by 
clang just defaulting to the root when some misconfiguration occurs. I have to 
wonder though, this patch only changes the clang RISC-V toolchain driver, but 
the problem isn't specific to RISC-V. Couldn't this tweak be generalized and 
made to apply to multiple/all target drivers?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68391/new/

https://reviews.llvm.org/D68391



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

[PATCH] D67638: Improve __builtin_constant_p lowering

2019-10-13 Thread Joerg Sonnenberger via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG529f4ed401ea: Improve __builtin_constant_p lowering 
(authored by joerg).

Changed prior to commit:
  https://reviews.llvm.org/D67638?vs=220398=224796#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67638/new/

https://reviews.llvm.org/D67638

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/builtin-constant-p.c
  clang/test/CodeGen/ppc-emmintrin.c
  clang/test/CodeGen/ppc-tmmintrin.c

Index: clang/test/CodeGen/ppc-tmmintrin.c
===
--- clang/test/CodeGen/ppc-tmmintrin.c
+++ clang/test/CodeGen/ppc-tmmintrin.c
@@ -108,7 +108,8 @@
 // CHECK: store <2 x i64> [[REG61]], <2 x i64>* [[REG65:[0-9a-zA-Z_%.]+]], align 16
 // CHECK-NEXT: store <2 x i64> [[REG62]], <2 x i64>* [[REG66:[0-9a-zA-Z_%.]+]], align 16
 // CHECK-NEXT: store i32 [[REG63]], i32* [[REG64:[0-9a-zA-Z_%.]+]], align 4
-// CHECK-NEXT: br i1 false, label %[[REG67:[0-9a-zA-Z_%.]+]], label %[[REG68:[0-9a-zA-Z_%.]+]]
+// CHECK: %[[REG64b:[0-9a-zA-Z_%.]+]] = call i1 @llvm.is.constant.i32(i32 %0)
+// CHECK-NEXT: br i1 %[[REG64b]], label %[[REG67:[0-9a-zA-Z_%.]+]], label %[[REG68:[0-9a-zA-Z_%.]+]]
 
 // CHECK: [[REG67]]:
 // CHECK-NEXT: [[REG69:[0-9a-zA-Z_%.]+]] = load i32, i32* [[REG64]], align 4
Index: clang/test/CodeGen/ppc-emmintrin.c
===
--- clang/test/CodeGen/ppc-emmintrin.c
+++ clang/test/CodeGen/ppc-emmintrin.c
@@ -231,7 +231,9 @@
 // CHECK-NEXT: br i1 [[REG147]], label %[[REG148:[0-9a-zA-Z_%.]+]], label %[[REG149:[0-9a-zA-Z_%.]+]]
 // CHECK: [[REG148]]:
 
-// CHECK-LE-NEXT: br i1 false, label %[[REG150:[0-9a-zA-Z_%.]+]], label %[[REG151:[0-9a-zA-Z_%.]+]]
+// CHECK-LE-NEXT: load
+// CHECK-LE-NEXT: call i1 @llvm.is.constant
+// CHECK-LE-NEXT: br i1 %[[REG1896a:[0-9a-zA-Z_%.]+]], label %[[REG150:[0-9a-zA-Z_%.]+]], label %[[REG151:[0-9a-zA-Z_%.]+]]
 // CHECK-LE: [[REG150]]:
 // CHECK-LE: [[REG152:[0-9a-zA-Z_%.]+]] = load <2 x i64>, <2 x i64>* [[REG143]], align 16
 // CHECK-LE-NEXT: [[REG153:[0-9a-zA-Z_%.]+]] = bitcast <2 x i64> [[REG152]] to <16 x i8>
@@ -2326,7 +2328,9 @@
 // CHECK-NEXT: br i1 [[REG1559]], label %[[REG1560:[0-9a-zA-Z_%.]+]], label %[[REG1557:[0-9a-zA-Z_%.]+]]
 
 // CHECK: [[REG1560]]:
-// CHECK-NEXT: br i1 false, label %[[REG1561:[0-9a-zA-Z_%.]+]], label %[[REG1562:[0-9a-zA-Z_%.]+]]
+// CHECK-NEXT: load
+// CHECK-NEXT: call i1 @llvm.is.constant
+// CHECK-NEXT: br i1 %[[REG1561a:[0-9a-zA-Z_%.]+]], label %[[REG1561:[0-9a-zA-Z_%.]+]], label %[[REG1562:[0-9a-zA-Z_%.]+]]
 
 // CHECK: [[REG1561]]:
 // CHECK-NEXT: [[REG1563:[0-9a-zA-Z_%.]+]] = load i32, i32* [[REG1552]], align 4
@@ -2369,7 +2373,9 @@
 // CHECK-NEXT: br i1 [[REG1587]], label %[[REG1588:[0-9a-zA-Z_%.]+]], label %[[REG1585:[0-9a-zA-Z_%.]+]]
 
 // CHECK: [[REG1588]]:
-// CHECK-NEXT: br i1 false, label %[[REG1589:[0-9a-zA-Z_%.]+]], label %[[REG1590:[0-9a-zA-Z_%.]+]]
+// CHECK-NEXT: load
+// CHECK-NEXT: call i1 @llvm.is.constant
+// CHECK-NEXT: br i1 %{{[0-9a-zA-Z_%.]+}}, label %[[REG1589:[0-9a-zA-Z_%.]+]], label %[[REG1590:[0-9a-zA-Z_%.]+]]
 
 // CHECK: [[REG1589]]:
 // CHECK-NEXT: [[REG1591:[0-9a-zA-Z_%.]+]] = load i32, i32* [[REG1580]], align 4
@@ -2416,7 +2422,9 @@
 // CHECK-NEXT: br i1 [[REG1617]], label %[[REG1618:[0-9a-zA-Z_%.]+]], label %[[REG1615:[0-9a-zA-Z_%.]+]]
 
 // CHECK: [[REG1618]]:
-// CHECK-NEXT: br i1 false, label %[[REG1619:[0-9a-zA-Z_%.]+]], label %[[REG1620:[0-9a-zA-Z_%.]+]]
+// CHECK-NEXT: load
+// CHECK-NEXT: call i1 @llvm.is.constant
+// CHECK-NEXT: br i1 %{{[0-9a-zA-Z_%.]+}}, label %[[REG1619:[0-9a-zA-Z_%.]+]], label %[[REG1620:[0-9a-zA-Z_%.]+]]
 
 // CHECK: [[REG1619]]:
 // CHECK-NEXT: [[REG1621:[0-9a-zA-Z_%.]+]] = load i32, i32* [[REG1610]], align 4
@@ -2563,7 +2571,9 @@
 // CHECK-NEXT: br i1 [[REG1712]], label %[[REG1713:[0-9a-zA-Z_%.]+]], label %[[REG1714:[0-9a-zA-Z_%.]+]]
 
 // CHECK: [[REG1713]]:
-// CHECK-NEXT: br i1 false, label %[[REG1715:[0-9a-zA-Z_%.]+]], label %[[REG1716:[0-9a-zA-Z_%.]+]]
+// CHECK-NEXT: load
+// CHECK-NEXT: call i1 @llvm.is.constant
+// CHECK-NEXT: br i1 %[[REG1715a:[0-9a-zA-Z_%.]+]], label %[[REG1715:[0-9a-zA-Z_%.]+]], label %[[REG1716:[0-9a-zA-Z_%.]+]]
 
 // CHECK: [[REG1715]]:
 // CHECK-NEXT: [[REG1717:[0-9a-zA-Z_%.]+]] = load i32, i32* [[REG1709]], align 4
@@ -2601,7 +2611,9 @@
 // CHECK-NEXT: br i1 [[REG1737]], label %[[REG1738:[0-9a-zA-Z_%.]+]], label %[[REG1739:[0-9a-zA-Z_%.]+]]
 
 // CHECK: [[REG1738]]:
-// CHECK-NEXT: br i1 false, label %[[REG1740:[0-9a-zA-Z_%.]+]], label %[[REG1741:[0-9a-zA-Z_%.]+]]
+// CHECK-NEXT: load
+// CHECK-NEXT: call i1 @llvm.is.constant
+// CHECK-NEXT: br i1 %[[REG1738:[0-9a-zA-Z_%.]+]], label %[[REG1740:[0-9a-zA-Z_%.]+]], label %[[REG1741:[0-9a-zA-Z_%.]+]]
 
 // CHECK: [[REG1740]]:
 // CHECK-NEXT: [[REG1742:[0-9a-zA-Z_%.]+]] = load i32, i32* [[REG1734]], align 4
@@ -2741,7 +2753,9 @@
 // CHECK-NEXT:

r374742 - Improve __builtin_constant_p lowering

2019-10-13 Thread Joerg Sonnenberger via cfe-commits

Author: joerg
Date: Sun Oct 13 15:33:46 2019
New Revision: 374742

URL: http://llvm.org/viewvc/llvm-project?rev=374742=rev
Log:
Improve __builtin_constant_p lowering

__builtin_constant_p used to be short-cut evaluated to false when
building with -O0. This is undesirable as it means that constant folding
in the front-end can give different results than folding in the back-end.
It can also create conditional branches on constant conditions that don't
get folded away. With the pending improvements to the llvm.is.constant
handling on the LLVM side, the short-cut is no longer useful.

Adjust various codegen tests to not depend on the short-cut or the
backend optimisations.

Differential Revision: https://reviews.llvm.org/D67638

Modified:
cfe/trunk/lib/CodeGen/CGBuiltin.cpp
cfe/trunk/test/CodeGen/builtin-constant-p.c
cfe/trunk/test/CodeGen/ppc-emmintrin.c
cfe/trunk/test/CodeGen/ppc-tmmintrin.c

Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=374742=374741=374742=diff
==
--- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Sun Oct 13 15:33:46 2019
@@ -2101,10 +2101,6 @@ RValue CodeGenFunction::EmitBuiltinExpr(
 
   case Builtin::BI__builtin_constant_p: {
 llvm::Type *ResultType = ConvertType(E->getType());
-if (CGM.getCodeGenOpts().OptimizationLevel == 0)
-  // At -O0, we don't perform inlining, so we don't need to delay the
-  // processing.
-  return RValue::get(ConstantInt::get(ResultType, 0));
 
 const Expr *Arg = E->getArg(0);
 QualType ArgType = Arg->getType();

Modified: cfe/trunk/test/CodeGen/builtin-constant-p.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/builtin-constant-p.c?rev=374742=374741=374742=diff
==
--- cfe/trunk/test/CodeGen/builtin-constant-p.c (original)
+++ cfe/trunk/test/CodeGen/builtin-constant-p.c Sun Oct 13 15:33:46 2019
@@ -1,12 +1,8 @@
-// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s -O2 | 
FileCheck %s
-// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s -O0 | 
FileCheck --check-prefix=O0 %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm 
-disable-llvm-optzns -o - %s -O2 | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm 
-disable-llvm-optzns -o - %s -O0 | FileCheck %s
 
 int a = 42;
 
-inline int bcp(int x) {
-  return __builtin_constant_p(x);
-}
-
 /* --- Compound literals */
 
 struct foo { int x, y; };
@@ -15,85 +11,56 @@ int y;
 struct foo f = (struct foo){ __builtin_constant_p(y), 42 };
 
 struct foo test0(int expr) {
-  // CHECK: define i64 @test0(i32 %expr)
-  // CHECK: call i1 @llvm.is.constant.i32(i32 %expr)
+  // CHECK-LABEL: test0
+  // CHECK: call i1 @llvm.is.constant.i32
   struct foo f = (struct foo){ __builtin_constant_p(expr), 42 };
   return f;
 }
 
 /* --- Pointer types */
 
-inline int test1_i(int *x) {
-  return *x;
-}
-
 int test1() {
-  // CHECK: define i32 @test1
-  // CHECK: add nsw i32 %0, -13
-  // CHECK-NEXT: call i1 @llvm.is.constant.i32(i32 %sub)
-  return bcp(test1_i() - 13);
-}
-
-int test2() {
-  // CHECK: define i32 @test2
+  // CHECK-LABEL: test1
   // CHECK: ret i32 0
   return __builtin_constant_p( - 13);
 }
 
-inline int test3_i(int *x) {
-  return 42;
-}
-
-int test3() {
-  // CHECK: define i32 @test3
-  // CHECK: ret i32 1
-  return bcp(test3_i() - 13);
-}
-
 /* --- Aggregate types */
 
 int b[] = {1, 2, 3};
 
-int test4() {
-  // CHECK: define i32 @test4
+int test2() {
+  // CHECK-LABEL: test2
   // CHECK: ret i32 0
   return __builtin_constant_p(b);
 }
 
-const char test5_c[] = {1, 2, 3, 0};
+const char test3_c[] = {1, 2, 3, 0};
 
-int test5() {
-  // CHECK: define i32 @test5
+int test3() {
+  // CHECK-LABEL: test3
   // CHECK: ret i32 0
-  return __builtin_constant_p(test5_c);
+  return __builtin_constant_p(test3_c);
 }
 
-inline char test6_i(const char *x) {
+inline char test4_i(const char *x) {
   return x[1];
 }
 
-int test6() {
-  // CHECK: define i32 @test6
+int test4() {
+  // CHECK: define i32 @test4
   // CHECK: ret i32 0
-  return __builtin_constant_p(test6_i(test5_c));
-}
-
-/* --- Non-constant global variables */
-
-int test7() {
-  // CHECK: define i32 @test7
-  // CHECK: call i1 @llvm.is.constant.i32(i32 %0)
-  return bcp(a);
+  return __builtin_constant_p(test4_i(test3_c));
 }
 
 /* --- Constant global variables */
 
 const int c = 42;
 
-int test8() {
-  // CHECK: define i32 @test8
+int test5() {
+  // CHECK-LABEL: test5
   // CHECK: ret i32 1
-  return bcp(c);
+  return __builtin_constant_p(c);
 }
 
 /* --- Array types */
@@ -101,34 +68,34 @@ int test8() {
 int arr[] = { 1, 2, 3 };
 const int c_arr[] = { 1, 2, 3 };
 
-int test9() {
-  // CHECK: define i32 @test9
-  // CHECK: call i1 @llvm.is.constant.i32(i32 %0)
+int

[PATCH] D68931: [clang] [clang-offload-bundler] Fix finding installed llvm-objcopy

2019-10-13 Thread Alex Brachet via Phabricator via cfe-commits

abrachet added inline comments.



Comment at: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp:469
 // Find llvm-objcopy in order to create the bundle binary.
 ErrorOr Objcopy = sys::findProgramByName(
 "llvm-objcopy", sys::path::parent_path(BundlerExecutable));

I'd be fine removing this FWIW. Is there a high chance they will be in the same 
directory? I think on a mac, there is very little chance this will be true 
because Apple preinstalls clang in /usr/bin, but not llvm-objcopy that would 
have to go in /usr/local/bin. Is it enough of an optimization to keep? Up to 
you I think.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68931/new/

https://reviews.llvm.org/D68931



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

[PATCH] D62686: [RISCV] Add support for save/restore of callee-saved registers via libcalls

2019-10-13 Thread Luís Marques via Phabricator via cfe-commits

luismarques added a comment.

The priority for this patch is to address the issues reported by @apazos but 
after that please check the clang-format output. There are some cases in this 
patch where it might make sense to use a different formatting than clang-format 
indicates, but the remaining should be addressed.

@apazos Have you considered tweaking the patch code to not do a tail call, just 
to check if that's what's causing the remaining failures? I'm not sure if 
that's too hard, but it could eventually be easier than drilling into the 
failing cases.




Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:27
+// registers.
+static int getLibCallID(const MachineFunction ,
+const std::vector ) {

The return value isn't used as just an opaque index, it also reflects the frame 
size and is used for that purpose. The function comment should probably reflect 
that.



Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:34
+
+  unsigned MaxReg = 0;
+  for (auto  : CSI)

Use `Register` and `RISCV::NoRegister`. (You'll have to use `MaxReg.id()` 
instead in the call to `max`).



Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:36
+  for (auto  : CSI)
+if (CS.getFrameIdx() < 0)
+  MaxReg = std::max(MaxReg, CS.getReg());

Might be worth adding a small comment explaining how this serves as a filters 
for the registers we are interested in. Or point to a later relevant comment?



Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:39
+
+  if (MaxReg == 0)
+return -1;

Ditto `NoRegister`.



Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:66
+const std::vector ) {
+  static const char *const spillLibCalls[] = {
+"__riscv_save_0",

Check LLVM naming convention capitalization. Ditto other vars here.



Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:93
+  const std::vector ) {
+  static const char *const restoreLibCalls[] = {
+"__riscv_restore_0",

Check LLVM naming convention capitalization. Ditto other vars here.



Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:190
 
+static std::vector
+getNonLibcallCSI(const std::vector ) {

This could probably use `SmallVector`.



Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:706
+  for (auto  : reverse(NonLibcallCSI)) {
+unsigned Reg = CS.getReg();
+const TargetRegisterClass *RC = TRI->getMinimalPhysRegClass(Reg);

Ditto `Register`.



Comment at: llvm/lib/Target/RISCV/RISCVRegisterInfo.cpp:95
+// by save/restore libcalls.
+static const std::map FixedCSRFIMap = {
+  {/*ra*/  RISCV::X1,   -1},

Use `IndexedMap` instead?



Comment at: llvm/test/CodeGen/RISCV/saverestore.ll:348
+
+; Check that functions with varargs do not use save/restore code
+

Maybe for these tests just put a -NOT check that __riscv_save_ isn't called?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62686/new/

https://reviews.llvm.org/D62686



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

[PATCH] D68931: [clang] [clang-offload-bundler] Fix finding installed llvm-objcopy

2019-10-13 Thread Michał Górny via Phabricator via cfe-commits

mgorny created this revision.
mgorny added reviewers: sdmitriev, ABataev.
Herald added a reviewer: alexshap.
Herald added a subscriber: abrachet.

Allow finding installed llvm-objcopy in PATH if it's not present
in the directory containing clang-offload-bundler.  This is the case
if clang is being built stand-alone, and llvm-objcopy is already
installed while the c-o-b tool is still present in build directory.

This is consistent with how e.g. llvm-symbolizer is found in LLVM.
However, most of similar searches in LLVM and Clang are performed
without special-casing the program directory.

Fixes r369955.


https://reviews.llvm.org/D68931

Files:
  clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp


Index: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
===
--- clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
+++ clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
@@ -468,6 +468,8 @@
 // Find llvm-objcopy in order to create the bundle binary.
 ErrorOr Objcopy = sys::findProgramByName(
 "llvm-objcopy", sys::path::parent_path(BundlerExecutable));
+if (!Objcopy)
+  Objcopy = sys::findProgramByName("llvm-objcopy");
 if (!Objcopy) {
   errs() << "error: unable to find 'llvm-objcopy' in path.\n";
   return true;


Index: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
===
--- clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
+++ clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
@@ -468,6 +468,8 @@
 // Find llvm-objcopy in order to create the bundle binary.
 ErrorOr Objcopy = sys::findProgramByName(
 "llvm-objcopy", sys::path::parent_path(BundlerExecutable));
+if (!Objcopy)
+  Objcopy = sys::findProgramByName("llvm-objcopy");
 if (!Objcopy) {
   errs() << "error: unable to find 'llvm-objcopy' in path.\n";
   return true;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68913: Adds fixit hints to the Wrange-loop-analysis

2019-10-13 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added inline comments.



Comment at: clang/lib/Sema/SemaStmt.cpp:2759
 SemaRef.Context.getLValueReferenceType(E->getType().withConst());
-SemaRef.Diag(VD->getBeginLoc(), diag::note_use_type_or_non_reference)
-<< NonReferenceType << NewReferenceType << VD->getSourceRange();
+SemaRef.Diag(VD->getTypeSpecEndLoc(), diag::note_use_type_or_non_reference)
+<< NonReferenceType << NewReferenceType << VD->getSourceRange()

I don't think the diagnostic should point to the end of the type specification 
rather than the beginning. That puts the caret in a rather strange location for 
the diagnostic.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68913/new/

https://reviews.llvm.org/D68913



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

[PATCH] D68824: Fix __builtin_assume_aligned with too large values.

2019-10-13 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added inline comments.



Comment at: clang/lib/Sema/SemaChecking.cpp:6067-6070
+// Alignment calculations can wrap around if it's greater than 2**28.
+unsigned MaximumAlignment =
+Context.getTargetInfo().getTriple().isOSBinFormatCOFF() ? 8192
+: 268435456;

erichkeane wrote:
> rsmith wrote:
> > Why is there a different limit depending on bin format? We can support this 
> > at the IR level regardless, can't we? (I don't see how the binary format is 
> > relevant.)
> I'd copied it from the Sema::AddAlignedAttr implementation, but I cannot seem 
> to figure out the origin of that.  @majnemer added the 2**28 business back in 
> 2015, but @aaron.ballman put the limit of 8192 in here: 
> https://reviews.llvm.org/rL158717#change-N0HH8qtBJv7d
> (note it was reverted and relanded).
> 
> I don't see sufficient justification in that history now that I've looked 
> back to keep that log in here, so I'll keep us at 2**28.
> 
It's been a while, but I seem to recall this matching a behavior of MSVC at the 
time. 


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68824/new/

https://reviews.llvm.org/D68824



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

[PATCH] D67159: [clang] New attribute((__clang_arm_mve_alias)).

2019-10-13 Thread Aaron Ballman via Phabricator via cfe-commits

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

LGTM!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67159/new/

https://reviews.llvm.org/D67159



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

[PATCH] D68581: Include leading attributes in DeclStmt's SourceRange

2019-10-13 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added inline comments.



Comment at: clang/test/AST/sourceranges.cpp:147
+
+// CHECK-1Z: NamespaceDecl {{.*}} attributed_decl
+namespace attributed_decl {

I think these CHECK lines should just be a normal check instead of a C++17 
check.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68581/new/

https://reviews.llvm.org/D68581



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

[PATCH] D68117: [DWARF-5] Support for C++11 defaulted, deleted member functions.

2019-10-13 Thread Sourabh Singh Tomar via Phabricator via cfe-commits

SouraVX added a comment.

In D68117#1702595 , @probinson wrote:

> We really do want to pack the four mutually exclusive cases into two bits.  I 
> have tried to give more explicit comments inline to explain how you would do 
> this.  It really should work fine, recognizing that the "not defaulted" case 
> is not explicitly represented in the textual IR because it uses a zero value 
> in the defaulted/deleted subfield of SPFlags.

Thanks Paul, for suggesting this. Your approach works fine. But as I was 
working on some lvm-dwarfdump test cases. We seems to miss one corner case --
Consider this test case;
class foo{

  foo() = default;
  ~foo() = default;
   void not_special() {}

};
void not_a_member_of_foo(){}

Now I'm getting DW_AT_defaulted getting emitted with value DW_DEFAULTED_no, for 
functions "not_special" and "not_a_member_of_foo". This behavior is undesirable 
since, DW_AT_defaulted attributes is only valid for C++ special member 
functions{Constructors/Destructors, ...}.

Please correct me if I'm wrong -- Now This attributes to- implicitly defined 
"0" NotDefaulted bit.  which is getting checked{that's fine as long as we have 
a dedicated bits for distinguishing} and true for every subprogram or function 
in a CU.
void DwarfUnit::applySubprogramAttributes( ...
...
else if (SP->isNotDefaulted())

  addUInt(SPDie, dwarf::DW_AT_defaulted, dwarf::DW_FORM_data1,
  dwarf::DW_DEFAULTED_no);

...

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68117/new/

https://reviews.llvm.org/D68117

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

[PATCH] D41217: [Concepts] Concept Specialization Expressions

2019-10-13 Thread Saar Raz via Phabricator via cfe-commits

saar.raz marked 14 inline comments as done and 2 inline comments as done.
saar.raz added inline comments.



Comment at: lib/Sema/SemaConcept.cpp:51-68
+  if (auto *BO = dyn_cast(ConstraintExpr)) {
+if (BO->getOpcode() == BO_LAnd) {
+  if (CalculateConstraintSatisfaction(NamedConcept, MLTAL, BO->getLHS(),
+  IsSatisfied))
+return true;
+  if (!IsSatisfied)
+return false;

rsmith wrote:
> If an `operator&&` or `operator||` function is declared, this could be a 
> `CXXOperatorCallExpr` instead. You will need to recurse into those constructs 
> too.
Atomic constraints will have bool types anyway, can this really happen?



Comment at: lib/Sema/SemaConcept.cpp:72-74
+  else if (auto *C = dyn_cast(ConstraintExpr))
+return CalculateConstraintSatisfaction(NamedConcept, MLTAL, 
C->getSubExpr(),
+   IsSatisfied);

rsmith wrote:
> This case is not handled by `CheckConstraintExpression`; we should be 
> consistent in whether we step over these or not.
> 
> Perhaps it would make sense to factor out a mechanism to take an expression 
> and classify it as atomic constraint (with the corresponding expression), or 
> conjunction (with a pair of subexpressions), or disjunction (with a pair of 
> subexpressions), and use that everywhere we traverse a constraint expression.
Since this part will be changed in later patches, I'd like to delay this fix to 
a later patch for now.



Comment at: lib/Sema/SemaTemplateInstantiate.cpp:679-681
+  Diags.Report(Active->PointOfInstantiation,
+   diag::note_constraint_substitution_here)
+  << Active->InstantiationRange;

saar.raz wrote:
> saar.raz wrote:
> > rsmith wrote:
> > > Is this note ever useful? It will presumably always point into the same 
> > > concept definition that the prior diagnostic also pointed at, and doesn't 
> > > seem to add anything in the testcases.
> > > 
> > > Maybe we could keep the CodeSynthesisContext around as a marker that 
> > > we've entered a SFINAE context, but not have any corresponding 
> > > diagnostic. (The note produced for the enclosing `ConstraintsCheck` 
> > > context covers that.) Or we could remove this and store a flag on the 
> > > `ConstraintsCheck` to indicate whether we're in a SFINAEable portion of 
> > > it.
> > If the concept definition is multiline/contains macros, this would point at 
> > the exact place where the problematic constraint occured, we should 
> > probably add tests for this case though.
> > Maybe we can omit the diagnostic when the concept and the constraint are on 
> > the same line or something?
> > 
> Correction - I just now realized you were referring to the 'in instantiation 
> of template class' note and not the 'while checking the satisfaction...' note.
> In this case - the only case I can think of where the problematic 
> instantiation and the constraint expression would be in very different places 
> is indeed multiline constraint expressions or macros in a constraint 
> expression (but you know better than I do - can you think of any other cases? 
> or maybe there are other non-SFINAE errors that can result from substitution 
> and would not produce a note?). Maybe these cases are remote enough to 
> warrant removing this diagnostic or as I said earlier omit them in cases 
> where they are unhelpful.
In any case, I'm in favor of pushing this to a separate patch (after we have 
diagnostics for requires exprs, which would suffer from a similar problem)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D41217/new/

https://reviews.llvm.org/D41217



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

[PATCH] D41217: [Concepts] Concept Specialization Expressions

2019-10-13 Thread Saar Raz via Phabricator via cfe-commits

saar.raz updated this revision to Diff 224789.
saar.raz marked 2 inline comments as done.
saar.raz added a comment.

Address CR comments by rsmith


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D41217/new/

https://reviews.llvm.org/D41217

Files:
  clang/include/clang/AST/ExprCXX.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/StmtNodes.td
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/lib/AST/Expr.cpp
  clang/lib/AST/ExprCXX.cpp
  clang/lib/AST/ExprClassification.cpp
  clang/lib/AST/ExprConstant.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/StmtPrinter.cpp
  clang/lib/AST/StmtProfile.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/Frontend/FrontendActions.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Sema/CMakeLists.txt
  clang/lib/Sema/SemaConcept.cpp
  clang/lib/Sema/SemaExceptionSpec.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  clang/test/CXX/concepts-ts/expr/expr.prim/expr.prim.id/p3.cpp
  clang/test/Parser/cxx2a-concept-declaration.cpp
  clang/tools/libclang/CXCursor.cpp

Index: clang/tools/libclang/CXCursor.cpp
===
--- clang/tools/libclang/CXCursor.cpp
+++ clang/tools/libclang/CXCursor.cpp
@@ -256,6 +256,7 @@
   case Stmt::BinaryConditionalOperatorClass:
   case Stmt::TypeTraitExprClass:
   case Stmt::CoawaitExprClass:
+  case Stmt::ConceptSpecializationExprClass:
   case Stmt::DependentCoawaitExprClass:
   case Stmt::CoyieldExprClass:
   case Stmt::CXXBindTemporaryExprClass:
Index: clang/test/Parser/cxx2a-concept-declaration.cpp
===
--- clang/test/Parser/cxx2a-concept-declaration.cpp
+++ clang/test/Parser/cxx2a-concept-declaration.cpp
@@ -14,8 +14,6 @@
 // expected-error@-2{{template template parameter requires 'class' after the parameter list}}
 // expected-error@-3{{concept template parameter list must have at least one parameter; explicit specialization of concepts is not allowed}}
 
-template concept C2 = 0.f; // expected-error {{constraint expression must be of type 'bool' but is of type 'float'}}
-
 struct S1 {
   template concept C1 = true; // expected-error {{concept declarations may only appear in global or namespace scope}}
 };
@@ -26,15 +24,15 @@
 
 template
 template
-concept C4 = true; // expected-error {{extraneous template parameter list in concept definition}}
+concept C2 = true; // expected-error {{extraneous template parameter list in concept definition}}
 
-template concept C5 = true; // expected-note {{previous}} expected-note {{previous}}
-int C5; // expected-error {{redefinition}}
-struct C5 {}; // expected-error {{redefinition}}
+template concept C3 = true; // expected-note {{previous}} expected-note {{previous}}
+int C3; // expected-error {{redefinition}}
+struct C3 {}; // expected-error {{redefinition}}
 
-struct C6 {}; // expected-note{{previous definition is here}}
-template concept C6 = true;
-// expected-error@-1{{redefinition of 'C6' as different kind of symbol}}
+struct C4 {}; // expected-note{{previous definition is here}}
+template concept C4 = true;
+// expected-error@-1{{redefinition of 'C4' as different kind of symbol}}
 
 // TODO: Add test to prevent explicit specialization, partial specialization
 // and explicit instantiation of concepts.
@@ -43,31 +41,60 @@
 struct integral_constant { static constexpr T value = v; };
 
 namespace N {
-  template concept C7 = true;
+  template concept C5 = true;
 }
-using N::C7;
+using N::C5;
 
-template  concept C8 = integral_constant::value;
+template  concept C6 = integral_constant::value;
 // expected-error@-1{{use of undeclared identifier 'wor'; did you mean 'word'?}}
 // expected-note@-2{{'word' declared here}}
 
-template concept bool C9 = true;
+template concept bool C7 = true;
 // expected-warning@-1{{ISO C++2a does not permit the 'bool' keyword after 'concept'}}
 
-template<> concept C10 = false;
+template<> concept C8 = false;
 // expected-error@-1{{concept template parameter list must have at least one parameter; explicit specialization of concepts is not allowed}}
 
-template<> concept C9 = false;
+template<> concept C7 = false;
 // expected-error@-1{{name defined in concept definition must be an identifier}}
 
-template concept N::C11 = false;
+template concept N::C9 = false;
 // expected-error@-1{{name defined in concept definition must be an identifier}}
 
 class A { };
 // expected-note@-1{{'A' declared here}}
 
-template concept A::C12 = false;
+template concept A::C10 = false;
 // expected-error@-1{{expected namespace name}}
 
 template concept operator int = false;
 // expected-error@-1{{name

[clang-tools-extra] r374730 - Add missing "REQUIRES: shell" to system-include-extractor.test

2019-10-13 Thread Nico Weber via cfe-commits

Author: nico
Date: Sun Oct 13 10:43:16 2019
New Revision: 374730

URL: http://llvm.org/viewvc/llvm-project?rev=374730=rev
Log:
Add missing "REQUIRES: shell" to system-include-extractor.test

Part of PR43592.

Modified:
clang-tools-extra/trunk/clangd/test/system-include-extractor.test

Modified: clang-tools-extra/trunk/clangd/test/system-include-extractor.test
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/test/system-include-extractor.test?rev=374730=374729=374730=diff
==
--- clang-tools-extra/trunk/clangd/test/system-include-extractor.test (original)
+++ clang-tools-extra/trunk/clangd/test/system-include-extractor.test Sun Oct 
13 10:43:16 2019
@@ -1,5 +1,8 @@
 # RUN: rm -rf %t.dir && mkdir -p %t.dir
 
+# The mock driver below is a shell script:
+# REQUIRES: shell
+
 # Generate a mock-driver that will print %temp_dir%/my/dir and
 # %temp_dir%/my/dir2 as include search paths.
 # RUN: echo '#!/bin/bash' >> %t.dir/my_driver.sh


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

[PATCH] D66485: [Clang][Bundler] Use llvm-objcopy for creating fat object files

2019-10-13 Thread Michał Górny via Phabricator via cfe-commits

mgorny added inline comments.



Comment at: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp:467
+ErrorOr Objcopy = sys::findProgramByName(
+"llvm-objcopy", sys::path::parent_path(BundlerExecutable));
+if (!Objcopy) {

This is broken for running tests in clang standalone builds. `llvm-copy` is 
installed in the system, unlike `clang-offload-bundler` that is present in 
build directory. Not to mention it's simply wrong to assume both executables 
must be in the same directory.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66485/new/

https://reviews.llvm.org/D66485



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

[clang-tools-extra] r374727 - Make the last to clangd unit tests pass on Windows.

2019-10-13 Thread Nico Weber via cfe-commits

Author: nico
Date: Sun Oct 13 10:19:00 2019
New Revision: 374727

URL: http://llvm.org/viewvc/llvm-project?rev=374727=rev
Log:
Make the last to clangd unit tests pass on Windows.

(Some lit tests still fail.)

See FIXME in diff for details.

Part of PR43592.

Modified:
clang-tools-extra/trunk/clangd/unittests/DiagnosticsTests.cpp

Modified: clang-tools-extra/trunk/clangd/unittests/DiagnosticsTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/DiagnosticsTests.cpp?rev=374727=374726=374727=diff
==
--- clang-tools-extra/trunk/clangd/unittests/DiagnosticsTests.cpp (original)
+++ clang-tools-extra/trunk/clangd/unittests/DiagnosticsTests.cpp Sun Oct 13 
10:19:00 2019
@@ -721,7 +721,10 @@ $insert[[]]namespace ns {
 }
 void g() {  ns::$[[scope]]::X_Y();  }
   )cpp");
-  auto TU = TestTU::withCode(Test.code());
+  TestTU TU;
+  TU.Code = Test.code();
+  // FIXME: Figure out why this is needed and remove it, PR43662.
+  TU.ExtraArgs.push_back("-fno-ms-compatibility");
   auto Index = buildIndexWithSymbol(
   SymbolWithHeader{"ns::scope::X_Y", "unittest:///x.h", "\"x.h\""});
   TU.ExternalIndex = Index.get();
@@ -744,7 +747,10 @@ void f() {
 }
 }
   )cpp");
-  auto TU = TestTU::withCode(Test.code());
+  TestTU TU;
+  TU.Code = Test.code();
+  // FIXME: Figure out why this is needed and remove it, PR43662.
+  TU.ExtraArgs.push_back("-fno-ms-compatibility");
   auto Index = buildIndexWithSymbol(
   {SymbolWithHeader{"clang::clangd::X", "unittest:///x.h", "\"x.h\""},
SymbolWithHeader{"clang::clangd::ns::Y", "unittest:///y.h", 
"\"y.h\""}});


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

[PATCH] D68554: [clang-format] Proposal for clang-format to give compiler style warnings

2019-10-13 Thread Nico Weber via Phabricator via cfe-commits

thakis added a comment.

>   Could you or anyone else point me to a good example, I'd definitely like to 
> take a look.

http://llvm-cs.pcc.me.uk/include/llvm/Support/SourceMgr.h/rSMDiagnostic --

>   `via clang-format?` did you mean via clang.exe or clang-check.exe?   I did 
> think of this when I started doing this work, I just came to the conclusion 
> that if I did that it wouldn't be long before someone requested the same 
> features in clang-format itself.

Err, via clang-tidy. Sorry, that was a super confusing typo. That seems like a 
great place for this feature to me.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68554/new/

https://reviews.llvm.org/D68554



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

[PATCH] D68554: [clang-format] Proposal for clang-format to give compiler style warnings

2019-10-13 Thread MyDeveloperDay via Phabricator via cfe-commits

MyDeveloperDay added a comment.

In D68554#1707537 , @thakis wrote:

> > Thanks for doing this, I am struggling to find the MacOS bot log that 
> > failed, are any available via Buildbot? (I notice the log looks like your 
> > own machine)
>
> The mac bots are on a different system for some reason: 
> http://green.lab.llvm.org/green/ (see also 
> http://lists.llvm.org/pipermail/llvm-dev/2019-October/135771.html , I wasn't 
> aware of that until a few days ago either).
>
> > So as you said it will be a little slower to build clang-format, however, I 
> > do notice that all the other clang/tools are pretty much are all building 
> > with Frontend. so anyone building a larger range of clang tools probably 
> > has everything already built.
>
> I believe all the other clang tools conceptually do semantic analysis, so 
> that makes more sense for them :)




  Agreed..

> 
> 
>> I think the main build failure was just the lit tests, I think i would 
>> prefer to fix those and reland this as-is, then look for a better diags 
>> solution if @klimek thinks this is something I should be concerned about. I 
>> really wanted to be able to use a standard Diagnostic front end with all the 
>> support for console coloring etc...I'll have to take a look at LLVM's 
>> diagnostics to see how that is done.
> 
> I hope my view on this isn't entirely dismissed (see `git shortlog -nes 
> clang/lib/Format/`).



  Actually I hope not too, I thought this Frontend mechanism was a bit overkill 
too, but I held back from just doing `llvm::errs() << ..." because I felt that 
was reinventing the wheel (but it would probably be more space-efficient) 
especially in terms of dependencies

> In addition to the actual size cost of this change: about a third of 
> clang-format's size is the diagnostics table, but before this change here 
> it's kept alive by only two references from lib/Basic and it's reasonably 
> easy to remove these, which should make the binary 1 MB smaller. After this 
> change,

that's much harder to do.

  Yes I get that, if I can switch to LLVM diags then I guess can avoid that.  I 
also wondered if there was some way of me doing this without the 
`TextDiagnosticPrinter` (which is the only thing that caused Frontend to need 
to come in, the rest of the Diagnositcs stuff 
(DiagnosticOptions,DiagnosticEngine etc..) was already being used.

> LLVM's text diag stuff also supports colors and whatnot.



  Could you or anyone else point me to a good example, I'd definitely like to 
take a look.

> And another idea: maybe it makes sense to expose these clang-format 
> diagnostics you're adding (which I do think are a cool feature!) via 
> clang-format? That already depends on everything and the kitchen sync, so you 
> could easily output diagnostics from there – and it'd allow clang-format 
> itself to be a tool focused on doing one thing well (namely, formatting code).



  `via clang-format?` did you mean via clang.exe or clang-check.exe?   I did 
think of this when I started doing this work, I just came to the conclusion 
that if I did that it wouldn't be long before someone requested the same 
features in clang-format itself.
  
  I think from what you've said, trying to break the Frontend dependency is 
probably important, as for a dependency on lib/Basic I feel I was just asked to 
make that worse {D68914} perhaps that's why the code was duplicated in the 
first place...
  
  bear with me I'm still learning all this stuff.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68554/new/

https://reviews.llvm.org/D68554



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

[PATCH] D66795: [Mips] Use appropriate private label prefix based on Mips ABI

2019-10-13 Thread Mirko Brkusanin via Phabricator via cfe-commits

mbrkusanin updated this revision to Diff 224603.
mbrkusanin added a comment.

- Rebase
- Ping

@echristo @craig.topper @tstellar @dylanmckay @petecoup
If there are no objections then I'll split this into llvm, clang and lldb 
patches and commit them next week.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66795/new/

https://reviews.llvm.org/D66795

Files:
  clang/lib/Parse/ParseStmtAsm.cpp
  clang/tools/driver/cc1as_main.cpp
  lldb/source/Plugins/Disassembler/llvm/DisassemblerLLVMC.cpp
  lldb/source/Plugins/Instruction/MIPS/EmulateInstructionMIPS.cpp
  lldb/source/Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.cpp
  llvm/include/llvm/Support/TargetRegistry.h
  llvm/lib/CodeGen/LLVMTargetMachine.cpp
  llvm/lib/MC/MCDisassembler/Disassembler.cpp
  llvm/lib/Object/ModuleSymbolTable.cpp
  llvm/lib/Target/AArch64/MCTargetDesc/AArch64MCTargetDesc.cpp
  llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCAsmInfo.cpp
  llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCAsmInfo.h
  llvm/lib/Target/ARC/MCTargetDesc/ARCMCTargetDesc.cpp
  llvm/lib/Target/ARM/MCTargetDesc/ARMMCTargetDesc.cpp
  llvm/lib/Target/AVR/MCTargetDesc/AVRMCAsmInfo.cpp
  llvm/lib/Target/AVR/MCTargetDesc/AVRMCAsmInfo.h
  llvm/lib/Target/BPF/MCTargetDesc/BPFMCAsmInfo.h
  llvm/lib/Target/Hexagon/MCTargetDesc/HexagonMCTargetDesc.cpp
  llvm/lib/Target/Lanai/MCTargetDesc/LanaiMCAsmInfo.cpp
  llvm/lib/Target/Lanai/MCTargetDesc/LanaiMCAsmInfo.h
  llvm/lib/Target/MSP430/MCTargetDesc/MSP430MCAsmInfo.cpp
  llvm/lib/Target/MSP430/MCTargetDesc/MSP430MCAsmInfo.h
  llvm/lib/Target/Mips/MCTargetDesc/MipsMCAsmInfo.cpp
  llvm/lib/Target/Mips/MCTargetDesc/MipsMCAsmInfo.h
  llvm/lib/Target/Mips/MCTargetDesc/MipsMCTargetDesc.cpp
  llvm/lib/Target/NVPTX/MCTargetDesc/NVPTXMCAsmInfo.cpp
  llvm/lib/Target/NVPTX/MCTargetDesc/NVPTXMCAsmInfo.h
  llvm/lib/Target/PowerPC/MCTargetDesc/PPCMCTargetDesc.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCTargetDesc.cpp
  llvm/lib/Target/Sparc/MCTargetDesc/SparcMCTargetDesc.cpp
  llvm/lib/Target/SystemZ/MCTargetDesc/SystemZMCTargetDesc.cpp
  llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCAsmInfo.cpp
  llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCAsmInfo.h
  llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCTargetDesc.cpp
  llvm/lib/Target/X86/MCTargetDesc/X86MCTargetDesc.cpp
  llvm/lib/Target/XCore/MCTargetDesc/XCoreMCTargetDesc.cpp
  llvm/test/CodeGen/Mips/compactbranches/no-beqzc-bnezc.ll
  llvm/test/MC/Mips/macro-li.d.s
  llvm/test/MC/Mips/macro-li.s.s
  llvm/test/MC/Mips/private-prefix.s
  llvm/tools/dsymutil/DwarfStreamer.cpp
  llvm/tools/llvm-cfi-verify/lib/FileAnalysis.cpp
  llvm/tools/llvm-dwp/llvm-dwp.cpp
  llvm/tools/llvm-exegesis/lib/Analysis.cpp
  llvm/tools/llvm-jitlink/llvm-jitlink.cpp
  llvm/tools/llvm-mc-assemble-fuzzer/llvm-mc-assemble-fuzzer.cpp
  llvm/tools/llvm-mc/Disassembler.cpp
  llvm/tools/llvm-mc/Disassembler.h
  llvm/tools/llvm-mc/llvm-mc.cpp
  llvm/tools/llvm-mca/llvm-mca.cpp
  llvm/tools/llvm-objdump/MachODump.cpp
  llvm/tools/llvm-objdump/llvm-objdump.cpp
  llvm/tools/llvm-rtdyld/llvm-rtdyld.cpp
  llvm/tools/sancov/sancov.cpp
  llvm/unittests/DebugInfo/DWARF/DwarfGenerator.cpp
  llvm/unittests/ExecutionEngine/JITLink/JITLinkTestCommon.cpp
  llvm/unittests/MC/DwarfLineTables.cpp
  llvm/unittests/MC/MCInstPrinter.cpp

Index: llvm/unittests/MC/MCInstPrinter.cpp
===
--- llvm/unittests/MC/MCInstPrinter.cpp
+++ llvm/unittests/MC/MCInstPrinter.cpp
@@ -9,6 +9,7 @@
 #include "llvm/MC/MCInstPrinter.h"
 #include "llvm/MC/MCAsmInfo.h"
 #include "llvm/MC/MCInstrInfo.h"
+#include "llvm/MC/MCTargetOptions.h"
 #include "llvm/Support/TargetRegistry.h"
 #include "llvm/Support/TargetSelect.h"
 #include "llvm/Target/TargetMachine.h"
@@ -40,7 +41,8 @@
   return;
 
 MRI.reset(TheTarget->createMCRegInfo(TripleName));
-MAI.reset(TheTarget->createMCAsmInfo(*MRI, TripleName));
+MCTargetOptions MCOptions;
+MAI.reset(TheTarget->createMCAsmInfo(*MRI, TripleName, MCOptions));
 MII.reset(TheTarget->createMCInstrInfo());
 Printer.reset(TheTarget->createMCInstPrinter(
 Triple(TripleName), MAI->getAssemblerDialect(), *MAI, *MII, *MRI));
Index: llvm/unittests/MC/DwarfLineTables.cpp
===
--- llvm/unittests/MC/DwarfLineTables.cpp
+++ llvm/unittests/MC/DwarfLineTables.cpp
@@ -12,6 +12,7 @@
 #include "llvm/MC/MCContext.h"
 #include "llvm/MC/MCDwarf.h"
 #include "llvm/MC/MCRegisterInfo.h"
+#include "llvm/MC/MCTargetOptions.h"
 #include "llvm/Support/TargetRegistry.h"
 #include "llvm/Support/TargetSelect.h"
 #include "gtest/gtest.h"
@@ -37,7 +38,8 @@
   return;
 
 MRI.reset(TheTarget->createMCRegInfo(Triple));
-MAI.reset(TheTarget->createMCAsmInfo(*MRI, Triple));
+MCTargetOptions MCOptions;
+MAI.reset(TheTarget->createMCAsmInfo(*MRI, Triple, MCOptions));
 Ctx = std::make_unique(MAI.get(), MRI.get(), nullptr);
   }
 
Index:

r374366 - In openFileForRead don't cache erroneous entries if the error relates to them being directories. Add tests.

2019-10-13 Thread Kousik Kumar via cfe-commits

Author: kousikk
Date: Thu Oct 10 08:29:01 2019
New Revision: 374366

URL: http://llvm.org/viewvc/llvm-project?rev=374366=rev
Log:
In openFileForRead don't cache erroneous entries if the error relates to them 
being directories. Add tests.

Summary:
It seems that when the CachingFileSystem is first given a file to open that is 
actually a directory, it incorrectly
caches that path to be errenous and throws an error when subsequently a 
directory open call is made for the same
path.
This change makes it so that we do NOT cache a path if it turns out we asked 
for a file when its a directory.

Reviewers: arphaman

Subscribers: dexonsmith, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D68193

Added:
cfe/trunk/test/ClangScanDeps/Inputs/headerwithdirnamefollowedbyinclude.json
cfe/trunk/test/ClangScanDeps/headerwithdirnamefollowedbyinclude.cpp
Modified:

cfe/trunk/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
cfe/trunk/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp

Modified: 
cfe/trunk/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h?rev=374366=374365=374366=diff
==
--- 
cfe/trunk/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
 (original)
+++ 
cfe/trunk/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
 Thu Oct 10 08:29:01 2019
@@ -168,6 +168,9 @@ private:
 return It == Cache.end() ? nullptr : It->getValue();
   }
 
+  llvm::ErrorOr
+  getOrCreateFileSystemEntry(const StringRef Filename);
+
   DependencyScanningFilesystemSharedCache 
   /// The local cache is used by the worker thread to cache file system queries
   /// locally instead of querying the global cache every time.

Modified: 
cfe/trunk/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp?rev=374366=374365=374366=diff
==
--- cfe/trunk/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp 
(original)
+++ cfe/trunk/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp 
Thu Oct 10 08:29:01 2019
@@ -122,14 +122,11 @@ DependencyScanningFilesystemSharedCache:
   return It.first->getValue();
 }
 
-llvm::ErrorOr
-DependencyScanningWorkerFilesystem::status(const Twine ) {
-  SmallString<256> OwnedFilename;
-  StringRef Filename = Path.toStringRef(OwnedFilename);
-
-  // Check the local cache first.
-  if (const CachedFileSystemEntry *Entry = getCachedEntry(Filename))
-return Entry->getStatus();
+llvm::ErrorOr
+DependencyScanningWorkerFilesystem::getOrCreateFileSystemEntry(const StringRef 
Filename) {
+  if (const CachedFileSystemEntry* Entry = getCachedEntry(Filename)) {
+return Entry;
+  }
 
   // FIXME: Handle PCM/PCH files.
   // FIXME: Handle module map files.
@@ -160,7 +157,17 @@ DependencyScanningWorkerFilesystem::stat
 
   // Store the result in the local cache.
   setCachedEntry(Filename, Result);
-  return Result->getStatus();
+  return Result;
+}
+
+llvm::ErrorOr
+DependencyScanningWorkerFilesystem::status(const Twine ) {
+  SmallString<256> OwnedFilename;
+  StringRef Filename = Path.toStringRef(OwnedFilename);
+  const llvm::ErrorOr Result = 
getOrCreateFileSystemEntry(Filename);
+  if (!Result)
+return Result.getError();
+  return (*Result)->getStatus();
 }
 
 namespace {
@@ -217,30 +224,8 @@ DependencyScanningWorkerFilesystem::open
   SmallString<256> OwnedFilename;
   StringRef Filename = Path.toStringRef(OwnedFilename);
 
-  // Check the local cache first.
-  if (const CachedFileSystemEntry *Entry = getCachedEntry(Filename))
-return createFile(Entry, PPSkipMappings);
-
-  // FIXME: Handle PCM/PCH files.
-  // FIXME: Handle module map files.
-
-  bool KeepOriginalSource = IgnoredFiles.count(Filename);
-  DependencyScanningFilesystemSharedCache::SharedFileSystemEntry
-   = SharedCache.get(Filename);
-  const CachedFileSystemEntry *Result;
-  {
-std::unique_lock LockGuard(SharedCacheEntry.ValueLock);
-CachedFileSystemEntry  = SharedCacheEntry.Value;
-
-if (!CacheEntry.isValid()) {
-  CacheEntry = CachedFileSystemEntry::createFileEntry(
-  Filename, getUnderlyingFS(), !KeepOriginalSource);
-}
-
-Result = 
-  }
-
-  // Store the result in the local cache.
-  setCachedEntry(Filename, Result);
-  return createFile(Result, PPSkipMappings);
+  const llvm::ErrorOr Result = 
getOrCreateFileSystemEntry(Filename);
+  if (!Result)
+return Result.getError();
+  return createFile(Result.get(), PPSkipMappings);
 }

Added: 
cfe/trunk/test/ClangScanDeps/Inputs/headerwithdirnamefollowedbyinclude.json
URL:

[PATCH] D68554: [clang-format] Proposal for clang-format to give compiler style warnings

2019-10-13 Thread Nico Weber via Phabricator via cfe-commits

thakis added a comment.

> Thanks for doing this, I am struggling to find the MacOS bot log that failed, 
> are any available via Buildbot? (I notice the log looks like your own machine)

The mac bots are on a different system for some reason: 
http://green.lab.llvm.org/green/ (see also 
http://lists.llvm.org/pipermail/llvm-dev/2019-October/135771.html , I wasn't 
aware of that until a few days ago either).

> So as you said it will be a little slower to build clang-format, however, I 
> do notice that all the other clang/tools are pretty much are all building 
> with Frontend. so anyone building a larger range of clang tools probably has 
> everything already built.

I believe all the other clang tools conceptually do semantic analysis, so that 
makes more sense for them :)

> I think the main build failure was just the lit tests, I think i would prefer 
> to fix those and reland this as-is, then look for a better diags solution if 
> @klimek thinks this is something I should be concerned about. I really wanted 
> to be able to use a standard Diagnostic front end with all the support for 
> console coloring etc...I'll have to take a look at LLVM's diagnostics to see 
> how that is done.

I hope my view on this isn't entirely dismissed (see `git shortlog -nes 
clang/lib/Format/`).

In addition to the actual size cost of this change: about a third of 
clang-format's size is the diagnostics table, but before this change here it's 
kept alive by only two references from lib/Basic and it's reasonably easy to 
remove these, which should make the binary 1 MB smaller. After this change, 
that's much harder to do.

LLVM's text diag stuff also supports colors and whatnot.

And another idea: maybe it makes sense to expose these clang-format diagnostics 
you're adding (which I do think are a cool feature!) via clang-format? That 
already depends on everything and the kitchen sync, so you could easily output 
diagnostics from there – and it'd allow clang-format itself to be a tool 
focused on doing one thing well (namely, formatting code).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68554/new/

https://reviews.llvm.org/D68554



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

[PATCH] D67185: [RISCV] Add support for -ffixed-xX flags

2019-10-13 Thread Luís Marques via Phabricator via cfe-commits

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

Overall LGTM. Caveats:

- Address the issues in the inline comments;
- Shouldn't the TLS lowering also complain when `-ffixed-x4` is used?
- Is there a way to ensure we don't forget to check any such reserved reg uses? 
I'm not quite confident we haven't overlooked anything.
- (Remember to check for the `-ffixed-xX` flags when implementing the 
callee-saved regs via libcalls (D62686 ), etc.)

Apologies for the delayed review.




Comment at: clang/include/clang/Driver/Options.td:2224
   HelpText<"Don't workaround Cortex-A53 erratum 835769 (AArch64 only)">;
-foreach i = {1-7,9-15,18,20-28} in
-  def ffixed_x#i : Flag<["-"], "ffixed-x"#i>, Group,
-HelpText<"Reserve the "#i#" register (AArch64 only)">;
+foreach i = {1-31} in
+  def ffixed_x#i : Flag<["-"], "ffixed-x"#i>, Group,

Given the expansion of the flags here, the AArch64 driver should probably 
detect and reject the flags `-ffixed-x[8,16-17,19,29-31]`, to preserve the old 
behavior where passing those flags  would be an error and to ensure that 
erroneous flags are not silently accepted.



Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:2412
+  }))
+F.getContext().diagnose(DiagnosticInfoUnsupported{F, "Argument register"
+  " required, but has been reserved."});

clang-format indicates another formatting style here.



Comment at: llvm/lib/Target/RISCV/RISCVSubtarget.cpp:53
 : RISCVGenSubtargetInfo(TT, CPU, FS),
+  UserReservedRegister(RISCV::NUM_TARGET_REGS),
   FrameLowering(initializeSubtargetDependencies(TT, CPU, FS, ABIName)),

This includes more than the x0 - x31 registers. If the intent is to only allow 
reserving the GPRs then this should be tightened.



Comment at: llvm/lib/Target/RISCV/RISCVSubtarget.h:98
+  bool isRegisterReservedByUser(size_t i) const {
+return UserReservedRegister[i];
+  }

Consider adding a bounds checking assert.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67185/new/

https://reviews.llvm.org/D67185



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

r374720 - [clang-format] Proposal for clang-format to give compiler style warnings

2019-10-13 Thread Paul Hoad via cfe-commits

Author: paulhoad
Date: Sun Oct 13 07:51:45 2019
New Revision: 374720

URL: http://llvm.org/viewvc/llvm-project?rev=374720=rev
Log:
[clang-format] Proposal for clang-format to give compiler style warnings

relanding {D68554} with fixed lit tests, checked on Windows and MacOS

Added:
cfe/trunk/test/Format/dry-run-alias.cpp
cfe/trunk/test/Format/dry-run.cpp
Modified:
cfe/trunk/tools/clang-format/CMakeLists.txt
cfe/trunk/tools/clang-format/ClangFormat.cpp

Added: cfe/trunk/test/Format/dry-run-alias.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Format/dry-run-alias.cpp?rev=374720=auto
==
--- cfe/trunk/test/Format/dry-run-alias.cpp (added)
+++ cfe/trunk/test/Format/dry-run-alias.cpp Sun Oct 13 07:51:45 2019
@@ -0,0 +1,4 @@
+// RUN: clang-format -style=LLVM -i -n %s 2> %t.stderr
+// RUN: grep -E "(.*)code should be clang-formatted(.*)" %t.stderr
+
+int a ;

Added: cfe/trunk/test/Format/dry-run.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Format/dry-run.cpp?rev=374720=auto
==
--- cfe/trunk/test/Format/dry-run.cpp (added)
+++ cfe/trunk/test/Format/dry-run.cpp Sun Oct 13 07:51:45 2019
@@ -0,0 +1,4 @@
+// RUN: clang-format -style=LLVM -i --dry-run %s 2> %t.stderr
+// RUN: grep -E "(.*)code should be clang-formatted(.*)" %t.stderr
+
+int a ;

Modified: cfe/trunk/tools/clang-format/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/clang-format/CMakeLists.txt?rev=374720=374719=374720=diff
==
--- cfe/trunk/tools/clang-format/CMakeLists.txt (original)
+++ cfe/trunk/tools/clang-format/CMakeLists.txt Sun Oct 13 07:51:45 2019
@@ -7,6 +7,7 @@ add_clang_tool(clang-format
 set(CLANG_FORMAT_LIB_DEPS
   clangBasic
   clangFormat
+  clangFrontend
   clangRewrite
   clangToolingCore
   )

Modified: cfe/trunk/tools/clang-format/ClangFormat.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/clang-format/ClangFormat.cpp?rev=374720=374719=374720=diff
==
--- cfe/trunk/tools/clang-format/ClangFormat.cpp (original)
+++ cfe/trunk/tools/clang-format/ClangFormat.cpp Sun Oct 13 07:51:45 2019
@@ -18,6 +18,7 @@
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/Version.h"
 #include "clang/Format/Format.h"
+#include "clang/Frontend/TextDiagnosticPrinter.h"
 #include "clang/Rewrite/Core/Rewriter.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/FileSystem.h"
@@ -108,6 +109,54 @@ static cl::opt
 Verbose("verbose", cl::desc("If set, shows the list of processed files"),
 cl::cat(ClangFormatCategory));
 
+// Use --dry-run to match other LLVM tools when you mean do it but don't
+// actually do it
+static cl::opt
+DryRun("dry-run",
+   cl::desc("If set, do not actually make the formatting changes"),
+   cl::cat(ClangFormatCategory));
+
+// Use -n as a common command as an alias for --dry-run. (git and make use -n)
+static cl::alias DryRunShort("n", cl::desc("Alias for --dry-run"),
+ cl::cat(ClangFormatCategory), 
cl::aliasopt(DryRun),
+ cl::NotHidden);
+
+// Emulate being able to turn on/off the warning.
+static cl::opt
+WarnFormat("Wclang-format-violations",
+   cl::desc("Warnings about individual formatting changes needed. "
+"Used only with --dry-run or -n"),
+   cl::init(true), cl::cat(ClangFormatCategory), cl::Hidden);
+
+static cl::opt
+NoWarnFormat("Wno-clang-format-violations",
+ cl::desc("Do not warn about individual formatting changes "
+  "needed. Used only with --dry-run or -n"),
+ cl::init(false), cl::cat(ClangFormatCategory), cl::Hidden);
+
+static cl::opt ErrorLimit(
+"ferror-limit",
+cl::desc("Set the maximum number of clang-format errors to emit before "
+ "stopping (0 = no limit). Used only with --dry-run or -n"),
+cl::init(0), cl::cat(ClangFormatCategory));
+
+static cl::opt
+WarningsAsErrors("Werror",
+ cl::desc("If set, changes formatting warnings to errors"),
+ cl::cat(ClangFormatCategory));
+
+static cl::opt
+ShowColors("fcolor-diagnostics",
+   cl::desc("If set, and on a color-capable terminal controls "
+"whether or not to print diagnostics in color"),
+   cl::init(true), cl::cat(ClangFormatCategory), cl::Hidden);
+
+static cl::opt
+NoShowColors("fno-color-diagnostics",
+ cl::desc("If set, and on a color-capable terminal controls "
+  "whether or not to print diagnostics in color"),
+ cl::init(false), cl::cat(ClangFormatCategory), cl::Hidden);
+

[PATCH] D68539: [clang-tidy] fix for readability-identifier-naming incorrectly fixes variables which become keywords

2019-10-13 Thread Daniel via Phabricator via cfe-commits

Daniel599 updated this revision to Diff 224783.
Daniel599 added a comment.

Added check for macro definition, please re-review the enum `ShouldFixStatus` 
as now I use it as a switch-case within the diagnostic message.
Thanks.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68539/new/

https://reviews.llvm.org/D68539

Files:
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
  
clang-tools-extra/test/clang-tidy/readability-identifier-naming-bugfix-name-conflicts.cpp
  clang/include/clang/Basic/IdentifierTable.h

Index: clang/include/clang/Basic/IdentifierTable.h
===
--- clang/include/clang/Basic/IdentifierTable.h
+++ clang/include/clang/Basic/IdentifierTable.h
@@ -581,6 +581,8 @@
   iterator end() const   { return HashTable.end(); }
   unsigned size() const  { return HashTable.size(); }
 
+  iterator find(StringRef Name) const { return HashTable.find(Name); }
+
   /// Print some statistics to stderr that indicate how well the
   /// hashing is doing.
   void PrintStats() const;
Index: clang-tools-extra/test/clang-tidy/readability-identifier-naming-bugfix-name-conflicts.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/readability-identifier-naming-bugfix-name-conflicts.cpp
@@ -0,0 +1,27 @@
+// RUN: %check_clang_tidy %s readability-identifier-naming %t -- \
+// RUN:   -config='{CheckOptions: [ \
+// RUN: {key: readability-identifier-naming.ParameterCase, value: lower_case} \
+// RUN:   ]}'
+
+int func(int Break) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: invalid case style for parameter 'Break'; cannot be fixed because 'break' would conflict with a keyword
+  // CHECK-FIXES: {{^}}int func(int Break) {{{$}}
+  if (Break == 1) {
+// CHECK-FIXES: {{^}}  if (Break == 1) {{{$}}
+return 2;
+  }
+
+  return 0;
+}
+
+#define foo 3
+int func2(int Foo) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: invalid case style for parameter 'Foo'; cannot be fixed because 'foo' would conflict with a macro definition
+  // CHECK-FIXES: {{^}}int func2(int Foo) {{{$}}
+  if (Foo == 1) {
+// CHECK-FIXES: {{^}}  if (Foo == 1) {{{$}}
+return 2;
+  }
+
+  return 0;
+}
Index: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
===
--- clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
+++ clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
@@ -64,6 +64,24 @@
 std::string Suffix;
   };
 
+  /// This enum will be used in %select of the diagnostic message.
+  /// Each value below IgnoreFailureThreshold should have an error message.
+  enum class ShouldFixStatus {
+ShouldFix,
+ConflictsWithKeyword, /// The fixup will conflict with a language keyword,
+  /// so we can't fix it automatically.
+ConflictsWithMacroDefinition, /// The fixup will conflict with a macro
+  /// definition, so we can't fix it
+  /// automatically.
+
+/// Values pass this threshold will be ignored completely
+/// i.e no message, no fixup.
+IgnoreFailureThreshold,
+
+InsideMacro, /// If the identifier was used or declared within a macro we
+ /// won't offer a fixup for safety reasons.
+  };
+
   /// Holds an identifier name check failure, tracking the kind of the
   /// identifer, its possible fixup and the starting locations of all the
   /// identifier usages.
@@ -75,13 +93,19 @@
 ///
 /// ie: if the identifier was used or declared within a macro we won't offer
 /// a fixup for safety reasons.
-bool ShouldFix;
+bool ShouldFix() const { return FixStatus == ShouldFixStatus::ShouldFix; }
+
+bool ShouldNotify() const {
+  return FixStatus < ShouldFixStatus::IgnoreFailureThreshold;
+}
+
+ShouldFixStatus FixStatus = ShouldFixStatus::ShouldFix;
 
 /// A set of all the identifier usages starting SourceLocation, in
 /// their encoded form.
 llvm::DenseSet RawUsageLocs;
 
-NamingCheckFailure() : ShouldFix(true) {}
+NamingCheckFailure() = default;
   };
 
   typedef std::pair NamingCheckId;
Index: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
@@ -679,10 +679,11 @@
   if (!Failure.RawUsageLocs.insert(FixLocation.getRawEncoding()).second)
 return;
 
-  if (!Failure.ShouldFix)
+  if (!Failure.ShouldFix())
 return;
 
-  Failure.ShouldFix = utils::rangeCanBeFixed(Range, SourceMgr);
+  if (!utils::rangeCanBeFixed(Range, SourceMgr))
+Failure.FixStatus =

[clang-tools-extra] r374718 - Make most clangd unittests pass on Windows

2019-10-13 Thread Nico Weber via cfe-commits

Author: nico
Date: Sun Oct 13 06:15:27 2019
New Revision: 374718

URL: http://llvm.org/viewvc/llvm-project?rev=374718=rev
Log:
Make most clangd unittests pass on Windows

The Windows triple currently turns on delayed template parsing, which
confuses several unit tests that use templates.

For now, just explicitly disable delayed template parsing. This isn't
ideal, but:

- the Windows triple will soon no longer use delayed template parsing
  by default

- there's precedent for this in the clangd unit tests already

- let's get the clangd tests pass on Windows first before making
  behavioral changes

Part of PR43592.

Modified:
clang-tools-extra/trunk/clangd/unittests/FindTargetTests.cpp
clang-tools-extra/trunk/clangd/unittests/ParsedASTTests.cpp
clang-tools-extra/trunk/clangd/unittests/SelectionTests.cpp
clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp
clang-tools-extra/trunk/clangd/unittests/TweakTesting.cpp
clang-tools-extra/trunk/clangd/unittests/TweakTesting.h
clang-tools-extra/trunk/clangd/unittests/TweakTests.cpp
clang-tools-extra/trunk/clangd/unittests/XRefsTests.cpp

Modified: clang-tools-extra/trunk/clangd/unittests/FindTargetTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/FindTargetTests.cpp?rev=374718=374717=374718=diff
==
--- clang-tools-extra/trunk/clangd/unittests/FindTargetTests.cpp (original)
+++ clang-tools-extra/trunk/clangd/unittests/FindTargetTests.cpp Sun Oct 13 
06:15:27 2019
@@ -226,18 +226,26 @@ TEST_F(TargetDeclTest, Types) {
   EXPECT_DECLS("TypedefTypeLoc", {"typedef S X", Rel::Alias},
{"struct S", Rel::Underlying});
 
+  // FIXME: Auto-completion in a template requires disabling delayed template
+  // parsing.
+  Flags = {"-fno-delayed-template-parsing"};
   Code = R"cpp(
 template
 void foo() { [[T]] x; }
   )cpp";
   // FIXME: We don't do a good job printing TemplateTypeParmDecls, apparently!
   EXPECT_DECLS("TemplateTypeParmTypeLoc", "");
+  Flags.clear();
 
+  // FIXME: Auto-completion in a template requires disabling delayed template
+  // parsing.
+  Flags = {"-fno-delayed-template-parsing"};
   Code = R"cpp(
 template class T>
 void foo() { [[T]] x; }
   )cpp";
   EXPECT_DECLS("TemplateSpecializationTypeLoc", "template  class T");
+  Flags.clear();
 
   Code = R"cpp(
 struct S{};
@@ -394,6 +402,10 @@ TEST_F(TargetDeclTest, Lambda) {
 }
 
 TEST_F(TargetDeclTest, OverloadExpr) {
+  // FIXME: Auto-completion in a template requires disabling delayed template
+  // parsing.
+  Flags = {"-fno-delayed-template-parsing"};
+
   Code = R"cpp(
 void func(int*);
 void func(char*);
@@ -509,6 +521,10 @@ protected:
 TestTU TU;
 TU.Code = Code;
 
+// FIXME: Auto-completion in a template requires disabling delayed template
+// parsing.
+TU.ExtraArgs.push_back("-fno-delayed-template-parsing");
+
 auto AST = TU.build();
 
 auto *TestDecl = (AST, "foo");

Modified: clang-tools-extra/trunk/clangd/unittests/ParsedASTTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/ParsedASTTests.cpp?rev=374718=374717=374718=diff
==
--- clang-tools-extra/trunk/clangd/unittests/ParsedASTTests.cpp (original)
+++ clang-tools-extra/trunk/clangd/unittests/ParsedASTTests.cpp Sun Oct 13 
06:15:27 2019
@@ -144,6 +144,9 @@ TEST(ParsedASTTest,
 template <>
 int foo = 0;
   )cpp";
+  // FIXME: Auto-completion in a template requires disabling delayed template
+  // parsing.
+  TU.ExtraArgs.push_back("-fno-delayed-template-parsing");
 
   auto AST = TU.build();
   EXPECT_THAT(

Modified: clang-tools-extra/trunk/clangd/unittests/SelectionTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/SelectionTests.cpp?rev=374718=374717=374718=diff
==
--- clang-tools-extra/trunk/clangd/unittests/SelectionTests.cpp (original)
+++ clang-tools-extra/trunk/clangd/unittests/SelectionTests.cpp Sun Oct 13 
06:15:27 2019
@@ -289,7 +289,15 @@ TEST(SelectionTest, CommonAncestor) {
   };
   for (const Case  : Cases) {
 Annotations Test(C.Code);
-auto AST = TestTU::withCode(Test.code()).build();
+
+TestTU TU;
+TU.Code = Test.code();
+
+// FIXME: Auto-completion in a template requires disabling delayed template
+// parsing.
+TU.ExtraArgs.push_back("-fno-delayed-template-parsing");
+
+auto AST = TU.build();
 auto T = makeSelectionTree(C.Code, AST);
 EXPECT_EQ("TranslationUnitDecl", nodeKind(())) << C.Code;
 

Modified: clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp
URL:

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen(), size() or equal length

2019-10-13 Thread Csaba Dabis via Phabricator via cfe-commits

Charusso added a comment.

I think everything is working fine on the build-bot side. One tiny issue on 
Windows left where the `getLength()` could not obtain the length properly: 
rL374713 , I will look into that.

In D45050#1071898 , @xbolva00 wrote:

> Shouldn't it catch in curl also this code?
>
> urllen = strlen(url_clone);
>
>   
>
> memcpy(newest, url_clone, urllen);
>
> Edit: if possible, report these bugs to project developers :)

@xbolva00, now I answer it: it is out of the scope of the Tidy, sadly. I really 
wanted to make this work with tons of heuristics, which made that patch so 
enormous. I have not forgotten it, I will report the found issues some months 
later. Now I want to do the same patch with the help of the Analyzer, and 
research that, whether this type of matching is could be done more 
appropriately. I have already started that idea by D68725 
. Thanks again!

Thanks @JonasToth, you really wanted to see my first patch upstreamed, and also 
thanks everyone, your support really helped me to understand how to do reviews 
and how to Clang. I apologize for the size, I felt that the larger the better.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D45050/new/

https://reviews.llvm.org/D45050

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

r374717 - BlockInCriticalSectionChecker - silence static analyzer dyn_cast null dereference warning. NFCI.

2019-10-13 Thread Simon Pilgrim via cfe-commits

Author: rksimon
Date: Sun Oct 13 04:30:06 2019
New Revision: 374717

URL: http://llvm.org/viewvc/llvm-project?rev=374717=rev
Log:
BlockInCriticalSectionChecker - silence static analyzer dyn_cast null 
dereference warning. NFCI.

The static analyzer is warning about a potential null dereference, but we 
should be able to use cast<> directly and if not assert will fire for us.

Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp

Modified: 
cfe/trunk/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp?rev=374717=374716=374717=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp 
(original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp Sun 
Oct 13 04:30:06 2019
@@ -126,7 +126,7 @@ bool BlockInCriticalSectionChecker::isLo
 
 bool BlockInCriticalSectionChecker::isUnlockFunction(const CallEvent ) 
const {
   if (const auto *Dtor = dyn_cast()) {
-const auto *DRecordDecl = 
dyn_cast(Dtor->getDecl()->getParent());
+const auto *DRecordDecl = 
cast(Dtor->getDecl()->getParent());
 auto IdentifierInfo = DRecordDecl->getIdentifier();
 if (IdentifierInfo == IILockGuard || IdentifierInfo == IIUniqueLock)
   return true;


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

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen(), size() or equal length

2019-10-13 Thread Csaba Dabis via Phabricator via cfe-commits

Charusso added a comment.

In D45050#1707489 , @xbolva00 wrote:

> `typedef __SIZE_TYPE__ size_t;`


Hm, yes, I have overestimated the uploaded solution:

  $ clang/test grep -rn 'typedef __SIZE_TYPE__ size_t;' | wc -l
  60
  $ clang/test grep -rn 'typedef __typeof(sizeof(int)) size_t;' | wc -l
  45

Thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D45050/new/

https://reviews.llvm.org/D45050



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

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen(), size() or equal length

2019-10-13 Thread Dávid Bolvanský via Phabricator via cfe-commits

xbolva00 added a comment.

typedef __SIZE_TYPE__ size_t;


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D45050/new/

https://reviews.llvm.org/D45050



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

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen(), size() or equal length

2019-10-13 Thread Csaba Dabis via Phabricator via cfe-commits

Charusso added a comment.

In D45050#1707483 , @thakis wrote:

> This fails on Windows: http://45.33.8.238/win/386/step_7.txt
>
> Please take a look, and if it's not immediately clear how to fix, b please 
> revert while you investigate.


Oh, thanks! We define it like `typedef __typeof(sizeof(int)) size_t;`, so by 
rL374715  it needs to work.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D45050/new/

https://reviews.llvm.org/D45050



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

[clang-tools-extra] r374715 - [clang-tidy] bugprone-not-null-terminated-result: checker adjustments 4

2019-10-13 Thread Csaba Dabis via cfe-commits

Author: charusso
Date: Sun Oct 13 03:59:30 2019
New Revision: 374715

URL: http://llvm.org/viewvc/llvm-project?rev=374715=rev
Log:
[clang-tidy] bugprone-not-null-terminated-result: checker adjustments 4

Modified:

clang-tools-extra/trunk/test/clang-tidy/Inputs/bugprone-not-null-terminated-result/not-null-terminated-result-c.h

Modified: 
clang-tools-extra/trunk/test/clang-tidy/Inputs/bugprone-not-null-terminated-result/not-null-terminated-result-c.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/Inputs/bugprone-not-null-terminated-result/not-null-terminated-result-c.h?rev=374715=374714=374715=diff
==
--- 
clang-tools-extra/trunk/test/clang-tidy/Inputs/bugprone-not-null-terminated-result/not-null-terminated-result-c.h
 (original)
+++ 
clang-tools-extra/trunk/test/clang-tidy/Inputs/bugprone-not-null-terminated-result/not-null-terminated-result-c.h
 Sun Oct 13 03:59:30 2019
@@ -13,7 +13,7 @@
 
 #pragma clang system_header
 
-typedef unsigned int size_t;
+typedef __typeof__(sizeof(int)) size_t;
 typedef int errno_t;
 
 size_t strlen(const char *str);


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

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen(), size() or equal length

2019-10-13 Thread Nico Weber via Phabricator via cfe-commits

thakis added a comment.

This fails on Windows: http://45.33.8.238/win/386/step_7.txt

Please take a look, and if it's not immediately clear how to fix, b please 
revert while you investigate.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D45050/new/

https://reviews.llvm.org/D45050



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

[PATCH] D68554: [clang-format] Proposal for clang-format to give compiler style warnings

2019-10-13 Thread MyDeveloperDay via Phabricator via cfe-commits

MyDeveloperDay added a comment.

> I've reverted this for now to green up the bots.
> 
> For the reland, did y'all consider the impact of this on clang-format build 
> time (it now depends on Frontend and hence on Driver and Sema, and Sema is 
> slow to build) and binary size (it's now basically impossible to ever get the 
> diagnostics tables in clang-format gc'd by the linker)?
> 
> It might make sense to instead use LLVM's diag stuff (instead of clang's) 
> since all the actual diags will be disjoint.

Thanks for doing this, I am struggling to find the MacOS bot log that failed, 
are any available via Buildbot? (I notice the log looks like your own machine)

I did a quick test, with the backed out change and it's ~200KB bigger with this 
revision on windows. (that's probably an assertion, debug build) as the last 
one in the LLVM snapshot builds is only 3,518,976 bytes

  -rwxr-xr-x 1 None 15369728 Oct 12 17:36 clang-format-diags.exe
  -rwxr-xr-x 1 None 15080960 Oct 13 10:44 clang-format.exe

on my machine, the current release size is just 2,708,480 bytes (I'm using 
VS2017) - (not sure why its smaller)

`-rwxr-xr-x 1 None 2708480 Oct 13 11:00 clang-format.exe`

`make clang-format` build speed with reverted change is 4m12.504s  (Release 
build from clean)

`-rwxr-xr-x 1 None 2817024 Oct 13 11:22 clang-format.exe (117KB larger)`

`make clang-format` build speed with unreverted change is 4m51.504s  (Release 
build from clean)

Rebuilding when nothing to rebuild goes from from 5 seconds to 6.7

So as you said it will be a little slower to build clang-format, however, I do 
notice that all the other clang/tools are pretty much are all building with 
Frontend. so anyone building a larger range of clang tools probably has 
everything already built.

I think the main build failure was just the lit tests, I think i would prefer 
to fix those and reland this as-is, then look for a better diags solution if 
@klimek thinks this is something I should be concerned about. I really wanted 
to be able to use a standard Diagnostic front end with all the support for 
console coloring etc...I'll have to take a look at LLVM's diagnostics to see 
how that is done.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68554/new/

https://reviews.llvm.org/D68554



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

[clang-tools-extra] r374713 - [clang-tidy] bugprone-not-null-terminated-result: checker adjustments 3

2019-10-13 Thread Csaba Dabis via cfe-commits

Author: charusso
Date: Sun Oct 13 03:41:13 2019
New Revision: 374713

URL: http://llvm.org/viewvc/llvm-project?rev=374713=rev
Log:
[clang-tidy] bugprone-not-null-terminated-result: checker adjustments 3

On Windows the signed/unsigned int conversions of APInt seems broken, so that
two of the test files marked as unsupported on Windows, as a hotfix.

Modified:

clang-tools-extra/trunk/test/clang-tidy/bugprone-not-null-terminated-result-strlen.c

clang-tools-extra/trunk/test/clang-tidy/bugprone-not-null-terminated-result-wcslen.cpp

Modified: 
clang-tools-extra/trunk/test/clang-tidy/bugprone-not-null-terminated-result-strlen.c
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/bugprone-not-null-terminated-result-strlen.c?rev=374713=374712=374713=diff
==
--- 
clang-tools-extra/trunk/test/clang-tidy/bugprone-not-null-terminated-result-strlen.c
 (original)
+++ 
clang-tools-extra/trunk/test/clang-tidy/bugprone-not-null-terminated-result-strlen.c
 Sun Oct 13 03:41:13 2019
@@ -1,6 +1,11 @@
 // RUN: %check_clang_tidy %s bugprone-not-null-terminated-result %t -- \
 // RUN: -- -std=c11 -I %S/Inputs/bugprone-not-null-terminated-result
 
+// FIXME: Something wrong with the APInt un/signed conversion on Windows:
+// in 'strncmp(str6, "string", 7);' it tries to inject '4294967302' as length.
+
+// UNSUPPORTED: system-windows
+
 #include "not-null-terminated-result-c.h"
 
 #define __STDC_LIB_EXT1__ 1

Modified: 
clang-tools-extra/trunk/test/clang-tidy/bugprone-not-null-terminated-result-wcslen.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/bugprone-not-null-terminated-result-wcslen.cpp?rev=374713=374712=374713=diff
==
--- 
clang-tools-extra/trunk/test/clang-tidy/bugprone-not-null-terminated-result-wcslen.cpp
 (original)
+++ 
clang-tools-extra/trunk/test/clang-tidy/bugprone-not-null-terminated-result-wcslen.cpp
 Sun Oct 13 03:41:13 2019
@@ -1,6 +1,11 @@
 // RUN: %check_clang_tidy %s bugprone-not-null-terminated-result %t -- \
 // RUN: -- -std=c++11 -I %S/Inputs/bugprone-not-null-terminated-result
 
+// FIXME: Something wrong with the APInt un/signed conversion on Windows:
+// in 'wcsncmp(wcs6, L"string", 7);' it tries to inject '4294967302' as length.
+
+// UNSUPPORTED: system-windows
+
 #include "not-null-terminated-result-cxx.h"
 
 #define __STDC_LIB_EXT1__ 1


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

[clang-tools-extra] r374712 - [clang-tidy] bugprone-not-null-terminated-result: checker adjustments 2

2019-10-13 Thread Csaba Dabis via cfe-commits

Author: charusso
Date: Sun Oct 13 03:20:58 2019
New Revision: 374712

URL: http://llvm.org/viewvc/llvm-project?rev=374712=rev
Log:
[clang-tidy] bugprone-not-null-terminated-result: checker adjustments 2

Modified:
clang-tools-extra/trunk/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp

Modified: 
clang-tools-extra/trunk/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp?rev=374712=374711=374712=diff
==
--- 
clang-tools-extra/trunk/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp 
(original)
+++ 
clang-tools-extra/trunk/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp 
Sun Oct 13 03:20:58 2019
@@ -59,7 +59,8 @@ static const Expr *getDestCapacityExpr(c
 
 // Returns the length of \p E as an 'IntegerLiteral' or a 'StringLiteral'
 // without the null-terminator.
-static int getLength(const Expr *E, const MatchFinder::MatchResult ) {
+static unsigned getLength(const Expr *E,
+  const MatchFinder::MatchResult ) {
   if (!E)
 return 0;
 
@@ -71,10 +72,10 @@ static int getLength(const Expr *E, cons
   if (!isa(LengthVD))
 if (const Expr *LengthInit = LengthVD->getInit())
   if (LengthInit->EvaluateAsInt(Length, *Result.Context))
-return Length.Val.getInt().getSExtValue();
+return Length.Val.getInt().getZExtValue();
 
   if (const auto *LengthIL = dyn_cast(E))
-return LengthIL->getValue().getSExtValue();
+return LengthIL->getValue().getZExtValue();
 
   if (const auto *StrDRE = dyn_cast(E))
 if (const auto *StrVD = dyn_cast(StrDRE->getDecl()))
@@ -306,7 +307,7 @@ static void lengthExprHandle(const Expr
   // Try to obtain an 'IntegerLiteral' and adjust it.
   if (!IsMacroDefinition) {
 if (const auto *LengthIL = dyn_cast(LengthExpr)) {
-  size_t NewLength = LengthIL->getValue().getSExtValue() +
+  size_t NewLength = LengthIL->getValue().getZExtValue() +
  (LengthHandle == LengthHandleKind::Increase
   ? (isInjectUL(Result) ? 1UL : 1)
   : -1);
@@ -327,7 +328,7 @@ static void lengthExprHandle(const Expr
 const Expr *RhsExpr = BO->getRHS()->IgnoreImpCasts();
 
 if (const auto *LhsIL = dyn_cast(LhsExpr)) {
-  if (LhsIL->getValue().getSExtValue() == 1) {
+  if (LhsIL->getValue().getZExtValue() == 1) {
 Diag << FixItHint::CreateRemoval(
 {LhsIL->getBeginLoc(),
  RhsExpr->getBeginLoc().getLocWithOffset(-1)});
@@ -336,7 +337,7 @@ static void lengthExprHandle(const Expr
 }
 
 if (const auto *RhsIL = dyn_cast(RhsExpr)) {
-  if (RhsIL->getValue().getSExtValue() == 1) {
+  if (RhsIL->getValue().getZExtValue() == 1) {
 Diag << FixItHint::CreateRemoval(
 {LhsExpr->getEndLoc().getLocWithOffset(1), RhsIL->getEndLoc()});
 return;
@@ -803,7 +804,7 @@ void NotNullTerminatedResultCheck::check
 StringRef ValueStr = StringRef(T.getLiteralData(), T.getLength());
 llvm::APInt IntValue;
 ValueStr.getAsInteger(10, IntValue);
-AreSafeFunctionsWanted = IntValue.getSExtValue();
+AreSafeFunctionsWanted = IntValue.getZExtValue();
   }
 
   ++It;


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

[clang-tools-extra] r374711 - [clang-tidy] bugprone-not-null-terminated-result: checker adjustments

2019-10-13 Thread Csaba Dabis via cfe-commits

Author: charusso
Date: Sun Oct 13 02:46:56 2019
New Revision: 374711

URL: http://llvm.org/viewvc/llvm-project?rev=374711=rev
Log:
[clang-tidy] bugprone-not-null-terminated-result: checker adjustments

Modified:
clang-tools-extra/trunk/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp

Modified: 
clang-tools-extra/trunk/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp?rev=374711=374710=374711=diff
==
--- 
clang-tools-extra/trunk/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp 
(original)
+++ 
clang-tools-extra/trunk/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp 
Sun Oct 13 02:46:56 2019
@@ -71,10 +71,10 @@ static int getLength(const Expr *E, cons
   if (!isa(LengthVD))
 if (const Expr *LengthInit = LengthVD->getInit())
   if (LengthInit->EvaluateAsInt(Length, *Result.Context))
-return Length.Val.getInt().getZExtValue();
+return Length.Val.getInt().getSExtValue();
 
   if (const auto *LengthIL = dyn_cast(E))
-return LengthIL->getValue().getZExtValue();
+return LengthIL->getValue().getSExtValue();
 
   if (const auto *StrDRE = dyn_cast(E))
 if (const auto *StrVD = dyn_cast(StrDRE->getDecl()))
@@ -306,7 +306,7 @@ static void lengthExprHandle(const Expr
   // Try to obtain an 'IntegerLiteral' and adjust it.
   if (!IsMacroDefinition) {
 if (const auto *LengthIL = dyn_cast(LengthExpr)) {
-  size_t NewLength = LengthIL->getValue().getZExtValue() +
+  size_t NewLength = LengthIL->getValue().getSExtValue() +
  (LengthHandle == LengthHandleKind::Increase
   ? (isInjectUL(Result) ? 1UL : 1)
   : -1);
@@ -327,7 +327,7 @@ static void lengthExprHandle(const Expr
 const Expr *RhsExpr = BO->getRHS()->IgnoreImpCasts();
 
 if (const auto *LhsIL = dyn_cast(LhsExpr)) {
-  if (LhsIL->getValue().getZExtValue() == 1) {
+  if (LhsIL->getValue().getSExtValue() == 1) {
 Diag << FixItHint::CreateRemoval(
 {LhsIL->getBeginLoc(),
  RhsExpr->getBeginLoc().getLocWithOffset(-1)});
@@ -336,7 +336,7 @@ static void lengthExprHandle(const Expr
 }
 
 if (const auto *RhsIL = dyn_cast(RhsExpr)) {
-  if (RhsIL->getValue().getZExtValue() == 1) {
+  if (RhsIL->getValue().getSExtValue() == 1) {
 Diag << FixItHint::CreateRemoval(
 {LhsExpr->getEndLoc().getLocWithOffset(1), RhsIL->getEndLoc()});
 return;
@@ -803,7 +803,7 @@ void NotNullTerminatedResultCheck::check
 StringRef ValueStr = StringRef(T.getLiteralData(), T.getLength());
 llvm::APInt IntValue;
 ValueStr.getAsInteger(10, IntValue);
-AreSafeFunctionsWanted = IntValue.getZExtValue();
+AreSafeFunctionsWanted = IntValue.getSExtValue();
   }
 
   ++It;


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

[PATCH] D68914: [clang-format] Remove duplciate code from Invalid BOM detection

2019-10-13 Thread MyDeveloperDay via Phabricator via cfe-commits

MyDeveloperDay updated this revision to Diff 224776.
MyDeveloperDay marked 2 inline comments as done.
MyDeveloperDay added a comment.

Updating with review comments


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68914/new/

https://reviews.llvm.org/D68914

Files:
  clang/include/clang/Basic/SourceManager.h
  clang/lib/Basic/SourceManager.cpp
  clang/tools/clang-format/ClangFormat.cpp
  clang/unittests/Basic/SourceManagerTest.cpp

Index: clang/unittests/Basic/SourceManagerTest.cpp
===
--- clang/unittests/Basic/SourceManagerTest.cpp
+++ clang/unittests/Basic/SourceManagerTest.cpp
@@ -200,6 +200,47 @@
 "");
 }
 
+TEST_F(SourceManagerTest, getInvalidBOM) {
+  ASSERT_EQ(SrcMgr::ContentCache::getInvalidBOM(""), nullptr);
+  ASSERT_EQ(SrcMgr::ContentCache::getInvalidBOM("\x00\x00\x00"), nullptr);
+  ASSERT_EQ(SrcMgr::ContentCache::getInvalidBOM("\xFF\xFF\xFF"), nullptr);
+  ASSERT_EQ(SrcMgr::ContentCache::getInvalidBOM("#include "),
+nullptr);
+
+  ASSERT_EQ(StringRef(SrcMgr::ContentCache::getInvalidBOM(
+"\xFE\xFF#include ")),
+"UTF-16 (BE)");
+  ASSERT_EQ(StringRef(SrcMgr::ContentCache::getInvalidBOM(
+"\xFF\xFE#include ")),
+"UTF-16 (LE)");
+  ASSERT_EQ(StringRef(SrcMgr::ContentCache::getInvalidBOM(
+"\x2B\x2F\x76#include ")),
+"UTF-7");
+  ASSERT_EQ(StringRef(SrcMgr::ContentCache::getInvalidBOM(
+"\xF7\x64\x4C#include ")),
+"UTF-1");
+  ASSERT_EQ(StringRef(SrcMgr::ContentCache::getInvalidBOM(
+"\xDD\x73\x66\x73#include ")),
+"UTF-EBCDIC");
+  ASSERT_EQ(StringRef(SrcMgr::ContentCache::getInvalidBOM(
+"\x0E\xFE\xFF#include ")),
+"SCSU");
+  ASSERT_EQ(StringRef(SrcMgr::ContentCache::getInvalidBOM(
+"\xFB\xEE\x28#include ")),
+"BOCU-1");
+  ASSERT_EQ(StringRef(SrcMgr::ContentCache::getInvalidBOM(
+"\x84\x31\x95\x33#include ")),
+"GB-18030");
+  ASSERT_EQ(StringRef(SrcMgr::ContentCache::getInvalidBOM(
+llvm::StringLiteral::withInnerNUL(
+"\x00\x00\xFE\xFF#include "))),
+"UTF-32 (BE)");
+  ASSERT_EQ(StringRef(SrcMgr::ContentCache::getInvalidBOM(
+llvm::StringLiteral::withInnerNUL(
+"\xFF\xFE\x00\x00#include "))),
+"UTF-32 (LE)");
+}
+
 #if defined(LLVM_ON_UNIX)
 
 TEST_F(SourceManagerTest, getMacroArgExpandedLocation) {
Index: clang/tools/clang-format/ClangFormat.cpp
===
--- clang/tools/clang-format/ClangFormat.cpp
+++ clang/tools/clang-format/ClangFormat.cpp
@@ -290,31 +290,6 @@
   }
 }
 
-// If BufStr has an invalid BOM, returns the BOM name; otherwise, returns
-// nullptr.
-static const char *getInValidBOM(StringRef BufStr) {
-  // Check to see if the buffer has a UTF Byte Order Mark (BOM).
-  // We only support UTF-8 with and without a BOM right now.  See
-  // https://en.wikipedia.org/wiki/Byte_order_mark#Byte_order_marks_by_encoding
-  // for more information.
-  const char *InvalidBOM =
-  llvm::StringSwitch(BufStr)
-  .StartsWith(llvm::StringLiteral::withInnerNUL("\x00\x00\xFE\xFF"),
-  "UTF-32 (BE)")
-  .StartsWith(llvm::StringLiteral::withInnerNUL("\xFF\xFE\x00\x00"),
-  "UTF-32 (LE)")
-  .StartsWith("\xFE\xFF", "UTF-16 (BE)")
-  .StartsWith("\xFF\xFE", "UTF-16 (LE)")
-  .StartsWith("\x2B\x2F\x76", "UTF-7")
-  .StartsWith("\xF7\x64\x4C", "UTF-1")
-  .StartsWith("\xDD\x73\x66\x73", "UTF-EBCDIC")
-  .StartsWith("\x0E\xFE\xFF", "SCSU")
-  .StartsWith("\xFB\xEE\x28", "BOCU-1")
-  .StartsWith("\x84\x31\x95\x33", "GB-18030")
-  .Default(nullptr);
-  return InvalidBOM;
-}
-
 static bool
 emitReplacementWarnings(const Replacements , StringRef AssumedFileName,
 const std::unique_ptr ) {
@@ -400,7 +375,7 @@
 
   StringRef BufStr = Code->getBuffer();
 
-  const char *InvalidBOM = getInValidBOM(BufStr);
+  const char *InvalidBOM = SrcMgr::ContentCache::getInvalidBOM(BufStr);
 
   if (InvalidBOM) {
 errs() << "error: encoding with unsupported byte order mark \""
Index: clang/lib/Basic/SourceManager.cpp
===
--- clang/lib/Basic/SourceManager.cpp
+++ clang/lib/Basic/SourceManager.cpp
@@ -95,6 +95,29 @@
   Buffer.setInt((B && DoNotFree) ? DoNotFreeFlag : 0);
 }
 
+const char *ContentCache::getInvalidBOM(const StringRef BufStr) {
+  // If the buffer is valid, check to see if it has a UTF Byte Order Mark
+  // (BOM).  We only support UTF-8 with and without a BOM right now.  See
+  // http://en.wikipedia.org/wiki/Byte_order_mark for more information.
+  const char *InvalidBOM =
+  llvm::StringSwitch(BufStr)
+

[PATCH] D68927: [clang-tools-extra] Fix overzealous linking of dylib to clangTidy

2019-10-13 Thread Michał Górny via Phabricator via cfe-commits

mgorny created this revision.
mgorny added reviewers: sylvestre.ledru, beanz.

Fix accidentally making clangTidy library link to dylib.  This causes
libclang.so to also link to dylib which results in duplicate symbols
from shared and static libraries, and effectively to registering
command-line options twice.

Thanks to Sylvestre Ledru for noticing this and tracking it down
to r373786.  Fixes PR#43589.


https://reviews.llvm.org/D68927

Files:
  clang-tools-extra/clang-tidy/CMakeLists.txt


Index: clang-tools-extra/clang-tidy/CMakeLists.txt
===
--- clang-tools-extra/clang-tidy/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/CMakeLists.txt
@@ -31,7 +31,7 @@
   )
 
 if(CLANG_ENABLE_STATIC_ANALYZER)
-  clang_target_link_libraries(clangTidy PRIVATE
+  target_link_libraries(clangTidy PRIVATE
 clangStaticAnalyzerCore
 clangStaticAnalyzerFrontend
   )


Index: clang-tools-extra/clang-tidy/CMakeLists.txt
===
--- clang-tools-extra/clang-tidy/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/CMakeLists.txt
@@ -31,7 +31,7 @@
   )
 
 if(CLANG_ENABLE_STATIC_ANALYZER)
-  clang_target_link_libraries(clangTidy PRIVATE
+  target_link_libraries(clangTidy PRIVATE
 clangStaticAnalyzerCore
 clangStaticAnalyzerFrontend
   )
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r374710 - [clang-tidy] bugprone-not-null-terminated-result: Sphinx adjustments 2

2019-10-13 Thread Csaba Dabis via cfe-commits

Author: charusso
Date: Sun Oct 13 01:49:43 2019
New Revision: 374710

URL: http://llvm.org/viewvc/llvm-project?rev=374710=rev
Log:
[clang-tidy] bugprone-not-null-terminated-result: Sphinx adjustments 2

Modified:

clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-not-null-terminated-result.rst

Modified: 
clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-not-null-terminated-result.rst
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-not-null-terminated-result.rst?rev=374710=374709=374710=diff
==
--- 
clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-not-null-terminated-result.rst
 (original)
+++ 
clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-not-null-terminated-result.rst
 Sun Oct 13 01:49:43 2019
@@ -91,32 +91,36 @@ respectively (where only ``strerror_s``
 Memory handler functions
 
 
-- ``memcpy`` Please visit the
-  :ref:`Transformation rules of 'memcpy()'` section.
+``memcpy``
+Please visit the
+:ref:`Transformation rules of 'memcpy()'` section.
+
+``memchr``
+Usually there is a C-style cast and it is needed to be removed, because the
+new function ``strchr``'s return type is correct. The given length is going
+to be removed.
+
+``memmove``
+If safe functions are available the new function is ``memmove_s``, which has
+a new second argument which is the length of the destination array, it is
+adjusted, and the length of the source string is incremented by one.
+If safe functions are not available the given length is incremented by one.
 
-- ``memchr``
-  Usually there is a C-style cast and it is needed to be removed, because the
-  new function ``strchr``'s return type is correct.
-  The given length is going to be removed.
-
-- ``memmove``
-  If safe functions are available the new function is ``memmove_s``, which has
-  a new second argument which is the length of the destination array, it is
-  adjusted, and the length of the source string is incremented by one.
-  If safe functions are not available the given length is incremented by one.
-
-- ``memmove_s``
-  The given length is incremented by one.
+``memmove_s``
+The given length is incremented by one.
 
 String handler functions
 
 
-- ``strerror_s``: given length is incremented by one.
+``strerror_s``
+The given length is incremented by one.
 
-- ``strncmp``: If the third argument is the first or the second argument's
-``length + 1`` it has to be truncated without the ``+ 1`` operation.
+``strncmp``
+If the third argument is the first or the second argument's ``length + 1``
+it has to be truncated without the ``+ 1`` operation.
 
-- ``strxfrm``: given length is incremented by one.
+``strxfrm``
+The given length is incremented by one.
 
 Options
 ---


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

[clang-tools-extra] r374709 - [clang-tidy] bugprone-not-null-terminated-result: Sphinx adjustments

2019-10-13 Thread Csaba Dabis via cfe-commits

Author: charusso
Date: Sun Oct 13 01:41:24 2019
New Revision: 374709

URL: http://llvm.org/viewvc/llvm-project?rev=374709=rev
Log:
[clang-tidy] bugprone-not-null-terminated-result: Sphinx adjustments

Modified:

clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-not-null-terminated-result.rst

Modified: 
clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-not-null-terminated-result.rst
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-not-null-terminated-result.rst?rev=374709=374708=374709=diff
==
--- 
clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-not-null-terminated-result.rst
 (original)
+++ 
clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-not-null-terminated-result.rst
 Sun Oct 13 01:41:24 2019
@@ -91,21 +91,22 @@ respectively (where only ``strerror_s``
 Memory handler functions
 
 
-- ``memcpy``: Visit the
-:ref:`Transformation rules of 'memcpy()'` section.
+- ``memcpy`` Please visit the
+  :ref:`Transformation rules of 'memcpy()'` section.
 
-- ``memchr``:
-  - Usually there is a C-style cast and it is needed to be removed, because the
-new function ``strchr``'s return type is correct.
-  - The given length is going to be removed.
+- ``memchr``
+  Usually there is a C-style cast and it is needed to be removed, because the
+  new function ``strchr``'s return type is correct.
+  The given length is going to be removed.
 
-- ``memmove``:
-  - If safe functions are available the new function is ``memmove_s``, which 
has
-a new second argument which is the length of the destination array, it is
-adjusted, and the length of the source string is incremented by one.
-  - If safe functions are not available the given length is incremented by one.
+- ``memmove``
+  If safe functions are available the new function is ``memmove_s``, which has
+  a new second argument which is the length of the destination array, it is
+  adjusted, and the length of the source string is incremented by one.
+  If safe functions are not available the given length is incremented by one.
 
-- ``memmove_s``: given length is incremented by one.
+- ``memmove_s``
+  The given length is incremented by one.
 
 String handler functions
 


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

[PATCH] D68877: [AArch64][SVE] Implement masked load intrinsics

2019-10-13 Thread Dave Green via Phabricator via cfe-commits

dmgreen added subscribers: samparker, dmgreen.
dmgreen added a comment.

Sam has been looking at extending masked loads and stores in D68337 
 and related patches. There looks like there 
would be some overlap with this, especially in the target independent parts. 
Make sure you co-ordinate with him.




Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:10393
+  ((!LegalOperations && !cast(N0)->isVolatile()) ||
+   TLI.isLoadExtLegal(ISD::SEXTLOAD, VT, EVT))) {
+MaskedLoadSDNode *LN0 = cast(N0);

I'm not convinced that just because a sext load is legal and a masked load is 
legal, that a sext masked load is always legal.



Comment at: llvm/lib/Target/AArch64/AArch64SVEInstrInfo.td:1077
+
+def _default_z : Pat<(Ty (Load  GPR64:$base, (PredTy PPR:$gp), 
(SVEUndef))),
+ (RegImmInst PPR:$gp, GPR64:$base, (i64 0))>;

What if the passthru isn't undef?



Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.h:151
+  bool isLegalMaskedLoad(Type *DataType) {
+return ST->hasSVE();
+  }

This can handle all masked loads? Of any type, extended into any other type, 
with any alignment?



Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.h:153
+  }
+  bool isLegalMaskedStore(Type *DataType) {
+return ST->hasSVE();

This patch doesn't handle stores yet.



Comment at: llvm/lib/Target/AArch64/SVEInstrFormats.td:296
 
+def SVEUndef : ComplexPattern;
+

Can this just use "undef"?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68877/new/

https://reviews.llvm.org/D68877



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

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen(), size() or equal length

2019-10-13 Thread Csaba Dabis via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG82f8f8b44cd0: [clang-tidy] New checker for not 
null-terminated result caused by strlen()… (authored by Charusso).

Changed prior to commit:
  https://reviews.llvm.org/D45050?vs=221096=224773#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D45050/new/

https://reviews.llvm.org/D45050

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/bugprone-not-null-terminated-result.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/Inputs/bugprone-not-null-terminated-result/not-null-terminated-result-c.h
  
clang-tools-extra/test/clang-tidy/Inputs/bugprone-not-null-terminated-result/not-null-terminated-result-cxx.h
  
clang-tools-extra/test/clang-tidy/bugprone-not-null-terminated-result-in-initialization-strlen.c
  
clang-tools-extra/test/clang-tidy/bugprone-not-null-terminated-result-memcpy-before-safe.c
  
clang-tools-extra/test/clang-tidy/bugprone-not-null-terminated-result-memcpy-safe-cxx.cpp
  
clang-tools-extra/test/clang-tidy/bugprone-not-null-terminated-result-memcpy-safe-other.c
  
clang-tools-extra/test/clang-tidy/bugprone-not-null-terminated-result-memcpy-safe.c
  clang-tools-extra/test/clang-tidy/bugprone-not-null-terminated-result-strlen.c
  
clang-tools-extra/test/clang-tidy/bugprone-not-null-terminated-result-wcslen.cpp
  
clang-tools-extra/test/clang-tidy/bugprone-not-null-terminated-result-wmemcpy-safe-cxx.cpp

Index: clang-tools-extra/test/clang-tidy/bugprone-not-null-terminated-result-wmemcpy-safe-cxx.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/bugprone-not-null-terminated-result-wmemcpy-safe-cxx.cpp
@@ -0,0 +1,111 @@
+// RUN: %check_clang_tidy %s bugprone-not-null-terminated-result %t -- \
+// RUN: -- -std=c++11 -I %S/Inputs/bugprone-not-null-terminated-result
+
+#include "not-null-terminated-result-cxx.h"
+
+#define __STDC_LIB_EXT1__ 1
+#define __STDC_WANT_LIB_EXT1__ 1
+
+//===--===//
+// wmemcpy() - destination array tests
+//===--===//
+
+void bad_wmemcpy_known_dest(const wchar_t *src) {
+  wchar_t dest01[13];
+  wmemcpy(dest01, src, wcslen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: the result from calling 'wmemcpy' is not null-terminated [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: wchar_t dest01[14];
+  // CHECK-FIXES-NEXT: wcscpy_s(dest01, src);
+}
+
+void good_wmemcpy_known_dest(const wchar_t *src) {
+  wchar_t dst01[14];
+  wcscpy_s(dst01, src);
+}
+
+//===--===//
+// wmemcpy() - length tests
+//===--===//
+
+void bad_wmemcpy_full_source_length(const wchar_t *src) {
+  wchar_t dest20[13];
+  wmemcpy(dest20, src, wcslen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: the result from calling 'wmemcpy' is not null-terminated [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: wchar_t dest20[14];
+  // CHECK-FIXES-NEXT: wcscpy_s(dest20, src);
+}
+
+void good_wmemcpy_full_source_length(const wchar_t *src) {
+  wchar_t dst20[14];
+  wcscpy_s(dst20, src);
+}
+
+void bad_wmemcpy_partial_source_length(const wchar_t *src) {
+  wchar_t dest21[13];
+  wmemcpy(dest21, src, wcslen(src) - 1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: the result from calling 'wmemcpy' is not null-terminated [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: wchar_t dest21[14];
+  // CHECK-FIXES-NEXT: wcsncpy_s(dest21, src, wcslen(src) - 1);
+}
+
+void good_wmemcpy_partial_source_length(const wchar_t *src) {
+  wchar_t dst21[14];
+  wcsncpy_s(dst21, src, wcslen(src) - 1);
+}
+
+//===--===//
+// wmemcpy_s() - destination array tests
+//===--===//
+
+void bad_wmemcpy_s_unknown_dest(wchar_t *dest40, const wchar_t *src) {
+  wmemcpy_s(dest40, 13, src, wcslen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: the result from calling 'wmemcpy_s' is not null-terminated [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: wcscpy_s(dest40, 13, src);
+}
+
+void good_wmemcpy_s_unknown_dest(wchar_t *dst40, const wchar_t *src) {
+  wcscpy_s(dst40, 13, src);
+}
+
+void bad_wmemcpy_s_known_dest(const wchar_t *src) {
+  wchar_t dest41[13];
+  wmemcpy_s(dest41, 13, src, wcslen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning:

[clang-tools-extra] r374707 - [clang-tidy] New checker for not null-terminated result caused by strlen(), size() or equal length

2019-10-13 Thread Csaba Dabis via cfe-commits

Author: charusso
Date: Sun Oct 13 01:28:27 2019
New Revision: 374707

URL: http://llvm.org/viewvc/llvm-project?rev=374707=rev
Log:
[clang-tidy] New checker for not null-terminated result caused by strlen(), 
size() or equal length

Summary:
New checker called bugprone-not-null-terminated-result. This checker finds
function calls where it is possible to cause a not null-terminated result.
Usually the proper length of a string is `strlen(src) + 1` or equal length
of this expression, because the null terminator needs an extra space.
Without the null terminator it can result in undefined behaviour when the
string is read.

The following and their respective `wchar_t` based functions are checked:

`memcpy`, `memcpy_s`, `memchr`, `memmove`, `memmove_s`, `strerror_s`,
`strncmp`, `strxfrm`

The following is a real-world example where the programmer forgot to
increase the passed third argument, which is `size_t length`.
That is why the length of the allocated memory is not enough to hold the
null terminator.

```
static char *stringCpy(const std::string ) {
  char *result = reinterpret_cast(malloc(str.size()));
  memcpy(result, str.data(), str.size());
  return result;
}
```

In addition to issuing warnings, fix-it rewrites all the necessary code.
It also tries to adjust the capacity of the destination array:

```
static char *stringCpy(const std::string ) {
  char *result = reinterpret_cast(malloc(str.size() + 1));
  strcpy(result, str.data());
  return result;
}
```

Note: It cannot guarantee to rewrite every of the path-sensitive memory
allocations.

Reviewed By: JonasToth, aaron.ballman, whisperity, alexfh

Tags: #clang-tools-extra, #clang

Differential Revision: https://reviews.llvm.org/D45050

Added:
clang-tools-extra/trunk/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
clang-tools-extra/trunk/clang-tidy/bugprone/NotNullTerminatedResultCheck.h

clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-not-null-terminated-result.rst
clang-tools-extra/trunk/test/clang-tidy/Inputs/

clang-tools-extra/trunk/test/clang-tidy/Inputs/bugprone-not-null-terminated-result/

clang-tools-extra/trunk/test/clang-tidy/Inputs/bugprone-not-null-terminated-result/not-null-terminated-result-c.h

clang-tools-extra/trunk/test/clang-tidy/Inputs/bugprone-not-null-terminated-result/not-null-terminated-result-cxx.h

clang-tools-extra/trunk/test/clang-tidy/bugprone-not-null-terminated-result-in-initialization-strlen.c

clang-tools-extra/trunk/test/clang-tidy/bugprone-not-null-terminated-result-memcpy-before-safe.c

clang-tools-extra/trunk/test/clang-tidy/bugprone-not-null-terminated-result-memcpy-safe-cxx.cpp

clang-tools-extra/trunk/test/clang-tidy/bugprone-not-null-terminated-result-memcpy-safe-other.c

clang-tools-extra/trunk/test/clang-tidy/bugprone-not-null-terminated-result-memcpy-safe.c

clang-tools-extra/trunk/test/clang-tidy/bugprone-not-null-terminated-result-strlen.c

clang-tools-extra/trunk/test/clang-tidy/bugprone-not-null-terminated-result-wcslen.cpp

clang-tools-extra/trunk/test/clang-tidy/bugprone-not-null-terminated-result-wmemcpy-safe-cxx.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp
clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt
clang-tools-extra/trunk/docs/ReleaseNotes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst

Modified: clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp?rev=374707=374706=374707=diff
==
--- clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp Sun Oct 
13 01:28:27 2019
@@ -32,6 +32,7 @@
 #include "MisplacedWideningCastCheck.h"
 #include "MoveForwardingReferenceCheck.h"
 #include "MultipleStatementMacroCheck.h"
+#include "NotNullTerminatedResultCheck.h"
 #include "ParentVirtualCallCheck.h"
 #include "PosixReturnCheck.h"
 #include "SizeofContainerCheck.h"
@@ -109,6 +110,8 @@ public:
 "bugprone-multiple-statement-macro");
 CheckFactories.registerCheck(
 "bugprone-narrowing-conversions");
+CheckFactories.registerCheck(
+"bugprone-not-null-terminated-result");
 CheckFactories.registerCheck(
 "bugprone-parent-virtual-call");
 CheckFactories.registerCheck(

Modified: clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt?rev=374707=374706=374707=diff
==
--- clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt Sun Oct 13 
01:28:27

62 matches

Mail list logo