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

[Clement Courbet via Phabricator via cfe-commits]

courbet added a comment.

ping


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

Re: [clang-tools-extra] r329448 - [clang-tidy] Check if grand-..parent's virtual method was called instead of overridden parent's.

[Mikael Holmén via cfe-commits]

Hi Zivony,

From this patch:

> +void ParentVirtualCallCheck::check(const MatchFinder::MatchResult 
&Result) {
> +  const auto *MatchedDecl = 
Result.Nodes.getNodeAs("call");

> +  assert(MatchedDecl);
> +


The code above yields a warning when compiled without asserts since 
MatchedDecl is not used then:


../tools/clang/tools/extra/clang-tidy/bugprone/ParentVirtualCallCheck.cpp:101:15: 
error: unused variable 'MatchedDecl' [-Werror,-Wunused-variable]
  const auto *MatchedDecl = 
Result.Nodes.getNodeAs("call");

  ^
1 error generated.

Regards,
Mikael



On 04/06/2018 10:02 PM, Zinovy Nis via cfe-commits wrote:

Author: zinovy.nis
Date: Fri Apr  6 13:02:50 2018
New Revision: 329448

URL: http://llvm.org/viewvc/llvm-project?rev=329448&view=rev
Log:
[clang-tidy] Check if grand-..parent's virtual method was called instead of 
overridden parent's.

class A {...int virtual foo() {...}...};
class B: public A {...int foo() override {...}...};
class C: public B {...int foo() override {... A::foo()...}};
    warning: qualified name 
A::foo refers to a member overridden in subclass; did you mean 'B'? 
[bugprone-parent-virtual-call]

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

Added:
 clang-tools-extra/trunk/clang-tidy/bugprone/ParentVirtualCallCheck.cpp   
(with props)
 clang-tools-extra/trunk/clang-tidy/bugprone/ParentVirtualCallCheck.h   
(with props)
 
clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-parent-virtual-call.rst 
  (with props)
 clang-tools-extra/trunk/test/clang-tidy/bugprone-parent-virtual-call.cpp   
(with props)
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=329448&r1=329447&r2=329448&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp Fri Apr  
6 13:02:50 2018
@@ -28,6 +28,7 @@
  #include "MisplacedWideningCastCheck.h"
  #include "MoveForwardingReferenceCheck.h"
  #include "MultipleStatementMacroCheck.h"
+#include "ParentVirtualCallCheck.h"
  #include "SizeofContainerCheck.h"
  #include "SizeofExpressionCheck.h"
  #include "StringConstructorCheck.h"
@@ -90,6 +91,8 @@ public:
  "bugprone-move-forwarding-reference");
  CheckFactories.registerCheck(
  "bugprone-multiple-statement-macro");
+CheckFactories.registerCheck(
+"bugprone-parent-virtual-call");
  CheckFactories.registerCheck(
  "bugprone-sizeof-container");
  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=329448&r1=329447&r2=329448&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt Fri Apr  6 
13:02:50 2018
@@ -20,6 +20,7 @@ add_clang_library(clangTidyBugproneModul
MisplacedWideningCastCheck.cpp
MoveForwardingReferenceCheck.cpp
MultipleStatementMacroCheck.cpp
+  ParentVirtualCallCheck.cpp
SizeofContainerCheck.cpp
SizeofExpressionCheck.cpp
StringConstructorCheck.cpp

Added: clang-tools-extra/trunk/clang-tidy/bugprone/ParentVirtualCallCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/bugprone/ParentVirtualCallCheck.cpp?rev=329448&view=auto
==
--- clang-tools-extra/trunk/clang-tidy/bugprone/ParentVirtualCallCheck.cpp 
(added)
+++ clang-tools-extra/trunk/clang-tidy/bugprone/ParentVirtualCallCheck.cpp Fri 
Apr  6 13:02:50 2018
@@ -0,0 +1,154 @@
+//===--- ParentVirtualCallCheck.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 "ParentVirtualCallCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Tooling/FixIt.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallVector.h"
+#include 
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+using BasesVector = llvm::SmallVector;

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

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

avt77 abandoned this revision.
avt77 added a comment.

It is not a bug.


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] r329550 - Fix unused variable warning.

[Chandler Carruth via cfe-commits]

Author: chandlerc
Date: Mon Apr  9 00:26:42 2018
New Revision: 329550

URL: http://llvm.org/viewvc/llvm-project?rev=329550&view=rev
Log:
Fix unused variable warning.

Modified:
clang-tools-extra/trunk/clang-tidy/bugprone/ParentVirtualCallCheck.cpp

Modified: clang-tools-extra/trunk/clang-tidy/bugprone/ParentVirtualCallCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/bugprone/ParentVirtualCallCheck.cpp?rev=329550&r1=329549&r2=329550&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/bugprone/ParentVirtualCallCheck.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/bugprone/ParentVirtualCallCheck.cpp Mon 
Apr  9 00:26:42 2018
@@ -99,6 +99,7 @@ void ParentVirtualCallCheck::registerMat
 
 void ParentVirtualCallCheck::check(const MatchFinder::MatchResult &Result) {
   const auto *MatchedDecl = Result.Nodes.getNodeAs("call");
+  (void)MatchedDecl;
   assert(MatchedDecl);
 
   const auto *Member = Result.Nodes.getNodeAs("member");


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

Re: [clang-tools-extra] r329550 - Fix unused variable warning.

[Zinovy Nis via cfe-commits]

Thanks, Chandler.

   assert(Result.Nodes.getNodeAs("call"));

would also be fine.

пн, 9 апр. 2018 г. в 10:29, Chandler Carruth via cfe-commits <
cfe-commits@lists.llvm.org>:

> Author: chandlerc
> Date: Mon Apr  9 00:26:42 2018
> New Revision: 329550
>
> URL: http://llvm.org/viewvc/llvm-project?rev=329550&view=rev
> Log:
> Fix unused variable warning.
>
> Modified:
> clang-tools-extra/trunk/clang-tidy/bugprone/ParentVirtualCallCheck.cpp
>
> Modified:
> clang-tools-extra/trunk/clang-tidy/bugprone/ParentVirtualCallCheck.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/bugprone/ParentVirtualCallCheck.cpp?rev=329550&r1=329549&r2=329550&view=diff
>
> ==
> --- clang-tools-extra/trunk/clang-tidy/bugprone/ParentVirtualCallCheck.cpp
> (original)
> +++ clang-tools-extra/trunk/clang-tidy/bugprone/ParentVirtualCallCheck.cpp
> Mon Apr  9 00:26:42 2018
> @@ -99,6 +99,7 @@ void ParentVirtualCallCheck::registerMat
>
>  void ParentVirtualCallCheck::check(const MatchFinder::MatchResult
> &Result) {
>const auto *MatchedDecl =
> Result.Nodes.getNodeAs("call");
> +  (void)MatchedDecl;
>assert(MatchedDecl);
>
>const auto *Member = Result.Nodes.getNodeAs("member");
>
>
> ___
> 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

Re: [clang-tools-extra] r329550 - Fix unused variable warning.

[Chandler Carruth via cfe-commits]

I have no opinion other than removing the warning. =D This seemed to be the
idiomatic pattern in the rest of LLVM and Clang. If someone wants to adjust
this locally, by all means.

On Mon, Apr 9, 2018 at 12:47 AM Zinovy Nis via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Thanks, Chandler.
>
>assert(Result.Nodes.getNodeAs("call"));
>
> would also be fine.
>
> пн, 9 апр. 2018 г. в 10:29, Chandler Carruth via cfe-commits <
> cfe-commits@lists.llvm.org>:
>
>> Author: chandlerc
>> Date: Mon Apr  9 00:26:42 2018
>> New Revision: 329550
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=329550&view=rev
>> Log:
>> Fix unused variable warning.
>>
>> Modified:
>> clang-tools-extra/trunk/clang-tidy/bugprone/ParentVirtualCallCheck.cpp
>>
>> Modified:
>> clang-tools-extra/trunk/clang-tidy/bugprone/ParentVirtualCallCheck.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/bugprone/ParentVirtualCallCheck.cpp?rev=329550&r1=329549&r2=329550&view=diff
>>
>> ==
>> ---
>> clang-tools-extra/trunk/clang-tidy/bugprone/ParentVirtualCallCheck.cpp
>> (original)
>> +++
>> clang-tools-extra/trunk/clang-tidy/bugprone/ParentVirtualCallCheck.cpp Mon
>> Apr  9 00:26:42 2018
>> @@ -99,6 +99,7 @@ void ParentVirtualCallCheck::registerMat
>>
>>  void ParentVirtualCallCheck::check(const MatchFinder::MatchResult
>> &Result) {
>>const auto *MatchedDecl =
>> Result.Nodes.getNodeAs("call");
>> +  (void)MatchedDecl;
>>assert(MatchedDecl);
>>
>>const auto *Member = Result.Nodes.getNodeAs("member");
>>
>>
>> ___
>> 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
>
___
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.

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

Hi,

my 2 cents:

- On which codebases did you run the check?
- did you consider looking for `implicitCastExpr`? You can capture all 
narrowing conversion with that and analyze them further. I think it is possible 
to warn for the subset mentioned in the guidelines.
- you match for `binaryOperator("+=", "-")` maybe all assignments can be looked 
at?  (`binaryOperator(isASsignmentOperator())`, defined in 
clang-tidy/util/Matchers.h or similar) That includes all calculate-and-assign 
operations. Those should be equally dangerous.


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] D44888: [RISCV] Default enable linker relaxation and add -mrelax, -mno-relax flags

[Shiva Chen via Phabricator via cfe-commits]

shiva0217 updated this revision to Diff 141591.
shiva0217 added a comment.

Update patch to address Alex's comments.


Repository:
  rL LLVM

https://reviews.llvm.org/D44888

Files:
  include/clang/Driver/Options.td
  lib/Driver/ToolChains/Arch/RISCV.cpp
  test/Driver/riscv-features.c


Index: test/Driver/riscv-features.c
===
--- test/Driver/riscv-features.c
+++ test/Driver/riscv-features.c
@@ -2,3 +2,10 @@
 // RUN: %clang -target riscv64-unknown-elf -### %s -fsyntax-only 2>&1 | 
FileCheck %s
 
 // CHECK: fno-signed-char
+
+
+// RUN: %clang -target riscv32-unknown-elf -### %s -mrelax 2>&1 | FileCheck %s 
-check-prefix=RELAX
+// RUN: %clang -target riscv32-unknown-elf -### %s -mno-relax 2>&1 | FileCheck 
%s -check-prefix=NO-RELAX
+
+// RELAX: "-target-feature" "+relax"
+// NO-RELAX: "-target-feature" "-relax"
Index: lib/Driver/ToolChains/Arch/RISCV.cpp
===
--- lib/Driver/ToolChains/Arch/RISCV.cpp
+++ lib/Driver/ToolChains/Arch/RISCV.cpp
@@ -14,6 +14,7 @@
 #include "llvm/Option/ArgList.h"
 #include "llvm/Support/TargetParser.h"
 #include "llvm/Support/raw_ostream.h"
+#include "ToolChains/CommonArgs.h"
 
 using namespace clang::driver;
 using namespace clang::driver::tools;
@@ -112,6 +113,21 @@
 if (HasD && !HasF)
   D.Diag(diag::err_drv_invalid_arch_name) << MArch;
   }
+
+  // -mrelax is default, unless -mno-relax is specified.
+  bool Relax = true;
+  if (auto *A = Args.getLastArg(options::OPT_mrelax, options::OPT_mno_relax))
+if (A->getOption().matches(options::OPT_mno_relax)) {
+  Relax = false;
+  Features.push_back("-relax");
+}
+
+  if (Relax)
+Features.push_back("+relax");
+
+  // Now add any that the user explicitly requested on the command line,
+  // which may override the defaults.
+  handleTargetFeaturesGroup(Args, Features, 
options::OPT_m_riscv_Features_Group);
 }
 
 StringRef riscv::getRISCVABI(const ArgList &Args, const llvm::Triple &Triple) {
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -150,6 +150,9 @@
 def m_x86_Features_Group : OptionGroup<"">,
Group, Flags<[CoreOption]>, DocName<"X86">;
 
+def m_riscv_Features_Group : OptionGroup<"">,
+ Group, DocName<"RISCV">;
+
 def m_libc_Group : OptionGroup<"">, Group,
Flags<[HelpHidden]>;
 
@@ -1880,6 +1883,11 @@
 def meabi : Separate<["-"], "meabi">, Group, Flags<[CC1Option]>,
   HelpText<"Set EABI type, e.g. 4, 5 or gnu (default depends on triple)">, 
Values<"default,4,5,gnu">;
 
+def mrelax : Flag<["-"], "mrelax">,
+Group;
+def mno_relax : Flag<["-"], "mno-relax">,
+Group;
+
 def mno_constant_cfstrings : Flag<["-"], "mno-constant-cfstrings">, 
Group;
 def mno_global_merge : Flag<["-"], "mno-global-merge">, Group, 
Flags<[CC1Option]>,
   HelpText<"Disable merging of globals">;


Index: test/Driver/riscv-features.c
===
--- test/Driver/riscv-features.c
+++ test/Driver/riscv-features.c
@@ -2,3 +2,10 @@
 // RUN: %clang -target riscv64-unknown-elf -### %s -fsyntax-only 2>&1 | FileCheck %s
 
 // CHECK: fno-signed-char
+
+
+// RUN: %clang -target riscv32-unknown-elf -### %s -mrelax 2>&1 | FileCheck %s -check-prefix=RELAX
+// RUN: %clang -target riscv32-unknown-elf -### %s -mno-relax 2>&1 | FileCheck %s -check-prefix=NO-RELAX
+
+// RELAX: "-target-feature" "+relax"
+// NO-RELAX: "-target-feature" "-relax"
Index: lib/Driver/ToolChains/Arch/RISCV.cpp
===
--- lib/Driver/ToolChains/Arch/RISCV.cpp
+++ lib/Driver/ToolChains/Arch/RISCV.cpp
@@ -14,6 +14,7 @@
 #include "llvm/Option/ArgList.h"
 #include "llvm/Support/TargetParser.h"
 #include "llvm/Support/raw_ostream.h"
+#include "ToolChains/CommonArgs.h"
 
 using namespace clang::driver;
 using namespace clang::driver::tools;
@@ -112,6 +113,21 @@
 if (HasD && !HasF)
   D.Diag(diag::err_drv_invalid_arch_name) << MArch;
   }
+
+  // -mrelax is default, unless -mno-relax is specified.
+  bool Relax = true;
+  if (auto *A = Args.getLastArg(options::OPT_mrelax, options::OPT_mno_relax))
+if (A->getOption().matches(options::OPT_mno_relax)) {
+  Relax = false;
+  Features.push_back("-relax");
+}
+
+  if (Relax)
+Features.push_back("+relax");
+
+  // Now add any that the user explicitly requested on the command line,
+  // which may override the defaults.
+  handleTargetFeaturesGroup(Args, Features, options::OPT_m_riscv_Features_Group);
 }
 
 StringRef riscv::getRISCVABI(const ArgList &Args, const llvm::Triple &Triple) {
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@

[PATCH] D41938: [Analyzer] SValBuilder Comparison Rearrangement (with Restrictions and Analyzer Option)

[Balogh , Ádám via Phabricator via cfe-commits]

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

George or Devin, please accept it or give me some feedback if not. Since this 
patch affects the core infrastructure I think it is wise to merge it only if at 
least two of you have accepted it. Artem is one, I need a second one as well.


https://reviews.llvm.org/D41938



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

[PATCH] D45409: [cmake] Include LLVMTestingSupport when doing stand-alone build

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a subscriber: sammccall.
ilya-biryukov added a comment.

This LG to fix the standalone build.
However, why not install LLVMTestingSupport in llvm? It feels that we should 
either include it or disable the tests for standalone builds. And disabling the 
tests seems wrong.
WDYT?
@sammccall, in case he also has an opinion on this.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45409



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

[PATCH] D45285: [clangd-vscode] Update VScode dependencies

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a subscriber: hokein.
ilya-biryukov added a comment.

In https://reviews.llvm.org/D45285#1058567, @malaperle wrote:

> Do we need to bump the version of the extension and do a new release or 
> anything like that? Or leave this for later?


We should bump the version and republish the extension into VSCode marketplace. 
@hokein has more context on how to properly do that.

In https://reviews.llvm.org/D45285#1058578, @MaskRay wrote:

> Do we really want to keep editor plugins in the repository?


As long as we are the ones who own it, probably yes. Any downsides to keeping 
it here?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45285



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

[PATCH] D43764: [clang-apply-replacements] Convert tooling::Replacements to tooling::AtomicChange for conflict resolving of changes, code cleanup, and code formatting.

[Malcolm Parsons via Phabricator via cfe-commits]

malcolm.parsons added a comment.

In https://reviews.llvm.org/D43764#1061042, @jdemeule wrote:

> I suspect some undefined order when applying replacements as directly 
> dependent on which order OS reads `file1.yaml`, `file2.yaml`and `file3.yaml`.


$ ls -f 
/mparsons/llvm/llvm-build/tools/clang/tools/extra/test/clang-apply-replacements/Output/Inputs/conflict/
file1.yaml  ..  output.txt  .  file3.yaml  file2.yaml  expected.txt

> As I am not able to reproduce it on my machine, can you share with me the 
> content of `output.txt`?

The new replacement overlaps with an existing replacement.
New replacement: 
/mparsons/llvm/llvm/tools/clang/tools/extra/test/clang-apply-replacements/Inputs/conflict/common.h:
 169:+0:"(int*)"
Existing replacement: 
/mparsons/llvm/llvm/tools/clang/tools/extra/test/clang-apply-replacements/Inputs/conflict/common.h:
 160:+12:""
The new replacement overlaps with an existing replacement.
New replacement: 
/mparsons/llvm/llvm/tools/clang/tools/extra/test/clang-apply-replacements/Inputs/conflict/common.h:
 106:+26:"int & elem : ints"
Existing replacement: 
/mparsons/llvm/llvm/tools/clang/tools/extra/test/clang-apply-replacements/Inputs/conflict/common.h:
 106:+26:"auto & i : ints"
The new replacement overlaps with an existing replacement.
New replacement: 
/mparsons/llvm/llvm/tools/clang/tools/extra/test/clang-apply-replacements/Inputs/conflict/common.h:
 140:+7:"elem"
Existing replacement: 
/mparsons/llvm/llvm/tools/clang/tools/extra/test/clang-apply-replacements/Inputs/conflict/common.h:
 140:+7:"i"
The new replacement overlaps with an existing replacement.
New replacement: 
/mparsons/llvm/llvm/tools/clang/tools/extra/test/clang-apply-replacements/Inputs/conflict/common.h:
 169:+1:"nullptr"
Existing replacement: 
/mparsons/llvm/llvm/tools/clang/tools/extra/test/clang-apply-replacements/Inputs/conflict/common.h:
 160:+12:""


https://reviews.llvm.org/D43764



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

[PATCH] D45409: [cmake] Include LLVMTestingSupport when doing stand-alone build

[Sam McCall via Phabricator via cfe-commits]

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

This seems fine to me too to fix the immediate issue.

Personally I'd rather drop our usage of llvmtestingsupport as it provides 
marginal value anyway. But also happy to do this as a cleanup afterwards.

Installing it in llvm seems like a good idea to me too, but outside my area.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45409



___
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 a comment.

Hi Jonas,

In https://reviews.llvm.org/D38455#1061228, @JonasToth wrote:

> Hi,
>
> my 2 cents:
>
> - On which codebases did you run the check?


A large repository of open-source code, plus internal code at google. External 
code includes e.g. code from ffmpeg, Eigen, R, Chromium, gnuplot, lua ,...

> - did you consider looking for `implicitCastExpr`? You can capture all 
> narrowing conversion with that and analyze them further. I think it is 
> possible to warn for the subset mentioned in the guidelines.

Yes, that's the version for which I have provided analysis.  I'll update the 
diff with that version.

> - you match for `binaryOperator("+=", "-")` maybe all assignments can be 
> looked at?  (`binaryOperator(isASsignmentOperator())`, defined in 
> clang-tidy/util/Matchers.h or similar) That includes all calculate-and-assign 
> operations. Those should be equally dangerous.

The "normal" assignments are covered by the implicitCastExpr() above.


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] D45356: [clangd] Adapt index interfaces to D45014, and fix the old bugs.

[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


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45356



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

[PATCH] D45426: [clangd] Allow using customized include path in URI.

[Haojian Wu via Phabricator via cfe-commits]

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

Calculating the include path from absolute file path does not always
work for all build system, e.g. bazel uses symlink as the build working
directory. The absolute file path from editor and clang is diverged from
each other. We need to address it properly in build sysmtem integration.

This patch worksarounds the issue by providing a hook in URI which allows
clients to provide their customized include path.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45426

Files:
  clangd/ClangdServer.cpp
  clangd/URI.cpp
  clangd/URI.h
  unittests/clangd/ClangdTests.cpp
  unittests/clangd/TestScheme.h
  unittests/clangd/URITests.cpp


Index: unittests/clangd/URITests.cpp
===
--- unittests/clangd/URITests.cpp
+++ unittests/clangd/URITests.cpp
@@ -47,6 +47,10 @@
 return URI(Scheme, /*Authority=*/"",
AbsolutePath.substr(Pos + llvm::StringRef(TestRoot).size()));
   }
+
+  llvm::Expected getIncludePath(const URI &U) const override {
+return ("\"" + U.body() + "\"").str();
+  }
 };
 
 const char *TestScheme::Scheme = "unittest";
Index: unittests/clangd/ClangdTests.cpp
===
--- unittests/clangd/ClangdTests.cpp
+++ unittests/clangd/ClangdTests.cpp
@@ -961,6 +961,10 @@
/*Preferred=*/"", ""));
   EXPECT_TRUE(Inserted(OriginalHeader, PreferredHeader, "\"Y.h\""));
   EXPECT_TRUE(Inserted("", PreferredHeader, "\"Y.h\""));
+  auto TestURIHeader = URI::create("/abc/test-root/x/y/z.h", "unittest");
+  EXPECT_TRUE(static_cast(TestURIHeader));
+  EXPECT_TRUE(Inserted(TestURIHeader->toString(), "", "\"x/y/z.h\""));
+
 
   // Check that includes are sorted.
   const auto Expected = R"cpp(
Index: clangd/URI.h
===
--- clangd/URI.h
+++ clangd/URI.h
@@ -60,6 +60,14 @@
   static llvm::Expected resolve(const URI &U,
  llvm::StringRef HintPath = "");
 
+  /// Tries to get the include path of the file corresponding to the URI.
+  /// This allows clients to provide their customized include paths.
+  ///
+  /// If the returned string is non-empty, clangd will use it directly when
+  /// doing include insertion; otherwise we will fall back to the clang to
+  /// calculate the include path from the resolved absolute path.
+  static llvm::Expected includePath(const URI &U);
+
   friend bool operator==(const URI &LHS, const URI &RHS) {
 return std::tie(LHS.Scheme, LHS.Authority, LHS.Body) ==
std::tie(RHS.Scheme, RHS.Authority, RHS.Body);
@@ -94,6 +102,13 @@
 
   virtual llvm::Expected
   uriFromAbsolutePath(llvm::StringRef AbsolutePath) const = 0;
+
+  /// Returns the include path of the file corresponding to the URI, which can
+  /// be #include directly. See URI::resolveIncludePath for details.
+  virtual llvm::Expected
+  getIncludePath(const URI& U) const {
+return "";  // no customized include path for this scheme.
+  }
 };
 
 /// By default, a "file" scheme is supported where URI paths are always 
absolute
Index: clangd/URI.cpp
===
--- clangd/URI.cpp
+++ clangd/URI.cpp
@@ -196,5 +196,12 @@
   return S->get()->getAbsolutePath(Uri.Authority, Uri.Body, HintPath);
 }
 
+llvm::Expected URI::includePath(const URI &Uri) {
+  auto S = findSchemeByName(Uri.Scheme);
+  if (!S)
+return S.takeError();
+  return S->get()->getIncludePath(Uri);
+}
+
 } // namespace clangd
 } // namespace clang
Index: clangd/ClangdServer.cpp
===
--- clangd/ClangdServer.cpp
+++ clangd/ClangdServer.cpp
@@ -286,6 +286,13 @@
   auto U = URI::parse(Header);
   if (!U)
 return U.takeError();
+
+  auto IncludePath = URI::includePath(*U);
+  if (!IncludePath)
+return IncludePath.takeError();
+  if (!IncludePath->empty())
+return HeaderFile{std::move(*IncludePath), /*Verbatim=*/true};
+
   auto Resolved = URI::resolve(*U, HintPath);
   if (!Resolved)
 return Resolved.takeError();


Index: unittests/clangd/URITests.cpp
===
--- unittests/clangd/URITests.cpp
+++ unittests/clangd/URITests.cpp
@@ -47,6 +47,10 @@
 return URI(Scheme, /*Authority=*/"",
AbsolutePath.substr(Pos + llvm::StringRef(TestRoot).size()));
   }
+
+  llvm::Expected getIncludePath(const URI &U) const override {
+return ("\"" + U.body() + "\"").str();
+  }
 };
 
 const char *TestScheme::Scheme = "unittest";
Index: unittests/clangd/ClangdTests.cpp
===
--- unittests/clangd/ClangdTests.cpp
+++ unittests/clangd/ClangdTests.cpp
@@ -961,6 +961,10 @@

[PATCH] D45405: [clang-tidy] [modernize-use-auto] Add a threshold for minimal type name length to be replaced with 'auto'

[Malcolm Parsons via Phabricator via cfe-commits]

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

LGTM!


https://reviews.llvm.org/D45405



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

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

That sounds good.

> Removing from my dashboard for now.

@aaron.ballman seems to be busy now, maybe you should add alexfh again and 
discuss the results.


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] D45405: [clang-tidy] [modernize-use-auto] Add a threshold for minimal type name length to be replaced with 'auto'

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: docs/clang-tidy/checks/modernize-use-auto.rst:202
+   neither warn nor fix type names having a length less than the option value.
+   The option affects expressions only, not iterators.
+

nit: document the default value.


https://reviews.llvm.org/D45405



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

[PATCH] D45006: [Tooling] A CompilationDatabase wrapper that infers header commands.

[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 with a small nit.
Really excited about this landing!




Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:353
+  }
+  Best = Candidate.first;
+  BestPreferred = Preferred;

Maybe put these fields into `struct Candidate {}`?
The code would, arguably, be easier to read. Up to you.


Repository:
  rC Clang

https://reviews.llvm.org/D45006



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

[PATCH] D45409: [cmake] Include LLVMTestingSupport when doing stand-alone build

[Michał Górny via Phabricator via cfe-commits]

mgorny added a comment.

I've based this change on an earlier fix to lldb when that project started 
using LLVMTestingSupport.

That change in turn was based on how we handle gtest across the projects. As 
for gtest, I think installing was not considered as an option since different 
projects may build it with different flags. However, I did not investigate it 
in detail.

I don't specifically mind installing LLVMTestingSupport in any way. This 
solution was simply the least effort way of solving the immediate problem, 
given that we need LLVM sources for gtest anyway.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45409



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

[PATCH] D45285: [clangd-vscode] Update VScode dependencies

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

In https://reviews.llvm.org/D45285#1061243, @ilya-biryukov wrote:

> In https://reviews.llvm.org/D45285#1058567, @malaperle wrote:
>
> > Do we need to bump the version of the extension and do a new release or 
> > anything like that? Or leave this for later?
>
>
> We should bump the version and republish the extension into VSCode 
> marketplace. 
>  @hokein has more context on how to properly do that.


Once you commit this patch, I'm happy to make a new release.

> In https://reviews.llvm.org/D45285#1058578, @MaskRay wrote:
> 
>> Do we really want to keep editor plugins in the repository?
> 
> 
> As long as we are the ones who own it, probably yes. Any downsides to keeping 
> it here?

We used to have a separate GitHub repository for the extension, but it 
increased the maintaining burden (we need to syn whenever a new change is made 
in this repository).
Having the vscode extension in the clangd repository is the sensible way IMO.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45285



___
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 a comment.

Ping.

@aaron.ballman ping, do you have any further thoughts on that macro 
false-negative?

  #define vardecl(type, name) type name;
  void variables_15() {
// FIXME: surely we should still warn here?
vardecl(int, a);
vardecl(int, b);
  }
  // CHECK-MESSAGES: :[[@LINE-5]]:6: warning: function 'variables_15' exceeds 
recommended size/complexity thresholds [readability-function-size]
  // CHECK-MESSAGES: :[[@LINE-6]]:6: note: 4 lines including whitespace and 
comments (threshold 0)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44602



___
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 updated this revision to Diff 141610.
courbet edited the summary of this revision.
courbet added a comment.

- Add support for bad cast detection.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D38455

Files:
  clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
  clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/cppcoreguidelines-narrowing-conversions.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/cppcoreguidelines-narrowing-conversions.cpp

Index: test/clang-tidy/cppcoreguidelines-narrowing-conversions.cpp
===
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-narrowing-conversions.cpp
@@ -0,0 +1,60 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-narrowing-conversions %t
+
+float ceil(float);
+namespace std {
+double ceil(double);
+long double floor(long double);
+} // namespace std
+
+void not_ok(double d) {
+  int i = 0;
+  i = d;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: narrowing conversion from 'double' to 'int' [cppcoreguidelines-narrowing-conversions]
+  i = 0.5f;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: narrowing conversion from 'float' to 'int' [cppcoreguidelines-narrowing-conversions]
+}
+
+void not_ok_binary_ops(double d) {
+  int i = 0;
+  i += 0.5;
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: narrowing conversion from 'double' to 'int' [cppcoreguidelines-narrowing-conversions]
+  i += 0.5f;
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: narrowing conversion from 'float' to 'int' [cppcoreguidelines-narrowing-conversions]
+  i += d;
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: narrowing conversion from 'double' to 'int' [cppcoreguidelines-narrowing-conversions]
+  // We warn on the following even though it's not dangerous because there is no
+  // reason to use a double literal here.
+  // TODO(courbet): Provide an automatic fix.
+  i += 2.0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: narrowing conversion from 'double' to 'int' [cppcoreguidelines-narrowing-conversions]
+  i += 2.0f;
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: narrowing conversion from 'float' to 'int' [cppcoreguidelines-narrowing-conversions]
+}
+
+void ok(double d) {
+  int i = 0;
+  i = 1;
+  i = static_cast(0.5);
+  i = static_cast(d);
+  i = std::ceil(0.5);
+  i = ::std::floor(0.5);
+  {
+using std::ceil;
+i = ceil(0.5f);
+  }
+  i = ceil(0.5f);
+}
+
+void ok_binary_ops(double d) {
+  int i = 0;
+  i += 1;
+  i += static_cast(0.5);
+  i += static_cast(d);
+  i += std::ceil(0.5);
+  i += ::std::floor(0.5);
+  {
+using std::ceil;
+i += ceil(0.5f);
+  }
+  i += ceil(0.5f);
+}
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -76,6 +76,7 @@
cppcoreguidelines-avoid-goto
cppcoreguidelines-c-copy-assignment-signature (redirects to misc-unconventional-assign-operator) 
cppcoreguidelines-interfaces-global-init
+   cppcoreguidelines-narrowing-conversions
cppcoreguidelines-no-malloc
cppcoreguidelines-owning-memory
cppcoreguidelines-pro-bounds-array-to-pointer-decay
Index: docs/clang-tidy/checks/cppcoreguidelines-narrowing-conversions.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/cppcoreguidelines-narrowing-conversions.rst
@@ -0,0 +1,13 @@
+.. title:: clang-tidy - cppcoreguidelines-narrowing-conversions
+
+cppcoreguidelines-narrowing-conversions
+===
+
+Checks for silent narrowing conversions, e.g: ``int i = 0; i += 0.1;``. While
+the issue is obvious in this former example, it might not be so in the
+following: ``void MyClass::f(double d) { int_member_ += d; }``.
+
+This rule is part of the "Expressions and statements" profile of the C++ Core
+Guidelines, corresponding to rule ES.46. See
+
+https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Res-narrowing.
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -196,6 +196,11 @@
 
 - The 'google-runtime-member-string-references' check was removed.
 
+- New `cppcoreguidelines-narrowing-conversions
+  `_ check
+
+  Checks for narrowing conversions, e.g. ``int i = 0; i += 0.1;``.
+
 Improvements to include-fixer
 -
 
Index: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.h
===
--- /dev/null
+++ clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.h
@@ -0,0 +1,37 @@
+//===--- NarrowingConversionsCheck.h - clang-tidy---

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

[Clement Courbet via Phabricator via cfe-commits]

courbet added a comment.

In https://reviews.llvm.org/D38455#1061300, @JonasToth wrote:

> That sounds good.
>
> > Removing from my dashboard for now.
>
> maybe you should add alexfh again and discuss the results.


Is there anything I need to do for the diff to change state ? I thought 
updating the code would automatically mark it "ready for review" again.


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] D38455: [clang-tidy] new cppcoreguidelines-narrowing-conversions check.

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

> Is there anything I need to do for the diff to change state ? I

thought updating the code would automatically mark it "ready for review"
again.

I think all comments must be marked as done to be ready for review
again. Usually the reviewer reacts to changed code, too.

But aaron seems busy right now and i think alexfh did disable the
notifications for now. Add/ping him again.

Am 09.04.2018 um 12:55 schrieb Clement Courbet via Phabricator:

> courbet added a comment.
> 
> In https://reviews.llvm.org/D38455#1061300, @JonasToth wrote:
> 
>> That sounds good.
>> 
>>> Removing from my dashboard for now.
>> 
>> maybe you should add alexfh again and discuss the results.
> 
> Is there anything I need to do for the diff to change state ? I thought 
> updating the code would automatically mark it "ready for review" again.
> 
> Repository:
> 
>   rCTE Clang Tools Extra
> 
> https://reviews.llvm.org/D38455


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] D38455: [clang-tidy] new cppcoreguidelines-narrowing-conversions check.

[Clement Courbet via Phabricator via cfe-commits]

courbet added a comment.

In https://reviews.llvm.org/D38455#1061406, @JonasToth wrote:

>




> I think all comments must be marked as done to be ready for review
>  again.
> 
> I think alexfh did disable the notifications for now. Add/ping him again.

I see, thanks.


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] D38455: [clang-tidy] new cppcoreguidelines-narrowing-conversions check.

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.



Comment at: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:32
+  Finder->addMatcher(
+  implicitCastExpr(hasImplicitDestinationType(isInteger()),
+   hasSourceExpression(IsFloatExpr),

Do you plan to check for `double` -> `float` conversion, too?

Having a limited scope for now is ok, but a FIXME would be nice.



Comment at: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:34
+   hasSourceExpression(IsFloatExpr),
+   unless(hasParent(castExpr(.bind("cast"),
+  this);

Given C++ is weird sometimes: Is there a way that a cast might imply an 
narrowing conversion?

```
double value = 0.4;
int i = static_cast(value);
```

or 

```
void some_function(int);

double d = 0.42;
some_function(static_cast(d));
```
would come to mind.

Nice to have, IMHO not necessary.



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

I would really like to add all other calc-and-assign operations. At least "*=" 
and "/=".


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] D45405: [clang-tidy] [modernize-use-auto] Add a threshold for minimal type name length to be replaced with 'auto'

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added inline comments.



Comment at: clang-tidy/modernize/UseAutoCheck.cpp:290
+: ClangTidyCheck(Name, Context), RemoveStars(Options.get("RemoveStars", 
0)),
+  MinTypeNameLength(Options.get("MinTypeNameLength", 0)) {}
 

Maybe make the default 5? Or does anyone really want to replace 
`int/long/char/bool/...` with `auto`?


https://reviews.llvm.org/D45405



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

[PATCH] D45392: [clang-tidy] add new check to find out objc ivars which do not have prefix '_'

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

I wonder whether the readability-identifier-naming check could be extended to 
support this use case instead of adding a new check specifically for 
underscores in ivar names?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45392



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

[PATCH] D43779: [Tooling] [0/1] Refactor FrontendActionFactory::create() to return std::unique_ptr<>

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

In https://reviews.llvm.org/D43779#1061043, @lebedev.ri wrote:

> Hmm.
>  Got back to this issue.
>
> Not reproducible with gcc-4.8.5 and gcc-4.9.3 in ubuntu 16.04 chroot.
>  *Reproducible* with gcc-4.8.4 and gcc-4.9.2 in debian oldstable (Jessie) 
> chroot.
>  So one might assume that it was fixed.
>
> I did try to creduce the crasher, but not not much success, since gcc takes 
> 10+ sec to crash.


That's unfortunate, but it could be even worse. My experience with creducing 
crashes is that the process can take up to several days. In this case, however, 
we know what changes cause the crash and it should be possible to construct the 
test case manually instead of creducing it.

> Should i simply try to re-commit and see if the buildbots got upgraded so 
> this is no longer an issue?

I wouldn't expect buildbots to have been upgraded without someone doing this on 
purpose. If you have the list of buildbots that crashed, you could look at 
their recent logs to figure out which GCC version they are using now. It 
*might* be fine to require a patched version of GCC, but in that case you would 
have to:
0. Ask llvm-dev+cfe-dev whether anyone has concerns in raising the minimum 
required version of GCC

1. Fix the documentation, in particular this list: 
https://llvm.org/docs/GettingStarted.html#software and these instructions: 
https://llvm.org/docs/GettingStarted.html#getting-a-modern-host-c-toolchain
2. Get buildbot maintainers to upgrade their GCC to at least the new required 
version

An alternative would be to try working around the bug.


Repository:
  rL LLVM

https://reviews.llvm.org/D43779



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

[PATCH] D43779: [Tooling] [0/1] Refactor FrontendActionFactory::create() to return std::unique_ptr<>

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In https://reviews.llvm.org/D43779#1061444, @alexfh wrote:

> In https://reviews.llvm.org/D43779#1061043, @lebedev.ri wrote:
>
> > Hmm.
> >  Got back to this issue.
> >
> > Not reproducible with gcc-4.8.5 and gcc-4.9.3 in ubuntu 16.04 chroot.
> >  *Reproducible* with gcc-4.8.4 and gcc-4.9.2 in debian oldstable (Jessie) 
> > chroot.
> >  So one might assume that it was fixed.
> >
> > I did try to creduce the crasher, but not not much success, since gcc takes 
> > 10+ sec to crash.
>
>
> That's unfortunate, but it could be even worse. My experience with creducing 
> crashes is that the process can take up to several days. In this case, 
> however, we know what changes cause the crash and it should be possible to 
> construct the test case manually instead of creducing it.
>
> > Should i simply try to re-commit and see if the buildbots got upgraded so 
> > this is no longer an issue?




> I wouldn't expect buildbots to have been upgraded without someone doing this 
> on purpose. If you have the list of buildbots that crashed, you could look at 
> their recent logs to figure out which GCC version they are using now.

Yes, it seems they weren't upgraded yet as of this time.

> It *might* be fine to require a patched version of GCC, but in that case you 
> would have to:
>  0. Ask llvm-dev+cfe-dev whether anyone has concerns in raising the minimum 
> required version of GCC

Yep, did that right after posting that comment, 
http://lists.llvm.org/pipermail/llvm-dev/2018-April/122438.html

> 1. Fix the documentation, in particular this list: 
> https://llvm.org/docs/GettingStarted.html#software and these instructions: 
> https://llvm.org/docs/GettingStarted.html#getting-a-modern-host-c-toolchain
> 2. Get buildbot maintainers to upgrade their GCC to at least the new required 
> version
> 
>   An alternative would be to try working around the bug.




Repository:
  rL LLVM

https://reviews.llvm.org/D43779



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

[PATCH] D45258: [clang-tidy] Return non-zero exit code for clang errors.

[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


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45258



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

[PATCH] D45405: [clang-tidy] [modernize-use-auto] Add a threshold for minimal type name length to be replaced with 'auto'

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: clang-tidy/modernize/UseAutoCheck.cpp:290
+: ClangTidyCheck(Name, Context), RemoveStars(Options.get("RemoveStars", 
0)),
+  MinTypeNameLength(Options.get("MinTypeNameLength", 0)) {}
 

alexfh wrote:
> Maybe make the default 5? Or does anyone really want to replace 
> `int/long/char/bool/...` with `auto`?
That might be a bit surprising behavioral change..
At least it should be spelled out in the release notes.
(my 5 cent: defaulting to 0 would be best)


https://reviews.llvm.org/D45405



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

[PATCH] D45094: [LibTooling] Make interface of VFS injection into ClangTool more user-friendly

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: include/clang/Basic/VirtualFileSystem.h:313
+/// \brief Creates a \p vfs::OverlayFileSystem which overlays the given file
+/// system above the 'real' file system, as seen by the operating system.
+IntrusiveRefCntPtr

I suggest leaving out the quotes around 'real'



Comment at: lib/Basic/VirtualFileSystem.cpp:328
 void OverlayFileSystem::pushOverlay(IntrusiveRefCntPtr FS) {
+  // FIXME: OverlayFS containing another one in its stack could be flattened.
   FSList.push_back(FS);

I generally agree that it might be useful, but given that we can't use 
`dynamic_cast` in LLVM code addressing this `FIXME` is probably not worth the 
effort.

And this patch is probably not the right place to add this comment, since it 
doesn't change `OverlayFileSystem` in any way.



Comment at: unittests/Tooling/ToolingTest.cpp:411
   InMemoryFileSystem->addFile(
-  "a.cpp", 0, llvm::MemoryBuffer::getMemBuffer("int main() {}"));
+  "/a.cpp", 0, llvm::MemoryBuffer::getMemBuffer("int main() {}"));
 

Using `'/'` in paths would probably break on Windows.
Why do we need to add it in the first place?



Comment at: unittests/Tooling/ToolingTest.cpp:435
+  newFrontendActionFactory());
+  EXPECT_NE(0, Tool.run(Action.get()));
+}

It's not clear what this tests expects when looking at the code
A comment would be helpful.

Also consider matching on an actual error diagnostic (i.e. " not 
found", right?)


https://reviews.llvm.org/D45094



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

[PATCH] D45094: [LibTooling] Make interface of VFS injection into ClangTool more user-friendly

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: include/clang/Basic/VirtualFileSystem.h:315
+IntrusiveRefCntPtr
+createOverlayOnRealFilesystem(IntrusiveRefCntPtr TopFS);
+

NIT: I'm not an expert in English, but shouldn't it be 
createOverlay**Over**Real.
Also maybe shorten the suffix: `createOverlayOverRealFS`?



Comment at: lib/Basic/VirtualFileSystem.cpp:372
+vfs::createOverlayOnRealFilesystem(IntrusiveRefCntPtr TopFS) {
+  IntrusiveRefCntPtr OverlayFS =
+new OverlayFileSystem(getRealFileSystem());

Maybe add an assert the parameter is non-null?


https://reviews.llvm.org/D45094



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

[PATCH] D45095: [clang-tidy] Align usage of ClangTool interface with new VFS injection

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added subscribers: hokein, alexfh.
ilya-biryukov added inline comments.



Comment at: clang-tidy/ClangTidy.cpp:91
 public:
-  ErrorReporter(ClangTidyContext &Context, bool ApplyFixes,
-llvm::IntrusiveRefCntPtr BaseFS)
-  : Files(FileSystemOptions(), BaseFS), DiagOpts(new DiagnosticOptions()),
+  ErrorReporter(ClangTidyContext &Context, bool ApplyFixes, ClangTool &Tool)
+  : Files(FileSystemOptions(), Tool.getFiles().getVirtualFileSystem()),

Why do we need to change the signature of `ErrorReporter` constructor?
Broadening the accepted interface does not seem right. It only needs the vfs 
and the clients could get vfs from `ClangTool` themselves.



Comment at: clang-tidy/ClangTidy.h:229
 void runClangTidy(clang::tidy::ClangTidyContext &Context,
-  const tooling::CompilationDatabase &Compilations,
-  ArrayRef InputFiles,
-  llvm::IntrusiveRefCntPtr BaseFS,
+  clang::tooling::ClangTool &Tool,
   ProfileData *Profile = nullptr);

I'm not sure if passing `ClangTool` here is an improvement.
Let's ask someone more familiar with clang-tidy. @hokein, @alexfh, WDYT? 



Comment at: clang-tidy/tool/CMakeLists.txt:19
   clangBasic
+  clangFrontend
   clangTidy

Why do we need an extra dependency?



Comment at: clang-tidy/tool/ClangTidyMain.cpp:432
   }
-  llvm::IntrusiveRefCntPtr BaseFS(
-  VfsOverlay.empty() ? vfs::getRealFileSystem()
+  llvm::IntrusiveRefCntPtr VirtualFileSystem(
+  VfsOverlay.empty() ? nullptr

Maybe use a shorter name, e.g. `FileSystem`?


https://reviews.llvm.org/D45095



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

[PATCH] D45405: [clang-tidy] [modernize-use-auto] Add a threshold for minimal type name length to be replaced with 'auto'

[Zinovy Nis via Phabricator via cfe-commits]

zinovy.nis added inline comments.



Comment at: clang-tidy/modernize/UseAutoCheck.cpp:290
+: ClangTidyCheck(Name, Context), RemoveStars(Options.get("RemoveStars", 
0)),
+  MinTypeNameLength(Options.get("MinTypeNameLength", 0)) {}
 

lebedev.ri wrote:
> alexfh wrote:
> > Maybe make the default 5? Or does anyone really want to replace 
> > `int/long/char/bool/...` with `auto`?
> That might be a bit surprising behavioral change..
> At least it should be spelled out in the release notes.
> (my 5 cent: defaulting to 0 would be best)
Maybe we can somehow mention the current option value in a warning message?


https://reviews.llvm.org/D45405



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

r329558 - Try to fix libclang reproducer tests after r329465

[Hans Wennborg via cfe-commits]

Author: hans
Date: Mon Apr  9 05:21:12 2018
New Revision: 329558

URL: http://llvm.org/viewvc/llvm-project?rev=329558&view=rev
Log:
Try to fix libclang reproducer tests after r329465

They were failing on Windows because the output YAML didn't parse:

  YAML:1:664: error: Unrecognized escape code!

  {"toolchain":"D:\\buildslave\\clang-x64-ninja-win7\\stage1",
"libclang.operation":"complete", "libclang.opts":1, "args":["clang",
"-fno-spell-checking",

"D:\buildslave\clang-x64-ninja-win7\llvm\tools\clang\test\Index\create-libclang-completion-reproducer.c",
"-Xclang", "-detailed-preprocessing-record",
"-fallow-editor-placeholders"],

"invocation-args":["-code-completion-at=D:\buildslave\clang-x64-ninja-win7\llvm\tools\clang\test\Index\create-libclang-completion-reproducer.c:10:1"],

"unsaved_file_hashes":[{"name":"D:\\buildslave\\clang-x64-ninja-win7\\llvm\\tools\\clang\\test\\Index\\create-libclang-completion-reproducer.c",
  "md5":"aee23773de90e665992b48209351d70e"}]}

This adds some more escaping to try to make it work.

Modified:
cfe/trunk/tools/libclang/CIndexer.cpp

Modified: cfe/trunk/tools/libclang/CIndexer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/libclang/CIndexer.cpp?rev=329558&r1=329557&r2=329558&view=diff
==
--- cfe/trunk/tools/libclang/CIndexer.cpp (original)
+++ cfe/trunk/tools/libclang/CIndexer.cpp Mon Apr  9 05:21:12 2018
@@ -127,14 +127,14 @@ LibclangInvocationReporter::LibclangInvo
   for (const auto &I : llvm::enumerate(Args)) {
 if (I.index())
   OS << ',';
-OS << '"' << I.value() << '"';
+OS << '"' << llvm::yaml::escape(I.value()) << '"';
   }
   if (!InvocationArgs.empty()) {
 OS << R"(],"invocation-args":[)";
 for (const auto &I : llvm::enumerate(InvocationArgs)) {
   if (I.index())
 OS << ',';
-  OS << '"' << I.value() << '"';
+  OS << '"' << llvm::yaml::escape(I.value()) << '"';
 }
   }
   if (!UnsavedFiles.empty()) {


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

Re: r329465 - Recommit r329442: Generate Libclang invocation reproducers using a new

[Hans Wennborg via cfe-commits]

Hopefully r329558 should fix it.

On Sat, Apr 7, 2018 at 9:44 PM, via cfe-commits
 wrote:
> Hi Alex,
>
> The two tests you added in this commit seem to be failing because of a crash 
> on one of the Windows bots. Can you take a look?
>
> http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win/builds/8912
>
> FAIL: Clang :: Index/create-libclang-parsing-reproducer.c (7891 of 37034)
>  TEST 'Clang :: 
> Index/create-libclang-parsing-reproducer.c' FAILED 
> Script:
> --
> rm -rf 
> C:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\build\tools\clang\test\Index\Output\create-libclang-parsing-reproducer.c.tmp
> mkdir 
> C:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\build\tools\clang\test\Index\Output\create-libclang-parsing-reproducer.c.tmp
> env 
> CINDEXTEST_INVOCATION_EMISSION_PATH=C:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\build\tools\clang\test\Index\Output\create-libclang-parsing-reproducer.c.tmp
>  not 
> C:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\build\bin\c-index-test.EXE
>  -test-load-source all 
> C:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\llvm\tools\clang\test\Index\create-libclang-parsing-reproducer.c
> c:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\build\bin\clang.EXE 
> -cc1gen-reproducer 
> C:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\build\tools\clang\test\Index\Output\create-libclang-parsing-reproducer.c.tmp/libclang-*
>  -v | 
> C:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\build\bin\FileCheck.EXE
>  
> C:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\llvm\tools\clang\test\Index\create-libclang-parsing-reproducer.c
> ls 
> C:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\build\tools\clang\test\Index\Output\create-libclang-parsing-reproducer.c.tmp
>  | 
> C:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\build\bin\count.EXE > 0
> rm -rf 
> C:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\build\tools\clang\test\Index\Output\create-libclang-parsing-reproducer.c.tmp
> mkdir 
> C:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\build\tools\clang\test\Index\Output\create-libclang-parsing-reproducer.c.tmp
> env 
> CINDEXTEST_INVOCATION_EMISSION_PATH=C:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\build\tools\clang\test\Index\Output\create-libclang-parsing-reproducer.c.tmp
>  not 
> C:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\build\bin\c-index-test.EXE
>  -test-load-source all 
> "-remap-file=C:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\llvm\tools\clang\test\Index\create-libclang-parsing-reproducer.c,C:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\llvm\tools\clang\test\Index/Inputs/record-parsing-invocation-remap.c"
>  
> C:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\llvm\tools\clang\test\Index\create-libclang-parsing-reproducer.c
> c:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\build\bin\clang.EXE 
> -cc1gen-reproducer 
> C:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\build\tools\clang\test\Index\Output\create-libclang-parsing-reproducer.c.tmp/libclang-*
>  -v | 
> C:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\build\bin\FileCheck.EXE
>  
> C:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\llvm\tools\clang\test\Index\create-libclang-parsing-reproducer.c
> --
> Exit Code: 2
>
> Command Output (stdout):
> --
> $ "rm" "-rf" 
> "C:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\build\tools\clang\test\Index\Output\create-libclang-parsing-reproducer.c.tmp"
> $ "mkdir" 
> "C:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\build\tools\clang\test\Index\Output\create-libclang-parsing-reproducer.c.tmp"
> $ "not" 
> "C:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\build\bin\c-index-test.EXE"
>  "-test-load-source" "all" 
> "C:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\llvm\tools\clang\test\Index\create-libclang-parsing-reproducer.c"
> # command stderr:
> libclang: crash detected during parsing: {
>
>   'source_filename' : '(null)'
>
>   'command_line_args' : ['clang', 
> 'C:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\llvm\tools\clang\test\Index\create-libclang-parsing-reproducer.c'],
>
>   'unsaved_files' : [],
>
>   'options' : 1,
>
> }
>
> Unable to load translation unit!
>
> Failure: libclang crashed
>
>
> $ 
> "c:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\build\bin\clang.EXE"
>  "-cc1gen-reproducer" 
> "C:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\build\tools\clang\test\Index\Output\create-libclang-parsing-reproducer.c.tmp/libclang-*"
>  "-v"
> # command stderr:
> YAML:1:366: error: Unrecognized escape code!
>
> {"toolchain":"C:\\ps4-buildslave2\\llvm-clang-x86_64-expensive-checks-win\\build","libclang.operation":"parse","libclang.opts":1,"args":["clang","-fno-spell-checking","C:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\llvm

[PATCH] D45094: [LibTooling] Make interface of VFS injection into ClangTool more user-friendly

[Whisperity via Phabricator via cfe-commits]

whisperity added inline comments.



Comment at: include/clang/Basic/VirtualFileSystem.h:315
+IntrusiveRefCntPtr
+createOverlayOnRealFilesystem(IntrusiveRefCntPtr TopFS);
+

ilya-biryukov wrote:
> NIT: I'm not an expert in English, but shouldn't it be 
> createOverlay**Over**Real.
> Also maybe shorten the suffix: `createOverlayOverRealFS`?
According to [[https://www.thefreedictionary.com/overlay|this dictionary]] 
overlay can mean "to lay //on// " something. Although `above` could also work 
to eliminate saying "over" repeatedly.



Comment at: unittests/Tooling/ToolingTest.cpp:411
   InMemoryFileSystem->addFile(
-  "a.cpp", 0, llvm::MemoryBuffer::getMemBuffer("int main() {}"));
+  "/a.cpp", 0, llvm::MemoryBuffer::getMemBuffer("int main() {}"));
 

ilya-biryukov wrote:
> Using `'/'` in paths would probably break on Windows.
> Why do we need to add it in the first place?
Agreed, this will be removed. It was added because overlaying the real-FS 
changes the working directory in the memory filesystem too, which in the 
previous patch (where the overlay was done by the ClangTool constructor) broke 
where the file resides. It's not applicable anymore.

Although it's strange that saying `make check-clang-tooling` did **NOT** 
execute these tests and said everything passed, I had to run the build 
`ToolingTest` binary manually!



Comment at: unittests/Tooling/ToolingTest.cpp:435
+  newFrontendActionFactory());
+  EXPECT_NE(0, Tool.run(Action.get()));
+}

ilya-biryukov wrote:
> It's not clear what this tests expects when looking at the code
> A comment would be helpful.
> 
> Also consider matching on an actual error diagnostic (i.e. " not 
> found", right?)
How can I match on the error message?


https://reviews.llvm.org/D45094



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

[PATCH] D45095: [clang-tidy] Align usage of ClangTool interface with new VFS injection

[Whisperity via Phabricator via cfe-commits]

whisperity added inline comments.



Comment at: clang-tidy/ClangTidy.cpp:91
 public:
-  ErrorReporter(ClangTidyContext &Context, bool ApplyFixes,
-llvm::IntrusiveRefCntPtr BaseFS)
-  : Files(FileSystemOptions(), BaseFS), DiagOpts(new DiagnosticOptions()),
+  ErrorReporter(ClangTidyContext &Context, bool ApplyFixes, ClangTool &Tool)
+  : Files(FileSystemOptions(), Tool.getFiles().getVirtualFileSystem()),

ilya-biryukov wrote:
> Why do we need to change the signature of `ErrorReporter` constructor?
> Broadening the accepted interface does not seem right. It only needs the vfs 
> and the clients could get vfs from `ClangTool` themselves.
Is it okay interface-wise if the FS used by the `ErrorReporter` is **not** the 
same as the one used by the module that produced the errors? It seems like an 
undocumented consensus/convention that the same VFS should have been passed 
here.



Comment at: clang-tidy/tool/CMakeLists.txt:19
   clangBasic
+  clangFrontend
   clangTidy

ilya-biryukov wrote:
> Why do we need an extra dependency?
In the current state of the patch, `clang-tidy/tool/ClangTidyMain.cpp` 
constructs the `ClangTool` which constructor requires a 
`std::shared_ptr`. `PCHContainerOperations`'s 
definition and implementation is part of the `clangFrontend` library, and 
without this dependency, there would be a symbol resolution error.


https://reviews.llvm.org/D45095



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

[PATCH] D45383: Strip reference from a va_list object in C when merging parameter types.

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In https://reviews.llvm.org/D45383#1060354, @efriedma wrote:

> How could this patch possibly affect vprintf?


vprintf is defined in the builtins.def file, and other than our messages 
considering it a 'library function', it is otherwise identical.


Repository:
  rC Clang

https://reviews.llvm.org/D45383



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

Re: r311958 - Revert "Revert r311552: [Bash-autocompletion] Add support for static analyzer flags"

[Yuka Takahashi via cfe-commits]

Hi Nico,

Thanks for your comment!

I do agree that this code is hacky. Do you mean to ask tablegen to
generate Checkers.inc under Driver so that we can do like this? :
  #define CHECKER(FULLNAME, CLASS, DESCFILE, HT, G, H)  FULLNAME ","
  #include "clang/Driver/Checkers.inc"
  #undef GET_CHECKERS

Cheers,
Yuka

2018-04-07 4:28 GMT+02:00 Nico Weber :

> Hi Yuka,
>
> sorry about the late review comment on this. I just happened to see that
> this lets Driver's Option.inc depend on StaticAnalyzer/Checker's
> Checker.inc. However, Driver does not depend on StaticAnalyzer/Checker. In
> practice, it works ok because of all tablegen targets being collected
> into clang-tablegen-targets and driver depending on that (
> http://llvm-cs.pcc.me.uk/tools/clang/CMakeLists.txt#442), but it still
> feels a bit hacky that Driver's tablegen output depends on code generated
> by StaticAnalyzer/Checker. Maybe we should move Checker.td into Driver now?
>
> Nico
>
> On Mon, Aug 28, 2017 at 8:09 PM, Yuka Takahashi via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: yamaguchi
>> Date: Mon Aug 28 17:09:31 2017
>> New Revision: 311958
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=311958&view=rev
>> Log:
>> Revert "Revert r311552: [Bash-autocompletion] Add support for static
>> analyzer flags"
>>
>> This reverts commit 7c46b80c022e18d43c1fdafb117b0c409c5a6d1e.
>>
>> r311552 broke lld buildbot because I've changed OptionInfos type from
>> ArrayRef to vector. However the bug is fixed, so I'll commit this again.
>>
>> Modified:
>> cfe/trunk/include/clang/Driver/CC1Options.td
>> cfe/trunk/lib/Driver/DriverOptions.cpp
>> cfe/trunk/test/Driver/autocomplete.c
>>
>> Modified: cfe/trunk/include/clang/Driver/CC1Options.td
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/
>> Driver/CC1Options.td?rev=311958&r1=311957&r2=311958&view=diff
>> 
>> ==
>> --- cfe/trunk/include/clang/Driver/CC1Options.td (original)
>> +++ cfe/trunk/include/clang/Driver/CC1Options.td Mon Aug 28 17:09:31 2017
>> @@ -99,7 +99,19 @@ def analyzer_stats : Flag<["-"], "analyz
>>HelpText<"Print internal analyzer statistics.">;
>>
>>  def analyzer_checker : Separate<["-"], "analyzer-checker">,
>> -  HelpText<"Choose analyzer checkers to enable">;
>> +  HelpText<"Choose analyzer checkers to enable">,
>> +  ValuesCode<[{
>> +const char *Values =
>> +#define GET_CHECKERS
>> +#define CHECKER(FULLNAME, CLASS, DESCFILE, HT, G, H)  FULLNAME ","
>> +#include "clang/StaticAnalyzer/Checkers/Checkers.inc"
>> +#undef GET_CHECKERS
>> +#define GET_PACKAGES
>> +#define PACKAGE(FULLNAME, G, D)  FULLNAME ","
>> +#include "clang/StaticAnalyzer/Checkers/Checkers.inc"
>> +#undef GET_PACKAGES
>> +;
>> +  }]>;
>>  def analyzer_checker_EQ : Joined<["-"], "analyzer-checker=">,
>>Alias;
>>
>>
>> Modified: cfe/trunk/lib/Driver/DriverOptions.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Dri
>> verOptions.cpp?rev=311958&r1=311957&r2=311958&view=diff
>> 
>> ==
>> --- cfe/trunk/lib/Driver/DriverOptions.cpp (original)
>> +++ cfe/trunk/lib/Driver/DriverOptions.cpp Mon Aug 28 17:09:31 2017
>> @@ -11,6 +11,7 @@
>>  #include "llvm/ADT/STLExtras.h"
>>  #include "llvm/Option/OptTable.h"
>>  #include "llvm/Option/Option.h"
>> +#include 
>>
>>  using namespace clang::driver;
>>  using namespace clang::driver::options;
>> @@ -40,5 +41,13 @@ public:
>>  }
>>
>>  std::unique_ptr clang::driver::createDriverOptTable() {
>> -  return llvm::make_unique();
>> +  auto Result = llvm::make_unique();
>> +  // Options.inc is included in DriverOptions.cpp, and calls OptTable's
>> +  // addValues function.
>> +  // Opt is a variable used in the code fragment in Options.inc.
>> +  OptTable &Opt = *Result;
>> +#define OPTTABLE_ARG_INIT
>> +#include "clang/Driver/Options.inc"
>> +#undef OPTTABLE_ARG_INIT
>> +  return std::move(Result);
>>  }
>>
>> Modified: cfe/trunk/test/Driver/autocomplete.c
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/au
>> tocomplete.c?rev=311958&r1=311957&r2=311958&view=diff
>> 
>> ==
>> --- cfe/trunk/test/Driver/autocomplete.c (original)
>> +++ cfe/trunk/test/Driver/autocomplete.c Mon Aug 28 17:09:31 2017
>> @@ -93,3 +93,5 @@
>>  // WARNING-NEXT: -Wmax-unsigned-zero
>>  // RUN: %clang --autocomplete=-Wno-invalid-pp- | FileCheck %s
>> -check-prefix=NOWARNING
>>  // NOWARNING: -Wno-invalid-pp-token
>> +// RUN: %clang --autocomplete=-analyzer-checker, | FileCheck %s
>> -check-prefix=ANALYZER
>> +// ANALYZER: unix.Malloc
>>
>>
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>
>
___

Re: r311958 - Revert "Revert r311552: [Bash-autocompletion] Add support for static analyzer flags"

[Nico Weber via cfe-commits]

Yes.

On Mon, Apr 9, 2018 at 9:00 AM, Yuka Takahashi  wrote:

> Hi Nico,
>
> Thanks for your comment!
>
> I do agree that this code is hacky. Do you mean to ask tablegen to
> generate Checkers.inc under Driver so that we can do like this? :
>   #define CHECKER(FULLNAME, CLASS, DESCFILE, HT, G, H)  FULLNAME ","
>   #include "clang/Driver/Checkers.inc"
>   #undef GET_CHECKERS
>
> Cheers,
> Yuka
>
> 2018-04-07 4:28 GMT+02:00 Nico Weber :
>
>> Hi Yuka,
>>
>> sorry about the late review comment on this. I just happened to see that
>> this lets Driver's Option.inc depend on StaticAnalyzer/Checker's
>> Checker.inc. However, Driver does not depend on StaticAnalyzer/Checker. In
>> practice, it works ok because of all tablegen targets being collected
>> into clang-tablegen-targets and driver depending on that (
>> http://llvm-cs.pcc.me.uk/tools/clang/CMakeLists.txt#442), but it still
>> feels a bit hacky that Driver's tablegen output depends on code generated
>> by StaticAnalyzer/Checker. Maybe we should move Checker.td into Driver now?
>>
>> Nico
>>
>> On Mon, Aug 28, 2017 at 8:09 PM, Yuka Takahashi via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>>> Author: yamaguchi
>>> Date: Mon Aug 28 17:09:31 2017
>>> New Revision: 311958
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=311958&view=rev
>>> Log:
>>> Revert "Revert r311552: [Bash-autocompletion] Add support for static
>>> analyzer flags"
>>>
>>> This reverts commit 7c46b80c022e18d43c1fdafb117b0c409c5a6d1e.
>>>
>>> r311552 broke lld buildbot because I've changed OptionInfos type from
>>> ArrayRef to vector. However the bug is fixed, so I'll commit this again.
>>>
>>> Modified:
>>> cfe/trunk/include/clang/Driver/CC1Options.td
>>> cfe/trunk/lib/Driver/DriverOptions.cpp
>>> cfe/trunk/test/Driver/autocomplete.c
>>>
>>> Modified: cfe/trunk/include/clang/Driver/CC1Options.td
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/
>>> Driver/CC1Options.td?rev=311958&r1=311957&r2=311958&view=diff
>>> 
>>> ==
>>> --- cfe/trunk/include/clang/Driver/CC1Options.td (original)
>>> +++ cfe/trunk/include/clang/Driver/CC1Options.td Mon Aug 28 17:09:31
>>> 2017
>>> @@ -99,7 +99,19 @@ def analyzer_stats : Flag<["-"], "analyz
>>>HelpText<"Print internal analyzer statistics.">;
>>>
>>>  def analyzer_checker : Separate<["-"], "analyzer-checker">,
>>> -  HelpText<"Choose analyzer checkers to enable">;
>>> +  HelpText<"Choose analyzer checkers to enable">,
>>> +  ValuesCode<[{
>>> +const char *Values =
>>> +#define GET_CHECKERS
>>> +#define CHECKER(FULLNAME, CLASS, DESCFILE, HT, G, H)  FULLNAME ","
>>> +#include "clang/StaticAnalyzer/Checkers/Checkers.inc"
>>> +#undef GET_CHECKERS
>>> +#define GET_PACKAGES
>>> +#define PACKAGE(FULLNAME, G, D)  FULLNAME ","
>>> +#include "clang/StaticAnalyzer/Checkers/Checkers.inc"
>>> +#undef GET_PACKAGES
>>> +;
>>> +  }]>;
>>>  def analyzer_checker_EQ : Joined<["-"], "analyzer-checker=">,
>>>Alias;
>>>
>>>
>>> Modified: cfe/trunk/lib/Driver/DriverOptions.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Dri
>>> verOptions.cpp?rev=311958&r1=311957&r2=311958&view=diff
>>> 
>>> ==
>>> --- cfe/trunk/lib/Driver/DriverOptions.cpp (original)
>>> +++ cfe/trunk/lib/Driver/DriverOptions.cpp Mon Aug 28 17:09:31 2017
>>> @@ -11,6 +11,7 @@
>>>  #include "llvm/ADT/STLExtras.h"
>>>  #include "llvm/Option/OptTable.h"
>>>  #include "llvm/Option/Option.h"
>>> +#include 
>>>
>>>  using namespace clang::driver;
>>>  using namespace clang::driver::options;
>>> @@ -40,5 +41,13 @@ public:
>>>  }
>>>
>>>  std::unique_ptr clang::driver::createDriverOptTable() {
>>> -  return llvm::make_unique();
>>> +  auto Result = llvm::make_unique();
>>> +  // Options.inc is included in DriverOptions.cpp, and calls OptTable's
>>> +  // addValues function.
>>> +  // Opt is a variable used in the code fragment in Options.inc.
>>> +  OptTable &Opt = *Result;
>>> +#define OPTTABLE_ARG_INIT
>>> +#include "clang/Driver/Options.inc"
>>> +#undef OPTTABLE_ARG_INIT
>>> +  return std::move(Result);
>>>  }
>>>
>>> Modified: cfe/trunk/test/Driver/autocomplete.c
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/au
>>> tocomplete.c?rev=311958&r1=311957&r2=311958&view=diff
>>> 
>>> ==
>>> --- cfe/trunk/test/Driver/autocomplete.c (original)
>>> +++ cfe/trunk/test/Driver/autocomplete.c Mon Aug 28 17:09:31 2017
>>> @@ -93,3 +93,5 @@
>>>  // WARNING-NEXT: -Wmax-unsigned-zero
>>>  // RUN: %clang --autocomplete=-Wno-invalid-pp- | FileCheck %s
>>> -check-prefix=NOWARNING
>>>  // NOWARNING: -Wno-invalid-pp-token
>>> +// RUN: %clang --autocomplete=-analyzer-checker, | FileCheck %s
>>> -check-prefix=ANALYZER
>>> +// ANALYZER: unix.Malloc
>>>
>>>
>>> ___

[PATCH] D45405: [clang-tidy] [modernize-use-auto] Add a threshold for minimal type name length to be replaced with 'auto'

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: clang-tidy/modernize/UseAutoCheck.cpp:290
+: ClangTidyCheck(Name, Context), RemoveStars(Options.get("RemoveStars", 
0)),
+  MinTypeNameLength(Options.get("MinTypeNameLength", 0)) {}
 

zinovy.nis wrote:
> lebedev.ri wrote:
> > alexfh wrote:
> > > Maybe make the default 5? Or does anyone really want to replace 
> > > `int/long/char/bool/...` with `auto`?
> > That might be a bit surprising behavioral change..
> > At least it should be spelled out in the release notes.
> > (my 5 cent: defaulting to 0 would be best)
> Maybe we can somehow mention the current option value in a warning message?
Sure, can be done, but that will only be seen when it does fire, not when it 
suddenly stops firing...


https://reviews.llvm.org/D45405



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

Re: r311958 - Revert "Revert r311552: [Bash-autocompletion] Add support for static analyzer flags"

[Yuka Takahashi via cfe-commits]

Sounds good!

2018-04-09 15:03 GMT+02:00 Nico Weber :

> Yes.
>
> On Mon, Apr 9, 2018 at 9:00 AM, Yuka Takahashi  wrote:
>
>> Hi Nico,
>>
>> Thanks for your comment!
>>
>> I do agree that this code is hacky. Do you mean to ask tablegen to
>> generate Checkers.inc under Driver so that we can do like this? :
>>   #define CHECKER(FULLNAME, CLASS, DESCFILE, HT, G, H)  FULLNAME ","
>>   #include "clang/Driver/Checkers.inc"
>>   #undef GET_CHECKERS
>>
>> Cheers,
>> Yuka
>>
>> 2018-04-07 4:28 GMT+02:00 Nico Weber :
>>
>>> Hi Yuka,
>>>
>>> sorry about the late review comment on this. I just happened to see that
>>> this lets Driver's Option.inc depend on StaticAnalyzer/Checker's
>>> Checker.inc. However, Driver does not depend on StaticAnalyzer/Checker. In
>>> practice, it works ok because of all tablegen targets being collected
>>> into clang-tablegen-targets and driver depending on that (
>>> http://llvm-cs.pcc.me.uk/tools/clang/CMakeLists.txt#442), but it still
>>> feels a bit hacky that Driver's tablegen output depends on code generated
>>> by StaticAnalyzer/Checker. Maybe we should move Checker.td into Driver now?
>>>
>>> Nico
>>>
>>> On Mon, Aug 28, 2017 at 8:09 PM, Yuka Takahashi via cfe-commits <
>>> cfe-commits@lists.llvm.org> wrote:
>>>
 Author: yamaguchi
 Date: Mon Aug 28 17:09:31 2017
 New Revision: 311958

 URL: http://llvm.org/viewvc/llvm-project?rev=311958&view=rev
 Log:
 Revert "Revert r311552: [Bash-autocompletion] Add support for static
 analyzer flags"

 This reverts commit 7c46b80c022e18d43c1fdafb117b0c409c5a6d1e.

 r311552 broke lld buildbot because I've changed OptionInfos type from
 ArrayRef to vector. However the bug is fixed, so I'll commit this again.

 Modified:
 cfe/trunk/include/clang/Driver/CC1Options.td
 cfe/trunk/lib/Driver/DriverOptions.cpp
 cfe/trunk/test/Driver/autocomplete.c

 Modified: cfe/trunk/include/clang/Driver/CC1Options.td
 URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/
 Driver/CC1Options.td?rev=311958&r1=311957&r2=311958&view=diff
 
 ==
 --- cfe/trunk/include/clang/Driver/CC1Options.td (original)
 +++ cfe/trunk/include/clang/Driver/CC1Options.td Mon Aug 28 17:09:31
 2017
 @@ -99,7 +99,19 @@ def analyzer_stats : Flag<["-"], "analyz
HelpText<"Print internal analyzer statistics.">;

  def analyzer_checker : Separate<["-"], "analyzer-checker">,
 -  HelpText<"Choose analyzer checkers to enable">;
 +  HelpText<"Choose analyzer checkers to enable">,
 +  ValuesCode<[{
 +const char *Values =
 +#define GET_CHECKERS
 +#define CHECKER(FULLNAME, CLASS, DESCFILE, HT, G, H)  FULLNAME ","
 +#include "clang/StaticAnalyzer/Checkers/Checkers.inc"
 +#undef GET_CHECKERS
 +#define GET_PACKAGES
 +#define PACKAGE(FULLNAME, G, D)  FULLNAME ","
 +#include "clang/StaticAnalyzer/Checkers/Checkers.inc"
 +#undef GET_PACKAGES
 +;
 +  }]>;
  def analyzer_checker_EQ : Joined<["-"], "analyzer-checker=">,
Alias;


 Modified: cfe/trunk/lib/Driver/DriverOptions.cpp
 URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Dri
 verOptions.cpp?rev=311958&r1=311957&r2=311958&view=diff
 
 ==
 --- cfe/trunk/lib/Driver/DriverOptions.cpp (original)
 +++ cfe/trunk/lib/Driver/DriverOptions.cpp Mon Aug 28 17:09:31 2017
 @@ -11,6 +11,7 @@
  #include "llvm/ADT/STLExtras.h"
  #include "llvm/Option/OptTable.h"
  #include "llvm/Option/Option.h"
 +#include 

  using namespace clang::driver;
  using namespace clang::driver::options;
 @@ -40,5 +41,13 @@ public:
  }

  std::unique_ptr clang::driver::createDriverOptTable() {
 -  return llvm::make_unique();
 +  auto Result = llvm::make_unique();
 +  // Options.inc is included in DriverOptions.cpp, and calls OptTable's
 +  // addValues function.
 +  // Opt is a variable used in the code fragment in Options.inc.
 +  OptTable &Opt = *Result;
 +#define OPTTABLE_ARG_INIT
 +#include "clang/Driver/Options.inc"
 +#undef OPTTABLE_ARG_INIT
 +  return std::move(Result);
  }

 Modified: cfe/trunk/test/Driver/autocomplete.c
 URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/au
 tocomplete.c?rev=311958&r1=311957&r2=311958&view=diff
 
 ==
 --- cfe/trunk/test/Driver/autocomplete.c (original)
 +++ cfe/trunk/test/Driver/autocomplete.c Mon Aug 28 17:09:31 2017
 @@ -93,3 +93,5 @@
  // WARNING-NEXT: -Wmax-unsigned-zero
  // RUN: %clang --autocomplete=-Wno-invalid-pp- | FileCheck %s
 -check-prefix=NOWARNING
  // NOWARN

[PATCH] D27621: [clang-tidy] check to find declarations declaring more than one name

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

@firolino do you plan on finishing this?


https://reviews.llvm.org/D27621



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

[PATCH] D27621: [clang-tidy] check to find declarations declaring more than one name

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

Still work on this one?


https://reviews.llvm.org/D27621



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

[PATCH] D43969: Improve completion experience for headers

[Manuel Klimek via Phabricator via cfe-commits]

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

lg


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43969



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

[clang-tools-extra] r329566 - Improve completion experience for headers

[Philipp Stephani via cfe-commits]

Author: phst
Date: Mon Apr  9 06:31:44 2018
New Revision: 329566

URL: http://llvm.org/viewvc/llvm-project?rev=329566&view=rev
Log:
Improve completion experience for headers

Summary: When calling `completing-read', we should provide a default to prevent 
the behavior described in 
https://github.com/DarwinAwardWinner/ido-completing-read-plus#why-does-ret-sometimes-not-select-the-first-completion-on-the-list--why-is-there-an-empty-entry-at-the-beginning-of-the-completion-list--what-happened-to-old-style-default-selection.
  Also, don't use an assertion to check whether the user selected a header; 
raise a proper signal instead.

Reviewers: klimek

Reviewed By: klimek

Subscribers: cfe-commits

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

Modified:
clang-tools-extra/trunk/include-fixer/tool/clang-include-fixer.el

Modified: clang-tools-extra/trunk/include-fixer/tool/clang-include-fixer.el
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/include-fixer/tool/clang-include-fixer.el?rev=329566&r1=329565&r2=329566&view=diff
==
--- clang-tools-extra/trunk/include-fixer/tool/clang-include-fixer.el (original)
+++ clang-tools-extra/trunk/include-fixer/tool/clang-include-fixer.el Mon Apr  
9 06:31:44 2018
@@ -314,14 +314,18 @@ They are replaced by the single element
   (goto-char (clang-include-fixer--closest-overlay overlays)))
 (cl-flet ((header (info) (let-alist info .Header)))
   ;; The header-infos is already sorted by include-fixer.
-  (let* ((header (completing-read
+  (let* ((headers (mapcar #'header .HeaderInfos))
+ (header (completing-read
   (clang-include-fixer--format-message
"Select include for '%s': " symbol)
-  (mapcar #'header .HeaderInfos)
-  nil :require-match nil
-  'clang-include-fixer--history))
+  headers nil :require-match nil
+  'clang-include-fixer--history
+  ;; Specify a default to prevent the behavior
+  ;; described in
+  ;; 
https://github.com/DarwinAwardWinner/ido-completing-read-plus#why-does-ret-sometimes-not-select-the-first-completion-on-the-list--why-is-there-an-empty-entry-at-the-beginning-of-the-completion-list--what-happened-to-old-style-default-selection.
+  (car headers)))
  (info (cl-find header .HeaderInfos :key #'header :test 
#'string=)))
-(cl-assert info)
+(unless info (user-error "No header selected"))
 (setcar .HeaderInfos info)
 (setcdr .HeaderInfos nil
 (mapc #'delete-overlay overlays)


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

[PATCH] D27621: [clang-tidy] check to find declarations declaring more than one name

[Firat Kasmis via Phabricator via cfe-commits]

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

In https://reviews.llvm.org/D27621#1061562, @lebedev.ri wrote:

> @firolino do you plan on finishing this?


Sometimes the universe is really funny! I have just re-started working on it 
today.


https://reviews.llvm.org/D27621



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

[PATCH] D43969: Improve completion experience for headers

[Philipp via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL329566: Improve completion experience for headers (authored 
by phst, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D43969

Files:
  clang-tools-extra/trunk/include-fixer/tool/clang-include-fixer.el


Index: clang-tools-extra/trunk/include-fixer/tool/clang-include-fixer.el
===
--- clang-tools-extra/trunk/include-fixer/tool/clang-include-fixer.el
+++ clang-tools-extra/trunk/include-fixer/tool/clang-include-fixer.el
@@ -314,14 +314,18 @@
   (goto-char (clang-include-fixer--closest-overlay overlays)))
 (cl-flet ((header (info) (let-alist info .Header)))
   ;; The header-infos is already sorted by include-fixer.
-  (let* ((header (completing-read
+  (let* ((headers (mapcar #'header .HeaderInfos))
+ (header (completing-read
   (clang-include-fixer--format-message
"Select include for '%s': " symbol)
-  (mapcar #'header .HeaderInfos)
-  nil :require-match nil
-  'clang-include-fixer--history))
+  headers nil :require-match nil
+  'clang-include-fixer--history
+  ;; Specify a default to prevent the behavior
+  ;; described in
+  ;; 
https://github.com/DarwinAwardWinner/ido-completing-read-plus#why-does-ret-sometimes-not-select-the-first-completion-on-the-list--why-is-there-an-empty-entry-at-the-beginning-of-the-completion-list--what-happened-to-old-style-default-selection.
+  (car headers)))
  (info (cl-find header .HeaderInfos :key #'header :test 
#'string=)))
-(cl-assert info)
+(unless info (user-error "No header selected"))
 (setcar .HeaderInfos info)
 (setcdr .HeaderInfos nil
 (mapc #'delete-overlay overlays)


Index: clang-tools-extra/trunk/include-fixer/tool/clang-include-fixer.el
===
--- clang-tools-extra/trunk/include-fixer/tool/clang-include-fixer.el
+++ clang-tools-extra/trunk/include-fixer/tool/clang-include-fixer.el
@@ -314,14 +314,18 @@
   (goto-char (clang-include-fixer--closest-overlay overlays)))
 (cl-flet ((header (info) (let-alist info .Header)))
   ;; The header-infos is already sorted by include-fixer.
-  (let* ((header (completing-read
+  (let* ((headers (mapcar #'header .HeaderInfos))
+ (header (completing-read
   (clang-include-fixer--format-message
"Select include for '%s': " symbol)
-  (mapcar #'header .HeaderInfos)
-  nil :require-match nil
-  'clang-include-fixer--history))
+  headers nil :require-match nil
+  'clang-include-fixer--history
+  ;; Specify a default to prevent the behavior
+  ;; described in
+  ;; https://github.com/DarwinAwardWinner/ido-completing-read-plus#why-does-ret-sometimes-not-select-the-first-completion-on-the-list--why-is-there-an-empty-entry-at-the-beginning-of-the-completion-list--what-happened-to-old-style-default-selection.
+  (car headers)))
  (info (cl-find header .HeaderInfos :key #'header :test #'string=)))
-(cl-assert info)
+(unless info (user-error "No header selected"))
 (setcar .HeaderInfos info)
 (setcdr .HeaderInfos nil
 (mapc #'delete-overlay overlays)
___
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 marked 2 inline comments as done.
courbet added a comment.

Thanks.




Comment at: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:32
+  Finder->addMatcher(
+  implicitCastExpr(hasImplicitDestinationType(isInteger()),
+   hasSourceExpression(IsFloatExpr),

JonasToth wrote:
> Do you plan to check for `double` -> `float` conversion, too?
> 
> Having a limited scope for now is ok, but a FIXME would be nice.
I've added a FIXME.



Comment at: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:34
+   hasSourceExpression(IsFloatExpr),
+   unless(hasParent(castExpr(.bind("cast"),
+  this);

JonasToth wrote:
> Given C++ is weird sometimes: Is there a way that a cast might imply an 
> narrowing conversion?
> 
> ```
> double value = 0.4;
> int i = static_cast(value);
> ```
> 
> or 
> 
> ```
> void some_function(int);
> 
> double d = 0.42;
> some_function(static_cast(d));
> ```
> would come to mind.
> 
> Nice to have, IMHO not necessary.
Luckily that does not seem to be implicit:

```
void f(double value) {
  int i = static_cast(value);
}
```

```
FunctionDecl 0x7fe5ffbd4150  line:1:6 f 'void (double)'
|-ParmVarDecl 0x7fe5ffbd4088  col:15 used value 'double'
`-CompoundStmt 0x7fe5ffbd4380 
  `-DeclStmt 0x7fe5ffbd4368 
`-VarDecl 0x7fe5ffbd4250  col:7 i 'int' cinit
  `-ImplicitCastExpr 0x7fe5ffbd4350  'int' 

`-CXXStaticCastExpr 0x7fe5ffbd4320  'float' 
static_cast 
  `-ImplicitCastExpr 0x7fe5ffbd4308  'float' 
`-ImplicitCastExpr 0x7fe5ffbd42f0  'double' 
  `-DeclRefExpr 0x7fe5ffbd42b0  'double' lvalue ParmVar 
0x7fe5ffbd4088 'value' 'double'
```






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

JonasToth wrote:
> I would really like to add all other calc-and-assign operations. At least 
> "*=" and "/=".
Makes sense, I've added `*=` and `/=`. All others (`%=`, `&=`, `<<=` and 
variations thereof)  trigger a compilation error if the RHS is a float 
(rightfully).




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] D38455: [clang-tidy] new cppcoreguidelines-narrowing-conversions check.

[Clement Courbet via Phabricator via cfe-commits]

courbet updated this revision to Diff 141635.
courbet marked 2 inline comments as done.
courbet added a comment.

- Add `*=` and `/=`.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D38455

Files:
  clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
  clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/cppcoreguidelines-narrowing-conversions.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/cppcoreguidelines-narrowing-conversions.cpp

Index: test/clang-tidy/cppcoreguidelines-narrowing-conversions.cpp
===
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-narrowing-conversions.cpp
@@ -0,0 +1,65 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-narrowing-conversions %t
+
+float ceil(float);
+namespace std {
+double ceil(double);
+long double floor(long double);
+} // namespace std
+
+void not_ok(double d) {
+  int i = 0;
+  i = d;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: narrowing conversion from 'double' to 'int' [cppcoreguidelines-narrowing-conversions]
+  i = 0.5f;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: narrowing conversion from 'float' to 'int' [cppcoreguidelines-narrowing-conversions]
+}
+
+void not_ok_binary_ops(double d) {
+  int i = 0;
+  i += 0.5;
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: narrowing conversion from 'double' to 'int' [cppcoreguidelines-narrowing-conversions]
+  i += 0.5f;
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: narrowing conversion from 'float' to 'int' [cppcoreguidelines-narrowing-conversions]
+  i += d;
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: narrowing conversion from 'double' to 'int' [cppcoreguidelines-narrowing-conversions]
+  // We warn on the following even though it's not dangerous because there is no
+  // reason to use a double literal here.
+  // TODO(courbet): Provide an automatic fix.
+  i += 2.0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: narrowing conversion from 'double' to 'int' [cppcoreguidelines-narrowing-conversions]
+  i += 2.0f;
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: narrowing conversion from 'float' to 'int' [cppcoreguidelines-narrowing-conversions]
+
+  i *= 0.5f;
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: narrowing conversion from 'float' to 'int' [cppcoreguidelines-narrowing-conversions]
+  i /= 0.5f;
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: narrowing conversion from 'float' to 'int' [cppcoreguidelines-narrowing-conversions]
+}
+
+void ok(double d) {
+  int i = 0;
+  i = 1;
+  i = static_cast(0.5);
+  i = static_cast(d);
+  i = std::ceil(0.5);
+  i = ::std::floor(0.5);
+  {
+using std::ceil;
+i = ceil(0.5f);
+  }
+  i = ceil(0.5f);
+}
+
+void ok_binary_ops(double d) {
+  int i = 0;
+  i += 1;
+  i += static_cast(0.5);
+  i += static_cast(d);
+  i += std::ceil(0.5);
+  i += ::std::floor(0.5);
+  {
+using std::ceil;
+i += ceil(0.5f);
+  }
+  i += ceil(0.5f);
+}
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -76,6 +76,7 @@
cppcoreguidelines-avoid-goto
cppcoreguidelines-c-copy-assignment-signature (redirects to misc-unconventional-assign-operator) 
cppcoreguidelines-interfaces-global-init
+   cppcoreguidelines-narrowing-conversions
cppcoreguidelines-no-malloc
cppcoreguidelines-owning-memory
cppcoreguidelines-pro-bounds-array-to-pointer-decay
Index: docs/clang-tidy/checks/cppcoreguidelines-narrowing-conversions.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/cppcoreguidelines-narrowing-conversions.rst
@@ -0,0 +1,13 @@
+.. title:: clang-tidy - cppcoreguidelines-narrowing-conversions
+
+cppcoreguidelines-narrowing-conversions
+===
+
+Checks for silent narrowing conversions, e.g: ``int i = 0; i += 0.1;``. While
+the issue is obvious in this former example, it might not be so in the
+following: ``void MyClass::f(double d) { int_member_ += d; }``.
+
+This rule is part of the "Expressions and statements" profile of the C++ Core
+Guidelines, corresponding to rule ES.46. See
+
+https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Res-narrowing.
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -196,6 +196,11 @@
 
 - The 'google-runtime-member-string-references' check was removed.
 
+- New `cppcoreguidelines-narrowing-conversions
+  `_ check
+
+  Checks for narrowing conversions, e.g. ``int i = 0; i += 0.1;``.
+
 Improvements to include-fixer
 -
 
Ind

[PATCH] D45421: [X86] Emit native IR for pmuldq/pmuludq builtins.

[Sanjay Patel via Phabricator via cfe-commits]

spatel added a comment.

> The only question I have is whether its ok to emit the v2i32 intermediate 
> type for the 128-bit version. I wasn't sure of any examples where we use an 
> illegal type in our intrinsic/builtin handling. At least not a narrower type. 
> I know pavg uses a wider type.

I don't know of any precedence at this level, but we created illegal scalar int 
types (i128/i256) as part of memcmp expansion knowing that we'd match and 
combine those specific patterns in the DAG for x86. I figured that as long as 
we take responsibility for handling the illegal types, it's ok to do 
that...nobody has complained so far. :)


Repository:
  rC Clang

https://reviews.llvm.org/D45421



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

[PATCH] D45426: [clangd] Allow using customized include path in URI.

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clangd/URI.h:63
 
+  /// Tries to get the include path of the file corresponding to the URI.
+  /// This allows clients to provide their customized include paths.

Avoid "tries to" which is vague about what cases are failures.
e.g.
Get the preferred spelling of this file for #include, if there is one, e.g. 
.
Returns the empty string if normal include-shortening based on the path should 
be used.
Fails if the URI is not valid in the schema.



Comment at: clangd/URI.h:64
+  /// Tries to get the include path of the file corresponding to the URI.
+  /// This allows clients to provide their customized include paths.
+  ///

what are "clients"? maybe "URI schemas"?



Comment at: clangd/URI.h:66
+  ///
+  /// If the returned string is non-empty, clangd will use it directly when
+  /// doing include insertion; otherwise we will fall back to the clang to

does this include ? it probably should, if we're allowing schemes to 
customize how includes are spelled.



Comment at: clangd/URI.h:69
+  /// calculate the include path from the resolved absolute path.
+  static llvm::Expected includePath(const URI &U);
+

i'd avoid the name "include path" because it can be confused with a) the set of 
directories passed to -I and b) the filesystem path to the file to be included.
Suggest includeString or so



Comment at: clangd/URI.h:107
+  /// Returns the include path of the file corresponding to the URI, which can
+  /// be #include directly. See URI::resolveIncludePath for details.
+  virtual llvm::Expected

#included



Comment at: unittests/clangd/ClangdTests.cpp:964
   EXPECT_TRUE(Inserted("", PreferredHeader, "\"Y.h\""));
+  auto TestURIHeader = URI::create("/abc/test-root/x/y/z.h", "unittest");
+  EXPECT_TRUE(static_cast(TestURIHeader));

this relies on ClangdTests and URITests being linked together, which we chose 
to avoid last time this came up. Just define a scheme here?
(This is one place where putting a field on URI would have been simpler)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45426



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

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.



Comment at: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:34
+   hasSourceExpression(IsFloatExpr),
+   unless(hasParent(castExpr(.bind("cast"),
+  this);

courbet wrote:
> JonasToth wrote:
> > Given C++ is weird sometimes: Is there a way that a cast might imply an 
> > narrowing conversion?
> > 
> > ```
> > double value = 0.4;
> > int i = static_cast(value);
> > ```
> > 
> > or 
> > 
> > ```
> > void some_function(int);
> > 
> > double d = 0.42;
> > some_function(static_cast(d));
> > ```
> > would come to mind.
> > 
> > Nice to have, IMHO not necessary.
> Luckily that does not seem to be implicit:
> 
> ```
> void f(double value) {
>   int i = static_cast(value);
> }
> ```
> 
> ```
> FunctionDecl 0x7fe5ffbd4150 
>  line:1:6 f 'void 
> (double)'
> |-ParmVarDecl 0x7fe5ffbd4088  col:15 used value 'double'
> `-CompoundStmt 0x7fe5ffbd4380 
>   `-DeclStmt 0x7fe5ffbd4368 
> `-VarDecl 0x7fe5ffbd4250  col:7 i 'int' cinit
>   `-ImplicitCastExpr 0x7fe5ffbd4350  'int' 
> 
> `-CXXStaticCastExpr 0x7fe5ffbd4320  'float' 
> static_cast 
>   `-ImplicitCastExpr 0x7fe5ffbd4308  'float' 
> `-ImplicitCastExpr 0x7fe5ffbd42f0  'double' 
> 
>   `-DeclRefExpr 0x7fe5ffbd42b0  'double' lvalue ParmVar 
> 0x7fe5ffbd4088 'value' 'double'
> ```
> 
> 
> 
The interesting one should get diagnosed. Could you add a test?



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

courbet wrote:
> JonasToth wrote:
> > I would really like to add all other calc-and-assign operations. At least 
> > "*=" and "/=".
> Makes sense, I've added `*=` and `/=`. All others (`%=`, `&=`, `<<=` and 
> variations thereof)  trigger a compilation error if the RHS is a float 
> (rightfully).
> 
> 
For floats that is true.

If we care about the `ìnt->char` casts later, they should be added.


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

r329570 - [Index] Return SourceLocation to consumers, not FileID/Offset pair.

[Sam McCall via cfe-commits]

Author: sammccall
Date: Mon Apr  9 07:12:51 2018
New Revision: 329570

URL: http://llvm.org/viewvc/llvm-project?rev=329570&view=rev
Log:
[Index] Return SourceLocation to consumers, not FileID/Offset pair.

Summary:
The FileID/Offset conversion is lossy. The code takes the fileLoc, which loses
e.g. the spelling location in some macro cases.
Instead, pass the original SourceLocation which preserves all information, and
update consumers to match current behavior.

This allows us to fix two bugs in clangd that need the spelling location.

Reviewers: akyrtzi, arphaman

Subscribers: ilya-biryukov, ioeric, cfe-commits

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

Modified:
cfe/trunk/include/clang/Index/IndexDataConsumer.h
cfe/trunk/lib/Index/IndexingAction.cpp
cfe/trunk/lib/Index/IndexingContext.cpp
cfe/trunk/tools/c-index-test/core_main.cpp
cfe/trunk/tools/libclang/CXIndexDataConsumer.cpp
cfe/trunk/tools/libclang/CXIndexDataConsumer.h

Modified: cfe/trunk/include/clang/Index/IndexDataConsumer.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Index/IndexDataConsumer.h?rev=329570&r1=329569&r2=329570&view=diff
==
--- cfe/trunk/include/clang/Index/IndexDataConsumer.h (original)
+++ cfe/trunk/include/clang/Index/IndexDataConsumer.h Mon Apr  9 07:12:51 2018
@@ -42,18 +42,16 @@ public:
   /// \returns true to continue indexing, or false to abort.
   virtual bool handleDeclOccurence(const Decl *D, SymbolRoleSet Roles,
ArrayRef Relations,
-   FileID FID, unsigned Offset,
-   ASTNodeInfo ASTNode);
+   SourceLocation Loc, ASTNodeInfo ASTNode);
 
   /// \returns true to continue indexing, or false to abort.
   virtual bool handleMacroOccurence(const IdentifierInfo *Name,
 const MacroInfo *MI, SymbolRoleSet Roles,
-FileID FID, unsigned Offset);
+SourceLocation Loc);
 
   /// \returns true to continue indexing, or false to abort.
   virtual bool handleModuleOccurence(const ImportDecl *ImportD,
- SymbolRoleSet Roles,
- FileID FID, unsigned Offset);
+ SymbolRoleSet Roles, SourceLocation Loc);
 
   virtual void finish() {}
 

Modified: cfe/trunk/lib/Index/IndexingAction.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Index/IndexingAction.cpp?rev=329570&r1=329569&r2=329570&view=diff
==
--- cfe/trunk/lib/Index/IndexingAction.cpp (original)
+++ cfe/trunk/lib/Index/IndexingAction.cpp Mon Apr  9 07:12:51 2018
@@ -23,20 +23,21 @@ void IndexDataConsumer::_anchor() {}
 
 bool IndexDataConsumer::handleDeclOccurence(const Decl *D, SymbolRoleSet Roles,
 ArrayRef Relations,
-FileID FID, unsigned Offset,
+SourceLocation Loc,
 ASTNodeInfo ASTNode) {
   return true;
 }
 
 bool IndexDataConsumer::handleMacroOccurence(const IdentifierInfo *Name,
- const MacroInfo *MI, 
SymbolRoleSet Roles,
- FileID FID, unsigned Offset) {
+ const MacroInfo *MI,
+ SymbolRoleSet Roles,
+ SourceLocation Loc) {
   return true;
 }
 
 bool IndexDataConsumer::handleModuleOccurence(const ImportDecl *ImportD,
   SymbolRoleSet Roles,
-  FileID FID, unsigned Offset) {
+  SourceLocation Loc) {
   return true;
 }
 

Modified: cfe/trunk/lib/Index/IndexingContext.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Index/IndexingContext.cpp?rev=329570&r1=329569&r2=329570&view=diff
==
--- cfe/trunk/lib/Index/IndexingContext.cpp (original)
+++ cfe/trunk/lib/Index/IndexingContext.cpp Mon Apr  9 07:12:51 2018
@@ -82,14 +82,9 @@ bool IndexingContext::importedModule(con
 Loc = IdLocs.front();
   else
 Loc = ImportD->getLocation();
-  SourceManager &SM = Ctx->getSourceManager();
-  Loc = SM.getFileLoc(Loc);
-  if (Loc.isInvalid())
-return true;
 
-  FileID FID;
-  unsigned Offset;
-  std::tie(FID, Offset) = SM.getDecomposedLoc(Loc);
+  SourceManager &SM = Ctx->getSourceManager();
+  FileID FID = SM.getFileID(SM.getFileLoc(Loc));
   if (FID.isInvalid())
 return true;
 
@@ -112,7 +107,7 @@ bool IndexingContext::impor

[PATCH] D45014: [Index] Return SourceLocation to consumers, not FileID/Offset pair.

[Sam McCall via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rC329570: [Index] Return SourceLocation to consumers, not 
FileID/Offset pair. (authored by sammccall, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D45014?vs=140182&id=141640#toc

Repository:
  rC Clang

https://reviews.llvm.org/D45014

Files:
  include/clang/Index/IndexDataConsumer.h
  lib/Index/IndexingAction.cpp
  lib/Index/IndexingContext.cpp
  tools/c-index-test/core_main.cpp
  tools/libclang/CXIndexDataConsumer.cpp
  tools/libclang/CXIndexDataConsumer.h

Index: lib/Index/IndexingContext.cpp
===
--- lib/Index/IndexingContext.cpp
+++ lib/Index/IndexingContext.cpp
@@ -82,14 +82,9 @@
 Loc = IdLocs.front();
   else
 Loc = ImportD->getLocation();
-  SourceManager &SM = Ctx->getSourceManager();
-  Loc = SM.getFileLoc(Loc);
-  if (Loc.isInvalid())
-return true;
 
-  FileID FID;
-  unsigned Offset;
-  std::tie(FID, Offset) = SM.getDecomposedLoc(Loc);
+  SourceManager &SM = Ctx->getSourceManager();
+  FileID FID = SM.getFileID(SM.getFileLoc(Loc));
   if (FID.isInvalid())
 return true;
 
@@ -112,7 +107,7 @@
   if (ImportD->isImplicit())
 Roles |= (unsigned)SymbolRole::Implicit;
 
-  return DataConsumer.handleModuleOccurence(ImportD, Roles, FID, Offset);
+  return DataConsumer.handleModuleOccurence(ImportD, Roles, Loc);
 }
 
 bool IndexingContext::isTemplateImplicitInstantiation(const Decl *D) {
@@ -327,13 +322,7 @@
 return true;
 
   SourceManager &SM = Ctx->getSourceManager();
-  Loc = SM.getFileLoc(Loc);
-  if (Loc.isInvalid())
-return true;
-
-  FileID FID;
-  unsigned Offset;
-  std::tie(FID, Offset) = SM.getDecomposedLoc(Loc);
+  FileID FID = SM.getFileID(SM.getFileLoc(Loc));
   if (FID.isInvalid())
 return true;
 
@@ -414,7 +403,6 @@
Rel.RelatedSymbol->getCanonicalDecl()));
   }
 
-  IndexDataConsumer::ASTNodeInfo Node{ OrigE, OrigD, Parent, ContainerDC };
-  return DataConsumer.handleDeclOccurence(D, Roles, FinalRelations, FID, Offset,
-  Node);
+  IndexDataConsumer::ASTNodeInfo Node{OrigE, OrigD, Parent, ContainerDC};
+  return DataConsumer.handleDeclOccurence(D, Roles, FinalRelations, Loc, Node);
 }
Index: lib/Index/IndexingAction.cpp
===
--- lib/Index/IndexingAction.cpp
+++ lib/Index/IndexingAction.cpp
@@ -23,20 +23,21 @@
 
 bool IndexDataConsumer::handleDeclOccurence(const Decl *D, SymbolRoleSet Roles,
 ArrayRef Relations,
-FileID FID, unsigned Offset,
+SourceLocation Loc,
 ASTNodeInfo ASTNode) {
   return true;
 }
 
 bool IndexDataConsumer::handleMacroOccurence(const IdentifierInfo *Name,
- const MacroInfo *MI, SymbolRoleSet Roles,
- FileID FID, unsigned Offset) {
+ const MacroInfo *MI,
+ SymbolRoleSet Roles,
+ SourceLocation Loc) {
   return true;
 }
 
 bool IndexDataConsumer::handleModuleOccurence(const ImportDecl *ImportD,
   SymbolRoleSet Roles,
-  FileID FID, unsigned Offset) {
+  SourceLocation Loc) {
   return true;
 }
 
Index: tools/libclang/CXIndexDataConsumer.h
===
--- tools/libclang/CXIndexDataConsumer.h
+++ tools/libclang/CXIndexDataConsumer.h
@@ -465,12 +465,11 @@
 private:
   bool handleDeclOccurence(const Decl *D, index::SymbolRoleSet Roles,
ArrayRef Relations,
-   FileID FID, unsigned Offset,
-   ASTNodeInfo ASTNode) override;
+   SourceLocation Loc, ASTNodeInfo ASTNode) override;
 
   bool handleModuleOccurence(const ImportDecl *ImportD,
  index::SymbolRoleSet Roles,
- FileID FID, unsigned Offset) override;
+ SourceLocation Loc) override;
 
   void finish() override;
 
Index: tools/libclang/CXIndexDataConsumer.cpp
===
--- tools/libclang/CXIndexDataConsumer.cpp
+++ tools/libclang/CXIndexDataConsumer.cpp
@@ -155,13 +155,10 @@
 }
 }
 
-bool CXIndexDataConsumer::handleDeclOccurence(const Decl *D,
-  SymbolRoleSet Roles,
- ArrayRef Relations,
-

[PATCH] D45014: [Index] Return SourceLocation to consumers, not FileID/Offset pair.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Oops, I got my reviews crossed and thought @ilya-biryukov had already approved 
it (we chatted about this today)


Repository:
  rC Clang

https://reviews.llvm.org/D45014



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

[PATCH] D45014: [Index] Return SourceLocation to consumers, not FileID/Offset pair.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

LGTM, but let's watch out for @akyrtzi's and @arphaman's comments, just in case 
they're not happy with the change. In that case we'll have to revert it.


Repository:
  rC Clang

https://reviews.llvm.org/D45014



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

[PATCH] D45285: [clangd-vscode] Update VScode dependencies

[Marc-Andre Laperle via Phabricator via cfe-commits]

malaperle added a comment.

In https://reviews.llvm.org/D45285#1061374, @hokein wrote:

> In https://reviews.llvm.org/D45285#1061243, @ilya-biryukov wrote:
>
> > In https://reviews.llvm.org/D45285#1058567, @malaperle wrote:
> >
> > > Do we need to bump the version of the extension and do a new release or 
> > > anything like that? Or leave this for later?
> >
> >
> > We should bump the version and republish the extension into VSCode 
> > marketplace. 
> >  @hokein has more context on how to properly do that.
>
>
> Once you commit this patch, I'm happy to make a new release.


OK, I'll commit as-is then. Thanks!


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45285



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

[clang-tools-extra] r329571 - [clangd] Adapt index interfaces to D45014, and fix the old bugs.

[Sam McCall via cfe-commits]

Author: sammccall
Date: Mon Apr  9 07:28:52 2018
New Revision: 329571

URL: http://llvm.org/viewvc/llvm-project?rev=329571&view=rev
Log:
[clangd] Adapt index interfaces to D45014, and fix the old bugs.

Summary:
Fix bugs:
- don't count occurrences of decls where we don't spell the name
- findDefinitions at MACRO(^X) goes to the definition of MACRO

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

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

Modified:
clang-tools-extra/trunk/clangd/XRefs.cpp
clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
clang-tools-extra/trunk/clangd/index/SymbolCollector.h
clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp

Modified: clang-tools-extra/trunk/clangd/XRefs.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/XRefs.cpp?rev=329571&r1=329570&r2=329571&view=diff
==
--- clang-tools-extra/trunk/clangd/XRefs.cpp (original)
+++ clang-tools-extra/trunk/clangd/XRefs.cpp Mon Apr  9 07:28:52 2018
@@ -79,10 +79,10 @@ public:
 
   bool
   handleDeclOccurence(const Decl *D, index::SymbolRoleSet Roles,
-  ArrayRef Relations, FileID FID,
-  unsigned Offset,
+  ArrayRef Relations,
+  SourceLocation Loc,
   index::IndexDataConsumer::ASTNodeInfo ASTNode) override {
-if (isSearchedLocation(FID, Offset)) {
+if (Loc == SearchedLocation) {
   // Find and add definition declarations (for GoToDefinition).
   // We don't use parameter `D`, as Parameter `D` is the canonical
   // declaration, which is the first declaration of a redeclarable
@@ -98,18 +98,12 @@ public:
   }
 
 private:
-  bool isSearchedLocation(FileID FID, unsigned Offset) const {
-const SourceManager &SourceMgr = AST.getSourceManager();
-return SourceMgr.getFileOffset(SearchedLocation) == Offset &&
-   SourceMgr.getFileID(SearchedLocation) == FID;
-  }
-
   void finish() override {
 // Also handle possible macro at the searched location.
 Token Result;
 auto &Mgr = AST.getSourceManager();
-if (!Lexer::getRawToken(SearchedLocation, Result, Mgr, AST.getLangOpts(),
-false)) {
+if (!Lexer::getRawToken(Mgr.getSpellingLoc(SearchedLocation), Result, Mgr,
+AST.getLangOpts(), false)) {
   if (Result.is(tok::raw_identifier)) {
 PP.LookUpIdentifierInfo(Result);
   }
@@ -127,16 +121,7 @@ private:
 MacroInfo *MacroInf = MacroDef.getMacroInfo();
 if (MacroInf) {
   MacroInfos.push_back(MacroDecl{IdentifierInfo->getName(), MacroInf});
-  // Clear all collected delcarations if this is a macro search.
-  //
-  // In theory, there should be no declarataions being collected when 
we
-  // search a source location that refers to a macro.
-  // The occurrence location returned by `handleDeclOccurence` is
-  // limited (FID, Offset are from expansion location), we will collect
-  // all declarations inside the macro.
-  //
-  // FIXME: Avoid adding decls from inside macros in 
handlDeclOccurence.
-  Decls.clear();
+  assert(Decls.empty());
 }
   }
 }
@@ -252,21 +237,19 @@ public:
 
   bool
   handleDeclOccurence(const Decl *D, index::SymbolRoleSet Roles,
-  ArrayRef Relations, FileID FID,
-  unsigned Offset,
+  ArrayRef Relations,
+  SourceLocation Loc,
   index::IndexDataConsumer::ASTNodeInfo ASTNode) override {
 const SourceManager &SourceMgr = AST.getSourceManager();
-if (SourceMgr.getMainFileID() != FID ||
+SourceLocation HighlightStartLoc = SourceMgr.getFileLoc(Loc);
+if (SourceMgr.getMainFileID() != SourceMgr.getFileID(HighlightStartLoc) ||
 std::find(Decls.begin(), Decls.end(), D) == Decls.end()) {
   return true;
 }
 SourceLocation End;
 const LangOptions &LangOpts = AST.getLangOpts();
-SourceLocation StartOfFileLoc = SourceMgr.getLocForStartOfFile(FID);
-SourceLocation HightlightStartLoc = 
StartOfFileLoc.getLocWithOffset(Offset);
-End =
-Lexer::getLocForEndOfToken(HightlightStartLoc, 0, SourceMgr, LangOpts);
-SourceRange SR(HightlightStartLoc, End);
+End = Lexer::getLocForEndOfToken(HighlightStartLoc, 0, SourceMgr, 
LangOpts);
+SourceRange SR(HighlightStartLoc, End);
 
 DocumentHighlightKind Kind = DocumentHighlightKind::Text;
 if (static_cast(index::SymbolRole::Write) & Roles)

Modified: clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp?rev=329571&r1=329570&r2=329571&view=diff

[PATCH] D45356: [clangd] Adapt index interfaces to D45014, and fix the old bugs.

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL329571: [clangd] Adapt index interfaces to D45014, and fix 
the old bugs. (authored by sammccall, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D45356

Files:
  clang-tools-extra/trunk/clangd/XRefs.cpp
  clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
  clang-tools-extra/trunk/clangd/index/SymbolCollector.h
  clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
  clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp

Index: clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp
@@ -222,6 +222,18 @@
}
   )cpp",
 
+  R"cpp(// Macro argument
+   int [[i]];
+   #define ADDRESSOF(X) &X;
+   int *j = ADDRESSOF(^i);
+  )cpp",
+
+  R"cpp(// Symbol concatenated inside macro (not supported)
+   int *pi;
+   #define POINTER(X) p # X;
+   int i = *POINTER(^i);
+  )cpp",
+
   R"cpp(// Forward class declaration
 class Foo;
 class [[Foo]] {};
@@ -249,8 +261,11 @@
   for (const char *Test : Tests) {
 Annotations T(Test);
 auto AST = build(T.code());
+std::vector> ExpectedLocations;
+for (const auto &R : T.ranges())
+  ExpectedLocations.push_back(RangeIs(R));
 EXPECT_THAT(findDefinitions(AST, T.point()),
-ElementsAre(RangeIs(T.range(
+ElementsAreArray(ExpectedLocations))
 << Test;
   }
 }
Index: clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
@@ -250,14 +250,16 @@
 class Y;
 class Z {}; // not used anywhere
 Y* y = nullptr;  // used in header doesn't count
+#define GLOBAL_Z(name) Z name;
   )";
   const std::string Main = R"(
 W* w = nullptr;
 W* w2 = nullptr; // only one usage counts
 X x();
 class V;
 V* v = nullptr; // Used, but not eligible for indexing.
 class Y{}; // definition doesn't count as a reference
+GLOBAL_Z(z); // Not a reference to Z, we don't spell the type.
   )";
   CollectorOpts.CountReferences = true;
   runSymbolCollector(Header, Main);
Index: clang-tools-extra/trunk/clangd/index/SymbolCollector.h
===
--- clang-tools-extra/trunk/clangd/index/SymbolCollector.h
+++ clang-tools-extra/trunk/clangd/index/SymbolCollector.h
@@ -59,8 +59,8 @@
 
   bool
   handleDeclOccurence(const Decl *D, index::SymbolRoleSet Roles,
-  ArrayRef Relations, FileID FID,
-  unsigned Offset,
+  ArrayRef Relations,
+  SourceLocation Loc,
   index::IndexDataConsumer::ASTNodeInfo ASTNode) override;
 
   SymbolSlab takeSymbols() { return std::move(Symbols).build(); }
Index: clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
===
--- clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
@@ -225,7 +225,7 @@
 // Always return true to continue indexing.
 bool SymbolCollector::handleDeclOccurence(
 const Decl *D, index::SymbolRoleSet Roles,
-ArrayRef Relations, FileID FID, unsigned Offset,
+ArrayRef Relations, SourceLocation Loc,
 index::IndexDataConsumer::ASTNodeInfo ASTNode) {
   assert(ASTCtx && PP.get() && "ASTContext and Preprocessor must be set.");
   assert(CompletionAllocator && CompletionTUInfo);
@@ -235,9 +235,10 @@
 
   // Mark D as referenced if this is a reference coming from the main file.
   // D may not be an interesting symbol, but it's cheaper to check at the end.
+  auto &SM = ASTCtx->getSourceManager();
   if (Opts.CountReferences &&
   (Roles & static_cast(index::SymbolRole::Reference)) &&
-  ASTCtx->getSourceManager().getMainFileID() == FID)
+  SM.getFileID(SM.getSpellingLoc(Loc)) == SM.getMainFileID())
 ReferencedDecls.insert(ND);
 
   // Don't continue indexing if this is a mere reference.
Index: clang-tools-extra/trunk/clangd/XRefs.cpp
===
--- clang-tools-extra/trunk/clangd/XRefs.cpp
+++ clang-tools-extra/trunk/clangd/XRefs.cpp
@@ -79,10 +79,10 @@
 
   bool
   handleDeclOccurence(const Decl *D, index::SymbolRoleSet Roles,
-  ArrayRef Relations, FileID FID,
-  unsigned Offset,
+  ArrayRef Relations,
+  SourceLocation Loc,
   index::IndexDataConsumer::ASTNodeInfo ASTNode)

[PATCH] D45356: [clangd] Adapt index interfaces to D45014, and fix the old bugs.

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE329571: [clangd] Adapt index interfaces to D45014, and fix 
the old bugs. (authored by sammccall, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D45356?vs=141300&id=141643#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D45356

Files:
  clangd/XRefs.cpp
  clangd/index/SymbolCollector.cpp
  clangd/index/SymbolCollector.h
  unittests/clangd/SymbolCollectorTests.cpp
  unittests/clangd/XRefsTests.cpp

Index: unittests/clangd/XRefsTests.cpp
===
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -222,6 +222,18 @@
}
   )cpp",
 
+  R"cpp(// Macro argument
+   int [[i]];
+   #define ADDRESSOF(X) &X;
+   int *j = ADDRESSOF(^i);
+  )cpp",
+
+  R"cpp(// Symbol concatenated inside macro (not supported)
+   int *pi;
+   #define POINTER(X) p # X;
+   int i = *POINTER(^i);
+  )cpp",
+
   R"cpp(// Forward class declaration
 class Foo;
 class [[Foo]] {};
@@ -249,8 +261,11 @@
   for (const char *Test : Tests) {
 Annotations T(Test);
 auto AST = build(T.code());
+std::vector> ExpectedLocations;
+for (const auto &R : T.ranges())
+  ExpectedLocations.push_back(RangeIs(R));
 EXPECT_THAT(findDefinitions(AST, T.point()),
-ElementsAre(RangeIs(T.range(
+ElementsAreArray(ExpectedLocations))
 << Test;
   }
 }
Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -250,14 +250,16 @@
 class Y;
 class Z {}; // not used anywhere
 Y* y = nullptr;  // used in header doesn't count
+#define GLOBAL_Z(name) Z name;
   )";
   const std::string Main = R"(
 W* w = nullptr;
 W* w2 = nullptr; // only one usage counts
 X x();
 class V;
 V* v = nullptr; // Used, but not eligible for indexing.
 class Y{}; // definition doesn't count as a reference
+GLOBAL_Z(z); // Not a reference to Z, we don't spell the type.
   )";
   CollectorOpts.CountReferences = true;
   runSymbolCollector(Header, Main);
Index: clangd/index/SymbolCollector.h
===
--- clangd/index/SymbolCollector.h
+++ clangd/index/SymbolCollector.h
@@ -59,8 +59,8 @@
 
   bool
   handleDeclOccurence(const Decl *D, index::SymbolRoleSet Roles,
-  ArrayRef Relations, FileID FID,
-  unsigned Offset,
+  ArrayRef Relations,
+  SourceLocation Loc,
   index::IndexDataConsumer::ASTNodeInfo ASTNode) override;
 
   SymbolSlab takeSymbols() { return std::move(Symbols).build(); }
Index: clangd/index/SymbolCollector.cpp
===
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -225,7 +225,7 @@
 // Always return true to continue indexing.
 bool SymbolCollector::handleDeclOccurence(
 const Decl *D, index::SymbolRoleSet Roles,
-ArrayRef Relations, FileID FID, unsigned Offset,
+ArrayRef Relations, SourceLocation Loc,
 index::IndexDataConsumer::ASTNodeInfo ASTNode) {
   assert(ASTCtx && PP.get() && "ASTContext and Preprocessor must be set.");
   assert(CompletionAllocator && CompletionTUInfo);
@@ -235,9 +235,10 @@
 
   // Mark D as referenced if this is a reference coming from the main file.
   // D may not be an interesting symbol, but it's cheaper to check at the end.
+  auto &SM = ASTCtx->getSourceManager();
   if (Opts.CountReferences &&
   (Roles & static_cast(index::SymbolRole::Reference)) &&
-  ASTCtx->getSourceManager().getMainFileID() == FID)
+  SM.getFileID(SM.getSpellingLoc(Loc)) == SM.getMainFileID())
 ReferencedDecls.insert(ND);
 
   // Don't continue indexing if this is a mere reference.
Index: clangd/XRefs.cpp
===
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -79,10 +79,10 @@
 
   bool
   handleDeclOccurence(const Decl *D, index::SymbolRoleSet Roles,
-  ArrayRef Relations, FileID FID,
-  unsigned Offset,
+  ArrayRef Relations,
+  SourceLocation Loc,
   index::IndexDataConsumer::ASTNodeInfo ASTNode) override {
-if (isSearchedLocation(FID, Offset)) {
+if (Loc == SearchedLocation) {
   // Find and add definition declarations (for GoToDefinition).
   // We don't use parameter `D`, as Parameter `D` is the canonical
   // declaration, which is the first declaration of a redeclarable
@@ -98,18 +98,12 @@
   }
 
 private:
-  bool isSearchedLocation(FileID FID, unsigned Offset) const {
-const SourceManager &Sou

[clang-tools-extra] r329574 - [clangd-vscode] Update VScode dependencies

[Marc-Andre Laperle via cfe-commits]

Author: malaperle
Date: Mon Apr  9 07:32:12 2018
New Revision: 329574

URL: http://llvm.org/viewvc/llvm-project?rev=329574&view=rev
Log:
[clangd-vscode] Update VScode dependencies

Summary:
This allows the extension to work with LSP 3.0 and is useful for testing.

Signed-off-by: Marc-Andre Laperle 

Reviewers: ilya-biryukov

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

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

Modified:
clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json

Modified: clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json?rev=329574&r1=329573&r2=329574&view=diff
==
--- clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json (original)
+++ clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json Mon Apr  
9 07:32:12 2018
@@ -6,7 +6,7 @@
 "publisher": "llvm-vs-code-extensions",
 "homepage": "https://clang.llvm.org/extra/clangd.html";,
 "engines": {
-"vscode": "^1.15.0"
+"vscode": "^1.18.0"
 },
 "categories": [
 "Languages",
@@ -32,12 +32,12 @@
 "test": "node ./node_modules/vscode/bin/test"
 },
 "dependencies": {
-"vscode-languageclient": "^3.3.0",
-"vscode-languageserver": "^3.3.0"
+"vscode-languageclient": "^4.0.0",
+"vscode-languageserver": "^4.0.0"
 },
 "devDependencies": {
 "typescript": "^2.0.3",
-"vscode": "^1.0.3",
+"vscode": "^1.1.0",
 "mocha": "^2.3.3",
 "@types/node": "^6.0.40",
 "@types/mocha": "^2.2.32"


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

[PATCH] D45285: [clangd-vscode] Update VScode dependencies

[Marc-Andre Laperle via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE329574: [clangd-vscode] Update VScode dependencies 
(authored by malaperle, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D45285?vs=141036&id=141645#toc

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45285

Files:
  clangd/clients/clangd-vscode/package.json


Index: clangd/clients/clangd-vscode/package.json
===
--- clangd/clients/clangd-vscode/package.json
+++ clangd/clients/clangd-vscode/package.json
@@ -6,7 +6,7 @@
 "publisher": "llvm-vs-code-extensions",
 "homepage": "https://clang.llvm.org/extra/clangd.html";,
 "engines": {
-"vscode": "^1.15.0"
+"vscode": "^1.18.0"
 },
 "categories": [
 "Languages",
@@ -32,12 +32,12 @@
 "test": "node ./node_modules/vscode/bin/test"
 },
 "dependencies": {
-"vscode-languageclient": "^3.3.0",
-"vscode-languageserver": "^3.3.0"
+"vscode-languageclient": "^4.0.0",
+"vscode-languageserver": "^4.0.0"
 },
 "devDependencies": {
 "typescript": "^2.0.3",
-"vscode": "^1.0.3",
+"vscode": "^1.1.0",
 "mocha": "^2.3.3",
 "@types/node": "^6.0.40",
 "@types/mocha": "^2.2.32"


Index: clangd/clients/clangd-vscode/package.json
===
--- clangd/clients/clangd-vscode/package.json
+++ clangd/clients/clangd-vscode/package.json
@@ -6,7 +6,7 @@
 "publisher": "llvm-vs-code-extensions",
 "homepage": "https://clang.llvm.org/extra/clangd.html";,
 "engines": {
-"vscode": "^1.15.0"
+"vscode": "^1.18.0"
 },
 "categories": [
 "Languages",
@@ -32,12 +32,12 @@
 "test": "node ./node_modules/vscode/bin/test"
 },
 "dependencies": {
-"vscode-languageclient": "^3.3.0",
-"vscode-languageserver": "^3.3.0"
+"vscode-languageclient": "^4.0.0",
+"vscode-languageserver": "^4.0.0"
 },
 "devDependencies": {
 "typescript": "^2.0.3",
-"vscode": "^1.0.3",
+"vscode": "^1.1.0",
 "mocha": "^2.3.3",
 "@types/node": "^6.0.40",
 "@types/mocha": "^2.2.32"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45014: [Index] Return SourceLocation to consumers, not FileID/Offset pair.

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

Some bots are sad:
http://lab.llvm.org:8011/builders/clang-atom-d525-fedora-rel/builds/16346/
http://lab.llvm.org:8011/builders/clang-ppc64be-linux/builds/17364/
http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/28078/


Repository:
  rC Clang

https://reviews.llvm.org/D45014



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

Re: [PATCH] D45014: [Index] Return SourceLocation to consumers, not FileID/Offset pair.

[Sam McCall via cfe-commits]

The matching clangd patch is r329571, I got distracted and it took me a few
minutes to land it. Apologies!

On Mon, Apr 9, 2018 at 4:35 PM Hans Wennborg via Phabricator <
revi...@reviews.llvm.org> wrote:

> hans added a comment.
>
> Some bots are sad:
> http://lab.llvm.org:8011/builders/clang-atom-d525-fedora-rel/builds/16346/
> http://lab.llvm.org:8011/builders/clang-ppc64be-linux/builds/17364/
>
> http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/28078/
>
>
> Repository:
>   rC Clang
>
> https://reviews.llvm.org/D45014
>
>
>
>
___
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.

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

Could you please add some tests that include user defined literals and there 
interaction with other literals. We should catch narrowing conversions from 
them, too.


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] D38455: [clang-tidy] new cppcoreguidelines-narrowing-conversions check.

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

In https://reviews.llvm.org/D38455#1061195, @courbet wrote:

> ping


Sorry for the long review due to the holidays.

Generally, I would also like Aaron to take a look when he's back, since he had 
some concerns. While we're waiting, one minor comment from me.




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

JonasToth wrote:
> courbet wrote:
> > JonasToth wrote:
> > > I would really like to add all other calc-and-assign operations. At least 
> > > "*=" and "/=".
> > Makes sense, I've added `*=` and `/=`. All others (`%=`, `&=`, `<<=` and 
> > variations thereof)  trigger a compilation error if the RHS is a float 
> > (rightfully).
> > 
> > 
> For floats that is true.
> 
> If we care about the `ìnt->char` casts later, they should be added.
Does it make sense to match all assignment operators including just `=`? If so, 
the matcher will become a bit simpler: `binaryOperator(isAssignmentOperator(), 
...)`. If there's a reason not to do this, maybe adding a comment would be 
useful

Apart from that, I wonder whether the matcher above would also cover the case 
of assignment operators? I would expect the AST for assignment expressions with 
different types to have ImplicitCast nodes anyway.


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] D45441: [HIP] Add predefined macros __HIPCC__ and __HIP_DEVICE_COMPILE__

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl created this revision.
yaxunl added a reviewer: tra.

also disable `__CUDA_ARCH__` for HIP.

This patch depends on https://reviews.llvm.org/D44984


https://reviews.llvm.org/D45441

Files:
  lib/Frontend/InitPreprocessor.cpp
  test/Preprocessor/predefined-macros.c


Index: test/Preprocessor/predefined-macros.c
===
--- test/Preprocessor/predefined-macros.c
+++ test/Preprocessor/predefined-macros.c
@@ -271,3 +271,20 @@
 // RUN: %clang_cc1 %s -E -dM -o - -x cl -triple spir-unknown-unknown \
 // RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-SPIR
 // CHECK-SPIR: #define __IMAGE_SUPPORT__ 1
+
+// RUN: %clang_cc1 %s -E -dM -o - -x hip -triple amdgcn-amd-amdhsa \
+// RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-HIP
+// CHECK-HIP-NOT: #define __CUDA_ARCH__
+// CHECK-HIP: #define __CUDA__ 1
+// CHECK-HIP: #define __HIPCC__ 1
+// CHECK-HIP-NOT: #define __HIP_DEVICE_COMPILE__ 1
+// CHECK-HIP: #define __HIP__ 1
+
+// RUN: %clang_cc1 %s -E -dM -o - -x hip -triple amdgcn-amd-amdhsa \
+// RUN:   -fcuda-is-device \
+// RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-HIP-DEV
+// CHECK-HIP-DEV-NOT: #define __CUDA_ARCH__
+// CHECK-HIP-DEV: #define __CUDA__ 1
+// CHECK-HIP-DEV: #define __HIPCC__ 1
+// CHECK-HIP-DEV: #define __HIP_DEVICE_COMPILE__ 1
+// CHECK-HIP-DEV: #define __HIP__ 1
Index: lib/Frontend/InitPreprocessor.cpp
===
--- lib/Frontend/InitPreprocessor.cpp
+++ lib/Frontend/InitPreprocessor.cpp
@@ -465,8 +465,12 @@
 Builder.defineMacro("__ASSEMBLER__");
   if (LangOpts.CUDA)
 Builder.defineMacro("__CUDA__");
-  if (LangOpts.HIP)
+  if (LangOpts.HIP) {
 Builder.defineMacro("__HIP__");
+Builder.defineMacro("__HIPCC__");
+if (LangOpts.CUDAIsDevice)
+  Builder.defineMacro("__HIP_DEVICE_COMPILE__");
+  }
 }
 
 /// Initialize the predefined C++ language feature test macros defined in
@@ -1025,7 +1029,7 @@
   }
 
   // CUDA device path compilaton
-  if (LangOpts.CUDAIsDevice) {
+  if (LangOpts.CUDAIsDevice && !LangOpts.HIP) {
 // The CUDA_ARCH value is set for the GPU target specified in the NVPTX
 // backend's target defines.
 Builder.defineMacro("__CUDA_ARCH__");


Index: test/Preprocessor/predefined-macros.c
===
--- test/Preprocessor/predefined-macros.c
+++ test/Preprocessor/predefined-macros.c
@@ -271,3 +271,20 @@
 // RUN: %clang_cc1 %s -E -dM -o - -x cl -triple spir-unknown-unknown \
 // RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-SPIR
 // CHECK-SPIR: #define __IMAGE_SUPPORT__ 1
+
+// RUN: %clang_cc1 %s -E -dM -o - -x hip -triple amdgcn-amd-amdhsa \
+// RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-HIP
+// CHECK-HIP-NOT: #define __CUDA_ARCH__
+// CHECK-HIP: #define __CUDA__ 1
+// CHECK-HIP: #define __HIPCC__ 1
+// CHECK-HIP-NOT: #define __HIP_DEVICE_COMPILE__ 1
+// CHECK-HIP: #define __HIP__ 1
+
+// RUN: %clang_cc1 %s -E -dM -o - -x hip -triple amdgcn-amd-amdhsa \
+// RUN:   -fcuda-is-device \
+// RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-HIP-DEV
+// CHECK-HIP-DEV-NOT: #define __CUDA_ARCH__
+// CHECK-HIP-DEV: #define __CUDA__ 1
+// CHECK-HIP-DEV: #define __HIPCC__ 1
+// CHECK-HIP-DEV: #define __HIP_DEVICE_COMPILE__ 1
+// CHECK-HIP-DEV: #define __HIP__ 1
Index: lib/Frontend/InitPreprocessor.cpp
===
--- lib/Frontend/InitPreprocessor.cpp
+++ lib/Frontend/InitPreprocessor.cpp
@@ -465,8 +465,12 @@
 Builder.defineMacro("__ASSEMBLER__");
   if (LangOpts.CUDA)
 Builder.defineMacro("__CUDA__");
-  if (LangOpts.HIP)
+  if (LangOpts.HIP) {
 Builder.defineMacro("__HIP__");
+Builder.defineMacro("__HIPCC__");
+if (LangOpts.CUDAIsDevice)
+  Builder.defineMacro("__HIP_DEVICE_COMPILE__");
+  }
 }
 
 /// Initialize the predefined C++ language feature test macros defined in
@@ -1025,7 +1029,7 @@
   }
 
   // CUDA device path compilaton
-  if (LangOpts.CUDAIsDevice) {
+  if (LangOpts.CUDAIsDevice && !LangOpts.HIP) {
 // The CUDA_ARCH value is set for the GPU target specified in the NVPTX
 // backend's target defines.
 Builder.defineMacro("__CUDA_ARCH__");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45426: [clangd] Allow using customized include path in URI.

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 141650.
hokein marked 3 inline comments as done.
hokein added a comment.

Address review comments.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45426

Files:
  clangd/ClangdServer.cpp
  clangd/URI.cpp
  clangd/URI.h
  unittests/clangd/ClangdTests.cpp
  unittests/clangd/TestScheme.h

Index: unittests/clangd/ClangdTests.cpp
===
--- unittests/clangd/ClangdTests.cpp
+++ unittests/clangd/ClangdTests.cpp
@@ -152,6 +152,28 @@
   }
 };
 
+constexpr const char* ClangdTestScheme = "ClangdTests";
+class TestURIScheme : public URIScheme {
+public:
+  llvm::Expected
+  getAbsolutePath(llvm::StringRef /*Authority*/, llvm::StringRef Body,
+  llvm::StringRef /*HintPath*/) const override {
+llvm_unreachable("ClangdTests never makes absolute path.");
+  }
+
+  llvm::Expected
+  uriFromAbsolutePath(llvm::StringRef AbsolutePath) const override {
+llvm_unreachable("ClangdTest never creates a test URI.");
+  }
+
+  llvm::Expected getSpellingInclude(const URI &U) const override {
+return ("\"" + U.body().trim("/") + "\"").str();
+  }
+};
+
+static URISchemeRegistry::Add
+X(ClangdTestScheme, "Test scheme for ClangdTests.");
+
 TEST_F(ClangdVFSTest, Parse) {
   // FIXME: figure out a stable format for AST dumps, so that we can check the
   // output of the dump itself is equal to the expected one, not just that it's
@@ -961,6 +983,10 @@
/*Preferred=*/"", ""));
   EXPECT_TRUE(Inserted(OriginalHeader, PreferredHeader, "\"Y.h\""));
   EXPECT_TRUE(Inserted("", PreferredHeader, "\"Y.h\""));
+  auto TestURIHeader =
+  URI::parse(llvm::formatv("{0}:///x/y/z.h", ClangdTestScheme).str());
+  EXPECT_TRUE(static_cast(TestURIHeader));
+  EXPECT_TRUE(Inserted(TestURIHeader->toString(), "", "\"x/y/z.h\""));
 
   // Check that includes are sorted.
   const auto Expected = R"cpp(
Index: clangd/URI.h
===
--- clangd/URI.h
+++ clangd/URI.h
@@ -60,6 +60,16 @@
   static llvm::Expected resolve(const URI &U,
  llvm::StringRef HintPath = "");
 
+  /// Gets the preferred spelling of this file for #include, if there is one,
+  /// e.g. , "path/to/x.h".
+  ///
+  /// This allows URI schemas to provide their customized include paths.
+  ///
+  /// Returns an empty string if normal include-shortening based on the absolute
+  /// path should be used.
+  /// Fails if the URI is not valid in the schema.
+  static llvm::Expected spellingInclude(const URI &U);
+
   friend bool operator==(const URI &LHS, const URI &RHS) {
 return std::tie(LHS.Scheme, LHS.Authority, LHS.Body) ==
std::tie(RHS.Scheme, RHS.Authority, RHS.Body);
@@ -94,6 +104,13 @@
 
   virtual llvm::Expected
   uriFromAbsolutePath(llvm::StringRef AbsolutePath) const = 0;
+
+  /// Returns the include path of the file (e.g. , "path"), which can be
+  /// #included directly. See URI::spellingInclude for details.
+  virtual llvm::Expected
+  getSpellingInclude(const URI& U) const {
+return "";  // no customized include path for this scheme.
+  }
 };
 
 /// By default, a "file" scheme is supported where URI paths are always absolute
Index: clangd/URI.cpp
===
--- clangd/URI.cpp
+++ clangd/URI.cpp
@@ -196,5 +196,12 @@
   return S->get()->getAbsolutePath(Uri.Authority, Uri.Body, HintPath);
 }
 
+llvm::Expected URI::spellingInclude(const URI &Uri) {
+  auto S = findSchemeByName(Uri.Scheme);
+  if (!S)
+return S.takeError();
+  return S->get()->getSpellingInclude(Uri);
+}
+
 } // namespace clangd
 } // namespace clang
Index: clangd/ClangdServer.cpp
===
--- clangd/ClangdServer.cpp
+++ clangd/ClangdServer.cpp
@@ -286,6 +286,13 @@
   auto U = URI::parse(Header);
   if (!U)
 return U.takeError();
+
+  auto IncludePath = URI::spellingInclude(*U);
+  if (!IncludePath)
+return IncludePath.takeError();
+  if (!IncludePath->empty())
+return HeaderFile{std::move(*IncludePath), /*Verbatim=*/true};
+
   auto Resolved = URI::resolve(*U, HintPath);
   if (!Resolved)
 return Resolved.takeError();
___
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:41
+  binaryOperator(
+  anyOf(hasOperatorName("+="), hasOperatorName("-=")),
+  hasLHS(hasType(isInteger())),

alexfh wrote:
> JonasToth wrote:
> > courbet wrote:
> > > JonasToth wrote:
> > > > I would really like to add all other calc-and-assign operations. At 
> > > > least "*=" and "/=".
> > > Makes sense, I've added `*=` and `/=`. All others (`%=`, `&=`, `<<=` and 
> > > variations thereof)  trigger a compilation error if the RHS is a float 
> > > (rightfully).
> > > 
> > > 
> > For floats that is true.
> > 
> > If we care about the `ìnt->char` casts later, they should be added.
> Does it make sense to match all assignment operators including just `=`? If 
> so, the matcher will become a bit simpler: 
> `binaryOperator(isAssignmentOperator(), ...)`. If there's a reason not to do 
> this, maybe adding a comment would be useful
> 
> Apart from that, I wonder whether the matcher above would also cover the case 
> of assignment operators? I would expect the AST for assignment expressions 
> with different types to have ImplicitCast nodes anyway.
> If we care about the ìnt->char casts later, they should be added.

Will do. I'd like to start by implementing only floats first though, because 
they are the ones where I have data.

> Apart from that, I wonder whether the matcher above would also cover the case 
> of assignment operators?

It does cover `=` (which generates an implicit cast), but not `+=`.


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] D45426: [clangd] Allow using customized include path in URI.

[Haojian Wu via Phabricator via cfe-commits]

hokein marked an inline comment as done.
hokein added inline comments.



Comment at: clangd/URI.h:66
+  ///
+  /// If the returned string is non-empty, clangd will use it directly when
+  /// doing include insertion; otherwise we will fall back to the clang to

sammccall wrote:
> does this include ? it probably should, if we're allowing schemes to 
> customize how includes are spelled.
Yes, the returned string is a literal string quoted with either <> or "". 



Comment at: clangd/URI.h:69
+  /// calculate the include path from the resolved absolute path.
+  static llvm::Expected includePath(const URI &U);
+

sammccall wrote:
> i'd avoid the name "include path" because it can be confused with a) the set 
> of directories passed to -I and b) the filesystem path to the file to be 
> included.
> Suggest includeString or so
Renamed it to `spellingInclude`.



Comment at: unittests/clangd/ClangdTests.cpp:964
   EXPECT_TRUE(Inserted("", PreferredHeader, "\"Y.h\""));
+  auto TestURIHeader = URI::create("/abc/test-root/x/y/z.h", "unittest");
+  EXPECT_TRUE(static_cast(TestURIHeader));

sammccall wrote:
> this relies on ClangdTests and URITests being linked together, which we chose 
> to avoid last time this came up. Just define a scheme here?
> (This is one place where putting a field on URI would have been simpler)
I think we probably want a scheme for unittest purpose (we also have one for 
lit test), but this case only needs a trivial one, added here.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45426



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

[PATCH] D45426: [clangd] Allow using customized include path in URI.

[Sam McCall via Phabricator via cfe-commits]

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



Comment at: clangd/URI.h:69
+  /// calculate the include path from the resolved absolute path.
+  static llvm::Expected includePath(const URI &U);
+

hokein wrote:
> sammccall wrote:
> > i'd avoid the name "include path" because it can be confused with a) the 
> > set of directories passed to -I and b) the filesystem path to the file to 
> > be included.
> > Suggest includeString or so
> Renamed it to `spellingInclude`.
nit: I think `includeSpelling` would be clearer, since include is the 
adjective. Up to you


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45426



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

[PATCH] D45426: [clangd] Allow using customized include path in URI.

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 141654.
hokein marked an inline comment as done.
hokein added a comment.

includeSpelling


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45426

Files:
  clangd/ClangdServer.cpp
  clangd/URI.cpp
  clangd/URI.h
  unittests/clangd/ClangdTests.cpp
  unittests/clangd/TestScheme.h

Index: unittests/clangd/ClangdTests.cpp
===
--- unittests/clangd/ClangdTests.cpp
+++ unittests/clangd/ClangdTests.cpp
@@ -152,6 +152,28 @@
   }
 };
 
+constexpr const char* ClangdTestScheme = "ClangdTests";
+class TestURIScheme : public URIScheme {
+public:
+  llvm::Expected
+  getAbsolutePath(llvm::StringRef /*Authority*/, llvm::StringRef Body,
+  llvm::StringRef /*HintPath*/) const override {
+llvm_unreachable("ClangdTests never makes absolute path.");
+  }
+
+  llvm::Expected
+  uriFromAbsolutePath(llvm::StringRef AbsolutePath) const override {
+llvm_unreachable("ClangdTest never creates a test URI.");
+  }
+
+  llvm::Expected getIncludeSpelling(const URI &U) const override {
+return ("\"" + U.body().trim("/") + "\"").str();
+  }
+};
+
+static URISchemeRegistry::Add
+X(ClangdTestScheme, "Test scheme for ClangdTests.");
+
 TEST_F(ClangdVFSTest, Parse) {
   // FIXME: figure out a stable format for AST dumps, so that we can check the
   // output of the dump itself is equal to the expected one, not just that it's
@@ -961,6 +983,10 @@
/*Preferred=*/"", ""));
   EXPECT_TRUE(Inserted(OriginalHeader, PreferredHeader, "\"Y.h\""));
   EXPECT_TRUE(Inserted("", PreferredHeader, "\"Y.h\""));
+  auto TestURIHeader =
+  URI::parse(llvm::formatv("{0}:///x/y/z.h", ClangdTestScheme).str());
+  EXPECT_TRUE(static_cast(TestURIHeader));
+  EXPECT_TRUE(Inserted(TestURIHeader->toString(), "", "\"x/y/z.h\""));
 
   // Check that includes are sorted.
   const auto Expected = R"cpp(
Index: clangd/URI.h
===
--- clangd/URI.h
+++ clangd/URI.h
@@ -60,6 +60,16 @@
   static llvm::Expected resolve(const URI &U,
  llvm::StringRef HintPath = "");
 
+  /// Gets the preferred spelling of this file for #include, if there is one,
+  /// e.g. , "path/to/x.h".
+  ///
+  /// This allows URI schemas to provide their customized include paths.
+  ///
+  /// Returns an empty string if normal include-shortening based on the absolute
+  /// path should be used.
+  /// Fails if the URI is not valid in the schema.
+  static llvm::Expected includeSpelling(const URI &U);
+
   friend bool operator==(const URI &LHS, const URI &RHS) {
 return std::tie(LHS.Scheme, LHS.Authority, LHS.Body) ==
std::tie(RHS.Scheme, RHS.Authority, RHS.Body);
@@ -94,6 +104,13 @@
 
   virtual llvm::Expected
   uriFromAbsolutePath(llvm::StringRef AbsolutePath) const = 0;
+
+  /// Returns the include path of the file (e.g. , "path"), which can be
+  /// #included directly. See URI::includeSpelling for details.
+  virtual llvm::Expected
+  getIncludeSpelling(const URI& U) const {
+return "";  // no customized include path for this scheme.
+  }
 };
 
 /// By default, a "file" scheme is supported where URI paths are always absolute
Index: clangd/URI.cpp
===
--- clangd/URI.cpp
+++ clangd/URI.cpp
@@ -196,5 +196,12 @@
   return S->get()->getAbsolutePath(Uri.Authority, Uri.Body, HintPath);
 }
 
+llvm::Expected URI::includeSpelling(const URI &Uri) {
+  auto S = findSchemeByName(Uri.Scheme);
+  if (!S)
+return S.takeError();
+  return S->get()->getIncludeSpelling(Uri);
+}
+
 } // namespace clangd
 } // namespace clang
Index: clangd/ClangdServer.cpp
===
--- clangd/ClangdServer.cpp
+++ clangd/ClangdServer.cpp
@@ -286,6 +286,13 @@
   auto U = URI::parse(Header);
   if (!U)
 return U.takeError();
+
+  auto IncludePath = URI::includeSpelling(*U);
+  if (!IncludePath)
+return IncludePath.takeError();
+  if (!IncludePath->empty())
+return HeaderFile{std::move(*IncludePath), /*Verbatim=*/true};
+
   auto Resolved = URI::resolve(*U, HintPath);
   if (!Resolved)
 return Resolved.takeError();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r329578 - [clangd] Allow using customized include path in URI.

[Haojian Wu via cfe-commits]

Author: hokein
Date: Mon Apr  9 08:09:44 2018
New Revision: 329578

URL: http://llvm.org/viewvc/llvm-project?rev=329578&view=rev
Log:
[clangd] Allow using customized include path in URI.

Summary:
Calculating the include path from absolute file path does not always
work for all build system, e.g. bazel uses symlink as the build working
directory. The absolute file path from editor and clang is diverged from
each other. We need to address it properly in build sysmtem integration.

This patch worksarounds the issue by providing a hook in URI which allows
clients to provide their customized include path.

Reviewers: sammccall

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

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

Added:
clang-tools-extra/trunk/unittests/clangd/TestScheme.h
Modified:
clang-tools-extra/trunk/clangd/ClangdServer.cpp
clang-tools-extra/trunk/clangd/URI.cpp
clang-tools-extra/trunk/clangd/URI.h
clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp

Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=329578&r1=329577&r2=329578&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Mon Apr  9 08:09:44 2018
@@ -286,6 +286,13 @@ static llvm::Expected toHead
   auto U = URI::parse(Header);
   if (!U)
 return U.takeError();
+
+  auto IncludePath = URI::includeSpelling(*U);
+  if (!IncludePath)
+return IncludePath.takeError();
+  if (!IncludePath->empty())
+return HeaderFile{std::move(*IncludePath), /*Verbatim=*/true};
+
   auto Resolved = URI::resolve(*U, HintPath);
   if (!Resolved)
 return Resolved.takeError();

Modified: clang-tools-extra/trunk/clangd/URI.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/URI.cpp?rev=329578&r1=329577&r2=329578&view=diff
==
--- clang-tools-extra/trunk/clangd/URI.cpp (original)
+++ clang-tools-extra/trunk/clangd/URI.cpp Mon Apr  9 08:09:44 2018
@@ -196,5 +196,12 @@ llvm::Expected URI::resolve
   return S->get()->getAbsolutePath(Uri.Authority, Uri.Body, HintPath);
 }
 
+llvm::Expected URI::includeSpelling(const URI &Uri) {
+  auto S = findSchemeByName(Uri.Scheme);
+  if (!S)
+return S.takeError();
+  return S->get()->getIncludeSpelling(Uri);
+}
+
 } // namespace clangd
 } // namespace clang

Modified: clang-tools-extra/trunk/clangd/URI.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/URI.h?rev=329578&r1=329577&r2=329578&view=diff
==
--- clang-tools-extra/trunk/clangd/URI.h (original)
+++ clang-tools-extra/trunk/clangd/URI.h Mon Apr  9 08:09:44 2018
@@ -60,6 +60,16 @@ public:
   static llvm::Expected resolve(const URI &U,
  llvm::StringRef HintPath = "");
 
+  /// Gets the preferred spelling of this file for #include, if there is one,
+  /// e.g. , "path/to/x.h".
+  ///
+  /// This allows URI schemas to provide their customized include paths.
+  ///
+  /// Returns an empty string if normal include-shortening based on the 
absolute
+  /// path should be used.
+  /// Fails if the URI is not valid in the schema.
+  static llvm::Expected includeSpelling(const URI &U);
+
   friend bool operator==(const URI &LHS, const URI &RHS) {
 return std::tie(LHS.Scheme, LHS.Authority, LHS.Body) ==
std::tie(RHS.Scheme, RHS.Authority, RHS.Body);
@@ -94,6 +104,13 @@ public:
 
   virtual llvm::Expected
   uriFromAbsolutePath(llvm::StringRef AbsolutePath) const = 0;
+
+  /// Returns the include path of the file (e.g. , "path"), which can be
+  /// #included directly. See URI::includeSpelling for details.
+  virtual llvm::Expected
+  getIncludeSpelling(const URI& U) const {
+return "";  // no customized include path for this scheme.
+  }
 };
 
 /// By default, a "file" scheme is supported where URI paths are always 
absolute

Modified: clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp?rev=329578&r1=329577&r2=329578&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp Mon Apr  9 
08:09:44 2018
@@ -152,6 +152,28 @@ protected:
   }
 };
 
+constexpr const char* ClangdTestScheme = "ClangdTests";
+class TestURIScheme : public URIScheme {
+public:
+  llvm::Expected
+  getAbsolutePath(llvm::StringRef /*Authority*/, llvm::StringRef Body,
+  llvm::StringRef /*HintPath*/) const override {
+llvm_unreachable("ClangdTests 

[PATCH] D45426: [clangd] Allow using customized include path in URI.

[Haojian Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE329578: [clangd] Allow using customized include path in 
URI. (authored by hokein, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D45426?vs=141654&id=141655#toc

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45426

Files:
  clangd/ClangdServer.cpp
  clangd/URI.cpp
  clangd/URI.h
  unittests/clangd/ClangdTests.cpp
  unittests/clangd/TestScheme.h

Index: unittests/clangd/ClangdTests.cpp
===
--- unittests/clangd/ClangdTests.cpp
+++ unittests/clangd/ClangdTests.cpp
@@ -152,6 +152,28 @@
   }
 };
 
+constexpr const char* ClangdTestScheme = "ClangdTests";
+class TestURIScheme : public URIScheme {
+public:
+  llvm::Expected
+  getAbsolutePath(llvm::StringRef /*Authority*/, llvm::StringRef Body,
+  llvm::StringRef /*HintPath*/) const override {
+llvm_unreachable("ClangdTests never makes absolute path.");
+  }
+
+  llvm::Expected
+  uriFromAbsolutePath(llvm::StringRef AbsolutePath) const override {
+llvm_unreachable("ClangdTest never creates a test URI.");
+  }
+
+  llvm::Expected getIncludeSpelling(const URI &U) const override {
+return ("\"" + U.body().trim("/") + "\"").str();
+  }
+};
+
+static URISchemeRegistry::Add
+X(ClangdTestScheme, "Test scheme for ClangdTests.");
+
 TEST_F(ClangdVFSTest, Parse) {
   // FIXME: figure out a stable format for AST dumps, so that we can check the
   // output of the dump itself is equal to the expected one, not just that it's
@@ -961,6 +983,10 @@
/*Preferred=*/"", ""));
   EXPECT_TRUE(Inserted(OriginalHeader, PreferredHeader, "\"Y.h\""));
   EXPECT_TRUE(Inserted("", PreferredHeader, "\"Y.h\""));
+  auto TestURIHeader =
+  URI::parse(llvm::formatv("{0}:///x/y/z.h", ClangdTestScheme).str());
+  EXPECT_TRUE(static_cast(TestURIHeader));
+  EXPECT_TRUE(Inserted(TestURIHeader->toString(), "", "\"x/y/z.h\""));
 
   // Check that includes are sorted.
   const auto Expected = R"cpp(
Index: clangd/ClangdServer.cpp
===
--- clangd/ClangdServer.cpp
+++ clangd/ClangdServer.cpp
@@ -286,6 +286,13 @@
   auto U = URI::parse(Header);
   if (!U)
 return U.takeError();
+
+  auto IncludePath = URI::includeSpelling(*U);
+  if (!IncludePath)
+return IncludePath.takeError();
+  if (!IncludePath->empty())
+return HeaderFile{std::move(*IncludePath), /*Verbatim=*/true};
+
   auto Resolved = URI::resolve(*U, HintPath);
   if (!Resolved)
 return Resolved.takeError();
Index: clangd/URI.cpp
===
--- clangd/URI.cpp
+++ clangd/URI.cpp
@@ -196,5 +196,12 @@
   return S->get()->getAbsolutePath(Uri.Authority, Uri.Body, HintPath);
 }
 
+llvm::Expected URI::includeSpelling(const URI &Uri) {
+  auto S = findSchemeByName(Uri.Scheme);
+  if (!S)
+return S.takeError();
+  return S->get()->getIncludeSpelling(Uri);
+}
+
 } // namespace clangd
 } // namespace clang
Index: clangd/URI.h
===
--- clangd/URI.h
+++ clangd/URI.h
@@ -60,6 +60,16 @@
   static llvm::Expected resolve(const URI &U,
  llvm::StringRef HintPath = "");
 
+  /// Gets the preferred spelling of this file for #include, if there is one,
+  /// e.g. , "path/to/x.h".
+  ///
+  /// This allows URI schemas to provide their customized include paths.
+  ///
+  /// Returns an empty string if normal include-shortening based on the absolute
+  /// path should be used.
+  /// Fails if the URI is not valid in the schema.
+  static llvm::Expected includeSpelling(const URI &U);
+
   friend bool operator==(const URI &LHS, const URI &RHS) {
 return std::tie(LHS.Scheme, LHS.Authority, LHS.Body) ==
std::tie(RHS.Scheme, RHS.Authority, RHS.Body);
@@ -94,6 +104,13 @@
 
   virtual llvm::Expected
   uriFromAbsolutePath(llvm::StringRef AbsolutePath) const = 0;
+
+  /// Returns the include path of the file (e.g. , "path"), which can be
+  /// #included directly. See URI::includeSpelling for details.
+  virtual llvm::Expected
+  getIncludeSpelling(const URI& U) const {
+return "";  // no customized include path for this scheme.
+  }
 };
 
 /// By default, a "file" scheme is supported where URI paths are always absolute
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r329579 - [clang-tidy] Return non-zero exit code for clang errors.

[Alexander Kornienko via cfe-commits]

Author: alexfh
Date: Mon Apr  9 08:12:10 2018
New Revision: 329579

URL: http://llvm.org/viewvc/llvm-project?rev=329579&view=rev
Log:
[clang-tidy] Return non-zero exit code for clang errors.

Summary:
Updated tests broken by this change.
Fixes https://bugs.llvm.org/show_bug.cgi?id=27628

Reviewers: ilya-biryukov

Reviewed By: ilya-biryukov

Subscribers: klimek, xazax.hun, cfe-commits

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

Modified:
clang-tools-extra/trunk/clang-tidy/tool/ClangTidyMain.cpp

clang-tools-extra/trunk/test/clang-tidy/bugprone-suspicious-semicolon-fail.cpp
clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy.py
clang-tools-extra/trunk/test/clang-tidy/diagnostic.cpp
clang-tools-extra/trunk/test/clang-tidy/fix-errors.cpp
clang-tools-extra/trunk/test/clang-tidy/misc-misplaced-const-cxx17.cpp
clang-tools-extra/trunk/test/clang-tidy/misc-unused-using-decls-errors.cpp

clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-assert-failure.cpp

clang-tools-extra/trunk/test/clang-tidy/readability-braces-around-statements-assert-failure.cpp
clang-tools-extra/trunk/test/clang-tidy/serialize-diagnostics.cpp

Modified: clang-tools-extra/trunk/clang-tidy/tool/ClangTidyMain.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/tool/ClangTidyMain.cpp?rev=329579&r1=329578&r2=329579&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/tool/ClangTidyMain.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/tool/ClangTidyMain.cpp Mon Apr  9 
08:12:10 2018
@@ -448,10 +448,9 @@ static int clangTidyMain(int argc, const
   runClangTidy(Context, OptionsParser.getCompilations(), PathList, BaseFS,
EnableCheckProfile ? &Profile : nullptr);
   ArrayRef Errors = Context.getErrors();
-  bool FoundErrors =
-  std::find_if(Errors.begin(), Errors.end(), [](const ClangTidyError &E) {
-return E.DiagLevel == ClangTidyError::Error;
-  }) != Errors.end();
+  bool FoundErrors = llvm::find_if(Errors, [](const ClangTidyError &E) {
+   return E.DiagLevel == ClangTidyError::Error;
+ }) != Errors.end();
 
   const bool DisableFixes = Fix && FoundErrors && !FixErrors;
 
@@ -491,6 +490,19 @@ static int clangTidyMain(int argc, const
 return WErrorCount;
   }
 
+  if (FoundErrors) {
+// TODO: Figure out when zero exit code should be used with -fix-errors:
+//   a. when a fix has been applied for an error
+//   b. when a fix has been applied for all errors
+//   c. some other condition.
+// For now always returning zero when -fix-errors is used.
+if (FixErrors)
+  return 0;
+if (!Quiet)
+  llvm::errs() << "Found compiler error(s).\n";
+return 1;
+  }
+
   return 0;
 }
 

Modified: 
clang-tools-extra/trunk/test/clang-tidy/bugprone-suspicious-semicolon-fail.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/bugprone-suspicious-semicolon-fail.cpp?rev=329579&r1=329578&r2=329579&view=diff
==
--- 
clang-tools-extra/trunk/test/clang-tidy/bugprone-suspicious-semicolon-fail.cpp 
(original)
+++ 
clang-tools-extra/trunk/test/clang-tidy/bugprone-suspicious-semicolon-fail.cpp 
Mon Apr  9 08:12:10 2018
@@ -1,7 +1,9 @@
-// RUN: clang-tidy %s -checks="-*,bugprone-suspicious-semicolon" -- -DERROR 
2>&1 \
+// RUN: not clang-tidy %s \
+// RUN: -checks="-*,bugprone-suspicious-semicolon" -- -DERROR 2>&1 \
 // RUN:   | FileCheck %s -check-prefix=CHECK-ERROR \
 // RUN:   -implicit-check-not="{{warning|error}}:"
-// RUN: clang-tidy %s 
-checks="-*,bugprone-suspicious-semicolon,clang-diagnostic*" \
+// RUN: not clang-tidy %s \
+// RUN: -checks="-*,bugprone-suspicious-semicolon,clang-diagnostic*" \
 // RUN:-- -DWERROR -Wno-everything -Werror=unused-variable 2>&1 \
 // RUN:   | FileCheck %s -check-prefix=CHECK-WERROR \
 // RUN:   -implicit-check-not="{{warning|error}}:"

Modified: clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy.py
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy.py?rev=329579&r1=329578&r2=329579&view=diff
==
--- clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy.py (original)
+++ clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy.py Mon Apr  9 
08:12:10 2018
@@ -39,6 +39,7 @@ def write_file(file_name, text):
 
 def main():
   parser = argparse.ArgumentParser()
+  parser.add_argument('-expect-clang-tidy-error', action='store_true')
   parser.add_argument('-resource-dir')
   parser.add_argument('-assume-filename')
   parser.add_argument('input_file_name')
@@ -52,6 +53,7 @@ def main():
   input_file_name = args.input_file_name
   check_name = args.check_name
   temp_file_name = args.temp_file_name
+  

[PATCH] D45405: [clang-tidy] [modernize-use-auto] Add a threshold for minimal type name length to be replaced with 'auto'

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh requested changes to this revision.
alexfh added inline comments.
This revision now requires changes to proceed.



Comment at: clang-tidy/modernize/UseAutoCheck.cpp:290
+: ClangTidyCheck(Name, Context), RemoveStars(Options.get("RemoveStars", 
0)),
+  MinTypeNameLength(Options.get("MinTypeNameLength", 0)) {}
 

lebedev.ri wrote:
> zinovy.nis wrote:
> > lebedev.ri wrote:
> > > alexfh wrote:
> > > > Maybe make the default 5? Or does anyone really want to replace 
> > > > `int/long/char/bool/...` with `auto`?
> > > That might be a bit surprising behavioral change..
> > > At least it should be spelled out in the release notes.
> > > (my 5 cent: defaulting to 0 would be best)
> > Maybe we can somehow mention the current option value in a warning message?
> Sure, can be done, but that will only be seen when it does fire, not when it 
> suddenly stops firing...
> Maybe we can somehow mention the current option value in a warning message?

We don't do that for options of other checks (and for the other option here). I 
don't think this case is substantially different.



Comment at: clang-tidy/modernize/UseAutoCheck.cpp:290
+: ClangTidyCheck(Name, Context), RemoveStars(Options.get("RemoveStars", 
0)),
+  MinTypeNameLength(Options.get("MinTypeNameLength", 0)) {}
 

alexfh wrote:
> lebedev.ri wrote:
> > zinovy.nis wrote:
> > > lebedev.ri wrote:
> > > > alexfh wrote:
> > > > > Maybe make the default 5? Or does anyone really want to replace 
> > > > > `int/long/char/bool/...` with `auto`?
> > > > That might be a bit surprising behavioral change..
> > > > At least it should be spelled out in the release notes.
> > > > (my 5 cent: defaulting to 0 would be best)
> > > Maybe we can somehow mention the current option value in a warning 
> > > message?
> > Sure, can be done, but that will only be seen when it does fire, not when 
> > it suddenly stops firing...
> > Maybe we can somehow mention the current option value in a warning message?
> 
> We don't do that for options of other checks (and for the other option here). 
> I don't think this case is substantially different.
> That might be a bit surprising behavioral change..

For many it will be a welcome change ;)

> At least it should be spelled out in the release notes.

No objections here.

> (my 5 cent: defaulting to 0 would be best)

You see it, 5 is a better default, otherwise you'd say "0 cent" ;)

On a serious note, the style guides I'm more or less familiar with recommend 
the use of `auto` for "long/cluttery type names" [1], "if and only if it makes 
the code more readable or easier to maintain" [2], or to "save writing a 
longish, hard-to-remember type that the compiler already knows but a programmer 
could get wrong" [3]. None of these guidelines seem to endorse the use of 
`auto` instead of `int`, `bool` or the like.

From my perspective, the default value of 5 would cover a larger fraction of 
real-world use cases.

[1] https://google.github.io/styleguide/cppguide.html#auto
[2] 
http://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable
[3] 
https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#es11-use-auto-to-avoid-redundant-repetition-of-type-names



Comment at: clang-tidy/modernize/UseAutoCheck.cpp:420-421
+  if (MinTypeNameLength != 0 &&
+  Lexer::getSourceText(CharSourceRange::getTokenRange(Range),
+   Context->getSourceManager(), Context->getLangOpts())
+  .size() < MinTypeNameLength)

Consider using tooling::fixit::getText(Loc, Context) instead.


https://reviews.llvm.org/D45405



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

[PATCH] D45258: [clang-tidy] Return non-zero exit code for clang errors.

[Alexander Kornienko via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE329579: [clang-tidy] Return non-zero exit code for clang 
errors. (authored by alexfh, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D45258?vs=140956&id=141656#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D45258

Files:
  clang-tidy/tool/ClangTidyMain.cpp
  test/clang-tidy/bugprone-suspicious-semicolon-fail.cpp
  test/clang-tidy/check_clang_tidy.py
  test/clang-tidy/diagnostic.cpp
  test/clang-tidy/fix-errors.cpp
  test/clang-tidy/misc-misplaced-const-cxx17.cpp
  test/clang-tidy/misc-unused-using-decls-errors.cpp
  test/clang-tidy/modernize-loop-convert-assert-failure.cpp
  test/clang-tidy/readability-braces-around-statements-assert-failure.cpp
  test/clang-tidy/serialize-diagnostics.cpp

Index: test/clang-tidy/misc-misplaced-const-cxx17.cpp
===
--- test/clang-tidy/misc-misplaced-const-cxx17.cpp
+++ test/clang-tidy/misc-misplaced-const-cxx17.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s misc-misplaced-const %t -- -- -std=c++17
+// RUN: %check_clang_tidy -expect-clang-tidy-error %s misc-misplaced-const %t -- -- -std=c++17
 
 // This test previously would cause a failed assertion because the structured
 // binding declaration had no valid type associated with it. This ensures the
Index: test/clang-tidy/bugprone-suspicious-semicolon-fail.cpp
===
--- test/clang-tidy/bugprone-suspicious-semicolon-fail.cpp
+++ test/clang-tidy/bugprone-suspicious-semicolon-fail.cpp
@@ -1,7 +1,9 @@
-// RUN: clang-tidy %s -checks="-*,bugprone-suspicious-semicolon" -- -DERROR 2>&1 \
+// RUN: not clang-tidy %s \
+// RUN: -checks="-*,bugprone-suspicious-semicolon" -- -DERROR 2>&1 \
 // RUN:   | FileCheck %s -check-prefix=CHECK-ERROR \
 // RUN:   -implicit-check-not="{{warning|error}}:"
-// RUN: clang-tidy %s -checks="-*,bugprone-suspicious-semicolon,clang-diagnostic*" \
+// RUN: not clang-tidy %s \
+// RUN: -checks="-*,bugprone-suspicious-semicolon,clang-diagnostic*" \
 // RUN:-- -DWERROR -Wno-everything -Werror=unused-variable 2>&1 \
 // RUN:   | FileCheck %s -check-prefix=CHECK-WERROR \
 // RUN:   -implicit-check-not="{{warning|error}}:"
Index: test/clang-tidy/readability-braces-around-statements-assert-failure.cpp
===
--- test/clang-tidy/readability-braces-around-statements-assert-failure.cpp
+++ test/clang-tidy/readability-braces-around-statements-assert-failure.cpp
@@ -1,4 +1,4 @@
-// RUN: clang-tidy -checks='-*,readability-braces-around-statements' %s --
+// RUN: not clang-tidy -checks='-*,readability-braces-around-statements' %s --
 
 // Note: this test expects no assert failure happened in clang-tidy.
 
Index: test/clang-tidy/fix-errors.cpp
===
--- test/clang-tidy/fix-errors.cpp
+++ test/clang-tidy/fix-errors.cpp
@@ -1,5 +1,5 @@
 // RUN: grep -Ev "// *[A-Z-]+:" %s > %t.cpp
-// RUN: clang-tidy %t.cpp -checks='-*,google-explicit-constructor' -fix -- > %t.msg 2>&1
+// RUN: not clang-tidy %t.cpp -checks='-*,google-explicit-constructor' -fix -- > %t.msg 2>&1
 // RUN: FileCheck -input-file=%t.cpp -check-prefix=CHECK-FIX %s
 // RUN: FileCheck -input-file=%t.msg -check-prefix=CHECK-MESSAGES %s
 // RUN: grep -Ev "// *[A-Z-]+:" %s > %t.cpp
Index: test/clang-tidy/modernize-loop-convert-assert-failure.cpp
===
--- test/clang-tidy/modernize-loop-convert-assert-failure.cpp
+++ test/clang-tidy/modernize-loop-convert-assert-failure.cpp
@@ -1,4 +1,4 @@
-// RUN: clang-tidy %s -checks=-*,modernize-loop-convert --
+// RUN: not clang-tidy %s -checks=-*,modernize-loop-convert --
 
 // Note: this test expects no assert failure happened in clang-tidy.
 
Index: test/clang-tidy/diagnostic.cpp
===
--- test/clang-tidy/diagnostic.cpp
+++ test/clang-tidy/diagnostic.cpp
@@ -1,25 +1,28 @@
-// RUN: clang-tidy -checks='-*,modernize-use-override' %s.nonexistent.cpp -- | FileCheck -check-prefix=CHECK1 -implicit-check-not='{{warning:|error:}}' %s
-// RUN: clang-tidy -checks='-*,clang-diagnostic-*,google-explicit-constructor' %s -- -fan-unknown-option | FileCheck -check-prefix=CHECK2 -implicit-check-not='{{warning:|error:}}' %s
-// RUN: clang-tidy -checks='-*,google-explicit-constructor,clang-diagnostic-literal-conversion' %s -- -fan-unknown-option | FileCheck -check-prefix=CHECK3 -implicit-check-not='{{warning:|error:}}' %s
+// RUN: not clang-tidy -checks='-*,modernize-use-override' %s.nonexistent.cpp -- | FileCheck -check-prefix=CHECK1 -implicit-check-not='{{warning:|error:}}' %s
+// RUN: not clang-tidy -checks='-*,clang-diagnostic-*,google-explicit-constructor' %s -- -fan-unknown-option | FileCheck -check-prefix=CHECK2 -implicit-check-not

[PATCH] D45258: [clang-tidy] Return non-zero exit code for clang errors.

[Alexander Kornienko via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL329579: [clang-tidy] Return non-zero exit code for clang 
errors. (authored by alexfh, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D45258

Files:
  clang-tools-extra/trunk/clang-tidy/tool/ClangTidyMain.cpp
  clang-tools-extra/trunk/test/clang-tidy/bugprone-suspicious-semicolon-fail.cpp
  clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy.py
  clang-tools-extra/trunk/test/clang-tidy/diagnostic.cpp
  clang-tools-extra/trunk/test/clang-tidy/fix-errors.cpp
  clang-tools-extra/trunk/test/clang-tidy/misc-misplaced-const-cxx17.cpp
  clang-tools-extra/trunk/test/clang-tidy/misc-unused-using-decls-errors.cpp
  
clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-assert-failure.cpp
  
clang-tools-extra/trunk/test/clang-tidy/readability-braces-around-statements-assert-failure.cpp
  clang-tools-extra/trunk/test/clang-tidy/serialize-diagnostics.cpp

Index: clang-tools-extra/trunk/clang-tidy/tool/ClangTidyMain.cpp
===
--- clang-tools-extra/trunk/clang-tidy/tool/ClangTidyMain.cpp
+++ clang-tools-extra/trunk/clang-tidy/tool/ClangTidyMain.cpp
@@ -448,10 +448,9 @@
   runClangTidy(Context, OptionsParser.getCompilations(), PathList, BaseFS,
EnableCheckProfile ? &Profile : nullptr);
   ArrayRef Errors = Context.getErrors();
-  bool FoundErrors =
-  std::find_if(Errors.begin(), Errors.end(), [](const ClangTidyError &E) {
-return E.DiagLevel == ClangTidyError::Error;
-  }) != Errors.end();
+  bool FoundErrors = llvm::find_if(Errors, [](const ClangTidyError &E) {
+   return E.DiagLevel == ClangTidyError::Error;
+ }) != Errors.end();
 
   const bool DisableFixes = Fix && FoundErrors && !FixErrors;
 
@@ -491,6 +490,19 @@
 return WErrorCount;
   }
 
+  if (FoundErrors) {
+// TODO: Figure out when zero exit code should be used with -fix-errors:
+//   a. when a fix has been applied for an error
+//   b. when a fix has been applied for all errors
+//   c. some other condition.
+// For now always returning zero when -fix-errors is used.
+if (FixErrors)
+  return 0;
+if (!Quiet)
+  llvm::errs() << "Found compiler error(s).\n";
+return 1;
+  }
+
   return 0;
 }
 
Index: clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-assert-failure.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-assert-failure.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-assert-failure.cpp
@@ -1,4 +1,4 @@
-// RUN: clang-tidy %s -checks=-*,modernize-loop-convert --
+// RUN: not clang-tidy %s -checks=-*,modernize-loop-convert --
 
 // Note: this test expects no assert failure happened in clang-tidy.
 
Index: clang-tools-extra/trunk/test/clang-tidy/fix-errors.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/fix-errors.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/fix-errors.cpp
@@ -1,5 +1,5 @@
 // RUN: grep -Ev "// *[A-Z-]+:" %s > %t.cpp
-// RUN: clang-tidy %t.cpp -checks='-*,google-explicit-constructor' -fix -- > %t.msg 2>&1
+// RUN: not clang-tidy %t.cpp -checks='-*,google-explicit-constructor' -fix -- > %t.msg 2>&1
 // RUN: FileCheck -input-file=%t.cpp -check-prefix=CHECK-FIX %s
 // RUN: FileCheck -input-file=%t.msg -check-prefix=CHECK-MESSAGES %s
 // RUN: grep -Ev "// *[A-Z-]+:" %s > %t.cpp
Index: clang-tools-extra/trunk/test/clang-tidy/readability-braces-around-statements-assert-failure.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/readability-braces-around-statements-assert-failure.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/readability-braces-around-statements-assert-failure.cpp
@@ -1,4 +1,4 @@
-// RUN: clang-tidy -checks='-*,readability-braces-around-statements' %s --
+// RUN: not clang-tidy -checks='-*,readability-braces-around-statements' %s --
 
 // Note: this test expects no assert failure happened in clang-tidy.
 
Index: clang-tools-extra/trunk/test/clang-tidy/diagnostic.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/diagnostic.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/diagnostic.cpp
@@ -1,25 +1,28 @@
-// RUN: clang-tidy -checks='-*,modernize-use-override' %s.nonexistent.cpp -- | FileCheck -check-prefix=CHECK1 -implicit-check-not='{{warning:|error:}}' %s
-// RUN: clang-tidy -checks='-*,clang-diagnostic-*,google-explicit-constructor' %s -- -fan-unknown-option | FileCheck -check-prefix=CHECK2 -implicit-check-not='{{warning:|error:}}' %s
-// RUN: clang-tidy -checks='-*,google-explicit-constructor,clang-diagnostic-literal-conversion' %s -- -fan-unknown-option | FileCheck -check

r329580 - [Tooling] A CompilationDatabase wrapper that infers header commands.

[Sam McCall via cfe-commits]

Author: sammccall
Date: Mon Apr  9 08:17:39 2018
New Revision: 329580

URL: http://llvm.org/viewvc/llvm-project?rev=329580&view=rev
Log:
[Tooling] A CompilationDatabase wrapper that infers header commands.

Summary:
The wrapper finds the closest matching compile command using filename heuristics
and makes minimal tweaks so it can be used with the header.

Subscribers: klimek, mgorny, cfe-commits

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

Added:
cfe/trunk/lib/Tooling/InterpolatingCompilationDatabase.cpp
Modified:
cfe/trunk/include/clang/Tooling/CompilationDatabase.h
cfe/trunk/lib/Tooling/CMakeLists.txt
cfe/trunk/unittests/Tooling/CompilationDatabaseTest.cpp

Modified: cfe/trunk/include/clang/Tooling/CompilationDatabase.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/CompilationDatabase.h?rev=329580&r1=329579&r2=329580&view=diff
==
--- cfe/trunk/include/clang/Tooling/CompilationDatabase.h (original)
+++ cfe/trunk/include/clang/Tooling/CompilationDatabase.h Mon Apr  9 08:17:39 
2018
@@ -213,6 +213,13 @@ private:
   std::vector CompileCommands;
 };
 
+/// Returns a wrapped CompilationDatabase that defers to the provided one,
+/// but getCompileCommands() will infer commands for unknown files.
+/// The return value of getAllFiles() or getAllCompileCommands() is unchanged.
+/// See InterpolatingCompilationDatabase.cpp for details on heuristics.
+std::unique_ptr
+inferMissingCompileCommands(std::unique_ptr);
+
 } // namespace tooling
 } // namespace clang
 

Modified: cfe/trunk/lib/Tooling/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/CMakeLists.txt?rev=329580&r1=329579&r2=329580&view=diff
==
--- cfe/trunk/lib/Tooling/CMakeLists.txt (original)
+++ cfe/trunk/lib/Tooling/CMakeLists.txt Mon Apr  9 08:17:39 2018
@@ -15,6 +15,7 @@ add_clang_library(clangTooling
   Execution.cpp
   FileMatchTrie.cpp
   FixIt.cpp
+  InterpolatingCompilationDatabase.cpp
   JSONCompilationDatabase.cpp
   Refactoring.cpp
   RefactoringCallbacks.cpp

Added: cfe/trunk/lib/Tooling/InterpolatingCompilationDatabase.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/InterpolatingCompilationDatabase.cpp?rev=329580&view=auto
==
--- cfe/trunk/lib/Tooling/InterpolatingCompilationDatabase.cpp (added)
+++ cfe/trunk/lib/Tooling/InterpolatingCompilationDatabase.cpp Mon Apr  9 
08:17:39 2018
@@ -0,0 +1,458 @@
+//===- InterpolatingCompilationDatabase.cpp -*- C++ 
-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// InterpolatingCompilationDatabase wraps another CompilationDatabase and
+// attempts to heuristically determine appropriate compile commands for files
+// that are not included, such as headers or newly created files.
+//
+// Motivating cases include:
+//   Header files that live next to their implementation files. These typically
+// share a base filename. (libclang/CXString.h, libclang/CXString.cpp).
+//   Some projects separate headers from includes. Filenames still typically
+// match, maybe other path segments too. (include/llvm/IR/Use.h, 
lib/IR/Use.cc).
+//   Matches are sometimes only approximate (Sema.h, SemaDecl.cpp). This goes
+// for directories too (Support/Unix/Process.inc, lib/Support/Process.cpp).
+//   Even if we can't find a "right" compile command, even a random one from
+// the project will tend to get important flags like -I and -x right.
+//
+// We "borrow" the compile command for the closest available file:
+//   - points are awarded if the filename matches (ignoring extension)
+//   - points are awarded if the directory structure matches
+//   - ties are broken by length of path prefix match
+//
+// The compile command is adjusted, replacing the filename and removing output
+// file arguments. The -x and -std flags may be affected too.
+//
+// Source language is a tricky issue: is it OK to use a .c file's command
+// for building a .cc file? What language is a .h file in?
+//   - We only consider compile commands for c-family languages as candidates.
+//   - For files whose language is implied by the filename (e.g. .m, .hpp)
+// we prefer candidates from the same language.
+// If we must cross languages, we drop any -x and -std flags.
+//   - For .h files, candidates from any c-family language are acceptable.
+// We use the candidate's language, inserting  e.g. -x c++-header.
+//
+// This class is only useful when wrapping databases that can enumerate all
+// their compile commands. If getAllFilenames() is empty, no inference occu

[PATCH] D45006: [Tooling] A CompilationDatabase wrapper that infers header commands.

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL329580: [Tooling] A CompilationDatabase wrapper that infers 
header commands. (authored by sammccall, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D45006?vs=141172&id=141659#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D45006

Files:
  cfe/trunk/include/clang/Tooling/CompilationDatabase.h
  cfe/trunk/lib/Tooling/CMakeLists.txt
  cfe/trunk/lib/Tooling/InterpolatingCompilationDatabase.cpp
  cfe/trunk/unittests/Tooling/CompilationDatabaseTest.cpp

Index: cfe/trunk/include/clang/Tooling/CompilationDatabase.h
===
--- cfe/trunk/include/clang/Tooling/CompilationDatabase.h
+++ cfe/trunk/include/clang/Tooling/CompilationDatabase.h
@@ -213,6 +213,13 @@
   std::vector CompileCommands;
 };
 
+/// Returns a wrapped CompilationDatabase that defers to the provided one,
+/// but getCompileCommands() will infer commands for unknown files.
+/// The return value of getAllFiles() or getAllCompileCommands() is unchanged.
+/// See InterpolatingCompilationDatabase.cpp for details on heuristics.
+std::unique_ptr
+inferMissingCompileCommands(std::unique_ptr);
+
 } // namespace tooling
 } // namespace clang
 
Index: cfe/trunk/lib/Tooling/InterpolatingCompilationDatabase.cpp
===
--- cfe/trunk/lib/Tooling/InterpolatingCompilationDatabase.cpp
+++ cfe/trunk/lib/Tooling/InterpolatingCompilationDatabase.cpp
@@ -0,0 +1,458 @@
+//===- InterpolatingCompilationDatabase.cpp -*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// InterpolatingCompilationDatabase wraps another CompilationDatabase and
+// attempts to heuristically determine appropriate compile commands for files
+// that are not included, such as headers or newly created files.
+//
+// Motivating cases include:
+//   Header files that live next to their implementation files. These typically
+// share a base filename. (libclang/CXString.h, libclang/CXString.cpp).
+//   Some projects separate headers from includes. Filenames still typically
+// match, maybe other path segments too. (include/llvm/IR/Use.h, lib/IR/Use.cc).
+//   Matches are sometimes only approximate (Sema.h, SemaDecl.cpp). This goes
+// for directories too (Support/Unix/Process.inc, lib/Support/Process.cpp).
+//   Even if we can't find a "right" compile command, even a random one from
+// the project will tend to get important flags like -I and -x right.
+//
+// We "borrow" the compile command for the closest available file:
+//   - points are awarded if the filename matches (ignoring extension)
+//   - points are awarded if the directory structure matches
+//   - ties are broken by length of path prefix match
+//
+// The compile command is adjusted, replacing the filename and removing output
+// file arguments. The -x and -std flags may be affected too.
+//
+// Source language is a tricky issue: is it OK to use a .c file's command
+// for building a .cc file? What language is a .h file in?
+//   - We only consider compile commands for c-family languages as candidates.
+//   - For files whose language is implied by the filename (e.g. .m, .hpp)
+// we prefer candidates from the same language.
+// If we must cross languages, we drop any -x and -std flags.
+//   - For .h files, candidates from any c-family language are acceptable.
+// We use the candidate's language, inserting  e.g. -x c++-header.
+//
+// This class is only useful when wrapping databases that can enumerate all
+// their compile commands. If getAllFilenames() is empty, no inference occurs.
+//
+//===--===//
+
+#include "clang/Driver/Options.h"
+#include "clang/Driver/Types.h"
+#include "clang/Frontend/LangStandard.h"
+#include "clang/Tooling/CompilationDatabase.h"
+#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/StringSwitch.h"
+#include "llvm/Option/ArgList.h"
+#include "llvm/Option/OptTable.h"
+#include "llvm/Support/Debug.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/StringSaver.h"
+#include "llvm/Support/raw_ostream.h"
+#include 
+
+namespace clang {
+namespace tooling {
+namespace {
+using namespace llvm;
+namespace types = clang::driver::types;
+namespace path = llvm::sys::path;
+
+// The length of the prefix these two strings have in common.
+size_t matchingPrefix(StringRef L, StringRef R) {
+  size_t Limit = std::min(L.size(), R.size());
+  for (size_t I = 0; I < Limit; ++I)
+if (L[I] != R[I])
+  return I;
+  return Limit;
+}
+
+// A comparator for searching Subst

[PATCH] D45006: [Tooling] A CompilationDatabase wrapper that infers header commands.

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
sammccall marked an inline comment as done.
Closed by commit rC329580: [Tooling] A CompilationDatabase wrapper that infers 
header commands. (authored by sammccall, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D45006?vs=141172&id=141658#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D45006

Files:
  include/clang/Tooling/CompilationDatabase.h
  lib/Tooling/CMakeLists.txt
  lib/Tooling/InterpolatingCompilationDatabase.cpp
  unittests/Tooling/CompilationDatabaseTest.cpp

Index: unittests/Tooling/CompilationDatabaseTest.cpp
===
--- unittests/Tooling/CompilationDatabaseTest.cpp
+++ unittests/Tooling/CompilationDatabaseTest.cpp
@@ -626,5 +626,115 @@
   EXPECT_EQ(2, Argc);
 }
 
+struct MemCDB : public CompilationDatabase {
+  using EntryMap = llvm::StringMap>;
+  EntryMap Entries;
+  MemCDB(const EntryMap &E) : Entries(E) {}
+
+  std::vector getCompileCommands(StringRef F) const override {
+auto Ret = Entries.lookup(F);
+return {Ret.begin(), Ret.end()};
+  }
+
+  std::vector getAllFiles() const override {
+std::vector Result;
+for (const auto &Entry : Entries)
+  Result.push_back(Entry.first());
+return Result;
+  }
+};
+
+class InterpolateTest : public ::testing::Test {
+protected:
+  // Adds an entry to the underlying compilation database.
+  // A flag is injected: -D , so the command used can be identified.
+  void add(llvm::StringRef File, llvm::StringRef Flags = "") {
+llvm::SmallVector Argv = {"clang", File, "-D", File};
+llvm::SplitString(Flags, Argv);
+llvm::SmallString<32> Dir;
+llvm::sys::path::system_temp_directory(false, Dir);
+Entries[path(File)].push_back(
+{Dir, path(File), {Argv.begin(), Argv.end()}, "foo.o"});
+  }
+
+  // Turn a unix path fragment (foo/bar.h) into a native path (C:\tmp\foo\bar.h)
+  std::string path(llvm::SmallString<32> File) {
+llvm::SmallString<32> Dir;
+llvm::sys::path::system_temp_directory(false, Dir);
+llvm::sys::path::native(File);
+llvm::SmallString<64> Result;
+llvm::sys::path::append(Result, Dir, File);
+return Result.str();
+  }
+
+  // Look up the command from a relative path, and return it in string form.
+  // The input file is not included in the returned command.
+  std::string getCommand(llvm::StringRef F) {
+auto Results =
+inferMissingCompileCommands(llvm::make_unique(Entries))
+->getCompileCommands(path(F));
+if (Results.empty())
+  return "none";
+// drop the input file argument, so tests don't have to deal with path().
+EXPECT_EQ(Results[0].CommandLine.back(), path(F))
+<< "Last arg should be the file";
+Results[0].CommandLine.pop_back();
+return llvm::join(Results[0].CommandLine, " ");
+  }
+
+  MemCDB::EntryMap Entries;
+};
+
+TEST_F(InterpolateTest, Nearby) {
+  add("dir/foo.cpp");
+  add("dir/bar.cpp");
+  add("an/other/foo.cpp");
+
+  // great: dir and name both match (prefix or full, case insensitive)
+  EXPECT_EQ(getCommand("dir/f.cpp"), "clang -D dir/foo.cpp");
+  EXPECT_EQ(getCommand("dir/FOO.cpp"), "clang -D dir/foo.cpp");
+  // no name match. prefer matching dir, break ties by alpha
+  EXPECT_EQ(getCommand("dir/a.cpp"), "clang -D dir/bar.cpp");
+  // an exact name match beats one segment of directory match
+  EXPECT_EQ(getCommand("some/other/bar.h"),
+"clang -D dir/bar.cpp -x c++-header");
+  // two segments of directory match beat a prefix name match
+  EXPECT_EQ(getCommand("an/other/b.cpp"), "clang -D an/other/foo.cpp");
+  // if nothing matches at all, we still get the closest alpha match
+  EXPECT_EQ(getCommand("below/some/obscure/path.cpp"),
+"clang -D an/other/foo.cpp");
+}
+
+TEST_F(InterpolateTest, Language) {
+  add("dir/foo.cpp", "-std=c++17");
+  add("dir/baz.cee", "-x c");
+
+  // .h is ambiguous, so we add explicit language flags
+  EXPECT_EQ(getCommand("foo.h"),
+"clang -D dir/foo.cpp -x c++-header -std c++17");
+  // and don't add -x if the inferred language is correct.
+  EXPECT_EQ(getCommand("foo.hpp"), "clang -D dir/foo.cpp -std c++17");
+  // respect -x if it's already there.
+  EXPECT_EQ(getCommand("baz.h"), "clang -D dir/baz.cee -x c-header");
+  // prefer a worse match with the right language
+  EXPECT_EQ(getCommand("foo.c"), "clang -D dir/baz.cee");
+  Entries.erase(path(StringRef("dir/baz.cee")));
+  // Now we transfer across languages, so drop -std too.
+  EXPECT_EQ(getCommand("foo.c"), "clang -D dir/foo.cpp");
+}
+
+TEST_F(InterpolateTest, Strip) {
+  add("dir/foo.cpp", "-o foo.o -Wall");
+  // the -o option and the input file are removed, but -Wall is preserved.
+  EXPECT_EQ(getCommand("dir/bar.cpp"), "clang -D dir/foo.cpp -Wall");
+}
+
+TEST_F(InterpolateTest, Case) {
+  add("FOO/BAR/BAZ/SHOUT.cc");
+  add("foo/bar/baz/quiet.cc");
+  // Case mismatches are completely ignored, so we choos

[clang-tools-extra] r329582 - [clang] Use compile-command interpolation to provide commands for header files.

[Sam McCall via cfe-commits]

Author: sammccall
Date: Mon Apr  9 08:22:08 2018
New Revision: 329582

URL: http://llvm.org/viewvc/llvm-project?rev=329582&view=rev
Log:
[clang] Use compile-command interpolation to provide commands for header files.

Summary: This uses the inferring wrapper introduced in D45006.

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

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

Modified:
clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp

Modified: clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp?rev=329582&r1=329581&r2=329582&view=diff
==
--- clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp (original)
+++ clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp Mon Apr  9 
08:22:08 2018
@@ -86,6 +86,8 @@ DirectoryBasedGlobalCompilationDatabase:
 return CachedIt->second.get();
   std::string Error = "";
   auto CDB = tooling::CompilationDatabase::loadFromDirectory(Dir, Error);
+  if (CDB)
+CDB = tooling::inferMissingCompileCommands(std::move(CDB));
   auto Result = CDB.get();
   CompilationDatabases.insert(std::make_pair(Dir, std::move(CDB)));
   return Result;


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

[PATCH] D45007: [clangd] Use compile-command interpolation to provide commands for header files.

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL329582: [clang] Use compile-command interpolation to provide 
commands for header files. (authored by sammccall, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D45007

Files:
  clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp


Index: clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp
===
--- clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp
+++ clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp
@@ -86,6 +86,8 @@
 return CachedIt->second.get();
   std::string Error = "";
   auto CDB = tooling::CompilationDatabase::loadFromDirectory(Dir, Error);
+  if (CDB)
+CDB = tooling::inferMissingCompileCommands(std::move(CDB));
   auto Result = CDB.get();
   CompilationDatabases.insert(std::make_pair(Dir, std::move(CDB)));
   return Result;


Index: clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp
===
--- clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp
+++ clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp
@@ -86,6 +86,8 @@
 return CachedIt->second.get();
   std::string Error = "";
   auto CDB = tooling::CompilationDatabase::loadFromDirectory(Dir, Error);
+  if (CDB)
+CDB = tooling::inferMissingCompileCommands(std::move(CDB));
   auto Result = CDB.get();
   CompilationDatabases.insert(std::make_pair(Dir, std::move(CDB)));
   return Result;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45442: Parse .h files as objective-c++ if we don't have a compile command.

[Sam McCall via Phabricator via cfe-commits]

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

This makes C++/objC not totally broken, without hurting C files too much.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45442

Files:
  clangd/GlobalCompilationDatabase.cpp


Index: clangd/GlobalCompilationDatabase.cpp
===
--- clangd/GlobalCompilationDatabase.cpp
+++ clangd/GlobalCompilationDatabase.cpp
@@ -18,9 +18,15 @@
 
 tooling::CompileCommand
 GlobalCompilationDatabase::getFallbackCommand(PathRef File) const {
+  std::vector Argv = {"clang"};
+  // Clang treats .h files as C by default, resulting in unhelpful diagnostics.
+  // Parsing as Objective C++ is friendly to more cases.
+  if (llvm::sys::path::extension(File) == ".h")
+Argv.push_back("-xobjective-c++-header");
+  Argv.push_back(File);
   return tooling::CompileCommand(llvm::sys::path::parent_path(File),
  llvm::sys::path::filename(File),
- {"clang", File.str()},
+ std::move(Argv),
  /*Output=*/"");
 }
 


Index: clangd/GlobalCompilationDatabase.cpp
===
--- clangd/GlobalCompilationDatabase.cpp
+++ clangd/GlobalCompilationDatabase.cpp
@@ -18,9 +18,15 @@
 
 tooling::CompileCommand
 GlobalCompilationDatabase::getFallbackCommand(PathRef File) const {
+  std::vector Argv = {"clang"};
+  // Clang treats .h files as C by default, resulting in unhelpful diagnostics.
+  // Parsing as Objective C++ is friendly to more cases.
+  if (llvm::sys::path::extension(File) == ".h")
+Argv.push_back("-xobjective-c++-header");
+  Argv.push_back(File);
   return tooling::CompileCommand(llvm::sys::path::parent_path(File),
  llvm::sys::path::filename(File),
- {"clang", File.str()},
+ std::move(Argv),
  /*Output=*/"");
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45405: [clang-tidy] [modernize-use-auto] Add a threshold for minimal type name length to be replaced with 'auto'

[Zinovy Nis via Phabricator via cfe-commits]

zinovy.nis added inline comments.



Comment at: clang-tidy/modernize/UseAutoCheck.cpp:290
+: ClangTidyCheck(Name, Context), RemoveStars(Options.get("RemoveStars", 
0)),
+  MinTypeNameLength(Options.get("MinTypeNameLength", 0)) {}
 

alexfh wrote:
> alexfh wrote:
> > lebedev.ri wrote:
> > > zinovy.nis wrote:
> > > > lebedev.ri wrote:
> > > > > alexfh wrote:
> > > > > > Maybe make the default 5? Or does anyone really want to replace 
> > > > > > `int/long/char/bool/...` with `auto`?
> > > > > That might be a bit surprising behavioral change..
> > > > > At least it should be spelled out in the release notes.
> > > > > (my 5 cent: defaulting to 0 would be best)
> > > > Maybe we can somehow mention the current option value in a warning 
> > > > message?
> > > Sure, can be done, but that will only be seen when it does fire, not when 
> > > it suddenly stops firing...
> > > Maybe we can somehow mention the current option value in a warning 
> > > message?
> > 
> > We don't do that for options of other checks (and for the other option 
> > here). I don't think this case is substantially different.
> > That might be a bit surprising behavioral change..
> 
> For many it will be a welcome change ;)
> 
> > At least it should be spelled out in the release notes.
> 
> No objections here.
> 
> > (my 5 cent: defaulting to 0 would be best)
> 
> You see it, 5 is a better default, otherwise you'd say "0 cent" ;)
> 
> On a serious note, the style guides I'm more or less familiar with recommend 
> the use of `auto` for "long/cluttery type names" [1], "if and only if it 
> makes the code more readable or easier to maintain" [2], or to "save writing 
> a longish, hard-to-remember type that the compiler already knows but a 
> programmer could get wrong" [3]. None of these guidelines seem to endorse the 
> use of `auto` instead of `int`, `bool` or the like.
> 
> From my perspective, the default value of 5 would cover a larger fraction of 
> real-world use cases.
> 
> [1] https://google.github.io/styleguide/cppguide.html#auto
> [2] 
> http://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable
> [3] 
> https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#es11-use-auto-to-avoid-redundant-repetition-of-type-names
Or even 7 to ignore 'double' too.


https://reviews.llvm.org/D45405



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

[clang-tools-extra] r329583 - [clangd] Bump v0.0.6 for vscode-clangd.

[Haojian Wu via cfe-commits]

Author: hokein
Date: Mon Apr  9 08:37:09 2018
New Revision: 329583

URL: http://llvm.org/viewvc/llvm-project?rev=329583&view=rev
Log:
[clangd] Bump v0.0.6 for vscode-clangd.

Modified:
clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json

Modified: clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json?rev=329583&r1=329582&r2=329583&view=diff
==
--- clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json (original)
+++ clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json Mon Apr  
9 08:37:09 2018
@@ -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] D45177: CStringChecker, check strlcpy/strlcat

[David CARLIER via Phabricator via cfe-commits]

devnexen added a comment.

ping


Repository:
  rC Clang

https://reviews.llvm.org/D45177



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

[PATCH] D45149: MallocChecker, adding specific BSD calls

[David CARLIER via Phabricator via cfe-commits]

devnexen added a comment.

ping


https://reviews.llvm.org/D45149



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

r329584 - [CUDA] Revert defining __CUDA_ARCH__ for amdgcn targets

[Yaxun Liu via cfe-commits]

Author: yaxunl
Date: Mon Apr  9 08:43:01 2018
New Revision: 329584

URL: http://llvm.org/viewvc/llvm-project?rev=329584&view=rev
Log:
[CUDA] Revert defining __CUDA_ARCH__ for amdgcn targets

amdgcn targets only support HIP, which does not define __CUDA_ARCH__.

this is a partial unroll of r329232 / D45277.

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

Modified:
cfe/trunk/lib/Basic/Targets.cpp
cfe/trunk/lib/Basic/Targets.h
cfe/trunk/lib/Basic/Targets/AMDGPU.cpp
cfe/trunk/lib/Basic/Targets/AMDGPU.h
cfe/trunk/lib/Basic/Targets/NVPTX.cpp

Modified: cfe/trunk/lib/Basic/Targets.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets.cpp?rev=329584&r1=329583&r2=329584&view=diff
==
--- cfe/trunk/lib/Basic/Targets.cpp (original)
+++ cfe/trunk/lib/Basic/Targets.cpp Mon Apr  9 08:43:01 2018
@@ -112,61 +112,6 @@ void addMinGWDefines(const llvm::Triple
   addCygMingDefines(Opts, Builder);
 }
 
-void defineCudaArchMacro(CudaArch GPU, clang::MacroBuilder &Builder) {
-  std::string CUDAArchCode = [GPU] {
-switch (GPU) {
-case CudaArch::LAST:
-  break;
-case CudaArch::SM_20:
-  return "200";
-case CudaArch::SM_21:
-  return "210";
-case CudaArch::SM_30:
-  return "300";
-case CudaArch::SM_32:
-  return "320";
-case CudaArch::SM_35:
-  return "350";
-case CudaArch::SM_37:
-  return "370";
-case CudaArch::SM_50:
-  return "500";
-case CudaArch::SM_52:
-  return "520";
-case CudaArch::SM_53:
-  return "530";
-case CudaArch::SM_60:
-  return "600";
-case CudaArch::SM_61:
-  return "610";
-case CudaArch::SM_62:
-  return "620";
-case CudaArch::SM_70:
-  return "700";
-case CudaArch::SM_72:
-  return "720";
-case CudaArch::GFX600:
-case CudaArch::GFX601:
-case CudaArch::GFX700:
-case CudaArch::GFX701:
-case CudaArch::GFX702:
-case CudaArch::GFX703:
-case CudaArch::GFX704:
-case CudaArch::GFX801:
-case CudaArch::GFX802:
-case CudaArch::GFX803:
-case CudaArch::GFX810:
-case CudaArch::GFX900:
-case CudaArch::GFX902:
-  return "1";
-case CudaArch::UNKNOWN:
-  llvm_unreachable("unhandled Cuda/HIP Arch");
-}
-llvm_unreachable("unhandled Cuda/HIP Arch");
-  }();
-  Builder.defineMacro("__CUDA_ARCH__", CUDAArchCode);
-}
-
 
//===--===//
 // Driver code
 
//===--===//

Modified: cfe/trunk/lib/Basic/Targets.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets.h?rev=329584&r1=329583&r2=329584&view=diff
==
--- cfe/trunk/lib/Basic/Targets.h (original)
+++ cfe/trunk/lib/Basic/Targets.h Mon Apr  9 08:43:01 2018
@@ -16,7 +16,6 @@
 #ifndef LLVM_CLANG_LIB_BASIC_TARGETS_H
 #define LLVM_CLANG_LIB_BASIC_TARGETS_H
 
-#include "clang/Basic/Cuda.h"
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/MacroBuilder.h"
 #include "clang/Basic/TargetInfo.h"
@@ -47,9 +46,6 @@ void addMinGWDefines(const llvm::Triple
 LLVM_LIBRARY_VISIBILITY
 void addCygMingDefines(const clang::LangOptions &Opts,
clang::MacroBuilder &Builder);
-
-LLVM_LIBRARY_VISIBILITY
-void defineCudaArchMacro(CudaArch GPU, clang::MacroBuilder &Builder);
 } // namespace targets
 } // namespace clang
 #endif // LLVM_CLANG_LIB_BASIC_TARGETS_H

Modified: cfe/trunk/lib/Basic/Targets/AMDGPU.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/AMDGPU.cpp?rev=329584&r1=329583&r2=329584&view=diff
==
--- cfe/trunk/lib/Basic/Targets/AMDGPU.cpp (original)
+++ cfe/trunk/lib/Basic/Targets/AMDGPU.cpp Mon Apr  9 08:43:01 2018
@@ -12,7 +12,6 @@
 
//===--===//
 
 #include "AMDGPU.h"
-#include "Targets.h"
 #include "clang/Basic/Builtins.h"
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/MacroBuilder.h"
@@ -264,7 +263,6 @@ AMDGPUTargetInfo::AMDGPUTargetInfo(const
   resetDataLayout(isAMDGCN(getTriple()) ? DataLayoutStringAMDGCN
 : DataLayoutStringR600);
   assert(DataLayout->getAllocaAddrSpace() == Private);
-  GCN_Subarch = CudaArch::GFX803; // Default to fiji
 
   setAddressSpaceMap(Triple.getOS() == llvm::Triple::Mesa3D ||
  !isAMDGCN(Triple));
@@ -309,9 +307,6 @@ void AMDGPUTargetInfo::getTargetDefines(
   if (GPU.Kind != GK_NONE)
 Builder.defineMacro(Twine("__") + Twine(GPU.CanonicalName) + Twine("__"));
 
-  if (Opts.CUDAIsDevice)
-defineCudaArchMacro(GCN_Subarch, Builder);
-
   // TODO: __HAS_FMAF__, __HAS_LDEXPF__, __HAS_FP64__ are deprecated and will 
be
   // removed in the near future.

[PATCH] D45387: [CUDA] Revert defining __CUDA_ARCH__ for amdgcn targets

[Yaxun Liu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL329584: [CUDA] Revert defining __CUDA_ARCH__ for amdgcn 
targets (authored by yaxunl, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D45387?vs=141403&id=141664#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D45387

Files:
  cfe/trunk/lib/Basic/Targets.cpp
  cfe/trunk/lib/Basic/Targets.h
  cfe/trunk/lib/Basic/Targets/AMDGPU.cpp
  cfe/trunk/lib/Basic/Targets/AMDGPU.h
  cfe/trunk/lib/Basic/Targets/NVPTX.cpp

Index: cfe/trunk/lib/Basic/Targets.h
===
--- cfe/trunk/lib/Basic/Targets.h
+++ cfe/trunk/lib/Basic/Targets.h
@@ -16,7 +16,6 @@
 #ifndef LLVM_CLANG_LIB_BASIC_TARGETS_H
 #define LLVM_CLANG_LIB_BASIC_TARGETS_H
 
-#include "clang/Basic/Cuda.h"
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/MacroBuilder.h"
 #include "clang/Basic/TargetInfo.h"
@@ -47,9 +46,6 @@
 LLVM_LIBRARY_VISIBILITY
 void addCygMingDefines(const clang::LangOptions &Opts,
clang::MacroBuilder &Builder);
-
-LLVM_LIBRARY_VISIBILITY
-void defineCudaArchMacro(CudaArch GPU, clang::MacroBuilder &Builder);
 } // namespace targets
 } // namespace clang
 #endif // LLVM_CLANG_LIB_BASIC_TARGETS_H
Index: cfe/trunk/lib/Basic/Targets/NVPTX.cpp
===
--- cfe/trunk/lib/Basic/Targets/NVPTX.cpp
+++ cfe/trunk/lib/Basic/Targets/NVPTX.cpp
@@ -153,8 +153,61 @@
MacroBuilder &Builder) const {
   Builder.defineMacro("__PTX__");
   Builder.defineMacro("__NVPTX__");
-  if (Opts.CUDAIsDevice)
-defineCudaArchMacro(GPU, Builder);
+  if (Opts.CUDAIsDevice) {
+// Set __CUDA_ARCH__ for the GPU specified.
+std::string CUDAArchCode = [this] {
+  switch (GPU) {
+  case CudaArch::GFX600:
+  case CudaArch::GFX601:
+  case CudaArch::GFX700:
+  case CudaArch::GFX701:
+  case CudaArch::GFX702:
+  case CudaArch::GFX703:
+  case CudaArch::GFX704:
+  case CudaArch::GFX801:
+  case CudaArch::GFX802:
+  case CudaArch::GFX803:
+  case CudaArch::GFX810:
+  case CudaArch::GFX900:
+  case CudaArch::GFX902:
+  case CudaArch::LAST:
+break;
+  case CudaArch::UNKNOWN:
+assert(false && "No GPU arch when compiling CUDA device code.");
+return "";
+  case CudaArch::SM_20:
+return "200";
+  case CudaArch::SM_21:
+return "210";
+  case CudaArch::SM_30:
+return "300";
+  case CudaArch::SM_32:
+return "320";
+  case CudaArch::SM_35:
+return "350";
+  case CudaArch::SM_37:
+return "370";
+  case CudaArch::SM_50:
+return "500";
+  case CudaArch::SM_52:
+return "520";
+  case CudaArch::SM_53:
+return "530";
+  case CudaArch::SM_60:
+return "600";
+  case CudaArch::SM_61:
+return "610";
+  case CudaArch::SM_62:
+return "620";
+  case CudaArch::SM_70:
+return "700";
+  case CudaArch::SM_72:
+return "720";
+  }
+  llvm_unreachable("unhandled CudaArch");
+}();
+Builder.defineMacro("__CUDA_ARCH__", CUDAArchCode);
+  }
 }
 
 ArrayRef NVPTXTargetInfo::getTargetBuiltins() const {
Index: cfe/trunk/lib/Basic/Targets/AMDGPU.cpp
===
--- cfe/trunk/lib/Basic/Targets/AMDGPU.cpp
+++ cfe/trunk/lib/Basic/Targets/AMDGPU.cpp
@@ -12,7 +12,6 @@
 //===--===//
 
 #include "AMDGPU.h"
-#include "Targets.h"
 #include "clang/Basic/Builtins.h"
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/MacroBuilder.h"
@@ -264,7 +263,6 @@
   resetDataLayout(isAMDGCN(getTriple()) ? DataLayoutStringAMDGCN
 : DataLayoutStringR600);
   assert(DataLayout->getAllocaAddrSpace() == Private);
-  GCN_Subarch = CudaArch::GFX803; // Default to fiji
 
   setAddressSpaceMap(Triple.getOS() == llvm::Triple::Mesa3D ||
  !isAMDGCN(Triple));
@@ -309,9 +307,6 @@
   if (GPU.Kind != GK_NONE)
 Builder.defineMacro(Twine("__") + Twine(GPU.CanonicalName) + Twine("__"));
 
-  if (Opts.CUDAIsDevice)
-defineCudaArchMacro(GCN_Subarch, Builder);
-
   // TODO: __HAS_FMAF__, __HAS_LDEXPF__, __HAS_FP64__ are deprecated and will be
   // removed in the near future.
   if (GPU.HasFMAF)
Index: cfe/trunk/lib/Basic/Targets/AMDGPU.h
===
--- cfe/trunk/lib/Basic/Targets/AMDGPU.h
+++ cfe/trunk/lib/Basic/Targets/AMDGPU.h
@@ -14,7 +14,6 @@
 #ifndef LLVM_CLANG_LIB_BASIC_TARGETS_AMDGPU_H
 #define LLVM_CLANG_LIB_BASIC_TARGETS_AMDGPU_H
 
-#include "clang/Basic/Cuda.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Basic/TargetOptions.h"
 #include "llvm/ADT/StringSet.h"
@@ -175,7 

[PATCH] D45383: Strip reference from a va_list object in C when merging parameter types.

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

Identical to what?  `__builtin_va_start` and `__builtin_va_end` specifically 
are weird because they're builtins which have a signature which can't be 
expressed in C.  vprintf doesn't have that problem.

Builtins.def makes the relevant distinction already: a "BUILTIN" is a reserved 
identifier the user isn't allowed to redeclare, and a "LIBBUILTIN" is a library 
function which the compiler has special knowledge of.


Repository:
  rC Clang

https://reviews.llvm.org/D45383



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

[PATCH] D45383: Strip reference from a va_list object in C when merging parameter types.

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In https://reviews.llvm.org/D45383#1061780, @efriedma wrote:

> Identical to what?  `__builtin_va_start` and `__builtin_va_end` specifically 
> are weird because they're builtins which have a signature which can't be 
> expressed in C.  vprintf doesn't have that problem.
>
> Builtins.def makes the relevant distinction already: a "BUILTIN" is a 
> reserved identifier the user isn't allowed to redeclare, and a "LIBBUILTIN" 
> is a library function which the compiler has special knowledge of.


In the context of this patch, vprintf is handled using the exact same 
code-paths.  SO, it'll have its 2nd parameter created with type 'char*&', which 
can cause the same crash that I observed with overloading va_end.  Currently, 
you ARE permitted to redeclare a BUILTIN (see my 1st test above), but a 
conflicting type is an error.  A LIBBUILTIN is a warning to have a conflicting 
redeclaration.


Repository:
  rC Clang

https://reviews.llvm.org/D45383



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

[PATCH] D45383: Strip reference from a va_list object in C when merging parameter types.

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

> vprintf is handled using the exact same code-paths. SO, it'll have its 2nd 
> parameter created with type 'char*&'

vprintf is defined in the C standard as "int vprintf(const char *format, 
va_list arg);"; on Windows, that's equivalent to "int vprintf(const char 
*format, char* arg);".  If a reference is showing up anywhere, something has 
gone seriously wrong before we get anywhere near the type merging code.


Repository:
  rC Clang

https://reviews.llvm.org/D45383



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

[PATCH] D45444: [clang-tidy] WIP: implement new check for const-correctness

[Jonas Toth via Phabricator via cfe-commits]

JonasToth created this revision.
JonasToth added reviewers: alexfh, aaron.ballman, lebedev.ri, hokein.
Herald added subscribers: cfe-commits, kbarton, xazax.hun, mgorny, nemanjai, 
klimek.

This check aims to determine values and references that could be declared
as const, but are not.

The first version I am aiming at only analyzes local variables.
No fixits are emitted.

This check aims to implement `CPPCG-ES. 25: Declare an object const or 
constexpr unless you want to modify its value later on 
`
and `HICPP-7.1.2 Use const whenever possible 
`.

Because const-correctness includes many issues this check will be implemented
in multiple stages.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45444

Files:
  clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tidy/cppcoreguidelines/ConstCheck.cpp
  clang-tidy/cppcoreguidelines/ConstCheck.h
  clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/cppcoreguidelines-const.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/cppcoreguidelines-const-primtive-builtin.cpp

Index: test/clang-tidy/cppcoreguidelines-const-primtive-builtin.cpp
===
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-const-primtive-builtin.cpp
@@ -0,0 +1,187 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-const %t
+
+// --- Provide test samples for primitive builtins -
+// - every 'p_*' variable is a 'potential_const_*' variable
+// - every 'np_*' variable is a 'non_potential_const_*' variable
+
+bool global;
+char np_global = 0; // globals can't be known to be const
+
+namespace foo {
+int scoped;
+float np_scoped = 1; // namespace variables are like globals
+} // namespace foo
+
+void some_function(double, wchar_t);
+
+void some_function(double np_arg0, wchar_t np_arg1) {
+  int p_local0 = 2;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int' can be declared const
+
+  int np_local0;
+  const int np_local1 = 42;
+
+  unsigned int np_local2 = 3;
+  np_local2 <<= 4;
+
+  int np_local3 = 4;
+  ++np_local3;
+  int np_local4 = 4;
+  np_local4++;
+
+  int np_local5 = 4;
+  --np_local5;
+  int np_local6 = 4;
+  np_local6--;
+}
+
+void some_lambda_environment_capture_all_by_reference(double np_arg0) {
+  int np_local0 = 0;
+  int p_local0 = 1;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int' can be declared const
+
+  int np_local2;
+  const int np_local3 = 2;
+
+  // Capturing all variables by reference prohibits making them const.
+  [&]() { ++np_local0; };
+
+  int p_local1 = 0;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local1' of type 'int' can be declared const
+}
+
+void some_lambda_environment_capture_all_by_value(double np_arg0) {
+  int np_local0 = 0;
+  int p_local0 = 1;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int' can be declared const
+
+  int np_local1;
+  const int np_local2 = 2;
+
+  // Capturing by value has no influence on them.
+  [=]() { (void)p_local0; };
+
+  np_local0 += 10;
+}
+
+void function_inout_pointer(int *inout);
+void function_in_pointer(const int *in);
+
+void some_pointer_taking(int *out) {
+  int np_local0 = 42;
+  const int *const p0_np_local0 = &np_local0;
+  int *const p1_np_local0 = &np_local0;
+
+  int np_local1 = 42;
+  function_inout_pointer(&np_local1);
+
+  // Prevents const.
+  int np_local2 = 42;
+  out = &np_local2; // This returns and invalid address, its just about the AST
+
+  int p_local0 = 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int' can be declared const
+  const int *const p0_p_local0 = &p_local0;
+
+  int p_local1 = 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local1' of type 'int' can be declared const
+  function_in_pointer(&p_local1);
+}
+
+void function_inout_ref(int &inout);
+void function_in_ref(const int &in);
+
+void some_reference_taking() {
+  int np_local0 = 42;
+  const int &r0_np_local0 = np_local0;
+  int &r1_np_local0 = np_local0;
+  const int &r2_np_local0 = r1_np_local0;
+
+  int np_local1 = 42;
+  function_inout_ref(np_local1);
+
+  int p_local0 = 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int' can be declared const
+  const int &r0_p_local0 = p_local0;
+
+  int p_local1 = 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local1' of type 'int' can be declared const
+  function_in_ref(p_local1);
+}
+
+double *non_const_pointer_return() {
+  double p_local0 = 0.0;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'double' can be declared const
+  double np_local0 = 24.4;
+
+  return &np_local0;
+}
+
+cons