[PATCH] D44189: [RISCV] Verify the input value of -march=

[Kito Cheng via Phabricator via cfe-commits]

kito-cheng updated this revision to Diff 139256.
kito-cheng added a comment.

Update revision according Alex's review.

Changes:

- Add testcase for uppercase of -march string.
- Add testcase for repeated letter in -march.
- Add more comment.
- Add several TODO item for diagnostic message improvement.
- Fix coding style issue.


Repository:
  rC Clang

https://reviews.llvm.org/D44189

Files:
  lib/Driver/ToolChains/Arch/RISCV.cpp
  test/Driver/riscv-arch.c

Index: test/Driver/riscv-arch.c
===
--- /dev/null
+++ test/Driver/riscv-arch.c
@@ -0,0 +1,89 @@
+// RUN: %clang -target riscv32-unknown-elf -march=rv32i -### %s -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32im -### %s -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32ima -### %s -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32imaf -### %s -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32imafd -### %s -fsyntax-only 2>&1 | FileCheck %s
+
+// RUN: %clang -target riscv32-unknown-elf -march=rv32ic -### %s -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32imc -### %s -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32imac -### %s -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32imafc -### %s -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32imafdc -### %s -fsyntax-only 2>&1 | FileCheck %s
+
+// RUN: %clang -target riscv32-unknown-elf -march=rv32ia -### %s -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32iaf -### %s -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32iafd -### %s -fsyntax-only 2>&1 | FileCheck %s
+
+// RUN: %clang -target riscv32-unknown-elf -march=rv32iac -### %s -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32iafc -### %s -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32iafdc -### %s -fsyntax-only 2>&1 | FileCheck %s
+
+// RUN: %clang -target riscv32-unknown-elf -march=rv32g -### %s -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32gc -### %s -fsyntax-only 2>&1 | FileCheck %s
+
+// RUN: %clang -target riscv64-unknown-elf -march=rv64i -### %s -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv64-unknown-elf -march=rv64im -### %s -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv64-unknown-elf -march=rv64ima -### %s -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv64-unknown-elf -march=rv64imaf -### %s -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv64-unknown-elf -march=rv64imafd -### %s -fsyntax-only 2>&1 | FileCheck %s
+
+// RUN: %clang -target riscv64-unknown-elf -march=rv64ic -### %s -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv64-unknown-elf -march=rv64imc -### %s -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv64-unknown-elf -march=rv64imac -### %s -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv64-unknown-elf -march=rv64imafc -### %s -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv64-unknown-elf -march=rv64imafdc -### %s -fsyntax-only 2>&1 | FileCheck %s
+
+// RUN: %clang -target riscv64-unknown-elf -march=rv64ia -### %s -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv64-unknown-elf -march=rv64iaf -### %s -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv64-unknown-elf -march=rv64iafd -### %s -fsyntax-only 2>&1 | FileCheck %s
+
+// RUN: %clang -target riscv64-unknown-elf -march=rv64iac -### %s -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv64-unknown-elf -march=rv64iafc -### %s -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv64-unknown-elf -march=rv64iafdc -### %s -fsyntax-only 2>&1 | FileCheck %s
+
+// RUN: %clang -target riscv64-unknown-elf -march=rv64g -### %s -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv64-unknown-elf -march=rv64gc -### %s -fsyntax-only 2>&1 | FileCheck %s
+
+// CHECK-NOT: error: invalid arch name '
+
+// RUN: %clang -target riscv32-unknown-elf -march=rv32 -### %s -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32 %s
+// RV32: error: invalid arch name 'rv32'
+
+// RUN: %clang -target riscv32-unknown-elf -march=rv32m -### %s -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32M %s
+// RV32M: error: invalid arch name 'rv32m'
+
+// RUN: %clang -target riscv32-unknown-elf -march=rv32id -### %s -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32ID %s
+// RV32ID: error: invalid arch name 'rv32id'
+
+// RUN: %clang -target riscv32-unknown-elf -march=rv32l -### %s -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32L %s
+// RV32L: error: invalid arch name 'rv32l'
+
+// RUN

[PATCH] D44189: [RISCV] Verify the input value of -march=

[Kito Cheng via Phabricator via cfe-commits]

kito-cheng marked 4 inline comments as done.
kito-cheng added a comment.

Hi Alex:

Thanks for your input, check for repeated letter was missed in my last patch :)




Comment at: lib/Driver/ToolChains/Arch/RISCV.cpp:34
+
+// The canonical order specified in ISA manual.
+StringRef StdExts = "mafdc";

asb wrote:
> I'd reference Table 22.1 in RISC-V User-Level ISA V2.2 for anyone who wants 
> to verify this.
Done.



Comment at: lib/Driver/ToolChains/Arch/RISCV.cpp:37-38
+
+bool hasF = false, hasD = false;
+char baseline = MArch[4];
+

asb wrote:
> Should be HasF, HasD, and Baseline to conform to standard LLVM naming 
> conventions.
Fixed.



Comment at: lib/Driver/ToolChains/Arch/RISCV.cpp:65
+for (char c : Exts) {
+  // Check march is satisfied the canonical order.
+  while (StdExtsItr != StdExts.end() && *StdExtsItr != c)

asb wrote:
> I'd phrase this as "Check ISA extensions are specified in the canonical 
> order."
Done.



Comment at: lib/Driver/ToolChains/Arch/RISCV.cpp:99
+
+// Dependency check
+if (hasD && !hasF)

asb wrote:
> I'd be tempted to give a bit more explanation a bit more "It's illegal to 
> specify the 'd' (double-precision floating point) extension without also 
> specifying the 'f' (single precision floating-point) extension".
Done.


Repository:
  rC Clang

https://reviews.llvm.org/D44189



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

[PATCH] D44720: [clangd] Simplify fuzzy matcher (sequence alignment) by removing some condition checks.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Can you elaborate on what this patch is improving, and how?
There are some stylistic changes, and also what look like subtle logic changes, 
and some rearrangement of the algorithm - to what end?

Canonical way to run all tests is `ninja check-clang-tools`, the way you 
suggested is the right thing for rapid iteration with unit tests.
Personally, I don't use a debugger.




Comment at: clangd/FuzzyMatch.cpp:93
+  for (int I = PatN; I <= WordN; I++)
+Best = std::max(Best, Scores[PatN][I][Match].Score);
   if (isAwful(Best))

this looks like a behavior change - why?



Comment at: clangd/FuzzyMatch.cpp:241
 
-  auto MatchMissScore = PreMiss[Match].Score;
-  auto MissMissScore = PreMiss[Miss].Score;
-  if (P < PatN - 1) { // Skipping trailing characters is always free.
-MatchMissScore -= skipPenalty(W, Match);
-MissMissScore -= skipPenalty(W, Miss);
-  }
+  auto MatchMissScore = PreMiss[Match].Score + missScore(W, Match);
+  auto MissMissScore = PreMiss[Miss].Score + missScore(W, Miss);

adding the penalty unconditionally seems like a behavior change, why?



Comment at: clangd/FuzzyMatch.h:61
   bool allowMatch(int P, int W) const;
-  int skipPenalty(int W, Action Last) const;
-  int matchBonus(int P, int W, Action Last) const;
+  int missScore(int W, Action Last) const;
+  int matchScore(int P, int W, Action Last) const;

FWIW, I don't think this is an improvement - I think the clarity of purpose in 
names is more useful than having consistent signs in this case.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44720



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

[PATCH] D44720: [clangd] Simplify fuzzy matcher (sequence alignment) by removing some condition checks.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

(Sorry if I sound gruff - I'm sure there are improvements to be had here. But 
since the code is a bit dense (my fault) I have trouble inferring them from the 
deltas.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44720



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

[PATCH] D44559: [Sema] Wrong width of result of mul operation

[Andrew V. Tischenko via Phabricator via cfe-commits]

avt77 added a comment.

>> In https://reviews.llvm.org/D44559#1040799, @rjmccall wrote:
>> 
>>> I think we're correct not to warn here and that GCC/ICC are being noisy.  
>>> The existence of a temporary promotion to a wider type doesn't justify 
>>> warning on arithmetic between two operands that are the same size as the 
>>> ultimate result.  It is totally fair for users to think of this operation 
>>> as being "closed" on the original type.
>> 
>> 
>> Could you please clarify, are you saying that PR35409 
>>  is not a bug, and clang should 
>> continue to not warn in those cases?
> 
> Correct.

Does it mean we should abandon this revision? On the other hand it's a real 
bug, isn't it?


Repository:
  rC Clang

https://reviews.llvm.org/D44559



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

[PATCH] D44426: Fix llvm + clang build with Intel compiler

[Ivan Donchevskii via Phabricator via cfe-commits]

yvvan added a comment.

In https://reviews.llvm.org/D44426#1042162, @mibintc wrote:

> I added some inline comments. You are using the Intel 18.0 compiler on 
> Windows - what version of Visual Studio is in the environment?


Yes, I'm using 18.0




Comment at: include/llvm-c/Target.h:25
 
-#if defined(_MSC_VER) && !defined(inline)
+#if defined(_MSC_VER) && !defined(inline) && !defined(__INTEL_COMPILER)
 #define inline __inline

mibintc wrote:
> I really think all the Intel-compiler bug workarounds should be version 
> specific. For example, in the boost.org customizations we have checks like 
> this:
> #if defined(__INTEL_COMPILER) && (__INTEL_COMPILER >= 1500)
> 
> I believe you are using the most recent Intel compiler, 18.0, which has 
> version 1800.  Let's assume the bug will either be fixed in our 18.0 compiler 
> or in our next compiler which would be numbered 1900, so the check could be 
> like this:
> #if defined(__INTEL_COMPILER) && (__INTEL_COMPILER < 1900) // employ the 
> workaround for the Intel 18.0 compiler and older
> 
> This way the workaround will "disappear" when the bug gets fixed.
> 
> For that matter I wonder if it's still necessary to use the workaround for 
> the Microsoft compiler?
> 
> Thanks for reporting the bug into the Intel forum. If we put a bug workaround 
> into LLVM it would be very nice to have the bug reported to the offending 
> compiler. 
> 
This one will probably not be fixed because I was answered that it's the 
predicted behavior and that I need not to "#define inline __inline" if I use 
intel compiler
I agree though about versions which need to be taken into account



Comment at: include/llvm/ADT/BitmaskEnum.h:73
 
+#ifdef __INTEL_COMPILER
+template 

mibintc wrote:
> what problem is being worked around here? I checked your post into the Intel 
> compiler forum "enum members are not visible to template specialization" and 
> I can observe the bug on our Windows compiler but not our Linux compiler. 
it was recently accepted. the issues is that E::LLVM_BITMASK_LARGEST_ENUMERATOR 
from line 80 here is always invalid with intel compiler and the whole set of 
operator overloads is ignored
at least on Windows :)



Comment at: include/llvm/Analysis/AliasAnalysis.h:254
   FMRB_OnlyAccessesInaccessibleOrArgMem = FMRL_InaccessibleMem |
-  FMRL_ArgumentPointees |
+  
static_cast(FMRL_ArgumentPointees) |
   static_cast(ModRefInfo::ModRef),

mibintc wrote:
> is this a necessary workaround for the Intel compiler? It's not pretty.  
> Suggest we add the #if around it? Long term i hope this would not be 
> necessary.
i'm not sure that adding #if around makes it easier to understand :)
of course I can add it everywhere I append enums with static_cast



Comment at: lib/Target/AMDGPU/SIISelLowering.cpp:6131
 
+#ifndef __INTEL_COMPILER
 static_assert(((~(SIInstrFlags::S_NAN |

mibintc wrote:
> this assesrts with Intel compiler?  Or the expression needs to be rewritten 
> with static_cast as above?
"needs to be rewritten with static_cast" - correct. it can be done but I was a 
bit lazy and statioc_assert does not affect the outcome of build :)


https://reviews.llvm.org/D44426



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

[PATCH] D41537: Optionally add code completion results for arrow instead of dot

[Ivan Donchevskii via Phabricator via cfe-commits]

yvvan added inline comments.



Comment at: include/clang/Sema/CodeCompleteConsumer.h:565
+  /// \brief For this completion result correction is required.
+  Optional Corr = None;
+

ilya-biryukov wrote:
> Having a string replacement without an actual range to replace still moves a 
> lot of responsibility onto the clients. We should probably model corrections 
> after the fix-its for diagnostics:
> - replacements need to provide a range to be replaced, alongside with a text 
> for the replacement,
> - we should provide a list of edits to allow corrections that touch two 
> separate pieces of code.
> 
> For the fix-its in the diagnostics, see 
> [[https://reviews.llvm.org/source/clang/browse/cfe/trunk/tools/libclang/CXLoadedDiagnostic.h;327861$84
>  | CXLoadedDiagnostic.h]] for an interface exported via libclang and 
> [[https://reviews.llvm.org/source/clang/browse/cfe/trunk/include/clang/Basic/Diagnostic.h;327861$947|Diagnostic.h]]
>  for an interface exported in C++ API.
> WDYT?
I thought that fixits always exist separately from diagnostics. I will check 
this out, thanks.


https://reviews.llvm.org/D41537



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

Re: [clang-tools-extra] r328069 - Revert "[lit] Adding config initialization to lit tests in clang-tools-extra"

[Roman Lebedev via cfe-commits]

On Wed, Mar 21, 2018 at 5:28 AM, Petr Hosek via cfe-commits
 wrote:
> Author: phosek
> Date: Tue Mar 20 19:28:22 2018
> New Revision: 328069
>
> URL: http://llvm.org/viewvc/llvm-project?rev=328069&view=rev
> Log:
> Revert "[lit] Adding config initialization to lit tests in clang-tools-extra"
>
> This reverts commit r328060 because a test that was inteded to run on
> Windows but never ran before due to the missing config initialization
> is now being executed and is failing.
Only one test?
Would it be possible to just completely disable that test for now?
Since it has never run, it's not like it is crucial for the CI...

This revision is the only thing that is blocking https://reviews.llvm.org/D41102

> Modified:
> clang-tools-extra/trunk/test/lit.site.cfg.in
>
> Modified: clang-tools-extra/trunk/test/lit.site.cfg.in
> URL: 
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/lit.site.cfg.in?rev=328069&r1=328068&r2=328069&view=diff
> ==
> --- clang-tools-extra/trunk/test/lit.site.cfg.in (original)
> +++ clang-tools-extra/trunk/test/lit.site.cfg.in Tue Mar 20 19:28:22 2018
> @@ -23,7 +23,5 @@ except KeyError:
>  key, = e.args
>  lit_config.fatal("unable to find %r parameter, use '--param=%s=VALUE'" % 
> (key,key))
>
> -@LIT_SITE_CFG_IN_FOOTER@
> -
>  # Let the main config do the real work.
>  lit_config.load_config(config, "@CLANG_TOOLS_SOURCE_DIR@/test/lit.cfg")

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

[PATCH] D38455: [clang-tidy] new cppcoreguidelines-narrowing-conversions check.

[Clement Courbet via Phabricator via cfe-commits]

courbet added inline comments.



Comment at: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:26
+  binaryOperator(
+  anyOf(hasOperatorName("+="), hasOperatorName("-=")),
+  hasLHS(hasType(isInteger())),

aaron.ballman wrote:
> courbet wrote:
> > aaron.ballman wrote:
> > > Why only these two operators? This does not match the behavior from the 
> > > C++ Core Guideline check itself 
> > > (https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Res-narrowing).
> > These provided the best signal to noise ratio. Also they are the most 
> > dangerous (in a loop, you might end up losing one unit per iteration). I'll 
> > add other operators later if that's fine with you.
> If the check doesn't cover anything listed in the C++ Core Guideline examples 
> or enforcement wording, I have a hard time accepting it as the initial commit 
> for such a check.
> 
> Of special interest to me is the request from the C++ Core Guideline authors 
> themselves: 
> ```
>   - flag all floating-point to integer conversions (maybe only float->char 
> and double->int. Here be dragons! we need data)
>   - flag all long->char (I suspect int->char is very common. Here be dragons! 
> we need data)
> ```
> I think we need data as well -- writing the check and then testing it over 
> several million lines of source could give us some of that data, which we can 
> then feed back to the guideline authors, so that we come up with a check 
> that's realistic.
Quoting the guideline:
"A good analyzer can detect all narrowing conversions. However, flagging all 
narrowing conversions will lead to a lot of false positives." Then follows a 
list of suggestions. While +=/-= are not in the list of suggestions, they are a 
subset of "flag all floating-point to integer conversions" that I've found to 
be useful (very low rate of false positives) by running on our codebase.
I agree that more data would be useful, so I'll do an analysis of flagging all 
(non-ceil/floor float/double)->integral conversions.






Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D38455



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

[PATCH] D44559: [Sema] Wrong width of result of mul operation

[Andrew V. Tischenko via Phabricator via cfe-commits]

avt77 updated this revision to Diff 139271.
avt77 added a comment.

I removed the dependence on arch and updated the tests.


https://reviews.llvm.org/D44559

Files:
  lib/Sema/SemaChecking.cpp
  test/Sema/conversion.c
  test/SemaCXX/conversion.cpp


Index: test/SemaCXX/conversion.cpp
===
--- test/SemaCXX/conversion.cpp
+++ test/SemaCXX/conversion.cpp
@@ -298,3 +298,15 @@
   conditional_run_13(NULL);
 }
 }
+
+template
+T mul_t(T a, T b) {
+  int c = a * b;
+  T d1 = c; // expected-warning{{implicit conversion loses integer precision: 
'int' to 'char'}}
+  T d2 = a * b; // expected-warning{{implicit conversion loses integer 
precision: 'int' to 'char'}}
+  return d1 + d2;
+}
+
+char mul_t_test (char a, char b) {
+return mul_t(a, b); // expected-note{{in instantiation of function 
template specialization 'mul_t' requested here}}
+}
Index: test/Sema/conversion.c
===
--- test/Sema/conversion.c
+++ test/Sema/conversion.c
@@ -302,7 +302,7 @@
 }
 
 void test15(char c) {
-  c = c + 1 + c * 2;
+  c = c + 1 + c * 2; // expected-warning {{implicit conversion loses integer 
precision: 'int'}}
   c = (short) c + 1 + c * 2; // expected-warning {{implicit conversion loses 
integer precision}}
 }
 
@@ -448,3 +448,11 @@
   b -= a; // expected-warning {{implicit conversion when assigning computation 
result loses floating-point precision: 'double' to 'float'}}
   return b;
 }
+
+unsigned short foo1(unsigned char a) {
+  return a * a; // expected-warning {{implicit conversion loses integer 
precision: 'int' to 'unsigned short'}}
+}
+
+signed short bar1(signed char a) {
+  return a * a; // expected-warning {{implicit conversion loses integer 
precision: 'int' to 'short'}}
+}
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -8536,8 +8536,13 @@
   return meet;
 }
 
+case BO_Mul: {
+  // The result width should be calculated in advance
+  unsigned opWidth = C.getIntWidth(GetExprType(E));
+  return IntRange(opWidth, false);
+}
+
 // The default behavior is okay for these.
-case BO_Mul:
 case BO_Add:
 case BO_Xor:
 case BO_Or:


Index: test/SemaCXX/conversion.cpp
===
--- test/SemaCXX/conversion.cpp
+++ test/SemaCXX/conversion.cpp
@@ -298,3 +298,15 @@
   conditional_run_13(NULL);
 }
 }
+
+template
+T mul_t(T a, T b) {
+  int c = a * b;
+  T d1 = c; // expected-warning{{implicit conversion loses integer precision: 'int' to 'char'}}
+  T d2 = a * b; // expected-warning{{implicit conversion loses integer precision: 'int' to 'char'}}
+  return d1 + d2;
+}
+
+char mul_t_test (char a, char b) {
+return mul_t(a, b); // expected-note{{in instantiation of function template specialization 'mul_t' requested here}}
+}
Index: test/Sema/conversion.c
===
--- test/Sema/conversion.c
+++ test/Sema/conversion.c
@@ -302,7 +302,7 @@
 }
 
 void test15(char c) {
-  c = c + 1 + c * 2;
+  c = c + 1 + c * 2; // expected-warning {{implicit conversion loses integer precision: 'int'}}
   c = (short) c + 1 + c * 2; // expected-warning {{implicit conversion loses integer precision}}
 }
 
@@ -448,3 +448,11 @@
   b -= a; // expected-warning {{implicit conversion when assigning computation result loses floating-point precision: 'double' to 'float'}}
   return b;
 }
+
+unsigned short foo1(unsigned char a) {
+  return a * a; // expected-warning {{implicit conversion loses integer precision: 'int' to 'unsigned short'}}
+}
+
+signed short bar1(signed char a) {
+  return a * a; // expected-warning {{implicit conversion loses integer precision: 'int' to 'short'}}
+}
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -8536,8 +8536,13 @@
   return meet;
 }
 
+case BO_Mul: {
+  // The result width should be calculated in advance
+  unsigned opWidth = C.getIntWidth(GetExprType(E));
+  return IntRange(opWidth, false);
+}
+
 // The default behavior is okay for these.
-case BO_Mul:
 case BO_Add:
 case BO_Xor:
 case BO_Or:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r328086 - [ASTMatchers] Regenerate doc.

[Clement Courbet via cfe-commits]

Author: courbet
Date: Wed Mar 21 03:48:00 2018
New Revision: 328086

URL: http://llvm.org/viewvc/llvm-project?rev=328086&view=rev
Log:
[ASTMatchers] Regenerate doc.

Modified:
cfe/trunk/docs/LibASTMatchersReference.html

Modified: cfe/trunk/docs/LibASTMatchersReference.html
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/LibASTMatchersReference.html?rev=328086&r1=328085&r2=328086&view=diff
==
--- cfe/trunk/docs/LibASTMatchersReference.html (original)
+++ cfe/trunk/docs/LibASTMatchersReference.html Wed Mar 21 03:48:00 2018
@@ -4353,7 +4353,7 @@ with hasAnyArgument(...)
 
 For ObjectiveC, given
   @interface I - (void) f:(int) y; @end
-  void foo(I *i) { [i f:12] }
+  void foo(I *i) { [i f:12]; }
 objcMessageExpr(hasAnyArgument(integerLiteral(equals(12
   matches [i f:12]
 
@@ -4706,7 +4706,7 @@ with hasAnyArgument(...)
 
 For ObjectiveC, given
   @interface I - (void) f:(int) y; @end
-  void foo(I *i) { [i f:12] }
+  void foo(I *i) { [i f:12]; }
 objcMessageExpr(hasAnyArgument(integerLiteral(equals(12
   matches [i f:12]
 
@@ -5659,7 +5659,7 @@ with hasAnyArgument(...)
 
 For ObjectiveC, given
   @interface I - (void) f:(int) y; @end
-  void foo(I *i) { [i f:12] }
+  void foo(I *i) { [i f:12]; }
 objcMessageExpr(hasAnyArgument(integerLiteral(equals(12
   matches [i f:12]
 


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

[PATCH] D44729: [ASTMatchers] Add hasSubExpr() matcher.

[Clement Courbet via Phabricator via cfe-commits]

courbet created this revision.
courbet added reviewers: aaron.ballman, alexfh.
Herald added a subscriber: klimek.

This is needed to implement more checks in https://reviews.llvm.org/D38455.


Repository:
  rC Clang

https://reviews.llvm.org/D44729

Files:
  docs/LibASTMatchersReference.html
  include/clang/ASTMatchers/ASTMatchers.h
  unittests/ASTMatchers/ASTMatchersTraversalTest.cpp


Index: unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -1232,6 +1232,13 @@
 cxxMethodDecl(ofClass(hasName("X"))), true, "-std=gnu++98"));
 }
 
+TEST(HasSubExpr, MatchesSimpleCase) {
+  EXPECT_TRUE(matches("int i = 0.0;",
+  implicitCastExpr(hasSubExpr(floatLiteral();
+  EXPECT_TRUE(notMatches("int i = '0';",
+  implicitCastExpr(hasSubExpr(floatLiteral();
+}
+
 TEST(HasDestinationType, MatchesSimpleCase) {
   EXPECT_TRUE(matches("char* p = static_cast(0);",
   cxxStaticCastExpr(hasDestinationType(
Index: include/clang/ASTMatchers/ASTMatchers.h
===
--- include/clang/ASTMatchers/ASTMatchers.h
+++ include/clang/ASTMatchers/ASTMatchers.h
@@ -4109,6 +4109,21 @@
   return InnerMatcher.matches(Node.getType(), Finder, Builder);
 }
 
+/// \brief Matches casts whose subexpr matches a given matcher.
+///
+/// Example matches the first cast
+/// (matcher = implicitCastExpr(hasSubExpr(floatLiteral())
+///   int i = 0.0;
+///   int j = '0';
+AST_POLYMORPHIC_MATCHER_P(
+hasSubExpr,
+AST_POLYMORPHIC_SUPPORTED_TYPES(CastExpr),
+internal::Matcher, InnerMatcher) {
+  const Expr *const SubExpr = Node.getSubExpr();
+  return (SubExpr != nullptr &&
+  InnerMatcher.matches(*SubExpr, Finder, Builder));
+}
+
 /// \brief Matches RecordDecl object that are spelled with "struct."
 ///
 /// Example matches S, but not C or U.
Index: docs/LibASTMatchersReference.html
===
--- docs/LibASTMatchersReference.html
+++ docs/LibASTMatchersReference.html
@@ -4781,6 +4781,16 @@
 
 
 
+MatcherCastExpr>hasSubExprMatcherExpr> 
InnerMatcher
+Matches casts whose 
subexpr matches a given matcher.
+
+Example matches the first cast
+(matcher = implicitCastExpr(hasSubExpr(floatLiteral())
+  int i = 0.0;
+  int j = '0';
+
+
+
 MatcherClassTemplateSpecializationDecl>hasAnyTemplateArgumentMatcherTemplateArgument>
 InnerMatcher
 Matches 
classTemplateSpecializations, templateSpecializationType and
 functionDecl that have at least one TemplateArgument matching the given


Index: unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -1232,6 +1232,13 @@
 cxxMethodDecl(ofClass(hasName("X"))), true, "-std=gnu++98"));
 }
 
+TEST(HasSubExpr, MatchesSimpleCase) {
+  EXPECT_TRUE(matches("int i = 0.0;",
+  implicitCastExpr(hasSubExpr(floatLiteral();
+  EXPECT_TRUE(notMatches("int i = '0';",
+  implicitCastExpr(hasSubExpr(floatLiteral();
+}
+
 TEST(HasDestinationType, MatchesSimpleCase) {
   EXPECT_TRUE(matches("char* p = static_cast(0);",
   cxxStaticCastExpr(hasDestinationType(
Index: include/clang/ASTMatchers/ASTMatchers.h
===
--- include/clang/ASTMatchers/ASTMatchers.h
+++ include/clang/ASTMatchers/ASTMatchers.h
@@ -4109,6 +4109,21 @@
   return InnerMatcher.matches(Node.getType(), Finder, Builder);
 }
 
+/// \brief Matches casts whose subexpr matches a given matcher.
+///
+/// Example matches the first cast
+/// (matcher = implicitCastExpr(hasSubExpr(floatLiteral())
+///   int i = 0.0;
+///   int j = '0';
+AST_POLYMORPHIC_MATCHER_P(
+hasSubExpr,
+AST_POLYMORPHIC_SUPPORTED_TYPES(CastExpr),
+internal::Matcher, InnerMatcher) {
+  const Expr *const SubExpr = Node.getSubExpr();
+  return (SubExpr != nullptr &&
+  InnerMatcher.matches(*SubExpr, Finder, Builder));
+}
+
 /// \brief Matches RecordDecl object that are spelled with "struct."
 ///
 /// Example matches S, but not C or U.
Index: docs/LibASTMatchersReference.html
===
--- docs/LibASTMatchersReference.html
+++ docs/LibASTMatchersReference.html
@@ -4781,6 +4781,16 @@
 
 
 
+MatcherCastExpr>hasSubExpr

r328087 - [ASTMatchers] Remove extra qualifier for consistency (LibASTMatchersReference.html)

[Clement Courbet via cfe-commits]

Author: courbet
Date: Wed Mar 21 03:54:29 2018
New Revision: 328087

URL: http://llvm.org/viewvc/llvm-project?rev=328087&view=rev
Log:
[ASTMatchers] Remove extra qualifier for consistency 
(LibASTMatchersReference.html)

+ Regenerate doc.

Modified:
cfe/trunk/docs/LibASTMatchersReference.html
cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h

Modified: cfe/trunk/docs/LibASTMatchersReference.html
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/LibASTMatchersReference.html?rev=328087&r1=328086&r2=328087&view=diff
==
--- cfe/trunk/docs/LibASTMatchersReference.html (original)
+++ cfe/trunk/docs/LibASTMatchersReference.html Wed Mar 21 03:54:29 2018
@@ -5152,7 +5152,7 @@ only match the declarations for b, c, an
 
 
 
-MatcherExpr>ignoringImplicitast_matchers::MatcherExpr> 
InnerMatcher
+MatcherExpr>ignoringImplicitMatcherExpr> 
InnerMatcher
 Matches 
expressions that match InnerMatcher after any implicit AST
 nodes are stripped off.
 

Modified: cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h?rev=328087&r1=328086&r2=328087&view=diff
==
--- cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h (original)
+++ cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h Wed Mar 21 03:54:29 2018
@@ -693,7 +693,7 @@ AST_POLYMORPHIC_MATCHER_P(
 ///varDecl(hasInitializer(cxxConstructExpr()))
 /// \endcode
 /// only match the declarations for b and c.
-AST_MATCHER_P(Expr, ignoringImplicit, ast_matchers::internal::Matcher,
+AST_MATCHER_P(Expr, ignoringImplicit, internal::Matcher,
   InnerMatcher) {
   return InnerMatcher.matches(*Node.IgnoreImplicit(), Finder, Builder);
 }


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

[PATCH] D41808: Rename clang link from clang-X.Y to clang-X

[Dimitry Andric via Phabricator via cfe-commits]

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

LGTM.


https://reviews.llvm.org/D41808



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

[PATCH] D44673: Make positionToOffset return llvm::Expected

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

LGTM. See the empty line NITS, though.




Comment at: clangd/ClangdServer.cpp:199
+return End.takeError();
+
+  return formatCode(Code, File, {tooling::Range(*Begin, *End - *Begin)});

NIT: unnecessary empty line



Comment at: clangd/ClangdServer.cpp:216
+return CursorPos.takeError();
+
+  size_t PreviousLBracePos = StringRef(Code).find_last_of('{', *CursorPos);

NIT: unnecessary empty line





Comment at: clangd/SourceCode.cpp:29
+llvm::errc::invalid_argument);
+
   size_t StartOfLine = 0;

NIT: unnecessary empty line





Comment at: clangd/SourceCode.cpp:48
+llvm::errc::invalid_argument);
+
   // FIXME: officially P.character counts UTF-16 code units, not UTF-8 bytes!

NIT: unnecessary empty line





Comment at: unittests/clangd/Annotations.cpp:91
+  llvm::Expected End = positionToOffset(Code, R.end);
+
+  assert(Start);

NIT: unnecessary empty line





Comment at: unittests/clangd/Annotations.cpp:94
+  assert(End);
+
+  return {*Start, *End};

NIT: unnecessary empty line




Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44673



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

[PATCH] D44632: [clang-format] Add a few more Core Graphics identifiers to ObjC heuristic

[Jacek Olesiak via Phabricator via cfe-commits]

jolesiak added inline comments.



Comment at: lib/Format/Format.cpp:1450
 // Keep this array sorted, since we are binary searching over it.
 static constexpr llvm::StringLiteral FoundationIdentifiers[] = {
 "CGFloat",

krasimir wrote:
> jolesiak wrote:
> > benhamilton wrote:
> > > djasper wrote:
> > > > benhamilton wrote:
> > > > > jolesiak wrote:
> > > > > > djasper wrote:
> > > > > > > I have concerns about this growing lists of things. Specifically:
> > > > > > > - Keeping it sorted is a maintenance concern.
> > > > > > > - Doing binary search for basically every identifier in a header 
> > > > > > > seems an unnecessary waste.
> > > > > > > 
> > > > > > > Could we just create a hash set of these?
> > > > > > It was a hash set initially: D42135
> > > > > > Changed in: D42189
> > > > > Happy to do whatever folks recommend; I assume @krasimir's concern in 
> > > > > D42189 was the startup cost of creating the (read-only) hash set.
> > > > > 
> > > > > We can automate keeping this sorted with an `arc lint` check, they're 
> > > > > quite easy to write:
> > > > > 
> > > > > https://secure.phabricator.com/book/phabricator/article/arcanist_lint/
> > > > Krasimir clarified this to me offline. I have no concerns staying with 
> > > > binary search here and for this patch so long as someone builds in an 
> > > > assert that warns us when the strings here are not in the right order 
> > > > at some point.
> > > Good call, added `assert(std::is_sorted(...))`.
> > > 
> > > I tried `static_assert`, but `std::is_sorted()` is not `constexpr` until 
> > > c++2x.
> > Checking this type of constraints in `arc lint` sounds rather weird. I like 
> > this assert as testing private methods is painful.
> I now think a hash set here is better. Sent https://reviews.llvm.org/D44695 
> to replace the array with that.
> 
> Sorry for wasting everybody's time.
Then assertion is no longer needed.


Repository:
  rC Clang

https://reviews.llvm.org/D44632



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

[PATCH] D44555: clang-interpreter example cmake fix

[Luke Cheeseman via Phabricator via cfe-commits]

LukeCheeseman closed this revision.
LukeCheeseman added a comment.

closed by https://reviews.llvm.org/rC328092


Repository:
  rC Clang

https://reviews.llvm.org/D44555



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

[PATCH] D43967: [ASTImporter] Add test helper Fixture

[Aleksei Sidorin via Phabricator via cfe-commits]

a.sidorin added a comment.

Hi Gabor! This looks much better.




Comment at: unittests/AST/ASTImporterTest.cpp:1000
+  R"(
+  template
+  struct X {};

In C++, names started with underscore are reserved for implementation so it's 
undesirable to use them.



Comment at: unittests/AST/ASTImporterTest.cpp:1003
+
+  void declToImport(int y,X &x){}
+

I still can see a number of style violations in inline code. Some examples: 
lack of space between arguments and parameters, no space before '{' in `g(){`, 
etc. Could you please fix them?



Comment at: unittests/AST/ASTImporterTest.cpp:1169
+  MatchVerifier Verifier;
+  ASSERT_TRUE(Verifier.match(From, cxxRecordDecl(has(fieldDecl();
+  ASSERT_TRUE(Verifier.match(To, cxxRecordDecl(has(fieldDecl();

Tests with `has(fieldDecl())` duplicate tests with `hasFieldOrder()`. Is it 
intentional?



Comment at: unittests/AST/ASTImporterTest.cpp:1171
+  ASSERT_TRUE(Verifier.match(To, cxxRecordDecl(has(fieldDecl();
+  ASSERT_TRUE(Verifier.match(To, cxxRecordDecl(hasFieldOrder({"a", "b"};
+  EXPECT_TRUE(Verifier.match(To, cxxRecordDecl(hasFieldOrder({"a", "b"};

Should this be `match(From, ...)` instead of `To`?



Comment at: unittests/AST/ASTImporterTest.cpp:1177
+ASTImporterTestBase,
+
DISABLED_CXXRecordDeclFieldsShouldBeInCorrectOrderEvenWhenWeImportFirstTheLastDecl)
 {
+  Decl *From, *To;

This identifier is very long. How about renaming it to 
DISABLED_CXXRecordDeclFieldOrderShouldNotDependOnImportOrder? It is 60 char 
long instead of 82.



Comment at: unittests/AST/ASTImporterTest.cpp:1194
+  ASSERT_TRUE(Verifier.match(To, cxxRecordDecl(has(fieldDecl();
+  ASSERT_TRUE(
+  Verifier.match(To, cxxRecordDecl(hasFieldOrder({"a", "b", "c"};

Same as upper.



Comment at: unittests/AST/ASTImporterTest.cpp:1211
+  MatchVerifier Verifier;
+  // matches the implicit decl
+  auto Matcher = classTemplateDecl(has(cxxRecordDecl(has(cxxRecordDecl();

Please make it a complete sentence.



Comment at: unittests/AST/ASTImporterTest.cpp:1258
+  Lang_CXX);
+  Decl* From = LastDeclMatcher{}.match(FromTU, functionDecl());
+  const Decl* To = Import(From, Lang_CXX);

Please stick `*` to the name (below too).


Repository:
  rC Clang

https://reviews.llvm.org/D43967



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

[PATCH] D43957: Fixing issue where a space was not added before a global namespace variable when SpacesInParentheses is set

[Daniel Jasper via Phabricator via cfe-commits]

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

Please generally upload diffs with the full file as context so that Phabricator 
can properly expand the context where necessary.

This looks good, though.


Repository:
  rC Clang

https://reviews.llvm.org/D43957



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

[PATCH] D44729: [ASTMatchers] Add hasSubExpr() matcher.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

I think `hasSourceExpression()` already does what you're looking for.

  void f() {
int i = 1.0f;
  }

clang-query> match implicitCastExpr(hasSourceExpression(floatLiteral()))

Match #1:

C:\Users\aballman.GRAMMATECH\Desktop\test.c:2:11: note: "root" binds here

  int i = 1.0f;
  ^~~~

1 match.


Repository:
  rC Clang

https://reviews.llvm.org/D44729



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

[PATCH] D44673: Make positionToOffset return llvm::Expected

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clangd/SourceCode.h:41
 /// Turn an offset in Code into a [line, column] pair.
 Position offsetToPosition(llvm::StringRef Code, size_t Offset);
 

We should be consistent for all functions inside this fail. They may all fail 
for the same reasons, and it doesn't make sense for them to have a different 
interfaces.
Could you add a FIXME that we should update other functions to report errors?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44673



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

[PATCH] D44589: [Sema] Make deprecation fix-it replace all multi-parameter ObjC method slots.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

This generally looks reasonable to me, but @rsmith should weigh in before you 
commit because `MultiSourceLocation` is novel.


https://reviews.llvm.org/D44589



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

r328092 - clang-interpreter example cmake fix

[Luke Cheeseman via cfe-commits]

Author: lukecheeseman
Date: Wed Mar 21 05:05:19 2018
New Revision: 328092

URL: http://llvm.org/viewvc/llvm-project?rev=328092&view=rev
Log:
clang-interpreter example cmake fix

Add in a space when appending the export to the linker options. Without
the space the export is appended onto whatever the last link option
was, which might be a file.


Modified:
cfe/trunk/examples/clang-interpreter/CMakeLists.txt

Modified: cfe/trunk/examples/clang-interpreter/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/examples/clang-interpreter/CMakeLists.txt?rev=328092&r1=328091&r2=328092&view=diff
==
--- cfe/trunk/examples/clang-interpreter/CMakeLists.txt (original)
+++ cfe/trunk/examples/clang-interpreter/CMakeLists.txt Wed Mar 21 05:05:19 2018
@@ -34,7 +34,7 @@ if (MSVC)
   # Is this a CMake bug that even with export_executable_symbols, Windows
   # needs to explictly export the type_info vtable
   set_property(TARGET clang-interpreter
-   APPEND_STRING PROPERTY LINK_FLAGS /EXPORT:??_7type_info@@6B@)
+   APPEND_STRING PROPERTY LINK_FLAGS " /EXPORT:??_7type_info@@6B@")
 endif()
 
 function(clang_enable_exceptions TARGET)


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

[PATCH] D44732: [clangd] Set CLANGD_EDITOR environment variable in vscode extension.

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: sammccall.
Herald added subscribers: ioeric, jkorous-apple, ilya-biryukov, klimek.

This would allow us to log the editor using custom-built clangd.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44732

Files:
  clangd/clients/clangd-vscode/package.json
  clangd/clients/clangd-vscode/src/extension.ts


Index: clangd/clients/clangd-vscode/src/extension.ts
===
--- clangd/clients/clangd-vscode/src/extension.ts
+++ clangd/clients/clangd-vscode/src/extension.ts
@@ -23,6 +23,9 @@
 command: getConfig('path'),
 args: getConfig('arguments')
 };
+// Set "CLANGD_EDITOR" environment variable allowing us to log which editor
+// uses clangd.
+process.env['CLANGD_EDITOR'] = 'vscode';
 const traceFile = getConfig('trace');
 if (!!traceFile) {
 const trace = { CLANGD_TRACE: traceFile };
Index: clangd/clients/clangd-vscode/package.json
===
--- clangd/clients/clangd-vscode/package.json
+++ clangd/clients/clangd-vscode/package.json
@@ -2,7 +2,7 @@
 "name": "vscode-clangd",
 "displayName": "vscode-clangd",
 "description": "Clang Language Server",
-"version": "0.0.5",
+"version": "0.0.6",
 "publisher": "llvm-vs-code-extensions",
 "homepage": "https://clang.llvm.org/extra/clangd.html";,
 "engines": {


Index: clangd/clients/clangd-vscode/src/extension.ts
===
--- clangd/clients/clangd-vscode/src/extension.ts
+++ clangd/clients/clangd-vscode/src/extension.ts
@@ -23,6 +23,9 @@
 command: getConfig('path'),
 args: getConfig('arguments')
 };
+// Set "CLANGD_EDITOR" environment variable allowing us to log which editor
+// uses clangd.
+process.env['CLANGD_EDITOR'] = 'vscode';
 const traceFile = getConfig('trace');
 if (!!traceFile) {
 const trace = { CLANGD_TRACE: traceFile };
Index: clangd/clients/clangd-vscode/package.json
===
--- clangd/clients/clangd-vscode/package.json
+++ clangd/clients/clangd-vscode/package.json
@@ -2,7 +2,7 @@
 "name": "vscode-clangd",
 "displayName": "vscode-clangd",
 "description": "Clang Language Server",
-"version": "0.0.5",
+"version": "0.0.6",
 "publisher": "llvm-vs-code-extensions",
 "homepage": "https://clang.llvm.org/extra/clangd.html";,
 "engines": {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44673: Make positionToOffset return llvm::Expected

[Simon Marchi via Phabricator via cfe-commits]

simark updated this revision to Diff 139292.
simark marked 3 inline comments as done.
simark added a comment.

Remove spaces, add fixmes


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44673

Files:
  clangd/ClangdServer.cpp
  clangd/SourceCode.cpp
  clangd/SourceCode.h
  unittests/clangd/Annotations.cpp
  unittests/clangd/CMakeLists.txt
  unittests/clangd/SourceCodeTests.cpp

Index: unittests/clangd/SourceCodeTests.cpp
===
--- unittests/clangd/SourceCodeTests.cpp
+++ unittests/clangd/SourceCodeTests.cpp
@@ -7,14 +7,19 @@
 //
 //===--===//
 #include "SourceCode.h"
+#include "llvm/Support/Error.h"
 #include "llvm/Support/raw_os_ostream.h"
+#include "llvm/Testing/Support/Error.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
 namespace clang{
 namespace clangd {
 namespace {
 
+using llvm::Failed;
+using llvm::HasValue;
+
 MATCHER_P2(Pos, Line, Col, "") {
   return arg.line == Line && arg.character == Col;
 }
@@ -33,30 +38,57 @@
 
 TEST(SourceCodeTests, PositionToOffset) {
   // line out of bounds
-  EXPECT_EQ(0u, positionToOffset(File, position(-1, 2)));
+  EXPECT_THAT_EXPECTED(positionToOffset(File, position(-1, 2)), Failed());
   // first line
-  EXPECT_EQ(0u, positionToOffset(File, position(0, -1))); // out of range
-  EXPECT_EQ(0u, positionToOffset(File, position(0, 0)));  // first character
-  EXPECT_EQ(3u, positionToOffset(File, position(0, 3)));  // middle character
-  EXPECT_EQ(6u, positionToOffset(File, position(0, 6)));  // last character
-  EXPECT_EQ(7u, positionToOffset(File, position(0, 7)));  // the newline itself
-  EXPECT_EQ(8u, positionToOffset(File, position(0, 8)));  // out of range
+  EXPECT_THAT_EXPECTED(positionToOffset(File, position(0, -1)),
+   Failed()); // out of range
+  EXPECT_THAT_EXPECTED(positionToOffset(File, position(0, 0)),
+   HasValue(0)); // first character
+  EXPECT_THAT_EXPECTED(positionToOffset(File, position(0, 3)),
+   HasValue(3)); // middle character
+  EXPECT_THAT_EXPECTED(positionToOffset(File, position(0, 6)),
+   HasValue(6)); // last character
+  EXPECT_THAT_EXPECTED(positionToOffset(File, position(0, 7)),
+   HasValue(7)); // the newline itself
+  EXPECT_THAT_EXPECTED(positionToOffset(File, position(0, 7), false),
+   HasValue(7));
+  EXPECT_THAT_EXPECTED(positionToOffset(File, position(0, 8)),
+   HasValue(7)); // out of range
+  EXPECT_THAT_EXPECTED(positionToOffset(File, position(0, 8), false),
+   Failed()); // out of range
   // middle line
-  EXPECT_EQ(8u, positionToOffset(File, position(1, -1))); // out of range
-  EXPECT_EQ(8u, positionToOffset(File, position(1, 0)));  // first character
-  EXPECT_EQ(11u, positionToOffset(File, position(1, 3))); // middle character
-  EXPECT_EQ(14u, positionToOffset(File, position(1, 6))); // last character
-  EXPECT_EQ(15u, positionToOffset(File, position(1, 7))); // the newline itself
-  EXPECT_EQ(16u, positionToOffset(File, position(1, 8))); // out of range
+  EXPECT_THAT_EXPECTED(positionToOffset(File, position(1, -1)),
+   Failed()); // out of range
+  EXPECT_THAT_EXPECTED(positionToOffset(File, position(1, 0)),
+   HasValue(8)); // first character
+  EXPECT_THAT_EXPECTED(positionToOffset(File, position(1, 3)),
+   HasValue(11)); // middle character
+  EXPECT_THAT_EXPECTED(positionToOffset(File, position(1, 3), false),
+   HasValue(11));
+  EXPECT_THAT_EXPECTED(positionToOffset(File, position(1, 6)),
+   HasValue(14)); // last character
+  EXPECT_THAT_EXPECTED(positionToOffset(File, position(1, 7)),
+   HasValue(15)); // the newline itself
+  EXPECT_THAT_EXPECTED(positionToOffset(File, position(1, 8)),
+   HasValue(15)); // out of range
+  EXPECT_THAT_EXPECTED(positionToOffset(File, position(1, 8), false),
+   Failed()); // out of range
   // last line
-  EXPECT_EQ(16u, positionToOffset(File, position(2, -1))); // out of range
-  EXPECT_EQ(16u, positionToOffset(File, position(2, 0)));  // first character
-  EXPECT_EQ(19u, positionToOffset(File, position(2, 3)));  // middle character
-  EXPECT_EQ(23u, positionToOffset(File, position(2, 7)));  // last character
-  EXPECT_EQ(24u, positionToOffset(File, position(2, 8)));  // EOF
-  EXPECT_EQ(24u, positionToOffset(File, position(2, 9)));  // out of range
+  EXPECT_THAT_EXPECTED(positionToOffset(File, position(2, -1)),
+   Failed()); // out of range
+  EXPECT_THAT_EXPECTED(positionToOffset(File, position(2, 0)),
+   HasValue(16)); // first character
+  EXPECT_THAT_EXPECTED(positionToOffset(File, position(2, 3)),
+   HasValue(19)); // middl

[PATCH] D44673: Make positionToOffset return llvm::Expected

[Simon Marchi via Phabricator via cfe-commits]

simark marked 6 inline comments as done.
simark added inline comments.



Comment at: clangd/ClangdServer.cpp:199
+return End.takeError();
+
+  return formatCode(Code, File, {tooling::Range(*Begin, *End - *Begin)});

ilya-biryukov wrote:
> NIT: unnecessary empty line
In general I like spacing out the different logical blocks of code a little 
bit, especially after a "return", but I don't mind removing them.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44673



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

[PATCH] D40737: [clang-tidy] Resubmit hicpp-multiway-paths-covered without breaking test

[Jonas Toth via Phabricator via cfe-commits]

JonasToth updated this revision to Diff 139293.
JonasToth marked 4 inline comments as done.
JonasToth added a comment.

- adress review comments


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40737

Files:
  clang-tidy/hicpp/CMakeLists.txt
  clang-tidy/hicpp/HICPPTidyModule.cpp
  clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp
  clang-tidy/hicpp/MultiwayPathsCoveredCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/hicpp-multiway-paths-covered.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/hicpp-multiway-paths-covered-else.cpp
  test/clang-tidy/hicpp-multiway-paths-covered.cpp

Index: test/clang-tidy/hicpp-multiway-paths-covered.cpp
===
--- /dev/null
+++ test/clang-tidy/hicpp-multiway-paths-covered.cpp
@@ -0,0 +1,468 @@
+// RUN: %check_clang_tidy %s hicpp-multiway-paths-covered %t
+
+enum OS { Mac,
+  Windows,
+  Linux };
+
+struct Bitfields {
+  unsigned UInt : 3;
+  int SInt : 1;
+};
+
+int return_integer() { return 42; }
+
+void bad_switch(int i) {
+  switch (i) {
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: switch with only one case; use an if statement
+  case 0:
+break;
+  }
+  // No default in this switch
+  switch (i) {
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: potential uncovered code path; add a default label
+  case 0:
+break;
+  case 1:
+break;
+  case 2:
+break;
+  }
+
+  // degenerate, maybe even warning
+  switch (i) {
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: switch statement without labels has no effect
+  }
+
+  switch (int j = return_integer()) {
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: potential uncovered code path; add a default label
+  case 0:
+  case 1:
+  case 2:
+break;
+  }
+
+  // Degenerated, only default case.
+  switch (i) {
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: degenerated switch with default label only
+  default:
+break;
+  }
+
+  // Degenerated, only one case label and default case -> Better as if-stmt.
+  switch (i) {
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: switch could be better written as an if/else statement
+  case 0:
+break;
+  default:
+break;
+  }
+
+  unsigned long long BigNumber = 0;
+  switch (BigNumber) {
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: potential uncovered code path; add a default label
+  case 0:
+  case 1:
+break;
+  }
+
+  const int &IntRef = i;
+  switch (IntRef) {
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: potential uncovered code path; add a default label
+  case 0:
+  case 1:
+break;
+  }
+
+  char C = 'A';
+  switch (C) {
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: potential uncovered code path; add a default label
+  case 'A':
+break;
+  case 'B':
+break;
+  }
+
+  Bitfields Bf;
+  // UInt has 3 bits size.
+  switch (Bf.UInt) {
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: potential uncovered code path; add a default label
+  case 0:
+  case 1:
+break;
+  }
+  // All paths explicitly covered.
+  switch (Bf.UInt) {
+  case 0:
+  case 1:
+  case 2:
+  case 3:
+  case 4:
+  case 5:
+  case 6:
+  case 7:
+break;
+  }
+  // SInt has 1 bit size, so this is somewhat degenerated.
+  switch (Bf.SInt) {
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: switch with only one case; use an if statement
+  case 0:
+break;
+  }
+  // All paths explicitly covered.
+  switch (Bf.SInt) {
+  case 0:
+  case 1:
+break;
+  }
+
+  bool Flag = false;
+  switch (Flag) {
+// CHECK-MESSAGES:[[@LINE-1]]:3: warning: switch with only one case; use an if statement
+  case true:
+break;
+  }
+
+  switch (Flag) {
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: degenerated switch with default label only
+  default:
+break;
+  }
+
+  // This `switch` will create a frontend warning from '-Wswitch-bool' but is
+  // ok for this check.
+  switch (Flag) {
+  case true:
+break;
+  case false:
+break;
+  }
+}
+
+void unproblematic_switch(unsigned char c) {
+  //
+  switch (c) {
+  case 0:
+  case 1:
+  case 2:
+  case 3:
+  case 4:
+  case 5:
+  case 6:
+  case 7:
+  case 8:
+  case 9:
+  case 10:
+  case 11:
+  case 12:
+  case 13:
+  case 14:
+  case 15:
+  case 16:
+  case 17:
+  case 18:
+  case 19:
+  case 20:
+  case 21:
+  case 22:
+  case 23:
+  case 24:
+  case 25:
+  case 26:
+  case 27:
+  case 28:
+  case 29:
+  case 30:
+  case 31:
+  case 32:
+  case 33:
+  case 34:
+  case 35:
+  case 36:
+  case 37:
+  case 38:
+  case 39:
+  case 40:
+  case 41:
+  case 42:
+  case 43:
+  case 44:
+  case 45:
+  case 46:
+  case 47:
+  case 48:
+  case 49:
+  case 50:
+  case 51:
+  case 52:
+  case 53:
+  case 54:
+  case 55:
+  case 56:
+  case 57:
+  case 58:
+  case 59:
+  case 60:
+  case 61:
+  case 62:
+  case 63:
+  case 64:
+  case 65:
+  case 66:
+  case 67:
+  case 68:
+  case 69:
+  case 70:
+  case 71:
+  case 72:
+  case 73:
+  case 74:
+  case 75:
+  case 76:
+  case 77:
+  case 78:
+  case 79:
+  case 80:
+  case 81:
+  case 82:

[PATCH] D40737: [clang-tidy] Resubmit hicpp-multiway-paths-covered without breaking test

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.



Comment at: clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp:119
+  if (!SwitchHasDefault && SwitchCaseCount == 0) {
+diag(Switch->getLocStart(), "degenerated switch without labels");
+return;

aaron.ballman wrote:
> I think a better way to phrase this one is: "switch statement without labels 
> has no effect" and perhaps have a fix-it to replace the entire switch 
> construct with its predicate?
The fixit would only aim for the sideeffects the predicate has, right? I would 
consider such a switch as a bug or are there reasonable things to accomplish? 
Given its most likely unintended code i dont think a fixit is good.

Fixing the code removes the warning and might introduce a bug.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40737



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

[PATCH] D43967: [ASTImporter] Add test helper Fixture

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.



Comment at: unittests/AST/ASTImporterTest.cpp:1169
+  MatchVerifier Verifier;
+  ASSERT_TRUE(Verifier.match(From, cxxRecordDecl(has(fieldDecl();
+  ASSERT_TRUE(Verifier.match(To, cxxRecordDecl(has(fieldDecl();

a.sidorin wrote:
> Tests with `has(fieldDecl())` duplicate tests with `hasFieldOrder()`. Is it 
> intentional?
Ok, I removed the redundant `has(fieldDecl())` checks.



Comment at: unittests/AST/ASTImporterTest.cpp:1171
+  ASSERT_TRUE(Verifier.match(To, cxxRecordDecl(has(fieldDecl();
+  ASSERT_TRUE(Verifier.match(To, cxxRecordDecl(hasFieldOrder({"a", "b"};
+  EXPECT_TRUE(Verifier.match(To, cxxRecordDecl(hasFieldOrder({"a", "b"};

a.sidorin wrote:
> Should this be `match(From, ...)` instead of `To`?

That's right, good catch.



Comment at: unittests/AST/ASTImporterTest.cpp:1177
+ASTImporterTestBase,
+
DISABLED_CXXRecordDeclFieldsShouldBeInCorrectOrderEvenWhenWeImportFirstTheLastDecl)
 {
+  Decl *From, *To;

a.sidorin wrote:
> This identifier is very long. How about renaming it to 
> DISABLED_CXXRecordDeclFieldOrderShouldNotDependOnImportOrder? It is 60 char 
> long instead of 82.
Ok, I like the shorter name, changed it.


Repository:
  rC Clang

https://reviews.llvm.org/D43967



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

[PATCH] D43967: [ASTImporter] Add test helper Fixture

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 139295.
martong marked 7 inline comments as done.
martong added a comment.

Add minor syntax changes and rename


Repository:
  rC Clang

https://reviews.llvm.org/D43967

Files:
  unittests/AST/ASTImporterTest.cpp
  unittests/AST/DeclMatcher.h

Index: unittests/AST/DeclMatcher.h
===
--- /dev/null
+++ unittests/AST/DeclMatcher.h
@@ -0,0 +1,68 @@
+//===- unittest/AST/DeclMatcher.h - AST unit test support ---===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_UNITTESTS_AST_DECLMATCHER_H
+#define LLVM_CLANG_UNITTESTS_AST_DECLMATCHER_H
+
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+namespace clang {
+namespace ast_matchers {
+
+enum class DeclMatcherKind { First, Last };
+
+// Matcher class to retrieve the first/last matched node under a given AST.
+template 
+class DeclMatcher : public MatchFinder::MatchCallback {
+  NodeType *Node = nullptr;
+  void run(const MatchFinder::MatchResult &Result) override {
+if ((MatcherKind == DeclMatcherKind::First && Node == nullptr) ||
+MatcherKind == DeclMatcherKind::Last) {
+  Node = const_cast(Result.Nodes.getNodeAs(""));
+}
+  }
+public:
+  // Returns the first/last matched node under the tree rooted in `D`.
+  template 
+  NodeType *match(const Decl *D, const MatcherType &AMatcher) {
+MatchFinder Finder;
+Finder.addMatcher(AMatcher.bind(""), this);
+Finder.matchAST(D->getASTContext());
+assert(Node);
+return Node;
+  }
+};
+template 
+using LastDeclMatcher = DeclMatcher;
+template 
+using FirstDeclMatcher = DeclMatcher;
+
+template 
+class DeclCounter : public MatchFinder::MatchCallback {
+  unsigned Count = 0;
+  void run(const MatchFinder::MatchResult &Result) override {
+  if(Result.Nodes.getNodeAs("")) {
+++Count;
+  }
+  }
+public:
+  // Returns the number of matched nodes under the tree rooted in `D`.
+  template 
+  unsigned match(const Decl *D, const MatcherType &AMatcher) {
+MatchFinder Finder;
+Finder.addMatcher(AMatcher.bind(""), this);
+Finder.matchAST(D->getASTContext());
+return Count;
+  }
+};
+
+} // end namespace ast_matchers
+} // end namespace clang
+
+#endif
Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -17,6 +17,8 @@
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Tooling/Tooling.h"
+
+#include "DeclMatcher.h"
 #include "gtest/gtest.h"
 
 namespace clang {
@@ -29,7 +31,7 @@
   return Lang == Lang_CXX || Lang == Lang_CXX11;
 }
 
-static RunOptions getRunOptionsForLanguage(Language Lang) {
+static ArgVector getBasicRunOptionsForLanguage(Language Lang) {
   ArgVector BasicArgs;
   // Test with basic arguments.
   switch (Lang) {
@@ -49,6 +51,11 @@
   case Lang_OBJCXX:
 llvm_unreachable("Not implemented yet!");
   }
+  return BasicArgs;
+}
+
+static RunOptions getRunOptionsForLanguage(Language Lang) {
+  ArgVector BasicArgs = getBasicRunOptionsForLanguage(Lang);
 
   // For C++, test with "-fdelayed-template-parsing" enabled to handle MSVC
   // default behaviour.
@@ -61,6 +68,19 @@
   return {BasicArgs};
 }
 
+// Creates a virtual file and assigns that to the context of given AST. If the
+// file already exists then the file will not be created again as a duplicate.
+static void createVirtualFile(ASTUnit *ToAST, StringRef FileName,
+  const std::string &Code) {
+  assert(ToAST);
+  ASTContext &ToCtx = ToAST->getASTContext();
+  auto *OFS = static_cast(
+  ToCtx.getSourceManager().getFileManager().getVirtualFileSystem().get());
+  auto *MFS =
+  static_cast(OFS->overlays_begin()->get());
+  MFS->addFile(FileName, 0, llvm::MemoryBuffer::getMemBuffer(Code.c_str()));
+}
+
 template
 testing::AssertionResult
 testImport(const std::string &FromCode, const ArgVector &FromArgs,
@@ -79,11 +99,7 @@
 
   // Add input.cc to virtual file system so importer can 'find' it
   // while importing SourceLocations.
-  vfs::OverlayFileSystem *OFS = static_cast(
-ToCtx.getSourceManager().getFileManager().getVirtualFileSystem().get());
-  vfs::InMemoryFileSystem *MFS = static_cast(
-OFS->overlays_begin()->get());
-  MFS->addFile(InputFileName, 0, llvm::MemoryBuffer::getMemBuffer(FromCode));
+  createVirtualFile(ToAST.get(), InputFileName, FromCode);
 
   ASTImporter Importer(ToCtx, ToAST->getFileManager(),
FromCtx, FromAST->getFileManager(), false);
@@ -133,6 +149,167 @@
  Verifier, AMatcher));
 }
 
+const StringRef DeclToImportID = "declToImport

[PATCH] D44673: Make positionToOffset return llvm::Expected

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clangd/ClangdServer.cpp:199
+return End.takeError();
+
+  return formatCode(Code, File, {tooling::Range(*Begin, *End - *Begin)});

simark wrote:
> ilya-biryukov wrote:
> > NIT: unnecessary empty line
> In general I like spacing out the different logical blocks of code a little 
> bit, especially after a "return", but I don't mind removing them.
We don't really have a style rule for that, but most of clangd code does not 
have that many empty lines.
I personally think it's fine to have reasonable style differences in new 
functions, but I prefer to stick to the original author's style when 
refactoring code. Hence my suggestions to remove the empty lines here.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44673



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

[PATCH] D44694: [clang-tidy] Use :doc: for check links in Release Notes

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh accepted this revision.
alexfh added a comment.

LG. Thanks!


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44694



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

[clang-tools-extra] r328101 - [clang-tidy][modernize-make-unique] Checks c++14 flag before using std::make_unique

[Alexander Kornienko via cfe-commits]

Author: alexfh
Date: Wed Mar 21 07:39:24 2018
New Revision: 328101

URL: http://llvm.org/viewvc/llvm-project?rev=328101&view=rev
Log:
[clang-tidy][modernize-make-unique] Checks c++14 flag before using 
std::make_unique

Summary: For a c++11 code, the clang-tidy rule "modernize-make-unique" should 
return immediately, as std::make_unique is not supported.

Reviewers: hokein, aaron.ballman, ilya-biryukov, alexfh

Reviewed By: hokein, aaron.ballman, alexfh

Subscribers: Quuxplusone, xazax.hun, cfe-commits

Tags: #clang-tools-extra

Patch by Frederic Tingaud!

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

Added:
clang-tools-extra/trunk/test/clang-tidy/modernize-make-unique-cxx11.cpp
clang-tools-extra/trunk/test/clang-tidy/modernize-make-unique-cxx14.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/modernize/MakeSmartPtrCheck.cpp
clang-tools-extra/trunk/clang-tidy/modernize/MakeSmartPtrCheck.h
clang-tools-extra/trunk/clang-tidy/modernize/MakeUniqueCheck.cpp
clang-tools-extra/trunk/clang-tidy/modernize/MakeUniqueCheck.h
clang-tools-extra/trunk/test/clang-tidy/modernize-make-unique-macros.cpp
clang-tools-extra/trunk/test/clang-tidy/modernize-make-unique.cpp

Modified: clang-tools-extra/trunk/clang-tidy/modernize/MakeSmartPtrCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/MakeSmartPtrCheck.cpp?rev=328101&r1=328100&r2=328101&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/modernize/MakeSmartPtrCheck.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/modernize/MakeSmartPtrCheck.cpp Wed Mar 
21 07:39:24 2018
@@ -61,8 +61,13 @@ void MakeSmartPtrCheck::storeOptions(Cla
   Options.store(Opts, "IgnoreMacros", IgnoreMacros);
 }
 
+bool MakeSmartPtrCheck::isLanguageVersionSupported(
+const LangOptions &LangOpts) const {
+  return LangOpts.CPlusPlus11;
+}
+
 void MakeSmartPtrCheck::registerPPCallbacks(CompilerInstance &Compiler) {
-  if (getLangOpts().CPlusPlus11) {
+  if (isLanguageVersionSupported(getLangOpts())) {
 Inserter.reset(new utils::IncludeInserter(
 Compiler.getSourceManager(), Compiler.getLangOpts(), IncludeStyle));
 Compiler.getPreprocessor().addPPCallbacks(Inserter->CreatePPCallbacks());
@@ -70,7 +75,7 @@ void MakeSmartPtrCheck::registerPPCallba
 }
 
 void MakeSmartPtrCheck::registerMatchers(ast_matchers::MatchFinder *Finder) {
-  if (!getLangOpts().CPlusPlus11)
+  if (!isLanguageVersionSupported(getLangOpts()))
 return;
 
   // Calling make_smart_ptr from within a member function of a type with a

Modified: clang-tools-extra/trunk/clang-tidy/modernize/MakeSmartPtrCheck.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/MakeSmartPtrCheck.h?rev=328101&r1=328100&r2=328101&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/modernize/MakeSmartPtrCheck.h (original)
+++ clang-tools-extra/trunk/clang-tidy/modernize/MakeSmartPtrCheck.h Wed Mar 21 
07:39:24 2018
@@ -40,6 +40,9 @@ protected:
   /// in this class.
   virtual SmartPtrTypeMatcher getSmartPointerTypeMatcher() const = 0;
 
+  /// Returns whether the C++ version is compatible with current check.
+  virtual bool isLanguageVersionSupported(const LangOptions &LangOpts) const;
+
   static const char PointerType[];
   static const char ConstructorCall[];
   static const char ResetCall[];

Modified: clang-tools-extra/trunk/clang-tidy/modernize/MakeUniqueCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/MakeUniqueCheck.cpp?rev=328101&r1=328100&r2=328101&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/modernize/MakeUniqueCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/modernize/MakeUniqueCheck.cpp Wed Mar 21 
07:39:24 2018
@@ -17,7 +17,8 @@ namespace modernize {
 
 MakeUniqueCheck::MakeUniqueCheck(StringRef Name,
  clang::tidy::ClangTidyContext *Context)
-: MakeSmartPtrCheck(Name, Context, "std::make_unique") {}
+: MakeSmartPtrCheck(Name, Context, "std::make_unique"),
+  RequireCPlusPlus14(Options.get("MakeSmartPtrFunction", "").empty()) {}
 
 MakeUniqueCheck::SmartPtrTypeMatcher
 MakeUniqueCheck::getSmartPointerTypeMatcher() const {
@@ -36,6 +37,11 @@ MakeUniqueCheck::getSmartPointerTypeMatc
 
equalsBoundNode(PointerType;
 }
 
+bool MakeUniqueCheck::isLanguageVersionSupported(
+const LangOptions &LangOpts) const {
+  return RequireCPlusPlus14 ? LangOpts.CPlusPlus14 : LangOpts.CPlusPlus11;
+}
+
 } // namespace modernize
 } // namespace tidy
 } // namespace clang

Modified: clang-tools-extra/trunk/clang-tidy/modernize/MakeUniqueCheck.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/Mak

[PATCH] D44673: Make positionToOffset return llvm::Expected

[Simon Marchi via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL328100: Make positionToOffset return 
llvm::Expected (authored by simark, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D44673

Files:
  clang-tools-extra/trunk/clangd/ClangdServer.cpp
  clang-tools-extra/trunk/clangd/SourceCode.cpp
  clang-tools-extra/trunk/clangd/SourceCode.h
  clang-tools-extra/trunk/unittests/clangd/Annotations.cpp
  clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt
  clang-tools-extra/trunk/unittests/clangd/SourceCodeTests.cpp

Index: clang-tools-extra/trunk/unittests/clangd/Annotations.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/Annotations.cpp
+++ clang-tools-extra/trunk/unittests/clangd/Annotations.cpp
@@ -86,7 +86,11 @@
 std::pair
 Annotations::offsetRange(llvm::StringRef Name) const {
   auto R = range(Name);
-  return {positionToOffset(Code, R.start), positionToOffset(Code, R.end)};
+  llvm::Expected Start = positionToOffset(Code, R.start);
+  llvm::Expected End = positionToOffset(Code, R.end);
+  assert(Start);
+  assert(End);
+  return {*Start, *End};
 }
 
 } // namespace clangd
Index: clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt
===
--- clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt
+++ clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt
@@ -43,4 +43,5 @@
   clangTooling
   clangToolingCore
   LLVMSupport
+  LLVMTestingSupport
   )
Index: clang-tools-extra/trunk/unittests/clangd/SourceCodeTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/SourceCodeTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/SourceCodeTests.cpp
@@ -7,14 +7,19 @@
 //
 //===--===//
 #include "SourceCode.h"
+#include "llvm/Support/Error.h"
 #include "llvm/Support/raw_os_ostream.h"
+#include "llvm/Testing/Support/Error.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
 namespace clang{
 namespace clangd {
 namespace {
 
+using llvm::Failed;
+using llvm::HasValue;
+
 MATCHER_P2(Pos, Line, Col, "") {
   return arg.line == Line && arg.character == Col;
 }
@@ -33,30 +38,57 @@
 
 TEST(SourceCodeTests, PositionToOffset) {
   // line out of bounds
-  EXPECT_EQ(0u, positionToOffset(File, position(-1, 2)));
+  EXPECT_THAT_EXPECTED(positionToOffset(File, position(-1, 2)), Failed());
   // first line
-  EXPECT_EQ(0u, positionToOffset(File, position(0, -1))); // out of range
-  EXPECT_EQ(0u, positionToOffset(File, position(0, 0)));  // first character
-  EXPECT_EQ(3u, positionToOffset(File, position(0, 3)));  // middle character
-  EXPECT_EQ(6u, positionToOffset(File, position(0, 6)));  // last character
-  EXPECT_EQ(7u, positionToOffset(File, position(0, 7)));  // the newline itself
-  EXPECT_EQ(8u, positionToOffset(File, position(0, 8)));  // out of range
+  EXPECT_THAT_EXPECTED(positionToOffset(File, position(0, -1)),
+   Failed()); // out of range
+  EXPECT_THAT_EXPECTED(positionToOffset(File, position(0, 0)),
+   HasValue(0)); // first character
+  EXPECT_THAT_EXPECTED(positionToOffset(File, position(0, 3)),
+   HasValue(3)); // middle character
+  EXPECT_THAT_EXPECTED(positionToOffset(File, position(0, 6)),
+   HasValue(6)); // last character
+  EXPECT_THAT_EXPECTED(positionToOffset(File, position(0, 7)),
+   HasValue(7)); // the newline itself
+  EXPECT_THAT_EXPECTED(positionToOffset(File, position(0, 7), false),
+   HasValue(7));
+  EXPECT_THAT_EXPECTED(positionToOffset(File, position(0, 8)),
+   HasValue(7)); // out of range
+  EXPECT_THAT_EXPECTED(positionToOffset(File, position(0, 8), false),
+   Failed()); // out of range
   // middle line
-  EXPECT_EQ(8u, positionToOffset(File, position(1, -1))); // out of range
-  EXPECT_EQ(8u, positionToOffset(File, position(1, 0)));  // first character
-  EXPECT_EQ(11u, positionToOffset(File, position(1, 3))); // middle character
-  EXPECT_EQ(14u, positionToOffset(File, position(1, 6))); // last character
-  EXPECT_EQ(15u, positionToOffset(File, position(1, 7))); // the newline itself
-  EXPECT_EQ(16u, positionToOffset(File, position(1, 8))); // out of range
+  EXPECT_THAT_EXPECTED(positionToOffset(File, position(1, -1)),
+   Failed()); // out of range
+  EXPECT_THAT_EXPECTED(positionToOffset(File, position(1, 0)),
+   HasValue(8)); // first character
+  EXPECT_THAT_EXPECTED(positionToOffset(File, position(1, 3)),
+   HasValue(11)); // middle character
+  EXPECT_THAT_EXPECTED(positionToOffset(File, position(1, 3), false),
+   HasValue(11));
+  EXPECT_THAT_EXPECTED

[PATCH] D43766: [clang-tidy][modernize-make-unique] Checks c++14 flag before using std::make_unique

[Alexander Kornienko via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL328101: [clang-tidy][modernize-make-unique] Checks c++14 
flag before using std… (authored by alexfh, committed by ).
Herald added subscribers: llvm-commits, klimek.

Changed prior to commit:
  https://reviews.llvm.org/D43766?vs=138398&id=139298#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D43766

Files:
  clang-tools-extra/trunk/clang-tidy/modernize/MakeSmartPtrCheck.cpp
  clang-tools-extra/trunk/clang-tidy/modernize/MakeSmartPtrCheck.h
  clang-tools-extra/trunk/clang-tidy/modernize/MakeUniqueCheck.cpp
  clang-tools-extra/trunk/clang-tidy/modernize/MakeUniqueCheck.h
  clang-tools-extra/trunk/test/clang-tidy/modernize-make-unique-cxx11.cpp
  clang-tools-extra/trunk/test/clang-tidy/modernize-make-unique-cxx14.cpp
  clang-tools-extra/trunk/test/clang-tidy/modernize-make-unique-macros.cpp
  clang-tools-extra/trunk/test/clang-tidy/modernize-make-unique.cpp

Index: clang-tools-extra/trunk/clang-tidy/modernize/MakeSmartPtrCheck.h
===
--- clang-tools-extra/trunk/clang-tidy/modernize/MakeSmartPtrCheck.h
+++ clang-tools-extra/trunk/clang-tidy/modernize/MakeSmartPtrCheck.h
@@ -40,6 +40,9 @@
   /// in this class.
   virtual SmartPtrTypeMatcher getSmartPointerTypeMatcher() const = 0;
 
+  /// Returns whether the C++ version is compatible with current check.
+  virtual bool isLanguageVersionSupported(const LangOptions &LangOpts) const;
+
   static const char PointerType[];
   static const char ConstructorCall[];
   static const char ResetCall[];
Index: clang-tools-extra/trunk/clang-tidy/modernize/MakeUniqueCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/modernize/MakeUniqueCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/modernize/MakeUniqueCheck.cpp
@@ -17,7 +17,8 @@
 
 MakeUniqueCheck::MakeUniqueCheck(StringRef Name,
  clang::tidy::ClangTidyContext *Context)
-: MakeSmartPtrCheck(Name, Context, "std::make_unique") {}
+: MakeSmartPtrCheck(Name, Context, "std::make_unique"),
+  RequireCPlusPlus14(Options.get("MakeSmartPtrFunction", "").empty()) {}
 
 MakeUniqueCheck::SmartPtrTypeMatcher
 MakeUniqueCheck::getSmartPointerTypeMatcher() const {
@@ -36,6 +37,11 @@
 equalsBoundNode(PointerType;
 }
 
+bool MakeUniqueCheck::isLanguageVersionSupported(
+const LangOptions &LangOpts) const {
+  return RequireCPlusPlus14 ? LangOpts.CPlusPlus14 : LangOpts.CPlusPlus11;
+}
+
 } // namespace modernize
 } // namespace tidy
 } // namespace clang
Index: clang-tools-extra/trunk/clang-tidy/modernize/MakeSmartPtrCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/modernize/MakeSmartPtrCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/modernize/MakeSmartPtrCheck.cpp
@@ -61,16 +61,21 @@
   Options.store(Opts, "IgnoreMacros", IgnoreMacros);
 }
 
+bool MakeSmartPtrCheck::isLanguageVersionSupported(
+const LangOptions &LangOpts) const {
+  return LangOpts.CPlusPlus11;
+}
+
 void MakeSmartPtrCheck::registerPPCallbacks(CompilerInstance &Compiler) {
-  if (getLangOpts().CPlusPlus11) {
+  if (isLanguageVersionSupported(getLangOpts())) {
 Inserter.reset(new utils::IncludeInserter(
 Compiler.getSourceManager(), Compiler.getLangOpts(), IncludeStyle));
 Compiler.getPreprocessor().addPPCallbacks(Inserter->CreatePPCallbacks());
   }
 }
 
 void MakeSmartPtrCheck::registerMatchers(ast_matchers::MatchFinder *Finder) {
-  if (!getLangOpts().CPlusPlus11)
+  if (!isLanguageVersionSupported(getLangOpts()))
 return;
 
   // Calling make_smart_ptr from within a member function of a type with a
Index: clang-tools-extra/trunk/clang-tidy/modernize/MakeUniqueCheck.h
===
--- clang-tools-extra/trunk/clang-tidy/modernize/MakeUniqueCheck.h
+++ clang-tools-extra/trunk/clang-tidy/modernize/MakeUniqueCheck.h
@@ -31,6 +31,11 @@
 
 protected:
   SmartPtrTypeMatcher getSmartPointerTypeMatcher() const override;
+
+  bool isLanguageVersionSupported(const LangOptions &LangOpts) const override;
+
+private:
+  const bool RequireCPlusPlus14;
 };
 
 } // namespace modernize
Index: clang-tools-extra/trunk/test/clang-tidy/modernize-make-unique.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/modernize-make-unique.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/modernize-make-unique.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s modernize-make-unique %t -- -- -std=c++11 \
+// RUN: %check_clang_tidy %s modernize-make-unique %t -- -- -std=c++14 \
 // RUN:   -I%S/Inputs/modernize-smart-ptr
 
 #include "unique_ptr.h"
Index: clang-tools-extra/trunk/test/clang-tidy/modernize-make-unique-cxx11.cpp
=

[PATCH] D44673: Make positionToOffset return llvm::Expected

[Simon Marchi via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE328100: Make positionToOffset return 
llvm::Expected (authored by simark, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D44673?vs=139292&id=139297#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D44673

Files:
  clangd/ClangdServer.cpp
  clangd/SourceCode.cpp
  clangd/SourceCode.h
  unittests/clangd/Annotations.cpp
  unittests/clangd/CMakeLists.txt
  unittests/clangd/SourceCodeTests.cpp

Index: unittests/clangd/SourceCodeTests.cpp
===
--- unittests/clangd/SourceCodeTests.cpp
+++ unittests/clangd/SourceCodeTests.cpp
@@ -7,14 +7,19 @@
 //
 //===--===//
 #include "SourceCode.h"
+#include "llvm/Support/Error.h"
 #include "llvm/Support/raw_os_ostream.h"
+#include "llvm/Testing/Support/Error.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
 namespace clang{
 namespace clangd {
 namespace {
 
+using llvm::Failed;
+using llvm::HasValue;
+
 MATCHER_P2(Pos, Line, Col, "") {
   return arg.line == Line && arg.character == Col;
 }
@@ -33,30 +38,57 @@
 
 TEST(SourceCodeTests, PositionToOffset) {
   // line out of bounds
-  EXPECT_EQ(0u, positionToOffset(File, position(-1, 2)));
+  EXPECT_THAT_EXPECTED(positionToOffset(File, position(-1, 2)), Failed());
   // first line
-  EXPECT_EQ(0u, positionToOffset(File, position(0, -1))); // out of range
-  EXPECT_EQ(0u, positionToOffset(File, position(0, 0)));  // first character
-  EXPECT_EQ(3u, positionToOffset(File, position(0, 3)));  // middle character
-  EXPECT_EQ(6u, positionToOffset(File, position(0, 6)));  // last character
-  EXPECT_EQ(7u, positionToOffset(File, position(0, 7)));  // the newline itself
-  EXPECT_EQ(8u, positionToOffset(File, position(0, 8)));  // out of range
+  EXPECT_THAT_EXPECTED(positionToOffset(File, position(0, -1)),
+   Failed()); // out of range
+  EXPECT_THAT_EXPECTED(positionToOffset(File, position(0, 0)),
+   HasValue(0)); // first character
+  EXPECT_THAT_EXPECTED(positionToOffset(File, position(0, 3)),
+   HasValue(3)); // middle character
+  EXPECT_THAT_EXPECTED(positionToOffset(File, position(0, 6)),
+   HasValue(6)); // last character
+  EXPECT_THAT_EXPECTED(positionToOffset(File, position(0, 7)),
+   HasValue(7)); // the newline itself
+  EXPECT_THAT_EXPECTED(positionToOffset(File, position(0, 7), false),
+   HasValue(7));
+  EXPECT_THAT_EXPECTED(positionToOffset(File, position(0, 8)),
+   HasValue(7)); // out of range
+  EXPECT_THAT_EXPECTED(positionToOffset(File, position(0, 8), false),
+   Failed()); // out of range
   // middle line
-  EXPECT_EQ(8u, positionToOffset(File, position(1, -1))); // out of range
-  EXPECT_EQ(8u, positionToOffset(File, position(1, 0)));  // first character
-  EXPECT_EQ(11u, positionToOffset(File, position(1, 3))); // middle character
-  EXPECT_EQ(14u, positionToOffset(File, position(1, 6))); // last character
-  EXPECT_EQ(15u, positionToOffset(File, position(1, 7))); // the newline itself
-  EXPECT_EQ(16u, positionToOffset(File, position(1, 8))); // out of range
+  EXPECT_THAT_EXPECTED(positionToOffset(File, position(1, -1)),
+   Failed()); // out of range
+  EXPECT_THAT_EXPECTED(positionToOffset(File, position(1, 0)),
+   HasValue(8)); // first character
+  EXPECT_THAT_EXPECTED(positionToOffset(File, position(1, 3)),
+   HasValue(11)); // middle character
+  EXPECT_THAT_EXPECTED(positionToOffset(File, position(1, 3), false),
+   HasValue(11));
+  EXPECT_THAT_EXPECTED(positionToOffset(File, position(1, 6)),
+   HasValue(14)); // last character
+  EXPECT_THAT_EXPECTED(positionToOffset(File, position(1, 7)),
+   HasValue(15)); // the newline itself
+  EXPECT_THAT_EXPECTED(positionToOffset(File, position(1, 8)),
+   HasValue(15)); // out of range
+  EXPECT_THAT_EXPECTED(positionToOffset(File, position(1, 8), false),
+   Failed()); // out of range
   // last line
-  EXPECT_EQ(16u, positionToOffset(File, position(2, -1))); // out of range
-  EXPECT_EQ(16u, positionToOffset(File, position(2, 0)));  // first character
-  EXPECT_EQ(19u, positionToOffset(File, position(2, 3)));  // middle character
-  EXPECT_EQ(23u, positionToOffset(File, position(2, 7)));  // last character
-  EXPECT_EQ(24u, positionToOffset(File, position(2, 8)));  // EOF
-  EXPECT_EQ(24u, positionToOffset(File, position(2, 9)));  // out of range
+  EXPECT_THAT_EXPECTED(positionToOffset(File, position(2, -1)),
+   Failed()); // out of range
+  EXPECT_THAT_EXPECTED(positionToOffset(File, position(2, 0)),
+   HasValue(16)); // fir

Re: [clang-tools-extra] r327833 - [clang-tidy] New check bugprone-unused-return-value

[Alexander Kornienko via cfe-commits]

On Mon, Mar 19, 2018 at 10:43 PM  wrote:

> This should be fixed by r327911.
>

Thanks!


>
>
> Douglas Yung
>
>
>
> *From:* cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] *On
> Behalf Of *Galina Kistanova via cfe-commits
> *Sent:* Monday, March 19, 2018 13:13
> *To:* Alexander Kornienko
> *Cc:* cfe-commits
> *Subject:* Re: [clang-tools-extra] r327833 - [clang-tidy] New check
> bugprone-unused-return-value
>
>
>
> Hello Alexander,
>
> This commit broke at least two of our builders:
>
>
> http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/26909
>
> http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast
>
> . . .
> Failing Tests (1):
> Clang Tools :: clang-tidy/bugprone-unused-return-value.cpp
>
> Please have a look?
>
> It is not good idea to keep the bot red for too long. This hides new
> problem which later hard to track down.
>
> Thanks
>
> Galina
>
>
>
> On Mon, Mar 19, 2018 at 6:02 AM, Alexander Kornienko via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
> Author: alexfh
> Date: Mon Mar 19 06:02:32 2018
> New Revision: 327833
>
> URL: http://llvm.org/viewvc/llvm-project?rev=327833&view=rev
> Log:
> [clang-tidy] New check bugprone-unused-return-value
>
> Summary:
> Detects function calls where the return value is unused.
>
> Checked functions can be configured.
>
> Reviewers: alexfh, aaron.ballman, ilya-biryukov, hokein
>
> Reviewed By: alexfh, aaron.ballman
>
> Subscribers: hintonda, JonasToth, Eugene.Zelenko, mgorny, xazax.hun,
> cfe-commits
>
> Tags: #clang-tools-extra
>
> Patch by Kalle Huttunen!
>
> Differential Revision: https://reviews.llvm.org/D41655
>
> Added:
> clang-tools-extra/trunk/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
> clang-tools-extra/trunk/clang-tidy/bugprone/UnusedReturnValueCheck.h
>
> clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-unused-return-value.rst
>
> clang-tools-extra/trunk/test/clang-tidy/bugprone-unused-return-value-custom.cpp
>
> clang-tools-extra/trunk/test/clang-tidy/bugprone-unused-return-value.cpp
> Modified:
> clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp
> clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt
> clang-tools-extra/trunk/docs/ReleaseNotes.rst
> clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
>
> Modified:
> clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp?rev=327833&r1=327832&r2=327833&view=diff
>
> ==
> --- clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp
> (original)
> +++ clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp Mon
> Mar 19 06:02:32 2018
> @@ -43,6 +43,7 @@
>  #include "UndefinedMemoryManipulationCheck.h"
>  #include "UndelegatedConstructorCheck.h"
>  #include "UnusedRaiiCheck.h"
> +#include "UnusedReturnValueCheck.h"
>  #include "UseAfterMoveCheck.h"
>  #include "VirtualNearMissCheck.h"
>
> @@ -119,6 +120,8 @@ public:
>  "bugprone-undelegated-constructor");
>  CheckFactories.registerCheck(
>  "bugprone-unused-raii");
> +CheckFactories.registerCheck(
> +"bugprone-unused-return-value");
>  CheckFactories.registerCheck(
>  "bugprone-use-after-move");
>  CheckFactories.registerCheck(
>
> Modified: clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt?rev=327833&r1=327832&r2=327833&view=diff
>
> ==
> --- clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt (original)
> +++ clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt Mon Mar 19
> 06:02:32 2018
> @@ -35,6 +35,7 @@ add_clang_library(clangTidyBugproneModul
>UndefinedMemoryManipulationCheck.cpp
>UndelegatedConstructorCheck.cpp
>UnusedRaiiCheck.cpp
> +  UnusedReturnValueCheck.cpp
>UseAfterMoveCheck.cpp
>VirtualNearMissCheck.cpp
>
>
> Added:
> clang-tools-extra/trunk/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/bugprone/UnusedReturnValueCheck.cpp?rev=327833&view=auto
>
> ==
> --- clang-tools-extra/trunk/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
> (added)
> +++ clang-tools-extra/trunk/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
> Mon Mar 19 06:02:32 2018
> @@ -0,0 +1,82 @@
> +//===--- UnusedReturnValueCheck.cpp -
> clang-tidy---===//
> +//
> +// The LLVM Compiler Infrastructure
> +//
> +// This file is distributed under the University of Illinois Open Source
> +// License. See LICENSE.TXT for details.
> +//
>
> +//===---

[PATCH] D44231: [clang-tidy] Check for sizeof that call functions

[Alexander Kornienko via Phabricator via cfe-commits]

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

LG modulo other reviewers' open comments.


https://reviews.llvm.org/D44231



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

[clang-tools-extra] r328100 - Make positionToOffset return llvm::Expected

[Simon Marchi via cfe-commits]

Author: simark
Date: Wed Mar 21 07:36:46 2018
New Revision: 328100

URL: http://llvm.org/viewvc/llvm-project?rev=328100&view=rev
Log:
Make positionToOffset return llvm::Expected

Summary:

To implement incremental document syncing, we want to verify that the
ranges provided by the front-end are valid.  Currently, positionToOffset
deals with invalid Positions by returning 0 or Code.size(), which are
two valid offsets.  Instead, return an llvm:Expected with an
error if the position is invalid.

According to the LSP, if the character value exceeds the number of
characters of the given line, it should default back to the end of the
line.  It makes sense in some contexts to have this behavior, and does
not in other contexts.  The AllowColumnsBeyondLineLength parameter
allows to decide what to do in that case, default back to the end of the
line, or return an error.

Reviewers: ilya-biryukov

Subscribers: klimek, ilya-biryukov, jkorous-apple, ioeric, cfe-commits

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

Modified:
clang-tools-extra/trunk/clangd/ClangdServer.cpp
clang-tools-extra/trunk/clangd/SourceCode.cpp
clang-tools-extra/trunk/clangd/SourceCode.h
clang-tools-extra/trunk/unittests/clangd/Annotations.cpp
clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt
clang-tools-extra/trunk/unittests/clangd/SourceCodeTests.cpp

Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=328100&r1=328099&r2=328100&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Wed Mar 21 07:36:46 2018
@@ -190,9 +190,13 @@ void ClangdServer::signatureHelp(PathRef
 
 llvm::Expected
 ClangdServer::formatRange(StringRef Code, PathRef File, Range Rng) {
-  size_t Begin = positionToOffset(Code, Rng.start);
-  size_t Len = positionToOffset(Code, Rng.end) - Begin;
-  return formatCode(Code, File, {tooling::Range(Begin, Len)});
+  llvm::Expected Begin = positionToOffset(Code, Rng.start);
+  if (!Begin)
+return Begin.takeError();
+  llvm::Expected End = positionToOffset(Code, Rng.end);
+  if (!End)
+return End.takeError();
+  return formatCode(Code, File, {tooling::Range(*Begin, *End - *Begin)});
 }
 
 llvm::Expected ClangdServer::formatFile(StringRef Code,
@@ -205,11 +209,13 @@ llvm::Expected
 ClangdServer::formatOnType(StringRef Code, PathRef File, Position Pos) {
   // Look for the previous opening brace from the character position and
   // format starting from there.
-  size_t CursorPos = positionToOffset(Code, Pos);
-  size_t PreviousLBracePos = StringRef(Code).find_last_of('{', CursorPos);
+  llvm::Expected CursorPos = positionToOffset(Code, Pos);
+  if (!CursorPos)
+return CursorPos.takeError();
+  size_t PreviousLBracePos = StringRef(Code).find_last_of('{', *CursorPos);
   if (PreviousLBracePos == StringRef::npos)
-PreviousLBracePos = CursorPos;
-  size_t Len = CursorPos - PreviousLBracePos;
+PreviousLBracePos = *CursorPos;
+  size_t Len = *CursorPos - PreviousLBracePos;
 
   return formatCode(Code, File, {tooling::Range(PreviousLBracePos, Len)});
 }

Modified: clang-tools-extra/trunk/clangd/SourceCode.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/SourceCode.cpp?rev=328100&r1=328099&r2=328100&view=diff
==
--- clang-tools-extra/trunk/clangd/SourceCode.cpp (original)
+++ clang-tools-extra/trunk/clangd/SourceCode.cpp Wed Mar 21 07:36:46 2018
@@ -9,23 +9,43 @@
 #include "SourceCode.h"
 
 #include "clang/Basic/SourceManager.h"
+#include "llvm/Support/Errc.h"
+#include "llvm/Support/Error.h"
 
 namespace clang {
 namespace clangd {
 using namespace llvm;
 
-size_t positionToOffset(StringRef Code, Position P) {
+llvm::Expected positionToOffset(StringRef Code, Position P,
+bool AllowColumnsBeyondLineLength) {
   if (P.line < 0)
-return 0;
+return llvm::make_error(
+llvm::formatv("Line value can't be negative ({0})", P.line),
+llvm::errc::invalid_argument);
+  if (P.character < 0)
+return llvm::make_error(
+llvm::formatv("Character value can't be negative ({0})", P.character),
+llvm::errc::invalid_argument);
   size_t StartOfLine = 0;
   for (int I = 0; I != P.line; ++I) {
 size_t NextNL = Code.find('\n', StartOfLine);
 if (NextNL == StringRef::npos)
-  return Code.size();
+  return llvm::make_error(
+  llvm::formatv("Line value is out of range ({0})", P.line),
+  llvm::errc::invalid_argument);
 StartOfLine = NextNL + 1;
   }
+
+  size_t NextNL = Code.find('\n', StartOfLine);
+  if (NextNL == StringRef::npos)
+NextNL = Code.size();
+
+  if (StartOfLine + P.character > NextNL && !AllowColumnsBeyondLineLength)
+retur

[PATCH] D44692: [clang-format] Don't insert space between r_paren and 'new' in ObjC decl

[Daniel Jasper via Phabricator via cfe-commits]

djasper accepted this revision.
djasper added a comment.

Looks good.


Repository:
  rC Clang

https://reviews.llvm.org/D44692



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

[PATCH] D44632: [clang-format] Add a few more Core Graphics identifiers to ObjC heuristic

[Daniel Jasper via Phabricator via cfe-commits]

djasper accepted this revision.
djasper added a comment.

Looks good.


Repository:
  rC Clang

https://reviews.llvm.org/D44632



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

[PATCH] D43026: [OpenMP] CodeGen for the "declare target" directive - variables, functions, ctors/dtors

[George Rokos via Phabricator via cfe-commits]

grokos abandoned this revision.
grokos added a comment.

@ABataev came up with a much simpler solution to the implementation of `declare 
target`: https://reviews.llvm.org/rL327636

I am abandoning this obsolete revision.


Repository:
  rC Clang

https://reviews.llvm.org/D43026



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

[PATCH] D38798: [OpenMP] Support for implicit "declare target" functions - Sema patch

[George Rokos via Phabricator via cfe-commits]

grokos abandoned this revision.
grokos added a comment.

@ABataev came up with a much simpler solution to the implementation of `declare 
target`: https://reviews.llvm.org/rL327636

I am abandoning this obsolete revision.


Repository:
  rL LLVM

https://reviews.llvm.org/D38798



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

[PATCH] D40737: [clang-tidy] Resubmit hicpp-multiway-paths-covered without breaking test

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM!




Comment at: clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp:119
+  if (!SwitchHasDefault && SwitchCaseCount == 0) {
+diag(Switch->getLocStart(), "degenerated switch without labels");
+return;

JonasToth wrote:
> aaron.ballman wrote:
> > I think a better way to phrase this one is: "switch statement without 
> > labels has no effect" and perhaps have a fix-it to replace the entire 
> > switch construct with its predicate?
> The fixit would only aim for the sideeffects the predicate has, right? I 
> would consider such a switch as a bug or are there reasonable things to 
> accomplish? Given its most likely unintended code i dont think a fixit is 
> good.
> 
> Fixing the code removes the warning and might introduce a bug.
This code pattern comes up with machine-generated code with some frequency, so 
I was thinking it would be nice to automatically fix that code. However, I 
think you're right that "fixing" the code may introduce bugs because you don't 
want to keep around non-side effecting operations and that's complicated. e.g.,
```
switch (i) { // Don't replace this with i;
}

switch (some_function_call()) { // Maybe replace this with some_function_call()?
}

switch (i = 12) { // Should definitely be replaced with i = 12;
}
```
Perhaps only diagnosing is the best option.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40737



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

[PATCH] D40737: [clang-tidy] Resubmit hicpp-multiway-paths-covered without breaking test

[Jonas Toth via Phabricator via cfe-commits]

JonasToth updated this revision to Diff 139304.
JonasToth marked 3 inline comments as done.
JonasToth added a comment.

- add fixme for degenerated switch


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40737

Files:
  clang-tidy/hicpp/CMakeLists.txt
  clang-tidy/hicpp/HICPPTidyModule.cpp
  clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp
  clang-tidy/hicpp/MultiwayPathsCoveredCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/hicpp-multiway-paths-covered.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/hicpp-multiway-paths-covered-else.cpp
  test/clang-tidy/hicpp-multiway-paths-covered.cpp

Index: test/clang-tidy/hicpp-multiway-paths-covered.cpp
===
--- /dev/null
+++ test/clang-tidy/hicpp-multiway-paths-covered.cpp
@@ -0,0 +1,468 @@
+// RUN: %check_clang_tidy %s hicpp-multiway-paths-covered %t
+
+enum OS { Mac,
+  Windows,
+  Linux };
+
+struct Bitfields {
+  unsigned UInt : 3;
+  int SInt : 1;
+};
+
+int return_integer() { return 42; }
+
+void bad_switch(int i) {
+  switch (i) {
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: switch with only one case; use an if statement
+  case 0:
+break;
+  }
+  // No default in this switch
+  switch (i) {
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: potential uncovered code path; add a default label
+  case 0:
+break;
+  case 1:
+break;
+  case 2:
+break;
+  }
+
+  // degenerate, maybe even warning
+  switch (i) {
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: switch statement without labels has no effect
+  }
+
+  switch (int j = return_integer()) {
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: potential uncovered code path; add a default label
+  case 0:
+  case 1:
+  case 2:
+break;
+  }
+
+  // Degenerated, only default case.
+  switch (i) {
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: degenerated switch with default label only
+  default:
+break;
+  }
+
+  // Degenerated, only one case label and default case -> Better as if-stmt.
+  switch (i) {
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: switch could be better written as an if/else statement
+  case 0:
+break;
+  default:
+break;
+  }
+
+  unsigned long long BigNumber = 0;
+  switch (BigNumber) {
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: potential uncovered code path; add a default label
+  case 0:
+  case 1:
+break;
+  }
+
+  const int &IntRef = i;
+  switch (IntRef) {
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: potential uncovered code path; add a default label
+  case 0:
+  case 1:
+break;
+  }
+
+  char C = 'A';
+  switch (C) {
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: potential uncovered code path; add a default label
+  case 'A':
+break;
+  case 'B':
+break;
+  }
+
+  Bitfields Bf;
+  // UInt has 3 bits size.
+  switch (Bf.UInt) {
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: potential uncovered code path; add a default label
+  case 0:
+  case 1:
+break;
+  }
+  // All paths explicitly covered.
+  switch (Bf.UInt) {
+  case 0:
+  case 1:
+  case 2:
+  case 3:
+  case 4:
+  case 5:
+  case 6:
+  case 7:
+break;
+  }
+  // SInt has 1 bit size, so this is somewhat degenerated.
+  switch (Bf.SInt) {
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: switch with only one case; use an if statement
+  case 0:
+break;
+  }
+  // All paths explicitly covered.
+  switch (Bf.SInt) {
+  case 0:
+  case 1:
+break;
+  }
+
+  bool Flag = false;
+  switch (Flag) {
+// CHECK-MESSAGES:[[@LINE-1]]:3: warning: switch with only one case; use an if statement
+  case true:
+break;
+  }
+
+  switch (Flag) {
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: degenerated switch with default label only
+  default:
+break;
+  }
+
+  // This `switch` will create a frontend warning from '-Wswitch-bool' but is
+  // ok for this check.
+  switch (Flag) {
+  case true:
+break;
+  case false:
+break;
+  }
+}
+
+void unproblematic_switch(unsigned char c) {
+  //
+  switch (c) {
+  case 0:
+  case 1:
+  case 2:
+  case 3:
+  case 4:
+  case 5:
+  case 6:
+  case 7:
+  case 8:
+  case 9:
+  case 10:
+  case 11:
+  case 12:
+  case 13:
+  case 14:
+  case 15:
+  case 16:
+  case 17:
+  case 18:
+  case 19:
+  case 20:
+  case 21:
+  case 22:
+  case 23:
+  case 24:
+  case 25:
+  case 26:
+  case 27:
+  case 28:
+  case 29:
+  case 30:
+  case 31:
+  case 32:
+  case 33:
+  case 34:
+  case 35:
+  case 36:
+  case 37:
+  case 38:
+  case 39:
+  case 40:
+  case 41:
+  case 42:
+  case 43:
+  case 44:
+  case 45:
+  case 46:
+  case 47:
+  case 48:
+  case 49:
+  case 50:
+  case 51:
+  case 52:
+  case 53:
+  case 54:
+  case 55:
+  case 56:
+  case 57:
+  case 58:
+  case 59:
+  case 60:
+  case 61:
+  case 62:
+  case 63:
+  case 64:
+  case 65:
+  case 66:
+  case 67:
+  case 68:
+  case 69:
+  case 70:
+  case 71:
+  case 72:
+  case 73:
+  case 74:
+  case 75:
+  case 76:
+  case 77:
+  case 78:
+  case 79:
+  case 80:
+  case 81:
+

[PATCH] D40737: [clang-tidy] Resubmit hicpp-multiway-paths-covered without breaking test

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.



Comment at: clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp:119
+  if (!SwitchHasDefault && SwitchCaseCount == 0) {
+diag(Switch->getLocStart(), "degenerated switch without labels");
+return;

aaron.ballman wrote:
> JonasToth wrote:
> > aaron.ballman wrote:
> > > I think a better way to phrase this one is: "switch statement without 
> > > labels has no effect" and perhaps have a fix-it to replace the entire 
> > > switch construct with its predicate?
> > The fixit would only aim for the sideeffects the predicate has, right? I 
> > would consider such a switch as a bug or are there reasonable things to 
> > accomplish? Given its most likely unintended code i dont think a fixit is 
> > good.
> > 
> > Fixing the code removes the warning and might introduce a bug.
> This code pattern comes up with machine-generated code with some frequency, 
> so I was thinking it would be nice to automatically fix that code. However, I 
> think you're right that "fixing" the code may introduce bugs because you 
> don't want to keep around non-side effecting operations and that's 
> complicated. e.g.,
> ```
> switch (i) { // Don't replace this with i;
> }
> 
> switch (some_function_call()) { // Maybe replace this with 
> some_function_call()?
> }
> 
> switch (i = 12) { // Should definitely be replaced with i = 12;
> }
> ```
> Perhaps only diagnosing is the best option.
I will add another FIXME. The if-is-better pattern might be reasonable 
transformable and doing this allows addressing the issue again later.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40737



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

[PATCH] D44426: Fix llvm + clang build with Intel compiler

[Melanie Blower via Phabricator via cfe-commits]

mibintc added inline comments.



Comment at: include/llvm-c/Target.h:25
 
-#if defined(_MSC_VER) && !defined(inline)
+#if defined(_MSC_VER) && !defined(inline) && !defined(__INTEL_COMPILER)
 #define inline __inline

yvvan wrote:
> mibintc wrote:
> > I really think all the Intel-compiler bug workarounds should be version 
> > specific. For example, in the boost.org customizations we have checks like 
> > this:
> > #if defined(__INTEL_COMPILER) && (__INTEL_COMPILER >= 1500)
> > 
> > I believe you are using the most recent Intel compiler, 18.0, which has 
> > version 1800.  Let's assume the bug will either be fixed in our 18.0 
> > compiler or in our next compiler which would be numbered 1900, so the check 
> > could be like this:
> > #if defined(__INTEL_COMPILER) && (__INTEL_COMPILER < 1900) // employ the 
> > workaround for the Intel 18.0 compiler and older
> > 
> > This way the workaround will "disappear" when the bug gets fixed.
> > 
> > For that matter I wonder if it's still necessary to use the workaround for 
> > the Microsoft compiler?
> > 
> > Thanks for reporting the bug into the Intel forum. If we put a bug 
> > workaround into LLVM it would be very nice to have the bug reported to the 
> > offending compiler. 
> > 
> This one will probably not be fixed because I was answered that it's the 
> predicted behavior and that I need not to "#define inline __inline" if I use 
> intel compiler
> I agree though about versions which need to be taken into account
thanks



Comment at: include/llvm/ADT/BitmaskEnum.h:73
 
+#ifdef __INTEL_COMPILER
+template 

yvvan wrote:
> mibintc wrote:
> > what problem is being worked around here? I checked your post into the 
> > Intel compiler forum "enum members are not visible to template 
> > specialization" and I can observe the bug on our Windows compiler but not 
> > our Linux compiler. 
> it was recently accepted. the issues is that 
> E::LLVM_BITMASK_LARGEST_ENUMERATOR from line 80 here is always invalid with 
> intel compiler and the whole set of operator overloads is ignored
> at least on Windows :)
Thanks. I found the bug report for this one in the icc bug tracking database. 
It's CMPLRS-47898. With luck it will be fixed in the next update (ballpark 
based on previous release cadence, 3 months)



Comment at: include/llvm/Analysis/AliasAnalysis.h:254
   FMRB_OnlyAccessesInaccessibleOrArgMem = FMRL_InaccessibleMem |
-  FMRL_ArgumentPointees |
+  
static_cast(FMRL_ArgumentPointees) |
   static_cast(ModRefInfo::ModRef),

yvvan wrote:
> mibintc wrote:
> > is this a necessary workaround for the Intel compiler? It's not pretty.  
> > Suggest we add the #if around it? Long term i hope this would not be 
> > necessary.
> i'm not sure that adding #if around makes it easier to understand :)
> of course I can add it everywhere I append enums with static_cast
The modification for the Intel compiler is so ugly that I'd rather it get 
wrapped with the #if, and eventually the workaround could get discarded. 
Imagine yourself 20 years from now staring at that code and wondering why it is 
written that way.  The #if makes it stand out, and easier to find later, and 
easier to restore to pristine state, when you want to clean it up.  

I tried a small test case with the Intel compiler but i couldn't see the 
problem, must be too simple-minded.
typedef enum { a,b,c,d,e } letters;
char conv( letters l) {
  switch(l) {
  case a: return 'a';
  case a | b : return 'b';
  default : return 'z';
  }
}




Comment at: lib/Target/AMDGPU/SIISelLowering.cpp:6131
 
+#ifndef __INTEL_COMPILER
 static_assert(((~(SIInstrFlags::S_NAN |

yvvan wrote:
> mibintc wrote:
> > this assesrts with Intel compiler?  Or the expression needs to be rewritten 
> > with static_cast as above?
> "needs to be rewritten with static_cast" - correct. it can be done but I was 
> a bit lazy and statioc_assert does not affect the outcome of build :)

ok, it's got the if intel so: lgtm


https://reviews.llvm.org/D44426



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

[clang-tools-extra] r328107 - [clang-tidy] Resubmit hicpp-multiway-paths-covered without breaking test

[Jonas Toth via cfe-commits]

Author: jonastoth
Date: Wed Mar 21 08:34:15 2018
New Revision: 328107

URL: http://llvm.org/viewvc/llvm-project?rev=328107&view=rev
Log:
[clang-tidy] Resubmit hicpp-multiway-paths-covered without breaking test

The original check did break the green buildbot in the sanitizer build.
It took a while to redroduce and understand the issue.

There occured a stackoverflow while parsing the AST. The testcase with
256 case labels was the problem because each case label added another
stackframe. It seemed that the issue occured only in 'RelWithDebInfo' builds
and not in normal sanitizer builds.

To simplify the matchers the recognition for the different kinds of switch
statements has been moved into a seperate function and will not be done with
ASTMatchers. This is an attempt to reduce recursion and stacksize as well.

The new check removed this big testcase. Covering all possible values is still
implemented for bitfields and works there. The same logic on integer types
will lead to the issue.

Running it over LLVM gives the following results:


Differential: https://reviews.llvm.org/D40737

Added:
clang-tools-extra/trunk/clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp
clang-tools-extra/trunk/clang-tidy/hicpp/MultiwayPathsCoveredCheck.h

clang-tools-extra/trunk/docs/clang-tidy/checks/hicpp-multiway-paths-covered.rst

clang-tools-extra/trunk/test/clang-tidy/hicpp-multiway-paths-covered-else.cpp
clang-tools-extra/trunk/test/clang-tidy/hicpp-multiway-paths-covered.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/hicpp/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/hicpp/HICPPTidyModule.cpp
clang-tools-extra/trunk/docs/ReleaseNotes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst

Modified: clang-tools-extra/trunk/clang-tidy/hicpp/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/hicpp/CMakeLists.txt?rev=328107&r1=328106&r2=328107&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/hicpp/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/hicpp/CMakeLists.txt Wed Mar 21 08:34:15 
2018
@@ -2,6 +2,7 @@ set(LLVM_LINK_COMPONENTS support)
 
 add_clang_library(clangTidyHICPPModule
   ExceptionBaseclassCheck.cpp
+  MultiwayPathsCoveredCheck.cpp
   NoAssemblerCheck.cpp
   HICPPTidyModule.cpp
   SignedBitwiseCheck.cpp

Modified: clang-tools-extra/trunk/clang-tidy/hicpp/HICPPTidyModule.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/hicpp/HICPPTidyModule.cpp?rev=328107&r1=328106&r2=328107&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/hicpp/HICPPTidyModule.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/hicpp/HICPPTidyModule.cpp Wed Mar 21 
08:34:15 2018
@@ -36,6 +36,7 @@
 #include "../readability/FunctionSizeCheck.h"
 #include "../readability/IdentifierNamingCheck.h"
 #include "ExceptionBaseclassCheck.h"
+#include "MultiwayPathsCoveredCheck.h"
 #include "NoAssemblerCheck.h"
 #include "SignedBitwiseCheck.h"
 
@@ -54,6 +55,8 @@ public:
 "hicpp-deprecated-headers");
 CheckFactories.registerCheck(
 "hicpp-exception-baseclass");
+CheckFactories.registerCheck(
+"hicpp-multiway-paths-covered");
 CheckFactories.registerCheck("hicpp-signed-bitwise");
 CheckFactories.registerCheck(
 "hicpp-explicit-conversions");

Added: clang-tools-extra/trunk/clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp?rev=328107&view=auto
==
--- clang-tools-extra/trunk/clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp 
(added)
+++ clang-tools-extra/trunk/clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp Wed 
Mar 21 08:34:15 2018
@@ -0,0 +1,181 @@
+//===--- MultiwayPathsCoveredCheck.cpp - 
clang-tidy===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "MultiwayPathsCoveredCheck.h"
+#include "clang/AST/ASTContext.h"
+
+#include 
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace hicpp {
+
+void MultiwayPathsCoveredCheck::storeOptions(
+ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "WarnOnMissingElse", WarnOnMissingElse);
+}
+
+void MultiwayPathsCoveredCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(
+  switchStmt(
+  hasCondition(allOf(
+  // Match on switch statements that have either a bit-field or
+  // an integer condition. The ordering in 'anyOf()' is
+  // important

Re: r327959 - [ms] Parse #pragma optimize and ignore it behind its own flag

[Nico Weber via cfe-commits]

>From the bot changes, it seems that -Wunknown-pragma doesn't disable this
new warning. Shouldn't it do that?

On Tue, Mar 20, 2018, 9:55 AM Hans Wennborg via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: hans
> Date: Tue Mar 20 01:53:11 2018
> New Revision: 327959
>
> URL: http://llvm.org/viewvc/llvm-project?rev=327959&view=rev
> Log:
> [ms] Parse #pragma optimize and ignore it behind its own flag
>
> This allows users to turn off warnings about this pragma specifically,
> while still receiving warnings about other ignored pragmas.
>
> Differential Revision: https://reviews.llvm.org/D44630
>
> Modified:
> cfe/trunk/include/clang/Basic/DiagnosticGroups.td
> cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
> cfe/trunk/include/clang/Parse/Parser.h
> cfe/trunk/lib/Parse/ParsePragma.cpp
> cfe/trunk/test/Preprocessor/pragma_microsoft.c
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=327959&r1=327958&r2=327959&view=diff
>
> ==
> --- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Tue Mar 20 01:53:11
> 2018
> @@ -515,8 +515,13 @@ def UninitializedStaticSelfInit : DiagGr
>  def Uninitialized  : DiagGroup<"uninitialized", [UninitializedSometimes,
>
> UninitializedStaticSelfInit]>;
>  def IgnoredPragmaIntrinsic : DiagGroup<"ignored-pragma-intrinsic">;
> +// #pragma optimize is often used to avoid to work around MSVC codegen
> bugs or
> +// to disable inlining. It's not completely clear what alternative to
> suggest
> +// (#pragma clang optimize, noinline) so suggest nothing for now.
> +def IgnoredPragmaOptimize : DiagGroup<"ignored-pragma-optimize">;
>  def UnknownPragmas : DiagGroup<"unknown-pragmas">;
> -def IgnoredPragmas : DiagGroup<"ignored-pragmas",
> [IgnoredPragmaIntrinsic]>;
> +def IgnoredPragmas : DiagGroup<"ignored-pragmas",
> +[IgnoredPragmaIntrinsic, IgnoredPragmaOptimize]>;
>  def PragmaClangAttribute : DiagGroup<"pragma-clang-attribute">;
>  def PragmaPackSuspiciousInclude :
> DiagGroup<"pragma-pack-suspicious-include">;
>  def PragmaPack : DiagGroup<"pragma-pack", [PragmaPackSuspiciousInclude]>;
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td?rev=327959&r1=327958&r2=327959&view=diff
>
> ==
> --- cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td Tue Mar 20
> 01:53:11 2018
> @@ -895,6 +895,12 @@ def warn_pragma_expected_rparen : Warnin
>"missing ')' after '#pragma %0' - ignoring">, InGroup;
>  def warn_pragma_expected_identifier : Warning<
>"expected identifier in '#pragma %0' - ignored">,
> InGroup;
> +def warn_pragma_expected_string : Warning<
> +  "expected string literal in '#pragma %0' - ignoring">,
> InGroup;
> +def warn_pragma_missing_argument : Warning<
> +  "missing argument to '#pragma %0'%select{|; expected %2}1">,
> InGroup;
> +def warn_pragma_invalid_argument : Warning<
> +  "unexpected argument '%0' to '#pragma %1'%select{|; expected %3}2">,
> InGroup;
>
>  // '#pragma clang section' related errors
>  def err_pragma_expected_clang_section_name : Error<
> @@ -923,6 +929,8 @@ def warn_pragma_ms_struct : Warning<
>  def warn_pragma_extra_tokens_at_eol : Warning<
>"extra tokens at end of '#pragma %0' - ignored">,
>InGroup;
> +def warn_pragma_expected_comma : Warning<
> +  "expected ',' in '#pragma %0'">, InGroup;
>  def warn_pragma_expected_punc : Warning<
>"expected ')' or ',' in '#pragma %0'">, InGroup;
>  def warn_pragma_expected_non_wide_string : Warning<
> @@ -960,6 +968,10 @@ def warn_pragma_pack_malformed : Warning
>  def warn_pragma_intrinsic_builtin : Warning<
>"%0 is not a recognized builtin%select{|; consider including 
> to access non-builtin intrinsics}1">,
>InGroup;
> +// - #pragma optimize
> +def warn_pragma_optimize : Warning<
> +  "'#pragma optimize' is not supported">,
> +  InGroup;
>  // - #pragma unused
>  def warn_pragma_unused_expected_var : Warning<
>"expected '#pragma unused' argument to be a variable name">,
>
> Modified: cfe/trunk/include/clang/Parse/Parser.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Parse/Parser.h?rev=327959&r1=327958&r2=327959&view=diff
>
> ==
> --- cfe/trunk/include/clang/Parse/Parser.h (original)
> +++ cfe/trunk/include/clang/Parse/Parser.h Tue Mar 20 01:53:11 2018
> @@ -179,6 +179,7 @@ class Parser : public CodeCompletionHand
>std::unique_ptr MSSection;
>std::unique_ptr MSRuntimeChecks;
>std::unique_ptr MSIntrinsic;
> +  std::unique_ptr MS

[PATCH] D44729: [ASTMatchers] Add hasSubExpr() matcher.

[Clement Courbet via Phabricator via cfe-commits]

courbet added a comment.

Thanks for the pointer. I missed the trampoline through 
GetSourceExpressionMatcher.


Repository:
  rC Clang

https://reviews.llvm.org/D44729



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

[PATCH] D44272: [clangd] Support incremental document syncing

[Simon Marchi via Phabricator via cfe-commits]

simark marked 34 inline comments as done.
simark added inline comments.



Comment at: clangd/DraftStore.h:36
   /// Replace contents of the draft for \p File with \p Contents.
-  void updateDraft(PathRef File, StringRef Contents);
+  void addDraft(PathRef File, StringRef Contents);
+

ilya-biryukov wrote:
> Could we add versions from LSP's `VersionedTextDocumentIdentifier` here and 
> make the relevant sanity checks?
> Applying diffs to the wrong version will cause everything to fall apart, so 
> we should detect this error and signal it to the client as soon as possible.
I agree that applying diffs to the wrong version will break basically 
everything, but even if we detect a version mismatch, I don't see what we could 
do, since we don't have a mean to report the error to the client.  The only 
thing we could do is log it (which we already do.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44272



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

[PATCH] D42730: [clang-tidy]] Add check for use of types/classes/functions from header which are deprecated and removed in C++17

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added inline comments.



Comment at: clang-tidy/modernize/DeprecatedFunctionalCheck.cpp:48-54
+  } else if (const auto *const Call =
+ Result.Nodes.getNodeAs("ptr_fun_call")) {
+diag(Call->getLocStart(), Message) << "'std::ptr_fun'";
+  } else if (const auto *const Call =
+ Result.Nodes.getNodeAs("mem_fun_call")) {
+diag(Call->getLocStart(), Message) << "'std::mem_fun'";
+  }

aaron.ballman wrote:
> alexfh wrote:
> > aaron.ballman wrote:
> > > alexfh wrote:
> > > > aaron.ballman wrote:
> > > > > alexfh wrote:
> > > > > > alexfh wrote:
> > > > > > > aaron.ballman wrote:
> > > > > > > > massberg wrote:
> > > > > > > > > aaron.ballman wrote:
> > > > > > > > > > I think that this code should be generalized (same with the 
> > > > > > > > > > matchers) so that you match on `hasAnyName()` for the 
> > > > > > > > > > function calls and use `CallExpr::getCalleeDecl()` to get 
> > > > > > > > > > the declaration. e.g.,
> > > > > > > > > > ```
> > > > > > > > > > if (const auto *Call = 
> > > > > > > > > > Result.Nodes.getNodeAs("blech")) {
> > > > > > > > > >   if (const Decl *Callee = Call->getCalleeDecl())
> > > > > > > > > > diag(Call->getLocStart(), Message) << Calleee;
> > > > > > > > > > }
> > > > > > > > > > ```
> > > > > > > > > > This way you can add more named without having to add extra 
> > > > > > > > > > logic for the diagnostics.
> > > > > > > > > I generalized the code and the matcher.
> > > > > > > > > When we use
> > > > > > > > > ```
> > > > > > > > > << cast(Callee);
> > > > > > > > > ```
> > > > > > > > > we get the template arguments in the name , e.g. 
> > > > > > > > > `ptr_fun`, so I chose to use 
> > > > > > > > > `getQualifiedNameAsString`.
> > > > > > > > > If there is there a better way to get the function name 
> > > > > > > > > without template arguments I appreciate any suggestions.
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > Nope, in that case, your code is correct. However, we generally 
> > > > > > > > provide the template arguments in diagnostics. I see @alexfh 
> > > > > > > > was asking for them to be removed as not being useful, but I'm 
> > > > > > > > not certain I agree with his rationale. Yes, all instances are 
> > > > > > > > deprecated and thus the template arguments don't discern 
> > > > > > > > between good and bad cases, but showing the template arguments 
> > > > > > > > is also consistent with the other diagnostics we emit. For 
> > > > > > > > instance, other "deprecated" diagnostics also emit the template 
> > > > > > > > arguments. I'm not certain we should be inconsistent with the 
> > > > > > > > way we produce diagnostics, but I'll defer to Alex if he still 
> > > > > > > > feels strongly about leaving them off here.
> > > > > > > Indeed, -Wdeprecated-declarations warnings print template 
> > > > > > > arguments. Moreover, they seem to be issued only on 
> > > > > > > instantiations, see https://godbolt.org/g/W563gw.
> > > > > > > 
> > > > > > > But I have a number of concerns with template arguments in the 
> > > > > > > deprecation warnings:
> > > > > > > 
> > > > > > > 1. The note attached to the warning lies. Consider a warning from 
> > > > > > > the test above:
> > > > > > >   ...
> > > > > > >   :11:1: warning: 'B' is deprecated: bbb 
> > > > > > > [-Wdeprecated-declarations]
> > > > > > >   B e;
> > > > > > >   ^
> > > > > > >   :7:10: note: 'B' has been explicitly marked 
> > > > > > > deprecated here
> > > > > > >   struct [[deprecated("bbb")]] B {};
> > > > > > > 
> > > > > > >  But `B` hasn't been explicitly marked deprecated, only the 
> > > > > > > template definition of `B` has been. Template arguments are 
> > > > > > > important in the case of the explicit template specialization 
> > > > > > > `A` in the same example, but not in cases where the template 
> > > > > > > definition was marked deprecated, since template arguments only 
> > > > > > > add noise and no useful information there.
> > > > > > > 2. Warnings can easily become unreadable: 
> > > > > > > https://godbolt.org/g/AFdznH
> > > > > > > 3. Certain coding patterns can result in numerous deprecation 
> > > > > > > warnings differing only in template arguments: 
> > > > > > > https://godbolt.org/g/U2JCbG. Clang-tidy can deduplicate 
> > > > > > > warnings, if they have identical text and location, but adding 
> > > > > > > template arguments to the message will prevent deduplication. 
> > > > > > > I've seen a case where thousands of deprecation warnings were 
> > > > > > > generated for a single line in a widely used header.
> > > > > > > 
> > > > > > > So yes, I feel strongly about leaving off template arguments in 
> > > > > > > case the whole template was marked deprecated. I think it would 
> > > > > > > be the right thing to do for the -Wdeprecated-declarations 
> > > > > > > diagnostic as well.
> > > > > > s/leaving off/leaving out/
> > > > > > The note attached to the warning lie

[PATCH] D43967: [ASTImporter] Add test helper Fixture

[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.

This looks good to me. Thank you!
@xazax.hun Gabor, could you please take a look?


Repository:
  rC Clang

https://reviews.llvm.org/D43967



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

[clang-tools-extra] r328108 - [Fix] fix type deduction on ARM and MSVC

[Jonas Toth via cfe-commits]

Author: jonastoth
Date: Wed Mar 21 08:50:15 2018
New Revision: 328108

URL: http://llvm.org/viewvc/llvm-project?rev=328108&view=rev
Log:
[Fix] fix type deduction on ARM and MSVC

Modified:
clang-tools-extra/trunk/clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp

Modified: clang-tools-extra/trunk/clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp?rev=328108&r1=328107&r2=328108&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp Wed 
Mar 21 08:50:15 2018
@@ -167,7 +167,7 @@ void MultiwayPathsCoveredCheck::handleSw
   return twoPow(BitfieldDecl->getBitWidthValue(*Result.Context));
 }
 
-return 0ul;
+return static_cast(0);
   }();
 
   // FIXME: Transform the 'switch' into an 'if' for CaseCount == 1.


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

[PATCH] D42730: [clang-tidy]] Add check for use of types/classes/functions from header which are deprecated and removed in C++17

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tidy/modernize/DeprecatedFunctionalCheck.cpp:48-54
+  } else if (const auto *const Call =
+ Result.Nodes.getNodeAs("ptr_fun_call")) {
+diag(Call->getLocStart(), Message) << "'std::ptr_fun'";
+  } else if (const auto *const Call =
+ Result.Nodes.getNodeAs("mem_fun_call")) {
+diag(Call->getLocStart(), Message) << "'std::mem_fun'";
+  }

alexfh wrote:
> aaron.ballman wrote:
> > alexfh wrote:
> > > aaron.ballman wrote:
> > > > alexfh wrote:
> > > > > aaron.ballman wrote:
> > > > > > alexfh wrote:
> > > > > > > alexfh wrote:
> > > > > > > > aaron.ballman wrote:
> > > > > > > > > massberg wrote:
> > > > > > > > > > aaron.ballman wrote:
> > > > > > > > > > > I think that this code should be generalized (same with 
> > > > > > > > > > > the matchers) so that you match on `hasAnyName()` for the 
> > > > > > > > > > > function calls and use `CallExpr::getCalleeDecl()` to get 
> > > > > > > > > > > the declaration. e.g.,
> > > > > > > > > > > ```
> > > > > > > > > > > if (const auto *Call = 
> > > > > > > > > > > Result.Nodes.getNodeAs("blech")) {
> > > > > > > > > > >   if (const Decl *Callee = Call->getCalleeDecl())
> > > > > > > > > > > diag(Call->getLocStart(), Message) << Calleee;
> > > > > > > > > > > }
> > > > > > > > > > > ```
> > > > > > > > > > > This way you can add more named without having to add 
> > > > > > > > > > > extra logic for the diagnostics.
> > > > > > > > > > I generalized the code and the matcher.
> > > > > > > > > > When we use
> > > > > > > > > > ```
> > > > > > > > > > << cast(Callee);
> > > > > > > > > > ```
> > > > > > > > > > we get the template arguments in the name , e.g. 
> > > > > > > > > > `ptr_fun`, so I chose to use 
> > > > > > > > > > `getQualifiedNameAsString`.
> > > > > > > > > > If there is there a better way to get the function name 
> > > > > > > > > > without template arguments I appreciate any suggestions.
> > > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > Nope, in that case, your code is correct. However, we 
> > > > > > > > > generally provide the template arguments in diagnostics. I 
> > > > > > > > > see @alexfh was asking for them to be removed as not being 
> > > > > > > > > useful, but I'm not certain I agree with his rationale. Yes, 
> > > > > > > > > all instances are deprecated and thus the template arguments 
> > > > > > > > > don't discern between good and bad cases, but showing the 
> > > > > > > > > template arguments is also consistent with the other 
> > > > > > > > > diagnostics we emit. For instance, other "deprecated" 
> > > > > > > > > diagnostics also emit the template arguments. I'm not certain 
> > > > > > > > > we should be inconsistent with the way we produce 
> > > > > > > > > diagnostics, but I'll defer to Alex if he still feels 
> > > > > > > > > strongly about leaving them off here.
> > > > > > > > Indeed, -Wdeprecated-declarations warnings print template 
> > > > > > > > arguments. Moreover, they seem to be issued only on 
> > > > > > > > instantiations, see https://godbolt.org/g/W563gw.
> > > > > > > > 
> > > > > > > > But I have a number of concerns with template arguments in the 
> > > > > > > > deprecation warnings:
> > > > > > > > 
> > > > > > > > 1. The note attached to the warning lies. Consider a warning 
> > > > > > > > from the test above:
> > > > > > > >   ...
> > > > > > > >   :11:1: warning: 'B' is deprecated: bbb 
> > > > > > > > [-Wdeprecated-declarations]
> > > > > > > >   B e;
> > > > > > > >   ^
> > > > > > > >   :7:10: note: 'B' has been explicitly marked 
> > > > > > > > deprecated here
> > > > > > > >   struct [[deprecated("bbb")]] B {};
> > > > > > > > 
> > > > > > > >  But `B` hasn't been explicitly marked deprecated, only 
> > > > > > > > the template definition of `B` has been. Template arguments are 
> > > > > > > > important in the case of the explicit template specialization 
> > > > > > > > `A` in the same example, but not in cases where the 
> > > > > > > > template definition was marked deprecated, since template 
> > > > > > > > arguments only add noise and no useful information there.
> > > > > > > > 2. Warnings can easily become unreadable: 
> > > > > > > > https://godbolt.org/g/AFdznH
> > > > > > > > 3. Certain coding patterns can result in numerous deprecation 
> > > > > > > > warnings differing only in template arguments: 
> > > > > > > > https://godbolt.org/g/U2JCbG. Clang-tidy can deduplicate 
> > > > > > > > warnings, if they have identical text and location, but adding 
> > > > > > > > template arguments to the message will prevent deduplication. 
> > > > > > > > I've seen a case where thousands of deprecation warnings were 
> > > > > > > > generated for a single line in a widely used header.
> > > > > > > > 
> > > > > > > > So yes, I feel strongly about leaving off template arguments in 
> > > > > > > > case the whole template was marked deprecated. I think i

[PATCH] D44589: [Sema] Make deprecation fix-it replace all multi-parameter ObjC method slots.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

In https://reviews.llvm.org/D44589#1044350, @aaron.ballman wrote:

> This generally looks reasonable to me, but @rsmith should weigh in before you 
> commit because `MultiSourceLocation` is novel.


Thanks for the review, Aaron. I tried not to do anything stupid with 
`MultiSourceLocation` but another opinion will be definitely useful.


https://reviews.llvm.org/D44589



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

[PATCH] D44272: [clangd] Support incremental document syncing

[Simon Marchi via Phabricator via cfe-commits]

simark updated this revision to Diff 139310.
simark marked an inline comment as done.
simark added a comment.

Address review comments, except LSP version check


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44272

Files:
  clangd/ClangdLSPServer.cpp
  clangd/DraftStore.cpp
  clangd/DraftStore.h
  clangd/Protocol.cpp
  clangd/Protocol.h
  test/clangd/initialize-params-invalid.test
  test/clangd/initialize-params.test
  unittests/clangd/CMakeLists.txt
  unittests/clangd/DraftStoreTests.cpp

Index: unittests/clangd/DraftStoreTests.cpp
===
--- /dev/null
+++ unittests/clangd/DraftStoreTests.cpp
@@ -0,0 +1,355 @@
+//===-- DraftStoreTests.cpp - Clangd unit tests -*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "Annotations.h"
+#include "DraftStore.h"
+#include "SourceCode.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+using namespace llvm;
+
+struct IncrementalTestStep {
+  StringRef Src;
+  StringRef Contents;
+};
+
+static int rangeLength(StringRef Code, const Range &Rng) {
+  llvm::Expected Start = positionToOffset(Code, Rng.start);
+  llvm::Expected End = positionToOffset(Code, Rng.end);
+  assert(Start);
+  assert(End);
+  return *End - *Start;
+}
+
+// Send the changes one by one to updateDraft, verify the intermediate results.
+
+static void stepByStep(llvm::ArrayRef Steps) {
+  DraftStore DS;
+  Annotations InitialSrc(Steps.front().Src);
+  const char Path[] = "/hello.cpp";
+
+  // Set the initial content.
+  DS.addDraft(Path, InitialSrc.code());
+
+  for (size_t i = 1; i < Steps.size(); i++) {
+Annotations SrcBefore(Steps[i - 1].Src);
+Annotations SrcAfter(Steps[i].Src);
+StringRef Contents = Steps[i - 1].Contents;
+TextDocumentContentChangeEvent Event{
+SrcBefore.range(),
+rangeLength(SrcBefore.code(), SrcBefore.range()),
+Contents.str(),
+};
+
+llvm::Expected Result = DS.updateDraft(Path, {Event});
+EXPECT_TRUE(!!Result);
+EXPECT_EQ(*Result, SrcAfter.code());
+EXPECT_EQ(*DS.getDraft(Path), SrcAfter.code());
+  }
+}
+
+// Send all the changes at once to updateDraft, check only the final result.
+
+static void allAtOnce(llvm::ArrayRef Steps) {
+  DraftStore DS;
+  Annotations InitialSrc(Steps.front().Src);
+  Annotations FinalSrc(Steps.back().Src);
+  const char Path[] = "/hello.cpp";
+  std::vector Changes;
+
+  for (size_t i = 0; i < Steps.size() - 1; i++) {
+Annotations Src(Steps[i].Src);
+StringRef Contents = Steps[i].Contents;
+
+Changes.push_back({
+Src.range(),
+rangeLength(Src.code(), Src.range()),
+Contents.str(),
+});
+  }
+
+  // Set the initial content.
+  DS.addDraft(Path, InitialSrc.code());
+
+  llvm::Expected Result = DS.updateDraft(Path, Changes);
+
+  EXPECT_TRUE(!!Result) << llvm::toString(Result.takeError());
+  EXPECT_EQ(*Result, FinalSrc.code());
+  EXPECT_EQ(*DS.getDraft(Path), FinalSrc.code());
+}
+
+TEST(DraftStoreIncrementalUpdateTest, Simple) {
+  // clang-format off
+  IncrementalTestStep Steps[] =
+{
+  // Replace a range
+  {
+R"cpp(static int
+hello[[World]]()
+{})cpp",
+"Universe"
+  },
+  // Delete a range
+  {
+R"cpp(static int
+hello[[Universe]]()
+{})cpp",
+""
+  },
+  // Add a range
+  {
+R"cpp(static int
+hello[[]]()
+{})cpp",
+"Monde"
+  },
+  {
+R"cpp(static int
+helloMonde()
+{})cpp",
+""
+  }
+};
+  // clang-format on
+
+  stepByStep(Steps);
+  allAtOnce(Steps);
+}
+
+TEST(DraftStoreIncrementalUpdateTest, MultiLine) {
+  // clang-format off
+  IncrementalTestStep Steps[] =
+{
+  // Replace a range
+  {
+R"cpp(static [[int
+helloWorld]]()
+{})cpp",
+R"cpp(char
+welcome)cpp"
+  },
+  // Delete a range
+  {
+R"cpp(static char[[
+welcome]]()
+{})cpp",
+""
+  },
+  // Add a range
+  {
+R"cpp(static char[[]]()
+{})cpp",
+R"cpp(
+cookies)cpp"
+  },
+  // Replace the whole file
+  {
+R"cpp([[static char
+cookies()
+{}]])cpp",
+R"cpp(#include 
+)cpp"
+  },
+  // Delete the whole file
+  {
+R"cpp([[#include 
+]])cpp",
+"",
+  },
+  // Add something to an empty file
+  {
+"[[]]",
+R"cpp(int main() {
+)cpp",
+  },
+  {
+R"cpp(int main() {
+)cpp",
+""
+  }
+};
+  // clang-format on
+
+  stepByStep(Steps);
+  allAtOnce(Steps);
+}
+
+TEST(DraftStoreIncrementalUpdateTest, WrongRangeLength) {
+  DraftStore DS;
+  Path File = "foo.cpp";
+
+  DS.addDraft(File, "int main() {}\n");
+
+  TextDocumentContentChangeEvent Change;
+  Change.range.e

[PATCH] D44724: [Fuchsia] Don't install libc++, libc++abi or libunwind on Darwin

[Petr Hosek via Phabricator via cfe-commits]

phosek updated this revision to Diff 139311.

https://reviews.llvm.org/D44724

Files:
  clang/cmake/caches/Fuchsia-stage2.cmake


Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -38,17 +38,28 @@
 set(LLVM_RUNTIME_TARGETS 
"default;x86_64-fuchsia;aarch64-fuchsia;x86_64-fuchsia-asan:x86_64-fuchsia;aarch64-fuchsia-asan:aarch64-fuchsia"
 CACHE STRING "")
 
 # Set the default target runtimes options.
-set(LIBUNWIND_ENABLE_SHARED OFF CACHE BOOL "")
-set(LIBUNWIND_USE_COMPILER_RT ON CACHE BOOL "")
-set(LIBUNWIND_INSTALL_LIBRARY OFF CACHE BOOL "")
-set(LIBCXXABI_USE_COMPILER_RT ON CACHE BOOL "")
-set(LIBCXXABI_ENABLE_SHARED OFF CACHE BOOL "")
-set(LIBCXXABI_USE_LLVM_UNWINDER ON CACHE BOOL "")
-set(LIBCXXABI_ENABLE_STATIC_UNWINDER ON CACHE BOOL "")
-set(LIBCXXABI_INSTALL_LIBRARY OFF CACHE BOOL "")
-set(LIBCXX_USE_COMPILER_RT ON CACHE BOOL "")
-set(LIBCXX_ENABLE_SHARED OFF CACHE BOOL "")
-set(LIBCXX_ENABLE_STATIC_ABI_LIBRARY ON CACHE BOOL "")
+if(APPLE)
+  # Disable installing libc++, libc++abi or libunwind on Darwin, since Clang
+  # driver doesn't know how to use libraries that are part of the toolchain.
+  set(LIBUNWIND_INSTALL_HEADERS OFF CACHE BOOL "")
+  set(LIBUNWIND_INSTALL_LIBRARY OFF CACHE BOOL "")
+  set(LIBCXXABI_INSTALL_HEADERS OFF CACHE BOOL "")
+  set(LIBCXXABI_INSTALL_LIBRARY OFF CACHE BOOL "")
+  set(LIBCXX_INSTALL_HEADERS OFF CACHE BOOL "")
+  set(LIBCXX_INSTALL_LIBRARY OFF CACHE BOOL "")
+else()
+  set(LIBUNWIND_ENABLE_SHARED OFF CACHE BOOL "")
+  set(LIBUNWIND_USE_COMPILER_RT ON CACHE BOOL "")
+  set(LIBUNWIND_INSTALL_LIBRARY OFF CACHE BOOL "")
+  set(LIBCXXABI_USE_COMPILER_RT ON CACHE BOOL "")
+  set(LIBCXXABI_ENABLE_SHARED OFF CACHE BOOL "")
+  set(LIBCXXABI_USE_LLVM_UNWINDER ON CACHE BOOL "")
+  set(LIBCXXABI_ENABLE_STATIC_UNWINDER ON CACHE BOOL "")
+  set(LIBCXXABI_INSTALL_LIBRARY OFF CACHE BOOL "")
+  set(LIBCXX_USE_COMPILER_RT ON CACHE BOOL "")
+  set(LIBCXX_ENABLE_SHARED OFF CACHE BOOL "")
+  set(LIBCXX_ENABLE_STATIC_ABI_LIBRARY ON CACHE BOOL "")
+endif()
 
 # Set the per-target runtimes options.
 foreach(target x86_64;aarch64)


Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -38,17 +38,28 @@
 set(LLVM_RUNTIME_TARGETS "default;x86_64-fuchsia;aarch64-fuchsia;x86_64-fuchsia-asan:x86_64-fuchsia;aarch64-fuchsia-asan:aarch64-fuchsia" CACHE STRING "")
 
 # Set the default target runtimes options.
-set(LIBUNWIND_ENABLE_SHARED OFF CACHE BOOL "")
-set(LIBUNWIND_USE_COMPILER_RT ON CACHE BOOL "")
-set(LIBUNWIND_INSTALL_LIBRARY OFF CACHE BOOL "")
-set(LIBCXXABI_USE_COMPILER_RT ON CACHE BOOL "")
-set(LIBCXXABI_ENABLE_SHARED OFF CACHE BOOL "")
-set(LIBCXXABI_USE_LLVM_UNWINDER ON CACHE BOOL "")
-set(LIBCXXABI_ENABLE_STATIC_UNWINDER ON CACHE BOOL "")
-set(LIBCXXABI_INSTALL_LIBRARY OFF CACHE BOOL "")
-set(LIBCXX_USE_COMPILER_RT ON CACHE BOOL "")
-set(LIBCXX_ENABLE_SHARED OFF CACHE BOOL "")
-set(LIBCXX_ENABLE_STATIC_ABI_LIBRARY ON CACHE BOOL "")
+if(APPLE)
+  # Disable installing libc++, libc++abi or libunwind on Darwin, since Clang
+  # driver doesn't know how to use libraries that are part of the toolchain.
+  set(LIBUNWIND_INSTALL_HEADERS OFF CACHE BOOL "")
+  set(LIBUNWIND_INSTALL_LIBRARY OFF CACHE BOOL "")
+  set(LIBCXXABI_INSTALL_HEADERS OFF CACHE BOOL "")
+  set(LIBCXXABI_INSTALL_LIBRARY OFF CACHE BOOL "")
+  set(LIBCXX_INSTALL_HEADERS OFF CACHE BOOL "")
+  set(LIBCXX_INSTALL_LIBRARY OFF CACHE BOOL "")
+else()
+  set(LIBUNWIND_ENABLE_SHARED OFF CACHE BOOL "")
+  set(LIBUNWIND_USE_COMPILER_RT ON CACHE BOOL "")
+  set(LIBUNWIND_INSTALL_LIBRARY OFF CACHE BOOL "")
+  set(LIBCXXABI_USE_COMPILER_RT ON CACHE BOOL "")
+  set(LIBCXXABI_ENABLE_SHARED OFF CACHE BOOL "")
+  set(LIBCXXABI_USE_LLVM_UNWINDER ON CACHE BOOL "")
+  set(LIBCXXABI_ENABLE_STATIC_UNWINDER ON CACHE BOOL "")
+  set(LIBCXXABI_INSTALL_LIBRARY OFF CACHE BOOL "")
+  set(LIBCXX_USE_COMPILER_RT ON CACHE BOOL "")
+  set(LIBCXX_ENABLE_SHARED OFF CACHE BOOL "")
+  set(LIBCXX_ENABLE_STATIC_ABI_LIBRARY ON CACHE BOOL "")
+endif()
 
 # Set the per-target runtimes options.
 foreach(target x86_64;aarch64)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44272: [clangd] Support incremental document syncing

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clangd/DraftStore.h:36
   /// Replace contents of the draft for \p File with \p Contents.
-  void updateDraft(PathRef File, StringRef Contents);
+  void addDraft(PathRef File, StringRef Contents);
+

simark wrote:
> ilya-biryukov wrote:
> > Could we add versions from LSP's `VersionedTextDocumentIdentifier` here and 
> > make the relevant sanity checks?
> > Applying diffs to the wrong version will cause everything to fall apart, so 
> > we should detect this error and signal it to the client as soon as possible.
> I agree that applying diffs to the wrong version will break basically 
> everything, but even if we detect a version mismatch, I don't see what we 
> could do, since we don't have a mean to report the error to the client.  The 
> only thing we could do is log it (which we already do.
If we couldn't apply a diff, we should return errors from all future operations 
on this file until it gets closed and reopened. Otherwise clangd and the editor 
would see inconsistent contents for the file and results provided by clangd 
would be unreliable.
The errors from any actions on the invalid file would actually be visible to 
the users.

The simplest way to achieve that is to remove the file from `DraftStore` and 
`ClangdServer` on errors from `updateDraft`.
This will give "calling action on non-tracked file" errors for future 
operations and the actual root cause can be figured out from the logs.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44272



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

r328114 - [Fuchsia] Don't install libc++, libc++abi or libunwind on Darwin

[Petr Hosek via cfe-commits]

Author: phosek
Date: Wed Mar 21 09:48:26 2018
New Revision: 328114

URL: http://llvm.org/viewvc/llvm-project?rev=328114&view=rev
Log:
[Fuchsia] Don't install libc++, libc++abi or libunwind on Darwin

The Clang driver doesn't currently know how to use the libraries
that are shipped as part of the toolchain so there's no reason to
ship them at all.

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

Modified:
cfe/trunk/cmake/caches/Fuchsia-stage2.cmake

Modified: cfe/trunk/cmake/caches/Fuchsia-stage2.cmake
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/cmake/caches/Fuchsia-stage2.cmake?rev=328114&r1=328113&r2=328114&view=diff
==
--- cfe/trunk/cmake/caches/Fuchsia-stage2.cmake (original)
+++ cfe/trunk/cmake/caches/Fuchsia-stage2.cmake Wed Mar 21 09:48:26 2018
@@ -38,17 +38,28 @@ endforeach()
 set(LLVM_RUNTIME_TARGETS 
"default;x86_64-fuchsia;aarch64-fuchsia;x86_64-fuchsia-asan:x86_64-fuchsia;aarch64-fuchsia-asan:aarch64-fuchsia"
 CACHE STRING "")
 
 # Set the default target runtimes options.
-set(LIBUNWIND_ENABLE_SHARED OFF CACHE BOOL "")
-set(LIBUNWIND_USE_COMPILER_RT ON CACHE BOOL "")
-set(LIBUNWIND_INSTALL_LIBRARY OFF CACHE BOOL "")
-set(LIBCXXABI_USE_COMPILER_RT ON CACHE BOOL "")
-set(LIBCXXABI_ENABLE_SHARED OFF CACHE BOOL "")
-set(LIBCXXABI_USE_LLVM_UNWINDER ON CACHE BOOL "")
-set(LIBCXXABI_ENABLE_STATIC_UNWINDER ON CACHE BOOL "")
-set(LIBCXXABI_INSTALL_LIBRARY OFF CACHE BOOL "")
-set(LIBCXX_USE_COMPILER_RT ON CACHE BOOL "")
-set(LIBCXX_ENABLE_SHARED OFF CACHE BOOL "")
-set(LIBCXX_ENABLE_STATIC_ABI_LIBRARY ON CACHE BOOL "")
+if(APPLE)
+  # Disable installing libc++, libc++abi or libunwind on Darwin, since Clang
+  # driver doesn't know how to use libraries that are part of the toolchain.
+  set(LIBUNWIND_INSTALL_HEADERS OFF CACHE BOOL "")
+  set(LIBUNWIND_INSTALL_LIBRARY OFF CACHE BOOL "")
+  set(LIBCXXABI_INSTALL_HEADERS OFF CACHE BOOL "")
+  set(LIBCXXABI_INSTALL_LIBRARY OFF CACHE BOOL "")
+  set(LIBCXX_INSTALL_HEADERS OFF CACHE BOOL "")
+  set(LIBCXX_INSTALL_LIBRARY OFF CACHE BOOL "")
+else()
+  set(LIBUNWIND_ENABLE_SHARED OFF CACHE BOOL "")
+  set(LIBUNWIND_USE_COMPILER_RT ON CACHE BOOL "")
+  set(LIBUNWIND_INSTALL_LIBRARY OFF CACHE BOOL "")
+  set(LIBCXXABI_USE_COMPILER_RT ON CACHE BOOL "")
+  set(LIBCXXABI_ENABLE_SHARED OFF CACHE BOOL "")
+  set(LIBCXXABI_USE_LLVM_UNWINDER ON CACHE BOOL "")
+  set(LIBCXXABI_ENABLE_STATIC_UNWINDER ON CACHE BOOL "")
+  set(LIBCXXABI_INSTALL_LIBRARY OFF CACHE BOOL "")
+  set(LIBCXX_USE_COMPILER_RT ON CACHE BOOL "")
+  set(LIBCXX_ENABLE_SHARED OFF CACHE BOOL "")
+  set(LIBCXX_ENABLE_STATIC_ABI_LIBRARY ON CACHE BOOL "")
+endif()
 
 # Set the per-target runtimes options.
 foreach(target x86_64;aarch64)


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

[PATCH] D44724: [Fuchsia] Don't install libc++, libc++abi or libunwind on Darwin

[Petr Hosek via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL328114: [Fuchsia] Don't install libc++, libc++abi or 
libunwind on Darwin (authored by phosek, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D44724?vs=139311&id=139312#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D44724

Files:
  cfe/trunk/cmake/caches/Fuchsia-stage2.cmake


Index: cfe/trunk/cmake/caches/Fuchsia-stage2.cmake
===
--- cfe/trunk/cmake/caches/Fuchsia-stage2.cmake
+++ cfe/trunk/cmake/caches/Fuchsia-stage2.cmake
@@ -38,17 +38,28 @@
 set(LLVM_RUNTIME_TARGETS 
"default;x86_64-fuchsia;aarch64-fuchsia;x86_64-fuchsia-asan:x86_64-fuchsia;aarch64-fuchsia-asan:aarch64-fuchsia"
 CACHE STRING "")
 
 # Set the default target runtimes options.
-set(LIBUNWIND_ENABLE_SHARED OFF CACHE BOOL "")
-set(LIBUNWIND_USE_COMPILER_RT ON CACHE BOOL "")
-set(LIBUNWIND_INSTALL_LIBRARY OFF CACHE BOOL "")
-set(LIBCXXABI_USE_COMPILER_RT ON CACHE BOOL "")
-set(LIBCXXABI_ENABLE_SHARED OFF CACHE BOOL "")
-set(LIBCXXABI_USE_LLVM_UNWINDER ON CACHE BOOL "")
-set(LIBCXXABI_ENABLE_STATIC_UNWINDER ON CACHE BOOL "")
-set(LIBCXXABI_INSTALL_LIBRARY OFF CACHE BOOL "")
-set(LIBCXX_USE_COMPILER_RT ON CACHE BOOL "")
-set(LIBCXX_ENABLE_SHARED OFF CACHE BOOL "")
-set(LIBCXX_ENABLE_STATIC_ABI_LIBRARY ON CACHE BOOL "")
+if(APPLE)
+  # Disable installing libc++, libc++abi or libunwind on Darwin, since Clang
+  # driver doesn't know how to use libraries that are part of the toolchain.
+  set(LIBUNWIND_INSTALL_HEADERS OFF CACHE BOOL "")
+  set(LIBUNWIND_INSTALL_LIBRARY OFF CACHE BOOL "")
+  set(LIBCXXABI_INSTALL_HEADERS OFF CACHE BOOL "")
+  set(LIBCXXABI_INSTALL_LIBRARY OFF CACHE BOOL "")
+  set(LIBCXX_INSTALL_HEADERS OFF CACHE BOOL "")
+  set(LIBCXX_INSTALL_LIBRARY OFF CACHE BOOL "")
+else()
+  set(LIBUNWIND_ENABLE_SHARED OFF CACHE BOOL "")
+  set(LIBUNWIND_USE_COMPILER_RT ON CACHE BOOL "")
+  set(LIBUNWIND_INSTALL_LIBRARY OFF CACHE BOOL "")
+  set(LIBCXXABI_USE_COMPILER_RT ON CACHE BOOL "")
+  set(LIBCXXABI_ENABLE_SHARED OFF CACHE BOOL "")
+  set(LIBCXXABI_USE_LLVM_UNWINDER ON CACHE BOOL "")
+  set(LIBCXXABI_ENABLE_STATIC_UNWINDER ON CACHE BOOL "")
+  set(LIBCXXABI_INSTALL_LIBRARY OFF CACHE BOOL "")
+  set(LIBCXX_USE_COMPILER_RT ON CACHE BOOL "")
+  set(LIBCXX_ENABLE_SHARED OFF CACHE BOOL "")
+  set(LIBCXX_ENABLE_STATIC_ABI_LIBRARY ON CACHE BOOL "")
+endif()
 
 # Set the per-target runtimes options.
 foreach(target x86_64;aarch64)


Index: cfe/trunk/cmake/caches/Fuchsia-stage2.cmake
===
--- cfe/trunk/cmake/caches/Fuchsia-stage2.cmake
+++ cfe/trunk/cmake/caches/Fuchsia-stage2.cmake
@@ -38,17 +38,28 @@
 set(LLVM_RUNTIME_TARGETS "default;x86_64-fuchsia;aarch64-fuchsia;x86_64-fuchsia-asan:x86_64-fuchsia;aarch64-fuchsia-asan:aarch64-fuchsia" CACHE STRING "")
 
 # Set the default target runtimes options.
-set(LIBUNWIND_ENABLE_SHARED OFF CACHE BOOL "")
-set(LIBUNWIND_USE_COMPILER_RT ON CACHE BOOL "")
-set(LIBUNWIND_INSTALL_LIBRARY OFF CACHE BOOL "")
-set(LIBCXXABI_USE_COMPILER_RT ON CACHE BOOL "")
-set(LIBCXXABI_ENABLE_SHARED OFF CACHE BOOL "")
-set(LIBCXXABI_USE_LLVM_UNWINDER ON CACHE BOOL "")
-set(LIBCXXABI_ENABLE_STATIC_UNWINDER ON CACHE BOOL "")
-set(LIBCXXABI_INSTALL_LIBRARY OFF CACHE BOOL "")
-set(LIBCXX_USE_COMPILER_RT ON CACHE BOOL "")
-set(LIBCXX_ENABLE_SHARED OFF CACHE BOOL "")
-set(LIBCXX_ENABLE_STATIC_ABI_LIBRARY ON CACHE BOOL "")
+if(APPLE)
+  # Disable installing libc++, libc++abi or libunwind on Darwin, since Clang
+  # driver doesn't know how to use libraries that are part of the toolchain.
+  set(LIBUNWIND_INSTALL_HEADERS OFF CACHE BOOL "")
+  set(LIBUNWIND_INSTALL_LIBRARY OFF CACHE BOOL "")
+  set(LIBCXXABI_INSTALL_HEADERS OFF CACHE BOOL "")
+  set(LIBCXXABI_INSTALL_LIBRARY OFF CACHE BOOL "")
+  set(LIBCXX_INSTALL_HEADERS OFF CACHE BOOL "")
+  set(LIBCXX_INSTALL_LIBRARY OFF CACHE BOOL "")
+else()
+  set(LIBUNWIND_ENABLE_SHARED OFF CACHE BOOL "")
+  set(LIBUNWIND_USE_COMPILER_RT ON CACHE BOOL "")
+  set(LIBUNWIND_INSTALL_LIBRARY OFF CACHE BOOL "")
+  set(LIBCXXABI_USE_COMPILER_RT ON CACHE BOOL "")
+  set(LIBCXXABI_ENABLE_SHARED OFF CACHE BOOL "")
+  set(LIBCXXABI_USE_LLVM_UNWINDER ON CACHE BOOL "")
+  set(LIBCXXABI_ENABLE_STATIC_UNWINDER ON CACHE BOOL "")
+  set(LIBCXXABI_INSTALL_LIBRARY OFF CACHE BOOL "")
+  set(LIBCXX_USE_COMPILER_RT ON CACHE BOOL "")
+  set(LIBCXX_ENABLE_SHARED OFF CACHE BOOL "")
+  set(LIBCXX_ENABLE_STATIC_ABI_LIBRARY ON CACHE BOOL "")
+endif()
 
 # Set the per-target runtimes options.
 foreach(target x86_64;aarch64)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44559: [Sema] Wrong width of result of mul operation

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In https://reviews.llvm.org/D44559#1044186, @avt77 wrote:

> >> In https://reviews.llvm.org/D44559#1040799, @rjmccall wrote:
> >> 
> >>> I think we're correct not to warn here and that GCC/ICC are being noisy.  
> >>> The existence of a temporary promotion to a wider type doesn't justify 
> >>> warning on arithmetic between two operands that are the same size as the 
> >>> ultimate result.  It is totally fair for users to think of this operation 
> >>> as being "closed" on the original type.
> >> 
> >> 
> >> Could you please clarify, are you saying that PR35409 
> >>  is not a bug, and clang 
> >> should continue to not warn in those cases?
> > 
> > Correct.
>
> Does it mean we should abandon this revision? On the other hand it's a real 
> bug, isn't it?


Not as I see it, no.


https://reviews.llvm.org/D44559



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

[PATCH] D44720: [clangd] Simplify fuzzy matcher (sequence alignment) by removing some condition checks.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay updated this revision to Diff 139313.
MaskRay added a comment.

Update summary


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44720

Files:
  clangd/FuzzyMatch.cpp
  clangd/FuzzyMatch.h

Index: clangd/FuzzyMatch.h
===
--- clangd/FuzzyMatch.h
+++ clangd/FuzzyMatch.h
@@ -58,8 +58,8 @@
   void buildGraph();
   void calculateRoles(const char *Text, CharRole *Out, int &Types, int N);
   bool allowMatch(int P, int W) const;
-  int skipPenalty(int W, Action Last) const;
-  int matchBonus(int P, int W, Action Last) const;
+  int missScore(int W, Action Last) const;
+  int matchScore(int P, int W, Action Last) const;
 
   // Pattern data is initialized by the constructor, then constant.
   char Pat[MaxPat]; // Pattern data
Index: clangd/FuzzyMatch.cpp
===
--- clangd/FuzzyMatch.cpp
+++ clangd/FuzzyMatch.cpp
@@ -58,6 +58,7 @@
 
 #include "FuzzyMatch.h"
 #include "llvm/ADT/Optional.h"
+#include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/Format.h"
 
 namespace clang {
@@ -67,7 +68,6 @@
 constexpr int FuzzyMatcher::MaxPat;
 constexpr int FuzzyMatcher::MaxWord;
 
-static char lower(char C) { return C >= 'A' && C <= 'Z' ? C + ('a' - 'A') : C; }
 // A "negative infinity" score that won't overflow.
 // We use this to mark unreachable states and forbidden solutions.
 // Score field is 15 bits wide, min value is -2^14, we use half of that.
@@ -80,25 +80,17 @@
   ScoreScale(PatN ? float{1} / (PerfectBonus * PatN) : 0), WordN(0) {
   std::copy(Pattern.begin(), Pattern.begin() + PatN, Pat);
   for (int I = 0; I < PatN; ++I)
-LowPat[I] = lower(Pat[I]);
-  Scores[0][0][Miss] = {0, Miss};
-  Scores[0][0][Match] = {AwfulScore, Miss};
-  for (int P = 0; P <= PatN; ++P)
-for (int W = 0; W < P; ++W)
-  for (Action A : {Miss, Match})
-Scores[P][W][A] = {AwfulScore, Miss};
-  if (PatN > 0)
-calculateRoles(Pat, PatRole, PatTypeSet, PatN);
+LowPat[I] = toLower(Pat[I]);
+  calculateRoles(Pat, PatRole, PatTypeSet, PatN);
 }
 
 Optional FuzzyMatcher::match(StringRef Word) {
   if (!(WordContainsPattern = init(Word)))
 return None;
-  if (!PatN)
-return 1;
   buildGraph();
-  auto Best = std::max(Scores[PatN][WordN][Miss].Score,
-   Scores[PatN][WordN][Match].Score);
+  int Best = AwfulScore;
+  for (int I = PatN; I <= WordN; I++)
+Best = std::max(Best, Scores[PatN][I][Match].Score);
   if (isAwful(Best))
 return None;
   return ScoreScale * std::min(PerfectBonus * PatN, std::max(0, Best));
@@ -179,7 +171,8 @@
 }
 void FuzzyMatcher::calculateRoles(const char *Text, CharRole *Out, int &TypeSet,
   int N) {
-  assert(N > 0);
+  if (!N)
+return;
   CharType Type = packedLookup(CharTypes, Text[0]);
   TypeSet = 1 << Type;
   // Types holds a sliding window of (Prev, Curr, Next) types.
@@ -206,10 +199,8 @@
   if (PatN > WordN)
 return false;
   std::copy(NewWord.begin(), NewWord.begin() + WordN, Word);
-  if (PatN == 0)
-return true;
   for (int I = 0; I < WordN; ++I)
-LowWord[I] = lower(Word[I]);
+LowWord[I] = toLower(Word[I]);
 
   // Cheap subsequence check.
   for (int W = 0, P = 0; P != PatN; ++W) {
@@ -236,31 +227,29 @@
 // This range is not strict: we can apply larger bonuses/penalties, or penalize
 // non-matched characters.
 void FuzzyMatcher::buildGraph() {
+  Scores[0][0][Miss] = Scores[0][0][Match] = {0, Miss};
   for (int W = 0; W < WordN; ++W) {
-Scores[0][W + 1][Miss] = {Scores[0][W][Miss].Score - skipPenalty(W, Miss),
+Scores[0][W + 1][Miss] = {Scores[0][W][Miss].Score + missScore(W, Miss),
   Miss};
 Scores[0][W + 1][Match] = {AwfulScore, Miss};
   }
   for (int P = 0; P < PatN; ++P) {
+Scores[P + 1][P][Miss] = Scores[P + 1][P][Match] = {AwfulScore, Miss};
 for (int W = P; W < WordN; ++W) {
   auto &Score = Scores[P + 1][W + 1], &PreMiss = Scores[P + 1][W];
 
-  auto MatchMissScore = PreMiss[Match].Score;
-  auto MissMissScore = PreMiss[Miss].Score;
-  if (P < PatN - 1) { // Skipping trailing characters is always free.
-MatchMissScore -= skipPenalty(W, Match);
-MissMissScore -= skipPenalty(W, Miss);
-  }
+  auto MatchMissScore = PreMiss[Match].Score + missScore(W, Match);
+  auto MissMissScore = PreMiss[Miss].Score + missScore(W, Miss);
   Score[Miss] = (MatchMissScore > MissMissScore)
 ? ScoreInfo{MatchMissScore, Match}
 : ScoreInfo{MissMissScore, Miss};
 
   if (!allowMatch(P, W)) {
 Score[Match] = {AwfulScore, Miss};
   } else {
 auto &PreMatch = Scores[P][W];
-auto MatchMatchScore = PreMatch[Match].Score + matchBonus(P, W, Match);
-auto MissMatchScore = PreMatch[Miss].Score + matchBonus(P, W, Miss);
+auto MatchMatchScore = PreMatch[Match].Score + matchSco

[PATCH] D44720: [clangd] Simplify fuzzy matcher (sequence alignment) by removing some condition checks.

[Fangrui Song via Phabricator via cfe-commits]

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

Update summary


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44720

Files:
  clangd/FuzzyMatch.cpp
  clangd/FuzzyMatch.h

Index: clangd/FuzzyMatch.h
===
--- clangd/FuzzyMatch.h
+++ clangd/FuzzyMatch.h
@@ -58,8 +58,8 @@
   void buildGraph();
   void calculateRoles(const char *Text, CharRole *Out, int &Types, int N);
   bool allowMatch(int P, int W) const;
-  int skipPenalty(int W, Action Last) const;
-  int matchBonus(int P, int W, Action Last) const;
+  int missScore(int W, Action Last) const;
+  int matchScore(int P, int W, Action Last) const;
 
   // Pattern data is initialized by the constructor, then constant.
   char Pat[MaxPat]; // Pattern data
Index: clangd/FuzzyMatch.cpp
===
--- clangd/FuzzyMatch.cpp
+++ clangd/FuzzyMatch.cpp
@@ -58,6 +58,7 @@
 
 #include "FuzzyMatch.h"
 #include "llvm/ADT/Optional.h"
+#include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/Format.h"
 
 namespace clang {
@@ -67,7 +68,6 @@
 constexpr int FuzzyMatcher::MaxPat;
 constexpr int FuzzyMatcher::MaxWord;
 
-static char lower(char C) { return C >= 'A' && C <= 'Z' ? C + ('a' - 'A') : C; }
 // A "negative infinity" score that won't overflow.
 // We use this to mark unreachable states and forbidden solutions.
 // Score field is 15 bits wide, min value is -2^14, we use half of that.
@@ -80,25 +80,17 @@
   ScoreScale(PatN ? float{1} / (PerfectBonus * PatN) : 0), WordN(0) {
   std::copy(Pattern.begin(), Pattern.begin() + PatN, Pat);
   for (int I = 0; I < PatN; ++I)
-LowPat[I] = lower(Pat[I]);
-  Scores[0][0][Miss] = {0, Miss};
-  Scores[0][0][Match] = {AwfulScore, Miss};
-  for (int P = 0; P <= PatN; ++P)
-for (int W = 0; W < P; ++W)
-  for (Action A : {Miss, Match})
-Scores[P][W][A] = {AwfulScore, Miss};
-  if (PatN > 0)
-calculateRoles(Pat, PatRole, PatTypeSet, PatN);
+LowPat[I] = toLower(Pat[I]);
+  calculateRoles(Pat, PatRole, PatTypeSet, PatN);
 }
 
 Optional FuzzyMatcher::match(StringRef Word) {
   if (!(WordContainsPattern = init(Word)))
 return None;
-  if (!PatN)
-return 1;
   buildGraph();
-  auto Best = std::max(Scores[PatN][WordN][Miss].Score,
-   Scores[PatN][WordN][Match].Score);
+  int Best = AwfulScore;
+  for (int I = PatN; I <= WordN; I++)
+Best = std::max(Best, Scores[PatN][I][Match].Score);
   if (isAwful(Best))
 return None;
   return ScoreScale * std::min(PerfectBonus * PatN, std::max(0, Best));
@@ -179,7 +171,8 @@
 }
 void FuzzyMatcher::calculateRoles(const char *Text, CharRole *Out, int &TypeSet,
   int N) {
-  assert(N > 0);
+  if (!N)
+return;
   CharType Type = packedLookup(CharTypes, Text[0]);
   TypeSet = 1 << Type;
   // Types holds a sliding window of (Prev, Curr, Next) types.
@@ -206,10 +199,8 @@
   if (PatN > WordN)
 return false;
   std::copy(NewWord.begin(), NewWord.begin() + WordN, Word);
-  if (PatN == 0)
-return true;
   for (int I = 0; I < WordN; ++I)
-LowWord[I] = lower(Word[I]);
+LowWord[I] = toLower(Word[I]);
 
   // Cheap subsequence check.
   for (int W = 0, P = 0; P != PatN; ++W) {
@@ -236,31 +227,29 @@
 // This range is not strict: we can apply larger bonuses/penalties, or penalize
 // non-matched characters.
 void FuzzyMatcher::buildGraph() {
+  Scores[0][0][Miss] = Scores[0][0][Match] = {0, Miss};
   for (int W = 0; W < WordN; ++W) {
-Scores[0][W + 1][Miss] = {Scores[0][W][Miss].Score - skipPenalty(W, Miss),
+Scores[0][W + 1][Miss] = {Scores[0][W][Miss].Score + missScore(W, Miss),
   Miss};
 Scores[0][W + 1][Match] = {AwfulScore, Miss};
   }
   for (int P = 0; P < PatN; ++P) {
+Scores[P + 1][P][Miss] = Scores[P + 1][P][Match] = {AwfulScore, Miss};
 for (int W = P; W < WordN; ++W) {
   auto &Score = Scores[P + 1][W + 1], &PreMiss = Scores[P + 1][W];
 
-  auto MatchMissScore = PreMiss[Match].Score;
-  auto MissMissScore = PreMiss[Miss].Score;
-  if (P < PatN - 1) { // Skipping trailing characters is always free.
-MatchMissScore -= skipPenalty(W, Match);
-MissMissScore -= skipPenalty(W, Miss);
-  }
+  auto MatchMissScore = PreMiss[Match].Score + missScore(W, Match);
+  auto MissMissScore = PreMiss[Miss].Score + missScore(W, Miss);
   Score[Miss] = (MatchMissScore > MissMissScore)
 ? ScoreInfo{MatchMissScore, Match}
 : ScoreInfo{MissMissScore, Miss};
 
   if (!allowMatch(P, W)) {
 Score[Match] = {AwfulScore, Miss};
   } else {
 auto &PreMatch = Scores[P][W];
-auto MatchMatchScore = PreMatch[Match].Score + matchBonus(P, W, Match);
-auto MissMatchScore = PreMatch[Miss].Score + matchBonus(P, W, Miss);
+auto Match

[PATCH] D44559: [Sema] Wrong width of result of mul operation

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D44559#1044639, @rjmccall wrote:

> In https://reviews.llvm.org/D44559#1044186, @avt77 wrote:
>
> > >> In https://reviews.llvm.org/D44559#1040799, @rjmccall wrote:
> > >> 
> > >>> I think we're correct not to warn here and that GCC/ICC are being 
> > >>> noisy.  The existence of a temporary promotion to a wider type doesn't 
> > >>> justify warning on arithmetic between two operands that are the same 
> > >>> size as the ultimate result.  It is totally fair for users to think of 
> > >>> this operation as being "closed" on the original type.
> > >> 
> > >> 
> > >> Could you please clarify, are you saying that PR35409 
> > >>  is not a bug, and clang 
> > >> should continue to not warn in those cases?
> > > 
> > > Correct.
> >
> > Does it mean we should abandon this revision? On the other hand it's a real 
> > bug, isn't it?
>
>
> Not as I see it, no.


Do you see this code as having a bug when `a` is >= 182?

  short foo(unsigned char a) {
return a * a;
  }

(If you don't like seeing `unsigned char`  you can imagine it was spelled as 
`uint8_t`.)


https://reviews.llvm.org/D44559



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

[PATCH] D44720: [clangd] Simplify fuzzy matcher (sequence alignment) by removing some condition checks.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clangd/FuzzyMatch.cpp:93
+  for (int I = PatN; I <= WordN; I++)
+Best = std::max(Best, Scores[PatN][I][Match].Score);
   if (isAwful(Best))

sammccall wrote:
> this looks like a behavior change - why?
This is a behavior change. Instead of choosing between `Match/Miss` in the last 
position, we enumerate the last matching position in `Word`.

This saves `if (P < PatN - 1) {` check in the main loop at the cost of a for 
loop here (use sites of ending values)



Comment at: clangd/FuzzyMatch.cpp:241
 
-  auto MatchMissScore = PreMiss[Match].Score;
-  auto MissMissScore = PreMiss[Miss].Score;
-  if (P < PatN - 1) { // Skipping trailing characters is always free.
-MatchMissScore -= skipPenalty(W, Match);
-MissMissScore -= skipPenalty(W, Miss);
-  }
+  auto MatchMissScore = PreMiss[Match].Score + missScore(W, Match);
+  auto MissMissScore = PreMiss[Miss].Score + missScore(W, Miss);

sammccall wrote:
> adding the penalty unconditionally seems like a behavior change, why?
Because now we use a different method to calculate the final value. I believe 
this makes the loop simpler.

This was not regular because 

Scores[0][W + 1][Miss] = {Scores[0][W][Miss].Score + missScore(W, Miss),
  Miss};

This unconditionally added a trailing penalty but the main loop did not.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44720



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

[PATCH] D44720: [clangd] Simplify fuzzy matcher (sequence alignment) by removing some condition checks.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clangd/FuzzyMatch.h:61
   bool allowMatch(int P, int W) const;
-  int skipPenalty(int W, Action Last) const;
-  int matchBonus(int P, int W, Action Last) const;
+  int missScore(int W, Action Last) const;
+  int matchScore(int P, int W, Action Last) const;

sammccall wrote:
> FWIW, I don't think this is an improvement - I think the clarity of purpose 
> in names is more useful than having consistent signs in this case.
Keep `matchBonus` but rename `skipPenalty` to `missPenalty` ?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44720



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

[PATCH] D44720: [clangd] Simplify fuzzy matcher (sequence alignment) by removing some condition checks.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clangd/FuzzyMatch.cpp:96
 return None;
   return ScoreScale * std::min(PerfectBonus * PatN, std::max(0, Best));
 }

I also don't understand why it clamps the value to zero here. Negative values 
are also meaningful to me. Given that perfectBonus is only 3 it is very easy to 
get a negative value here.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44720



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

[clang-tools-extra] r328115 - [clang-tidy] Use :doc: for check links in Release Notes.

[Eugene Zelenko via cfe-commits]

Author: eugenezelenko
Date: Wed Mar 21 10:06:13 2018
New Revision: 328115

URL: http://llvm.org/viewvc/llvm-project?rev=328115&view=rev
Log:
[clang-tidy] Use :doc: for check links in Release Notes.

Differential revision: https://reviews.llvm.org/D44694

Modified:
clang-tools-extra/trunk/clang-tidy/add_new_check.py
clang-tools-extra/trunk/clang-tidy/rename_check.py
clang-tools-extra/trunk/docs/ReleaseNotes.rst

Modified: clang-tools-extra/trunk/clang-tidy/add_new_check.py
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/add_new_check.py?rev=328115&r1=328114&r2=328115&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/add_new_check.py (original)
+++ clang-tools-extra/trunk/clang-tidy/add_new_check.py Wed Mar 21 10:06:13 2018
@@ -211,8 +211,8 @@ def add_release_notes(module_path, modul
 elif header_found:
   if not line.startswith(''):
 f.write("""
-- New `%s
-  `_ check
+- New :doc:`%s
+  ` check
 
   FIXME: add release notes.
 """ % (check_name_dashes, check_name_dashes))

Modified: clang-tools-extra/trunk/clang-tidy/rename_check.py
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/rename_check.py?rev=328115&r1=328114&r2=328115&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/rename_check.py (original)
+++ clang-tools-extra/trunk/clang-tidy/rename_check.py Wed Mar 21 10:06:13 2018
@@ -171,8 +171,8 @@ def add_release_notes(clang_tidy_path, o
 elif header_found:
   if not line.startswith(''):
 f.write("""
-- The '%s' check was renamed to `%s
-  `_
+- The '%s' check was renamed to :doc:`%s
+  `
 """ % (old_check_name, new_check_name, new_check_name))
 note_added = True
 

Modified: clang-tools-extra/trunk/docs/ReleaseNotes.rst
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/ReleaseNotes.rst?rev=328115&r1=328114&r2=328115&view=diff
==
--- clang-tools-extra/trunk/docs/ReleaseNotes.rst (original)
+++ clang-tools-extra/trunk/docs/ReleaseNotes.rst Wed Mar 21 10:06:13 2018
@@ -57,136 +57,136 @@ The improvements are...
 Improvements to clang-tidy
 --
 
+- New module `abseil` for checks related to the `Abseil `_
+  library.
+
 - New module ``portability``.
 
 - New module ``zircon`` for checks related to Fuchsia's Zircon kernel.
 
-- New `bugprone-throw-keyword-missing
-  
`_
 check
+- New :doc:`bugprone-throw-keyword-missing
+  ` check
 
   Diagnoses when a temporary object that appears to be an exception is
   constructed but not thrown.
 
-- New `bugprone-unused-return-value
-  
`_
 check
+- New :doc:`bugprone-unused-return-value
+  ` check
 
   Warns on unused function return values.
 
-- New `cppcoreguidelines-avoid-goto
-  
`_
 check
+- New :doc:`cppcoreguidelines-avoid-goto
+  ` check
 
   The usage of ``goto`` for control flow is error prone and should be replaced
   with looping constructs. Every backward jump is rejected. Forward jumps are
   only allowed in nested loops.
 
-- New `fuchsia-multiple-inheritance
-  
`_
 check
+- New :doc:`fuchsia-multiple-inheritance
+  ` check
 
   Warns if a class inherits from multiple classes that are not pure virtual.
 
-- New `abseil` module for checks related to the `Abseil `_
-  library.
-
-- New `abseil-string-find-startswith
-  
`_
 check
+- New :doc:`abseil-string-find-startswith
+  ` check
 
   Checks whether a ``std::string::find()`` result is compared with 0, and
   suggests replacing with ``absl::StartsWith()``.
 
-- New `fuchsia-statically-constructed-objects
-  
`_
 check
+- New :doc:`fuchsia-statically-constructed-objects
+  ` check
 
   Warns if global, non-trivial objects with static storage are constructed,
   unless the object is statically initialized with a ``constexpr`` constructor
   or has no explicit constructor.
 
-- New `fuchsia-trailing-return
-  
`_ 
check
+- New :doc:`fuchsia-trailing-return
+  ` check
 
   Functions that have trailing returns are disallowed, except for those
   using ``decltype`` specifiers and lambda w

[PATCH] D44694: [clang-tidy] Use :doc: for check links in Release Notes

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE328115: [clang-tidy] Use :doc: for check links in Release 
Notes. (authored by eugenezelenko, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D44694?vs=139167&id=139320#toc

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44694

Files:
  clang-tidy/add_new_check.py
  clang-tidy/rename_check.py
  docs/ReleaseNotes.rst

Index: clang-tidy/add_new_check.py
===
--- clang-tidy/add_new_check.py
+++ clang-tidy/add_new_check.py
@@ -211,8 +211,8 @@
 elif header_found:
   if not line.startswith(''):
 f.write("""
-- New `%s
-  `_ check
+- New :doc:`%s
+  ` check
 
   FIXME: add release notes.
 """ % (check_name_dashes, check_name_dashes))
Index: clang-tidy/rename_check.py
===
--- clang-tidy/rename_check.py
+++ clang-tidy/rename_check.py
@@ -171,8 +171,8 @@
 elif header_found:
   if not line.startswith(''):
 f.write("""
-- The '%s' check was renamed to `%s
-  `_
+- The '%s' check was renamed to :doc:`%s
+  `
 """ % (old_check_name, new_check_name, new_check_name))
 note_added = True
 
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -57,136 +57,136 @@
 Improvements to clang-tidy
 --
 
+- New module `abseil` for checks related to the `Abseil `_
+  library.
+
 - New module ``portability``.
 
 - New module ``zircon`` for checks related to Fuchsia's Zircon kernel.
 
-- New `bugprone-throw-keyword-missing
-  `_ check
+- New :doc:`bugprone-throw-keyword-missing
+  ` check
 
   Diagnoses when a temporary object that appears to be an exception is
   constructed but not thrown.
 
-- New `bugprone-unused-return-value
-  `_ check
+- New :doc:`bugprone-unused-return-value
+  ` check
 
   Warns on unused function return values.
 
-- New `cppcoreguidelines-avoid-goto
-  `_ check
+- New :doc:`cppcoreguidelines-avoid-goto
+  ` check
 
   The usage of ``goto`` for control flow is error prone and should be replaced
   with looping constructs. Every backward jump is rejected. Forward jumps are
   only allowed in nested loops.
 
-- New `fuchsia-multiple-inheritance
-  `_ check
+- New :doc:`fuchsia-multiple-inheritance
+  ` check
 
   Warns if a class inherits from multiple classes that are not pure virtual.
 
-- New `abseil` module for checks related to the `Abseil `_
-  library.
-
-- New `abseil-string-find-startswith
-  `_ check
+- New :doc:`abseil-string-find-startswith
+  ` check
 
   Checks whether a ``std::string::find()`` result is compared with 0, and
   suggests replacing with ``absl::StartsWith()``.
 
-- New `fuchsia-statically-constructed-objects
-  `_ check
+- New :doc:`fuchsia-statically-constructed-objects
+  ` check
 
   Warns if global, non-trivial objects with static storage are constructed,
   unless the object is statically initialized with a ``constexpr`` constructor
   or has no explicit constructor.
 
-- New `fuchsia-trailing-return
-  `_ check
+- New :doc:`fuchsia-trailing-return
+  ` check
 
   Functions that have trailing returns are disallowed, except for those
   using ``decltype`` specifiers and lambda with otherwise unutterable
   return types.
 
-- New `hicpp-multiway-paths-covered
-  `_ check
+- New :doc:`hicpp-multiway-paths-covered
+  ` check
 
   Checks on ``switch`` and ``if`` - ``else if`` constructs that do not cover all possible code paths.
 
-- New `modernize-use-uncaught-exceptions
-  `_ check
+- New :doc:`modernize-use-uncaught-exceptions
+  ` check
 
   Finds and replaces deprecated uses of ``std::uncaught_exception`` to
   ``std::uncaught_exceptions``.
 
-- New `portability-simd-intrinsics
-  `_ check
+- New :doc:`portability-simd-intrinsics
+  ` check
 
   Warns or suggests alternatives if 

[PATCH] D44272: [clangd] Support incremental document syncing

[Simon Marchi via Phabricator via cfe-commits]

simark updated this revision to Diff 139321.
simark added a comment.

Remove draft if update fails


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44272

Files:
  clangd/ClangdLSPServer.cpp
  clangd/DraftStore.cpp
  clangd/DraftStore.h
  clangd/Protocol.cpp
  clangd/Protocol.h
  test/clangd/initialize-params-invalid.test
  test/clangd/initialize-params.test
  unittests/clangd/CMakeLists.txt
  unittests/clangd/DraftStoreTests.cpp

Index: unittests/clangd/DraftStoreTests.cpp
===
--- /dev/null
+++ unittests/clangd/DraftStoreTests.cpp
@@ -0,0 +1,355 @@
+//===-- DraftStoreTests.cpp - Clangd unit tests -*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "Annotations.h"
+#include "DraftStore.h"
+#include "SourceCode.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+using namespace llvm;
+
+struct IncrementalTestStep {
+  StringRef Src;
+  StringRef Contents;
+};
+
+static int rangeLength(StringRef Code, const Range &Rng) {
+  llvm::Expected Start = positionToOffset(Code, Rng.start);
+  llvm::Expected End = positionToOffset(Code, Rng.end);
+  assert(Start);
+  assert(End);
+  return *End - *Start;
+}
+
+// Send the changes one by one to updateDraft, verify the intermediate results.
+
+static void stepByStep(llvm::ArrayRef Steps) {
+  DraftStore DS;
+  Annotations InitialSrc(Steps.front().Src);
+  const char Path[] = "/hello.cpp";
+
+  // Set the initial content.
+  DS.addDraft(Path, InitialSrc.code());
+
+  for (size_t i = 1; i < Steps.size(); i++) {
+Annotations SrcBefore(Steps[i - 1].Src);
+Annotations SrcAfter(Steps[i].Src);
+StringRef Contents = Steps[i - 1].Contents;
+TextDocumentContentChangeEvent Event{
+SrcBefore.range(),
+rangeLength(SrcBefore.code(), SrcBefore.range()),
+Contents.str(),
+};
+
+llvm::Expected Result = DS.updateDraft(Path, {Event});
+EXPECT_TRUE(!!Result);
+EXPECT_EQ(*Result, SrcAfter.code());
+EXPECT_EQ(*DS.getDraft(Path), SrcAfter.code());
+  }
+}
+
+// Send all the changes at once to updateDraft, check only the final result.
+
+static void allAtOnce(llvm::ArrayRef Steps) {
+  DraftStore DS;
+  Annotations InitialSrc(Steps.front().Src);
+  Annotations FinalSrc(Steps.back().Src);
+  const char Path[] = "/hello.cpp";
+  std::vector Changes;
+
+  for (size_t i = 0; i < Steps.size() - 1; i++) {
+Annotations Src(Steps[i].Src);
+StringRef Contents = Steps[i].Contents;
+
+Changes.push_back({
+Src.range(),
+rangeLength(Src.code(), Src.range()),
+Contents.str(),
+});
+  }
+
+  // Set the initial content.
+  DS.addDraft(Path, InitialSrc.code());
+
+  llvm::Expected Result = DS.updateDraft(Path, Changes);
+
+  EXPECT_TRUE(!!Result) << llvm::toString(Result.takeError());
+  EXPECT_EQ(*Result, FinalSrc.code());
+  EXPECT_EQ(*DS.getDraft(Path), FinalSrc.code());
+}
+
+TEST(DraftStoreIncrementalUpdateTest, Simple) {
+  // clang-format off
+  IncrementalTestStep Steps[] =
+{
+  // Replace a range
+  {
+R"cpp(static int
+hello[[World]]()
+{})cpp",
+"Universe"
+  },
+  // Delete a range
+  {
+R"cpp(static int
+hello[[Universe]]()
+{})cpp",
+""
+  },
+  // Add a range
+  {
+R"cpp(static int
+hello[[]]()
+{})cpp",
+"Monde"
+  },
+  {
+R"cpp(static int
+helloMonde()
+{})cpp",
+""
+  }
+};
+  // clang-format on
+
+  stepByStep(Steps);
+  allAtOnce(Steps);
+}
+
+TEST(DraftStoreIncrementalUpdateTest, MultiLine) {
+  // clang-format off
+  IncrementalTestStep Steps[] =
+{
+  // Replace a range
+  {
+R"cpp(static [[int
+helloWorld]]()
+{})cpp",
+R"cpp(char
+welcome)cpp"
+  },
+  // Delete a range
+  {
+R"cpp(static char[[
+welcome]]()
+{})cpp",
+""
+  },
+  // Add a range
+  {
+R"cpp(static char[[]]()
+{})cpp",
+R"cpp(
+cookies)cpp"
+  },
+  // Replace the whole file
+  {
+R"cpp([[static char
+cookies()
+{}]])cpp",
+R"cpp(#include 
+)cpp"
+  },
+  // Delete the whole file
+  {
+R"cpp([[#include 
+]])cpp",
+"",
+  },
+  // Add something to an empty file
+  {
+"[[]]",
+R"cpp(int main() {
+)cpp",
+  },
+  {
+R"cpp(int main() {
+)cpp",
+""
+  }
+};
+  // clang-format on
+
+  stepByStep(Steps);
+  allAtOnce(Steps);
+}
+
+TEST(DraftStoreIncrementalUpdateTest, WrongRangeLength) {
+  DraftStore DS;
+  Path File = "foo.cpp";
+
+  DS.addDraft(File, "int main() {}\n");
+
+  TextDocumentContentChangeEvent Change;
+  Change.range.emplace();
+  Change.range->start.line = 0;
+  Change.range->st

[PATCH] D42730: [clang-tidy]] Add check for use of types/classes/functions from header which are deprecated and removed in C++17

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added inline comments.



Comment at: clang-tidy/modernize/DeprecatedFunctionalCheck.cpp:48-54
+  } else if (const auto *const Call =
+ Result.Nodes.getNodeAs("ptr_fun_call")) {
+diag(Call->getLocStart(), Message) << "'std::ptr_fun'";
+  } else if (const auto *const Call =
+ Result.Nodes.getNodeAs("mem_fun_call")) {
+diag(Call->getLocStart(), Message) << "'std::mem_fun'";
+  }

aaron.ballman wrote:
> alexfh wrote:
> > aaron.ballman wrote:
> > > alexfh wrote:
> > > > aaron.ballman wrote:
> > > > > alexfh wrote:
> > > > > > aaron.ballman wrote:
> > > > > > > alexfh wrote:
> > > > > > > > alexfh wrote:
> > > > > > > > > aaron.ballman wrote:
> > > > > > > > > > massberg wrote:
> > > > > > > > > > > aaron.ballman wrote:
> > > > > > > > > > > > I think that this code should be generalized (same with 
> > > > > > > > > > > > the matchers) so that you match on `hasAnyName()` for 
> > > > > > > > > > > > the function calls and use `CallExpr::getCalleeDecl()` 
> > > > > > > > > > > > to get the declaration. e.g.,
> > > > > > > > > > > > ```
> > > > > > > > > > > > if (const auto *Call = 
> > > > > > > > > > > > Result.Nodes.getNodeAs("blech")) {
> > > > > > > > > > > >   if (const Decl *Callee = Call->getCalleeDecl())
> > > > > > > > > > > > diag(Call->getLocStart(), Message) << Calleee;
> > > > > > > > > > > > }
> > > > > > > > > > > > ```
> > > > > > > > > > > > This way you can add more named without having to add 
> > > > > > > > > > > > extra logic for the diagnostics.
> > > > > > > > > > > I generalized the code and the matcher.
> > > > > > > > > > > When we use
> > > > > > > > > > > ```
> > > > > > > > > > > << cast(Callee);
> > > > > > > > > > > ```
> > > > > > > > > > > we get the template arguments in the name , e.g. 
> > > > > > > > > > > `ptr_fun`, so I chose to use 
> > > > > > > > > > > `getQualifiedNameAsString`.
> > > > > > > > > > > If there is there a better way to get the function name 
> > > > > > > > > > > without template arguments I appreciate any suggestions.
> > > > > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > > Nope, in that case, your code is correct. However, we 
> > > > > > > > > > generally provide the template arguments in diagnostics. I 
> > > > > > > > > > see @alexfh was asking for them to be removed as not being 
> > > > > > > > > > useful, but I'm not certain I agree with his rationale. 
> > > > > > > > > > Yes, all instances are deprecated and thus the template 
> > > > > > > > > > arguments don't discern between good and bad cases, but 
> > > > > > > > > > showing the template arguments is also consistent with the 
> > > > > > > > > > other diagnostics we emit. For instance, other "deprecated" 
> > > > > > > > > > diagnostics also emit the template arguments. I'm not 
> > > > > > > > > > certain we should be inconsistent with the way we produce 
> > > > > > > > > > diagnostics, but I'll defer to Alex if he still feels 
> > > > > > > > > > strongly about leaving them off here.
> > > > > > > > > Indeed, -Wdeprecated-declarations warnings print template 
> > > > > > > > > arguments. Moreover, they seem to be issued only on 
> > > > > > > > > instantiations, see https://godbolt.org/g/W563gw.
> > > > > > > > > 
> > > > > > > > > But I have a number of concerns with template arguments in 
> > > > > > > > > the deprecation warnings:
> > > > > > > > > 
> > > > > > > > > 1. The note attached to the warning lies. Consider a warning 
> > > > > > > > > from the test above:
> > > > > > > > >   ...
> > > > > > > > >   :11:1: warning: 'B' is deprecated: bbb 
> > > > > > > > > [-Wdeprecated-declarations]
> > > > > > > > >   B e;
> > > > > > > > >   ^
> > > > > > > > >   :7:10: note: 'B' has been explicitly marked 
> > > > > > > > > deprecated here
> > > > > > > > >   struct [[deprecated("bbb")]] B {};
> > > > > > > > > 
> > > > > > > > >  But `B` hasn't been explicitly marked deprecated, only 
> > > > > > > > > the template definition of `B` has been. Template arguments 
> > > > > > > > > are important in the case of the explicit template 
> > > > > > > > > specialization `A` in the same example, but not in cases 
> > > > > > > > > where the template definition was marked deprecated, since 
> > > > > > > > > template arguments only add noise and no useful information 
> > > > > > > > > there.
> > > > > > > > > 2. Warnings can easily become unreadable: 
> > > > > > > > > https://godbolt.org/g/AFdznH
> > > > > > > > > 3. Certain coding patterns can result in numerous deprecation 
> > > > > > > > > warnings differing only in template arguments: 
> > > > > > > > > https://godbolt.org/g/U2JCbG. Clang-tidy can deduplicate 
> > > > > > > > > warnings, if they have identical text and location, but 
> > > > > > > > > adding template arguments to the message will prevent 
> > > > > > > > > deduplication. I've seen a case where thousands of 
> > > > > > > > > deprecation warnings were generated for a single line 

[PATCH] D44743: [clang-tidy] Marking hicpp-no-assembler-msvc unsupported on Windows

[Julie Hockett via Phabricator via cfe-commits]

juliehockett created this revision.
juliehockett added reviewers: zturner, phosek.
juliehockett added a project: clang-tools-extra.
Herald added a subscriber: xazax.hun.

After changes to lit.site.cfg.in, the test is now running (and failing) on 
windows, so temporarily marking it unsupported. See PR36855 for more details.


https://reviews.llvm.org/D44743

Files:
  test/clang-tidy/hicpp-no-assembler-msvc.cpp


Index: test/clang-tidy/hicpp-no-assembler-msvc.cpp
===
--- test/clang-tidy/hicpp-no-assembler-msvc.cpp
+++ test/clang-tidy/hicpp-no-assembler-msvc.cpp
@@ -1,4 +1,6 @@
 // REQUIRES: system-windows
+// FIXME: Re-enable test on windows (PR36855)
+// UNSUPPORTED: system-windows
 // RUN: %check_clang_tidy %s hicpp-no-assembler %t
 
 void f() {


Index: test/clang-tidy/hicpp-no-assembler-msvc.cpp
===
--- test/clang-tidy/hicpp-no-assembler-msvc.cpp
+++ test/clang-tidy/hicpp-no-assembler-msvc.cpp
@@ -1,4 +1,6 @@
 // REQUIRES: system-windows
+// FIXME: Re-enable test on windows (PR36855)
+// UNSUPPORTED: system-windows
 // RUN: %check_clang_tidy %s hicpp-no-assembler %t
 
 void f() {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44745: [HWASan] Port HWASan to Linux x86-64 (clang)

[Aleksey Shlyapnikov via Phabricator via cfe-commits]

alekseyshl created this revision.
alekseyshl added a reviewer: eugenis.
Herald added a subscriber: cryptoad.

Porting HWASan to Linux x86-64, the third of the three patches, clang part.


Repository:
  rC Clang

https://reviews.llvm.org/D44745

Files:
  lib/Driver/SanitizerArgs.cpp
  lib/Driver/ToolChains/Linux.cpp
  test/Driver/Inputs/resource_dir/lib/linux/libclang_rt.hwasan-x86_64.a.syms
  test/Driver/asan.c
  test/Driver/fsanitize-blacklist.c
  test/Driver/fsanitize.c
  test/Driver/sanitizer-ld.c

Index: test/Driver/sanitizer-ld.c
===
--- test/Driver/sanitizer-ld.c
+++ test/Driver/sanitizer-ld.c
@@ -696,54 +696,94 @@
 // CHECK-SCUDO-ANDROID-STATIC: "-lpthread"
 
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
-// RUN: -target aarch64-unknown-linux -fuse-ld=ld -fsanitize=hwaddress \
+// RUN: -target x86_64-unknown-linux -fuse-ld=ld -fsanitize=hwaddress \
 // RUN: -resource-dir=%S/Inputs/resource_dir \
 // RUN: --sysroot=%S/Inputs/basic_linux_tree \
-// RUN:   | FileCheck --check-prefix=CHECK-HWASAN-LINUX %s
+// RUN:   | FileCheck --check-prefix=CHECK-HWASAN-X86-64-LINUX %s
 //
-// CHECK-HWASAN-LINUX: "{{(.*[^-.0-9A-Z_a-z])?}}ld{{(.exe)?}}"
-// CHECK-HWASAN-LINUX-NOT: "-lc"
-// CHECK-HWASAN-LINUX: libclang_rt.hwasan-aarch64.a"
-// CHECK-HWASAN-LINUX-NOT: "--export-dynamic"
-// CHECK-HWASAN-LINUX: "--dynamic-list={{.*}}libclang_rt.hwasan-aarch64.a.syms"
-// CHECK-HWASAN-LINUX-NOT: "--export-dynamic"
-// CHECK-HWASAN-LINUX: "-lpthread"
-// CHECK-HWASAN-LINUX: "-lrt"
-// CHECK-HWASAN-LINUX: "-ldl"
+// CHECK-HWASAN-X86-64-LINUX: "{{(.*[^-.0-9A-Z_a-z])?}}ld{{(.exe)?}}"
+// CHECK-HWASAN-X86-64-LINUX-NOT: "-lc"
+// CHECK-HWASAN-X86-64-LINUX: libclang_rt.hwasan-x86_64.a"
+// CHECK-HWASAN-X86-64-LINUX-NOT: "--export-dynamic"
+// CHECK-HWASAN-X86-64-LINUX: "--dynamic-list={{.*}}libclang_rt.hwasan-x86_64.a.syms"
+// CHECK-HWASAN-X86-64-LINUX-NOT: "--export-dynamic"
+// CHECK-HWASAN-X86-64-LINUX: "-lpthread"
+// CHECK-HWASAN-X86-64-LINUX: "-lrt"
+// CHECK-HWASAN-X86-64-LINUX: "-ldl"
 
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
-// RUN: -target aarch64-unknown-linux -fuse-ld=ld -fsanitize=hwaddress -shared-libsan \
-// RUN: -resource-dir=%S/Inputs/resource_dir \
+// RUN: -target x86_64-unknown-linux -fuse-ld=ld -fsanitize=hwaddress \
+// RUN: -shared-libsan -resource-dir=%S/Inputs/resource_dir \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-SHARED-HWASAN-X86-64-LINUX %s
+//
+// CHECK-SHARED-HWASAN-X86-64-LINUX: "{{(.*[^-.0-9A-Z_a-z])?}}ld{{(.exe)?}}"
+// CHECK-SHARED-HWASAN-X86-64-LINUX-NOT: "-lc"
+// CHECK-SHARED-HWASAN-X86-64-LINUX: libclang_rt.hwasan-x86_64.so"
+// CHECK-SHARED-HWASAN-X86-64-LINUX-NOT: "-lpthread"
+// CHECK-SHARED-HWASAN-X86-64-LINUX-NOT: "-lrt"
+// CHECK-SHARED-HWASAN-X86-64-LINUX-NOT: "-ldl"
+// CHECK-SHARED-HWASAN-X86-64-LINUX-NOT: "--export-dynamic"
+// CHECK-SHARED-HWASAN-X86-64-LINUX-NOT: "--dynamic-list"
+
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.so -shared 2>&1 \
+// RUN: -target x86_64-unknown-linux -fuse-ld=ld -fsanitize=hwaddress \
+// RUN: -shared-libsan -resource-dir=%S/Inputs/resource_dir \
 // RUN: --sysroot=%S/Inputs/basic_linux_tree \
-// RUN:   | FileCheck --check-prefix=CHECK-SHARED-HWASAN-LINUX %s
+// RUN:   | FileCheck --check-prefix=CHECK-DSO-SHARED-HWASAN-X86-64-LINUX %s
+//
+// CHECK-DSO-SHARED-HWASAN-X86-64-LINUX: "{{(.*[^-.0-9A-Z_a-z])?}}ld{{(.exe)?}}"
+// CHECK-DSO-SHARED-HWASAN-X86-64-LINUX-NOT: "-lc"
+// CHECK-DSO-SHARED-HWASAN-X86-64-LINUX: libclang_rt.hwasan-x86_64.so"
+// CHECK-DSO-SHARED-HWASAN-X86-64-LINUX-NOT: "-lpthread"
+// CHECK-DSO-SHARED-HWASAN-X86-64-LINUX-NOT: "-lrt"
+// CHECK-DSO-SHARED-HWASAN-X86-64-LINUX-NOT: "-ldl"
+// CHECK-DSO-SHARED-HWASAN-X86-64-LINUX-NOT: "--export-dynamic"
+// CHECK-DSO-SHARED-HWASAN-X86-64-LINUX-NOT: "--dynamic-list"
 
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: -target aarch64-unknown-linux -fuse-ld=ld -fsanitize=hwaddress \
-// RUN: -shared-libsan \
 // RUN: -resource-dir=%S/Inputs/resource_dir \
 // RUN: --sysroot=%S/Inputs/basic_linux_tree \
-// RUN:   | FileCheck --check-prefix=CHECK-SHARED-HWASAN-LINUX %s
+// RUN:   | FileCheck --check-prefix=CHECK-HWASAN-AARCH64-LINUX %s
 //
-// CHECK-SHARED-HWASAN-LINUX: "{{(.*[^-.0-9A-Z_a-z])?}}ld{{(.exe)?}}"
-// CHECK-SHARED-HWASAN-LINUX-NOT: "-lc"
-// CHECK-SHARED-HWASAN-LINUX: libclang_rt.hwasan-aarch64.so"
-// CHECK-SHARED-HWASAN-LINUX-NOT: "-lpthread"
-// CHECK-SHARED-HWASAN-LINUX-NOT: "-lrt"
-// CHECK-SHARED-HWASAN-LINUX-NOT: "-ldl"
-// CHECK-SHARED-HWASAN-LINUX-NOT: "--export-dynamic"
-// CHECK-SHARED-HWASAN-LINUX-NOT: "--dynamic-list"
+// CHECK-HWASAN-AARCH64-LINUX: "{{(.*[^-.0-9A-Z_a-z])?}}ld{{(.exe)?}}"
+// CHECK-HWASAN-AARCH64-LINUX-NOT: "-lc"
+// CHECK-HWASAN-AARCH64-LINUX: libclang_rt.hwasan-aarch64.a"
+// CHECK-HWASAN-AARCH64-LINUX

[PATCH] D43737: Improve -Winfinite-recursion

[Robert Widmann via Phabricator via cfe-commits]

CodaFi updated this revision to Diff 139325.

https://reviews.llvm.org/D43737

Files:
  lib/Sema/AnalysisBasedWarnings.cpp
  test/SemaCXX/warn-infinite-recursion.cpp

Index: test/SemaCXX/warn-infinite-recursion.cpp
===
--- test/SemaCXX/warn-infinite-recursion.cpp
+++ test/SemaCXX/warn-infinite-recursion.cpp
@@ -29,8 +29,7 @@
 void e() { f(); }
 void f() { e(); }
 
-// Don't warn on infinite loops
-void g() {
+void g() {  // expected-warning{{call itself}}
   while (true)
 g();
 
@@ -54,6 +53,19 @@
   return 5 + j();
 }
 
+void k() {  // expected-warning{{call itself}}
+  while(true) {
+k();
+  }
+}
+
+// Don't warn on infinite loops
+void l() {
+  while (true) {}
+
+  l();
+}
+
 class S {
   static void a();
   void b();
Index: lib/Sema/AnalysisBasedWarnings.cpp
===
--- lib/Sema/AnalysisBasedWarnings.cpp
+++ lib/Sema/AnalysisBasedWarnings.cpp
@@ -200,60 +200,41 @@
   return false;
 }
 
-// All blocks are in one of three states.  States are ordered so that blocks
-// can only move to higher states.
-enum RecursiveState {
-  FoundNoPath,
-  FoundPath,
-  FoundPathWithNoRecursiveCall
-};
-
-// Returns true if there exists a path to the exit block and every path
-// to the exit block passes through a call to FD.
+// Returns true if every path from the entry block passes through a call to FD.
 static bool checkForRecursiveFunctionCall(const FunctionDecl *FD, CFG *cfg) {
+  llvm::SmallPtrSet Visited;
+  llvm::SmallVector WorkList;
+  // Keep track of whether we found at least one recursive path.
+  bool foundRecursion = false;
 
   const unsigned ExitID = cfg->getExit().getBlockID();
 
-  // Mark all nodes as FoundNoPath, then set the status of the entry block.
-  SmallVector States(cfg->getNumBlockIDs(), FoundNoPath);
-  States[cfg->getEntry().getBlockID()] = FoundPathWithNoRecursiveCall;
-
-  // Make the processing stack and seed it with the entry block.
-  SmallVector Stack;
-  Stack.push_back(&cfg->getEntry());
-
-  while (!Stack.empty()) {
-CFGBlock *CurBlock = Stack.back();
-Stack.pop_back();
+  // Seed the work list with the entry block.
+  WorkList.push_back(&cfg->getEntry());
 
-unsigned ID = CurBlock->getBlockID();
-RecursiveState CurState = States[ID];
+  while (!WorkList.empty()) {
+CFGBlock *Block = WorkList.pop_back_val();
 
-if (CurState == FoundPathWithNoRecursiveCall) {
-  // Found a path to the exit node without a recursive call.
-  if (ExitID == ID)
-return false;
+for (auto I = Block->succ_begin(), E = Block->succ_end(); I != E; ++I) {
+  if (CFGBlock *SuccBlock = *I) {
+if (!Visited.insert(SuccBlock).second)
+  continue;
 
-  // Only change state if the block has a recursive call.
-  if (hasRecursiveCallInPath(FD, *CurBlock))
-CurState = FoundPath;
-}
+// Found a path to the exit node without a recursive call.
+if (ExitID == SuccBlock->getBlockID())
+  return false;
 
-// Loop over successor blocks and add them to the Stack if their state
-// changes.
-for (auto I = CurBlock->succ_begin(), E = CurBlock->succ_end(); I != E; ++I)
-  if (*I) {
-unsigned next_ID = (*I)->getBlockID();
-if (States[next_ID] < CurState) {
-  States[next_ID] = CurState;
-  Stack.push_back(*I);
+// If the successor block contains a recursive call, end analysis there.
+if (hasRecursiveCallInPath(FD, *SuccBlock)) {
+  foundRecursion = true;
+  continue;
 }
+
+WorkList.push_back(SuccBlock);
   }
+}
   }
-
-  // Return true if the exit node is reachable, and only reachable through
-  // a recursive call.
-  return States[ExitID] == FoundPath;
+  return foundRecursion;
 }
 
 static void checkRecursiveFunction(Sema &S, const FunctionDecl *FD,
@@ -269,10 +250,6 @@
   CFG *cfg = AC.getCFG();
   if (!cfg) return;
 
-  // If the exit block is unreachable, skip processing the function.
-  if (cfg->getExit().pred_empty())
-return;
-
   // Emit diagnostic if a recursive function call is detected for all paths.
   if (checkForRecursiveFunctionCall(FD, cfg))
 S.Diag(Body->getLocStart(), diag::warn_infinite_recursive_function);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44533: [AMDGPU] Fix codegen for inline assembly

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

Ping


https://reviews.llvm.org/D44533



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

[PATCH] D44272: [clangd] Support incremental document syncing

[Simon Marchi via Phabricator via cfe-commits]

simark updated this revision to Diff 139328.
simark added a comment.

Add lit test case


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44272

Files:
  clangd/ClangdLSPServer.cpp
  clangd/DraftStore.cpp
  clangd/DraftStore.h
  clangd/Protocol.cpp
  clangd/Protocol.h
  test/clangd/initialize-params-invalid.test
  test/clangd/initialize-params.test
  test/clangd/textdocument-didchange-fail.test
  unittests/clangd/CMakeLists.txt
  unittests/clangd/DraftStoreTests.cpp

Index: unittests/clangd/DraftStoreTests.cpp
===
--- /dev/null
+++ unittests/clangd/DraftStoreTests.cpp
@@ -0,0 +1,355 @@
+//===-- DraftStoreTests.cpp - Clangd unit tests -*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "Annotations.h"
+#include "DraftStore.h"
+#include "SourceCode.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+using namespace llvm;
+
+struct IncrementalTestStep {
+  StringRef Src;
+  StringRef Contents;
+};
+
+static int rangeLength(StringRef Code, const Range &Rng) {
+  llvm::Expected Start = positionToOffset(Code, Rng.start);
+  llvm::Expected End = positionToOffset(Code, Rng.end);
+  assert(Start);
+  assert(End);
+  return *End - *Start;
+}
+
+// Send the changes one by one to updateDraft, verify the intermediate results.
+
+static void stepByStep(llvm::ArrayRef Steps) {
+  DraftStore DS;
+  Annotations InitialSrc(Steps.front().Src);
+  const char Path[] = "/hello.cpp";
+
+  // Set the initial content.
+  DS.addDraft(Path, InitialSrc.code());
+
+  for (size_t i = 1; i < Steps.size(); i++) {
+Annotations SrcBefore(Steps[i - 1].Src);
+Annotations SrcAfter(Steps[i].Src);
+StringRef Contents = Steps[i - 1].Contents;
+TextDocumentContentChangeEvent Event{
+SrcBefore.range(),
+rangeLength(SrcBefore.code(), SrcBefore.range()),
+Contents.str(),
+};
+
+llvm::Expected Result = DS.updateDraft(Path, {Event});
+EXPECT_TRUE(!!Result);
+EXPECT_EQ(*Result, SrcAfter.code());
+EXPECT_EQ(*DS.getDraft(Path), SrcAfter.code());
+  }
+}
+
+// Send all the changes at once to updateDraft, check only the final result.
+
+static void allAtOnce(llvm::ArrayRef Steps) {
+  DraftStore DS;
+  Annotations InitialSrc(Steps.front().Src);
+  Annotations FinalSrc(Steps.back().Src);
+  const char Path[] = "/hello.cpp";
+  std::vector Changes;
+
+  for (size_t i = 0; i < Steps.size() - 1; i++) {
+Annotations Src(Steps[i].Src);
+StringRef Contents = Steps[i].Contents;
+
+Changes.push_back({
+Src.range(),
+rangeLength(Src.code(), Src.range()),
+Contents.str(),
+});
+  }
+
+  // Set the initial content.
+  DS.addDraft(Path, InitialSrc.code());
+
+  llvm::Expected Result = DS.updateDraft(Path, Changes);
+
+  EXPECT_TRUE(!!Result) << llvm::toString(Result.takeError());
+  EXPECT_EQ(*Result, FinalSrc.code());
+  EXPECT_EQ(*DS.getDraft(Path), FinalSrc.code());
+}
+
+TEST(DraftStoreIncrementalUpdateTest, Simple) {
+  // clang-format off
+  IncrementalTestStep Steps[] =
+{
+  // Replace a range
+  {
+R"cpp(static int
+hello[[World]]()
+{})cpp",
+"Universe"
+  },
+  // Delete a range
+  {
+R"cpp(static int
+hello[[Universe]]()
+{})cpp",
+""
+  },
+  // Add a range
+  {
+R"cpp(static int
+hello[[]]()
+{})cpp",
+"Monde"
+  },
+  {
+R"cpp(static int
+helloMonde()
+{})cpp",
+""
+  }
+};
+  // clang-format on
+
+  stepByStep(Steps);
+  allAtOnce(Steps);
+}
+
+TEST(DraftStoreIncrementalUpdateTest, MultiLine) {
+  // clang-format off
+  IncrementalTestStep Steps[] =
+{
+  // Replace a range
+  {
+R"cpp(static [[int
+helloWorld]]()
+{})cpp",
+R"cpp(char
+welcome)cpp"
+  },
+  // Delete a range
+  {
+R"cpp(static char[[
+welcome]]()
+{})cpp",
+""
+  },
+  // Add a range
+  {
+R"cpp(static char[[]]()
+{})cpp",
+R"cpp(
+cookies)cpp"
+  },
+  // Replace the whole file
+  {
+R"cpp([[static char
+cookies()
+{}]])cpp",
+R"cpp(#include 
+)cpp"
+  },
+  // Delete the whole file
+  {
+R"cpp([[#include 
+]])cpp",
+"",
+  },
+  // Add something to an empty file
+  {
+"[[]]",
+R"cpp(int main() {
+)cpp",
+  },
+  {
+R"cpp(int main() {
+)cpp",
+""
+  }
+};
+  // clang-format on
+
+  stepByStep(Steps);
+  allAtOnce(Steps);
+}
+
+TEST(DraftStoreIncrementalUpdateTest, WrongRangeLength) {
+  DraftStore DS;
+  Path File = "foo.cpp";
+
+  DS.addDraft(File, "int main() {}\n");
+
+  TextDocumentContentChangeEvent Change;
+  Change.range.emplace();
+  Change.range-

[PATCH] D44743: [clang-tidy] Marking hicpp-no-assembler-msvc unsupported on Windows

[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


https://reviews.llvm.org/D44743



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

[PATCH] D44589: [Sema] Make deprecation fix-it replace all multi-parameter ObjC method slots.

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington added a comment.

Hi Volodymyr, thanks for working on this! Overall this looks good, I just have 
a few nits.




Comment at: clang/include/clang/Basic/SourceLocation.h:202
+/// Can be used transparently in places where SourceLocation is expected.
+class MultiSourceLocation {
+  bool IsSingleLoc;

Why can't we just use an ArrayRef for this? It looks like that 
type already has a converting constructor from SourceLocation, so we should be 
able to use in in DiagnoseUseOfDecl without any noise.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:6934-6938
+// Returns a number of method parameters if parsing is successful.
+// In case of unsuccessful parsing SlotNames can contain invalid data.
+static Optional
+parseObjCMethodName(StringRef Name, SmallVectorImpl &SlotNames,
+const LangOptions &LangOpts) {

Maybe tryParseReplacementObjCMethodName or something would be better? 
parseObjCMethodName() is pretty vague. Also the comment above should probably 
be a `///` doxygen comment. It would be nice if you mentioned that `Name` 
originated from an availability attribute in the comment.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:6959-6966
+if (S.empty())
+  continue;
+if (!isIdentifierHead(S[0], AllowDollar))
+  return None;
+for (char C : S)
+  if (!isIdentifierBody(C, AllowDollar))
+return None;

Might be better to add a defaulted AllowDollar parameter to isValidIdentifier() 
in CharInfo.h and use that rather than duplicating it here.


https://reviews.llvm.org/D44589



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

[PATCH] D44559: [Sema] Wrong width of result of mul operation

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In https://reviews.llvm.org/D44559#1044653, @aaron.ballman wrote:

> In https://reviews.llvm.org/D44559#1044639, @rjmccall wrote:
>
> > In https://reviews.llvm.org/D44559#1044186, @avt77 wrote:
> >
> > > >> In https://reviews.llvm.org/D44559#1040799, @rjmccall wrote:
> > > >> 
> > > >>> I think we're correct not to warn here and that GCC/ICC are being 
> > > >>> noisy.  The existence of a temporary promotion to a wider type 
> > > >>> doesn't justify warning on arithmetic between two operands that are 
> > > >>> the same size as the ultimate result.  It is totally fair for users 
> > > >>> to think of this operation as being "closed" on the original type.
> > > >> 
> > > >> 
> > > >> Could you please clarify, are you saying that PR35409 
> > > >>  is not a bug, and clang 
> > > >> should continue to not warn in those cases?
> > > > 
> > > > Correct.
> > >
> > > Does it mean we should abandon this revision? On the other hand it's a 
> > > real bug, isn't it?
> >
> >
> > Not as I see it, no.
>
>
> Do you see this code as having a bug when `a` is >= 182?
>
>   short foo(unsigned char a) {
> return a * a;
>   }
>
>
> (If you don't like seeing `unsigned char`  you can imagine it was spelled as 
> `uint8_t`.)


That's an interesting question.  In general, these warnings do try to ignore 
the effects of implicit promotion.  We would not want -Wsign-conversion to fire 
on `unsigned short x = an_unsigned_short + 1;` (or `- 1`, for that matter), 
even though formally this coerces a `signed int` to `unsigned short`.  
Similarly, -Wsign-conversion *should* warn on `signed short x = 
an_unsigned_short + 1;`, even though formally the promotion from `unsigned 
short` to `signed int` is not problematic and the final conversion from `signed 
int` to `signed short` is not a signedness change.  (This second example should 
also generate a -Wconversion warning, but the questions are independent.)  
Applying that strictly here would say that the user is entitled to think of 
this as an operation on `unsigned char` that then gets losslessly promoted to 
`signed short`, even though arithmetically that's not what happens.  On the 
other hand, I do think there's some room for -Wsign-conversion to be more 
aggressive than -Wconversion about this sort of thing; -Wsign-conversion should 
generally fire for any changes in signedness from the original operand types 
(with the usual complexities around constant values), and there's just an 
exception for computations whose value is known to fit within the expressible 
range of the result type, which is not true of this multiplication.  So I think 
it would be acceptable to warn on this.

John.


https://reviews.llvm.org/D44559



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

[clang-tools-extra] r328127 - [clang-tidy] Marking hicpp-no-assembler-msvc unsupported on Windows

[Julie Hockett via cfe-commits]

Author: juliehockett
Date: Wed Mar 21 11:03:41 2018
New Revision: 328127

URL: http://llvm.org/viewvc/llvm-project?rev=328127&view=rev
Log:
[clang-tidy] Marking hicpp-no-assembler-msvc unsupported on Windows

After changes to lit.site.cfg.in, the test is now running (and failing)
on windows, so temporarily marking it unsupported. See PR36855 for more
details.

Modified:
clang-tools-extra/trunk/test/clang-tidy/hicpp-no-assembler-msvc.cpp

Modified: clang-tools-extra/trunk/test/clang-tidy/hicpp-no-assembler-msvc.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/hicpp-no-assembler-msvc.cpp?rev=328127&r1=328126&r2=328127&view=diff
==
--- clang-tools-extra/trunk/test/clang-tidy/hicpp-no-assembler-msvc.cpp 
(original)
+++ clang-tools-extra/trunk/test/clang-tidy/hicpp-no-assembler-msvc.cpp Wed Mar 
21 11:03:41 2018
@@ -1,4 +1,6 @@
 // REQUIRES: system-windows
+// FIXME: Re-enable test on windows (PR36855)
+// UNSUPPORTED: system-windows
 // RUN: %check_clang_tidy %s hicpp-no-assembler %t
 
 void f() {


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

[PATCH] D44743: [clang-tidy] Marking hicpp-no-assembler-msvc unsupported on Windows

[Julie Hockett via Phabricator via cfe-commits]

juliehockett closed this revision.
juliehockett added a comment.

Committed in r328127.


https://reviews.llvm.org/D44743



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

[PATCH] D44747: [AMDGPU] Set calling convention for CUDA kernel

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl created this revision.
yaxunl added reviewers: rjmccall, arsenm.
Herald added subscribers: t-tye, tpr, dstuttard, wdng, kzhuravl.

This patch sets the calling convention for CUDA kernels required by AMDGPU 
target.

Patch by Greg Rodgers.
Lit test added by Yaxun Liu.


https://reviews.llvm.org/D44747

Files:
  lib/CodeGen/CodeGenModule.cpp
  test/CodeGenCUDA/kernel-amdgcn.cu


Index: test/CodeGenCUDA/kernel-amdgcn.cu
===
--- /dev/null
+++ test/CodeGenCUDA/kernel-amdgcn.cu
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -triple amdgcn -fcuda-is-device -emit-llvm %s -o - | 
FileCheck %s
+#include "Inputs/cuda.h"
+
+// CHECK: define amdgpu_kernel void @_ZN1A6kernelEv()
+class A {
+public:
+  static __global__ void kernel(){}
+};
+
+// CHECK: define void @_Z10non_kernelv()
+__device__ void non_kernel(){}
+
+// CHECK: define amdgpu_kernel void @_Z6kerneli(i32 %x)
+__global__ void kernel(int x) {
+  non_kernel();
+}
+
+// CHECK: define amdgpu_kernel void @_Z15template_kernelI1AEvT_()
+template
+__global__ void template_kernel(T x) {}
+
+void launch(void *f);
+
+int main() {
+  launch((void*)A::kernel);
+  launch((void*)kernel);
+  launch((void*)template_kernel);
+  return 0;
+}
Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -3590,6 +3590,9 @@
 
   MaybeHandleStaticInExternC(D, Fn);
 
+  if ((getTriple().getArch() == llvm::Triple::amdgcn) &&
+  D->hasAttr())
+Fn->setCallingConv(llvm::CallingConv::AMDGPU_KERNEL);
   maybeSetTrivialComdat(*D, *Fn);
 
   CodeGenFunction(*this).GenerateCode(D, Fn, FI);


Index: test/CodeGenCUDA/kernel-amdgcn.cu
===
--- /dev/null
+++ test/CodeGenCUDA/kernel-amdgcn.cu
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -triple amdgcn -fcuda-is-device -emit-llvm %s -o - | FileCheck %s
+#include "Inputs/cuda.h"
+
+// CHECK: define amdgpu_kernel void @_ZN1A6kernelEv()
+class A {
+public:
+  static __global__ void kernel(){}
+};
+
+// CHECK: define void @_Z10non_kernelv()
+__device__ void non_kernel(){}
+
+// CHECK: define amdgpu_kernel void @_Z6kerneli(i32 %x)
+__global__ void kernel(int x) {
+  non_kernel();
+}
+
+// CHECK: define amdgpu_kernel void @_Z15template_kernelI1AEvT_()
+template
+__global__ void template_kernel(T x) {}
+
+void launch(void *f);
+
+int main() {
+  launch((void*)A::kernel);
+  launch((void*)kernel);
+  launch((void*)template_kernel);
+  return 0;
+}
Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -3590,6 +3590,9 @@
 
   MaybeHandleStaticInExternC(D, Fn);
 
+  if ((getTriple().getArch() == llvm::Triple::amdgcn) &&
+  D->hasAttr())
+Fn->setCallingConv(llvm::CallingConv::AMDGPU_KERNEL);
   maybeSetTrivialComdat(*D, *Fn);
 
   CodeGenFunction(*this).GenerateCode(D, Fn, FI);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42730: [clang-tidy]] Add check for use of types/classes/functions from header which are deprecated and removed in C++17

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tidy/modernize/DeprecatedFunctionalCheck.cpp:48-54
+  } else if (const auto *const Call =
+ Result.Nodes.getNodeAs("ptr_fun_call")) {
+diag(Call->getLocStart(), Message) << "'std::ptr_fun'";
+  } else if (const auto *const Call =
+ Result.Nodes.getNodeAs("mem_fun_call")) {
+diag(Call->getLocStart(), Message) << "'std::mem_fun'";
+  }

alexfh wrote:
> aaron.ballman wrote:
> > alexfh wrote:
> > > aaron.ballman wrote:
> > > > alexfh wrote:
> > > > > aaron.ballman wrote:
> > > > > > alexfh wrote:
> > > > > > > aaron.ballman wrote:
> > > > > > > > alexfh wrote:
> > > > > > > > > alexfh wrote:
> > > > > > > > > > aaron.ballman wrote:
> > > > > > > > > > > massberg wrote:
> > > > > > > > > > > > aaron.ballman wrote:
> > > > > > > > > > > > > I think that this code should be generalized (same 
> > > > > > > > > > > > > with the matchers) so that you match on 
> > > > > > > > > > > > > `hasAnyName()` for the function calls and use 
> > > > > > > > > > > > > `CallExpr::getCalleeDecl()` to get the declaration. 
> > > > > > > > > > > > > e.g.,
> > > > > > > > > > > > > ```
> > > > > > > > > > > > > if (const auto *Call = 
> > > > > > > > > > > > > Result.Nodes.getNodeAs("blech")) {
> > > > > > > > > > > > >   if (const Decl *Callee = Call->getCalleeDecl())
> > > > > > > > > > > > > diag(Call->getLocStart(), Message) << Calleee;
> > > > > > > > > > > > > }
> > > > > > > > > > > > > ```
> > > > > > > > > > > > > This way you can add more named without having to add 
> > > > > > > > > > > > > extra logic for the diagnostics.
> > > > > > > > > > > > I generalized the code and the matcher.
> > > > > > > > > > > > When we use
> > > > > > > > > > > > ```
> > > > > > > > > > > > << cast(Callee);
> > > > > > > > > > > > ```
> > > > > > > > > > > > we get the template arguments in the name , e.g. 
> > > > > > > > > > > > `ptr_fun`, so I chose to use 
> > > > > > > > > > > > `getQualifiedNameAsString`.
> > > > > > > > > > > > If there is there a better way to get the function name 
> > > > > > > > > > > > without template arguments I appreciate any suggestions.
> > > > > > > > > > > > 
> > > > > > > > > > > > 
> > > > > > > > > > > Nope, in that case, your code is correct. However, we 
> > > > > > > > > > > generally provide the template arguments in diagnostics. 
> > > > > > > > > > > I see @alexfh was asking for them to be removed as not 
> > > > > > > > > > > being useful, but I'm not certain I agree with his 
> > > > > > > > > > > rationale. Yes, all instances are deprecated and thus the 
> > > > > > > > > > > template arguments don't discern between good and bad 
> > > > > > > > > > > cases, but showing the template arguments is also 
> > > > > > > > > > > consistent with the other diagnostics we emit. For 
> > > > > > > > > > > instance, other "deprecated" diagnostics also emit the 
> > > > > > > > > > > template arguments. I'm not certain we should be 
> > > > > > > > > > > inconsistent with the way we produce diagnostics, but 
> > > > > > > > > > > I'll defer to Alex if he still feels strongly about 
> > > > > > > > > > > leaving them off here.
> > > > > > > > > > Indeed, -Wdeprecated-declarations warnings print template 
> > > > > > > > > > arguments. Moreover, they seem to be issued only on 
> > > > > > > > > > instantiations, see https://godbolt.org/g/W563gw.
> > > > > > > > > > 
> > > > > > > > > > But I have a number of concerns with template arguments in 
> > > > > > > > > > the deprecation warnings:
> > > > > > > > > > 
> > > > > > > > > > 1. The note attached to the warning lies. Consider a 
> > > > > > > > > > warning from the test above:
> > > > > > > > > >   ...
> > > > > > > > > >   :11:1: warning: 'B' is deprecated: bbb 
> > > > > > > > > > [-Wdeprecated-declarations]
> > > > > > > > > >   B e;
> > > > > > > > > >   ^
> > > > > > > > > >   :7:10: note: 'B' has been explicitly marked 
> > > > > > > > > > deprecated here
> > > > > > > > > >   struct [[deprecated("bbb")]] B {};
> > > > > > > > > > 
> > > > > > > > > >  But `B` hasn't been explicitly marked deprecated, 
> > > > > > > > > > only the template definition of `B` has been. Template 
> > > > > > > > > > arguments are important in the case of the explicit 
> > > > > > > > > > template specialization `A` in the same example, but 
> > > > > > > > > > not in cases where the template definition was marked 
> > > > > > > > > > deprecated, since template arguments only add noise and no 
> > > > > > > > > > useful information there.
> > > > > > > > > > 2. Warnings can easily become unreadable: 
> > > > > > > > > > https://godbolt.org/g/AFdznH
> > > > > > > > > > 3. Certain coding patterns can result in numerous 
> > > > > > > > > > deprecation warnings differing only in template arguments: 
> > > > > > > > > > https://godbolt.org/g/U2JCbG. Clang-tidy can deduplicate 
> > > > > > > > > > warnings, if they have identical text and

[clang-tools-extra] r328131 - Reland "[lit] Adding config initialization to lit tests in clang-tools-extra"

[Julie Hockett via cfe-commits]

Author: juliehockett
Date: Wed Mar 21 11:50:26 2018
New Revision: 328131

URL: http://llvm.org/viewvc/llvm-project?rev=328131&view=rev
Log:
Reland "[lit] Adding config initialization to lit tests in clang-tools-extra"

Adding the config initialization to clang-tools-extra so that tests that
use REQUIRES, UNSUPPORTED, and XFAIL based on platform or target triple
work properly.

Modified:
clang-tools-extra/trunk/test/lit.site.cfg.in

Modified: clang-tools-extra/trunk/test/lit.site.cfg.in
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/lit.site.cfg.in?rev=328131&r1=328130&r2=328131&view=diff
==
--- clang-tools-extra/trunk/test/lit.site.cfg.in (original)
+++ clang-tools-extra/trunk/test/lit.site.cfg.in Wed Mar 21 11:50:26 2018
@@ -23,5 +23,7 @@ except KeyError:
 key, = e.args
 lit_config.fatal("unable to find %r parameter, use '--param=%s=VALUE'" % 
(key,key))
 
+@LIT_SITE_CFG_IN_FOOTER@
+
 # Let the main config do the real work.
 lit_config.load_config(config, "@CLANG_TOOLS_SOURCE_DIR@/test/lit.cfg")


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

[PATCH] D44749: [OpenMP][Clang] Add call to global data sharing stack initialization on the workers side

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

gtbercea created this revision.
gtbercea added reviewers: ABataev, grokos, carlo.bertolli, caomhin.
Herald added subscribers: cfe-commits, guansong, jholewinski.

The workers also need to initialize the global stack. The call to the 
initialization function needs to happen after the kernel_init() function is 
called by the master. This ensures that the per-team data structures of the 
runtime have been initialized.


Repository:
  rC Clang

https://reviews.llvm.org/D44749

Files:
  lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
  test/OpenMP/nvptx_data_sharing.cpp


Index: test/OpenMP/nvptx_data_sharing.cpp
===
--- test/OpenMP/nvptx_data_sharing.cpp
+++ test/OpenMP/nvptx_data_sharing.cpp
@@ -27,6 +27,11 @@
   }
 }
 
+/// = In the worker function = ///
+// CK1: {{.*}}define internal void 
@__omp_offloading{{.*}}test_ds{{.*}}_worker()
+// CK1: call void @llvm.nvvm.barrier0()
+// CK1: call void @__kmpc_data_sharing_init_stack
+
 /// = In the kernel function = ///
 
 // CK1: {{.*}}define void @__omp_offloading{{.*}}test_ds{{.*}}()
Index: lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
===
--- lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
+++ lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
@@ -801,6 +801,11 @@
   // Wait for parallel work
   syncCTAThreads(CGF);
 
+  // For data sharing, we need to initialize the stack for workers.
+  CGF.EmitRuntimeCall(
+  createNVPTXRuntimeFunction(
+  OMPRTL_NVPTX__kmpc_data_sharing_init_stack));
+
   Address WorkFn =
   CGF.CreateDefaultAlignTempAlloca(CGF.Int8PtrTy, /*Name=*/"work_fn");
   Address ExecStatus =


Index: test/OpenMP/nvptx_data_sharing.cpp
===
--- test/OpenMP/nvptx_data_sharing.cpp
+++ test/OpenMP/nvptx_data_sharing.cpp
@@ -27,6 +27,11 @@
   }
 }
 
+/// = In the worker function = ///
+// CK1: {{.*}}define internal void @__omp_offloading{{.*}}test_ds{{.*}}_worker()
+// CK1: call void @llvm.nvvm.barrier0()
+// CK1: call void @__kmpc_data_sharing_init_stack
+
 /// = In the kernel function = ///
 
 // CK1: {{.*}}define void @__omp_offloading{{.*}}test_ds{{.*}}()
Index: lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
===
--- lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
+++ lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
@@ -801,6 +801,11 @@
   // Wait for parallel work
   syncCTAThreads(CGF);
 
+  // For data sharing, we need to initialize the stack for workers.
+  CGF.EmitRuntimeCall(
+  createNVPTXRuntimeFunction(
+  OMPRTL_NVPTX__kmpc_data_sharing_init_stack));
+
   Address WorkFn =
   CGF.CreateDefaultAlignTempAlloca(CGF.Int8PtrTy, /*Name=*/"work_fn");
   Address ExecStatus =
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44541: [OpenMP][Clang] Move device global stack init before master-workers split

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

gtbercea abandoned this revision.
gtbercea added a comment.

This leads to usage of statically allocated shared data before their 
initialization in runtime structures by master thread in kernel_init() 
function. New patch available with worker and master-side initialization.


Repository:
  rC Clang

https://reviews.llvm.org/D44541



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

[PATCH] D44602: [clang-tidy] readability-function-size: add VariableThreshold param.

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: test/clang-tidy/readability-function-size.cpp:207-212
+void variables_8() {
+  int a, b;
+  struct A {
+A(int c, int d);
+  };
+}

aaron.ballman wrote:
> lebedev.ri wrote:
> > aaron.ballman wrote:
> > > lebedev.ri wrote:
> > > > aaron.ballman wrote:
> > > > > lebedev.ri wrote:
> > > > > > aaron.ballman wrote:
> > > > > > > I think the current behavior here is correct and the previous 
> > > > > > > behavior was incorrect. However, it brings up an interesting 
> > > > > > > question about what to do here:
> > > > > > > ```
> > > > > > > void f() {
> > > > > > >   struct S {
> > > > > > > void bar() {
> > > > > > >   int a, b;
> > > > > > > }
> > > > > > >   };
> > > > > > > }
> > > > > > > ```
> > > > > > > Does `f()` contain zero variables or two? I would contend that it 
> > > > > > > has no variables because S::bar() is a different scope than f(). 
> > > > > > > But I can see a case being made about the complexity of f() being 
> > > > > > > increased by the presence of the local class definition. Perhaps 
> > > > > > > this is a different facet of the test about number of types?
> > > > > > As previously briefly discussed in IRC, i **strongly** believe that 
> > > > > > the current behavior is correct, and `readability-function-size`
> > > > > > should analyze/diagnose the function as a whole, including all 
> > > > > > sub-classes/sub-functions.
> > > > > Do you know of any coding standards related to this check that weigh 
> > > > > in on this?
> > > > > 
> > > > > What do you think about this:
> > > > > ```
> > > > > #define SWAP(x, y) ({__typeof__(x) temp = x; x = y; y = x;})
> > > > > 
> > > > > void f() {
> > > > >   int a = 10, b = 12;
> > > > >   SWAP(a, b);
> > > > > }
> > > > > ```
> > > > > Does f() have two variables or three? Should presence of the `SWAP` 
> > > > > macro cause this code to be more complex due to having too many 
> > > > > variables?
> > > > Datapoint: the doc 
> > > > (`docs/clang-tidy/checks/readability-function-size.rst`) actually 
> > > > already states that macros *are* counted.
> > > > 
> > > > ```
> > > > .. option:: StatementThreshold
> > > > 
> > > >Flag functions exceeding this number of statements. This may differ
> > > >significantly from the number of lines for macro-heavy code. The 
> > > > default is
> > > >`800`.
> > > > ```
> > > > ```
> > > > .. option:: NestingThreshold
> > > > 
> > > > Flag compound statements which create next nesting level after
> > > > `NestingThreshold`. This may differ significantly from the expected 
> > > > value
> > > > for macro-heavy code. The default is `-1` (ignore the nesting 
> > > > level).
> > > > ```
> > > My concerns relate to what's considered a "variable declared in the body" 
> > > (per the documentation) in relation to function complexity. To me, if the 
> > > variable is not accessible lexically within the body of the function, 
> > > it's not adding to the function's complexity *for local variables*. It 
> > > may certainly be adding other complexity, of course.
> > > 
> > > I would have a very hard time explaining to a user that variables they 
> > > cannot see or change (assuming the macro is in a header file out of their 
> > > control) contribute to their function's complexity. Similarly, I would 
> > > have difficulty explaining that variables in an locally declared class 
> > > member function contribute to the number of variables in the outer 
> > > function body, but the class data members somehow do not.
> > > 
> > > (per the documentation) 
> > 
> > Please note that the word `complexity` is not used in the 
> > **documentation**, only `size` is.
> > 
> > There also is the other side of the coin:
> > 
> > ```
> > #define simple_macro_please_ignore \
> >   the; \
> >   actual; \
> >   content; \
> >   of; \
> >   the; \
> >   foo();
> > 
> > // Very simple function, nothing to see.
> > void foo() {
> >   simple_macro_please_ignore();
> > }
> > 
> > #undef simple_macro_please_ignore
> > ```
> > 
> > In other words, if we ignore macros, it would be possible to abuse them to 
> > artificially reduce complexity, by hiding it in the macros.
> > I agree that it's total abuse of macros, but macros are in general not 
> > nice, and it would not be good to give such things a pass.
> > 
> > 
> > > My concerns relate to what's considered a "variable declared in the body" 
> > > (per the documentation) in relation to function complexity.
> > 
> > Could you please clarify, at this point, your concerns are only about this 
> > new part of the check (variables), or for the entire check?
> > In other words, if we ignore macros, it would be possible to abuse them to 
> > artificially reduce complexity, by hiding it in the macros.
> 
> I don't disagree, that's why I'm trying to explore the boundaries. Your 
> example does artificially reduce complexity. My example using swap does not 
> -- it's an idiomat

[PATCH] D41458: [libc++][C++17] Elementary string conversions for integral types

[Marshall Clow via Phabricator via cfe-commits]

mclow.lists added a comment.

Sorry I've let this lie fallow for so long.




Comment at: include/charconv:234
+to_chars(char* __first, char* __last, _Tp __value, int __base)
+-> to_chars_result
+{

Why use the trailing return type here?
I don't see any advantage - it doesn't depend on the parameters (template or 
runtime).





Comment at: src/support/itoa/itoa.cpp:1
+// -*- C++ -*-
+//===--===//

lichray wrote:
> EricWF wrote:
> > This file should be renamed `charconv.cpp` and moved into the main source 
> > directory.
> We are going to have floating point cpp files so I don't think that one 
> charconv.cpp is enough.
We can put both integral and floating point routines in the same source file ;-)

What Eric said - there should be just `charconv.cpp`, and no subdirectory.

Also, if this is not your work, then I need some notice (an email is fine) by 
the author saying that they're OK with contributing this under the LLVM license.



Comment at: src/support/itoa/itoa.cpp:35
+
+#define APPEND1(i)  \
+do  \

lichray wrote:
> EricWF wrote:
> > Any reason these can't be `static` functions? The compiler should optimize 
> > them away nicely.
> Although yes, but that's what the author provides.  It's an implementation 
> file, so it doesn't matter I guess.
It *does* matter, since we'll have to maintain this.

It would also be nice if they had meaningful names.


Repository:
  rCXX libc++

https://reviews.llvm.org/D41458



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

[PATCH] D44272: [clangd] Support incremental document syncing

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: unittests/clangd/DraftStoreTests.cpp:27
+
+static int rangeLength(StringRef Code, const Range &Rng) {
+  size_t Start = positionToOffset(Code, Rng.start);

ilya-biryukov wrote:
> no need for `static`, since function is already inside an anonymous namespace.
> remove it?
This comment is marked as done, but not addressed.



Comment at: unittests/clangd/DraftStoreTests.cpp:38
+// Send the changes one by one to updateDraft, verify the intermediate results.
+
+static void stepByStep(llvm::ArrayRef Steps) {

ilya-biryukov wrote:
> NIT: remove empty line after comment?
> NIT: make the comment doxygen-like (i.e. start with `///`)?
This comment is marked as done, but not addressed.



Comment at: unittests/clangd/DraftStoreTests.cpp:42
+  Annotations InitialSrc(Steps.front().Src);
+  const char Path[] = "/hello.cpp";
+

ilya-biryukov wrote:
> NIT: use `constexpr StringLiteral Path("/hello.cpp")` instead?
> It's a `StringRef` with size known at compile-time.
This comment is marked as done, but not addressed.



Comment at: unittests/clangd/DraftStoreTests.cpp:58
+llvm::Expected Result = DS.updateDraft(Path, {Event});
+EXPECT_TRUE(!!Result);
+EXPECT_EQ(*Result, SrcAfter.code());

ilya-biryukov wrote:
> Use `ASSERT_TRUE(!!Result)`, the code below won't work for failure results 
> anyway.
This comment is marked as done, but not addressed.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44272



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

[PATCH] D43047: [Builtins] Overload __builtin_operator_new/delete to allow forwarding to usual allocation/deallocation functions.

[Eric Fiselier via Phabricator via cfe-commits]

EricWF updated this revision to Diff 139347.
EricWF marked an inline comment as done.
EricWF added a comment.

- Merge with upstream.
- Add requested assertion.


https://reviews.llvm.org/D43047

Files:
  include/clang/Basic/Builtins.def
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/Sema.h
  lib/CodeGen/CGBuiltin.cpp
  lib/CodeGen/CGExprCXX.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/Lex/PPMacroExpansion.cpp
  lib/Sema/SemaChecking.cpp
  lib/Sema/SemaExprCXX.cpp
  test/CodeGenCXX/builtin-operator-new-delete.cpp
  test/SemaCXX/builtin-operator-new-delete.cpp

Index: test/SemaCXX/builtin-operator-new-delete.cpp
===
--- /dev/null
+++ test/SemaCXX/builtin-operator-new-delete.cpp
@@ -0,0 +1,153 @@
+// RUN: %clang_cc1 -std=c++1z -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++03 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++03 -faligned-allocation -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify -fsized-deallocation %s
+
+#if !__has_builtin(__builtin_operator_new) || !__has_builtin(__builtin_operator_delete)
+#error builtins should always be available
+#endif
+
+#if __has_builtin(__builtin_operator_new) != 201802L || \
+__has_builtin(__builtin_operator_delete) != 201802L
+#error builtin should report updated value
+#endif
+
+typedef __SIZE_TYPE__ size_t;
+namespace std {
+  struct nothrow_t {};
+#if __cplusplus >= 201103L
+enum class align_val_t : size_t {};
+#else
+  enum align_val_t { __zero = 0,
+ __max = (size_t)-1 };
+#endif
+}
+std::nothrow_t nothrow;
+
+void *operator new(size_t); // expected-note 1+ {{candidate function}}
+void operator delete(void *); // expected-note 1+ {{candidate function}}
+
+// Declare the reserved placement operators.
+void *operator new(size_t, void*) throw(); // expected-note 1+ {{candidate function}}
+void operator delete(void *, void *)throw(); // expected-note 1+ {{candidate function}}
+void *operator new[](size_t, void*) throw();
+void operator delete[](void*, void*) throw();
+
+// Declare the replaceable global allocation operators.
+void *operator new(size_t, const std::nothrow_t &) throw(); // expected-note 1+ {{candidate function}}
+void *operator new[](size_t, const std::nothrow_t &) throw();
+void operator delete(void *, const std::nothrow_t &)throw(); // expected-note 1+ {{candidate function}}
+void operator delete[](void *, const std::nothrow_t &) throw();
+
+// aligned allocation and deallocation functions.
+void* operator new  ( size_t count, std::align_val_t al); // expected-note 1+ {{candidate function}}
+void operator delete(void *, std::align_val_t); // expected-note 1+ {{candidate}}
+#ifndef __cpp_aligned_new
+// expected-note@-3 1+ {{non-usual 'operator new' declared here}}
+// expected-note@-3 1+ {{non-usual 'operator delete' declared here}}
+#endif
+void *operator new[](size_t count, std::align_val_t al);
+void operator delete[](void*, std::align_val_t);
+
+void operator delete(void *, size_t); // expected-note 1+ {{candidate}}
+#ifndef __cpp_sized_deallocation
+// expected-note@-2 1+ {{non-usual 'operator delete' declared here}}
+#endif
+void operator delete[](void*, size_t);
+
+// Declare some other placemenet operators.
+void *operator new(size_t, void*, bool) throw(); // expected-note 1+ {{candidate function}}
+void *operator new[](size_t, void*, bool) throw();
+
+void *NP = 0;
+
+void test_typo_in_args() {
+  __builtin_operator_new(DNE);  // expected-error {{undeclared identifier 'DNE'}}
+  __builtin_operator_new(DNE, DNE2);// expected-error {{undeclared identifier 'DNE'}} expected-error {{'DNE2'}}
+  __builtin_operator_delete(DNE);   // expected-error {{'DNE'}}
+  __builtin_operator_delete(DNE, DNE2); // expected-error {{'DNE'}} expected-error {{'DNE2'}}
+}
+
+void test_arg_types() {
+  __builtin_operator_new(NP);  // expected-error {{no matching function for call to 'operator new'}}
+  __builtin_operator_new(NP, std::align_val_t(0)); // expected-error {{no matching function for call to 'operator new'}}}
+}
+void test_return_type() {
+  int w = __builtin_operator_new(42);// expected-error {{cannot initialize a variable of type 'int' with an rvalue of type 'void *'}}
+  int y = __builtin_operator_delete(NP); // expected-error {{cannot initialize a variable of type 'int' with an rvalue of type 'void'}}
+}
+
+void test_aligned_new() {
+#ifdef __cpp_aligned_new
+  void *p = __builtin_operator_new(42, std::align_val_t(2));
+  __builtin_operator_delete(p, std::align_val_t(2));
+#else
+  // FIXME: We've manually declared the aligned new/delete overloads,
+  // but LangOpts::AlignedAllocation is false. Should our overloads be considered
+  // usual allocation/deallocation functions?
+  void *p = __builtin_operator_new(42, std::align_val_t(2)); // expected-error {{call to '__builtin_operator_new' selects non-usual

[PATCH] D43047: [Builtins] Overload __builtin_operator_new/delete to allow forwarding to usual allocation/deallocation functions.

[Eric Fiselier via Phabricator via cfe-commits]

EricWF added inline comments.



Comment at: lib/Sema/SemaExprCXX.cpp:3458
+  }
+  TheCall->getCallee()->setType(OperatorNewOrDelete->getType());
+

rsmith wrote:
> It would be nice to assert that the callee you're setting the type of is an 
> ImplicitCastExpr doing a BuiltinFnToFnPtr cast (just so that it's obvious 
> that this is the only type we need to update and that it's freshly-created).
Ack. Done.


https://reviews.llvm.org/D43047



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

r328134 - [Builtins] Overload __builtin_operator_new/delete to allow forwarding to usual allocation/deallocation functions.

[Eric Fiselier via cfe-commits]

Author: ericwf
Date: Wed Mar 21 12:19:48 2018
New Revision: 328134

URL: http://llvm.org/viewvc/llvm-project?rev=328134&view=rev
Log:
[Builtins] Overload __builtin_operator_new/delete to allow forwarding to usual 
allocation/deallocation functions.

Summary:
Libc++'s default allocator uses `__builtin_operator_new` and 
`__builtin_operator_delete` in order to allow the calls to new/delete to be 
ellided. However, libc++ now needs to support over-aligned types in the default 
allocator. In order to support this without disabling the existing optimization 
Clang needs to support calling the aligned new overloads from the builtins.

See llvm.org/PR22634 for more information about the libc++ bug.

This patch changes `__builtin_operator_new`/`__builtin_operator_delete` to call 
any usual `operator new`/`operator delete` function. It does this by performing 
overload resolution with the arguments passed to the builtin to determine which 
allocation function to call. If the selected function is not a usual allocation 
function a diagnostic is issued.

One open issue is if the `align_val_t` overloads should be considered "usual" 
when `LangOpts::AlignedAllocation` is disabled.


In order to allow libc++ to detect this new behavior the value for 
`__has_builtin(__builtin_operator_new)` has been updated to `201802`.

Reviewers: rsmith, majnemer, aaron.ballman, erik.pilkington, bogner, ahatanak

Reviewed By: rsmith

Subscribers: cfe-commits

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

Added:
cfe/trunk/test/CodeGenCXX/builtin-operator-new-delete.cpp
cfe/trunk/test/SemaCXX/builtin-operator-new-delete.cpp
Modified:
cfe/trunk/include/clang/Basic/Builtins.def
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/CodeGen/CGBuiltin.cpp
cfe/trunk/lib/CodeGen/CGExprCXX.cpp
cfe/trunk/lib/CodeGen/CodeGenFunction.h
cfe/trunk/lib/Lex/PPMacroExpansion.cpp
cfe/trunk/lib/Sema/SemaChecking.cpp
cfe/trunk/lib/Sema/SemaExprCXX.cpp

Modified: cfe/trunk/include/clang/Basic/Builtins.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Builtins.def?rev=328134&r1=328133&r2=328134&view=diff
==
--- cfe/trunk/include/clang/Basic/Builtins.def (original)
+++ cfe/trunk/include/clang/Basic/Builtins.def Wed Mar 21 12:19:48 2018
@@ -1371,8 +1371,8 @@ BUILTIN(__builtin_smulll_overflow, "bSLL
 
 // Clang builtins (not available in GCC).
 BUILTIN(__builtin_addressof, "v*v&", "nct")
-BUILTIN(__builtin_operator_new, "v*z", "c")
-BUILTIN(__builtin_operator_delete, "vv*", "n")
+BUILTIN(__builtin_operator_new, "v*z", "tc")
+BUILTIN(__builtin_operator_delete, "vv*", "tn")
 BUILTIN(__builtin_char_memchr, "c*cC*iz", "n")
 
 // Safestack builtins

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=328134&r1=328133&r2=328134&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Mar 21 12:19:48 
2018
@@ -7627,6 +7627,11 @@ def err_destroying_operator_delete_not_u
   "alignment parameter">;
 def note_implicit_delete_this_in_destructor_here : Note<
   "while checking implicit 'delete this' for virtual destructor">;
+def err_builtin_operator_new_delete_not_usual : Error<
+  "call to '%select{__builtin_operator_new|__builtin_operator_delete}0' "
+  "selects non-usual %select{allocation|deallocation}0 function">;
+def note_non_usual_function_declared_here : Note<
+  "non-usual %0 declared here">;
 
 // C++ literal operators
 def err_literal_operator_outside_namespace : Error<

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=328134&r1=328133&r2=328134&view=diff
==
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Wed Mar 21 12:19:48 2018
@@ -10376,6 +10376,8 @@ private:
   ExprResult SemaBuiltinNontemporalOverloaded(ExprResult TheCallResult);
   ExprResult SemaAtomicOpsOverloaded(ExprResult TheCallResult,
  AtomicExpr::AtomicOp Op);
+  ExprResult SemaBuiltinOperatorNewDeleteOverloaded(ExprResult TheCallResult,
+bool IsDelete);
   bool SemaBuiltinConstantArg(CallExpr *TheCall, int ArgNum,
   llvm::APSInt &Result);
   bool SemaBuiltinConstantArgRange(CallExpr *TheCall, int ArgNum,

Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=328134&r1=328133&r2=328134&view=diff
=

[PATCH] D43047: [Builtins] Overload __builtin_operator_new/delete to allow forwarding to usual allocation/deallocation functions.

[Eric Fiselier via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC328134: [Builtins] Overload __builtin_operator_new/delete to 
allow forwarding to usual… (authored by EricWF, committed by ).

Repository:
  rC Clang

https://reviews.llvm.org/D43047

Files:
  include/clang/Basic/Builtins.def
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/Sema.h
  lib/CodeGen/CGBuiltin.cpp
  lib/CodeGen/CGExprCXX.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/Lex/PPMacroExpansion.cpp
  lib/Sema/SemaChecking.cpp
  lib/Sema/SemaExprCXX.cpp
  test/CodeGenCXX/builtin-operator-new-delete.cpp
  test/SemaCXX/builtin-operator-new-delete.cpp

Index: lib/Sema/SemaExprCXX.cpp
===
--- lib/Sema/SemaExprCXX.cpp
+++ lib/Sema/SemaExprCXX.cpp
@@ -1443,7 +1443,7 @@
   CUDAPref = S.IdentifyCUDAPreference(Caller, FD);
 }
 
-operator bool() const { return FD; }
+explicit operator bool() const { return FD; }
 
 bool isBetterThan(const UsualDeallocFnInfo &Other, bool WantSize,
   bool WantAlign) const {
@@ -2271,7 +2271,6 @@
   llvm_unreachable("Unreachable, bad result from BestViableFunction");
 }
 
-
 /// FindAllocationFunctions - Finds the overloads of operator new and delete
 /// that are appropriate for the allocation.
 bool Sema::FindAllocationFunctions(SourceLocation StartLoc, SourceRange Range,
@@ -3343,6 +3342,128 @@
   return Result;
 }
 
+static bool resolveBuiltinNewDeleteOverload(Sema &S, CallExpr *TheCall,
+bool IsDelete,
+FunctionDecl *&Operator) {
+
+  DeclarationName NewName = S.Context.DeclarationNames.getCXXOperatorName(
+  IsDelete ? OO_Delete : OO_New);
+
+  LookupResult R(S, NewName, TheCall->getLocStart(), Sema::LookupOrdinaryName);
+  S.LookupQualifiedName(R, S.Context.getTranslationUnitDecl());
+  assert(!R.empty() && "implicitly declared allocation functions not found");
+  assert(!R.isAmbiguous() && "global allocation functions are ambiguous");
+
+  // We do our own custom access checks below.
+  R.suppressDiagnostics();
+
+  SmallVector Args(TheCall->arg_begin(), TheCall->arg_end());
+  OverloadCandidateSet Candidates(R.getNameLoc(),
+  OverloadCandidateSet::CSK_Normal);
+  for (LookupResult::iterator FnOvl = R.begin(), FnOvlEnd = R.end();
+   FnOvl != FnOvlEnd; ++FnOvl) {
+// Even member operator new/delete are implicitly treated as
+// static, so don't use AddMemberCandidate.
+NamedDecl *D = (*FnOvl)->getUnderlyingDecl();
+
+if (FunctionTemplateDecl *FnTemplate = dyn_cast(D)) {
+  S.AddTemplateOverloadCandidate(FnTemplate, FnOvl.getPair(),
+ /*ExplicitTemplateArgs=*/nullptr, Args,
+ Candidates,
+ /*SuppressUserConversions=*/false);
+  continue;
+}
+
+FunctionDecl *Fn = cast(D);
+S.AddOverloadCandidate(Fn, FnOvl.getPair(), Args, Candidates,
+   /*SuppressUserConversions=*/false);
+  }
+
+  SourceRange Range = TheCall->getSourceRange();
+
+  // Do the resolution.
+  OverloadCandidateSet::iterator Best;
+  switch (Candidates.BestViableFunction(S, R.getNameLoc(), Best)) {
+  case OR_Success: {
+// Got one!
+FunctionDecl *FnDecl = Best->Function;
+assert(R.getNamingClass() == nullptr &&
+   "class members should not be considered");
+
+if (!FnDecl->isReplaceableGlobalAllocationFunction()) {
+  S.Diag(R.getNameLoc(), diag::err_builtin_operator_new_delete_not_usual)
+  << (IsDelete ? 1 : 0) << Range;
+  S.Diag(FnDecl->getLocation(), diag::note_non_usual_function_declared_here)
+  << R.getLookupName() << FnDecl->getSourceRange();
+  return true;
+}
+
+Operator = FnDecl;
+return false;
+  }
+
+  case OR_No_Viable_Function:
+S.Diag(R.getNameLoc(), diag::err_ovl_no_viable_function_in_call)
+<< R.getLookupName() << Range;
+Candidates.NoteCandidates(S, OCD_AllCandidates, Args);
+return true;
+
+  case OR_Ambiguous:
+S.Diag(R.getNameLoc(), diag::err_ovl_ambiguous_call)
+<< R.getLookupName() << Range;
+Candidates.NoteCandidates(S, OCD_ViableCandidates, Args);
+return true;
+
+  case OR_Deleted: {
+S.Diag(R.getNameLoc(), diag::err_ovl_deleted_call)
+<< Best->Function->isDeleted() << R.getLookupName()
+<< S.getDeletedOrUnavailableSuffix(Best->Function) << Range;
+Candidates.NoteCandidates(S, OCD_AllCandidates, Args);
+return true;
+  }
+  }
+  llvm_unreachable("Unreachable, bad result from BestViableFunction");
+}
+
+ExprResult
+Sema::SemaBuiltinOperatorNewDeleteOverloaded(ExprResult TheCallResult,
+ bool IsDelete) {
+  CallExpr *TheCall = cast(TheCallResult.get());
+  if (!getLangOpts().CPlusPlus) {
+Diag(TheCall->getExp

[PATCH] D43047: [Builtins] Overload __builtin_operator_new/delete to allow forwarding to usual allocation/deallocation functions.

[Eric Fiselier via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL328134: [Builtins] Overload __builtin_operator_new/delete to 
allow forwarding to usual… (authored by EricWF, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D43047?vs=139347&id=139350#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D43047

Files:
  cfe/trunk/include/clang/Basic/Builtins.def
  cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
  cfe/trunk/include/clang/Sema/Sema.h
  cfe/trunk/lib/CodeGen/CGBuiltin.cpp
  cfe/trunk/lib/CodeGen/CGExprCXX.cpp
  cfe/trunk/lib/CodeGen/CodeGenFunction.h
  cfe/trunk/lib/Lex/PPMacroExpansion.cpp
  cfe/trunk/lib/Sema/SemaChecking.cpp
  cfe/trunk/lib/Sema/SemaExprCXX.cpp
  cfe/trunk/test/CodeGenCXX/builtin-operator-new-delete.cpp
  cfe/trunk/test/SemaCXX/builtin-operator-new-delete.cpp

Index: cfe/trunk/lib/Lex/PPMacroExpansion.cpp
===
--- cfe/trunk/lib/Lex/PPMacroExpansion.cpp
+++ cfe/trunk/lib/Lex/PPMacroExpansion.cpp
@@ -1801,12 +1801,21 @@
   [this](Token &Tok, bool &HasLexedNextToken) -> int {
 IdentifierInfo *II = ExpectFeatureIdentifierInfo(Tok, *this,
diag::err_feature_check_malformed);
+const LangOptions &LangOpts = getLangOpts();
 if (!II)
   return false;
-else if (II->getBuiltinID() != 0)
+else if (II->getBuiltinID() != 0) {
+  switch (II->getBuiltinID()) {
+  case Builtin::BI__builtin_operator_new:
+  case Builtin::BI__builtin_operator_delete:
+// denotes date of behavior change to support calling arbitrary
+// usual allocation and deallocation functions. Required by libc++
+return 201802;
+  default:
+return true;
+  }
   return true;
-else {
-  const LangOptions &LangOpts = getLangOpts();
+} else {
   return llvm::StringSwitch(II->getName())
   .Case("__make_integer_seq", LangOpts.CPlusPlus)
   .Case("__type_pack_element", LangOpts.CPlusPlus)
Index: cfe/trunk/lib/Sema/SemaChecking.cpp
===
--- cfe/trunk/lib/Sema/SemaChecking.cpp
+++ cfe/trunk/lib/Sema/SemaChecking.cpp
@@ -1097,20 +1097,14 @@
   return ExprError();
 break;
   case Builtin::BI__builtin_operator_new:
-  case Builtin::BI__builtin_operator_delete:
-if (!getLangOpts().CPlusPlus) {
-  Diag(TheCall->getExprLoc(), diag::err_builtin_requires_language)
-<< (BuiltinID == Builtin::BI__builtin_operator_new
-? "__builtin_operator_new"
-: "__builtin_operator_delete")
-<< "C++";
-  return ExprError();
-}
-// CodeGen assumes it can find the global new and delete to call,
-// so ensure that they are declared.
-DeclareGlobalNewDelete();
-break;
-
+  case Builtin::BI__builtin_operator_delete: {
+bool IsDelete = BuiltinID == Builtin::BI__builtin_operator_delete;
+ExprResult Res =
+SemaBuiltinOperatorNewDeleteOverloaded(TheCallResult, IsDelete);
+if (Res.isInvalid())
+  CorrectDelayedTyposInExpr(TheCallResult.get());
+return Res;
+  }
   // check secure string manipulation functions where overflows
   // are detectable at compile time
   case Builtin::BI__builtin___memcpy_chk:
Index: cfe/trunk/lib/Sema/SemaExprCXX.cpp
===
--- cfe/trunk/lib/Sema/SemaExprCXX.cpp
+++ cfe/trunk/lib/Sema/SemaExprCXX.cpp
@@ -1443,7 +1443,7 @@
   CUDAPref = S.IdentifyCUDAPreference(Caller, FD);
 }
 
-operator bool() const { return FD; }
+explicit operator bool() const { return FD; }
 
 bool isBetterThan(const UsualDeallocFnInfo &Other, bool WantSize,
   bool WantAlign) const {
@@ -2271,7 +2271,6 @@
   llvm_unreachable("Unreachable, bad result from BestViableFunction");
 }
 
-
 /// FindAllocationFunctions - Finds the overloads of operator new and delete
 /// that are appropriate for the allocation.
 bool Sema::FindAllocationFunctions(SourceLocation StartLoc, SourceRange Range,
@@ -3343,6 +3342,128 @@
   return Result;
 }
 
+static bool resolveBuiltinNewDeleteOverload(Sema &S, CallExpr *TheCall,
+bool IsDelete,
+FunctionDecl *&Operator) {
+
+  DeclarationName NewName = S.Context.DeclarationNames.getCXXOperatorName(
+  IsDelete ? OO_Delete : OO_New);
+
+  LookupResult R(S, NewName, TheCall->getLocStart(), Sema::LookupOrdinaryName);
+  S.LookupQualifiedName(R, S.Context.getTranslationUnitDecl());
+  assert(!R.empty() && "implicitly declared allocation functions not found");
+  assert(!R.isAmbiguous() && "global allocation functions are ambiguous");
+
+  // We do our own custom access

[PATCH] D44720: [clangd] Simplify fuzzy matcher (sequence alignment) by removing some condition checks.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clangd/FuzzyMatch.h:61
   bool allowMatch(int P, int W) const;
-  int skipPenalty(int W, Action Last) const;
-  int matchBonus(int P, int W, Action Last) const;
+  int missScore(int W, Action Last) const;
+  int matchScore(int P, int W, Action Last) const;

MaskRay wrote:
> sammccall wrote:
> > FWIW, I don't think this is an improvement - I think the clarity of purpose 
> > in names is more useful than having consistent signs in this case.
> Keep `matchBonus` but rename `skipPenalty` to `missPenalty` ?
Also note in the original scheme, the skip score does not need to be negative. 
Because Scores[PatN][WordN][] is used and each path takes the same number of 
positions (WordN). We can add an offset to all positional scores to make all of 
them non-negative. In the new scheme it does make sense to make them negative, 
as each path has now different number of positions.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44720



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

[PATCH] D44272: [clangd] Support incremental document syncing

[Simon Marchi via Phabricator via cfe-commits]

simark marked 6 inline comments as done.
simark added inline comments.
Herald added a subscriber: MaskRay.



Comment at: unittests/clangd/DraftStoreTests.cpp:27
+
+static int rangeLength(StringRef Code, const Range &Rng) {
+  size_t Start = positionToOffset(Code, Rng.start);

ilya-biryukov wrote:
> ilya-biryukov wrote:
> > no need for `static`, since function is already inside an anonymous 
> > namespace.
> > remove it?
> This comment is marked as done, but not addressed.
Hmm I addressed the comments at two different times, but I probably messed up 
my branches or something, so I lost the changes of my first pass.  Sorry about 
that.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44272



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

[PATCH] D44272: [clangd] Support incremental document syncing

[Simon Marchi via Phabricator via cfe-commits]

simark updated this revision to Diff 139355.
simark marked an inline comment as done.
simark added a comment.

Address review comments I failed to address previously


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44272

Files:
  clangd/ClangdLSPServer.cpp
  clangd/DraftStore.cpp
  clangd/DraftStore.h
  clangd/Protocol.cpp
  clangd/Protocol.h
  test/clangd/initialize-params-invalid.test
  test/clangd/initialize-params.test
  test/clangd/textdocument-didchange-fail.test
  unittests/clangd/CMakeLists.txt
  unittests/clangd/DraftStoreTests.cpp

Index: unittests/clangd/DraftStoreTests.cpp
===
--- /dev/null
+++ unittests/clangd/DraftStoreTests.cpp
@@ -0,0 +1,353 @@
+//===-- DraftStoreTests.cpp - Clangd unit tests -*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "Annotations.h"
+#include "DraftStore.h"
+#include "SourceCode.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+using namespace llvm;
+
+struct IncrementalTestStep {
+  StringRef Src;
+  StringRef Contents;
+};
+
+int rangeLength(StringRef Code, const Range &Rng) {
+  llvm::Expected Start = positionToOffset(Code, Rng.start);
+  llvm::Expected End = positionToOffset(Code, Rng.end);
+  assert(Start);
+  assert(End);
+  return *End - *Start;
+}
+
+/// Send the changes one by one to updateDraft, verify the intermediate results.
+static void stepByStep(llvm::ArrayRef Steps) {
+  DraftStore DS;
+  Annotations InitialSrc(Steps.front().Src);
+  constexpr StringLiteral Path("/hello.cpp");
+
+  // Set the initial content.
+  DS.addDraft(Path, InitialSrc.code());
+
+  for (size_t i = 1; i < Steps.size(); i++) {
+Annotations SrcBefore(Steps[i - 1].Src);
+Annotations SrcAfter(Steps[i].Src);
+StringRef Contents = Steps[i - 1].Contents;
+TextDocumentContentChangeEvent Event{
+SrcBefore.range(),
+rangeLength(SrcBefore.code(), SrcBefore.range()),
+Contents.str(),
+};
+
+llvm::Expected Result = DS.updateDraft(Path, {Event});
+ASSERT_TRUE(!!Result);
+EXPECT_EQ(*Result, SrcAfter.code());
+EXPECT_EQ(*DS.getDraft(Path), SrcAfter.code());
+  }
+}
+
+/// Send all the changes at once to updateDraft, check only the final result.
+static void allAtOnce(llvm::ArrayRef Steps) {
+  DraftStore DS;
+  Annotations InitialSrc(Steps.front().Src);
+  Annotations FinalSrc(Steps.back().Src);
+  const char Path[] = "/hello.cpp";
+  std::vector Changes;
+
+  for (size_t i = 0; i < Steps.size() - 1; i++) {
+Annotations Src(Steps[i].Src);
+StringRef Contents = Steps[i].Contents;
+
+Changes.push_back({
+Src.range(),
+rangeLength(Src.code(), Src.range()),
+Contents.str(),
+});
+  }
+
+  // Set the initial content.
+  DS.addDraft(Path, InitialSrc.code());
+
+  llvm::Expected Result = DS.updateDraft(Path, Changes);
+
+  ASSERT_TRUE(!!Result) << llvm::toString(Result.takeError());
+  EXPECT_EQ(*Result, FinalSrc.code());
+  EXPECT_EQ(*DS.getDraft(Path), FinalSrc.code());
+}
+
+TEST(DraftStoreIncrementalUpdateTest, Simple) {
+  // clang-format off
+  IncrementalTestStep Steps[] =
+{
+  // Replace a range
+  {
+R"cpp(static int
+hello[[World]]()
+{})cpp",
+"Universe"
+  },
+  // Delete a range
+  {
+R"cpp(static int
+hello[[Universe]]()
+{})cpp",
+""
+  },
+  // Add a range
+  {
+R"cpp(static int
+hello[[]]()
+{})cpp",
+"Monde"
+  },
+  {
+R"cpp(static int
+helloMonde()
+{})cpp",
+""
+  }
+};
+  // clang-format on
+
+  stepByStep(Steps);
+  allAtOnce(Steps);
+}
+
+TEST(DraftStoreIncrementalUpdateTest, MultiLine) {
+  // clang-format off
+  IncrementalTestStep Steps[] =
+{
+  // Replace a range
+  {
+R"cpp(static [[int
+helloWorld]]()
+{})cpp",
+R"cpp(char
+welcome)cpp"
+  },
+  // Delete a range
+  {
+R"cpp(static char[[
+welcome]]()
+{})cpp",
+""
+  },
+  // Add a range
+  {
+R"cpp(static char[[]]()
+{})cpp",
+R"cpp(
+cookies)cpp"
+  },
+  // Replace the whole file
+  {
+R"cpp([[static char
+cookies()
+{}]])cpp",
+R"cpp(#include 
+)cpp"
+  },
+  // Delete the whole file
+  {
+R"cpp([[#include 
+]])cpp",
+"",
+  },
+  // Add something to an empty file
+  {
+"[[]]",
+R"cpp(int main() {
+)cpp",
+  },
+  {
+R"cpp(int main() {
+)cpp",
+""
+  }
+};
+  // clang-format on
+
+  stepByStep(Steps);
+  allAtOnce(Steps);
+}
+
+TEST(DraftStoreIncrementalUpdateTest, WrongRangeLength) {
+  DraftStore DS;
+  Path File = "foo.cpp";
+
+  DS.addDraft(File, "int main() {}\n");
+
+  Tex

[PATCH] D44753: [Preprocessor] Rename __is_{target -> host}_* function-like builtin macros

[John Ericson via Phabricator via cfe-commits]

Ericson2314 created this revision.
Ericson2314 added reviewers: arphaman, compnerd, dexonsmith, bob.wilson, 
steven_wu.
Herald added subscribers: cfe-commits, javed.absar.

Per my belated [reply] to the mailing list, I believe the "target"
nomenclature incorrect for cross compilation.

[reply]: http://lists.llvm.org/pipermail/cfe-dev/2018-March/057258.html


Repository:
  rC Clang

https://reviews.llvm.org/D44753

Files:
  include/clang/Lex/Preprocessor.h
  lib/Lex/PPMacroExpansion.cpp
  test/Preprocessor/is_host.c
  test/Preprocessor/is_host_arm.c
  test/Preprocessor/is_host_arm64.c
  test/Preprocessor/is_host_environment_version.c
  test/Preprocessor/is_host_os_darwin.c
  test/Preprocessor/is_host_unknown.c
  test/Preprocessor/is_target.c
  test/Preprocessor/is_target_arm.c
  test/Preprocessor/is_target_arm64.c
  test/Preprocessor/is_target_environment_version.c
  test/Preprocessor/is_target_os_darwin.c
  test/Preprocessor/is_target_unknown.c

Index: test/Preprocessor/is_target.c
===
--- test/Preprocessor/is_target.c
+++ /dev/null
@@ -1,67 +0,0 @@
-// RUN: %clang_cc1 -fsyntax-only -triple x86_64-apple-darwin-simulator -verify %s
-
-#if !__is_target_arch(x86_64) || !__is_target_arch(X86_64)
-  #error "mismatching arch"
-#endif
-
-#if __is_target_arch(arm64)
-  #error "mismatching arch"
-#endif
-
-// Silently ignore invalid archs. This will ensure that older compilers will
-// accept headers that support new arches/vendors/os variants.
-#if __is_target_arch(foo)
-  #error "invalid arch"
-#endif
-
-#if !__is_target_vendor(apple) || !__is_target_vendor(APPLE)
-  #error "mismatching vendor"
-#endif
-
-#if __is_target_vendor(unknown)
-  #error "mismatching vendor"
-#endif
-
-#if __is_target_vendor(foo)
-  #error "invalid vendor"
-#endif
-
-#if !__is_target_os(darwin) || !__is_target_os(DARWIN)
-  #error "mismatching os"
-#endif
-
-#if __is_target_os(ios)
-  #error "mismatching os"
-#endif
-
-#if __is_target_os(foo)
-  #error "invalid os"
-#endif
-
-#if !__is_target_environment(simulator) || !__is_target_environment(SIMULATOR)
-  #error "mismatching environment"
-#endif
-
-#if __is_target_environment(unknown)
-  #error "mismatching environment"
-#endif
-
-#if __is_target_environment(foo)
-  #error "invalid environment"
-#endif
-
-#if !__has_builtin(__is_target_arch) || !__has_builtin(__is_target_os) || !__has_builtin(__is_target_vendor) || !__has_builtin(__is_target_environment)
-  #error "has builtin doesn't work"
-#endif
-
-#if __is_target_arch(11) // expected-error {{builtin feature check macro requires a parenthesized identifier}}
-  #error "invalid arch"
-#endif
-
-#if __is_target_arch x86 // expected-error{{missing '(' after '__is_target_arch'}}
-  #error "invalid arch"
-#endif
-
-#if __is_target_arch ( x86  // expected-error {{unterminated function-like macro invocation}}
-  #error "invalid arch"
-#endif // expected-error@-2 {{expected value in expression}}
Index: test/Preprocessor/is_host_unknown.c
===
--- test/Preprocessor/is_host_unknown.c
+++ test/Preprocessor/is_host_unknown.c
@@ -2,21 +2,21 @@
 // RUN: %clang_cc1 -fsyntax-only -triple i686-- -verify %s
 // expected-no-diagnostics
 
-#if __is_target_arch(unknown)
+#if __is_host_arch(unknown)
   #error "mismatching arch"
 #endif
 
 // Unknown vendor is allowed.
-#if !__is_target_vendor(unknown)
+#if !__is_host_vendor(unknown)
   #error "mismatching vendor"
 #endif
 
 // Unknown OS is allowed.
-#if !__is_target_os(unknown)
+#if !__is_host_os(unknown)
   #error "mismatching OS"
 #endif
 
 // Unknown environment is allowed.
-#if !__is_target_environment(unknown)
+#if !__is_host_environment(unknown)
   #error "mismatching environment"
 #endif
Index: test/Preprocessor/is_host_os_darwin.c
===
--- test/Preprocessor/is_host_os_darwin.c
+++ test/Preprocessor/is_host_os_darwin.c
@@ -4,22 +4,22 @@
 // RUN: %clang_cc1 -fsyntax-only -triple x86_64-apple-watchos -verify %s
 // expected-no-diagnostics
 
-#if !__is_target_os(darwin)
+#if !__is_host_os(darwin)
   #error "mismatching os"
 #endif
 
 // macOS matches both macOS and macOSX.
 #ifdef MAC
 
-#if !__is_target_os(macos)
+#if !__is_host_os(macos)
   #error "mismatching os"
 #endif
 
-#if !__is_target_os(macosx)
+#if !__is_host_os(macosx)
   #error "mismatching os"
 #endif
 
-#if __is_target_os(ios)
+#if __is_host_os(ios)
   #error "mismatching os"
 #endif
 
Index: test/Preprocessor/is_host_environment_version.c
===
--- test/Preprocessor/is_host_environment_version.c
+++ test/Preprocessor/is_host_environment_version.c
@@ -1,6 +1,6 @@
 // RUN: %clang_cc1 -fsyntax-only -triple x86_64-pc-windows-msvc18.0.0 -verify %s
 // expected-no-diagnostics
 
-#if !__is_target_environment(msvc)
+#if !__is_host_environment(msvc)
   #error "mismatching environment"
 #endif
Inde

[PATCH] D44747: [AMDGPU] Set calling convention for CUDA kernel

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Is there a reason for this to be done as a special case in IRGen instead of 
just implicitly applying the calling convention in Sema?


https://reviews.llvm.org/D44747



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