[PATCH] D77811: [clangd] Implement semanticTokens modifiers

[Nathan Ridge via Phabricator via cfe-commits]

nridge added a comment.

Thanks a lot for working on this! I agree this is well aligned with the goals 
of D77702  (and in fact goes even further and 
meets the goals of D66990  via the 
`declaration` modifier). The other modifiers are neat as well; I like the 
expressiveness of the kind + modifier model in general.

> This addresses some of the goals of D77702 , 
> but leaves some things undone.
> Mostly because I think these will want some discussion.
>
> - no split between dependent type/name. (We may want to model this as a 
> modifier, type+dependent vs ???+dependent)

+1

> - no split between primitive/typedef. (Is introducing a nonstandard kind is 
> worth this distinction?)

Here's my general take on this:

- There is too much variety between programming languages to expect that a 
standard list of highlighting kinds is going to satisfactorily cover all 
languages.
- If LSP aims to be the basis for tools that are competitive with purpose-built 
language-specific tools, it's reasonable to allow for clients willing to go the 
extra mile in terms of customization (e.g. use a language-specific theme which 
provides colors for language-specific token kinds).
- Therefore, I would say yes, we should take the liberty of adding nonstandard 
highlighting kinds and modifiers where it makes sense from a language POV.

That said... for typedef specifically, I wonder if it actually makes more sense 
as a modifier than a kind. That is, have the kind be the target type (falling 
back to Unknown/Dependent if we don't know), and have a modifier flag for "the 
type is referred to via an alias" (as opposed to "the type is referred to 
directly"). WDYT?

> - no nonstandard local attribute This probably makes sense, I'm wondering if 
> we want others and how they fit together.

Are you referring to local-scope variables (i.e. `FunctionScope` in D95701 
)? If so, I think the approach in that patch 
is promising.




Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:132
+return isConst(AT->getElementType());
+  return isConst(T->getPointeeType());
+}

It seems a bit unintuitive that the path which non-pointer types (e.g. 
unqualified class or builtin types) take is to return false in this recursive 
call because `getPointeeType()` returns null... but I guess that works.



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:136
+// Whether D is const in a loose sense (should it be highlighted as such?)
+bool isConst(const Decl *D) {
+  if (llvm::isa(D) || llvm::isa(D))

Do you think in the future it might make sense to have the `readonly` modifier 
reflect, at least for variables, whether //the particular reference// to the 
variable is a readonly reference (e.g. binding the variable to a `const X&` vs. 
an `X&`)?



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:184
+  if (const auto *FD = llvm::dyn_cast(D))
+return FD->isCXXClassMember() && !FD->isCXXInstanceMember();
+  if (const auto *OMD = llvm::dyn_cast(D))

Will anything take this path (rather than being covered by 
`VD->isStaticDataMember()` above)?



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:476
+  if (!Kind || (Result && Kind != Result))
+continue;
+  Result = Kind;

This is a change of behaviour from before, in the case where the `ReferenceLoc` 
has multiple targets.

Before, we would produce at most one highlighting token for the `ReferenceLoc`. 
In the case where different target decls had different highlighting kinds, we 
wouldn't produce any.

Now, it looks like we produce a separate token for every target whose kind 
matches the kind of the first target (and skip targets with a conflicting kind).

Is that the intention?

It seems a bit strange: if we allow multiple tokens, why couldn't they have 
different kinds?



Comment at: 
clang-tools-extra/clangd/refactor/tweaks/AnnotateHighlightings.cpp:11
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Support/ScopedPrinter.h"
 

Maybe add `// for llvm::to_string` at it's not obvious



Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp:264
+$Class[[D]] $Field_decl[[E]];
+static double $StaticField_decl_static[[S]];
+static void $StaticMethod_decl_static[[bar]]() {}

Presumably, the highlighting kinds `StaticField` and `StaticMethod` are going 
to be collapsed into `Field` and `Method` in a future change (after the removal 
of TheiaSemanticHighlighting, I guess)?



Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp:718
+  };
+  )cpp",
+  };

Should we add a test case for `_deprecated` as well?

[PATCH] D95670: [clangd] Don't rely on builtin headers for document-link.test.

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

thanks, lgtm! but I would wait for a while to see if someone(that remembers why 
this test was specifically asserting for built-in headers) will object.




Comment at: clang-tools-extra/clangd/test/document-link.test:1
-# for %resource_dir: REQUIRES: clang
-# %resource_dir actually points at builtin_include_dir, go up one directory.
-# RUN: clangd -lit-test -resource-dir=%resource_dir/.. < %s | FileCheck 
-strict-whitespace %s
+
+# create a fake resource_dir so that the test can find the headers.

nit: drop empty line



Comment at: clang-tools-extra/clangd/test/document-link.test:2
+
+# create a fake resource_dir so that the test can find the headers.
+# RUN: mkdir -p %t/include/ && touch %t/include/foo.h

s/create/Create/



Comment at: clang-tools-extra/clangd/test/document-link.test:3
+# create a fake resource_dir so that the test can find the headers.
+# RUN: mkdir -p %t/include/ && touch %t/include/foo.h
+# RUN: clangd -lit-test -resource-dir=%t < %s | FileCheck -strict-whitespace %s

this is clever! I believe there was a reason for this test to be asserting 
built-in headers (I can't seem to remember, maybe @sammccall does), but 
asserting through a mock header should also be fine, I suppose.

one thing though, we need to clean up `%t`, e.g. `rm -rf %t`.

(and regrading the failure at head, resource_dir path has changed with release 
cut from `something/12.0.0` to `something/13.0.0` so it should go away once you 
build new clang via `ninja clang`)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95670

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

[PATCH] D95759: [clangd] Rename: merge index/AST refs path-insensitively where needed

[Haojian Wu via Phabricator via cfe-commits]

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

Thanks for fixing this!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95759

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

[PATCH] D95725: clang-extra: fix incorrect use of std::lock_guard by adding variable name (identified by MSVC [[nodiscard]] error)

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

thanks, lgtm!




Comment at: clang-tools-extra/clangd/support/Function.h:54
   if (Parent) {
-std::lock_guard(Parent->ListenersMu);
+std::lock_guard Guard(Parent->ListenersMu);
 llvm::erase_if(Parent->Listeners,

nit: we prefer `Lock` rather than `Guard` in other places. so `s/Guard/Lock/`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95725

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

[PATCH] D76802: [InstrProfiling] Use !associated metadata for counters, data and values

[Petr Hosek via Phabricator via cfe-commits]

phosek added a comment.

@MaskRay I saw on https://sourceware.org/bugzilla/show_bug.cgi?id=27259 that 
the infinite loop fix has been cherry picked into 2.36 branch so I updated the 
condition to `-fbinutils-version=2.36`. Is this OK to land?


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

https://reviews.llvm.org/D76802

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

[PATCH] D76802: [InstrProfiling] Use !associated metadata for counters, data and values

[Petr Hosek via Phabricator via cfe-commits]

phosek updated this revision to Diff 320398.

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

https://reviews.llvm.org/D76802

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  compiler-rt/test/profile/instrprof-gc-sections.c
  llvm/include/llvm/Transforms/Instrumentation.h
  llvm/include/llvm/Transforms/Instrumentation/InstrProfiling.h
  llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
  llvm/test/Instrumentation/InstrProfiling/icall.ll
  llvm/test/Instrumentation/InstrProfiling/linkage.ll
  llvm/test/Transforms/PGOProfile/associated.ll
  llvm/test/Transforms/PGOProfile/counter_promo.ll
  llvm/test/Transforms/PGOProfile/counter_promo_mexits.ll

Index: llvm/test/Transforms/PGOProfile/counter_promo_mexits.ll
===
--- llvm/test/Transforms/PGOProfile/counter_promo_mexits.ll
+++ llvm/test/Transforms/PGOProfile/counter_promo_mexits.ll
@@ -69,7 +69,7 @@
 ; PROMO-NEXT:  %pgocount{{.*}} = load {{.*}} @__profc_foo{{.*}} 4)
 ; PROMO-NEXT: add 
 ; PROMO-NEXT: store {{.*}}@__profc_foo{{.*}}4)
-; PROMO-NOT: @__profc_foo
+; PROMO-NOT: @__profc_foo{{.*}})
 
 
 bb15: ; preds = %bb14, %bb4
Index: llvm/test/Transforms/PGOProfile/counter_promo.ll
===
--- llvm/test/Transforms/PGOProfile/counter_promo.ll
+++ llvm/test/Transforms/PGOProfile/counter_promo.ll
@@ -60,7 +60,7 @@
 ; ATOMIC_PROMO: atomicrmw add {{.*}} @__profc_foo{{.*}}0), i64 %[[LIVEOUT1]] seq_cst
 ; ATOMIC_PROMO-NEXT: atomicrmw add {{.*}} @__profc_foo{{.*}}1), i64 %[[LIVEOUT2]] seq_cst
 ; ATOMIC_PROMO-NEXT: atomicrmw add {{.*}} @__profc_foo{{.*}}2), i64 %[[LIVEOUT3]] seq_cst
-; PROMO-NOT: @__profc_foo
+; PROMO-NOT: @__profc_foo{{.*}})
 
 
 }
Index: llvm/test/Transforms/PGOProfile/associated.ll
===
--- /dev/null
+++ llvm/test/Transforms/PGOProfile/associated.ll
@@ -0,0 +1,11 @@
+; RUN: opt < %s -pgo-instr-gen -instrprof -counter-link-order -S | FileCheck %s
+; RUN: opt < %s -passes=pgo-instr-gen,instrprof -counter-link-order -S | FileCheck %s
+
+; CHECK: @__profc_foo = private global [1 x i64] zeroinitializer, section "__llvm_prf_cnts", align 8, !associated !0
+; CHECK: @__profd_foo = private global {{.*}}, section "__llvm_prf_data", align 8, !associated !0
+
+define void @foo() {
+  ret void
+}
+
+; CHECK: !0 = !{[1 x i64]* @__profc_foo}
Index: llvm/test/Instrumentation/InstrProfiling/linkage.ll
===
--- llvm/test/Instrumentation/InstrProfiling/linkage.ll
+++ llvm/test/Instrumentation/InstrProfiling/linkage.ll
@@ -8,6 +8,8 @@
 ; RUN: opt < %s -mtriple=x86_64-unknown-linux -passes=instrprof -S | FileCheck %s --check-prefixes=POSIX,LINUX
 ; RUN: opt < %s -mtriple=x86_64-unknown-fuchsia -passes=instrprof -S | FileCheck %s --check-prefixes=POSIX,LINUX
 ; RUN: opt < %s  -mtriple=x86_64-pc-win32-coff -passes=instrprof -S | FileCheck %s --check-prefixes=COFF
+; RUN: opt < %s -mtriple=x86_64-unknown-linux -instrprof -counter-link-order -S | FileCheck %s --check-prefixes=LINUX,POSIX,METADATA
+; RUN: opt < %s -mtriple=x86_64-unknown-linux -passes=instrprof -counter-link-order -S | FileCheck %s --check-prefixes=LINUX,POSIX,METADATA
 
 ; MACHO: @__llvm_profile_runtime = external global i32
 ; LINUX-NOT: @__llvm_profile_runtime = external global i32
@@ -19,7 +21,9 @@
 @__profn_foo_extern = linkonce_odr hidden constant [10 x i8] c"foo_extern"
 
 ; POSIX: @__profc_foo = hidden global
+; METADATA-SAME: !associated !0
 ; POSIX: @__profd_foo = hidden global
+; METADATA-SAME: !associated !0
 ; COFF: @__profc_foo = internal global
 ; COFF-NOT: comdat
 ; COFF: @__profd_foo = internal global
@@ -29,7 +33,9 @@
 }
 
 ; POSIX: @__profc_foo_weak = weak hidden global
+; METADATA: !associated !1
 ; POSIX: @__profd_foo_weak = weak hidden global
+; METADATA: !associated !1
 ; COFF: @__profc_foo_weak = internal global
 ; COFF: @__profd_foo_weak = internal global
 define weak void @foo_weak() {
@@ -38,7 +44,9 @@
 }
 
 ; POSIX: @"__profc_linkage.ll:foo_internal" = internal global
+; METADATA-SAME: !associated !2
 ; POSIX: @"__profd_linkage.ll:foo_internal" = internal global
+; METADATA-SAME: !associated !2
 ; COFF: @"__profc_linkage.ll:foo_internal" = internal global
 ; COFF: @"__profd_linkage.ll:foo_internal" = internal global
 define internal void @foo_internal() {
@@ -47,7 +55,9 @@
 }
 
 ; POSIX: @__profc_foo_inline = linkonce_odr hidden global
+; METADATA-SAME: !associated !3
 ; POSIX: @__profd_foo_inline = linkonce_odr hidden global
+; METADATA-SAME: !associated !3
 ; COFF: @__profc_foo_inline = internal global{{.*}} section ".lprfc$M", align 8
 ; COFF: @__profd_foo_inline = internal global{{.*}} section ".lprfd$M", align 8
 define linkonce_odr void @foo_inline() {
@@ -56,7 +66,9 @@
 }
 
 ; LINUX: @__profc_foo_extern = linkonce_odr hidden global {{.*}}section "__llvm_prf_cnts", 

[PATCH] D95772: [DebugInfo] Normalize paths by removing unnecessary dots

[Petr Hosek via Phabricator via cfe-commits]

phosek created this revision.
phosek added reviewers: dblaikie, jhenderson.
phosek requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This was suggested in https://reviews.llvm.org/D87657 as a better
alternative which doesn't require having to guess separators.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D95772

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGen/debug-info-abspath.c
  clang/test/CodeGen/debug-info-relpath.c
  clang/test/CodeGenCXX/linetable-fnbegin.cpp
  clang/test/Modules/debug-info-moduleimport.m


Index: clang/test/Modules/debug-info-moduleimport.m
===
--- clang/test/Modules/debug-info-moduleimport.m
+++ clang/test/Modules/debug-info-moduleimport.m
@@ -16,7 +16,7 @@
 // RUN:-debugger-tuning=lldb -o - | FileCheck %s
 
 // CHECK: ![[CU:.*]] = distinct !DICompileUnit
-// CHECK-SAME:  sysroot: "/tmp/..")
+// CHECK-SAME:  sysroot: "/")
 @import DebugObjC;
 // CHECK: !DIImportedEntity(tag: DW_TAG_imported_declaration, scope: ![[CU]],
 // CHECK-SAME:  entity: ![[MODULE:.*]], file: ![[F:[0-9]+]],
Index: clang/test/CodeGenCXX/linetable-fnbegin.cpp
===
--- clang/test/CodeGenCXX/linetable-fnbegin.cpp
+++ clang/test/CodeGenCXX/linetable-fnbegin.cpp
@@ -4,7 +4,7 @@
 // CHECK: define{{.*}}bar
 // CHECK-NOT: define
 // CHECK: ret {{.*}}, !dbg [[DBG:.*]]
-// CHECK: [[HPP:.*]] = !DIFile(filename: "./template.hpp",
+// CHECK: [[HPP:.*]] = !DIFile(filename: "template.hpp",
 // CHECK: [[SP:.*]] = distinct !DISubprogram(name: "bar",
 // CHECK-SAME:   file: [[HPP]], line: 22
 // CHECK-SAME:   DISPFlagDefinition
Index: clang/test/CodeGen/debug-info-relpath.c
===
--- /dev/null
+++ clang/test/CodeGen/debug-info-relpath.c
@@ -0,0 +1,21 @@
+// RUN: mkdir -p %t/UNIQUEISH_SENTINEL
+// RUN: cp %s %t/UNIQUEISH_SENTINEL/debug-info-abspath.c
+// RUN: cd %t
+
+// RUN: %clang_cc1 -debug-info-kind=limited -triple %itanium_abi_triple \
+// RUN:   UNIQUEISH_SENTINEL/debug-info-abspath.c -emit-llvm -o - \
+// RUN:   | FileCheck %s
+
+// RUN: %clang_cc1 -debug-info-kind=limited -triple %itanium_abi_triple \
+// RUN:   ./UNIQUEISH_SENTINEL/../UNIQUEISH_SENTINEL/debug-info-abspath.c \
+// RUN:   -emit-llvm -o - | FileCheck %s
+
+void foo() {}
+
+// Since %s is a relative path, directory should be the common
+// prefix, but the directory part should be part of the filename.
+
+// CHECK: = distinct !DISubprogram({{.*}}file: ![[SPFILE:[0-9]+]]
+// CHECK: ![[SPFILE]] = !DIFile(filename: "UNIQUEISH_SENTINEL
+// CHECK-SAME:  debug-info-abspath.c"
+// CHECK-NOT:   directory: "{{.*}}UNIQUEISH_SENTINEL")
Index: clang/test/CodeGen/debug-info-abspath.c
===
--- clang/test/CodeGen/debug-info-abspath.c
+++ clang/test/CodeGen/debug-info-abspath.c
@@ -5,6 +5,10 @@
 // RUN:   %t/UNIQUEISH_SENTINEL/debug-info-abspath.c -emit-llvm -o - \
 // RUN:   | FileCheck %s
 
+// RUN: %clang_cc1 -debug-info-kind=limited -triple %itanium_abi_triple \
+// RUN:   %t/UNIQUEISH_SENTINEL/../UNIQUEISH_SENTINEL/debug-info-abspath.c \
+// RUN:   -emit-llvm -o - | FileCheck %s
+
 // RUN: cp %s %t.c
 // RUN: %clang_cc1 -debug-info-kind=limited -triple %itanium_abi_triple \
 // RUN:   %t.c -emit-llvm -o - | FileCheck %s --check-prefix=INTREE
@@ -24,7 +28,7 @@
 // CHECK: = distinct !DISubprogram({{.*}}file: ![[SPFILE:[0-9]+]]
 // CHECK: ![[SPFILE]] = !DIFile(filename: "{{.*}}UNIQUEISH_SENTINEL
 // CHECK-SAME:  debug-info-abspath.c"
-// CHECK-NOT:   directory: "{{.*}}UNIQUEISH_SENTINEL
+// CHECK-NOT:   directory: "{{.*}}UNIQUEISH_SENTINEL)
 
 // INTREE: = distinct !DISubprogram({{.*}}![[SPFILE:[0-9]+]]
 // INTREE: DIFile({{.*}}directory: "{{.+}}CodeGen{{.*}}")
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -477,10 +477,8 @@
 }
 
 std::string CGDebugInfo::remapDIPath(StringRef Path) const {
-  if (DebugPrefixMap.empty())
-return Path.str();
-
   SmallString<256> P = Path;
+  llvm::sys::path::remove_dots(P, /*remove_dot_dot=*/true);
   for (const auto  : DebugPrefixMap)
 if (llvm::sys::path::replace_path_prefix(P, Entry.first, Entry.second))
   break;


Index: clang/test/Modules/debug-info-moduleimport.m
===
--- clang/test/Modules/debug-info-moduleimport.m
+++ clang/test/Modules/debug-info-moduleimport.m
@@ -16,7 +16,7 @@
 // RUN:-debugger-tuning=lldb -o - | FileCheck %s
 
 // CHECK: ![[CU:.*]] = distinct !DICompileUnit
-// CHECK-SAME:  sysroot: "/tmp/..")
+// 

[PATCH] D95771: [clang-tidy] fix modernize-loop-convert to retain needed array-like operator[]

[Conrad Poelman via Phabricator via cfe-commits]

poelmanc created this revision.
poelmanc added reviewers: sammccall, hokein.
poelmanc added a project: clang-tools-extra.
Herald added a subscriber: xazax.hun.
poelmanc requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

`modernize-loop-convert` handles //array-like// objects like vectors fairly 
well, but strips slightly too much information from the iteration expression by 
converting:

  Vector> X;
  for (int J = 0; J < X[5].size(); ++J)
copyArg(X[5][J]);

to

  Vector> X;
  for (int J : X) // should be for (int J : X[5]) 
copyArg(J);

The `[5]` is a call to `operator[]` and gets stripped by 
`LoopConvertCheck::getContainerString`. This patch fixes that and adds several 
test cases.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D95771

Files:
  clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-multidimensional.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-multidimensional.cpp
===
--- /dev/null
+++ 
clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-multidimensional.cpp
@@ -0,0 +1,70 @@
+// RUN: %check_clang_tidy %s modernize-loop-convert %t
+
+template 
+struct Vector {
+  using iterator = T*;
+  unsigned size() const;
+  const T [](int) const;
+  T [](int);
+  T *begin();
+  T *end();
+  const T *begin() const;
+  const T *end() const;
+};
+
+template 
+void copyArg(T);
+
+class TestArrayOfVector {
+  Vector W[2];
+
+  void foo() const {
+for (int I = 0; I < W[0].size(); ++I) {
+  if (W[0][I])
+copyArg(W[0][I]);
+}
+// CHECK-MESSAGES: :[[@LINE-4]]:5: warning: use range-based for loop
+// CHECK-FIXES: for (int I : W[0]) {
+// CHECK-FIXES-NEXT: if (I)
+// CHECK-FIXES-NEXT: copyArg(I);
+  }
+};
+
+class TestVectorOfVector {
+  Vector> X;
+
+  void foo() const {
+for (int J = 0; J < X[0].size(); ++J) {
+  if (X[0][J])
+copyArg(X[0][J]);
+}
+// CHECK-MESSAGES: :[[@LINE-4]]:5: warning: use range-based for loop
+// CHECK-FIXES: for (int J : X[0]) {
+// CHECK-FIXES-NEXT: if (J)
+// CHECK-FIXES-NEXT: copyArg(J);
+  }
+};
+
+void testVectorOfVectorOfVector() {
+  Vector>> Y;
+  for (int J = 0; J < Y[3].size(); ++J) {
+if (Y[3][J][7])
+  copyArg(Y[3][J][8]);
+  }
+  // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop
+  // CHECK-FIXES: for (auto & J : Y[3]) {
+  // CHECK-FIXES-NEXT: if (J[7])
+  // CHECK-FIXES-NEXT: copyArg(J[8]);
+};
+
+void testVectorOfVectorIterator() {
+  Vector> Z;
+  for (Vector::iterator it = Z[4].begin(); it != Z[4].end();  ++it) {
+if (*it)
+  copyArg(*it);
+  }
+  // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop
+  // CHECK-FIXES: for (int & it : Z[4]) {
+  // CHECK-FIXES-NEXT: if (it)
+  // CHECK-FIXES-NEXT: copyArg(it);
+};
Index: clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
@@ -722,10 +722,11 @@
   if (isa(ContainerExpr)) {
 ContainerString = "this";
   } else {
-// For CXXOperatorCallExpr (e.g. vector_ptr->size()), its first argument is
-// the class object (vector_ptr) we are targeting.
+// For CXXOperatorCallExpr such as vector_ptr->size() we want the class
+// object vector_ptr, but for vector[2] we need the whole expression.
 if (const auto* E = dyn_cast(ContainerExpr))
-  ContainerExpr = E->getArg(0);
+  if ( E->getOperator() != OO_Subscript )
+ContainerExpr = E->getArg(0);
 ContainerString =
 getStringFromRange(Context->getSourceManager(), Context->getLangOpts(),
ContainerExpr->getSourceRange());


Index: clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-multidimensional.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-multidimensional.cpp
@@ -0,0 +1,70 @@
+// RUN: %check_clang_tidy %s modernize-loop-convert %t
+
+template 
+struct Vector {
+  using iterator = T*;
+  unsigned size() const;
+  const T [](int) const;
+  T [](int);
+  T *begin();
+  T *end();
+  const T *begin() const;
+  const T *end() const;
+};
+
+template 
+void copyArg(T);
+
+class TestArrayOfVector {
+  Vector W[2];
+
+  void foo() const {
+for (int I = 0; I < W[0].size(); ++I) {
+  if (W[0][I])
+copyArg(W[0][I]);
+}
+// CHECK-MESSAGES: :[[@LINE-4]]:5: warning: use range-based for loop
+// CHECK-FIXES: for (int I : W[0]) {
+// CHECK-FIXES-NEXT: if (I)
+// CHECK-FIXES-NEXT: copyArg(I);
+  }
+};
+
+class TestVectorOfVector {
+  Vector> X;
+
+  void foo() const {
+for 

[PATCH] D95714: [clang-tidy] fix modernize-use-nullptr false positive with spaceship operator comparisons

[Conrad Poelman via Phabricator via cfe-commits]

poelmanc updated this revision to Diff 320393.
poelmanc added a comment.

Thanks @steveire, that suggestion worked perfectly! I added the additional test 
case and shortened the mimicked `strong_ordering` code to a version from 
`clang/unittests.ASTMatchers/ASTMatchersTraversalTest.cpp`, but also manually 
tested this using both MSVC's and libstdc++v3's `` header.


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

https://reviews.llvm.org/D95714

Files:
  clang-tools-extra/clang-tidy/modernize/UseNullptrCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/modernize-use-nullptr-cxx20.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/modernize-use-nullptr-cxx20.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-use-nullptr-cxx20.cpp
@@ -0,0 +1,34 @@
+// RUN: %check_clang_tidy -std=c++20 %s modernize-use-nullptr %t
+
+namespace std {
+struct strong_ordering {
+  int n;
+  constexpr operator int() const { return n; }
+  static const strong_ordering equal, greater, less;
+};
+constexpr strong_ordering strong_ordering::equal = {0};
+constexpr strong_ordering strong_ordering::greater = {1};
+constexpr strong_ordering strong_ordering::less = {-1};
+}
+
+class A {
+public:
+  auto operator<=>(const A ) const = default;
+};
+
+void test_cxx_rewritten_binary_ops() {
+  A a1, a2;
+  bool result;
+  // should not change next line to (a1 nullptr a2)
+  result = (a1 < a2);
+  // CHECK-FIXES: result = (a1 < a2);
+  // should not change next line to (a1 nullptr a2)
+  result = (a1 >= a2);
+  // CHECK-FIXES: result = (a1 >= a2);
+  int *ptr = 0;
+  // CHECK-FIXES: int *ptr = nullptr;
+  result = (a1 > (ptr == 0 ? a1 : a2));
+  // CHECK-FIXES: result = (a1 > (ptr == nullptr ? a1 : a2));
+  result = (a1 > ((a1 > (ptr == 0 ? a1 : a2)) ? a1 : a2));
+  // CHECK-FIXES: result = (a1 > ((a1 > (ptr == nullptr ? a1 : a2)) ? a1 : 
a2));
+}
Index: clang-tools-extra/clang-tidy/modernize/UseNullptrCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/UseNullptrCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/UseNullptrCheck.cpp
@@ -41,12 +41,28 @@
   
unless(hasImplicitDestinationType(qualType(substTemplateTypeParmType(,
   unless(hasSourceExpression(hasType(sugaredNullptrType();
 
+  auto isOrHasDescendant = [](auto InnerMatcher) {
+return anyOf(InnerMatcher, hasDescendant(InnerMatcher));
+  };
+
   return traverse(
   TK_AsIs,
-  castExpr(anyOf(ImplicitCastToNull,
- explicitCastExpr(hasDescendant(ImplicitCastToNull))),
-   unless(hasAncestor(explicitCastExpr(
-  .bind(CastSequence));
+  anyOf(castExpr(anyOf(ImplicitCastToNull,
+   
explicitCastExpr(hasDescendant(ImplicitCastToNull))),
+ unless(hasAncestor(explicitCastExpr())),
+ unless(hasAncestor(cxxRewrittenBinaryOperator(
+.bind(CastSequence),
+cxxRewrittenBinaryOperator(
+// Match rewritten operators, but verify (in the check method)
+// that if an implicit cast is found, it is not from another
+// nested rewritten operator
+expr().bind("matchBinopOperands"),
+hasEitherOperand(isOrHasDescendant(
+implicitCastExpr(
+ImplicitCastToNull,
+hasAncestor(cxxRewrittenBinaryOperator().bind(
+"checkBinopOperands")))
+.bind(CastSequence));
 }
 
 bool isReplaceableRange(SourceLocation StartLoc, SourceLocation EndLoc,
@@ -480,6 +496,11 @@
   const auto *NullCast = Result.Nodes.getNodeAs(CastSequence);
   assert(NullCast && "Bad Callback. No node provided");
 
+  if (Result.Nodes.getNodeAs(
+  "matchBinopOperands") !=
+  Result.Nodes.getNodeAs("checkBinopOperands"))
+return;
+
   // Given an implicit null-ptr cast or an explicit cast with an implicit
   // null-to-pointer cast within use CastSequenceVisitor to identify sequences
   // of explicit casts that can be converted into 'nullptr'.


Index: clang-tools-extra/test/clang-tidy/checkers/modernize-use-nullptr-cxx20.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-use-nullptr-cxx20.cpp
@@ -0,0 +1,34 @@
+// RUN: %check_clang_tidy -std=c++20 %s modernize-use-nullptr %t
+
+namespace std {
+struct strong_ordering {
+  int n;
+  constexpr operator int() const { return n; }
+  static const strong_ordering equal, greater, less;
+};
+constexpr strong_ordering strong_ordering::equal = {0};
+constexpr strong_ordering strong_ordering::greater = {1};
+constexpr strong_ordering strong_ordering::less = {-1};
+}
+
+class A {
+public:
+  auto operator<=>(const A ) const = default;
+};
+
+void 

[clang] e94a35a - [OpenMP] Fix comment and assertion strings (NFC).

[Mike Rice via cfe-commits]

Author: Mike Rice
Date: 2021-01-31T17:17:33-08:00
New Revision: e94a35a744b780fcbe18e8bc6a4f774191588d45

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

LOG: [OpenMP] Fix comment and assertion strings (NFC).

Added: 


Modified: 
clang/lib/Sema/SemaOpenMP.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index 78707484f588..df42767f6ce4 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -12484,7 +12484,7 @@ static OpenMPDirectiveKind 
getOpenMPCaptureRegionForClause(
 case OMPD_distribute_parallel_for_simd:
 case OMPD_distribute:
 case OMPD_distribute_simd:
-  // Do not capture thread_limit-clause expressions.
+  // Do not capture dist_schedule-clause expressions.
   break;
 case OMPD_parallel_for:
 case OMPD_parallel_for_simd:
@@ -12539,7 +12539,7 @@ static OpenMPDirectiveKind 
getOpenMPCaptureRegionForClause(
 case OMPD_atomic:
 case OMPD_target_teams:
 case OMPD_requires:
-  llvm_unreachable("Unexpected OpenMP directive with schedule clause");
+  llvm_unreachable("Unexpected OpenMP directive with dist_schedule 
clause");
 case OMPD_unknown:
 default:
   llvm_unreachable("Unknown OpenMP directive");
@@ -12616,7 +12616,7 @@ static OpenMPDirectiveKind 
getOpenMPCaptureRegionForClause(
 case OMPD_atomic:
 case OMPD_distribute_simd:
 case OMPD_requires:
-  llvm_unreachable("Unexpected OpenMP directive with num_teams-clause");
+  llvm_unreachable("Unexpected OpenMP directive with device-clause");
 case OMPD_unknown:
 default:
   llvm_unreachable("Unknown OpenMP directive");



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

[PATCH] D95714: [clang-tidy] fix modernize-use-nullptr false positive with spaceship operator comparisons

[Stephen Kelly via Phabricator via cfe-commits]

steveire added a comment.

In D95714#2532957 , @poelmanc wrote:

> Thoughts on which option seems like the best path forward?

I think you should be able to correctly match everything. Try a matcher like 
(can probably be cleaned up a bit):

  auto isOrHasDescendant = [](auto InnerMatcher) {
return anyOf(InnerMatcher, hasDescendant(InnerMatcher));
  };
  
  return traverse(
  TK_AsIs,
  anyOf(castExpr(anyOf(ImplicitCastToNull,
   explicitCastExpr(hasDescendant(ImplicitCastToNull))),
 unless(hasAncestor(explicitCastExpr())),
 unless(hasAncestor(cxxRewrittenBinaryOperator(
.bind(CastSequence),
cxxRewrittenBinaryOperator(
// Match rewritten operators, but verify (in the check method)
// that if an implicit cast is found, it is not from another
// nested rewritten operator
expr().bind("matchBinopOperands"),
hasEitherOperand(isOrHasDescendant(
implicitCastExpr(
ImplicitCastToNull,
hasAncestor(cxxRewrittenBinaryOperator().bind(
"checkBinopOperands")))
.bind(CastSequence));

ie, add an `anyOf`, ignore all descendants below `cxxRewrittenBinaryOperator` 
in the first part of the `anyOf`, and look only and 
`cxxRewrittenBinaryOperator` in the second part of it.

Here's another testcase for nested rewritten operators:

  result = (a1 > ((a1 > (ptr == 0 ? a1 : a2)) ? a1 : a2));
  // CHECK-FIXES: result = (a1 > ((a1 > (ptr == nullptr ? a1 : a2)) ? a1 : a2));




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

https://reviews.llvm.org/D95714

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

[clang] 20b1c13 - Fix test in "CFG: Create scope for non-compound range-for body."

[James Y Knight via cfe-commits]

Author: James Y Knight
Date: 2021-01-31T19:56:26-05:00
New Revision: 20b1c1300c8f00d85e1292158f865c1bd0d1e12c

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

LOG: Fix test in "CFG: Create scope for non-compound range-for body."

The constant 4 is sometimes printed as "4L", or "4LL", in CFG dump
output, depending on platform; accept all variants.

Ammends commit 8f670d5b6d8f39bf9bf1d142dacef3afaed6d70b.

Added: 


Modified: 
clang/test/Analysis/auto-obj-dtors-cfg-output.cpp

Removed: 




diff  --git a/clang/test/Analysis/auto-obj-dtors-cfg-output.cpp 
b/clang/test/Analysis/auto-obj-dtors-cfg-output.cpp
index 8e396bddf09d..22df86e1773c 100644
--- a/clang/test/Analysis/auto-obj-dtors-cfg-output.cpp
+++ b/clang/test/Analysis/auto-obj-dtors-cfg-output.cpp
@@ -,9 +,9 @@ void test_for_implicit_scope() {
 // CHECK-NEXT:   3: auto &&__range1 = nums;
 // CHECK-NEXT:   4: __range1
 // CHECK-NEXT:   5: [B4.4] (ImplicitCastExpr, ArrayToPointerDecay, int *)
-// CHECK-NEXT:   6: 4L
+// CHECK-NEXT:   6: 4{{L*}}
 // CHECK-NEXT:   7: [B4.5] + [B4.6]
-// CHECK-NEXT:   8: auto __end1 = __range1 + 4L;
+// CHECK-NEXT:   8: auto __end1 = __range1 + 4{{L*}};
 // CHECK-NEXT:   9: __range1
 // CHECK-NEXT:  10: [B4.9] (ImplicitCastExpr, ArrayToPointerDecay, int *)
 // CHECK-NEXT:  11: auto __begin1 = __range1;



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

[PATCH] D69764: [clang-format] Add East/West Const fixer capability

[Florin Iucha via Phabricator via cfe-commits]

0x8000- added a comment.

In D69764#2532952 , @sammccall wrote:

> In D69764#2532666 , @MyDeveloperDay 
> wrote:
>
>>> What can be done to move this change along?
>>
>> I feel there has to be a fundamental acceptance that it is ok for 
>> clang-format to alter code (something it already does with sorting of 
>> includes, namespace comments).
>>
>> There were fairly strong opinions that clang-format isn't the best tool to 
>> do this (which actually I don't agree with, I think it is, as long as those 
>> capabilities are off by default and there is an acceptance they won't be 
>> perfect especially in the presence of macros due to lack of AST)
>>
>> My only thought about building another tool would be if it was a drop in 
>> replacement for clang-format (tooling allows setting of a path), but it 
>> would need to inherit all of clang-format.
>> but to me, this just feels like extra grunt work just to work around why 
>> some community don't like it.
>
> Yeah, this seems like adding a flag with extra steps.
>
> clang-format's brand is:
>
> - fast
> - semantic no-op
> - applies a consistent, project-specific style
>
> I think putting it (permanently) behind a flag or alternate binary would cut 
> against #3. I don't like that.

It can naturally be behind a flag, with is: east, west, leave_it_be. Skittish 
people/teams can leave_it_be for a release or two.

> If it's buggy, this feature risks cutting against #2 (more than usual). So 
> code supporting this feature is more critical than it was previously, and 
> that might be a lot of heuristics.

How is the risk of this bugginess greater than the risk of any other 
transformation? (header sorting can expose invalid dependencies - and break the 
build).


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

https://reviews.llvm.org/D69764

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

[clang] 8f670d5 - CFG: Create scope for non-compound range-for body.

[James Y Knight via cfe-commits]

Author: James Y Knight
Date: 2021-01-31T18:43:00-05:00
New Revision: 8f670d5b6d8f39bf9bf1d142dacef3afaed6d70b

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

LOG: CFG: Create scope for non-compound range-for body.

Previously, it was omitting the destructor call from the CFG, which
could result in incorrect diagnostics.

Added: 


Modified: 
clang/lib/Analysis/CFG.cpp
clang/test/Analysis/auto-obj-dtors-cfg-output.cpp

Removed: 




diff  --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp
index edc86c41c3b9..56445881873f 100644
--- a/clang/lib/Analysis/CFG.cpp
+++ b/clang/lib/Analysis/CFG.cpp
@@ -4475,8 +4475,14 @@ CFGBlock 
*CFGBuilder::VisitCXXForRangeStmt(CXXForRangeStmt *S) {
 // Add implicit scope and dtors for loop variable.
 addLocalScopeAndDtors(S->getLoopVarStmt());
 
+// If body is not a compound statement create implicit scope
+// and add destructors.
+if (!isa(S->getBody()))
+  addLocalScopeAndDtors(S->getBody());
+
 // Populate a new block to contain the loop body and loop variable.
 addStmt(S->getBody());
+
 if (badCFG)
   return nullptr;
 CFGBlock *LoopVarStmtBlock = addStmt(S->getLoopVarStmt());

diff  --git a/clang/test/Analysis/auto-obj-dtors-cfg-output.cpp 
b/clang/test/Analysis/auto-obj-dtors-cfg-output.cpp
index 5b98dea4c6b1..8e396bddf09d 100644
--- a/clang/test/Analysis/auto-obj-dtors-cfg-output.cpp
+++ b/clang/test/Analysis/auto-obj-dtors-cfg-output.cpp
@@ -1075,6 +1075,59 @@ void test_for_implicit_scope() {
 A c;
 }
 
+// CHECK-LABEL: void test_for_range_implicit_scope()
+// CHECK:  [B5 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B4
+// CHECK:  [B1]
+// CHECK-NEXT:   1: __begin1
+// CHECK-NEXT:   2: [B1.1] (ImplicitCastExpr, LValueToRValue, int *)
+// CHECK-NEXT:   3: __end1
+// CHECK-NEXT:   4: [B1.3] (ImplicitCastExpr, LValueToRValue, int *)
+// CHECK-NEXT:   5: [B1.2] != [B1.4]
+// CHECK-NEXT:   T: for (int n : [B4.2])
+// CHECK-NEXT:[B3.7]
+// CHECK-NEXT:   Preds (2): B2 B4
+// CHECK-NEXT:   Succs (2): B3 B0
+// CHECK:  [B2]
+// CHECK-NEXT:   1: __begin1
+// CHECK-NEXT:   2: ++[B2.1]
+// CHECK-NEXT:   Preds (1): B3
+// CHECK-NEXT:   Succs (1): B1
+// CHECK:  [B3]
+// CHECK-NEXT:   1: __begin1
+// CHECK-NEXT:   2: [B3.1] (ImplicitCastExpr, LValueToRValue, int *)
+// CHECK-NEXT:   3: *[B3.2]
+// CHECK-NEXT:   4: [B3.3] (ImplicitCastExpr, LValueToRValue, int)
+// CHECK-NEXT:   5: int n = *__begin1;
+// WARNINGS-NEXT:   6:  (CXXConstructExpr, class A)
+// ANALYZER-NEXT:   6:  (CXXConstructExpr, [B3.7], class A)
+// CHECK-NEXT:   7: A c;
+// CHECK-NEXT:   8: [B3.7].~A() (Implicit destructor)
+// CHECK-NEXT:   Preds (1): B1
+// CHECK-NEXT:   Succs (1): B2
+// CHECK:  [B4]
+// CHECK-NEXT:   1: int nums[4];
+// CHECK-NEXT:   2: nums
+// CHECK-NEXT:   3: auto &&__range1 = nums;
+// CHECK-NEXT:   4: __range1
+// CHECK-NEXT:   5: [B4.4] (ImplicitCastExpr, ArrayToPointerDecay, int *)
+// CHECK-NEXT:   6: 4L
+// CHECK-NEXT:   7: [B4.5] + [B4.6]
+// CHECK-NEXT:   8: auto __end1 = __range1 + 4L;
+// CHECK-NEXT:   9: __range1
+// CHECK-NEXT:  10: [B4.9] (ImplicitCastExpr, ArrayToPointerDecay, int *)
+// CHECK-NEXT:  11: auto __begin1 = __range1;
+// CHECK-NEXT:   Preds (1): B5
+// CHECK-NEXT:   Succs (1): B1
+// CHECK:  [B0 (EXIT)]
+// CHECK-NEXT:   Preds (1): B1
+void test_for_range_implicit_scope() {
+  int nums[4];
+  for (int n : nums)
+A c;
+}
+
+
 // CHECK:  [B12 (ENTRY)]
 // CHECK-NEXT:   Succs (1): B11
 // CHECK:  [B1]



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

[PATCH] D92403: [LSan][RISCV] Enable LSan for RISCV64

[Luís Marques 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 rG2de4f19ecdb2: [LSan][RISCV] Enable LSan for RISCV64 
(authored by luismarques).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92403

Files:
  clang/lib/Driver/ToolChains/Linux.cpp
  clang/test/Driver/fsanitize.c
  compiler-rt/cmake/config-ix.cmake
  compiler-rt/lib/lsan/lsan_allocator.h
  compiler-rt/lib/lsan/lsan_common.h
  compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp
  compiler-rt/lib/sanitizer_common/sanitizer_platform_interceptors.h
  compiler-rt/test/asan/lit.cfg.py
  compiler-rt/test/lsan/TestCases/use_registers.cpp
  compiler-rt/test/lsan/lit.common.cfg.py
  compiler-rt/test/sanitizer_common/print_address.h

Index: compiler-rt/test/sanitizer_common/print_address.h
===
--- compiler-rt/test/sanitizer_common/print_address.h
+++ compiler-rt/test/sanitizer_common/print_address.h
@@ -8,7 +8,7 @@
   while (n--) {
 void *p = va_arg(ap, void *);
 #if defined(__x86_64__) || defined(__aarch64__) || defined(__powerpc64__) || \
-defined(__s390x__)
+defined(__s390x__) || (defined(__riscv) && __riscv_xlen == 64)
 // On FreeBSD, the %p conversion specifier works as 0x%x and thus does not
 // match to the format used in the diagnotic message.
 fprintf(stderr, "0x%012lx ", (unsigned long) p);
Index: compiler-rt/test/lsan/lit.common.cfg.py
===
--- compiler-rt/test/lsan/lit.common.cfg.py
+++ compiler-rt/test/lsan/lit.common.cfg.py
@@ -76,7 +76,7 @@
 # LeakSanitizer tests are currently supported on
 # Android{aarch64, x86, x86_64}, x86-64 Linux, PowerPC64 Linux, arm Linux, mips64 Linux, s390x Linux and x86_64 Darwin.
 supported_android = config.android and config.target_arch in ['x86_64', 'i386', 'aarch64'] and 'android-thread-properties-api' in config.available_features
-supported_linux = (not config.android) and config.host_os == 'Linux' and config.host_arch in ['x86_64', 'ppc64', 'ppc64le', 'mips64', 'arm', 'armhf', 'armv7l', 's390x']
+supported_linux = (not config.android) and config.host_os == 'Linux' and config.host_arch in ['x86_64', 'ppc64', 'ppc64le', 'mips64', 'riscv64', 'arm', 'armhf', 'armv7l', 's390x']
 supported_darwin = config.host_os == 'Darwin' and config.target_arch in ['x86_64']
 supported_netbsd = config.host_os == 'NetBSD' and config.target_arch in ['x86_64', 'i386']
 if not (supported_android or supported_linux or supported_darwin or supported_netbsd):
Index: compiler-rt/test/lsan/TestCases/use_registers.cpp
===
--- compiler-rt/test/lsan/TestCases/use_registers.cpp
+++ compiler-rt/test/lsan/TestCases/use_registers.cpp
@@ -50,6 +50,10 @@
   asm("lgr %%r10, %0"
   :
   : "r"(p));
+#elif defined(__riscv)
+  asm("mv s11, %0"
+  :
+  : "r"(p));
 #else
 #error "Test is not supported on this architecture."
 #endif
Index: compiler-rt/test/asan/lit.cfg.py
===
--- compiler-rt/test/asan/lit.cfg.py
+++ compiler-rt/test/asan/lit.cfg.py
@@ -210,7 +210,7 @@
 
 # Turn on leak detection on 64-bit Linux.
 leak_detection_android = config.android and 'android-thread-properties-api' in config.available_features and (config.target_arch in ['x86_64', 'i386', 'i686', 'aarch64'])
-leak_detection_linux = (config.host_os == 'Linux') and (not config.android) and (config.target_arch in ['x86_64', 'i386'])
+leak_detection_linux = (config.host_os == 'Linux') and (not config.android) and (config.target_arch in ['x86_64', 'i386', 'riscv64'])
 leak_detection_mac = (config.host_os == 'Darwin') and (config.target_arch == 'x86_64')
 leak_detection_netbsd = (config.host_os == 'NetBSD') and (config.target_arch in ['x86_64', 'i386'])
 if leak_detection_android or leak_detection_linux or leak_detection_mac or leak_detection_netbsd:
Index: compiler-rt/lib/sanitizer_common/sanitizer_platform_interceptors.h
===
--- compiler-rt/lib/sanitizer_common/sanitizer_platform_interceptors.h
+++ compiler-rt/lib/sanitizer_common/sanitizer_platform_interceptors.h
@@ -486,7 +486,7 @@
   (!SI_FREEBSD && !SI_MAC && !SI_NETBSD && SI_NOT_RTEMS)
 #define SANITIZER_INTERCEPT___LIBC_MEMALIGN SI_GLIBC
 #define SANITIZER_INTERCEPT_PVALLOC (SI_GLIBC || SI_ANDROID)
-#define SANITIZER_INTERCEPT_CFREE SI_GLIBC
+#define SANITIZER_INTERCEPT_CFREE (SI_GLIBC && !SANITIZER_RISCV64)
 #define SANITIZER_INTERCEPT_REALLOCARRAY SI_POSIX
 #define SANITIZER_INTERCEPT_ALIGNED_ALLOC (!SI_MAC && SI_NOT_RTEMS)
 #define SANITIZER_INTERCEPT_MALLOC_USABLE_SIZE (!SI_MAC && !SI_NETBSD)
Index: compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp

[clang] 2de4f19 - [LSan][RISCV] Enable LSan for RISCV64

[Luís Marques via cfe-commits]

Author: Luís Marques
Date: 2021-01-31T21:53:25Z
New Revision: 2de4f19ecdb275bcbc6e7ee8368c19a63f99db88

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

LOG: [LSan][RISCV] Enable LSan for RISCV64

Fixes the broken RISCV64 implementation of `internal_clone` and
adds RISCV64 support for LSan.

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

Added: 


Modified: 
clang/lib/Driver/ToolChains/Linux.cpp
clang/test/Driver/fsanitize.c
compiler-rt/cmake/config-ix.cmake
compiler-rt/lib/lsan/lsan_allocator.h
compiler-rt/lib/lsan/lsan_common.h
compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp
compiler-rt/lib/sanitizer_common/sanitizer_platform_interceptors.h
compiler-rt/test/asan/lit.cfg.py
compiler-rt/test/lsan/TestCases/use_registers.cpp
compiler-rt/test/lsan/lit.common.cfg.py
compiler-rt/test/sanitizer_common/print_address.h

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Linux.cpp 
b/clang/lib/Driver/ToolChains/Linux.cpp
index 5f0ce69fc5e6..b7b992124d30 100644
--- a/clang/lib/Driver/ToolChains/Linux.cpp
+++ b/clang/lib/Driver/ToolChains/Linux.cpp
@@ -872,6 +872,7 @@ SanitizerMask Linux::getSupportedSanitizers() const {
  getTriple().getArch() == llvm::Triple::thumb ||
  getTriple().getArch() == llvm::Triple::armeb ||
  getTriple().getArch() == llvm::Triple::thumbeb;
+  const bool IsRISCV64 = getTriple().getArch() == llvm::Triple::riscv64;
   const bool IsSystemZ = getTriple().getArch() == llvm::Triple::systemz;
   SanitizerMask Res = ToolChain::getSupportedSanitizers();
   Res |= SanitizerKind::Address;
@@ -886,7 +887,7 @@ SanitizerMask Linux::getSupportedSanitizers() const {
   if (IsX86_64 || IsMIPS64 || IsAArch64)
 Res |= SanitizerKind::DataFlow;
   if (IsX86_64 || IsMIPS64 || IsAArch64 || IsX86 || IsArmArch || IsPowerPC64 ||
-  IsSystemZ)
+  IsRISCV64 || IsSystemZ)
 Res |= SanitizerKind::Leak;
   if (IsX86_64 || IsMIPS64 || IsAArch64 || IsPowerPC64)
 Res |= SanitizerKind::Thread;

diff  --git a/clang/test/Driver/fsanitize.c b/clang/test/Driver/fsanitize.c
index 8926d55a0cf4..63953c042992 100644
--- a/clang/test/Driver/fsanitize.c
+++ b/clang/test/Driver/fsanitize.c
@@ -434,6 +434,12 @@
 // RUN: %clang -target powerpc-unknown-linux -fsanitize=leak %s -### 2>&1 | 
FileCheck %s --check-prefix=CHECK-SANL-PPC
 // CHECK-SANL-PPC: unsupported option '-fsanitize=leak' for target 
'powerpc-unknown-linux'
 
+// RUN: %clang -target riscv64-linux-gnu -fsanitize=leak %s -### 2>&1 | 
FileCheck %s --check-prefix=CHECK-SANL-RISCV64
+// CHECK-SANL-RISCV64: "-fsanitize=leak"
+
+// RUN: %clang -target riscv64-linux-gnu -fsanitize=address,leak 
-fno-sanitize=address %s -### 2>&1 | FileCheck %s 
--check-prefix=CHECK-SANA-SANL-NO-SANA-RISCV64
+// CHECK-SANA-SANL-NO-SANA-RISCV64: "-fsanitize=leak"
+
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=memory %s -### 2>&1 | 
FileCheck %s --check-prefix=CHECK-MSAN
 // CHECK-MSAN: "-fno-assume-sane-operator-new"
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=address %s -### 2>&1 | 
FileCheck %s --check-prefix=CHECK-ASAN

diff  --git a/compiler-rt/cmake/config-ix.cmake 
b/compiler-rt/cmake/config-ix.cmake
index f81b8384cbd5..813573802aec 100644
--- a/compiler-rt/cmake/config-ix.cmake
+++ b/compiler-rt/cmake/config-ix.cmake
@@ -322,7 +322,7 @@ set(ALL_GWP_ASAN_SUPPORTED_ARCH ${X86} ${X86_64})
 if(APPLE)
   set(ALL_LSAN_SUPPORTED_ARCH ${X86} ${X86_64} ${MIPS64} ${ARM64})
 else()
-  set(ALL_LSAN_SUPPORTED_ARCH ${X86} ${X86_64} ${MIPS64} ${ARM64} ${ARM32} 
${PPC64} ${S390X})
+  set(ALL_LSAN_SUPPORTED_ARCH ${X86} ${X86_64} ${MIPS64} ${ARM64} ${ARM32} 
${PPC64} ${S390X} ${RISCV64})
 endif()
 set(ALL_MSAN_SUPPORTED_ARCH ${X86_64} ${MIPS64} ${ARM64} ${PPC64} ${S390X})
 set(ALL_HWASAN_SUPPORTED_ARCH ${X86_64} ${ARM64})

diff  --git a/compiler-rt/lib/lsan/lsan_allocator.h 
b/compiler-rt/lib/lsan/lsan_allocator.h
index 17e13cd014ba..9d763789154f 100644
--- a/compiler-rt/lib/lsan/lsan_allocator.h
+++ b/compiler-rt/lib/lsan/lsan_allocator.h
@@ -50,7 +50,7 @@ struct ChunkMetadata {
 };
 
 #if defined(__mips64) || defined(__aarch64__) || defined(__i386__) || \
-defined(__arm__)
+defined(__arm__) || SANITIZER_RISCV64
 template 
 struct AP32 {
   static const uptr kSpaceBeg = 0;

diff  --git a/compiler-rt/lib/lsan/lsan_common.h 
b/compiler-rt/lib/lsan/lsan_common.h
index b0ae6f020b63..f579d6115ba3 100644
--- a/compiler-rt/lib/lsan/lsan_common.h
+++ b/compiler-rt/lib/lsan/lsan_common.h
@@ -41,6 +41,8 @@
 #define CAN_SANITIZE_LEAKS 1
 #elif defined(__arm__) && SANITIZER_LINUX
 #define CAN_SANITIZE_LEAKS 1
+#elif SANITIZER_RISCV64 && SANITIZER_LINUX
+#define CAN_SANITIZE_LEAKS 1
 #elif SANITIZER_NETBSD || 

[PATCH] D95714: [clang-tidy] fix modernize-use-nullptr false positive with spaceship operator comparisons

[Conrad Poelman via Phabricator via cfe-commits]

poelmanc added a comment.

In D95714#2532565 , @njames93 wrote:

> This does highlight an issue where the mimicked std library stubs used for 
> tests don't correspond exactly to what the stdlib actually looks like and can 
> result in subtly different ASTs that have added/removed implicit nodes.
>
> Going a little off point here but a few months back I pushed a fix for 
> another check that passed its tests. 
> However the bug report was re-opened as the bug was still observable in the 
> real word. 
> Turned out the implementation of std::string used for the test had a trivial 
> destructor resulting in the AST not needed to emit `CXXBindTemporaryExpr`s 
> all over the place, which threw off the matching logic.
>
> Unfortunately this kind of disparity is hard to detect in tests so it may be 
> wise to test this locally using the compare header from a real standard 
> library implementation (preferably all 3 main ones if you have the machines) 
> and see if this behaviour is correct.

Indeed the //mimicked std library// approach has real advantages like running 
fast and giving consistent results across platforms. But it also has drawbacks 
like the replication of //mimicked std// code across tests, and possible 
differences between test and real world behaviour. We could give tests a 
`CLANG_TIDY_TEST_NATIVE_STD` macro to switch between //mimicked std// code and 
real headers, and set up CI to test both ways, to catch these issues at the 
expense of added complexity. But that's a broader discussion.

Back to `modernize-use-nullptr`, these days I'm working in MSVC and the tests 
pass with MSVC's ``, with my mimicked compare, and a shorter mimicked 
`strong_ordering` that I found in `ASTTraverserTest.cpp`. But I fetched gcc's 
`` from 
https://raw.githubusercontent.com/gcc-mirror/gcc/master/libstdc%2B%2B-v3/libsupc%2B%2B/compare
 and now see the same ASTs as you, and the proposed fix fails.

Thoughts on which option seems like the best path forward?

1. In the ASTs I've seen so far the third child of the 
`CXXRewrittenBinaryOperator`'s `CXXOperatorCallExpr ` is the one to ignore. But 
comments in :268-283 imply that perhaps sometimes the `0` 
may appear first? There should be some way to determine which child was added 
by the `CXXRewrittenBinaryOperator` rewrite and skip that child, regardless of 
`std` implementation, but I have yet to figure that out.
2. Roll back to `hasAncestor`, fixing the immediate problem of creating invalid 
fixes (`a1 nullptr 2a`) and accepting that some rare but fixable code (`a1 < ( 
ptr == 0 ? a2 : a3`) will not be modernized to `nullptr`.
3. Change the bugfix approach here from AST-based to text-based. In 
`replaceWithNullptr` in `UseNullptrCheck.cpp:62`, add code to skip the planned 
replacement if the text we're about to replace with `nullptr` consists of `<`, 
`=`, and `>`. As a quick test, this version seems to make all tests pass and 
doesn't depend on std header implementation:

  static const std::string SuspiciousChars("<=>");
  char FirstCharToReplace = *SM.getCharacterData(StartLoc);
  char LastCharToReplace = *SM.getCharacterData(EndLoc);
  if (SuspiciousChars.find(FirstCharToReplace) != std::string::npos &&
  SuspiciousChars.find(LastCharToReplace) != std::string::npos)
return;


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

https://reviews.llvm.org/D95714

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

[PATCH] D92403: [LSan][RISCV] Enable LSan for RISCV64

[Luís Marques via Phabricator via cfe-commits]

luismarques updated this revision to Diff 320375.
luismarques added a comment.
Herald added a subscriber: vkmr.

Rebase and address formatting issues.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92403

Files:
  clang/lib/Driver/ToolChains/Linux.cpp
  clang/test/Driver/fsanitize.c
  compiler-rt/cmake/config-ix.cmake
  compiler-rt/lib/lsan/lsan_allocator.h
  compiler-rt/lib/lsan/lsan_common.h
  compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp
  compiler-rt/lib/sanitizer_common/sanitizer_platform_interceptors.h
  compiler-rt/test/asan/lit.cfg.py
  compiler-rt/test/lsan/TestCases/use_registers.cpp
  compiler-rt/test/lsan/lit.common.cfg.py
  compiler-rt/test/sanitizer_common/print_address.h

Index: compiler-rt/test/sanitizer_common/print_address.h
===
--- compiler-rt/test/sanitizer_common/print_address.h
+++ compiler-rt/test/sanitizer_common/print_address.h
@@ -8,7 +8,7 @@
   while (n--) {
 void *p = va_arg(ap, void *);
 #if defined(__x86_64__) || defined(__aarch64__) || defined(__powerpc64__) || \
-defined(__s390x__)
+defined(__s390x__) || (defined(__riscv) && __riscv_xlen == 64)
 // On FreeBSD, the %p conversion specifier works as 0x%x and thus does not
 // match to the format used in the diagnotic message.
 fprintf(stderr, "0x%012lx ", (unsigned long) p);
Index: compiler-rt/test/lsan/lit.common.cfg.py
===
--- compiler-rt/test/lsan/lit.common.cfg.py
+++ compiler-rt/test/lsan/lit.common.cfg.py
@@ -76,7 +76,7 @@
 # LeakSanitizer tests are currently supported on
 # Android{aarch64, x86, x86_64}, x86-64 Linux, PowerPC64 Linux, arm Linux, mips64 Linux, s390x Linux and x86_64 Darwin.
 supported_android = config.android and config.target_arch in ['x86_64', 'i386', 'aarch64'] and 'android-thread-properties-api' in config.available_features
-supported_linux = (not config.android) and config.host_os == 'Linux' and config.host_arch in ['x86_64', 'ppc64', 'ppc64le', 'mips64', 'arm', 'armhf', 'armv7l', 's390x']
+supported_linux = (not config.android) and config.host_os == 'Linux' and config.host_arch in ['x86_64', 'ppc64', 'ppc64le', 'mips64', 'riscv64', 'arm', 'armhf', 'armv7l', 's390x']
 supported_darwin = config.host_os == 'Darwin' and config.target_arch in ['x86_64']
 supported_netbsd = config.host_os == 'NetBSD' and config.target_arch in ['x86_64', 'i386']
 if not (supported_android or supported_linux or supported_darwin or supported_netbsd):
Index: compiler-rt/test/lsan/TestCases/use_registers.cpp
===
--- compiler-rt/test/lsan/TestCases/use_registers.cpp
+++ compiler-rt/test/lsan/TestCases/use_registers.cpp
@@ -50,6 +50,10 @@
   asm("lgr %%r10, %0"
   :
   : "r"(p));
+#elif defined(__riscv)
+  asm("mv s11, %0"
+  :
+  : "r"(p));
 #else
 #error "Test is not supported on this architecture."
 #endif
Index: compiler-rt/test/asan/lit.cfg.py
===
--- compiler-rt/test/asan/lit.cfg.py
+++ compiler-rt/test/asan/lit.cfg.py
@@ -210,7 +210,7 @@
 
 # Turn on leak detection on 64-bit Linux.
 leak_detection_android = config.android and 'android-thread-properties-api' in config.available_features and (config.target_arch in ['x86_64', 'i386', 'i686', 'aarch64'])
-leak_detection_linux = (config.host_os == 'Linux') and (not config.android) and (config.target_arch in ['x86_64', 'i386'])
+leak_detection_linux = (config.host_os == 'Linux') and (not config.android) and (config.target_arch in ['x86_64', 'i386', 'riscv64'])
 leak_detection_mac = (config.host_os == 'Darwin') and (config.target_arch == 'x86_64')
 leak_detection_netbsd = (config.host_os == 'NetBSD') and (config.target_arch in ['x86_64', 'i386'])
 if leak_detection_android or leak_detection_linux or leak_detection_mac or leak_detection_netbsd:
Index: compiler-rt/lib/sanitizer_common/sanitizer_platform_interceptors.h
===
--- compiler-rt/lib/sanitizer_common/sanitizer_platform_interceptors.h
+++ compiler-rt/lib/sanitizer_common/sanitizer_platform_interceptors.h
@@ -486,7 +486,7 @@
   (!SI_FREEBSD && !SI_MAC && !SI_NETBSD && SI_NOT_RTEMS)
 #define SANITIZER_INTERCEPT___LIBC_MEMALIGN SI_GLIBC
 #define SANITIZER_INTERCEPT_PVALLOC (SI_GLIBC || SI_ANDROID)
-#define SANITIZER_INTERCEPT_CFREE SI_GLIBC
+#define SANITIZER_INTERCEPT_CFREE (SI_GLIBC && !SANITIZER_RISCV64)
 #define SANITIZER_INTERCEPT_REALLOCARRAY SI_POSIX
 #define SANITIZER_INTERCEPT_ALIGNED_ALLOC (!SI_MAC && SI_NOT_RTEMS)
 #define SANITIZER_INTERCEPT_MALLOC_USABLE_SIZE (!SI_MAC && !SI_NETBSD)
Index: compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp
===
--- compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp
+++ 

[PATCH] D69764: [clang-format] Add East/West Const fixer capability

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D69764#2532666 , @MyDeveloperDay 
wrote:

>> What can be done to move this change along?
>
> I feel there has to be a fundamental acceptance that it is ok for 
> clang-format to alter code (something it already does with sorting of 
> includes, namespace comments).
>
> There were fairly strong opinions that clang-format isn't the best tool to do 
> this (which actually I don't agree with, I think it is, as long as those 
> capabilities are off by default and there is an acceptance they won't be 
> perfect especially in the presence of macros due to lack of AST)
>
> My only thought about building another tool would be if it was a drop in 
> replacement for clang-format (tooling allows setting of a path), but it would 
> need to inherit all of clang-format.
> but to me, this just feels like extra grunt work just to work around why some 
> community don't like it.

Yeah, this seems like adding a flag with extra steps.

clang-format's brand is:

- fast
- semantic no-op
- applies a consistent, project-specific style

I think putting it (permanently) behind a flag or alternate binary would cut 
against #3. I don't like that.

If it's buggy, this feature risks cutting against #2 (more than usual). So code 
supporting this feature is more critical than it was previously, and that might 
be a lot of heuristics.
So I'm wary, but also not really an active maintainer. As long as this concern 
has been considered, I'm not opposed!

> I guess a consensus is hard to come by, but I don't really know who owns the 
> decision around the future direction of clang-format.

In terms of practical maintainership, you're in a strong position to make this 
call. We've had a robust discussion, there are clear pros, cons, and some bits 
that aren't agreed.
@rsmith is CODE_OWNER for clang/... and so has a veto here, but doesn't sound 
inclined to use it :-)


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

https://reviews.llvm.org/D69764

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

[PATCH] D95764: [OpenMP] Do not propagate match extensions to nested contexts

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

In D95764#2532927 , @JonChesterfield 
wrote:

> I would have expected a nested match statement to hit on a subset of those 
> that matched in the parent.
>
> If (match x64)
> If (match Intel)
>
> - expect x64 and intel to be true here
>
> I think that's how the normal openmp variant match works.

It is.

> Why do we want to diverge from that for (all?) extensions?

For the same reason we have `match_any` and other things that modify the 
standard behavior, it is useful or even necessary to solve certain problems.

> It would make for a semantic break if one of the extensions was standardised.

Yes and no. Like a breaking change in the standard, that could happen on a 
conceptual level. However, if the standard would adopt part of the `match_any`
semantics, it would not assume it to be in the `extension` trait set. Doing so 
would introduce problems for downstream extensions which are basically
breaking changes. That said, you have to remember that the semantic of 
`match_any` (and `match_none`) was made up for LLVM and is already changing the
behavior of the variant selector compared to the default described in the 
standard. The argument of this patch is that this made up semantics should be
tweaked. FWIW, when I added the extension I haven't had implemented the 
selector inheritance yet, so at that point there was no practical difference.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95764

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

[PATCH] D95764: [OpenMP] Do not propagate match extensions to nested contexts

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a comment.

I would have expected a nested match statement to hit on a subset of those that 
matched in the parent.

If (match x64)
If (match Intel)

- expect x64 and intel to be true here

I think that's how the normal openmp variant match works. Why do we want to 
diverge from that for (all?) extensions? It would make for a semantic break if 
one of the extensions was standardised.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95764

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

[PATCH] D95764: [OpenMP] Do not propagate match extensions to nested contexts

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

In D95764#2532908 , @tianshilei1992 
wrote:

> In D95764#2532877 , @jdoerfert wrote:
>
>> In D95764#2532875 , @tianshilei1992 
>> wrote:
>>
>>> So I suppose D95765  can replace this 
>>> patch, right?
>>
>> I think we want both. Propagating match clauses doesn't make sense to me so 
>> we should never do it.
>
> Is there any place documenting the propagation?

The standard documents that nested contexts will inherit the selectors, in a 
specific way. given that this is an llvm extension, we can overwrite that 
though. I added the documentation changes now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95764

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

[PATCH] D95764: [OpenMP] Do not propagate match extensions to nested contexts

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert updated this revision to Diff 320371.
jdoerfert added a comment.
Herald added a reviewer: aaron.ballman.

Update docs


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95764

Files:
  clang/include/clang/Basic/AttrDocs.td
  clang/lib/Parse/ParseOpenMP.cpp
  clang/test/OpenMP/begin_declare_variant_nested_propagation.c


Index: clang/test/OpenMP/begin_declare_variant_nested_propagation.c
===
--- /dev/null
+++ clang/test/OpenMP/begin_declare_variant_nested_propagation.c
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -triple=x86_64-pc-win32 -verify -fopenmp -x c -std=c99 
-fms-extensions -Wno-pragma-pack %s
+// expected-no-diagnostics
+
+#pragma omp begin declare variant match(implementation={extension(match_any)})
+#pragma omp begin declare variant match(device = {vendor(cray, ibm)})
+ this is never reached, we cannot have a cray ibm compiler hybrid, I hope.
+#pragma omp end declare variant
+#pragma omp end declare variant
Index: clang/lib/Parse/ParseOpenMP.cpp
===
--- clang/lib/Parse/ParseOpenMP.cpp
+++ clang/lib/Parse/ParseOpenMP.cpp
@@ -1477,6 +1477,14 @@
   MergedSelector = true;
   for (const OMPTraitProperty  :
ParentSelector.Properties) {
+// Do not propagate match extensions to nested contexts.
+if (ParentProperty.Kind == llvm::omp::TraitProperty::
+   implementation_extension_match_any 
||
+ParentProperty.Kind == llvm::omp::TraitProperty::
+   implementation_extension_match_all 
||
+ParentProperty.Kind == llvm::omp::TraitProperty::
+   implementation_extension_match_none)
+  continue;
 bool MergedProperty = false;
 for (OMPTraitProperty  : Selector.Properties) {
   // Ignore "equivalent" properties.
Index: clang/include/clang/Basic/AttrDocs.td
===
--- clang/include/clang/Basic/AttrDocs.td
+++ clang/include/clang/Basic/AttrDocs.td
@@ -3965,7 +3965,8 @@
 match for an OpenMP context. The default is ``all``, with ``none`` no trait in 
the
 selector is allowed to be in the OpenMP context, with ``any`` a single trait in
 both the selector and OpenMP context is sufficient. Only a single match
-extension trait is allowed per context selector.
+extension trait is allowed per context selector. Note that match extensions are
+not propagated to nested selectors like the standard defines it for other 
traits.
 The disable extensions remove default effects of the ``begin declare variant``
 applied to a definition. If ``disable_implicit_base`` is given, we will not
 introduce an implicit base function for a variant if no base function was


Index: clang/test/OpenMP/begin_declare_variant_nested_propagation.c
===
--- /dev/null
+++ clang/test/OpenMP/begin_declare_variant_nested_propagation.c
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -triple=x86_64-pc-win32 -verify -fopenmp -x c -std=c99 -fms-extensions -Wno-pragma-pack %s
+// expected-no-diagnostics
+
+#pragma omp begin declare variant match(implementation={extension(match_any)})
+#pragma omp begin declare variant match(device = {vendor(cray, ibm)})
+ this is never reached, we cannot have a cray ibm compiler hybrid, I hope.
+#pragma omp end declare variant
+#pragma omp end declare variant
Index: clang/lib/Parse/ParseOpenMP.cpp
===
--- clang/lib/Parse/ParseOpenMP.cpp
+++ clang/lib/Parse/ParseOpenMP.cpp
@@ -1477,6 +1477,14 @@
   MergedSelector = true;
   for (const OMPTraitProperty  :
ParentSelector.Properties) {
+// Do not propagate match extensions to nested contexts.
+if (ParentProperty.Kind == llvm::omp::TraitProperty::
+   implementation_extension_match_any ||
+ParentProperty.Kind == llvm::omp::TraitProperty::
+   implementation_extension_match_all ||
+ParentProperty.Kind == llvm::omp::TraitProperty::
+   implementation_extension_match_none)
+  continue;
 bool MergedProperty = false;
 for (OMPTraitProperty  : Selector.Properties) {
   // Ignore "equivalent" properties.
Index: clang/include/clang/Basic/AttrDocs.td
===
--- clang/include/clang/Basic/AttrDocs.td
+++ clang/include/clang/Basic/AttrDocs.td
@@ -3965,7 +3965,8 @@
 match for an OpenMP context. The default is ``all``, with ``none`` no trait in the
 

[PATCH] D95764: [OpenMP] Do not propagate match extensions to nested contexts

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 added a comment.

In D95764#2532877 , @jdoerfert wrote:

> In D95764#2532875 , @tianshilei1992 
> wrote:
>
>> So I suppose D95765  can replace this 
>> patch, right?
>
> I think we want both. Propagating match clauses doesn't make sense to me so 
> we should never do it.

Is there any place documenting the propagation?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95764

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

[PATCH] D95764: [OpenMP] Do not propagate match extensions to nested contexts

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

In D95764#2532875 , @tianshilei1992 
wrote:

> So I suppose D95765  can replace this patch, 
> right?

I think we want both. Propagating match clauses doesn't make sense to me so we 
should never do it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95764

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

[PATCH] D95764: [OpenMP] Do not propagate match extensions to nested contexts

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 added a comment.

So I suppose D95765  can replace this patch, 
right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95764

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

[PATCH] D95765: [OpenMP] Introduce the `disable_selector_propagation` variant selector trait

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert created this revision.
jdoerfert added reviewers: JonChesterfield, tianshilei1992.
Herald added subscribers: guansong, yaxunl.
Herald added a reviewer: bollu.
Herald added a reviewer: aaron.ballman.
jdoerfert requested review of this revision.
Herald added subscribers: llvm-commits, cfe-commits, sstefan1.
Herald added projects: clang, LLVM.

Nested `omp [begin|end] declare variant` inherit the selectors from
surrounding `omp (begin|end) declare variant` constructs. To stop such
propagation the user can add the `disable_selector_propagation` to the
`extension` set in the `implementation` selector.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D95765

Files:
  clang/include/clang/Basic/AttrDocs.td
  clang/lib/Parse/ParseOpenMP.cpp
  clang/test/OpenMP/begin_declare_variant_nested_propagation.c
  clang/test/OpenMP/declare_variant_messages.c
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def

Index: llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
===
--- llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
+++ llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
@@ -1057,6 +1057,7 @@
 __OMP_TRAIT_PROPERTY(implementation, extension, match_none)
 __OMP_TRAIT_PROPERTY(implementation, extension, disable_implicit_base)
 __OMP_TRAIT_PROPERTY(implementation, extension, allow_templates)
+__OMP_TRAIT_PROPERTY(implementation, extension, disable_selector_propagation)
 
 __OMP_TRAIT_SET(user)
 
Index: clang/test/OpenMP/declare_variant_messages.c
===
--- clang/test/OpenMP/declare_variant_messages.c
+++ clang/test/OpenMP/declare_variant_messages.c
@@ -170,6 +170,14 @@
 int conflicting_nested_score(void);
 #pragma omp end declare variant
 
+#pragma omp begin declare variant match(implementation = {vendor(score(1) \
+ : llvm),extension(disable_selector_propagation)})
+#pragma omp declare variant(foo) match(implementation = {vendor(score(2) \
+: llvm)})
+int conflicting_nested_score_no_prop(void);
+#pragma omp end declare variant
+
+
 // FIXME: We should build the conjuction of different conditions, see also the score fixme above.
 #pragma omp begin declare variant match(user = {condition(1)})
 #pragma omp declare variant(foo) match(user = {condition(1)}) // expected-error {{nested user conditions in OpenMP context selector not supported (yet)}}
Index: clang/test/OpenMP/begin_declare_variant_nested_propagation.c
===
--- clang/test/OpenMP/begin_declare_variant_nested_propagation.c
+++ clang/test/OpenMP/begin_declare_variant_nested_propagation.c
@@ -1,8 +1,22 @@
 // RUN: %clang_cc1 -triple=x86_64-pc-win32 -verify -fopenmp -x c -std=c99 -fms-extensions -Wno-pragma-pack %s
-// expected-no-diagnostics
 
 #pragma omp begin declare variant match(implementation={extension(match_any)})
 #pragma omp begin declare variant match(device = {vendor(cray, ibm)})
  this is never reached, we cannot have a cray ibm compiler hybrid, I hope.
 #pragma omp end declare variant
 #pragma omp end declare variant
+
+#pragma omp begin declare variant match(implementation={extension(disable_implicit_base, disable_selector_propagation)})
+
+ void without_implicit_base() {}
+
+#pragma omp begin declare variant match(implementation = {vendor(llvm)})
+ void with_implicit_base() {}
+#pragma omp end declare variant
+
+#pragma omp end declare variant
+
+ void test() {
+   without_implicit_base(); // expected-warning{{implicit declaration of function 'without_implicit_base' is invalid in C99}}
+   with_implicit_base();
+ }
Index: clang/lib/Parse/ParseOpenMP.cpp
===
--- clang/lib/Parse/ParseOpenMP.cpp
+++ clang/lib/Parse/ParseOpenMP.cpp
@@ -947,6 +947,10 @@
   TraitProperty::implementation_extension_disable_implicit_base)
 return true;
 
+  if (TIProperty.Kind ==
+  TraitProperty::implementation_extension_disable_selector_propagation)
+return true;
+
   if (TIProperty.Kind ==
   TraitProperty::implementation_extension_allow_templates)
 return true;
@@ -1459,7 +1463,11 @@
 return false;
 
   // Merge the parent/outer trait info into the one we just parsed and diagnose
-  // problems.
+  // problems. Can be disabled by the disable_selector_propagation extension.
+  if (ParentTI->isExtensionActive(
+  llvm::omp::TraitProperty::
+  implementation_extension_disable_selector_propagation))
+return false;
   // TODO: Keep some source location in the TI to provide better diagnostics.
   // TODO: Perform some kind of equivalence check on the condition and score
   //   expressions.
Index: clang/include/clang/Basic/AttrDocs.td
===
--- 

[PATCH] D95764: [OpenMP] Do not propagate match extensions to nested contexts

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert created this revision.
jdoerfert added reviewers: JonChesterfield, tianshilei1992.
Herald added subscribers: guansong, yaxunl.
Herald added a reviewer: bollu.
jdoerfert requested review of this revision.
Herald added subscribers: cfe-commits, sstefan1.
Herald added a project: clang.

If we have nested declare variant context, it doesn't make sense to
inherit the match extension from the parent. Instead, just skip it.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D95764

Files:
  clang/lib/Parse/ParseOpenMP.cpp
  clang/test/OpenMP/begin_declare_variant_nested_propagation.c


Index: clang/test/OpenMP/begin_declare_variant_nested_propagation.c
===
--- /dev/null
+++ clang/test/OpenMP/begin_declare_variant_nested_propagation.c
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -triple=x86_64-pc-win32 -verify -fopenmp -x c -std=c99 
-fms-extensions -Wno-pragma-pack %s
+// expected-no-diagnostics
+
+#pragma omp begin declare variant match(implementation={extension(match_any)})
+#pragma omp begin declare variant match(device = {vendor(cray, ibm)})
+ this is never reached, we cannot have a cray ibm compiler hybrid, I hope.
+#pragma omp end declare variant
+#pragma omp end declare variant
Index: clang/lib/Parse/ParseOpenMP.cpp
===
--- clang/lib/Parse/ParseOpenMP.cpp
+++ clang/lib/Parse/ParseOpenMP.cpp
@@ -1477,6 +1477,14 @@
   MergedSelector = true;
   for (const OMPTraitProperty  :
ParentSelector.Properties) {
+// Do not propagate match extensions to nested contexts.
+if (ParentProperty.Kind == llvm::omp::TraitProperty::
+   implementation_extension_match_any 
||
+ParentProperty.Kind == llvm::omp::TraitProperty::
+   implementation_extension_match_all 
||
+ParentProperty.Kind == llvm::omp::TraitProperty::
+   implementation_extension_match_none)
+  continue;
 bool MergedProperty = false;
 for (OMPTraitProperty  : Selector.Properties) {
   // Ignore "equivalent" properties.


Index: clang/test/OpenMP/begin_declare_variant_nested_propagation.c
===
--- /dev/null
+++ clang/test/OpenMP/begin_declare_variant_nested_propagation.c
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -triple=x86_64-pc-win32 -verify -fopenmp -x c -std=c99 -fms-extensions -Wno-pragma-pack %s
+// expected-no-diagnostics
+
+#pragma omp begin declare variant match(implementation={extension(match_any)})
+#pragma omp begin declare variant match(device = {vendor(cray, ibm)})
+ this is never reached, we cannot have a cray ibm compiler hybrid, I hope.
+#pragma omp end declare variant
+#pragma omp end declare variant
Index: clang/lib/Parse/ParseOpenMP.cpp
===
--- clang/lib/Parse/ParseOpenMP.cpp
+++ clang/lib/Parse/ParseOpenMP.cpp
@@ -1477,6 +1477,14 @@
   MergedSelector = true;
   for (const OMPTraitProperty  :
ParentSelector.Properties) {
+// Do not propagate match extensions to nested contexts.
+if (ParentProperty.Kind == llvm::omp::TraitProperty::
+   implementation_extension_match_any ||
+ParentProperty.Kind == llvm::omp::TraitProperty::
+   implementation_extension_match_all ||
+ParentProperty.Kind == llvm::omp::TraitProperty::
+   implementation_extension_match_none)
+  continue;
 bool MergedProperty = false;
 for (OMPTraitProperty  : Selector.Properties) {
   // Ignore "equivalent" properties.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92257: [clang-format] Add option to control the space at the front of a line comment

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

In D92257#2532077 , @MyDeveloperDay 
wrote:

> I have a script that runs clang-format -n on various directories in clang
> that are clang format clean, polly is one of them because they have clang
> format as a unit test
>
> I use this to ensure I don’t regress behaviour
>
> Maybe we should formalise this with some sort of clang-format-check cmake
> rule
>
> Mydeveloperday

That would be ok for me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92257

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

[PATCH] D69764: [clang-format] Add East/West Const fixer capability

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

In D69764#2532666 , @MyDeveloperDay 
wrote:

>> What can be done to move this change along?
>
> I feel there has to be a fundamental acceptance that it is ok for 
> clang-format to alter code (something it already does with sorting of 
> includes, namespace comments).
>
> There were fairly strong opinions that clang-format isn't the best tool to do 
> this (which actually I don't agree with, I think it is, as long as those 
> capabilities are off by default and there is an acceptance they won't be 
> perfect especially in the presence of macros due to lack of AST)
>
> My only thought about building another tool would be if it was a drop in 
> replacement for clang-format (tooling allows setting of a path), but it would 
> need to inherit all of clang-format.
>
> but to me, this just feels like extra grunt work just to work around why some 
> community don't like it.
>
> I guess a consensus is hard to come by, but I don't really know who owns the 
> decision around the future direction of clang-format.

I wouldn't mind if it was implemented in clang-format.


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

https://reviews.llvm.org/D69764

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

[clang-tools-extra] 7de711e - Reland [clangd] Quote/escape argv included in log messages.

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2021-01-31T16:39:47+01:00
New Revision: 7de711ecca99f81da3c2ae1705cefe0b4bda70b3

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

LOG: Reland [clangd] Quote/escape argv included in log messages.

... but don't apply it where we're using hasSubstr

This reverts commit 7a8008d0e8885d22ff9a1fa7f9965c7b2ad2569a.

Added: 


Modified: 
clang-tools-extra/clangd/CompileCommands.cpp
clang-tools-extra/clangd/CompileCommands.h
clang-tools-extra/clangd/QueryDriverDatabase.cpp
clang-tools-extra/clangd/TUScheduler.cpp
clang-tools-extra/clangd/tool/Check.cpp
clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/CompileCommands.cpp 
b/clang-tools-extra/clangd/CompileCommands.cpp
index 96cbd8806ff6..b55d1b03dee6 100644
--- a/clang-tools-extra/clangd/CompileCommands.cpp
+++ b/clang-tools-extra/clangd/CompileCommands.cpp
@@ -503,5 +503,32 @@ void ArgStripper::process(std::vector ) 
const {
   Args.resize(Write);
 }
 
+
+std::string printArgv(llvm::ArrayRef Args) {
+  std::string Buf;
+  llvm::raw_string_ostream OS(Buf);
+  bool Sep = false;
+  for (llvm::StringRef Arg : Args) {
+if (Sep)
+  OS << ' ';
+Sep = true;
+if (llvm::all_of(Arg, llvm::isPrint) &&
+Arg.find_first_of(" \t\n\"\\") == llvm::StringRef::npos) {
+  OS << Arg;
+  continue;
+}
+OS << '"';
+OS.write_escaped(Arg, /*UseHexEscapes=*/true);
+OS << '"';
+  }
+  return std::move(OS.str());
+}
+
+std::string printArgv(llvm::ArrayRef Args) {
+  std::vector Refs(Args.size());
+  llvm::copy(Args, Refs.begin());
+  return printArgv(Refs);
+}
+
 } // namespace clangd
 } // namespace clang

diff  --git a/clang-tools-extra/clangd/CompileCommands.h 
b/clang-tools-extra/clangd/CompileCommands.h
index 2ba17a0e6c0d..6e958d271c87 100644
--- a/clang-tools-extra/clangd/CompileCommands.h
+++ b/clang-tools-extra/clangd/CompileCommands.h
@@ -96,6 +96,11 @@ class ArgStripper {
   std::deque Storage; // Store strings not found in option table.
 };
 
+// Renders an argv list, with arguments separated by spaces.
+// Where needed, arguments are "quoted" and escaped.
+std::string printArgv(llvm::ArrayRef Args);
+std::string printArgv(llvm::ArrayRef Args);
+
 } // namespace clangd
 } // namespace clang
 

diff  --git a/clang-tools-extra/clangd/QueryDriverDatabase.cpp 
b/clang-tools-extra/clangd/QueryDriverDatabase.cpp
index 0bb2c46189b2..94faec9f3ed9 100644
--- a/clang-tools-extra/clangd/QueryDriverDatabase.cpp
+++ b/clang-tools-extra/clangd/QueryDriverDatabase.cpp
@@ -222,8 +222,8 @@ extractSystemIncludesAndTarget(llvm::SmallString<128> 
Driver,
   if (int RC = llvm::sys::ExecuteAndWait(Driver, Args, /*Env=*/llvm::None,
  Redirects)) {
 elog("System include extraction: driver execution failed with return code: 
"
- "{0}. Args: ['{1}']",
- llvm::to_string(RC), llvm::join(Args, "', '"));
+ "{0}. Args: [{1}]",
+ llvm::to_string(RC), printArgv(Args));
 return llvm::None;
   }
 

diff  --git a/clang-tools-extra/clangd/TUScheduler.cpp 
b/clang-tools-extra/clangd/TUScheduler.cpp
index 1d0ca1fee29d..1cd669945198 100644
--- a/clang-tools-extra/clangd/TUScheduler.cpp
+++ b/clang-tools-extra/clangd/TUScheduler.cpp
@@ -648,7 +648,7 @@ void ASTWorker::update(ParseInputs Inputs, WantDiagnostics 
WantDiags,
 log("ASTWorker building file {0} version {1} with command {2}\n[{3}]\n{4}",
 FileName, Inputs.Version, Inputs.CompileCommand.Heuristic,
 Inputs.CompileCommand.Directory,
-llvm::join(Inputs.CompileCommand.CommandLine, " "));
+printArgv(Inputs.CompileCommand.CommandLine));
 
 StoreDiags CompilerInvocationDiagConsumer;
 std::vector CC1Args;
@@ -656,7 +656,7 @@ void ASTWorker::update(ParseInputs Inputs, WantDiagnostics 
WantDiags,
 Inputs, CompilerInvocationDiagConsumer, );
 // Log cc1 args even (especially!) if creating invocation failed.
 if (!CC1Args.empty())
-  vlog("Driver produced command: cc1 {0}", llvm::join(CC1Args, " "));
+  vlog("Driver produced command: cc1 {0}", printArgv(CC1Args));
 std::vector CompilerInvocationDiags =
 CompilerInvocationDiagConsumer.take();
 if (!Invocation) {

diff  --git a/clang-tools-extra/clangd/tool/Check.cpp 
b/clang-tools-extra/clangd/tool/Check.cpp
index e42596bb4bf4..9e3e439ae70d 100644
--- a/clang-tools-extra/clangd/tool/Check.cpp
+++ b/clang-tools-extra/clangd/tool/Check.cpp
@@ -107,10 +107,10 @@ class Checker {
 
 if (auto TrueCmd = CDB->getCompileCommand(File)) {
   Cmd = std::move(*TrueCmd);
-  log("Compile command from CDB is: 

[clang-tools-extra] 7a8008d - Revert "[clangd] Quote/escape argv included in log messages."

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2021-01-31T16:22:00+01:00
New Revision: 7a8008d0e8885d22ff9a1fa7f9965c7b2ad2569a

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

LOG: Revert "[clangd] Quote/escape argv included in log messages."

This reverts commit 0962f1d72b1606f3224a14434c7b4500a23f8728.
http://45.33.8.238/win/32346/step_9.txt

Added: 


Modified: 
clang-tools-extra/clangd/CompileCommands.cpp
clang-tools-extra/clangd/CompileCommands.h
clang-tools-extra/clangd/QueryDriverDatabase.cpp
clang-tools-extra/clangd/TUScheduler.cpp
clang-tools-extra/clangd/tool/Check.cpp
clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/CompileCommands.cpp 
b/clang-tools-extra/clangd/CompileCommands.cpp
index b55d1b03dee6..96cbd8806ff6 100644
--- a/clang-tools-extra/clangd/CompileCommands.cpp
+++ b/clang-tools-extra/clangd/CompileCommands.cpp
@@ -503,32 +503,5 @@ void ArgStripper::process(std::vector ) 
const {
   Args.resize(Write);
 }
 
-
-std::string printArgv(llvm::ArrayRef Args) {
-  std::string Buf;
-  llvm::raw_string_ostream OS(Buf);
-  bool Sep = false;
-  for (llvm::StringRef Arg : Args) {
-if (Sep)
-  OS << ' ';
-Sep = true;
-if (llvm::all_of(Arg, llvm::isPrint) &&
-Arg.find_first_of(" \t\n\"\\") == llvm::StringRef::npos) {
-  OS << Arg;
-  continue;
-}
-OS << '"';
-OS.write_escaped(Arg, /*UseHexEscapes=*/true);
-OS << '"';
-  }
-  return std::move(OS.str());
-}
-
-std::string printArgv(llvm::ArrayRef Args) {
-  std::vector Refs(Args.size());
-  llvm::copy(Args, Refs.begin());
-  return printArgv(Refs);
-}
-
 } // namespace clangd
 } // namespace clang

diff  --git a/clang-tools-extra/clangd/CompileCommands.h 
b/clang-tools-extra/clangd/CompileCommands.h
index 6e958d271c87..2ba17a0e6c0d 100644
--- a/clang-tools-extra/clangd/CompileCommands.h
+++ b/clang-tools-extra/clangd/CompileCommands.h
@@ -96,11 +96,6 @@ class ArgStripper {
   std::deque Storage; // Store strings not found in option table.
 };
 
-// Renders an argv list, with arguments separated by spaces.
-// Where needed, arguments are "quoted" and escaped.
-std::string printArgv(llvm::ArrayRef Args);
-std::string printArgv(llvm::ArrayRef Args);
-
 } // namespace clangd
 } // namespace clang
 

diff  --git a/clang-tools-extra/clangd/QueryDriverDatabase.cpp 
b/clang-tools-extra/clangd/QueryDriverDatabase.cpp
index 94faec9f3ed9..0bb2c46189b2 100644
--- a/clang-tools-extra/clangd/QueryDriverDatabase.cpp
+++ b/clang-tools-extra/clangd/QueryDriverDatabase.cpp
@@ -222,8 +222,8 @@ extractSystemIncludesAndTarget(llvm::SmallString<128> 
Driver,
   if (int RC = llvm::sys::ExecuteAndWait(Driver, Args, /*Env=*/llvm::None,
  Redirects)) {
 elog("System include extraction: driver execution failed with return code: 
"
- "{0}. Args: [{1}]",
- llvm::to_string(RC), printArgv(Args));
+ "{0}. Args: ['{1}']",
+ llvm::to_string(RC), llvm::join(Args, "', '"));
 return llvm::None;
   }
 

diff  --git a/clang-tools-extra/clangd/TUScheduler.cpp 
b/clang-tools-extra/clangd/TUScheduler.cpp
index 1cd669945198..1d0ca1fee29d 100644
--- a/clang-tools-extra/clangd/TUScheduler.cpp
+++ b/clang-tools-extra/clangd/TUScheduler.cpp
@@ -648,7 +648,7 @@ void ASTWorker::update(ParseInputs Inputs, WantDiagnostics 
WantDiags,
 log("ASTWorker building file {0} version {1} with command {2}\n[{3}]\n{4}",
 FileName, Inputs.Version, Inputs.CompileCommand.Heuristic,
 Inputs.CompileCommand.Directory,
-printArgv(Inputs.CompileCommand.CommandLine));
+llvm::join(Inputs.CompileCommand.CommandLine, " "));
 
 StoreDiags CompilerInvocationDiagConsumer;
 std::vector CC1Args;
@@ -656,7 +656,7 @@ void ASTWorker::update(ParseInputs Inputs, WantDiagnostics 
WantDiags,
 Inputs, CompilerInvocationDiagConsumer, );
 // Log cc1 args even (especially!) if creating invocation failed.
 if (!CC1Args.empty())
-  vlog("Driver produced command: cc1 {0}", printArgv(CC1Args));
+  vlog("Driver produced command: cc1 {0}", llvm::join(CC1Args, " "));
 std::vector CompilerInvocationDiags =
 CompilerInvocationDiagConsumer.take();
 if (!Invocation) {

diff  --git a/clang-tools-extra/clangd/tool/Check.cpp 
b/clang-tools-extra/clangd/tool/Check.cpp
index 9e3e439ae70d..e42596bb4bf4 100644
--- a/clang-tools-extra/clangd/tool/Check.cpp
+++ b/clang-tools-extra/clangd/tool/Check.cpp
@@ -107,10 +107,10 @@ class Checker {
 
 if (auto TrueCmd = CDB->getCompileCommand(File)) {
   Cmd = std::move(*TrueCmd);
-  log("Compile command from CDB is: {0}", 

[PATCH] D95759: [clangd] Rename: merge index/AST refs path-insensitively where needed

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 320353.
sammccall added a comment.

revert unintended formatting changes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95759

Files:
  clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
  clang-tools-extra/clangd/refactor/Rename.cpp
  clang-tools-extra/clangd/support/CMakeLists.txt
  clang-tools-extra/clangd/support/Path.cpp
  clang-tools-extra/clangd/support/Path.h
  clang-tools-extra/clangd/unittests/RenameTests.cpp

Index: clang-tools-extra/clangd/unittests/RenameTests.cpp
===
--- clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -1067,6 +1067,52 @@
   }
 }
 
+MATCHER_P(newText, T, "") { return arg.newText == T; }
+
+TEST(RenameTest, IndexMergeMainFile) {
+  Annotations Code("int ^x();");
+  TestTU TU = TestTU::withCode(Code.code());
+  TU.Filename = "main.cc";
+  auto AST = TU.build();
+
+  auto Main = testPath("main.cc");
+
+  auto Rename = [&](const SymbolIndex *Idx) {
+auto GetDirtyBuffer = [&](PathRef Path) -> llvm::Optional {
+  return Code.code().str(); // Every file has the same content.
+};
+RenameOptions Opts;
+Opts.AllowCrossFile = true;
+RenameInputs Inputs{Code.point(), "xPrime", AST,   Main,
+Idx,  Opts, GetDirtyBuffer};
+auto Results = rename(Inputs);
+EXPECT_TRUE(bool(Results)) << llvm::toString(Results.takeError());
+return std::move(*Results);
+  };
+
+  // We do not expect to see duplicated edits from AST vs index.
+  auto Results = Rename(TU.index().get());
+  EXPECT_THAT(Results.GlobalChanges.keys(), ElementsAre(Main));
+  EXPECT_THAT(Results.GlobalChanges[Main].asTextEdits(),
+  ElementsAre(newText("xPrime")));
+
+  // Sanity check: we do expect to see index results!
+  TU.Filename = "other.cc";
+  Results = Rename(TU.index().get());
+  EXPECT_THAT(Results.GlobalChanges.keys(),
+  UnorderedElementsAre(Main, testPath("other.cc")));
+
+#if defined(_WIN32) || defined(__APPLE__)
+  // On case-insensitive systems, no duplicates if AST vs index case differs.
+  // https://github.com/clangd/clangd/issues/665
+  TU.Filename = "MAIN.CC";
+  Results = Rename(TU.index().get());
+  EXPECT_THAT(Results.GlobalChanges.keys(), ElementsAre(Main));
+  EXPECT_THAT(Results.GlobalChanges[Main].asTextEdits(),
+  ElementsAre(newText("xPrime")));
+#endif
+}
+
 TEST(RenameTest, MainFileReferencesOnly) {
   // filter out references not from main file.
   llvm::StringRef Test =
Index: clang-tools-extra/clangd/support/Path.h
===
--- clang-tools-extra/clangd/support/Path.h
+++ clang-tools-extra/clangd/support/Path.h
@@ -22,6 +22,12 @@
 /// signatures.
 using PathRef = llvm::StringRef;
 
+// For platforms where paths are case-insensitive (but case-preserving),
+// we need to do case-insensitive comparisons and use lowercase keys.
+// FIXME: Make Path a real class with desired semantics instead.
+std::string maybeCaseFoldPath(PathRef Path);
+bool pathEqual(PathRef, PathRef);
+
 } // namespace clangd
 } // namespace clang
 
Index: clang-tools-extra/clangd/support/Path.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/support/Path.cpp
@@ -0,0 +1,30 @@
+//===--- Path.cpp ---*- C++-*--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "support/Path.h"
+namespace clang {
+namespace clangd {
+
+std::string maybeCaseFoldPath(PathRef Path) {
+#if defined(_WIN32) || defined(__APPLE__)
+  return Path.lower();
+#else
+  return std::string(Path);
+#endif
+}
+
+bool pathEqual(PathRef A, PathRef B) {
+#if defined(_WIN32) || defined(__APPLE__)
+  return A.equals_lower(B);
+#else
+  return A == B;
+#endif
+}
+
+} // namespace clangd
+} // namespace clang
Index: clang-tools-extra/clangd/support/CMakeLists.txt
===
--- clang-tools-extra/clangd/support/CMakeLists.txt
+++ clang-tools-extra/clangd/support/CMakeLists.txt
@@ -23,6 +23,7 @@
   Logger.cpp
   Markup.cpp
   MemoryTree.cpp
+  Path.cpp
   Shutdown.cpp
   Threading.cpp
   ThreadsafeFS.cpp
Index: clang-tools-extra/clangd/refactor/Rename.cpp
===
--- clang-tools-extra/clangd/refactor/Rename.cpp
+++ clang-tools-extra/clangd/refactor/Rename.cpp
@@ -68,7 +68,7 @@
 if (OtherFile)
   return;
 if (auto RefFilePath = filePath(R.Location, 

[PATCH] D95759: [clangd] Rename: merge index/AST refs path-insensitively where needed

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 320352.
sammccall added a comment.

remove debug printfs


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95759

Files:
  clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
  clang-tools-extra/clangd/refactor/Rename.cpp
  clang-tools-extra/clangd/support/CMakeLists.txt
  clang-tools-extra/clangd/support/Path.cpp
  clang-tools-extra/clangd/support/Path.h
  clang-tools-extra/clangd/unittests/RenameTests.cpp

Index: clang-tools-extra/clangd/unittests/RenameTests.cpp
===
--- clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -1067,6 +1067,52 @@
   }
 }
 
+MATCHER_P(newText, T, "") { return arg.newText == T; }
+
+TEST(RenameTest, IndexMergeMainFile) {
+  Annotations Code("int ^x();");
+  TestTU TU = TestTU::withCode(Code.code());
+  TU.Filename = "main.cc";
+  auto AST = TU.build();
+
+  auto Main = testPath("main.cc");
+
+  auto Rename = [&](const SymbolIndex *Idx) {
+auto GetDirtyBuffer = [&](PathRef Path) -> llvm::Optional {
+  return Code.code().str(); // Every file has the same content.
+};
+RenameOptions Opts;
+Opts.AllowCrossFile = true;
+RenameInputs Inputs{Code.point(), "xPrime", AST,   Main,
+Idx,  Opts, GetDirtyBuffer};
+auto Results = rename(Inputs);
+EXPECT_TRUE(bool(Results)) << llvm::toString(Results.takeError());
+return std::move(*Results);
+  };
+
+  // We do not expect to see duplicated edits from AST vs index.
+  auto Results = Rename(TU.index().get());
+  EXPECT_THAT(Results.GlobalChanges.keys(), ElementsAre(Main));
+  EXPECT_THAT(Results.GlobalChanges[Main].asTextEdits(),
+  ElementsAre(newText("xPrime")));
+
+  // Sanity check: we do expect to see index results!
+  TU.Filename = "other.cc";
+  Results = Rename(TU.index().get());
+  EXPECT_THAT(Results.GlobalChanges.keys(),
+  UnorderedElementsAre(Main, testPath("other.cc")));
+
+#if defined(_WIN32) || defined(__APPLE__)
+  // On case-insensitive systems, no duplicates if AST vs index case differs.
+  // https://github.com/clangd/clangd/issues/665
+  TU.Filename = "MAIN.CC";
+  Results = Rename(TU.index().get());
+  EXPECT_THAT(Results.GlobalChanges.keys(), ElementsAre(Main));
+  EXPECT_THAT(Results.GlobalChanges[Main].asTextEdits(),
+  ElementsAre(newText("xPrime")));
+#endif
+}
+
 TEST(RenameTest, MainFileReferencesOnly) {
   // filter out references not from main file.
   llvm::StringRef Test =
@@ -1576,43 +1622,43 @@
 llvm::StringRef IndexedCode;
 llvm::StringRef DraftCode;
   } Tests[] = {
-{
-  // both line and column are changed, not a near miss.
-  R"cpp(
+  {
+  // both line and column are changed, not a near miss.
+  R"cpp(
 int [[x]] = 0;
   )cpp",
-  R"cpp(
+  R"cpp(
 // insert a line.
 double x = 0;
   )cpp",
-},
-{
-  // subset.
-  R"cpp(
+  },
+  {
+  // subset.
+  R"cpp(
 int [[x]] = 0;
   )cpp",
-  R"cpp(
+  R"cpp(
 int [[x]] = 0;
 {int x = 0; }
   )cpp",
-},
-{
-  // shift columns.
-  R"cpp(int [[x]] = 0; void foo(int x);)cpp",
-  R"cpp(double [[x]] = 0; void foo(double x);)cpp",
-},
-{
-  // shift lines.
-  R"cpp(
+  },
+  {
+  // shift columns.
+  R"cpp(int [[x]] = 0; void foo(int x);)cpp",
+  R"cpp(double [[x]] = 0; void foo(double x);)cpp",
+  },
+  {
+  // shift lines.
+  R"cpp(
 int [[x]] = 0;
 void foo(int x);
   )cpp",
-  R"cpp(
+  R"cpp(
 // insert a line.
 int [[x]] = 0;
 void foo(int x);
   )cpp",
-},
+  },
   };
   LangOptions LangOpts;
   LangOpts.CPlusPlus = true;
@@ -1635,69 +1681,62 @@
   struct {
 llvm::StringRef IndexedCode;
 llvm::StringRef LexedCode;
-  } Tests[] = {
-{
-  // no lexed ranges.
-  "[[]]",
-  "",
-},
-{
-  // both line and column are changed, not a near miss.
-  R"([[]])",
-  R"(
+  } Tests[] = {{
+   // no lexed ranges.
+   "[[]]",
+   "",
+   },
+   {
+   // both line and column are changed, not a near miss.
+   R"([[]])",
+   R"(
 [[]]
   )",
-},
-{
-  // subset.
-  "[[]]",
-  "^[[]]  [[]]"
-},
-{
-  // shift columns.
-  "[[]]   [[]]",
-  "  ^[[]]   ^[[]]  [[]]"
-},
-{
-  R"(
+   },
+   {// subset.
+"[[]]", "^[[]]  [[]]"},
+   {// shift columns.
+"[[]]   [[]]", "  ^[[]]   ^[[]]  [[]]"},
+   {
+ 

[PATCH] D95759: [clangd] Rename: merge index/AST refs path-insensitively where needed

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added a reviewer: hokein.
Herald added subscribers: usaxena95, kadircet, arphaman, mgorny.
sammccall requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang.

If you have c:\foo open, and C:\foo indexed (case difference) then these
need to be considered the same file. Otherwise we emit edits to both,
and editors do... something that isn't pretty.

Maybe more centralized normalization is called for, but it's not trivial
to do this while also being case-preserving. see
https://github.com/clangd/clangd/issues/108

Fixes https://github.com/clangd/clangd/issues/665


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D95759

Files:
  clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
  clang-tools-extra/clangd/refactor/Rename.cpp
  clang-tools-extra/clangd/support/CMakeLists.txt
  clang-tools-extra/clangd/support/Path.cpp
  clang-tools-extra/clangd/support/Path.h
  clang-tools-extra/clangd/unittests/RenameTests.cpp

Index: clang-tools-extra/clangd/unittests/RenameTests.cpp
===
--- clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -1067,6 +1067,52 @@
   }
 }
 
+MATCHER_P(newText, T, "") { return arg.newText == T; }
+
+TEST(RenameTest, IndexMergeMainFile) {
+  Annotations Code("int ^x();");
+  TestTU TU = TestTU::withCode(Code.code());
+  TU.Filename = "main.cc";
+  auto AST = TU.build();
+
+  auto Main = testPath("main.cc");
+
+  auto Rename = [&](const SymbolIndex *Idx) {
+auto GetDirtyBuffer = [&](PathRef Path) -> llvm::Optional {
+  return Code.code().str(); // Every file has the same content.
+};
+RenameOptions Opts;
+Opts.AllowCrossFile = true;
+RenameInputs Inputs{Code.point(), "xPrime", AST,   Main,
+Idx,  Opts, GetDirtyBuffer};
+auto Results = rename(Inputs);
+EXPECT_TRUE(bool(Results)) << llvm::toString(Results.takeError());
+return std::move(*Results);
+  };
+
+  // We do not expect to see duplicated edits from AST vs index.
+  auto Results = Rename(TU.index().get());
+  EXPECT_THAT(Results.GlobalChanges.keys(), ElementsAre(Main));
+  EXPECT_THAT(Results.GlobalChanges[Main].asTextEdits(),
+  ElementsAre(newText("xPrime")));
+
+  // Sanity check: we do expect to see index results!
+  TU.Filename = "other.cc";
+  Results = Rename(TU.index().get());
+  EXPECT_THAT(Results.GlobalChanges.keys(),
+  UnorderedElementsAre(Main, testPath("other.cc")));
+
+#if defined(_WIN32) || defined(__APPLE__)
+  // On case-insensitive systems, no duplicates if AST vs index case differs.
+  // https://github.com/clangd/clangd/issues/665
+  TU.Filename = "MAIN.CC";
+  Results = Rename(TU.index().get());
+  EXPECT_THAT(Results.GlobalChanges.keys(), ElementsAre(Main));
+  EXPECT_THAT(Results.GlobalChanges[Main].asTextEdits(),
+  ElementsAre(newText("xPrime")));
+#endif
+}
+
 TEST(RenameTest, MainFileReferencesOnly) {
   // filter out references not from main file.
   llvm::StringRef Test =
@@ -1576,43 +1622,43 @@
 llvm::StringRef IndexedCode;
 llvm::StringRef DraftCode;
   } Tests[] = {
-{
-  // both line and column are changed, not a near miss.
-  R"cpp(
+  {
+  // both line and column are changed, not a near miss.
+  R"cpp(
 int [[x]] = 0;
   )cpp",
-  R"cpp(
+  R"cpp(
 // insert a line.
 double x = 0;
   )cpp",
-},
-{
-  // subset.
-  R"cpp(
+  },
+  {
+  // subset.
+  R"cpp(
 int [[x]] = 0;
   )cpp",
-  R"cpp(
+  R"cpp(
 int [[x]] = 0;
 {int x = 0; }
   )cpp",
-},
-{
-  // shift columns.
-  R"cpp(int [[x]] = 0; void foo(int x);)cpp",
-  R"cpp(double [[x]] = 0; void foo(double x);)cpp",
-},
-{
-  // shift lines.
-  R"cpp(
+  },
+  {
+  // shift columns.
+  R"cpp(int [[x]] = 0; void foo(int x);)cpp",
+  R"cpp(double [[x]] = 0; void foo(double x);)cpp",
+  },
+  {
+  // shift lines.
+  R"cpp(
 int [[x]] = 0;
 void foo(int x);
   )cpp",
-  R"cpp(
+  R"cpp(
 // insert a line.
 int [[x]] = 0;
 void foo(int x);
   )cpp",
-},
+  },
   };
   LangOptions LangOpts;
   LangOpts.CPlusPlus = true;
@@ -1635,69 +1681,62 @@
   struct {
 llvm::StringRef IndexedCode;
 llvm::StringRef LexedCode;
-  } Tests[] = {
-{
-  // no lexed ranges.
-  "[[]]",
-  "",
-},
-{
-  // both line and column are changed, not a near miss.
-  R"([[]])",
-  R"(
+  } Tests[] = {{
+   // no lexed ranges.
+   "[[]]",
+   "",
+   },
+   {
+   // both 

[PATCH] D95307: [StaticAnalyzer] Add checking for degenerate base class in MemRegion

[Deep Majumder via Phabricator via cfe-commits]

RedDocMD abandoned this revision.
RedDocMD added a comment.

I clearly am taking the wrong approach to this problem. I will need to 
reconsider how PointerToMember is actually modelled before solving this problem


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95307

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

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

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.



> If it's a drop in replacement (does everything clang-format does and more), 
> what's the benefit for that cost?

I'm in agreement here, It was a suggestion just to appease those who don't like 
clang-format doing such things, but still allowing us to add code changing 
capabilities. (which I don't understand why that isn't controlled simply by 
turning the capability off.)


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] D69764: [clang-format] Add East/West Const fixer capability

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.



> What can be done to move this change along?

I feel there has to be a fundamental acceptance that it is ok for clang-format 
to alter code (something it already does with sorting of includes, namespace 
comments).

There were fairly strong opinions that clang-format isn't the best tool to do 
this (which actually I don't agree with, I think it is, as long as those 
capabilities are off by default and there is an acceptance they won't be 
perfect especially in the presence of macros due to lack of AST)

My only thought about building another tool would be if it was a drop in 
replacement for clang-format (tooling allows setting of a path), but it would 
need to inherit all of clang-format.

but to me, this just feels like extra grunt work just to work around why some 
community don't like it.

I guess a consensus is hard to come by, but I don't really know who owns the 
decision around the future direction of clang-format.


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

https://reviews.llvm.org/D69764

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

[clang-tools-extra] 60053a9 - [clangd] Remove references to old future-based API. NFC

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2021-01-31T12:13:20+01:00
New Revision: 60053a9ce28655fc6f635567c62599fa3aad57d2

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

LOG: [clangd] Remove references to old future-based API. NFC

Added: 


Modified: 
clang-tools-extra/clangd/ClangdServer.h
clang-tools-extra/clangd/support/Context.h

Removed: 




diff  --git a/clang-tools-extra/clangd/ClangdServer.h 
b/clang-tools-extra/clangd/ClangdServer.h
index 0a452daa79ed..fd5a4bf9a40d 100644
--- a/clang-tools-extra/clangd/ClangdServer.h
+++ b/clang-tools-extra/clangd/ClangdServer.h
@@ -34,7 +34,6 @@
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringRef.h"
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -194,13 +193,8 @@ class ClangdServer {
   void removeDocument(PathRef File);
 
   /// Run code completion for \p File at \p Pos.
-  /// Request is processed asynchronously.
   ///
-  /// This method should only be called for currently tracked files. However, 
it
-  /// is safe to call removeDocument for \p File after this method returns, 
even
-  /// while returned future is not yet ready.
-  /// A version of `codeComplete` that runs \p Callback on the processing 
thread
-  /// when codeComplete results become available.
+  /// This method should only be called for currently tracked files.
   void codeComplete(PathRef File, Position Pos,
 const clangd::CodeCompleteOptions ,
 Callback CB);

diff  --git a/clang-tools-extra/clangd/support/Context.h 
b/clang-tools-extra/clangd/support/Context.h
index 894032bdd883..815962b523d6 100644
--- a/clang-tools-extra/clangd/support/Context.h
+++ b/clang-tools-extra/clangd/support/Context.h
@@ -82,8 +82,6 @@ class Context {
 
 public:
   /// Same as Context::empty(), please use Context::empty() instead.
-  /// Constructor is defined to workaround a bug in MSVC's version of STL.
-  /// (arguments of std::future<> must be default-constructible in MSVC).
   Context() = default;
 
   /// Copy operations for this class are deleted, use an explicit clone() 
method



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

[clang-tools-extra] 0962f1d - [clangd] Quote/escape argv included in log messages.

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2021-01-31T12:00:08+01:00
New Revision: 0962f1d72b1606f3224a14434c7b4500a23f8728

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

LOG: [clangd] Quote/escape argv included in log messages.

https://github.com/clangd/clangd/issues/637

Added: 


Modified: 
clang-tools-extra/clangd/CompileCommands.cpp
clang-tools-extra/clangd/CompileCommands.h
clang-tools-extra/clangd/QueryDriverDatabase.cpp
clang-tools-extra/clangd/TUScheduler.cpp
clang-tools-extra/clangd/tool/Check.cpp
clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/CompileCommands.cpp 
b/clang-tools-extra/clangd/CompileCommands.cpp
index 96cbd8806ff6..b55d1b03dee6 100644
--- a/clang-tools-extra/clangd/CompileCommands.cpp
+++ b/clang-tools-extra/clangd/CompileCommands.cpp
@@ -503,5 +503,32 @@ void ArgStripper::process(std::vector ) 
const {
   Args.resize(Write);
 }
 
+
+std::string printArgv(llvm::ArrayRef Args) {
+  std::string Buf;
+  llvm::raw_string_ostream OS(Buf);
+  bool Sep = false;
+  for (llvm::StringRef Arg : Args) {
+if (Sep)
+  OS << ' ';
+Sep = true;
+if (llvm::all_of(Arg, llvm::isPrint) &&
+Arg.find_first_of(" \t\n\"\\") == llvm::StringRef::npos) {
+  OS << Arg;
+  continue;
+}
+OS << '"';
+OS.write_escaped(Arg, /*UseHexEscapes=*/true);
+OS << '"';
+  }
+  return std::move(OS.str());
+}
+
+std::string printArgv(llvm::ArrayRef Args) {
+  std::vector Refs(Args.size());
+  llvm::copy(Args, Refs.begin());
+  return printArgv(Refs);
+}
+
 } // namespace clangd
 } // namespace clang

diff  --git a/clang-tools-extra/clangd/CompileCommands.h 
b/clang-tools-extra/clangd/CompileCommands.h
index 2ba17a0e6c0d..6e958d271c87 100644
--- a/clang-tools-extra/clangd/CompileCommands.h
+++ b/clang-tools-extra/clangd/CompileCommands.h
@@ -96,6 +96,11 @@ class ArgStripper {
   std::deque Storage; // Store strings not found in option table.
 };
 
+// Renders an argv list, with arguments separated by spaces.
+// Where needed, arguments are "quoted" and escaped.
+std::string printArgv(llvm::ArrayRef Args);
+std::string printArgv(llvm::ArrayRef Args);
+
 } // namespace clangd
 } // namespace clang
 

diff  --git a/clang-tools-extra/clangd/QueryDriverDatabase.cpp 
b/clang-tools-extra/clangd/QueryDriverDatabase.cpp
index 0bb2c46189b2..94faec9f3ed9 100644
--- a/clang-tools-extra/clangd/QueryDriverDatabase.cpp
+++ b/clang-tools-extra/clangd/QueryDriverDatabase.cpp
@@ -222,8 +222,8 @@ extractSystemIncludesAndTarget(llvm::SmallString<128> 
Driver,
   if (int RC = llvm::sys::ExecuteAndWait(Driver, Args, /*Env=*/llvm::None,
  Redirects)) {
 elog("System include extraction: driver execution failed with return code: 
"
- "{0}. Args: ['{1}']",
- llvm::to_string(RC), llvm::join(Args, "', '"));
+ "{0}. Args: [{1}]",
+ llvm::to_string(RC), printArgv(Args));
 return llvm::None;
   }
 

diff  --git a/clang-tools-extra/clangd/TUScheduler.cpp 
b/clang-tools-extra/clangd/TUScheduler.cpp
index 1d0ca1fee29d..1cd669945198 100644
--- a/clang-tools-extra/clangd/TUScheduler.cpp
+++ b/clang-tools-extra/clangd/TUScheduler.cpp
@@ -648,7 +648,7 @@ void ASTWorker::update(ParseInputs Inputs, WantDiagnostics 
WantDiags,
 log("ASTWorker building file {0} version {1} with command {2}\n[{3}]\n{4}",
 FileName, Inputs.Version, Inputs.CompileCommand.Heuristic,
 Inputs.CompileCommand.Directory,
-llvm::join(Inputs.CompileCommand.CommandLine, " "));
+printArgv(Inputs.CompileCommand.CommandLine));
 
 StoreDiags CompilerInvocationDiagConsumer;
 std::vector CC1Args;
@@ -656,7 +656,7 @@ void ASTWorker::update(ParseInputs Inputs, WantDiagnostics 
WantDiags,
 Inputs, CompilerInvocationDiagConsumer, );
 // Log cc1 args even (especially!) if creating invocation failed.
 if (!CC1Args.empty())
-  vlog("Driver produced command: cc1 {0}", llvm::join(CC1Args, " "));
+  vlog("Driver produced command: cc1 {0}", printArgv(CC1Args));
 std::vector CompilerInvocationDiags =
 CompilerInvocationDiagConsumer.take();
 if (!Invocation) {

diff  --git a/clang-tools-extra/clangd/tool/Check.cpp 
b/clang-tools-extra/clangd/tool/Check.cpp
index e42596bb4bf4..9e3e439ae70d 100644
--- a/clang-tools-extra/clangd/tool/Check.cpp
+++ b/clang-tools-extra/clangd/tool/Check.cpp
@@ -107,10 +107,10 @@ class Checker {
 
 if (auto TrueCmd = CDB->getCompileCommand(File)) {
   Cmd = std::move(*TrueCmd);
-  log("Compile command from CDB is: {0}", llvm::join(Cmd.CommandLine, " 
"));
+  log("Compile command from CDB 

[PATCH] D95746: clang: Exclude efi_main from -Wmissing-prototypes

[Daan De Meyer via Phabricator via cfe-commits]

DaanDeMeyer added a comment.

CI failures seem unrelated?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95746

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

[clang] 095f086 - [docs] Clarify compile_flags.txt subtleties

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2021-01-31T11:16:59+01:00
New Revision: 095f08653f3ab0917a474888abac95c4fa99697d

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

LOG: [docs] Clarify compile_flags.txt subtleties

See confusion e.g. in https://github.com/clangd/clangd/issues/637

Added: 


Modified: 
clang/docs/JSONCompilationDatabase.rst

Removed: 




diff  --git a/clang/docs/JSONCompilationDatabase.rst 
b/clang/docs/JSONCompilationDatabase.rst
index b5766402e2d6..bd91db258398 100644
--- a/clang/docs/JSONCompilationDatabase.rst
+++ b/clang/docs/JSONCompilationDatabase.rst
@@ -94,6 +94,18 @@ to parse C++ code in the source tree.
 
 Alternatives
 
-For simple projects, Clang tools also recognize a compile_flags.txt file.
-This should contain one flag per line. The same flags will be used to compile
-any file.
+For simple projects, Clang tools also recognize a ``compile_flags.txt`` file.
+This should contain one argument per line. The same flags will be used to
+compile any file.
+
+Example:
+
+::
+
+-xc++
+-I
+libwidget/include/
+
+Here ``-I libwidget/include`` is two arguments, and so becomes two lines.
+Paths are relative to the directory containing ``compile_flags.txt``.
+



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

[PATCH] D95366: [clang][cli] Generate and round-trip preprocessor options

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 updated this revision to Diff 320344.
jansvoboda11 added a comment.

Rebase, rename parse function


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95366

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Frontend/CompilerInvocation.cpp

Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -3100,9 +3100,96 @@
   llvm_unreachable("invalid frontend action");
 }
 
-static void ParsePreprocessorArgs(PreprocessorOptions , ArgList ,
-  DiagnosticsEngine ,
-  frontend::ActionKind Action) {
+static void GeneratePreprocessorArgs(PreprocessorOptions ,
+ SmallVectorImpl ,
+ CompilerInvocation::StringAllocator SA,
+ const LangOptions ,
+ const FrontendOptions ,
+ const CodeGenOptions ) {
+  PreprocessorOptions *PreprocessorOpts = 
+
+#define PREPROCESSOR_OPTION_WITH_MARSHALLING(  \
+PREFIX_TYPE, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,\
+HELPTEXT, METAVAR, VALUES, SPELLING, SHOULD_PARSE, ALWAYS_EMIT, KEYPATH,   \
+DEFAULT_VALUE, IMPLIED_CHECK, IMPLIED_VALUE, NORMALIZER, DENORMALIZER, \
+MERGER, EXTRACTOR, TABLE_INDEX)\
+  GENERATE_OPTION_WITH_MARSHALLING(\
+  Args, SA, KIND, FLAGS, SPELLING, ALWAYS_EMIT, KEYPATH, DEFAULT_VALUE,\
+  IMPLIED_CHECK, IMPLIED_VALUE, DENORMALIZER, EXTRACTOR, TABLE_INDEX)
+#include "clang/Driver/Options.inc"
+#undef PREPROCESSOR_OPTION_WITH_MARSHALLING
+
+  if (Opts.PCHWithHdrStop && !Opts.PCHWithHdrStopCreate)
+GenerateArg(Args, OPT_pch_through_hdrstop_use, SA);
+
+  for (const auto  : Opts.DeserializedPCHDeclsToErrorOn)
+GenerateArg(Args, OPT_error_on_deserialized_pch_decl, D, SA);
+
+  for (const auto  : Opts.MacroPrefixMap)
+GenerateArg(Args, OPT_fmacro_prefix_map_EQ, MP.first + "=" + MP.second, SA);
+
+  if (Opts.PrecompiledPreambleBytes != std::make_pair(0u, false))
+GenerateArg(Args, OPT_preamble_bytes_EQ,
+Twine(Opts.PrecompiledPreambleBytes.first) + "," +
+(Opts.PrecompiledPreambleBytes.second ? "1" : "0"),
+SA);
+
+  for (const auto  : Opts.Macros) {
+// Don't generate __CET__ macro definitions. They are implied by the
+// -fcf-protection option that is generated elsewhere.
+if (M.first == "__CET__=1" && !M.second &&
+!CodeGenOpts.CFProtectionReturn && CodeGenOpts.CFProtectionBranch)
+  continue;
+if (M.first == "__CET__=2" && !M.second && CodeGenOpts.CFProtectionReturn &&
+!CodeGenOpts.CFProtectionBranch)
+  continue;
+if (M.first == "__CET__=3" && !M.second && CodeGenOpts.CFProtectionReturn &&
+CodeGenOpts.CFProtectionBranch)
+  continue;
+
+GenerateArg(Args, M.second ? OPT_U : OPT_D, M.first, SA);
+  }
+
+  for (const auto  : Opts.Includes) {
+// Don't generate OpenCL includes. They are implied by other flags that are
+// generated elsewhere.
+if (LangOpts.OpenCL && LangOpts.IncludeDefaultHeader &&
+((LangOpts.DeclareOpenCLBuiltins && I == "opencl-c-base.h") ||
+ I == "opencl-c.h"))
+  continue;
+
+GenerateArg(Args, OPT_include, I, SA);
+  }
+
+  for (const auto  : Opts.ChainedIncludes)
+GenerateArg(Args, OPT_chain_include, CI, SA);
+
+  for (const auto  : Opts.RemappedFiles)
+GenerateArg(Args, OPT_remap_file, RF.first + ";" + RF.second, SA);
+
+  // Don't handle LexEditorPlaceholders. It is implied by the action that is
+  // generated elsewhere.
+}
+
+static bool ParsePreprocessorArgsImpl(PreprocessorOptions , ArgList ,
+  DiagnosticsEngine ,
+  frontend::ActionKind Action,
+  const FrontendOptions ) {
+  PreprocessorOptions *PreprocessorOpts = 
+  bool Success = true;
+
+#define PREPROCESSOR_OPTION_WITH_MARSHALLING(  \
+PREFIX_TYPE, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,\
+HELPTEXT, METAVAR, VALUES, SPELLING, SHOULD_PARSE, ALWAYS_EMIT, KEYPATH,   \
+DEFAULT_VALUE, IMPLIED_CHECK, IMPLIED_VALUE, NORMALIZER, DENORMALIZER, \
+MERGER, EXTRACTOR, TABLE_INDEX)\
+  PARSE_OPTION_WITH_MARSHALLING(Args, Diags, Success, ID, FLAGS, PARAM,\
+SHOULD_PARSE, KEYPATH, DEFAULT_VALUE,  \
+IMPLIED_CHECK, IMPLIED_VALUE, NORMALIZER,  \
+ 

[PATCH] D95369: [clang][cli] Generate and round-trip analyzer options

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 updated this revision to Diff 320343.
jansvoboda11 added a comment.

Rebase, rename original parse function


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95369

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Frontend/CompilerInvocation.cpp

Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -756,9 +756,121 @@
   Funcs.insert(Funcs.end(), Values.begin(), Values.end());
 }
 
-static bool ParseAnalyzerArgs(AnalyzerOptions , ArgList ,
-  DiagnosticsEngine ) {
+static void GenerateAnalyzerArgs(AnalyzerOptions ,
+ SmallVectorImpl ,
+ CompilerInvocation::StringAllocator SA) {
+  const AnalyzerOptions *AnalyzerOpts = 
+
+#define ANALYZER_OPTION_WITH_MARSHALLING(  \
+PREFIX_TYPE, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,\
+HELPTEXT, METAVAR, VALUES, SPELLING, SHOULD_PARSE, ALWAYS_EMIT, KEYPATH,   \
+DEFAULT_VALUE, IMPLIED_CHECK, IMPLIED_VALUE, NORMALIZER, DENORMALIZER, \
+MERGER, EXTRACTOR, TABLE_INDEX)\
+  GENERATE_OPTION_WITH_MARSHALLING(\
+  Args, SA, KIND, FLAGS, SPELLING, ALWAYS_EMIT, KEYPATH, DEFAULT_VALUE,\
+  IMPLIED_CHECK, IMPLIED_VALUE, DENORMALIZER, EXTRACTOR, TABLE_INDEX)
+#include "clang/Driver/Options.inc"
+#undef ANALYZER_OPTION_WITH_MARSHALLING
+
+  if (Opts.AnalysisStoreOpt != RegionStoreModel) {
+switch (Opts.AnalysisStoreOpt) {
+#define ANALYSIS_STORE(NAME, CMDFLAG, DESC, CREATFN)   \
+  case NAME##Model:\
+GenerateArg(Args, OPT_analyzer_store, CMDFLAG, SA);\
+break;
+#include "clang/StaticAnalyzer/Core/Analyses.def"
+default:
+  llvm_unreachable("Tried to generate unknown analysis store.");
+}
+  }
+
+  if (Opts.AnalysisConstraintsOpt != RangeConstraintsModel) {
+switch (Opts.AnalysisConstraintsOpt) {
+#define ANALYSIS_CONSTRAINTS(NAME, CMDFLAG, DESC, CREATFN) \
+  case NAME##Model:\
+GenerateArg(Args, OPT_analyzer_constraints, CMDFLAG, SA);  \
+break;
+#include "clang/StaticAnalyzer/Core/Analyses.def"
+default:
+  llvm_unreachable("Tried to generate unknown analysis constraint.");
+}
+  }
+
+  if (Opts.AnalysisDiagOpt != PD_HTML) {
+switch (Opts.AnalysisDiagOpt) {
+#define ANALYSIS_DIAGNOSTICS(NAME, CMDFLAG, DESC, CREATFN) \
+  case PD_##NAME:  \
+GenerateArg(Args, OPT_analyzer_output, CMDFLAG, SA);   \
+break;
+#include "clang/StaticAnalyzer/Core/Analyses.def"
+default:
+  llvm_unreachable("Tried to generate unknown analysis diagnostic client.");
+}
+  }
+
+  if (Opts.AnalysisPurgeOpt != PurgeStmt) {
+switch (Opts.AnalysisPurgeOpt) {
+#define ANALYSIS_PURGE(NAME, CMDFLAG, DESC)\
+  case NAME:   \
+GenerateArg(Args, OPT_analyzer_purge, CMDFLAG, SA);\
+break;
+#include "clang/StaticAnalyzer/Core/Analyses.def"
+default:
+  llvm_unreachable("Tried to generate unknown analysis purge mode.");
+}
+  }
+
+  if (Opts.InliningMode != NoRedundancy) {
+switch (Opts.InliningMode) {
+#define ANALYSIS_INLINING_MODE(NAME, CMDFLAG, DESC)\
+  case NAME:   \
+GenerateArg(Args, OPT_analyzer_inlining_mode, CMDFLAG, SA);\
+break;
+#include "clang/StaticAnalyzer/Core/Analyses.def"
+default:
+  llvm_unreachable("Tried to generate unknown analysis inlining mode.");
+}
+  }
+
+  for (const auto  : Opts.CheckersAndPackages) {
+OptSpecifier Opt =
+CP.second ? OPT_analyzer_checker : OPT_analyzer_disable_checker;
+GenerateArg(Args, Opt, CP.first, SA);
+  }
+
+  AnalyzerOptions ConfigOpts;
+  parseAnalyzerConfigs(ConfigOpts, nullptr);
+
+  for (const auto  : Opts.Config) {
+// Don't generate anything that came from parseAnalyzerConfigs. It would be
+// redundant and may not be valid on the command line.
+auto Entry = ConfigOpts.Config.find(C.getKey());
+if (Entry != ConfigOpts.Config.end() && Entry->getValue() == C.getValue())
+  continue;
+
+GenerateArg(Args, OPT_analyzer_config, C.getKey() + "=" + C.getValue(), SA);
+  }
+
+  // Nothing to generate for FullCompilerInvocation.
+}
+
+static bool 

[PATCH] D94472: [clang][cli] Command line round-trip for HeaderSearch options

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 updated this revision to Diff 320342.
jansvoboda11 added a comment.

Fix formatting and comment wording


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94472

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Driver/Options.td
  clang/include/clang/Frontend/CompilerInvocation.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Frontend/command-line-round-trip.c
  clang/tools/driver/cc1_main.cpp
  llvm/include/llvm/Option/ArgList.h
  llvm/lib/Option/ArgList.cpp

Index: llvm/lib/Option/ArgList.cpp
===
--- llvm/lib/Option/ArgList.cpp
+++ llvm/lib/Option/ArgList.cpp
@@ -90,11 +90,22 @@
 }
 
 std::vector ArgList::getAllArgValues(OptSpecifier Id) const {
+  recordQueriedOpts(Id);
   SmallVector Values;
   AddAllArgValues(Values, Id);
   return std::vector(Values.begin(), Values.end());
 }
 
+void ArgList::AddAllArgsExcept(ArgStringList ,
+   const DenseSet ) const {
+  for (const Arg *Arg : *this) {
+if (!ExcludeIds.contains(Arg->getOption().getID())) {
+  Arg->claim();
+  Arg->render(*this, Output);
+}
+  }
+}
+
 void ArgList::AddAllArgsExcept(ArgStringList ,
ArrayRef Ids,
ArrayRef ExcludeIds) const {
Index: llvm/include/llvm/Option/ArgList.h
===
--- llvm/include/llvm/Option/ArgList.h
+++ llvm/include/llvm/Option/ArgList.h
@@ -137,6 +137,16 @@
   /// The first and last index of each different OptSpecifier ID.
   DenseMap OptRanges;
 
+  /// The OptSpecifiers that were queried from this argument list.
+  mutable DenseSet QueriedOpts;
+
+  /// Record the queried OptSpecifiers.
+  template 
+  void recordQueriedOpts(OptSpecifiers... Ids) const {
+SmallVector OptsSpecifiers({toOptSpecifier(Ids).getID()...});
+QueriedOpts.insert(OptsSpecifiers.begin(), OptsSpecifiers.end());
+  }
+
   /// Get the range of indexes in which options with the specified IDs might
   /// reside, or (0, 0) if there are no such options.
   OptRange getRange(std::initializer_list Ids) const;
@@ -203,6 +213,7 @@
   template
   iterator_range>
   filtered(OptSpecifiers ...Ids) const {
+recordQueriedOpts(Ids...);
 OptRange Range = getRange({toOptSpecifier(Ids)...});
 auto B = Args.begin() + Range.first;
 auto E = Args.begin() + Range.second;
@@ -214,6 +225,7 @@
   template
   iterator_range>
   filtered_reverse(OptSpecifiers ...Ids) const {
+recordQueriedOpts(Ids...);
 OptRange Range = getRange({toOptSpecifier(Ids)...});
 auto B = Args.rend() - Range.second;
 auto E = Args.rend() - Range.first;
@@ -308,6 +320,10 @@
   A->render(*this, Output);
   }
 
+  /// AddAllArgsExcept - Render all arguments not matching any of the excluded
+  /// ids.
+  void AddAllArgsExcept(ArgStringList ,
+const DenseSet ) const;
   /// AddAllArgsExcept - Render all arguments matching any of the given ids
   /// and not matching any of the excluded ids.
   void AddAllArgsExcept(ArgStringList , ArrayRef Ids,
@@ -342,6 +358,12 @@
   ///
   void ClaimAllArgs() const;
 
+  /// Return the OptSpecifiers queried from this argument list.
+  const DenseSet () const { return QueriedOpts; }
+
+  /// Clear the set of queried OptSpecifiers.
+  void clearQueriedOpts() const { QueriedOpts.clear(); }
+
   /// @}
   /// @name Arg Synthesis
   /// @{
Index: clang/tools/driver/cc1_main.cpp
===
--- clang/tools/driver/cc1_main.cpp
+++ clang/tools/driver/cc1_main.cpp
@@ -203,6 +203,12 @@
   IntrusiveRefCntPtr DiagOpts = new DiagnosticOptions();
   TextDiagnosticBuffer *DiagsBuffer = new TextDiagnosticBuffer;
   DiagnosticsEngine Diags(DiagID, &*DiagOpts, DiagsBuffer);
+
+  // Setup round-trip remarks for the DiagnosticsEngine used in CreateFromArgs.
+  if (find(Argv, StringRef("-Rround-trip")) != Argv.end())
+Diags.setSeverity(diag::remark_cc1_round_trip_generated,
+  diag::Severity::Remark, {});
+
   bool Success = CompilerInvocation::CreateFromArgs(Clang->getInvocation(),
 Argv, Diags, Argv0);
 
Index: clang/test/Frontend/command-line-round-trip.c
===
--- /dev/null
+++ clang/test/Frontend/command-line-round-trip.c
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 %s -Rround-trip 2>&1 | FileCheck %s -check-prefix=CHECK-WITHOUT-ROUND-TRIP -allow-empty
+// RUN: %clang_cc1 %s -round-trip-args 2>&1 | FileCheck %s -check-prefix=CHECK-ROUND-TRIP-WITHOUT-REMARKS -allow-empty
+// RUN: %clang_cc1 %s -round-trip-args -Rround-trip 2>&1 | FileCheck %s -check-prefix=CHECK-ROUND-TRIP-WITH-REMARKS