[clang] aed6a1b - Add tests for clang -fno-zero-initialized-in-bss and llc -nozero-initialized-in-bss

2020-07-04 Thread Fangrui Song via cfe-commits

Author: Fangrui Song
Date: 2020-07-04T23:26:57-07:00
New Revision: aed6a1b137dc17426e3da8b85e1f9966c8229c05

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

LOG: Add tests for clang -fno-zero-initialized-in-bss and llc 
-nozero-initialized-in-bss

And rename the CC1 option.

Added: 
clang/test/Driver/fzero-initialized-in-bss.c
llvm/test/CodeGen/X86/zero-initialized-in-bss.ll

Modified: 
clang/include/clang/Driver/CC1Options.td
clang/include/clang/Driver/Options.td
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp

Removed: 




diff  --git a/clang/include/clang/Driver/CC1Options.td 
b/clang/include/clang/Driver/CC1Options.td
index 8729512454c3..75f8aa610d04 100644
--- a/clang/include/clang/Driver/CC1Options.td
+++ b/clang/include/clang/Driver/CC1Options.td
@@ -307,8 +307,6 @@ def mlimit_float_precision : Separate<["-"], 
"mlimit-float-precision">,
   HelpText<"Limit float precision to the given value">;
 def split_stacks : Flag<["-"], "split-stacks">,
   HelpText<"Try to use a split stack if possible.">;
-def mno_zero_initialized_in_bss : Flag<["-"], "mno-zero-initialized-in-bss">,
-  HelpText<"Do not put zero initialized data in the BSS">;
 def mregparm : Separate<["-"], "mregparm">,
   HelpText<"Limit the number of registers available for integer arguments">;
 def msmall_data_limit : Separate<["-"], "msmall-data-limit">,

diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index d030468514c3..e636ee62d202 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -1589,7 +1589,6 @@ def fno_unwind_tables : Flag<["-"], "fno-unwind-tables">, 
Group;
 def fno_verbose_asm : Flag<["-"], "fno-verbose-asm">, Group, 
Flags<[CC1Option]>;
 def fno_working_directory : Flag<["-"], "fno-working-directory">, 
Group;
 def fno_wrapv : Flag<["-"], "fno-wrapv">, Group;
-def fno_zero_initialized_in_bss : Flag<["-"], "fno-zero-initialized-in-bss">, 
Group;
 def fobjc_arc : Flag<["-"], "fobjc-arc">, Group, Flags<[CC1Option]>,
   HelpText<"Synthesize retain and release calls for Objective-C pointers">;
 def fno_objc_arc : Flag<["-"], "fno-objc-arc">, Group;
@@ -1926,7 +1925,7 @@ def fwrapv : Flag<["-"], "fwrapv">, Group, 
Flags<[CC1Option]>,
   HelpText<"Treat signed integer overflow as two's complement">;
 def fwritable_strings : Flag<["-"], "fwritable-strings">, Group, 
Flags<[CC1Option]>,
   HelpText<"Store string literals as writable data">;
-def fzero_initialized_in_bss : Flag<["-"], "fzero-initialized-in-bss">, 
Group;
+defm zero_initialized_in_bss : OptOutFFlag<"zero-initialized-in-bss", "", 
"Don't place zero initialized data in BSS">;
 defm function_sections : OptInFFlag<"function-sections", "Place each function 
in its own section">;
 def fbasic_block_sections_EQ : Joined<["-"], "fbasic-block-sections=">, 
Group,
   Flags<[CC1Option, CC1AsOption]>,

diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index e6dd6ce0a95e..a0d638e7366d 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -4556,8 +4556,8 @@ void Clang::ConstructJob(Compilation &C, const JobAction 
&JA,
   CmdArgs.push_back(FPKeepKindStr);
 
   if (!Args.hasFlag(options::OPT_fzero_initialized_in_bss,
-options::OPT_fno_zero_initialized_in_bss))
-CmdArgs.push_back("-mno-zero-initialized-in-bss");
+options::OPT_fno_zero_initialized_in_bss, true))
+CmdArgs.push_back("-fno-zero-initialized-in-bss");
 
   bool OFastEnabled = isOptimizationLevelFast(Args);
   // If -Ofast is the optimization level, then -fstrict-aliasing should be

diff  --git a/clang/lib/Frontend/CompilerInvocation.cpp 
b/clang/lib/Frontend/CompilerInvocation.cpp
index e12931a5a1b4..821e40c5ea8d 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -942,7 +942,7 @@ static bool ParseCodeGenArgs(CodeGenOptions &Opts, ArgList 
&Args, InputKind IK,
   Opts.StrictFloatCastOverflow =
   !Args.hasArg(OPT_fno_strict_float_cast_overflow);
 
-  Opts.NoZeroInitializedInBSS = Args.hasArg(OPT_mno_zero_initialized_in_bss);
+  Opts.NoZeroInitializedInBSS = Args.hasArg(OPT_fno_zero_initialized_in_bss);
   Opts.NumRegisterParameters = getLastArgIntValue(Args, OPT_mregparm, 0, 
Diags);
   Opts.NoExecStack = Args.hasArg(OPT_mno_exec_stack);
   Opts.SmallDataLimit =

diff  --git a/clang/test/Driver/fzero-initialized-in-bss.c 
b/clang/test/Driver/fzero-initialized-in-bss.c
new file mode 100644
index ..34a1dc9788cd
--- /dev/null
+++ b/clang/test/Driver/fzero-initialized-in-bss.c
@@ -0,0 +1,8 @@
+// RUN: %clang -### %s -c -fzero-initialized-in-bss 2>&1 | FileCheck %

[PATCH] D81355: [PowerPC] Enable -fstack-clash-protection option for ppc64

2020-07-04 Thread Kai Luo via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG68e07da3e5d5: [clang][PowerPC] Enable 
-fstack-clash-protection option for ppc64 (authored by lkail).

Changed prior to commit:
  https://reviews.llvm.org/D81355?vs=269094&id=275538#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81355

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Basic/Targets/PPC.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/CodeGen/stack-clash-protection.c


Index: clang/test/CodeGen/stack-clash-protection.c
===
--- clang/test/CodeGen/stack-clash-protection.c
+++ clang/test/CodeGen/stack-clash-protection.c
@@ -1,6 +1,8 @@
 // Check the correct function attributes are generated
 // RUN: %clang_cc1 -triple x86_64-linux -O0 -S -emit-llvm -o- %s 
-fstack-clash-protection | FileCheck %s
 // RUN: %clang_cc1 -triple s390x-linux-gnu -O0 -S -emit-llvm -o- %s 
-fstack-clash-protection | FileCheck %s
+// RUN: %clang_cc1 -triple powerpc64le-linux-gnu -O0 -S -emit-llvm -o- %s 
-fstack-clash-protection | FileCheck %s
+// RUN: %clang_cc1 -triple powerpc64-linux-gnu -O0 -S -emit-llvm -o- %s 
-fstack-clash-protection | FileCheck %s
 
 // CHECK: define void @large_stack() #[[A:.*]] {
 void large_stack() {
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -2966,7 +2966,8 @@
   if (!EffectiveTriple.isOSLinux())
 return;
 
-  if (!EffectiveTriple.isX86() && !EffectiveTriple.isSystemZ())
+  if (!EffectiveTriple.isX86() && !EffectiveTriple.isSystemZ() &&
+  !EffectiveTriple.isPPC64())
 return;
 
   if (Args.hasFlag(options::OPT_fstack_clash_protection,
Index: clang/lib/Basic/Targets/PPC.h
===
--- clang/lib/Basic/Targets/PPC.h
+++ clang/lib/Basic/Targets/PPC.h
@@ -343,6 +343,10 @@
   const char *getFloat128Mangling() const override { return "u9__ieee128"; }
 
   bool hasExtIntType() const override { return true; }
+
+  bool isSPRegName(StringRef RegName) const override {
+return RegName.equals("r1") || RegName.equals("x1");
+  }
 };
 
 class LLVM_LIBRARY_VISIBILITY PPC32TargetInfo : public PPCTargetInfo {
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -94,8 +94,8 @@
 --
 
 - -fstack-clash-protection will provide a protection against the stack clash
-  attack for x86 and s390x architectures through automatic probing of each page
-  of allocated stack.
+  attack for x86, s390x and ppc64 architectures through automatic probing of
+  each page of allocated stack.
 
 - -ffp-exception-behavior={ignore,maytrap,strict} allows the user to specify
   the floating-point exception behavior. The default setting is ``ignore``.


Index: clang/test/CodeGen/stack-clash-protection.c
===
--- clang/test/CodeGen/stack-clash-protection.c
+++ clang/test/CodeGen/stack-clash-protection.c
@@ -1,6 +1,8 @@
 // Check the correct function attributes are generated
 // RUN: %clang_cc1 -triple x86_64-linux -O0 -S -emit-llvm -o- %s -fstack-clash-protection | FileCheck %s
 // RUN: %clang_cc1 -triple s390x-linux-gnu -O0 -S -emit-llvm -o- %s -fstack-clash-protection | FileCheck %s
+// RUN: %clang_cc1 -triple powerpc64le-linux-gnu -O0 -S -emit-llvm -o- %s -fstack-clash-protection | FileCheck %s
+// RUN: %clang_cc1 -triple powerpc64-linux-gnu -O0 -S -emit-llvm -o- %s -fstack-clash-protection | FileCheck %s
 
 // CHECK: define void @large_stack() #[[A:.*]] {
 void large_stack() {
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -2966,7 +2966,8 @@
   if (!EffectiveTriple.isOSLinux())
 return;
 
-  if (!EffectiveTriple.isX86() && !EffectiveTriple.isSystemZ())
+  if (!EffectiveTriple.isX86() && !EffectiveTriple.isSystemZ() &&
+  !EffectiveTriple.isPPC64())
 return;
 
   if (Args.hasFlag(options::OPT_fstack_clash_protection,
Index: clang/lib/Basic/Targets/PPC.h
===
--- clang/lib/Basic/Targets/PPC.h
+++ clang/lib/Basic/Targets/PPC.h
@@ -343,6 +343,10 @@
   const char *getFloat128Mangling() const override { return "u9__ieee128"; }
 
   bool hasExtIntType() const override { return true; }
+
+  bool isSPRegName(StringRef RegName) const override {
+return RegName.equals("r1") || RegName.equals("x1");
+  }
 };
 
 class LLVM_LIBRARY_VISIBILITY PPC32TargetInfo : public PPCTargetInfo {
Index: clang/docs/ReleaseNotes.rst
==

Re: [clang-tools-extra] 7e2d27b - Fix two -Wrange-loop-analysis warnings that Xcode 12 beta incorrectly complains about

2020-07-04 Thread James Y Knight via cfe-commits
Seems like we should disable the warning for this compiler instead of
making the code worse for the benefit of a temporarily broken warning?

On Sat, Jul 4, 2020, 8:42 PM Nico Weber via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

>
> Author: Nico Weber
> Date: 2020-07-04T20:41:33-04:00
> New Revision: 7e2d27bc554eb607c90e55c89d2537f5d711234c
>
> URL:
> https://github.com/llvm/llvm-project/commit/7e2d27bc554eb607c90e55c89d2537f5d711234c
> DIFF:
> https://github.com/llvm/llvm-project/commit/7e2d27bc554eb607c90e55c89d2537f5d711234c.diff
>
> LOG: Fix two -Wrange-loop-analysis warnings that Xcode 12 beta incorrectly
> complains about
>
> Xcode 12 beta apparently has the Wrange-loop-analysis changes from
> half a year ago, but it seems to lack https://reviews.llvm.org/D72212
> which made the warning usable again.
>
> Added:
>
>
> Modified:
> clang-tools-extra/clangd/XRefs.cpp
> clang-tools-extra/clangd/unittests/PreambleTests.cpp
>
> Removed:
>
>
>
>
> 
> diff  --git a/clang-tools-extra/clangd/XRefs.cpp
> b/clang-tools-extra/clangd/XRefs.cpp
> index 9b44edce95da..c208e953f2ab 100644
> --- a/clang-tools-extra/clangd/XRefs.cpp
> +++ b/clang-tools-extra/clangd/XRefs.cpp
> @@ -1049,7 +1049,7 @@ ReferencesResult findReferences(ParsedAST &AST,
> Position Pos, uint32_t Limit,
>const auto &IDToRefs = AST.getMacros().MacroRefs;
>auto Refs = IDToRefs.find(*MacroSID);
>if (Refs != IDToRefs.end()) {
> -for (const auto Ref : Refs->second) {
> +for (const auto &Ref : Refs->second) {
>Location Result;
>Result.range = Ref;
>Result.uri = URIMainFile;
>
> diff  --git a/clang-tools-extra/clangd/unittests/PreambleTests.cpp
> b/clang-tools-extra/clangd/unittests/PreambleTests.cpp
> index 5bbcb292610e..8c9669a945dd 100644
> --- a/clang-tools-extra/clangd/unittests/PreambleTests.cpp
> +++ b/clang-tools-extra/clangd/unittests/PreambleTests.cpp
> @@ -125,7 +125,7 @@ TEST(PreamblePatchTest, IncludeParsing) {
>  #/**/include )cpp",
>};
>
> -  for (const auto Case : Cases) {
> +  for (const auto &Case : Cases) {
>  Annotations Test(Case);
>  const auto Code = Test.code();
>  SCOPED_TRACE(Code);
>
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] 68e07da - [clang][PowerPC] Enable -fstack-clash-protection option for ppc64

2020-07-04 Thread Kai Luo via cfe-commits

Author: Kai Luo
Date: 2020-07-05T03:43:56Z
New Revision: 68e07da3e5d5175e24caa309e2b13cb65c8c

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

LOG: [clang][PowerPC] Enable -fstack-clash-protection option for ppc64

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

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/lib/Basic/Targets/PPC.h
clang/lib/Driver/ToolChains/Clang.cpp
clang/test/CodeGen/stack-clash-protection.c

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 4d271bfdcd31..d0328b0ef54c 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -94,8 +94,8 @@ New Compiler Flags
 --
 
 - -fstack-clash-protection will provide a protection against the stack clash
-  attack for x86 and s390x architectures through automatic probing of each page
-  of allocated stack.
+  attack for x86, s390x and ppc64 architectures through automatic probing of
+  each page of allocated stack.
 
 - -ffp-exception-behavior={ignore,maytrap,strict} allows the user to specify
   the floating-point exception behavior. The default setting is ``ignore``.

diff  --git a/clang/lib/Basic/Targets/PPC.h b/clang/lib/Basic/Targets/PPC.h
index 46670eaf423b..858059bacb86 100644
--- a/clang/lib/Basic/Targets/PPC.h
+++ b/clang/lib/Basic/Targets/PPC.h
@@ -343,6 +343,10 @@ class LLVM_LIBRARY_VISIBILITY PPCTargetInfo : public 
TargetInfo {
   const char *getFloat128Mangling() const override { return "u9__ieee128"; }
 
   bool hasExtIntType() const override { return true; }
+
+  bool isSPRegName(StringRef RegName) const override {
+return RegName.equals("r1") || RegName.equals("x1");
+  }
 };
 
 class LLVM_LIBRARY_VISIBILITY PPC32TargetInfo : public PPCTargetInfo {

diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index a2cc84805c9c..e6dd6ce0a95e 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -2966,7 +2966,8 @@ static void RenderSCPOptions(const ToolChain &TC, const 
ArgList &Args,
   if (!EffectiveTriple.isOSLinux())
 return;
 
-  if (!EffectiveTriple.isX86() && !EffectiveTriple.isSystemZ())
+  if (!EffectiveTriple.isX86() && !EffectiveTriple.isSystemZ() &&
+  !EffectiveTriple.isPPC64())
 return;
 
   if (Args.hasFlag(options::OPT_fstack_clash_protection,

diff  --git a/clang/test/CodeGen/stack-clash-protection.c 
b/clang/test/CodeGen/stack-clash-protection.c
index eb48da8ff9e9..54699f044ae4 100644
--- a/clang/test/CodeGen/stack-clash-protection.c
+++ b/clang/test/CodeGen/stack-clash-protection.c
@@ -1,6 +1,8 @@
 // Check the correct function attributes are generated
 // RUN: %clang_cc1 -triple x86_64-linux -O0 -S -emit-llvm -o- %s 
-fstack-clash-protection | FileCheck %s
 // RUN: %clang_cc1 -triple s390x-linux-gnu -O0 -S -emit-llvm -o- %s 
-fstack-clash-protection | FileCheck %s
+// RUN: %clang_cc1 -triple powerpc64le-linux-gnu -O0 -S -emit-llvm -o- %s 
-fstack-clash-protection | FileCheck %s
+// RUN: %clang_cc1 -triple powerpc64-linux-gnu -O0 -S -emit-llvm -o- %s 
-fstack-clash-protection | FileCheck %s
 
 // CHECK: define void @large_stack() #[[A:.*]] {
 void large_stack() {



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


[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

2020-07-04 Thread Johel Ernesto Guerrero Peña via Phabricator via cfe-commits
JohelEGP requested changes to this revision.
JohelEGP added a comment.
This revision now requires changes to proceed.

Thank you. Everything I reported works fine now.

I have two more cases for now. First are your examples in the OP.
You have

  template 
  concept bool EqualityComparable = requires(T a, T b) {
{ a == b } -> bool;
  };

But this is what I get:

  template 
  concept bool EqualityComparable = requires(T a, T b)
  {
  {
  a == b
  } -> bool;
  };

Perhaps it's because I compiled against commit 
`71d88cebfb42c8c5ac2d54b42afdcca956e55660` (Fri Jul 3 13:46:41 2020 -0400).
Next:

  template 
  requires std::invocable...>
  struct [[nodiscard]] constant : std::bool_constant < requires
  {
  typename _require_constant_<(std::invoke(F{}, Args{}()...), 0)>;
  } > {};

There's spaces around the template brackets. The lack of indentation is also 
odd, considering that in this context things are indented.


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

https://reviews.llvm.org/D79773



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


[clang-tools-extra] 7e2d27b - Fix two -Wrange-loop-analysis warnings that Xcode 12 beta incorrectly complains about

2020-07-04 Thread Nico Weber via cfe-commits

Author: Nico Weber
Date: 2020-07-04T20:41:33-04:00
New Revision: 7e2d27bc554eb607c90e55c89d2537f5d711234c

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

LOG: Fix two -Wrange-loop-analysis warnings that Xcode 12 beta incorrectly 
complains about

Xcode 12 beta apparently has the Wrange-loop-analysis changes from
half a year ago, but it seems to lack https://reviews.llvm.org/D72212
which made the warning usable again.

Added: 


Modified: 
clang-tools-extra/clangd/XRefs.cpp
clang-tools-extra/clangd/unittests/PreambleTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/XRefs.cpp 
b/clang-tools-extra/clangd/XRefs.cpp
index 9b44edce95da..c208e953f2ab 100644
--- a/clang-tools-extra/clangd/XRefs.cpp
+++ b/clang-tools-extra/clangd/XRefs.cpp
@@ -1049,7 +1049,7 @@ ReferencesResult findReferences(ParsedAST &AST, Position 
Pos, uint32_t Limit,
   const auto &IDToRefs = AST.getMacros().MacroRefs;
   auto Refs = IDToRefs.find(*MacroSID);
   if (Refs != IDToRefs.end()) {
-for (const auto Ref : Refs->second) {
+for (const auto &Ref : Refs->second) {
   Location Result;
   Result.range = Ref;
   Result.uri = URIMainFile;

diff  --git a/clang-tools-extra/clangd/unittests/PreambleTests.cpp 
b/clang-tools-extra/clangd/unittests/PreambleTests.cpp
index 5bbcb292610e..8c9669a945dd 100644
--- a/clang-tools-extra/clangd/unittests/PreambleTests.cpp
+++ b/clang-tools-extra/clangd/unittests/PreambleTests.cpp
@@ -125,7 +125,7 @@ TEST(PreamblePatchTest, IncludeParsing) {
 #/**/include )cpp",
   };
 
-  for (const auto Case : Cases) {
+  for (const auto &Case : Cases) {
 Annotations Test(Case);
 const auto Code = Test.code();
 SCOPED_TRACE(Code);



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


[PATCH] D82188: [clang-tidy] Reworked enum options handling(again)

2020-07-04 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 275520.
njames93 added a comment.

Solved the mac issue


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82188

Files:
  clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
  clang-tools-extra/clang-tidy/ClangTidyCheck.h
  clang-tools-extra/clang-tidy/abseil/StringFindStartswithCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
  clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
  clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
  clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp
  clang-tools-extra/clang-tidy/modernize/ReplaceAutoPtrCheck.cpp
  clang-tools-extra/clang-tidy/modernize/ReplaceRandomShuffleCheck.cpp
  clang-tools-extra/clang-tidy/performance/MoveConstructorInitCheck.cpp
  clang-tools-extra/clang-tidy/performance/TypePromotionInMathFnCheck.cpp
  clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
  clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp
  clang-tools-extra/clang-tidy/utils/IncludeSorter.h
  clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp
  clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp

Index: clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
@@ -6,6 +6,20 @@
 
 namespace clang {
 namespace tidy {
+
+enum class Colours { Red, Orange, Yellow, Green, Blue, Indigo, Violet };
+
+template <> struct OptionEnumMapping {
+  static llvm::ArrayRef> getEnumMapping() {
+static constexpr std::pair Mapping[] = {
+{Colours::Red, "Red"},   {Colours::Orange, "Orange"},
+{Colours::Yellow, "Yellow"}, {Colours::Green, "Green"},
+{Colours::Blue, "Blue"}, {Colours::Indigo, "Indigo"},
+{Colours::Violet, "Violet"}};
+return makeArrayRef(Mapping);
+  }
+};
+
 namespace test {
 
 TEST(ParseLineFilter, EmptyFilter) {
@@ -208,16 +222,10 @@
 #undef CHECK_ERROR_INT
 }
 
+// FIXME: Figure out why this test causes crashes on mac os.
+#ifndef __APPLE__
 TEST(ValidConfiguration, ValidEnumOptions) {
 
-  enum class Colours { Red, Orange, Yellow, Green, Blue, Indigo, Violet };
-  static constexpr std::pair Mapping[] = {
-  {"Red", Colours::Red},   {"Orange", Colours::Orange},
-  {"Yellow", Colours::Yellow}, {"Green", Colours::Green},
-  {"Blue", Colours::Blue}, {"Indigo", Colours::Indigo},
-  {"Violet", Colours::Violet}};
-  static const auto Map = makeArrayRef(Mapping);
-
   ClangTidyOptions Options;
   auto &CheckOptions = Options.CheckOptions;
 
@@ -237,34 +245,37 @@
 #define CHECK_ERROR_ENUM(Name, Expected)   \
   CHECK_ERROR(Name, UnparseableEnumOptionError, Expected)
 
-  CHECK_VAL(TestCheck.getLocal("Valid", Map), Colours::Red);
-  CHECK_VAL(TestCheck.getGlobal("GlobalValid", Map), Colours::Violet);
-  CHECK_VAL(TestCheck.getLocal("ValidWrongCase", Map, /*IgnoreCase*/ true),
-Colours::Red);
+  CHECK_VAL(TestCheck.getIntLocal("Valid"), Colours::Red);
+  CHECK_VAL(TestCheck.getIntGlobal("GlobalValid"), Colours::Violet);
+
   CHECK_VAL(
-  TestCheck.getGlobal("GlobalValidWrongCase", Map, /*IgnoreCase*/ true),
-  Colours::Violet);
-  CHECK_ERROR_ENUM(TestCheck.getLocal("Invalid", Map),
+  TestCheck.getIntLocal("ValidWrongCase", /*IgnoreCase*/ true),
+  Colours::Red);
+  CHECK_VAL(TestCheck.getIntGlobal("GlobalValidWrongCase",
+/*IgnoreCase*/ true),
+Colours::Violet);
+  CHECK_ERROR_ENUM(TestCheck.getIntLocal("Invalid"),
"invalid configuration value "
"'Scarlet' for option 'test.Invalid'");
-  CHECK_ERROR_ENUM(TestCheck.getLocal("ValidWrongCase", Map),
+  CHECK_ERROR_ENUM(TestCheck.getIntLocal("ValidWrongCase"),
"invalid configuration value 'rED' for option "
"'test.ValidWrongCase'; did you mean 'Red'?");
-  CHECK_ERROR_ENUM(TestCheck.getLocal("NearMiss", Map),
+  CHECK_ERROR_ENUM(TestCheck.getIntLocal("NearMiss"),
"invalid configuration value 'Oragne' for option "
"'test.NearMiss'; did you mean 'Orange'?");
-  CHECK_ERROR_ENUM(TestCheck.getGlobal("GlobalInvalid", Map),
+  CHECK_ERROR_ENUM(TestCheck.getIntGlobal("GlobalInvalid"),
"invalid configuration value "
"'Purple' for option 'GlobalInvalid'");
-  CHECK_ERROR_ENUM(TestCheck.getGlobal("GlobalValidWrongCase", Map),
+  CHECK_ERROR_ENUM(TestCheck.getInt

[PATCH] D71739: [AssumeBundles] Use operand bundles to encode alignment assumptions

2020-07-04 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri reopened this revision.
lebedev.ri added a comment.
This revision is now accepted and ready to land.

Reverted in 7ea46aee3670981827c04df89b2c3a1cbdc7561b 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71739



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


[PATCH] D82188: [clang-tidy] Reworked enum options handling(again)

2020-07-04 Thread Nathan James via Phabricator via cfe-commits
njames93 marked an inline comment as done.
njames93 added inline comments.



Comment at: clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp:225
 
+// FIXME: Figure out why this test causes crashes on mac os.
+#ifndef __APPLE__

@aaron.ballman @thakis I figured out a way to prevent the test cases failing on 
mac, still can't figure out the root cause. Would this be acceptable for now?

As far as I can see some subtle linker bug in the default mac os linker isn't 
happy when this test case is included. This actual test case runs just fine, 
but the test in ClangTidyDiagnosticsConsumerTest doesn't run properly. It 
appears to instantiate the context and then instantiate the check, but it never 
attempts to register it, causing the whole test case to fail.
clang-tidy builds just fine which has many uses of this new enum handling and 
check-clang-tools also runs without a hitch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82188



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


[PATCH] D83015: [Driver] Add -fld-path= and deprecate -fuse-ld=/abs/path and -fuse-ld=rel/path

2020-07-04 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 275517.
MaskRay added a comment.

Add a -working-directory test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83015

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChain.cpp
  clang/test/Driver/fld-path.c
  clang/test/Driver/fuse-ld.c

Index: clang/test/Driver/fuse-ld.c
===
--- clang/test/Driver/fuse-ld.c
+++ clang/test/Driver/fuse-ld.c
@@ -2,6 +2,7 @@
 // RUN: -fuse-ld=/usr/local/bin/or1k-linux-ld 2>&1 \
 // RUN: -target x86_64-unknown-linux \
 // RUN:   | FileCheck %s --check-prefix=CHECK-ABSOLUTE-LD
+// CHECK-ABSOLUTE-LD: warning: '-fuse-ld=' taking a path is deprecated. Use '-fld-path=' instead
 // CHECK-ABSOLUTE-LD: /usr/local/bin/or1k-linux-ld
 
 
Index: clang/test/Driver/fld-path.c
===
--- /dev/null
+++ clang/test/Driver/fld-path.c
@@ -0,0 +1,65 @@
+/// This tests uses the PATH environment variable.
+// UNSUPPORTED: system-windows
+
+// RUN: cd %S
+
+/// If -fld-path specifies a word (without /), PATH is consulted to locate the linker.
+// RUN: env PATH=%S/Inputs/basic_freebsd_tree/usr/bin %clang %s -### -fld-path=ld.bfd \
+// RUN:   --target=x86_64-unknown-freebsd --sysroot=%S/Inputs/basic_freebsd_tree 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=BFD
+
+// BFD: Inputs/basic_freebsd_tree/usr/bin/ld.bfd
+
+// RUN: env PATH=%S/Inputs/basic_freebsd_tree/usr/bin %clang %s -### -fld-path=ld.gold \
+// RUN:   --target=x86_64-unknown-freebsd --sysroot=%S/Inputs/basic_freebsd_tree 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=GOLD
+
+// GOLD: Inputs/basic_freebsd_tree/usr/bin/ld.gold
+
+// RUN: env PATH=%S/Inputs/basic_freebsd_tree/usr/bin %clang %s -### -fld-path=not_exist \
+// RUN:   --target=x86_64-unknown-freebsd --sysroot=%S/Inputs/basic_freebsd_tree 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=NOT_EXIST
+
+// NOT_EXIST: error: invalid linker name in argument '-fld-path=not_exist'
+
+// RUN: %clang %s -### -fld-path= \
+// RUN:   --target=x86_64-unknown-freebsd --sysroot=%S/Inputs/basic_freebsd_tree 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=EMPTY
+
+// EMPTY: error: invalid linker name in argument '-fld-path='
+
+/// If -fld-path= contains a slash, PATH is not consulted.
+// RUN: env PATH=%S/Inputs/basic_freebsd_tree/usr/bin %clang %s -### -fld-path=./ld.bfd \
+// RUN:   --target=x86_64-unknown-freebsd --sysroot=%S/Inputs/basic_freebsd_tree 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=NO_BFD
+
+// NO_BFD: error: invalid linker name in argument '-fld-path=./ld.bfd'
+
+/// -fld-path's argument is searched in PATH. --sysroot and -B have no effect.
+// RUN: env PATH= %clang %s -### -fld-path=ld.bfd -B %S/Inputs/basic_freebsd_tree/usr/bin \
+// RUN:   --target=x86_64-unknown-freebsd --sysroot=%S/Inputs/basic_freebsd_tree 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=NOT_EXIST2
+
+// NOT_EXIST2: error: invalid linker name in argument '-fld-path=ld.bfd'
+
+/// -fld-path can specify a path
+// RUN: %clang %s -### -fld-path=%S/Inputs/basic_freebsd_tree/usr/bin/ld.bfd \
+// RUN:   --target=x86_64-unknown-freebsd --sysroot=%S/Inputs/basic_freebsd_tree 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=BFD
+
+// RUN: %clang %s -### -fld-path=Inputs/basic_freebsd_tree/usr/bin/ld.bfd \
+// RUN:   --target=x86_64-unknown-freebsd --sysroot=%S/Inputs/basic_freebsd_tree 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=BFD
+
+/// -fld-path= and -fuse-ld= can be used together. -fld-path= takes precedence.
+/// -fuse-ld= can be used to specify the linker flavor.
+// RUN: %clang %s -### -Werror -fld-path=%S/Inputs/basic_freebsd_tree/usr/bin/ld.bfd -fuse-ld=gold \
+// RUN:   --target=x86_64-unknown-freebsd --sysroot=%S/Inputs/basic_freebsd_tree 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=BFD --implicit-check-not=error:
+
+/// -fld-path= respects -working-directory
+// RUN: %clang %s -### -fld-path=usr/bin/ld.bfd -working-directory=%S/Inputs/basic_freebsd_tree \
+// RUN:   --target=x86_64-unknown-freebsd --sysroot=%S/Inputs/basic_freebsd_tree 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=USR_BIN_BFD
+
+// USR_BIN_BFD: "usr/bin/ld.bfd"
Index: clang/lib/Driver/ToolChain.cpp
===
--- clang/lib/Driver/ToolChain.cpp
+++ clang/lib/Driver/ToolChain.cpp
@@ -547,18 +547,45 @@
 }
 
 std::string ToolChain::GetLinkerPath() const {
+  // Get -fuse-ld= first to prevent -Wunused-command-line-argument. -fuse-ld= is
+  // considered as the linker flavor, e.g. "bfd", "gold", or "lld".
   const Arg* A = Args.getLastArg(options::OPT_fuse_ld_EQ);
   StringRef UseLinker = A ? A->getValue() : CLANG_DEFAULT_LINKER;
 
+  // -fld-path= takes precedence over -fuse-ld= and specifies the executable
+  // name. PATH is consulted if the value

[PATCH] D83154: clang: Add -fcoverage-prefix-map

2020-07-04 Thread Keith Smiley via Phabricator via cfe-commits
keith updated this revision to Diff 275506.
keith added a comment.

Fix tests on Windows


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83154

Files:
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CoverageMappingGen.cpp
  clang/lib/CodeGen/CoverageMappingGen.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CoverageMapping/coverage-prefix-map.c
  clang/test/Driver/debug-prefix-map.c

Index: clang/test/Driver/debug-prefix-map.c
===
--- clang/test/Driver/debug-prefix-map.c
+++ clang/test/Driver/debug-prefix-map.c
@@ -1,28 +1,39 @@
 // RUN: %clang -### -fdebug-prefix-map=old %s 2>&1 | FileCheck %s -check-prefix CHECK-DEBUG-INVALID
 // RUN: %clang -### -fmacro-prefix-map=old %s 2>&1 | FileCheck %s -check-prefix CHECK-MACRO-INVALID
+// RUN: %clang -### -fcoverage-prefix-map=old %s 2>&1 | FileCheck %s -check-prefix CHECK-COVERAGE-INVALID
 // RUN: %clang -### -ffile-prefix-map=old %s 2>&1 | FileCheck %s -check-prefix CHECK-FILE-INVALID
 
 // RUN: %clang -### -fdebug-prefix-map=old=new %s 2>&1 | FileCheck %s -check-prefix CHECK-DEBUG-SIMPLE
 // RUN: %clang -### -fmacro-prefix-map=old=new %s 2>&1 | FileCheck %s -check-prefix CHECK-MACRO-SIMPLE
+// RUN: %clang -### -fcoverage-prefix-map=old=new %s 2>&1 | FileCheck %s -check-prefix CHECK-COVERAGE-SIMPLE
 // RUN: %clang -### -ffile-prefix-map=old=new %s 2>&1 | FileCheck %s -check-prefix CHECK-DEBUG-SIMPLE
 // RUN: %clang -### -ffile-prefix-map=old=new %s 2>&1 | FileCheck %s -check-prefix CHECK-MACRO-SIMPLE
+// RUN: %clang -### -ffile-prefix-map=old=new %s 2>&1 | FileCheck %s -check-prefix CHECK-COVERAGE-SIMPLE
 
 // RUN: %clang -### -fdebug-prefix-map=old=n=ew %s 2>&1 | FileCheck %s -check-prefix CHECK-DEBUG-COMPLEX
 // RUN: %clang -### -fmacro-prefix-map=old=n=ew %s 2>&1 | FileCheck %s -check-prefix CHECK-MACRO-COMPLEX
+// RUN: %clang -### -fcoverage-prefix-map=old=n=ew %s 2>&1 | FileCheck %s -check-prefix CHECK-COVERAGE-COMPLEX
 // RUN: %clang -### -ffile-prefix-map=old=n=ew %s 2>&1 | FileCheck %s -check-prefix CHECK-DEBUG-COMPLEX
 // RUN: %clang -### -ffile-prefix-map=old=n=ew %s 2>&1 | FileCheck %s -check-prefix CHECK-MACRO-COMPLEX
+// RUN: %clang -### -ffile-prefix-map=old=n=ew %s 2>&1 | FileCheck %s -check-prefix CHECK-COVERAGE-COMPLEX
 
 // RUN: %clang -### -fdebug-prefix-map=old= %s 2>&1 | FileCheck %s -check-prefix CHECK-DEBUG-EMPTY
 // RUN: %clang -### -fmacro-prefix-map=old= %s 2>&1 | FileCheck %s -check-prefix CHECK-MACRO-EMPTY
+// RUN: %clang -### -fcoverage-prefix-map=old= %s 2>&1 | FileCheck %s -check-prefix CHECK-COVERAGE-EMPTY
 // RUN: %clang -### -ffile-prefix-map=old= %s 2>&1 | FileCheck %s -check-prefix CHECK-DEBUG-EMPTY
 // RUN: %clang -### -ffile-prefix-map=old= %s 2>&1 | FileCheck %s -check-prefix CHECK-MACRO-EMPTY
+// RUN: %clang -### -ffile-prefix-map=old= %s 2>&1 | FileCheck %s -check-prefix CHECK-COVERAGE-EMPTY
 
 // CHECK-DEBUG-INVALID: error: invalid argument 'old' to -fdebug-prefix-map
 // CHECK-MACRO-INVALID: error: invalid argument 'old' to -fmacro-prefix-map
+// CHECK-COVERAGE-INVALID: error: invalid argument 'old' to -fcoverage-prefix-map
 // CHECK-FILE-INVALID: error: invalid argument 'old' to -ffile-prefix-map
 // CHECK-DEBUG-SIMPLE: fdebug-prefix-map=old=new
 // CHECK-MACRO-SIMPLE: fmacro-prefix-map=old=new
+// CHECK-COVERAGE-SIMPLE: fcoverage-prefix-map=old=new
 // CHECK-DEBUG-COMPLEX: fdebug-prefix-map=old=n=ew
 // CHECK-MACRO-COMPLEX: fmacro-prefix-map=old=n=ew
+// CHECK-COVERAGE-COMPLEX: fcoverage-prefix-map=old=n=ew
 // CHECK-DEBUG-EMPTY: fdebug-prefix-map=old=
 // CHECK-MACRO-EMPTY: fmacro-prefix-map=old=
+// CHECK-COVERAGE-EMPTY: fcoverage-prefix-map=old=
Index: clang/test/CoverageMapping/coverage-prefix-map.c
===
--- /dev/null
+++ clang/test/CoverageMapping/coverage-prefix-map.c
@@ -0,0 +1,14 @@
+// %s expands to an absolute path, so to test relative paths we need to create a
+// clean directory, put the source there, and cd into it.
+// RUN: rm -rf %t
+// RUN: mkdir -p %t/root/nested
+// RUN: echo "void f1() {}" > %t/root/nested/coverage-prefix-map.c
+// RUN: cd %t/root
+
+// RUN: %clang_cc1 -fprofile-instrument=clang -fcoverage-mapping -emit-llvm -mllvm -enable-name-compression=false -main-file-name coverage-prefix-map.c nested/coverage-prefix-map.c -o - | FileCheck --check-prefix=ABSOLUTE %s
+//
+// ABSOLUTE: @__llvm_coverage_mapping = {{.*"\\01.*root.*nested.*coverage-prefix-map\.c}}
+
+// RUN: %clang_cc1 -fprofile-instrument=clang -fcoverage-mapping -emit-llvm -mllvm -enable-name-compression=false -main-file-name coverage-prefix-map.c nested/coverage-prefix-map.c -fcoverage-prefix-map=%/t/root=. -o - | FileCheck --check-prefix=COVERAGE-PREFIX-MAP %s --implicit-check-not=root
+//
+// COVERAGE-PREFIX-MAP: 

[PATCH] D83015: [Driver] Add -fld-path= and deprecate -fuse-ld=/abs/path and -fuse-ld=rel/path

2020-07-04 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 275507.
MaskRay added a comment.

Improve tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83015

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChain.cpp
  clang/test/Driver/fld-path.c
  clang/test/Driver/fuse-ld.c

Index: clang/test/Driver/fuse-ld.c
===
--- clang/test/Driver/fuse-ld.c
+++ clang/test/Driver/fuse-ld.c
@@ -2,6 +2,7 @@
 // RUN: -fuse-ld=/usr/local/bin/or1k-linux-ld 2>&1 \
 // RUN: -target x86_64-unknown-linux \
 // RUN:   | FileCheck %s --check-prefix=CHECK-ABSOLUTE-LD
+// CHECK-ABSOLUTE-LD: warning: '-fuse-ld=' taking a path is deprecated. Use '-fld-path=' instead
 // CHECK-ABSOLUTE-LD: /usr/local/bin/or1k-linux-ld
 
 
Index: clang/test/Driver/fld-path.c
===
--- /dev/null
+++ clang/test/Driver/fld-path.c
@@ -0,0 +1,58 @@
+/// This tests uses the PATH environment variable.
+// UNSUPPORTED: system-windows
+
+// RUN: cd %S
+
+/// If -fld-path specifies a word (without /), PATH is consulted to locate the linker.
+// RUN: env PATH=%S/Inputs/basic_freebsd_tree/usr/bin %clang %s -### -fld-path=ld.bfd \
+// RUN:   --target=x86_64-unknown-freebsd --sysroot=%S/Inputs/basic_freebsd_tree 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=BFD
+
+// BFD: Inputs/basic_freebsd_tree/usr/bin/ld.bfd
+
+// RUN: env PATH=%S/Inputs/basic_freebsd_tree/usr/bin %clang %s -### -fld-path=ld.gold \
+// RUN:   --target=x86_64-unknown-freebsd --sysroot=%S/Inputs/basic_freebsd_tree 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=GOLD
+
+// GOLD: Inputs/basic_freebsd_tree/usr/bin/ld.gold
+
+// RUN: env PATH=%S/Inputs/basic_freebsd_tree/usr/bin %clang %s -### -fld-path=not_exist \
+// RUN:   --target=x86_64-unknown-freebsd --sysroot=%S/Inputs/basic_freebsd_tree 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=NOT_EXIST
+
+// NOT_EXIST: error: invalid linker name in argument '-fld-path=not_exist'
+
+// RUN: %clang %s -### -fld-path= \
+// RUN:   --target=x86_64-unknown-freebsd --sysroot=%S/Inputs/basic_freebsd_tree 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=EMPTY
+
+// EMPTY: error: invalid linker name in argument '-fld-path='
+
+/// If -fld-path= contains a slash, PATH is not consulted.
+// RUN: env PATH=%S/Inputs/basic_freebsd_tree/usr/bin %clang %s -### -fld-path=./ld.bfd \
+// RUN:   --target=x86_64-unknown-freebsd --sysroot=%S/Inputs/basic_freebsd_tree 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=NO_BFD
+
+// NO_BFD: error: invalid linker name in argument '-fld-path=./ld.bfd'
+
+/// -fld-path's argument is searched in PATH. --sysroot and -B have no effect.
+// RUN: env PATH= %clang %s -### -fld-path=ld.bfd -B %S/Inputs/basic_freebsd_tree/usr/bin \
+// RUN:   --target=x86_64-unknown-freebsd --sysroot=%S/Inputs/basic_freebsd_tree 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=NOT_EXIST2
+
+// NOT_EXIST2: error: invalid linker name in argument '-fld-path=ld.bfd'
+
+/// -fld-path can specify a path
+// RUN: %clang %s -### -fld-path=%S/Inputs/basic_freebsd_tree/usr/bin/ld.bfd \
+// RUN:   --target=x86_64-unknown-freebsd --sysroot=%S/Inputs/basic_freebsd_tree 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=BFD
+
+// RUN: %clang %s -### -fld-path=Inputs/basic_freebsd_tree/usr/bin/ld.bfd \
+// RUN:   --target=x86_64-unknown-freebsd --sysroot=%S/Inputs/basic_freebsd_tree 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=BFD
+
+/// -fld-path= and -fuse-ld= can be used together. -fld-path= takes precedence.
+/// -fuse-ld= can be used to specify the linker flavor.
+// RUN: %clang %s -### -Werror -fld-path=%S/Inputs/basic_freebsd_tree/usr/bin/ld.bfd -fuse-ld=gold \
+// RUN:   --target=x86_64-unknown-freebsd --sysroot=%S/Inputs/basic_freebsd_tree 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=BFD --implicit-check-not=error:
Index: clang/lib/Driver/ToolChain.cpp
===
--- clang/lib/Driver/ToolChain.cpp
+++ clang/lib/Driver/ToolChain.cpp
@@ -547,18 +547,45 @@
 }
 
 std::string ToolChain::GetLinkerPath() const {
+  // Get -fuse-ld= first to prevent -Wunused-command-line-argument. -fuse-ld= is
+  // considered as the linker flavor, e.g. "bfd", "gold", or "lld".
   const Arg* A = Args.getLastArg(options::OPT_fuse_ld_EQ);
   StringRef UseLinker = A ? A->getValue() : CLANG_DEFAULT_LINKER;
 
+  // -fld-path= takes precedence over -fuse-ld= and specifies the executable
+  // name. PATH is consulted if the value does not contain /. -B is
+  // intentionally ignored.
+  if (const Arg *A = Args.getLastArg(options::OPT_fld_path_EQ)) {
+StringRef Value(A->getValue());
+if (!Value.empty()) {
+  if (llvm::sys::path::parent_path(Value).empty()) {
+if (ErrorOr Path = llvm::sys::findProgramByName(Value))
+  return *Path;
+   

[clang] 7fed3cf - [clang] Fix two tests that are affected by llvm opt change

2020-07-04 Thread Roman Lebedev via cfe-commits

Author: Roman Lebedev
Date: 2020-07-04T18:26:22+03:00
New Revision: 7fed3cfadbdfe1880e16c217a0edac97cbe288d2

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

LOG: [clang] Fix two tests that are affected by llvm opt change

Added: 


Modified: 
clang/test/CodeGen/thinlto-distributed-cfi-devirt.ll
clang/test/CodeGenOpenCL/convergent.cl

Removed: 




diff  --git a/clang/test/CodeGen/thinlto-distributed-cfi-devirt.ll 
b/clang/test/CodeGen/thinlto-distributed-cfi-devirt.ll
index b46845afba26..5c753ba6f93c 100644
--- a/clang/test/CodeGen/thinlto-distributed-cfi-devirt.ll
+++ b/clang/test/CodeGen/thinlto-distributed-cfi-devirt.ll
@@ -93,7 +93,7 @@ cont2:
   ; CHECK-IR: br i1 {{.*}}, label %trap
 
   ; We still have to call it as virtual.
-  ; CHECK-IR: %call3 = tail call i32 %8
+  ; CHECK-IR: %call3 = tail call i32 %7
   %call3 = tail call i32 %8(%struct.A* nonnull %obj, i32 %call)
   ret i32 %call3
 }

diff  --git a/clang/test/CodeGenOpenCL/convergent.cl 
b/clang/test/CodeGenOpenCL/convergent.cl
index 193d391ced20..49d182579e4c 100644
--- a/clang/test/CodeGenOpenCL/convergent.cl
+++ b/clang/test/CodeGenOpenCL/convergent.cl
@@ -70,9 +70,9 @@ void test_merge_if(int a) {
 // CHECK-NOT: call spir_func void @g()
 // CHECK: br label %[[if_end]]
 // CHECK: [[if_end]]:
-// CHECK:  %[[tobool_pr:.+]] = phi i1 [ true, %[[if_then]] ], [ false, %{{.+}} 
]
+// CHECK:  %[[tobool_not_pr:.+]] = phi i1 [ true, %{{.+}} ], [ false, 
%[[if_then]] ]
 // CHECK:  tail call spir_func void @convfun() #[[attr4:.+]]
-// CHECK:  br i1 %[[tobool_pr]], label %[[if_then2:.+]], label %[[if_end3:.+]]
+// CHECK:  br i1 %[[tobool_not_pr]], label %[[if_end3:.+]], label 
%[[if_then2:.+]]
 // CHECK: [[if_then2]]:
 // CHECK: tail call spir_func void @g()
 // CHECK:  br label %[[if_end3:.+]]



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


[PATCH] D83157: [clangd] Extract BackgroundIndex::Options struct. NFC

2020-07-04 Thread Sam McCall via Phabricator via cfe-commits
sammccall marked an inline comment as done.
sammccall added inline comments.



Comment at: clang-tools-extra/clangd/ClangdServer.cpp:184
+TFS, CDB,
 BackgroundIndexStorage::createDiskBackedStorageFactory(
 [&CDB](llvm::StringRef File) { return CDB.getProjectInfo(File); }),

kadircet wrote:
> I think we should move storage factory into options too ?
The storage factory isn't optional, nor does it have a good default. So it 
seems mechanically awkward to put it in the struct, and I'm not sure there's 
much benefit... 

How would you see this being initialized?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83157



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


[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

2020-07-04 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments.



Comment at: clang/lib/AST/ASTContext.cpp:2424
+  (T->isSpecificBuiltinType(BuiltinType::LongDouble) &&
+   Target->supportsAIXPowerAlignment()))
 // Don't increase the alignment if an alignment attribute was specified on 
a

hubert.reinterpretcast wrote:
> Xiangling_L wrote:
> > hubert.reinterpretcast wrote:
> > > Does `supportsAIXPowerAlignment` express the condition we want to check 
> > > here? That might be true for an implementation operating with `mac68k` 
> > > alignment rules.
> > Yeah, `supportsAIXPowerAlignment` cannot separate the preferred alignment 
> > of double, long double between `power/natural` and `mac68k` alignment 
> > rules. But I noticed that currently, AIX target on wyvern or XL don't 
> > support `mac68k` , so maybe we should leave further changes to the patch 
> > which is gonna implement `mac68k` alignment rules? The possible solution I 
> > am thinking is we can add checking if the decl has `AlignMac68kAttr` into 
> > query to separate things out.
> > 
> > Another thing is that once we start supporting mac68k alignment rule(if we 
> > will), should we also change the ABI align values as well? (e.g. for 
> > double, it should be 2 instead)
> If the "base state" is AIX `power` alignment for a platform, I suggest that 
> the name be `defaultsToAIXPowerAlignment`.
This last question about the ABI align values is relevant to considerations for 
`natural` alignment support as well. More generally, the question is whether 
the "minimum alignment" of the type in a context subject to alternative 
alignment rules is altered to match said alignment rule. This is observable via 
the diagnostic associated with C++11 alignment specifiers.

The existing behaviour of `mac68k` alignment suggests that the "minimum 
alignment" is context-free.

```
#pragma options align=mac68k
struct Q {
  double x alignas(2);  // expected-error {{less than minimum alignment}}
};
#pragma options align=reset
```

Compiler Explorer link: https://godbolt.org/z/9NM5_-


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79719



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


[PATCH] D82585: [analyzer][NFC] Move the data structures from CheckerRegistry to the Core library

2020-07-04 Thread Kristóf Umann via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGb6cbe6cb0399: [analyzer][NFC] Move the data structures from 
CheckerRegistry to the Core… (authored by Szelethus).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82585

Files:
  clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
  clang/include/clang/StaticAnalyzer/Core/CheckerRegistryData.h
  clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
  clang/lib/StaticAnalyzer/Core/CMakeLists.txt
  clang/lib/StaticAnalyzer/Core/CheckerRegistryData.cpp
  clang/lib/StaticAnalyzer/Core/Environment.cpp
  clang/lib/StaticAnalyzer/Frontend/AnalyzerHelpFlags.cpp
  clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
  clang/lib/StaticAnalyzer/Frontend/CreateCheckerManager.cpp
  clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp

Index: clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp
===
--- clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp
+++ clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp
@@ -11,6 +11,7 @@
 #include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/CheckerRegistryData.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
 #include "clang/StaticAnalyzer/Frontend/AnalysisConsumer.h"
@@ -100,7 +101,7 @@
 llvm::raw_svector_ostream OS(Buf);
 C.getAnalysisManager()
 .getCheckerManager()
-->getCheckerRegistry()
+->getCheckerRegistryData()
 .printEnabledCheckerList(OS);
 // Strip a newline off.
 auto R =
Index: clang/lib/StaticAnalyzer/Frontend/CreateCheckerManager.cpp
===
--- clang/lib/StaticAnalyzer/Frontend/CreateCheckerManager.cpp
+++ clang/lib/StaticAnalyzer/Frontend/CreateCheckerManager.cpp
@@ -23,11 +23,11 @@
 ArrayRef> checkerRegistrationFns)
 : Context(&Context), LangOpts(Context.getLangOpts()), AOptions(AOptions),
   PP(&PP), Diags(Context.getDiagnostics()),
-  Registry(
-  std::make_unique(plugins, Context.getDiagnostics(),
-AOptions, checkerRegistrationFns)) {
-  Registry->initializeRegistry(*this);
-  Registry->initializeManager(*this);
+  RegistryData(std::make_unique()) {
+  CheckerRegistry Registry(*RegistryData, plugins, Context.getDiagnostics(),
+   AOptions, checkerRegistrationFns);
+  Registry.initializeRegistry(*this);
+  Registry.initializeManager(*this);
   finishedCheckerRegistration();
 }
 
@@ -36,8 +36,9 @@
DiagnosticsEngine &Diags,
ArrayRef plugins)
 : LangOpts(LangOpts), AOptions(AOptions), Diags(Diags),
-  Registry(std::make_unique(plugins, Diags, AOptions)) {
-  Registry->initializeRegistry(*this);
+  RegistryData(std::make_unique()) {
+  CheckerRegistry Registry(*RegistryData, plugins, Diags, AOptions, {});
+  Registry.initializeRegistry(*this);
 }
 
 CheckerManager::~CheckerManager() {
Index: clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
===
--- clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
+++ clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
@@ -25,14 +25,13 @@
 
 using namespace clang;
 using namespace ento;
+using namespace checker_registry;
 using llvm::sys::DynamicLibrary;
 
 //===--===//
 // Utilities.
 //===--===//
 
-using RegisterCheckersFn = void (*)(CheckerRegistry &);
-
 static bool isCompatibleAPIVersion(const char *VersionString) {
   // If the version string is null, its not an analyzer plugin.
   if (!VersionString)
@@ -43,140 +42,17 @@
   return strcmp(VersionString, CLANG_ANALYZER_API_VERSION_STRING) == 0;
 }
 
-namespace {
-template  struct FullNameLT {
-  bool operator()(const T &Lhs, const T &Rhs) {
-return Lhs.FullName < Rhs.FullName;
-  }
-};
-
-using PackageNameLT = FullNameLT;
-using CheckerNameLT = FullNameLT;
-} // end of anonymous namespace
-
-template 
-static std::conditional_t::value,
-  typename CheckerOrPackageInfoList::const_iterator,
-  typename CheckerOrPackageInfoList::iterator>
-binaryFind(CheckerOrPackageInfoList &Collection, StringRef FullName) {
-
-  using CheckerOrPackage = typename CheckerOrPackageInfoList::value_type;
-  using CheckerOrPackageFullNameLT = FullNameLT;
-
-  assert(llvm::is_sorted(Collection, CheckerOrPackageFullNameLT{}) &&
- "I

[clang] b6cbe6c - [analyzer][NFC] Move the data structures from CheckerRegistry to the Core library

2020-07-04 Thread Kirstóf Umann via cfe-commits

Author: Kirstóf Umann
Date: 2020-07-04T12:31:51+02:00
New Revision: b6cbe6cb0399d4671e5384dcc326af56bc6bd122

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

LOG: [analyzer][NFC] Move the data structures from CheckerRegistry to the Core 
library

If you were around the analyzer for a while now, you must've seen a lot of
patches that awkwardly puts code from one library to the other:

* D75360 moves the constructors of CheckerManager, which lies in the Core
  library, to the Frontend library. Most the patch itself was a struggle along
  the library lines.
* D78126 had to be reverted because dependency information would be utilized
  in the Core library, but the actual data lied in the frontend.
  D78126#inline-751477 touches on this issue as well.

This stems from the often mentioned problem: the Frontend library depends on
Core and Checkers, Checkers depends on Core. The checker registry functions
(`registerMallocChecker`, etc) lie in the Checkers library in order to keep each
checker its own module. What this implies is that checker registration cannot
take place in the Core, but the Core might still want to use the data that
results from it (which checker/package is enabled, dependencies, etc).

D54436 was the patch that initiated this. Back in the days when CheckerRegistry
was super dumb and buggy, it implemented a non-documented solution to this
problem by keeping the data in the Core, and leaving the logic in the Frontend.
At the time when the patch landed, the merger to the Frontend made sense,
because the data hadn't been utilized anywhere, and the whole workaround without
any documentation made little sense to me.

So, lets put the data back where it belongs, in the Core library. This patch
introduces `CheckerRegistryData`, and turns `CheckerRegistry` into a short lived
wrapper around this data that implements the logic of checker registration. The
data is tied to CheckerManager because it is required to parse it.

Side note: I can't help but cringe at the fact how ridiculously awkward the
library lines are. I feel like I'm thinking too much inside the box, but I guess
this is just the price of keeping the checkers so modularized.

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

Added: 
clang/include/clang/StaticAnalyzer/Core/CheckerRegistryData.h
clang/lib/StaticAnalyzer/Core/CheckerRegistryData.cpp

Modified: 
clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
clang/lib/StaticAnalyzer/Core/CMakeLists.txt
clang/lib/StaticAnalyzer/Core/Environment.cpp
clang/lib/StaticAnalyzer/Frontend/AnalyzerHelpFlags.cpp
clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
clang/lib/StaticAnalyzer/Frontend/CreateCheckerManager.cpp
clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp

Removed: 




diff  --git a/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h 
b/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
index 1f3c6a1b780b..d2f71baa56a4 100644
--- a/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
+++ b/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
@@ -43,6 +43,7 @@ class CallEvent;
 class CheckerBase;
 class CheckerContext;
 class CheckerRegistry;
+struct CheckerRegistryData;
 class ExplodedGraph;
 class ExplodedNode;
 class ExplodedNodeSet;
@@ -130,7 +131,7 @@ class CheckerManager {
   const Preprocessor *PP = nullptr;
   CheckerNameRef CurrentCheckerName;
   DiagnosticsEngine &Diags;
-  std::unique_ptr Registry;
+  std::unique_ptr RegistryData;
 
 public:
   // These constructors are defined in the Frontend library, because
@@ -152,8 +153,8 @@ class CheckerManager {
   : CheckerManager(Context, AOptions, PP, {}, {}) {}
 
   /// Constructs a CheckerManager without requiring an AST. No checker
-  /// registration will take place. Only useful for retrieving the
-  /// CheckerRegistry and print for help flags where the AST is unavalaible.
+  /// registration will take place. Only useful when one needs to print the
+  /// help flags through CheckerRegistryData, and the AST is unavalaible.
   CheckerManager(AnalyzerOptions &AOptions, const LangOptions &LangOpts,
  DiagnosticsEngine &Diags, ArrayRef plugins);
 
@@ -172,7 +173,9 @@ class CheckerManager {
 assert(PP);
 return *PP;
   }
-  const CheckerRegistry &getCheckerRegistry() const { return *Registry; }
+  const CheckerRegistryData &getCheckerRegistryData() const {
+return *RegistryData;
+  }
   DiagnosticsEngine &getDiagnostics() const { return Diags; }
   ASTContext &getASTContext() const {
 assert(Context);

diff  --git a/clang/include/clang/StaticAnalyzer/Core/CheckerRegistryData.h 
b/clang/include/clang/StaticAnalyzer/Core/CheckerRegistr

[PATCH] D83157: [clangd] Extract BackgroundIndex::Options struct. NFC

2020-07-04 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision.
kadircet marked an inline comment as done.
kadircet added a comment.
This revision is now accepted and ready to land.

thanks for doing this!




Comment at: clang-tools-extra/clangd/ClangdServer.cpp:184
+TFS, CDB,
 BackgroundIndexStorage::createDiskBackedStorageFactory(
 [&CDB](llvm::StringRef File) { return CDB.getProjectInfo(File); }),

I think we should move storage factory into options too ?



Comment at: clang-tools-extra/clangd/index/Background.h:127
 // all commands in a compilation database. Indexing happens in the background.
-// FIXME: it should also persist its state on disk for fast start.
 // FIXME: it should watch for changes to files on disk.

🎉 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83157



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


[PATCH] D83157: [clangd] Extract BackgroundIndex::Options struct. NFC

2020-07-04 Thread Sam McCall via Phabricator via cfe-commits
sammccall created this revision.
sammccall added a reviewer: kadircet.
Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous, MaskRay, 
ilya-biryukov.
Herald added a project: clang.

I've dropped the background context parameter, since we in practice just pass 
the
current context there, and we now have a different way to specify context too.
While here, clean up a couple of comments.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D83157

Files:
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/index/Background.cpp
  clang-tools-extra/clangd/index/Background.h
  clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp

Index: clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
===
--- clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
+++ clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
@@ -96,8 +96,8 @@
   size_t CacheHits = 0;
   MemoryShardStorage MSS(Storage, CacheHits);
   OverlayCDB CDB(/*Base=*/nullptr);
-  BackgroundIndex Idx(Context::empty(), FS, CDB,
-  [&](llvm::StringRef) { return &MSS; });
+  BackgroundIndex Idx(FS, CDB, [&](llvm::StringRef) { return &MSS; },
+  /*Opts=*/{});
 
   tooling::CompileCommand Cmd;
   Cmd.Filename = testPath("root/A.cc");
@@ -126,7 +126,8 @@
   }
   // Context provider that installs a configuration mutating foo's command.
   // This causes it to define foo::two instead of foo::one.
-  auto ContextProvider = [](PathRef P) {
+  BackgroundIndex::Options Opts;
+  Opts.ContextProvider = [](PathRef P) {
 Config C;
 if (P.endswith("foo.cpp"))
   C.CompileFlags.Edits.push_back(
@@ -140,9 +141,9 @@
   // We need the CommandMangler, because that applies the config we're testing.
   OverlayCDB CDB(/*Base=*/nullptr, /*FallbackFlags=*/{},
  tooling::ArgumentsAdjuster(CommandMangler::forTests()));
+
   BackgroundIndex Idx(
-  Context::empty(), FS, CDB, [&](llvm::StringRef) { return &MSS; },
-  /*ThreadPoolSize=*/4, /*OnProgress=*/nullptr, std::move(ContextProvider));
+  FS, CDB, [&](llvm::StringRef) { return &MSS; }, std::move(Opts));
   // Index the two files.
   for (auto &Cmd : Cmds)
 CDB.setCompileCommand(testPath(Cmd.Filename), std::move(Cmd));
@@ -181,8 +182,8 @@
   size_t CacheHits = 0;
   MemoryShardStorage MSS(Storage, CacheHits);
   OverlayCDB CDB(/*Base=*/nullptr);
-  BackgroundIndex Idx(Context::empty(), FS, CDB,
-  [&](llvm::StringRef) { return &MSS; });
+  BackgroundIndex Idx(FS, CDB, [&](llvm::StringRef) { return &MSS; },
+  /*Opts=*/{});
 
   tooling::CompileCommand Cmd;
   Cmd.Filename = testPath("root/A.cc");
@@ -248,8 +249,8 @@
   // Check nothing is loaded from Storage, but A.cc and A.h has been stored.
   {
 OverlayCDB CDB(/*Base=*/nullptr);
-BackgroundIndex Idx(Context::empty(), FS, CDB,
-[&](llvm::StringRef) { return &MSS; });
+BackgroundIndex Idx(FS, CDB, [&](llvm::StringRef) { return &MSS; },
+/*Opts=*/{});
 CDB.setCompileCommand(testPath("root/A.cc"), Cmd);
 ASSERT_TRUE(Idx.blockUntilIdleForTest());
   }
@@ -258,8 +259,8 @@
 
   {
 OverlayCDB CDB(/*Base=*/nullptr);
-BackgroundIndex Idx(Context::empty(), FS, CDB,
-[&](llvm::StringRef) { return &MSS; });
+BackgroundIndex Idx(FS, CDB, [&](llvm::StringRef) { return &MSS; },
+/*Opts=*/{});
 CDB.setCompileCommand(testPath("root/A.cc"), Cmd);
 ASSERT_TRUE(Idx.blockUntilIdleForTest());
   }
@@ -316,8 +317,8 @@
   Cmd.CommandLine = {"clang++", testPath("root/A.cc")};
   {
 OverlayCDB CDB(/*Base=*/nullptr);
-BackgroundIndex Idx(Context::empty(), FS, CDB,
-[&](llvm::StringRef) { return &MSS; });
+BackgroundIndex Idx(FS, CDB, [&](llvm::StringRef) { return &MSS; },
+/*Opts=*/{});
 CDB.setCompileCommand(testPath("root/A.cc"), Cmd);
 ASSERT_TRUE(Idx.blockUntilIdleForTest());
   }
@@ -366,8 +367,8 @@
   // Check nothing is loaded from Storage, but A.cc and A.h has been stored.
   {
 OverlayCDB CDB(/*Base=*/nullptr);
-BackgroundIndex Idx(Context::empty(), FS, CDB,
-[&](llvm::StringRef) { return &MSS; });
+BackgroundIndex Idx(FS, CDB, [&](llvm::StringRef) { return &MSS; },
+/*Opts=*/{});
 CDB.setCompileCommand(testPath("root/A.cc"), Cmd);
 ASSERT_TRUE(Idx.blockUntilIdleForTest());
   }
@@ -381,8 +382,8 @@
   )cpp";
   {
 OverlayCDB CDB(/*Base=*/nullptr);
-BackgroundIndex Idx(Context::empty(), FS, CDB,
-[&](llvm::StringRef) { return &MSS; });
+BackgroundIndex Idx(FS, CDB, [&](llvm::StringRef) { return &MSS; },
+/*Opts=*/{});
 CDB.setCompileCommand(testPath("root/A.cc"), Cmd);
 ASSERT_TRUE(Idx.blockUntilIdleForTest());

[clang-tools-extra] 4f2e7f6 - [clangd] Try to fix windows buildbot. NFC

2020-07-04 Thread Sam McCall via cfe-commits

Author: Sam McCall
Date: 2020-07-04T12:03:46+02:00
New Revision: 4f2e7f6fb1f212a84d1647920963b66b21175a24

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

LOG: [clangd] Try to fix windows buildbot. NFC

http://45.33.8.238/win/19116/step_9.txt

Added: 


Modified: 
clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp 
b/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
index 352a58bf818e..cb4d23e0be34 100644
--- a/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
+++ b/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
@@ -144,8 +144,10 @@ TEST_F(BackgroundIndexTest, Config) {
   Context::empty(), FS, CDB, [&](llvm::StringRef) { return &MSS; },
   /*ThreadPoolSize=*/4, /*OnProgress=*/nullptr, 
std::move(ContextProvider));
   // Index the two files.
-  for (auto &Cmd : Cmds)
-CDB.setCompileCommand(testPath(Cmd.Filename), std::move(Cmd));
+  for (auto &Cmd : Cmds) {
+std::string FullPath = testPath(Cmd.Filename);
+CDB.setCompileCommand(FullPath, std::move(Cmd));
+  }
   // Wait for both files to be indexed.
   ASSERT_TRUE(Idx.blockUntilIdleForTest());
   EXPECT_THAT(runFuzzyFind(Idx, ""),



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


[PATCH] D83095: [clangd] Config: compute config in TUScheduler and BackgroundIndex

2020-07-04 Thread Sam McCall via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG15a60fe09f46: [clangd] Config: compute config in TUScheduler 
and BackgroundIndex (authored by sammccall).

Changed prior to commit:
  https://reviews.llvm.org/D83095?vs=275388&id=275487#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83095

Files:
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/TUScheduler.cpp
  clang-tools-extra/clangd/TUScheduler.h
  clang-tools-extra/clangd/index/Background.cpp
  clang-tools-extra/clangd/index/Background.h
  clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
  clang-tools-extra/clangd/unittests/ClangdTests.cpp
  clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp

Index: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
===
--- clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
+++ clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
@@ -65,8 +65,20 @@
   return true;
 }
 
+// Dummy ContextProvider to verify the provider is invoked & contexts are used.
+static Key BoundPath;
+Context bindPath(PathRef F) {
+  return Context::current().derive(BoundPath, F.str());
+}
+llvm::StringRef boundPath() {
+  const std::string *V = Context::current().get(BoundPath);
+  return V ? *V : llvm::StringRef("");
+}
+
 TUScheduler::Options optsForTest() {
-  return TUScheduler::Options(ClangdServer::optsForTest());
+  TUScheduler::Options Opts(ClangdServer::optsForTest());
+  Opts.ContextProvider = bindPath;
+  return Opts;
 }
 
 class TUSchedulerTests : public ::testing::Test {
@@ -454,6 +466,7 @@
   [File, Nonce, Version(Inputs.Version), &Mut, &TotalUpdates,
&LatestDiagVersion](std::vector) {
 EXPECT_THAT(Context::current().get(NonceKey), Pointee(Nonce));
+EXPECT_EQ(File, boundPath());
 
 std::lock_guard Lock(Mut);
 ++TotalUpdates;
@@ -474,6 +487,7 @@
   [File, Inputs, Nonce, &Mut,
&TotalASTReads](Expected AST) {
 EXPECT_THAT(Context::current().get(NonceKey), Pointee(Nonce));
+EXPECT_EQ(File, boundPath());
 
 ASSERT_TRUE((bool)AST);
 EXPECT_EQ(AST->Inputs.Contents, Inputs.Contents);
@@ -493,6 +507,7 @@
   [File, Inputs, Nonce, &Mut,
&TotalPreambleReads](Expected Preamble) {
 EXPECT_THAT(Context::current().get(NonceKey), Pointee(Nonce));
+EXPECT_EQ(File, boundPath());
 
 ASSERT_TRUE((bool)Preamble);
 EXPECT_EQ(Preamble->Contents, Inputs.Contents);
@@ -849,18 +864,22 @@
 }
 
 TEST_F(TUSchedulerTests, Run) {
-  TUScheduler S(CDB, optsForTest());
+  auto Opts = optsForTest();
+  Opts.ContextProvider = bindPath;
+  TUScheduler S(CDB, Opts);
   std::atomic Counter(0);
-  S.run("add 1", [&] { ++Counter; });
-  S.run("add 2", [&] { Counter += 2; });
+  S.run("add 1", /*Path=*/"", [&] { ++Counter; });
+  S.run("add 2", /*Path=*/"", [&] { Counter += 2; });
   ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10)));
   EXPECT_EQ(Counter.load(), 3);
 
   Notification TaskRun;
   Key TestKey;
   WithContextValue CtxWithKey(TestKey, 10);
-  S.run("props context", [&] {
+  const char *Path = "somepath";
+  S.run("props context", Path, [&] {
 EXPECT_EQ(Context::current().getExisting(TestKey), 10);
+EXPECT_EQ(Path, boundPath());
 TaskRun.notify();
   });
   TaskRun.wait();
Index: clang-tools-extra/clangd/unittests/ClangdTests.cpp
===
--- clang-tools-extra/clangd/unittests/ClangdTests.cpp
+++ clang-tools-extra/clangd/unittests/ClangdTests.cpp
@@ -10,6 +10,7 @@
 #include "ClangdLSPServer.h"
 #include "ClangdServer.h"
 #include "CodeComplete.h"
+#include "ConfigFragment.h"
 #include "GlobalCompilationDatabase.h"
 #include "Matchers.h"
 #include "SyncAPI.h"
@@ -19,6 +20,7 @@
 #include "support/Threading.h"
 #include "clang/Config/config.h"
 #include "clang/Sema/CodeCompleteConsumer.h"
+#include "clang/Tooling/ArgumentsAdjusters.h"
 #include "llvm/ADT/None.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SmallVector.h"
@@ -47,6 +49,7 @@
 using ::testing::Gt;
 using ::testing::IsEmpty;
 using ::testing::Pair;
+using ::testing::SizeIs;
 using ::testing::UnorderedElementsAre;
 
 MATCHER_P2(DeclAt, File, Range, "") {
@@ -301,6 +304,48 @@
   EXPECT_EQ(Callbacks.Got, 42);
 }
 
+TEST(ClangdServerTest, RespectsConfig) {
+  // Go-to-definition will resolve as marked if FOO is defined.
+  Annotations Example(R"cpp(
+  #ifdef FOO
+  int [[x]];
+  #else
+  int x;
+  #endif
+  int y = ^x;
+  )cpp");
+  // Provide conditional config that defines FOO for foo.cc.
+  class ConfigProvider : public config::Provider {
+std::vector
+getFragments

[PATCH] D83095: [clangd] Config: compute config in TUScheduler and BackgroundIndex

2020-07-04 Thread Sam McCall via Phabricator via cfe-commits
sammccall marked 2 inline comments as done.
sammccall added inline comments.



Comment at: clang-tools-extra/clangd/ClangdServer.cpp:748
 
+Context ClangdServer::createProcessingContext(PathRef File) const {
+  if (!ConfigProvider)

kadircet wrote:
> sammccall wrote:
> > kadircet wrote:
> > > why not make this a free function in `ConfigProvider.h` ?
> > > 
> > > that way we could just keep passing ConfigProvider around rather than a 
> > > derived lambda.
> > > It makes testing more cumbersome, but enables us to move the logic out of 
> > > ClangdServer
> > Oh, I thought hiding this logic in ClangdServer was a feature!
> > 
> > For instance, if we want to report configuration errors as LSP diagnostics 
> > or as notifications, we need to have access to the ClangdServer to do that.
> hmm, I thought we would rather make DiagnosticHandler an input parameter in 
> such a scenario, (we even have the alias `config::DiagnosticCallback` 😅) with 
> a nullptr just logging the errors and warnings (as in current implementation).
> 
> I don't see any other interaction but surfacing diagnostics, and I believe it 
> would look nicer if we provided a callback for diags when creating context 
> with configs. But this one also gets the job done, so up to you.
If i'm understanding what you mean (pass a callback in clangdserver::options?) 
I don't think it's the right level of abstraction. E.g. for diagnostics, we do 
have a lifecycle and data model around diagnostics that clangdserver is 
responsible for producing, and delivering raw diagnostics one at a time doesn't 
seem enough.
Similarly if we want warning/error notifications that seems like a higher level 
thing to add to clangdserver::callbacks.
(If it's the ClangdServer builder's responsibility to handle the errors, then 
the ConfigProvider interface isn't quite the right one in ClangdServer::Options 
- why expose the errors to clangdserver if they're just going to be passed back 
again?)

Anyway, we probably can't know for sure until we come to do richer error 
reporting, which isn't a top priority at the moment. This is my best guess for 
where we should handle errors (or at least translate them) but I don't think 
it's terribly hard to move later. So I'll leave as-is for now, to unblock the 
more critical parts of this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83095



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


[clang-tools-extra] 15a60fe - [clangd] Config: compute config in TUScheduler and BackgroundIndex

2020-07-04 Thread Sam McCall via cfe-commits

Author: Sam McCall
Date: 2020-07-04T11:18:14+02:00
New Revision: 15a60fe09f4618a7fb451f37aebfd1a671f83713

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

LOG: [clangd] Config: compute config in TUScheduler and BackgroundIndex

Summary:
ClangdServer owns the question of exactly which config to create, but
TUScheduler/BackgroundIndex control threads and so decide at which point
to inject it.

Reviewers: kadircet

Subscribers: ilya-biryukov, javed.absar, MaskRay, jkorous, arphaman, usaxena95, 
cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang-tools-extra/clangd/ClangdServer.cpp
clang-tools-extra/clangd/ClangdServer.h
clang-tools-extra/clangd/TUScheduler.cpp
clang-tools-extra/clangd/TUScheduler.h
clang-tools-extra/clangd/index/Background.cpp
clang-tools-extra/clangd/index/Background.h
clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
clang-tools-extra/clangd/unittests/ClangdTests.cpp
clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/ClangdServer.cpp 
b/clang-tools-extra/clangd/ClangdServer.cpp
index b33d53699405..6ac2f67d55b3 100644
--- a/clang-tools-extra/clangd/ClangdServer.cpp
+++ b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -8,6 +8,7 @@
 
 #include "ClangdServer.h"
 #include "CodeComplete.h"
+#include "Config.h"
 #include "FindSymbols.h"
 #include "Format.h"
 #include "HeaderSourceSwitch.h"
@@ -132,7 +133,7 @@ ClangdServer::Options::operator TUScheduler::Options() 
const {
 ClangdServer::ClangdServer(const GlobalCompilationDatabase &CDB,
const ThreadsafeFS &TFS, const Options &Opts,
Callbacks *Callbacks)
-: TFS(TFS),
+: ConfigProvider(Opts.ConfigProvider), TFS(TFS),
   DynamicIdx(Opts.BuildDynamicSymbolIndex
  ? new FileIndex(Opts.HeavyweightDynamicSymbolIndex)
  : nullptr),
@@ -147,7 +148,14 @@ ClangdServer::ClangdServer(const GlobalCompilationDatabase 
&CDB,
   // FIXME(ioeric): this can be slow and we may be able to index on less
   // critical paths.
   WorkScheduler(
-  CDB, TUScheduler::Options(Opts),
+  CDB,
+  [&, this] {
+TUScheduler::Options O(Opts);
+O.ContextProvider = [this](PathRef P) {
+  return createProcessingContext(P);
+};
+return O;
+  }(),
   std::make_unique(
   DynamicIdx.get(), Callbacks, Opts.TheiaSemanticHighlighting)) {
   // Adds an index to the stack, at higher priority than existing indexes.
@@ -170,7 +178,8 @@ ClangdServer::ClangdServer(const GlobalCompilationDatabase 
&CDB,
 [Callbacks](BackgroundQueue::Stats S) {
   if (Callbacks)
 Callbacks->onBackgroundIndexProgress(S);
-});
+},
+[this](PathRef P) { return createProcessingContext(P); });
 AddIndex(BackgroundIdx.get());
   }
   if (DynamicIdx)
@@ -335,7 +344,7 @@ void ClangdServer::formatOnType(PathRef File, 
llvm::StringRef Code,
   Result.push_back(replacementToEdit(Code, R));
 return CB(Result);
   };
-  WorkScheduler.run("FormatOnType", std::move(Action));
+  WorkScheduler.run("FormatOnType", File, std::move(Action));
 }
 
 void ClangdServer::prepareRename(PathRef File, Position Pos,
@@ -585,7 +594,7 @@ void ClangdServer::formatCode(PathRef File, llvm::StringRef 
Code,
 tooling::calculateRangesAfterReplacements(IncludeReplaces, Ranges),
 File)));
   };
-  WorkScheduler.run("Format", std::move(Action));
+  WorkScheduler.run("Format", File, std::move(Action));
 }
 
 void ClangdServer::findDocumentHighlights(
@@ -646,7 +655,7 @@ void ClangdServer::workspaceSymbols(
 llvm::StringRef Query, int Limit,
 Callback> CB) {
   WorkScheduler.run(
-  "getWorkspaceSymbols",
+  "getWorkspaceSymbols", /*Path=*/"",
   [Query = Query.str(), Limit, CB = std::move(CB), this]() mutable {
 CB(clangd::getWorkspaceSymbols(Query, Limit, Index,
WorkspaceRoot.getValueOr("")));
@@ -736,6 +745,31 @@ llvm::StringMap 
ClangdServer::fileStats() const {
   return WorkScheduler.fileStats();
 }
 
+Context ClangdServer::createProcessingContext(PathRef File) const {
+  if (!ConfigProvider)
+return Context::current().clone();
+
+  config::Params Params;
+  if (!File.empty()) {
+assert(llvm::sys::path::is_absolute(File));
+llvm::SmallString<256> PosixPath = File;
+llvm::sys::path::native(PosixPath, llvm::sys::path::Style::posix);
+Params.Path = PosixPath.str();
+  }
+
+  auto DiagnosticHandler = [](const llvm::SMDiagnostic &Diag) {
+if (Diag.getKind() == llvm::SourceMgr

[PATCH] D82964: [clangd] Config: loading and caching config from disk.

2020-07-04 Thread Sam McCall via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG8bd000a65fe4: [clangd] Config: loading and caching config 
from disk. (authored by sammccall).

Changed prior to commit:
  https://reviews.llvm.org/D82964?vs=275384&id=275484#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82964

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/ConfigProvider.cpp
  clang-tools-extra/clangd/ConfigProvider.h
  clang-tools-extra/clangd/unittests/CMakeLists.txt
  clang-tools-extra/clangd/unittests/ConfigProviderTests.cpp
  clang-tools-extra/clangd/unittests/TestFS.cpp
  clang-tools-extra/clangd/unittests/TestFS.h

Index: clang-tools-extra/clangd/unittests/TestFS.h
===
--- clang-tools-extra/clangd/unittests/TestFS.h
+++ clang-tools-extra/clangd/unittests/TestFS.h
@@ -69,7 +69,8 @@
 const char *testRoot();
 
 // Returns a suitable absolute path for this OS.
-std::string testPath(PathRef File);
+std::string testPath(PathRef File,
+ llvm::sys::path::Style = llvm::sys::path::Style::native);
 
 // unittest: is a scheme that refers to files relative to testRoot()
 // This anchor is used to force the linker to link in the generated object file
Index: clang-tools-extra/clangd/unittests/TestFS.cpp
===
--- clang-tools-extra/clangd/unittests/TestFS.cpp
+++ clang-tools-extra/clangd/unittests/TestFS.cpp
@@ -79,13 +79,13 @@
 #endif
 }
 
-std::string testPath(PathRef File) {
+std::string testPath(PathRef File, llvm::sys::path::Style Style) {
   assert(llvm::sys::path::is_relative(File) && "FileName should be relative");
 
   llvm::SmallString<32> NativeFile = File;
-  llvm::sys::path::native(NativeFile);
+  llvm::sys::path::native(NativeFile, Style);
   llvm::SmallString<32> Path;
-  llvm::sys::path::append(Path, testRoot(), NativeFile);
+  llvm::sys::path::append(Path, Style, testRoot(), NativeFile);
   return std::string(Path.str());
 }
 
Index: clang-tools-extra/clangd/unittests/ConfigProviderTests.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/unittests/ConfigProviderTests.cpp
@@ -0,0 +1,156 @@
+//===-- ConfigProviderTests.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 "Config.h"
+#include "ConfigProvider.h"
+#include "ConfigTesting.h"
+#include "TestFS.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+#include "llvm/Support/SourceMgr.h"
+#include 
+
+namespace clang {
+namespace clangd {
+namespace config {
+namespace {
+using ::testing::ElementsAre;
+using ::testing::IsEmpty;
+
+// Provider that appends an arg to compile flags.
+// The arg is prefix, where N is the times getFragments() was called.
+// It also yields a diagnostic each time it's called.
+class FakeProvider : public Provider {
+  std::string Prefix;
+  mutable std::atomic Index = {0};
+
+  std::vector
+  getFragments(const Params &, DiagnosticCallback DC) const override {
+DC(llvm::SMDiagnostic("", llvm::SourceMgr::DK_Error, Prefix));
+CompiledFragment F =
+[Arg(Prefix + std::to_string(++Index))](const Params &P, Config &C) {
+  C.CompileFlags.Edits.push_back(
+  [Arg](std::vector &Argv) { Argv.push_back(Arg); });
+  return true;
+};
+return {F};
+  }
+
+public:
+  FakeProvider(llvm::StringRef Prefix) : Prefix(Prefix) {}
+};
+
+std::vector getAddedArgs(Config &C) {
+  std::vector Argv;
+  for (auto &Edit : C.CompileFlags.Edits)
+Edit(Argv);
+  return Argv;
+}
+
+// The provider from combine() should invoke its providers in order, and not
+// cache their results.
+TEST(ProviderTest, Combine) {
+  CapturedDiags Diags;
+  std::vector> Providers;
+  Providers.push_back(std::make_unique("foo"));
+  Providers.push_back(std::make_unique("bar"));
+  auto Combined = Provider::combine(std::move(Providers));
+  Config Cfg = Combined->getConfig(Params(), Diags.callback());
+  EXPECT_THAT(Diags.Diagnostics,
+  ElementsAre(DiagMessage("foo"), DiagMessage("bar")));
+  EXPECT_THAT(getAddedArgs(Cfg), ElementsAre("foo1", "bar1"));
+  Diags.Diagnostics.clear();
+
+  Cfg = Combined->getConfig(Params(), Diags.callback());
+  EXPECT_THAT(Diags.Diagnostics,
+  ElementsAre(DiagMessage("foo"), DiagMessage("bar")));
+  EXPECT_THAT(getAddedArgs(Cfg), ElementsAre("foo2", "bar2"));
+}
+
+const char *AddFooWithErr = R"yaml(
+CompileFlags:
+  Add: foo
+  Unknown: 42
+)yaml";
+
+const char *AddBarBaz = R"yaml(
+CompileFlags:
+  Add: bar
+---
+C

[clang-tools-extra] 8bd000a - [clangd] Config: loading and caching config from disk.

2020-07-04 Thread Sam McCall via cfe-commits

Author: Sam McCall
Date: 2020-07-04T10:48:31+02:00
New Revision: 8bd000a65fe4452c09855115d5204a2a46838004

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

LOG: [clangd] Config: loading and caching config from disk.

Summary:
The Provider extension point is designed to also be implemented by
ClangdLSPServer (to inject config-over-lsp) and likely by embedders.

Reviewers: kadircet

Subscribers: mgorny, ilya-biryukov, MaskRay, jkorous, arphaman, jfb, usaxena95, 
cfe-commits

Tags: #clang

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

Added: 
clang-tools-extra/clangd/ConfigProvider.cpp
clang-tools-extra/clangd/unittests/ConfigProviderTests.cpp

Modified: 
clang-tools-extra/clangd/CMakeLists.txt
clang-tools-extra/clangd/ConfigProvider.h
clang-tools-extra/clangd/unittests/CMakeLists.txt
clang-tools-extra/clangd/unittests/TestFS.cpp
clang-tools-extra/clangd/unittests/TestFS.h

Removed: 




diff  --git a/clang-tools-extra/clangd/CMakeLists.txt 
b/clang-tools-extra/clangd/CMakeLists.txt
index 916826a8679b..9eb06941e4dd 100644
--- a/clang-tools-extra/clangd/CMakeLists.txt
+++ b/clang-tools-extra/clangd/CMakeLists.txt
@@ -38,6 +38,7 @@ add_clang_library(clangDaemon
   Compiler.cpp
   Config.cpp
   ConfigCompile.cpp
+  ConfigProvider.cpp
   ConfigYAML.cpp
   Diagnostics.cpp
   DraftStore.cpp

diff  --git a/clang-tools-extra/clangd/ConfigProvider.cpp 
b/clang-tools-extra/clangd/ConfigProvider.cpp
new file mode 100644
index ..4b466d53e293
--- /dev/null
+++ b/clang-tools-extra/clangd/ConfigProvider.cpp
@@ -0,0 +1,207 @@
+//===--- ConfigProvider.cpp - Loading of user configuration 
---===//
+//
+// 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 "ConfigProvider.h"
+#include "Config.h"
+#include "ConfigFragment.h"
+#include "support/ThreadsafeFS.h"
+#include "support/Trace.h"
+#include "llvm/ADT/StringMap.h"
+#include "llvm/Support/Path.h"
+#include 
+
+namespace clang {
+namespace clangd {
+namespace config {
+
+// Threadsafe cache around reading a YAML config file from disk.
+class FileConfigCache {
+  std::mutex Mu;
+  llvm::SmallVector CachedValue;
+  llvm::sys::TimePoint<> MTime = {};
+  unsigned Size = -1;
+
+  void updateCacheLocked(const llvm::vfs::Status &Stat,
+ llvm::vfs::FileSystem &FS, DiagnosticCallback DC) {
+if (Size == Stat.getSize() && MTime == Stat.getLastModificationTime())
+  return; // Already valid.
+
+Size = Stat.getSize();
+MTime = Stat.getLastModificationTime();
+CachedValue.clear();
+
+auto Buf = FS.getBufferForFile(Path);
+// If stat() succeeds but we failed to read, don't cache failure.
+if (!Buf) {
+  Size = -1;
+  MTime = {};
+  return;
+}
+
+// If file changed between stat and open, we don't know its mtime.
+// For simplicity, don't cache the value in this case (use a bad key).
+if (Buf->get()->getBufferSize() != Size) {
+  Size = -1;
+  MTime = {};
+}
+
+// Finally parse and compile the actual fragments.
+for (auto &Fragment :
+ Fragment::parseYAML(Buf->get()->getBuffer(), Path, DC))
+  CachedValue.push_back(std::move(Fragment).compile(DC));
+  }
+
+public:
+  // Must be set before the cache is used. Not a constructor param to allow
+  // computing ancestor-relative paths to be deferred.
+  std::string Path;
+
+  // Retrieves up-to-date config fragments from disk.
+  // A cached result may be reused if the mtime and size are unchanged.
+  // (But several concurrent read()s can miss the cache after a single change).
+  // Future performance ideas:
+  // - allow caches to be reused based on short elapsed walltime
+  // - allow latency-sensitive operations to skip revalidating the cache
+  void read(const ThreadsafeFS &TFS, DiagnosticCallback DC,
+std::vector &Out) {
+assert(llvm::sys::path::is_absolute(Path));
+auto FS = TFS.view(/*CWD=*/llvm::None);
+auto Stat = FS->status(Path);
+if (!Stat || !Stat->isRegularFile()) {
+  // No point taking the lock to clear the cache. We know what to return.
+  // If the file comes back we'll invalidate the cache at that point.
+  return;
+}
+
+std::lock_guard Lock(Mu);
+updateCacheLocked(*Stat, *FS, DC);
+llvm::copy(CachedValue, std::back_inserter(Out));
+  }
+};
+
+std::unique_ptr Provider::fromYAMLFile(llvm::StringRef AbsPath,
+ const ThreadsafeFS &FS) {
+  class AbsFileProvider : public Provider {
+mutab