[PATCH] D111490: [DOCS] Update ninja build doc (new: github link, build command, and chmod requirements)

2021-10-09 Thread Shivam Gupta via Phabricator via cfe-commits
xgupta added a reviewer: Mordante.
xgupta added a subscriber: Mordante.
xgupta added a comment.

The patch is not applying cleanly so can't be committed. please follow the 
above phabricator docs I suggested before to upload it.

add @Mordante for libcxx change.


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

https://reviews.llvm.org/D111490

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


[PATCH] D108441: [clang] Fix JSON AST output when a filter is used

2021-10-09 Thread Shivam Gupta via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG451d0596d706: [clang] Fix JSON AST output when a filter is 
used (authored by woodruffw, committed by xgupta).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108441

Files:
  clang/lib/Frontend/ASTConsumers.cpp
  clang/test/AST/ast-dump-comment-json.cpp
  clang/test/AST/ast-dump-decl-context-json.cpp
  clang/test/AST/ast-dump-decl-json.c
  clang/test/AST/ast-dump-decl-json.m
  clang/test/AST/ast-dump-enum-json.cpp
  clang/test/AST/ast-dump-expr-json.c
  clang/test/AST/ast-dump-expr-json.cpp
  clang/test/AST/ast-dump-expr-json.m
  clang/test/AST/ast-dump-file-line-json.c
  clang/test/AST/ast-dump-funcs-json.cpp
  clang/test/AST/ast-dump-if-json.cpp
  clang/test/AST/ast-dump-macro-json.c
  clang/test/AST/ast-dump-namespace-json.cpp
  clang/test/AST/ast-dump-record-definition-data-json.cpp
  clang/test/AST/ast-dump-records-json.cpp
  clang/test/AST/ast-dump-stmt-json.c
  clang/test/AST/ast-dump-stmt-json.cpp
  clang/test/AST/ast-dump-stmt-json.m
  clang/test/AST/ast-dump-template-decls-json.cpp
  clang/test/AST/ast-dump-temporaries-json.cpp
  clang/test/AST/ast-dump-types-json.cpp
  clang/test/AST/gen_ast_dump_json_test.py

Index: clang/test/AST/gen_ast_dump_json_test.py
===
--- clang/test/AST/gen_ast_dump_json_test.py
+++ clang/test/AST/gen_ast_dump_json_test.py
@@ -1,4 +1,4 @@
-#!/usr/bin/env python
+#!/usr/bin/env python3
 
 from __future__ import print_function
 from collections import OrderedDict
@@ -6,7 +6,6 @@
 import argparse
 import json
 import os
-import pprint
 import re
 import subprocess
 import sys
@@ -21,20 +20,19 @@
 for e in v:
 if isinstance(e, OrderedDict):
 normalize(e)
-elif type(v) is unicode:
-st = v.encode('utf-8')
+elif type(v) is str:
 if v != "0x0" and re.match(r"0x[0-9A-Fa-f]+", v):
-dict_var[k] = u'0x{{.*}}'
+dict_var[k] = '0x{{.*}}'
 elif os.path.isfile(v):
-dict_var[k] = u'{{.*}}'
+dict_var[k] = '{{.*}}'
 else:
-splits = (v.split(u' '))
+splits = (v.split(' '))
 out_splits = []
 for split in splits:
-inner_splits = split.rsplit(u':',2)
+inner_splits = split.rsplit(':',2)
 if os.path.isfile(inner_splits[0]):
 out_splits.append(
-u'{{.*}}:%s:%s'
+'{{.*}}:%s:%s'
 %(inner_splits[1],
   inner_splits[2]))
 continue
@@ -42,11 +40,11 @@
 
 dict_var[k] = ' '.join(out_splits)
 
+
 def filter_json(dict_var, filters, out):
 for k, v in dict_var.items():
-if type(v) is unicode:
-st = v.encode('utf-8')
-if st in filters:
+if type(v) is str:
+if v in filters:
 out.append(dict_var)
 break
 elif isinstance(v, OrderedDict):
@@ -154,33 +152,39 @@
 print("Will use the following filters:", filters)
 
 try:
-json_str = subprocess.check_output(cmd)
+json_str = subprocess.check_output(cmd).decode()
 except Exception as ex:
 print("The clang command failed with %s" % ex)
 return -1
 
 out_asts = []
 if using_ast_dump_filter:
-splits = re.split('Dumping .*:\n', json_str)
-if len(splits) > 1:
-for split in splits[1:]:
-j = json.loads(split.decode('utf-8'), object_pairs_hook=OrderedDict)
+# If we're using a filter, then we might have multiple JSON objects
+# in the output. To parse each out, we use a manual JSONDecoder in
+# "raw" mode and update our location in the string based on where the
+# last document ended.
+decoder = json.JSONDecoder(object_hook=OrderedDict)
+doc_start = 0
+prev_end = 0
+while True:
+try:
+prev_end = doc_start
+(j, doc_start) = decoder.raw_decode(json_str[doc_start:])
+doc_start += prev_end + 1
 normalize(j)
 out_asts.append(j)
+except:
+break
 else:
-j = json.loads(json_str.decode('utf-8'), object_pairs_hook=OrderedDict)
+j = json.loads(json_str, object_pairs_hook=OrderedDict)
 normalize(j)
 
 if len(filters) == 0:
 out_asts.append(j)
 else:
-#assert using_ast_dump_filter is False,\
-#"Does not support using compiler's ast-dump-filter "\
-#"and the tool's filter option at the same time yet."
-
 

[clang] 451d059 - [clang] Fix JSON AST output when a filter is used

2021-10-09 Thread Shivam Gupta via cfe-commits

Author: william woodruff
Date: 2021-10-10T07:46:17+05:30
New Revision: 451d0596d70689190b5ac911ae3ab9fc4c1d7485

URL: 
https://github.com/llvm/llvm-project/commit/451d0596d70689190b5ac911ae3ab9fc4c1d7485
DIFF: 
https://github.com/llvm/llvm-project/commit/451d0596d70689190b5ac911ae3ab9fc4c1d7485.diff

LOG: [clang] Fix JSON AST output when a filter is used

Without this, the combination of `-ast-dump=json` and `-ast-dump-filter FILTER` 
produces invalid JSON: the first line is a string that says `Dumping 
$SOME_DECL_NAME: `.

Reviewed By: aaron.ballman

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

Added: 


Modified: 
clang/lib/Frontend/ASTConsumers.cpp
clang/test/AST/ast-dump-comment-json.cpp
clang/test/AST/ast-dump-decl-context-json.cpp
clang/test/AST/ast-dump-decl-json.c
clang/test/AST/ast-dump-decl-json.m
clang/test/AST/ast-dump-enum-json.cpp
clang/test/AST/ast-dump-expr-json.c
clang/test/AST/ast-dump-expr-json.cpp
clang/test/AST/ast-dump-expr-json.m
clang/test/AST/ast-dump-file-line-json.c
clang/test/AST/ast-dump-funcs-json.cpp
clang/test/AST/ast-dump-if-json.cpp
clang/test/AST/ast-dump-macro-json.c
clang/test/AST/ast-dump-namespace-json.cpp
clang/test/AST/ast-dump-record-definition-data-json.cpp
clang/test/AST/ast-dump-records-json.cpp
clang/test/AST/ast-dump-stmt-json.c
clang/test/AST/ast-dump-stmt-json.cpp
clang/test/AST/ast-dump-stmt-json.m
clang/test/AST/ast-dump-template-decls-json.cpp
clang/test/AST/ast-dump-temporaries-json.cpp
clang/test/AST/ast-dump-types-json.cpp
clang/test/AST/gen_ast_dump_json_test.py

Removed: 




diff  --git a/clang/lib/Frontend/ASTConsumers.cpp 
b/clang/lib/Frontend/ASTConsumers.cpp
index a73cc8876d5d8..96f5926c0d7ed 100644
--- a/clang/lib/Frontend/ASTConsumers.cpp
+++ b/clang/lib/Frontend/ASTConsumers.cpp
@@ -57,8 +57,11 @@ namespace {
 bool ShowColors = Out.has_colors();
 if (ShowColors)
   Out.changeColor(raw_ostream::BLUE);
-Out << (OutputKind != Print ? "Dumping " : "Printing ") << getName(D)
-<< ":\n";
+
+if (OutputFormat == ADOF_Default)
+  Out << (OutputKind != Print ? "Dumping " : "Printing ") << getName(D)
+  << ":\n";
+
 if (ShowColors)
   Out.resetColor();
 print(D);

diff  --git a/clang/test/AST/ast-dump-comment-json.cpp 
b/clang/test/AST/ast-dump-comment-json.cpp
index 2263ed8a170ff..5787fd7ae4bf1 100644
--- a/clang/test/AST/ast-dump-comment-json.cpp
+++ b/clang/test/AST/ast-dump-comment-json.cpp
@@ -38,7 +38,7 @@ void Test_TemplatedFunctionVariadic(int arg, ...);
 // NOTE: CHECK lines have been autogenerated by gen_ast_dump_json_test.py
 // using --filters=FullComment
 
-
+// CHECK-NOT: {{^}}Dumping
 // CHECK:  "kind": "FullComment",
 // CHECK-NEXT:  "loc": {
 // CHECK-NEXT:   "offset": 72,
@@ -107,7 +107,7 @@ void Test_TemplatedFunctionVariadic(int arg, ...);
 // CHECK-NEXT:  ]
 // CHECK-NEXT: }
 
-
+// CHECK-NOT: {{^}}Dumping
 // CHECK:  "kind": "FullComment",
 // CHECK-NEXT:  "loc": {},
 // CHECK-NEXT:  "range": {
@@ -116,7 +116,7 @@ void Test_TemplatedFunctionVariadic(int arg, ...);
 // CHECK-NEXT:  }
 // CHECK-NEXT: }
 
-
+// CHECK-NOT: {{^}}Dumping
 // CHECK:  "kind": "FullComment",
 // CHECK-NEXT:  "loc": {
 // CHECK-NEXT:   "offset": 120,
@@ -185,7 +185,7 @@ void Test_TemplatedFunctionVariadic(int arg, ...);
 // CHECK-NEXT:  ]
 // CHECK-NEXT: }
 
-
+// CHECK-NOT: {{^}}Dumping
 // CHECK:  "kind": "FullComment",
 // CHECK-NEXT:  "loc": {
 // CHECK-NEXT:   "offset": 151,
@@ -323,7 +323,7 @@ void Test_TemplatedFunctionVariadic(int arg, ...);
 // CHECK-NEXT:  ]
 // CHECK-NEXT: }
 
-
+// CHECK-NOT: {{^}}Dumping
 // CHECK:  "kind": "FullComment",
 // CHECK-NEXT:  "loc": {
 // CHECK-NEXT:   "offset": 197,
@@ -564,7 +564,7 @@ void Test_TemplatedFunctionVariadic(int arg, ...);
 // CHECK-NEXT:  ]
 // CHECK-NEXT: }
 
-
+// CHECK-NOT: {{^}}Dumping
 // CHECK:  "kind": "FullComment",
 // CHECK-NEXT:  "loc": {
 // CHECK-NEXT:   "offset": 294,
@@ -702,7 +702,7 @@ void Test_TemplatedFunctionVariadic(int arg, ...);
 // CHECK-NEXT:  ]
 // CHECK-NEXT: }
 
-
+// CHECK-NOT: {{^}}Dumping
 // CHECK:  "kind": "FullComment",
 // CHECK-NEXT:  "loc": {
 // CHECK-NEXT:   "offset": 294,
@@ -842,7 +842,7 @@ void Test_TemplatedFunctionVariadic(int arg, ...);
 // CHECK-NEXT:  ]
 // CHECK-NEXT: }
 
-
+// CHECK-NOT: {{^}}Dumping
 // CHECK:  "kind": "FullComment",
 // CHECK-NEXT:  "loc": {
 // CHECK-NEXT:   "offset": 372,
@@ -937,7 +937,7 @@ void Test_TemplatedFunctionVariadic(int arg, ...);
 // CHECK-NEXT:  ]
 // CHECK-NEXT: }
 
-
+// CHECK-NOT: {{^}}Dumping
 // CHECK:  "kind": "FullComment",
 // CHECK-NEXT:  "loc": {
 // CHECK-NEXT:   "offset": 415,
@@ -1122,7 +1122,7 @@ void Test_TemplatedFunctionVariadic(int arg, ...);
 // CHECK-NEXT:  ]
 // CHECK-NEXT: }
 
-
+// CHECK-NOT: {{^}}Dumping
 // 

[PATCH] D111488: [Clang][clang-nvlink-wrapper] Pass nvlink path to the wrapper

2021-10-09 Thread Michael Kruse via Phabricator via cfe-commits
Meinersbur requested changes to this revision.
Meinersbur added a comment.
This revision now requires changes to proceed.

Thanks for the path, but command line parsing should be done properly.




Comment at: clang/tools/clang-nvlink-wrapper/ClangNvlinkWrapper.cpp:50
+static cl::opt
+NvlinkUserPath("path", cl::desc("path of directory containing nvlink"),
+   cl::cat(ClangNvlinkWrapperCategory));

I suggest a more descriptive flag, `path` is too general, like `--nvlink` or 
`--nvlink-command` (bugpoint uses `--opt-command` for the path to `opt`)



Comment at: clang/tools/clang-nvlink-wrapper/ClangNvlinkWrapper.cpp:124
   sys::PrintStackTraceOnErrorSignal(argv[0]);
-
   if (Help) {

[nit] unrelated whitespace change



Comment at: clang/tools/clang-nvlink-wrapper/ClangNvlinkWrapper.cpp:133
   sys::PrintStackTraceOnErrorSignal(argv[0]);
-
   if (Help) {
 cl::PrintHelpMessage();

`cl::ParseCommandLineOptions(argc, argv,"Program description")` must have been 
called beforehand to make this work.



Comment at: clang/tools/clang-nvlink-wrapper/ClangNvlinkWrapper.cpp:154
+StringRef ArgRef(Arg);
+auto NvlPath = ArgRef.startswith_insensitive("--path=");
 if (sys::path::extension(Arg) == ".a") {

This way of parsing does not use the declared `NvlinkUserPath` option. Use 
`cl::ParseCommandLineOptions` do parse the arguments and 
```
cl::list InputFiles(cl::Positional, cl::desc("..."));
```
to catch all non-option arguments.

The way it is done here also does not catch the `--path ` (without 
equal sign) syntax.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111488

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


[PATCH] D111208: [clang-tidy] Support globbing in NOLINT* expressions

2021-10-09 Thread Salman Javed via Phabricator via cfe-commits
salman-javed-nz added a comment.

In D111208#3053370 , @carlosgalvezp 
wrote:

> By the way, is `NOLINTBEGIN/END` expected to work/give errors when the check 
> name is something else than a real check name? See for example:
>
> https://godbolt.org/z/b6cbTeezs
>
> I would expect to get a warning that an `END` was found that didn't match a 
> `BEGIN`.

The functions that we have been discussing in this patch only run when 
Clang-Tidy has detected a check violation and is about to print a warning about 
it. The `NOLINT` comment checks happen at the last minute. Since `foo` and 
`bar` are not real checks, Clang-Tidy won't find any violations, and so the 
`NOLINT` checks won't get the opportunity to run.


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

https://reviews.llvm.org/D111208

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


[PATCH] D108441: [clang] Fix JSON AST output when a filter is used

2021-10-09 Thread William Woodruff via Phabricator via cfe-commits
woodruffw updated this revision to Diff 378476.
woodruffw added a comment.

Resolve conflicts.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108441

Files:
  clang/lib/Frontend/ASTConsumers.cpp
  clang/test/AST/ast-dump-comment-json.cpp
  clang/test/AST/ast-dump-decl-context-json.cpp
  clang/test/AST/ast-dump-decl-json.c
  clang/test/AST/ast-dump-decl-json.m
  clang/test/AST/ast-dump-enum-json.cpp
  clang/test/AST/ast-dump-expr-json.c
  clang/test/AST/ast-dump-expr-json.cpp
  clang/test/AST/ast-dump-expr-json.m
  clang/test/AST/ast-dump-file-line-json.c
  clang/test/AST/ast-dump-funcs-json.cpp
  clang/test/AST/ast-dump-if-json.cpp
  clang/test/AST/ast-dump-macro-json.c
  clang/test/AST/ast-dump-namespace-json.cpp
  clang/test/AST/ast-dump-record-definition-data-json.cpp
  clang/test/AST/ast-dump-records-json.cpp
  clang/test/AST/ast-dump-stmt-json.c
  clang/test/AST/ast-dump-stmt-json.cpp
  clang/test/AST/ast-dump-stmt-json.m
  clang/test/AST/ast-dump-template-decls-json.cpp
  clang/test/AST/ast-dump-temporaries-json.cpp
  clang/test/AST/ast-dump-types-json.cpp
  clang/test/AST/gen_ast_dump_json_test.py

Index: clang/test/AST/gen_ast_dump_json_test.py
===
--- clang/test/AST/gen_ast_dump_json_test.py
+++ clang/test/AST/gen_ast_dump_json_test.py
@@ -1,4 +1,4 @@
-#!/usr/bin/env python
+#!/usr/bin/env python3
 
 from __future__ import print_function
 from collections import OrderedDict
@@ -6,7 +6,6 @@
 import argparse
 import json
 import os
-import pprint
 import re
 import subprocess
 import sys
@@ -21,20 +20,19 @@
 for e in v:
 if isinstance(e, OrderedDict):
 normalize(e)
-elif type(v) is unicode:
-st = v.encode('utf-8')
+elif type(v) is str:
 if v != "0x0" and re.match(r"0x[0-9A-Fa-f]+", v):
-dict_var[k] = u'0x{{.*}}'
+dict_var[k] = '0x{{.*}}'
 elif os.path.isfile(v):
-dict_var[k] = u'{{.*}}'
+dict_var[k] = '{{.*}}'
 else:
-splits = (v.split(u' '))
+splits = (v.split(' '))
 out_splits = []
 for split in splits:
-inner_splits = split.rsplit(u':',2)
+inner_splits = split.rsplit(':',2)
 if os.path.isfile(inner_splits[0]):
 out_splits.append(
-u'{{.*}}:%s:%s'
+'{{.*}}:%s:%s'
 %(inner_splits[1],
   inner_splits[2]))
 continue
@@ -42,11 +40,11 @@
 
 dict_var[k] = ' '.join(out_splits)
 
+
 def filter_json(dict_var, filters, out):
 for k, v in dict_var.items():
-if type(v) is unicode:
-st = v.encode('utf-8')
-if st in filters:
+if type(v) is str:
+if v in filters:
 out.append(dict_var)
 break
 elif isinstance(v, OrderedDict):
@@ -154,33 +152,39 @@
 print("Will use the following filters:", filters)
 
 try:
-json_str = subprocess.check_output(cmd)
+json_str = subprocess.check_output(cmd).decode()
 except Exception as ex:
 print("The clang command failed with %s" % ex)
 return -1
 
 out_asts = []
 if using_ast_dump_filter:
-splits = re.split('Dumping .*:\n', json_str)
-if len(splits) > 1:
-for split in splits[1:]:
-j = json.loads(split.decode('utf-8'), object_pairs_hook=OrderedDict)
+# If we're using a filter, then we might have multiple JSON objects
+# in the output. To parse each out, we use a manual JSONDecoder in
+# "raw" mode and update our location in the string based on where the
+# last document ended.
+decoder = json.JSONDecoder(object_hook=OrderedDict)
+doc_start = 0
+prev_end = 0
+while True:
+try:
+prev_end = doc_start
+(j, doc_start) = decoder.raw_decode(json_str[doc_start:])
+doc_start += prev_end + 1
 normalize(j)
 out_asts.append(j)
+except:
+break
 else:
-j = json.loads(json_str.decode('utf-8'), object_pairs_hook=OrderedDict)
+j = json.loads(json_str, object_pairs_hook=OrderedDict)
 normalize(j)
 
 if len(filters) == 0:
 out_asts.append(j)
 else:
-#assert using_ast_dump_filter is False,\
-#"Does not support using compiler's ast-dump-filter "\
-#"and the tool's filter option at the same time yet."
-
 filter_json(j, filters, out_asts)
 
-with tempfile.NamedTemporaryFile("wb", 

[PATCH] D111482: [Clang] [PowerPC] Fix header include typo in smmintrin.h

2021-10-09 Thread Jinsong Ji via Phabricator via cfe-commits
jsji accepted this revision as: jsji.
jsji added a comment.
This revision is now accepted and ready to land.

Can you please add a testcase that include `smmintrin.h` but use SSSE3? Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111482

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


[PATCH] D111490: [DOCS] Update ninja build doc (new: github link, build command, and chmod requirements)

2021-10-09 Thread Samuel Marks via Phabricator via cfe-commits
SamuelMarks updated this revision to Diff 378467.
SamuelMarks added a comment.

push/popd for build dirs


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

https://reviews.llvm.org/D111490

Files:
  clang/docs/HowToSetupToolingForLLVM.rst
  clang/docs/LibASTMatchersTutorial.rst
  libcxx/appveyor-reqs-install.cmd


Index: libcxx/appveyor-reqs-install.cmd
===
--- libcxx/appveyor-reqs-install.cmd
+++ libcxx/appveyor-reqs-install.cmd
@@ -38,7 +38,7 @@
 :: Install Ninja
 ::###
 if NOT EXIST ninja (
-  appveyor DownloadFile 
https://github.com/ninja-build/ninja/releases/download/v1.6.0/ninja-win.zip 
-FileName ninja.zip
+  appveyor DownloadFile 
https://github.com/ninja-build/ninja/releases/download/v1.10.2/ninja-win.zip 
-FileName ninja.zip
   7z x ninja.zip -oC:\projects\deps\ninja > nul
   rm ninja.zip
 )
Index: clang/docs/LibASTMatchersTutorial.rst
===
--- clang/docs/LibASTMatchersTutorial.rst
+++ clang/docs/LibASTMatchersTutorial.rst
@@ -30,29 +30,31 @@
 .. code-block:: console
 
   cd ~/clang-llvm
-  git clone git://github.com/ninja-build/ninja.git
+  git clone https://github.com/ninja-build/ninja
+  pushd ninja
   ./configure.py --bootstrap
   sudo cp ninja /usr/local/bin/
+  popd
 
-  cd ~/clang-llvm
   git clone git://cmake.org/stage/cmake.git
   cd cmake
   git checkout next
   ./bootstrap
   make
   sudo make install
+  popd
 
 Okay. Now we'll build Clang!
 
 .. code-block:: console
 
-  cd ~/clang-llvm
-  mkdir build && cd build
+  mkdir build && pushd build
   cmake -G Ninja ../llvm -DLLVM_ENABLE_PROJECTS="clang;clang-tools-extra" 
-DLLVM_BUILD_TESTS=ON  # Enable tests; default is off.
   ninja
   ninja check   # Test LLVM only.
   ninja clang-test  # Test Clang only.
   ninja install
+  popd
 
 And we're live.
 
Index: clang/docs/HowToSetupToolingForLLVM.rst
===
--- clang/docs/HowToSetupToolingForLLVM.rst
+++ clang/docs/HowToSetupToolingForLLVM.rst
@@ -154,7 +154,7 @@
 
 .. code-block:: console
 
-  $ git clone git://github.com/ninja-build/ninja.git
+  $ git clone https://github.com/ninja-build/ninja
   $ cd ninja/
   $ ./configure.py --bootstrap
 


Index: libcxx/appveyor-reqs-install.cmd
===
--- libcxx/appveyor-reqs-install.cmd
+++ libcxx/appveyor-reqs-install.cmd
@@ -38,7 +38,7 @@
 :: Install Ninja
 ::###
 if NOT EXIST ninja (
-  appveyor DownloadFile https://github.com/ninja-build/ninja/releases/download/v1.6.0/ninja-win.zip -FileName ninja.zip
+  appveyor DownloadFile https://github.com/ninja-build/ninja/releases/download/v1.10.2/ninja-win.zip -FileName ninja.zip
   7z x ninja.zip -oC:\projects\deps\ninja > nul
   rm ninja.zip
 )
Index: clang/docs/LibASTMatchersTutorial.rst
===
--- clang/docs/LibASTMatchersTutorial.rst
+++ clang/docs/LibASTMatchersTutorial.rst
@@ -30,29 +30,31 @@
 .. code-block:: console
 
   cd ~/clang-llvm
-  git clone git://github.com/ninja-build/ninja.git
+  git clone https://github.com/ninja-build/ninja
+  pushd ninja
   ./configure.py --bootstrap
   sudo cp ninja /usr/local/bin/
+  popd
 
-  cd ~/clang-llvm
   git clone git://cmake.org/stage/cmake.git
   cd cmake
   git checkout next
   ./bootstrap
   make
   sudo make install
+  popd
 
 Okay. Now we'll build Clang!
 
 .. code-block:: console
 
-  cd ~/clang-llvm
-  mkdir build && cd build
+  mkdir build && pushd build
   cmake -G Ninja ../llvm -DLLVM_ENABLE_PROJECTS="clang;clang-tools-extra" -DLLVM_BUILD_TESTS=ON  # Enable tests; default is off.
   ninja
   ninja check   # Test LLVM only.
   ninja clang-test  # Test Clang only.
   ninja install
+  popd
 
 And we're live.
 
Index: clang/docs/HowToSetupToolingForLLVM.rst
===
--- clang/docs/HowToSetupToolingForLLVM.rst
+++ clang/docs/HowToSetupToolingForLLVM.rst
@@ -154,7 +154,7 @@
 
 .. code-block:: console
 
-  $ git clone git://github.com/ninja-build/ninja.git
+  $ git clone https://github.com/ninja-build/ninja
   $ cd ninja/
   $ ./configure.py --bootstrap
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D111490: [DOCS] Update ninja build doc (new: github link, build command, and chmod requirements)

2021-10-09 Thread Samuel Marks via Phabricator via cfe-commits
SamuelMarks added inline comments.



Comment at: clang/docs/LibASTMatchersTutorial.rst:34
-  git clone https://github.com/martine/ninja.git
-  cd ninja
-  git checkout release

xgupta wrote:
> SamuelMarks wrote:
> > xgupta wrote:
> > > why removed `cd ninja` & `git checkout release`?
> > @xgupta To match how ninja is installed everywhere else in LLVM guides. 
> > Also ninja's release branch is almost a year old, whereas its master is 5 
> > days old.
> fine, but we need to cd into ninja directory to run configure step, right?
Good point, I'll fix all of them with pushd and popd


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

https://reviews.llvm.org/D111490

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


[PATCH] D111490: [DOCS] Update ninja build doc (new: github link, build command, and chmod requirements)

2021-10-09 Thread Samuel Marks via Phabricator via cfe-commits
SamuelMarks updated this revision to Diff 378466.
SamuelMarks marked an inline comment as done.
SamuelMarks added a comment.

pushd/popd to the various build dirs


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

https://reviews.llvm.org/D111490

Files:
  clang/docs/HowToSetupToolingForLLVM.rst
  clang/docs/LibASTMatchersTutorial.rst
  libcxx/appveyor-reqs-install.cmd


Index: libcxx/appveyor-reqs-install.cmd
===
--- libcxx/appveyor-reqs-install.cmd
+++ libcxx/appveyor-reqs-install.cmd
@@ -38,7 +38,7 @@
 :: Install Ninja
 ::###
 if NOT EXIST ninja (
-  appveyor DownloadFile 
https://github.com/ninja-build/ninja/releases/download/v1.6.0/ninja-win.zip 
-FileName ninja.zip
+  appveyor DownloadFile 
https://github.com/ninja-build/ninja/releases/download/v1.10.2/ninja-win.zip 
-FileName ninja.zip
   7z x ninja.zip -oC:\projects\deps\ninja > nul
   rm ninja.zip
 )
Index: clang/docs/LibASTMatchersTutorial.rst
===
--- clang/docs/LibASTMatchersTutorial.rst
+++ clang/docs/LibASTMatchersTutorial.rst
@@ -30,29 +30,31 @@
 .. code-block:: console
 
   cd ~/clang-llvm
-  git clone git://github.com/ninja-build/ninja.git
+  git clone https://github.com/ninja-build/ninja
+  pushd ninja
   ./configure.py --bootstrap
   sudo cp ninja /usr/local/bin/
+  popd
 
-  cd ~/clang-llvm
   git clone git://cmake.org/stage/cmake.git
   cd cmake
   git checkout next
   ./bootstrap
   make
   sudo make install
+  popd
 
 Okay. Now we'll build Clang!
 
 .. code-block:: console
 
-  cd ~/clang-llvm
-  mkdir build && cd build
+  mkdir build && pushd build
   cmake -G Ninja ../llvm -DLLVM_ENABLE_PROJECTS="clang;clang-tools-extra" 
-DLLVM_BUILD_TESTS=ON  # Enable tests; default is off.
   ninja
   ninja check   # Test LLVM only.
   ninja clang-test  # Test Clang only.
   ninja install
+  popd
 
 And we're live.
 
Index: clang/docs/HowToSetupToolingForLLVM.rst
===
--- clang/docs/HowToSetupToolingForLLVM.rst
+++ clang/docs/HowToSetupToolingForLLVM.rst
@@ -154,7 +154,7 @@
 
 .. code-block:: console
 
-  $ git clone git://github.com/ninja-build/ninja.git
+  $ git clone https://github.com/ninja-build/ninja
   $ cd ninja/
   $ ./configure.py --bootstrap
 


Index: libcxx/appveyor-reqs-install.cmd
===
--- libcxx/appveyor-reqs-install.cmd
+++ libcxx/appveyor-reqs-install.cmd
@@ -38,7 +38,7 @@
 :: Install Ninja
 ::###
 if NOT EXIST ninja (
-  appveyor DownloadFile https://github.com/ninja-build/ninja/releases/download/v1.6.0/ninja-win.zip -FileName ninja.zip
+  appveyor DownloadFile https://github.com/ninja-build/ninja/releases/download/v1.10.2/ninja-win.zip -FileName ninja.zip
   7z x ninja.zip -oC:\projects\deps\ninja > nul
   rm ninja.zip
 )
Index: clang/docs/LibASTMatchersTutorial.rst
===
--- clang/docs/LibASTMatchersTutorial.rst
+++ clang/docs/LibASTMatchersTutorial.rst
@@ -30,29 +30,31 @@
 .. code-block:: console
 
   cd ~/clang-llvm
-  git clone git://github.com/ninja-build/ninja.git
+  git clone https://github.com/ninja-build/ninja
+  pushd ninja
   ./configure.py --bootstrap
   sudo cp ninja /usr/local/bin/
+  popd
 
-  cd ~/clang-llvm
   git clone git://cmake.org/stage/cmake.git
   cd cmake
   git checkout next
   ./bootstrap
   make
   sudo make install
+  popd
 
 Okay. Now we'll build Clang!
 
 .. code-block:: console
 
-  cd ~/clang-llvm
-  mkdir build && cd build
+  mkdir build && pushd build
   cmake -G Ninja ../llvm -DLLVM_ENABLE_PROJECTS="clang;clang-tools-extra" -DLLVM_BUILD_TESTS=ON  # Enable tests; default is off.
   ninja
   ninja check   # Test LLVM only.
   ninja clang-test  # Test Clang only.
   ninja install
+  popd
 
 And we're live.
 
Index: clang/docs/HowToSetupToolingForLLVM.rst
===
--- clang/docs/HowToSetupToolingForLLVM.rst
+++ clang/docs/HowToSetupToolingForLLVM.rst
@@ -154,7 +154,7 @@
 
 .. code-block:: console
 
-  $ git clone git://github.com/ninja-build/ninja.git
+  $ git clone https://github.com/ninja-build/ninja
   $ cd ninja/
   $ ./configure.py --bootstrap
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D105191: [Clang][OpenMP] Add partial support for Static Device Libraries

2021-10-09 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments.



Comment at: clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp:120
+  /* bitcode SDL?*/ true,
+  /* PostClang Link? */ false);
   // Add an intermediate output file.

This file now fails clang-format


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105191

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


[clang] 3019898 - [clang-format][NFC] improve the visual of the "clang-formatted %"

2021-10-09 Thread via cfe-commits

Author: mydeveloperday
Date: 2021-10-09T19:37:03+01:00
New Revision: 3019898e0d1b494b7e7e76790adb3d83eff4aca1

URL: 
https://github.com/llvm/llvm-project/commit/3019898e0d1b494b7e7e76790adb3d83eff4aca1
DIFF: 
https://github.com/llvm/llvm-project/commit/3019898e0d1b494b7e7e76790adb3d83eff4aca1.diff

LOG: [clang-format][NFC] improve the visual of the "clang-formatted %"

NOTE: some files are being removed from those files that are clang-formatted
which means some lack of formatting is slipping through the net on reviews

Added: 


Modified: 
clang/docs/ClangFormattedStatus.rst
clang/docs/tools/clang-formatted-files.txt
clang/docs/tools/generate_formatted_state.py

Removed: 




diff  --git a/clang/docs/ClangFormattedStatus.rst 
b/clang/docs/ClangFormattedStatus.rst
index d9af218518509..106fea916e03f 100644
--- a/clang/docs/ClangFormattedStatus.rst
+++ b/clang/docs/ClangFormattedStatus.rst
@@ -1,10 +1,10 @@
 .. raw:: html
 
   
-.none { background-color: #FFCC99 }
-.part { background-color: #99 }
-.good { background-color: #2CCCFF }
 .total { font-weight: bold; }
+.none { background-color: #99; height: 20px; display: 
inline-block; width: 120px; text-align: center; border-radius: 5px; color: 
#00; font-family="Verdana,Geneva,DejaVu Sans,sans-serif" }
+.part { background-color: #FFCC99; height: 20px; display: 
inline-block; width: 120px; text-align: center; border-radius: 5px; color: 
#00; font-family="Verdana,Geneva,DejaVu Sans,sans-serif" }
+.good { background-color: #2CCCFF; height: 20px; display: 
inline-block; width: 120px; text-align: center; border-radius: 5px; color: 
#00; font-family="Verdana,Geneva,DejaVu Sans,sans-serif" }
   
 
 .. role:: none
@@ -17,7 +17,7 @@ Clang Formatted Status
 ==
 
 :doc:`ClangFormattedStatus` describes the state of LLVM source
-tree in terms of conformance to :doc:`ClangFormat` as of: October 02, 2021 
15:06:27 (`3d209c76ddb5 
`_).
+tree in terms of conformance to :doc:`ClangFormat` as of: October 09, 2021 
17:11:06 (`3e553791caa0 
`_).
 
 
 .. list-table:: LLVM Clang-Format Status
@@ -29,11 +29,6 @@ tree in terms of conformance to :doc:`ClangFormat` as of: 
October 02, 2021 15:06
  - Formatted Files
  - Unformatted Files
  - % Complete
-   * - .
- - `1`
- - `1`
- - `0`
- - :good:`100%`
* - clang/bindings/python/tests/cindex/INPUTS
  - `5`
  - `3`
@@ -151,9 +146,9 @@ tree in terms of conformance to :doc:`ClangFormat` as of: 
October 02, 2021 15:06
  - :part:`20%`
* - clang/include/clang/Format
  - `1`
- - `0`
  - `1`
- - :none:`0%`
+ - `0`
+ - :good:`100%`
* - clang/include/clang/Frontend
  - `28`
  - `7`
@@ -256,14 +251,14 @@ tree in terms of conformance to :doc:`ClangFormat` as of: 
October 02, 2021 15:06
  - :none:`0%`
* - clang/include/clang/Tooling/DependencyScanning
  - `5`
- - `4`
- - `1`
- - :part:`80%`
+ - `5`
+ - `0`
+ - :good:`100%`
* - clang/include/clang/Tooling/Inclusions
  - `2`
- - `1`
- - `1`
- - :part:`50%`
+ - `2`
+ - `0`
+ - :good:`100%`
* - clang/include/clang/Tooling/Refactoring
  - `15`
  - `12`
@@ -401,9 +396,9 @@ tree in terms of conformance to :doc:`ClangFormat` as of: 
October 02, 2021 15:06
  - :part:`14%`
* - clang/lib/Driver/ToolChains
  - `87`
- - `33`
- - `54`
- - :part:`37%`
+ - `32`
+ - `55`
+ - :part:`36%`
* - clang/lib/Driver/ToolChains/Arch
  - `20`
  - `7`
@@ -435,10 +430,10 @@ tree in terms of conformance to :doc:`ClangFormat` as of: 
October 02, 2021 15:06
  - `1`
  - :none:`0%`
* - clang/lib/Headers
- - `144`
- - `15`
+ - `145`
+ - `16`
  - `129`
- - :part:`10%`
+ - :part:`11%`
* - clang/lib/Headers/openmp_wrappers
  - `5`
  - `5`
@@ -495,8 +490,8 @@ tree in terms of conformance to :doc:`ClangFormat` as of: 
October 02, 2021 15:06
  - `102`
  - :part:`12%`
* - clang/lib/StaticAnalyzer/Checkers/cert
- - `1`
- - `1`
+ - `2`
+ - `2`
  - `0`
  - :good:`100%`
* - clang/lib/StaticAnalyzer/Checkers/MPI-Checker
@@ -721,9 +716,9 @@ tree in terms of conformance to :doc:`ClangFormat` as of: 
October 02, 2021 15:06
  - :part:`25%`
* - clang/tools/libclang
  - `35`
- - `6`
- - `29`
- - :part:`17%`
+ - `5`
+ - `30`
+ - :part:`14%`
* - clang/tools/scan-build-py/tests/functional/src/include
  - `1`
  - `1`
@@ -841,9 +836,9 @@ tree in terms of conformance to :doc:`ClangFormat` as of: 
October 02, 2021 15:06
  - :part:`40%`
* - clang/unittests/Tooling
  - `29`
- - `7`
- 

[PATCH] D111490: [DOCS] Update ninja build doc (new: github link, build command, and chmod requirements)

2021-10-09 Thread Shivam Gupta via Phabricator via cfe-commits
xgupta added a comment.

Please update the patch full context,  see 
https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface
 if using the web interface.
(git diff HEAD~1 -U99 > mypatch.patch)


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

https://reviews.llvm.org/D111490

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


[PATCH] D111490: [DOCS] Update ninja build doc (new: github link, build command, and chmod requirements)

2021-10-09 Thread Shivam Gupta via Phabricator via cfe-commits
xgupta added inline comments.



Comment at: clang/docs/LibASTMatchersTutorial.rst:34
-  git clone https://github.com/martine/ninja.git
-  cd ninja
-  git checkout release

SamuelMarks wrote:
> xgupta wrote:
> > why removed `cd ninja` & `git checkout release`?
> @xgupta To match how ninja is installed everywhere else in LLVM guides. Also 
> ninja's release branch is almost a year old, whereas its master is 5 days old.
fine, but we need to cd into ninja directory to run configure step, right?


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

https://reviews.llvm.org/D111490

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


[PATCH] D111490: [DOCS] Update ninja build doc (new: github link, build command, and chmod requirements)

2021-10-09 Thread Samuel Marks via Phabricator via cfe-commits
SamuelMarks updated this revision to Diff 378463.
SamuelMarks added a comment.

Also updates appveyor to latest release version of ninja


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

https://reviews.llvm.org/D111490

Files:
  clang/docs/HowToSetupToolingForLLVM.rst
  clang/docs/LibASTMatchersTutorial.rst
  libcxx/appveyor-reqs-install.cmd


Index: libcxx/appveyor-reqs-install.cmd
===
--- libcxx/appveyor-reqs-install.cmd
+++ libcxx/appveyor-reqs-install.cmd
@@ -38,7 +38,7 @@
 :: Install Ninja
 ::###
 if NOT EXIST ninja (
-  appveyor DownloadFile 
https://github.com/ninja-build/ninja/releases/download/v1.6.0/ninja-win.zip 
-FileName ninja.zip
+  appveyor DownloadFile 
https://github.com/ninja-build/ninja/releases/download/v1.10.2/ninja-win.zip 
-FileName ninja.zip
   7z x ninja.zip -oC:\projects\deps\ninja > nul
   rm ninja.zip
 )
Index: clang/docs/LibASTMatchersTutorial.rst
===
--- clang/docs/LibASTMatchersTutorial.rst
+++ clang/docs/LibASTMatchersTutorial.rst
@@ -30,7 +30,7 @@
 .. code-block:: console
 
   cd ~/clang-llvm
-  git clone git://github.com/ninja-build/ninja.git
+  git clone https://github.com/ninja-build/ninja
   ./configure.py --bootstrap
   sudo cp ninja /usr/local/bin/
 
Index: clang/docs/HowToSetupToolingForLLVM.rst
===
--- clang/docs/HowToSetupToolingForLLVM.rst
+++ clang/docs/HowToSetupToolingForLLVM.rst
@@ -154,7 +154,7 @@
 
 .. code-block:: console
 
-  $ git clone git://github.com/ninja-build/ninja.git
+  $ git clone https://github.com/ninja-build/ninja
   $ cd ninja/
   $ ./configure.py --bootstrap
 


Index: libcxx/appveyor-reqs-install.cmd
===
--- libcxx/appveyor-reqs-install.cmd
+++ libcxx/appveyor-reqs-install.cmd
@@ -38,7 +38,7 @@
 :: Install Ninja
 ::###
 if NOT EXIST ninja (
-  appveyor DownloadFile https://github.com/ninja-build/ninja/releases/download/v1.6.0/ninja-win.zip -FileName ninja.zip
+  appveyor DownloadFile https://github.com/ninja-build/ninja/releases/download/v1.10.2/ninja-win.zip -FileName ninja.zip
   7z x ninja.zip -oC:\projects\deps\ninja > nul
   rm ninja.zip
 )
Index: clang/docs/LibASTMatchersTutorial.rst
===
--- clang/docs/LibASTMatchersTutorial.rst
+++ clang/docs/LibASTMatchersTutorial.rst
@@ -30,7 +30,7 @@
 .. code-block:: console
 
   cd ~/clang-llvm
-  git clone git://github.com/ninja-build/ninja.git
+  git clone https://github.com/ninja-build/ninja
   ./configure.py --bootstrap
   sudo cp ninja /usr/local/bin/
 
Index: clang/docs/HowToSetupToolingForLLVM.rst
===
--- clang/docs/HowToSetupToolingForLLVM.rst
+++ clang/docs/HowToSetupToolingForLLVM.rst
@@ -154,7 +154,7 @@
 
 .. code-block:: console
 
-  $ git clone git://github.com/ninja-build/ninja.git
+  $ git clone https://github.com/ninja-build/ninja
   $ cd ninja/
   $ ./configure.py --bootstrap
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D111490: [DOCS] Update ninja build doc (new: github link, build command, and chmod requirements)

2021-10-09 Thread Samuel Marks via Phabricator via cfe-commits
SamuelMarks added inline comments.



Comment at: clang/docs/LibASTMatchersTutorial.rst:34
-  git clone https://github.com/martine/ninja.git
-  cd ninja
-  git checkout release

xgupta wrote:
> why removed `cd ninja` & `git checkout release`?
@xgupta To match how ninja is installed everywhere else in LLVM guides. Also 
ninja's release branch is almost a year old, whereas its master is 5 days old.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111490

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


[PATCH] D111208: [clang-tidy] Support globbing in NOLINT* expressions

2021-10-09 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added a comment.

Thanks for the clarification, makes a lot of sense now! I'll see what I can do 
with that.

By the way, is `NOLINTBEGIN/END` expected to work/give errors when the check 
name is something else than a real check name? See for example:

https://godbolt.org/z/b6cbTeezs

I would expect to get a warning that an `END` was found that didn't match a 
`BEGIN`.


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

https://reviews.llvm.org/D111208

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


[PATCH] D111490: [DOCS] Update ninja build doc (new: github link, build command, and chmod requirements)

2021-10-09 Thread Shivam Gupta via Phabricator via cfe-commits
xgupta added inline comments.



Comment at: clang/docs/HowToSetupToolingForLLVM.rst:157
 
-  $ git clone git://github.com/martine/ninja.git
+  $ git clone git://github.com/ninja-build/ninja.git
   $ cd ninja/

git -> https



Comment at: clang/docs/LibASTMatchersTutorial.rst:33
   cd ~/clang-llvm
-  git clone https://github.com/martine/ninja.git
-  cd ninja
-  git checkout release
-  ./bootstrap.py
-  sudo cp ninja /usr/bin/
+  git clone git://github.com/ninja-build/ninja.git
+  ./configure.py --bootstrap

https would be the right protocol as per the consistency with other places of 
docs.



Comment at: clang/docs/LibASTMatchersTutorial.rst:34
-  git clone https://github.com/martine/ninja.git
-  cd ninja
-  git checkout release

why removed `cd ninja` & `git checkout release`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111490

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


[PATCH] D111490: [DOCS] Update ninja build doc (new: github link, build command, and chmod requirements)

2021-10-09 Thread Samuel Marks via Phabricator via cfe-commits
SamuelMarks created this revision.
SamuelMarks added a project: LLVM.
SamuelMarks requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D111490

Files:
  clang/docs/HowToSetupToolingForLLVM.rst
  clang/docs/LibASTMatchersTutorial.rst


Index: clang/docs/LibASTMatchersTutorial.rst
===
--- clang/docs/LibASTMatchersTutorial.rst
+++ clang/docs/LibASTMatchersTutorial.rst
@@ -30,11 +30,9 @@
 .. code-block:: console
 
   cd ~/clang-llvm
-  git clone https://github.com/martine/ninja.git
-  cd ninja
-  git checkout release
-  ./bootstrap.py
-  sudo cp ninja /usr/bin/
+  git clone git://github.com/ninja-build/ninja.git
+  ./configure.py --bootstrap
+  sudo cp ninja /usr/local/bin/
 
   cd ~/clang-llvm
   git clone git://cmake.org/stage/cmake.git
Index: clang/docs/HowToSetupToolingForLLVM.rst
===
--- clang/docs/HowToSetupToolingForLLVM.rst
+++ clang/docs/HowToSetupToolingForLLVM.rst
@@ -143,7 +143,7 @@
 Using Ninja Build System
 ===
 
-Optionally you can use the `Ninja `_
+Optionally you can use the `Ninja `_
 build system instead of make. It is aimed at making your builds faster.
 Currently this step will require building Ninja from sources.
 
@@ -154,9 +154,9 @@
 
 .. code-block:: console
 
-  $ git clone git://github.com/martine/ninja.git
+  $ git clone git://github.com/ninja-build/ninja.git
   $ cd ninja/
-  $ ./bootstrap.py
+  $ ./configure.py --bootstrap
 
 This will result in a single binary ``ninja`` in the current directory.
 It doesn't require installation and can just be copied to any location
@@ -165,7 +165,6 @@
 .. code-block:: console
 
   $ sudo cp ninja /usr/local/bin/
-  $ sudo chmod a+rx /usr/local/bin/ninja
 
 After doing all of this, you'll need to generate Ninja build files for
 LLVM with CMake. You need to make a build directory and run CMake from


Index: clang/docs/LibASTMatchersTutorial.rst
===
--- clang/docs/LibASTMatchersTutorial.rst
+++ clang/docs/LibASTMatchersTutorial.rst
@@ -30,11 +30,9 @@
 .. code-block:: console
 
   cd ~/clang-llvm
-  git clone https://github.com/martine/ninja.git
-  cd ninja
-  git checkout release
-  ./bootstrap.py
-  sudo cp ninja /usr/bin/
+  git clone git://github.com/ninja-build/ninja.git
+  ./configure.py --bootstrap
+  sudo cp ninja /usr/local/bin/
 
   cd ~/clang-llvm
   git clone git://cmake.org/stage/cmake.git
Index: clang/docs/HowToSetupToolingForLLVM.rst
===
--- clang/docs/HowToSetupToolingForLLVM.rst
+++ clang/docs/HowToSetupToolingForLLVM.rst
@@ -143,7 +143,7 @@
 Using Ninja Build System
 ===
 
-Optionally you can use the `Ninja `_
+Optionally you can use the `Ninja `_
 build system instead of make. It is aimed at making your builds faster.
 Currently this step will require building Ninja from sources.
 
@@ -154,9 +154,9 @@
 
 .. code-block:: console
 
-  $ git clone git://github.com/martine/ninja.git
+  $ git clone git://github.com/ninja-build/ninja.git
   $ cd ninja/
-  $ ./bootstrap.py
+  $ ./configure.py --bootstrap
 
 This will result in a single binary ``ninja`` in the current directory.
 It doesn't require installation and can just be copied to any location
@@ -165,7 +165,6 @@
 .. code-block:: console
 
   $ sudo cp ninja /usr/local/bin/
-  $ sudo chmod a+rx /usr/local/bin/ninja
 
 After doing all of this, you'll need to generate Ninja build files for
 LLVM with CMake. You need to make a build directory and run CMake from
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D111242: Add `TypeLoc`-related matchers.

2021-10-09 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D111242#3052562 , @jcking1034 
wrote:

> @aaron.ballman that sounds like a good idea, what's the process for adding a 
> release note? Does it involve editing `clang/docs/ReleaseNotes.rst`?

That's correct! And because it's not a functionality change, you don't need to 
get a review before committing those changes unless you'd like the review 
(which is also totally fine). I'd add these details under the `AST Matchers` 
section of the document.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111242

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


[PATCH] D111459: Fix a diagnoses-valid in C++20 with variadic macros

2021-10-09 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman closed this revision.
aaron.ballman added a comment.

Committed in af971365a2a8b0d982814c0652bb86844fd19cda 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111459

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


[PATCH] D99484: Use `GNUInstallDirs` to support custom installation dirs.

2021-10-09 Thread John Ericson via Phabricator via cfe-commits
Ericson2314 updated this revision to Diff 378392.
Ericson2314 added a comment.
Herald added a project: Flang.

rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99484

Files:
  clang-tools-extra/clang-doc/tool/CMakeLists.txt
  clang-tools-extra/clang-include-fixer/find-all-symbols/tool/CMakeLists.txt
  clang-tools-extra/clang-include-fixer/tool/CMakeLists.txt
  clang-tools-extra/clang-tidy/CMakeLists.txt
  clang-tools-extra/clang-tidy/tool/CMakeLists.txt
  clang-tools-extra/modularize/CMakeLists.txt
  clang/CMakeLists.txt
  clang/cmake/modules/AddClang.cmake
  clang/tools/c-index-test/CMakeLists.txt
  clang/tools/clang-format/CMakeLists.txt
  clang/tools/clang-rename/CMakeLists.txt
  clang/tools/libclang/CMakeLists.txt
  clang/tools/scan-build-py/CMakeLists.txt
  clang/tools/scan-build/CMakeLists.txt
  clang/tools/scan-view/CMakeLists.txt
  clang/utils/hmaptool/CMakeLists.txt
  compiler-rt/cmake/base-config-ix.cmake
  flang/CMakeLists.txt
  flang/cmake/modules/AddFlang.cmake
  flang/tools/f18/CMakeLists.txt
  flang/tools/flang-driver/CMakeLists.txt
  libc/CMakeLists.txt
  libcxx/CMakeLists.txt
  libcxx/cmake/Modules/HandleLibCXXABI.cmake
  libcxx/include/CMakeLists.txt
  libcxxabi/CMakeLists.txt
  libunwind/CMakeLists.txt
  lld/CMakeLists.txt
  lld/cmake/modules/AddLLD.cmake
  lld/tools/lld/CMakeLists.txt
  lldb/CMakeLists.txt
  lldb/cmake/modules/AddLLDB.cmake
  lldb/cmake/modules/LLDBConfig.cmake
  mlir/CMakeLists.txt
  mlir/cmake/modules/AddMLIR.cmake
  openmp/CMakeLists.txt
  openmp/runtime/src/CMakeLists.txt
  openmp/tools/multiplex/CMakeLists.txt
  polly/CMakeLists.txt
  polly/cmake/CMakeLists.txt
  polly/lib/External/CMakeLists.txt
  pstl/CMakeLists.txt

Index: pstl/CMakeLists.txt
===
--- pstl/CMakeLists.txt
+++ pstl/CMakeLists.txt
@@ -7,6 +7,8 @@
 #===--===##
 cmake_minimum_required(VERSION 3.13.4)
 
+include(GNUInstallDirs)
+
 set(PARALLELSTL_VERSION_FILE "${CMAKE_CURRENT_SOURCE_DIR}/include/pstl/internal/pstl_config.h")
 file(STRINGS "${PARALLELSTL_VERSION_FILE}" PARALLELSTL_VERSION_SOURCE REGEX "#define _PSTL_VERSION .*$")
 string(REGEX REPLACE "#define _PSTL_VERSION (.*)$" "\\1" PARALLELSTL_VERSION_SOURCE "${PARALLELSTL_VERSION_SOURCE}")
@@ -86,10 +88,10 @@
   "${CMAKE_CURRENT_BINARY_DIR}/ParallelSTLConfigVersion.cmake"
 DESTINATION lib/cmake/ParallelSTL)
 install(DIRECTORY include/
-DESTINATION include
+DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}"
 PATTERN "*.in" EXCLUDE)
 install(FILES "${PSTL_CONFIG_SITE_PATH}"
-DESTINATION include)
+DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}")
 
 add_custom_target(install-pstl
   COMMAND "${CMAKE_COMMAND}" -P "${PROJECT_BINARY_DIR}/cmake_install.cmake" -DCOMPONENT=ParallelSTL)
Index: polly/lib/External/CMakeLists.txt
===
--- polly/lib/External/CMakeLists.txt
+++ polly/lib/External/CMakeLists.txt
@@ -290,7 +290,7 @@
 install(DIRECTORY
   ${ISL_SOURCE_DIR}/include/
   ${ISL_BINARY_DIR}/include/
-  DESTINATION include/polly
+  DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}/polly"
   FILES_MATCHING
   PATTERN "*.h"
   PATTERN "CMakeFiles" EXCLUDE
Index: polly/cmake/CMakeLists.txt
===
--- polly/cmake/CMakeLists.txt
+++ polly/cmake/CMakeLists.txt
@@ -83,14 +83,15 @@
 set(POLLY_CONFIG_LLVM_CMAKE_DIR "${LLVM_BINARY_DIR}/${LLVM_INSTALL_PACKAGE_DIR}")
 set(POLLY_CONFIG_CMAKE_DIR "${POLLY_INSTALL_PREFIX}/${POLLY_INSTALL_PACKAGE_DIR}")
 set(POLLY_CONFIG_LIBRARY_DIRS "${POLLY_INSTALL_PREFIX}/lib${LLVM_LIBDIR_SUFFIX}")
+get_filename_component(base_includedir "${CMAKE_INSTALL_INCLUDEDIR}" ABSOLUTE BASE_DIR "${POLLY_INSTALL_PREFIX}")
 if (POLLY_BUNDLED_ISL)
   set(POLLY_CONFIG_INCLUDE_DIRS
-"${POLLY_INSTALL_PREFIX}/include"
-"${POLLY_INSTALL_PREFIX}/include/polly"
+"${base_includedir}"
+"${base_includedir}/polly"
 )
 else()
   set(POLLY_CONFIG_INCLUDE_DIRS
-"${POLLY_INSTALL_PREFIX}/include"
+"${base_includedir}"
 ${ISL_INCLUDE_DIRS}
 )
 endif()
@@ -100,12 +101,12 @@
 foreach(tgt IN LISTS POLLY_CONFIG_EXPORTED_TARGETS)
   get_target_property(tgt_type ${tgt} TYPE)
   if (tgt_type STREQUAL "EXECUTABLE")
-set(tgt_prefix "bin/")
+set(tgt_prefix "${CMAKE_INSTALL_FULL_BINDIR}/")
   else()
-set(tgt_prefix "lib/")
+set(tgt_prefix "${CMAKE_INSTALL_FULL_LIBDIR}/")
   endif()
 
-  set(tgt_path "${CMAKE_INSTALL_PREFIX}/${tgt_prefix}$")
+  set(tgt_path "${tgt_prefix}$")
   file(RELATIVE_PATH tgt_path ${POLLY_CONFIG_CMAKE_DIR} ${tgt_path})
 
   if (NOT tgt_type STREQUAL "INTERFACE_LIBRARY")
Index: polly/CMakeLists.txt
===
--- 

[clang] af97136 - Fix a diagnoses-valid in C++20 with variadic macros

2021-10-09 Thread Aaron Ballman via cfe-commits

Author: Aaron Ballman
Date: 2021-10-09T08:20:20-04:00
New Revision: af971365a2a8b0d982814c0652bb86844fd19cda

URL: 
https://github.com/llvm/llvm-project/commit/af971365a2a8b0d982814c0652bb86844fd19cda
DIFF: 
https://github.com/llvm/llvm-project/commit/af971365a2a8b0d982814c0652bb86844fd19cda.diff

LOG: Fix a diagnoses-valid in C++20 with variadic macros

C++20 and later allow you to pass no argument for the ... parameter in
a variadic macro, whereas earlier language modes and C disallow it.

We no longer diagnose in C++20 and later modes. This fixes PR51609.

Added: 
clang/test/Preprocessor/empty_va_arg.cpp

Modified: 
clang/include/clang/Basic/DiagnosticLexKinds.td
clang/lib/Lex/PPMacroExpansion.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticLexKinds.td 
b/clang/include/clang/Basic/DiagnosticLexKinds.td
index 763b7b0086369..7fbf19ed6cb3f 100644
--- a/clang/include/clang/Basic/DiagnosticLexKinds.td
+++ b/clang/include/clang/Basic/DiagnosticLexKinds.td
@@ -412,6 +412,10 @@ def ext_embedded_directive : Extension<
 def ext_missing_varargs_arg : Extension<
   "must specify at least one argument for '...' parameter of variadic macro">,
   InGroup;
+def warn_cxx17_compat_missing_varargs_arg : Warning<
+  "passing no argument for the '...' parameter of a variadic macro is "
+  "incompatible with C++ standards before C++20">,
+  InGroup, DefaultIgnore;
 def ext_empty_fnmacro_arg : Extension<
   "empty macro arguments are a C99 feature">, InGroup;
 def warn_cxx98_compat_empty_fnmacro_arg : Warning<

diff  --git a/clang/lib/Lex/PPMacroExpansion.cpp 
b/clang/lib/Lex/PPMacroExpansion.cpp
index 136f61ab9a507..7b5232d462749 100644
--- a/clang/lib/Lex/PPMacroExpansion.cpp
+++ b/clang/lib/Lex/PPMacroExpansion.cpp
@@ -988,7 +988,11 @@ MacroArgs *Preprocessor::ReadMacroCallArgumentList(Token 
,
   // If the macro contains the comma pasting extension, the diagnostic
   // is suppressed; we know we'll get another diagnostic later.
   if (!MI->hasCommaPasting()) {
-Diag(Tok, diag::ext_missing_varargs_arg);
+// C++20 allows this construct, but standards before C++20 and all C
+// standards do not allow the construct (we allow it as an extension).
+Diag(Tok, getLangOpts().CPlusPlus20
+  ? diag::warn_cxx17_compat_missing_varargs_arg
+  : diag::ext_missing_varargs_arg);
 Diag(MI->getDefinitionLoc(), diag::note_macro_here)
   << MacroName.getIdentifierInfo();
   }

diff  --git a/clang/test/Preprocessor/empty_va_arg.cpp 
b/clang/test/Preprocessor/empty_va_arg.cpp
new file mode 100644
index 0..2ee431f6bde83
--- /dev/null
+++ b/clang/test/Preprocessor/empty_va_arg.cpp
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -Eonly -std=c++17 -pedantic -verify %s
+// RUN: %clang_cc1 -Eonly -std=c17 -pedantic -verify -x c %s
+// RUN: %clang_cc1 -Eonly -std=c++20 -pedantic -Wpre-c++20-compat 
-verify=compat %s
+
+#define FOO(x, ...) // expected-note {{macro 'FOO' defined here}} \
+// compat-note {{macro 'FOO' defined here}}
+
+int main() {
+  FOO(42) // expected-warning {{must specify at least one argument for '...' 
parameter of variadic macro}} \
+  // compat-warning {{passing no argument for the '...' parameter of a 
variadic macro is incompatible with C++ standards before C++20}}
+}
+



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


[PATCH] D111488: [Clang][clang-nvlink-wrapper] Pass nvlink path to the wrapper

2021-10-09 Thread Saiyedul Islam via Phabricator via cfe-commits
saiislam created this revision.
saiislam added reviewers: Meinersbur, ye-luo, JonChesterfield.
saiislam requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: cfe-commits, sstefan1.
Herald added a project: clang.

Added support of a "--path=" option in clang-nvlink-wrapper which
takes the path of directory containing nvlink binary.

Static Device Library support for OpenMP (D105191 
) now searches for
nvlink binary and passes its location via this option. In absence
of this option, nvlink binary is searched in locations in PATH.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D111488

Files:
  clang/lib/Driver/ToolChains/Cuda.cpp
  clang/tools/clang-nvlink-wrapper/ClangNvlinkWrapper.cpp


Index: clang/tools/clang-nvlink-wrapper/ClangNvlinkWrapper.cpp
===
--- clang/tools/clang-nvlink-wrapper/ClangNvlinkWrapper.cpp
+++ clang/tools/clang-nvlink-wrapper/ClangNvlinkWrapper.cpp
@@ -41,6 +41,15 @@
 
 static cl::opt Help("h", cl::desc("Alias for -help"), cl::Hidden);
 
+// Mark all our options with this category, everything else (except for -help)
+// will be hidden.
+static cl::OptionCategory
+ClangNvlinkWrapperCategory("clang-nvlink-wrapper options");
+
+static cl::opt
+NvlinkUserPath("path", cl::desc("path of directory containing nvlink"),
+   cl::cat(ClangNvlinkWrapperCategory));
+
 static Error runNVLink(std::string NVLinkPath,
SmallVectorImpl ) {
   std::vector NVLArgs;
@@ -121,7 +130,6 @@
 
 int main(int argc, const char **argv) {
   sys::PrintStackTraceOnErrorSignal(argv[0]);
-
   if (Help) {
 cl::PrintHelpMessage();
 return 0;
@@ -132,12 +140,7 @@
 exit(1);
   };
 
-  ErrorOr NvlinkPath = sys::findProgramByName("nvlink");
-  if (!NvlinkPath) {
-reportError(createStringError(NvlinkPath.getError(),
-  "unable to find 'nvlink' in path"));
-  }
-
+  std::string NvlinkPath;
   SmallVector Argv(argv, argv + argc);
   SmallVector ArgvSubst;
   SmallVector TmpFiles;
@@ -147,15 +150,28 @@
 
   for (size_t i = 1; i < Argv.size(); ++i) {
 std::string Arg = Argv[i];
+StringRef ArgRef(Arg);
+auto NvlPath = ArgRef.startswith_insensitive("--path=");
 if (sys::path::extension(Arg) == ".a") {
   if (Error Err = extractArchiveFiles(Arg, ArgvSubst, TmpFiles))
 reportError(std::move(Err));
+} else if (NvlPath) {
+  NvlinkPath = ArgRef.substr(7).str().append("/nvlink");
 } else {
   ArgvSubst.push_back(Arg);
 }
   }
 
-  if (Error Err = runNVLink(NvlinkPath.get(), ArgvSubst))
+  if (NvlinkPath.empty()) {
+ErrorOr NvlinkPathErr = sys::findProgramByName("nvlink");
+if (!NvlinkPathErr) {
+  reportError(createStringError(NvlinkPathErr.getError(),
+"unable to find 'nvlink' in path"));
+}
+NvlinkPath = NvlinkPathErr.get();
+  }
+
+  if (Error Err = runNVLink(NvlinkPath, ArgvSubst))
 reportError(std::move(Err));
   if (Error Err = cleanupTmpFiles(TmpFiles))
 reportError(std::move(Err));
Index: clang/lib/Driver/ToolChains/Cuda.cpp
===
--- clang/lib/Driver/ToolChains/Cuda.cpp
+++ clang/lib/Driver/ToolChains/Cuda.cpp
@@ -613,6 +613,13 @@
   AddStaticDeviceLibsLinking(C, *this, JA, Inputs, Args, CmdArgs, "nvptx", 
GPUArch,
   false, false);
 
+  // Find nvlink and pass it as "--path=" argument of clang-nvlink-wrapper.
+  auto NvlinkDir =
+  llvm::sys::path::parent_path(getToolChain().GetProgramPath("nvlink"))
+  .str();
+  const char *NvlinkPath = Args.MakeArgString(Twine("--path=" + NvlinkDir));
+  CmdArgs.push_back(NvlinkPath);
+
   const char *Exec =
   
Args.MakeArgString(getToolChain().GetProgramPath("clang-nvlink-wrapper"));
   C.addCommand(std::make_unique(


Index: clang/tools/clang-nvlink-wrapper/ClangNvlinkWrapper.cpp
===
--- clang/tools/clang-nvlink-wrapper/ClangNvlinkWrapper.cpp
+++ clang/tools/clang-nvlink-wrapper/ClangNvlinkWrapper.cpp
@@ -41,6 +41,15 @@
 
 static cl::opt Help("h", cl::desc("Alias for -help"), cl::Hidden);
 
+// Mark all our options with this category, everything else (except for -help)
+// will be hidden.
+static cl::OptionCategory
+ClangNvlinkWrapperCategory("clang-nvlink-wrapper options");
+
+static cl::opt
+NvlinkUserPath("path", cl::desc("path of directory containing nvlink"),
+   cl::cat(ClangNvlinkWrapperCategory));
+
 static Error runNVLink(std::string NVLinkPath,
SmallVectorImpl ) {
   std::vector NVLArgs;
@@ -121,7 +130,6 @@
 
 int main(int argc, const char **argv) {
   sys::PrintStackTraceOnErrorSignal(argv[0]);
-
   if (Help) {
 cl::PrintHelpMessage();
 return 0;
@@ -132,12 +140,7 @@
 exit(1);
   };
 
-  ErrorOr 

[clang] 3e55379 - [clang-format][NFC] Fix spelling mistakes

2021-10-09 Thread via cfe-commits

Author: mydeveloperday
Date: 2021-10-09T12:27:08+01:00
New Revision: 3e553791caa0c1f940cc91df0bb30c1b557f7c8a

URL: 
https://github.com/llvm/llvm-project/commit/3e553791caa0c1f940cc91df0bb30c1b557f7c8a
DIFF: 
https://github.com/llvm/llvm-project/commit/3e553791caa0c1f940cc91df0bb30c1b557f7c8a.diff

LOG: [clang-format][NFC] Fix spelling mistakes

Added: 


Modified: 
clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h

Removed: 




diff  --git 
a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h 
b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
index 824013fd60816..fb0d923a53ac9 100644
--- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
+++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
@@ -76,10 +76,10 @@ class DependencyScanningWorker {
   /// The in-memory filesystem laid on top the physical filesystem in `RealFS`.
   llvm::IntrusiveRefCntPtr InMemoryFS;
   /// The file system that is used by each worker when scanning for
-  /// dependencies. This filesystem persists accross multiple compiler
+  /// dependencies. This filesystem persists across multiple compiler
   /// invocations.
   llvm::IntrusiveRefCntPtr DepFS;
-  /// The file manager that is reused accross multiple invocations by this
+  /// The file manager that is reused across multiple invocations by this
   /// worker. If null, the file manager will not be reused.
   llvm::IntrusiveRefCntPtr Files;
   ScanningOutputFormat Format;



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


[PATCH] D111462: [Driver][OpenBSD] Use ToolChain reference instead of getToolChain().

2021-10-09 Thread Frederic Cambus via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG6417260a57dd: [Driver][OpenBSD] Use ToolChain reference 
instead of getToolChain(). (authored by fcambus).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111462

Files:
  clang/lib/Driver/ToolChains/OpenBSD.cpp


Index: clang/lib/Driver/ToolChains/OpenBSD.cpp
===
--- clang/lib/Driver/ToolChains/OpenBSD.cpp
+++ clang/lib/Driver/ToolChains/OpenBSD.cpp
@@ -28,10 +28,13 @@
   const InputInfoList ,
   const ArgList ,
   const char *LinkingOutput) const {
+  const toolchains::OpenBSD  =
+  static_cast(getToolChain());
+
   claimNoWarnArgs(Args);
   ArgStringList CmdArgs;
 
-  switch (getToolChain().getArch()) {
+  switch (ToolChain.getArch()) {
   case llvm::Triple::x86:
 // When building 32-bit code on OpenBSD/amd64, we have to explicitly
 // instruct as in the base system to assemble 32-bit code.
@@ -45,11 +48,11 @@
 
   case llvm::Triple::sparcv9: {
 CmdArgs.push_back("-64");
-std::string CPU = getCPUName(getToolChain().getDriver(), Args,
- getToolChain().getTriple());
+std::string CPU = getCPUName(ToolChain.getDriver(), Args,
+ ToolChain.getTriple());
 CmdArgs.push_back(
-sparc::getSparcAsmModeForCPU(CPU, getToolChain().getTriple()));
-AddAssemblerKPIC(getToolChain(), Args, CmdArgs);
+sparc::getSparcAsmModeForCPU(CPU, ToolChain.getTriple()));
+AddAssemblerKPIC(ToolChain, Args, CmdArgs);
 break;
   }
 
@@ -57,17 +60,17 @@
   case llvm::Triple::mips64el: {
 StringRef CPUName;
 StringRef ABIName;
-mips::getMipsCPUAndABI(Args, getToolChain().getTriple(), CPUName, ABIName);
+mips::getMipsCPUAndABI(Args, ToolChain.getTriple(), CPUName, ABIName);
 
 CmdArgs.push_back("-mabi");
 CmdArgs.push_back(mips::getGnuCompatibleMipsABIName(ABIName).data());
 
-if (getToolChain().getTriple().isLittleEndian())
+if (ToolChain.getTriple().isLittleEndian())
   CmdArgs.push_back("-EL");
 else
   CmdArgs.push_back("-EB");
 
-AddAssemblerKPIC(getToolChain(), Args, CmdArgs);
+AddAssemblerKPIC(ToolChain, Args, CmdArgs);
 break;
   }
 
@@ -83,7 +86,7 @@
   for (const auto  : Inputs)
 CmdArgs.push_back(II.getFilename());
 
-  const char *Exec = Args.MakeArgString(getToolChain().GetProgramPath("as"));
+  const char *Exec = Args.MakeArgString(ToolChain.GetProgramPath("as"));
   C.addCommand(std::make_unique(JA, *this,
  ResponseFileSupport::AtFileCurCP(),
  Exec, CmdArgs, Inputs, Output));
@@ -96,7 +99,7 @@
const char *LinkingOutput) const {
   const toolchains::OpenBSD  =
   static_cast(getToolChain());
-  const Driver  = getToolChain().getDriver();
+  const Driver  = ToolChain.getDriver();
   ArgStringList CmdArgs;
 
   // Silence warning for "clang -g foo.o -o foo"


Index: clang/lib/Driver/ToolChains/OpenBSD.cpp
===
--- clang/lib/Driver/ToolChains/OpenBSD.cpp
+++ clang/lib/Driver/ToolChains/OpenBSD.cpp
@@ -28,10 +28,13 @@
   const InputInfoList ,
   const ArgList ,
   const char *LinkingOutput) const {
+  const toolchains::OpenBSD  =
+  static_cast(getToolChain());
+
   claimNoWarnArgs(Args);
   ArgStringList CmdArgs;
 
-  switch (getToolChain().getArch()) {
+  switch (ToolChain.getArch()) {
   case llvm::Triple::x86:
 // When building 32-bit code on OpenBSD/amd64, we have to explicitly
 // instruct as in the base system to assemble 32-bit code.
@@ -45,11 +48,11 @@
 
   case llvm::Triple::sparcv9: {
 CmdArgs.push_back("-64");
-std::string CPU = getCPUName(getToolChain().getDriver(), Args,
- getToolChain().getTriple());
+std::string CPU = getCPUName(ToolChain.getDriver(), Args,
+ ToolChain.getTriple());
 CmdArgs.push_back(
-sparc::getSparcAsmModeForCPU(CPU, getToolChain().getTriple()));
-AddAssemblerKPIC(getToolChain(), Args, CmdArgs);
+sparc::getSparcAsmModeForCPU(CPU, ToolChain.getTriple()));
+AddAssemblerKPIC(ToolChain, Args, CmdArgs);
 break;
   }
 
@@ -57,17 +60,17 @@
   case llvm::Triple::mips64el: {
 StringRef CPUName;
 StringRef ABIName;
-mips::getMipsCPUAndABI(Args, getToolChain().getTriple(), CPUName, ABIName);
+mips::getMipsCPUAndABI(Args, ToolChain.getTriple(), CPUName, ABIName);
 
 CmdArgs.push_back("-mabi");
 

[clang] 6417260 - [Driver][OpenBSD] Use ToolChain reference instead of getToolChain().

2021-10-09 Thread Frederic Cambus via cfe-commits

Author: Frederic Cambus
Date: 2021-10-09T13:21:39+02:00
New Revision: 6417260a57dd4292ce91f2357479831e3fcf177e

URL: 
https://github.com/llvm/llvm-project/commit/6417260a57dd4292ce91f2357479831e3fcf177e
DIFF: 
https://github.com/llvm/llvm-project/commit/6417260a57dd4292ce91f2357479831e3fcf177e.diff

LOG: [Driver][OpenBSD] Use ToolChain reference instead of getToolChain().

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

Added: 


Modified: 
clang/lib/Driver/ToolChains/OpenBSD.cpp

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/OpenBSD.cpp 
b/clang/lib/Driver/ToolChains/OpenBSD.cpp
index 451975d0b4e69..a2404e0d62067 100644
--- a/clang/lib/Driver/ToolChains/OpenBSD.cpp
+++ b/clang/lib/Driver/ToolChains/OpenBSD.cpp
@@ -28,10 +28,13 @@ void openbsd::Assembler::ConstructJob(Compilation , const 
JobAction ,
   const InputInfoList ,
   const ArgList ,
   const char *LinkingOutput) const {
+  const toolchains::OpenBSD  =
+  static_cast(getToolChain());
+
   claimNoWarnArgs(Args);
   ArgStringList CmdArgs;
 
-  switch (getToolChain().getArch()) {
+  switch (ToolChain.getArch()) {
   case llvm::Triple::x86:
 // When building 32-bit code on OpenBSD/amd64, we have to explicitly
 // instruct as in the base system to assemble 32-bit code.
@@ -45,11 +48,11 @@ void openbsd::Assembler::ConstructJob(Compilation , const 
JobAction ,
 
   case llvm::Triple::sparcv9: {
 CmdArgs.push_back("-64");
-std::string CPU = getCPUName(getToolChain().getDriver(), Args,
- getToolChain().getTriple());
+std::string CPU = getCPUName(ToolChain.getDriver(), Args,
+ ToolChain.getTriple());
 CmdArgs.push_back(
-sparc::getSparcAsmModeForCPU(CPU, getToolChain().getTriple()));
-AddAssemblerKPIC(getToolChain(), Args, CmdArgs);
+sparc::getSparcAsmModeForCPU(CPU, ToolChain.getTriple()));
+AddAssemblerKPIC(ToolChain, Args, CmdArgs);
 break;
   }
 
@@ -57,17 +60,17 @@ void openbsd::Assembler::ConstructJob(Compilation , const 
JobAction ,
   case llvm::Triple::mips64el: {
 StringRef CPUName;
 StringRef ABIName;
-mips::getMipsCPUAndABI(Args, getToolChain().getTriple(), CPUName, ABIName);
+mips::getMipsCPUAndABI(Args, ToolChain.getTriple(), CPUName, ABIName);
 
 CmdArgs.push_back("-mabi");
 CmdArgs.push_back(mips::getGnuCompatibleMipsABIName(ABIName).data());
 
-if (getToolChain().getTriple().isLittleEndian())
+if (ToolChain.getTriple().isLittleEndian())
   CmdArgs.push_back("-EL");
 else
   CmdArgs.push_back("-EB");
 
-AddAssemblerKPIC(getToolChain(), Args, CmdArgs);
+AddAssemblerKPIC(ToolChain, Args, CmdArgs);
 break;
   }
 
@@ -83,7 +86,7 @@ void openbsd::Assembler::ConstructJob(Compilation , const 
JobAction ,
   for (const auto  : Inputs)
 CmdArgs.push_back(II.getFilename());
 
-  const char *Exec = Args.MakeArgString(getToolChain().GetProgramPath("as"));
+  const char *Exec = Args.MakeArgString(ToolChain.GetProgramPath("as"));
   C.addCommand(std::make_unique(JA, *this,
  ResponseFileSupport::AtFileCurCP(),
  Exec, CmdArgs, Inputs, Output));
@@ -96,7 +99,7 @@ void openbsd::Linker::ConstructJob(Compilation , const 
JobAction ,
const char *LinkingOutput) const {
   const toolchains::OpenBSD  =
   static_cast(getToolChain());
-  const Driver  = getToolChain().getDriver();
+  const Driver  = ToolChain.getDriver();
   ArgStringList CmdArgs;
 
   // Silence warning for "clang -g foo.o -o foo"



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


[clang] bbf4b3d - [clang-format][NFC] Fix spelling mistake

2021-10-09 Thread via cfe-commits

Author: mydeveloperday
Date: 2021-10-09T12:18:25+01:00
New Revision: bbf4b3dbbe3bfaeecdff2a29ed49da172895d82c

URL: 
https://github.com/llvm/llvm-project/commit/bbf4b3dbbe3bfaeecdff2a29ed49da172895d82c
DIFF: 
https://github.com/llvm/llvm-project/commit/bbf4b3dbbe3bfaeecdff2a29ed49da172895d82c.diff

LOG: [clang-format][NFC] Fix spelling mistake

Added: 


Modified: 
clang/include/clang/Tooling/Inclusions/HeaderIncludes.h

Removed: 




diff  --git a/clang/include/clang/Tooling/Inclusions/HeaderIncludes.h 
b/clang/include/clang/Tooling/Inclusions/HeaderIncludes.h
index 02fb2875671a5..c1b7baec7ec51 100644
--- a/clang/include/clang/Tooling/Inclusions/HeaderIncludes.h
+++ b/clang/include/clang/Tooling/Inclusions/HeaderIncludes.h
@@ -84,7 +84,7 @@ class HeaderIncludes {
 
 // An include header quoted with either <> or "".
 std::string Name;
-// The range of the whole line of include directive including any eading
+// The range of the whole line of include directive including any leading
 // whitespaces and trailing comment.
 tooling::Range R;
   };
@@ -127,7 +127,6 @@ class HeaderIncludes {
   llvm::Regex IncludeRegex;
 };
 
-
 } // namespace tooling
 } // namespace clang
 



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


[PATCH] D111208: [clang-tidy] Support globbing in NOLINT* expressions

2021-10-09 Thread Salman Javed via Phabricator via cfe-commits
salman-javed-nz added a comment.

In D111208#3053061 , @carlosgalvezp 
wrote:

> Looking at the code, I see that you use `SuppressionIsSpecific` in order to 
> separate the errors into 2 error lists: `SpecificNolintBegins` and 
> `GlobalNolintBegins`. However you don't do anything different with either 
> list, so why do they need to be different lists?
>
> Here checking that a combined list is empty would be equivalent:
>
>   bool WithinNolintBegin =
>   !SpecificNolintBegins.empty() || !GlobalNolintBegins.empty();
>
> And here, you are running identical code for both lists:
>
>   for (const auto NolintBegin : SpecificNolintBegins) {
> auto Error = createNolintError(Context, SM, NolintBegin, true);
> SuppressionErrors.emplace_back(Error);
>   }
>   for (const auto NolintBegin : GlobalNolintBegins) {
> auto Error = createNolintError(Context, SM, NolintBegin, true);
> SuppressionErrors.emplace_back(Error);
>   }
>
> And then these lists are not used any further than the scope of the function 
> where they are declared. So to me it feels like they could be combined, and 
> this logic of `SuppressionIsSpecific` be removed. Let me know if I'm missing 
> something obvious!

`tallyNolintBegins()` goes through every line, and whenever it sees a 
`NOLINTBEGIN`, it pushes its SourceLocation onto a stack. When it sees a 
`NOLINTEND`, it pops one entry off the stack. At the end of the file, all 
entries on the stack should have been popped off - if not, it's a unmatched 
`NOLINTBEGIN/END` expression error.

`NOLINTBEGIN(check-name)` expressions are pushed onto the 
`SpecificNolintBegins` list, while `NOLINTBEGIN` & `NOLINTBEGIN(*)` expressions 
are pushed onto the `GlobalNolintBegins` list. The reason for the two lists is 
so that `NOLINTBEGIN(check-name)` cannot be popped off by `NOLINTEND` or 
`NOLINTEND(*)`; and `NOLINTBEGIN` and `NOLINTBEGIN(*)` cannot be popped off by 
a `NOLINTEND(check-name)`.

See this line in `tallyNolintBegins()` where it figures out which list to use 
for the `NOLINTBEGIN/END` expression currently being evaluated...

  auto List = [&]() -> SmallVector * {
return SuppressionIsSpecific ?  : 
  };

I'm sure there is an alternate implementation which achieves the same thing 
with just one list object... however the list will probably need to store more 
information about the `NOLINTBEGIN` expression than just its SourceLocation to 
be able to achieve the same goals though... probably a list of 
`pair`, otherwise you can't match 
a `NOLINTEND` to its corresponding `NOLINTBEGIN`. If you want to take a crack 
at simplifying the logic here, all improvements are appreciated.




Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:353
+  if (SuppressionIsSpecific)
+*SuppressionIsSpecific = true;
 }

carlosgalvezp wrote:
> salman-javed-nz wrote:
> > salman-javed-nz wrote:
> > > carlosgalvezp wrote:
> > > > salman-javed-nz wrote:
> > > > > So when this function is called with args `Line = 
> > > > > "NOLINTBEGIN(google*)"` and `CheckName = 
> > > > > "google-explicit-constructor"`, it will return true with 
> > > > > `*SuppressionIsSpecific = true`.
> > > > > Should `*SuppressionIsSpecific = false` instead here?
> > > > Good point. I honestly don't understand what SuppressionIsSpecific 
> > > > means and how it's used downstream, could you clarify?
> > > `SuppressionIsSpecific = true` means when a check is suppressed by name, 
> > > i.e. `NOLINTBEGIN(check-name)`.
> > > 
> > > `SuppressionIsSpecific = false` means when `NOLINTBEGIN`  or 
> > > `NOLINTBEGIN(*)` is used. My gut feeling is that It should probably be 
> > > false when a glob is used too.
> > > 
> > > The point of this variable is so that a check-specific `BEGIN` can only 
> > > be `END`ed with a check-specific END.
> > I think the behaviour has changed in this patch.
> > 
> > It used to return `*SuppressionIsSpecific = false` when `ChecksStr = "*"`. 
> > `"*"` is disabling all checks, not a specific check.
> > 
> > Now it returns `*SuppressionIsSpecific = true`. `Globs.contains(CheckName)` 
> > is true regardless of whether `ChecksStr = "*"` or `ChecksStr = 
> > "check-name"`, so it continues running onto line 353 and sets 
> > `*SuppressionIsSpecific = true` no matter what.
> You are right! It's interesting however that this hasn't triggered any unit 
> test error, so I wonder if this variable is needed at all then? I will have a 
> deeper look at the code to get a better understanding and update the patch 
> accordingly.
> You are right! It's interesting however that this hasn't triggered any unit 
> test error

That's because the unit tests are missing a test case that checks this specific 
combination of BEGIN/END expressions. The tests predominantly use `NOLINTBEGIN` 
, not `NOLINTBEGIN(*)`.
It would be great if this shortcoming in the test coverage could be plugged in 
this patch.


[PATCH] D110803: [clang-format][docs][NFC] correct the "first supported versions" of some of the clang-format options

2021-10-09 Thread MyDeveloperDay via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa2a826d8b66c: [clang-format][docs][NFC] correct the 
first supported versions of some of theā€¦ (authored by 
MyDeveloperDay).

Changed prior to commit:
  https://reviews.llvm.org/D110803?vs=376121=378428#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110803

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h

Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -208,7 +208,7 @@
   ///  /* some comment */
   ///  #define bar(y, z)(y + z)
   ///\endcode
-  /// \version 13
+  /// \version 9
   AlignConsecutiveStyle AlignConsecutiveMacros;
 
   /// Style of aligning consecutive assignments.
@@ -277,7 +277,7 @@
   ///  /* A comment. */
   ///  double e = 4;
   ///\endcode
-  /// \version 13
+  /// \version 3.8
   AlignConsecutiveStyle AlignConsecutiveAssignments;
 
   /// Style of aligning consecutive bit field.
@@ -347,7 +347,7 @@
   ///  /* A comment. */
   ///  int ee   : 3;
   ///\endcode
-  /// \version 13
+  /// \version 11
   AlignConsecutiveStyle AlignConsecutiveBitFields;
 
   /// Style of aligning consecutive declarations.
@@ -417,7 +417,7 @@
   ///  /* A comment. */
   ///  boolc = false;
   ///\endcode
-  /// \version 13
+  /// \version 3.8
   AlignConsecutiveStyle AlignConsecutiveDeclarations;
 
   /// Different styles for aligning escaped newlines.
@@ -930,7 +930,7 @@
   ///   AttributeMacros: ['__capability', '__output', '__ununsed']
   /// \endcode
   ///
-  /// \version 13
+  /// \version 12
   std::vector AttributeMacros;
 
   /// If ``false``, a function call's arguments will either be all on the
@@ -1919,8 +1919,9 @@
   ///   * type
   ///
   /// Note: it MUST contain 'type'.
-  /// Items to the left of 'type' will be placed to the left of the type and aligned in the order supplied.
-  /// Items to the right of 'type' will be placed to the right of the type and aligned in the order supplied.
+  /// Items to the left of 'type' will be placed to the left of the type and
+  /// aligned in the order supplied. Items to the right of 'type' will be placed
+  /// to the right of the type and aligned in the order supplied.
   ///
   /// \code{.yaml}
   ///   QualifierOrder: ['inline', 'static', 'type', 'const', 'volatile' ]
@@ -2904,7 +2905,7 @@
 
   /// Penalty for each character of whitespace indentation
   /// (counted relative to leading non-whitespace column).
-  /// \version 13
+  /// \version 12
   unsigned PenaltyIndentedWhitespace;
 
   /// The ``&``, ``&&`` and ``*`` alignment style.
@@ -3131,7 +3132,7 @@
   /// When sorting Java imports, by default static imports are placed before
   /// non-static imports. If ``JavaStaticImportAfterImport`` is ``After``,
   /// static imports are placed after non-static imports.
-  /// \version 13
+  /// \version 12
   SortJavaStaticImportOptions SortJavaStaticImport;
 
   /// If ``true``, clang-format will sort using declarations.
@@ -3205,7 +3206,7 @@
   };
 
   ///  Defines in which cases to put a space before or after pointer qualifiers
-  /// \version 13
+  /// \version 12
   SpaceAroundPointerQualifiersStyle SpaceAroundPointerQualifiers;
 
   /// If ``false``, spaces will be removed before assignment operators.
@@ -3224,7 +3225,7 @@
   /// case 1 : break; case 1: break;
   ///   }   }
   /// \endcode
-  /// \version 13
+  /// \version 12
   bool SpaceBeforeCaseColon;
 
   /// If ``true``, a space will be inserted before a C++11 braced list
@@ -3511,7 +3512,7 @@
 BFCS_After
   };
   /// The BitFieldColonSpacingStyle to use for bitfields.
-  /// \version 13
+  /// \version 12
   BitFieldColonSpacingStyle BitFieldColonSpacing;
 
   /// Supported language standards for parsing and formatting C++ constructs.
@@ -3563,7 +3564,7 @@
   ///   unsigned char data = 'x';
   ///   emit signal(data); // Now it's fine again.
   /// \endcode
-  /// \version 13
+  /// \version 12
   std::vector StatementAttributeLikeMacros;
 
   /// The number of columns used for tab stops.
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -247,7 +247,7 @@
 
 
 
-**AlignConsecutiveAssignments** (``AlignConsecutiveStyle``) :versionbadge:`clang-format 13`
+**AlignConsecutiveAssignments** (``AlignConsecutiveStyle``) :versionbadge:`clang-format 3.8`
   Style of aligning consecutive assignments.
 
   ``Consecutive`` will result in formattings like:
@@ -320,7 +320,7 @@
/* A comment. */
double e = 4;
 

[clang] a2a826d - [clang-format][docs][NFC] correct the "first supported versions" of some of the clang-format options

2021-10-09 Thread via cfe-commits

Author: mydeveloperday
Date: 2021-10-09T11:02:49+01:00
New Revision: a2a826d8b66cfc85499a92949767d153563078a0

URL: 
https://github.com/llvm/llvm-project/commit/a2a826d8b66cfc85499a92949767d153563078a0
DIFF: 
https://github.com/llvm/llvm-project/commit/a2a826d8b66cfc85499a92949767d153563078a0.diff

LOG: [clang-format][docs][NFC] correct the "first supported versions" of some 
of the clang-format options

Some of the first supported version field were incorrectly attributed to a 
later branch.

It wasn't possible to correctly determine the "introduced version" with my 
naive implementation
using git blame alone, (especially if the type had been changed from a bool -> 
enum)

I saw more things attributed to clang-format 13 than I remembered and reviewed
those options to determine their introduced version.

Reviewed By: HazardyKnusperkeks

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

Added: 


Modified: 
clang/docs/ClangFormatStyleOptions.rst
clang/include/clang/Format/Format.h

Removed: 




diff  --git a/clang/docs/ClangFormatStyleOptions.rst 
b/clang/docs/ClangFormatStyleOptions.rst
index c05fbc753614e..e2e044145018b 100644
--- a/clang/docs/ClangFormatStyleOptions.rst
+++ b/clang/docs/ClangFormatStyleOptions.rst
@@ -247,7 +247,7 @@ the configuration (without a prefix: ``Auto``).
 
 
 
-**AlignConsecutiveAssignments** (``AlignConsecutiveStyle``) 
:versionbadge:`clang-format 13`
+**AlignConsecutiveAssignments** (``AlignConsecutiveStyle``) 
:versionbadge:`clang-format 3.8`
   Style of aligning consecutive assignments.
 
   ``Consecutive`` will result in formattings like:
@@ -320,7 +320,7 @@ the configuration (without a prefix: ``Auto``).
/* A comment. */
double e = 4;
 
-**AlignConsecutiveBitFields** (``AlignConsecutiveStyle``) 
:versionbadge:`clang-format 13`
+**AlignConsecutiveBitFields** (``AlignConsecutiveStyle``) 
:versionbadge:`clang-format 11`
   Style of aligning consecutive bit field.
 
   ``Consecutive`` will align the bitfield separators of consecutive lines.
@@ -394,7 +394,7 @@ the configuration (without a prefix: ``Auto``).
/* A comment. */
int ee   : 3;
 
-**AlignConsecutiveDeclarations** (``AlignConsecutiveStyle``) 
:versionbadge:`clang-format 13`
+**AlignConsecutiveDeclarations** (``AlignConsecutiveStyle``) 
:versionbadge:`clang-format 3.8`
   Style of aligning consecutive declarations.
 
   ``Consecutive`` will align the declaration names of consecutive lines.
@@ -468,7 +468,7 @@ the configuration (without a prefix: ``Auto``).
/* A comment. */
boolc = false;
 
-**AlignConsecutiveMacros** (``AlignConsecutiveStyle``) 
:versionbadge:`clang-format 13`
+**AlignConsecutiveMacros** (``AlignConsecutiveStyle``) 
:versionbadge:`clang-format 9`
   Style of aligning consecutive macro definitions.
 
   ``Consecutive`` will result in formattings like:
@@ -1082,7 +1082,7 @@ the configuration (without a prefix: ``Auto``).
 
 
 
-**AttributeMacros** (``List of Strings``) :versionbadge:`clang-format 13`
+**AttributeMacros** (``List of Strings``) :versionbadge:`clang-format 12`
   A vector of strings that should be interpreted as attributes/qualifiers
   instead of identifiers. This can be useful for language extensions or
   static analyzer annotations.
@@ -1135,7 +1135,7 @@ the configuration (without a prefix: ``Auto``).
int ,
int aaa) {}
 
-**BitFieldColonSpacing** (``BitFieldColonSpacingStyle``) 
:versionbadge:`clang-format 13`
+**BitFieldColonSpacing** (``BitFieldColonSpacingStyle``) 
:versionbadge:`clang-format 12`
   The BitFieldColonSpacingStyle to use for bitfields.
 
   Possible values:
@@ -3197,7 +3197,7 @@ the configuration (without a prefix: ``Auto``).
 **PenaltyExcessCharacter** (``Unsigned``) :versionbadge:`clang-format 3.7`
   The penalty for each character outside of the column limit.
 
-**PenaltyIndentedWhitespace** (``Unsigned``) :versionbadge:`clang-format 13`
+**PenaltyIndentedWhitespace** (``Unsigned``) :versionbadge:`clang-format 12`
   Penalty for each character of whitespace indentation
   (counted relative to leading non-whitespace column).
 
@@ -3301,8 +3301,9 @@ the configuration (without a prefix: ``Auto``).
 * type
 
   Note: it MUST contain 'type'.
-  Items to the left of 'type' will be placed to the left of the type and 
aligned in the order supplied.
-  Items to the right of 'type' will be placed to the right of the type and 
aligned in the order supplied.
+  Items to the left of 'type' will be placed to the left of the type and
+  aligned in the order supplied. Items to the right of 'type' will be placed
+  to the right of the type and aligned in the order supplied.
 
 
   .. code-block:: yaml
@@ -3461,7 +3462,7 @@ the configuration (without a prefix: ``Auto``).
 
 
 
-**SortJavaStaticImport** 

[PATCH] D95168: [clang-format] Add InsertBraces option

2021-10-09 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment.

ping @tiagoma  are you still around to pursue this feature request or can this 
be commandeered?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95168

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


[PATCH] D111208: [clang-tidy] Support globbing in NOLINT* expressions

2021-10-09 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp updated this revision to Diff 378427.
carlosgalvezp marked 5 inline comments as done.
carlosgalvezp added a comment.

-Fixed formatting comments.
-Added test for NOLINT(), NOLINTNEXTLINE() and NOLINTBEGIN()
-Changed last test to more effectively test NOLINT(*,-google*)


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

https://reviews.llvm.org/D111208

Files:
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/index.rst
  clang-tools-extra/test/clang-tidy/infrastructure/nolint.cpp
  clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend.cpp
  clang-tools-extra/test/clang-tidy/infrastructure/nolintnextline.cpp

Index: clang-tools-extra/test/clang-tidy/infrastructure/nolintnextline.cpp
===
--- clang-tools-extra/test/clang-tidy/infrastructure/nolintnextline.cpp
+++ clang-tools-extra/test/clang-tidy/infrastructure/nolintnextline.cpp
@@ -11,21 +11,25 @@
 class D { D(int i); };
 // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
 
-// NOLINTNEXTLINE(*)
+// NOLINTNEXTLINE()
 class D1 { D1(int i); };
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: single-argument constructors must be marked explicit
 
-// NOLINTNEXTLINE(not-closed-bracket-is-treated-as-skip-all
+// NOLINTNEXTLINE(*)
 class D2 { D2(int i); };
 
-// NOLINTNEXTLINE(google-explicit-constructor)
+// NOLINTNEXTLINE(not-closed-bracket-is-treated-as-skip-all
 class D3 { D3(int i); };
 
-// NOLINTNEXTLINE(some-check, google-explicit-constructor)
+// NOLINTNEXTLINE(google-explicit-constructor)
 class D4 { D4(int i); };
 
-// NOLINTNEXTLINE without-brackets-skip-all, another-check
+// NOLINTNEXTLINE(some-check, google-explicit-constructor)
 class D5 { D5(int i); };
 
+// NOLINTNEXTLINE without-brackets-skip-all, another-check
+class D6 { D6(int i); };
+
 // NOLINTNEXTLINE
 
 class E { E(int i); };
@@ -46,6 +50,21 @@
 // NOLINTNEXTLINE
 MACRO_NOARG
 
-// CHECK-MESSAGES: Suppressed 9 warnings (9 NOLINT)
+// NOLINTNEXTLINE(google*)
+class I1 { I1(int i); };
+
+// NOLINTNEXTLINE(google*,-google*)
+class I2 { I2(int i); };
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: single-argument constructors must be marked explicit
+
+class I3 { I3(int x); void f() { int i; } };
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: single-argument constructors must be marked explicit
+// CHECK-MESSAGES: :[[@LINE-2]]:38: warning: unused variable 'i' [clang-diagnostic-unused-variable]
+
+// NOLINTNEXTLINE(*,-google*)
+class I4 { I4(int x); void f() { int i; } };
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: single-argument constructors must be marked explicit
+
+// CHECK-MESSAGES: Suppressed 11 warnings (11 NOLINT)
 
-// RUN: %check_clang_tidy %s google-explicit-constructor %t --
+// RUN: %check_clang_tidy %s google-explicit-constructor,clang-diagnostic-unused-variable %t -- -extra-arg=-Wunused-variable
Index: clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend.cpp
===
--- clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend.cpp
+++ clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s google-explicit-constructor %t
+// RUN: %check_clang_tidy %s google-explicit-constructor,clang-diagnostic-unused-variable %t -- -extra-arg=-Wunused-variable
 
 class A { A(int i); };
 // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
@@ -34,60 +34,65 @@
 class C1 { C1(int i); };
 // NOLINTEND(google-explicit-constructor)
 
-// NOLINTBEGIN(*)
+// NOLINTBEGIN()
 class C2 { C2(int i); };
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: single-argument constructors must be marked explicit
+// NOLINTEND()
+
+// NOLINTBEGIN(*)
+class C3 { C3(int i); };
 // NOLINTEND(*)
 
 // NOLINTBEGIN(some-other-check)
-class C3 { C3(int i); };
+class C4 { C4(int i); };
 // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: single-argument constructors must be marked explicit
 // NOLINTEND(some-other-check)
 
 // NOLINTBEGIN(some-other-check, google-explicit-constructor)
-class C4 { C4(int i); };
+class C5 { C5(int i); };
 // NOLINTEND(some-other-check, google-explicit-constructor)
 
 // NOLINTBEGIN(some-other-check, google-explicit-constructor)
 // NOLINTEND(some-other-check)
-class C5 { C5(int i); };
+class C6 { C6(int i); };
 // NOLINTEND(google-explicit-constructor)
 
 // NOLINTBEGIN(some-other-check, google-explicit-constructor)
 // NOLINTEND(google-explicit-constructor)
-class C6 { C6(int i); };
+class C7 { C7(int i); };
 // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: single-argument constructors must be marked explicit
 // NOLINTEND(some-other-check)
 
 // NOLINTBEGIN(google-explicit-constructor)
 // NOLINTBEGIN(some-other-check)
-class C7 { C7(int i); };
+class C8 { C8(int i); };
 // 

[PATCH] D110257: [CFE][Codegen] Do not break the contiguity of static allocas.

2021-10-09 Thread Mahesha S via Phabricator via cfe-commits
hsmhsm updated this revision to Diff 378424.
hsmhsm added a comment.

Introduce a new instruction pointer which aid all the addressspace casts of 
static allocas
to appear in the source order immediately after all static allocas.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110257

Files:
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/CodeGenCUDA/builtins-amdgcn.cu
  clang/test/CodeGenCXX/amdgcn-automatic-variable.cpp
  clang/test/CodeGenCXX/amdgcn-func-arg.cpp
  clang/test/CodeGenCXX/builtin-amdgcn-atomic-inc-dec.cpp
  clang/test/CodeGenCXX/vla.cpp
  clang/test/CodeGenSYCL/address-space-deduction.cpp
  clang/test/OpenMP/amdgcn_target_init_temp_alloca.cpp

Index: clang/test/OpenMP/amdgcn_target_init_temp_alloca.cpp
===
--- clang/test/OpenMP/amdgcn_target_init_temp_alloca.cpp
+++ clang/test/OpenMP/amdgcn_target_init_temp_alloca.cpp
@@ -12,7 +12,9 @@
   int arr[N];
 
   // CHECK:  [[VAR_ADDR:%.+]] = alloca [100 x i32]*, align 8, addrspace(5)
+  // CHECK-NEXT: [[VAR2_ADDR:%.+]] = alloca i32, align 4, addrspace(5)
   // CHECK-NEXT: [[VAR_ADDR_CAST:%.+]] = addrspacecast [100 x i32]* addrspace(5)* [[VAR_ADDR]] to [100 x i32]**
+  // CHECK-NEXT: [[VAR2_ADDR_CAST:%.+]] = addrspacecast i32 addrspace(5)* [[VAR2_ADDR]] to i32*
   // CHECK:  store [100 x i32]* [[VAR:%.+]], [100 x i32]** [[VAR_ADDR_CAST]], align 8
 
 #pragma omp target
Index: clang/test/CodeGenSYCL/address-space-deduction.cpp
===
--- clang/test/CodeGenSYCL/address-space-deduction.cpp
+++ clang/test/CodeGenSYCL/address-space-deduction.cpp
@@ -1,34 +1,33 @@
 // NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
 // RUN: %clang_cc1 -triple spir64 -fsycl-is-device -disable-llvm-passes -emit-llvm %s -o - | FileCheck %s
 
-
 // CHECK-LABEL: @_Z4testv(
 // CHECK-NEXT:  entry:
 // CHECK-NEXT:[[I:%.*]] = alloca i32, align 4
-// CHECK-NEXT:[[I_ASCAST:%.*]] = addrspacecast i32* [[I]] to i32 addrspace(4)*
 // CHECK-NEXT:[[PPTR:%.*]] = alloca i32 addrspace(4)*, align 8
-// CHECK-NEXT:[[PPTR_ASCAST:%.*]] = addrspacecast i32 addrspace(4)** [[PPTR]] to i32 addrspace(4)* addrspace(4)*
 // CHECK-NEXT:[[IS_I_PTR:%.*]] = alloca i8, align 1
-// CHECK-NEXT:[[IS_I_PTR_ASCAST:%.*]] = addrspacecast i8* [[IS_I_PTR]] to i8 addrspace(4)*
 // CHECK-NEXT:[[VAR23:%.*]] = alloca i32, align 4
-// CHECK-NEXT:[[VAR23_ASCAST:%.*]] = addrspacecast i32* [[VAR23]] to i32 addrspace(4)*
 // CHECK-NEXT:[[CP:%.*]] = alloca i8 addrspace(4)*, align 8
-// CHECK-NEXT:[[CP_ASCAST:%.*]] = addrspacecast i8 addrspace(4)** [[CP]] to i8 addrspace(4)* addrspace(4)*
 // CHECK-NEXT:[[ARR:%.*]] = alloca [42 x i32], align 4
-// CHECK-NEXT:[[ARR_ASCAST:%.*]] = addrspacecast [42 x i32]* [[ARR]] to [42 x i32] addrspace(4)*
 // CHECK-NEXT:[[CPP:%.*]] = alloca i8 addrspace(4)*, align 8
-// CHECK-NEXT:[[CPP_ASCAST:%.*]] = addrspacecast i8 addrspace(4)** [[CPP]] to i8 addrspace(4)* addrspace(4)*
 // CHECK-NEXT:[[APTR:%.*]] = alloca i32 addrspace(4)*, align 8
-// CHECK-NEXT:[[APTR_ASCAST:%.*]] = addrspacecast i32 addrspace(4)** [[APTR]] to i32 addrspace(4)* addrspace(4)*
 // CHECK-NEXT:[[STR:%.*]] = alloca i8 addrspace(4)*, align 8
-// CHECK-NEXT:[[STR_ASCAST:%.*]] = addrspacecast i8 addrspace(4)** [[STR]] to i8 addrspace(4)* addrspace(4)*
 // CHECK-NEXT:[[PHI_STR:%.*]] = alloca i8 addrspace(4)*, align 8
-// CHECK-NEXT:[[PHI_STR_ASCAST:%.*]] = addrspacecast i8 addrspace(4)** [[PHI_STR]] to i8 addrspace(4)* addrspace(4)*
 // CHECK-NEXT:[[SELECT_NULL:%.*]] = alloca i8 addrspace(4)*, align 8
-// CHECK-NEXT:[[SELECT_NULL_ASCAST:%.*]] = addrspacecast i8 addrspace(4)** [[SELECT_NULL]] to i8 addrspace(4)* addrspace(4)*
 // CHECK-NEXT:[[SELECT_STR_TRIVIAL1:%.*]] = alloca i8 addrspace(4)*, align 8
-// CHECK-NEXT:[[SELECT_STR_TRIVIAL1_ASCAST:%.*]] = addrspacecast i8 addrspace(4)** [[SELECT_STR_TRIVIAL1]] to i8 addrspace(4)* addrspace(4)*
 // CHECK-NEXT:[[SELECT_STR_TRIVIAL2:%.*]] = alloca i8 addrspace(4)*, align 8
+// CHECK-NEXT:[[I_ASCAST:%.*]] = addrspacecast i32* [[I]] to i32 addrspace(4)*
+// CHECK-NEXT:[[PPTR_ASCAST:%.*]] = addrspacecast i32 addrspace(4)** [[PPTR]] to i32 addrspace(4)* addrspace(4)*
+// CHECK-NEXT:[[IS_I_PTR_ASCAST:%.*]] = addrspacecast i8* [[IS_I_PTR]] to i8 addrspace(4)*
+// CHECK-NEXT:[[VAR23_ASCAST:%.*]] = addrspacecast i32* [[VAR23]] to i32 addrspace(4)*
+// CHECK-NEXT:[[CP_ASCAST:%.*]] = addrspacecast i8 addrspace(4)** [[CP]] to i8 addrspace(4)* addrspace(4)*
+// CHECK-NEXT:[[ARR_ASCAST:%.*]] = addrspacecast [42 x i32]* [[ARR]] to [42 x i32] addrspace(4)*
+// CHECK-NEXT:[[CPP_ASCAST:%.*]] = addrspacecast i8 addrspace(4)** [[CPP]] to i8 addrspace(4)* 

[PATCH] D111208: [clang-tidy] Support globbing in NOLINT* expressions

2021-10-09 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp marked an inline comment as done.
carlosgalvezp added inline comments.



Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:349
+  // Allow specifying a few checks with a glob expression.
+  GlobList Globs(ChecksStr);
+  if (!Globs.contains(CheckName))

salman-javed-nz wrote:
> What happens when `CheckStr` is empty?
> How has Clang-Tidy treated `// NOLINT()` in the past? Does this patch change 
> the behaviour? What //should // be the "right" behaviour?
Very good question! Currently on master `// NOLINT()` will *not* suppress 
warnings. However `// NOLINT(` will. My patch shouldn't change existing 
behavior - an empty list of globs will return false when calling `contains`.

I'll add a unit test to cover this case!


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

https://reviews.llvm.org/D111208

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


[PATCH] D111208: [clang-tidy] Support globbing in NOLINT* expressions

2021-10-09 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added a comment.

Looking at the code, I see that you use `SuppressionIsSpecific` in order to 
separate the errors into 2 error lists: `SpecificNolintBegins` and 
`GlobalNolintBegins`. However you don't do anything different with either list, 
so why do they need to be different lists?

Here checking that a combined list is empty would be equivalent:

  bool WithinNolintBegin =
  !SpecificNolintBegins.empty() || !GlobalNolintBegins.empty();

And here, you are running identical code for both lists:

  for (const auto NolintBegin : SpecificNolintBegins) {
auto Error = createNolintError(Context, SM, NolintBegin, true);
SuppressionErrors.emplace_back(Error);
  }
  for (const auto NolintBegin : GlobalNolintBegins) {
auto Error = createNolintError(Context, SM, NolintBegin, true);
SuppressionErrors.emplace_back(Error);
  }

And then these lists are not used any further than the scope of the function 
where they are declared. So to me it feels like they could be combined, and 
this logic of `SuppressionIsSpecific` be removed. Let me know if I'm missing 
something obvious!


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

https://reviews.llvm.org/D111208

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


[PATCH] D111208: [clang-tidy] Support globbing in NOLINT* expressions

2021-10-09 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added inline comments.



Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:353
+  if (SuppressionIsSpecific)
+*SuppressionIsSpecific = true;
 }

salman-javed-nz wrote:
> salman-javed-nz wrote:
> > carlosgalvezp wrote:
> > > salman-javed-nz wrote:
> > > > So when this function is called with args `Line = 
> > > > "NOLINTBEGIN(google*)"` and `CheckName = 
> > > > "google-explicit-constructor"`, it will return true with 
> > > > `*SuppressionIsSpecific = true`.
> > > > Should `*SuppressionIsSpecific = false` instead here?
> > > Good point. I honestly don't understand what SuppressionIsSpecific means 
> > > and how it's used downstream, could you clarify?
> > `SuppressionIsSpecific = true` means when a check is suppressed by name, 
> > i.e. `NOLINTBEGIN(check-name)`.
> > 
> > `SuppressionIsSpecific = false` means when `NOLINTBEGIN`  or 
> > `NOLINTBEGIN(*)` is used. My gut feeling is that It should probably be 
> > false when a glob is used too.
> > 
> > The point of this variable is so that a check-specific `BEGIN` can only be 
> > `END`ed with a check-specific END.
> I think the behaviour has changed in this patch.
> 
> It used to return `*SuppressionIsSpecific = false` when `ChecksStr = "*"`. 
> `"*"` is disabling all checks, not a specific check.
> 
> Now it returns `*SuppressionIsSpecific = true`. `Globs.contains(CheckName)` 
> is true regardless of whether `ChecksStr = "*"` or `ChecksStr = 
> "check-name"`, so it continues running onto line 353 and sets 
> `*SuppressionIsSpecific = true` no matter what.
You are right! It's interesting however that this hasn't triggered any unit 
test error, so I wonder if this variable is needed at all then? I will have a 
deeper look at the code to get a better understanding and update the patch 
accordingly.



Comment at: clang-tools-extra/docs/clang-tidy/index.rst:347
+// No warnings are suppressed, due to double negation
+Foo(bool param);  // NOLINT(-google*)
   };

salman-javed-nz wrote:
> salman-javed-nz wrote:
> > carlosgalvezp wrote:
> > > salman-javed-nz wrote:
> > > > Would anyone do this on purpose, or is this a user error?
> > > I don't see any use case here, no. I just thought to document it to 
> > > clarify the double negation that's happening, in case a user gets 
> > > confused and doesn't see the warning being suppressed.
> > > 
> > > Do you think it brings value or does more harm than good?
> > 
> > I don't see any use case here, no. I just thought to document it to clarify 
> > the double negation that's happening, in case a user gets confused and 
> > doesn't see the warning being suppressed.
> > 
> > Do you think it brings value or does more harm than good?
> 
> I'd be happy with just the basic `+` glob functionality. The first thing I'd 
> use it on is to suppress checks that are an alias of another check, e.g. 
> `NOLINT(*no-malloc)` to cover both `hicpp-no-malloc` and 
> `cppcoreguidelines-no-malloc`.
> 
> As for glob expressions that begin with a `-`... you get the functionality 
> for free thanks to the `GlobList` class but it's not a feature I think I will 
> need. I speak only for myself though. Maybe someone else here has a strong 
> need for this?
> 
> Is `NOLINTBEGIN(-check)` equivalent to `NOLINTEND(check)`? It hursts my head 
> even thinking about it.
> 
> Your unit test where you test `NOLINT(google*,-google*,google*)`, Clang-Tidy 
> does the right thing in that situation, but I have to wonder why any user 
> would want to add, remove, then add the same check group in the one 
> expression in the first place? Should we even be entertaining this kind of 
> use?
> Is NOLINTBEGIN(-check) equivalent to NOLINTEND(check)?

To me it's clear that it's not. Any NOLINTBEGIN(X) must have a matching 
NOLINTEND(X), regardless of whether X is a glob or not.

Totally agree that negative globs are probably not needed, the main use case is 
being able to suppress all aliases as you say.

I just thought it was neat to be able to reuse code in that way, the code is 
very clean and easy to read. Plus users are familiar with the syntax. If users 
want to abuse it that would be at their own maintenance expense - clang-tidy 
would just do what it was asked to do, without crashing or anything bad 
happening.

The same thing can be done when running the tool - you can run 
"--checks=google*,-google*" and I don't think clang-tidy has code to warn the 
user/prevent this from happening. Considering all these edge cases would 
perhaps complicate the design of the tool for no real use case? I.e. too 
defensive programming.

I added the unit test mostly to make sure clang-tidy doesn't crash or does 
something strange, it just does what the user instructed it to do. I thought 
such edge cases are encouraged in unit tests to increase coverage.

Would it be reasonable to keep negative globs in the implementation (to 
simplify maintenance, reuse 

[PATCH] D111208: [clang-tidy] Support globbing in NOLINT* expressions

2021-10-09 Thread Salman Javed via Phabricator via cfe-commits
salman-javed-nz added inline comments.



Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:353
+  if (SuppressionIsSpecific)
+*SuppressionIsSpecific = true;
 }

salman-javed-nz wrote:
> carlosgalvezp wrote:
> > salman-javed-nz wrote:
> > > So when this function is called with args `Line = "NOLINTBEGIN(google*)"` 
> > > and `CheckName = "google-explicit-constructor"`, it will return true with 
> > > `*SuppressionIsSpecific = true`.
> > > Should `*SuppressionIsSpecific = false` instead here?
> > Good point. I honestly don't understand what SuppressionIsSpecific means 
> > and how it's used downstream, could you clarify?
> `SuppressionIsSpecific = true` means when a check is suppressed by name, i.e. 
> `NOLINTBEGIN(check-name)`.
> 
> `SuppressionIsSpecific = false` means when `NOLINTBEGIN`  or 
> `NOLINTBEGIN(*)` is used. My gut feeling is that It should probably be false 
> when a glob is used too.
> 
> The point of this variable is so that a check-specific `BEGIN` can only be 
> `END`ed with a check-specific END.
I think the behaviour has changed in this patch.

It used to return `*SuppressionIsSpecific = false` when `ChecksStr = "*"`. 
`"*"` is disabling all checks, not a specific check.

Now it returns `*SuppressionIsSpecific = true`. `Globs.contains(CheckName)` is 
true regardless of whether `ChecksStr = "*"` or `ChecksStr = "check-name"`, so 
it continues running onto line 353 and sets `*SuppressionIsSpecific = true` no 
matter what.


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

https://reviews.llvm.org/D111208

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


[PATCH] D111371: [Support][ThinLTO] Move ThinLTO caching to LLVM Support library.

2021-10-09 Thread Teresa Johnson via Phabricator via cfe-commits
tejohnson added inline comments.



Comment at: llvm/lib/Support/Caching.cpp:36
 
+  SmallString<10> CacheName = CacheNameRef;
   return [=](unsigned Task, StringRef Key) -> AddStreamFn {

noajshu wrote:
> noajshu wrote:
> > tejohnson wrote:
> > > Is this copy necessary? I believe sys::path::append takes a Twine and 
> > > eventually makes a copy of it.
> > Thanks, removed!
> Thanks again for this suggestion. On second thought I am concerned about 
> lifetime issues with this approach. The `localCache` function returns a 
> lambda which captures the arguments by copy. The Twine and StringRef classes 
> just point to some temporary memory which could be modified/freed before the 
> returned lambda is invoked. Making a copy (or just running the 
> `sys::path::append` before returning the lambda) would protect from such bugs.
> 
> Here is a stripped-down example of the issue I'm concerned about:
> ```
> #include 
> #include 
> #include "llvm/ADT/Twine.h"
> 
> using namespace llvm;
> 
> auto localCacheDummy(Twine FooTwine) {
>   return [=] () {
>   std::cout << FooTwine.str();
>   };
> }
> 
> int main() {
>   std::string SpecialString = "hello here is a string\n";
>   auto myFunction = localCacheDummy(SpecialString);
>   myFunction();
>   SpecialString = "ok I have replaced the string contents\n";
>   myFunction();
>   return 0;
> }
> ```
> This outputs:
> ```
> hello here is a string
> ok I have replaced the string contents
> ```
> This is an issue for `StringRef` as well, so I am concerned the [[ 
> https://github.com/llvm/llvm-project/blob/dc066888bd98c0500ca7b590317dc91ccce0fd38/llvm/lib/LTO/Caching.cpp#L31
>  | existing caching code ]] is unsafe as well. This was probably fine when it 
> was used solely with the usage pattern in ThinLTO. As we move it to the 
> support library should we make it more safe? In particular, we could keep the 
> parameters Twines but only access them in the top part of the `localCache` 
> body outside the lambdas.
Hmm that is a good catch. I think you are right that we should be making a copy 
of all of these string reference parameters outside of the lambda (which we 
then capture by copy) to address this issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111371

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