date:20190527

[PATCH] D61861: DeleteNullPointerCheck now deletes until the end brace of the condition

2019-05-27 Thread Mads Ravn via Phabricator via cfe-commits

madsravn added a comment.

This patch has now been merged. 
https://github.com/llvm/llvm-project/commit/bd324fa2273778430a4fdf8371fec5d64d2231bb


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61861



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

[PATCH] D62412: [clang-tidy] Fix unused-variable warning after r361647.

2019-05-27 Thread 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




Comment at: clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp:21
 : ClangTidyCheck(Name, Context), Rule(std::move(R)) {
-  for (const auto &Case : Rule.Cases) {
-assert(Case.Explanation != nullptr &&
-   "clang-tidy checks must have an explanation by default;"
-   " explicitly provide an empty explanation if none is desired");
-  }
+  assert(std::all_of(Rule.Cases.begin(), Rule.Cases.end(),
+ [](const RewriteRule::Case &C) {

NIT: use `llvm::all_of` avoid typing the container name twice.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62412



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

[PATCH] D59407: [clangd] Add RelationSlab

2019-05-27 Thread Kadir Cetinkaya via Phabricator via cfe-commits

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

LGTM, except the duplication issue. Thanks!




Comment at: clang-tools-extra/clangd/index/Relation.h:75
+  private:
+std::vector Relations;
+  };

maybe use a set so that we can be sure that there won't be any duplicates. 
sorry for missing that in the previous iteration.



Comment at: clang-tools-extra/clangd/unittests/IndexTests.cpp:79
 
+TEST(RelationSlab, Lookup) {
+  SymbolID A{"A"};

could you also add a test case that's inserting duplicate relations?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59407



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

[PATCH] D37813: clang-format: better handle namespace macros

2019-05-27 Thread Manuel Klimek via Phabricator via cfe-commits

klimek added inline comments.



Comment at: unittests/Format/NamespaceEndCommentsFixerTest.cpp:526
   EXPECT_EQ("namespace A {\n"
 "  int i;\n"
 "} // namespace A",

Typz wrote:
> Should I also fix these tests?
> 
> They already existed before this patch, but do not follow LLVM namespace 
> indent style either.
If you don't mind, that would be useful - doing it in a separate CL is fine, so 
we keep this one focused (and no review needed).


Repository:
  rC Clang

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

https://reviews.llvm.org/D37813



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

[PATCH] D62391: [CodeComplete] Complete 'return true/false' in boolean functions

2019-05-27 Thread Kadir Cetinkaya via Phabricator via cfe-commits

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

LGTM, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62391



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

[PATCH] D61861: DeleteNullPointerCheck now deletes until the end brace of the condition

2019-05-27 Thread Jonathan Camilleri via Phabricator via cfe-commits

J-Camilleri added a comment.

In D61861#1517812 , @madsravn wrote:

> This patch has now been merged. 
> https://github.com/llvm/llvm-project/commit/bd324fa2273778430a4fdf8371fec5d64d2231bb


Great, thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61861



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

[PATCH] D59407: [clangd] Add RelationSlab

2019-05-27 Thread Sam McCall via Phabricator via cfe-commits

sammccall accepted this revision.
sammccall added a comment.

Awesome, thank you!




Comment at: clang-tools-extra/clangd/index/Relation.h:75
+  private:
+std::vector Relations;
+  };

kadircet wrote:
> maybe use a set so that we can be sure that there won't be any duplicates. 
> sorry for missing that in the previous iteration.
(you're sorting the array at the end anyway, so sort + unique + erase seems 
neater)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59407



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

[PATCH] D62412: [clang-tidy] Fix unused-variable warning after r361647.

2019-05-27 Thread Haojian Wu via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE361749: [clang-tidy] Fix unused-variable warning after 
r361647. (authored by hokein, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D62412?vs=201289&id=201481#toc

Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D62412

Files:
  clang-tidy/utils/TransformerClangTidyCheck.cpp


Index: clang-tidy/utils/TransformerClangTidyCheck.cpp
===
--- clang-tidy/utils/TransformerClangTidyCheck.cpp
+++ clang-tidy/utils/TransformerClangTidyCheck.cpp
@@ -7,21 +7,22 @@
 
//===--===//
 
 #include "TransformerClangTidyCheck.h"
+#include "llvm/ADT/STLExtras.h"
 
 namespace clang {
 namespace tidy {
 namespace utils {
 using tooling::RewriteRule;
 
-TransformerClangTidyCheck::TransformerClangTidyCheck(tooling::RewriteRule R,
+TransformerClangTidyCheck::TransformerClangTidyCheck(RewriteRule R,
  StringRef Name,
  ClangTidyContext *Context)
 : ClangTidyCheck(Name, Context), Rule(std::move(R)) {
-  for (const auto &Case : Rule.Cases) {
-assert(Case.Explanation != nullptr &&
-   "clang-tidy checks must have an explanation by default;"
-   " explicitly provide an empty explanation if none is desired");
-  }
+  assert(llvm::all_of(Rule.Cases, [](const RewriteRule::Case &C) {
+   return C.Explanation != nullptr;
+ }) &&
+ "clang-tidy checks must have an explanation by default;"
+ " explicitly provide an empty explanation if none is desired");
 }
 
 void TransformerClangTidyCheck::registerMatchers(


Index: clang-tidy/utils/TransformerClangTidyCheck.cpp
===
--- clang-tidy/utils/TransformerClangTidyCheck.cpp
+++ clang-tidy/utils/TransformerClangTidyCheck.cpp
@@ -7,21 +7,22 @@
 //===--===//
 
 #include "TransformerClangTidyCheck.h"
+#include "llvm/ADT/STLExtras.h"
 
 namespace clang {
 namespace tidy {
 namespace utils {
 using tooling::RewriteRule;
 
-TransformerClangTidyCheck::TransformerClangTidyCheck(tooling::RewriteRule R,
+TransformerClangTidyCheck::TransformerClangTidyCheck(RewriteRule R,
  StringRef Name,
  ClangTidyContext *Context)
 : ClangTidyCheck(Name, Context), Rule(std::move(R)) {
-  for (const auto &Case : Rule.Cases) {
-assert(Case.Explanation != nullptr &&
-   "clang-tidy checks must have an explanation by default;"
-   " explicitly provide an empty explanation if none is desired");
-  }
+  assert(llvm::all_of(Rule.Cases, [](const RewriteRule::Case &C) {
+   return C.Explanation != nullptr;
+ }) &&
+ "clang-tidy checks must have an explanation by default;"
+ " explicitly provide an empty explanation if none is desired");
 }
 
 void TransformerClangTidyCheck::registerMatchers(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r361749 - [clang-tidy] Fix unused-variable warning after r361647.

2019-05-27 Thread Haojian Wu via cfe-commits

Author: hokein
Date: Mon May 27 01:09:02 2019
New Revision: 361749

URL: http://llvm.org/viewvc/llvm-project?rev=361749&view=rev
Log:
[clang-tidy] Fix unused-variable warning after r361647.

Summary:
A range-for was added in r361647 where the range variable was only used in an
assertion.  As a result, it warned for Release builds. This revision
restructures the assertion to avoid the problem.

Patch by Yitzhak Mandelbaum.

Reviewers: ilya-biryukov

Reviewed By: ilya-biryukov

Subscribers: xazax.hun, cfe-commits

Tags: #clang-tools-extra, #clang

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

Modified:
clang-tools-extra/trunk/clang-tidy/utils/TransformerClangTidyCheck.cpp

Modified: clang-tools-extra/trunk/clang-tidy/utils/TransformerClangTidyCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/utils/TransformerClangTidyCheck.cpp?rev=361749&r1=361748&r2=361749&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/utils/TransformerClangTidyCheck.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/utils/TransformerClangTidyCheck.cpp Mon 
May 27 01:09:02 2019
@@ -7,21 +7,22 @@
 
//===--===//
 
 #include "TransformerClangTidyCheck.h"
+#include "llvm/ADT/STLExtras.h"
 
 namespace clang {
 namespace tidy {
 namespace utils {
 using tooling::RewriteRule;
 
-TransformerClangTidyCheck::TransformerClangTidyCheck(tooling::RewriteRule R,
+TransformerClangTidyCheck::TransformerClangTidyCheck(RewriteRule R,
  StringRef Name,
  ClangTidyContext *Context)
 : ClangTidyCheck(Name, Context), Rule(std::move(R)) {
-  for (const auto &Case : Rule.Cases) {
-assert(Case.Explanation != nullptr &&
-   "clang-tidy checks must have an explanation by default;"
-   " explicitly provide an empty explanation if none is desired");
-  }
+  assert(llvm::all_of(Rule.Cases, [](const RewriteRule::Case &C) {
+   return C.Explanation != nullptr;
+ }) &&
+ "clang-tidy checks must have an explanation by default;"
+ " explicitly provide an empty explanation if none is desired");
 }
 
 void TransformerClangTidyCheck::registerMatchers(


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

[PATCH] D62412: [clang-tidy] Fix unused-variable warning after r361647.

2019-05-27 Thread Haojian Wu via Phabricator via cfe-commits

hokein added a comment.

I have commit this patch on behalf of @ymandel, as this warning blocks our 
internal integration.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D62412



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

Re: r361518 - lld-link, clang: Treat non-existent input files as possible spellos for option flags

2019-05-27 Thread Hans Wennborg via cfe-commits

Want to put a note in ReleaseNotes.rst?

This is fixing something that a lot of users have run in to, so seems
worth mentioning :-)

On Thu, May 23, 2019 at 7:55 PM Nico Weber via cfe-commits
 wrote:
>
> Author: nico
> Date: Thu May 23 10:58:33 2019
> New Revision: 361518
>
> URL: http://llvm.org/viewvc/llvm-project?rev=361518&view=rev
> Log:
> lld-link, clang: Treat non-existent input files as possible spellos for 
> option flags
>
> OptTable treats arguments starting with / that aren't a known option
> as filenames. This means lld-link's and clang-cl's typo correction for
> unknown flags didn't do spell checking for misspelled options that start
> with /.
>
> I first tried changing OptTable, but that got pretty messy, see PR41787
> comments 2 and 3.
>
> Instead, let lld-link's and clang's (including clang-cl's) "file not
> found" diagnostic check if a non-existent file looks like it could be a
> mis-spelled option, and if so add a "did you mean" suggestion to the
> "file not found" diagnostic.
>
> While here, make formatting of a few diagnostics a bit more
> self-consistent.
>
> Fixes PR41787.
>
> Differential Revision: https://reviews.llvm.org/D62276
>
> Modified:
> cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
> cfe/trunk/include/clang/Driver/Driver.h
> cfe/trunk/lib/Driver/Driver.cpp
> cfe/trunk/test/Driver/unknown-arg.c
> cfe/trunk/test/Driver/unsupported-option.c
> cfe/trunk/test/Frontend/unknown-arg.c
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td?rev=361518&r1=361517&r2=361518&view=diff
> ==
> --- cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td Thu May 23 
> 10:58:33 2019
> @@ -9,9 +9,11 @@
>  let Component = "Driver" in {
>
>  def err_drv_no_such_file : Error<"no such file or directory: '%0'">;
> +def err_drv_no_such_file_with_suggestion : Error<
> +  "no such file or directory: '%0'; did you mean '%1'?">;
>  def err_drv_unsupported_opt : Error<"unsupported option '%0'">;
> -def err_drv_unsupported_opt_with_suggestion
> -  : Error<"unsupported option '%0', did you mean '%1'?">;
> +def err_drv_unsupported_opt_with_suggestion : Error<
> +  "unsupported option '%0'; did you mean '%1'?">;
>  def err_drv_unsupported_opt_for_target : Error<
>"unsupported option '%0' for target '%1'">;
>  def err_drv_unsupported_option_argument : Error<
> @@ -166,13 +168,13 @@ def err_arch_unsupported_isa
>  def err_drv_I_dash_not_supported : Error<
>"'%0' not supported, please use -iquote instead">;
>  def err_drv_unknown_argument : Error<"unknown argument: '%0'">;
> -def err_drv_unknown_argument_with_suggestion
> -  : Error<"unknown argument '%0', did you mean '%1'?">;
> +def err_drv_unknown_argument_with_suggestion : Error<
> +  "unknown argument '%0'; did you mean '%1'?">;
>  def warn_drv_unknown_argument_clang_cl : Warning<
>"unknown argument ignored in clang-cl: '%0'">,
>InGroup;
>  def warn_drv_unknown_argument_clang_cl_with_suggestion : Warning<
> -  "unknown argument ignored in clang-cl '%0' (did you mean '%1'?)">,
> +  "unknown argument ignored in clang-cl '%0'; did you mean '%1'?">,
>InGroup;
>
>  def warn_drv_ycyu_different_arg_clang_cl : Warning<
>
> Modified: cfe/trunk/include/clang/Driver/Driver.h
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Driver.h?rev=361518&r1=361517&r2=361518&view=diff
> ==
> --- cfe/trunk/include/clang/Driver/Driver.h (original)
> +++ cfe/trunk/include/clang/Driver/Driver.h Thu May 23 10:58:33 2019
> @@ -394,6 +394,14 @@ public:
>void BuildUniversalActions(Compilation &C, const ToolChain &TC,
>   const InputList &BAInputs) const;
>
> +  /// Check that the file referenced by Value exists. If it doesn't,
> +  /// issue a diagnostic and return false.
> +  /// If TypoCorrect is true and the file does not exist, see if it looks
> +  /// like a likely typo for a flag and if so print a "did you mean" blurb.
> +  bool DiagnoseInputExistence(const llvm::opt::DerivedArgList &Args,
> +  StringRef Value, types::ID Ty,
> +  bool TypoCorrect) const;
> +
>/// BuildJobs - Bind actions to concrete tools and translate
>/// arguments to form the list of jobs to run.
>///
>
> Modified: cfe/trunk/lib/Driver/Driver.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Driver.cpp?rev=361518&r1=361517&r2=361518&view=diff
> ==
> --- cfe/trunk/lib/Driver/Driver.cpp (original)
> +++ cfe/trunk/lib/Driver/Driver.cpp Thu May 23 10:58:33 2019
> @@ -1975,11 +1975,9 @@ void Driver::BuildUniversa

[PATCH] D62389: [clangd] Place cursor better after completing patterns

2019-05-27 Thread Haojian Wu via Phabricator via cfe-commits

hokein added a comment.

I'm not sure the new behavior suits all cases, it seems to me that this will 
change a typical behavior (usually when users invoke function calls).

Imaging a case:

  void foo(int a, int b);
  
  void call() {
fo^
  }



- before, the final cursor is at the end of the snippet
- after, the final cursor is at the last parameter of `foo`.

I think the previous cursor location is more convenient (user can continue 
typing `;`).




Comment at: clang-tools-extra/clangd/CodeCompletionStrings.h:41
 ///   *Signature = "(size_type) const"
-///   *Snippet = "(${0:size_type})"
+///   *Snippet = "(${1:size_type})"
 /// If set, RequiredQualifiers is the text that must be typed before the name.

IIUC, I think we will still return `${0}` with the new behavior, `size_type` is 
the only placeholder here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62389



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

[PATCH] D62404: [clang-tidy] Fix null pointer dereference in readability-identifier-naming

2019-05-27 Thread Haojian Wu via Phabricator via cfe-commits

hokein added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:804
+if (const auto *Typedef = TypePtr->getAs()) {
+  addUsage(NamingCheckFailures, Typedef->getDecl(),
+   Value->getSourceRange());

We are risky of dereferencing a nullptr, as the `TypePtr` can be null now.


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

https://reviews.llvm.org/D62404



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

[PATCH] D53847: [C++2a] P0634r3: Down with typename!

2019-05-27 Thread Nicolas Lesser via Phabricator via cfe-commits

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



Comment at: clang/lib/Sema/SemaTemplate.cpp:3377-3379
   // FIXME: This is not quite correct recovery as we don't transform SS
   // into the corresponding dependent form (and we don't diagnose missing
   // 'template' keywords within SS as a result).

rsmith wrote:
> Rakete wrote:
> > Rakete wrote:
> > > rsmith wrote:
> > > > This FIXME is concerning. Is this a problem with this patch? (Is the 
> > > > FIXME wrong now?)
> > > Yes you're right, I believe it not longer applies due to the change in 
> > > ParseDecl.cpp in ParseDeclarationSpecifiers.
> > Although I'm not that sure I fully understand what that comments alludes to.
> The example would be something like this:
> 
> ```
> template struct X {
>   template struct Y { Y(); using V = int; };
>   template typename Y::V f();
> };
> template template
> X::Y::V X::f() {} 
> ```
> 
> Clang trunk today points out that the `typename` keyword is missing on the 
> final line, but fails to point out that the `template` keyword is also 
> missing. The reason is that in the case where that construct is valid:
> 
> ```
> template template
> X::Y::Y() {}
> ```
> 
> ... we are "entering the context" of the //nested-name-specifier//, which 
> means we don't need a `template` keyword.
> 
> If the FIXME is fixed, then we should diagnose the missing `template` in the 
> above program.
> 
> Also, because we don't rebuild the //nested-name-specifier// as a dependent 
> nested name specifier in this case, we fail to match it against the 
> declaration in the class, so in my example above, we also produce an 
> "out-of-line definition does not match" error.
> 
> 
> A closely-related issue can be seen in an example such as:
> 
> ```
> template struct X {
>   template struct Y { Y(); typedef void V; }; 
>   template typename Y::V::W f();
> };
> template template
> X::template Y::V::W X::f() { return 0; } 
> ```
> 
> Here, we produce a completely bogus error:
> 
> ```
> :6:13: error: 'X::Y::V' (aka 'void') is not a class, namespace, or 
> enumeration
> X::template Y::V::W X::f() { return 0; } 
> ^
> ```
> 
> ... because we parse this in "entering context" mode and so resolve 
> `X::Y::V` to the type in the primary template (that is, `void`). That's 
> wrong: we should defer resolving this name to a type until we know what `T` 
> and `U` are, or until we know that we're *actually* entering the context. 
> Specifically, the above program can be extended as follows:
> 
> ```
> template<> template<> struct X::Y {
>   struct V { using W = int; };
> };
> void call(X x) { x.f(); } // OK, f returns int
> ```
> 
> The above program should be accepted by this patch, if the FIXME is indeed 
> now fixed. Please add it as a testcase :)
Oh ok, got it thanks. So no, the program is still not accepted in clang, and a 
pretty dumb side effect of it is that 

```
template struct X {
  template struct Y { Y(); using V = int; };
  template typename Y::V f();
};
template template
X::Y::V X::f() {}
```

is accepted without the diagnostic for the missing `template`. :(


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D53847



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

[PATCH] D62471: [clangd] SymbolCollector support for relations

2019-05-27 Thread Kadir Cetinkaya via Phabricator via cfe-commits

kadircet added a comment.

Can you also add some tests ?

we have some tests in `unittests/SymbolCollectorTests.cpp`




Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:298
+
+  processRelations(*ND, *ID, Relations);
+

why do we want to process these relations for references?



Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:426
+  // Store subtype relations.
+  const CXXRecordDecl *RD = dyn_cast(&ND);
+  if (!RD)

maybe rather check `TagDecl` to be as generic as possible.

also the check can be inlined since you are not using RD afterwards, i.e 
```
if(!dyn_cast(&ND))
  return;
```



Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:431
+  for (const auto &R : Relations) {
+if (!(R.Roles & static_cast(index::SymbolRole::RelationBaseOf))) 
{
+  continue;

this might get complicated, maybe isolate it to a `bool 
shouldIndexRelation(const Relation&)`



Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:434
+}
+const Decl *Parent = R.RelatedSymbol;
+

maybe call it `Object` ?



Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:436
+
+if (const auto *CTSD = dyn_cast(Parent)) {
+  if (!CTSD->isExplicitSpecialization()) {

why? I believe we have those in the index since rL356125.

even if we don't I believe this part should also be isolated to somehow get 
canonical version of a given symbol.



Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:441
+}
+if (auto ParentID = getSymbolID(Parent)) {
+  // We are a subtype to each of our parents.

nit: invert the condition to reduce nesting



Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:453
+  this->Relations.insert(
+  Relation{ID, index::SymbolRole::RelationBaseOf, *ParentID});
+}

I believe subject is `ParentID` and object is `ID` in a baseof relation so 
these should be replaced ?



Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:453
+  this->Relations.insert(
+  Relation{ID, index::SymbolRole::RelationBaseOf, *ParentID});
+}

kadircet wrote:
> I believe subject is `ParentID` and object is `ID` in a baseof relation so 
> these should be replaced ?
also we should generalize this with something like 
`index::applyForEachSymbolRole`, which should also get rid of the check at 
top(`shouldIndexRelation`)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62471



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

[PATCH] D62476: [clangd] Support offsets for parameters in signatureHelp

2019-05-27 Thread Ilya Biryukov via Phabricator via cfe-commits

ilya-biryukov created this revision.
ilya-biryukov added a reviewer: hokein.
Herald added subscribers: kadircet, arphaman, jkorous, MaskRay.
Herald added a project: clang.

Added to LSP in version 3.14


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D62476

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp

Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -928,19 +928,37 @@
   return signatures(Test.code(), Test.point(), std::move(IndexSymbols));
 }
 
+struct ExpectedParameter {
+  std::string Text;
+  std::pair Offsets;
+};
 MATCHER_P(ParamsAre, P, "") {
   if (P.size() != arg.parameters.size())
 return false;
-  for (unsigned I = 0; I < P.size(); ++I)
-if (P[I] != arg.parameters[I].label)
+  for (unsigned I = 0; I < P.size(); ++I) {
+if (P[I].Text != arg.parameters[I].labelString ||
+P[I].Offsets != arg.parameters[I].labelOffsets)
   return false;
+  }
   return true;
 }
 MATCHER_P(SigDoc, Doc, "") { return arg.documentation == Doc; }
 
-Matcher Sig(std::string Label,
-  std::vector Params) {
-  return AllOf(SigHelpLabeled(Label), ParamsAre(Params));
+/// \p AnnotatedLabel is a signature label with ranges marking parameters, e.g.
+///foo([[int p1]], [[double p2]]) -> void
+Matcher Sig(llvm::StringRef AnnotatedLabel) {
+  llvm::Annotations A(AnnotatedLabel);
+  std::string Label = A.code();
+  std::vector Parameters;
+  for (auto Range : A.ranges()) {
+Parameters.emplace_back();
+
+ExpectedParameter &P = Parameters.back();
+P.Text = Label.substr(Range.Begin, Range.End - Range.Begin);
+P.Offsets.first = lspLength(llvm::StringRef(Label).substr(0, Range.Begin));
+P.Offsets.second = lspLength(llvm::StringRef(Label).substr(1, Range.End));
+  }
+  return AllOf(SigHelpLabeled(Label), ParamsAre(Parameters));
 }
 
 TEST(SignatureHelpTest, Overloads) {
@@ -953,11 +971,10 @@
 int main() { foo(^); }
   )cpp");
   EXPECT_THAT(Results.signatures,
-  UnorderedElementsAre(
-  Sig("foo(float x, float y) -> void", {"float x", "float y"}),
-  Sig("foo(float x, int y) -> void", {"float x", "int y"}),
-  Sig("foo(int x, float y) -> void", {"int x", "float y"}),
-  Sig("foo(int x, int y) -> void", {"int x", "int y"})));
+  UnorderedElementsAre(Sig("foo([[float x]], [[float y]]) -> void"),
+   Sig("foo([[float x]], [[int y]]) -> void"),
+   Sig("foo([[int x]], [[float y]]) -> void"),
+   Sig("foo([[int x]], [[int y]]) -> void")));
   // We always prefer the first signature.
   EXPECT_EQ(0, Results.activeSignature);
   EXPECT_EQ(0, Results.activeParameter);
@@ -971,9 +988,8 @@
   )cpp");
   EXPECT_THAT(Results.signatures,
   UnorderedElementsAre(
-  Sig("bar(int x, int y = 0) -> void", {"int x", "int y = 0"}),
-  Sig("bar(float x = 0, int y = 42) -> void",
-  {"float x = 0", "int y = 42"})));
+  Sig("bar([[int x]], [[int y = 0]]) -> void"),
+  Sig("bar([[float x = 0]], [[int y = 42]]) -> void")));
   EXPECT_EQ(0, Results.activeSignature);
   EXPECT_EQ(0, Results.activeParameter);
 }
@@ -984,8 +1000,7 @@
 int main() { baz(baz(1,2,3), ^); }
   )cpp");
   EXPECT_THAT(Results.signatures,
-  ElementsAre(Sig("baz(int a, int b, int c) -> int",
-  {"int a", "int b", "int c"})));
+  ElementsAre(Sig("baz([[int a]], [[int b]], [[int c]]) -> int")));
   EXPECT_EQ(0, Results.activeSignature);
   EXPECT_EQ(1, Results.activeParameter);
 }
@@ -1722,14 +1737,12 @@
 void foo(int x, int y = 0);
 int main() { foo(^); }
   )cpp");
-  EXPECT_THAT(
-  Results.signatures,
-  ElementsAre(
-  Sig("foo(int x) -> void", {"int x"}),
-  Sig("foo(int x, int y = 0) -> void", {"int x", "int y = 0"}),
-  Sig("foo(float x, int y) -> void", {"float x", "int y"}),
-  Sig("foo(int x, float y) -> void", {"int x", "float y"}),
-  Sig("foo(float x, float y) -> void", {"float x", "float y"})));
+  EXPECT_THAT(Results.signatures,
+  ElementsAre(Sig("foo([[int x]]) -> void"),
+  Sig("foo([[int x]], [[int y = 0]]) -> void"),
+  Sig("foo([[float x]], [[int y]]) -> void"),
+  Sig("foo([[int x]], [[float y]]) -> void"),
+  Sig("foo([[float x]], [[float y]]

[PATCH] D62476: [clangd] Support offsets for parameters in signatureHelp

2019-05-27 Thread Ilya Biryukov via Phabricator via cfe-commits

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

- Fix compilation when assertions are enabled


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62476

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp

Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -928,19 +928,37 @@
   return signatures(Test.code(), Test.point(), std::move(IndexSymbols));
 }
 
+struct ExpectedParameter {
+  std::string Text;
+  std::pair Offsets;
+};
 MATCHER_P(ParamsAre, P, "") {
   if (P.size() != arg.parameters.size())
 return false;
-  for (unsigned I = 0; I < P.size(); ++I)
-if (P[I] != arg.parameters[I].label)
+  for (unsigned I = 0; I < P.size(); ++I) {
+if (P[I].Text != arg.parameters[I].labelString ||
+P[I].Offsets != arg.parameters[I].labelOffsets)
   return false;
+  }
   return true;
 }
 MATCHER_P(SigDoc, Doc, "") { return arg.documentation == Doc; }
 
-Matcher Sig(std::string Label,
-  std::vector Params) {
-  return AllOf(SigHelpLabeled(Label), ParamsAre(Params));
+/// \p AnnotatedLabel is a signature label with ranges marking parameters, e.g.
+///foo([[int p1]], [[double p2]]) -> void
+Matcher Sig(llvm::StringRef AnnotatedLabel) {
+  llvm::Annotations A(AnnotatedLabel);
+  std::string Label = A.code();
+  std::vector Parameters;
+  for (auto Range : A.ranges()) {
+Parameters.emplace_back();
+
+ExpectedParameter &P = Parameters.back();
+P.Text = Label.substr(Range.Begin, Range.End - Range.Begin);
+P.Offsets.first = lspLength(llvm::StringRef(Label).substr(0, Range.Begin));
+P.Offsets.second = lspLength(llvm::StringRef(Label).substr(1, Range.End));
+  }
+  return AllOf(SigHelpLabeled(Label), ParamsAre(Parameters));
 }
 
 TEST(SignatureHelpTest, Overloads) {
@@ -953,11 +971,10 @@
 int main() { foo(^); }
   )cpp");
   EXPECT_THAT(Results.signatures,
-  UnorderedElementsAre(
-  Sig("foo(float x, float y) -> void", {"float x", "float y"}),
-  Sig("foo(float x, int y) -> void", {"float x", "int y"}),
-  Sig("foo(int x, float y) -> void", {"int x", "float y"}),
-  Sig("foo(int x, int y) -> void", {"int x", "int y"})));
+  UnorderedElementsAre(Sig("foo([[float x]], [[float y]]) -> void"),
+   Sig("foo([[float x]], [[int y]]) -> void"),
+   Sig("foo([[int x]], [[float y]]) -> void"),
+   Sig("foo([[int x]], [[int y]]) -> void")));
   // We always prefer the first signature.
   EXPECT_EQ(0, Results.activeSignature);
   EXPECT_EQ(0, Results.activeParameter);
@@ -971,9 +988,8 @@
   )cpp");
   EXPECT_THAT(Results.signatures,
   UnorderedElementsAre(
-  Sig("bar(int x, int y = 0) -> void", {"int x", "int y = 0"}),
-  Sig("bar(float x = 0, int y = 42) -> void",
-  {"float x = 0", "int y = 42"})));
+  Sig("bar([[int x]], [[int y = 0]]) -> void"),
+  Sig("bar([[float x = 0]], [[int y = 42]]) -> void")));
   EXPECT_EQ(0, Results.activeSignature);
   EXPECT_EQ(0, Results.activeParameter);
 }
@@ -984,8 +1000,7 @@
 int main() { baz(baz(1,2,3), ^); }
   )cpp");
   EXPECT_THAT(Results.signatures,
-  ElementsAre(Sig("baz(int a, int b, int c) -> int",
-  {"int a", "int b", "int c"})));
+  ElementsAre(Sig("baz([[int a]], [[int b]], [[int c]]) -> int")));
   EXPECT_EQ(0, Results.activeSignature);
   EXPECT_EQ(1, Results.activeParameter);
 }
@@ -1722,14 +1737,12 @@
 void foo(int x, int y = 0);
 int main() { foo(^); }
   )cpp");
-  EXPECT_THAT(
-  Results.signatures,
-  ElementsAre(
-  Sig("foo(int x) -> void", {"int x"}),
-  Sig("foo(int x, int y = 0) -> void", {"int x", "int y = 0"}),
-  Sig("foo(float x, int y) -> void", {"float x", "int y"}),
-  Sig("foo(int x, float y) -> void", {"int x", "float y"}),
-  Sig("foo(float x, float y) -> void", {"float x", "float y"})));
+  EXPECT_THAT(Results.signatures,
+  ElementsAre(Sig("foo([[int x]]) -> void"),
+  Sig("foo([[int x]], [[int y = 0]]) -> void"),
+  Sig("foo([[float x]], [[int y]]) -> void"),
+  Sig("foo([[int x]], [[float y]]) -> void"),
+  Sig("foo([[float x]], [[float y]]) -> v

[PATCH] D61497: [clangd] Introduce a structured hover response

2019-05-27 Thread Sam McCall via Phabricator via cfe-commits

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

Thanks, I think you're right about all of this. Implementation looks OK too. 
Please add TODOs for the cases where we're punting to later vs deciding not to 
fix.

In D61497#1512003 , @kadircet wrote:

> - Behavior on lambdas, currently it just keeps the old behaviour. I believe 
> they should look more like functions. Do you think it is necessary for first 
> patch?

Agree. Not necessary in first patch IMO. Add a TODO though.

> - Template classes/functions, what we have in definition is not great but I 
> don't think it is too much of a problem since semantic representation carries 
> all the useful information.

These look OK to me, though examples are complicated and I might be missing 
something.

> - Declared in #SOME_KIND# #SOME_PLACE#, currently we drop SOME_KIND and it 
> needs additional info to be propogated since it is not the type of the symbol 
> but rather it is immediate container. I don't think it is necessary for 
> initial version.

This seems OK to me. As noted you can keep it in the case that it's 
"namespace". If you want to propagate the container kind in this patch that's 
also fine.

> - Discrepencies between hovering over auto/decltype and explicitly spelled 
> types, again this keeps the existing behaviour. You can see details in 
> testcases.

Testcases look pretty good I think.

Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:567
+const HoverInfo Expected;
+  } // namespace
+  Cases[] = {

namespace?!

might be a clang-format bug?

Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:574
+)cpp",
+   {/*NamespaceScope=*/std::string(""),
+/*LocalScope=*/std::string(""),

using the constructor/aggregate init here has a couple of downsides:
 - vebosity: you need to list every field, even those that are not set for this 
case (TemplateParameters), or are misleading (SymRange is none)
 - brittleness: it makes it hard to change the struct at all
 - readability: the /*foo=*/ comments aren't bad, but not ideal either

Using field-by-field initialization would be better I think, though it's a bit 
awkward here.

but e.g. you could make the member std::function ExpectedBuilder, 
and write

```

[&](HoverInfo &Expected) {
  Expected.Name = "foo";
  Expected.Kind = SymbolKind::Function;
  ...
}
```

Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:592
+)cpp",
+   {/*NamespaceScope=*/std::string("ns1::ns2"),
+/*LocalScope=*/std::string(""),

I think this should be "ns1::ns2::", as we use scope internally.
This means we can simply concatenate parts to form a qname.

For the current rendering, it's easy to strip ::

Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:599
+/*Definition=*/"void foo()",
+/*Type=*/llvm::None,
+/*ReturnType=*/std::string("void"),

It would be nice to add `void()` or `void (&)()` or so if it's easy.
This is what `:YcmCompleter GetType` would show

Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:678
+/*TemplateParameters=*/llvm::None}},
+  // Class template
+  {R"cpp(

can we have a class template example where the instantiation isn't templated?
e.g.
```
template class vector{};
[[vector]] X;
```

(just because this is an example that we've discussed a lot, and these 
edge-case tests make it hard to see the common behavior)

Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:782
+/*Definition=*/
+"auto lamb = [&bar](int T, bool B) -> bool {\nreturn T && B && "
+"bar;\n}",

is it easy to omit the body?

Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:951
   )cpp",
-  "Declared in namespace ns1\n\nstruct MyClass {}",
+  "Declared in ns1\n\nstruct MyClass {}",
   },

note that if you want to it's easy to keep "Declared in namespace" - the 
LocalScope is empty.
(Though we can't provide this info for non-namespace containers)

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61497

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

r361752 - [ASTImporter] Added visibility context check for CXXRecordDecl.

2019-05-27 Thread Balazs Keri via cfe-commits

Author: balazske
Date: Mon May 27 02:36:00 2019
New Revision: 361752

URL: http://llvm.org/viewvc/llvm-project?rev=361752&view=rev
Log:
[ASTImporter] Added visibility context check for CXXRecordDecl.

Summary:
ASTImporter makes now difference between classes with same name in different
translation units if these are not visible outside. These classes are not linked
into one decl chain.

Reviewers: martong, a.sidorin, shafik

Reviewed By: shafik

Subscribers: rnkovacs, dkrupp, Szelethus, gamesh411, cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/lib/AST/ASTImporter.cpp
cfe/trunk/unittests/AST/ASTImporterVisibilityTest.cpp

Modified: cfe/trunk/lib/AST/ASTImporter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTImporter.cpp?rev=361752&r1=361751&r2=361752&view=diff
==
--- cfe/trunk/lib/AST/ASTImporter.cpp (original)
+++ cfe/trunk/lib/AST/ASTImporter.cpp Mon May 27 02:36:00 2019
@@ -2559,6 +2559,9 @@ ExpectedDecl ASTNodeImporter::VisitRecor
   if (!IsStructuralMatch(D, FoundRecord, false))
 continue;
 
+if (!hasSameVisibilityContext(FoundRecord, D))
+  continue;
+
 if (IsStructuralMatch(D, FoundRecord)) {
   RecordDecl *FoundDef = FoundRecord->getDefinition();
   if (D->isThisDeclarationADefinition() && FoundDef) {

Modified: cfe/trunk/unittests/AST/ASTImporterVisibilityTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/AST/ASTImporterVisibilityTest.cpp?rev=361752&r1=361751&r2=361752&view=diff
==
--- cfe/trunk/unittests/AST/ASTImporterVisibilityTest.cpp (original)
+++ cfe/trunk/unittests/AST/ASTImporterVisibilityTest.cpp Mon May 27 02:36:00 
2019
@@ -31,6 +31,10 @@ struct GetVarPattern {
   using DeclTy = VarDecl;
   BindableMatcher operator()() { return varDecl(hasName("v")); }
 };
+struct GetClassPattern {
+  using DeclTy = CXXRecordDecl;
+  BindableMatcher operator()() { return cxxRecordDecl(hasName("X")); }
+};
 
 // Values for the value-parameterized test fixtures.
 // FunctionDecl:
@@ -41,6 +45,9 @@ const auto *AnonF = "namespace { void f(
 const auto *ExternV = "extern int v;";
 const auto *StaticV = "static int v;";
 const auto *AnonV = "namespace { extern int v; }";
+// CXXRecordDecl:
+const auto *ExternC = "class X;";
+const auto *AnonC = "namespace { class X; }";
 
 // First value in tuple: Compile options.
 // Second value in tuple: Source code to be used in the test.
@@ -84,14 +91,19 @@ protected:
 // Manual instantiation of the fixture with each type.
 using ImportFunctionsVisibilityChain = ImportVisibilityChain;
 using ImportVariablesVisibilityChain = ImportVisibilityChain;
-// Value-parameterized test for the first type.
+using ImportClassesVisibilityChain = ImportVisibilityChain;
+// Value-parameterized test for functions.
 TEST_P(ImportFunctionsVisibilityChain, ImportChain) {
   TypedTest_ImportChain();
 }
-// Value-parameterized test for the second type.
+// Value-parameterized test for variables.
 TEST_P(ImportVariablesVisibilityChain, ImportChain) {
   TypedTest_ImportChain();
 }
+// Value-parameterized test for classes.
+TEST_P(ImportClassesVisibilityChain, ImportChain) {
+  TypedTest_ImportChain();
+}
 
 // Automatic instantiation of the value-parameterized tests.
 INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportFunctionsVisibilityChain,
@@ -110,6 +122,11 @@ INSTANTIATE_TEST_CASE_P(
 // provided but they must have the same linkage.  See also the test
 // ImportVariableChainInC which test for this special C Lang case.
 ::testing::Values(ExternV, AnonV)), );
+INSTANTIATE_TEST_CASE_P(
+ParameterizedTests, ImportClassesVisibilityChain,
+::testing::Combine(
+DefaultTestValuesForRunOptions,
+::testing::Values(ExternC, AnonC)), );
 
 // First value in tuple: Compile options.
 // Second value in tuple: Tuple with informations for the test.
@@ -169,6 +186,7 @@ protected:
 };
 using ImportFunctionsVisibility = ImportVisibility;
 using ImportVariablesVisibility = ImportVisibility;
+using ImportClassesVisibility = ImportVisibility;
 
 // FunctionDecl.
 TEST_P(ImportFunctionsVisibility, ImportAfter) {
@@ -184,6 +202,13 @@ TEST_P(ImportVariablesVisibility, Import
 TEST_P(ImportVariablesVisibility, ImportAfterImport) {
   TypedTest_ImportAfterImport();
 }
+// CXXRecordDecl.
+TEST_P(ImportClassesVisibility, ImportAfter) {
+  TypedTest_ImportAfter();
+}
+TEST_P(ImportClassesVisibility, ImportAfterImport) {
+  TypedTest_ImportAfterImport();
+}
 
 const bool ExpectLink = true;
 const bool ExpectNotLink = false;
@@ -214,6 +239,14 @@ INSTANTIATE_TEST_CASE_P(
   std::make_tuple(AnonV, ExternV, ExpectNotLink),
   std::make_tuple(AnonV, StaticV, ExpectNotLink),
   std::make_tuple(Anon

[PATCH] D62389: [clangd] Place cursor better after completing patterns

2019-05-27 Thread Ilya Biryukov via Phabricator via cfe-commits

ilya-biryukov added a comment.

In D62389#1517844 , @hokein wrote:

> I'm not sure the new behavior suits all cases, it seems to me that this will 
> change a typical behavior (usually when users invoke function calls).
>
> Imaging a case:
>
>   void foo(int a, int b);
>  
>   void call() {
> fo^
>   }
>
>
>
>
> - before, the final cursor is at the end of the snippet
> - after, the final cursor is at the last parameter of `foo`.
>
>   I think the previous cursor location is more convenient (user can continue 
> typing `;`).

Agree! The new behavior does not apply to function calls, they are not 
considered 'patterns' in the completion results.
I.e. in your examples we're still getting `foo(${1:int a}, ${2:int b})` and the 
cursor is placed at the end.

This is definitely worth a comment, I'll try to find a good place for it.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62389

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

[PATCH] D62389: [clangd] Place cursor better after completing patterns

2019-05-27 Thread Ilya Biryukov via Phabricator via cfe-commits

ilya-biryukov marked an inline comment as done.
ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clangd/CodeCompletionStrings.h:41
 ///   *Signature = "(size_type) const"
-///   *Snippet = "(${0:size_type})"
+///   *Snippet = "(${1:size_type})"
 /// If set, RequiredQualifiers is the text that must be typed before the name.

hokein wrote:
> IIUC, I think we will still return `${0}` with the new behavior, `size_type` 
> is the only placeholder here.
The comment was incorrect, we did not return `${0:}` placeholders before.
Note that behavior for function call arguments is the same in both the old and 
the new version.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62389



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

[PATCH] D62312: [ASTImporter] Added visibility context check for CXXRecordDecl.

2019-05-27 Thread Balázs Kéri via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL361752: [ASTImporter] Added visibility context check for 
CXXRecordDecl. (authored by balazske, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D62312

Files:
  cfe/trunk/lib/AST/ASTImporter.cpp
  cfe/trunk/unittests/AST/ASTImporterVisibilityTest.cpp

Index: cfe/trunk/lib/AST/ASTImporter.cpp
===
--- cfe/trunk/lib/AST/ASTImporter.cpp
+++ cfe/trunk/lib/AST/ASTImporter.cpp
@@ -2559,6 +2559,9 @@
   if (!IsStructuralMatch(D, FoundRecord, false))
 continue;
 
+if (!hasSameVisibilityContext(FoundRecord, D))
+  continue;
+
 if (IsStructuralMatch(D, FoundRecord)) {
   RecordDecl *FoundDef = FoundRecord->getDefinition();
   if (D->isThisDeclarationADefinition() && FoundDef) {
Index: cfe/trunk/unittests/AST/ASTImporterVisibilityTest.cpp
===
--- cfe/trunk/unittests/AST/ASTImporterVisibilityTest.cpp
+++ cfe/trunk/unittests/AST/ASTImporterVisibilityTest.cpp
@@ -31,6 +31,10 @@
   using DeclTy = VarDecl;
   BindableMatcher operator()() { return varDecl(hasName("v")); }
 };
+struct GetClassPattern {
+  using DeclTy = CXXRecordDecl;
+  BindableMatcher operator()() { return cxxRecordDecl(hasName("X")); }
+};
 
 // Values for the value-parameterized test fixtures.
 // FunctionDecl:
@@ -41,6 +45,9 @@
 const auto *ExternV = "extern int v;";
 const auto *StaticV = "static int v;";
 const auto *AnonV = "namespace { extern int v; }";
+// CXXRecordDecl:
+const auto *ExternC = "class X;";
+const auto *AnonC = "namespace { class X; }";
 
 // First value in tuple: Compile options.
 // Second value in tuple: Source code to be used in the test.
@@ -84,14 +91,19 @@
 // Manual instantiation of the fixture with each type.
 using ImportFunctionsVisibilityChain = ImportVisibilityChain;
 using ImportVariablesVisibilityChain = ImportVisibilityChain;
-// Value-parameterized test for the first type.
+using ImportClassesVisibilityChain = ImportVisibilityChain;
+// Value-parameterized test for functions.
 TEST_P(ImportFunctionsVisibilityChain, ImportChain) {
   TypedTest_ImportChain();
 }
-// Value-parameterized test for the second type.
+// Value-parameterized test for variables.
 TEST_P(ImportVariablesVisibilityChain, ImportChain) {
   TypedTest_ImportChain();
 }
+// Value-parameterized test for classes.
+TEST_P(ImportClassesVisibilityChain, ImportChain) {
+  TypedTest_ImportChain();
+}
 
 // Automatic instantiation of the value-parameterized tests.
 INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportFunctionsVisibilityChain,
@@ -110,6 +122,11 @@
 // provided but they must have the same linkage.  See also the test
 // ImportVariableChainInC which test for this special C Lang case.
 ::testing::Values(ExternV, AnonV)), );
+INSTANTIATE_TEST_CASE_P(
+ParameterizedTests, ImportClassesVisibilityChain,
+::testing::Combine(
+DefaultTestValuesForRunOptions,
+::testing::Values(ExternC, AnonC)), );
 
 // First value in tuple: Compile options.
 // Second value in tuple: Tuple with informations for the test.
@@ -169,6 +186,7 @@
 };
 using ImportFunctionsVisibility = ImportVisibility;
 using ImportVariablesVisibility = ImportVisibility;
+using ImportClassesVisibility = ImportVisibility;
 
 // FunctionDecl.
 TEST_P(ImportFunctionsVisibility, ImportAfter) {
@@ -184,6 +202,13 @@
 TEST_P(ImportVariablesVisibility, ImportAfterImport) {
   TypedTest_ImportAfterImport();
 }
+// CXXRecordDecl.
+TEST_P(ImportClassesVisibility, ImportAfter) {
+  TypedTest_ImportAfter();
+}
+TEST_P(ImportClassesVisibility, ImportAfterImport) {
+  TypedTest_ImportAfterImport();
+}
 
 const bool ExpectLink = true;
 const bool ExpectNotLink = false;
@@ -214,6 +239,14 @@
   std::make_tuple(AnonV, ExternV, ExpectNotLink),
   std::make_tuple(AnonV, StaticV, ExpectNotLink),
   std::make_tuple(AnonV, AnonV, ExpectNotLink))), );
+INSTANTIATE_TEST_CASE_P(
+ParameterizedTests, ImportClassesVisibility,
+::testing::Combine(
+DefaultTestValuesForRunOptions,
+::testing::Values(std::make_tuple(ExternC, ExternC, ExpectLink),
+  std::make_tuple(ExternC, AnonC, ExpectNotLink),
+  std::make_tuple(AnonC, ExternC, ExpectNotLink),
+  std::make_tuple(AnonC, AnonC, ExpectNotLink))), );
 
 } // end namespace ast_matchers
 } // end namespace clang
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r361753 - [CodeComplete] Complete 'return true/false' in boolean functions

2019-05-27 Thread Ilya Biryukov via cfe-commits

Author: ibiryukov
Date: Mon May 27 02:52:09 2019
New Revision: 361753

URL: http://llvm.org/viewvc/llvm-project?rev=361753&view=rev
Log:
[CodeComplete] Complete 'return true/false' in boolean functions

Reviewers: kadircet

Reviewed By: kadircet

Subscribers: cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/lib/Sema/SemaCodeComplete.cpp
cfe/trunk/test/CodeCompletion/patterns.cpp

Modified: cfe/trunk/lib/Sema/SemaCodeComplete.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCodeComplete.cpp?rev=361753&r1=361752&r2=361753&view=diff
==
--- cfe/trunk/lib/Sema/SemaCodeComplete.cpp (original)
+++ cfe/trunk/lib/Sema/SemaCodeComplete.cpp Mon May 27 02:52:09 2019
@@ -2168,23 +2168,38 @@ static void AddOrdinaryNameResults(Sema:
   Results.AddResult(Result(Builder.TakeString()));
 }
 
-// "return expression ;" or "return ;", depending on whether we
-// know the function is void or not.
-bool isVoid = false;
+// "return expression ;" or "return ;", depending on the return type.
+QualType ReturnType;
 if (const auto *Function = dyn_cast(SemaRef.CurContext))
-  isVoid = Function->getReturnType()->isVoidType();
+  ReturnType = Function->getReturnType();
 else if (const auto *Method = dyn_cast(SemaRef.CurContext))
-  isVoid = Method->getReturnType()->isVoidType();
+  ReturnType = Method->getReturnType();
 else if (SemaRef.getCurBlock() &&
  !SemaRef.getCurBlock()->ReturnType.isNull())
-  isVoid = SemaRef.getCurBlock()->ReturnType->isVoidType();
-Builder.AddTypedTextChunk("return");
-if (!isVoid) {
-  Builder.AddChunk(CodeCompletionString::CK_HorizontalSpace);
+  ReturnType = SemaRef.getCurBlock()->ReturnType;;
+if (ReturnType.isNull() || ReturnType->isVoidType()) {
+  Builder.AddTypedTextChunk("return");
+  Builder.AddChunk(CodeCompletionString::CK_SemiColon);
+  Results.AddResult(Result(Builder.TakeString()));
+} else {
+  assert(!ReturnType.isNull());
+  // "return expression ;"
+  Builder.AddTypedTextChunk("return");
+  Builder.AddChunk(clang::CodeCompletionString::CK_HorizontalSpace);
   Builder.AddPlaceholderChunk("expression");
+  Builder.AddChunk(CodeCompletionString::CK_SemiColon);
+  Results.AddResult(Result(Builder.TakeString()));
+  // When boolean, also add 'return true;' and 'return false;'.
+  if (ReturnType->isBooleanType()) {
+Builder.AddTypedTextChunk("return true");
+Builder.AddChunk(CodeCompletionString::CK_SemiColon);
+Results.AddResult(Result(Builder.TakeString()));
+
+Builder.AddTypedTextChunk("return false");
+Builder.AddChunk(CodeCompletionString::CK_SemiColon);
+Results.AddResult(Result(Builder.TakeString()));
+  }
 }
-Builder.AddChunk(CodeCompletionString::CK_SemiColon);
-Results.AddResult(Result(Builder.TakeString()));
 
 // goto identifier ;
 Builder.AddTypedTextChunk("goto");

Modified: cfe/trunk/test/CodeCompletion/patterns.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeCompletion/patterns.cpp?rev=361753&r1=361752&r2=361753&view=diff
==
--- cfe/trunk/test/CodeCompletion/patterns.cpp (original)
+++ cfe/trunk/test/CodeCompletion/patterns.cpp Mon May 27 02:52:09 2019
@@ -30,10 +30,23 @@ int value_return() {
 void void_return() {
   // line 31
 }
+bool bool_return() {
+  // line 34
+}
 // RUN: %clang_cc1 -fsyntax-only -code-completion-patterns 
-code-completion-at=%s:28:1 %s -o - | FileCheck -check-prefix=RETURN-VAL %s
-// RETURN-VAL-NOT: COMPLETION: Pattern : return;{{$}}
+// RETURN-VAL-NOT: COMPLETION: Pattern : return;
+// RETURN-VAL-NOT: COMPLETION: Pattern : return false;
+// RETURN-VAL-NOT: COMPLETION: Pattern : return true;
 // RETURN-VAL: COMPLETION: Pattern : return <#expression#>;{{$}}
 
 // RUN: %clang_cc1 -fsyntax-only -code-completion-patterns 
-code-completion-at=%s:31:1 %s -o - | FileCheck -check-prefix=RETURN-VOID %s
-// RETURN-VOID-NOT: COMPLETION: Pattern : return <#expression#>;{{$}}
+// RETURN-VOID-NOT: COMPLETION: Pattern : return false;
+// RETURN-VOID-NOT: COMPLETION: Pattern : return true;
+// RETURN-VOID-NOT: COMPLETION: Pattern : return <#expression#>;
 // RETURN-VOID: COMPLETION: Pattern : return;{{$}}
+
+// RUN: %clang_cc1 -fsyntax-only -code-completion-patterns 
-code-completion-at=%s:34:1 %s -o - | FileCheck -check-prefix=RETURN-BOOL %s
+// RETURN-BOOL-NOT: COMPLETION: Pattern : return;
+// RETURN-BOOL: COMPLETION: Pattern : return <#expression#>;{{$}}
+// RETURN-BOOL: COMPLETION: Pattern : return false;{{$}}
+// RETURN-BOOL: COMPLETION: Pattern : return true;{{$}}


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

[PATCH] D62459: [clangd] Serialization support for RelationSlab

2019-05-27 Thread Kadir Cetinkaya via Phabricator via cfe-commits

kadircet added inline comments.



Comment at: clang-tools-extra/clangd/index/Serialization.cpp:29
+
+llvm::Expected symbolRoleToRelationKind(index::SymbolRole Role) {
+  // SymbolRole is used to record relations in the index.

no need for errors, use `llvm_unreachable` instead.



Comment at: clang-tools-extra/clangd/index/Serialization.cpp:45
+
+llvm::Expected relationKindToSymbolRole(RelationKind Kind) {
+  switch (Kind) {

same for error



Comment at: clang-tools-extra/clangd/index/Serialization.cpp:79
+  // A caller can set the error bit if an invalid value was read.
+  void setErr() { Err = true; }
   // Did we read all the data, or encounter an error?

I believe reader should not care about the high level representation of data. 
therefore error state should be internal and only set if the low level read has 
failed.

high level reads are out of `Reader`s scope and should be propagated by the 
caller.



Comment at: clang-tools-extra/clangd/index/Serialization.cpp:394
 
+// RELATIONS ENCODING
+// A relations section is a flat list of relations. Each relation has:

could you also add a comment saying, we might prefer a packed representation if 
need arises in future?



Comment at: clang-tools-extra/clangd/index/Serialization.h:81
+// Used for serializing SymbolRole as used in Relation.
+enum class RelationKind : uint8_t { ChildOf = 1, BaseOf };
+llvm::Expected symbolRoleToRelationKind(index::SymbolRole);

why not start from zero?

Also let's change the `index::SymbolRole` in `Relation` with this one.



Comment at: clang-tools-extra/clangd/index/Serialization.h:81
+// Used for serializing SymbolRole as used in Relation.
+enum class RelationKind : uint8_t { ChildOf = 1, BaseOf };
+llvm::Expected symbolRoleToRelationKind(index::SymbolRole);

kadircet wrote:
> why not start from zero?
> 
> Also let's change the `index::SymbolRole` in `Relation` with this one.
why not just store baseof ? do we need both directions for typehierarchy?



Comment at: clang-tools-extra/clangd/unittests/SerializationTests.cpp:225
 
+const char *RelationsYAML = R"(
+--- !Symbol

let's incorporate these data into existing yaml string at top of the file, I 
believe you just need to add ```--- !Relations``` section


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62459



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

[PATCH] D62391: [CodeComplete] Complete 'return true/false' in boolean functions

2019-05-27 Thread Phabricator via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rC361753: [CodeComplete] Complete 'return 
true/false' in boolean functions (authored by ibiryukov, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D62391?vs=201240&id=201489#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D62391

Files:
  lib/Sema/SemaCodeComplete.cpp
  test/CodeCompletion/patterns.cpp


Index: test/CodeCompletion/patterns.cpp
===
--- test/CodeCompletion/patterns.cpp
+++ test/CodeCompletion/patterns.cpp
@@ -30,10 +30,23 @@
 void void_return() {
   // line 31
 }
+bool bool_return() {
+  // line 34
+}
 // RUN: %clang_cc1 -fsyntax-only -code-completion-patterns 
-code-completion-at=%s:28:1 %s -o - | FileCheck -check-prefix=RETURN-VAL %s
-// RETURN-VAL-NOT: COMPLETION: Pattern : return;{{$}}
+// RETURN-VAL-NOT: COMPLETION: Pattern : return;
+// RETURN-VAL-NOT: COMPLETION: Pattern : return false;
+// RETURN-VAL-NOT: COMPLETION: Pattern : return true;
 // RETURN-VAL: COMPLETION: Pattern : return <#expression#>;{{$}}
 
 // RUN: %clang_cc1 -fsyntax-only -code-completion-patterns 
-code-completion-at=%s:31:1 %s -o - | FileCheck -check-prefix=RETURN-VOID %s
-// RETURN-VOID-NOT: COMPLETION: Pattern : return <#expression#>;{{$}}
+// RETURN-VOID-NOT: COMPLETION: Pattern : return false;
+// RETURN-VOID-NOT: COMPLETION: Pattern : return true;
+// RETURN-VOID-NOT: COMPLETION: Pattern : return <#expression#>;
 // RETURN-VOID: COMPLETION: Pattern : return;{{$}}
+
+// RUN: %clang_cc1 -fsyntax-only -code-completion-patterns 
-code-completion-at=%s:34:1 %s -o - | FileCheck -check-prefix=RETURN-BOOL %s
+// RETURN-BOOL-NOT: COMPLETION: Pattern : return;
+// RETURN-BOOL: COMPLETION: Pattern : return <#expression#>;{{$}}
+// RETURN-BOOL: COMPLETION: Pattern : return false;{{$}}
+// RETURN-BOOL: COMPLETION: Pattern : return true;{{$}}
Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -2168,23 +2168,38 @@
   Results.AddResult(Result(Builder.TakeString()));
 }
 
-// "return expression ;" or "return ;", depending on whether we
-// know the function is void or not.
-bool isVoid = false;
+// "return expression ;" or "return ;", depending on the return type.
+QualType ReturnType;
 if (const auto *Function = dyn_cast(SemaRef.CurContext))
-  isVoid = Function->getReturnType()->isVoidType();
+  ReturnType = Function->getReturnType();
 else if (const auto *Method = dyn_cast(SemaRef.CurContext))
-  isVoid = Method->getReturnType()->isVoidType();
+  ReturnType = Method->getReturnType();
 else if (SemaRef.getCurBlock() &&
  !SemaRef.getCurBlock()->ReturnType.isNull())
-  isVoid = SemaRef.getCurBlock()->ReturnType->isVoidType();
-Builder.AddTypedTextChunk("return");
-if (!isVoid) {
-  Builder.AddChunk(CodeCompletionString::CK_HorizontalSpace);
+  ReturnType = SemaRef.getCurBlock()->ReturnType;;
+if (ReturnType.isNull() || ReturnType->isVoidType()) {
+  Builder.AddTypedTextChunk("return");
+  Builder.AddChunk(CodeCompletionString::CK_SemiColon);
+  Results.AddResult(Result(Builder.TakeString()));
+} else {
+  assert(!ReturnType.isNull());
+  // "return expression ;"
+  Builder.AddTypedTextChunk("return");
+  Builder.AddChunk(clang::CodeCompletionString::CK_HorizontalSpace);
   Builder.AddPlaceholderChunk("expression");
+  Builder.AddChunk(CodeCompletionString::CK_SemiColon);
+  Results.AddResult(Result(Builder.TakeString()));
+  // When boolean, also add 'return true;' and 'return false;'.
+  if (ReturnType->isBooleanType()) {
+Builder.AddTypedTextChunk("return true");
+Builder.AddChunk(CodeCompletionString::CK_SemiColon);
+Results.AddResult(Result(Builder.TakeString()));
+
+Builder.AddTypedTextChunk("return false");
+Builder.AddChunk(CodeCompletionString::CK_SemiColon);
+Results.AddResult(Result(Builder.TakeString()));
+  }
 }
-Builder.AddChunk(CodeCompletionString::CK_SemiColon);
-Results.AddResult(Result(Builder.TakeString()));
 
 // goto identifier ;
 Builder.AddTypedTextChunk("goto");


Index: test/CodeCompletion/patterns.cpp
===
--- test/CodeCompletion/patterns.cpp
+++ test/CodeCompletion/patterns.cpp
@@ -30,10 +30,23 @@
 void void_return() {
   // line 31
 }
+bool bool_return() {
+  // line 34
+}
 // RUN: %clang_cc1 -fsyntax-only -code-completion-patterns -code-completion-at=%s:28:1 %s -o - | FileCheck -check-prefix=RETURN-VAL %s
-// RETURN-VAL-NOT: COMPLETION: Pattern : return;{{$}}
+// RETURN-VAL-NOT: COMPLETION: Pattern : return;
+// RETURN-VAL-NOT: COMPLETION: Pattern : return

[PATCH] D62476: [clangd] Support offsets for parameters in signatureHelp

2019-05-27 Thread Ilya Biryukov via Phabricator via cfe-commits

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

- Fix parsing of a client capability, add a test for a capability


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62476

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/test/signature-help-with-offsets.test
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp

Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -928,19 +928,37 @@
   return signatures(Test.code(), Test.point(), std::move(IndexSymbols));
 }
 
+struct ExpectedParameter {
+  std::string Text;
+  std::pair Offsets;
+};
 MATCHER_P(ParamsAre, P, "") {
   if (P.size() != arg.parameters.size())
 return false;
-  for (unsigned I = 0; I < P.size(); ++I)
-if (P[I] != arg.parameters[I].label)
+  for (unsigned I = 0; I < P.size(); ++I) {
+if (P[I].Text != arg.parameters[I].labelString ||
+P[I].Offsets != arg.parameters[I].labelOffsets)
   return false;
+  }
   return true;
 }
 MATCHER_P(SigDoc, Doc, "") { return arg.documentation == Doc; }
 
-Matcher Sig(std::string Label,
-  std::vector Params) {
-  return AllOf(SigHelpLabeled(Label), ParamsAre(Params));
+/// \p AnnotatedLabel is a signature label with ranges marking parameters, e.g.
+///foo([[int p1]], [[double p2]]) -> void
+Matcher Sig(llvm::StringRef AnnotatedLabel) {
+  llvm::Annotations A(AnnotatedLabel);
+  std::string Label = A.code();
+  std::vector Parameters;
+  for (auto Range : A.ranges()) {
+Parameters.emplace_back();
+
+ExpectedParameter &P = Parameters.back();
+P.Text = Label.substr(Range.Begin, Range.End - Range.Begin);
+P.Offsets.first = lspLength(llvm::StringRef(Label).substr(0, Range.Begin));
+P.Offsets.second = lspLength(llvm::StringRef(Label).substr(1, Range.End));
+  }
+  return AllOf(SigHelpLabeled(Label), ParamsAre(Parameters));
 }
 
 TEST(SignatureHelpTest, Overloads) {
@@ -953,11 +971,10 @@
 int main() { foo(^); }
   )cpp");
   EXPECT_THAT(Results.signatures,
-  UnorderedElementsAre(
-  Sig("foo(float x, float y) -> void", {"float x", "float y"}),
-  Sig("foo(float x, int y) -> void", {"float x", "int y"}),
-  Sig("foo(int x, float y) -> void", {"int x", "float y"}),
-  Sig("foo(int x, int y) -> void", {"int x", "int y"})));
+  UnorderedElementsAre(Sig("foo([[float x]], [[float y]]) -> void"),
+   Sig("foo([[float x]], [[int y]]) -> void"),
+   Sig("foo([[int x]], [[float y]]) -> void"),
+   Sig("foo([[int x]], [[int y]]) -> void")));
   // We always prefer the first signature.
   EXPECT_EQ(0, Results.activeSignature);
   EXPECT_EQ(0, Results.activeParameter);
@@ -971,9 +988,8 @@
   )cpp");
   EXPECT_THAT(Results.signatures,
   UnorderedElementsAre(
-  Sig("bar(int x, int y = 0) -> void", {"int x", "int y = 0"}),
-  Sig("bar(float x = 0, int y = 42) -> void",
-  {"float x = 0", "int y = 42"})));
+  Sig("bar([[int x]], [[int y = 0]]) -> void"),
+  Sig("bar([[float x = 0]], [[int y = 42]]) -> void")));
   EXPECT_EQ(0, Results.activeSignature);
   EXPECT_EQ(0, Results.activeParameter);
 }
@@ -984,8 +1000,7 @@
 int main() { baz(baz(1,2,3), ^); }
   )cpp");
   EXPECT_THAT(Results.signatures,
-  ElementsAre(Sig("baz(int a, int b, int c) -> int",
-  {"int a", "int b", "int c"})));
+  ElementsAre(Sig("baz([[int a]], [[int b]], [[int c]]) -> int")));
   EXPECT_EQ(0, Results.activeSignature);
   EXPECT_EQ(1, Results.activeParameter);
 }
@@ -1722,14 +1737,12 @@
 void foo(int x, int y = 0);
 int main() { foo(^); }
   )cpp");
-  EXPECT_THAT(
-  Results.signatures,
-  ElementsAre(
-  Sig("foo(int x) -> void", {"int x"}),
-  Sig("foo(int x, int y = 0) -> void", {"int x", "int y = 0"}),
-  Sig("foo(float x, int y) -> void", {"float x", "int y"}),
-  Sig("foo(int x, float y) -> void", {"int x", "float y"}),
-  Sig("foo(float x, float y) -> void", {"float x", "float y"})));
+  EXPECT_THAT(Results.signatures,
+  ElementsAre(Sig("foo([[int x]]) -> void"),
+  Sig("foo([[int x]], [[int y = 0]]) -> void"),
+  Sig("foo([[float x]], [[int y]]) -> void"),
+  Sig("foo([[int x]], [[flo

[PATCH] D62479: Replace `CXXSelfAssignmentBRVisitor` with `NoteTags`

2019-05-27 Thread Balogh, Ádám via Phabricator via cfe-commits

baloghadamsoftware created this revision.
baloghadamsoftware added a reviewer: NoQ.
baloghadamsoftware added a project: clang.
Herald added subscribers: gamesh411, rnkovacs.

The `cplusplus.SelfAssignment` checker has a visitor that is added to every 
`BugReport` to mark the to branch of the self assignment operator with e.g. 
`rhs == *this` and `rhs != *this`. With the new `NoteTag` feature this visitor 
is not needed anymore. Instead the checker itself marks the two branches using 
the `NoteTag`s.


Repository:
  rC Clang

https://reviews.llvm.org/D62479

Files:
  include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
  lib/StaticAnalyzer/Checkers/CXXSelfAssignmentChecker.cpp
  lib/StaticAnalyzer/Core/BugReporter.cpp
  lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  lib/StaticAnalyzer/Core/PathDiagnostic.cpp

Index: lib/StaticAnalyzer/Core/PathDiagnostic.cpp
===
--- lib/StaticAnalyzer/Core/PathDiagnostic.cpp
+++ lib/StaticAnalyzer/Core/PathDiagnostic.cpp
@@ -732,6 +732,13 @@
 
 } else {
   S = BSrc->getTerminatorCondition();
+  if (!S) {
+// If the BlockEdge has no terminator condition statement (e.g. a
+// checker crated the branch at the beginning of a function), use the
+// function's declaration instead.
+return PathDiagnosticLocation::createBegin(
+P.getLocationContext()->getDecl(), SMng);
+  }
 }
   } else if (Optional SP = P.getAs()) {
 S = SP->getStmt();
Index: lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===
--- lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -2371,59 +2371,6 @@
   return nullptr;
 }
 
-std::shared_ptr
-CXXSelfAssignmentBRVisitor::VisitNode(const ExplodedNode *Succ,
-  BugReporterContext &BRC, BugReport &) {
-  if (Satisfied)
-return nullptr;
-
-  const auto Edge = Succ->getLocation().getAs();
-  if (!Edge.hasValue())
-return nullptr;
-
-  auto Tag = Edge->getTag();
-  if (!Tag)
-return nullptr;
-
-  if (Tag->getTagDescription() != "cplusplus.SelfAssignment")
-return nullptr;
-
-  Satisfied = true;
-
-  const auto *Met =
-  dyn_cast(Succ->getCodeDecl().getAsFunction());
-  assert(Met && "Not a C++ method.");
-  assert((Met->isCopyAssignmentOperator() || Met->isMoveAssignmentOperator()) &&
- "Not a copy/move assignment operator.");
-
-  const auto *LCtx = Edge->getLocationContext();
-
-  const auto &State = Succ->getState();
-  auto &SVB = State->getStateManager().getSValBuilder();
-
-  const auto Param =
-  State->getSVal(State->getRegion(Met->getParamDecl(0), LCtx));
-  const auto This =
-  State->getSVal(SVB.getCXXThis(Met, LCtx->getStackFrame()));
-
-  auto L = PathDiagnosticLocation::create(Met, BRC.getSourceManager());
-
-  if (!L.isValid() || !L.asLocation().isValid())
-return nullptr;
-
-  SmallString<256> Buf;
-  llvm::raw_svector_ostream Out(Buf);
-
-  Out << "Assuming " << Met->getParamDecl(0)->getName() <<
-((Param == This) ? " == " : " != ") << "*this";
-
-  auto Piece = std::make_shared(L, Out.str());
-  Piece->addRange(Met->getSourceRange());
-
-  return std::move(Piece);
-}
-
-
 FalsePositiveRefutationBRVisitor::FalsePositiveRefutationBRVisitor()
 : Constraints(ConstraintRangeTy::Factory().getEmptyMap()) {}
 
Index: lib/StaticAnalyzer/Core/BugReporter.cpp
===
--- lib/StaticAnalyzer/Core/BugReporter.cpp
+++ lib/StaticAnalyzer/Core/BugReporter.cpp
@@ -2610,7 +2610,6 @@
 // Register additional node visitors.
 R->addVisitor(llvm::make_unique());
 R->addVisitor(llvm::make_unique());
-R->addVisitor(llvm::make_unique());
 R->addVisitor(llvm::make_unique());
 
 BugReporterContext BRC(Reporter, ErrorGraph.BackMap);
Index: lib/StaticAnalyzer/Checkers/CXXSelfAssignmentChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/CXXSelfAssignmentChecker.cpp
+++ lib/StaticAnalyzer/Checkers/CXXSelfAssignmentChecker.cpp
@@ -50,10 +50,26 @@
   State->getSVal(SVB.getCXXThis(MD, LCtx->getStackFrame()));
   auto Param = SVB.makeLoc(State->getRegion(MD->getParamDecl(0), LCtx));
   auto ParamVal = State->getSVal(Param);
+
   ProgramStateRef SelfAssignState = State->bindLoc(Param, ThisVal, LCtx);
-  C.addTransition(SelfAssignState);
+  const NoteTag *SelfAssignTag =
+C.getNoteTag([MD](BugReport &BR) -> std::string {
+SmallString<256> Msg;
+llvm::raw_svector_ostream Out(Msg);
+Out << "Assuming " << MD->getParamDecl(0)->getName() << " == *this";
+return Out.str();
+  });
+  C.addTransition(SelfAssignState, SelfAssignTag);
+
   ProgramStateRef NonSelfAssignState = State->bindLoc(Param, ParamVal, LCtx);
-  C.addTransition(NonSelfAssignState);
+  const NoteTag *Non

r361757 - [OpenCL] Fix file-scope const sampler variable for 2.0

2019-05-27 Thread Yaxun Liu via cfe-commits

Author: yaxunl
Date: Mon May 27 04:19:07 2019
New Revision: 361757

URL: http://llvm.org/viewvc/llvm-project?rev=361757&view=rev
Log:
[OpenCL] Fix file-scope const sampler variable for 2.0

OpenCL spec v2.0 s6.13.14:

Samplers can also be declared as global constants in the program
source using the following syntax.

   const sampler_t  = 
This works fine for OpenCL 1.2 but fails for 2.0, because clang duduces
address space of file-scope const sampler variable to be in global address
space whereas spec v2.0 s6.9.b forbids file-scope sampler variable to be
in global address space.

The fix is not to deduce address space for file-scope sampler variables.

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

Modified:
cfe/trunk/lib/Sema/SemaType.cpp
cfe/trunk/test/CodeGenOpenCL/sampler.cl
cfe/trunk/test/SemaOpenCL/sampler_t.cl

Modified: cfe/trunk/lib/Sema/SemaType.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaType.cpp?rev=361757&r1=361756&r2=361757&view=diff
==
--- cfe/trunk/lib/Sema/SemaType.cpp (original)
+++ cfe/trunk/lib/Sema/SemaType.cpp Mon May 27 04:19:07 2019
@@ -7363,7 +7363,21 @@ static void deduceOpenCLImplicitAddrSpac
   T->isDependentType() ||
   // Do not deduce addr space of decltype because it will be taken from
   // its argument.
-  T->isDecltypeType())
+  T->isDecltypeType() ||
+  // OpenCL spec v2.0 s6.9.b:
+  // The sampler type cannot be used with the __local and __global address
+  // space qualifiers.
+  // OpenCL spec v2.0 s6.13.14:
+  // Samplers can also be declared as global constants in the program
+  // source using the following syntax.
+  //   const sampler_t  = 
+  // In codegen, file-scope sampler type variable has special handing and
+  // does not rely on address space qualifier. On the other hand, deducing
+  // address space of const sampler file-scope variable as global address
+  // space causes spurious diagnostic about __global address space
+  // qualifier, therefore do not deduce address space of file-scope sampler
+  // type variable.
+  (D.getContext() == DeclaratorContext::FileContext && T->isSamplerT()))
 return;
 
   LangAS ImpAddr = LangAS::Default;

Modified: cfe/trunk/test/CodeGenOpenCL/sampler.cl
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenOpenCL/sampler.cl?rev=361757&r1=361756&r2=361757&view=diff
==
--- cfe/trunk/test/CodeGenOpenCL/sampler.cl (original)
+++ cfe/trunk/test/CodeGenOpenCL/sampler.cl Mon May 27 04:19:07 2019
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 %s -emit-llvm -triple spir-unknown-unknown -o - -O0 | 
FileCheck %s
+// RUN: %clang_cc1 %s -cl-std=CL2.0 -emit-llvm -triple spir-unknown-unknown -o 
- -O0 | FileCheck %s
 //
 // This test covers 5 cases of sampler initialzation:
 //   1. function argument passing
@@ -6,8 +7,9 @@
 //  1b. argument is a function-scope variable
 //  1c. argument is one of caller function's parameters
 //   2. variable initialization
-//  2a. initializing a file-scope variable
+//  2a. initializing a file-scope variable with constant addr space 
qualifier
 //  2b. initializing a function-scope variable
+//  2c. initializing a file-scope variable with const qualifier
 
 #define CLK_ADDRESS_CLAMP_TO_EDGE   2
 #define CLK_NORMALIZED_COORDS_TRUE  1
@@ -20,6 +22,10 @@
 constant sampler_t glb_smp = CLK_ADDRESS_CLAMP_TO_EDGE | 
CLK_NORMALIZED_COORDS_TRUE | CLK_FILTER_LINEAR;
 // CHECK-NOT: glb_smp
 
+// Case 2c
+const sampler_t glb_smp_const = CLK_ADDRESS_CLAMP_TO_EDGE | 
CLK_NORMALIZED_COORDS_TRUE | CLK_FILTER_LINEAR;
+// CHECK-NOT: glb_smp_const
+
 int get_sampler_initializer(void);
 
 void fnc4smp(sampler_t s) {}
@@ -47,11 +53,16 @@ kernel void foo(sampler_t smp_par) {
   // CHECK: [[SAMP:%[0-9]+]] = load %opencl.sampler_t addrspace(2)*, 
%opencl.sampler_t addrspace(2)** [[smp_ptr]]
   // CHECK: call spir_func void @fnc4smp(%opencl.sampler_t addrspace(2)* 
[[SAMP]])
 
-  // Case 1a
+  // Case 1a/2a
   fnc4smp(glb_smp);
   // CHECK: [[SAMP:%[0-9]+]] = call %opencl.sampler_t addrspace(2)* 
@__translate_sampler_initializer(i32 35)
   // CHECK: call spir_func void @fnc4smp(%opencl.sampler_t addrspace(2)* 
[[SAMP]])
 
+  // Case 1a/2c
+  fnc4smp(glb_smp_const);
+  // CHECK: [[SAMP:%[0-9]+]] = call %opencl.sampler_t addrspace(2)* 
@__translate_sampler_initializer(i32 35)
+  // CHECK: call spir_func void @fnc4smp(%opencl.sampler_t addrspace(2)* 
[[SAMP]])
+
   // Case 1c
   fnc4smp(smp_par);
   // CHECK: [[SAMP:%[0-9]+]] = load %opencl.sampler_t addrspace(2)*, 
%opencl.sampler_t addrspace(2)** [[smp_par_ptr]]

Modified: cfe/trunk/test/SemaOpenCL/sampler_t.cl
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaOpenCL/sampler_t.cl?rev=361757&r1=361756&r2=361757&view=diff
=

[PATCH] D62197: [OpenCL] Fix file-scope const sampler variable for 2.0

2019-05-27 Thread Yaxun Liu via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rC361757: [OpenCL] Fix file-scope const sampler variable for 
2.0 (authored by yaxunl, committed by ).
Herald added a project: clang.

Repository:
  rC Clang

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

https://reviews.llvm.org/D62197

Files:
  lib/Sema/SemaType.cpp
  test/CodeGenOpenCL/sampler.cl
  test/SemaOpenCL/sampler_t.cl

Index: test/SemaOpenCL/sampler_t.cl
===
--- test/SemaOpenCL/sampler_t.cl
+++ test/SemaOpenCL/sampler_t.cl
@@ -1,6 +1,9 @@
 // RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only
 // RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -DCHECK_SAMPLER_VALUE -Wspir-compat -triple amdgcn--amdhsa
 // RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -DCHECK_SAMPLER_VALUE -triple spir-unknown-unknown
+// RUN: %clang_cc1 %s -cl-std=CL2.0 -verify -pedantic -fsyntax-only
+// RUN: %clang_cc1 %s -cl-std=CL2.0 -verify -pedantic -fsyntax-only -DCHECK_SAMPLER_VALUE -Wspir-compat -triple amdgcn--amdhsa
+// RUN: %clang_cc1 %s -cl-std=CL2.0 -verify -pedantic -fsyntax-only -DCHECK_SAMPLER_VALUE -triple spir-unknown-unknown
 
 #define CLK_ADDRESS_CLAMP_TO_EDGE   2
 #define CLK_NORMALIZED_COORDS_TRUE  1
@@ -55,7 +58,11 @@
   sampler_t sa[] = {argsmp, glb_smp}; // expected-error {{array of 'sampler_t' type is invalid in OpenCL}}
 }
 
+#if __OPENCL_C_VERSION__ == 200
+void bad(sampler_t*); // expected-error{{pointer to type '__generic sampler_t' is invalid in OpenCL}}
+#else
 void bad(sampler_t*); // expected-error{{pointer to type 'sampler_t' is invalid in OpenCL}}
+#endif
 
 void bar() {
   sampler_t smp1 = CLK_ADDRESS_CLAMP_TO_EDGE | CLK_NORMALIZED_COORDS_TRUE | CLK_FILTER_LINEAR;
Index: test/CodeGenOpenCL/sampler.cl
===
--- test/CodeGenOpenCL/sampler.cl
+++ test/CodeGenOpenCL/sampler.cl
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 %s -emit-llvm -triple spir-unknown-unknown -o - -O0 | FileCheck %s
+// RUN: %clang_cc1 %s -cl-std=CL2.0 -emit-llvm -triple spir-unknown-unknown -o - -O0 | FileCheck %s
 //
 // This test covers 5 cases of sampler initialzation:
 //   1. function argument passing
@@ -6,8 +7,9 @@
 //  1b. argument is a function-scope variable
 //  1c. argument is one of caller function's parameters
 //   2. variable initialization
-//  2a. initializing a file-scope variable
+//  2a. initializing a file-scope variable with constant addr space qualifier
 //  2b. initializing a function-scope variable
+//  2c. initializing a file-scope variable with const qualifier
 
 #define CLK_ADDRESS_CLAMP_TO_EDGE   2
 #define CLK_NORMALIZED_COORDS_TRUE  1
@@ -20,6 +22,10 @@
 constant sampler_t glb_smp = CLK_ADDRESS_CLAMP_TO_EDGE | CLK_NORMALIZED_COORDS_TRUE | CLK_FILTER_LINEAR;
 // CHECK-NOT: glb_smp
 
+// Case 2c
+const sampler_t glb_smp_const = CLK_ADDRESS_CLAMP_TO_EDGE | CLK_NORMALIZED_COORDS_TRUE | CLK_FILTER_LINEAR;
+// CHECK-NOT: glb_smp_const
+
 int get_sampler_initializer(void);
 
 void fnc4smp(sampler_t s) {}
@@ -47,11 +53,16 @@
   // CHECK: [[SAMP:%[0-9]+]] = load %opencl.sampler_t addrspace(2)*, %opencl.sampler_t addrspace(2)** [[smp_ptr]]
   // CHECK: call spir_func void @fnc4smp(%opencl.sampler_t addrspace(2)* [[SAMP]])
 
-  // Case 1a
+  // Case 1a/2a
   fnc4smp(glb_smp);
   // CHECK: [[SAMP:%[0-9]+]] = call %opencl.sampler_t addrspace(2)* @__translate_sampler_initializer(i32 35)
   // CHECK: call spir_func void @fnc4smp(%opencl.sampler_t addrspace(2)* [[SAMP]])
 
+  // Case 1a/2c
+  fnc4smp(glb_smp_const);
+  // CHECK: [[SAMP:%[0-9]+]] = call %opencl.sampler_t addrspace(2)* @__translate_sampler_initializer(i32 35)
+  // CHECK: call spir_func void @fnc4smp(%opencl.sampler_t addrspace(2)* [[SAMP]])
+
   // Case 1c
   fnc4smp(smp_par);
   // CHECK: [[SAMP:%[0-9]+]] = load %opencl.sampler_t addrspace(2)*, %opencl.sampler_t addrspace(2)** [[smp_par_ptr]]
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -7363,7 +7363,21 @@
   T->isDependentType() ||
   // Do not deduce addr space of decltype because it will be taken from
   // its argument.
-  T->isDecltypeType())
+  T->isDecltypeType() ||
+  // OpenCL spec v2.0 s6.9.b:
+  // The sampler type cannot be used with the __local and __global address
+  // space qualifiers.
+  // OpenCL spec v2.0 s6.13.14:
+  // Samplers can also be declared as global constants in the program
+  // source using the following syntax.
+  //   const sampler_t  = 
+  // In codegen, file-scope sampler type variable has special handing and
+  // does not rely on address space qualifier. On the other hand, deducing
+  // address space of const sampler file-scope variable as global address
+  // space causes spurious diagnostic about __global addres

[PATCH] D62389: [clangd] Place cursor better after completing patterns

2019-05-27 Thread Haojian Wu via Phabricator via cfe-commits

hokein added a comment.

In D62389#1517907 , @ilya-biryukov 
wrote:

> In D62389#1517844 , @hokein wrote:
>
> > I'm not sure the new behavior suits all cases, it seems to me that this 
> > will change a typical behavior (usually when users invoke function calls).
> >
> > Imaging a case:
> >
> >   void foo(int a, int b);
> >  
> >   void call() {
> > fo^
> >   }
> >
> >
> >
> >
> > - before, the final cursor is at the end of the snippet
> > - after, the final cursor is at the last parameter of `foo`.
> >
> >   I think the previous cursor location is more convenient (user can 
> > continue typing `;`).
>
>
> Agree! The new behavior does not apply to function calls, they are not 
> considered 'patterns' in the completion results.
>  I.e. in your examples we're still getting `foo(${1:int a}, ${2:int b})` and 
> the cursor is placed at the end.
>
> This is definitely worth a comment, I'll try to find a good place for it.


Ah, that makes sense! Thanks for clarifying it.

How about adding one more test in CodeCompleteTests.cpp, we already have a 
snippet test there, and it seems more straightforward.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62389



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

[PATCH] D60605: [clangd] Revamp textDocument/onTypeFormatting.

2019-05-27 Thread 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 few NITs and a few questions about the possibility of moving this 
to `clang-format` at some point in the future.
From what I can to simplify the calling, we need to:

1. teach `clang-format` about a cursor (by making it a special token or somehow 
else), this would allow to avoid some book-keeping;
2. teach `clang-format` to format in presence of unmatched parentheses. Again, 
would simplify things. Although not sure it's easy to do in `clang-format` 
either;
3. teach `clang-format` to respect indentation before the cursor (added by the 
editor in our case).

After that we don't seem to need the complicated multi-pass replacements.

- Am I missing something that we also need from `clang-format` here?
- How would you estimate the amount of work needed to move this functionality 
to `clang-format`?
- In general, do you think it's worth moving it there or not?




Comment at: clangd/Format.cpp:83
+
+llvm::StringRef Filename = "";
+

NIT:  put a comment about filename being required but ignored here to explain 
why we need this variable.



Comment at: clangd/Format.h:25
+/// Applies limited formatting around new \p InsertedText.
+/// The \p Code already contains the updated text near \p Cursor, and may have
+/// had additional / characters (such as indentation) inserted by the editor.

NIT: maybe be more concrete? `near \p Cursor` means `right before \p Cursor`? 
Or can it have other characters (e.g. whitespace in your example)?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D60605



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

[PATCH] D58033: Add option for emitting dbg info for call site parameters

2019-05-27 Thread Djordje Todorovic via Phabricator via cfe-commits

djtodoro updated this revision to Diff 201503.
djtodoro added a comment.

-Rebase


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

https://reviews.llvm.org/D58033

Files:
  include/clang/Basic/CodeGenOptions.def
  include/clang/Driver/CC1Options.td
  lib/CodeGen/BackendUtil.cpp
  lib/CodeGen/CGDebugInfo.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGenCXX/dbg-info-all-calls-described.cpp


Index: test/CodeGenCXX/dbg-info-all-calls-described.cpp
===
--- test/CodeGenCXX/dbg-info-all-calls-described.cpp
+++ test/CodeGenCXX/dbg-info-all-calls-described.cpp
@@ -15,6 +15,19 @@
 // RUN: | FileCheck %s -check-prefix=HAS-ATTR \
 // RUN: -implicit-check-not=DISubprogram 
-implicit-check-not=DIFlagAllCallsDescribed
 
+// Supported: DWARF4 + GDB tuning by using '-femit-debug-entry-values'
+// RUN: %clang_cc1 -femit-debug-entry-values -emit-llvm %s -o - \
+// RUN:   -O1 -disable-llvm-passes -debugger-tuning=gdb \
+// RUN:   -debug-info-kind=standalone -dwarf-version=4 \
+// RUN: | FileCheck %s -check-prefix=HAS-ATTR \
+// RUN: -implicit-check-not=DIFlagAllCallsDescribed
+
+// Unsupported: -O0 + '-femit-debug-entry-values'
+// RUN: %clang_cc1 -femit-debug-entry-values -emit-llvm %s -o - \
+// RUN:   -O0 -disable-llvm-passes -debugger-tuning=gdb \
+// RUN:   -debug-info-kind=standalone -dwarf-version=4 \
+// RUN: | FileCheck %s -check-prefix=NO-ATTR
+
 // Supported: DWARF4 + LLDB tuning, -O1, line-tables only DI
 // RUN: %clang_cc1 -emit-llvm -triple %itanium_abi_triple %s -o - \
 // RUN:   -O1 -disable-llvm-passes \
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -747,6 +747,8 @@
 
   Opts.DisableLLVMPasses = Args.hasArg(OPT_disable_llvm_passes);
   Opts.DisableLifetimeMarkers = Args.hasArg(OPT_disable_lifetimemarkers);
+  if (Opts.OptimizationLevel > 0)
+Opts.EnableDebugEntryValues = Args.hasArg(OPT_femit_debug_entry_values);
   Opts.DisableO0ImplyOptNone = Args.hasArg(OPT_disable_O0_optnone);
   Opts.DisableRedZone = Args.hasArg(OPT_disable_red_zone);
   Opts.IndirectTlsSegRefs = Args.hasArg(OPT_mno_tls_direct_seg_refs);
Index: lib/CodeGen/CGDebugInfo.cpp
===
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -4619,7 +4619,10 @@
   // were part of DWARF v4.
   bool SupportsDWARFv4Ext =
   CGM.getCodeGenOpts().DwarfVersion == 4 &&
-  CGM.getCodeGenOpts().getDebuggerTuning() == llvm::DebuggerKind::LLDB;
+  (CGM.getCodeGenOpts().getDebuggerTuning() == llvm::DebuggerKind::LLDB ||
+   (CGM.getCodeGenOpts().EnableDebugEntryValues &&
+   CGM.getCodeGenOpts().getDebuggerTuning() == llvm::DebuggerKind::GDB));
+
   if (!SupportsDWARFv4Ext && CGM.getCodeGenOpts().DwarfVersion < 5)
 return llvm::DINode::FlagZero;
 
Index: lib/CodeGen/BackendUtil.cpp
===
--- lib/CodeGen/BackendUtil.cpp
+++ lib/CodeGen/BackendUtil.cpp
@@ -466,6 +466,7 @@
   Options.DebuggerTuning = CodeGenOpts.getDebuggerTuning();
   Options.EmitStackSizeSection = CodeGenOpts.StackSizeSection;
   Options.EmitAddrsig = CodeGenOpts.Addrsig;
+  Options.EnableDebugEntryValues = CodeGenOpts.EnableDebugEntryValues;
 
   if (CodeGenOpts.getSplitDwarfMode() != CodeGenOptions::NoFission)
 Options.MCOptions.SplitDwarfFile = CodeGenOpts.SplitDwarfFile;
Index: include/clang/Driver/CC1Options.td
===
--- include/clang/Driver/CC1Options.td
+++ include/clang/Driver/CC1Options.td
@@ -368,6 +368,8 @@
 def fno_lto_unit: Flag<["-"], "fno-lto-unit">;
 def fthin_link_bitcode_EQ : Joined<["-"], "fthin-link-bitcode=">,
 HelpText<"Write minimized bitcode to  for the ThinLTO thin link 
only">;
+def femit_debug_entry_values : Flag<["-"], "femit-debug-entry-values">,
+HelpText<"Enables debug info about call site parameter's entry values">;
 def fdebug_pass_manager : Flag<["-"], "fdebug-pass-manager">,
 HelpText<"Prints debug information for the new pass manager">;
 def fno_debug_pass_manager : Flag<["-"], "fno-debug-pass-manager">,
Index: include/clang/Basic/CodeGenOptions.def
===
--- include/clang/Basic/CodeGenOptions.def
+++ include/clang/Basic/CodeGenOptions.def
@@ -61,6 +61,7 @@
 CODEGENOPT(DebugPassManager, 1, 0) ///< Prints debug information for the new
///< pass manager.
 CODEGENOPT(DisableRedZone, 1, 0) ///< Set when -mno-red-zone is enabled.
+CODEGENOPT(EnableDebugEntryValues, 1, 0) ///< Emit call site parameter dbg info
 CODEGENOPT(IndirectTlsSegRefs, 1, 0) ///< Set when -mno-tls-direct-seg-refs
  ///< is specified.
 CODEGENOPT(DisableTailCalls  , 1, 0) ///<

[PATCH] D58035: [clang/DIVar] Emit flag for params that have unchanged values

2019-05-27 Thread Djordje Todorovic via Phabricator via cfe-commits

djtodoro updated this revision to Diff 201505.
djtodoro added a comment.

-Rebase
-Add a test


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

https://reviews.llvm.org/D58035

Files:
  lib/CodeGen/CGDebugInfo.cpp
  lib/CodeGen/CGDebugInfo.h
  test/CodeGen/debug-info-param-modification.c

Index: test/CodeGen/debug-info-param-modification.c
===
--- /dev/null
+++ test/CodeGen/debug-info-param-modification.c
@@ -0,0 +1,12 @@
+// RUN: %clang -Xclang -femit-debug-entry-values -g -O2 -S -emit-llvm %s -o - | FileCheck %s -check-prefix=CHECK-ENTRY-VAL-OPT
+// CHECK-ENTRY-VAL-OPT: !DILocalVariable(name: "a", arg: 1, scope: {{.*}}, file: {{.*}}, line: {{.*}}, type: {{.*}})
+// CHECK-ENTRY-VAL-OPT: !DILocalVariable(name: "b", arg: 2, scope: {{.*}}, file: {{.*}}, line: {{.*}}, type: {{.*}}, flags: DIFlagArgumentNotModified)
+//
+// RUN: %clang -g -O2 -S -emit-llvm %s -o - | FileCheck %s
+// CHECK-NOT: !DILocalVariable(name: "b", arg: 2, scope: {{.*}}, file: {{.*}}, line: {{.*}}, type: {{.*}}, flags: DIFlagArgumentNotModified)
+//
+
+int fn2 (int a, int b) {
+  ++a;
+  return b;
+}
Index: lib/CodeGen/CGDebugInfo.h
===
--- lib/CodeGen/CGDebugInfo.h
+++ lib/CodeGen/CGDebugInfo.h
@@ -134,6 +134,10 @@
 
   llvm::DenseMap DIFileCache;
   llvm::DenseMap SPCache;
+  /// Cache function definitions relevant to use for parameters mutation
+  /// analysis.
+  llvm::DenseMap SPDefCache;
+  llvm::DenseMap ParamCache;
   /// Cache declarations relevant to DW_TAG_imported_declarations (C++
   /// using declarations) that aren't covered by other more specific caches.
   llvm::DenseMap DeclCache;
Index: lib/CodeGen/CGDebugInfo.cpp
===
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -18,6 +18,7 @@
 #include "CodeGenFunction.h"
 #include "CodeGenModule.h"
 #include "ConstantEmitter.h"
+#include "clang/Analysis/Analyses/ExprMutationAnalyzer.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/DeclFriend.h"
 #include "clang/AST/DeclObjC.h"
@@ -3577,6 +3578,12 @@
   if (HasDecl && isa(D))
 DeclCache[D->getCanonicalDecl()].reset(SP);
 
+  // We use the SPDefCache only in the case when the debug entry values option
+  // is set, in order to speed up parameters modification analysis.
+  if (CGM.getCodeGenOpts().EnableDebugEntryValues && HasDecl &&
+  isa(D))
+SPDefCache[cast(D)].reset(SP);
+
   if (CGM.getCodeGenOpts().DwarfVersion >= 5) {
 // Starting with DWARF V5 method declarations are emitted as children of
 // the interface type.
@@ -3942,6 +3949,11 @@
  llvm::DebugLoc::get(Line, Column, Scope, CurInlinedAt),
  Builder.GetInsertBlock());
 
+  if (CGM.getCodeGenOpts().EnableDebugEntryValues && ArgNo) {
+if (auto *PD = dyn_cast(VD))
+  ParamCache[PD].reset(D);
+  }
+
   return D;
 }
 
@@ -4515,6 +4527,29 @@
   TheCU->setDWOId(Signature);
 }
 
+/// Analyzes each function parameter to determine whether it is constant
+/// throughout the function body.
+static void analyzeParametersModification(
+ASTContext &Ctx,
+llvm::DenseMap &SPDefCache,
+llvm::DenseMap &ParamCache) {
+  for (auto &SP : SPDefCache) {
+auto *FD = SP.first;
+assert(FD->hasBody() && "Functions must have body here");
+const Stmt *FuncBody = (*FD).getBody();
+for (auto Parm : FD->parameters()) {
+  ExprMutationAnalyzer FuncAnalyzer(*FuncBody, Ctx);
+  if (FuncAnalyzer.isMutated(Parm))
+continue;
+
+  auto I = ParamCache.find(Parm);
+  assert(I != ParamCache.end() && "Parameters should be already cached");
+  auto *DIParm = cast(I->second);
+  DIParm->setIsNotModified();
+}
+  }
+}
+
 void CGDebugInfo::finalize() {
   // Creating types might create further types - invalidating the current
   // element and the size(), so don't cache/reference them.
@@ -4587,6 +4622,10 @@
 if (auto MD = TypeCache[RT])
   DBuilder.retainType(cast(MD));
 
+  if (CGM.getCodeGenOpts().EnableDebugEntryValues)
+// This will be used to emit debug entry values.
+analyzeParametersModification(CGM.getContext(), SPDefCache, ParamCache);
+
   DBuilder.finalize();
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D62483: [CUDA][HIP] Emit dependent libs for host only

2019-05-27 Thread Yaxun Liu via Phabricator via cfe-commits

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

Recently D60274  was introduced to allow lld 
to handle dependent libs. However current
usage of dependent libs (e.g. pragma comment(lib, *)  in windows header files) 
are intended
for host only. Emitting the metadata in device IR causes link error in device 
path.

Until there is a way to different it dependent libs for device or host, 
metadata for dependent
libs should be emitted for host only. This patch enforces that.


https://reviews.llvm.org/D62483

Files:
  lib/CodeGen/CodeGenModule.cpp
  test/CodeGenCUDA/dependent-libs.cu


Index: test/CodeGenCUDA/dependent-libs.cu
===
--- /dev/null
+++ test/CodeGenCUDA/dependent-libs.cu
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -emit-llvm -o - -fcuda-is-device -x hip %s | FileCheck 
--check-prefix=DEV %s
+// RUN: %clang_cc1 -emit-llvm -o - -x hip %s | FileCheck --check-prefix=HOST %s
+
+// DEV-NOT: llvm.dependent-libraries
+// HOST: llvm.dependent-libraries
+#pragma comment(lib, "libabc")
Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -460,7 +460,12 @@
   // that ELF linkers tend to handle libraries in a more complicated fashion
   // than on other platforms. This forces us to defer handling the dependent
   // libs to the linker.
-  if (!ELFDependentLibraries.empty()) {
+  //
+  // CUDA/HIP device and host libraries are different. Currently there is no
+  // way to differentiate dependent libraries for host or device. Existing
+  // usage of #pragma comment(lib, *) is intended for host libraries on
+  // Windows. Therefore emit llvm.dependent-libraries only for host.
+  if (!ELFDependentLibraries.empty() && !Context.getLangOpts().CUDAIsDevice) {
 auto *NMD = 
getModule().getOrInsertNamedMetadata("llvm.dependent-libraries");
 for (auto *MD : ELFDependentLibraries)
   NMD->addOperand(MD);


Index: test/CodeGenCUDA/dependent-libs.cu
===
--- /dev/null
+++ test/CodeGenCUDA/dependent-libs.cu
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -emit-llvm -o - -fcuda-is-device -x hip %s | FileCheck --check-prefix=DEV %s
+// RUN: %clang_cc1 -emit-llvm -o - -x hip %s | FileCheck --check-prefix=HOST %s
+
+// DEV-NOT: llvm.dependent-libraries
+// HOST: llvm.dependent-libraries
+#pragma comment(lib, "libabc")
Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -460,7 +460,12 @@
   // that ELF linkers tend to handle libraries in a more complicated fashion
   // than on other platforms. This forces us to defer handling the dependent
   // libs to the linker.
-  if (!ELFDependentLibraries.empty()) {
+  //
+  // CUDA/HIP device and host libraries are different. Currently there is no
+  // way to differentiate dependent libraries for host or device. Existing
+  // usage of #pragma comment(lib, *) is intended for host libraries on
+  // Windows. Therefore emit llvm.dependent-libraries only for host.
+  if (!ELFDependentLibraries.empty() && !Context.getLangOpts().CUDAIsDevice) {
 auto *NMD = getModule().getOrInsertNamedMetadata("llvm.dependent-libraries");
 for (auto *MD : ELFDependentLibraries)
   NMD->addOperand(MD);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D62389: [clangd] Place cursor better after completing patterns

2019-05-27 Thread Ilya Biryukov via Phabricator via cfe-commits

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

- Add a code completion test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62389

Files:
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/CodeCompletionStrings.cpp
  clang-tools-extra/clangd/CodeCompletionStrings.h
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
  clang-tools-extra/clangd/unittests/CodeCompletionStringsTests.cpp

Index: clang-tools-extra/clangd/unittests/CodeCompletionStringsTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompletionStringsTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompletionStringsTests.cpp
@@ -22,10 +22,12 @@
 CCTUInfo(Allocator), Builder(*Allocator, CCTUInfo) {}
 
 protected:
-  void computeSignature(const CodeCompletionString &CCS) {
+  void computeSignature(const CodeCompletionString &CCS,
+bool CompletingPattern = false) {
 Signature.clear();
 Snippet.clear();
-getSignature(CCS, &Signature, &Snippet);
+getSignature(CCS, &Signature, &Snippet, /*RequiredQualifier=*/nullptr,
+ CompletingPattern);
   }
 
   std::shared_ptr Allocator;
@@ -99,6 +101,25 @@
   EXPECT_EQ(Snippet, "(${1:\\$p\\}1})");
 }
 
+TEST_F(CompletionStringTest, SnippetsInPatterns) {
+  auto MakeCCS = [this]() -> const CodeCompletionString & {
+CodeCompletionBuilder Builder(*Allocator, CCTUInfo);
+Builder.AddTypedTextChunk("namespace");
+Builder.AddChunk(CodeCompletionString::CK_HorizontalSpace);
+Builder.AddPlaceholderChunk("name");
+Builder.AddChunk(CodeCompletionString::CK_Equal);
+Builder.AddPlaceholderChunk("target");
+Builder.AddChunk(CodeCompletionString::CK_SemiColon);
+return *Builder.TakeString();
+  };
+  computeSignature(MakeCCS(), /*CompletingPattern=*/false);
+  EXPECT_EQ(Snippet, " ${1:name} = ${2:target};");
+
+  // When completing a pattern, the last placeholder holds the cursor position.
+  computeSignature(MakeCCS(), /*CompletingPattern=*/true);
+  EXPECT_EQ(Snippet, " ${1:name} = ${0:target};");
+}
+
 TEST_F(CompletionStringTest, IgnoreInformativeQualifier) {
   Builder.AddTypedTextChunk("X");
   Builder.AddInformativeChunk("info ok");
Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -2382,6 +2382,28 @@
   EXPECT_THAT(C, ElementsAre(SnippetSuffix("${1:(unsigned int)}")));
 }
 
+TEST(CompletionTest, CursorInSnippets) {
+  clangd::CodeCompleteOptions Options;
+  Options.EnableSnippets = true;
+  auto Results = completions(
+  R"cpp(
+void while_foo(int a, int b);
+void test() {
+  whil^
+})cpp",
+  /*IndexSymbols=*/{}, Options);
+
+  // Last placeholder in code patterns should be $0 to put the cursor there.
+  EXPECT_THAT(
+  Results.Completions,
+  Contains(AllOf(Named("while"),
+ SnippetSuffix("(${1:condition}){${0:statements}\n}";
+  // However, snippets for functions must *not* end with $0.
+  EXPECT_THAT(Results.Completions,
+  Contains(AllOf(Named("while_foo"),
+ SnippetSuffix("(${1:int a}, ${2:int b})";
+}
+
 TEST(CompletionTest, WorksWithNullType) {
   auto R = completions(R"cpp(
 int main() {
Index: clang-tools-extra/clangd/CodeCompletionStrings.h
===
--- clang-tools-extra/clangd/CodeCompletionStrings.h
+++ clang-tools-extra/clangd/CodeCompletionStrings.h
@@ -38,12 +38,16 @@
 /// Formats the signature for an item, as a display string and snippet.
 /// e.g. for const_reference std::vector::at(size_type) const, this returns:
 ///   *Signature = "(size_type) const"
-///   *Snippet = "(${0:size_type})"
+///   *Snippet = "(${1:size_type})"
 /// If set, RequiredQualifiers is the text that must be typed before the name.
 /// e.g "Base::" when calling a base class member function that's hidden.
+///
+/// When \p CompletingPattern is true, the last placeholder will be of the form
+/// ${0:…}, indicating the cursor should stay there.
 void getSignature(const CodeCompletionString &CCS, std::string *Signature,
   std::string *Snippet,
-  std::string *RequiredQualifiers = nullptr);
+  std::string *RequiredQualifiers = nullptr,
+  bool CompletingPattern = false);
 
 /// Assembles formatted documentation for a completion result. This includes
 /// documentation comments and other relevant information like annotations.
Index: clang-tools-extra/clangd/CodeCompletionStrings.cpp
===
--- clang-tools-extra/clangd/CodeCompleti

[PATCH] D62389: [clangd] Place cursor better after completing patterns

2019-05-27 Thread Ilya Biryukov via Phabricator via cfe-commits

ilya-biryukov added a comment.

In D62389#1518030 , @hokein wrote:

> How about adding one more test in CodeCompleteTests.cpp, we already have a 
> snippet test there, and it seems more straightforward.


Done!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62389



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

[PATCH] D62484: [ASTImporter] Added visibility context check for EnumDecl.

2019-05-27 Thread Balázs Kéri via Phabricator via cfe-commits

balazske created this revision.
Herald added subscribers: cfe-commits, gamesh411, Szelethus, dkrupp.
Herald added a reviewer: martong.
Herald added a reviewer: a.sidorin.
Herald added a reviewer: shafik.
Herald added a project: clang.

ASTImporter makes now difference between enums with same name in different 
translation
units if these are not visible outside.
("Scoped enums" are not handled yet.)


Repository:
  rC Clang

https://reviews.llvm.org/D62484

Files:
  lib/AST/ASTImporter.cpp
  unittests/AST/ASTImporterVisibilityTest.cpp

Index: unittests/AST/ASTImporterVisibilityTest.cpp
===
--- unittests/AST/ASTImporterVisibilityTest.cpp
+++ unittests/AST/ASTImporterVisibilityTest.cpp
@@ -35,6 +35,10 @@
   using DeclTy = CXXRecordDecl;
   BindableMatcher operator()() { return cxxRecordDecl(hasName("X")); }
 };
+struct GetEnumPattern {
+  using DeclTy = EnumDecl;
+  BindableMatcher operator()() { return enumDecl(hasName("E")); }
+};
 
 // Values for the value-parameterized test fixtures.
 // FunctionDecl:
@@ -48,6 +52,9 @@
 // CXXRecordDecl:
 const auto *ExternC = "class X;";
 const auto *AnonC = "namespace { class X; }";
+// EnumDecl:
+const auto *ExternE = "enum E {};";
+const auto *AnonE = "namespace { enum E {}; }";
 
 // First value in tuple: Compile options.
 // Second value in tuple: Source code to be used in the test.
@@ -183,10 +190,53 @@
 else
   EXPECT_FALSE(ToD1->getPreviousDecl());
   }
+
+  void TypedTest_ImportAfterWithMerge() {
+TranslationUnitDecl *ToTu = getToTuDecl(getCode0(), Lang_CXX);
+TranslationUnitDecl *FromTu = getTuDecl(getCode1(), Lang_CXX, "input1.cc");
+
+auto *ToF0 = FirstDeclMatcher().match(ToTu, getPattern());
+auto *FromF1 = FirstDeclMatcher().match(FromTu, getPattern());
+
+auto *ToF1 = Import(FromF1, Lang_CXX);
+
+ASSERT_TRUE(ToF0);
+ASSERT_TRUE(ToF1);
+
+if (shouldBeLinked())
+  EXPECT_EQ(ToF0, ToF1);
+else
+  EXPECT_NE(ToF0, ToF1);
+
+// We expect no (ODR) warning during the import.
+EXPECT_EQ(0u, ToTu->getASTContext().getDiagnostics().getNumWarnings());
+  }
+
+  void TypedTest_ImportAfterImportWithMerge() {
+TranslationUnitDecl *FromTu0 = getTuDecl(getCode0(), Lang_CXX, "input0.cc");
+TranslationUnitDecl *FromTu1 = getTuDecl(getCode1(), Lang_CXX, "input1.cc");
+auto *FromF0 = FirstDeclMatcher().match(FromTu0, getPattern());
+auto *FromF1 = FirstDeclMatcher().match(FromTu1, getPattern());
+auto *ToF0 = Import(FromF0, Lang_CXX);
+auto *ToF1 = Import(FromF1, Lang_CXX);
+ASSERT_TRUE(ToF0);
+ASSERT_TRUE(ToF1);
+if (shouldBeLinked())
+  EXPECT_EQ(ToF0, ToF1);
+else
+  EXPECT_NE(ToF0, ToF1);
+
+// We expect no (ODR) warning during the import.
+EXPECT_EQ(0u, ToF0->getTranslationUnitDecl()
+  ->getASTContext()
+  .getDiagnostics()
+  .getNumWarnings());
+  }
 };
 using ImportFunctionsVisibility = ImportVisibility;
 using ImportVariablesVisibility = ImportVisibility;
 using ImportClassesVisibility = ImportVisibility;
+using ImportEnumsVisibility = ImportVisibility;
 
 // FunctionDecl.
 TEST_P(ImportFunctionsVisibility, ImportAfter) {
@@ -209,6 +259,13 @@
 TEST_P(ImportClassesVisibility, ImportAfterImport) {
   TypedTest_ImportAfterImport();
 }
+// EnumDecl.
+TEST_P(ImportEnumsVisibility, ImportAfter) {
+  TypedTest_ImportAfterWithMerge();
+}
+TEST_P(ImportEnumsVisibility, ImportAfterImport) {
+  TypedTest_ImportAfterImportWithMerge();
+}
 
 const bool ExpectLink = true;
 const bool ExpectNotLink = false;
@@ -247,6 +304,14 @@
   std::make_tuple(ExternC, AnonC, ExpectNotLink),
   std::make_tuple(AnonC, ExternC, ExpectNotLink),
   std::make_tuple(AnonC, AnonC, ExpectNotLink))), );
+INSTANTIATE_TEST_CASE_P(
+ParameterizedTests, ImportEnumsVisibility,
+::testing::Combine(
+DefaultTestValuesForRunOptions,
+::testing::Values(std::make_tuple(ExternE, ExternE, ExpectLink),
+  std::make_tuple(ExternE, AnonE, ExpectNotLink),
+  std::make_tuple(AnonE, ExternE, ExpectNotLink),
+  std::make_tuple(AnonE, AnonE, ExpectNotLink))), );
 
 } // end namespace ast_matchers
 } // end namespace clang
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -2445,6 +2445,8 @@
   }
 
   if (auto *FoundEnum = dyn_cast(FoundDecl)) {
+if (!hasSameVisibilityContext(FoundEnum, D))
+  continue;
 if (IsStructuralMatch(D, FoundEnum))
   return Importer.MapImported(D, FoundEnum);
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60499: [ASTImporter] Various source location and range import fixes.

2019-05-27 Thread Balázs Kéri via Phabricator via cfe-commits

balazske updated this revision to Diff 201521.
balazske added a comment.

- Import BraceRange of EnumDecl.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60499

Files:
  lib/AST/ASTImporter.cpp

Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -2146,6 +2146,9 @@
   ExpectedSLoc BeginLocOrErr = import(D->getBeginLoc());
   if (!BeginLocOrErr)
 return BeginLocOrErr.takeError();
+  ExpectedSLoc RBraceLocOrErr = import(D->getRBraceLoc());
+  if (!RBraceLocOrErr)
+return RBraceLocOrErr.takeError();
 
   // Create the "to" namespace, if needed.
   NamespaceDecl *ToNamespace = MergeWithNamespace;
@@ -2155,6 +2158,7 @@
 *BeginLocOrErr, Loc, Name.getAsIdentifierInfo(),
 /*PrevDecl=*/nullptr))
   return ToNamespace;
+ToNamespace->setRBraceLoc(*RBraceLocOrErr);
 ToNamespace->setLexicalDeclContext(LexicalDC);
 LexicalDC->addDeclInternal(ToNamespace);
 
@@ -2452,9 +2456,10 @@
   SourceLocation ToBeginLoc;
   NestedNameSpecifierLoc ToQualifierLoc;
   QualType ToIntegerType;
-  if (auto Imp = importSeq(
-  D->getBeginLoc(), D->getQualifierLoc(), D->getIntegerType()))
-std::tie(ToBeginLoc, ToQualifierLoc, ToIntegerType) = *Imp;
+  SourceRange ToBraceRange;
+  if (auto Imp = importSeq(D->getBeginLoc(), D->getQualifierLoc(),
+   D->getIntegerType(), D->getBraceRange()))
+std::tie(ToBeginLoc, ToQualifierLoc, ToIntegerType, ToBraceRange) = *Imp;
   else
 return Imp.takeError();
 
@@ -2468,6 +2473,7 @@
 
   D2->setQualifierInfo(ToQualifierLoc);
   D2->setIntegerType(ToIntegerType);
+  D2->setBraceRange(ToBraceRange);
   D2->setAccess(D->getAccess());
   D2->setLexicalDeclContext(LexicalDC);
   LexicalDC->addDeclInternal(D2);
@@ -2697,6 +2703,10 @@
 LexicalDC->addDeclInternal(D2);
   }
 
+  if (auto BraceRangeOrErr = import(D->getBraceRange()))
+D2->setBraceRange(*BraceRangeOrErr);
+  else
+return BraceRangeOrErr.takeError();
   if (auto QualifierLocOrErr = import(D->getQualifierLoc()))
 D2->setQualifierInfo(*QualifierLocOrErr);
   else
@@ -5118,6 +5128,11 @@
 LexicalDC->addDeclInternal(D2);
   }
 
+  if (auto BraceRangeOrErr = import(D->getBraceRange()))
+D2->setBraceRange(*BraceRangeOrErr);
+  else
+return BraceRangeOrErr.takeError();
+
   // Import the qualifier, if any.
   if (auto LocOrErr = import(D->getQualifierLoc()))
 D2->setQualifierInfo(*LocOrErr);
@@ -6111,7 +6126,8 @@
   TemplateArgumentListInfo *ToResInfo = nullptr;
   if (E->hasExplicitTemplateArgs()) {
 if (Error Err =
-ImportTemplateArgumentListInfo(E->template_arguments(), ToTAInfo))
+ImportTemplateArgumentListInfo(E->getLAngleLoc(), E->getRAngleLoc(),
+   E->template_arguments(), ToTAInfo))
   return std::move(Err);
 ToResInfo = &ToTAInfo;
   }
@@ -7130,20 +7146,18 @@
 
 ExpectedStmt
 ASTNodeImporter::VisitDependentScopeDeclRefExpr(DependentScopeDeclRefExpr *E) {
-  auto Imp = importSeq(
-  E->getQualifierLoc(), E->getTemplateKeywordLoc(), E->getDeclName(),
-  E->getExprLoc(), E->getLAngleLoc(), E->getRAngleLoc());
+  auto Imp = importSeq(E->getQualifierLoc(), E->getTemplateKeywordLoc(),
+   E->getDeclName(), E->getNameInfo().getLoc(),
+   E->getLAngleLoc(), E->getRAngleLoc());
   if (!Imp)
 return Imp.takeError();
 
   NestedNameSpecifierLoc ToQualifierLoc;
-  SourceLocation ToTemplateKeywordLoc, ToExprLoc, ToLAngleLoc, ToRAngleLoc;
+  SourceLocation ToTemplateKeywordLoc, ToNameLoc, ToLAngleLoc, ToRAngleLoc;
   DeclarationName ToDeclName;
-  std::tie(
-  ToQualifierLoc, ToTemplateKeywordLoc, ToDeclName, ToExprLoc,
-  ToLAngleLoc, ToRAngleLoc) = *Imp;
-
-  DeclarationNameInfo ToNameInfo(ToDeclName, ToExprLoc);
+  std::tie(ToQualifierLoc, ToTemplateKeywordLoc, ToDeclName, ToNameLoc,
+   ToLAngleLoc, ToRAngleLoc) = *Imp;
+  DeclarationNameInfo ToNameInfo(ToDeclName, ToNameLoc);
   if (Error Err = ImportDeclarationNameLoc(E->getNameInfo(), ToNameInfo))
 return std::move(Err);
 
@@ -7208,7 +7222,7 @@
 else
   return ToDOrErr.takeError();
 
-  if (E->hasExplicitTemplateArgs() && E->getTemplateKeywordLoc().isValid()) {
+  if (E->hasExplicitTemplateArgs()) {
 TemplateArgumentListInfo ToTAInfo;
 if (Error Err = ImportTemplateArgumentListInfo(
 E->getLAngleLoc(), E->getRAngleLoc(), E->template_arguments(),
@@ -7262,8 +7276,9 @@
   TemplateArgumentListInfo ToTAInfo;
   TemplateArgumentListInfo *ResInfo = nullptr;
   if (E->hasExplicitTemplateArgs()) {
-if (Error Err =
-ImportTemplateArgumentListInfo(E->template_arguments(), ToTAInfo))
+TemplateArgumentListInfo FromTAInfo;
+E->copyTemplateArgumentsInto(FromTAInfo);
+if (Error Err = ImportTemplateArgumentListInfo(FromTAInfo, ToTAInfo))

[PATCH] D57795: [RISCV] Add FreeBSD targets

2019-05-27 Thread Ed Maste via Phabricator via cfe-commits

emaste added inline comments.



Comment at: lib/Basic/Targets.cpp:379
   return new LinuxTargetInfo(Triple, Opts);
-return new RISCV32TargetInfo(Triple, Opts);
+case llvm::Triple::FreeBSD:
+  return new FreeBSDTargetInfo(Triple, Opts);

My usual pedantic point: let's start these off in alpha order for consistency 
when additional cases are added


Repository:
  rC Clang

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

https://reviews.llvm.org/D57795



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

[PATCH] D62389: [clangd] Place cursor better after completing patterns

2019-05-27 Thread Haojian Wu via Phabricator via cfe-commits

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62389



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

r361766 - When dumping the AST to JSON, dump the argument name to a sizeof pack expression.

2019-05-27 Thread Aaron Ballman via cfe-commits

Author: aaronballman
Date: Mon May 27 07:17:32 2019
New Revision: 361766

URL: http://llvm.org/viewvc/llvm-project?rev=361766&view=rev
Log:
When dumping the AST to JSON, dump the argument name to a sizeof pack 
expression.

Modified:
cfe/trunk/include/clang/AST/JSONNodeDumper.h
cfe/trunk/lib/AST/JSONNodeDumper.cpp
cfe/trunk/test/AST/ast-dump-expr-json.cpp

Modified: cfe/trunk/include/clang/AST/JSONNodeDumper.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/JSONNodeDumper.h?rev=361766&r1=361765&r2=361766&view=diff
==
--- cfe/trunk/include/clang/AST/JSONNodeDumper.h (original)
+++ cfe/trunk/include/clang/AST/JSONNodeDumper.h Mon May 27 07:17:32 2019
@@ -244,6 +244,7 @@ public:
   void VisitImplicitCastExpr(const ImplicitCastExpr *ICE);
   void VisitCallExpr(const CallExpr *CE);
   void VisitUnaryExprOrTypeTraitExpr(const UnaryExprOrTypeTraitExpr *TTE);
+  void VisitSizeOfPackExpr(const SizeOfPackExpr *SOPE);
   void VisitUnresolvedLookupExpr(const UnresolvedLookupExpr *ULE);
   void VisitAddrLabelExpr(const AddrLabelExpr *ALE);
 

Modified: cfe/trunk/lib/AST/JSONNodeDumper.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/JSONNodeDumper.cpp?rev=361766&r1=361765&r2=361766&view=diff
==
--- cfe/trunk/lib/AST/JSONNodeDumper.cpp (original)
+++ cfe/trunk/lib/AST/JSONNodeDumper.cpp Mon May 27 07:17:32 2019
@@ -894,6 +894,10 @@ void JSONNodeDumper::VisitUnaryExprOrTyp
 JOS.attribute("argType", createQualType(TTE->getArgumentType()));
 }
 
+void JSONNodeDumper::VisitSizeOfPackExpr(const SizeOfPackExpr *SOPE) {
+  VisitNamedDecl(SOPE->getPack());
+}
+
 void JSONNodeDumper::VisitUnresolvedLookupExpr(
 const UnresolvedLookupExpr *ULE) {
   JOS.attribute("usesADL", ULE->requiresADL());

Modified: cfe/trunk/test/AST/ast-dump-expr-json.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/AST/ast-dump-expr-json.cpp?rev=361766&r1=361765&r2=361766&view=diff
==
--- cfe/trunk/test/AST/ast-dump-expr-json.cpp (original)
+++ cfe/trunk/test/AST/ast-dump-expr-json.cpp Mon May 27 07:17:32 2019
@@ -1505,7 +1505,8 @@ void TestNonADLCall3() {
 // CHECK-NEXT:"type": {
 // CHECK-NEXT: "qualType": "unsigned long"
 // CHECK-NEXT:},
-// CHECK-NEXT:"valueCategory": "rvalue"
+// CHECK-NEXT:"valueCategory": "rvalue",
+// CHECK-NEXT:"name": "Ts"
 // CHECK-NEXT:   },
 // CHECK-NEXT:   {
 // CHECK-NEXT:"id": "0x{{.*}}",


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

[PATCH] D60455: [SYCL] Implement SYCL device code outlining

2019-05-27 Thread Alexey Bader via Phabricator via cfe-commits

bader added a comment.

In D60455#1513499 , @Anastasia wrote:

> //**Design question**//: since you are not aware what functions are to be run 
> on a device while parsing them, at what point do you plan to diagnose the 
> invalid behavior from the standard C++ i.e. using function pointers in kernel 
> code is likely to cause issues?


We are going to use DeviceDiagBuilder and related infrastructure implemented in 
Clang to diagnose device side code errors in OpenMP/CUDA modes.
More details are in the comments here:
https://clang.llvm.org/doxygen/classclang_1_1Sema_1_1DeviceDiagBuilder.html#details

> Also do you need to outline the data structures too? For example classes used 
> on device are not allowed to have virtual function.

Yes. This restriction is already implemented in our code base on GitHub.




Comment at: clang/include/clang/Basic/Attr.td:1017
+  let LangOpts = [SYCL];
+  let Documentation = [Undocumented];
+}

Anastasia wrote:
> Anastasia wrote:
> > Fznamznon wrote:
> > > bader wrote:
> > > > Anastasia wrote:
> > > > > Ok, I thought the earlier request was not to add undocumented 
> > > > > attributes with the spelling?
> > > > > 
> > > > > Also did `__kernel` attribute not work at the end?
> > > > > 
> > > > > I can't quite get where the current disconnect comes from but I find 
> > > > > it extremely unhelpful.
> > > > Hi @Anastasia, let me try to help.
> > > > 
> > > > > Ok, I thought the earlier request was not to add undocumented 
> > > > > attributes with the spelling?
> > > > 
> > > > Right. @Fznamznon, could you document `sycl_kernel` attribute, please?
> > > > 
> > > > > Also did __kernel attribute not work at the end?
> > > > 
> > > > Maria left a comment with the summary of our experiment: 
> > > > https://reviews.llvm.org/D60455#1472705. There is a link to pull 
> > > > request, where @keryell and @agozillon expressed preference to have 
> > > > separate SYCL attributes. Let me copy their feedback here:
> > > > 
> > > > @keryell :
> > > > 
> > > > > Thank you for the experiment.
> > > > > That looks like a straight forward change.
> > > > > The interesting part is that it does not expose any advantage from 
> > > > > reusing OpenCL __kernel marker So I am not more convinced that it 
> > > > > is the way to go, because we would use any other keyword or attribute 
> > > > > and it would be the same...
> > > > > 
> > > > 
> > > > @agozillon :
> > > > 
> > > > > Just my two cents, I think a separation of concerns and having 
> > > > > separate attributes will simplify things long-term.
> > > > > 
> > > > > While possibly similar just now, the SYCL specification is evolving 
> > > > > and may end up targeting more than just OpenCL. So the semantics of 
> > > > > the attributes may end up being quite different, even if at the 
> > > > > moment the SYCL attribute is there mostly just to mark something for 
> > > > > outlining.
> > > > > 
> > > > > If it doesn't then the case for refactoring and merging them in a 
> > > > > future patch could be brought up again.
> > > > 
> > > > To summarize: we don't have good arguments to justify re-use of OpenCL 
> > > > `__kernel` keyword for SYCL mode requested by @aaron.ballman here 
> > > > https://reviews.llvm.org/D60455#1469150.
> > > > 
> > > > > I can't quite get where the current disconnect comes from but I find 
> > > > > it extremely unhelpful.
> > > > 
> > > > Let me know how I can help here.
> > > > 
> > > > Additional note. I've submitted initial version of SYCL compiler design 
> > > > document to the GItHub: 
> > > > https://github.com/intel/llvm/blob/sycl/sycl/doc/SYCL_compiler_and_runtime_design.md.
> > > >  Please, take a look and let me know if you have questions.
> > > >> Ok, I thought the earlier request was not to add undocumented 
> > > >> attributes with the spelling?
> > > > Right. @Fznamznon, could you document sycl_kernel attribute, please?
> > > 
> > > Do we really need add documentation for attribute which is not presented 
> > > in SYCL spec and used for internal implementation details only because it 
> > > has spelling?
> > > 
> > > Ok, I thought the earlier request was not to add undocumented 
> > > attributes with the spelling?
> > > 
> > > Right. @Fznamznon, could you document sycl_kernel attribute, please?
> > > 
> > > Do we really need add documentation for attribute which is not presented 
> > > in SYCL spec and used for internal implementation details only because it 
> > > has spelling?
> > 
> > 
> > 
> > You are adding an attribute that is exposed to the programmers that use 
> > clang to compile their code, so unless you come up with some way to reject 
> > it in the non-toolchain mode it has to be documented. And for clang it will 
> > become "hidden" SYCL dialect so absolutely not different to `__kernel`.
> > 
> > Another aspect to consider is that clang used `TypePrinter` in diagnostics 
> > and even though printing

r361767 - When dumping the AST to JSON, dump the declared name of a MemberExpr operand.

2019-05-27 Thread Aaron Ballman via cfe-commits

Author: aaronballman
Date: Mon May 27 07:25:04 2019
New Revision: 361767

URL: http://llvm.org/viewvc/llvm-project?rev=361767&view=rev
Log:
When dumping the AST to JSON, dump the declared name of a MemberExpr operand.

Modified:
cfe/trunk/lib/AST/JSONNodeDumper.cpp
cfe/trunk/test/AST/ast-dump-expr-json.c
cfe/trunk/test/AST/ast-dump-expr-json.cpp
cfe/trunk/test/AST/ast-dump-stmt-json.cpp

Modified: cfe/trunk/lib/AST/JSONNodeDumper.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/JSONNodeDumper.cpp?rev=361767&r1=361766&r2=361767&view=diff
==
--- cfe/trunk/lib/AST/JSONNodeDumper.cpp (original)
+++ cfe/trunk/lib/AST/JSONNodeDumper.cpp Mon May 27 07:25:04 2019
@@ -829,9 +829,10 @@ void JSONNodeDumper::VisitCompoundAssign
 void JSONNodeDumper::VisitMemberExpr(const MemberExpr *ME) {
   // Note, we always write this Boolean field because the information it 
conveys
   // is critical to understanding the AST node.
+  ValueDecl *VD = ME->getMemberDecl();
+  JOS.attribute("name", VD && VD->getDeclName() ? VD->getNameAsString() : "");
   JOS.attribute("isArrow", ME->isArrow());
-  JOS.attribute("referencedMemberDecl",
-createPointerRepresentation(ME->getMemberDecl()));
+  JOS.attribute("referencedMemberDecl", createPointerRepresentation(VD));
 }
 
 void JSONNodeDumper::VisitCXXNewExpr(const CXXNewExpr *NE) {

Modified: cfe/trunk/test/AST/ast-dump-expr-json.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/AST/ast-dump-expr-json.c?rev=361767&r1=361766&r2=361767&view=diff
==
--- cfe/trunk/test/AST/ast-dump-expr-json.c (original)
+++ cfe/trunk/test/AST/ast-dump-expr-json.c Mon May 27 07:25:04 2019
@@ -4433,6 +4433,7 @@ void PrimaryExpressions(int a) {
 // CHECK-NEXT: "qualType": "int"
 // CHECK-NEXT:},
 // CHECK-NEXT:"valueCategory": "lvalue",
+// CHECK-NEXT:"name": "a",
 // CHECK-NEXT:"isArrow": false,
 // CHECK-NEXT:"referencedMemberDecl": "0x{{.*}}",
 // CHECK-NEXT:"inner": [
@@ -4510,6 +4511,7 @@ void PrimaryExpressions(int a) {
 // CHECK-NEXT: "qualType": "int"
 // CHECK-NEXT:},
 // CHECK-NEXT:"valueCategory": "lvalue",
+// CHECK-NEXT:"name": "a",
 // CHECK-NEXT:"isArrow": true,
 // CHECK-NEXT:"referencedMemberDecl": "0x{{.*}}",
 // CHECK-NEXT:"inner": [

Modified: cfe/trunk/test/AST/ast-dump-expr-json.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/AST/ast-dump-expr-json.cpp?rev=361767&r1=361766&r2=361767&view=diff
==
--- cfe/trunk/test/AST/ast-dump-expr-json.cpp (original)
+++ cfe/trunk/test/AST/ast-dump-expr-json.cpp Mon May 27 07:25:04 2019
@@ -2584,6 +2584,7 @@ void TestNonADLCall3() {
 // CHECK-NEXT: "qualType": ""
 // CHECK-NEXT:},
 // CHECK-NEXT:"valueCategory": "rvalue",
+// CHECK-NEXT:"name": "func",
 // CHECK-NEXT:"isArrow": false,
 // CHECK-NEXT:"referencedMemberDecl": "0x{{.*}}",
 // CHECK-NEXT:"inner": [
@@ -2679,6 +2680,7 @@ void TestNonADLCall3() {
 // CHECK-NEXT: "qualType": ""
 // CHECK-NEXT:},
 // CHECK-NEXT:"valueCategory": "rvalue",
+// CHECK-NEXT:"name": "func",
 // CHECK-NEXT:"isArrow": true,
 // CHECK-NEXT:"referencedMemberDecl": "0x{{.*}}",
 // CHECK-NEXT:"inner": [
@@ -2798,6 +2800,7 @@ void TestNonADLCall3() {
 // CHECK-NEXT: "qualType": ""
 // CHECK-NEXT:},
 // CHECK-NEXT:"valueCategory": "rvalue",
+// CHECK-NEXT:"name": "foo",
 // CHECK-NEXT:"isArrow": true,
 // CHECK-NEXT:"referencedMemberDecl": "0x{{.*}}",
 // CHECK-NEXT:"inner": [
@@ -2896,6 +2899,7 @@ void TestNonADLCall3() {
 // CHECK-NEXT: "qualType": ""
 // CHECK-NEXT:},
 // CHECK-NEXT:"valueCategory": "rvalue",
+// CHECK-NEXT:"name": "foo",
 // CHECK-NEXT:"isArrow": false,
 // CHECK-NEXT:"referencedMemberDecl": "0x{{.*}}",
 // CHECK-NEXT:"inner": [
@@ -2970,6 +2974,7 @@ void TestNonADLCall3() {
 // CHECK-NEXT: "qualType": ""
 // CHECK-NEXT:},
 // CHECK-NEXT:"valueCategory": "rvalue",
+// CHECK-NEXT:"name": "~S",
 // CHECK-NEXT:"isArrow": true,
 // CHECK-NEXT:"referencedMemberDecl": "0x{{.*}}",
 // CHECK-NEXT:"inner": [
@@ -3067,6 +3072,7 @@ void TestNonADLCall3() {
 // CHECK-NEXT: "qualType": ""
 // CHECK-NEXT:},
 // CHECK-NEXT:"valueCategory": "rvalue",
+// CHECK-NEXT:"name": "~S",
 // CHECK-NEXT:"isArrow": false,
 // CHECK-NEXT:"referencedMemberDecl": "0x{{.*}}",
 // CHECK-NEXT:"inner": [
@@ -3141,6 +3147,7 @@ void TestNonADLCall3() {
 // CHECK-NEXT: "qualType": ""
 // CHECK-NEXT:

r361768 - When dumping the AST to JSON, dump whether a function is variadic or not.

2019-05-27 Thread Aaron Ballman via cfe-commits

Author: aaronballman
Date: Mon May 27 07:29:10 2019
New Revision: 361768

URL: http://llvm.org/viewvc/llvm-project?rev=361768&view=rev
Log:
When dumping the AST to JSON, dump whether a function is variadic or not.

Modified:
cfe/trunk/lib/AST/JSONNodeDumper.cpp
cfe/trunk/test/AST/ast-dump-expr-json.cpp

Modified: cfe/trunk/lib/AST/JSONNodeDumper.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/JSONNodeDumper.cpp?rev=361768&r1=361767&r2=361768&view=diff
==
--- cfe/trunk/lib/AST/JSONNodeDumper.cpp (original)
+++ cfe/trunk/lib/AST/JSONNodeDumper.cpp Mon May 27 07:29:10 2019
@@ -572,6 +572,8 @@ void JSONNodeDumper::VisitFunctionDecl(c
   attributeOnlyIfTrue("pure", FD->isPure());
   attributeOnlyIfTrue("explicitlyDeleted", FD->isDeletedAsWritten());
   attributeOnlyIfTrue("constexpr", FD->isConstexpr());
+  attributeOnlyIfTrue("variadic", FD->isVariadic());
+
   if (FD->isDefaulted())
 JOS.attribute("explicitlyDefaulted",
   FD->isDeleted() ? "deleted" : "default");

Modified: cfe/trunk/test/AST/ast-dump-expr-json.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/AST/ast-dump-expr-json.cpp?rev=361768&r1=361767&r2=361768&view=diff
==
--- cfe/trunk/test/AST/ast-dump-expr-json.cpp (original)
+++ cfe/trunk/test/AST/ast-dump-expr-json.cpp Mon May 27 07:29:10 2019
@@ -4574,6 +4574,7 @@ void TestNonADLCall3() {
 // CHECK-NEXT: "qualType": "auto (int, ...) const"
 // CHECK-NEXT:},
 // CHECK-NEXT:"inline": true,
+// CHECK-NEXT:"variadic": true,
 // CHECK-NEXT:"inner": [
 // CHECK-NEXT: {
 // CHECK-NEXT:  "id": "0x{{.*}}",
@@ -4673,6 +4674,7 @@ void TestNonADLCall3() {
 // CHECK-NEXT:},
 // CHECK-NEXT:"storageClass": "static",
 // CHECK-NEXT:"inline": true,
+// CHECK-NEXT:"variadic": true,
 // CHECK-NEXT:"inner": [
 // CHECK-NEXT: {
 // CHECK-NEXT:  "id": "0x{{.*}}",


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

r361769 - When dumping the AST to JSON, dump the type information from a typeid expression with a type operand.

2019-05-27 Thread Aaron Ballman via cfe-commits

Author: aaronballman
Date: Mon May 27 07:34:31 2019
New Revision: 361769

URL: http://llvm.org/viewvc/llvm-project?rev=361769&view=rev
Log:
When dumping the AST to JSON, dump the type information from a typeid 
expression with a type operand.

Modified:
cfe/trunk/include/clang/AST/JSONNodeDumper.h
cfe/trunk/lib/AST/ASTDumper.cpp
cfe/trunk/lib/AST/JSONNodeDumper.cpp
cfe/trunk/test/AST/ast-dump-expr-json.cpp

Modified: cfe/trunk/include/clang/AST/JSONNodeDumper.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/JSONNodeDumper.h?rev=361769&r1=361768&r2=361769&view=diff
==
--- cfe/trunk/include/clang/AST/JSONNodeDumper.h (original)
+++ cfe/trunk/include/clang/AST/JSONNodeDumper.h Mon May 27 07:34:31 2019
@@ -121,6 +121,7 @@ class JSONNodeDumper
   friend class JSONDumper;
 
   const SourceManager &SM;
+  ASTContext& Ctx;
   PrintingPolicy PrintPolicy;
   const comments::CommandTraits *Traits;
 
@@ -172,11 +173,11 @@ class JSONNodeDumper
   StringRef getCommentCommandName(unsigned CommandID) const;
 
 public:
-  JSONNodeDumper(raw_ostream &OS, const SourceManager &SrcMgr,
+  JSONNodeDumper(raw_ostream &OS, const SourceManager &SrcMgr, ASTContext &Ctx,
  const PrintingPolicy &PrintPolicy,
  const comments::CommandTraits *Traits)
-  : NodeStreamer(OS), SM(SrcMgr), PrintPolicy(PrintPolicy), Traits(Traits) 
{
-  }
+  : NodeStreamer(OS), SM(SrcMgr), Ctx(Ctx), PrintPolicy(PrintPolicy),
+Traits(Traits) {}
 
   void Visit(const Attr *A);
   void Visit(const Stmt *Node);
@@ -247,6 +248,7 @@ public:
   void VisitSizeOfPackExpr(const SizeOfPackExpr *SOPE);
   void VisitUnresolvedLookupExpr(const UnresolvedLookupExpr *ULE);
   void VisitAddrLabelExpr(const AddrLabelExpr *ALE);
+  void VisitCXXTypeidExpr(const CXXTypeidExpr *CTE);
 
   void VisitIntegerLiteral(const IntegerLiteral *IL);
   void VisitCharacterLiteral(const CharacterLiteral *CL);
@@ -360,10 +362,10 @@ class JSONDumper : public ASTNodeTravers
   }
 
 public:
-  JSONDumper(raw_ostream &OS, const SourceManager &SrcMgr,
+  JSONDumper(raw_ostream &OS, const SourceManager &SrcMgr, ASTContext &Ctx,
  const PrintingPolicy &PrintPolicy,
  const comments::CommandTraits *Traits)
-  : NodeDumper(OS, SrcMgr, PrintPolicy, Traits) {}
+  : NodeDumper(OS, SrcMgr, Ctx, PrintPolicy, Traits) {}
 
   JSONNodeDumper &doGetNodeDelegate() { return NodeDumper; }
 

Modified: cfe/trunk/lib/AST/ASTDumper.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTDumper.cpp?rev=361769&r1=361768&r2=361769&view=diff
==
--- cfe/trunk/lib/AST/ASTDumper.cpp (original)
+++ cfe/trunk/lib/AST/ASTDumper.cpp Mon May 27 07:34:31 2019
@@ -180,11 +180,11 @@ LLVM_DUMP_METHOD void Decl::dump() const
 
 LLVM_DUMP_METHOD void Decl::dump(raw_ostream &OS, bool Deserialize,
  ASTDumpOutputFormat Format) const {
-  const ASTContext &Ctx = getASTContext();
+  ASTContext &Ctx = getASTContext();
   const SourceManager &SM = Ctx.getSourceManager();
 
   if (ADOF_JSON == Format) {
-JSONDumper P(OS, SM, Ctx.getPrintingPolicy(),
+JSONDumper P(OS, SM, Ctx, Ctx.getPrintingPolicy(),
  &Ctx.getCommentCommandTraits());
 (void)Deserialize; // FIXME?
 P.Visit(this);

Modified: cfe/trunk/lib/AST/JSONNodeDumper.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/JSONNodeDumper.cpp?rev=361769&r1=361768&r2=361769&view=diff
==
--- cfe/trunk/lib/AST/JSONNodeDumper.cpp (original)
+++ cfe/trunk/lib/AST/JSONNodeDumper.cpp Mon May 27 07:34:31 2019
@@ -917,6 +917,16 @@ void JSONNodeDumper::VisitAddrLabelExpr(
   JOS.attribute("labelDeclId", createPointerRepresentation(ALE->getLabel()));
 }
 
+void JSONNodeDumper::VisitCXXTypeidExpr(const CXXTypeidExpr *CTE) {
+  if (CTE->isTypeOperand()) {
+QualType Adjusted = CTE->getTypeOperand(Ctx);
+QualType Unadjusted = CTE->getTypeOperandSourceInfo()->getType();
+JOS.attribute("typeArg", createQualType(Unadjusted));
+if (Adjusted != Unadjusted)
+  JOS.attribute("adjustedTypeArg", createQualType(Adjusted));
+  }
+}
+
 void JSONNodeDumper::VisitIntegerLiteral(const IntegerLiteral *IL) {
   JOS.attribute("value",
 IL->getValue().toString(

Modified: cfe/trunk/test/AST/ast-dump-expr-json.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/AST/ast-dump-expr-json.cpp?rev=361769&r1=361768&r2=361769&view=diff
==
--- cfe/trunk/test/AST/ast-dump-expr-json.cpp (original)
+++ cfe/trunk/test/AST/ast-dump-expr-json.cpp Mon May 27 07:34:31 2019
@@ -3447,7 +3447,10 @@ void TestNonADLCall3() {
 // CHECK-NEXT:  "type": {
 // CHECK-NEXT:   "qualType": "const std::

[PATCH] D62404: [clang-tidy] Fix null pointer dereference in readability-identifier-naming

2019-05-27 Thread Mads Ravn via Phabricator via cfe-commits

madsravn added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:804
+if (const auto *Typedef = TypePtr->getAs()) {
+  addUsage(NamingCheckFailures, Typedef->getDecl(),
+   Value->getSourceRange());

hokein wrote:
> We are risky of dereferencing a nullptr, as the `TypePtr` can be null now.
Are we not checking for null in the line where `TypePtr` gets declared? Or am I 
misunderstanding? 


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

https://reviews.llvm.org/D62404



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

[PATCH] D62404: [clang-tidy] Fix null pointer dereference in readability-identifier-naming

2019-05-27 Thread Aaron Ballman via Phabricator via cfe-commits

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

LGTM!




Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:804
+if (const auto *Typedef = TypePtr->getAs()) {
+  addUsage(NamingCheckFailures, Typedef->getDecl(),
+   Value->getSourceRange());

madsravn wrote:
> hokein wrote:
> > We are risky of dereferencing a nullptr, as the `TypePtr` can be null now.
> Are we not checking for null in the line where `TypePtr` gets declared? Or am 
> I misunderstanding? 
I don't see a null pointer dereference here for `TypePtr`; I think the code is 
fine as-is.


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

https://reviews.llvm.org/D62404



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

[PATCH] D60543: [clang] Update isDerivedFrom to support Objective-C classes 🔍

2019-05-27 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added inline comments.



Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:2638
+  // Check if the node is a C++ struct/union/class.
+  if (const auto *RecordDecl = dyn_cast(&Node))
+return Finder->classIsDerivedFrom(RecordDecl, Base, Builder);

I'd prefer this be named `RD` so that it doesn't conflict with the name of the 
`RecordDecl` type.



Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:2642-2649
+  if (const auto *InterfaceDecl = dyn_cast(&Node)) {
+// Check if any of the superclasses of the class match.
+for (const ObjCInterfaceDecl *SuperClass = InterfaceDecl->getSuperClass();
+ SuperClass != nullptr; SuperClass = SuperClass->getSuperClass()) {
+  if (Base.matches(*SuperClass, Finder, Builder))
+return true;
+}

This should probably be done similar to how `classIsDerivedFrom()` works. For 
instance, there's some type alias matching logic that this does not replicate, 
but we probably want.



Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:2667-2672
+  if (const auto *InterfaceDecl = dyn_cast(&Node)) {
+return Matcher(M).matches(*InterfaceDecl, Finder,
+ Builder);
+  }
+
+  llvm_unreachable("Not a valid polymorphic type");

How about:
```
const auto *InterfaceDecl = cast(&Node);
return Matcher...
```
We can rely on `cast<>` to assert if the node is of an unexpected type. Same 
suggestions below.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60543



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

[PATCH] D62470: Clarify when fix-it hints on warnings are appropriate

2019-05-27 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added a comment.

Thank you for working on this!




Comment at: docs/InternalsManual.rst:426-428
+* Fix-it hints on a warning should only clarify the meaning of the existing
+  code, not change it. Examples for such hints are added parentheses in cases
+  of counter-intuitive precedence, when they clarify the order of operations.

How about:
```
Fix-it hints on  a warning must not change the meaning of code, only clarify 
it. For example, adding parentheses in case of counterintuitive precedence to 
clarify the order of operations.
```


Repository:
  rC Clang

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

https://reviews.llvm.org/D62470



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

[PATCH] D62435: Add Attribute NoThrow as an Exception Specifier Type

2019-05-27 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added inline comments.



Comment at: clang/include/clang-c/Index.h:227
+  /**
+   * The cursor has a declspec(nothrow) exception specification.
+   */

`__declspec(nothrow)`



Comment at: clang/include/clang/Basic/ExceptionSpecificationType.h:25
   EST_MSAny,///< Microsoft throw(...) extension
+  EST_NoThrow,  ///< Microsoft declspec(nothrow) extension
   EST_BasicNoexcept,///< noexcept

`__declspec(nothrow)`



Comment at: clang/lib/Sema/SemaType.cpp:6968
+
+// MSVC Ignores nothrow for exception specification if it is in conflict.
+if (Proto->hasExceptionSpec())

Ignores -> ignores



Comment at: clang/lib/Sema/SemaType.cpp:6970
+if (Proto->hasExceptionSpec())
+  return true;
+

I think we should diagnose that the `nothrow` is ignored in this case to let 
users know they've done something in conflict and what wins.


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

https://reviews.llvm.org/D62435



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

[PATCH] D62437: [clang-tidy] Splits fuchsia-default-arguments

2019-05-27 Thread Aaron Ballman via Phabricator via cfe-commits

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

It seems that the functional changes are missing from the patch -- all I see 
are formatting changes; have I missed something?




Comment at: 
clang-tools-extra/clang-tidy/fuchsia/DefaultArgumentsCallsCheck.cpp:24-25
   const auto *S = Result.Nodes.getNodeAs("stmt");
-  if (S == nullptr) return;
+  if (S == nullptr)
+return;
 

Given that the only change in the file is a formatting change, I would revert 
this. Feel free to commit separately as an NFC commit if you plan to do future 
work in the file.



Comment at: 
clang-tools-extra/clang-tidy/fuchsia/DefaultArgumentsDeclarationsCheck.h:1
+//===--- DefaultArgumentsDeclarationsCheck.h - clang-tidy --*- 
C++ -*-===//
+//

Eugene.Zelenko wrote:
> Please narrow to 80 symbols.
This should similarly be a separate NFC commit.


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

https://reviews.llvm.org/D62437



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

[PATCH] D62487: [clang] Respect TerseOutput when printing lambdas

2019-05-27 Thread Kadir Cetinkaya via Phabricator via cfe-commits

kadircet created this revision.
kadircet added reviewers: ilya-biryukov, hokein, sammccall.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
kadircet added a child revision: D61497: [clangd] Introduce a structured hover 
response.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D62487

Files:
  clang/lib/AST/StmtPrinter.cpp
  clang/unittests/AST/StmtPrinterTest.cpp


Index: clang/unittests/AST/StmtPrinterTest.cpp
===
--- clang/unittests/AST/StmtPrinterTest.cpp
+++ clang/unittests/AST/StmtPrinterTest.cpp
@@ -231,3 +231,17 @@
   ASSERT_TRUE(PrintedStmtObjCMatches(ObjCSource, returnStmt().bind("id"),
  "return self->ivar;\n"));
 }
+
+TEST(StmtPrinter, TerseOutputWithLambdas) {
+  const char *CPPSource = "auto lamb = []{ return 0; };";
+
+  // body is printed when TerseOutput is off(default).
+  ASSERT_TRUE(PrintedStmtCXXMatches(StdVer::CXX11, CPPSource,
+lambdaExpr(anything()).bind("id"),
+"[] {\nreturn 0;\n}"));
+
+  // body not printed when TerseOutput is on.
+  ASSERT_TRUE(PrintedStmtCXXMatches(
+  StdVer::CXX11, CPPSource, lambdaExpr(anything()).bind("id"), "[] {}",
+  PolicyAdjusterType([](PrintingPolicy &PP) { PP.TerseOutput = true; })));
+}
Index: clang/lib/AST/StmtPrinter.cpp
===
--- clang/lib/AST/StmtPrinter.cpp
+++ clang/lib/AST/StmtPrinter.cpp
@@ -1950,7 +1950,10 @@
 
   // Print the body.
   OS << ' ';
-  PrintRawCompoundStmt(Node->getBody());
+  if (Policy.TerseOutput)
+OS << "{}";
+  else
+PrintRawCompoundStmt(Node->getBody());
 }
 
 void StmtPrinter::VisitCXXScalarValueInitExpr(CXXScalarValueInitExpr *Node) {


Index: clang/unittests/AST/StmtPrinterTest.cpp
===
--- clang/unittests/AST/StmtPrinterTest.cpp
+++ clang/unittests/AST/StmtPrinterTest.cpp
@@ -231,3 +231,17 @@
   ASSERT_TRUE(PrintedStmtObjCMatches(ObjCSource, returnStmt().bind("id"),
  "return self->ivar;\n"));
 }
+
+TEST(StmtPrinter, TerseOutputWithLambdas) {
+  const char *CPPSource = "auto lamb = []{ return 0; };";
+
+  // body is printed when TerseOutput is off(default).
+  ASSERT_TRUE(PrintedStmtCXXMatches(StdVer::CXX11, CPPSource,
+lambdaExpr(anything()).bind("id"),
+"[] {\nreturn 0;\n}"));
+
+  // body not printed when TerseOutput is on.
+  ASSERT_TRUE(PrintedStmtCXXMatches(
+  StdVer::CXX11, CPPSource, lambdaExpr(anything()).bind("id"), "[] {}",
+  PolicyAdjusterType([](PrintingPolicy &PP) { PP.TerseOutput = true; })));
+}
Index: clang/lib/AST/StmtPrinter.cpp
===
--- clang/lib/AST/StmtPrinter.cpp
+++ clang/lib/AST/StmtPrinter.cpp
@@ -1950,7 +1950,10 @@
 
   // Print the body.
   OS << ' ';
-  PrintRawCompoundStmt(Node->getBody());
+  if (Policy.TerseOutput)
+OS << "{}";
+  else
+PrintRawCompoundStmt(Node->getBody());
 }
 
 void StmtPrinter::VisitCXXScalarValueInitExpr(CXXScalarValueInitExpr *Node) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61497: [clangd] Introduce a structured hover response

2019-05-27 Thread Kadir Cetinkaya via Phabricator via cfe-commits

kadircet updated this revision to Diff 201536.
kadircet marked 10 inline comments as done.
kadircet added a comment.

- Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61497

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/XRefs.h
  clang-tools-extra/clangd/test/hover.test
  clang-tools-extra/clangd/unittests/TestTU.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp

Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -9,6 +9,7 @@
 #include "ClangdUnit.h"
 #include "Compiler.h"
 #include "Matchers.h"
+#include "Protocol.h"
 #include "SyncAPI.h"
 #include "TestFS.h"
 #include "TestTU.h"
@@ -16,6 +17,7 @@
 #include "index/FileIndex.h"
 #include "index/SymbolCollector.h"
 #include "clang/Index/IndexingAction.h"
+#include "llvm/ADT/None.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include "gmock/gmock.h"
@@ -558,6 +560,304 @@
   HeaderNotInPreambleAnnotations.range(;
 }
 
+TEST(Hover, Structured) {
+  struct {
+const char *const Code;
+const std::function ExpectedBuilder;
+  } Cases[] = {
+  // Global scope.
+  {R"cpp(
+  // Best foo ever.
+  void [[fo^o]]() {}
+  )cpp",
+   [](HoverInfo &HI) {
+ HI.NamespaceScope = "";
+ HI.Name = "foo";
+ HI.Kind = SymbolKind::Function;
+ HI.Documentation = "Best foo ever.";
+ HI.Definition = "void foo()";
+ HI.ReturnType = "void";
+ HI.Type = "void()";
+ HI.Parameters.emplace();
+   }},
+  // Inside namespace
+  {R"cpp(
+  namespace ns1 { namespace ns2 {
+/// Best foo ever.
+void [[fo^o]]() {}
+  }}
+  )cpp",
+   [](HoverInfo &HI) {
+ HI.NamespaceScope = "ns1::ns2";
+ HI.Name = "foo";
+ HI.Kind = SymbolKind::Function;
+ HI.Documentation = "Best foo ever.";
+ HI.Definition = "void foo()";
+ HI.ReturnType = "void";
+ HI.Type = "void()";
+ HI.Parameters.emplace();
+   }},
+  // Field
+  {R"cpp(
+  namespace ns1 { namespace ns2 {
+struct Foo {
+  int [[b^ar]];
+};
+  }}
+  )cpp",
+   [](HoverInfo &HI) {
+ HI.NamespaceScope = "ns1::ns2";
+ HI.LocalScope = "Foo";
+ HI.Name = "bar";
+ HI.Kind = SymbolKind::Field;
+ HI.Definition = "int bar";
+ HI.Type = "int";
+   }},
+  // Local to class method.
+  {R"cpp(
+  namespace ns1 { namespace ns2 {
+struct Foo {
+  void foo() {
+int [[b^ar]];
+  }
+};
+  }}
+  )cpp",
+   [](HoverInfo &HI) {
+ HI.NamespaceScope = "ns1::ns2";
+ HI.LocalScope = "Foo::foo";
+ HI.Name = "bar";
+ HI.Kind = SymbolKind::Variable;
+ HI.Definition = "int bar";
+ HI.Type = "int";
+   }},
+  // Anon namespace and local scope.
+  {R"cpp(
+  namespace ns1 { namespace {
+struct {
+  int [[b^ar]];
+} T;
+  }}
+  )cpp",
+   [](HoverInfo &HI) {
+ HI.NamespaceScope = "ns1::(anonymous)";
+ HI.LocalScope = "(anonymous struct)";
+ HI.Name = "bar";
+ HI.Kind = SymbolKind::Field;
+ HI.Definition = "int bar";
+ HI.Type = "int";
+   }},
+  // Variable with template type
+  {R"cpp(
+  template  class Foo {};
+  Foo [[fo^o]];
+  )cpp",
+   [](HoverInfo &HI) {
+ HI.NamespaceScope = "";
+ HI.Name = "foo";
+ HI.Kind = SymbolKind::Variable;
+ HI.Definition = "Foo foo";
+ HI.Type = "Foo";
+   }},
+  // Implicit template instantiation
+  {R"cpp(
+  template  class vector{};
+  [[vec^tor]] foo;
+  )cpp",
+   [](HoverInfo &HI) {
+ HI.NamespaceScope = "";
+ HI.Name = "vector";
+ HI.Kind = SymbolKind::Class;
+ HI.Definition = "template  class vector {}";
+ HI.TemplateParameters = {
+ {std::string("typename"), std::string("T"), llvm::None},
+ };
+   }},
+  // Class template
+  {R"cpp(
+  template  class C,
+typename = char,
+int = 0,
+bool Q = false,
+class... Ts> class Foo {};
+  template  class T>
+  [[F^oo]] foo;
+  )cpp",
+

[PATCH] D61497: [clangd] Introduce a structured hover response

2019-05-27 Thread Kadir Cetinkaya via Phabricator via cfe-commits

kadircet marked 10 inline comments as done.
kadircet added inline comments.



Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:567
+const HoverInfo Expected;
+  } // namespace
+  Cases[] = {

sammccall wrote:
> namespace?!
> 
> might be a clang-format bug?
I must have messed the brackets at some point :D



Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:574
+)cpp",
+   {/*NamespaceScope=*/std::string(""),
+/*LocalScope=*/std::string(""),

sammccall wrote:
> using the constructor/aggregate init here has a couple of downsides:
>  - vebosity: you need to list every field, even those that are not set for 
> this case (TemplateParameters), or are misleading (SymRange is none)
>  - brittleness: it makes it hard to change the struct at all
>  - readability: the /*foo=*/ comments aren't bad, but not ideal either
> 
> Using field-by-field initialization would be better I think, though it's a 
> bit awkward here.
> 
> but e.g. you could make the member std::function ExpectedBuilder, 
> and write
> 
> ```
> 
> [&](HoverInfo &Expected) {
>   Expected.Name = "foo";
>   Expected.Kind = SymbolKind::Function;
>   ...
> }
> ```
yeah that looks a lot better, thanks!



Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:592
+)cpp",
+   {/*NamespaceScope=*/std::string("ns1::ns2"),
+/*LocalScope=*/std::string(""),

sammccall wrote:
> I think this should be "ns1::ns2::", as we use scope internally.
> This means we can simply concatenate parts to form a qname.
> 
> For the current rendering, it's easy to strip ::
should it also be "::" for global namespace ? which would also result in 
prefixing any symbol in global namespace with "::" when printing.



Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:599
+/*Definition=*/"void foo()",
+/*Type=*/llvm::None,
+/*ReturnType=*/std::string("void"),

sammccall wrote:
> It would be nice to add `void()` or `void (&)()` or so if it's easy.
> This is what `:YcmCompleter GetType` would show
just put the type without any parameter names, but I am not sure whether users 
would want that. I believe people find current hover behavior a lot more useful 
then just showing type(which is done by libclang)



Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:782
+/*Definition=*/
+"auto lamb = [&bar](int T, bool B) -> bool {\nreturn T && B && "
+"bar;\n}",

sammccall wrote:
> is it easy to omit the body?
yes, sent out D62487



Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:951
   )cpp",
-  "Declared in namespace ns1\n\nstruct MyClass {}",
+  "Declared in ns1\n\nstruct MyClass {}",
   },

sammccall wrote:
> note that if you want to it's easy to keep "Declared in namespace" - the 
> LocalScope is empty.
> (Though we can't provide this info for non-namespace containers)
I believe it doesn't make sense to add only one, so leaving it until we hear 
from users that they need the kind information


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61497



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

[PATCH] D61790: [C++20] add Basic consteval specifier

2019-05-27 Thread Tyker via Phabricator via cfe-commits

Tyker updated this revision to Diff 201535.
Tyker marked 13 inline comments as done.
Tyker retitled this revision from "[C++20] add consteval specifier" to "[C++20] 
add Basic consteval specifier".
Tyker edited the summary of this revision.
Tyker added a comment.
Herald added a subscriber: eraman.

previous revision was spitted in multiple part as requested
consteval specific semantic and code-gen have been removed from the patch.
other parts:

- fixing semantic checking in constant context : https://reviews.llvm.org/D62009
- adding storage for APValue in ConstantExpr : https://reviews.llvm.org/D62399

some remarks weren't fixed because the changes are not present in this patch.


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

https://reviews.llvm.org/D61790

Files:
  clang/include/clang/AST/Decl.h
  clang/include/clang/AST/DeclBase.h
  clang/include/clang/AST/DeclCXX.h
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/Specifiers.h
  clang/include/clang/Basic/TokenKinds.def
  clang/include/clang/Sema/DeclSpec.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/Decl.cpp
  clang/lib/AST/DeclCXX.cpp
  clang/lib/AST/DeclPrinter.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/Parse/ParseCXXInlineMethods.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseExprCXX.cpp
  clang/lib/Sema/DeclSpec.cpp
  clang/lib/Sema/SemaCoroutine.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaLambda.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/lib/Sema/SemaType.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/test/SemaCXX/cxx2a-compat.cpp
  clang/test/SemaCXX/cxx2a-consteval.cpp

Index: clang/test/SemaCXX/cxx2a-consteval.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/cxx2a-consteval.cpp
@@ -0,0 +1,49 @@
+// RUN: %clang_cc1 -std=c++2a -fsyntax-only %s -verify
+
+namespace basic_sema {
+
+consteval int f1(int i) {
+  return i;
+}
+
+consteval constexpr int f2(int i) { 
+  //expected-error@-1 {{cannot combine}}
+  return i;
+}
+
+constexpr auto l_eval = [](int i) consteval {
+
+  return i;
+};
+
+constexpr consteval int f3(int i) {
+  //expected-error@-1 {{cannot combine}}
+  return i;
+}
+
+struct A {
+  consteval int f1(int i) const {
+return i;
+  }
+  consteval A(int i);
+  consteval A() = default;
+  consteval ~A() = default; // expected-error {{destructor cannot be marked consteval}}
+};
+
+consteval struct B {}; // expected-error {{struct cannot be marked consteval}}
+
+consteval typedef B b; // expected-error {{typedef cannot be consteval}}
+
+consteval int redecl() {return 0;} // expected-note {{previous declaration is here}}
+constexpr int redecl() {return 0;} // expected-error {{constexpr declaration of 'redecl' follows consteval declaration}}
+
+consteval int i = 0; // expected-error {{consteval can only be used in function declarations}}
+
+consteval int; // expected-error {{consteval can only be used in function declarations}}
+
+consteval int f1() {} // expected-error {{no return statement in consteval function}}
+consteval int main() {
+  return 0;
+}
+
+}
Index: clang/test/SemaCXX/cxx2a-compat.cpp
===
--- clang/test/SemaCXX/cxx2a-compat.cpp
+++ clang/test/SemaCXX/cxx2a-compat.cpp
@@ -56,4 +56,13 @@
 #if !defined(__cpp_conditional_explicit) || __cpp_conditional_explicit != 201806L
 #error "the feature test macro __cpp_conditional_explicit isn't correct"
 #endif
+#endif
+
+auto l = []() consteval {};
+int consteval();
+#if __cplusplus <= 201703L
+// expected-warning@-3 {{'consteval' is a keyword in C++2a}}
+// expected-error@-4 {{expected body of lambda expression}}
+#else
+// expected-error@-5 {{expected unqualified-id}}
 #endif
\ No newline at end of file
Index: clang/lib/Serialization/ASTReaderDecl.cpp
===
--- clang/lib/Serialization/ASTReaderDecl.cpp
+++ clang/lib/Serialization/ASTReaderDecl.cpp
@@ -868,7 +868,7 @@
   FD->setDefaulted(Record.readInt());
   FD->setExplicitlyDefaulted(Record.readInt());
   FD->setHasImplicitReturnZero(Record.readInt());
-  FD->setConstexpr(Record.readInt());
+  FD->setConstexprKind(static_cast(Record.readInt()));
   FD->setUsesSEHTry(Record.readInt());
   FD->setHasSkippedBody(Record.readInt());
   FD->setIsMultiVersion(Record.readInt());
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -11301,7 +11301,7 @@
   Class, E->getIntroducerRange(), NewCallOpTSI,
   E->getCallOperator()->getEndLoc(),

[PATCH] D61497: [clangd] Introduce a structured hover response

2019-05-27 Thread Sam McCall via Phabricator via cfe-commits

sammccall added a comment.

Still LG

I forgot to mention - it would be nice to clang-format the definition, what do 
you think?




Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:592
+)cpp",
+   {/*NamespaceScope=*/std::string("ns1::ns2"),
+/*LocalScope=*/std::string(""),

kadircet wrote:
> sammccall wrote:
> > I think this should be "ns1::ns2::", as we use scope internally.
> > This means we can simply concatenate parts to form a qname.
> > 
> > For the current rendering, it's easy to strip ::
> should it also be "::" for global namespace ? which would also result in 
> prefixing any symbol in global namespace with "::" when printing.
We tend to use empty string for global namespace, and explicitly add it where 
we need it.
Using :: for global causes as many problems as it solves (e.g. in C).



Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:599
+/*Definition=*/"void foo()",
+/*Type=*/llvm::None,
+/*ReturnType=*/std::string("void"),

kadircet wrote:
> sammccall wrote:
> > It would be nice to add `void()` or `void (&)()` or so if it's easy.
> > This is what `:YcmCompleter GetType` would show
> just put the type without any parameter names, but I am not sure whether 
> users would want that. I believe people find current hover behavior a lot 
> more useful then just showing type(which is done by libclang)
I don't think we should include it in the actual hover, but we can handle that 
in rendering (if it's a Function, don't print the type).
It does seem odd not to include it in the structured API, but up to you.

I thought specifically the `:GetType` might still be useful, but maybe not.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61497



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

[PATCH] D61790: [C++20] add Basic consteval specifier

2019-05-27 Thread Tyker via Phabricator via cfe-commits

Tyker added inline comments.



Comment at: clang/lib/Sema/SemaChecking.cpp:10471
 
+  bool IsConstantContext = S.ExprEvalContexts.back().isConstantEvaluated();
+

rsmith wrote:
> Are the changes to this file really specific to `consteval` evaluation? They 
> look more general than that; I think this would fix the evaluation of 
> `std::is_constant_evaluated()` in any constant-evaluated context that we 
> analyzed. Can this be split out of this patch into a separate change?
this can be splited in and other patch. https://reviews.llvm.org/D62009.



Comment at: clang/lib/Sema/SemaExpr.cpp:5720-5769
+bool Sema::CheckInvalidConstevalCall(FunctionDecl *FDecl, SourceLocation Loc,
+ ArrayRef Args, Expr *This) {
+  bool Failed = false;
+  if (FDecl && FDecl->isConsteval() &&
+  !ExprEvalContexts.back().isConstantEvaluated()) {
+
+SmallVector Diags;

rsmith wrote:
> This checking doesn't make sense: we should be checking that the invocation 
> as a whole is a constant expression, not whether the arguments (evaluated in 
> isolation) are constant expressions. This check should be applied to the 
> `Expr` representing the call, and should wrap it in a `ConstantExpr` holding 
> the evaluated value if evaluation succeeds.
the idea behind this check was "if the function satisfies the rules of 
constexpr and all it arguments can be constant evaluated. this function call 
can be constant evaluated". this was a easy way of checking that the call could 
be evaluated without evaluating it.

this will be added in later patch after https://reviews.llvm.org/D62399.



Comment at: clang/lib/Sema/SemaExpr.cpp:13417-13435
+bool Sema::CheckInvalidConstevalTakeAddress(Expr *Input) {
+  if (Input) {
+if (auto *DeclRef = dyn_cast(Input->IgnoreParens())) {
+  if (auto *Function = dyn_cast(DeclRef->getDecl())) {
+if (Function->isConsteval()) {
+  const char *DeclName = Function->isOverloadedOperator()
+ ? "consteval operator"

rsmith wrote:
> This isn't correct: you can take the address of a `consteval` function within 
> an immediate invocation. In general, we need to delay this checking until 
> we've finished processing any potentially-enclosing immediate invocations. 
> Trying to capture all of the places where we might form a reference to a 
> `consteval` function name without forming an immediate invocation is also 
> fragile: you'll miss some, and as Clang is extended, there'll be more cases 
> introduced that you miss.
> 
> Here's how I was intending to handle this:
> 
>  * Change `Sema::MarkFunctionReferenced` to take an `Expr*` instead of a 
> location, and update all of its callers. (We'll need a separate function for 
> marking destructors referenced, because destructor calls typically don't have 
> expressions, but destructors can't be `consteval` so that's OK.)
>  * In `Sema::MarkFunctionReferenced`, if `Func` is a `consteval` function and 
> our current context is not a `consteval` function, add it to a list on the 
> `ExpressionEvaluationContext`.
>  * When forming an immediate invocation, walk all of its subexpressions and 
> remove them all from the list on the `ExpressionEvaluationContext`
>  * When popping the `ExpressionEvaluationContext`, diagnose any remaining 
> expressions in its list.
> 
> (The first step should be done in a separate patch to keep the diffs smaller 
> and easier to review.)
delayed to later patch.


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

https://reviews.llvm.org/D61790



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

r361771 - [clang] Respect TerseOutput when printing lambdas

2019-05-27 Thread Kadir Cetinkaya via cfe-commits

Author: kadircet
Date: Mon May 27 09:20:45 2019
New Revision: 361771

URL: http://llvm.org/viewvc/llvm-project?rev=361771&view=rev
Log:
[clang] Respect TerseOutput when printing lambdas

Reviewers: ilya-biryukov, hokein, sammccall

Subscribers: cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/lib/AST/StmtPrinter.cpp
cfe/trunk/unittests/AST/StmtPrinterTest.cpp

Modified: cfe/trunk/lib/AST/StmtPrinter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/StmtPrinter.cpp?rev=361771&r1=361770&r2=361771&view=diff
==
--- cfe/trunk/lib/AST/StmtPrinter.cpp (original)
+++ cfe/trunk/lib/AST/StmtPrinter.cpp Mon May 27 09:20:45 2019
@@ -1950,7 +1950,10 @@ void StmtPrinter::VisitLambdaExpr(Lambda
 
   // Print the body.
   OS << ' ';
-  PrintRawCompoundStmt(Node->getBody());
+  if (Policy.TerseOutput)
+OS << "{}";
+  else
+PrintRawCompoundStmt(Node->getBody());
 }
 
 void StmtPrinter::VisitCXXScalarValueInitExpr(CXXScalarValueInitExpr *Node) {

Modified: cfe/trunk/unittests/AST/StmtPrinterTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/AST/StmtPrinterTest.cpp?rev=361771&r1=361770&r2=361771&view=diff
==
--- cfe/trunk/unittests/AST/StmtPrinterTest.cpp (original)
+++ cfe/trunk/unittests/AST/StmtPrinterTest.cpp Mon May 27 09:20:45 2019
@@ -231,3 +231,17 @@ class A {
   ASSERT_TRUE(PrintedStmtObjCMatches(ObjCSource, returnStmt().bind("id"),
  "return self->ivar;\n"));
 }
+
+TEST(StmtPrinter, TerseOutputWithLambdas) {
+  const char *CPPSource = "auto lamb = []{ return 0; };";
+
+  // body is printed when TerseOutput is off(default).
+  ASSERT_TRUE(PrintedStmtCXXMatches(StdVer::CXX11, CPPSource,
+lambdaExpr(anything()).bind("id"),
+"[] {\nreturn 0;\n}"));
+
+  // body not printed when TerseOutput is on.
+  ASSERT_TRUE(PrintedStmtCXXMatches(
+  StdVer::CXX11, CPPSource, lambdaExpr(anything()).bind("id"), "[] {}",
+  PolicyAdjusterType([](PrintingPolicy &PP) { PP.TerseOutput = true; })));
+}


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

[PATCH] D62487: [clang] Respect TerseOutput when printing lambdas

2019-05-27 Thread Kadir Cetinkaya via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL361771: [clang] Respect TerseOutput when printing lambdas 
(authored by kadircet, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D62487?vs=201534&id=201540#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D62487

Files:
  cfe/trunk/lib/AST/StmtPrinter.cpp
  cfe/trunk/unittests/AST/StmtPrinterTest.cpp


Index: cfe/trunk/unittests/AST/StmtPrinterTest.cpp
===
--- cfe/trunk/unittests/AST/StmtPrinterTest.cpp
+++ cfe/trunk/unittests/AST/StmtPrinterTest.cpp
@@ -231,3 +231,17 @@
   ASSERT_TRUE(PrintedStmtObjCMatches(ObjCSource, returnStmt().bind("id"),
  "return self->ivar;\n"));
 }
+
+TEST(StmtPrinter, TerseOutputWithLambdas) {
+  const char *CPPSource = "auto lamb = []{ return 0; };";
+
+  // body is printed when TerseOutput is off(default).
+  ASSERT_TRUE(PrintedStmtCXXMatches(StdVer::CXX11, CPPSource,
+lambdaExpr(anything()).bind("id"),
+"[] {\nreturn 0;\n}"));
+
+  // body not printed when TerseOutput is on.
+  ASSERT_TRUE(PrintedStmtCXXMatches(
+  StdVer::CXX11, CPPSource, lambdaExpr(anything()).bind("id"), "[] {}",
+  PolicyAdjusterType([](PrintingPolicy &PP) { PP.TerseOutput = true; })));
+}
Index: cfe/trunk/lib/AST/StmtPrinter.cpp
===
--- cfe/trunk/lib/AST/StmtPrinter.cpp
+++ cfe/trunk/lib/AST/StmtPrinter.cpp
@@ -1950,7 +1950,10 @@
 
   // Print the body.
   OS << ' ';
-  PrintRawCompoundStmt(Node->getBody());
+  if (Policy.TerseOutput)
+OS << "{}";
+  else
+PrintRawCompoundStmt(Node->getBody());
 }
 
 void StmtPrinter::VisitCXXScalarValueInitExpr(CXXScalarValueInitExpr *Node) {


Index: cfe/trunk/unittests/AST/StmtPrinterTest.cpp
===
--- cfe/trunk/unittests/AST/StmtPrinterTest.cpp
+++ cfe/trunk/unittests/AST/StmtPrinterTest.cpp
@@ -231,3 +231,17 @@
   ASSERT_TRUE(PrintedStmtObjCMatches(ObjCSource, returnStmt().bind("id"),
  "return self->ivar;\n"));
 }
+
+TEST(StmtPrinter, TerseOutputWithLambdas) {
+  const char *CPPSource = "auto lamb = []{ return 0; };";
+
+  // body is printed when TerseOutput is off(default).
+  ASSERT_TRUE(PrintedStmtCXXMatches(StdVer::CXX11, CPPSource,
+lambdaExpr(anything()).bind("id"),
+"[] {\nreturn 0;\n}"));
+
+  // body not printed when TerseOutput is on.
+  ASSERT_TRUE(PrintedStmtCXXMatches(
+  StdVer::CXX11, CPPSource, lambdaExpr(anything()).bind("id"), "[] {}",
+  PolicyAdjusterType([](PrintingPolicy &PP) { PP.TerseOutput = true; })));
+}
Index: cfe/trunk/lib/AST/StmtPrinter.cpp
===
--- cfe/trunk/lib/AST/StmtPrinter.cpp
+++ cfe/trunk/lib/AST/StmtPrinter.cpp
@@ -1950,7 +1950,10 @@
 
   // Print the body.
   OS << ' ';
-  PrintRawCompoundStmt(Node->getBody());
+  if (Policy.TerseOutput)
+OS << "{}";
+  else
+PrintRawCompoundStmt(Node->getBody());
 }
 
 void StmtPrinter::VisitCXXScalarValueInitExpr(CXXScalarValueInitExpr *Node) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D62435: Add Attribute NoThrow as an Exception Specifier Type

2019-05-27 Thread Erich Keane via Phabricator via cfe-commits

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



Comment at: clang/lib/Sema/SemaType.cpp:6970
+if (Proto->hasExceptionSpec())
+  return true;
+

aaron.ballman wrote:
> I think we should diagnose that the `nothrow` is ignored in this case to let 
> users know they've done something in conflict and what wins.
I've been considering that over the weekend, and agree it is a good idea. I am 
on the fence about the when however. For example, should we warn with throw() 
or noexcept or noexcept(true)? In these cases, ignoring the attribute is the 
same as enforcing it.

My concern is that noexcept (expression-evaluating-to-true) gets ignored with a 
diagnostic, but has the same effect.

Did you have an opinion here?


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

https://reviews.llvm.org/D62435



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

[PATCH] D61497: [clangd] Introduce a structured hover response

2019-05-27 Thread Kadir Cetinkaya via Phabricator via cfe-commits

kadircet updated this revision to Diff 201543.
kadircet marked 7 inline comments as done.
kadircet added a comment.

- Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61497

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/XRefs.h
  clang-tools-extra/clangd/test/hover.test
  clang-tools-extra/clangd/unittests/TestTU.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp

Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -9,6 +9,7 @@
 #include "ClangdUnit.h"
 #include "Compiler.h"
 #include "Matchers.h"
+#include "Protocol.h"
 #include "SyncAPI.h"
 #include "TestFS.h"
 #include "TestTU.h"
@@ -16,6 +17,7 @@
 #include "index/FileIndex.h"
 #include "index/SymbolCollector.h"
 #include "clang/Index/IndexingAction.h"
+#include "llvm/ADT/None.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include "gmock/gmock.h"
@@ -558,6 +560,304 @@
   HeaderNotInPreambleAnnotations.range(;
 }
 
+TEST(Hover, Structured) {
+  struct {
+const char *const Code;
+const std::function ExpectedBuilder;
+  } Cases[] = {
+  // Global scope.
+  {R"cpp(
+  // Best foo ever.
+  void [[fo^o]]() {}
+  )cpp",
+   [](HoverInfo &HI) {
+ HI.NamespaceScope = "";
+ HI.Name = "foo";
+ HI.Kind = SymbolKind::Function;
+ HI.Documentation = "Best foo ever.";
+ HI.Definition = "void foo()";
+ HI.ReturnType = "void";
+ HI.Type = "void()";
+ HI.Parameters.emplace();
+   }},
+  // Inside namespace
+  {R"cpp(
+  namespace ns1 { namespace ns2 {
+/// Best foo ever.
+void [[fo^o]]() {}
+  }}
+  )cpp",
+   [](HoverInfo &HI) {
+ HI.NamespaceScope = "ns1::ns2::";
+ HI.Name = "foo";
+ HI.Kind = SymbolKind::Function;
+ HI.Documentation = "Best foo ever.";
+ HI.Definition = "void foo()";
+ HI.ReturnType = "void";
+ HI.Type = "void()";
+ HI.Parameters.emplace();
+   }},
+  // Field
+  {R"cpp(
+  namespace ns1 { namespace ns2 {
+struct Foo {
+  int [[b^ar]];
+};
+  }}
+  )cpp",
+   [](HoverInfo &HI) {
+ HI.NamespaceScope = "ns1::ns2::";
+ HI.LocalScope = "Foo::";
+ HI.Name = "bar";
+ HI.Kind = SymbolKind::Field;
+ HI.Definition = "int bar";
+ HI.Type = "int";
+   }},
+  // Local to class method.
+  {R"cpp(
+  namespace ns1 { namespace ns2 {
+struct Foo {
+  void foo() {
+int [[b^ar]];
+  }
+};
+  }}
+  )cpp",
+   [](HoverInfo &HI) {
+ HI.NamespaceScope = "ns1::ns2::";
+ HI.LocalScope = "Foo::foo::";
+ HI.Name = "bar";
+ HI.Kind = SymbolKind::Variable;
+ HI.Definition = "int bar";
+ HI.Type = "int";
+   }},
+  // Anon namespace and local scope.
+  {R"cpp(
+  namespace ns1 { namespace {
+struct {
+  int [[b^ar]];
+} T;
+  }}
+  )cpp",
+   [](HoverInfo &HI) {
+ HI.NamespaceScope = "ns1::(anonymous)::";
+ HI.LocalScope = "(anonymous struct)::";
+ HI.Name = "bar";
+ HI.Kind = SymbolKind::Field;
+ HI.Definition = "int bar";
+ HI.Type = "int";
+   }},
+  // Variable with template type
+  {R"cpp(
+  template  class Foo {};
+  Foo [[fo^o]];
+  )cpp",
+   [](HoverInfo &HI) {
+ HI.NamespaceScope = "";
+ HI.Name = "foo";
+ HI.Kind = SymbolKind::Variable;
+ HI.Definition = "Foo foo";
+ HI.Type = "Foo";
+   }},
+  // Implicit template instantiation
+  {R"cpp(
+  template  class vector{};
+  [[vec^tor]] foo;
+  )cpp",
+   [](HoverInfo &HI) {
+ HI.NamespaceScope = "";
+ HI.Name = "vector";
+ HI.Kind = SymbolKind::Class;
+ HI.Definition = "template  class vector {}";
+ HI.TemplateParameters = {
+ {std::string("typename"), std::string("T"), llvm::None},
+ };
+   }},
+  // Class template
+  {R"cpp(
+  template  class C,
+typename = char,
+int = 0,
+bool Q = false,
+class... Ts> class Foo {};
+  template  class T>
+  [[F^oo]] foo;
+

[PATCH] D62490: clang-cl: Fix mangling of catchable types with names longer than 4kiB

2019-05-27 Thread Nico Weber via Phabricator via cfe-commits

thakis created this revision.
thakis added a reviewer: rnk.

The mangling used to contain the MD5 name of both the RTTI type
descriptor and the name of the copy ctor in MSVC2013, but it changed
to just the former in 2015. It looks like it changed back to the old
mangling in VS2017 version 15.7 and onwards, including VS2019 (version
16.0). VS2017 version 15.0 still has the VS2015 mangling. Versions
between 15.0 and 15.7 are't on godbolt and I don't have them locally,
so I'm not sure where exactly it changed back.


https://reviews.llvm.org/D62490

Files:
  clang/include/clang/Basic/LangOptions.h
  clang/lib/AST/MicrosoftMangle.cpp
  clang/test/CodeGenCXX/mangle-ms-md5.cpp


Index: clang/test/CodeGenCXX/mangle-ms-md5.cpp
===
--- clang/test/CodeGenCXX/mangle-ms-md5.cpp
+++ clang/test/CodeGenCXX/mangle-ms-md5.cpp
@@ -9,3 +9,17 @@
 
yyy

[PATCH] D61487: [clang-tidy] Make the plugin honor NOLINT

2019-05-27 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik added a comment.

Ping :)


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D61487



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

[PATCH] D48116: [libclang] Allow skipping warnings from all included files

2019-05-27 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik added a comment.

Ping.


Repository:
  rC Clang

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

https://reviews.llvm.org/D48116



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

[PATCH] D62435: Add Attribute NoThrow as an Exception Specifier Type

2019-05-27 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added inline comments.



Comment at: clang/lib/Sema/SemaType.cpp:6970
+if (Proto->hasExceptionSpec())
+  return true;
+

erichkeane wrote:
> aaron.ballman wrote:
> > I think we should diagnose that the `nothrow` is ignored in this case to 
> > let users know they've done something in conflict and what wins.
> I've been considering that over the weekend, and agree it is a good idea. I 
> am on the fence about the when however. For example, should we warn with 
> throw() or noexcept or noexcept(true)? In these cases, ignoring the attribute 
> is the same as enforcing it.
> 
> My concern is that noexcept (expression-evaluating-to-true) gets ignored with 
> a diagnostic, but has the same effect.
> 
> Did you have an opinion here?
I think the metric should be: if there are conflicting attributes where we need 
to ignore one of them, we should always diagnose when the attributes specify 
conflicting semantics, but we don't have to always diagnose when the attributes 
specify the same semantics. In this case, e.g.,
```
__declspec(nothrow) void foo1() noexcept; // Don't bother warning
__declspec(nothrow) void foo2() noexcept(true); // Don't bother warning
__declspec(nothrow) void foo3() noexcept(false); // Warn
__declspec(nothrow) void foo4() noexcept(foo1()); // Warning is debatable
__declspec(nothrow) void foo5() noexcept(foo3()); // Warn
```


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

https://reviews.llvm.org/D62435



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

[PATCH] D62470: Clarify when fix-it hints on warnings are appropriate

2019-05-27 Thread Aaron Puchert via Phabricator via cfe-commits

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



Comment at: docs/InternalsManual.rst:426-428
+* Fix-it hints on a warning should only clarify the meaning of the existing
+  code, not change it. Examples for such hints are added parentheses in cases
+  of counter-intuitive precedence, when they clarify the order of operations.

aaron.ballman wrote:
> How about:
> ```
> Fix-it hints on  a warning must not change the meaning of code, only clarify 
> it. For example, adding parentheses in case of counterintuitive precedence to 
> clarify the order of operations.
> ```
> must not change the meaning of code, only clarify it
Wasn't sure about the order between “not change”/“only clarify”. If it sounds 
better to you, I'll take that.

> For example, adding parentheses in case of counterintuitive precedence to 
> clarify the order of operations.
Can we add a finite verb, like “For example, it would be fine to add 
parantheses ...”

Also, I would probably shorten “in case of counterintuitive precedence to 
clarify the order of operations” to “to clarify the precedence of operators.” 
So we have

//For example, it would be fine to add parantheses to clarify the precedence of 
operators.//


Repository:
  rC Clang

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

https://reviews.llvm.org/D62470



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

r361779 - [Preprocessor] Fix crash emitting note with framework location for "file not found" error.

2019-05-27 Thread Volodymyr Sapsai via cfe-commits

Author: vsapsai
Date: Mon May 27 12:15:30 2019
New Revision: 361779

URL: http://llvm.org/viewvc/llvm-project?rev=361779&view=rev
Log:
[Preprocessor] Fix crash emitting note with framework location for "file not 
found" error.

A filename can be remapped with a header map to point to a framework
header and we can find the corresponding framework without the header.
But if the original filename doesn't have a remapped framework name,
we'll fail to find its location and will dereference a null pointer
during diagnostics emission.

Fix by tracking remappings better and emit the note only if a framework
is found before any of the remappings.

rdar://problem/48883447

Reviewers: arphaman, erik.pilkington, jkorous

Reviewed By: arphaman

Subscribers: dexonsmith, cfe-commits

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

Added:
cfe/trunk/test/Preprocessor/Inputs/include-header-missing-in-framework/

cfe/trunk/test/Preprocessor/Inputs/include-header-missing-in-framework/TestFramework.hmap.json

cfe/trunk/test/Preprocessor/include-header-missing-in-framework-with-headermap.c
Modified:
cfe/trunk/include/clang/Lex/HeaderSearch.h
cfe/trunk/lib/Lex/HeaderSearch.cpp

Modified: cfe/trunk/include/clang/Lex/HeaderSearch.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/HeaderSearch.h?rev=361779&r1=361778&r2=361779&view=diff
==
--- cfe/trunk/include/clang/Lex/HeaderSearch.h (original)
+++ cfe/trunk/include/clang/Lex/HeaderSearch.h Mon May 27 12:15:30 2019
@@ -392,8 +392,9 @@ public:
   /// true.
   ///
   /// \param IsFrameworkFound If non-null, will be set to true if a framework 
is
-  /// found in any of searched SearchDirs. Doesn't guarantee the requested file
-  /// is found.
+  /// found in any of searched SearchDirs. Will be set to false if a framework
+  /// is found only through header maps. Doesn't guarantee the requested file 
is
+  /// found.
   const FileEntry *LookupFile(
   StringRef Filename, SourceLocation IncludeLoc, bool isAngled,
   const DirectoryLookup *FromDir, const DirectoryLookup *&CurDir,

Modified: cfe/trunk/lib/Lex/HeaderSearch.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/HeaderSearch.cpp?rev=361779&r1=361778&r2=361779&view=diff
==
--- cfe/trunk/lib/Lex/HeaderSearch.cpp (original)
+++ cfe/trunk/lib/Lex/HeaderSearch.cpp Mon May 27 12:15:30 2019
@@ -869,7 +869,10 @@ const FileEntry *HeaderSearch::LookupFil
 *IsMapped = true;
 }
 if (IsFrameworkFound)
-  *IsFrameworkFound |= IsFrameworkFoundInDir;
+  // Because we keep a filename remapped for subsequent search directory
+  // lookups, ignore IsFrameworkFoundInDir after the first remapping and 
not
+  // just for remapping in a current search directory.
+  *IsFrameworkFound |= (IsFrameworkFoundInDir && !CacheLookup.MappedName);
 if (!FE) continue;
 
 CurDir = &SearchDirs[i];

Added: 
cfe/trunk/test/Preprocessor/Inputs/include-header-missing-in-framework/TestFramework.hmap.json
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/Inputs/include-header-missing-in-framework/TestFramework.hmap.json?rev=361779&view=auto
==
--- 
cfe/trunk/test/Preprocessor/Inputs/include-header-missing-in-framework/TestFramework.hmap.json
 (added)
+++ 
cfe/trunk/test/Preprocessor/Inputs/include-header-missing-in-framework/TestFramework.hmap.json
 Mon May 27 12:15:30 2019
@@ -0,0 +1,7 @@
+{
+  "mappings" :
+{
+ "RemappedHeader.h" : "TestFramework/RemappedHeader.h",
+ "TestFramework/BeforeRemapping.h" : "TestFramework/AfterRemapping.h"
+}
+}

Added: 
cfe/trunk/test/Preprocessor/include-header-missing-in-framework-with-headermap.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/include-header-missing-in-framework-with-headermap.c?rev=361779&view=auto
==
--- 
cfe/trunk/test/Preprocessor/include-header-missing-in-framework-with-headermap.c
 (added)
+++ 
cfe/trunk/test/Preprocessor/include-header-missing-in-framework-with-headermap.c
 Mon May 27 12:15:30 2019
@@ -0,0 +1,20 @@
+// RUN: rm -f %t.hmap
+// RUN: %hmaptool write 
%S/Inputs/include-header-missing-in-framework/TestFramework.hmap.json %t.hmap
+// RUN: %clang_cc1 -fsyntax-only -F %S/Inputs -I %t.hmap -verify %s 
-DLATE_REMAPPING
+// RUN: %clang_cc1 -fsyntax-only -I %t.hmap -F %S/Inputs -verify %s
+
+// The test is similar to 'include-header-missing-in-framework.c' but covers
+// the case when a header is remapped to a framework-like path with a .hmap
+// file. And we can find the framework but not the header.
+
+#ifdef LATE_REMAPPING
+// Framework is found before remapping.
+#include 
+// expected-error@-1 {{'TestFramework/BeforeRemapping.h' file no

[PATCH] D61707: [Preprocessor] Fix crash emitting note with framework location for "file not found" error.

2019-05-27 Thread Volodymyr Sapsai via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rC361779: [Preprocessor] Fix crash emitting note with 
framework location for "file not… (authored by vsapsai, committed by ).
Herald added a project: clang.

Changed prior to commit:
  https://reviews.llvm.org/D61707?vs=200864&id=201559#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D61707

Files:
  include/clang/Lex/HeaderSearch.h
  lib/Lex/HeaderSearch.cpp
  
test/Preprocessor/Inputs/include-header-missing-in-framework/TestFramework.hmap.json
  test/Preprocessor/include-header-missing-in-framework-with-headermap.c


Index: include/clang/Lex/HeaderSearch.h
===
--- include/clang/Lex/HeaderSearch.h
+++ include/clang/Lex/HeaderSearch.h
@@ -392,8 +392,9 @@
   /// true.
   ///
   /// \param IsFrameworkFound If non-null, will be set to true if a framework 
is
-  /// found in any of searched SearchDirs. Doesn't guarantee the requested file
-  /// is found.
+  /// found in any of searched SearchDirs. Will be set to false if a framework
+  /// is found only through header maps. Doesn't guarantee the requested file 
is
+  /// found.
   const FileEntry *LookupFile(
   StringRef Filename, SourceLocation IncludeLoc, bool isAngled,
   const DirectoryLookup *FromDir, const DirectoryLookup *&CurDir,
Index: test/Preprocessor/include-header-missing-in-framework-with-headermap.c
===
--- test/Preprocessor/include-header-missing-in-framework-with-headermap.c
+++ test/Preprocessor/include-header-missing-in-framework-with-headermap.c
@@ -0,0 +1,20 @@
+// RUN: rm -f %t.hmap
+// RUN: %hmaptool write 
%S/Inputs/include-header-missing-in-framework/TestFramework.hmap.json %t.hmap
+// RUN: %clang_cc1 -fsyntax-only -F %S/Inputs -I %t.hmap -verify %s 
-DLATE_REMAPPING
+// RUN: %clang_cc1 -fsyntax-only -I %t.hmap -F %S/Inputs -verify %s
+
+// The test is similar to 'include-header-missing-in-framework.c' but covers
+// the case when a header is remapped to a framework-like path with a .hmap
+// file. And we can find the framework but not the header.
+
+#ifdef LATE_REMAPPING
+// Framework is found before remapping.
+#include 
+// expected-error@-1 {{'TestFramework/BeforeRemapping.h' file not found}}
+// expected-note@-2 {{did not find header 'BeforeRemapping.h' in framework 
'TestFramework' (loaded from}}
+
+#else
+// Framework is found after remapping.
+#include "RemappedHeader.h"
+// expected-error@-1 {{'RemappedHeader.h' file not found}}
+#endif // LATE_REMAPPING
Index: 
test/Preprocessor/Inputs/include-header-missing-in-framework/TestFramework.hmap.json
===
--- 
test/Preprocessor/Inputs/include-header-missing-in-framework/TestFramework.hmap.json
+++ 
test/Preprocessor/Inputs/include-header-missing-in-framework/TestFramework.hmap.json
@@ -0,0 +1,7 @@
+{
+  "mappings" :
+{
+ "RemappedHeader.h" : "TestFramework/RemappedHeader.h",
+ "TestFramework/BeforeRemapping.h" : "TestFramework/AfterRemapping.h"
+}
+}
Index: lib/Lex/HeaderSearch.cpp
===
--- lib/Lex/HeaderSearch.cpp
+++ lib/Lex/HeaderSearch.cpp
@@ -869,7 +869,10 @@
 *IsMapped = true;
 }
 if (IsFrameworkFound)
-  *IsFrameworkFound |= IsFrameworkFoundInDir;
+  // Because we keep a filename remapped for subsequent search directory
+  // lookups, ignore IsFrameworkFoundInDir after the first remapping and 
not
+  // just for remapping in a current search directory.
+  *IsFrameworkFound |= (IsFrameworkFoundInDir && !CacheLookup.MappedName);
 if (!FE) continue;
 
 CurDir = &SearchDirs[i];


Index: include/clang/Lex/HeaderSearch.h
===
--- include/clang/Lex/HeaderSearch.h
+++ include/clang/Lex/HeaderSearch.h
@@ -392,8 +392,9 @@
   /// true.
   ///
   /// \param IsFrameworkFound If non-null, will be set to true if a framework is
-  /// found in any of searched SearchDirs. Doesn't guarantee the requested file
-  /// is found.
+  /// found in any of searched SearchDirs. Will be set to false if a framework
+  /// is found only through header maps. Doesn't guarantee the requested file is
+  /// found.
   const FileEntry *LookupFile(
   StringRef Filename, SourceLocation IncludeLoc, bool isAngled,
   const DirectoryLookup *FromDir, const DirectoryLookup *&CurDir,
Index: test/Preprocessor/include-header-missing-in-framework-with-headermap.c
===
--- test/Preprocessor/include-header-missing-in-framework-with-headermap.c
+++ test/Preprocessor/include-header-missing-in-framework-with-headermap.c
@@ -0,0 +1,20 @@
+// RUN: rm -f %t.hmap
+// RUN: %hmaptool write %S/Inputs/include-header-missin

[PATCH] D61707: [Preprocessor] Fix crash emitting note with framework location for "file not found" error.

2019-05-27 Thread Volodymyr Sapsai via Phabricator via cfe-commits

vsapsai added a comment.

Thanks for the review, Alex.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61707



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

[PATCH] D62470: Clarify when fix-it hints on warnings are appropriate

2019-05-27 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added inline comments.



Comment at: docs/InternalsManual.rst:426-428
+* Fix-it hints on a warning should only clarify the meaning of the existing
+  code, not change it. Examples for such hints are added parentheses in cases
+  of counter-intuitive precedence, when they clarify the order of operations.

aaronpuchert wrote:
> aaron.ballman wrote:
> > How about:
> > ```
> > Fix-it hints on  a warning must not change the meaning of code, only 
> > clarify it. For example, adding parentheses in case of counterintuitive 
> > precedence to clarify the order of operations.
> > ```
> > must not change the meaning of code, only clarify it
> Wasn't sure about the order between “not change”/“only clarify”. If it sounds 
> better to you, I'll take that.
> 
> > For example, adding parentheses in case of counterintuitive precedence to 
> > clarify the order of operations.
> Can we add a finite verb, like “For example, it would be fine to add 
> parantheses ...”
> 
> Also, I would probably shorten “in case of counterintuitive precedence to 
> clarify the order of operations” to “to clarify the precedence of operators.” 
> So we have
> 
> //For example, it would be fine to add parantheses to clarify the precedence 
> of operators.//
> Wasn't sure about the order between “not change”/“only clarify”. If it sounds 
> better to you, I'll take that.

I think the most important bit is that it must not change code meaning, so 
that's why I suggest putting it first.

> For example, it would be fine to add parantheses to clarify the precedence of 
> operators.

I like it! Though I'd spell it parentheses instead. ;-)


Repository:
  rC Clang

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

https://reviews.llvm.org/D62470



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

[PATCH] D62469: [Driver] Change layout of per-target runtimes to resemble multiarch

2019-05-27 Thread Roland McGrath via Phabricator via cfe-commits

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

lgtm


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62469



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

[PATCH] D62470: Clarify when fix-it hints on warnings are appropriate

2019-05-27 Thread Aaron Puchert via Phabricator via cfe-commits

aaronpuchert marked 3 inline comments as done.
aaronpuchert added inline comments.



Comment at: docs/InternalsManual.rst:426-428
+* Fix-it hints on a warning should only clarify the meaning of the existing
+  code, not change it. Examples for such hints are added parentheses in cases
+  of counter-intuitive precedence, when they clarify the order of operations.

aaron.ballman wrote:
> aaronpuchert wrote:
> > aaron.ballman wrote:
> > > How about:
> > > ```
> > > Fix-it hints on  a warning must not change the meaning of code, only 
> > > clarify it. For example, adding parentheses in case of counterintuitive 
> > > precedence to clarify the order of operations.
> > > ```
> > > must not change the meaning of code, only clarify it
> > Wasn't sure about the order between “not change”/“only clarify”. If it 
> > sounds better to you, I'll take that.
> > 
> > > For example, adding parentheses in case of counterintuitive precedence to 
> > > clarify the order of operations.
> > Can we add a finite verb, like “For example, it would be fine to add 
> > parantheses ...”
> > 
> > Also, I would probably shorten “in case of counterintuitive precedence to 
> > clarify the order of operations” to “to clarify the precedence of 
> > operators.” So we have
> > 
> > //For example, it would be fine to add parantheses to clarify the 
> > precedence of operators.//
> > Wasn't sure about the order between “not change”/“only clarify”. If it 
> > sounds better to you, I'll take that.
> 
> I think the most important bit is that it must not change code meaning, so 
> that's why I suggest putting it first.
> 
> > For example, it would be fine to add parantheses to clarify the precedence 
> > of operators.
> 
> I like it! Though I'd spell it parentheses instead. ;-)
> Though I'd spell it parentheses instead. ;-)

There must have been a red squiggly line, but I've apparently decided to ignore 
it.


Repository:
  rC Clang

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

https://reviews.llvm.org/D62470



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

[PATCH] D62493: [Driver] Always use Unix-style paths in the Darwin driver

2019-05-27 Thread Louis Dionne via Phabricator via cfe-commits

ldionne created this revision.
ldionne added reviewers: jfb, rsmith.
Herald added subscribers: cfe-commits, dexonsmith, jkorous.
Herald added a project: clang.

Otherwise, the unit tests seem to be confused on Windows. Note that it
would in theory be better to always use the platform's path, however we
don't seem to have a good way of handling that with FileCheck right now.
This change makes the handling of paths on Darwin consistent with what
we do on Linux, and also with what we used to do (for several paths)
before the refactoring in r361278.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D62493

Files:
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/test/Driver/darwin-header-search-libcxx.cpp
  clang/test/Driver/darwin-header-search-libstdcxx.cpp
  clang/test/Driver/darwin-header-search-system.cpp

Index: clang/test/Driver/darwin-header-search-system.cpp
===
--- clang/test/Driver/darwin-header-search-system.cpp
+++ clang/test/Driver/darwin-header-search-system.cpp
@@ -1,5 +1,3 @@
-// UNSUPPORTED: system-windows
-
 // General tests that the system header search paths detected by the driver
 // and passed to CC1 are correct on Darwin platforms.
 
@@ -11,18 +9,14 @@
 // RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain_no_libcxx/usr/bin \
 // RUN: -resource-dir=%S/Inputs/resource_dir \
 // RUN: -isysroot %S/Inputs/basic_darwin_sdk_usr_and_usr_local \
-// RUN:   | FileCheck -DSYSROOT=%S/Inputs/basic_darwin_sdk_usr_and_usr_local \
-// RUN:   -DRESOURCE=%S/Inputs/resource_dir \
-// RUN:   --check-prefix=CHECK-SYSTEM %s
+// RUN:   | FileCheck --check-prefix=CHECK-SYSTEM %s
 //
 // RUN: %clang -no-canonical-prefixes %s -### -fsyntax-only 2>&1 \
 // RUN: -target x86_64-apple-darwin \
 // RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain_no_libcxx/usr/bin \
 // RUN: -resource-dir=%S/Inputs/resource_dir \
 // RUN: --sysroot %S/Inputs/basic_darwin_sdk_usr_and_usr_local \
-// RUN:   | FileCheck -DSYSROOT=%S/Inputs/basic_darwin_sdk_usr_and_usr_local \
-// RUN:   -DRESOURCE=%S/Inputs/resource_dir \
-// RUN:   --check-prefix=CHECK-SYSTEM %s
+// RUN:   | FileCheck --check-prefix=CHECK-SYSTEM %s
 //
 // RUN: %clang -no-canonical-prefixes %s -### -fsyntax-only 2>&1 \
 // RUN: -target x86_64-apple-darwin \
@@ -30,11 +24,10 @@
 // RUN: -resource-dir=%S/Inputs/resource_dir \
 // RUN: -isysroot %S/Inputs/basic_darwin_sdk_usr_and_usr_local \
 // RUN: --sysroot / \
-// RUN:   | FileCheck -DSYSROOT=%S/Inputs/basic_darwin_sdk_usr_and_usr_local \
-// RUN:   -DRESOURCE=%S/Inputs/resource_dir \
-// RUN:   --check-prefix=CHECK-SYSTEM %s
-//
+// RUN:   | FileCheck --check-prefix=CHECK-SYSTEM %s
 // CHECK-SYSTEM: "{{[^"]*}}clang{{[^"]*}}" "-cc1"
+// CHECK-SYSTEM: "-resource-dir" "[[RESOURCE:[^"]+]]"
+// CHECK-SYSTEM: "-isysroot" "[[SYSROOT:[^"]+]]"
 // CHECK-SYSTEM: "-internal-isystem" "[[SYSROOT]]/usr/local/include"
 // CHECK-SYSTEM: "-internal-isystem" "[[RESOURCE]]/include"
 // CHECK-SYSTEM: "-internal-externc-isystem" "[[SYSROOT]]/usr/include"
@@ -47,10 +40,10 @@
 // RUN: -resource-dir=%S/Inputs/resource_dir \
 // RUN: -isysroot %S/Inputs/basic_darwin_sdk_usr_and_usr_local \
 // RUN: -nobuiltininc \
-// RUN:   | FileCheck -DSYSROOT=%S/Inputs/basic_darwin_sdk_usr_and_usr_local \
-// RUN:   -DRESOURCE=%S/Inputs/resource_dir \
-// RUN:   --check-prefix=CHECK-NOBUILTININC %s
+// RUN:   | FileCheck --check-prefix=CHECK-NOBUILTININC %s
 // CHECK-NOBUILTININC: "{{[^"]*}}clang{{[^"]*}}" "-cc1"
+// CHECK-NOBUILTININC: "-resource-dir" "[[RESOURCE:[^"]+]]"
+// CHECK-NOBUILTININC: "-isysroot" "[[SYSROOT:[^"]+]]"
 // CHECK-NOBUILTININC: "-internal-isystem" "[[SYSROOT]]/usr/local/include"
 // CHECK-NOBUILTININC-NOT: "-internal-isystem" "[[RESOURCE]]/include"
 // CHECK-NOBUILTININC: "-internal-externc-isystem" "[[SYSROOT]]/usr/include"
@@ -64,10 +57,10 @@
 // RUN: -resource-dir=%S/Inputs/resource_dir \
 // RUN: -isysroot %S/Inputs/basic_darwin_sdk_usr_and_usr_local \
 // RUN: -nostdlibinc \
-// RUN:   | FileCheck -DSYSROOT=%S/Inputs/basic_darwin_sdk_usr_and_usr_local \
-// RUN:   -DRESOURCE=%S/Inputs/resource_dir \
-// RUN:   --check-prefix=CHECK-NOSTDLIBINC %s
+// RUN:   | FileCheck --check-prefix=CHECK-NOSTDLIBINC %s
 // CHECK-NOSTDLIBINC: "{{[^"]*}}clang{{[^"]*}}" "-cc1"
+// CHECK-NOSTDLIBINC: "-resource-dir" "[[RESOURCE:[^"]+]]"
+// CHECK-NOSTDLIBINC: "-isysroot" "[[SYSROOT:[^"]+]]"
 // CHECK-NOSTDLIBINC-NOT: "-internal-isystem" "[[SYSROOT]]/usr/local/include"
 // CHECK-NOSTDLIBINC: "-internal-isystem" "[[RESOURCE]]/include"
 // CHECK-NOSTDLIBINC-NOT: "-internal-externc-isystem" "[[SYSROOT]]/usr/include"
@@ -81,10 +74,10 @@
 // RUN: -resource-dir=%S/Inputs/resource_dir \
 // RUN: -isysroot %S/Inputs/basic_darwin_sdk_usr_and_usr_local \
 // RUN: -nostdin

[PATCH] D62493: [Driver] Always use Unix-style paths in the Darwin driver

2019-05-27 Thread Louis Dionne via Phabricator via cfe-commits

ldionne added a comment.

A few notes:

- I did setup a Windows machine and run the tests on it
- I'd welcome a better way to solve this problem than ripping out the use of 
`llvm::sys::path::append`, but I wasn't able to find anything in `FileCheck` 
that would do it


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62493



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

r361784 - [Driver] Change layout of per-target runtimes to resemble multiarch

2019-05-27 Thread Petr Hosek via cfe-commits

Author: phosek
Date: Mon May 27 16:23:50 2019
New Revision: 361784

URL: http://llvm.org/viewvc/llvm-project?rev=361784&view=rev
Log:
[Driver] Change layout of per-target runtimes to resemble multiarch

This is a follow up to r361432, changing the layout of per-target
runtimes to more closely resemble multiarch. While before, we used
the following layout:

[RESOURCE_DIR]//lib/libclang_rt..

Now we use the following layout:

[RESOURCE_DIR]/lib//libclang_rt..

This also more closely resembles the existing "non-per-target" layout:

[RESOURCE_DIR]/lib//libclang_rt.-.

This change will enable further simplification of the driver logic
in follow up changes.

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

Added:
cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/

cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/aarch64-fuchsia/

cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/aarch64-fuchsia/.keep

cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/aarch64-fuchsia/asan/

cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/aarch64-fuchsia/asan/.keep

cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/aarch64-fuchsia/libclang_rt.asan-preinit.a

cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/aarch64-fuchsia/libclang_rt.asan.so

cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/aarch64-fuchsia/libclang_rt.builtins.a

cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/aarch64-fuchsia/libclang_rt.fuzzer.a

cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/aarch64-fuchsia/libclang_rt.scudo.so

cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/aarch64-fuchsia/libclang_rt.xray-basic.a

cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/aarch64-fuchsia/libclang_rt.xray.a

cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/aarch64-fuchsia/noexcept/

cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/aarch64-fuchsia/noexcept/.keep

cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/i386-linux-gnu/

cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/i386-linux-gnu/.keep

cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/i386-linux-gnu/libclang_rt.builtins.a

cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/x86_64-fuchsia/

cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/x86_64-fuchsia/.keep

cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/x86_64-fuchsia/asan/

cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/x86_64-fuchsia/asan/.keep

cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/x86_64-fuchsia/libclang_rt.asan-preinit.a

cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/x86_64-fuchsia/libclang_rt.asan.so

cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/x86_64-fuchsia/libclang_rt.builtins.a

cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/x86_64-fuchsia/libclang_rt.fuzzer.a

cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/x86_64-fuchsia/libclang_rt.scudo.so

cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/x86_64-fuchsia/libclang_rt.xray-basic.a

cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/x86_64-fuchsia/libclang_rt.xray.a

cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/x86_64-fuchsia/noexcept/

cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/x86_64-fuchsia/noexcept/.keep

cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/x86_64-linux-gnu/

cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/x86_64-linux-gnu/.keep

cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/x86_64-linux-gnu/libclang_rt.builtins.a
Removed:

cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/aarch64-fuchsia/

cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/i386-linux-gnu/

cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/x86_64-fuchsia/

cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/x86_64-linux-gnu/
Modified:
cfe/trunk/lib/Driver/ToolChain.cpp
cfe/trunk/test/Driver/fuchsia.c
cfe/trunk/test/Driver/fuchsia.cpp
cfe/trunk/test/Driver/linux-per-target-runtime-dir.c

Modified: cfe/trunk/lib/Driver/ToolChain.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChain.cpp?rev=361784&r1=361783&r2=361784&view=diff
==
--- cfe/trunk/lib/Driver/ToolChain.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChain.

[PATCH] D62429: Fix linkage of _ZTS strings for types involving incomplete classes to match the Itanium ABI rules.

2019-05-27 Thread Eric Fiselier via Phabricator via cfe-commits

EricWF added a comment.

I think this is llvm.org/PR37398

I tried fixing this a while back in r332028 but the fix got reverted for 
causing llvm.org/PR37545.


Repository:
  rC Clang

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

https://reviews.llvm.org/D62429



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

[PATCH] D62469: [Driver] Change layout of per-target runtimes to resemble multiarch

2019-05-27 Thread Petr Hosek via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL361784: [Driver] Change layout of per-target runtimes to 
resemble multiarch (authored by phosek, committed by ).
Herald added a subscriber: delcypher.

Changed prior to commit:
  https://reviews.llvm.org/D62469?vs=201466&id=201595#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D62469

Files:
  cfe/trunk/lib/Driver/ToolChain.cpp
  
cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/aarch64-fuchsia/lib/.keep
  
cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/aarch64-fuchsia/lib/libclang_rt.asan-preinit.a
  
cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/aarch64-fuchsia/lib/libclang_rt.asan.so
  
cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/aarch64-fuchsia/lib/libclang_rt.builtins.a
  
cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/aarch64-fuchsia/lib/libclang_rt.fuzzer.a
  
cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/aarch64-fuchsia/lib/libclang_rt.scudo.so
  
cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/aarch64-fuchsia/lib/libclang_rt.xray-basic.a
  
cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/aarch64-fuchsia/lib/libclang_rt.xray.a
  
cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/i386-linux-gnu/lib/.keep
  
cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/i386-linux-gnu/lib/libclang_rt.builtins.a
  
cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/aarch64-fuchsia/.keep
  
cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/aarch64-fuchsia/asan/.keep
  
cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/aarch64-fuchsia/libclang_rt.asan-preinit.a
  
cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/aarch64-fuchsia/libclang_rt.asan.so
  
cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/aarch64-fuchsia/libclang_rt.builtins.a
  
cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/aarch64-fuchsia/libclang_rt.fuzzer.a
  
cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/aarch64-fuchsia/libclang_rt.scudo.so
  
cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/aarch64-fuchsia/libclang_rt.xray-basic.a
  
cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/aarch64-fuchsia/libclang_rt.xray.a
  
cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/aarch64-fuchsia/noexcept/.keep
  
cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/i386-linux-gnu/.keep
  
cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/i386-linux-gnu/libclang_rt.builtins.a
  
cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/x86_64-fuchsia/.keep
  
cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/x86_64-fuchsia/asan/.keep
  
cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/x86_64-fuchsia/libclang_rt.asan-preinit.a
  
cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/x86_64-fuchsia/libclang_rt.asan.so
  
cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/x86_64-fuchsia/libclang_rt.builtins.a
  
cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/x86_64-fuchsia/libclang_rt.fuzzer.a
  
cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/x86_64-fuchsia/libclang_rt.scudo.so
  
cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/x86_64-fuchsia/libclang_rt.xray-basic.a
  
cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/x86_64-fuchsia/libclang_rt.xray.a
  
cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/x86_64-fuchsia/noexcept/.keep
  
cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/x86_64-linux-gnu/.keep
  
cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/x86_64-linux-gnu/libclang_rt.builtins.a
  
cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/x86_64-fuchsia/lib/.keep
  
cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/x86_64-fuchsia/lib/libclang_rt.asan-preinit.a
  
cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/x86_64-fuchsia/lib/libclang_rt.asan.so
  
cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/x86_64-fuchsia/lib/libclang_rt.builtins.a
  
cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/x86_64-fuchsia/lib/libclang_rt.fuzzer.a
  
cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/x86_64-fuchsia/lib/libclang_rt.scudo.so
  
cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/x86_64-fuchsia/lib/libclang_rt.xray-basic.a
  
cfe/trunk/test/Driver/Inputs/resource_dir_with_per_target_subdir/x86_64-fuchsia/lib/libclang_rt.xray.a
  
cfe/trunk/test/Driver/Inputs/resource

[PATCH] D62470: Clarify when fix-it hints on warnings are appropriate

2019-05-27 Thread Aaron Puchert via Phabricator via cfe-commits

aaronpuchert updated this revision to Diff 201597.
aaronpuchert marked an inline comment as done.
aaronpuchert added a comment.

Put the important part first, and the rest in a separate sentence.


Repository:
  rC Clang

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

https://reviews.llvm.org/D62470

Files:
  docs/InternalsManual.rst


Index: docs/InternalsManual.rst
===
--- docs/InternalsManual.rst
+++ docs/InternalsManual.rst
@@ -423,6 +423,9 @@
   driver, they should only be used when it's very likely they match the user's
   intent.
 * Clang must recover from errors as if the fix-it had been applied.
+* Fix-it hints on a warning must not change the meaning of the code.
+  However, a hint may clarify the meaning as intentional, for example by adding
+  parentheses when the precedence of operators isn't obvious.
 
 If a fix-it can't obey these rules, put the fix-it on a note.  Fix-its on notes
 are not applied automatically.


Index: docs/InternalsManual.rst
===
--- docs/InternalsManual.rst
+++ docs/InternalsManual.rst
@@ -423,6 +423,9 @@
   driver, they should only be used when it's very likely they match the user's
   intent.
 * Clang must recover from errors as if the fix-it had been applied.
+* Fix-it hints on a warning must not change the meaning of the code.
+  However, a hint may clarify the meaning as intentional, for example by adding
+  parentheses when the precedence of operators isn't obvious.
 
 If a fix-it can't obey these rules, put the fix-it on a note.  Fix-its on notes
 are not applied automatically.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.

2019-05-27 Thread Ulrich Weigand via Phabricator via cfe-commits

uweigand added a comment.

Looks like this test is failing on SystemZ since it was added, making all our 
build bots red:

  
/home/uweigand/sandbox/buildbot/clang-s390x-linux/llvm/tools/clang/test/CodeGenCXX/builtin_FUNCTION.cpp:9:11:
 error: CHECK: expected string not found in input
  // CHECK: @[[EMPTY_STR:.+]] = private unnamed_addr constant [1 x i8] 
zeroinitializer, align 1
^
  
/home/uweigand/sandbox/buildbot/clang-s390x-linux/stage1/tools/clang/test/CodeGenCXX/Output/builtin_FUNCTION.cpp.tmp.ll:1:1:
 note: scanning from here
  ; ModuleID = 
'/home/uweigand/sandbox/buildbot/clang-s390x-linux/llvm/tools/clang/test/CodeGenCXX/builtin_FUNCTION.cpp'
  ^
  
/home/uweigand/sandbox/buildbot/clang-s390x-linux/stage1/tools/clang/test/CodeGenCXX/Output/builtin_FUNCTION.cpp.tmp.ll:8:1:
 note: possible intended match here
  @.str = private unnamed_addr constant [1 x i8] zeroinitializer, align 2
  ^

The problem is that string constants are 2-aligned according to the SystemZ ABI 
(this is a bit different, but deliberate in order to allow computing their 
addresses with a LARL instruction).  This makes all the "align 1" checks fail.

Is this test deliberately verifying the alignment, or could this just be 
removed?


Repository:
  rC Clang

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

https://reviews.llvm.org/D37035



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

[PATCH] D61742: [Driver][Windows] Add dependent lib argument for profile instr generate

2019-05-27 Thread Nico Weber via Phabricator via cfe-commits

thakis added a comment.

Hello, this embeds an absolute path into the generated .obj file, which means 
the output now is no longer deterministic (since it depends on the absolute 
path to clang_rt.profile-x86_64.lib). This means the output will be different 
on different machines, which breaks things like caching in distributed builds 
and whatnot.

I can't see an obvious way to save this patch either, since you won't know at 
compile time what the CWD will be at link time.

As-is, this breaks Chromium's deterministic builds. ("Only" the coverage 
builds, and they don't yet check for determinism, which is why it took us a 
while to notice.)

Suggestions?


Repository:
  rC Clang

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

https://reviews.llvm.org/D61742



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

[PATCH] D62429: Fix linkage of _ZTS strings for types involving incomplete classes to match the Itanium ABI rules.

2019-05-27 Thread Richard Smith - zygoloid via Phabricator via cfe-commits

rsmith added a comment.

In D62429#1518602 , @EricWF wrote:

> I think this is llvm.org/PR37398
>
> I tried fixing this a while back in r332028 but the fix got reverted for 
> causing llvm.org/PR37545.

Looks like I had a fix for that in https://reviews.llvm.org/D47092 but it seems 
we decided to not go that way because the issue is really an ASan 
instrumentation bug and should be fixed in ASan instead... so I guess this is 
blocked on a fix to ASan :-/

Repository:
  rC Clang

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

https://reviews.llvm.org/D62429

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

[PATCH] D62391: [CodeComplete] Complete 'return true/false' in boolean functions

2019-05-27 Thread Mike Lothian via Phabricator via cfe-commits

FireBurn added a comment.

Could this commit be causing the following build failure of Chromium, this same 
version build a few days ago with clang from git:

FAILED: 
obj/third_party/blink/renderer/bindings/core/v8/bindings_core_impl/v8_feature_policy.o
clang++ -MMD -MF 
obj/third_party/blink/renderer/bindings/core/v8/bindings_core_impl/v8_feature_policy.o.d
 -DUSE_UDEV -DUSE_AURA=1 -DUSE_GLIB=1 -DUSE_NSS_CERTS=1 -DUSE_X11=1 
-DNO_TCMALLOC -DFULL_SAFE_BROWSING -DSAFE_BROWSING_CSD -DSAFE_BROWSING_DB_LOCAL 
-DCHROMIUM_BUILD -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE 
-D_LARGEFILE64_SOURCE -D_GNU_SOURCE -DCR_CLANG_REVISION=\"360094-5\" 
-D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D_FORTIFY_SOURCE=2 -DNDEBUG 
-DNVALGRIND -DDYNAMIC_ANNOTATIONS_ENABLED=0 -DBLINK_CORE_IMPLEMENTATION=1 
-DENABLE_IPC_FUZZER -DGLIB_VERSION_MAX_ALLOWED=GLIB_VERSION_2_32 
-DGLIB_VERSION_MIN_REQUIRED=GLIB_VERSION_2_26 -DGL_GLEXT_PROTOTYPES -DUSE_GLX 
-DUSE_EGL -DVK_NO_PROTOTYPES -DBLINK_IMPLEMENTATION=1 -DINSIDE_BLINK 
-DWEBRTC_NON_STATIC_TRACE_EVENT_HANDLERS=0 -DWEBRTC_CHROMIUM_BUILD 
-DWEBRTC_POSIX -DWEBRTC_LINUX -DABSL_ALLOCATOR_NOTHROW=1 
-DNO_MAIN_THREAD_WRAPPING -DU_USING_ICU_NAMESPACE=0 -DU_ENABLE_DYLOAD=0 
-DUSE_CHROMIUM_ICU=1 -DU_STATIC_IMPLEMENTATION 
-DICU_UTIL_DATA_IMPL=ICU_UTIL_DATA_FILE -DUCHAR_TYPE=uint16_t 
-DGOOGLE_PROTOBUF_NO_RTTI -DGOOGLE_PROTOBUF_NO_STATIC_INITIALIZER 
-DHAVE_PTHREAD -DSK_HAS_PNG_LIBRARY -DSK_HAS_WEBP_LIBRARY 
-DSK_USER_CONFIG_HEADER=\"../../skia/config/SkUserConfig.h\" 
-DSK_HAS_JPEG_LIBRARY -DSK_VULKAN_HEADER=\"../../skia/config/SkVulkanConfig.h\" 
-DSK_VULKAN=1 -DSK_SUPPORT_GPU=1 
-DSK_GPU_WORKAROUNDS_HEADER=\"gpu/config/gpu_driver_bug_workaround_autogen.h\" 
-DVK_NO_PROTOTYPES -DV8_DEPRECATION_WARNINGS -DLEVELDB_PLATFORM_CHROMIUM=1 
-DLEVELDB_PLATFORM_CHROMIUM=1 -DWTF_USE_WEBAUDIO_FFMPEG=1 
-DSUPPORT_WEBGL2_COMPUTE_CONTEXT=1 -DUSE_SYSTEM_LIBJPEG 
-DV8_DEPRECATION_WARNINGS -DUSE_SYSTEM_ZLIB=1 -I../.. -Igen 
-Igen/shim_headers/zlib_shim -I../../third_party/libyuv/include 
-Igen/shim_headers/libpng_shim -Igen/shim_headers/libwebp_shim 
-Igen/shim_headers/libdrm_shim -Igen/shim_headers/re2_shim 
-Igen/shim_headers/snappy_shim -I../../third_party/khronos -I../../gpu 
-I../../third_party/vulkan/include -Igen/third_party/dawn 
-I../../third_party/dawn/src/include -I../../third_party/boringssl/src/include 
-I../../third_party/webrtc_overrides -I../../third_party/webrtc 
-Igen/third_party/webrtc -I../../third_party/abseil-cpp 
-I../../third_party/ced/src -I../../third_party/icu/source/common 
-I../../third_party/icu/source/i18n -I../../third_party/protobuf/src 
-I../../third_party/protobuf/src -Igen/protoc_out -I../../third_party/skia 
-I../../third_party/vulkan/include 
-I../../third_party/skia/third_party/vulkanmemoryallocator 
-I../../third_party/vulkan/include -I../../third_party/angle/include 
-I../../third_party/angle/src/common/third_party/base -Igen/angle 
-I../../third_party/angle/include -I../../v8/include -Igen/v8/include 
-I../../third_party/libwebm/source -I../../third_party/leveldatabase 
-I../../third_party/leveldatabase/src 
-I../../third_party/leveldatabase/src/include -I../../third_party/iccjpeg 
-I../../third_party/ots/include -I../../v8/include -Igen/v8/include 
-fno-strict-aliasing --param=ssp-buffer-size=4 -fstack-protector 
-funwind-tables -fPIC -pthread -fcolor-diagnostics -fmerge-all-constants 
-fcrash-diagnostics-dir=../../tools/clang/crashreports -Xclang -mllvm -Xclang 
-instcombine-lower-dbg-declare=0 -m64 -march=x86-64 
-Wno-builtin-macro-redefined -D__DATE__= -D__TIME__= -D__TIMESTAMP__= 
-no-canonical-prefixes -Wall -Wextra -Wimplicit-fallthrough -Wthread-safety 
-Wextra-semi -Wno-missing-field-initializers -Wno-unused-parameter 
-Wno-c++11-narrowing -Wno-unneeded-internal-declaration 
-Wno-undefined-var-template -Wno-ignored-pragma-optimize -O2 -fno-ident 
-fdata-sections -ffunction-sections -fno-omit-frame-pointer -fvisibility=hidden 
-Wheader-hygiene -Wstring-conversion -Wtautological-overlap-compare 
-Wexit-time-destructors -Wglobal-constructors -g0 -I/usr/include/glib-2.0 
-I/usr/lib64/glib-2.0/include -Wconversion -Wno-float-conversion 
-Wno-sign-conversion -Wno-implicit-float-conversion 
-Wno-implicit-int-conversion -Wno-shorten-64-to-32 -Wno-header-guard 
-I/usr/include/nss -I/usr/include/nspr -I/usr/include/libpng16 
-I/usr/include/libxml2 -I/usr/include/libxml2 -std=c++14 -fno-exceptions 
-fno-rtti -fvisibility-inlines-hidden -O3 -pipe -flto=thin -march=native -c 
gen/third_party/blink/renderer/bindings/core/v8/v8_feature_policy.cc -o 
obj/third_party/blink/renderer/bindings/core/v8/bindings_core_impl/v8_feature_policy.o
In file included from 
gen/third_party/blink/renderer/bindings/core/v8/v8_feature_policy.cc:11:
In file included from 
gen/third_party/blink/renderer/bindings/core/v8/v8_feature_policy.h:14:
In file included from 
../../third_party/blink/renderer/bindings/core/v8/generated_code_helper.h:14:
In file included from 
../../third_party/blink/rend

[PATCH] D62470: Clarify when fix-it hints on warnings are appropriate

2019-05-27 Thread Richard Smith - zygoloid via Phabricator via cfe-commits

rsmith added a comment.

Thanks, looks good to me.


Repository:
  rC Clang

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

https://reviews.llvm.org/D62470



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

[PATCH] D62363: [X86] Enable intrinsics that convert float and bf16 data to each other

2019-05-27 Thread Kan Shengchen via Phabricator via cfe-commits

skan updated this revision to Diff 201601.
skan added a comment.

rm clang-format fix in this patch


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

https://reviews.llvm.org/D62363

Files:
  include/clang/Basic/BuiltinsX86.def
  lib/CodeGen/CGBuiltin.cpp
  lib/Headers/avx512bf16intrin.h
  test/CodeGen/avx512bf16-builtins.c

Index: test/CodeGen/avx512bf16-builtins.c
===
--- test/CodeGen/avx512bf16-builtins.c
+++ test/CodeGen/avx512bf16-builtins.c
@@ -4,46 +4,64 @@
 
 #include 
 
-__m512bh test_mm512_cvtne2ps2bf16(__m512 A, __m512 B) {
-  // CHECK-LABEL: @test_mm512_cvtne2ps2bf16
+float test_mm_cvtsbh_ss(__bfloat16 A) {
+  // CHECK-LABEL: @test_mm_cvtsbh_ss
+  // CHECK: zext i16 %{{.*}} to i32
+  // CHECK: shl i32 %{{.*}}, 16
+  // CHECK: bitcast i32 %{{.*}} to float
+  // CHECK: ret float %{{.*}}
+  return _mm_cvtsbh_ss(A);
+}
+
+__bfloat16 test_mm_cvtness_sbh(float A) {
+  // CHECK-LABEL: @test_mm_cvtness_sbh
+  // CHECK: bitcast float %{{.*}} to i32
+  // CHECK: lshr i32 %{{.*}}, 16
+  // CHECK: trunc i32 %{{.*}} to i16
+  // CHECK: ret i16 %{{.*}}
+  return _mm_cvtness_sbh(A);
+}
+
+__m512bh test_mm512_cvtne2ps_pbh(__m512 A, __m512 B) {
+  // CHECK-LABEL: @test_mm512_cvtne2ps_pbh
   // CHECK: @llvm.x86.avx512bf16.cvtne2ps2bf16.512
   // CHECK: ret <32 x i16> %{{.*}}
   return _mm512_cvtne2ps_pbh(A, B);
 }
 
-__m512bh test_mm512_maskz_cvtne2ps2bf16(__m512 A, __m512 B, __mmask32 U) {
-  // CHECK-LABEL: @test_mm512_maskz_cvtne2ps2bf16
+__m512bh test_mm512_maskz_cvtne2ps_pbh(__m512 A, __m512 B, __mmask32 U) {
+  // CHECK-LABEL: @test_mm512_maskz_cvtne2ps_pbh
   // CHECK: @llvm.x86.avx512bf16.cvtne2ps2bf16.512
   // CHECK: select <32 x i1> %{{.*}}, <32 x i16> %{{.*}}, <32 x i16> %{{.*}}
   // CHECK: ret <32 x i16> %{{.*}}
   return _mm512_maskz_cvtne2ps_pbh(U, A, B);
 }
 
-__m512bh test_mm512_mask_cvtne2ps2bf16(__m512bh C, __mmask32 U, __m512 A, __m512 B) {
-  // CHECK-LABEL: @test_mm512_mask_cvtne2ps2bf16
+__m512bh test_mm512_mask_cvtne2ps_pbh(__m512bh C, __mmask32 U, __m512 A, __m512 B) {
+  // CHECK-LABEL: @test_mm512_mask_cvtne2ps_pbh
   // CHECK: @llvm.x86.avx512bf16.cvtne2ps2bf16.512
   // CHECK: select <32 x i1> %{{.*}}, <32 x i16> %{{.*}}, <32 x i16> %{{.*}}
   // CHECK: ret <32 x i16> %{{.*}}
   return _mm512_mask_cvtne2ps_pbh(C, U, A, B);
 }
 
-__m256bh test_mm512_cvtneps2bf16(__m512 A) {
-  // CHECK-LABEL: @test_mm512_cvtneps2bf16
+__m256bh test_mm512_cvtneps_pbh(__m512 A) {
+  // CHECK-LABEL: @test_mm512_cvtneps_pbh
   // CHECK: @llvm.x86.avx512bf16.cvtneps2bf16.512
   // CHECK: ret <16 x i16> %{{.*}}
   return _mm512_cvtneps_pbh(A);
 }
 
-__m256bh test_mm512_mask_cvtneps2bf16(__m256bh C, __mmask16 U, __m512 A) {
-  // CHECK-LABEL: @test_mm512_mask_cvtneps2bf16
+__m256bh test_mm512_mask_cvtneps_pbh(__m256bh C, __mmask16 U, __m512 A) {
+  // CHECK-LABEL: @test_mm512_mask_cvtneps_pbh
   // CHECK: @llvm.x86.avx512bf16.cvtneps2bf16.512
   // CHECK: select <16 x i1> %{{.*}}, <16 x i16> %{{.*}}, <16 x i16> %{{.*}}
   // CHECK: ret <16 x i16> %{{.*}}
   return _mm512_mask_cvtneps_pbh(C, U, A);
 }
 
-__m256bh test_mm512_maskz_cvtneps2bf16(__m512 A, __mmask16 U) {
-  // CHECK-LABEL: @test_mm512_maskz_cvtneps2bf16
+__m256bh test_mm512_maskz_cvtneps_pbh(__m512 A, __mmask16 U) {
+  // CHECK-LABEL: @test_mm512_maskz_cvtneps_pbh
   // CHECK: @llvm.x86.avx512bf16.cvtneps2bf16.512
   // CHECK: select <16 x i1> %{{.*}}, <16 x i16> %{{.*}}, <16 x i16> %{{.*}}
   // CHECK: ret <16 x i16> %{{.*}}
@@ -72,3 +90,61 @@
   // CHECK: ret <16 x float> %{{.*}}
   return _mm512_mask_dpbf16_ps(D, U, A, B);
 }
+
+__m512 test_mm512_cvtpbh_ps(__m256bh A) {
+  // CHECK-LABEL: @test_mm512_cvtpbh_ps
+  // CHECK: sext <16 x i16> %{{.*}} to <16 x i32>
+  // CHECK: @llvm.x86.avx512.pslli.d.512
+  // CHECK: bitcast <8 x i64> %{{.*}} to <16 x float>
+  // CHECK: ret <16 x float> %{{.*}}
+  return _mm512_cvtpbh_ps(A);
+}
+
+__m256 test_mm256_cvtpbh_ps(__m128bh A) {
+  // CHECK-LABEL: @test_mm256_cvtpbh_ps
+  // CHECK: sext <8 x i16> %{{.*}} to <8 x i32>
+  // CHECK: @llvm.x86.avx2.pslli.d
+  // CHECK: bitcast <4 x i64> %{{.*}} to <8 x float>
+  // CHECK: ret <8 x float> %{{.*}}
+  return _mm256_cvtpbh_ps(A);
+}
+
+__m512 test_mm512_maskz_cvtpbh_ps(__mmask16 M, __m256bh A) {
+  // CHECK-LABEL: @test_mm512_maskz_cvtpbh_ps
+  // CHECK: sext <16 x i16> %{{.*}} to <16 x i32>
+  // CHECK: select <16 x i1> %{{.*}}, <16 x i32> %{{.*}}, <16 x i32> %{{.*}}
+  // CHECK: @llvm.x86.avx512.pslli.d.512
+  // CHECK: bitcast <8 x i64> %{{.*}} to <16 x float>
+  // CHECK: ret <16 x float> %{{.*}}
+  return _mm512_maskz_cvtpbh_ps(M, A);
+}
+
+__m256 test_mm256_maskz_cvtpbh_ps(__mmask8 M, __m128bh A) {
+  // CHECK-LABEL: @test_mm256_maskz_cvtpbh_ps
+  // CHECK: sext <8 x i16> %{{.*}} to <8 x i32>
+  // CHECK: select <8 x i1> %{{.*}}, <8 x i32> %{{.*}}, <8 x i32> %{{.*}}
+  // CHECK: @llvm.x86.avx2.pslli.d
+  // CHECK: bitcast <4 x i64> %{{.*}} to <8 x float>
+  // CHECK: ret <

[PATCH] D62470: Clarify when fix-it hints on warnings are appropriate

2019-05-27 Thread Aaron Ballman via Phabricator via cfe-commits

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

LGTM!


Repository:
  rC Clang

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

https://reviews.llvm.org/D62470



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

[PATCH] D62363: [X86] Enable intrinsics that convert float and bf16 data to each other

2019-05-27 Thread Kan Shengchen via Phabricator via cfe-commits

skan marked 3 inline comments as done.
skan added inline comments.



Comment at: lib/Headers/avx512bf16intrin.h:52
+///
+/// \headerfile
+///

RKSimon wrote:
> \headerfile 
fixed in updated patch



Comment at: lib/Headers/avx512bf16intrin.h:66
+///
+/// \headerfile
+///

RKSimon wrote:
> \headerfile 
fixed in updated patch



Comment at: lib/Headers/avx512bf16intrin.h:98
+  return (__m512bh)__builtin_ia32_selectw_512(
+  (__mmask32)__U, (__v32hi)_mm512_cvtne2ps_pbh(__A, __B), (__v32hi)__W);
 }

RKSimon wrote:
> style/clang-format changes like these should be separated into their own patch
style changes are removed in the updated patch


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

https://reviews.llvm.org/D62363



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

[PATCH] D62363: [X86] Enable intrinsics that convert float and bf16 data to each other

2019-05-27 Thread Craig Topper via Phabricator via cfe-commits

craig.topper added inline comments.



Comment at: lib/Headers/avx512bf16intrin.h:37
+///
+/// This intrinsic corresponds to the  EmitX86CvtBF16ToFloatExpr 
+/// function.

This needs to be a comment that's useful to user's of the compiler not compiler 
developers. It can't reference an implementation function inside the compiler


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

https://reviews.llvm.org/D62363



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

[PATCH] D62363: [X86] Enable intrinsics that convert float and bf16 data to each other

2019-05-27 Thread Kan Shengchen via Phabricator via cfe-commits

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



Comment at: lib/Headers/avx512bf16intrin.h:37
+///
+/// This intrinsic corresponds to the  EmitX86CvtBF16ToFloatExpr 
+/// function.

craig.topper wrote:
> This needs to be a comment that's useful to user's of the compiler not 
> compiler developers. It can't reference an implementation function inside the 
> compiler
i can not think out a comment that 's needed  by users since other comments has 
explained this function clearly, can i just remove this comment?


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

https://reviews.llvm.org/D62363



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

[PATCH] D62493: [Driver] Always use Unix-style paths in the Darwin driver

2019-05-27 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits

dexonsmith added a comment.

This doesn’t look okay to me, because this would prevent building for Darwin 
when running on Windows.  I added a couple of reviewers that have Windows 
experience and might have ideas for how to fix the tests without breaking the 
portability of the driver.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62493



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

[PATCH] D62363: [X86] Enable intrinsics that convert float and bf16 data to each other

2019-05-27 Thread Craig Topper via Phabricator via cfe-commits

craig.topper added inline comments.



Comment at: lib/Headers/avx512bf16intrin.h:37
+///
+/// This intrinsic corresponds to the  EmitX86CvtBF16ToFloatExpr 
+/// function.

skan wrote:
> craig.topper wrote:
> > This needs to be a comment that's useful to user's of the compiler not 
> > compiler developers. It can't reference an implementation function inside 
> > the compiler
> i can not think out a comment that 's needed  by users since other comments 
> has explained this function clearly, can i just remove this comment?
Just write "This intrinsic does not correspond to a specific instruction,"


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

https://reviews.llvm.org/D62363



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

[PATCH] D62479: [Analyzer] Replace `CXXSelfAssignmentBRVisitor` with `NoteTags`

2019-05-27 Thread Artem Dergachev via Phabricator via cfe-commits

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

Yay, cool!




Comment at: lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:2388-2390
-  if (Tag->getTagDescription() != "cplusplus.SelfAssignment")
-return nullptr;
-

Looks like you almost invented note tags here :)



Comment at: lib/StaticAnalyzer/Core/PathDiagnostic.cpp:736-738
+// If the BlockEdge has no terminator condition statement (e.g. a
+// checker crated the branch at the beginning of a function), use the
+// function's declaration instead.

Could you assert that this is indeed the case? Like, check that the "from" 
`CFGBlock` of the edge is the current CFG's ENTRY block? 'Cause it's not 
obvious that the beginning of the function is the correct source location in 
all cases where the `CFGTerminator` is not present (you should also be able to 
assert that the `CFGTerminator` is not present, but i don't think it's the 
right thing to assert, as we might add more kinds of statement-less 
`CFGTerminator`s in the future).


Repository:
  rC Clang

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

https://reviews.llvm.org/D62479



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

[PATCH] D62363: [X86] Enable intrinsics that convert float and bf16 data to each other

2019-05-27 Thread Kan Shengchen via Phabricator via cfe-commits

skan updated this revision to Diff 201605.
skan added a comment.

make comments for  `_mm_cvtsbh_ss` and  `_mm_cvtness_sbh` better


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

https://reviews.llvm.org/D62363

Files:
  include/clang/Basic/BuiltinsX86.def
  lib/CodeGen/CGBuiltin.cpp
  lib/Headers/avx512bf16intrin.h
  test/CodeGen/avx512bf16-builtins.c

Index: test/CodeGen/avx512bf16-builtins.c
===
--- test/CodeGen/avx512bf16-builtins.c
+++ test/CodeGen/avx512bf16-builtins.c
@@ -4,46 +4,64 @@
 
 #include 
 
-__m512bh test_mm512_cvtne2ps2bf16(__m512 A, __m512 B) {
-  // CHECK-LABEL: @test_mm512_cvtne2ps2bf16
+float test_mm_cvtsbh_ss(__bfloat16 A) {
+  // CHECK-LABEL: @test_mm_cvtsbh_ss
+  // CHECK: zext i16 %{{.*}} to i32
+  // CHECK: shl i32 %{{.*}}, 16
+  // CHECK: bitcast i32 %{{.*}} to float
+  // CHECK: ret float %{{.*}}
+  return _mm_cvtsbh_ss(A);
+}
+
+__bfloat16 test_mm_cvtness_sbh(float A) {
+  // CHECK-LABEL: @test_mm_cvtness_sbh
+  // CHECK: bitcast float %{{.*}} to i32
+  // CHECK: lshr i32 %{{.*}}, 16
+  // CHECK: trunc i32 %{{.*}} to i16
+  // CHECK: ret i16 %{{.*}}
+  return _mm_cvtness_sbh(A);
+}
+
+__m512bh test_mm512_cvtne2ps_pbh(__m512 A, __m512 B) {
+  // CHECK-LABEL: @test_mm512_cvtne2ps_pbh
   // CHECK: @llvm.x86.avx512bf16.cvtne2ps2bf16.512
   // CHECK: ret <32 x i16> %{{.*}}
   return _mm512_cvtne2ps_pbh(A, B);
 }
 
-__m512bh test_mm512_maskz_cvtne2ps2bf16(__m512 A, __m512 B, __mmask32 U) {
-  // CHECK-LABEL: @test_mm512_maskz_cvtne2ps2bf16
+__m512bh test_mm512_maskz_cvtne2ps_pbh(__m512 A, __m512 B, __mmask32 U) {
+  // CHECK-LABEL: @test_mm512_maskz_cvtne2ps_pbh
   // CHECK: @llvm.x86.avx512bf16.cvtne2ps2bf16.512
   // CHECK: select <32 x i1> %{{.*}}, <32 x i16> %{{.*}}, <32 x i16> %{{.*}}
   // CHECK: ret <32 x i16> %{{.*}}
   return _mm512_maskz_cvtne2ps_pbh(U, A, B);
 }
 
-__m512bh test_mm512_mask_cvtne2ps2bf16(__m512bh C, __mmask32 U, __m512 A, __m512 B) {
-  // CHECK-LABEL: @test_mm512_mask_cvtne2ps2bf16
+__m512bh test_mm512_mask_cvtne2ps_pbh(__m512bh C, __mmask32 U, __m512 A, __m512 B) {
+  // CHECK-LABEL: @test_mm512_mask_cvtne2ps_pbh
   // CHECK: @llvm.x86.avx512bf16.cvtne2ps2bf16.512
   // CHECK: select <32 x i1> %{{.*}}, <32 x i16> %{{.*}}, <32 x i16> %{{.*}}
   // CHECK: ret <32 x i16> %{{.*}}
   return _mm512_mask_cvtne2ps_pbh(C, U, A, B);
 }
 
-__m256bh test_mm512_cvtneps2bf16(__m512 A) {
-  // CHECK-LABEL: @test_mm512_cvtneps2bf16
+__m256bh test_mm512_cvtneps_pbh(__m512 A) {
+  // CHECK-LABEL: @test_mm512_cvtneps_pbh
   // CHECK: @llvm.x86.avx512bf16.cvtneps2bf16.512
   // CHECK: ret <16 x i16> %{{.*}}
   return _mm512_cvtneps_pbh(A);
 }
 
-__m256bh test_mm512_mask_cvtneps2bf16(__m256bh C, __mmask16 U, __m512 A) {
-  // CHECK-LABEL: @test_mm512_mask_cvtneps2bf16
+__m256bh test_mm512_mask_cvtneps_pbh(__m256bh C, __mmask16 U, __m512 A) {
+  // CHECK-LABEL: @test_mm512_mask_cvtneps_pbh
   // CHECK: @llvm.x86.avx512bf16.cvtneps2bf16.512
   // CHECK: select <16 x i1> %{{.*}}, <16 x i16> %{{.*}}, <16 x i16> %{{.*}}
   // CHECK: ret <16 x i16> %{{.*}}
   return _mm512_mask_cvtneps_pbh(C, U, A);
 }
 
-__m256bh test_mm512_maskz_cvtneps2bf16(__m512 A, __mmask16 U) {
-  // CHECK-LABEL: @test_mm512_maskz_cvtneps2bf16
+__m256bh test_mm512_maskz_cvtneps_pbh(__m512 A, __mmask16 U) {
+  // CHECK-LABEL: @test_mm512_maskz_cvtneps_pbh
   // CHECK: @llvm.x86.avx512bf16.cvtneps2bf16.512
   // CHECK: select <16 x i1> %{{.*}}, <16 x i16> %{{.*}}, <16 x i16> %{{.*}}
   // CHECK: ret <16 x i16> %{{.*}}
@@ -72,3 +90,61 @@
   // CHECK: ret <16 x float> %{{.*}}
   return _mm512_mask_dpbf16_ps(D, U, A, B);
 }
+
+__m512 test_mm512_cvtpbh_ps(__m256bh A) {
+  // CHECK-LABEL: @test_mm512_cvtpbh_ps
+  // CHECK: sext <16 x i16> %{{.*}} to <16 x i32>
+  // CHECK: @llvm.x86.avx512.pslli.d.512
+  // CHECK: bitcast <8 x i64> %{{.*}} to <16 x float>
+  // CHECK: ret <16 x float> %{{.*}}
+  return _mm512_cvtpbh_ps(A);
+}
+
+__m256 test_mm256_cvtpbh_ps(__m128bh A) {
+  // CHECK-LABEL: @test_mm256_cvtpbh_ps
+  // CHECK: sext <8 x i16> %{{.*}} to <8 x i32>
+  // CHECK: @llvm.x86.avx2.pslli.d
+  // CHECK: bitcast <4 x i64> %{{.*}} to <8 x float>
+  // CHECK: ret <8 x float> %{{.*}}
+  return _mm256_cvtpbh_ps(A);
+}
+
+__m512 test_mm512_maskz_cvtpbh_ps(__mmask16 M, __m256bh A) {
+  // CHECK-LABEL: @test_mm512_maskz_cvtpbh_ps
+  // CHECK: sext <16 x i16> %{{.*}} to <16 x i32>
+  // CHECK: select <16 x i1> %{{.*}}, <16 x i32> %{{.*}}, <16 x i32> %{{.*}}
+  // CHECK: @llvm.x86.avx512.pslli.d.512
+  // CHECK: bitcast <8 x i64> %{{.*}} to <16 x float>
+  // CHECK: ret <16 x float> %{{.*}}
+  return _mm512_maskz_cvtpbh_ps(M, A);
+}
+
+__m256 test_mm256_maskz_cvtpbh_ps(__mmask8 M, __m128bh A) {
+  // CHECK-LABEL: @test_mm256_maskz_cvtpbh_ps
+  // CHECK: sext <8 x i16> %{{.*}} to <8 x i32>
+  // CHECK: select <8 x i1> %{{.*}}, <8 x i32> %{{.*}}, <8 x i32> %{{.*}}
+  // CHECK: @llvm.x86.avx2.pslli.d
+  // CHECK: bitcast <4 x i64> %{{.*}} to

[PATCH] D62445: [test] Fix plugin tests

2019-05-27 Thread Artem Dergachev via Phabricator via cfe-commits

NoQ added subscribers: mikhail.ramalho, ddcc.
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.

Thanks!!

I think we're losing the Z3 constraint manager test suite this way after all 
(details inline), but i feel we should downprioritize this when it comes to 
fixing everybody's buildbots.




Comment at: clang/test/CMakeLists.txt:141-147
-  if (LLVM_WITH_Z3)
-add_lit_testsuite(check-clang-analyzer-z3 "Running the Clang analyzer 
tests, using Z3 as a solver"
-  ${CMAKE_CURRENT_BINARY_DIR}/Analysis
-  PARAMS ${ANALYZER_TEST_PARAMS_Z3}
-  DEPENDS ${CLANG_TEST_DEPS})
-set_target_properties(check-clang-analyzer-z3 PROPERTIES FOLDER "Clang 
tests")
   endif()

As far as i remember, this chunk of code is responsible for running the whole 
analyzer test suite with different parameters (with Z3 as a constraint manager 
instead of the ad-hoc range constraint manager), rather than running a smaller 
chunk of the suite.

@mikhail.ramalho, @ddcc - i think i should leave it up to you to decide if you 
want to keep this working. Right now these extra tests aren't run under any 
buildbot, and the facility that they're testing is probably never going to be 
used by actual users (as opposed to the z3 refutation). But i wouldn't love 
losing this facility because it's a great way of evaluating the static 
analyzer, i.e. figuring out how good *could* it have been with a good 
constraint solver, so that we had something to look forward to.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62445



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

[PATCH] D62445: [test] Fix plugin tests

2019-05-27 Thread Don Hinton via Phabricator via cfe-commits

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



Comment at: clang/test/CMakeLists.txt:141-147
-  if (LLVM_WITH_Z3)
-add_lit_testsuite(check-clang-analyzer-z3 "Running the Clang analyzer 
tests, using Z3 as a solver"
-  ${CMAKE_CURRENT_BINARY_DIR}/Analysis
-  PARAMS ${ANALYZER_TEST_PARAMS_Z3}
-  DEPENDS ${CLANG_TEST_DEPS})
-set_target_properties(check-clang-analyzer-z3 PROPERTIES FOLDER "Clang 
tests")
   endif()

NoQ wrote:
> As far as i remember, this chunk of code is responsible for running the whole 
> analyzer test suite with different parameters (with Z3 as a constraint 
> manager instead of the ad-hoc range constraint manager), rather than running 
> a smaller chunk of the suite.
> 
> @mikhail.ramalho, @ddcc - i think i should leave it up to you to decide if 
> you want to keep this working. Right now these extra tests aren't run under 
> any buildbot, and the facility that they're testing is probably never going 
> to be used by actual users (as opposed to the z3 refutation). But i wouldn't 
> love losing this facility because it's a great way of evaluating the static 
> analyzer, i.e. figuring out how good *could* it have been with a good 
> constraint solver, so that we had something to look forward to.
Ah, that was unintentional.  I didn't intend to touch Z3.

Let me see if I can put that part back.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62445



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

[PATCH] D62445: [test] Fix plugin tests

2019-05-27 Thread Don Hinton via Phabricator via cfe-commits

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



Comment at: clang/test/CMakeLists.txt:141-147
-  if (LLVM_WITH_Z3)
-add_lit_testsuite(check-clang-analyzer-z3 "Running the Clang analyzer 
tests, using Z3 as a solver"
-  ${CMAKE_CURRENT_BINARY_DIR}/Analysis
-  PARAMS ${ANALYZER_TEST_PARAMS_Z3}
-  DEPENDS ${CLANG_TEST_DEPS})
-set_target_properties(check-clang-analyzer-z3 PROPERTIES FOLDER "Clang 
tests")
   endif()

hintonda wrote:
> NoQ wrote:
> > As far as i remember, this chunk of code is responsible for running the 
> > whole analyzer test suite with different parameters (with Z3 as a 
> > constraint manager instead of the ad-hoc range constraint manager), rather 
> > than running a smaller chunk of the suite.
> > 
> > @mikhail.ramalho, @ddcc - i think i should leave it up to you to decide if 
> > you want to keep this working. Right now these extra tests aren't run under 
> > any buildbot, and the facility that they're testing is probably never going 
> > to be used by actual users (as opposed to the z3 refutation). But i 
> > wouldn't love losing this facility because it's a great way of evaluating 
> > the static analyzer, i.e. figuring out how good *could* it have been with a 
> > good constraint solver, so that we had something to look forward to.
> Ah, that was unintentional.  I didn't intend to touch Z3.
> 
> Let me see if I can put that part back.
Actually, on second thought, these get picked up automatically, so they 
shouldn't be added like this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62445



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

[PATCH] D62509: [Driver] Render -fuse-init-array for -fembed-bitcode

2019-05-27 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay created this revision.
MaskRay added reviewers: compnerd, rsmith, t.p.northover.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Modern ELF platforms use -fuse-init-array to emit .init_array instead of
.ctors .  ld.bfd and gold merge .init_array and .ctors into .init_array
but lld doesn't do that. If crtbegin*.o crtend*.o don't provide
.ctors/.dtors, such .ctors in user object files can lead to crash
(see PR42002. The first and the last elements in .ctors/.dtors are ignored

- they are traditionally provided by crtbegin*.o crtend*.o).

Call addClangTargetOptions() to ensure -fuse-init-array is rendered on
modern ELF platforms.


Repository:
  rC Clang

https://reviews.llvm.org/D62509

Files:
  lib/Driver/ToolChains/Clang.cpp
  test/Driver/fembed-bitcode.c


Index: test/Driver/fembed-bitcode.c
===
--- test/Driver/fembed-bitcode.c
+++ test/Driver/fembed-bitcode.c
@@ -26,3 +26,6 @@
 // CHECK-AARCH64: "darwinpcs"
 // CHECK-AARCH64-NOT: "-fdebug-compilation-dir"
 
+// RUN: %clang -target x86_64-pc-freebsd12 -fembed-bitcode=all -c %s -o 
/dev/null -### 2>&1 \
+// RUN: | FileCheck -check-prefix CHECK-INITARRAY %s
+// CHECK-INITARRAY: "-fuse-init-array"
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -3671,6 +3671,9 @@
 // Disable all llvm IR level optimizations.
 CmdArgs.push_back("-disable-llvm-passes");
 
+// Render target options such as -fuse-init-array on modern ELF platforms.
+TC.addClangTargetOptions(Args, CmdArgs, JA.getOffloadingDeviceKind());
+
 // reject options that shouldn't be supported in bitcode
 // also reject kernel/kext
 static const constexpr unsigned kBitcodeOptionBlacklist[] = {


Index: test/Driver/fembed-bitcode.c
===
--- test/Driver/fembed-bitcode.c
+++ test/Driver/fembed-bitcode.c
@@ -26,3 +26,6 @@
 // CHECK-AARCH64: "darwinpcs"
 // CHECK-AARCH64-NOT: "-fdebug-compilation-dir"
 
+// RUN: %clang -target x86_64-pc-freebsd12 -fembed-bitcode=all -c %s -o /dev/null -### 2>&1 \
+// RUN: | FileCheck -check-prefix CHECK-INITARRAY %s
+// CHECK-INITARRAY: "-fuse-init-array"
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -3671,6 +3671,9 @@
 // Disable all llvm IR level optimizations.
 CmdArgs.push_back("-disable-llvm-passes");
 
+// Render target options such as -fuse-init-array on modern ELF platforms.
+TC.addClangTargetOptions(Args, CmdArgs, JA.getOffloadingDeviceKind());
+
 // reject options that shouldn't be supported in bitcode
 // also reject kernel/kext
 static const constexpr unsigned kBitcodeOptionBlacklist[] = {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D62367: [X86] VP2INTERSECT clang

2019-05-27 Thread Xiang Zhang via Phabricator via cfe-commits

xiangzhangllvm updated this revision to Diff 201611.

Repository:
  rC Clang

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

https://reviews.llvm.org/D62367

Files:
  docs/ClangCommandLineReference.rst
  include/clang/Basic/BuiltinsX86.def
  include/clang/Driver/Options.td
  lib/Basic/Targets/X86.cpp
  lib/Basic/Targets/X86.h
  lib/CodeGen/CGBuiltin.cpp
  lib/Headers/CMakeLists.txt
  lib/Headers/avx512vlvp2intersectintrin.h
  lib/Headers/avx512vp2intersectintrin.h
  lib/Headers/immintrin.h
  test/CodeGen/attr-target-x86.c
  test/CodeGen/intel-avx512vlvp2intersect.c
  test/CodeGen/intel-avx512vp2intersect.c
  test/Driver/x86-target-features.c
  test/Preprocessor/x86_target_features.c

Index: test/Preprocessor/x86_target_features.c
===
--- test/Preprocessor/x86_target_features.c
+++ test/Preprocessor/x86_target_features.c
@@ -458,3 +458,13 @@
 
 // AVX512BF16_NOAVX512VL: #define __AVX512BF16__ 1
 
+// RUN: %clang -target i386-unknown-linux-gnu -march=i386 -mavx512vp2intersect -x c -E -dM -o - %s | FileCheck  -check-prefix=VP2INTERSECT %s
+
+// VP2INTERSECT: #define __AVX512F__ 1
+// VP2INTERSECT: #define __AVX512VP2INTERSECT__ 1
+
+// RUN: %clang -target i386-unknown-linux-gnu -march=i386 -mno-avx512vp2intersect -x c -E -dM -o - %s | FileCheck  -check-prefix=NOVP2INTERSECT %s
+// RUN: %clang -target i386-unknown-linux-gnu -march=i386 -mavx512vp2intersect -mno-avx512f -x c -E -dM -o - %s | FileCheck  -check-prefix=NOVP2INTERSECT %s
+
+// NOVP2INTERSECT-NOT: #define __AVX512VP2INTERSECT__ 1
+
Index: test/Driver/x86-target-features.c
===
--- test/Driver/x86-target-features.c
+++ test/Driver/x86-target-features.c
@@ -125,6 +125,11 @@
 // VBMI2: "-target-feature" "+avx512vbmi2"
 // NO-VBMI2: "-target-feature" "-avx512vbmi2"
 
+// RUN: %clang -target i386-linux-gnu -mavx512vp2intersect %s -### -o %t.o 2>&1 | FileCheck -check-prefix=VP2INTERSECT %s
+// RUN: %clang -target i386-linux-gnu -mno-avx512vp2intersect %s -### -o %t.o 2>&1 | FileCheck -check-prefix=NO-VP2INTERSECT %s
+// VP2INTERSECT: "-target-feature" "+avx512vp2intersect"
+// NO-VP2INTERSECT: "-target-feature" "-avx512vp2intersect"
+
 // RUN: %clang -target i386-unknown-linux-gnu -march=i386 -mrdpid %s -### -o %t.o 2>&1 | FileCheck -check-prefix=RDPID %s
 // RUN: %clang -target i386-unknown-linux-gnu -march=i386 -mno-rdpid %s -### -o %t.o 2>&1 | FileCheck -check-prefix=NO-RDPID %s
 // RDPID: "-target-feature" "+rdpid"
Index: test/CodeGen/intel-avx512vp2intersect.c
===
--- /dev/null
+++ test/CodeGen/intel-avx512vp2intersect.c
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 %s -ffreestanding -triple=x86_64-unknown-unknown -target-feature +avx512vp2intersect -emit-llvm -o - -Wall -Werror | FileCheck %s
+// RUN: %clang_cc1 %s -ffreestanding -triple=i386-unknown-unknown -target-feature +avx512vp2intersect -emit-llvm -o - -Wall -Werror | FileCheck %s
+
+#include 
+
+void test_mm512_2intersect_epi32(__m512i a, __m512i b, __mmask16 *m0, __mmask16 *m1) {
+// CHECK-LABEL: test_mm512_2intersect_epi32
+// CHECK: call { <16 x i1>, <16 x i1> } @llvm.x86.avx512.vp2intersect.d.512(<16 x i32> %{{.*}}, <16 x i32> %{{.*}})
+// CHECK: extractvalue { <16 x i1>, <16 x i1> } %{{.*}}, 0
+// CHECK: extractvalue { <16 x i1>, <16 x i1> } %{{.*}}, 1
+  _mm512_2intersect_epi32(a, b, m0, m1);
+}
+
+void test_mm512_2intersect_epi64(__m512i a, __m512i b, __mmask8 *m0, __mmask8 *m1) {
+// CHECK-LABEL: test_mm512_2intersect_epi64
+// CHECK: call { <8 x i1>, <8 x i1> } @llvm.x86.avx512.vp2intersect.q.512(<8 x i64> %{{.*}}, <8 x i64> %{{.*}})
+// CHECK: extractvalue { <8 x i1>, <8 x i1> } %{{.*}}, 0
+// CHECK: extractvalue { <8 x i1>, <8 x i1> } %{{.*}}, 1
+  _mm512_2intersect_epi64(a, b, m0, m1);
+}
Index: test/CodeGen/intel-avx512vlvp2intersect.c
===
--- /dev/null
+++ test/CodeGen/intel-avx512vlvp2intersect.c
@@ -0,0 +1,36 @@
+// RUN: %clang_cc1 %s -ffreestanding -triple=x86_64-unknown-unknown -target-feature +avx512vp2intersect -target-feature +avx512vl -emit-llvm -o - -Wall -Werror | FileCheck %s
+// RUN: %clang_cc1 %s -ffreestanding -triple=i386-unknown-unknown -target-feature +avx512vp2intersect -target-feature +avx512vl -emit-llvm -o - -Wall -Werror | FileCheck %s
+
+#include 
+
+void test_mm256_2intersect_epi32(__m256i a, __m256i b, __mmask8 *m0, __mmask8 *m1) {
+// CHECK-LABEL: test_mm256_2intersect_epi32
+// CHECK: call { <8 x i1>, <8 x i1> } @llvm.x86.avx512.vp2intersect.d.256(<8 x i32> %{{.*}}, <8 x i32> %{{.*}})
+// CHECK: extractvalue { <8 x i1>, <8 x i1> } %{{.*}}, 0
+// CHECK: extractvalue { <8 x i1>, <8 x i1> } %{{.*}}, 1
+  _mm256_2intersect_epi32(a, b, m0, m1);
+}
+
+void test_mm256_2intersect_epi64(__m256i a, __m256i b, __mmask8 *m0, __mmask8 *m1) {
+// CHECK-LABEL: test_mm256_2intersect_epi64
+// CHECK: call {

[PATCH] D62367: [X86] VP2INTERSECT clang

2019-05-27 Thread Craig Topper via Phabricator via cfe-commits

craig.topper added inline comments.



Comment at: lib/Headers/avx512vlvp2intersectintrin.h:39
+
+static __inline__ void __DEFAULT_FN_ATTRS256
+_mm256_2intersect_epi32(__m256i __a, __m256i __b, __mmask8 *__m0, __mmask8 
*__m1) {

Can you add doxygen comments for the new intrinsics? @RKSimon has been asking 
for it on other reviews. I forgot to say something in our internal review.


Repository:
  rC Clang

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

https://reviews.llvm.org/D62367



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

[PATCH] D62367: [X86] VP2INTERSECT clang

2019-05-27 Thread Xiang Zhang via Phabricator via cfe-commits

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



Comment at: lib/Headers/avx512vlvp2intersectintrin.h:39
+
+static __inline__ void __DEFAULT_FN_ATTRS256
+_mm256_2intersect_epi32(__m256i __a, __m256i __b, __mmask8 *__m0, __mmask8 
*__m1) {

craig.topper wrote:
> Can you add doxygen comments for the new intrinsics? @RKSimon has been asking 
> for it on other reviews. I forgot to say something in our internal review.
OK! But I really find many ntrinsicxxx.h have no doxygen comments, Is it like 
this format: ?


```
/// Rounds up each element of the 128-bit vector of [4 x float] to an
///integer and returns the rounded values in a 128-bit vector of
///[4 x float].
///
/// \headerfile 
///
/// \code
/// __m128 _mm_ceil_ps(__m128 X);
/// \endcode
///
/// This intrinsic corresponds to the  VROUNDPS / ROUNDPS  instruction.
///
/// \param X
///A 128-bit vector of [4 x float] values to be rounded up.
/// \returns A 128-bit vector of [4 x float] containing the rounded values.
#define _mm_ceil_ps(X)   _mm_round_ps((X), _MM_FROUND_CEIL)
```



Repository:
  rC Clang

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

https://reviews.llvm.org/D62367



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

[PATCH] D62367: [X86] VP2INTERSECT clang

2019-05-27 Thread Craig Topper via Phabricator via cfe-commits

craig.topper added inline comments.



Comment at: lib/Headers/avx512vlvp2intersectintrin.h:39
+
+static __inline__ void __DEFAULT_FN_ATTRS256
+_mm256_2intersect_epi32(__m256i __a, __m256i __b, __mmask8 *__m0, __mmask8 
*__m1) {

xiangzhangllvm wrote:
> craig.topper wrote:
> > Can you add doxygen comments for the new intrinsics? @RKSimon has been 
> > asking for it on other reviews. I forgot to say something in our internal 
> > review.
> OK! But I really find many ntrinsicxxx.h have no doxygen comments, Is it like 
> this format: ?
> 
> 
> ```
> /// Rounds up each element of the 128-bit vector of [4 x float] to an
> ///integer and returns the rounded values in a 128-bit vector of
> ///[4 x float].
> ///
> /// \headerfile 
> ///
> /// \code
> /// __m128 _mm_ceil_ps(__m128 X);
> /// \endcode
> ///
> /// This intrinsic corresponds to the  VROUNDPS / ROUNDPS  instruction.
> ///
> /// \param X
> ///A 128-bit vector of [4 x float] values to be rounded up.
> /// \returns A 128-bit vector of [4 x float] containing the rounded values.
> #define _mm_ceil_ps(X)   _mm_round_ps((X), _MM_FROUND_CEIL)
> ```
> 
Yeah that looks right. Someone was kind enough to provide comments for some of 
the header files a few years ago, but it wasn't complete. I know avx512* 
especially are missing comments.


Repository:
  rC Clang

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

https://reviews.llvm.org/D62367



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

1 2 >

1 - 100 of 105 matches

Mail list logo