[PATCH] D59802: [clang-tidy] Add new checker: llvm-avoid-cast-in-conditional

[Don Hinton via Phabricator via cfe-commits]

hintonda updated this revision to Diff 192778.
hintonda added a comment.

- Add isa and dyn_cast when matching for dyn_cast_or_null replacement.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59802

Files:
  clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.cpp
  clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.h
  clang-tools-extra/clang-tidy/llvm/CMakeLists.txt
  clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
  clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
  clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/llvm-avoid-cast-in-conditional.rst
  clang-tools-extra/test/clang-tidy/llvm-avoid-cast-in-conditional.cpp
  clang/include/clang/Basic/LLVM.h

Index: clang/include/clang/Basic/LLVM.h
===
--- clang/include/clang/Basic/LLVM.h
+++ clang/include/clang/Basic/LLVM.h
@@ -31,6 +31,7 @@
   template class ArrayRef;
   template class MutableArrayRef;
   template class OwningArrayRef;
+  template  class SmallSet;
   template class SmallString;
   template class SmallVector;
   template class SmallVectorImpl;
@@ -66,6 +67,7 @@
   using llvm::Optional;
   using llvm::OwningArrayRef;
   using llvm::SaveAndRestore;
+  using llvm::SmallSet;
   using llvm::SmallString;
   using llvm::SmallVector;
   using llvm::SmallVectorImpl;
Index: clang-tools-extra/test/clang-tidy/llvm-avoid-cast-in-conditional.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/llvm-avoid-cast-in-conditional.cpp
@@ -0,0 +1,106 @@
+// RUN: %check_clang_tidy %s llvm-avoid-cast-in-conditional %t
+
+struct X;
+struct Y;
+struct Z {
+  int foo();
+  X *bar();
+};
+
+template 
+bool isa(Y *);
+template 
+X *cast(Y *);
+template 
+X *dyn_cast(Y *);
+template 
+X *dyn_cast_or_null(Y *);
+
+bool foo(Y *y) {
+  if (auto x = cast(y))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:16: warning: {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (auto x = dyn_cast(y))
+
+  if (cast(y))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning:  {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (isa(y))
+
+  if (dyn_cast(y))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning:  {{.*dyn_cast<> not used .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (isa(y))
+
+  // These don't trigger a warning.
+  if (auto x = cast(y)->foo())
+return true;
+  if (cast(y)->foo())
+return true;
+
+  while (auto x = cast(y))
+break;
+  // CHECK-MESSAGES: :[[@LINE-2]]:19: warning:  {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (auto x = dyn_cast(y))
+
+  while (cast(y))
+break;
+  // CHECK-MESSAGES: :[[@LINE-2]]:10: warning:  {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (isa(y))
+
+  while (dyn_cast(y))
+break;
+  // CHECK-MESSAGES: :[[@LINE-2]]:10: warning:  {{.*dyn_cast<> not used .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (isa(y))
+
+  // These don't trigger a warning.
+  while (auto x = cast(y)->foo())
+break;
+  while (cast(y)->foo())
+break;
+
+  do {
+break;
+  } while (cast(y));
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning:  {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (isa(y));
+
+  do {
+break;
+  } while (dyn_cast(y));
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning:  {{.*dyn_cast<> not used .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (isa(y));
+
+  if (y && isa(y))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning:  {{.*use dyn_cast_or_null .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (dyn_cast_or_null(y))
+
+  if (y && cast(y))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning:  {{.*use dyn_cast_or_null .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (dyn_cast_or_null(y))
+
+  if (y && dyn_cast(y))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning:  {{.*use dyn_cast_or_null .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (dyn_cast_or_null(y))
+
+  Z z;
+  if (z.bar() && cast(z.bar()))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning:  {{.*use dyn_cast_or_null .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (dyn_cast_or_null(z.bar()))
+
+  bool b = y && cast(y);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning:  {{.*use dyn_cast_or_null .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: bool b = dyn_cast_or_null(y);
+
+  // These don't trigger a warning.
+  if (y && cast(z.bar())) {
+return true;
+  }
+  bool b2 = y && cast();
+
+  return false;
+}
Index: 

[PATCH] D59923: [Driver] Simplify -g level computation and its interaction with -gsplit-dwarf

[Fangrui Song via Phabricator via cfe-commits]

MaskRay updated this revision to Diff 192777.
MaskRay edited the summary of this revision.
MaskRay added a comment.

Update description


Repository:
  rC Clang

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

https://reviews.llvm.org/D59923

Files:
  lib/Driver/ToolChains/Clang.cpp
  test/Driver/split-debug.c

Index: test/Driver/split-debug.c
===
--- test/Driver/split-debug.c
+++ test/Driver/split-debug.c
@@ -68,7 +68,7 @@
 // RUN: FileCheck -check-prefix=CHECK-SPLIT-WITH-GMLT < %t %s
 //
 // CHECK-SPLIT-WITH-GMLT: "-enable-split-dwarf"
-// CHECK-SPLIT-WITH-GMLT: "-debug-info-kind=line-tables-only"
+// CHECK-SPLIT-WITH-GMLT: "-debug-info-kind=limited"
 // CHECK-SPLIT-WITH-GMLT: "-split-dwarf-file"
 
 // RUN: %clang -target x86_64-unknown-linux-gnu -gsplit-dwarf -fno-split-dwarf-inlining -S -### %s 2> %t
@@ -100,6 +100,8 @@
 
 // RUN: %clang -target x86_64-unknown-linux-gnu -gsplit-dwarf -g0 -S -### %s 2> %t
 // RUN: FileCheck -check-prefix=CHECK-G0-OVER-SPLIT < %t %s
+// RUN: %clang -target x86_64-unknown-linux-gnu -gsplit-dwarf=split -g0 -S -### %s 2> %t
+// RUN: FileCheck -check-prefix=CHECK-G0-OVER-SPLIT < %t %s
 //
 // CHECK-G0-OVER-SPLIT-NOT: "-enable-split-dwarf"
 // CHECK-G0-OVER-SPLIT-NOT: "-debug-info-kind
@@ -107,6 +109,8 @@
 
 // RUN: %clang -target x86_64-unknown-linux-gnu -g0 -gsplit-dwarf -S -### %s 2> %t
 // RUN: FileCheck -check-prefix=CHECK-SPLIT-OVER-G0 < %t %s
+// RUN: %clang -target x86_64-unknown-linux-gnu -g0 -gsplit-dwarf=split -S -### %s 2> %t
+// RUN: FileCheck -check-prefix=CHECK-SPLIT-OVER-G0 < %t %s
 //
 // CHECK-SPLIT-OVER-G0: "-enable-split-dwarf" "-debug-info-kind=limited"
 // CHECK-SPLIT-OVER-G0: "-split-dwarf-file"
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -3158,35 +3158,24 @@
 SplitDWARFInlining = false;
   }
 
-  if (const Arg *A = Args.getLastArg(options::OPT_g_Group)) {
-if (checkDebugInfoOption(A, Args, D, TC)) {
-  // If the last option explicitly specified a debug-info level, use it.
-  if (A->getOption().matches(options::OPT_gN_Group)) {
-DebugInfoKind = DebugLevelToInfoKind(*A);
-// If you say "-gsplit-dwarf -gline-tables-only", -gsplit-dwarf loses.
-// But -gsplit-dwarf is not a g_group option, hence we have to check the
-// order explicitly. If -gsplit-dwarf wins, we fix DebugInfoKind later.
-// This gets a bit more complicated if you've disabled inline info in
-// the skeleton CUs (SplitDWARFInlining) - then there's value in
-// composing split-dwarf and line-tables-only, so let those compose
-// naturally in that case. And if you just turned off debug info,
-// (-gsplit-dwarf -g0) - do that.
-if (DwarfFission != DwarfFissionKind::None) {
-  if (A->getIndex() > SplitDWARFArg->getIndex()) {
-if (DebugInfoKind == codegenoptions::NoDebugInfo ||
-DebugInfoKind == codegenoptions::DebugDirectivesOnly ||
-(DebugInfoKind == codegenoptions::DebugLineTablesOnly &&
- SplitDWARFInlining))
-  DwarfFission = DwarfFissionKind::None;
-  } else if (SplitDWARFInlining)
-DebugInfoKind = codegenoptions::NoDebugInfo;
-}
-  } else {
-// For any other 'g' option, use Limited.
-DebugInfoKind = codegenoptions::LimitedDebugInfo;
-  }
-} else {
-  DebugInfoKind = codegenoptions::LimitedDebugInfo;
+  if (const Arg *A =
+  Args.getLastArg(options::OPT_g_Group, options::OPT_gsplit_dwarf,
+  options::OPT_gsplit_dwarf_EQ)) {
+DebugInfoKind = codegenoptions::LimitedDebugInfo;
+
+// If the last option explicitly specified a debug-info level, use it.
+if (checkDebugInfoOption(A, Args, D, TC) &&
+A->getOption().matches(options::OPT_gN_Group)) {
+  DebugInfoKind = DebugLevelToInfoKind(*A);
+  // For -g0 or -gline-tables-only, drop -gsplit-dwarf. This gets a bit more
+  // complicated if you've disabled inline info in the skeleton CUs
+  // (SplitDWARFInlining) - then there's value in composing split-dwarf and
+  // line-tables-only, so let those compose naturally in that case.
+  if (DebugInfoKind == codegenoptions::NoDebugInfo ||
+  DebugInfoKind == codegenoptions::DebugDirectivesOnly ||
+  (DebugInfoKind == codegenoptions::DebugLineTablesOnly &&
+   SplitDWARFInlining))
+DwarfFission = DwarfFissionKind::None;
 }
   }
 
@@ -3261,16 +3250,12 @@
   }
 }
 
-  // -gsplit-dwarf should turn on -g and enable the backend dwarf
-  // splitting and extraction.
+  // -gsplit-dwarf enables the backend dwarf splitting and extraction.
   if (T.isOSBinFormatELF()) {
 if (!SplitDWARFInlining)
   

[PATCH] D59923: [Driver] Simplify -g level computation and its interaction with -gsplit-dwarf

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D59923#1447245 , @dblaikie wrote:

> OK - could you include/describe which sets of options disable split-dwarf, 
> then? (adding that to the patch description along with the matrix of g1/2, 
> etc?


I've already updated the description to mention the computed debug info level 
for some composition: (didn't mention `-gline-directives-only` though)

`-g0`, `-gline-directives-only`, and `-gmlt -fsplit-dwarf-inling` will disable 
`-gsplit-dwarf`. The rule hasn't changed.

> (specifically, I'd expect "-gmlt -gsplit-dwarf" means 2+split, "-gsplit-dwarf 
> -gmlt" means 1+non-split, and "-fno-split-dwarf-inlining -gsplit-dwarf -gmlt" 
> (with -fno-split-dwarf-inlining anywhere in the command line (so long as it's 
> after an -fsplit-dwarf-inlining) is 1+split)

The following two composition rules (as you expect) haven't changed:

- `-gmlt -gsplit-dwarf` -> 2 + split
- `-fno-split-dwarf-inlining -gsplit-dwarf -gmlt` -> 1 + split

> I'm still not quite sure changing the meaning of "-gmlt -gsplit-dwarf 
> -fno-split-dwarf-inlining" is important. It feels to me like in that mode, 
> -gmlt and -gsplit-dwarf compose naturally in either order. Is it code 
> complexity or user interface complexity you're trying to address? I'm still a 
> bit curious/trying to better understand the motivation here.

Just the insistent handling of `-g0 -gsplit-dwarf` `-gmlt -gsplit-dwarf 
-fsplit-dwarf-inlining` and `-gmlt -gsplit-dwarf -fno-split-dwarf-inlining ` 
motivated me to create this patch. I don't have a specific use case.

The new logic is equivalent to the following patch. It reorders code to make it 
tighter, thoguh.

  if (DwarfFission != DwarfFissionKind::None) {
if (A->getIndex() > SplitDWARFArg->getIndex()) {
  if (DebugInfoKind == codegenoptions::NoDebugInfo ||
  DebugInfoKind == codegenoptions::DebugDirectivesOnly ||
  (DebugInfoKind == codegenoptions::DebugLineTablesOnly &&
   SplitDWARFInlining))
DwarfFission = DwarfFissionKind::None;
}
  - else if (SplitDWARFInlining)
  -   DebugInfoKind = codegenoptions::NoDebugInfo;
  }


Repository:
  rC Clang

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

https://reviews.llvm.org/D59923



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

[PATCH] D59900: [Sema] Fix a crash when nonnull checking

[Michael Liao via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL357236: [Sema] Fix a crash when nonnull checking (authored 
by hliao, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D59900?vs=192514=192776#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D59900

Files:
  cfe/trunk/lib/Sema/SemaChecking.cpp
  cfe/trunk/test/SemaCXX/pr30559.cpp


Index: cfe/trunk/test/SemaCXX/pr30559.cpp
===
--- cfe/trunk/test/SemaCXX/pr30559.cpp
+++ cfe/trunk/test/SemaCXX/pr30559.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only %s
+
+template < bool, class > struct A {};
+template < class, int > void f () {};
+template < class T, int >
+decltype (f < T, 1 >) f (T t, typename A < t == 0, int >::type) {};
+
+struct B {};
+
+int main ()
+{
+  f < B, 0 >;
+  return 0;
+}
+
+template 
+auto foo(T x) -> decltype((x == nullptr), *x) {
+  return *x;
+}
+
+void bar() {
+  foo(new int);
+}
Index: cfe/trunk/lib/Sema/SemaChecking.cpp
===
--- cfe/trunk/lib/Sema/SemaChecking.cpp
+++ cfe/trunk/lib/Sema/SemaChecking.cpp
@@ -11592,6 +11592,9 @@
   }
 
   if (const auto *FD = dyn_cast(PV->getDeclContext())) {
+// Skip function template not specialized yet.
+if (FD->getTemplatedKind() == FunctionDecl::TK_FunctionTemplate)
+  return;
 auto ParamIter = llvm::find(FD->parameters(), PV);
 assert(ParamIter != FD->param_end());
 unsigned ParamNo = std::distance(FD->param_begin(), ParamIter);


Index: cfe/trunk/test/SemaCXX/pr30559.cpp
===
--- cfe/trunk/test/SemaCXX/pr30559.cpp
+++ cfe/trunk/test/SemaCXX/pr30559.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only %s
+
+template < bool, class > struct A {};
+template < class, int > void f () {};
+template < class T, int >
+decltype (f < T, 1 >) f (T t, typename A < t == 0, int >::type) {};
+
+struct B {};
+
+int main ()
+{
+  f < B, 0 >;
+  return 0;
+}
+
+template 
+auto foo(T x) -> decltype((x == nullptr), *x) {
+  return *x;
+}
+
+void bar() {
+  foo(new int);
+}
Index: cfe/trunk/lib/Sema/SemaChecking.cpp
===
--- cfe/trunk/lib/Sema/SemaChecking.cpp
+++ cfe/trunk/lib/Sema/SemaChecking.cpp
@@ -11592,6 +11592,9 @@
   }
 
   if (const auto *FD = dyn_cast(PV->getDeclContext())) {
+// Skip function template not specialized yet.
+if (FD->getTemplatedKind() == FunctionDecl::TK_FunctionTemplate)
+  return;
 auto ParamIter = llvm::find(FD->parameters(), PV);
 assert(ParamIter != FD->param_end());
 unsigned ParamNo = std::distance(FD->param_begin(), ParamIter);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r357236 - [Sema] Fix a crash when nonnull checking

[Michael Liao via cfe-commits]

Author: hliao
Date: Thu Mar 28 20:55:52 2019
New Revision: 357236

URL: http://llvm.org/viewvc/llvm-project?rev=357236=rev
Log:
[Sema] Fix a crash when nonnull checking

Summary:
- If a parameter is used, nonnull checking needs function prototype to
  retrieve the corresponding parameter's attributes. However, at the
  prototype substitution phase when a template is being instantiated,
  expression may be created and checked without a fully specialized
  prototype. Under such a scenario, skip nonnull checking on that
  argument.

Reviewers: rjmccall, tra, yaxunl

Subscribers: javed.absar, kristof.beyls, cfe-commits

Tags: #clang

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

Added:
cfe/trunk/test/SemaCXX/pr30559.cpp
Modified:
cfe/trunk/lib/Sema/SemaChecking.cpp

Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=357236=357235=357236=diff
==
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Thu Mar 28 20:55:52 2019
@@ -11592,6 +11592,9 @@ void Sema::DiagnoseAlwaysNonNullPointer(
   }
 
   if (const auto *FD = dyn_cast(PV->getDeclContext())) {
+// Skip function template not specialized yet.
+if (FD->getTemplatedKind() == FunctionDecl::TK_FunctionTemplate)
+  return;
 auto ParamIter = llvm::find(FD->parameters(), PV);
 assert(ParamIter != FD->param_end());
 unsigned ParamNo = std::distance(FD->param_begin(), ParamIter);

Added: cfe/trunk/test/SemaCXX/pr30559.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/pr30559.cpp?rev=357236=auto
==
--- cfe/trunk/test/SemaCXX/pr30559.cpp (added)
+++ cfe/trunk/test/SemaCXX/pr30559.cpp Thu Mar 28 20:55:52 2019
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only %s
+
+template < bool, class > struct A {};
+template < class, int > void f () {};
+template < class T, int >
+decltype (f < T, 1 >) f (T t, typename A < t == 0, int >::type) {};
+
+struct B {};
+
+int main ()
+{
+  f < B, 0 >;
+  return 0;
+}
+
+template 
+auto foo(T x) -> decltype((x == nullptr), *x) {
+  return *x;
+}
+
+void bar() {
+  foo(new int);
+}


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

[PATCH] D59923: [Driver] Simplify -g level computation and its interaction with -gsplit-dwarf

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D59923#1447198 , @MaskRay wrote:

> In D59923#1446508 , @dblaikie wrote:
>
> > What's the general motivation for this work/changes?
>
>
> The current -gsplit-dwarf handling is a bit complex and the motivation is to 
> make it less confusing.
>
>   -g0 -gsplit-dwarf => 2
>   -gmlt -gsplit-dwarf -fsplit-dwarf-inlining => 2
>   -gmlt -gsplit-dwarf -fno-split-dwarf-inlining => 1 (before) 2 (after)
>   
>
> It is confusing because the composition of `-gmlt -gsplit-dwarf` is different 
> from `-g0 -gsplit-dwarf` when `SplitDWARFInlining` is false.
>
> >> -gmlt -gsplit-dwarf -fno-split-dwarf-inlining => special: 1 (before) 2 
> >> (after)
> > 
> > This ^ is the only semantic change due to this refactoring?
>
> This is the only semantic change.
>
> > There's a desire to be able to compose gmlt+gsplit-dwarf, /if/ 
> > -fno-split-dwarf-inlining is enabled. (for context, 
> > -fno-split-dwarf-inlining is the default in Google's optimized builds, 
> > since split-dwarf-inlining was never implemented in GCC & we didn't want to 
> > regress object size when switching from GCC to Clang (& no one had 
> > complained about that missing functionality))
> > 
> > So with -fno-split-dwarf-inlining, there's value in using gmlt with 
> > gsplit-dwarf (it reduces the size of the .dwo files - reducing the 
> > inputs/size of dwp, etc).
> > 
> > & it sounds like this change breaks that scenario?
>
> Acknowledged the desire to compose `-gsplit-dwarf -gmlt 
> -fno-split-dwarf-inlining`. After this patch, users should place `-gmplt` 
> after `-gsplit-dwarf`.
>
> In Bazel, the order of the command line options from left to right: feature 
> flags < BUILD `copts=` < `--copt=`. So for targets specifying `-g0`/`-gmlt` 
> in their `copts` / explict `--copts=` options, they will override 
> `-gsplit-dwarf` added by the debug feature.


OK - could you include/describe which sets of options disable split-dwarf, 
then? (adding that to the patch description along with the matrix of g1/2, etc? 
(specifically, I'd expect "-gmlt -gsplit-dwarf" means 2+split, "-gsplit-dwarf 
-gmlt" means 1+non-split, and "-fno-split-dwarf-inlining -gsplit-dwarf -gmlt" 
(with -fno-split-dwarf-inlining anywhere in the command line (so long as it's 
after an -fsplit-dwarf-inlining) is 1+split)

I'm still not quite sure changing the meaning of "-gmlt -gsplit-dwarf 
-fno-split-dwarf-inlining" is important. It feels to me like in that mode, 
-gmlt and -gsplit-dwarf compose naturally in either order. Is it code 
complexity or user interface complexity you're trying to address? I'm still a 
bit curious/trying to better understand the motivation here.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59923



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

[PATCH] D59923: [Driver] Simplify -g level computation and its interaction with -gsplit-dwarf

[Fangrui Song via Phabricator via cfe-commits]

MaskRay updated this revision to Diff 192770.
MaskRay edited the summary of this revision.
MaskRay added a comment.

Handle -gsplit-dwarf=


Repository:
  rC Clang

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

https://reviews.llvm.org/D59923

Files:
  lib/Driver/ToolChains/Clang.cpp
  test/Driver/split-debug.c

Index: test/Driver/split-debug.c
===
--- test/Driver/split-debug.c
+++ test/Driver/split-debug.c
@@ -68,7 +68,7 @@
 // RUN: FileCheck -check-prefix=CHECK-SPLIT-WITH-GMLT < %t %s
 //
 // CHECK-SPLIT-WITH-GMLT: "-enable-split-dwarf"
-// CHECK-SPLIT-WITH-GMLT: "-debug-info-kind=line-tables-only"
+// CHECK-SPLIT-WITH-GMLT: "-debug-info-kind=limited"
 // CHECK-SPLIT-WITH-GMLT: "-split-dwarf-file"
 
 // RUN: %clang -target x86_64-unknown-linux-gnu -gsplit-dwarf -fno-split-dwarf-inlining -S -### %s 2> %t
@@ -100,6 +100,8 @@
 
 // RUN: %clang -target x86_64-unknown-linux-gnu -gsplit-dwarf -g0 -S -### %s 2> %t
 // RUN: FileCheck -check-prefix=CHECK-G0-OVER-SPLIT < %t %s
+// RUN: %clang -target x86_64-unknown-linux-gnu -gsplit-dwarf=split -g0 -S -### %s 2> %t
+// RUN: FileCheck -check-prefix=CHECK-G0-OVER-SPLIT < %t %s
 //
 // CHECK-G0-OVER-SPLIT-NOT: "-enable-split-dwarf"
 // CHECK-G0-OVER-SPLIT-NOT: "-debug-info-kind
@@ -107,6 +109,8 @@
 
 // RUN: %clang -target x86_64-unknown-linux-gnu -g0 -gsplit-dwarf -S -### %s 2> %t
 // RUN: FileCheck -check-prefix=CHECK-SPLIT-OVER-G0 < %t %s
+// RUN: %clang -target x86_64-unknown-linux-gnu -g0 -gsplit-dwarf=split -S -### %s 2> %t
+// RUN: FileCheck -check-prefix=CHECK-SPLIT-OVER-G0 < %t %s
 //
 // CHECK-SPLIT-OVER-G0: "-enable-split-dwarf" "-debug-info-kind=limited"
 // CHECK-SPLIT-OVER-G0: "-split-dwarf-file"
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -3158,35 +3158,24 @@
 SplitDWARFInlining = false;
   }
 
-  if (const Arg *A = Args.getLastArg(options::OPT_g_Group)) {
-if (checkDebugInfoOption(A, Args, D, TC)) {
-  // If the last option explicitly specified a debug-info level, use it.
-  if (A->getOption().matches(options::OPT_gN_Group)) {
-DebugInfoKind = DebugLevelToInfoKind(*A);
-// If you say "-gsplit-dwarf -gline-tables-only", -gsplit-dwarf loses.
-// But -gsplit-dwarf is not a g_group option, hence we have to check the
-// order explicitly. If -gsplit-dwarf wins, we fix DebugInfoKind later.
-// This gets a bit more complicated if you've disabled inline info in
-// the skeleton CUs (SplitDWARFInlining) - then there's value in
-// composing split-dwarf and line-tables-only, so let those compose
-// naturally in that case. And if you just turned off debug info,
-// (-gsplit-dwarf -g0) - do that.
-if (DwarfFission != DwarfFissionKind::None) {
-  if (A->getIndex() > SplitDWARFArg->getIndex()) {
-if (DebugInfoKind == codegenoptions::NoDebugInfo ||
-DebugInfoKind == codegenoptions::DebugDirectivesOnly ||
-(DebugInfoKind == codegenoptions::DebugLineTablesOnly &&
- SplitDWARFInlining))
-  DwarfFission = DwarfFissionKind::None;
-  } else if (SplitDWARFInlining)
-DebugInfoKind = codegenoptions::NoDebugInfo;
-}
-  } else {
-// For any other 'g' option, use Limited.
-DebugInfoKind = codegenoptions::LimitedDebugInfo;
-  }
-} else {
-  DebugInfoKind = codegenoptions::LimitedDebugInfo;
+  if (const Arg *A =
+  Args.getLastArg(options::OPT_g_Group, options::OPT_gsplit_dwarf,
+  options::OPT_gsplit_dwarf_EQ)) {
+DebugInfoKind = codegenoptions::LimitedDebugInfo;
+
+// If the last option explicitly specified a debug-info level, use it.
+if (checkDebugInfoOption(A, Args, D, TC) &&
+A->getOption().matches(options::OPT_gN_Group)) {
+  DebugInfoKind = DebugLevelToInfoKind(*A);
+  // For -g0 or -gline-tables-only, drop -gsplit-dwarf. This gets a bit more
+  // complicated if you've disabled inline info in the skeleton CUs
+  // (SplitDWARFInlining) - then there's value in composing split-dwarf and
+  // line-tables-only, so let those compose naturally in that case.
+  if (DebugInfoKind == codegenoptions::NoDebugInfo ||
+  DebugInfoKind == codegenoptions::DebugDirectivesOnly ||
+  (DebugInfoKind == codegenoptions::DebugLineTablesOnly &&
+   SplitDWARFInlining))
+DwarfFission = DwarfFissionKind::None;
 }
   }
 
@@ -3261,16 +3250,12 @@
   }
 }
 
-  // -gsplit-dwarf should turn on -g and enable the backend dwarf
-  // splitting and extraction.
+  // -gsplit-dwarf enables the backend dwarf splitting and extraction.
   if (T.isOSBinFormatELF()) {
 if (!SplitDWARFInlining)
   

[PATCH] D59955: gn build: Add check-clang-tools to run clang-tools-extra lit tests

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE357232: gn build: Add check-clang-tools to run 
clang-tools-extra lit tests (authored by nico, committed by ).
Herald added a project: clang.

Changed prior to commit:
  https://reviews.llvm.org/D59955?vs=192701=192769#toc

Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59955

Files:
  clangd/indexer/CMakeLists.txt


Index: clangd/indexer/CMakeLists.txt
===
--- clangd/indexer/CMakeLists.txt
+++ clangd/indexer/CMakeLists.txt
@@ -11,10 +11,10 @@
 target_link_libraries(clangd-indexer
   PRIVATE
   clangAST
-  clangIndex
-  clangDaemon
   clangBasic
+  clangDaemon
   clangFrontend
+  clangIndex
   clangLex
   clangTooling
 )


Index: clangd/indexer/CMakeLists.txt
===
--- clangd/indexer/CMakeLists.txt
+++ clangd/indexer/CMakeLists.txt
@@ -11,10 +11,10 @@
 target_link_libraries(clangd-indexer
   PRIVATE
   clangAST
-  clangIndex
-  clangDaemon
   clangBasic
+  clangDaemon
   clangFrontend
+  clangIndex
   clangLex
   clangTooling
 )
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r357232 - gn build: Add check-clang-tools to run clang-tools-extra lit tests

[Nico Weber via cfe-commits]

Author: nico
Date: Thu Mar 28 19:49:13 2019
New Revision: 357232

URL: http://llvm.org/viewvc/llvm-project?rev=357232=rev
Log:
gn build: Add check-clang-tools to run clang-tools-extra lit tests

Only runs the clang-tools-extra lit tests; not yet the unit tests.

Add a build file for clangd-indexer too, since it's needed for
the tests.

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

Modified:
clang-tools-extra/trunk/clangd/indexer/CMakeLists.txt

Modified: clang-tools-extra/trunk/clangd/indexer/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/indexer/CMakeLists.txt?rev=357232=357231=357232=diff
==
--- clang-tools-extra/trunk/clangd/indexer/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clangd/indexer/CMakeLists.txt Thu Mar 28 19:49:13 
2019
@@ -11,10 +11,10 @@ add_clang_executable(clangd-indexer
 target_link_libraries(clangd-indexer
   PRIVATE
   clangAST
-  clangIndex
-  clangDaemon
   clangBasic
+  clangDaemon
   clangFrontend
+  clangIndex
   clangLex
   clangTooling
 )


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

[PATCH] D59953: Add .py extension to clang-tools-extra lit cfg files

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL357231: Add .py extension to clang-tools-extra lit cfg files 
(authored by nico, committed by ).
Herald added subscribers: llvm-commits, mstorsjo.
Herald added a project: LLVM.

Changed prior to commit:
  https://reviews.llvm.org/D59953?vs=192698=192768#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D59953

Files:
  clang-tools-extra/trunk/test/CMakeLists.txt
  clang-tools-extra/trunk/test/Unit/lit.cfg
  clang-tools-extra/trunk/test/Unit/lit.cfg.py
  clang-tools-extra/trunk/test/Unit/lit.site.cfg.in
  clang-tools-extra/trunk/test/Unit/lit.site.cfg.py.in
  clang-tools-extra/trunk/test/lit.cfg
  clang-tools-extra/trunk/test/lit.cfg.py
  clang-tools-extra/trunk/test/lit.site.cfg.in
  clang-tools-extra/trunk/test/lit.site.cfg.py.in

Index: clang-tools-extra/trunk/test/lit.site.cfg.py.in
===
--- clang-tools-extra/trunk/test/lit.site.cfg.py.in
+++ clang-tools-extra/trunk/test/lit.site.cfg.py.in
@@ -0,0 +1,31 @@
+@LIT_SITE_CFG_IN_HEADER@
+
+import sys
+
+config.llvm_tools_dir = "@LLVM_TOOLS_DIR@"
+config.llvm_libs_dir = "@LLVM_LIBS_DIR@"
+config.lit_tools_dir = "@LLVM_LIT_TOOLS_DIR@"
+config.clang_tools_binary_dir = "@CLANG_TOOLS_BINARY_DIR@"
+config.clang_tools_dir = "@CLANG_TOOLS_DIR@"
+config.clang_libs_dir = "@SHLIBDIR@"
+config.python_executable = "@PYTHON_EXECUTABLE@"
+config.target_triple = "@TARGET_TRIPLE@"
+config.clang_staticanalyzer = @CLANG_ENABLE_STATIC_ANALYZER@
+config.clangd_xpc_support = @CLANGD_BUILD_XPC_SUPPORT@
+
+# Support substitution of the tools and libs dirs with user parameters. This is
+# used when we can't determine the tool dir at configuration time.
+try:
+config.clang_tools_dir = config.clang_tools_dir % lit_config.params
+config.llvm_tools_dir = config.llvm_tools_dir % lit_config.params
+config.llvm_libs_dir = config.llvm_libs_dir % lit_config.params
+except KeyError:
+e = sys.exc_info()[1]
+key, = e.args
+lit_config.fatal("unable to find %r parameter, use '--param=%s=VALUE'" % (key,key))
+
+import lit.llvm
+lit.llvm.initialize(lit_config, config)
+
+# Let the main config do the real work.
+lit_config.load_config(config, "@CLANG_TOOLS_SOURCE_DIR@/test/lit.cfg.py")
Index: clang-tools-extra/trunk/test/Unit/lit.site.cfg.py.in
===
--- clang-tools-extra/trunk/test/Unit/lit.site.cfg.py.in
+++ clang-tools-extra/trunk/test/Unit/lit.site.cfg.py.in
@@ -0,0 +1,9 @@
+@LIT_SITE_CFG_IN_HEADER@
+
+config.extra_tools_obj_dir = "@CLANG_TOOLS_BINARY_DIR@/unittests"
+config.extra_tools_src_dir = "@CLANG_TOOLS_SOURCE_DIR@/unittests"
+config.llvm_libs_dir = "@LLVM_LIBS_DIR@"
+config.shlibdir = "@SHLIBDIR@"
+config.target_triple = "@TARGET_TRIPLE@"
+
+lit_config.load_config(config, "@CLANG_TOOLS_SOURCE_DIR@/test/Unit/lit.cfg.py")
Index: clang-tools-extra/trunk/test/Unit/lit.cfg.py
===
--- clang-tools-extra/trunk/test/Unit/lit.cfg.py
+++ clang-tools-extra/trunk/test/Unit/lit.cfg.py
@@ -0,0 +1,37 @@
+# -*- Python -*-
+
+import platform
+
+import lit.formats
+
+config.name = "Extra Tools Unit Tests"
+config.suffixes = [] # Seems not to matter for google tests?
+
+# Test Source and Exec root dirs both point to the same directory where google
+# test binaries are built.
+
+config.test_source_root = config.extra_tools_obj_dir
+config.test_exec_root = config.test_source_root
+
+# All GoogleTests are named to have 'Tests' as their suffix. The '.' option is
+# a special value for GoogleTest indicating that it should look through the
+# entire testsuite recursively for tests (alternatively, one could provide a
+# ;-separated list of subdirectories).
+config.test_format = lit.formats.GoogleTest('.', 'Tests')
+
+if platform.system() == 'Darwin':
+shlibpath_var = 'DYLD_LIBRARY_PATH'
+elif platform.system() == 'Windows':
+shlibpath_var = 'PATH'
+else:
+shlibpath_var = 'LD_LIBRARY_PATH'
+
+# Point the dynamic loader at dynamic libraries in 'lib'.
+shlibpath = os.path.pathsep.join((config.shlibdir, config.llvm_libs_dir,
+ config.environment.get(shlibpath_var,'')))
+
+# Win32 seeks DLLs along %PATH%.
+if sys.platform in ['win32', 'cygwin'] and os.path.isdir(config.shlibdir):
+shlibpath = os.path.pathsep.join((config.shlibdir, shlibpath))
+
+config.environment[shlibpath_var] = shlibpath
Index: clang-tools-extra/trunk/test/lit.cfg.py
===
--- clang-tools-extra/trunk/test/lit.cfg.py
+++ clang-tools-extra/trunk/test/lit.cfg.py
@@ -0,0 +1,145 @@
+# -*- Python -*-
+
+import os
+import platform
+import re
+import subprocess
+
+import lit.formats
+import lit.util
+
+# Configuration file for the 'lit' test runner.
+
+# name: The name of this 

[PATCH] D59923: [Driver] Simplify -g level computation and its interaction with -gsplit-dwarf

[Fangrui Song via Phabricator via cfe-commits]

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

In D59923#1446508 , @dblaikie wrote:

> What's the general motivation for this work/changes?


The current -gsplit-dwarf handling is a bit complex and the motivation is to 
make it less confused.

  -g0 -gsplit-dwarf => 2
  -gmlt -gsplit-dwarf -fsplit-dwarf-inlining => 2
  -gmlt -gsplit-dwarf -fno-split-dwarf-inlining => 1 (before) 2 (after)

It is confusing because the composition of `-gmlt -gsplit-dwarf` is different 
from `-g0 -gsplit-dwarf` when `SplitDWARFInlining` is false.

>> -gmlt -gsplit-dwarf -fno-split-dwarf-inlining => special: 1 (before) 2 
>> (after)
> 
> This ^ is the only semantic change due to this refactoring?

This is the only semantic change.

> There's a desire to be able to compose gmlt+gsplit-dwarf, /if/ 
> -fno-split-dwarf-inlining is enabled. (for context, -fno-split-dwarf-inlining 
> is the default in Google's optimized builds, since split-dwarf-inlining was 
> never implemented in GCC & we didn't want to regress object size when 
> switching from GCC to Clang (& no one had complained about that missing 
> functionality))
> 
> So with -fno-split-dwarf-inlining, there's value in using gmlt with 
> gsplit-dwarf (it reduces the size of the .dwo files - reducing the 
> inputs/size of dwp, etc).
> 
> & it sounds like this change breaks that scenario?

Acknowledged the desire to compose `-gsplit-dwarf -gmlt 
-fno-split-dwarf-inlining`. After this patch, users should place `-gmplt` after 
`-gsplit-dwarf`.

In Bazel, the order of the command line options from left to right: feature 
flags < BUILD `copts=` < `--copt=`. So for targets specifying `-g0`/`-gmlt` in 
their `copts` / explict `--copts=` options, they will override `-gsplit-dwarf` 
added by the debug feature.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59923



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

[clang-tools-extra] r357231 - Add .py extension to clang-tools-extra lit cfg files

[Nico Weber via cfe-commits]

Author: nico
Date: Thu Mar 28 19:46:31 2019
New Revision: 357231

URL: http://llvm.org/viewvc/llvm-project?rev=357231=rev
Log:
Add .py extension to clang-tools-extra lit cfg files

Follow-up to r313892, which did this for clang and llvm.

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

Added:
clang-tools-extra/trunk/test/Unit/lit.cfg.py
clang-tools-extra/trunk/test/Unit/lit.site.cfg.py.in
clang-tools-extra/trunk/test/lit.cfg.py
clang-tools-extra/trunk/test/lit.site.cfg.py.in
Removed:
clang-tools-extra/trunk/test/Unit/lit.cfg
clang-tools-extra/trunk/test/Unit/lit.site.cfg.in
clang-tools-extra/trunk/test/lit.cfg
clang-tools-extra/trunk/test/lit.site.cfg.in
Modified:
clang-tools-extra/trunk/test/CMakeLists.txt

Modified: clang-tools-extra/trunk/test/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/CMakeLists.txt?rev=357231=357230=357231=diff
==
--- clang-tools-extra/trunk/test/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/test/CMakeLists.txt Thu Mar 28 19:46:31 2019
@@ -20,13 +20,17 @@ llvm_canonicalize_cmake_booleans(
   CLANGD_BUILD_XPC_SUPPORT)
 
 configure_lit_site_cfg(
-  ${CMAKE_CURRENT_SOURCE_DIR}/lit.site.cfg.in
-  ${CMAKE_CURRENT_BINARY_DIR}/lit.site.cfg
+  ${CMAKE_CURRENT_SOURCE_DIR}/lit.site.cfg.py.in
+  ${CMAKE_CURRENT_BINARY_DIR}/lit.site.cfg.py
+  MAIN_CONFIG
+  ${CMAKE_CURRENT_SOURCE_DIR}/lit.cfg.py
   )
 
 configure_lit_site_cfg(
-  ${CMAKE_CURRENT_SOURCE_DIR}/Unit/lit.site.cfg.in
-  ${CMAKE_CURRENT_BINARY_DIR}/Unit/lit.site.cfg
+  ${CMAKE_CURRENT_SOURCE_DIR}/Unit/lit.site.cfg.py.in
+  ${CMAKE_CURRENT_BINARY_DIR}/Unit/lit.site.cfg.py
+  MAIN_CONFIG
+  ${CMAKE_CURRENT_SOURCE_DIR}/Unit/lit.cfg.py
   )
 
 option(CLANG_TOOLS_TEST_USE_VG "Run Clang tools' tests under Valgrind" OFF)

Removed: clang-tools-extra/trunk/test/Unit/lit.cfg
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/Unit/lit.cfg?rev=357230=auto
==
--- clang-tools-extra/trunk/test/Unit/lit.cfg (original)
+++ clang-tools-extra/trunk/test/Unit/lit.cfg (removed)
@@ -1,37 +0,0 @@
-# -*- Python -*-
-
-import platform
-
-import lit.formats
-
-config.name = "Extra Tools Unit Tests"
-config.suffixes = [] # Seems not to matter for google tests?
-
-# Test Source and Exec root dirs both point to the same directory where google
-# test binaries are built.
-
-config.test_source_root = config.extra_tools_obj_dir
-config.test_exec_root = config.test_source_root
-
-# All GoogleTests are named to have 'Tests' as their suffix. The '.' option is
-# a special value for GoogleTest indicating that it should look through the
-# entire testsuite recursively for tests (alternatively, one could provide a
-# ;-separated list of subdirectories).
-config.test_format = lit.formats.GoogleTest('.', 'Tests')
-
-if platform.system() == 'Darwin':
-shlibpath_var = 'DYLD_LIBRARY_PATH'
-elif platform.system() == 'Windows':
-shlibpath_var = 'PATH'
-else:
-shlibpath_var = 'LD_LIBRARY_PATH'
-
-# Point the dynamic loader at dynamic libraries in 'lib'.
-shlibpath = os.path.pathsep.join((config.shlibdir, config.llvm_libs_dir,
- config.environment.get(shlibpath_var,'')))
-
-# Win32 seeks DLLs along %PATH%.
-if sys.platform in ['win32', 'cygwin'] and os.path.isdir(config.shlibdir):
-shlibpath = os.path.pathsep.join((config.shlibdir, shlibpath))
-
-config.environment[shlibpath_var] = shlibpath

Added: clang-tools-extra/trunk/test/Unit/lit.cfg.py
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/Unit/lit.cfg.py?rev=357231=auto
==
--- clang-tools-extra/trunk/test/Unit/lit.cfg.py (added)
+++ clang-tools-extra/trunk/test/Unit/lit.cfg.py Thu Mar 28 19:46:31 2019
@@ -0,0 +1,37 @@
+# -*- Python -*-
+
+import platform
+
+import lit.formats
+
+config.name = "Extra Tools Unit Tests"
+config.suffixes = [] # Seems not to matter for google tests?
+
+# Test Source and Exec root dirs both point to the same directory where google
+# test binaries are built.
+
+config.test_source_root = config.extra_tools_obj_dir
+config.test_exec_root = config.test_source_root
+
+# All GoogleTests are named to have 'Tests' as their suffix. The '.' option is
+# a special value for GoogleTest indicating that it should look through the
+# entire testsuite recursively for tests (alternatively, one could provide a
+# ;-separated list of subdirectories).
+config.test_format = lit.formats.GoogleTest('.', 'Tests')
+
+if platform.system() == 'Darwin':
+shlibpath_var = 'DYLD_LIBRARY_PATH'
+elif platform.system() == 'Windows':
+shlibpath_var = 'PATH'
+else:
+shlibpath_var = 'LD_LIBRARY_PATH'
+
+# Point the dynamic loader at dynamic libraries in 'lib'.
+shlibpath = os.path.pathsep.join((config.shlibdir, 

[PATCH] D58797: [Sema] Add some compile time _FORTIFY_SOURCE diagnostics

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington added a comment.

Ah, looks like the problem is we're sending a dependent expression to the 
constant evaluator, we should just bail out in that case. I'll fix this 
tomorrow morning, thanks for tracking this down!


Repository:
  rC Clang

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

https://reviews.llvm.org/D58797



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

[PATCH] D59121: [analyzer] Fix macro names in diagnostics within bigger macros.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ updated this revision to Diff 192765.
NoQ added a comment.

An elegant solution for a more civilized age. Unfortunately, doesn't preserve 
the `UINT32_MAX` macro in the newly added test - i'll try a bit harder to 
preserve it. Relies on D59977  to work.


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

https://reviews.llvm.org/D59121

Files:
  clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  clang/test/Analysis/diagnostics/macros.cpp


Index: clang/test/Analysis/diagnostics/macros.cpp
===
--- clang/test/Analysis/diagnostics/macros.cpp
+++ clang/test/Analysis/diagnostics/macros.cpp
@@ -47,3 +47,14 @@
 // expected-note@-1 {{Dereference of null pointer (loaded 
from variable 'p')}}
   }
 }
+
+#define nested_null_split(x) if ((x) != UINT32_MAX) {}
+
+void testNestedNullSplitMacro(int i, int *p) {
+  nested_null_split(i); // expected-note {{Assuming 'i' is equal to -1}}
+// expected-note@-1 {{Taking false branch}}
+  if (!p) // expected-note {{Assuming 'p' is null}}
+  // expected-note@-1 {{Taking true branch}}
+*p = 1; // expected-warning {{Dereference of null pointer (loaded from 
variable 'p')}}
+// expected-note@-1 {{Dereference of null pointer (loaded from 
variable 'p')}}
+}
Index: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===
--- clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -1969,43 +1969,22 @@
   const Expr *OriginalExpr = Ex;
   Ex = Ex->IgnoreParenCasts();
 
-  // Use heuristics to determine if Ex is a macro expending to a literal and
-  // if so, use the macro's name.
-  SourceLocation LocStart = Ex->getBeginLoc();
-  SourceLocation LocEnd = Ex->getEndLoc();
-  if (LocStart.isMacroID() && LocEnd.isMacroID() &&
-  (isa(Ex) ||
-   isa(Ex) ||
-   isa(Ex) ||
-   isa(Ex) ||
-   isa(Ex))) {
-StringRef StartName = Lexer::getImmediateMacroNameForDiagnostics(LocStart,
-  BRC.getSourceManager(), BRC.getASTContext().getLangOpts());
-StringRef EndName = Lexer::getImmediateMacroNameForDiagnostics(LocEnd,
-  BRC.getSourceManager(), BRC.getASTContext().getLangOpts());
-bool beginAndEndAreTheSameMacro = StartName.equals(EndName);
-
-bool partOfParentMacro = false;
-if (ParentEx->getBeginLoc().isMacroID()) {
-  StringRef PName = Lexer::getImmediateMacroNameForDiagnostics(
-  ParentEx->getBeginLoc(), BRC.getSourceManager(),
-  BRC.getASTContext().getLangOpts());
-  partOfParentMacro = PName.equals(StartName);
-}
-
-if (beginAndEndAreTheSameMacro && !partOfParentMacro ) {
-  // Get the location of the macro name as written by the caller.
-  SourceLocation Loc = LocStart;
-  while (LocStart.isMacroID()) {
-Loc = LocStart;
-LocStart = BRC.getSourceManager().getImmediateMacroCallerLoc(LocStart);
+  if (isa(Ex) || isa(Ex) ||
+  isa(Ex) || isa(Ex) ||
+  isa(Ex)) {
+// Use heuristics to determine if the expression is a macro
+// expanding to a literal and if so, use the macro's name.
+SourceLocation LocStart = OriginalExpr->getBeginLoc();
+SourceLocation LocEnd = OriginalExpr->getEndLoc();
+if (LocStart.isMacroID() && LocEnd.isMacroID()) {
+  SourceManager  = BRC.getSourceManager();
+  const LangOptions  = BRC.getASTContext().getLangOpts();
+  if (Lexer::isAtStartOfMacroExpansion(LocStart, SM, LO) &&
+  Lexer::isAtEndOfMacroExpansion(LocEnd, SM, LO)) {
+SourceRange R{LocStart, LocEnd};
+Out << Lexer::getSourceText(Lexer::getAsCharRange(R, SM, LO), SM, LO);
+return false;
   }
-  StringRef MacroName = Lexer::getImmediateMacroNameForDiagnostics(
-Loc, BRC.getSourceManager(), BRC.getASTContext().getLangOpts());
-
-  // Return the macro name.
-  Out << MacroName;
-  return false;
 }
   }
 


Index: clang/test/Analysis/diagnostics/macros.cpp
===
--- clang/test/Analysis/diagnostics/macros.cpp
+++ clang/test/Analysis/diagnostics/macros.cpp
@@ -47,3 +47,14 @@
 // expected-note@-1 {{Dereference of null pointer (loaded from variable 'p')}}
   }
 }
+
+#define nested_null_split(x) if ((x) != UINT32_MAX) {}
+
+void testNestedNullSplitMacro(int i, int *p) {
+  nested_null_split(i); // expected-note {{Assuming 'i' is equal to -1}}
+// expected-note@-1 {{Taking false branch}}
+  if (!p) // expected-note {{Assuming 'p' is null}}
+  // expected-note@-1 {{Taking true branch}}
+*p = 1; // expected-warning {{Dereference of null pointer (loaded from variable 'p')}}
+// expected-note@-1 {{Dereference of null pointer (loaded from variable 'p')}}
+}
Index: 

[PATCH] D59977: [Lexer] Fix an off-by-one bug in Lexer::getAsCharRange() - NFC.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
NoQ added reviewers: rsmith, dcoughlin, Szelethus.
Herald added subscribers: cfe-commits, jdoerfert.
Herald added a project: clang.

As the unit test demonstrates, the `.getLocWithOffset(-1)` part was unnecessary.

@rsmith - you introduced these functions, WDYT?, hope i got the intent right.

The only user of this function was the plist file emitter (in Static Analyzer 
and ARCMigrator). It means that a lot of Static Analyzer's plist arrows are in 
fact off by one character. The patch carefully preserves this completely 
incorrect behavior and causes no functional change, i.e. no plist format 
breakage.


Repository:
  rC Clang

https://reviews.llvm.org/D59977

Files:
  clang/include/clang/Basic/PlistSupport.h
  clang/include/clang/Lex/Lexer.h
  clang/unittests/Lex/LexerTest.cpp


Index: clang/unittests/Lex/LexerTest.cpp
===
--- clang/unittests/Lex/LexerTest.cpp
+++ clang/unittests/Lex/LexerTest.cpp
@@ -513,4 +513,23 @@
   EXPECT_EQ(String6, R"(a\\\n\n\nb)");
 }
 
+TEST_F(LexerTest, CharRangeOffByOne) {
+  std::vector toks = Lex(R"(#define MOO 1
+void foo() { MOO; })");
+  const Token  = toks[5];
+
+  EXPECT_EQ(getSourceText(moo, moo), "MOO");
+
+  SourceRange R{moo.getLocation(), moo.getLocation()};
+
+  EXPECT_TRUE(
+  Lexer::isAtStartOfMacroExpansion(R.getBegin(), SourceMgr, LangOpts));
+  EXPECT_TRUE(
+  Lexer::isAtEndOfMacroExpansion(R.getEnd(), SourceMgr, LangOpts));
+
+  CharSourceRange CR = Lexer::getAsCharRange(R, SourceMgr, LangOpts);
+
+  EXPECT_EQ(Lexer::getSourceText(CR, SourceMgr, LangOpts), "MOO"); // Was "MO".
+}
+
 } // anonymous namespace
Index: clang/include/clang/Lex/Lexer.h
===
--- clang/include/clang/Lex/Lexer.h
+++ clang/include/clang/Lex/Lexer.h
@@ -382,7 +382,7 @@
 SourceLocation End = getLocForEndOfToken(Range.getEnd(), 0, SM, LangOpts);
 return End.isInvalid() ? CharSourceRange()
: CharSourceRange::getCharRange(
- Range.getBegin(), End.getLocWithOffset(-1));
+ Range.getBegin(), End);
   }
   static CharSourceRange getAsCharRange(CharSourceRange Range,
 const SourceManager ,
Index: clang/include/clang/Basic/PlistSupport.h
===
--- clang/include/clang/Basic/PlistSupport.h
+++ clang/include/clang/Basic/PlistSupport.h
@@ -127,7 +127,11 @@
   assert(R.isCharRange() && "cannot handle a token range");
   Indent(o, indent) << "\n";
   EmitLocation(o, SM, R.getBegin(), FM, indent + 1);
-  EmitLocation(o, SM, R.getEnd(), FM, indent + 1);
+
+  // The ".getLocWithOffset(-1)" emulates the behavior of an off-by-one bug
+  // in Lexer that is already fixed. It is here for backwards compatibility
+  // even though it is incorrect.
+  EmitLocation(o, SM, R.getEnd().getLocWithOffset(-1), FM, indent + 1);
   Indent(o, indent) << "\n";
 }
 


Index: clang/unittests/Lex/LexerTest.cpp
===
--- clang/unittests/Lex/LexerTest.cpp
+++ clang/unittests/Lex/LexerTest.cpp
@@ -513,4 +513,23 @@
   EXPECT_EQ(String6, R"(a\\\n\n\nb)");
 }
 
+TEST_F(LexerTest, CharRangeOffByOne) {
+  std::vector toks = Lex(R"(#define MOO 1
+void foo() { MOO; })");
+  const Token  = toks[5];
+
+  EXPECT_EQ(getSourceText(moo, moo), "MOO");
+
+  SourceRange R{moo.getLocation(), moo.getLocation()};
+
+  EXPECT_TRUE(
+  Lexer::isAtStartOfMacroExpansion(R.getBegin(), SourceMgr, LangOpts));
+  EXPECT_TRUE(
+  Lexer::isAtEndOfMacroExpansion(R.getEnd(), SourceMgr, LangOpts));
+
+  CharSourceRange CR = Lexer::getAsCharRange(R, SourceMgr, LangOpts);
+
+  EXPECT_EQ(Lexer::getSourceText(CR, SourceMgr, LangOpts), "MOO"); // Was "MO".
+}
+
 } // anonymous namespace
Index: clang/include/clang/Lex/Lexer.h
===
--- clang/include/clang/Lex/Lexer.h
+++ clang/include/clang/Lex/Lexer.h
@@ -382,7 +382,7 @@
 SourceLocation End = getLocForEndOfToken(Range.getEnd(), 0, SM, LangOpts);
 return End.isInvalid() ? CharSourceRange()
: CharSourceRange::getCharRange(
- Range.getBegin(), End.getLocWithOffset(-1));
+ Range.getBegin(), End);
   }
   static CharSourceRange getAsCharRange(CharSourceRange Range,
 const SourceManager ,
Index: clang/include/clang/Basic/PlistSupport.h
===
--- clang/include/clang/Basic/PlistSupport.h
+++ clang/include/clang/Basic/PlistSupport.h
@@ -127,7 +127,11 @@
   assert(R.isCharRange() && "cannot handle a token range");
   Indent(o, indent) << "\n";
   EmitLocation(o, SM, R.getBegin(), FM, indent + 1);
-  EmitLocation(o, 

[PATCH] D59900: [Sema] Fix a crash when nonnull checking

[Michael Liao via Phabricator via cfe-commits]

hliao added a comment.

just search bugzilla and, fortunately, found this issue is reported 2+ years 
ago @ https://bugs.llvm.org/show_bug.cgi?id=30559. I will revise the test case 
to PR30559 and move it into test/SemaCXX/nonnull.cpp


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59900



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

[PATCH] D58797: [Sema] Add some compile time _FORTIFY_SOURCE diagnostics

[Petr Hosek via Phabricator via cfe-commits]

phosek added a comment.

This is triggering a Clang assertion failure in Fuchsia build:

  clang/lib/AST/ExprConstant.cpp:5032: bool (anonymous 
namespace)::ExprEvaluatorBase<(anonymous 
namespace)::PointerExprEvaluator>::VisitMemberExpr(const clang::MemberExpr *) 
[Derived = (anonymous namespace)::PointerExprEvaluator]: Assertion 
`!E->isArrow() && "missing call to bound member function?"' failed.

I've filed PR41286 which also has a reproducer.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58797



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

[PATCH] D59802: [clang-tidy] Add new checker: llvm-avoid-cast-in-conditional

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

If the `dyn_cast_or_null<>` replacement is acceptable, I'll update the 
documentation and warning message -- suggestions are appreciated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59802



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

[PATCH] D59802: [clang-tidy] Add new checker: llvm-avoid-cast-in-conditional

[Don Hinton via Phabricator via cfe-commits]

hintonda updated this revision to Diff 192761.
hintonda added a comment.

- Remove spurious auto's.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59802

Files:
  clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.cpp
  clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.h
  clang-tools-extra/clang-tidy/llvm/CMakeLists.txt
  clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
  clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/llvm-avoid-cast-in-conditional.rst
  clang-tools-extra/test/clang-tidy/llvm-avoid-cast-in-conditional.cpp
  clang/include/clang/Basic/LLVM.h

Index: clang/include/clang/Basic/LLVM.h
===
--- clang/include/clang/Basic/LLVM.h
+++ clang/include/clang/Basic/LLVM.h
@@ -31,6 +31,7 @@
   template class ArrayRef;
   template class MutableArrayRef;
   template class OwningArrayRef;
+  template  class SmallSet;
   template class SmallString;
   template class SmallVector;
   template class SmallVectorImpl;
@@ -66,6 +67,7 @@
   using llvm::Optional;
   using llvm::OwningArrayRef;
   using llvm::SaveAndRestore;
+  using llvm::SmallSet;
   using llvm::SmallString;
   using llvm::SmallVector;
   using llvm::SmallVectorImpl;
Index: clang-tools-extra/test/clang-tidy/llvm-avoid-cast-in-conditional.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/llvm-avoid-cast-in-conditional.cpp
@@ -0,0 +1,96 @@
+// RUN: %check_clang_tidy %s llvm-avoid-cast-in-conditional %t
+
+struct X;
+struct Y;
+struct Z {
+  int foo();
+  X *bar();
+};
+
+template 
+bool isa(Y *);
+template 
+X *cast(Y *);
+template 
+X *dyn_cast(Y *);
+template 
+X *dyn_cast_or_null(Y *);
+
+bool foo(Y *y) {
+  if (auto x = cast(y))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:16: warning: {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (auto x = dyn_cast(y))
+
+  if (cast(y))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning:  {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (isa(y))
+
+  if (dyn_cast(y))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning:  {{.*dyn_cast<> not used .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (isa(y))
+
+  // These don't trigger a warning.
+  if (auto x = cast(y)->foo())
+return true;
+  if (cast(y)->foo())
+return true;
+
+  while (auto x = cast(y))
+break;
+  // CHECK-MESSAGES: :[[@LINE-2]]:19: warning:  {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (auto x = dyn_cast(y))
+
+  while (cast(y))
+break;
+  // CHECK-MESSAGES: :[[@LINE-2]]:10: warning:  {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (isa(y))
+
+  while (dyn_cast(y))
+break;
+  // CHECK-MESSAGES: :[[@LINE-2]]:10: warning:  {{.*dyn_cast<> not used .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (isa(y))
+
+  // These don't trigger a warning.
+  while (auto x = cast(y)->foo())
+break;
+  while (cast(y)->foo())
+break;
+
+  do {
+break;
+  } while (cast(y));
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning:  {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (isa(y));
+
+  do {
+break;
+  } while (dyn_cast(y));
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning:  {{.*dyn_cast<> not used .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (isa(y));
+
+  if (y && cast(y))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning:  {{.*use dyn_cast_or_null .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (dyn_cast_or_null(y))
+
+  Z z;
+  if (z.bar() && cast(z.bar()))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning:  {{.*use dyn_cast_or_null .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (dyn_cast_or_null(z.bar()))
+
+  bool b = y && cast(y);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning:  {{.*use dyn_cast_or_null .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: bool b = dyn_cast_or_null(y);
+
+  // These don't trigger a warning.
+  if (y && cast(z.bar())) {
+return true;
+  }
+  bool b2 = y && cast();
+
+  return false;
+}
Index: clang-tools-extra/docs/clang-tidy/checks/llvm-avoid-cast-in-conditional.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/llvm-avoid-cast-in-conditional.rst
@@ -0,0 +1,28 @@
+.. title:: clang-tidy - llvm-avoid-cast-in-conditional
+
+llvm-avoid-cast-in-conditional
+==
+
+  Finds uses of ``cast<>`` in conditionals of ``if``, ``while`` or
+  ``do`` statements, which will assert rather than return a null
+  pointer. It 

[PATCH] D59802: [clang-tidy] Add new checker: llvm-avoid-cast-in-conditional

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

In D59802#1447127 , @Eugene.Zelenko 
wrote:

> In D59802#1447108 , @hintonda wrote:
>
> >
>
>
> It's good idea to follow modernize-use-auto convention without introducing 
> exceptions.


No worries.  I'm just not sure I fully grok them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59802



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

[PATCH] D59975: [fuchsia] Add clang-doc to Fuchsia distribution

[Petr Hosek via Phabricator via cfe-commits]

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

LGTM


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

https://reviews.llvm.org/D59975



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

[PATCH] D59974: [clang-doc] Build as clang_tool

[Petr Hosek via Phabricator via cfe-commits]

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

LGTM


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

https://reviews.llvm.org/D59974



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

[PATCH] D59974: [clang-doc] Build as clang_tool

[Julie Hockett via Phabricator via cfe-commits]

juliehockett created this revision.
juliehockett added reviewers: phosek, leonardchan.
juliehockett added a project: clang-tools-extra.
Herald added a subscriber: mgorny.

Instead of as clang_executable.


https://reviews.llvm.org/D59974

Files:
  clang-tools-extra/clang-doc/tool/CMakeLists.txt


Index: clang-tools-extra/clang-doc/tool/CMakeLists.txt
===
--- clang-tools-extra/clang-doc/tool/CMakeLists.txt
+++ clang-tools-extra/clang-doc/tool/CMakeLists.txt
@@ -1,6 +1,6 @@
 include_directories(${CMAKE_CURRENT_SOURCE_DIR}/..)
 
-add_clang_executable(clang-doc
+add_clang_tool(clang-doc
   ClangDocMain.cpp
   )
 


Index: clang-tools-extra/clang-doc/tool/CMakeLists.txt
===
--- clang-tools-extra/clang-doc/tool/CMakeLists.txt
+++ clang-tools-extra/clang-doc/tool/CMakeLists.txt
@@ -1,6 +1,6 @@
 include_directories(${CMAKE_CURRENT_SOURCE_DIR}/..)
 
-add_clang_executable(clang-doc
+add_clang_tool(clang-doc
   ClangDocMain.cpp
   )
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59802: [clang-tidy] Add new checker: llvm-avoid-cast-in-conditional

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added a comment.

In D59802#1447108 , @hintonda wrote:

>


It's good idea to follow modernize-use-auto convention without introducing 
exceptions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59802



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

[PATCH] D54978: Move the SMT API to LLVM

[guangqing.chen via Phabricator via cfe-commits]

gou4shi1 added a comment.

My own out-of-tree pass `#include ` and use cmake's 
`add_llvm_library` to compile it into a `.so`
However, `opt -load-pass-plugin=my-pass.so -passes="foo" bar.ll` fails:
`opt: symbol lookup error: Passes/libStackPasses.so: undefined symbol: 
_ZN4llvm14CreateZ3SolverEv`
(c++filt: `llvm::CreateZ3Solver()`)
If I move the content of `Z3Solver.cpp` into another file of `llvm/Support` 
(like `llvm/Support/raw_ostream.cpp`)
everything works.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D54978



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

[PATCH] D59802: [clang-tidy] Add new checker: llvm-avoid-cast-in-conditional

[Don Hinton via Phabricator via cfe-commits]

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






Comment at: clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.cpp:73
+  Result.Nodes.getNodeAs("cast-assign")) {
+auto StartLoc = MatchedDecl->getCallee()->getExprLoc();
+auto EndLoc = StartLoc.getLocWithOffset(StringRef("cast").size() - 1);

Eugene.Zelenko wrote:
> Please don't use auto where type is not obvious.
Happy to make the change, but thought the `get.*Loc()` methods were obvious?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59802



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

[PATCH] D59802: [clang-tidy] Add new checker: llvm-avoid-cast-in-conditional

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.cpp:73
+  Result.Nodes.getNodeAs("cast-assign")) {
+auto StartLoc = MatchedDecl->getCallee()->getExprLoc();
+auto EndLoc = StartLoc.getLocWithOffset(StringRef("cast").size() - 1);

Please don't use auto where type is not obvious.



Comment at: clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.cpp:74
+auto StartLoc = MatchedDecl->getCallee()->getExprLoc();
+auto EndLoc = StartLoc.getLocWithOffset(StringRef("cast").size() - 1);
+

Please don't use auto where type is not obvious.



Comment at: clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.cpp:82
+ Result.Nodes.getNodeAs("cast")) {
+auto StartLoc = MatchedDecl->getCallee()->getExprLoc();
+auto EndLoc = StartLoc.getLocWithOffset(StringRef("cast").size() - 1);

Please don't use auto where type is not obvious.



Comment at: clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.cpp:83
+auto StartLoc = MatchedDecl->getCallee()->getExprLoc();
+auto EndLoc = StartLoc.getLocWithOffset(StringRef("cast").size() - 1);
+

Please don't use auto where type is not obvious.



Comment at: clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.cpp:90
+ Result.Nodes.getNodeAs("dyn_cast")) {
+auto StartLoc = MatchedDecl->getCallee()->getExprLoc();
+auto EndLoc = StartLoc.getLocWithOffset(StringRef("dyn_cast").size() - 1);

Please don't use auto where type is not obvious.



Comment at: clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.cpp:91
+auto StartLoc = MatchedDecl->getCallee()->getExprLoc();
+auto EndLoc = StartLoc.getLocWithOffset(StringRef("dyn_cast").size() - 1);
+

Please don't use auto where type is not obvious.



Comment at: 
clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.cpp:101
+
+auto ArgRange = CharSourceRange::getTokenRange(Arg->getSourceRange());
+std::string ArgString(

Please don't use auto where type is not obvious.



Comment at: 
clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.cpp:104
+Lexer::getSourceText(ArgRange, *Result.SourceManager, getLangOpts()));
+auto LHSRange = CharSourceRange::getTokenRange(LHS->getSourceRange());
+std::string LHSString(

Please don't use auto where type is not obvious.



Comment at: 
clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.cpp:107
+Lexer::getSourceText(LHSRange, *Result.SourceManager, getLangOpts()));
+auto RHSRange = CharSourceRange::getTokenRange(RHS->getSourceRange());
+std::string RHSString(

Please don't use auto where type is not obvious.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59802



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

[PATCH] D59802: [clang-tidy] Add new checker: llvm-avoid-cast-in-conditional

[Don Hinton via Phabricator via cfe-commits]

hintonda updated this revision to Diff 192756.
hintonda added a comment.

- Remove commented code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59802

Files:
  clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.cpp
  clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.h
  clang-tools-extra/clang-tidy/llvm/CMakeLists.txt
  clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
  clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/llvm-avoid-cast-in-conditional.rst
  clang-tools-extra/test/clang-tidy/llvm-avoid-cast-in-conditional.cpp
  clang/include/clang/Basic/LLVM.h

Index: clang/include/clang/Basic/LLVM.h
===
--- clang/include/clang/Basic/LLVM.h
+++ clang/include/clang/Basic/LLVM.h
@@ -31,6 +31,7 @@
   template class ArrayRef;
   template class MutableArrayRef;
   template class OwningArrayRef;
+  template  class SmallSet;
   template class SmallString;
   template class SmallVector;
   template class SmallVectorImpl;
@@ -66,6 +67,7 @@
   using llvm::Optional;
   using llvm::OwningArrayRef;
   using llvm::SaveAndRestore;
+  using llvm::SmallSet;
   using llvm::SmallString;
   using llvm::SmallVector;
   using llvm::SmallVectorImpl;
Index: clang-tools-extra/test/clang-tidy/llvm-avoid-cast-in-conditional.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/llvm-avoid-cast-in-conditional.cpp
@@ -0,0 +1,96 @@
+// RUN: %check_clang_tidy %s llvm-avoid-cast-in-conditional %t
+
+struct X;
+struct Y;
+struct Z {
+  int foo();
+  X *bar();
+};
+
+template 
+bool isa(Y *);
+template 
+X *cast(Y *);
+template 
+X *dyn_cast(Y *);
+template 
+X *dyn_cast_or_null(Y *);
+
+bool foo(Y *y) {
+  if (auto x = cast(y))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:16: warning: {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (auto x = dyn_cast(y))
+
+  if (cast(y))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning:  {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (isa(y))
+
+  if (dyn_cast(y))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning:  {{.*dyn_cast<> not used .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (isa(y))
+
+  // These don't trigger a warning.
+  if (auto x = cast(y)->foo())
+return true;
+  if (cast(y)->foo())
+return true;
+
+  while (auto x = cast(y))
+break;
+  // CHECK-MESSAGES: :[[@LINE-2]]:19: warning:  {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (auto x = dyn_cast(y))
+
+  while (cast(y))
+break;
+  // CHECK-MESSAGES: :[[@LINE-2]]:10: warning:  {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (isa(y))
+
+  while (dyn_cast(y))
+break;
+  // CHECK-MESSAGES: :[[@LINE-2]]:10: warning:  {{.*dyn_cast<> not used .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (isa(y))
+
+  // These don't trigger a warning.
+  while (auto x = cast(y)->foo())
+break;
+  while (cast(y)->foo())
+break;
+
+  do {
+break;
+  } while (cast(y));
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning:  {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (isa(y));
+
+  do {
+break;
+  } while (dyn_cast(y));
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning:  {{.*dyn_cast<> not used .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (isa(y));
+
+  if (y && cast(y))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning:  {{.*use dyn_cast_or_null .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (dyn_cast_or_null(y))
+
+  Z z;
+  if (z.bar() && cast(z.bar()))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning:  {{.*use dyn_cast_or_null .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (dyn_cast_or_null(z.bar()))
+
+  bool b = y && cast(y);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning:  {{.*use dyn_cast_or_null .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: bool b = dyn_cast_or_null(y);
+
+  // These don't trigger a warning.
+  if (y && cast(z.bar())) {
+return true;
+  }
+  bool b2 = y && cast();
+
+  return false;
+}
Index: clang-tools-extra/docs/clang-tidy/checks/llvm-avoid-cast-in-conditional.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/llvm-avoid-cast-in-conditional.rst
@@ -0,0 +1,28 @@
+.. title:: clang-tidy - llvm-avoid-cast-in-conditional
+
+llvm-avoid-cast-in-conditional
+==
+
+  Finds uses of ``cast<>`` in conditionals of ``if``, ``while`` or
+  ``do`` statements, which will assert rather than return a null
+  pointer. It 

[PATCH] D59802: [clang-tidy] Add new checker: llvm-avoid-cast-in-conditional

[Don Hinton via Phabricator via cfe-commits]

hintonda updated this revision to Diff 192754.
hintonda added a comment.

- Add replacement of `y && cast(y)` with `dyn_cast_or_null(y)`, which is 
at least as efficient, and can be a big win if `y` is a function call.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59802

Files:
  clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.cpp
  clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.h
  clang-tools-extra/clang-tidy/llvm/CMakeLists.txt
  clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
  clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/llvm-avoid-cast-in-conditional.rst
  clang-tools-extra/test/clang-tidy/llvm-avoid-cast-in-conditional.cpp
  clang/include/clang/Basic/LLVM.h

Index: clang/include/clang/Basic/LLVM.h
===
--- clang/include/clang/Basic/LLVM.h
+++ clang/include/clang/Basic/LLVM.h
@@ -31,6 +31,7 @@
   template class ArrayRef;
   template class MutableArrayRef;
   template class OwningArrayRef;
+  template  class SmallSet;
   template class SmallString;
   template class SmallVector;
   template class SmallVectorImpl;
@@ -66,6 +67,7 @@
   using llvm::Optional;
   using llvm::OwningArrayRef;
   using llvm::SaveAndRestore;
+  using llvm::SmallSet;
   using llvm::SmallString;
   using llvm::SmallVector;
   using llvm::SmallVectorImpl;
Index: clang-tools-extra/test/clang-tidy/llvm-avoid-cast-in-conditional.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/llvm-avoid-cast-in-conditional.cpp
@@ -0,0 +1,96 @@
+// RUN: %check_clang_tidy %s llvm-avoid-cast-in-conditional %t
+
+struct X;
+struct Y;
+struct Z {
+  int foo();
+  X *bar();
+};
+
+template 
+bool isa(Y *);
+template 
+X *cast(Y *);
+template 
+X *dyn_cast(Y *);
+template 
+X *dyn_cast_or_null(Y *);
+
+bool foo(Y *y) {
+  if (auto x = cast(y))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:16: warning: {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (auto x = dyn_cast(y))
+
+  if (cast(y))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning:  {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (isa(y))
+
+  if (dyn_cast(y))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning:  {{.*dyn_cast<> not used .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (isa(y))
+
+  // These don't trigger a warning.
+  if (auto x = cast(y)->foo())
+return true;
+  if (cast(y)->foo())
+return true;
+
+  while (auto x = cast(y))
+break;
+  // CHECK-MESSAGES: :[[@LINE-2]]:19: warning:  {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (auto x = dyn_cast(y))
+
+  while (cast(y))
+break;
+  // CHECK-MESSAGES: :[[@LINE-2]]:10: warning:  {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (isa(y))
+
+  while (dyn_cast(y))
+break;
+  // CHECK-MESSAGES: :[[@LINE-2]]:10: warning:  {{.*dyn_cast<> not used .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (isa(y))
+
+  // These don't trigger a warning.
+  while (auto x = cast(y)->foo())
+break;
+  while (cast(y)->foo())
+break;
+
+  do {
+break;
+  } while (cast(y));
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning:  {{cast<> in conditional .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (isa(y));
+
+  do {
+break;
+  } while (dyn_cast(y));
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning:  {{.*dyn_cast<> not used .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: while (isa(y));
+
+  if (y && cast(y))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning:  {{.*use dyn_cast_or_null .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (dyn_cast_or_null(y))
+
+  Z z;
+  if (z.bar() && cast(z.bar()))
+return true;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning:  {{.*use dyn_cast_or_null .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: if (dyn_cast_or_null(z.bar()))
+
+  bool b = y && cast(y);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning:  {{.*use dyn_cast_or_null .*llvm-avoid-cast-in-conditional}}
+  // CHECK-FIXES: bool b = dyn_cast_or_null(y);
+
+  // These don't trigger a warning.
+  if (y && cast(z.bar())) {
+return true;
+  }
+  bool b2 = y && cast();
+
+  return false;
+}
Index: clang-tools-extra/docs/clang-tidy/checks/llvm-avoid-cast-in-conditional.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/llvm-avoid-cast-in-conditional.rst
@@ -0,0 +1,28 @@
+.. title:: clang-tidy - llvm-avoid-cast-in-conditional
+
+llvm-avoid-cast-in-conditional
+==
+
+  Finds uses of ``cast<>`` in 

[PATCH] D59407: [clangd] Add RelationSlab

[Nathan Ridge via Phabricator via cfe-commits]

nridge added a comment.

@sammccall, thank you for having a look at this.

I have no objection to revising the data model if there's agreement on a better 
one.

In D59407#1446464 , @sammccall wrote:

> - I don't think we yet know what the more resource-critical (denser) 
> relations and queries are, so it's unclear what to optimize for


Based on a brief mental survey of C++ IDE features I'm familiar with, I can 
think of the following additional uses of the relations capability:

- A call hierarchy feature (which is also proposed for LSP 
, with client 
 and server 
 
implementation efforts) would need every caller-callee relationship to be 
recorded in the index (`RelationCalledBy`).
- Given a virtual method declaration, a user may want to see a list of 
implementations (overriders) and navigate to one or more of them. This would 
need every overrider relationship to be recorded in the index 
(`RelationOverrideOf`).

Intuitively, it seems like `RelationOverrideOf` would be slightly denser than 
`RelationChildOf` (though not by much), while `RelationCalledBy` would be 
significantly denser. In terms of queries, I believe the key for lookups for 
both of the above would be a (subject, predicate) pair, just like for subtypes.

Does that change your analysis at all?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59407



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

[PATCH] D59797: [COFF] Reorder fields in Chunk and SectionChunk to reduce their size

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

I added one last size optimization, replacing a std::vector with a forward 
linked list and a custom iterator for it. PTAL


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59797



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

[PATCH] D59797: [COFF] Reorder fields in Chunk and SectionChunk to reduce their size

[Reid Kleckner via Phabricator via cfe-commits]

rnk updated this revision to Diff 192752.
rnk added a comment.

- Replace std::vector<> with singly linked list


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59797

Files:
  lld/COFF/Chunks.cpp
  lld/COFF/Chunks.h
  lld/COFF/ICF.cpp
  lld/COFF/MarkLive.cpp
  llvm/include/llvm/BinaryFormat/COFF.h

Index: llvm/include/llvm/BinaryFormat/COFF.h
===
--- llvm/include/llvm/BinaryFormat/COFF.h
+++ llvm/include/llvm/BinaryFormat/COFF.h
@@ -402,7 +402,7 @@
   IMAGE_REL_ARM64_REL32 = 0x0011,
 };
 
-enum COMDATType : unsigned {
+enum COMDATType : uint8_t {
   IMAGE_COMDAT_SELECT_NODUPLICATES = 1,
   IMAGE_COMDAT_SELECT_ANY,
   IMAGE_COMDAT_SELECT_SAME_SIZE,
Index: lld/COFF/MarkLive.cpp
===
--- lld/COFF/MarkLive.cpp
+++ lld/COFF/MarkLive.cpp
@@ -64,8 +64,8 @@
 AddSym(B);
 
 // Mark associative sections if any.
-for (SectionChunk *C : SC->children())
-  Enqueue(C);
+for (SectionChunk  : SC->children())
+  Enqueue();
   }
 }
 
Index: lld/COFF/ICF.cpp
===
--- lld/COFF/ICF.cpp
+++ lld/COFF/ICF.cpp
@@ -129,10 +129,10 @@
 bool ICF::assocEquals(const SectionChunk *A, const SectionChunk *B) {
   auto ChildClasses = [&](const SectionChunk *SC) {
 std::vector Classes;
-for (const SectionChunk *C : SC->children())
-  if (!C->SectionName.startswith(".debug") &&
-  C->SectionName != ".gfids$y" && C->SectionName != ".gljmp$y")
-Classes.push_back(C->Class[Cnt % 2]);
+for (const SectionChunk  : SC->children())
+  if (!C.SectionName.startswith(".debug") &&
+  C.SectionName != ".gfids$y" && C.SectionName != ".gljmp$y")
+Classes.push_back(C.Class[Cnt % 2]);
 return Classes;
   };
   return ChildClasses(A) == ChildClasses(B);
Index: lld/COFF/Chunks.h
===
--- lld/COFF/Chunks.h
+++ lld/COFF/Chunks.h
@@ -50,7 +50,7 @@
 // doesn't even have actual data (if common or bss).
 class Chunk {
 public:
-  enum Kind { SectionKind, OtherKind };
+  enum Kind : uint8_t { SectionKind, OtherKind };
   Kind kind() const { return ChunkKind; }
   virtual ~Chunk() = default;
 
@@ -107,6 +107,12 @@
   Chunk(Kind K = OtherKind) : ChunkKind(K) {}
   const Kind ChunkKind;
 
+public:
+  // Whether this section needs to be kept distinct from other sections during
+  // ICF. This is set by the driver using address-significance tables.
+  bool KeepUnique = false;
+
+protected:
   // The RVA of this chunk in the output. The writer sets a value.
   uint64_t RVA = 0;
 
@@ -116,10 +122,6 @@
 public:
   // The offset from beginning of the output section. The writer sets a value.
   uint64_t OutputSectionOff = 0;
-
-  // Whether this section needs to be kept distinct from other sections during
-  // ICF. This is set by the driver using address-significance tables.
-  bool KeepUnique = false;
 };
 
 // A chunk corresponding a section of an input file.
@@ -192,8 +194,34 @@
 symbol_iterator(File, Relocs.end()));
   }
 
+  // Single linked list iterator for associated comdat children.
+  class AssociatedIterator
+  : public llvm::iterator_facade_base<
+AssociatedIterator, std::forward_iterator_tag, SectionChunk> {
+  public:
+AssociatedIterator() = default;
+AssociatedIterator(SectionChunk *Head) : Cur(Head) {}
+AssociatedIterator =(const AssociatedIterator ) {
+  Cur = R.Cur;
+  return *this;
+}
+bool operator==(const AssociatedIterator ) const { return Cur == R.Cur; }
+const SectionChunk *() const { return *Cur; }
+SectionChunk *() { return *Cur; }
+AssociatedIterator ++() {
+  Cur = Cur->AssocChildren;
+  return *this;
+}
+
+  private:
+SectionChunk *Cur = nullptr;
+  };
+
   // Allow iteration over the associated child chunks for this section.
-  ArrayRef children() const { return AssocChildren; }
+  llvm::iterator_range children() const {
+return llvm::make_range(AssociatedIterator(AssocChildren),
+AssociatedIterator(nullptr));
+  }
 
   // The section ID this chunk belongs to in its Obj.
   uint32_t getSectionNumber() const;
@@ -208,35 +236,37 @@
 
   bool isHotPatchable() const override { return File->HotPatchable; }
 
-  // A pointer pointing to a replacement for this chunk.
-  // Initially it points to "this" object. If this chunk is merged
-  // with other chunk by ICF, it points to another chunk,
-  // and this chunk is considered as dead.
-  SectionChunk *Repl;
-
-  // The CRC of the contents as described in the COFF spec 4.5.5.
-  // Auxiliary Format 5: Section Definitions. Used for ICF.
-  uint32_t Checksum = 0;
-
-  const coff_section *Header;
-
   // The file that this chunk was created from.
   ObjFile *File;
 

[PATCH] D53076: [analyzer] ConditionBRVisitor: enhance to write out more information

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Instead of having those as events similar to "Assuming", we could turn the new 
"Knowing" pieces into floating pop-ups - imagine you hover the mouse over a 
condition `foo()` and it says "`foo()` is false". That is, instead of 
`PathDiagnosticsEventPiece`, a new kind of piece can be introduced that is 
shown in scan-build as a pop-up similar to macro pop-ups. Like this:

F8556875: mock.png 

We already have infrastructure for hover pop-up hints in HTML reports - we use 
it to display macro expansions, so it could be something similar, just of a 
different color.


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

https://reviews.llvm.org/D53076



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

[PATCH] D59761: [ASTImporter] Convert ODR diagnostics inside ASTImporter implementation

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik accepted this revision.
shafik added a comment.

LGTM


Repository:
  rC Clang

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

https://reviews.llvm.org/D59761



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

r357229 - [CodeGen][ObjC] Adjust the addresses passed to calls to synthesized

[Akira Hatanaka via cfe-commits]

Author: ahatanak
Date: Thu Mar 28 17:23:20 2019
New Revision: 357229

URL: http://llvm.org/viewvc/llvm-project?rev=357229=rev
Log:
[CodeGen][ObjC] Adjust the addresses passed to calls to synthesized
copy/move constructor/assignment operator functions for non-trivial C
structs.

This commit fixes a bug where the offset of struct fields weren't being
taken into account when computing the addresses passed to calls to the
special functions.

For example, the copy constructor for S1 (__copy_constructor_8_8_s0_s8)
would pass the start addresses of the destination and source structs to
the call to S0's copy constructor (_copy_constructor_8_8_s0) without
adding the offset of field f1 to the addresses.

typedef struct {
  id f0;
  S0 f1;
} S1;

void test(S1 s1) {
  S1 t = s1;
}

rdar://problem/49400610

Modified:
cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp
cfe/trunk/test/CodeGenObjC/strong-in-c-struct.m

Modified: cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp?rev=357229=357228=357229=diff
==
--- cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp Thu Mar 28 17:23:20 2019
@@ -688,6 +688,8 @@ struct GenCopyConstructor : GenBinaryFun
 
   void callSpecialFunction(QualType FT, CharUnits Offset,
std::array Addrs) {
+Addrs[DstIdx] = getAddrWithOffset(Addrs[DstIdx], Offset);
+Addrs[SrcIdx] = getAddrWithOffset(Addrs[SrcIdx], Offset);
 CGF->callCStructCopyConstructor(CGF->MakeAddrLValue(Addrs[DstIdx], FT),
 CGF->MakeAddrLValue(Addrs[SrcIdx], FT));
   }
@@ -718,6 +720,8 @@ struct GenMoveConstructor : GenBinaryFun
 
   void callSpecialFunction(QualType FT, CharUnits Offset,
std::array Addrs) {
+Addrs[DstIdx] = getAddrWithOffset(Addrs[DstIdx], Offset);
+Addrs[SrcIdx] = getAddrWithOffset(Addrs[SrcIdx], Offset);
 CGF->callCStructMoveConstructor(CGF->MakeAddrLValue(Addrs[DstIdx], FT),
 CGF->MakeAddrLValue(Addrs[SrcIdx], FT));
   }
@@ -746,6 +750,8 @@ struct GenCopyAssignment : GenBinaryFunc
 
   void callSpecialFunction(QualType FT, CharUnits Offset,
std::array Addrs) {
+Addrs[DstIdx] = getAddrWithOffset(Addrs[DstIdx], Offset);
+Addrs[SrcIdx] = getAddrWithOffset(Addrs[SrcIdx], Offset);
 CGF->callCStructCopyAssignmentOperator(
 CGF->MakeAddrLValue(Addrs[DstIdx], FT),
 CGF->MakeAddrLValue(Addrs[SrcIdx], FT));
@@ -780,6 +786,8 @@ struct GenMoveAssignment : GenBinaryFunc
 
   void callSpecialFunction(QualType FT, CharUnits Offset,
std::array Addrs) {
+Addrs[DstIdx] = getAddrWithOffset(Addrs[DstIdx], Offset);
+Addrs[SrcIdx] = getAddrWithOffset(Addrs[SrcIdx], Offset);
 CGF->callCStructMoveAssignmentOperator(
 CGF->MakeAddrLValue(Addrs[DstIdx], FT),
 CGF->MakeAddrLValue(Addrs[SrcIdx], FT));

Modified: cfe/trunk/test/CodeGenObjC/strong-in-c-struct.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjC/strong-in-c-struct.m?rev=357229=357228=357229=diff
==
--- cfe/trunk/test/CodeGenObjC/strong-in-c-struct.m (original)
+++ cfe/trunk/test/CodeGenObjC/strong-in-c-struct.m Thu Mar 28 17:23:20 2019
@@ -30,6 +30,11 @@ typedef struct {
 } StrongOuter;
 
 typedef struct {
+  id f0;
+  Strong f1;
+} StrongOuter2;
+
+typedef struct {
   int f0;
   volatile id f1;
 } StrongVolatile;
@@ -81,6 +86,7 @@ typedef struct {
 
 StrongSmall getStrongSmall(void);
 StrongOuter getStrongOuter(void);
+StrongOuter2 getStrongOuter2(void);
 void calleeStrongSmall(StrongSmall);
 void func(Strong *);
 
@@ -289,6 +295,121 @@ void test_move_assignment_StrongOuter(St
   *p = getStrongOuter();
 }
 
+// CHECK: define linkonce_odr hidden void 
@__default_constructor_8_s0_S_s24(i8** %[[DST:.*]])
+// CHECK: %[[DST_ADDR:.*]] = alloca i8**, align 8
+// CHECK: store i8** %[[DST]], i8*** %[[DST_ADDR]], align 8
+// CHECK: %[[V0:.*]] = load i8**, i8*** %[[DST_ADDR]], align 8
+// CHECK: %[[V1:.*]] = bitcast i8** %[[V0]] to i8*
+// CHECK: call void @llvm.memset.p0i8.i64(i8* align 8 %[[V1]], i8 0, i64 8, i1 
false)
+// CHECK: %[[V2:.*]] = bitcast i8** %[[V0]] to i8*
+// CHECK: %[[V3:.*]] = getelementptr inbounds i8, i8* %[[V2]], i64 8
+// CHECK: %[[V4:.*]] = bitcast i8* %[[V3]] to i8**
+// CHECK: call void @__default_constructor_8_s16(i8** %[[V4]])
+
+// CHECK: define linkonce_odr hidden void @__destructor_8_s0_S_s24(i8** 
%[[DST:.*]])
+// CHECK: %[[DST_ADDR:.*]] = alloca i8**, align 8
+// CHECK: store i8** %[[DST]], i8*** %[[DST_ADDR]], align 8
+// CHECK: %[[V0:.*]] = load i8**, i8*** %[[DST_ADDR]], align 8
+// CHECK: call void @llvm.objc.storeStrong(i8** %[[V0]], i8* null)
+// CHECK: %[[V1:.*]] = bitcast i8** 

r357228 - Fix typos and formatting. NFC.

[Akira Hatanaka via cfe-commits]

Author: ahatanak
Date: Thu Mar 28 17:23:17 2019
New Revision: 357228

URL: http://llvm.org/viewvc/llvm-project?rev=357228=rev
Log:
Fix typos and formatting. NFC.

Modified:
cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp

Modified: cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp?rev=357228=357227=357228=diff
==
--- cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp Thu Mar 28 17:23:17 2019
@@ -83,23 +83,22 @@ struct CopyStructVisitor : StructVisitor
 
   template 
   void preVisit(QualType::PrimitiveCopyKind PCK, QualType FT,
-const FieldDecl *FD, CharUnits CurStructOffsset,
-Ts &&... Args) {
+const FieldDecl *FD, CharUnits CurStructOffset, Ts &&... Args) 
{
 if (PCK)
   asDerived().flushTrivialFields(std::forward(Args)...);
   }
 
   template 
   void visitWithKind(QualType::PrimitiveCopyKind PCK, QualType FT,
- const FieldDecl *FD, CharUnits CurStructOffsset,
+ const FieldDecl *FD, CharUnits CurStructOffset,
  Ts &&... Args) {
 if (const auto *AT = asDerived().getContext().getAsArrayType(FT)) {
   asDerived().visitArray(PCK, AT, FT.isVolatileQualified(), FD,
- CurStructOffsset, std::forward(Args)...);
+ CurStructOffset, std::forward(Args)...);
   return;
 }
 
-Super::visitWithKind(PCK, FT, FD, CurStructOffsset,
+Super::visitWithKind(PCK, FT, FD, CurStructOffset,
  std::forward(Args)...);
   }
 
@@ -253,11 +252,11 @@ struct GenBinaryFuncName : CopyStructVis
   }
 
   void visitVolatileTrivial(QualType FT, const FieldDecl *FD,
-CharUnits CurStackOffset) {
+CharUnits CurStructOffset) {
 // Because volatile fields can be bit-fields and are individually copied,
 // their offset and width are in bits.
 uint64_t OffsetInBits =
-this->Ctx.toBits(CurStackOffset) + this->getFieldOffsetInBits(FD);
+this->Ctx.toBits(CurStructOffset) + this->getFieldOffsetInBits(FD);
 this->appendStr("_tv" + llvm::to_string(OffsetInBits) + "w" +
 llvm::to_string(getFieldSize(FD, FT, this->Ctx)));
   }
@@ -286,8 +285,7 @@ struct GenDestructorFuncName : GenUnaryF
   using Super = DestructedTypeVisitor;
   GenDestructorFuncName(const char *Prefix, CharUnits DstAlignment,
 ASTContext )
-  : GenUnaryFuncName(Prefix, DstAlignment,
-Ctx) {}
+  : GenUnaryFuncName(Prefix, DstAlignment, Ctx) {}
   void visitWithKind(QualType::DestructionKind DK, QualType FT,
  const FieldDecl *FD, CharUnits CurStructOffset) {
 if (const auto *AT = getContext().getAsArrayType(FT)) {
@@ -322,19 +320,19 @@ static const CGFunctionInfo 
 // functions.
 template  struct GenFuncBase {
   template 
-  void visitStruct(QualType FT, const FieldDecl *FD, CharUnits CurStackOffset,
+  void visitStruct(QualType FT, const FieldDecl *FD, CharUnits CurStructOffset,
std::array Addrs) {
 this->asDerived().callSpecialFunction(
-FT, CurStackOffset + asDerived().getFieldOffset(FD), Addrs);
+FT, CurStructOffset + asDerived().getFieldOffset(FD), Addrs);
   }
 
   template 
   void visitArray(FieldKind FK, const ArrayType *AT, bool IsVolatile,
-  const FieldDecl *FD, CharUnits CurStackOffset,
+  const FieldDecl *FD, CharUnits CurStructOffset,
   std::array Addrs) {
 // Non-volatile trivial fields are copied when flushTrivialFields is 
called.
 if (!FK)
-  return asDerived().visitTrivial(QualType(AT, 0), FD, CurStackOffset,
+  return asDerived().visitTrivial(QualType(AT, 0), FD, CurStructOffset,
   Addrs);
 
 asDerived().flushTrivialFields(Addrs);
@@ -345,7 +343,7 @@ template  struct GenFuncB
 QualType BaseEltQT;
 std::array StartAddrs = Addrs;
 for (unsigned I = 0; I < N; ++I)
-  StartAddrs[I] = getAddrWithOffset(Addrs[I], CurStackOffset, FD);
+  StartAddrs[I] = getAddrWithOffset(Addrs[I], CurStructOffset, FD);
 Address DstAddr = StartAddrs[DstIdx];
 llvm::Value *NumElts = CGF.emitArrayLength(AT, BaseEltQT, DstAddr);
 unsigned BaseEltSize = Ctx.getTypeSizeInChars(BaseEltQT).getQuantity();
@@ -585,15 +583,15 @@ struct GenDestructor : StructVisitor Addrs) {
+  CharUnits CurStructOffset, std::array Addrs) 
{
 CGF->destroyARCStrongImprecise(
-*CGF, getAddrWithOffset(Addrs[DstIdx], CurStackOffset, FD), QT);
+*CGF, getAddrWithOffset(Addrs[DstIdx], CurStructOffset, FD), QT);
   }
 
-  void visitARCWeak(QualType QT, const FieldDecl *FD, 

[PATCH] D59922: [Attributor] Deduce "no-capture" argument attribute

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert marked 2 inline comments as done.
jdoerfert added inline comments.



Comment at: llvm/test/Transforms/FunctionAttrs/arg_nocapture.ll:103
-; FIXME: returned missing for %a
 ; FIXME: We should *not* derive any attributes for the return value not 
present on the argument!
 ; CHECK: define dso_local noalias nonnull i32* @srec16(i32* nocapture readnone 
%a)

The comments on this one were off from the very beginning, I'll fix them and 
there won't be a change during this commit.



Comment at: llvm/test/Transforms/FunctionAttrs/nocapture.ll:137
 
-; CHECK: define void @test1_1(i8* nocapture readnone %x1_1, i8* %y1_1)
 ; It would be acceptable to add readnone to %y1_1 and %y1_2.

So, the old FuncAttr deduction interleaves one of two memory behavior 
deductions with the capture analysis. With this patch capture analysis becomes 
obsolete, as the attributor added annotations, and certain memory behavior 
attributes are not detected anymore. There will be a memory behavior detection 
for the attributor soon which should make this problem go away.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59922



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

[PATCH] D59919: [Attributor] Deduce "returned" argument attribute

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert updated this revision to Diff 192738.
jdoerfert added a comment.

Fix the last bug exposed by llvm-test-suite & SPEC2006


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59919

Files:
  clang/test/CodeGenOpenCL/as_type.cl
  llvm/include/llvm/Transforms/IPO/Attributor.h
  llvm/lib/Transforms/IPO/Attributor.cpp
  llvm/test/Transforms/FunctionAttrs/SCC1.ll
  llvm/test/Transforms/FunctionAttrs/arg_nocapture.ll
  llvm/test/Transforms/FunctionAttrs/arg_returned.ll

Index: llvm/test/Transforms/FunctionAttrs/arg_returned.ll
===
--- llvm/test/Transforms/FunctionAttrs/arg_returned.ll
+++ llvm/test/Transforms/FunctionAttrs/arg_returned.ll
@@ -1,28 +1,43 @@
-; RUN: opt -functionattrs -attributor -S < %s | FileCheck %s
+; RUN: opt -functionattrs -S < %s | FileCheck %s --check-prefix=FNATTR
+; RUN: opt -attributor -S < %s | FileCheck %s --check-prefix=ATTRIBUTOR
+; RUN: opt -attributor -functionattrs -S < %s | FileCheck %s --check-prefix=BOTH
+; RUN: opt -attributor -attributor-max-iterations=18 -S < %s | FileCheck %s --check-prefix=FEW_IT
+; RUN: opt -attributor -attributor-max-iterations=19 -functionattrs -S < %s | FileCheck %s --check-prefix=BOTH
 ;
 ; Test cases specifically designed for the "returned" argument attribute.
 ; We use FIXME's to indicate problems and missing attributes.
 ;
-; TEST 1: SCC test returning an integer value argument
-; TEST 2: the same SCC as in 1 returning a pointer value argument
-; TEST 3: a singleton SCC with a lot of recursive calls
-; TEST 4: address taken function with call to an external functions
-; TEST 5: call to a function that might be redifined at link time
-; TEST 6: returned argument goes through select and phi
-; TEST 7: returned argument goes through recursion, select, and phi
-; TEST 8: returned argument goes through bitcasts
-; TEST 9: returned argument goes through select and phi interleaved with bitcasts
+; TEST  1: SCC test returning an integer value argument
+; TEST  2: the same SCC as in 1 returning a pointer value argument
+; TEST  3: a singleton SCC with a lot of recursive calls
+; TEST  4: address taken function with call to an external functions
+; TEST  5: call to a function that might be redifined at link time
+; TEST  6: returned argument goes through select and phi
+; TEST  7: returned argument goes through recursion, select, and phi
+; TEST  8: returned argument goes through bitcasts
+; TEST  9: returned argument goes through select and phi interleaved with bitcasts
+; TEST 10: return argument or argument or undef
+; TEST 11: return undef or argument or argument
+; TEST 12: return undef or argument or undef
+; TEST 13: return argument or unknown call result
 
 
 ; TEST 1
 ;
-; CHECK: define dso_local i32 @sink_r0(i32 returned %r) [[NoInlineNoRecurseNoUnwindReadnoneUwtable:#[0-9]]]
+; BOTH: define dso_local i32 @sink_r0(i32 returned %r) [[NoInlineNoRecurseNoUnwindReadnoneUwtable:#[0-9]]]
+; BOTH: define dso_local i32 @scc_r1(i32 %a, i32 returned %r, i32 %b) [[NoInlineNoUnwindReadnoneUwtable:#[0-9]]]
+; BOTH: define dso_local i32 @scc_r2(i32 %a, i32 %b, i32 returned %r) [[NoInlineNoUnwindReadnoneUwtable]]
+; BOTH: define dso_local i32 @scc_rX(i32 %a, i32 %b, i32 %r) [[NoInlineNoUnwindReadnoneUwtable]]
 ;
-; FIXME: returned on %r missing:
-; CHECK: define dso_local i32 @scc_r1(i32 %a, i32 %r, i32 %b) [[NoInlineNoUnwindReadnoneUwtable:#[0-9]]]
+; FNATTR: define dso_local i32 @sink_r0(i32 returned %r) [[NoInlineNoRecurseNoUnwindReadnoneUwtable:#[0-9]]]
+; FNATTR: define dso_local i32 @scc_r1(i32 %a, i32 %r, i32 %b) [[NoInlineNoUnwindReadnoneUwtable:#[0-9]]]
+; FNATTR: define dso_local i32 @scc_r2(i32 %a, i32 %b, i32 %r) [[NoInlineNoUnwindReadnoneUwtable]]
+; FNATTR: define dso_local i32 @scc_rX(i32 %a, i32 %b, i32 %r) [[NoInlineNoUnwindReadnoneUwtable]]
 ;
-; FIXME: returned on %r missing:
-; CHECK: define dso_local i32 @scc_r2(i32 %a, i32 %b, i32 %r) [[NoInlineNoUnwindReadnoneUwtable]]
+; ATTRIBUTOR: define dso_local i32 @sink_r0(i32 returned %r) [[NoInlineNoRecurseNoUnwindReadnoneUwtable:#[0-9]]]
+; ATTRIBUTOR: define dso_local i32 @scc_r1(i32 %a, i32 returned %r, i32 %b) [[NoInlineNoUnwindReadnoneUwtable:#[0-9]]]
+; ATTRIBUTOR: define dso_local i32 @scc_r2(i32 %a, i32 %b, i32 returned %r) [[NoInlineNoUnwindReadnoneUwtable]]
+; ATTRIBUTOR: define dso_local i32 @scc_rX(i32 %a, i32 %b, i32 %r) [[NoInlineNoUnwindReadnoneUwtable]]
 ;
 ; int scc_r1(int a, int b, int r);
 ; int scc_r2(int a, int b, int r);
@@ -157,13 +172,17 @@
 
 ; TEST 2
 ;
-; CHECK: define dso_local double* @ptr_sink_r0(double* readnone returned %r) [[NoInlineNoRecurseNoUnwindReadnoneUwtable]]
+; BOTH: define dso_local double* @ptr_sink_r0(double* readnone returned %r) [[NoInlineNoRecurseNoUnwindReadnoneUwtable]]
+; BOTH: define dso_local double* @ptr_scc_r1(double* %a, double* readnone returned %r, double* nocapture readnone %b) 

[PATCH] D59485: [ASTImporter] Add an ImportInternal method to allow customizing Import behavior.

[Aleksei Sidorin via Phabricator via cfe-commits]

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

Hello Raphael,
I think we should accept this change. I don't see an easy way to merge the LLDB 
stuff into ASTImporter; also, this patch provides a good extension point for 
ASTImporter since it is designed to be a parent class. @martong @shafik Gabor, 
Shafik, what do you think?


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

https://reviews.llvm.org/D59485



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

[PATCH] D59761: [ASTImporter] Convert ODR diagnostics inside ASTImporter implementation

[Aleksei Sidorin via Phabricator via cfe-commits]

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

Yes, I think this is fine. Thanks!


Repository:
  rC Clang

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

https://reviews.llvm.org/D59761



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

r357220 - [MS] Make __iso_volatile_* available on all targets

[Reid Kleckner via cfe-commits]

Author: rnk
Date: Thu Mar 28 15:59:09 2019
New Revision: 357220

URL: http://llvm.org/viewvc/llvm-project?rev=357220=rev
Log:
[MS] Make __iso_volatile_* available on all targets

Future versions of MSVC make these intrinsics available on x86 & x64,
according to:
http://lists.llvm.org/pipermail/cfe-dev/2019-March/061711.html

The purpose of these builtins is to emit plain, non-atomic, volatile
stores when /volatile:ms (-cc1 -fms-volatile) is enabled.

Removed:
cfe/trunk/test/CodeGen/ms-volatile-aarch64.c
cfe/trunk/test/CodeGen/ms-volatile-arm.c
Modified:
cfe/trunk/include/clang/Basic/Builtins.def
cfe/trunk/include/clang/Basic/BuiltinsAArch64.def
cfe/trunk/include/clang/Basic/BuiltinsARM.def
cfe/trunk/lib/CodeGen/CGBuiltin.cpp
cfe/trunk/lib/CodeGen/CodeGenFunction.h
cfe/trunk/test/CodeGen/ms-intrinsics.c

Modified: cfe/trunk/include/clang/Basic/Builtins.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Builtins.def?rev=357220=357219=357220=diff
==
--- cfe/trunk/include/clang/Basic/Builtins.def (original)
+++ cfe/trunk/include/clang/Basic/Builtins.def Thu Mar 28 15:59:09 2019
@@ -820,6 +820,14 @@ LANGBUILTIN(_interlockedbittestandset64,
 LANGBUILTIN(_interlockedbittestandset_acq,   "UcNiD*Ni", "n", ALL_MS_LANGUAGES)
 LANGBUILTIN(_interlockedbittestandset_nf,"UcNiD*Ni", "n", ALL_MS_LANGUAGES)
 LANGBUILTIN(_interlockedbittestandset_rel,   "UcNiD*Ni", "n", ALL_MS_LANGUAGES)
+LANGBUILTIN(__iso_volatile_load8,   "ccCD*", "n", ALL_MS_LANGUAGES)
+LANGBUILTIN(__iso_volatile_load16,  "ssCD*", "n", ALL_MS_LANGUAGES)
+LANGBUILTIN(__iso_volatile_load32,  "iiCD*", "n", ALL_MS_LANGUAGES)
+LANGBUILTIN(__iso_volatile_load64,  "LLiLLiCD*", "n", ALL_MS_LANGUAGES)
+LANGBUILTIN(__iso_volatile_store8,  "vcD*c", "n", ALL_MS_LANGUAGES)
+LANGBUILTIN(__iso_volatile_store16, "vsD*s", "n", ALL_MS_LANGUAGES)
+LANGBUILTIN(__iso_volatile_store32, "viD*i", "n", ALL_MS_LANGUAGES)
+LANGBUILTIN(__iso_volatile_store64, "vLLiD*LLi", "n", ALL_MS_LANGUAGES)
 LANGBUILTIN(__noop,   "i.",  "n", ALL_MS_LANGUAGES)
 LANGBUILTIN(__lzcnt16, "UsUs","nc", ALL_MS_LANGUAGES)
 LANGBUILTIN(__lzcnt,   "UiUi","nc", ALL_MS_LANGUAGES)

Modified: cfe/trunk/include/clang/Basic/BuiltinsAArch64.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsAArch64.def?rev=357220=357219=357220=diff
==
--- cfe/trunk/include/clang/Basic/BuiltinsAArch64.def (original)
+++ cfe/trunk/include/clang/Basic/BuiltinsAArch64.def Thu Mar 28 15:59:09 2019
@@ -78,16 +78,6 @@ LANGBUILTIN(__wfi,   "v", "",   ALL_MS_L
 LANGBUILTIN(__sev,   "v", "",   ALL_MS_LANGUAGES)
 LANGBUILTIN(__sevl,  "v", "",   ALL_MS_LANGUAGES)
 
-// MSVC intrinsics for volatile but non-acquire/release loads and stores
-LANGBUILTIN(__iso_volatile_load8,   "ccCD*", "n", ALL_MS_LANGUAGES)
-LANGBUILTIN(__iso_volatile_load16,  "ssCD*", "n", ALL_MS_LANGUAGES)
-LANGBUILTIN(__iso_volatile_load32,  "iiCD*", "n", ALL_MS_LANGUAGES)
-LANGBUILTIN(__iso_volatile_load64,  "LLiLLiCD*", "n", ALL_MS_LANGUAGES)
-LANGBUILTIN(__iso_volatile_store8,  "vcD*c", "n", ALL_MS_LANGUAGES)
-LANGBUILTIN(__iso_volatile_store16, "vsD*s", "n", ALL_MS_LANGUAGES)
-LANGBUILTIN(__iso_volatile_store32, "viD*i", "n", ALL_MS_LANGUAGES)
-LANGBUILTIN(__iso_volatile_store64, "vLLiD*LLi", "n", ALL_MS_LANGUAGES)
-
 TARGET_HEADER_BUILTIN(_BitScanForward, "UcUNi*UNi", "nh", "intrin.h", 
ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_BitScanReverse, "UcUNi*UNi", "nh", "intrin.h", 
ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_BitScanForward64, "UcUNi*ULLi", "nh", "intrin.h", 
ALL_MS_LANGUAGES, "")

Modified: cfe/trunk/include/clang/Basic/BuiltinsARM.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsARM.def?rev=357220=357219=357220=diff
==
--- cfe/trunk/include/clang/Basic/BuiltinsARM.def (original)
+++ cfe/trunk/include/clang/Basic/BuiltinsARM.def Thu Mar 28 15:59:09 2019
@@ -201,14 +201,6 @@ LANGBUILTIN(__sevl, "v", "", ALL_MS_LANG
 LANGBUILTIN(__dmb, "vUi", "nc", ALL_MS_LANGUAGES)
 LANGBUILTIN(__dsb, "vUi", "nc", ALL_MS_LANGUAGES)
 LANGBUILTIN(__isb, "vUi", "nc", ALL_MS_LANGUAGES)
-LANGBUILTIN(__iso_volatile_load8,   "ccCD*", "n", ALL_MS_LANGUAGES)
-LANGBUILTIN(__iso_volatile_load16,  "ssCD*", "n", ALL_MS_LANGUAGES)
-LANGBUILTIN(__iso_volatile_load32,  "iiCD*", "n", ALL_MS_LANGUAGES)
-LANGBUILTIN(__iso_volatile_load64,  "LLiLLiCD*", "n", ALL_MS_LANGUAGES)
-LANGBUILTIN(__iso_volatile_store8,  "vcD*c", "n", ALL_MS_LANGUAGES)
-LANGBUILTIN(__iso_volatile_store16, "vsD*s", "n", ALL_MS_LANGUAGES)
-LANGBUILTIN(__iso_volatile_store32, "viD*i", "n", ALL_MS_LANGUAGES)
-LANGBUILTIN(__iso_volatile_store64, "vLLiD*LLi", "n", ALL_MS_LANGUAGES)
 

[PATCH] D59665: Call to HandleNameConflict in VisitEnumDecl mistakeningly using Name instead of SearchName

[Aleksei Sidorin via Phabricator via cfe-commits]

a_sidorin accepted this revision.
a_sidorin added a comment.

Thanks!


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

https://reviews.llvm.org/D59665



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

[PATCH] D59963: Add a clang-tidy module for the Linux kernel.

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added a comment.

I think linuxkernel name will be less ambiguous.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59963



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

[PATCH] D59665: Call to HandleNameConflict in VisitEnumDecl mistakeningly using Name instead of SearchName

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

In D59665#1446764 , @a_sidorin wrote:

> Hi Shafik,
>  Thank you for the explanation, it is much more clear to me now. But, as I 
> see, D59692  is going to discard the changes 
> this patch introduces. @martong Gabor, do you expect the changes of this 
> patch to be merged into yours, or should this patch be abandoned?


Actually, I'll have to adjust D59692  to not 
discard these changes (otherwise we might end up a regression in lldb).


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

https://reviews.llvm.org/D59665



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

[PATCH] D59449: [clang-tidy] Integrate clang-tidy-diff.py machinery into run-clang-tidy.py

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Can we delete clang-tidy-diff.py with this? (Or delete most of its contents and 
make it forward to this script?)


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59449



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

[PATCH] D59900: [Sema] Fix a crash when nonnull checking

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.



Comment at: clang/test/SemaTemplate/decltype.cpp:1
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
+// no crash & no diag

Rakete wrote:
> test/SemaCXX/nonnull.cpp would be a better place to put this test.
`test/SemaCXX/nonnull.cpp` tests `attribute((nonnull))`. Placing this test 
there would be... odd.

The test could use a more descriptive name, though. Just `decltype` is way too 
generic. `early-func-template-decltype`? Or PR-somthing-something, if there's a 
bug for this crasher.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59900



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

[PATCH] D59963: Add a clang-tidy module for the Linux kernel.

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

Missing `docs/ReleaseNotes.rst` and `docs/clang-tidy/index.rst` changes.




Comment at: clang-tools-extra/clang-tidy/linux/LinuxTidyModule.cpp:13
+
+using namespace clang::ast_matchers;
+

You don't need this here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59963



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

[PATCH] D59963: Add a clang-tidy module for the Linux kernel.

[Tom Roeder via Phabricator via cfe-commits]

tmroeder added reviewers: aaron.ballman, gribozavr.
tmroeder added a comment.

Not sure if you are the right reviewers; I just looked back in the commit 
history to see who reviewed clang-tidy changes recently.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59963



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

[PATCH] D59955: gn build: Add check-clang-tools to run clang-tools-extra lit tests

[Mirko Bonadei via Phabricator via cfe-commits]

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

Really nice work!

LGTM (also patched locally and tested).


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

https://reviews.llvm.org/D59955



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

[PATCH] D59879: [ARM][CMSE] Add commandline option and feature macro

[Todd Snider via Phabricator via cfe-commits]

snidertm added inline comments.



Comment at: lib/Basic/Targets/ARM.cpp:438
+} else if (Feature == "+8msecext") {
+  if ((CPUProfile != "M" && CPUProfile != "B") || ArchVersion != 8) {
+Diags.Report(diag::err_target_unsupported_mcmse) << CPU;

How does CPUProfile get a value of "B"? I thought any Cortex-M processor would 
set CPUProfile to "M". Is CMSE available on a processor besides Cortex-m33?


Repository:
  rC Clang

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

https://reviews.llvm.org/D59879



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

[PATCH] D59963: Add a clang-tidy module for the Linux kernel.

[Tom Roeder via Phabricator via cfe-commits]

tmroeder created this revision.
Herald added subscribers: cfe-commits, jdoerfert, mgorny.
Herald added a project: clang.

Now that clang is going to be able to build the Linux kernel again on
x86, and we have gen_compile_commands.py upstream for generating
compile_commands.json, clang-tidy can be used on the Linux kernel
source.

To that end, this commit adds a new clang-tidy module to be used for
checks specific to Linux kernel source. The Linux kernel follows its own
style of C, and it will be useful to separate those checks into their
own module.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D59963

Files:
  clang-tools-extra/clang-tidy/CMakeLists.txt
  clang-tools-extra/clang-tidy/ClangTidyForceLinker.h
  clang-tools-extra/clang-tidy/linux/CMakeLists.txt
  clang-tools-extra/clang-tidy/linux/LinuxTidyModule.cpp
  clang-tools-extra/clang-tidy/plugin/CMakeLists.txt
  clang-tools-extra/clang-tidy/tool/CMakeLists.txt

Index: clang-tools-extra/clang-tidy/tool/CMakeLists.txt
===
--- clang-tools-extra/clang-tidy/tool/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/tool/CMakeLists.txt
@@ -26,6 +26,7 @@
   clangTidyFuchsiaModule
   clangTidyGoogleModule
   clangTidyHICPPModule
+  clangTidyLinuxModule
   clangTidyLLVMModule
   clangTidyMiscModule
   clangTidyModernizeModule
Index: clang-tools-extra/clang-tidy/plugin/CMakeLists.txt
===
--- clang-tools-extra/clang-tidy/plugin/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/plugin/CMakeLists.txt
@@ -17,6 +17,7 @@
   clangTidyFuchsiaModule
   clangTidyGoogleModule
   clangTidyHICPPModule
+  clangTidyLinuxModule
   clangTidyLLVMModule
   clangTidyMiscModule
   clangTidyModernizeModule
Index: clang-tools-extra/clang-tidy/linux/LinuxTidyModule.cpp
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/linux/LinuxTidyModule.cpp
@@ -0,0 +1,35 @@
+//===--- LinuxTidyModule.cpp - clang-tidy--===//
+//
+// 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 "../ClangTidy.h"
+#include "../ClangTidyModule.h"
+#include "../ClangTidyModuleRegistry.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace linux {
+
+/// This module is for Linux-kernel-specific checks.
+class LinuxModule : public ClangTidyModule {
+public:
+  void addCheckFactories(ClangTidyCheckFactories ) override {
+  }
+};
+// Register the LinuxTidyModule using this statically initialized variable.
+static ClangTidyModuleRegistry::Add
+X("linux-module", "Adds checks specific to the Linux kernel source.");
+} // namespace linux
+
+// This anchor is used to force the linker to link in the generated object file
+// and thus register the LinuxModule.
+volatile int LinuxModuleAnchorSource = 0;
+
+} // namespace tidy
+} // namespace clang
Index: clang-tools-extra/clang-tidy/linux/CMakeLists.txt
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/linux/CMakeLists.txt
@@ -0,0 +1,13 @@
+set(LLVM_LINK_COMPONENTS support)
+
+add_clang_library(clangTidyLinuxModule
+  LinuxTidyModule.cpp
+
+  LINK_LIBS
+  clangAST
+  clangASTMatchers
+  clangBasic
+  clangLex
+  clangTidy
+  clangTidyUtils
+  )
Index: clang-tools-extra/clang-tidy/ClangTidyForceLinker.h
===
--- clang-tools-extra/clang-tidy/ClangTidyForceLinker.h
+++ clang-tools-extra/clang-tidy/ClangTidyForceLinker.h
@@ -35,6 +35,11 @@
 static int LLVM_ATTRIBUTE_UNUSED BugproneModuleAnchorDestination =
 BugproneModuleAnchorSource;
 
+// This anchor is used to force the linker to link the LinuxModule.
+extern volatile int LinuxModuleAnchorSource;
+static int LLVM_ATTRIBUTE_UNUSED LinuxModuleAnchorDestination =
+LinuxModuleAnchorSource;
+
 // This anchor is used to force the linker to link the LLVMModule.
 extern volatile int LLVMModuleAnchorSource;
 static int LLVM_ATTRIBUTE_UNUSED LLVMModuleAnchorDestination =
Index: clang-tools-extra/clang-tidy/CMakeLists.txt
===
--- clang-tools-extra/clang-tidy/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/CMakeLists.txt
@@ -44,6 +44,7 @@
 add_subdirectory(fuchsia)
 add_subdirectory(google)
 add_subdirectory(hicpp)
+add_subdirectory(linux)
 add_subdirectory(llvm)
 add_subdirectory(misc)
 add_subdirectory(modernize)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59845: Fix IsStructuralMatch specialization for EnumDecl to prevent re-importing an EnumDecl while trying to complete it

[Aleksei Sidorin via Phabricator via cfe-commits]

a_sidorin added a comment.

Post-LGTM with some stylish nits.




Comment at: cfe/trunk/lib/AST/ASTImporter.cpp:1950
+  // Eliminate a potential failure point where we attempt to re-import
+  // something we're trying to import while completin ToEnum
+  if (Decl *ToOrigin = Importer.GetOriginalDecl(ToEnum))

`completin_g_`; also, comments are required to be ended with dot.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59845



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

[PATCH] D59900: [Sema] Fix a crash when nonnull checking

[Nicolas Lesser via Phabricator via cfe-commits]

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

Otherwise LGTM.




Comment at: clang/test/SemaTemplate/decltype.cpp:1
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
+// no crash & no diag

test/SemaCXX/nonnull.cpp would be a better place to put this test.



Comment at: clang/test/SemaTemplate/decltype.cpp:2
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
+// no crash & no diag
+

This is redundant :)



Comment at: clang/test/SemaTemplate/decltype.cpp:4
+
+// expected-no-diagnostics
+template 

If you move the test as above you can drop this line.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59900



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

[PATCH] D59665: Call to HandleNameConflict in VisitEnumDecl mistakeningly using Name instead of SearchName

[Aleksei Sidorin via Phabricator via cfe-commits]

a_sidorin added a comment.

Hi Shafik,
Thank you for the explanation, it is much more clear to me now. But, as I see, 
D59692  is going to discard the changes this 
patch introduces. @martong Gabor, do you expect the changes of this patch to be 
merged into yours, or should this patch be abandoned?


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

https://reviews.llvm.org/D59665



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

[PATCH] D59692: [ASTImporter] Fix name conflict handling

[Aleksei Sidorin via Phabricator via cfe-commits]

a_sidorin added a comment.

Thanks for the fixes! I'd like to clarify one moment, however.




Comment at: lib/AST/ASTImporter.cpp:2154
+return NameOrErr.takeError();
 }
   }

Should we write `else Name = *NameOrError`?


Repository:
  rC Clang

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

https://reviews.llvm.org/D59692



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

[PATCH] D59934: Compare SourceLocations from different TUs by FileID

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added a reviewer: Szelethus.
Szelethus added a comment.

In D59934#1446051 , @gamesh411 wrote:

> Hi!
>
> This issue came up during the generation BugReports of BugPaths containing 
> macro-expansions, where the spelling location and expansion locations were in 
> different files.
>  With this change, we make such SourceLocations comparable by their FileIDs.


Do you mean regular macro expansions or the one we're generating in the plist 
output via `-analyzer-config expand-macros=true`?




Comment at: lib/Basic/SourceManager.cpp:1984
 
 /// Determines the order of 2 source locations in the translation unit.
 ///

We should change this too.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59934



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

Re: r356598 - Recommit "Support attribute used in member funcs of class templates"

[Rafael Auler via cfe-commits]

Full LTO. Oh, so your build bot is using thinLTO. I’ll keep an eye in thinLTO 
builds.

From: Michael Spencer 
Date: Thursday, March 28, 2019 at 1:27 PM
To: Rafael Auler 
Cc: "cfe-commits@lists.llvm.org" 
Subject: Re: r356598 - Recommit "Support attribute used in member funcs of 
class templates"

On Tue, Mar 26, 2019 at 7:40 PM Rafael Auler 
mailto:rafaelau...@fb.com>> wrote:
I’m familiar with this issue as I had to fix a memory bug in LLVM IRLinker that 
was exposed by this commit. That’s why I initially reverted it. However, after 
fixing it, I was able to do the full clang LTO self-hosting with lld on Linux. 
Is there any way I can repro this issue? It’s probably a bad interaction of 
attribute used and an optimization. See 
https://reviews.llvm.org/D59552

Was this with full LTO or thin LTO?  I'm still working on getting a reproducer. 
 This may be related to this issue: 
https://bugs.llvm.org/show_bug.cgi?id=41236

- Michael Spencer



From: Michael Spencer mailto:bigchees...@gmail.com>>
Date: Tuesday, March 26, 2019 at 6:59 PM
To: Rafael Auler mailto:rafaelau...@fb.com>>
Cc: "cfe-commits@lists.llvm.org" 
mailto:cfe-commits@lists.llvm.org>>
Subject: Re: r356598 - Recommit "Support attribute used in member funcs of 
class templates"

On Wed, Mar 20, 2019 at 12:21 PM Rafael Auler via cfe-commits 
mailto:cfe-commits@lists.llvm.org>> wrote:
Author: rafauler
Date: Wed Mar 20 12:22:24 2019
New Revision: 356598

URL: 
http://llvm.org/viewvc/llvm-project?rev=356598=rev
Log:
Recommit "Support attribute used in member funcs of class templates"

This diff previously exposed a bug in LLVM's IRLinker, breaking
buildbots that tried to self-host LLVM with monolithic LTO.
The bug is now in LLVM by D59552

Original commit message:
As PR17480 describes, clang does not support the used attribute
for member functions of class templates. This means that if the member
function is not used, its definition is never instantiated. This patch
changes clang to emit the definition if it has the used attribute.

Test Plan: Added a testcase

Reviewed By: aaron.ballman

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

Added:

cfe/trunk/test/CodeGenCXX/attr-used-member-function-implicit-instantiation.cpp
Modified:
cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp

Modified: cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp?rev=356598=356597=356598=diff
==
--- cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp Wed Mar 20 12:22:24 2019
@@ -2232,6 +2232,20 @@ TemplateDeclInstantiator::VisitCXXMethod
 Owner->addDecl(Method);
   }

+  // PR17480: Honor the used attribute to instantiate member function
+  // definitions
+  if (Method->hasAttr()) {
+if (const auto *A = dyn_cast(Owner)) {
+  SourceLocation Loc;
+  if (const MemberSpecializationInfo *MSInfo =
+  A->getMemberSpecializationInfo())
+Loc = MSInfo->getPointOfInstantiation();
+  else if (const auto *Spec = dyn_cast(A))
+Loc = Spec->getPointOfInstantiation();
+  SemaRef.MarkFunctionReferenced(Loc, Method);
+}
+  }
+
   return Method;
 }


Added: 
cfe/trunk/test/CodeGenCXX/attr-used-member-function-implicit-instantiation.cpp
URL: 

Re: r356598 - Recommit "Support attribute used in member funcs of class templates"

[Michael Spencer via cfe-commits]

On Tue, Mar 26, 2019 at 7:40 PM Rafael Auler  wrote:

> I’m familiar with this issue as I had to fix a memory bug in LLVM IRLinker
> that was exposed by this commit. That’s why I initially reverted it.
> However, after fixing it, I was able to do the full clang LTO self-hosting
> with lld on Linux. Is there any way I can repro this issue? It’s probably a
> bad interaction of attribute used and an optimization. See
> https://reviews.llvm.org/D59552
>

Was this with full LTO or thin LTO?  I'm still working on getting a
reproducer.  This may be related to this issue:
https://bugs.llvm.org/show_bug.cgi?id=41236

- Michael Spencer


>
>
>
>
> *From: *Michael Spencer 
> *Date: *Tuesday, March 26, 2019 at 6:59 PM
> *To: *Rafael Auler 
> *Cc: *"cfe-commits@lists.llvm.org" 
> *Subject: *Re: r356598 - Recommit "Support attribute used in member funcs
> of class templates"
>
>
>
> On Wed, Mar 20, 2019 at 12:21 PM Rafael Auler via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
> Author: rafauler
> Date: Wed Mar 20 12:22:24 2019
> New Revision: 356598
>
> URL: http://llvm.org/viewvc/llvm-project?rev=356598=rev
> 
> Log:
> Recommit "Support attribute used in member funcs of class templates"
>
> This diff previously exposed a bug in LLVM's IRLinker, breaking
> buildbots that tried to self-host LLVM with monolithic LTO.
> The bug is now in LLVM by D59552
>
> Original commit message:
> As PR17480 describes, clang does not support the used attribute
> for member functions of class templates. This means that if the member
> function is not used, its definition is never instantiated. This patch
> changes clang to emit the definition if it has the used attribute.
>
> Test Plan: Added a testcase
>
> Reviewed By: aaron.ballman
>
> Differential Revision: https://reviews.llvm.org/D56928
> 
>
> Added:
>
> cfe/trunk/test/CodeGenCXX/attr-used-member-function-implicit-instantiation.cpp
> Modified:
> cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
>
> Modified: cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp?rev=356598=356597=356598=diff
> 
>
> ==
> --- cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp Wed Mar 20 12:22:24
> 2019
> @@ -2232,6 +2232,20 @@ TemplateDeclInstantiator::VisitCXXMethod
>  Owner->addDecl(Method);
>}
>
> +  // PR17480: Honor the used attribute to instantiate member function
> +  // definitions
> +  if (Method->hasAttr()) {
> +if (const auto *A = dyn_cast(Owner)) {
> +  SourceLocation Loc;
> +  if (const MemberSpecializationInfo *MSInfo =
> +  A->getMemberSpecializationInfo())
> +Loc = MSInfo->getPointOfInstantiation();
> +  else if (const auto *Spec =
> dyn_cast(A))
> +Loc = Spec->getPointOfInstantiation();
> +  SemaRef.MarkFunctionReferenced(Loc, Method);
> +}
> +  }
> +
>return Method;
>  }
>
>
> Added:
> cfe/trunk/test/CodeGenCXX/attr-used-member-function-implicit-instantiation.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/attr-used-member-function-implicit-instantiation.cpp?rev=356598=auto
> 
>
> ==
> ---
> cfe/trunk/test/CodeGenCXX/attr-used-member-function-implicit-instantiation.cpp
> (added)
> +++
> cfe/trunk/test/CodeGenCXX/attr-used-member-function-implicit-instantiation.cpp
> Wed Mar 20 12:22:24 2019
> @@ -0,0 +1,19 @@
> +// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s |
> FileCheck %s
> +
> +// Check that PR17480 is fixed: __attribute__((used)) ignored 

[PATCH] D59628: Add support for __attribute__((objc_class_stub))

[Slava Pestov via Phabricator via cfe-commits]

slavapestov added a comment.

I don't know what the etiquette is around here about pinging reviewers for a 
re-review, but this CL is ready for another look. Your feedback is much 
appreciated!


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

https://reviews.llvm.org/D59628



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

[PATCH] D59955: gn build: Add check-clang-tools to run clang-tools-extra lit tests

[Nico Weber via Phabricator via cfe-commits]

thakis created this revision.
thakis added a reviewer: mbonadei.
Herald added subscribers: kadircet, arphaman, jkorous, ioeric, ilya-biryukov, 
mgorny.
Herald added a project: LLVM.

Only runs the clang-tools-extra lit tests; not yet the unit tests.

Add a build file for clangd-indexer too, since it's needed for
the tests.


https://reviews.llvm.org/D59955

Files:
  clang-tools-extra/clangd/indexer/CMakeLists.txt
  llvm/utils/gn/secondary/BUILD.gn
  llvm/utils/gn/secondary/clang-tools-extra/clangd/indexer/BUILD.gn
  llvm/utils/gn/secondary/clang-tools-extra/test/BUILD.gn
  
llvm/utils/gn/secondary/clang-tools-extra/test/clang_tools_extra_lit_site_cfg_files.gni
  llvm/utils/gn/secondary/llvm/utils/llvm-lit/BUILD.gn

Index: llvm/utils/gn/secondary/llvm/utils/llvm-lit/BUILD.gn
===
--- llvm/utils/gn/secondary/llvm/utils/llvm-lit/BUILD.gn
+++ llvm/utils/gn/secondary/llvm/utils/llvm-lit/BUILD.gn
@@ -1,3 +1,4 @@
+import("//clang-tools-extra/test/clang_tools_extra_lit_site_cfg_files.gni")
 import("//clang/test/clang_lit_site_cfg_files.gni")
 import("//lld/test/lld_lit_site_cfg_files.gni")
 import("//llvm/test/llvm_lit_site_cfg_files.gni")
@@ -24,6 +25,8 @@
   config_map = ""
 
   deps += [
+"//clang-tools-extra/test:lit_site_cfg",
+"//clang-tools-extra/test:lit_unit_site_cfg",
 "//clang/test:lit_site_cfg",
 "//clang/test:lit_unit_site_cfg",
 "//lld/test:lit_site_cfg",
@@ -33,6 +36,12 @@
   ]
 
   # Note: \n is converted into a newline by write_cmake_config.py, not by gn.
+  config_map +=
+  "map_config('" + rebase_path("//clang-tools-extra/test/lit.cfg.py") +
+  "', '" + rebase_path(clang_tools_extra_lit_site_cfg_file) + "')\n"
+  config_map +=
+  "map_config('" + rebase_path("//clang-tools-extra/test/Unit/lit.cfg.py") +
+  "', '" + rebase_path(clang_tools_extra_lit_unit_site_cfg_file) + "')\n"
   config_map += "map_config('" + rebase_path("//clang/test/lit.cfg.py") +
 "', '" + rebase_path(clang_lit_site_cfg_file) + "')\n"
   config_map += "map_config('" + rebase_path("//clang/test/Unit/lit.cfg.py") +
Index: llvm/utils/gn/secondary/clang-tools-extra/test/clang_tools_extra_lit_site_cfg_files.gni
===
--- /dev/null
+++ llvm/utils/gn/secondary/clang-tools-extra/test/clang_tools_extra_lit_site_cfg_files.gni
@@ -0,0 +1,4 @@
+clang_tools_extra_lit_site_cfg_file =
+"$root_gen_dir/clang-tools-extra/test/lit.site.cfg.py"
+clang_tools_extra_lit_unit_site_cfg_file =
+"$root_gen_dir/clang-tools-extra/test/Unit/lit.site.cfg.py"
Index: llvm/utils/gn/secondary/clang-tools-extra/test/BUILD.gn
===
--- /dev/null
+++ llvm/utils/gn/secondary/clang-tools-extra/test/BUILD.gn
@@ -0,0 +1,120 @@
+import("//clang/lib/StaticAnalyzer/Frontend/enable.gni")
+import("//llvm/triples.gni")
+import("//llvm/utils/gn/build/write_cmake_config.gni")
+import("clang_tools_extra_lit_site_cfg_files.gni")
+
+template("write_lit_config") {
+  write_cmake_config(target_name) {
+input = invoker.input
+output = invoker.output
+values = [
+  "LIT_SITE_CFG_IN_HEADER=## Autogenerated from $input, do not edit",
+  "CLANG_TOOLS_BINARY_DIR=" +
+  rebase_path(get_label_info("//clang-tools-extra", "target_out_dir")),
+  "CLANG_TOOLS_SOURCE_DIR=" + rebase_path("//clang-tools-extra"),
+  "LLVM_LIBS_DIR=",  # needed only for shared builds
+  "TARGET_TRIPLE=$llvm_target_triple",
+]
+if (host_os == "win") {
+  # See comment for Windows solink in llvm/utils/gn/build/toolchain/BUILD.gn
+  values += [ "SHLIBDIR=" + rebase_path("$root_out_dir/bin") ]
+} else {
+  values += [ "SHLIBDIR=" + rebase_path("$root_out_dir/lib") ]
+}
+values += invoker.extra_values
+  }
+}
+
+write_lit_config("lit_site_cfg") {
+  # Fully-qualified instead of relative for LIT_SITE_CFG_IN_HEADER.
+  input = "//clang-tools-extra/test/lit.site.cfg.py.in"
+  output = clang_tools_extra_lit_site_cfg_file
+
+  extra_values = [
+"CLANG_TOOLS_DIR=" + rebase_path("$root_out_dir/bin"),
+"LLVM_LIT_TOOLS_DIR=",  # Intentionally empty, matches cmake build.
+"LLVM_TOOLS_DIR=" + rebase_path("$root_out_dir/bin"),
+"PYTHON_EXECUTABLE=$python_path",
+"CLANGD_BUILD_XPC_SUPPORT=0",  # FIXME
+  ]
+
+  if (clang_enable_static_analyzer) {
+extra_values += [ "CLANG_ENABLE_STATIC_ANALYZER=1" ]
+  } else {
+extra_values += [ "CLANG_ENABLE_STATIC_ANALYZER=0" ]
+  }
+}
+
+write_lit_config("lit_unit_site_cfg") {
+  # Fully-qualified instead of relative for LIT_SITE_CFG_IN_HEADER.
+  input = "//clang-tools-extra/test/Unit/lit.site.cfg.py.in"
+  output = clang_tools_extra_lit_unit_site_cfg_file
+  extra_values = []
+}
+
+# This target should contain all dependencies of check-clang-tools.
+# //:default depends on it, so that ninja's default target builds all
+# prerequisites for 

r357205 - [OPENMP]Add check for undefined behavior with thread allocators on

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Thu Mar 28 12:15:36 2019
New Revision: 357205

URL: http://llvm.org/viewvc/llvm-project?rev=357205=rev
Log:
[OPENMP]Add check for undefined behavior with thread allocators on
target and task-based directives.

According to OpenMP 5.0, 2.11.4 allocate Clause, Restrictions, For task,
taskloop or target directives, allocation requests to memory allocators
with the trait access set to thread result in unspecified behavior.
Patch introduces a check for omp_thread_mem_alloc predefined allocator
on target- and trask-based directives.

Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/SemaOpenMP.cpp
cfe/trunk/test/OpenMP/target_firstprivate_messages.cpp
cfe/trunk/test/OpenMP/target_parallel_firstprivate_messages.cpp
cfe/trunk/test/OpenMP/target_parallel_for_firstprivate_messages.cpp
cfe/trunk/test/OpenMP/target_parallel_for_lastprivate_messages.cpp
cfe/trunk/test/OpenMP/target_parallel_for_linear_messages.cpp
cfe/trunk/test/OpenMP/target_parallel_for_private_messages.cpp
cfe/trunk/test/OpenMP/target_parallel_for_reduction_messages.cpp
cfe/trunk/test/OpenMP/target_parallel_for_simd_firstprivate_messages.cpp
cfe/trunk/test/OpenMP/target_parallel_for_simd_lastprivate_messages.cpp
cfe/trunk/test/OpenMP/target_parallel_for_simd_linear_messages.cpp
cfe/trunk/test/OpenMP/target_parallel_for_simd_private_messages.cpp
cfe/trunk/test/OpenMP/target_parallel_for_simd_reduction_messages.cpp
cfe/trunk/test/OpenMP/target_parallel_private_messages.cpp
cfe/trunk/test/OpenMP/target_parallel_reduction_messages.cpp
cfe/trunk/test/OpenMP/target_private_messages.cpp
cfe/trunk/test/OpenMP/target_reduction_messages.cpp
cfe/trunk/test/OpenMP/target_simd_firstprivate_messages.cpp
cfe/trunk/test/OpenMP/target_simd_lastprivate_messages.cpp
cfe/trunk/test/OpenMP/target_simd_linear_messages.cpp
cfe/trunk/test/OpenMP/target_simd_private_messages.cpp
cfe/trunk/test/OpenMP/target_simd_reduction_messages.cpp
cfe/trunk/test/OpenMP/target_teams_distribute_firstprivate_messages.cpp
cfe/trunk/test/OpenMP/target_teams_distribute_lastprivate_messages.cpp

cfe/trunk/test/OpenMP/target_teams_distribute_parallel_for_firstprivate_messages.cpp

cfe/trunk/test/OpenMP/target_teams_distribute_parallel_for_lastprivate_messages.cpp

cfe/trunk/test/OpenMP/target_teams_distribute_parallel_for_private_messages.cpp

cfe/trunk/test/OpenMP/target_teams_distribute_parallel_for_reduction_messages.cpp

cfe/trunk/test/OpenMP/target_teams_distribute_parallel_for_simd_firstprivate_messages.cpp

cfe/trunk/test/OpenMP/target_teams_distribute_parallel_for_simd_lastprivate_messages.cpp

cfe/trunk/test/OpenMP/target_teams_distribute_parallel_for_simd_linear_messages.cpp

cfe/trunk/test/OpenMP/target_teams_distribute_parallel_for_simd_private_messages.cpp

cfe/trunk/test/OpenMP/target_teams_distribute_parallel_for_simd_reduction_messages.cpp
cfe/trunk/test/OpenMP/target_teams_distribute_private_messages.cpp
cfe/trunk/test/OpenMP/target_teams_distribute_reduction_messages.cpp
cfe/trunk/test/OpenMP/target_teams_distribute_simd_firstprivate_messages.cpp
cfe/trunk/test/OpenMP/target_teams_distribute_simd_lastprivate_messages.cpp
cfe/trunk/test/OpenMP/target_teams_distribute_simd_linear_messages.cpp
cfe/trunk/test/OpenMP/target_teams_distribute_simd_private_messages.cpp
cfe/trunk/test/OpenMP/target_teams_distribute_simd_reduction_messages.cpp
cfe/trunk/test/OpenMP/target_teams_firstprivate_messages.cpp
cfe/trunk/test/OpenMP/target_teams_private_messages.cpp
cfe/trunk/test/OpenMP/target_teams_reduction_messages.cpp
cfe/trunk/test/OpenMP/task_firstprivate_messages.cpp
cfe/trunk/test/OpenMP/task_in_reduction_message.cpp
cfe/trunk/test/OpenMP/task_private_messages.cpp
cfe/trunk/test/OpenMP/taskloop_firstprivate_messages.cpp
cfe/trunk/test/OpenMP/taskloop_in_reduction_messages.cpp
cfe/trunk/test/OpenMP/taskloop_lastprivate_messages.cpp
cfe/trunk/test/OpenMP/taskloop_private_messages.cpp
cfe/trunk/test/OpenMP/taskloop_reduction_messages.cpp
cfe/trunk/test/OpenMP/taskloop_simd_firstprivate_messages.cpp
cfe/trunk/test/OpenMP/taskloop_simd_in_reduction_messages.cpp
cfe/trunk/test/OpenMP/taskloop_simd_lastprivate_messages.cpp
cfe/trunk/test/OpenMP/taskloop_simd_linear_messages.cpp
cfe/trunk/test/OpenMP/taskloop_simd_private_messages.cpp
cfe/trunk/test/OpenMP/taskloop_simd_reduction_messages.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=357205=357204=357205=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu Mar 28 12:15:36 
2019
@@ 

[PATCH] D59953: Add .py extension to clang-tools-extra lit cfg files

[Nico Weber via Phabricator via cfe-commits]

thakis updated this revision to Diff 192698.
thakis retitled this revision from "Rename clang-tools-extra lit cfg files to 
lit.site.cfg.py" to "Add .py extension to clang-tools-extra lit cfg files".
thakis added a comment.

Ok, ptal.


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

https://reviews.llvm.org/D59953

Files:
  clang-tools-extra/test/CMakeLists.txt
  clang-tools-extra/test/Unit/lit.cfg
  clang-tools-extra/test/Unit/lit.cfg.py
  clang-tools-extra/test/Unit/lit.site.cfg.in
  clang-tools-extra/test/Unit/lit.site.cfg.py.in
  clang-tools-extra/test/lit.cfg
  clang-tools-extra/test/lit.cfg.py
  clang-tools-extra/test/lit.site.cfg.in
  clang-tools-extra/test/lit.site.cfg.py.in


Index: clang-tools-extra/test/lit.site.cfg.py.in
===
--- clang-tools-extra/test/lit.site.cfg.py.in
+++ clang-tools-extra/test/lit.site.cfg.py.in
@@ -28,4 +28,4 @@
 lit.llvm.initialize(lit_config, config)
 
 # Let the main config do the real work.
-lit_config.load_config(config, "@CLANG_TOOLS_SOURCE_DIR@/test/lit.cfg")
+lit_config.load_config(config, "@CLANG_TOOLS_SOURCE_DIR@/test/lit.cfg.py")
Index: clang-tools-extra/test/Unit/lit.site.cfg.py.in
===
--- clang-tools-extra/test/Unit/lit.site.cfg.py.in
+++ clang-tools-extra/test/Unit/lit.site.cfg.py.in
@@ -6,4 +6,4 @@
 config.shlibdir = "@SHLIBDIR@"
 config.target_triple = "@TARGET_TRIPLE@"
 
-lit_config.load_config(config, "@CLANG_TOOLS_SOURCE_DIR@/test/Unit/lit.cfg")
+lit_config.load_config(config, "@CLANG_TOOLS_SOURCE_DIR@/test/Unit/lit.cfg.py")
Index: clang-tools-extra/test/CMakeLists.txt
===
--- clang-tools-extra/test/CMakeLists.txt
+++ clang-tools-extra/test/CMakeLists.txt
@@ -20,13 +20,17 @@
   CLANGD_BUILD_XPC_SUPPORT)
 
 configure_lit_site_cfg(
-  ${CMAKE_CURRENT_SOURCE_DIR}/lit.site.cfg.in
-  ${CMAKE_CURRENT_BINARY_DIR}/lit.site.cfg
+  ${CMAKE_CURRENT_SOURCE_DIR}/lit.site.cfg.py.in
+  ${CMAKE_CURRENT_BINARY_DIR}/lit.site.cfg.py
+  MAIN_CONFIG
+  ${CMAKE_CURRENT_SOURCE_DIR}/lit.cfg.py
   )
 
 configure_lit_site_cfg(
-  ${CMAKE_CURRENT_SOURCE_DIR}/Unit/lit.site.cfg.in
-  ${CMAKE_CURRENT_BINARY_DIR}/Unit/lit.site.cfg
+  ${CMAKE_CURRENT_SOURCE_DIR}/Unit/lit.site.cfg.py.in
+  ${CMAKE_CURRENT_BINARY_DIR}/Unit/lit.site.cfg.py
+  MAIN_CONFIG
+  ${CMAKE_CURRENT_SOURCE_DIR}/Unit/lit.cfg.py
   )
 
 option(CLANG_TOOLS_TEST_USE_VG "Run Clang tools' tests under Valgrind" OFF)


Index: clang-tools-extra/test/lit.site.cfg.py.in
===
--- clang-tools-extra/test/lit.site.cfg.py.in
+++ clang-tools-extra/test/lit.site.cfg.py.in
@@ -28,4 +28,4 @@
 lit.llvm.initialize(lit_config, config)
 
 # Let the main config do the real work.
-lit_config.load_config(config, "@CLANG_TOOLS_SOURCE_DIR@/test/lit.cfg")
+lit_config.load_config(config, "@CLANG_TOOLS_SOURCE_DIR@/test/lit.cfg.py")
Index: clang-tools-extra/test/Unit/lit.site.cfg.py.in
===
--- clang-tools-extra/test/Unit/lit.site.cfg.py.in
+++ clang-tools-extra/test/Unit/lit.site.cfg.py.in
@@ -6,4 +6,4 @@
 config.shlibdir = "@SHLIBDIR@"
 config.target_triple = "@TARGET_TRIPLE@"
 
-lit_config.load_config(config, "@CLANG_TOOLS_SOURCE_DIR@/test/Unit/lit.cfg")
+lit_config.load_config(config, "@CLANG_TOOLS_SOURCE_DIR@/test/Unit/lit.cfg.py")
Index: clang-tools-extra/test/CMakeLists.txt
===
--- clang-tools-extra/test/CMakeLists.txt
+++ clang-tools-extra/test/CMakeLists.txt
@@ -20,13 +20,17 @@
   CLANGD_BUILD_XPC_SUPPORT)
 
 configure_lit_site_cfg(
-  ${CMAKE_CURRENT_SOURCE_DIR}/lit.site.cfg.in
-  ${CMAKE_CURRENT_BINARY_DIR}/lit.site.cfg
+  ${CMAKE_CURRENT_SOURCE_DIR}/lit.site.cfg.py.in
+  ${CMAKE_CURRENT_BINARY_DIR}/lit.site.cfg.py
+  MAIN_CONFIG
+  ${CMAKE_CURRENT_SOURCE_DIR}/lit.cfg.py
   )
 
 configure_lit_site_cfg(
-  ${CMAKE_CURRENT_SOURCE_DIR}/Unit/lit.site.cfg.in
-  ${CMAKE_CURRENT_BINARY_DIR}/Unit/lit.site.cfg
+  ${CMAKE_CURRENT_SOURCE_DIR}/Unit/lit.site.cfg.py.in
+  ${CMAKE_CURRENT_BINARY_DIR}/Unit/lit.site.cfg.py
+  MAIN_CONFIG
+  ${CMAKE_CURRENT_SOURCE_DIR}/Unit/lit.cfg.py
   )
 
 option(CLANG_TOOLS_TEST_USE_VG "Run Clang tools' tests under Valgrind" OFF)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros

[Nick Renieris via Phabricator via cfe-commits]

VelocityRa updated this revision to Diff 192694.
VelocityRa marked 4 inline comments as done.

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

https://reviews.llvm.org/D28462

Files:
  docs/ClangFormatStyleOptions.rst
  include/clang/Format/Format.h
  lib/Format/Format.cpp
  lib/Format/WhitespaceManager.cpp
  lib/Format/WhitespaceManager.h
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -9544,8 +9544,104 @@
NoSpaceStyle);
 }
 
+TEST_F(FormatTest, AlignConsecutiveMacros) {
+  FormatStyle Style = getLLVMStyle();
+  Style.AlignConsecutiveAssignments = true;
+  Style.AlignConsecutiveDeclarations = true;
+  Style.AlignConsecutiveMacros = false;
+
+  verifyFormat("#define a 3\n"
+   "#define  4\n"
+   "#define ccc (5)",
+   Style);
+
+  verifyFormat("#define f(x) (x * x)\n"
+   "#define fff(x, y, z) (x * y + z)\n"
+   "#define (x, y) (x - y)",
+   Style);
+
+  verifyFormat("#define foo(x, y) (x + y)\n"
+   "#define bar (5, 6)(2 + 2)",
+   Style);
+
+  verifyFormat("#define a 3\n"
+   "#define  4\n"
+   "#define ccc (5)\n"
+   "#define f(x) (x * x)\n"
+   "#define fff(x, y, z) (x * y + z)\n"
+   "#define (x, y) (x - y)",
+   Style);
+
+  Style.AlignConsecutiveMacros = true;
+  verifyFormat("#define a3\n"
+   "#define  4\n"
+   "#define ccc  (5)",
+   Style);
+
+  verifyFormat("#define f(x) (x * x)\n"
+   "#define fff(x, y, z) (x * y + z)\n"
+   "#define (x, y)   (x - y)",
+   Style);
+
+  verifyFormat("#define foo(x, y) (x + y)\n"
+   "#define bar   (5, 6)(2 + 2)",
+   Style);
+
+  verifyFormat("#define a3\n"
+   "#define  4\n"
+   "#define ccc  (5)\n"
+   "#define f(x) (x * x)\n"
+   "#define fff(x, y, z) (x * y + z)\n"
+   "#define (x, y)   (x - y)",
+   Style);
+
+  verifyFormat("#define a 5\n"
+   "#define foo(x, y) (x + y)\n"
+   "#define CCC   (6)\n"
+   "auto lambda = []() {\n"
+   "  auto  ii = 0;\n"
+   "  float j  = 0;\n"
+   "  return 0;\n"
+   "};\n"
+   "int   i  = 0;\n"
+   "float i2 = 0;\n"
+   "auto  v  = type{\n"
+   "i = 1,   //\n"
+   "(i = 2), //\n"
+   "i = 3//\n"
+   "};",
+   Style);
+
+  Style.AlignConsecutiveMacros = false;
+  Style.ColumnLimit = 20;
+
+  verifyFormat("#define a  \\\n"
+   "  \"aa\"\n"
+   "#define D  \\\n"
+   "  \"aa\" \\\n"
+   "  \"ccdde\"\n"
+   "#define B  \\\n"
+   "  \"Q\"  \\\n"
+   "  \"F\"  \\\n"
+   "  \"\"\n",
+   Style);
+
+  Style.AlignConsecutiveMacros = true;
+  verifyFormat("#define a  \\\n"
+   "  \"aa\"\n"
+   "#define D  \\\n"
+   "  \"aa\" \\\n"
+   "  \"ccdde\"\n"
+   "#define B  \\\n"
+   "  \"Q\"  \\\n"
+   "  \"F\"  \\\n"
+   "  \"\"\n",
+   Style);
+}
+
 TEST_F(FormatTest, AlignConsecutiveAssignments) {
   FormatStyle Alignment = getLLVMStyle();
+  Alignment.AlignConsecutiveMacros = true;
   Alignment.AlignConsecutiveAssignments = false;
   verifyFormat("int a = 5;\n"
"int oneTwoThree = 123;",
@@ -9735,6 +9831,7 @@
 
 TEST_F(FormatTest, AlignConsecutiveDeclarations) {
   FormatStyle Alignment = getLLVMStyle();
+  Alignment.AlignConsecutiveMacros = true;
   Alignment.AlignConsecutiveDeclarations = false;
   verifyFormat("float const a = 5;\n"
"int oneTwoThree = 123;",
@@ -10856,6 +10953,7 @@
   Style.Language = FormatStyle::LK_Cpp;
   CHECK_PARSE_BOOL(AlignOperands);
   CHECK_PARSE_BOOL(AlignTrailingComments);
+  CHECK_PARSE_BOOL(AlignConsecutiveMacros);
   CHECK_PARSE_BOOL(AlignConsecutiveAssignments);
   CHECK_PARSE_BOOL(AlignConsecutiveDeclarations);
   CHECK_PARSE_BOOL(AllowAllParametersOfDeclarationOnNextLine);
Index: lib/Format/WhitespaceManager.h
===
--- lib/Format/WhitespaceManager.h
+++ lib/Format/WhitespaceManager.h
@@ -169,6 +169,9 @@
   /// \c EscapedNewlineColumn for the first tokens or token parts in a line.
   void 

[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros

[Nick Renieris via Phabricator via cfe-commits]

VelocityRa added inline comments.



Comment at: lib/Format/WhitespaceManager.cpp:520
+
+  AlignMacroSequence(StartOfSequence, EndOfSequence, MinColumn, MaxColumn,
+ FoundMatchOnLine, AlignMacrosMatches, Changes);

klimek wrote:
> Why are we calling AlignMacroSequence(0, 0, ...) here?
Wasn't needed apparently.



Comment at: lib/Format/WhitespaceManager.cpp:524
+  unsigned I = 0;
+  for (unsigned E = Changes.size(); I != E; ++I) {
+if (Changes[I].NewlinesBefore != 0) {

klimek wrote:
> I still think the code in this loop either needs significantly more comments 
> to explain what's going on, or needs to be changed to be more straight 
> forward:
> What I don't understand is why we're calling AlignMacroSequence potentially 
> multiple times, and especially what things like the if in line 541 are for.
That `if` and surrounding logic also exists and is copied from `AlignTokens` 
and it's not documented there.
Anyway, apparently the extra `AlignMacroSequence` in this loop could be 
removed. I tried iteratively removing/tweaking other snippets like the `if` and 
tests failed.


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

https://reviews.llvm.org/D28462



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

[PATCH] D59953: Rename clang-tools-extra lit cfg files to lit.site.cfg.py

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Actually, let me rename lit.cfg to lit.cfg.py too, one sec...


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

https://reviews.llvm.org/D59953



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

[PATCH] D59953: Rename clang-tools-extra lit cfg files to lit.site.cfg.py

[Nico Weber via Phabricator via cfe-commits]

thakis created this revision.
thakis added a reviewer: zturner.
Herald added a subscriber: mgorny.
thakis added a comment.

Actually, let me rename lit.cfg to lit.cfg.py too, one sec...


Follow-up to r313892, which did this for clang and llvm.


https://reviews.llvm.org/D59953

Files:
  clang-tools-extra/test/CMakeLists.txt
  clang-tools-extra/test/Unit/lit.site.cfg.in
  clang-tools-extra/test/Unit/lit.site.cfg.py.in
  clang-tools-extra/test/lit.site.cfg.in
  clang-tools-extra/test/lit.site.cfg.py.in


Index: clang-tools-extra/test/CMakeLists.txt
===
--- clang-tools-extra/test/CMakeLists.txt
+++ clang-tools-extra/test/CMakeLists.txt
@@ -20,13 +20,13 @@
   CLANGD_BUILD_XPC_SUPPORT)
 
 configure_lit_site_cfg(
-  ${CMAKE_CURRENT_SOURCE_DIR}/lit.site.cfg.in
-  ${CMAKE_CURRENT_BINARY_DIR}/lit.site.cfg
+  ${CMAKE_CURRENT_SOURCE_DIR}/lit.site.cfg.py.in
+  ${CMAKE_CURRENT_BINARY_DIR}/lit.site.cfg.py
   )
 
 configure_lit_site_cfg(
-  ${CMAKE_CURRENT_SOURCE_DIR}/Unit/lit.site.cfg.in
-  ${CMAKE_CURRENT_BINARY_DIR}/Unit/lit.site.cfg
+  ${CMAKE_CURRENT_SOURCE_DIR}/Unit/lit.site.cfg.py.in
+  ${CMAKE_CURRENT_BINARY_DIR}/Unit/lit.site.cfg.py
   )
 
 option(CLANG_TOOLS_TEST_USE_VG "Run Clang tools' tests under Valgrind" OFF)


Index: clang-tools-extra/test/CMakeLists.txt
===
--- clang-tools-extra/test/CMakeLists.txt
+++ clang-tools-extra/test/CMakeLists.txt
@@ -20,13 +20,13 @@
   CLANGD_BUILD_XPC_SUPPORT)
 
 configure_lit_site_cfg(
-  ${CMAKE_CURRENT_SOURCE_DIR}/lit.site.cfg.in
-  ${CMAKE_CURRENT_BINARY_DIR}/lit.site.cfg
+  ${CMAKE_CURRENT_SOURCE_DIR}/lit.site.cfg.py.in
+  ${CMAKE_CURRENT_BINARY_DIR}/lit.site.cfg.py
   )
 
 configure_lit_site_cfg(
-  ${CMAKE_CURRENT_SOURCE_DIR}/Unit/lit.site.cfg.in
-  ${CMAKE_CURRENT_BINARY_DIR}/Unit/lit.site.cfg
+  ${CMAKE_CURRENT_SOURCE_DIR}/Unit/lit.site.cfg.py.in
+  ${CMAKE_CURRENT_BINARY_DIR}/Unit/lit.site.cfg.py
   )
 
 option(CLANG_TOOLS_TEST_USE_VG "Run Clang tools' tests under Valgrind" OFF)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59932: [clang-tidy] **Prototype**: Add fix description to clang-tidy checks.

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added a comment.

In D59932#1446533 , @JonasToth wrote:

> I think the idea is good and implementation, too. If we iterate all checks 
> anyway (probably?) we could think about adding a severity to the checks, too?
>
> I know that code-checker and code-compass have something like this to signal 
> importance of problems (say bugprone and cert differ from readability for 
> example).


Also Clazy  split checks by severity level.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59932



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

[PATCH] D59870: [clang-tidy] Add MagnitudeBitsUpperLimit option to bugprone-too-small-loop-variable

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

> I think it's the easiest way to specify the bits of the ineteger type to 
> limit the catches. In real life, I met with this overflow / infinite loop 
> problem with 16-bit short type, so I think the real use cases are 8 and 16 
> bit integers. It seems intuitive to me to use the size of the loop variable's 
> type to separate those catches which can lead broken functionality in 
> practice from those use cases which are just integer incompatibilities.

Given your experience and the false positive rate in some projects, should we 
maybe default to 16 for that option?


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

https://reviews.llvm.org/D59870



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

[PATCH] D59932: [clang-tidy] **Prototype**: Add fix description to clang-tidy checks.

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

I think the idea is good and implementation, too. If we iterate all checks 
anyway (probably?) we could think about adding a severity to the checks, too?

I know that code-checker and code-compass have something like this to signal 
importance of problems (say bugprone and cert differ from readability for 
example).


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59932



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

[PATCH] D59932: [clang-tidy] **Prototype**: Add fix description to clang-tidy checks.

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tidy/ClangTidyCheck.h:107
+  /// fixes for different cases.
+  ///   - clang compiler diagnostics and clang static analyzer diagnostics are
+  /// not supported.

I think Clang and Clang Static Analyzer should be capitalized.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59932



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

[PATCH] D59923: [Driver] Simplify -g level computation and its interaction with -gsplit-dwarf

[David Blaikie via Phabricator via cfe-commits]

dblaikie requested changes to this revision.
dblaikie added a comment.
This revision now requires changes to proceed.

What's the general motivation for this work/changes?

> -gmlt -gsplit-dwarf -fno-split-dwarf-inlining => special: 1 (before) 2 (after)

This ^ is the only semantic change due to this refactoring?

There's a desire to be able to compose gmlt+gsplit-dwarf, /if/ 
-fno-split-dwarf-inlining is enabled. (for context, -fno-split-dwarf-inlining 
is the default in Google's optimized builds, since split-dwarf-inlining was 
never implemented in GCC & we didn't want to regress object size when switching 
from GCC to Clang (& no one had complained about that missing functionality))

So with -fno-split-dwarf-inlining, there's value in using gmlt with 
gsplit-dwarf (it reduces the size of the .dwo files - reducing the inputs/size 
of dwp, etc).

& it sounds like this change breaks that scenario?


Repository:
  rC Clang

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

https://reviews.llvm.org/D59923



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

[PATCH] D59008: [AMDGPU] Switch default dwarf version to 5

[Scott Linder via Phabricator via cfe-commits]

scott.linder updated this revision to Diff 192688.
scott.linder added a comment.

Add a test to confirm split-dwarf is supported for the amdhsa OS in the driver.


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

https://reviews.llvm.org/D59008

Files:
  lib/Driver/ToolChains/AMDGPU.h
  test/Driver/amdgpu-toolchain.c
  test/Driver/split-debug.c


Index: test/Driver/split-debug.c
===
--- test/Driver/split-debug.c
+++ test/Driver/split-debug.c
@@ -38,6 +38,9 @@
 // RUN: %clang -target x86_64-pc-freebsd12 -gsplit-dwarf -c -### %s 2> %t
 // RUN: FileCheck -check-prefix=CHECK-OPTION < %t %s
 //
+// RUN: %clang -target amdgcn-amd-amdhsa -gsplit-dwarf -c -### %s 2> %t
+// RUN: FileCheck -check-prefix=CHECK-OPTION < %t %s
+//
 // CHECK-OPTION: "-split-dwarf-file" "split-debug.dwo"
 
 // RUN: %clang -target x86_64-unknown-linux-gnu -gsplit-dwarf -S -### %s 2> %t
Index: test/Driver/amdgpu-toolchain.c
===
--- test/Driver/amdgpu-toolchain.c
+++ test/Driver/amdgpu-toolchain.c
@@ -3,4 +3,4 @@
 // AS_LINK: ld.lld{{.*}} "-shared"
 
 // RUN: %clang -### -g -target amdgcn--amdhsa -mcpu=kaveri %s 2>&1 | FileCheck 
-check-prefix=DWARF_VER %s
-// DWARF_VER: "-dwarf-version=2"
+// DWARF_VER: "-dwarf-version=5"
Index: lib/Driver/ToolChains/AMDGPU.h
===
--- lib/Driver/ToolChains/AMDGPU.h
+++ lib/Driver/ToolChains/AMDGPU.h
@@ -55,7 +55,7 @@
 public:
   AMDGPUToolChain(const Driver , const llvm::Triple ,
   const llvm::opt::ArgList );
-  unsigned GetDefaultDwarfVersion() const override { return 2; }
+  unsigned GetDefaultDwarfVersion() const override { return 5; }
   bool IsIntegratedAssemblerDefault() const override { return true; }
   llvm::opt::DerivedArgList *
   TranslateArgs(const llvm::opt::DerivedArgList , StringRef BoundArch,


Index: test/Driver/split-debug.c
===
--- test/Driver/split-debug.c
+++ test/Driver/split-debug.c
@@ -38,6 +38,9 @@
 // RUN: %clang -target x86_64-pc-freebsd12 -gsplit-dwarf -c -### %s 2> %t
 // RUN: FileCheck -check-prefix=CHECK-OPTION < %t %s
 //
+// RUN: %clang -target amdgcn-amd-amdhsa -gsplit-dwarf -c -### %s 2> %t
+// RUN: FileCheck -check-prefix=CHECK-OPTION < %t %s
+//
 // CHECK-OPTION: "-split-dwarf-file" "split-debug.dwo"
 
 // RUN: %clang -target x86_64-unknown-linux-gnu -gsplit-dwarf -S -### %s 2> %t
Index: test/Driver/amdgpu-toolchain.c
===
--- test/Driver/amdgpu-toolchain.c
+++ test/Driver/amdgpu-toolchain.c
@@ -3,4 +3,4 @@
 // AS_LINK: ld.lld{{.*}} "-shared"
 
 // RUN: %clang -### -g -target amdgcn--amdhsa -mcpu=kaveri %s 2>&1 | FileCheck -check-prefix=DWARF_VER %s
-// DWARF_VER: "-dwarf-version=2"
+// DWARF_VER: "-dwarf-version=5"
Index: lib/Driver/ToolChains/AMDGPU.h
===
--- lib/Driver/ToolChains/AMDGPU.h
+++ lib/Driver/ToolChains/AMDGPU.h
@@ -55,7 +55,7 @@
 public:
   AMDGPUToolChain(const Driver , const llvm::Triple ,
   const llvm::opt::ArgList );
-  unsigned GetDefaultDwarfVersion() const override { return 2; }
+  unsigned GetDefaultDwarfVersion() const override { return 5; }
   bool IsIntegratedAssemblerDefault() const override { return true; }
   llvm::opt::DerivedArgList *
   TranslateArgs(const llvm::opt::DerivedArgList , StringRef BoundArch,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59887: [Syntax] Introduce TokenBuffer, start clangToolingSyntax library

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added inline comments.



Comment at: clang/include/clang/Tooling/Syntax/TokenBuffer.h:69
+  /// The tokens after preprocessor replacements.
+  llvm::ArrayRef tokens(const TokenBuffer ) const;
+  /// Tokens that appear in the text of the file, i.e. a name of an object-like

`expandedTokens`?



Comment at: clang/include/clang/Tooling/Syntax/TokenBuffer.h:72
+  /// macro or a name, arguments and parentheses of a function-like macro.
+  llvm::ArrayRef macroTokens(const TokenBuffer ) const;
+  /// The range covering macroTokens().

`invocationTokens` or `macroInvocationTokens`



Comment at: clang/include/clang/Tooling/Syntax/TokenBuffer.h:74
+  /// The range covering macroTokens().
+  std::pair macroRange(const TokenBuffer ,
+   const SourceManager ) const;

`invocationSourceRange` or `macroInvocationSourceRange` depending on what you 
choose for the function above.




Comment at: clang/include/clang/Tooling/Syntax/TokenBuffer.h:83
+  unsigned BeginMacroToken = 0;
+  unsigned EndMacroToken = 0;
+};

Please add brief doc comments to these variables. Having read the public API of 
this class, I still don't have an idea what these variables are.





Comment at: clang/include/clang/Tooling/Syntax/TokenBuffer.h:120
+  /// the original source file. The tranformation may not be possible if the
+  /// tokens cross macro invocations in the middle, e.g.
+  ///#define FOO 1*2

"The mapping fails if cross boundaries of macro expansions, that is, don't 
correspond to a complete top-level macro invocation"

Consider adding examples.

```
Given this source file:

#define FIRST f1 f2 f3
#define SECOND s1 s2 3

a FIRST b SECOND c  // expansion: a f1 f2 f3 b s1 s2 s3 c

toOffsetRange will map tokens like this:

input range => output range
a => a
s1 s2 s3 => SECOND
a f1 f2 f3 => a FIRST
a f1 => can't map
s1 s2 => s1 s2 within the macro definition
```

Could you add these to tests as well?



Comment at: clang/include/clang/Tooling/Syntax/TokenBuffer.h:153
+  /// FIXME: we do not yet store tokens of directives, like #include, #define,
+  ///#pragma, etc.
+  llvm::ArrayRef macroTokens() const { return MacroTokens; }

... For the input above, macroTokens() should return {desired output}



Comment at: clang/include/clang/Tooling/Syntax/TokenBuffer.h:191
+  /// i.e. after running Execute().
+  TokenBuffer consume() &&;
+

"Finalizes token collection"

Of course a function called "consume" consumes the result :)



Comment at: clang/include/clang/Tooling/Syntax/TokenBuffer.h:191
+  /// i.e. after running Execute().
+  TokenBuffer consume() &&;
+

gribozavr wrote:
> "Finalizes token collection"
> 
> Of course a function called "consume" consumes the result :)
LLVM_NODISCARD?



Comment at: clang/lib/Tooling/Syntax/TokenBuffer.cpp:295
+MacroExpansion::tokens(const TokenBuffer ) const {
+  return B.tokens().slice(BeginExpansionToken,
+  EndExpansionToken - BeginExpansionToken);

"ExpansionTokenBegin"?  "ExpansionTokenStartIndex"?



Comment at: clang/lib/Tooling/Syntax/TokenBuffer.cpp:367
+  if (FirstCall != Expansions.end() &&
+  (FirstCall->BeginExpansionToken < BeginIndex ||
+   EndIndex < FirstCall->EndExpansionToken)) {

`FirstCall->BeginExpansionToken != BeginIndex` ?



Comment at: clang/lib/Tooling/Syntax/TokenBuffer.cpp:368
+  (FirstCall->BeginExpansionToken < BeginIndex ||
+   EndIndex < FirstCall->EndExpansionToken)) {
+return llvm::None;

I don't understand why `EndIndex < FirstCall->EndExpansionToken` is needed -- 
isn't it redundant with the `LastCall` checks below?



Comment at: clang/lib/Tooling/Syntax/TokenBuffer.cpp:373
+  if (LastCall != Expansions.end() &&
+  (LastCall->BeginExpansionToken < BeginIndex ||
+   EndIndex < LastCall->EndExpansionToken)) {

`LastCall->BeginExpansionToken != BeginIndex`?



Comment at: clang/lib/Tooling/Syntax/TokenBuffer.cpp:374
+  (LastCall->BeginExpansionToken < BeginIndex ||
+   EndIndex < LastCall->EndExpansionToken)) {
+return llvm::None;

Also redundant with `FirstCall` checks?



Comment at: clang/unittests/Tooling/Syntax/TokenBufferTest.cpp:355
+  checkTokens("");
+  checkMacroInvocations({{"EMPTY", "", Code.range("m")},
+   {"EMPTY_FUNC(1+2+3)", "", Code.range("f")}});

`expectTokens`, `expectMacroInvocations`?



Comment at: clang/unittests/Tooling/Syntax/TokenBufferTest.cpp:376
+  // The parser eventually breaks the first '>>' into two tokens ('>' and '>'),
+  // but we chooses 

[PATCH] D53343: [Driver] Default Android toolchains to noexecstack.

[Dan Albert via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC357197: [Driver] Default Android toolchains to noexecstack. 
(authored by danalbert, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D53343?vs=169901=192686#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D53343

Files:
  include/clang/Driver/ToolChain.h
  lib/Driver/ToolChain.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Driver/ToolChains/Gnu.cpp
  lib/Driver/ToolChains/Linux.cpp
  lib/Driver/ToolChains/Linux.h
  test/Driver/integrated-as.c
  test/Driver/linux-as.c
  test/Driver/linux-ld.c

Index: lib/Driver/ToolChains/Gnu.cpp
===
--- lib/Driver/ToolChains/Gnu.cpp
+++ lib/Driver/ToolChains/Gnu.cpp
@@ -360,6 +360,11 @@
 CmdArgs.push_back("--no-dynamic-linker");
   }
 
+  if (ToolChain.isNoExecStackDefault()) {
+CmdArgs.push_back("-z");
+CmdArgs.push_back("noexecstack");
+  }
+
   if (Args.hasArg(options::OPT_rdynamic))
 CmdArgs.push_back("-export-dynamic");
 
@@ -609,6 +614,10 @@
 }
   }
 
+  if (getToolChain().isNoExecStackDefault()) {
+  CmdArgs.push_back("--noexecstack");
+  }
+
   switch (getToolChain().getArch()) {
   default:
 break;
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -2053,6 +2053,7 @@
   bool TakeNextArg = false;
 
   bool UseRelaxRelocations = C.getDefaultToolChain().useRelaxRelocations();
+  bool UseNoExecStack = C.getDefaultToolChain().isNoExecStackDefault();
   const char *MipsTargetFeature = nullptr;
   for (const Arg *A :
Args.filtered(options::OPT_Wa_COMMA, options::OPT_Xassembler)) {
@@ -2134,7 +2135,7 @@
   } else if (Value == "--fatal-warnings") {
 CmdArgs.push_back("-massembler-fatal-warnings");
   } else if (Value == "--noexecstack") {
-CmdArgs.push_back("-mnoexecstack");
+UseNoExecStack = true;
   } else if (Value.startswith("-compress-debug-sections") ||
  Value.startswith("--compress-debug-sections") ||
  Value == "-nocompress-debug-sections" ||
@@ -2197,6 +2198,8 @@
   }
   if (UseRelaxRelocations)
 CmdArgs.push_back("--mrelax-relocations");
+  if (UseNoExecStack)
+CmdArgs.push_back("-mnoexecstack");
   if (MipsTargetFeature != nullptr) {
 CmdArgs.push_back("-target-feature");
 CmdArgs.push_back(MipsTargetFeature);
Index: lib/Driver/ToolChains/Linux.h
===
--- lib/Driver/ToolChains/Linux.h
+++ lib/Driver/ToolChains/Linux.h
@@ -38,6 +38,7 @@
llvm::opt::ArgStringList ) const override;
   CXXStdlibType GetDefaultCXXStdlibType() const override;
   bool isPIEDefault() const override;
+  bool isNoExecStackDefault() const override;
   bool IsMathErrnoDefault() const override;
   SanitizerMask getSupportedSanitizers() const override;
   void addProfileRTLibs(const llvm::opt::ArgList ,
Index: lib/Driver/ToolChains/Linux.cpp
===
--- lib/Driver/ToolChains/Linux.cpp
+++ lib/Driver/ToolChains/Linux.cpp
@@ -971,6 +971,10 @@
   getTriple().isMusl() || getSanitizerArgs().requiresPIE();
 }
 
+bool Linux::isNoExecStackDefault() const {
+return getTriple().isAndroid();
+}
+
 bool Linux::IsMathErrnoDefault() const {
   if (getTriple().isAndroid())
 return false;
Index: lib/Driver/ToolChain.cpp
===
--- lib/Driver/ToolChain.cpp
+++ lib/Driver/ToolChain.cpp
@@ -112,6 +112,10 @@
   return ENABLE_X86_RELAX_RELOCATIONS;
 }
 
+bool ToolChain::isNoExecStackDefault() const {
+return false;
+}
+
 const SanitizerArgs& ToolChain::getSanitizerArgs() const {
   if (!SanitizerArguments.get())
 SanitizerArguments.reset(new SanitizerArgs(*this, Args));
Index: include/clang/Driver/ToolChain.h
===
--- include/clang/Driver/ToolChain.h
+++ include/clang/Driver/ToolChain.h
@@ -412,6 +412,9 @@
   /// Test whether this toolchain defaults to PIE.
   virtual bool isPIEDefault() const = 0;
 
+  /// Test whether this toolchaind defaults to non-executable stacks.
+  virtual bool isNoExecStackDefault() const;
+
   /// Tests whether this toolchain forces its default for PIC, PIE or
   /// non-PIC.  If this returns true, any PIC related flags should be ignored
   /// and instead the results of \c isPICDefault() and \c isPIEDefault() are
Index: test/Driver/linux-as.c
===
--- test/Driver/linux-as.c
+++ test/Driver/linux-as.c
@@ -108,12 +108,12 @@
 // RUN: %clang -target arm-linux-androideabi -### \
 // RUN:   -no-integrated-as -c %s 2>&1 \
 // RUN:   | FileCheck 

[PATCH] D59407: [clangd] Add RelationSlab

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

(Sorry to arrive late at this discussion, I'm just back from leave.
I have some suggestions and would appreciate your thoughts, but if simply this 
feels too much like going around in circles I'm happy to work out how we can 
address this after this patch lands instead.
Have discussed offline with @gribozavr and I think we're roughly on the same 
page. @kadircet is now out on leave)

I think the data model might be overly complicated here.
I can see how the discussion got here: it mirrors the other `*Slab` types quite 
closely. But:

- those types were designed to solve specific problems that we don't have here 
(string deduplication and symbol merging)
- I think the class-parent relation is pretty sparse (maybe 1 edge per 10 
symbols?) so lots of options will work fine
- I don't think we yet know what the more resource-critical (denser) relations 
and queries are, so it's unclear what to optimize for

I think the simplest model that can work here is something like:

- `Relation` is `struct { SymbolID Subject; SymbolRole Relation; SymbolID 
Object }`
- this is a value type, so could be passed around simply as 
`std::vector`. If `RelationSlab` is desired for symmetry, it could be 
an alias or simple wrapper around `std::vector`.

This has a few advantages:

- the `Relation` value_type is self-contained, has clearer semantics, and works 
as the result of any type of query: based on subject and/or predicate and/or 
object. (Similar to how it's convenient that Symbol contains SymbolID).
- if lookup is desired, lookup by subject, subject+predicate, or full-triple is 
possible by sorting in SPO order with binary search. Return type is simple 
`ArrayRef`. (Though fancy lookup maybe better belongs in MemIndex/Dex 
rather than in the slab). For more query types, storing a copies in OSP and/or 
POS order is pretty cheap too.
- memory efficiency: it costs `20*distinct(subject, predicate, object)` bytes, 
vs `28*distinct(subject, predicate) + 8 * distinct(subject, predicate, 
object)`. Unless the average number of objects for a `(subject, predicate)` 
pair (that has at least one object) is >2.3, the former is smaller. For the 
current use case, this average is certainly <1.1.
- simplicity: no arenas, easy mental model.

I see discussion of (something like) this option stalled around the index 
constructors, but I don't see a fundamental block there. The concept that the 
index constructor should be templated over would be `Iterable`. LMK 
if I'm missing something.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59407



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

r357197 - [Driver] Default Android toolchains to noexecstack.

[Dan Albert via cfe-commits]

Author: danalbert
Date: Thu Mar 28 11:08:28 2019
New Revision: 357197

URL: http://llvm.org/viewvc/llvm-project?rev=357197=rev
Log:
[Driver] Default Android toolchains to noexecstack.

Android does not support executable stacks.

Reviewers: srhines, pirama

Reviewed By: pirama

Subscribers: cfe-commits

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

Modified:
cfe/trunk/include/clang/Driver/ToolChain.h
cfe/trunk/lib/Driver/ToolChain.cpp
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
cfe/trunk/lib/Driver/ToolChains/Linux.cpp
cfe/trunk/lib/Driver/ToolChains/Linux.h
cfe/trunk/test/Driver/integrated-as.c
cfe/trunk/test/Driver/linux-as.c
cfe/trunk/test/Driver/linux-ld.c

Modified: cfe/trunk/include/clang/Driver/ToolChain.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/ToolChain.h?rev=357197=357196=357197=diff
==
--- cfe/trunk/include/clang/Driver/ToolChain.h (original)
+++ cfe/trunk/include/clang/Driver/ToolChain.h Thu Mar 28 11:08:28 2019
@@ -412,6 +412,9 @@ public:
   /// Test whether this toolchain defaults to PIE.
   virtual bool isPIEDefault() const = 0;
 
+  /// Test whether this toolchaind defaults to non-executable stacks.
+  virtual bool isNoExecStackDefault() const;
+
   /// Tests whether this toolchain forces its default for PIC, PIE or
   /// non-PIC.  If this returns true, any PIC related flags should be ignored
   /// and instead the results of \c isPICDefault() and \c isPIEDefault() are

Modified: cfe/trunk/lib/Driver/ToolChain.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChain.cpp?rev=357197=357196=357197=diff
==
--- cfe/trunk/lib/Driver/ToolChain.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChain.cpp Thu Mar 28 11:08:28 2019
@@ -112,6 +112,10 @@ bool ToolChain::useRelaxRelocations() co
   return ENABLE_X86_RELAX_RELOCATIONS;
 }
 
+bool ToolChain::isNoExecStackDefault() const {
+return false;
+}
+
 const SanitizerArgs& ToolChain::getSanitizerArgs() const {
   if (!SanitizerArguments.get())
 SanitizerArguments.reset(new SanitizerArgs(*this, Args));

Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=357197=357196=357197=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Thu Mar 28 11:08:28 2019
@@ -2053,6 +2053,7 @@ static void CollectArgsForIntegratedAsse
   bool TakeNextArg = false;
 
   bool UseRelaxRelocations = C.getDefaultToolChain().useRelaxRelocations();
+  bool UseNoExecStack = C.getDefaultToolChain().isNoExecStackDefault();
   const char *MipsTargetFeature = nullptr;
   for (const Arg *A :
Args.filtered(options::OPT_Wa_COMMA, options::OPT_Xassembler)) {
@@ -2134,7 +2135,7 @@ static void CollectArgsForIntegratedAsse
   } else if (Value == "--fatal-warnings") {
 CmdArgs.push_back("-massembler-fatal-warnings");
   } else if (Value == "--noexecstack") {
-CmdArgs.push_back("-mnoexecstack");
+UseNoExecStack = true;
   } else if (Value.startswith("-compress-debug-sections") ||
  Value.startswith("--compress-debug-sections") ||
  Value == "-nocompress-debug-sections" ||
@@ -2197,6 +2198,8 @@ static void CollectArgsForIntegratedAsse
   }
   if (UseRelaxRelocations)
 CmdArgs.push_back("--mrelax-relocations");
+  if (UseNoExecStack)
+CmdArgs.push_back("-mnoexecstack");
   if (MipsTargetFeature != nullptr) {
 CmdArgs.push_back("-target-feature");
 CmdArgs.push_back(MipsTargetFeature);

Modified: cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Gnu.cpp?rev=357197=357196=357197=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Gnu.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Gnu.cpp Thu Mar 28 11:08:28 2019
@@ -360,6 +360,11 @@ void tools::gnutools::Linker::ConstructJ
 CmdArgs.push_back("--no-dynamic-linker");
   }
 
+  if (ToolChain.isNoExecStackDefault()) {
+CmdArgs.push_back("-z");
+CmdArgs.push_back("noexecstack");
+  }
+
   if (Args.hasArg(options::OPT_rdynamic))
 CmdArgs.push_back("-export-dynamic");
 
@@ -609,6 +614,10 @@ void tools::gnutools::Assembler::Constru
 }
   }
 
+  if (getToolChain().isNoExecStackDefault()) {
+  CmdArgs.push_back("--noexecstack");
+  }
+
   switch (getToolChain().getArch()) {
   default:
 break;

Modified: cfe/trunk/lib/Driver/ToolChains/Linux.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Linux.cpp?rev=357197=357196=357197=diff

[PATCH] D59811: [clangd] Add fallback mode for code completion when compile command or preamble is not ready.

[Eric Liu via Phabricator via cfe-commits]

ioeric added a comment.

In D59811#1445701 , @ilya-biryukov 
wrote:

> I think this option should be configurable (and off by default) for the 
> transition period. A few reasons to do so:
>
> - Before we have an actual implementation of fallback completions, the 
> current behavior (waiting for the first preamble) actually seems like a 
> better experience than returning empty results.
> - Some clients do this kind of fallback on their own (e.g. VSCode), so until 
> we can provide actually semantically interesting results (anything more than 
> identifier-based really, e.g. keyword completions) we would be better off 
> keeping it off.
> - We can still test it if it's off by flipping the corresponding flag in the 
> test code.
> - Would make rollout easier (clients can flip the flag back and forth and 
> make sure it does not break stuff for them)


All very good points. Thanks! I've added an option.




Comment at: clangd/ClangdServer.cpp:197
   return CB(llvm::make_error());
+if (!IP->Preamble) {
+  vlog("File {0} is not ready for code completion. Enter fallback mode.",

ilya-biryukov wrote:
> This way we don't distinguish between the failure to build a preamble and the 
> fallback mode.
> Have you considered introducing a different callback instead to clearly 
> separate two cases for the clients?
> The code handling those would hardly have any similarities anyway, given that 
> the nature of those two cases is so different.
> 
> Would look something like this:
> ```
> /// When \p FallbackAction is not null, it would be called instead of \p 
> Action in cases when preamble is 
> /// not yet ready.
> void runWithPreamble(… Callback Action, Callback 
> FallbackAction);
> ```
> void runWithPreamble(… Callback Action, Callback 
> FallbackAction);
This is what I started with in the first iteration. The main problem is that we 
would need to bind `CB` to both actions; however, `CB` is not 
copyable/shareable by design, I think. This seems to leave us with the only 
option to handle everything in the same action. I thought this was fine because 
an parameter `AllowFallback` seems to be clear as well. I'm open to suggestions 
;)

> This way we don't distinguish between the failure to build a preamble and the 
> fallback mode.
I am assuming that a fallback for `runWithPreamble` is either compile command 
is missing/broken or preamble fails to be built. And in both cases, fallback 
mode could be useful. Do you think we should have more distinctions made here?



Comment at: clangd/TUScheduler.cpp:186
 
   std::shared_ptr getPossiblyStalePreamble() const;
+

ilya-biryukov wrote:
> Could we put the preamble and compile command into a single function?
> Getting them in the lock-step would mean they're always consistent, which is 
> a good thing.
Could you elaborate the advantages of keeping them consistent?

They seem to be updated and used relatively independently. Grouping them into 
one function seems to add more work to the call sites.  And it seems inevitable 
that we would some inconsistency between the two.



Comment at: clangd/TUScheduler.cpp:371
+  std::lock_guard Lock(Mutex);
+  this->CompileCommand = Inputs.CompileCommand;
+}

ilya-biryukov wrote:
> After this point the clients might start getting a new compile command and an 
> old preamble.
> This seems fine (the clients should be ready for this), but let's document it 
> in the methods that expose a compile command and a preamble.
> After this point the clients might start getting a new compile command and an 
> old preamble.
Did we have guarantee about this before? It seems that we could also have the 
similar situation.

> This seems fine (the clients should be ready for this), but let's document it 
> in the methods that expose a compile command and a preamble.
I added documentation for `getCurrentPreamble`; would it be the right place? 
`getPossiblyStalePreamble` seems self-explaining enough. And since compile 
command is always fresher than preamble, I think it would be fine without 
commenting?



Comment at: clangd/TUScheduler.cpp:918
+  // asynchronous mode, as TU update should finish before this is run.
+  if (!It->second->Worker->isFirstPreambleBuilt() && AllowFallback &&
+  PreambleTasks) {

ilya-biryukov wrote:
> It would be better to make a decision on whether to use a fallback mode in 
> the actual function scheduled on a different thread (i.e. inside the body of 
> `Task`).
> Imagine the preamble is being built right now and would finish before 
> scheduling this task. In that case we would have a chance to hit the 
> "preamble" if we make a decision in the body of the handler, but won't have 
> that chance in the current implementation.
It sounds like a very small window (~ms?) that we are trying to optimize for.  
Given that users type 

[PATCH] D59811: [clangd] Add fallback mode for code completion when compile command or preamble is not ready.

[Eric Liu via Phabricator via cfe-commits]

ioeric updated this revision to Diff 192683.
ioeric marked 9 inline comments as done.
ioeric added a comment.

- address review comments.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59811

Files:
  clangd/ClangdServer.cpp
  clangd/CodeComplete.h
  clangd/TUScheduler.cpp
  clangd/TUScheduler.h
  clangd/Threading.h
  clangd/tool/ClangdMain.cpp
  unittests/clangd/ClangdTests.cpp
  unittests/clangd/TUSchedulerTests.cpp

Index: unittests/clangd/TUSchedulerTests.cpp
===
--- unittests/clangd/TUSchedulerTests.cpp
+++ unittests/clangd/TUSchedulerTests.cpp
@@ -15,14 +15,13 @@
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include 
+#include 
 #include 
 
 namespace clang {
 namespace clangd {
 namespace {
 
-using ::testing::_;
-using ::testing::AllOf;
 using ::testing::AnyOf;
 using ::testing::Each;
 using ::testing::ElementsAre;
@@ -35,13 +34,18 @@
 
 class TUSchedulerTests : public ::testing::Test {
 protected:
-  ParseInputs getInputs(PathRef File, std::string Contents) {
+  FileUpdateInputs getInputs(PathRef File, std::string Contents) {
 ParseInputs Inputs;
-Inputs.CompileCommand = *CDB.getCompileCommand(File);
 Inputs.FS = buildTestFS(Files, Timestamps);
 Inputs.Contents = std::move(Contents);
 Inputs.Opts = ParseOptions();
-return Inputs;
+FileUpdateInputs UpdateInputs;
+UpdateInputs.InputSansCommand = std::move(Inputs);
+std::string FilePath = File;
+UpdateInputs.GetCompileCommand = [this, FilePath]() {
+  return *CDB.getCompileCommand(FilePath);
+};
+return UpdateInputs;
   }
 
   void updateWithCallback(TUScheduler , PathRef File,
@@ -72,7 +76,7 @@
   /// Schedule an update and call \p CB with the diagnostics it produces, if
   /// any. The TUScheduler should be created with captureDiags as a
   /// DiagsCallback for this to work.
-  void updateWithDiags(TUScheduler , PathRef File, ParseInputs Inputs,
+  void updateWithDiags(TUScheduler , PathRef File, FileUpdateInputs Inputs,
WantDiagnostics WD,
llvm::unique_function)> CB) {
 Path OrigFile = File.str();
@@ -245,7 +249,7 @@
 
 S.runWithPreamble("StaleRead", Path, TUScheduler::Stale,
   [&](Expected Pre) {
-ASSERT_TRUE(bool(Pre));
+EXPECT_TRUE((bool)Pre);
 assert(bool(Pre));
 EXPECT_THAT(includes(Pre->Preamble),
 ElementsAre(""));
@@ -397,8 +401,9 @@
 EXPECT_THAT(Context::current().get(NonceKey), Pointee(Nonce));
 
 ASSERT_TRUE((bool)AST);
-EXPECT_EQ(AST->Inputs.FS, Inputs.FS);
-EXPECT_EQ(AST->Inputs.Contents, Inputs.Contents);
+EXPECT_EQ(AST->Inputs.FS, Inputs.InputSansCommand.FS);
+EXPECT_EQ(AST->Inputs.Contents,
+  Inputs.InputSansCommand.Contents);
 
 std::lock_guard Lock(Mut);
 ++TotalASTReads;
@@ -415,7 +420,7 @@
 EXPECT_THAT(Context::current().get(NonceKey), Pointee(Nonce));
 
 ASSERT_TRUE((bool)Preamble);
-EXPECT_EQ(Preamble->Contents, Inputs.Contents);
+EXPECT_EQ(Preamble->Contents, Inputs.InputSansCommand.Contents);
 
 std::lock_guard Lock(Mut);
 ++TotalPreambleReads;
@@ -504,6 +509,7 @@
   )cpp";
   auto WithEmptyPreamble = R"cpp(int main() {})cpp";
   S.update(Foo, getInputs(Foo, WithPreamble), WantDiagnostics::Auto);
+
   S.runWithPreamble(
   "getNonEmptyPreamble", Foo, TUScheduler::Stale,
   [&](Expected Preamble) {
Index: unittests/clangd/ClangdTests.cpp
===
--- unittests/clangd/ClangdTests.cpp
+++ unittests/clangd/ClangdTests.cpp
@@ -13,8 +13,10 @@
 #include "Matchers.h"
 #include "SyncAPI.h"
 #include "TestFS.h"
+#include "Threading.h"
 #include "URI.h"
 #include "clang/Config/config.h"
+#include "clang/Sema/CodeCompleteConsumer.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/Support/Errc.h"
@@ -36,7 +38,6 @@
 namespace {
 
 using ::testing::ElementsAre;
-using ::testing::Eq;
 using ::testing::Field;
 using ::testing::Gt;
 using ::testing::IsEmpty;
@@ -1058,6 +1059,55 @@
   EXPECT_NE(Result, "");
 }
 
+TEST_F(ClangdVFSTest, FallbackWhenPreambleIsNotReady) {
+  MockFSProvider FS;
+  ErrorCheckingDiagConsumer DiagConsumer;
+  // Returns compile command only when notified.
+  class DelayedCompilationDatabase : public GlobalCompilationDatabase {
+  public:
+DelayedCompilationDatabase(Notification )
+: CanReturnCommand(CanReturnCommand) {}
+
+llvm::Optional
+getCompileCommand(PathRef File, ProjectInfo * = nullptr) const override {
+  

r357195 - [WebAssembly] Reland of rL356953 (4dcf3acce6)

[Sam Clegg via cfe-commits]

Author: sbc
Date: Thu Mar 28 10:45:18 2019
New Revision: 357195

URL: http://llvm.org/viewvc/llvm-project?rev=357195=rev
Log:
[WebAssembly] Reland of rL356953 (4dcf3acce6)

The previous patch was missing GetProgramPath() in the return value
of getLinkerPath().

See https://reviews.llvm.org/D59743

Modified:
cfe/trunk/lib/Driver/ToolChains/WebAssembly.cpp
cfe/trunk/lib/Driver/ToolChains/WebAssembly.h
cfe/trunk/test/Driver/wasm-toolchain.c
cfe/trunk/test/Driver/wasm-toolchain.cpp

Modified: cfe/trunk/lib/Driver/ToolChains/WebAssembly.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/WebAssembly.cpp?rev=357195=357194=357195=diff
==
--- cfe/trunk/lib/Driver/ToolChains/WebAssembly.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/WebAssembly.cpp Thu Mar 28 10:45:18 2019
@@ -12,6 +12,8 @@
 #include "clang/Driver/Driver.h"
 #include "clang/Driver/DriverDiagnostic.h"
 #include "clang/Driver/Options.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/Path.h"
 #include "llvm/Option/ArgList.h"
 
 using namespace clang::driver;
@@ -36,6 +38,25 @@ bool wasm::Linker::isLinkJob() const { r
 
 bool wasm::Linker::hasIntegratedCPP() const { return false; }
 
+std::string wasm::Linker::getLinkerPath(const ArgList ) const {
+  const ToolChain  = getToolChain();
+  if (const Arg* A = Args.getLastArg(options::OPT_fuse_ld_EQ)) {
+StringRef UseLinker = A->getValue();
+if (!UseLinker.empty()) {
+  if (llvm::sys::path::is_absolute(UseLinker) &&
+  llvm::sys::fs::can_execute(UseLinker))
+return UseLinker;
+
+  // Accept 'lld', and 'ld' as aliases for the default linker
+  if (UseLinker != "lld" && UseLinker != "ld")
+ToolChain.getDriver().Diag(diag::err_drv_invalid_linker_name)
+<< A->getAsString(Args);
+}
+  }
+
+  return ToolChain.GetProgramPath(ToolChain.getDefaultLinker());
+}
+
 void wasm::Linker::ConstructJob(Compilation , const JobAction ,
 const InputInfo ,
 const InputInfoList ,
@@ -43,7 +64,7 @@ void wasm::Linker::ConstructJob(Compilat
 const char *LinkingOutput) const {
 
   const ToolChain  = getToolChain();
-  const char *Linker = Args.MakeArgString(ToolChain.GetLinkerPath());
+  const char *Linker = Args.MakeArgString(getLinkerPath(Args));
   ArgStringList CmdArgs;
 
   if (Args.hasArg(options::OPT_s))

Modified: cfe/trunk/lib/Driver/ToolChains/WebAssembly.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/WebAssembly.h?rev=357195=357194=357195=diff
==
--- cfe/trunk/lib/Driver/ToolChains/WebAssembly.h (original)
+++ cfe/trunk/lib/Driver/ToolChains/WebAssembly.h Thu Mar 28 10:45:18 2019
@@ -23,6 +23,7 @@ public:
   explicit Linker(const ToolChain );
   bool isLinkJob() const override;
   bool hasIntegratedCPP() const override;
+  std::string getLinkerPath(const llvm::opt::ArgList ) const;
   void ConstructJob(Compilation , const JobAction ,
 const InputInfo , const InputInfoList ,
 const llvm::opt::ArgList ,

Modified: cfe/trunk/test/Driver/wasm-toolchain.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/wasm-toolchain.c?rev=357195=357194=357195=diff
==
--- cfe/trunk/test/Driver/wasm-toolchain.c (original)
+++ cfe/trunk/test/Driver/wasm-toolchain.c Thu Mar 28 10:45:18 2019
@@ -12,25 +12,25 @@
 
 // A basic C link command-line with unknown OS.
 
-// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown 
--sysroot=/foo -fuse-ld=wasm-ld %s 2>&1 | FileCheck -check-prefix=LINK %s
+// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown 
--sysroot=/foo %s 2>&1 | FileCheck -check-prefix=LINK %s
 // LINK: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
 // LINK: wasm-ld{{.*}}" "-L/foo/lib" "crt1.o" "[[temp]]" "-lc" 
"{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
 
 // A basic C link command-line with optimization with unknown OS.
 
-// RUN: %clang -### -O2 -no-canonical-prefixes -target wasm32-unknown-unknown 
--sysroot=/foo -fuse-ld=wasm-ld %s 2>&1 | FileCheck -check-prefix=LINK_OPT %s
+// RUN: %clang -### -O2 -no-canonical-prefixes -target wasm32-unknown-unknown 
--sysroot=/foo %s 2>&1 | FileCheck -check-prefix=LINK_OPT %s
 // LINK_OPT: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
 // LINK_OPT: wasm-ld{{.*}}" "-L/foo/lib" "crt1.o" "[[temp]]" "-lc" 
"{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
 
 // A basic C link command-line with known OS.
 
-// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-wasi-musl 
--sysroot=/foo -fuse-ld=wasm-ld %s 2>&1 | FileCheck -check-prefix=LINK_KNOWN %s
+// RUN: %clang -### -no-canonical-prefixes -target 

r357187 - Make helper functions static. NFC.

[Benjamin Kramer via cfe-commits]

Author: d0k
Date: Thu Mar 28 10:18:42 2019
New Revision: 357187

URL: http://llvm.org/viewvc/llvm-project?rev=357187=rev
Log:
Make helper functions static. NFC.

Modified:
cfe/trunk/lib/CodeGen/BackendUtil.cpp
cfe/trunk/lib/Sema/SemaOpenMP.cpp
cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp

Modified: cfe/trunk/lib/CodeGen/BackendUtil.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/BackendUtil.cpp?rev=357187=357186=357187=diff
==
--- cfe/trunk/lib/CodeGen/BackendUtil.cpp (original)
+++ cfe/trunk/lib/CodeGen/BackendUtil.cpp Thu Mar 28 10:18:42 2019
@@ -914,9 +914,10 @@ static PassBuilder::OptimizationLevel ma
   }
 }
 
-void addSanitizersAtO0(ModulePassManager , const Triple ,
-   const LangOptions ,
-   const CodeGenOptions ) {
+static void addSanitizersAtO0(ModulePassManager ,
+  const Triple ,
+  const LangOptions ,
+  const CodeGenOptions ) {
   if (LangOpts.Sanitize.has(SanitizerKind::Address)) {
 MPM.addPass(RequireAnalysisPass());
 bool Recover = CodeGenOpts.SanitizeRecover.has(SanitizerKind::Address);

Modified: cfe/trunk/lib/Sema/SemaOpenMP.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOpenMP.cpp?rev=357187=357186=357187=diff
==
--- cfe/trunk/lib/Sema/SemaOpenMP.cpp (original)
+++ cfe/trunk/lib/Sema/SemaOpenMP.cpp Thu Mar 28 10:18:42 2019
@@ -13352,10 +13352,11 @@ static bool checkMapConflicts(
 
 // Look up the user-defined mapper given the mapper name and mapped type, and
 // build a reference to it.
-ExprResult buildUserDefinedMapperRef(Sema , Scope *S,
- CXXScopeSpec ,
- const DeclarationNameInfo ,
- QualType Type, Expr *UnresolvedMapper) {
+static ExprResult buildUserDefinedMapperRef(Sema , Scope *S,
+CXXScopeSpec ,
+const DeclarationNameInfo 
,
+QualType Type,
+Expr *UnresolvedMapper) {
   if (MapperIdScopeSpec.isInvalid())
 return ExprError();
   // Find all user-defined mappers with the given MapperId.

Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp?rev=357187=357186=357187=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp Thu Mar 28 
10:18:42 2019
@@ -161,8 +161,8 @@ const Expr *bugreporter::getDerefExpr(co
 /// are the immediate snapshots of the tracked region's bindings within the
 /// node's respective states but not really checking that these snapshots
 /// actually contain the same set of bindings.
-bool hasVisibleUpdate(const ExplodedNode *LeftNode, SVal LeftVal,
-  const ExplodedNode *RightNode, SVal RightVal) {
+static bool hasVisibleUpdate(const ExplodedNode *LeftNode, SVal LeftVal,
+ const ExplodedNode *RightNode, SVal RightVal) {
   if (LeftVal == RightVal)
 return true;
 


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

[PATCH] D59814: [Testing] Move clangd::Annotations to llvm testing support

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

I've added a FIXME to the class.
Also want to add some basic tests before landing this.




Comment at: clang-tools-extra/unittests/clangd/Annotations.h:8
 
//===--===//
-//
-// Annotations lets you mark points and ranges inside source code, for tests:
-//
-//Annotations Example(R"cpp(
-//   int complete() { x.pri^ }  // ^ indicates a point
-//   void err() { [["hello" == 42]]; }  // [[this is a range]]
-//   $definition^class Foo{};   // points can be named: 
"definition"
-//   $fail[[static_assert(false, "")]]  // ranges can be named too: "fail"
-//)cpp");
-//
-//StringRef Code = Example.code();  // annotations stripped.
-//std::vector PP = Example.points();  // all unnamed points
-//Position P = Example.point(); // there must be exactly 
one
-//Range R = Example.range("fail");  // find named ranges
-//
-// Points/ranges are coordinates into `code()` which is stripped of 
annotations.
-//
-// Ranges may be nested (and points can be inside ranges), but there's no way
-// to define general overlapping ranges.
-//
+// A clangd-specific version of llvm/Testing/Support/Annotations.h, replaces
+// offsets and offset-based ranges with types from the LSP protocol.

sammccall wrote:
> The choice to shadow the base methods is interesting. I guess we can always 
> use llvm::Annotations if we want byte offsets (I know there are some), not 
> sure if we ever want a mix.
> Anyway this is nice and clean, and minimizes the diff. We can change later if 
> we care.
Exactly my thoughts. I don't expect us to have places that need both the 
offsets and the positions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59814



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

[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros

[Manuel Klimek via Phabricator via cfe-commits]

klimek added inline comments.



Comment at: lib/Format/WhitespaceManager.cpp:520
+
+  AlignMacroSequence(StartOfSequence, EndOfSequence, MinColumn, MaxColumn,
+ FoundMatchOnLine, AlignMacrosMatches, Changes);

Why are we calling AlignMacroSequence(0, 0, ...) here?



Comment at: lib/Format/WhitespaceManager.cpp:524
+  unsigned I = 0;
+  for (unsigned E = Changes.size(); I != E; ++I) {
+if (Changes[I].NewlinesBefore != 0) {

I still think the code in this loop either needs significantly more comments to 
explain what's going on, or needs to be changed to be more straight forward:
What I don't understand is why we're calling AlignMacroSequence potentially 
multiple times, and especially what things like the if in line 541 are for.


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

https://reviews.llvm.org/D28462



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

[PATCH] D59814: [Testing] Move clangd::Annotations to llvm testing support

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 192673.
ilya-biryukov marked 4 inline comments as done.
ilya-biryukov added a comment.

- Move Range into the body of Annotations
- Use triple-slash comments
- Added a FIXME that we might want to change the syntax
- Move the doc comment to the class


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59814

Files:
  clang-tools-extra/unittests/clangd/Annotations.cpp
  clang-tools-extra/unittests/clangd/Annotations.h
  clang/unittests/Sema/CMakeLists.txt
  clang/unittests/Sema/CodeCompleteTest.cpp
  llvm/include/llvm/Testing/Support/Annotations.h
  llvm/lib/Testing/Support/Annotations.cpp
  llvm/lib/Testing/Support/CMakeLists.txt

Index: llvm/lib/Testing/Support/CMakeLists.txt
===
--- llvm/lib/Testing/Support/CMakeLists.txt
+++ llvm/lib/Testing/Support/CMakeLists.txt
@@ -2,6 +2,7 @@
 add_definitions(-DGTEST_HAS_TR1_TUPLE=0)
 
 add_llvm_library(LLVMTestingSupport
+  Annotations.cpp
   Error.cpp
   SupportHelpers.cpp
 
Index: llvm/lib/Testing/Support/Annotations.cpp
===
--- llvm/lib/Testing/Support/Annotations.cpp
+++ llvm/lib/Testing/Support/Annotations.cpp
@@ -6,11 +6,12 @@
 //
 //===--===//
 
-#include "Annotations.h"
-#include "SourceCode.h"
+#include "llvm/Testing/Support/Annotations.h"
 
-namespace clang {
-namespace clangd {
+#include "llvm/ADT/StringExtras.h"
+#include "llvm/Support/raw_ostream.h"
+
+using namespace llvm;
 
 // Crash if the assertion fails, printing the message and testcase.
 // More elegant error handling isn't needed for unit tests.
@@ -22,30 +23,31 @@
 }
 
 Annotations::Annotations(llvm::StringRef Text) {
-  auto Here = [this] { return offsetToPosition(Code, Code.size()); };
   auto Require = [Text](bool Assertion, const char *Msg) {
 require(Assertion, Msg, Text);
   };
   llvm::Optional Name;
-  llvm::SmallVector, 8> OpenRanges;
+  llvm::SmallVector, 8> OpenRanges;
 
   Code.reserve(Text.size());
   while (!Text.empty()) {
 if (Text.consume_front("^")) {
-  Points[Name.getValueOr("")].push_back(Here());
-  Name = None;
+  Points[Name.getValueOr("")].push_back(Code.size());
+  Name = llvm::None;
   continue;
 }
 if (Text.consume_front("[[")) {
-  OpenRanges.emplace_back(Name.getValueOr(""), Here());
-  Name = None;
+  OpenRanges.emplace_back(Name.getValueOr(""), Code.size());
+  Name = llvm::None;
   continue;
 }
 Require(!Name, "$name should be followed by ^ or [[");
 if (Text.consume_front("]]")) {
   Require(!OpenRanges.empty(), "unmatched ]]");
-  Ranges[OpenRanges.back().first].push_back(
-  {OpenRanges.back().second, Here()});
+  Range R;
+  R.Begin = OpenRanges.back().second;
+  R.End = Code.size();
+  Ranges[OpenRanges.back().first].push_back(R);
   OpenRanges.pop_back();
   continue;
 }
@@ -61,26 +63,27 @@
   Require(OpenRanges.empty(), "unmatched [[");
 }
 
-Position Annotations::point(llvm::StringRef Name) const {
+size_t Annotations::point(llvm::StringRef Name) const {
   auto I = Points.find(Name);
   require(I != Points.end() && I->getValue().size() == 1,
   "expected exactly one point", Code);
   return I->getValue()[0];
 }
-std::vector Annotations::points(llvm::StringRef Name) const {
+
+std::vector Annotations::points(llvm::StringRef Name) const {
   auto P = Points.lookup(Name);
   return {P.begin(), P.end()};
 }
-Range Annotations::range(llvm::StringRef Name) const {
+
+Annotations::Range Annotations::range(llvm::StringRef Name) const {
   auto I = Ranges.find(Name);
   require(I != Ranges.end() && I->getValue().size() == 1,
   "expected exactly one range", Code);
   return I->getValue()[0];
 }
-std::vector Annotations::ranges(llvm::StringRef Name) const {
+
+std::vector
+Annotations::ranges(llvm::StringRef Name) const {
   auto R = Ranges.lookup(Name);
   return {R.begin(), R.end()};
 }
-
-} // namespace clangd
-} // namespace clang
Index: llvm/include/llvm/Testing/Support/Annotations.h
===
--- /dev/null
+++ llvm/include/llvm/Testing/Support/Annotations.h
@@ -0,0 +1,81 @@
+//===--- Annotations.h - Annotated source code for tests -*- 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
+//
+//===--===//
+#ifndef LLVM_TESTING_SUPPORT_ANNOTATIONS_H
+#define LLVM_TESTING_SUPPORT_ANNOTATIONS_H
+
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringMap.h"
+#include "llvm/ADT/StringRef.h"
+#include 
+
+namespace llvm 

[PATCH] D59935: Disable warnings when indexing as a standalone action.

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
sammccall marked an inline comment as done.
Closed by commit rL357186: Disable warnings when indexing as a standalone 
action. (authored by sammccall, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D59935?vs=192651=192672#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D59935

Files:
  clang-tools-extra/trunk/clangd/index/IndexAction.cpp
  clang-tools-extra/trunk/unittests/clangd/IndexActionTests.cpp


Index: clang-tools-extra/trunk/unittests/clangd/IndexActionTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/IndexActionTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/IndexActionTests.cpp
@@ -29,6 +29,8 @@
 
 MATCHER_P(HasDigest, Digest, "") { return arg.Digest == Digest; }
 
+MATCHER_P(HasName, Name, "") { return arg.Name == Name; }
+
 MATCHER(HasSameURI, "") {
   llvm::StringRef URI = testing::get<0>(arg);
   const std::string  = testing::get<1>(arg);
@@ -43,6 +45,7 @@
 
 void checkNodesAreInitialized(const IndexFileIn ,
   const std::vector ) {
+  ASSERT_TRUE(IndexFile.Sources);
   EXPECT_THAT(Paths.size(), IndexFile.Sources->size());
   for (llvm::StringRef Path : Paths) {
 auto URI = toUri(Path);
@@ -224,6 +227,27 @@
 HasDigest(digest(HeaderCode));
 }
 
+TEST_F(IndexActionTest, NoWarnings) {
+  std::string MainFilePath = testPath("main.cpp");
+  std::string MainCode = R"cpp(
+  void foo(int x) {
+if (x = 1) // -Wparentheses
+  return;
+if (x = 1) // -Wparentheses
+  return;
+  }
+  void bar() {}
+  )cpp";
+  addFile(MainFilePath, MainCode);
+  // We set -ferror-limit so the warning-promoted-to-error would be fatal.
+  // This would cause indexing to stop (if warnings weren't disabled).
+  IndexFileIn IndexFile = runIndexingAction(
+  MainFilePath, {"-ferror-limit=1", "-Wparentheses", "-Werror"});
+  ASSERT_TRUE(IndexFile.Sources);
+  ASSERT_NE(0u, IndexFile.Sources->size());
+  EXPECT_THAT(*IndexFile.Symbols, ElementsAre(HasName("foo"), HasName("bar")));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/trunk/clangd/index/IndexAction.cpp
===
--- clang-tools-extra/trunk/clangd/index/IndexAction.cpp
+++ clang-tools-extra/trunk/clangd/index/IndexAction.cpp
@@ -135,6 +135,11 @@
   bool BeginInvocation(CompilerInstance ) override {
 // We want all comments, not just the doxygen ones.
 CI.getLangOpts().CommentOpts.ParseAllComments = true;
+// Index the whole file even if there are warnings and -Werror is set.
+// Avoids some analyses too. Set in two places as we're late to the party.
+CI.getDiagnosticOpts().IgnoreWarnings = true;
+CI.getDiagnostics().setIgnoreAllWarnings(true);
+
 return WrapperFrontendAction::BeginInvocation(CI);
   }
 


Index: clang-tools-extra/trunk/unittests/clangd/IndexActionTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/IndexActionTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/IndexActionTests.cpp
@@ -29,6 +29,8 @@
 
 MATCHER_P(HasDigest, Digest, "") { return arg.Digest == Digest; }
 
+MATCHER_P(HasName, Name, "") { return arg.Name == Name; }
+
 MATCHER(HasSameURI, "") {
   llvm::StringRef URI = testing::get<0>(arg);
   const std::string  = testing::get<1>(arg);
@@ -43,6 +45,7 @@
 
 void checkNodesAreInitialized(const IndexFileIn ,
   const std::vector ) {
+  ASSERT_TRUE(IndexFile.Sources);
   EXPECT_THAT(Paths.size(), IndexFile.Sources->size());
   for (llvm::StringRef Path : Paths) {
 auto URI = toUri(Path);
@@ -224,6 +227,27 @@
 HasDigest(digest(HeaderCode));
 }
 
+TEST_F(IndexActionTest, NoWarnings) {
+  std::string MainFilePath = testPath("main.cpp");
+  std::string MainCode = R"cpp(
+  void foo(int x) {
+if (x = 1) // -Wparentheses
+  return;
+if (x = 1) // -Wparentheses
+  return;
+  }
+  void bar() {}
+  )cpp";
+  addFile(MainFilePath, MainCode);
+  // We set -ferror-limit so the warning-promoted-to-error would be fatal.
+  // This would cause indexing to stop (if warnings weren't disabled).
+  IndexFileIn IndexFile = runIndexingAction(
+  MainFilePath, {"-ferror-limit=1", "-Wparentheses", "-Werror"});
+  ASSERT_TRUE(IndexFile.Sources);
+  ASSERT_NE(0u, IndexFile.Sources->size());
+  EXPECT_THAT(*IndexFile.Symbols, ElementsAre(HasName("foo"), HasName("bar")));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/trunk/clangd/index/IndexAction.cpp

[PATCH] D54881: [clang-format] Prevent Clang-Format from editing leading whitespace on lines outside of the format range

[Manuel Klimek via Phabricator via cfe-commits]

klimek added a comment.

I just verified why this doesn't break for our CI:
The trick is that if the indent is actually off (as opposed to being correct, 
but tab vs spaces), we do re-indent until we find an indent that's correct.
The fix that would make me happier, I think, is to do the same thing here:
Basically, when we expand the range until nothing changes, do not only check 
whether the indent is correct, but also whether we'd change the spaces in the 
indent.
WDYT?


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

https://reviews.llvm.org/D54881



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

[clang-tools-extra] r357186 - Disable warnings when indexing as a standalone action.

[Sam McCall via cfe-commits]

Author: sammccall
Date: Thu Mar 28 10:07:28 2019
New Revision: 357186

URL: http://llvm.org/viewvc/llvm-project?rev=357186=rev
Log:
Disable warnings when indexing as a standalone action.

Summary:
- we don't record the warnings at all
- we don't want to stop indexing if we hit error-limit due to warnings
- this allows some analyses to be skipped which can save some CPU

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

Reviewers: hokein

Subscribers: ilya-biryukov, ioeric, jkorous, arphaman, kadircet, cfe-commits

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clangd/index/IndexAction.cpp
clang-tools-extra/trunk/unittests/clangd/IndexActionTests.cpp

Modified: clang-tools-extra/trunk/clangd/index/IndexAction.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/IndexAction.cpp?rev=357186=357185=357186=diff
==
--- clang-tools-extra/trunk/clangd/index/IndexAction.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/IndexAction.cpp Thu Mar 28 10:07:28 
2019
@@ -135,6 +135,11 @@ public:
   bool BeginInvocation(CompilerInstance ) override {
 // We want all comments, not just the doxygen ones.
 CI.getLangOpts().CommentOpts.ParseAllComments = true;
+// Index the whole file even if there are warnings and -Werror is set.
+// Avoids some analyses too. Set in two places as we're late to the party.
+CI.getDiagnosticOpts().IgnoreWarnings = true;
+CI.getDiagnostics().setIgnoreAllWarnings(true);
+
 return WrapperFrontendAction::BeginInvocation(CI);
   }
 

Modified: clang-tools-extra/trunk/unittests/clangd/IndexActionTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/IndexActionTests.cpp?rev=357186=357185=357186=diff
==
--- clang-tools-extra/trunk/unittests/clangd/IndexActionTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/IndexActionTests.cpp Thu Mar 28 
10:07:28 2019
@@ -29,6 +29,8 @@ MATCHER(IsTU, "") { return arg.IsTU; }
 
 MATCHER_P(HasDigest, Digest, "") { return arg.Digest == Digest; }
 
+MATCHER_P(HasName, Name, "") { return arg.Name == Name; }
+
 MATCHER(HasSameURI, "") {
   llvm::StringRef URI = testing::get<0>(arg);
   const std::string  = testing::get<1>(arg);
@@ -43,6 +45,7 @@ IncludesAre(const std::vector ) {
+  ASSERT_TRUE(IndexFile.Sources);
   EXPECT_THAT(Paths.size(), IndexFile.Sources->size());
   for (llvm::StringRef Path : Paths) {
 auto URI = toUri(Path);
@@ -224,6 +227,27 @@ TEST_F(IndexActionTest, IncludeGraphDyna
 HasDigest(digest(HeaderCode));
 }
 
+TEST_F(IndexActionTest, NoWarnings) {
+  std::string MainFilePath = testPath("main.cpp");
+  std::string MainCode = R"cpp(
+  void foo(int x) {
+if (x = 1) // -Wparentheses
+  return;
+if (x = 1) // -Wparentheses
+  return;
+  }
+  void bar() {}
+  )cpp";
+  addFile(MainFilePath, MainCode);
+  // We set -ferror-limit so the warning-promoted-to-error would be fatal.
+  // This would cause indexing to stop (if warnings weren't disabled).
+  IndexFileIn IndexFile = runIndexingAction(
+  MainFilePath, {"-ferror-limit=1", "-Wparentheses", "-Werror"});
+  ASSERT_TRUE(IndexFile.Sources);
+  ASSERT_NE(0u, IndexFile.Sources->size());
+  EXPECT_THAT(*IndexFile.Symbols, ElementsAre(HasName("foo"), HasName("bar")));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang


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

[PATCH] D59873: Add additional mangling for struct members of non trivial structs

[Shoaib Meenai via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL357184: [CodeGen] Add additional mangling for struct members 
of non trivial structs (authored by smeenai, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D59873?vs=192652=192671#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D59873

Files:
  cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp
  cfe/trunk/test/CodeGenObjC/nontrivial-c-struct-within-struct-name.m
  cfe/trunk/test/CodeGenObjC/strong-in-c-struct.m

Index: cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp
===
--- cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp
+++ cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp
@@ -139,8 +139,8 @@
 //  ::=  ["_" ]
 //  ::= +
 //  ::=  | 
-//  ::=  |  |
-//   
+//  ::= "_S"  |
+//| 
 //  ::= "_AB"  "s"  "n"
 //  "_AE"
 //  ::= 
@@ -175,6 +175,7 @@
   void visitStruct(QualType QT, const FieldDecl *FD,
CharUnits CurStructOffset) {
 CharUnits FieldOffset = CurStructOffset + asDerived().getFieldOffset(FD);
+appendStr("_S");
 asDerived().visitStructFields(QT, FieldOffset);
   }
 
Index: cfe/trunk/test/CodeGenObjC/nontrivial-c-struct-within-struct-name.m
===
--- cfe/trunk/test/CodeGenObjC/nontrivial-c-struct-within-struct-name.m
+++ cfe/trunk/test/CodeGenObjC/nontrivial-c-struct-within-struct-name.m
@@ -0,0 +1,44 @@
+// RUN: %clang_cc1 -triple arm64-apple-ios11 -fobjc-arc -emit-llvm -o - %s | FileCheck %s
+
+@class I;
+
+typedef struct {
+  I *name;
+} Foo;
+
+typedef struct {
+  Foo foo;
+} Bar;
+
+typedef struct {
+  Bar bar;
+} Baz;
+
+I *getI();
+
+void f() {
+  Foo foo = {getI()};
+  Bar bar = {foo};
+  Baz baz = {bar};
+}
+
+// CHECK: define linkonce_odr hidden void @__destructor_8_S_S_s0(i8** %[[DST:.*]])
+// CHECK: %[[DST_ADDR:.*]] = alloca i8**, align 8
+// CHECK: store i8** %[[DST]], i8*** %[[DST_ADDR]], align 8
+// CHECK: %[[V0:.*]] = load i8**, i8*** %[[DST_ADDR]], align 8
+// CHECK: call void @__destructor_8_S_s0(i8** %[[V0]])
+// CHECK: ret void
+
+// CHECK: define linkonce_odr hidden void @__destructor_8_S_s0(i8** %[[DST:.*]])
+// CHECK: %[[DST_ADDR:.*]] = alloca i8**, align 8
+// CHECK: store i8** %[[DST]], i8*** %[[DST_ADDR]], align 8
+// CHECK: %[[V0:.*]] = load i8**, i8*** %[[DST_ADDR]], align 8
+// CHECK: call void @__destructor_8_s0(i8** %[[V0]])
+// CHECK: ret void
+
+// CHECK: define linkonce_odr hidden void @__destructor_8_s0(i8** %dst)
+// CHECK: %[[DST_ADDR:.*]] = alloca i8**, align 8
+// CHECK: store i8** %[[DST]], i8*** %[[DST_ADDR]], align 8
+// CHECK: %[[V0:.*]] = load i8**, i8*** %[[DST_ADDR]], align 8
+// CHECK: call void @llvm.objc.storeStrong(i8** %[[V0]], i8* null)
+// CHECK: ret void
Index: cfe/trunk/test/CodeGenObjC/strong-in-c-struct.m
===
--- cfe/trunk/test/CodeGenObjC/strong-in-c-struct.m
+++ cfe/trunk/test/CodeGenObjC/strong-in-c-struct.m
@@ -89,12 +89,12 @@
 // CHECK: define void @test_constructor_destructor_StrongOuter()
 // CHECK: %[[T:.*]] = alloca %[[STRUCT_STRONGOUTER:.*]], align 8
 // CHECK: %[[V0:.*]] = bitcast %[[STRUCT_STRONGOUTER]]* %[[T]] to i8**
-// CHECK: call void @__default_constructor_8_s16_s24(i8** %[[V0]])
+// CHECK: call void @__default_constructor_8_S_s16_s24(i8** %[[V0]])
 // CHECK: %[[V1:.*]] = bitcast %[[STRUCT_STRONGOUTER]]* %[[T]] to i8**
-// CHECK: call void @__destructor_8_s16_s24(i8** %[[V1]])
+// CHECK: call void @__destructor_8_S_s16_s24(i8** %[[V1]])
 // CHECK: ret void
 
-// CHECK: define linkonce_odr hidden void @__default_constructor_8_s16_s24(i8** %[[DST:.*]])
+// CHECK: define linkonce_odr hidden void @__default_constructor_8_S_s16_s24(i8** %[[DST:.*]])
 // CHECK: %[[DST_ADDR:.*]] = alloca i8**, align 8
 // CHECK: store i8** %[[DST]], i8*** %[[DST_ADDR]], align 8
 // CHECK: %[[V0:.*]] = load i8**, i8*** %[[DST_ADDR]], align 8
@@ -117,7 +117,7 @@
 // CHECK: call void @llvm.memset.p0i8.i64(i8* align 8 %[[V4]], i8 0, i64 8, i1 false)
 // CHECK: ret void
 
-// CHECK: define linkonce_odr hidden void @__destructor_8_s16_s24(i8** %[[DST:.*]])
+// CHECK: define linkonce_odr hidden void @__destructor_8_S_s16_s24(i8** %[[DST:.*]])
 // CHECK: %[[DST_ADDR:.*]] = alloca i8**, align 8
 // CHECK: store i8** %[[DST]], i8*** %[[DST_ADDR]], align 8
 // CHECK: %[[V0:.*]] = load i8**, i8*** %[[DST_ADDR]], align 8
@@ -149,12 +149,12 @@
 // CHECK: %[[V0:.*]] = load %[[STRUCT_STRONGOUTER]]*, %[[STRUCT_STRONGOUTER]]** %[[S_ADDR]], align 8
 // CHECK: %[[V1:.*]] = bitcast %[[STRUCT_STRONGOUTER]]* %[[T]] to i8**
 // CHECK: %[[V2:.*]] = bitcast %[[STRUCT_STRONGOUTER]]* %[[V0]] to i8**
-// CHECK: call void 

[PATCH] D59725: Additions to creduce script

[Amy Huang via Phabricator via cfe-commits]

akhuang updated this revision to Diff 192669.
akhuang marked 3 inline comments as done.
akhuang added a comment.

Add preprocessing with clang -E only;
use `with` for opening files


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

https://reviews.llvm.org/D59725

Files:
  clang/utils/creduce-clang-crash.py

Index: clang/utils/creduce-clang-crash.py
===
--- clang/utils/creduce-clang-crash.py
+++ clang/utils/creduce-clang-crash.py
@@ -1,8 +1,14 @@
 #!/usr/bin/env python
 """Calls C-Reduce to create a minimal reproducer for clang crashes.
+
+Output files:
+  *.reduced.sh -- crash reproducer with minimal arguments
+  *.reduced.cpp -- the reduced file
+  *.test.sh -- interestingness test for C-Reduce
 """
 
-from argparse import ArgumentParser
+from __future__ import print_function
+from argparse import ArgumentParser, RawTextHelpFormatter
 import os
 import re
 import stat
@@ -15,10 +21,14 @@
 from distutils.spawn import find_executable
 
 verbose = False
-llvm_bin = None
 creduce_cmd = None
+clang_cmd = None
 not_cmd = None
 
+def verbose_print(*args, **kwargs):
+  if verbose:
+print(*args, **kwargs)
+
 def check_file(fname):
   if not os.path.isfile(fname):
 sys.exit("ERROR: %s does not exist" % (fname))
@@ -33,166 +43,340 @@
 cmd = find_executable(cmd_path)
 if cmd:
   return cmd
-sys.exit("ERROR: executable %s not found" % (cmd_path))
+sys.exit("ERROR: executable `%s` not found" % (cmd_path))
 
   cmd = find_executable(cmd_name, path=cmd_dir)
   if cmd:
 return cmd
-  sys.exit("ERROR: %s not found in %s" % (cmd_name, cmd_dir))
 
-def quote_cmd(cmd):
-  return ' '.join(arg if arg.startswith('$') else pipes.quote(arg)
-  for arg in cmd)
-
-def get_crash_cmd(crash_script):
-  with open(crash_script) as f:
-# Assume clang call is on the last line of the script
-line = f.readlines()[-1]
-cmd = shlex.split(line)
-
-# Overwrite the script's clang with the user's clang path
-new_clang = check_cmd('clang', llvm_bin)
-cmd[0] = pipes.quote(new_clang)
-return cmd
+  if not cmd_dir:
+cmd_dir = "$PATH"
+  sys.exit("ERROR: `%s` not found in %s" % (cmd_name, cmd_dir))
 
-def has_expected_output(crash_cmd, expected_output):
-  p = subprocess.Popen(crash_cmd,
-   stdout=subprocess.PIPE,
-   stderr=subprocess.STDOUT)
-  crash_output, _ = p.communicate()
-  return all(msg in crash_output for msg in expected_output)
-
-def get_expected_output(crash_cmd):
-  p = subprocess.Popen(crash_cmd,
-   stdout=subprocess.PIPE,
-   stderr=subprocess.STDOUT)
-  crash_output, _ = p.communicate()
-
-  # If there is an assertion failure, use that;
-  # otherwise use the last five stack trace functions
-  assertion_re = r'Assertion `([^\']+)\' failed'
-  assertion_match = re.search(assertion_re, crash_output)
-  if assertion_match:
-return [assertion_match.group(1)]
-  else:
-stacktrace_re = r'#[0-9]+\s+0[xX][0-9a-fA-F]+\s*([^(]+)\('
-matches = re.findall(stacktrace_re, crash_output)
-return matches[-5:]
-
-def write_interestingness_test(testfile, crash_cmd, expected_output,
-   file_to_reduce):
-  filename = os.path.basename(file_to_reduce)
-  if filename not in crash_cmd:
-sys.exit("ERROR: expected %s to be in the crash command" % filename)
-
-  # Replace all instances of file_to_reduce with a command line variable
-  output = ['#!/bin/bash',
-'if [ -z "$1" ] ; then',
-'  f=%s' % (pipes.quote(filename)),
-'else',
-'  f="$1"',
-'fi']
-  cmd = ['$f' if s == filename else s for s in crash_cmd]
-
-  output.append('%s --crash %s >& t.log || exit 1' % (pipes.quote(not_cmd),
-  quote_cmd(cmd)))
-
-  for msg in expected_output:
-output.append('grep %s t.log || exit 1' % pipes.quote(msg))
-
-  with open(testfile, 'w') as f:
-f.write('\n'.join(output))
-  os.chmod(testfile, os.stat(testfile).st_mode | stat.S_IEXEC)
-
-def check_interestingness(testfile, file_to_reduce):
-  testfile = os.path.abspath(testfile)
-
-  # Check that the test considers the original file interesting
-  with open(os.devnull, 'w') as devnull:
-returncode = subprocess.call(testfile, stdout=devnull)
-  if returncode:
-sys.exit("The interestingness test does not pass for the original file.")
-
-  # Check that an empty file is not interesting
-  _, empty_file = tempfile.mkstemp()
-  with open(os.devnull, 'w') as devnull:
-returncode = subprocess.call([testfile, empty_file], stdout=devnull)
-  os.remove(empty_file)
-  if not returncode:
-sys.exit("The interestingness test passes for an empty file.")
-
-def clang_preprocess(file_to_reduce, crash_cmd, expected_output):
-  _, tmpfile = tempfile.mkstemp()
-  shutil.copy(file_to_reduce, tmpfile)
-
-  cmd = crash_cmd + 

r357184 - [CodeGen] Add additional mangling for struct members of non trivial structs

[Shoaib Meenai via cfe-commits]

Author: smeenai
Date: Thu Mar 28 10:01:20 2019
New Revision: 357184

URL: http://llvm.org/viewvc/llvm-project?rev=357184=rev
Log:
[CodeGen] Add additional mangling for struct members of non trivial structs

In https://bugs.llvm.org/show_bug.cgi?id=41206 we observe bad codegen
when embedding a non-trivial C struct within a C struct. This is due to
the fact that name mangling for non-trivial structs marks the two
structs as identical. This diff contains a fix for this issue.

Patch by Dan Zimmerman .

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

Added:
cfe/trunk/test/CodeGenObjC/nontrivial-c-struct-within-struct-name.m
Modified:
cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp
cfe/trunk/test/CodeGenObjC/strong-in-c-struct.m

Modified: cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp?rev=357184=357183=357184=diff
==
--- cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp Thu Mar 28 10:01:20 2019
@@ -139,8 +139,8 @@ struct CopyStructVisitor : StructVisitor
 //  ::=  ["_" ]
 //  ::= +
 //  ::=  | 
-//  ::=  |  
|
-//   
+//  ::= "_S"  |
+//| 
 //  ::= "_AB"  "s"  "n"
 //  "_AE"
 //  ::= 
@@ -175,6 +175,7 @@ template  struct GenFuncN
   void visitStruct(QualType QT, const FieldDecl *FD,
CharUnits CurStructOffset) {
 CharUnits FieldOffset = CurStructOffset + asDerived().getFieldOffset(FD);
+appendStr("_S");
 asDerived().visitStructFields(QT, FieldOffset);
   }
 

Added: cfe/trunk/test/CodeGenObjC/nontrivial-c-struct-within-struct-name.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjC/nontrivial-c-struct-within-struct-name.m?rev=357184=auto
==
--- cfe/trunk/test/CodeGenObjC/nontrivial-c-struct-within-struct-name.m (added)
+++ cfe/trunk/test/CodeGenObjC/nontrivial-c-struct-within-struct-name.m Thu Mar 
28 10:01:20 2019
@@ -0,0 +1,44 @@
+// RUN: %clang_cc1 -triple arm64-apple-ios11 -fobjc-arc -emit-llvm -o - %s | 
FileCheck %s
+
+@class I;
+
+typedef struct {
+  I *name;
+} Foo;
+
+typedef struct {
+  Foo foo;
+} Bar;
+
+typedef struct {
+  Bar bar;
+} Baz;
+
+I *getI();
+
+void f() {
+  Foo foo = {getI()};
+  Bar bar = {foo};
+  Baz baz = {bar};
+}
+
+// CHECK: define linkonce_odr hidden void @__destructor_8_S_S_s0(i8** 
%[[DST:.*]])
+// CHECK: %[[DST_ADDR:.*]] = alloca i8**, align 8
+// CHECK: store i8** %[[DST]], i8*** %[[DST_ADDR]], align 8
+// CHECK: %[[V0:.*]] = load i8**, i8*** %[[DST_ADDR]], align 8
+// CHECK: call void @__destructor_8_S_s0(i8** %[[V0]])
+// CHECK: ret void
+
+// CHECK: define linkonce_odr hidden void @__destructor_8_S_s0(i8** 
%[[DST:.*]])
+// CHECK: %[[DST_ADDR:.*]] = alloca i8**, align 8
+// CHECK: store i8** %[[DST]], i8*** %[[DST_ADDR]], align 8
+// CHECK: %[[V0:.*]] = load i8**, i8*** %[[DST_ADDR]], align 8
+// CHECK: call void @__destructor_8_s0(i8** %[[V0]])
+// CHECK: ret void
+
+// CHECK: define linkonce_odr hidden void @__destructor_8_s0(i8** %dst)
+// CHECK: %[[DST_ADDR:.*]] = alloca i8**, align 8
+// CHECK: store i8** %[[DST]], i8*** %[[DST_ADDR]], align 8
+// CHECK: %[[V0:.*]] = load i8**, i8*** %[[DST_ADDR]], align 8
+// CHECK: call void @llvm.objc.storeStrong(i8** %[[V0]], i8* null)
+// CHECK: ret void

Modified: cfe/trunk/test/CodeGenObjC/strong-in-c-struct.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjC/strong-in-c-struct.m?rev=357184=357183=357184=diff
==
--- cfe/trunk/test/CodeGenObjC/strong-in-c-struct.m (original)
+++ cfe/trunk/test/CodeGenObjC/strong-in-c-struct.m Thu Mar 28 10:01:20 2019
@@ -89,12 +89,12 @@ void func(Strong *);
 // CHECK: define void @test_constructor_destructor_StrongOuter()
 // CHECK: %[[T:.*]] = alloca %[[STRUCT_STRONGOUTER:.*]], align 8
 // CHECK: %[[V0:.*]] = bitcast %[[STRUCT_STRONGOUTER]]* %[[T]] to i8**
-// CHECK: call void @__default_constructor_8_s16_s24(i8** %[[V0]])
+// CHECK: call void @__default_constructor_8_S_s16_s24(i8** %[[V0]])
 // CHECK: %[[V1:.*]] = bitcast %[[STRUCT_STRONGOUTER]]* %[[T]] to i8**
-// CHECK: call void @__destructor_8_s16_s24(i8** %[[V1]])
+// CHECK: call void @__destructor_8_S_s16_s24(i8** %[[V1]])
 // CHECK: ret void
 
-// CHECK: define linkonce_odr hidden void 
@__default_constructor_8_s16_s24(i8** %[[DST:.*]])
+// CHECK: define linkonce_odr hidden void 
@__default_constructor_8_S_s16_s24(i8** %[[DST:.*]])
 // CHECK: %[[DST_ADDR:.*]] = alloca i8**, align 8
 // CHECK: store i8** %[[DST]], i8*** %[[DST_ADDR]], align 8
 // CHECK: %[[V0:.*]] = load i8**, i8*** %[[DST_ADDR]], align 8
@@ -117,7 +117,7 @@ void func(Strong *);
 // CHECK: call void @llvm.memset.p0i8.i64(i8* align 

[PATCH] D59725: Additions to creduce script

[Amy Huang via Phabricator via cfe-commits]

akhuang added inline comments.



Comment at: clang/utils/creduce-clang-crash.py:212
+
+cmd = self.get_crash_cmd() + ['-E', '-P']
+try:

arichardson wrote:
> Some crash messages might include the line numbers, do you think it makes 
> sense to fall back to running with -E but without -P and also checking that? 
> I do it in my script but I'm not sure preprocessing saves that much time 
> since creduce will try to remove those statements early.
Makes sense-- in my experience preprocessing is still quite a bit faster than 
letting creduce remove all the statements.


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

https://reviews.llvm.org/D59725



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

[PATCH] D59814: [Testing] Move clangd::Annotations to llvm testing support

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr accepted this revision.
gribozavr added inline comments.
This revision is now accepted and ready to land.



Comment at: llvm/include/llvm/Testing/Support/Annotations.h:9
 //
 // Annotations lets you mark points and ranges inside source code, for tests:
 //

Move into a doc comment on the class Annonations?



Comment at: llvm/include/llvm/Testing/Support/Annotations.h:49
 public:
   // Parses the annotations from Text. Crashes if it's malformed.
   Annotations(llvm::StringRef Text);

Three slashes for doc comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59814



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

[PATCH] D59899: gn build: Add some build files for clangd

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL357182: gn build: Add some build files for clangd (authored 
by nico, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D59899?vs=192509=192663#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D59899

Files:
  clang-tools-extra/trunk/clangd/refactor/tweaks/CMakeLists.txt
  llvm/trunk/utils/gn/build/libs/atomic/BUILD.gn
  llvm/trunk/utils/gn/secondary/BUILD.gn
  
llvm/trunk/utils/gn/secondary/clang-tools-extra/clang-apply-replacements/BUILD.gn
  llvm/trunk/utils/gn/secondary/clang-tools-extra/clangd/BUILD.gn
  
llvm/trunk/utils/gn/secondary/clang-tools-extra/clangd/refactor/tweaks/BUILD.gn
  llvm/trunk/utils/gn/secondary/clang-tools-extra/clangd/tool/BUILD.gn

Index: llvm/trunk/utils/gn/secondary/clang-tools-extra/clangd/BUILD.gn
===
--- llvm/trunk/utils/gn/secondary/clang-tools-extra/clangd/BUILD.gn
+++ llvm/trunk/utils/gn/secondary/clang-tools-extra/clangd/BUILD.gn
@@ -0,0 +1,96 @@
+static_library("clangd") {
+  output_name = "clangDaemon"
+  configs += [ "//llvm/utils/gn/build:clang_code" ]
+  deps = [
+"//clang-tools-extra/clang-tidy/abseil",
+"//clang-tools-extra/clang-tidy/android",
+"//clang-tools-extra/clang-tidy/boost",
+"//clang-tools-extra/clang-tidy/bugprone",
+"//clang-tools-extra/clang-tidy/cert",
+"//clang-tools-extra/clang-tidy/cppcoreguidelines",
+"//clang-tools-extra/clang-tidy/fuchsia",
+"//clang-tools-extra/clang-tidy/google",
+"//clang-tools-extra/clang-tidy/hicpp",
+"//clang-tools-extra/clang-tidy/llvm",
+"//clang-tools-extra/clang-tidy/misc",
+"//clang-tools-extra/clang-tidy/modernize",
+"//clang-tools-extra/clang-tidy/objc",
+"//clang-tools-extra/clang-tidy/performance",
+"//clang-tools-extra/clang-tidy/portability",
+"//clang-tools-extra/clang-tidy/readability",
+"//clang-tools-extra/clang-tidy/zircon",
+"//clang/lib/AST",
+"//clang/lib/ASTMatchers",
+"//clang/lib/Basic",
+"//clang/lib/Driver",
+"//clang/lib/Format",
+"//clang/lib/Frontend",
+"//clang/lib/Index",
+"//clang/lib/Lex",
+"//clang/lib/Sema",
+"//clang/lib/Serialization",
+"//clang/lib/Tooling",
+"//clang/lib/Tooling/Core",
+"//clang/lib/Tooling/Inclusions",
+"//clang/lib/Tooling/Refactoring",
+"//llvm/lib/Support",
+"//llvm/utils/gn/build/libs/atomic",
+"//llvm/utils/gn/build/libs/pthread",
+  ]
+  include_dirs = [ "." ]
+  sources = [
+"AST.cpp",
+"Cancellation.cpp",
+"ClangdLSPServer.cpp",
+"ClangdServer.cpp",
+"ClangdUnit.cpp",
+"CodeComplete.cpp",
+"CodeCompletionStrings.cpp",
+"Compiler.cpp",
+"Context.cpp",
+"Diagnostics.cpp",
+"DraftStore.cpp",
+"ExpectedTypes.cpp",
+"FS.cpp",
+"FSProvider.cpp",
+"FileDistance.cpp",
+"FindSymbols.cpp",
+"FuzzyMatch.cpp",
+"GlobalCompilationDatabase.cpp",
+"Headers.cpp",
+"IncludeFixer.cpp",
+"JSONTransport.cpp",
+"Logger.cpp",
+"Protocol.cpp",
+"Quality.cpp",
+"RIFF.cpp",
+"Selection.cpp",
+"SourceCode.cpp",
+"TUScheduler.cpp",
+"Threading.cpp",
+"Trace.cpp",
+"URI.cpp",
+"XRefs.cpp",
+"index/Background.cpp",
+"index/BackgroundIndexStorage.cpp",
+"index/CanonicalIncludes.cpp",
+"index/FileIndex.cpp",
+"index/Index.cpp",
+"index/IndexAction.cpp",
+"index/MemIndex.cpp",
+"index/Merge.cpp",
+"index/Ref.cpp",
+"index/Serialization.cpp",
+"index/Symbol.cpp",
+"index/SymbolCollector.cpp",
+"index/SymbolID.cpp",
+"index/SymbolLocation.cpp",
+"index/SymbolOrigin.cpp",
+"index/YAMLSerialization.cpp",
+"index/dex/Dex.cpp",
+"index/dex/Iterator.cpp",
+"index/dex/PostingList.cpp",
+"index/dex/Trigram.cpp",
+"refactor/Tweak.cpp",
+  ]
+}
Index: llvm/trunk/utils/gn/secondary/clang-tools-extra/clangd/tool/BUILD.gn
===
--- llvm/trunk/utils/gn/secondary/clang-tools-extra/clangd/tool/BUILD.gn
+++ llvm/trunk/utils/gn/secondary/clang-tools-extra/clangd/tool/BUILD.gn
@@ -0,0 +1,50 @@
+import("//llvm/utils/gn/build/write_cmake_config.gni")
+
+declare_args() {
+  # Whether to build clangd's XPC components.
+  clangd_build_xpc = false
+}
+
+write_cmake_config("features") {
+  # FIXME: Try moving Features.inc.in to tools, seems like a better location.
+  input = "../Features.inc.in"
+  output = "$target_gen_dir/Features.inc"
+  values = []
+  if (clangd_build_xpc) {
+values += [ "CLANGD_BUILD_XPC=1" ]
+  } else {
+values += [ "CLANGD_BUILD_XPC=0" ]
+  }
+}
+
+executable("clangd") {
+  configs += [ "//llvm/utils/gn/build:clang_code" ]
+  deps = [
+":features",
+"//clang-tools-extra/clang-tidy",
+"//clang-tools-extra/clangd",
+

[clang-tools-extra] r357182 - gn build: Add some build files for clangd

[Nico Weber via cfe-commits]

Author: nico
Date: Thu Mar 28 09:53:32 2019
New Revision: 357182

URL: http://llvm.org/viewvc/llvm-project?rev=357182=rev
Log:
gn build: Add some build files for clangd

Enough to build the clangd binaries, but this is still missing build
files for:
- fuzzer
- indexer
- index/dex/dexp
- benchmarks
- xpc

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

Modified:
clang-tools-extra/trunk/clangd/refactor/tweaks/CMakeLists.txt

Modified: clang-tools-extra/trunk/clangd/refactor/tweaks/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/refactor/tweaks/CMakeLists.txt?rev=357182=357181=357182=diff
==
--- clang-tools-extra/trunk/clangd/refactor/tweaks/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clangd/refactor/tweaks/CMakeLists.txt Thu Mar 28 
09:53:32 2019
@@ -6,9 +6,9 @@ set(LLVM_LINK_COMPONENTS
 
 # A target containing all code tweaks (i.e. mini-refactorings) provided by
 # clangd.
-# Built as an object library to make sure linker does not remove global
+# Built as an object library to make sure the linker does not remove global
 # constructors that register individual tweaks in a global registry.
-# To enable these tweaks in exectubales or shared libraries, add
+# To enable these tweaks in executables or shared libraries, add
 # $ to a list of sources, see
 # clangd/tool/CMakeLists.txt for an example.
 add_clang_library(clangDaemonTweaks OBJECT


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

[PATCH] D59887: [Syntax] Introduce TokenBuffer, start clangToolingSyntax library

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 192661.
ilya-biryukov added a comment.

- s/macroMacroInvocation/something else...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59887

Files:
  clang/include/clang/Tooling/Syntax/TokenBuffer.h
  clang/lib/Tooling/CMakeLists.txt
  clang/lib/Tooling/Syntax/CMakeLists.txt
  clang/lib/Tooling/Syntax/TokenBuffer.cpp
  clang/unittests/Tooling/CMakeLists.txt
  clang/unittests/Tooling/Syntax/CMakeLists.txt
  clang/unittests/Tooling/Syntax/TokenBufferTest.cpp

Index: clang/unittests/Tooling/Syntax/TokenBufferTest.cpp
===
--- /dev/null
+++ clang/unittests/Tooling/Syntax/TokenBufferTest.cpp
@@ -0,0 +1,470 @@
+//===- TokenBufferTest.cpp ===//
+//
+// 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 "clang/Tooling/Syntax/TokenBuffer.h"
+#include "clang/AST/ASTConsumer.h"
+#include "clang/AST/Expr.h"
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/DiagnosticIDs.h"
+#include "clang/Basic/DiagnosticOptions.h"
+#include "clang/Basic/FileManager.h"
+#include "clang/Basic/FileSystemOptions.h"
+#include "clang/Basic/LLVM.h"
+#include "clang/Basic/LangOptions.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Basic/TokenKinds.def"
+#include "clang/Basic/TokenKinds.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/FrontendAction.h"
+#include "clang/Frontend/Utils.h"
+#include "clang/Lex/Lexer.h"
+#include "clang/Lex/PreprocessorOptions.h"
+#include "clang/Lex/Token.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/FormatVariadic.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/VirtualFileSystem.h"
+#include "llvm/Support/raw_os_ostream.h"
+#include "llvm/Support/raw_ostream.h"
+#include "llvm/Testing/Support/Annotations.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+using namespace clang;
+using namespace clang::syntax;
+
+using ::testing::AllOf;
+using ::testing::Contains;
+using ::testing::ElementsAre;
+using ::testing::IsEmpty;
+using ::testing::Matcher;
+using ::testing::Pointwise;
+
+// Debug printers.
+// FIXME: This should live somewhere else or be implemented as 'operator
+// <<(raw_ostream&, T)'.
+namespace clang {
+namespace tok {
+inline void PrintTo(TokenKind K, std::ostream *OS) {
+  *OS << tok::getTokenName(K);
+}
+} // namespace tok
+namespace syntax {
+inline void PrintTo(const syntax::Token , std::ostream *OS) {
+  PrintTo(T.kind(), OS);
+  OS->flush();
+}
+} // namespace syntax
+} // namespace clang
+
+namespace {
+// Matchers for clang::Token.
+MATCHER_P(Kind, K, "") { return arg.kind() == K; }
+MATCHER_P2(HasText, Text, SourceMgr, "") {
+  return arg.text(*SourceMgr) == Text;
+}
+MATCHER_P2(IsIdent, Text, SourceMgr, "") {
+  return arg.kind() == tok::identifier && arg.text(*SourceMgr) == Text;
+}
+/// Checks the start and end location of a token are equal to SourceRng.
+MATCHER_P(RangeIs, SourceRng, "") {
+  return arg.location() == SourceRng.first &&
+ arg.endLocation() == SourceRng.second;
+}
+/// Checks the passed tuple has two similar tokens, i.e. both are of the same
+/// kind and have the same text if they are identifiers.
+MATCHER_P(IsSameToken, SourceMgr, "") {
+  auto  = std::get<0>(arg);
+  auto  = std::get<1>(arg);
+  if (L.kind() != R.kind())
+return false;
+  return L.text(*SourceMgr) == L.text(*SourceMgr);
+}
+
+class TokenBufferTest : public ::testing::Test {
+public:
+  /// Run the clang frontend, collect the preprocessed tokens from the frontend
+  /// invocation and store them in this->Tokens.
+  /// This also clears SourceManager before running the compiler.
+  void recordTokens(llvm::StringRef Code) {
+class RecordTokens : public ASTFrontendAction {
+public:
+  explicit RecordTokens(TokenBuffer ) : Result(Result) {}
+
+  bool BeginSourceFileAction(CompilerInstance ) override {
+assert(!Collector && "expected only a single call to BeginSourceFile");
+Collector.emplace(CI.getPreprocessor());
+return true;
+  }
+  void EndSourceFileAction() override {
+assert(Collector && "BeginSourceFileAction was never called");
+Result = std::move(*Collector).consume();
+  }
+
+  std::unique_ptr
+  CreateASTConsumer(CompilerInstance , StringRef InFile) override {
+return llvm::make_unique();
+  }
+
+private:
+  TokenBuffer 
+  

[PATCH] D59899: gn build: Add some build files for clangd

[Nico Weber via Phabricator via cfe-commits]

thakis marked 2 inline comments as done.
thakis added a comment.

Thanks! Landing with comments addressed.


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

https://reviews.llvm.org/D59899



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

[PATCH] D59887: [Syntax] Introduce TokenBuffer, start clangToolingSyntax library

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 192660.
ilya-biryukov marked an inline comment as done.
ilya-biryukov added a comment.

- Rename macro expansion to macro invocation everywhere
- Tweak comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59887

Files:
  clang/include/clang/Tooling/Syntax/TokenBuffer.h
  clang/lib/Tooling/CMakeLists.txt
  clang/lib/Tooling/Syntax/CMakeLists.txt
  clang/lib/Tooling/Syntax/TokenBuffer.cpp
  clang/unittests/Tooling/CMakeLists.txt
  clang/unittests/Tooling/Syntax/CMakeLists.txt
  clang/unittests/Tooling/Syntax/TokenBufferTest.cpp

Index: clang/unittests/Tooling/Syntax/TokenBufferTest.cpp
===
--- /dev/null
+++ clang/unittests/Tooling/Syntax/TokenBufferTest.cpp
@@ -0,0 +1,470 @@
+//===- TokenBufferTest.cpp ===//
+//
+// 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 "clang/Tooling/Syntax/TokenBuffer.h"
+#include "clang/AST/ASTConsumer.h"
+#include "clang/AST/Expr.h"
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/DiagnosticIDs.h"
+#include "clang/Basic/DiagnosticOptions.h"
+#include "clang/Basic/FileManager.h"
+#include "clang/Basic/FileSystemOptions.h"
+#include "clang/Basic/LLVM.h"
+#include "clang/Basic/LangOptions.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Basic/TokenKinds.def"
+#include "clang/Basic/TokenKinds.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/FrontendAction.h"
+#include "clang/Frontend/Utils.h"
+#include "clang/Lex/Lexer.h"
+#include "clang/Lex/PreprocessorOptions.h"
+#include "clang/Lex/Token.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/FormatVariadic.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/VirtualFileSystem.h"
+#include "llvm/Support/raw_os_ostream.h"
+#include "llvm/Support/raw_ostream.h"
+#include "llvm/Testing/Support/Annotations.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+using namespace clang;
+using namespace clang::syntax;
+
+using ::testing::AllOf;
+using ::testing::Contains;
+using ::testing::ElementsAre;
+using ::testing::IsEmpty;
+using ::testing::Matcher;
+using ::testing::Pointwise;
+
+// Debug printers.
+// FIXME: This should live somewhere else or be implemented as 'operator
+// <<(raw_ostream&, T)'.
+namespace clang {
+namespace tok {
+inline void PrintTo(TokenKind K, std::ostream *OS) {
+  *OS << tok::getTokenName(K);
+}
+} // namespace tok
+namespace syntax {
+inline void PrintTo(const syntax::Token , std::ostream *OS) {
+  PrintTo(T.kind(), OS);
+  OS->flush();
+}
+} // namespace syntax
+} // namespace clang
+
+namespace {
+// Matchers for clang::Token.
+MATCHER_P(Kind, K, "") { return arg.kind() == K; }
+MATCHER_P2(HasText, Text, SourceMgr, "") {
+  return arg.text(*SourceMgr) == Text;
+}
+MATCHER_P2(IsIdent, Text, SourceMgr, "") {
+  return arg.kind() == tok::identifier && arg.text(*SourceMgr) == Text;
+}
+/// Checks the start and end location of a token are equal to SourceRng.
+MATCHER_P(RangeIs, SourceRng, "") {
+  return arg.location() == SourceRng.first &&
+ arg.endLocation() == SourceRng.second;
+}
+/// Checks the passed tuple has two similar tokens, i.e. both are of the same
+/// kind and have the same text if they are identifiers.
+MATCHER_P(IsSameToken, SourceMgr, "") {
+  auto  = std::get<0>(arg);
+  auto  = std::get<1>(arg);
+  if (L.kind() != R.kind())
+return false;
+  return L.text(*SourceMgr) == L.text(*SourceMgr);
+}
+
+class TokenBufferTest : public ::testing::Test {
+public:
+  /// Run the clang frontend, collect the preprocessed tokens from the frontend
+  /// invocation and store them in this->Tokens.
+  /// This also clears SourceManager before running the compiler.
+  void recordTokens(llvm::StringRef Code) {
+class RecordTokens : public ASTFrontendAction {
+public:
+  explicit RecordTokens(TokenBuffer ) : Result(Result) {}
+
+  bool BeginSourceFileAction(CompilerInstance ) override {
+assert(!Collector && "expected only a single call to BeginSourceFile");
+Collector.emplace(CI.getPreprocessor());
+return true;
+  }
+  void EndSourceFileAction() override {
+assert(Collector && "BeginSourceFileAction was never called");
+Result = std::move(*Collector).consume();
+  }
+
+  std::unique_ptr
+  CreateASTConsumer(CompilerInstance , StringRef InFile) override {
+return 

[PATCH] D59873: Add additional mangling for struct members of non trivial structs

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai added a comment.

Looks good. I'll commit this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59873



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

[PATCH] D59935: Disable warnings when indexing as a standalone action.

[Haojian Wu via Phabricator via cfe-commits]

hokein accepted this revision.
hokein added inline comments.
This revision is now accepted and ready to land.



Comment at: clangd/index/IndexAction.cpp:138
 CI.getLangOpts().CommentOpts.ParseAllComments = true;
+// Index the whole file even if there are warnings and -Werror is't set.
+// Avoids some analyses too. Set in two places as we're late to the party.

I think `-Werror is't set` should be `-Werror is set`?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59935



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

[PATCH] D59923: [Driver] Simplify -g level computation and its interaction with -gsplit-dwarf

[Paul Robinson via Phabricator via cfe-commits]

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

LGTM.  I wish it had occurred to me to pass both OPT_g_Group and 
OPT_gsplit_dwarf to the same getLastArgs call in the first place.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59923



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

[PATCH] D59873: Add additional mangling for struct members of non trivial structs

[Dan Zimmerman via Phabricator via cfe-commits]

danzimm added a comment.

@smeenai good idea on the third level!

Yep, I'll need somebody to commit this for me, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59873



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