[PATCH] D53587: [clangd] Truncate SymbolID to 16 bytes.

[Sam McCall via Phabricator via cfe-commits]

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



Comment at: clangd/index/Serialization.cpp:303
 // A refs section has data grouped by Symbol. Each symbol has:
-//  - SymbolID: 20 bytes
+//  - SymbolID: 16 bytes
 //  - NumRefs: varint

ioeric wrote:
> ioeric wrote:
> > A reminder from yourself: bump the binary version ;)
> maybe use RawSize here instead of hardcoding?
I think it's actually more important that this is concrete than that it's 
correct :-)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53587



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

[clang-tools-extra] r345113 - [clangd] Truncate SymbolID to 16 bytes.

[Sam McCall via cfe-commits]

Author: sammccall
Date: Tue Oct 23 23:58:42 2018
New Revision: 345113

URL: http://llvm.org/viewvc/llvm-project?rev=345113&view=rev
Log:
[clangd] Truncate SymbolID to 16 bytes.

Summary:
The goal is 8 bytes, which has a nonzero risk of collisions with huge indexes.
This patch should shake out any issues with truncation at all, we can lower
further later.

Reviewers: ioeric

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

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

Modified:
clang-tools-extra/trunk/clangd/index/Index.cpp
clang-tools-extra/trunk/clangd/index/Index.h
clang-tools-extra/trunk/clangd/index/Serialization.cpp
clang-tools-extra/trunk/unittests/clangd/SerializationTests.cpp

Modified: clang-tools-extra/trunk/clangd/index/Index.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Index.cpp?rev=345113&r1=345112&r2=345113&view=diff
==
--- clang-tools-extra/trunk/clangd/index/Index.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/Index.cpp Tue Oct 23 23:58:42 2018
@@ -43,8 +43,11 @@ raw_ostream &operator<<(raw_ostream &OS,
 << "-" << L.End.line() << ":" << L.End.column() << ")";
 }
 
-SymbolID::SymbolID(StringRef USR)
-: HashValue(SHA1::hash(arrayRefFromStringRef(USR))) {}
+SymbolID::SymbolID(StringRef USR) {
+  auto Hash = SHA1::hash(arrayRefFromStringRef(USR));
+  static_assert(sizeof(Hash) >= RawSize, "RawSize larger than SHA1");
+  memcpy(HashValue.data(), Hash.data(), RawSize);
+}
 
 raw_ostream &operator<<(raw_ostream &OS, const SymbolID &ID) {
   return OS << toHex(ID.raw());

Modified: clang-tools-extra/trunk/clangd/index/Index.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Index.h?rev=345113&r1=345112&r2=345113&view=diff
==
--- clang-tools-extra/trunk/clangd/index/Index.h (original)
+++ clang-tools-extra/trunk/clangd/index/Index.h Tue Oct 23 23:58:42 2018
@@ -89,7 +89,7 @@ llvm::raw_ostream &operator<<(llvm::raw_
 // The class identifies a particular C++ symbol (class, function, method, etc).
 //
 // As USRs (Unified Symbol Resolution) could be large, especially for functions
-// with long type arguments, SymbolID is using 160-bits SHA1(USR) values to
+// with long type arguments, SymbolID is using truncated SHA1(USR) values to
 // guarantee the uniqueness of symbols while using a relatively small amount of
 // memory (vs storing USRs directly).
 //
@@ -106,13 +106,16 @@ public:
 return HashValue < Sym.HashValue;
   }
 
-  constexpr static size_t RawSize = 20;
+  // The stored hash is truncated to RawSize bytes.
+  // This trades off memory against the number of symbols we can handle.
+  // FIXME: can we reduce this further to 8 bytes?
+  constexpr static size_t RawSize = 16;
   llvm::StringRef raw() const {
 return StringRef(reinterpret_cast(HashValue.data()), 
RawSize);
   }
   static SymbolID fromRaw(llvm::StringRef);
 
-  // Returns a 40-bytes hex encoded string.
+  // Returns a hex encoded string.
   std::string str() const;
   static llvm::Expected fromStr(llvm::StringRef);
 

Modified: clang-tools-extra/trunk/clangd/index/Serialization.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Serialization.cpp?rev=345113&r1=345112&r2=345113&view=diff
==
--- clang-tools-extra/trunk/clangd/index/Serialization.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/Serialization.cpp Tue Oct 23 23:58:42 
2018
@@ -300,7 +300,7 @@ Symbol readSymbol(Reader &Data, ArrayRef
 
 // REFS ENCODING
 // A refs section has data grouped by Symbol. Each symbol has:
-//  - SymbolID: 20 bytes
+//  - SymbolID: 16 bytes
 //  - NumRefs: varint
 //  - Ref[NumRefs]
 // Fields of Ref are encoded in turn, see implementation.
@@ -338,7 +338,7 @@ std::pair> re
 // The current versioning scheme is simple - non-current versions are rejected.
 // If you make a breaking change, bump this version number to invalidate stored
 // data. Later we may want to support some backward compatibility.
-constexpr static uint32_t Version = 5;
+constexpr static uint32_t Version = 6;
 
 Expected readRIFF(StringRef Data) {
   auto RIFF = riff::readFile(Data);

Modified: clang-tools-extra/trunk/unittests/clangd/SerializationTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/SerializationTests.cpp?rev=345113&r1=345112&r2=345113&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/SerializationTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/SerializationTests.cpp Tue Oct 23 
23:58:42 2018
@@ -27,7 +27,7 @@ namespace {
 const char *YAML = R"(
 ---
 !Symbol
-ID: 057557CEBF6E6B2DD437FBF60CC58F352D1DF856
+

[PATCH] D53587: [clangd] Truncate SymbolID to 16 bytes.

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL345113: [clangd] Truncate SymbolID to 16 bytes. (authored by 
sammccall, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D53587?vs=170684&id=170817#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D53587

Files:
  clang-tools-extra/trunk/clangd/index/Index.cpp
  clang-tools-extra/trunk/clangd/index/Index.h
  clang-tools-extra/trunk/clangd/index/Serialization.cpp
  clang-tools-extra/trunk/unittests/clangd/SerializationTests.cpp

Index: clang-tools-extra/trunk/unittests/clangd/SerializationTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/SerializationTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/SerializationTests.cpp
@@ -27,7 +27,7 @@
 const char *YAML = R"(
 ---
 !Symbol
-ID: 057557CEBF6E6B2DD437FBF60CC58F352D1DF856
+ID: 057557CEBF6E6B2DD437FBF60CC58F35
 Name:   'Foo1'
 Scope:   'clang::'
 SymInfo:
@@ -53,7 +53,7 @@
 ...
 ---
 !Symbol
-ID: 057557CEBF6E6B2DD437FBF60CC58F352D1DF858
+ID: 057557CEBF6E6B2DD437FBF60CC58F36
 Name:   'Foo2'
 Scope:   'clang::'
 SymInfo:
@@ -72,7 +72,7 @@
 CompletionSnippetSuffix:'-snippet'
 ...
 !Refs
-ID: 057557CEBF6E6B2DD437FBF60CC58F352D1DF856
+ID: 057557CEBF6E6B2DD437FBF60CC58F35
 References:
   - Kind: 4
 Location:
@@ -98,15 +98,14 @@
   auto ParsedYAML = readIndexFile(YAML);
   ASSERT_TRUE(bool(ParsedYAML)) << ParsedYAML.takeError();
   ASSERT_TRUE(bool(ParsedYAML->Symbols));
-  EXPECT_THAT(
-  *ParsedYAML->Symbols,
-  UnorderedElementsAre(ID("057557CEBF6E6B2DD437FBF60CC58F352D1DF856"),
-   ID("057557CEBF6E6B2DD437FBF60CC58F352D1DF858")));
+  EXPECT_THAT(*ParsedYAML->Symbols,
+  UnorderedElementsAre(ID("057557CEBF6E6B2DD437FBF60CC58F35"),
+   ID("057557CEBF6E6B2DD437FBF60CC58F36")));
 
   auto Sym1 = *ParsedYAML->Symbols->find(
-  cantFail(SymbolID::fromStr("057557CEBF6E6B2DD437FBF60CC58F352D1DF856")));
+  cantFail(SymbolID::fromStr("057557CEBF6E6B2DD437FBF60CC58F35")));
   auto Sym2 = *ParsedYAML->Symbols->find(
-  cantFail(SymbolID::fromStr("057557CEBF6E6B2DD437FBF60CC58F352D1DF858")));
+  cantFail(SymbolID::fromStr("057557CEBF6E6B2DD437FBF60CC58F36")));
 
   EXPECT_THAT(Sym1, QName("clang::Foo1"));
   EXPECT_EQ(Sym1.Signature, "");
@@ -128,11 +127,11 @@
   EXPECT_TRUE(Sym2.Flags & Symbol::Deprecated);
 
   ASSERT_TRUE(bool(ParsedYAML->Refs));
-  EXPECT_THAT(*ParsedYAML->Refs,
-  UnorderedElementsAre(
-  Pair(cantFail(SymbolID::fromStr(
-   "057557CEBF6E6B2DD437FBF60CC58F352D1DF856")),
-   testing::SizeIs(1;
+  EXPECT_THAT(
+  *ParsedYAML->Refs,
+  UnorderedElementsAre(
+  Pair(cantFail(SymbolID::fromStr("057557CEBF6E6B2DD437FBF60CC58F35")),
+   testing::SizeIs(1;
   auto Ref1 = ParsedYAML->Refs->begin()->second.front();
   EXPECT_EQ(Ref1.Kind, RefKind::Reference);
   EXPECT_EQ(Ref1.Location.FileURI, "file:///path/foo.cc");
Index: clang-tools-extra/trunk/clangd/index/Serialization.cpp
===
--- clang-tools-extra/trunk/clangd/index/Serialization.cpp
+++ clang-tools-extra/trunk/clangd/index/Serialization.cpp
@@ -300,7 +300,7 @@
 
 // REFS ENCODING
 // A refs section has data grouped by Symbol. Each symbol has:
-//  - SymbolID: 20 bytes
+//  - SymbolID: 16 bytes
 //  - NumRefs: varint
 //  - Ref[NumRefs]
 // Fields of Ref are encoded in turn, see implementation.
@@ -338,7 +338,7 @@
 // The current versioning scheme is simple - non-current versions are rejected.
 // If you make a breaking change, bump this version number to invalidate stored
 // data. Later we may want to support some backward compatibility.
-constexpr static uint32_t Version = 5;
+constexpr static uint32_t Version = 6;
 
 Expected readRIFF(StringRef Data) {
   auto RIFF = riff::readFile(Data);
Index: clang-tools-extra/trunk/clangd/index/Index.h
===
--- clang-tools-extra/trunk/clangd/index/Index.h
+++ clang-tools-extra/trunk/clangd/index/Index.h
@@ -89,7 +89,7 @@
 // The class identifies a particular C++ symbol (class, function, method, etc).
 //
 // As USRs (Unified Symbol Resolution) could be large, especially for functions
-// with long type arguments, SymbolID is using 160-bits SHA1(USR) values to
+// with long type arguments, SymbolID is using truncated SHA1(USR) values to
 // guarantee the uniqueness of symbols while using a relatively small amount of
 // memory (vs storing USRs directly).
 //
@@ -106,13 +106,16 @@
 return HashValue < Sym.HashValue;
   }
 
-  constexpr static size_t RawSize = 20;
+  // The stored hash is truncated to RawSize bytes.
+  // This trades off memory against the number of symbols we can handle.
+  // FIXME: c

[PATCH] D53066: [Driver] Use forward slashes in most linker arguments

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added inline comments.



Comment at: lib/Driver/Driver.cpp:1013-1014
   }
+  for (auto *Str : {&Dir, &InstalledDir, &SysRoot, &DyldPrefix, &ResourceDir})
+*Str = llvm::sys::path::convert_to_slash(*Str);
 

rnk wrote:
> zturner wrote:
> > Is this going to affect the cl driver as well?
> Yeah, if we change the resource dir, I think it's going to have knock-on 
> affects to the debug info we emit to describe the locations of compiler 
> intrinsic headers. I was imagining that this would be limited to flags which 
> only get passed to GNU-ld-compatible linkers.
Well, the first minimal version that was committed in rL345004 was enough to 
produce paths that worked with libtool, but I guess that one also would cause 
some amount of changes for the cl driver.

Now in order for all the tests to pass, I had to change (almost) all of these 
as I have here. (After rechecking, I think I can spare one or two of them.)

But I do have to touch ResourceDir for 
test/Driver/linux-per-target-runtime-dir.c to pass, since it requires that a 
`-resource-dir` parameter matches a later `-L` linker flag, and I do need to 
change the `-L` linker flag for the libtool case.

So I guess it's back to the drawing board with this change then. What do you 
guys think is the path of least impact on e.g. the cl driver and PDB generation 
in general (and least messy scattered rewrites), without breaking a bunch of 
tests (e.g. test/Driver/linux-ld.c requires the `--sysroot` parameter to match 
`-L`), while still getting libtool compatible paths with forward slashes out of 
`-v`. At least when targeting mingw, but maybe ideally for any non-msvc target? 
For cross compiling for a linux target from windows, I would guess forward 
slashes would be preferred as well.


https://reviews.llvm.org/D53066



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

[clang-tools-extra] r345119 - [clangd] Embed fixes as CodeAction, instead of clangd_fixes. Clean up serialization.

[Sam McCall via cfe-commits]

Author: sammccall
Date: Wed Oct 24 00:59:38 2018
New Revision: 345119

URL: http://llvm.org/viewvc/llvm-project?rev=345119&view=rev
Log:
[clangd] Embed fixes as CodeAction, instead of clangd_fixes. Clean up 
serialization.

Summary:
CodeAction provides us with a standard way of representing fixes inline, so
use it, replacing our existing ad-hoc extension.

After this, it's easy to serialize diagnostics using the structured
toJSON/Protocol.h mechanism rather than assembling JSON ad-hoc.

Reviewers: hokein, arphaman

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

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

Modified:
clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
clang-tools-extra/trunk/clangd/Diagnostics.cpp
clang-tools-extra/trunk/clangd/Diagnostics.h
clang-tools-extra/trunk/clangd/Protocol.cpp
clang-tools-extra/trunk/clangd/Protocol.h
clang-tools-extra/trunk/test/clangd/fixits-embed-in-diagnostic.test
clang-tools-extra/trunk/unittests/clangd/ClangdUnitTests.cpp

Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=345119&r1=345118&r2=345119&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Wed Oct 24 00:59:38 2018
@@ -549,14 +549,8 @@ void ClangdLSPServer::onCodeAction(const
   std::vector Actions;
   for (const Diagnostic &D : Params.context.diagnostics) {
 for (auto &F : getFixes(Params.textDocument.uri.file(), D)) {
-  Actions.emplace_back();
-  Actions.back().title = F.Message;
-  Actions.back().kind = CodeAction::QUICKFIX_KIND;
+  Actions.push_back(toCodeAction(F, Params.textDocument.uri));
   Actions.back().diagnostics = {D};
-  Actions.back().edit.emplace();
-  Actions.back().edit->changes.emplace();
-  (*Actions.back().edit->changes)[Params.textDocument.uri.uri()] = {
-  F.Edits.begin(), F.Edits.end()};
 }
   }
 
@@ -724,36 +718,16 @@ std::vector ClangdLSPServer::getFix
 
 void ClangdLSPServer::onDiagnosticsReady(PathRef File,
  std::vector Diagnostics) {
-  json::Array DiagnosticsJSON;
-
+  URIForFile URI(File);
+  std::vector LSPDiagnostics;
   DiagnosticToReplacementMap LocalFixIts; // Temporary storage
   for (auto &Diag : Diagnostics) {
-toLSPDiags(Diag, [&](clangd::Diagnostic Diag, ArrayRef Fixes) {
-  json::Object LSPDiag({
-  {"range", Diag.range},
-  {"severity", Diag.severity},
-  {"message", Diag.message},
-  });
-  // LSP extension: embed the fixes in the diagnostic.
-  if (DiagOpts.EmbedFixesInDiagnostics && !Fixes.empty()) {
-json::Array ClangdFixes;
-for (const auto &Fix : Fixes) {
-  WorkspaceEdit WE;
-  URIForFile URI{File};
-  WE.changes = {{URI.uri(), std::vector(Fix.Edits.begin(),
-  Fix.Edits.end())}};
-  ClangdFixes.push_back(
-  json::Object{{"edit", toJSON(WE)}, {"title", Fix.Message}});
-}
-LSPDiag["clangd_fixes"] = std::move(ClangdFixes);
-  }
-  if (DiagOpts.SendDiagnosticCategory && !Diag.category.empty())
-LSPDiag["category"] = Diag.category;
-  DiagnosticsJSON.push_back(std::move(LSPDiag));
-
-  auto &FixItsForDiagnostic = LocalFixIts[Diag];
-  llvm::copy(Fixes, std::back_inserter(FixItsForDiagnostic));
-});
+toLSPDiags(Diag, URI, DiagOpts,
+   [&](clangd::Diagnostic Diag, ArrayRef Fixes) {
+ auto &FixItsForDiagnostic = LocalFixIts[Diag];
+ llvm::copy(Fixes, std::back_inserter(FixItsForDiagnostic));
+ LSPDiagnostics.push_back(std::move(Diag));
+   });
   }
 
   // Cache FixIts
@@ -766,8 +740,8 @@ void ClangdLSPServer::onDiagnosticsReady
   // Publish diagnostics.
   notify("textDocument/publishDiagnostics",
  json::Object{
- {"uri", URIForFile{File}},
- {"diagnostics", std::move(DiagnosticsJSON)},
+ {"uri", URI},
+ {"diagnostics", std::move(LSPDiagnostics)},
  });
 }
 

Modified: clang-tools-extra/trunk/clangd/Diagnostics.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Diagnostics.cpp?rev=345119&r1=345118&r2=345119&view=diff
==
--- clang-tools-extra/trunk/clangd/Diagnostics.cpp (original)
+++ clang-tools-extra/trunk/clangd/Diagnostics.cpp Wed Oct 24 00:59:38 2018
@@ -227,19 +227,37 @@ raw_ostream &operator<<(raw_ostream &OS,
   return OS;
 }
 
-void toLSPDiags(const Diag &D,
-function_ref)> OutFn) {
+CodeAction toCodeAction(const Fix &F, const URIForFile &File) {
+  CodeAction Action;
+  Ac

[PATCH] D53391: [clangd] Embed fixes as CodeAction, instead of clangd_fixes. Clean up serialization.

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL345119: [clangd] Embed fixes as CodeAction, instead of 
clangd_fixes. Clean up… (authored by sammccall, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D53391?vs=170153&id=170824#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D53391

Files:
  clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
  clang-tools-extra/trunk/clangd/Diagnostics.cpp
  clang-tools-extra/trunk/clangd/Diagnostics.h
  clang-tools-extra/trunk/clangd/Protocol.cpp
  clang-tools-extra/trunk/clangd/Protocol.h
  clang-tools-extra/trunk/test/clangd/fixits-embed-in-diagnostic.test
  clang-tools-extra/trunk/unittests/clangd/ClangdUnitTests.cpp

Index: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
===
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
@@ -549,14 +549,8 @@
   std::vector Actions;
   for (const Diagnostic &D : Params.context.diagnostics) {
 for (auto &F : getFixes(Params.textDocument.uri.file(), D)) {
-  Actions.emplace_back();
-  Actions.back().title = F.Message;
-  Actions.back().kind = CodeAction::QUICKFIX_KIND;
+  Actions.push_back(toCodeAction(F, Params.textDocument.uri));
   Actions.back().diagnostics = {D};
-  Actions.back().edit.emplace();
-  Actions.back().edit->changes.emplace();
-  (*Actions.back().edit->changes)[Params.textDocument.uri.uri()] = {
-  F.Edits.begin(), F.Edits.end()};
 }
   }
 
@@ -724,36 +718,16 @@
 
 void ClangdLSPServer::onDiagnosticsReady(PathRef File,
  std::vector Diagnostics) {
-  json::Array DiagnosticsJSON;
-
+  URIForFile URI(File);
+  std::vector LSPDiagnostics;
   DiagnosticToReplacementMap LocalFixIts; // Temporary storage
   for (auto &Diag : Diagnostics) {
-toLSPDiags(Diag, [&](clangd::Diagnostic Diag, ArrayRef Fixes) {
-  json::Object LSPDiag({
-  {"range", Diag.range},
-  {"severity", Diag.severity},
-  {"message", Diag.message},
-  });
-  // LSP extension: embed the fixes in the diagnostic.
-  if (DiagOpts.EmbedFixesInDiagnostics && !Fixes.empty()) {
-json::Array ClangdFixes;
-for (const auto &Fix : Fixes) {
-  WorkspaceEdit WE;
-  URIForFile URI{File};
-  WE.changes = {{URI.uri(), std::vector(Fix.Edits.begin(),
-  Fix.Edits.end())}};
-  ClangdFixes.push_back(
-  json::Object{{"edit", toJSON(WE)}, {"title", Fix.Message}});
-}
-LSPDiag["clangd_fixes"] = std::move(ClangdFixes);
-  }
-  if (DiagOpts.SendDiagnosticCategory && !Diag.category.empty())
-LSPDiag["category"] = Diag.category;
-  DiagnosticsJSON.push_back(std::move(LSPDiag));
-
-  auto &FixItsForDiagnostic = LocalFixIts[Diag];
-  llvm::copy(Fixes, std::back_inserter(FixItsForDiagnostic));
-});
+toLSPDiags(Diag, URI, DiagOpts,
+   [&](clangd::Diagnostic Diag, ArrayRef Fixes) {
+ auto &FixItsForDiagnostic = LocalFixIts[Diag];
+ llvm::copy(Fixes, std::back_inserter(FixItsForDiagnostic));
+ LSPDiagnostics.push_back(std::move(Diag));
+   });
   }
 
   // Cache FixIts
@@ -766,8 +740,8 @@
   // Publish diagnostics.
   notify("textDocument/publishDiagnostics",
  json::Object{
- {"uri", URIForFile{File}},
- {"diagnostics", std::move(DiagnosticsJSON)},
+ {"uri", URI},
+ {"diagnostics", std::move(LSPDiagnostics)},
  });
 }
 
Index: clang-tools-extra/trunk/clangd/Diagnostics.cpp
===
--- clang-tools-extra/trunk/clangd/Diagnostics.cpp
+++ clang-tools-extra/trunk/clangd/Diagnostics.cpp
@@ -227,19 +227,37 @@
   return OS;
 }
 
-void toLSPDiags(const Diag &D,
-function_ref)> OutFn) {
+CodeAction toCodeAction(const Fix &F, const URIForFile &File) {
+  CodeAction Action;
+  Action.title = F.Message;
+  Action.kind = CodeAction::QUICKFIX_KIND;
+  Action.edit.emplace();
+  Action.edit->changes.emplace();
+  (*Action.edit->changes)[File.uri()] = {F.Edits.begin(), F.Edits.end()};
+  return Action;
+}
+
+void toLSPDiags(
+const Diag &D, const URIForFile &File, const ClangdDiagnosticOptions &Opts,
+function_ref)> OutFn) {
   auto FillBasicFields = [](const DiagBase &D) -> clangd::Diagnostic {
 clangd::Diagnostic Res;
 Res.range = D.Range;
 Res.severity = getSeverity(D.Severity);
-Res.category = D.Category;
 return Res;
   };
 
   {
 clangd::Diagnostic Main = FillBasicFields(D);
 Main.message = mainMessage(D);
+if (Opts.EmbedFixesInDiagnostics) {
+  Main.codeActions.emplace();
+  for (const auto &Fix : D.Fixes)
+

[PATCH] D53191: [clang] Introduce new completion context types

[Kadir Cetinkaya via Phabricator via cfe-commits]

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



Comment at: include/clang/Sema/CodeCompleteConsumer.h:277
+
+/// Code completion occurred where an existing name is expected.
+CCC_ExistingName,

sammccall wrote:
> It's not obvious what "name" means here, e.g. what distinguishes this from 
> "expression".
> 
> I think the answer is "we want the name of *something*, but we don't know 
> what". I think it's ok to say that explicitly. Or to call this CCC_Symbol, 
> etc.
Renaming to CCC_Symbol



Comment at: include/clang/Sema/CodeCompleteConsumer.h:286
+/// qualified name is permissible.
+CCC_PotentiallyQualifiedExistingName,
 

sammccall wrote:
> what distinguishes potentiallyqualifiedexistingname from existingname?
> 
> The one use of ExistingName is the invalid scope case. This seems a) pretty 
> marginal, and b) not to matter to clangd. Does it matter to libclang?
> 
> (The behavior is detectable through the API, but the current behavior doesn't 
> seem that sensible or obviously important)
> 
It kind of matters to libclang, but it is not that important as well. It is 
used when caching completion items, to decide whether they allow a name 
specifiers or not. But when I think about the use case, I believe nested name 
specifiers should be allowed in that context(when scope is invalid, but we 
still report for global code completion). So seems like it is redundant(not 
exactly, but currently we have no way of telling it apart due to mentioned 
fixme).


Repository:
  rC Clang

https://reviews.llvm.org/D53191



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

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

[Whisperity via Phabricator via cfe-commits]

whisperity added inline comments.



Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h:238
+  /// message on that constraint being changed.
+  bool isChangedOrInsertConstraint(ConstraintMap &Constraints, const Stmt 
*Cond,
+   StringRef Message) const {

`insertOrUpdateContraintMessage`

and mention in the documentation that it returns whether or not the actual 
insertion or update change took place


https://reviews.llvm.org/D53076



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

r345121 - [bash-autocompletion] Fix bug when a flag ends with '='

[Yuka Takahashi via cfe-commits]

Author: yamaguchi
Date: Wed Oct 24 01:24:16 2018
New Revision: 345121

URL: http://llvm.org/viewvc/llvm-project?rev=345121&view=rev
Log:
[bash-autocompletion] Fix bug when a flag ends with '='

There was a bug that when a flag ends with '=' and no value was suggested,
clang autocompletes the flag itself.
For example, in bash, it looked like this:
```
$ clang -fmodule-file=[tab]
-> $clang -fmodule-file=-fmodule-file
```
This is not what we expect. We expect a file autocompletion when no value
was found. With this patch, pressing tab suggests files in the current
directory.

Reviewers: teemperor, ruiu

Subscribers: cfe-commits

Modified:
cfe/trunk/lib/Driver/Driver.cpp
cfe/trunk/test/Driver/autocomplete.c

Modified: cfe/trunk/lib/Driver/Driver.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Driver.cpp?rev=345121&r1=345120&r2=345121&view=diff
==
--- cfe/trunk/lib/Driver/Driver.cpp (original)
+++ cfe/trunk/lib/Driver/Driver.cpp Wed Oct 24 01:24:16 2018
@@ -1534,7 +1534,9 @@ void Driver::HandleAutocompletions(Strin
   if (SuggestedCompletions.empty())
 SuggestedCompletions = Opts->suggestValueCompletions(Cur, "");
 
-  if (SuggestedCompletions.empty()) {
+  // When flag ends with '=' and there was no value completion, return empty
+  // string and fall back to the file autocompletion.
+  if (SuggestedCompletions.empty() && !Cur.endswith("=")) {
 // If the flag is in the form of "--autocomplete=-foo",
 // we were requested to print out all option names that start with "-foo".
 // For example, "--autocomplete=-fsyn" is expanded to "-fsyntax-only".

Modified: cfe/trunk/test/Driver/autocomplete.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/autocomplete.c?rev=345121&r1=345120&r2=345121&view=diff
==
--- cfe/trunk/test/Driver/autocomplete.c (original)
+++ cfe/trunk/test/Driver/autocomplete.c Wed Oct 24 01:24:16 2018
@@ -115,3 +115,9 @@
 // Check if they can autocomplete values with coron
 // RUN: %clang --autocomplete=foo,bar,,,-fno-sanitize-coverage=,f | FileCheck 
%s -check-prefix=FNOSANICOVER-CORON
 // FNOSANICOVER-CORON: func
+
+// Clang should return empty string when no value completion was found, which 
will fall back to file autocompletion
+// RUN: %clang --autocomplete=-fmodule-file= | FileCheck %s 
-check-prefix=MODULE_FILE_EQUAL
+// MODULE_FILE_EQUAL-NOT: -fmodule-file=
+// RUN: %clang --autocomplete=-fmodule-file | FileCheck %s 
-check-prefix=MODULE_FILE
+// MODULE_FILE: -fmodule-file=


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

[PATCH] D53191: [clang] Introduce new completion context types

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Logic looks good. Any way we can exercise it with a test via c-index-test?




Comment at: include/clang/Sema/CodeCompleteConsumer.h:281
+
+/// Code completion occurred where an existing symbol is expected.
+CCC_Symbol,

nit: try to describe what a symbol is in the comment, rather than just 
repeating the term?
e.g. "where an existing name is expected (such as a type, function, or 
variable)"


Repository:
  rC Clang

https://reviews.llvm.org/D53191



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

r345122 - [Sema] Do not show unused parameter warnings when body is skipped

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Wed Oct 24 01:29:24 2018
New Revision: 345122

URL: http://llvm.org/viewvc/llvm-project?rev=345122&view=rev
Log:
[Sema] Do not show unused parameter warnings when body is skipped

Summary: Without the function body, we cannot determine is parameter was used.

Reviewers: ioeric, sammccall

Reviewed By: sammccall

Subscribers: arphaman, cfe-commits

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

Added:
cfe/trunk/test/Index/skipped-bodies-unused.cpp
Modified:
cfe/trunk/lib/Sema/SemaDecl.cpp

Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=345122&r1=345121&r2=345122&view=diff
==
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Wed Oct 24 01:29:24 2018
@@ -13037,7 +13037,7 @@ Decl *Sema::ActOnFinishFunctionBody(Decl
 
 if (!FD->isInvalidDecl()) {
   // Don't diagnose unused parameters of defaulted or deleted functions.
-  if (!FD->isDeleted() && !FD->isDefaulted())
+  if (!FD->isDeleted() && !FD->isDefaulted() && !FD->hasSkippedBody())
 DiagnoseUnusedParameters(FD->parameters());
   DiagnoseSizeOfParametersAndReturnValue(FD->parameters(),
  FD->getReturnType(), FD);
@@ -13132,7 +13132,8 @@ Decl *Sema::ActOnFinishFunctionBody(Decl
 assert(MD == getCurMethodDecl() && "Method parsing confused");
 MD->setBody(Body);
 if (!MD->isInvalidDecl()) {
-  DiagnoseUnusedParameters(MD->parameters());
+  if (!MD->hasSkippedBody())
+DiagnoseUnusedParameters(MD->parameters());
   DiagnoseSizeOfParametersAndReturnValue(MD->parameters(),
  MD->getReturnType(), MD);
 

Added: cfe/trunk/test/Index/skipped-bodies-unused.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/skipped-bodies-unused.cpp?rev=345122&view=auto
==
--- cfe/trunk/test/Index/skipped-bodies-unused.cpp (added)
+++ cfe/trunk/test/Index/skipped-bodies-unused.cpp Wed Oct 24 01:29:24 2018
@@ -0,0 +1,8 @@
+// RUN: env CINDEXTEST_SKIP_FUNCTION_BODIES=1 c-index-test -test-load-source 
all %s -Wunused-parameter 2>&1 \
+// RUN: | FileCheck %s
+
+// No 'unused parameter' warnings should be shown when skipping the function 
bodies.
+inline int foo(int used, int unused) {
+used = 100;
+}
+// CHECK-NOT: warning: unused parameter


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

[PATCH] D53456: [Sema] Do not show unused parameter warnings when body is skipped

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC345122: [Sema] Do not show unused parameter warnings when 
body is skipped (authored by ibiryukov, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D53456?vs=170268&id=170827#toc

Repository:
  rC Clang

https://reviews.llvm.org/D53456

Files:
  lib/Sema/SemaDecl.cpp
  test/Index/skipped-bodies-unused.cpp


Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -13037,7 +13037,7 @@
 
 if (!FD->isInvalidDecl()) {
   // Don't diagnose unused parameters of defaulted or deleted functions.
-  if (!FD->isDeleted() && !FD->isDefaulted())
+  if (!FD->isDeleted() && !FD->isDefaulted() && !FD->hasSkippedBody())
 DiagnoseUnusedParameters(FD->parameters());
   DiagnoseSizeOfParametersAndReturnValue(FD->parameters(),
  FD->getReturnType(), FD);
@@ -13132,7 +13132,8 @@
 assert(MD == getCurMethodDecl() && "Method parsing confused");
 MD->setBody(Body);
 if (!MD->isInvalidDecl()) {
-  DiagnoseUnusedParameters(MD->parameters());
+  if (!MD->hasSkippedBody())
+DiagnoseUnusedParameters(MD->parameters());
   DiagnoseSizeOfParametersAndReturnValue(MD->parameters(),
  MD->getReturnType(), MD);
 
Index: test/Index/skipped-bodies-unused.cpp
===
--- test/Index/skipped-bodies-unused.cpp
+++ test/Index/skipped-bodies-unused.cpp
@@ -0,0 +1,8 @@
+// RUN: env CINDEXTEST_SKIP_FUNCTION_BODIES=1 c-index-test -test-load-source 
all %s -Wunused-parameter 2>&1 \
+// RUN: | FileCheck %s
+
+// No 'unused parameter' warnings should be shown when skipping the function 
bodies.
+inline int foo(int used, int unused) {
+used = 100;
+}
+// CHECK-NOT: warning: unused parameter


Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -13037,7 +13037,7 @@
 
 if (!FD->isInvalidDecl()) {
   // Don't diagnose unused parameters of defaulted or deleted functions.
-  if (!FD->isDeleted() && !FD->isDefaulted())
+  if (!FD->isDeleted() && !FD->isDefaulted() && !FD->hasSkippedBody())
 DiagnoseUnusedParameters(FD->parameters());
   DiagnoseSizeOfParametersAndReturnValue(FD->parameters(),
  FD->getReturnType(), FD);
@@ -13132,7 +13132,8 @@
 assert(MD == getCurMethodDecl() && "Method parsing confused");
 MD->setBody(Body);
 if (!MD->isInvalidDecl()) {
-  DiagnoseUnusedParameters(MD->parameters());
+  if (!MD->hasSkippedBody())
+DiagnoseUnusedParameters(MD->parameters());
   DiagnoseSizeOfParametersAndReturnValue(MD->parameters(),
  MD->getReturnType(), MD);
 
Index: test/Index/skipped-bodies-unused.cpp
===
--- test/Index/skipped-bodies-unused.cpp
+++ test/Index/skipped-bodies-unused.cpp
@@ -0,0 +1,8 @@
+// RUN: env CINDEXTEST_SKIP_FUNCTION_BODIES=1 c-index-test -test-load-source all %s -Wunused-parameter 2>&1 \
+// RUN: | FileCheck %s
+
+// No 'unused parameter' warnings should be shown when skipping the function bodies.
+inline int foo(int used, int unused) {
+used = 100;
+}
+// CHECK-NOT: warning: unused parameter
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53595: [C++17] Reject shadowing of capture by parameter in lambda

[Nicolas Lesser via Phabricator via cfe-commits]

Rakete updated this revision to Diff 170828.
Rakete added a comment.

Addresed review comments :)

I updated the dr status file but a lot of unrelated changes made it in. Is this 
okay?


Repository:
  rC Clang

https://reviews.llvm.org/D53595

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/Sema.h
  lib/Sema/SemaLambda.cpp
  test/CXX/drs/dr22xx.cpp
  test/SemaCXX/warn-shadow-in-lambdas.cpp
  www/cxx_dr_status.html

Index: www/cxx_dr_status.html
===
--- www/cxx_dr_status.html
+++ www/cxx_dr_status.html
@@ -7437,11 +7437,11 @@
 Brace elision in array temporary initialization
 Unknown
   
-  
-http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#1271";>1271
-drafting
+  
+http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#1271";>1271
+DR
 Imprecise wording regarding dependent types
-Not resolved
+Unknown
   
   
 http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_closed.html#1272";>1272
@@ -11147,7 +11147,7 @@
   
   
 http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#1889";>1889
-open
+drafting
 Unclear effect of #pragma on conformance
 Not resolved
   
@@ -11170,8 +11170,8 @@
 Unknown
   
   
-http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#1893";>1893
-tentatively ready
+http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#1893";>1893
+DR
 Function-style cast with braced-init-lists and empty pack expansions
 Unknown
   
@@ -11247,11 +11247,11 @@
 Dependent types and injected-class-names
 Unknown
   
-  
-http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#1906";>1906
-review
+  
+http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_closed.html#1906";>1906
+NAD
 Name lookup in member friend declaration
-Not resolved
+Unknown
   
   
 http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#1907";>1907
@@ -11272,8 +11272,8 @@
 Yes
   
   
-http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#1910";>1910
-tentatively ready
+http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#1910";>1910
+DR
 “Shall” requirement applied to runtime behavior
 Unknown
   
@@ -11710,8 +11710,8 @@
 Unknown
   
   
-http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#1983";>1983
-tentatively ready
+http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#1983";>1983
+DR
 Inappropriate use of virt-specifier
 Unknown
   
@@ -12166,8 +12166,8 @@
 Not resolved
   
   
-http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#2059";>2059
-tentatively ready
+http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#2059";>2059
+DR
 Linkage and deduced return types
 Unknown
   
@@ -12298,8 +12298,8 @@
 Not resolved
   
   
-http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#2081";>2081
-tentatively ready
+http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#2081";>2081
+DR
 Deduced return type in redeclaration or specialization of function template
 Unknown
   
@@ -12340,8 +12340,8 @@
 Not resolved
   
   
-http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#2088";>2088
-tentatively ready
+http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#2088";>2088
+DR
 Late tiebreakers in partial ordering
 Unknown
   
@@ -12364,8 +12364,8 @@
 Unknown
   
   
-http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#2092";>2092
-tentatively ready
+http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#2092";>2092
+DR
 Deduction failure and overload resolution
 Unknown
   
@@ -12796,8 +12796,8 @@
 Unknown
   
   
-http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#2164";>2164
-tentatively ready
+http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#2164";>2164
+DR
 Name hiding and using-directives
 Unknown
   
@@ -12935,7 +12935,7 @@
   
   
 http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#2187";>2187
-open
+drafting
 Protected members and access via qualified-id
 Not resolved
   
@@ -13025,7 +13025,7 @@
   
   
 http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#2202";>2202
-open
+drafting
 When does default argument instantiation occur?
 Not resolved
   
@@ -13081,7 +13081,7 @@
 http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#2211";>2211
 C++17
 Hiding by lambda captures and parameters
-Unknown
+SVN
   
   
 http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#2212";>2212
@@ -13127,7 +13127,7 @@
   
   
 http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#2219";>2219
-open
+drafting
 Dynamically-unreachable handlers
 Not resolved
   
@@ -13168,14 +13168,14 @@
 Un

[clang-tools-extra] r345125 - [clangd] Fix a link in documentation. NFC

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Wed Oct 24 02:00:30 2018
New Revision: 345125

URL: http://llvm.org/viewvc/llvm-project?rev=345125&view=rev
Log:
[clangd] Fix a link in documentation. NFC

Modified:
clang-tools-extra/trunk/docs/clangd.rst

Modified: clang-tools-extra/trunk/docs/clangd.rst
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clangd.rst?rev=345125&r1=345124&r2=345125&view=diff
==
--- clang-tools-extra/trunk/docs/clangd.rst (original)
+++ clang-tools-extra/trunk/docs/clangd.rst Wed Oct 24 02:00:30 2018
@@ -113,7 +113,7 @@ Editor Integration
 
 Any full-featured Language Server Protocol Client implementation should work
 with :program:`Clangd`. This `list
-` contains information about
+`_ contains information about
 extensions and plugins that are known to work for different editors.
 
 Vim Integration


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

[PATCH] D53191: [clang] Introduce new completion context types

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 170829.
kadircet marked an inline comment as done.
kadircet added a comment.

- Update comment.


Repository:
  rC Clang

https://reviews.llvm.org/D53191

Files:
  include/clang/Sema/CodeCompleteConsumer.h
  lib/Frontend/ASTUnit.cpp
  lib/Sema/CodeCompleteConsumer.cpp
  lib/Sema/SemaCodeComplete.cpp
  tools/libclang/CIndexCodeCompletion.cpp

Index: tools/libclang/CIndexCodeCompletion.cpp
===
--- tools/libclang/CIndexCodeCompletion.cpp
+++ tools/libclang/CIndexCodeCompletion.cpp
@@ -487,7 +487,8 @@
   contexts = CXCompletionContext_Namespace;
   break;
 }
-case CodeCompletionContext::CCC_PotentiallyQualifiedName: {
+case CodeCompletionContext::CCC_SymbolOrNewName:
+case CodeCompletionContext::CCC_Symbol: {
   contexts = CXCompletionContext_NestedNameSpecifier;
   break;
 }
@@ -539,7 +540,7 @@
 case CodeCompletionContext::CCC_Other:
 case CodeCompletionContext::CCC_ObjCInterface:
 case CodeCompletionContext::CCC_ObjCImplementation:
-case CodeCompletionContext::CCC_Name:
+case CodeCompletionContext::CCC_NewName:
 case CodeCompletionContext::CCC_MacroName:
 case CodeCompletionContext::CCC_PreprocessorExpression:
 case CodeCompletionContext::CCC_PreprocessorDirective:
Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -3748,11 +3748,14 @@
 bool AllowNonIdentifiers,
 bool AllowNestedNameSpecifiers) {
   typedef CodeCompletionResult Result;
-  ResultBuilder Results(*this, CodeCompleter->getAllocator(),
-CodeCompleter->getCodeCompletionTUInfo(),
-AllowNestedNameSpecifiers
-  ? CodeCompletionContext::CCC_PotentiallyQualifiedName
-  : CodeCompletionContext::CCC_Name);
+  ResultBuilder Results(
+  *this, CodeCompleter->getAllocator(),
+  CodeCompleter->getCodeCompletionTUInfo(),
+  AllowNestedNameSpecifiers
+  // FIXME: Try to separate codepath leading here to deduce whether we
+  // need an existing symbol or a new one.
+  ? CodeCompletionContext::CCC_SymbolOrNewName
+  : CodeCompletionContext::CCC_NewName);
   Results.EnterNewScope();
 
   // Type qualifiers can come after names.
@@ -4833,7 +4836,7 @@
   // it can be useful for global code completion which have information about
   // contexts/symbols that are not in the AST.
   if (SS.isInvalid()) {
-CodeCompletionContext CC(CodeCompletionContext::CCC_Name);
+CodeCompletionContext CC(CodeCompletionContext::CCC_Symbol);
 CC.setCXXScopeSpecifier(SS);
 HandleCodeCompleteResults(this, CodeCompleter, CC, nullptr, 0);
 return;
@@ -4851,7 +4854,7 @@
 
   ResultBuilder Results(*this, CodeCompleter->getAllocator(),
 CodeCompleter->getCodeCompletionTUInfo(),
-CodeCompletionContext::CCC_Name);
+CodeCompletionContext::CCC_Symbol);
   Results.EnterNewScope();
 
   // The "template" keyword can follow "::" in the grammar, but only
@@ -4891,7 +4894,10 @@
 
   ResultBuilder Results(*this, CodeCompleter->getAllocator(),
 CodeCompleter->getCodeCompletionTUInfo(),
-CodeCompletionContext::CCC_PotentiallyQualifiedName,
+// This can be both a using alias or using
+// declaration, in the former we expect a new name and a
+// symbol in the latter case.
+CodeCompletionContext::CCC_SymbolOrNewName,
 &ResultBuilder::IsNestedNameSpecifier);
   Results.EnterNewScope();
 
@@ -5043,7 +5049,7 @@
 
   ResultBuilder Results(*this, CodeCompleter->getAllocator(),
 CodeCompleter->getCodeCompletionTUInfo(),
-CodeCompletionContext::CCC_PotentiallyQualifiedName);
+CodeCompletionContext::CCC_Symbol);
   Results.EnterNewScope();
 
   // Fill in any already-initialized fields or base classes.
Index: lib/Sema/CodeCompleteConsumer.cpp
===
--- lib/Sema/CodeCompleteConsumer.cpp
+++ lib/Sema/CodeCompleteConsumer.cpp
@@ -49,6 +49,8 @@
   case CCC_Expression:
   case CCC_ObjCMessageReceiver:
   case CCC_ParenthesizedExpression:
+  case CCC_Symbol:
+  case CCC_SymbolOrNewName:
 return true;
 
   case CCC_TopLevel:
@@ -65,8 +67,7 @@
   case CCC_ObjCProtocolName:
   case CCC_Namespace:
   case CCC_Type:
-  case CCC_Name:
-  case CCC_PotentiallyQualifiedName:
+  case CCC_NewName:
   case CCC_MacroName:
   case CCC_MacroNameUse:
   case CCC_PreprocessorExpression:
@@ -128,10 +129,12 @@
 return "Namespace";
   case CCKind::CCC_Type:

[PATCH] D53191: [clang] Introduce new completion context types

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

Since it doesn't change any CXCompletionContext's in CIndex it doesn't seem 
possible. There are tests being added to 
clangd(https://reviews.llvm.org/D53192), since it uses context provided by sema 
to decide whether query the index or not, we can check the behavior.


Repository:
  rC Clang

https://reviews.llvm.org/D53191



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

[PATCH] D53339: [clang-tidy] Add the abseil-duration-factory-float check

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

looks good, just a few nits.




Comment at: clang-tidy/abseil/AbseilTidyModule.cpp:32
+CheckFactories.registerCheck(
+"abseil-duration-factory-float");
 CheckFactories.registerCheck(

Maybe drop the `factory`? we already have a duration-related check  
`abseil-duration-division`, for consistency. 

`clang-tidy/rename_check.py` may help you.



Comment at: clang-tidy/abseil/DurationFactoryFloatCheck.cpp:94
+  diag(MatchedCall->getBeginLoc(),
+   (llvm::Twine("Use integer version of absl::") +
+MatchedCall->getDirectCallee()->getName())

nit: clang-tidy message is not a complete sentence, use lower letter `use`.


https://reviews.llvm.org/D53339



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

[PATCH] D53406: [clangd] Provide excuses for bad code completions, based on diagnostics. C++ API only.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clangd/CodeComplete.cpp:700
+ unsigned DiagLoc = Loc.second;
+ if (DiagLoc < StartOfLine || DiagLoc > Offset)
+   return;

kadircet wrote:
> There are also a lot of cases where we can't find an include file(usually due 
> to confused compile commands) and fail miserably. Maybe in addition to 
> checking current line, we could also check for lines that are starting with 
> `#include` or diags of type `diag::err_pp_file_not_found` ?
Good point!
It's even more complicated than that: some of those missing files might be in 
preamble.
This will lead to preamble being incomplete. The problem with that is that 
DiagnosticsConsumer we create for completion will not see those errors (we have 
a separate consumer instance for preamble).
So we also need to a way to keep the reason why preamble is broken in the 
preamble itself.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53406



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

[PATCH] D52448: [clang-format] Break before next parameter after a formatted multiline raw string parameter

[Sam McCall via Phabricator via cfe-commits]

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



Comment at: lib/Format/ContinuationIndenter.cpp:1594
   StartColumn + NewPrefixSize - Style.ColumnLimit : 0;
-  return Fixes.second + PrefixExcessCharacters * Style.PenaltyExcessCharacter;
+  unsigned Penalty =
+  Fixes.second + PrefixExcessCharacters * Style.PenaltyExcessCharacter;

nit: why are you doing the multiline side-effect between computing the penalty 
and returning it?


Repository:
  rC Clang

https://reviews.llvm.org/D52448



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

[PATCH] D53334: [Frontend] Show diagnostics on prebuilt module configuration mismatch too

[Whisperity via Phabricator via cfe-commits]

whisperity added a comment.

@dblaikie I have created a test, but unfortunately `%clang_cpp` in LIT invokes 
`clang --driver-mode=cpp` which is not the same as if `clang++` is called. I'm 
trying to construct the following command-line

`clang++ -fmodules-ts -fprebuilt-module-path=%t/mods --precompile -c file.cppm 
-o file.pcm`

However, using `%clang_cc1` I can't get it to accept `--precompile` as a valid 
argument, and using `%clang_cpp` I get an "unused argument" warning for 
`--precompile` and the output file is just a preprocessed output (like `-E`), 
which will, of course, cause errors, but not the errors I am wanting to test 
about.


Repository:
  rC Clang

https://reviews.llvm.org/D53334



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

[PATCH] D53406: [clangd] Provide excuses for bad code completions, based on diagnostics. C++ API only.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

All these things are good ideas - I'm not addressing comments yet because I'm 
waiting for feedback from the team that's trying this out - maybe this needs 
patches, or maybe it'll never work.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53406



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

[clang-tools-extra] r345126 - [clangd] Remove outdated comment-out code. NFC

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Wed Oct 24 02:47:24 2018
New Revision: 345126

URL: http://llvm.org/viewvc/llvm-project?rev=345126&view=rev
Log:
[clangd] Remove outdated comment-out code. NFC

Modified:
clang-tools-extra/trunk/clangd/ClangdLSPServer.h

Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.h?rev=345126&r1=345125&r2=345126&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.h (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.h Wed Oct 24 02:47:24 2018
@@ -140,7 +140,6 @@ private:
 
 // if IsDirectoryBased is true, an instance of InMemoryCDB.
 // If IsDirectoryBased is false, an instance of DirectoryBasedCDB.
-// unique_ptr CDB;
 std::unique_ptr CDB;
 bool IsDirectoryBased;
   };


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

[PATCH] D53334: [Frontend/Modules] Show diagnostics on prebuilt module configuration mismatch too

[Whisperity via Phabricator via cfe-commits]

whisperity updated this revision to Diff 170832.
whisperity retitled this revision from "[Frontend] Show diagnostics on prebuilt 
module configuration mismatch too" to "[Frontend/Modules] Show diagnostics on 
prebuilt module configuration mismatch too".
whisperity added a comment.
Herald added a subscriber: jfb.

Updating the diff just in case so that I don't lose the test code.


Repository:
  rC Clang

https://reviews.llvm.org/D53334

Files:
  lib/Frontend/CompilerInstance.cpp
  test/Modules/Inputs/module-mismatch.cppm
  test/Modules/mismatch-diagnostics.cpp


Index: test/Modules/mismatch-diagnostics.cpp
===
--- /dev/null
+++ test/Modules/mismatch-diagnostics.cpp
@@ -0,0 +1,29 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t/prebuilt_modules
+//
+// RUN: exit 0
+//
+// RUN: %clang_cpp -triple %itanium_abi_triple \
+// RUN: -fmodules-ts -fprebuilt-module-path=%t/prebuilt-modules \
+// RUN: -pthread -DHAS_PTHREAD=1 \
+// RUN: --precompile -c %S/Inputs/module-mismatch.cppm \
+// RUN: -o %t/prebuilt_modules/module_mismatch.pcm
+//
+// RUN: %clang_cpp -triple %itanium_abi_triple \
+// RUN: -fmodules-ts -fprebuilt-module-path=%t/prebuilt-modules \
+// RUN: --precompile -c %S/Inputs/module-mismatch.cppm \
+// RUN: -o %t/prebuilt_modules/module_no_mismatch.pcm
+//
+// RUN: %clang_cc1 -triple %itanium_abi_triple -fmodules-ts \
+// RUN: -fprebuilt-module-path=%t/prebuilt_modules -DMISMATCH_CHCEK=1 %s
+//
+// RUN: %clang_cc1 -triple %itanium_abi_triple -fmodules-ts \
+// RUN: -fprebuilt-module-path=%t/prebuilt_modules %s
+
+#ifdef MISMATCH_CHECK
+import module_mismatch;
+// expected-error {{foo}}
+#else
+import module_no_mismatch; // no-warning
+#endif
+
Index: test/Modules/Inputs/module-mismatch.cppm
===
--- /dev/null
+++ test/Modules/Inputs/module-mismatch.cppm
@@ -0,0 +1,13 @@
+#ifdef HAS_PTHREAD
+export module module_mismatch;
+#else
+export module module_no_mismatch;
+#endif
+
+export bool hasPthreads() {
+#ifdef HAS_PTHREAD
+return true;
+#else
+return false;
+#endif
+}
Index: lib/Frontend/CompilerInstance.cpp
===
--- lib/Frontend/CompilerInstance.cpp
+++ lib/Frontend/CompilerInstance.cpp
@@ -1736,7 +1736,9 @@
 // module cache, we don't know how to rebuild modules.
 unsigned ARRFlags = Source == ModuleCache ?
 ASTReader::ARR_OutOfDate | ASTReader::ARR_Missing :
-ASTReader::ARR_ConfigurationMismatch;
+Source == PrebuiltModulePath ?
+0 :
+ASTReader::ARR_ConfigurationMismatch;
 switch (ModuleManager->ReadAST(ModuleFileName,
Source == PrebuiltModulePath
? serialization::MK_PrebuiltModule


Index: test/Modules/mismatch-diagnostics.cpp
===
--- /dev/null
+++ test/Modules/mismatch-diagnostics.cpp
@@ -0,0 +1,29 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t/prebuilt_modules
+//
+// RUN: exit 0
+//
+// RUN: %clang_cpp -triple %itanium_abi_triple \
+// RUN: -fmodules-ts -fprebuilt-module-path=%t/prebuilt-modules \
+// RUN: -pthread -DHAS_PTHREAD=1 \
+// RUN: --precompile -c %S/Inputs/module-mismatch.cppm \
+// RUN: -o %t/prebuilt_modules/module_mismatch.pcm
+//
+// RUN: %clang_cpp -triple %itanium_abi_triple \
+// RUN: -fmodules-ts -fprebuilt-module-path=%t/prebuilt-modules \
+// RUN: --precompile -c %S/Inputs/module-mismatch.cppm \
+// RUN: -o %t/prebuilt_modules/module_no_mismatch.pcm
+//
+// RUN: %clang_cc1 -triple %itanium_abi_triple -fmodules-ts \
+// RUN: -fprebuilt-module-path=%t/prebuilt_modules -DMISMATCH_CHCEK=1 %s
+//
+// RUN: %clang_cc1 -triple %itanium_abi_triple -fmodules-ts \
+// RUN: -fprebuilt-module-path=%t/prebuilt_modules %s
+
+#ifdef MISMATCH_CHECK
+import module_mismatch;
+// expected-error {{foo}}
+#else
+import module_no_mismatch; // no-warning
+#endif
+
Index: test/Modules/Inputs/module-mismatch.cppm
===
--- /dev/null
+++ test/Modules/Inputs/module-mismatch.cppm
@@ -0,0 +1,13 @@
+#ifdef HAS_PTHREAD
+export module module_mismatch;
+#else
+export module module_no_mismatch;
+#endif
+
+export bool hasPthreads() {
+#ifdef HAS_PTHREAD
+return true;
+#else
+return false;
+#endif
+}
Index: lib/Frontend/CompilerInstance.cpp
===
--- lib/Frontend/CompilerInstance.cpp
+++ lib/Frontend/CompilerInstance.cpp
@@ -1736,7 +1736,9 @@
 // module cache, we don't know how to rebuild modules.
 unsigned ARRFlags = Source == ModuleCache ?
 ASTReader::ARR_OutOfDate | ASTReader::ARR_Missing :
-ASTReade

[PATCH] D53635: [CodeComplete] Expose InBaseClass signal in code completion results.

[Eric Liu via Phabricator via cfe-commits]

ioeric created this revision.
ioeric added a reviewer: ilya-biryukov.
Herald added a subscriber: cfe-commits.

No new tests as the existing tests for result priority should give us
coverage. Also as the new flag is trivial enough, I'm reluctant to plumb the
flag to c-index-test output.


Repository:
  rC Clang

https://reviews.llvm.org/D53635

Files:
  include/clang/Sema/CodeCompleteConsumer.h
  lib/Sema/SemaCodeComplete.cpp

Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -970,6 +970,11 @@
 MaybeAddConstructorResults(R);
 }
 
+static void setInBaseClass(ResultBuilder::Result &R) {
+  R.Priority += CCD_InBaseClass;
+  R.InBaseClass = true;
+}
+
 void ResultBuilder::AddResult(Result R, DeclContext *CurContext,
   NamedDecl *Hiding, bool InBaseClass = false) {
   if (R.Kind != Result::RK_Declaration) {
@@ -1030,7 +1035,7 @@
 
   // Adjust the priority if this result comes from a base class.
   if (InBaseClass)
-R.Priority += CCD_InBaseClass;
+setInBaseClass(R);
 
   AdjustResultPriorityForDecl(R);
 
@@ -5659,7 +5664,7 @@
   R.StartParameter = SelIdents.size();
   R.AllParametersAreInformative = (WantKind != MK_Any);
   if (!InOriginalClass)
-R.Priority += CCD_InBaseClass;
+setInBaseClass(R);
   Results.MaybeAddResult(R, CurContext);
 }
   }
@@ -7763,10 +7768,10 @@
 }
 
 unsigned Priority = CCP_CodePattern;
+auto R = Result(Builder.TakeString(), Method, Priority);
 if (!M->second.getInt())
-  Priority += CCD_InBaseClass;
-
-Results.AddResult(Result(Builder.TakeString(), Method, Priority));
+  setInBaseClass(R);
+Results.AddResult(std::move(R));
   }
 
   // Add Key-Value-Coding and Key-Value-Observing accessor methods for all of
Index: include/clang/Sema/CodeCompleteConsumer.h
===
--- include/clang/Sema/CodeCompleteConsumer.h
+++ include/clang/Sema/CodeCompleteConsumer.h
@@ -821,6 +821,9 @@
   /// Whether this result is hidden by another name.
   bool Hidden : 1;
 
+  /// Whether this is a class member from base class.
+  bool InBaseClass : 1;
+
   /// Whether this result was found via lookup into a base class.
   bool QualifierIsInformative : 1;
 
@@ -859,7 +862,7 @@
bool Accessible = true,
std::vector FixIts = std::vector())
   : Declaration(Declaration), Priority(Priority), Kind(RK_Declaration),
-FixIts(std::move(FixIts)), Hidden(false),
+FixIts(std::move(FixIts)), Hidden(false), InBaseClass(false),
 QualifierIsInformative(QualifierIsInformative),
 StartsNestedNameSpecifier(false), AllParametersAreInformative(false),
 DeclaringEntity(false), Qualifier(Qualifier) {
@@ -870,37 +873,38 @@
   /// Build a result that refers to a keyword or symbol.
   CodeCompletionResult(const char *Keyword, unsigned Priority = CCP_Keyword)
   : Keyword(Keyword), Priority(Priority), Kind(RK_Keyword),
-CursorKind(CXCursor_NotImplemented), Hidden(false),
+CursorKind(CXCursor_NotImplemented), Hidden(false), InBaseClass(false),
 QualifierIsInformative(false), StartsNestedNameSpecifier(false),
 AllParametersAreInformative(false), DeclaringEntity(false) {}
 
   /// Build a result that refers to a macro.
   CodeCompletionResult(const IdentifierInfo *Macro,
const MacroInfo *MI = nullptr,
unsigned Priority = CCP_Macro)
   : Macro(Macro), Priority(Priority), Kind(RK_Macro),
-CursorKind(CXCursor_MacroDefinition), Hidden(false),
+CursorKind(CXCursor_MacroDefinition), Hidden(false), InBaseClass(false),
 QualifierIsInformative(false), StartsNestedNameSpecifier(false),
 AllParametersAreInformative(false), DeclaringEntity(false),
 MacroDefInfo(MI) {}
 
   /// Build a result that refers to a pattern.
-  CodeCompletionResult(CodeCompletionString *Pattern,
-   unsigned Priority = CCP_CodePattern,
-   CXCursorKind CursorKind = CXCursor_NotImplemented,
-   CXAvailabilityKind Availability = CXAvailability_Available,
-   const NamedDecl *D = nullptr)
+  CodeCompletionResult(
+  CodeCompletionString *Pattern, unsigned Priority = CCP_CodePattern,
+  CXCursorKind CursorKind = CXCursor_NotImplemented,
+  CXAvailabilityKind Availability = CXAvailability_Available,
+  const NamedDecl *D = nullptr)
   : Declaration(D), Pattern(Pattern), Priority(Priority), Kind(RK_Pattern),
 CursorKind(CursorKind), Availability(Availability), Hidden(false),
-QualifierIsInformative(false), StartsNestedNameSpecifier(false),
-AllParametersAreInformative(false), DeclaringEntity(false) {}
+InBaseClass(false), QualifierIsInformative(false),
+  

[PATCH] D53488: [clang-tidy] Catching narrowing from double to float.

[Guillaume Chatelet via Phabricator via cfe-commits]

gchatelet added inline comments.



Comment at: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:58
+   const QualType Rhs) {
+  assert(Lhs->isRealType()); // Either integer or floating point.
+  assert(Rhs->isFloatingType()); // Floating point only.

JonasToth wrote:
> Couldn't be the conversion from an `int` to an `enum` be considered narrowing 
> as well? (Not sure about the word of the standard) I think it makes sense to 
> change the `assert` to `return false`
This is a good point but I'd like to implement this as a second patch if you 
don't mind.
I created this bug to track it:
https://bugs.llvm.org/show_bug.cgi?id=39401



Comment at: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:83
+  } else {
+llvm_unreachable("Invalid state");
   }

hokein wrote:
> We expect that there are only two possible cases here, how about rearrange 
> the code like below, which I think it would simpler and improve code 
> readability.
> 
> ```
> if (const auto *Op = Nodes.getNodeAs("op")) {
>   // handle case for "op".
>   return;
> }
> const auto *Cast = Nodes.getNodeAs("cast");
> assert(Cast && "must be cast cases");
> // ...
> ```
I see.
However I plan to add two improvement to this clang tidy so the number of cases 
will increase.
What do you think about the following code?
```
  if (const auto *Op = Nodes.getNodeAs("op"))
return checkBinaryOperator(*Op);
  if (const auto *Cast = Nodes.getNodeAs("cast"))
return checkCastOperator(*Cast);
  llvm_unreachable("must be binary operator or cast expression");

```


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53488



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

[PATCH] D53635: [CodeComplete] Expose InBaseClass signal in code completion results.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

LG, but could we add a test for the new flag it by printing it in 
`PrintingCodeCompleteConsumer::ProcessCodeCompleteResults()` and adding 
corresponding tests to `clang/test/CodeCompletion`?


Repository:
  rC Clang

https://reviews.llvm.org/D53635



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

[PATCH] D53347: [clangd] Simplify auto hover

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL345128: [clangd] Simplify auto hover (authored by ibiryukov, 
committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D53347?vs=170273&id=170835#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D53347

Files:
  clang-tools-extra/trunk/clangd/XRefs.cpp
  clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp


Index: clang-tools-extra/trunk/clangd/XRefs.cpp
===
--- clang-tools-extra/trunk/clangd/XRefs.cpp
+++ clang-tools-extra/trunk/clangd/XRefs.cpp
@@ -585,17 +585,6 @@
 
   Optional getDeducedType() { return DeducedType; }
 
-  // Remove the surrounding Reference or Pointer type of the given type T.
-  QualType UnwrapReferenceOrPointer(QualType T) {
-// "auto &" is represented as a ReferenceType containing an AutoType
-if (const ReferenceType *RT = dyn_cast(T.getTypePtr()))
-  return RT->getPointeeType();
-// "auto *" is represented as a PointerType containing an AutoType
-if (const PointerType *PT = dyn_cast(T.getTypePtr()))
-  return PT->getPointeeType();
-return T;
-  }
-
   // Handle auto initializers:
   //- auto i = 1;
   //- decltype(auto) i = 1;
@@ -606,17 +595,9 @@
 D->getTypeSourceInfo()->getTypeLoc().getBeginLoc() != SearchedLocation)
   return true;
 
-auto DeclT = UnwrapReferenceOrPointer(D->getType());
-const AutoType *AT = dyn_cast(DeclT.getTypePtr());
-if (AT && !AT->getDeducedType().isNull()) {
-  // For auto, use the underlying type because the const& would be
-  // represented twice: written in the code and in the hover.
-  // Example: "const auto I = 1", we only want "int" when hovering on auto,
-  // not "const int".
-  //
-  // For decltype(auto), take the type as is because it cannot be written
-  // with qualifiers or references but its decuded type can be const-ref.
-  DeducedType = AT->isDecltypeAuto() ? DeclT : DeclT.getUnqualifiedType();
+if (auto *AT = D->getType()->getContainedAutoType()) {
+  if (!AT->getDeducedType().isNull())
+DeducedType = AT->getDeducedType();
 }
 return true;
   }
@@ -640,12 +621,13 @@
 if (CurLoc != SearchedLocation)
   return true;
 
-auto T = UnwrapReferenceOrPointer(D->getReturnType());
-const AutoType *AT = dyn_cast(T.getTypePtr());
+const AutoType *AT = D->getReturnType()->getContainedAutoType();
 if (AT && !AT->getDeducedType().isNull()) {
-  DeducedType = T.getUnqualifiedType();
-} else { // auto in a trailing return type just points to a DecltypeType.
-  const DecltypeType *DT = dyn_cast(T.getTypePtr());
+  DeducedType = AT->getDeducedType();
+} else {
+  // auto in a trailing return type just points to a DecltypeType and
+  // getContainedAutoType does not unwrap it.
+  const DecltypeType *DT = dyn_cast(D->getReturnType());
   if (!DT->getUnderlyingType().isNull())
 DeducedType = DT->getUnderlyingType();
 }
Index: clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp
@@ -999,6 +999,13 @@
   )cpp",
   "",
   },
+  {
+  R"cpp(// More compilcated structured types.
+int bar();
+^auto (*foo)() = bar;
+  )cpp",
+  "int",
+  },
   };
 
   for (const OneTest &Test : Tests) {


Index: clang-tools-extra/trunk/clangd/XRefs.cpp
===
--- clang-tools-extra/trunk/clangd/XRefs.cpp
+++ clang-tools-extra/trunk/clangd/XRefs.cpp
@@ -585,17 +585,6 @@
 
   Optional getDeducedType() { return DeducedType; }
 
-  // Remove the surrounding Reference or Pointer type of the given type T.
-  QualType UnwrapReferenceOrPointer(QualType T) {
-// "auto &" is represented as a ReferenceType containing an AutoType
-if (const ReferenceType *RT = dyn_cast(T.getTypePtr()))
-  return RT->getPointeeType();
-// "auto *" is represented as a PointerType containing an AutoType
-if (const PointerType *PT = dyn_cast(T.getTypePtr()))
-  return PT->getPointeeType();
-return T;
-  }
-
   // Handle auto initializers:
   //- auto i = 1;
   //- decltype(auto) i = 1;
@@ -606,17 +595,9 @@
 D->getTypeSourceInfo()->getTypeLoc().getBeginLoc() != SearchedLocation)
   return true;
 
-auto DeclT = UnwrapReferenceOrPointer(D->getType());
-const AutoType *AT = dyn_cast(DeclT.getTypePtr());
-if (AT && !AT->getDeducedType().isNull()) {
-  // For auto, use the underlying type because the const& would be
-  // represented twice: written in the code and in the hover.
-  // Example: "const auto I = 1", we only want "int" when hovering on 

[clang-tools-extra] r345128 - [clangd] Simplify auto hover

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Wed Oct 24 03:09:34 2018
New Revision: 345128

URL: http://llvm.org/viewvc/llvm-project?rev=345128&view=rev
Log:
[clangd] Simplify auto hover

Summary:
Use helper from clang. Also fixes some weird corner cases, e.g.
auto (*foo)() = bar;

Reviewers: kadircet, hokein

Reviewed By: kadircet, hokein

Subscribers: ioeric, MaskRay, jkorous, arphaman, cfe-commits

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

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

Modified: clang-tools-extra/trunk/clangd/XRefs.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/XRefs.cpp?rev=345128&r1=345127&r2=345128&view=diff
==
--- clang-tools-extra/trunk/clangd/XRefs.cpp (original)
+++ clang-tools-extra/trunk/clangd/XRefs.cpp Wed Oct 24 03:09:34 2018
@@ -585,17 +585,6 @@ public:
 
   Optional getDeducedType() { return DeducedType; }
 
-  // Remove the surrounding Reference or Pointer type of the given type T.
-  QualType UnwrapReferenceOrPointer(QualType T) {
-// "auto &" is represented as a ReferenceType containing an AutoType
-if (const ReferenceType *RT = dyn_cast(T.getTypePtr()))
-  return RT->getPointeeType();
-// "auto *" is represented as a PointerType containing an AutoType
-if (const PointerType *PT = dyn_cast(T.getTypePtr()))
-  return PT->getPointeeType();
-return T;
-  }
-
   // Handle auto initializers:
   //- auto i = 1;
   //- decltype(auto) i = 1;
@@ -606,17 +595,9 @@ public:
 D->getTypeSourceInfo()->getTypeLoc().getBeginLoc() != SearchedLocation)
   return true;
 
-auto DeclT = UnwrapReferenceOrPointer(D->getType());
-const AutoType *AT = dyn_cast(DeclT.getTypePtr());
-if (AT && !AT->getDeducedType().isNull()) {
-  // For auto, use the underlying type because the const& would be
-  // represented twice: written in the code and in the hover.
-  // Example: "const auto I = 1", we only want "int" when hovering on auto,
-  // not "const int".
-  //
-  // For decltype(auto), take the type as is because it cannot be written
-  // with qualifiers or references but its decuded type can be const-ref.
-  DeducedType = AT->isDecltypeAuto() ? DeclT : DeclT.getUnqualifiedType();
+if (auto *AT = D->getType()->getContainedAutoType()) {
+  if (!AT->getDeducedType().isNull())
+DeducedType = AT->getDeducedType();
 }
 return true;
   }
@@ -640,12 +621,13 @@ public:
 if (CurLoc != SearchedLocation)
   return true;
 
-auto T = UnwrapReferenceOrPointer(D->getReturnType());
-const AutoType *AT = dyn_cast(T.getTypePtr());
+const AutoType *AT = D->getReturnType()->getContainedAutoType();
 if (AT && !AT->getDeducedType().isNull()) {
-  DeducedType = T.getUnqualifiedType();
-} else { // auto in a trailing return type just points to a DecltypeType.
-  const DecltypeType *DT = dyn_cast(T.getTypePtr());
+  DeducedType = AT->getDeducedType();
+} else {
+  // auto in a trailing return type just points to a DecltypeType and
+  // getContainedAutoType does not unwrap it.
+  const DecltypeType *DT = dyn_cast(D->getReturnType());
   if (!DT->getUnderlyingType().isNull())
 DeducedType = DT->getUnderlyingType();
 }

Modified: clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp?rev=345128&r1=345127&r2=345128&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp Wed Oct 24 03:09:34 
2018
@@ -999,6 +999,13 @@ TEST(Hover, All) {
   )cpp",
   "",
   },
+  {
+  R"cpp(// More compilcated structured types.
+int bar();
+^auto (*foo)() = bar;
+  )cpp",
+  "int",
+  },
   };
 
   for (const OneTest &Test : Tests) {


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

[PATCH] D53488: [clang-tidy] Catching narrowing from double to float.

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.



Comment at: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:58
+   const QualType Rhs) {
+  assert(Lhs->isRealType()); // Either integer or floating point.
+  assert(Rhs->isFloatingType()); // Floating point only.

gchatelet wrote:
> JonasToth wrote:
> > Couldn't be the conversion from an `int` to an `enum` be considered 
> > narrowing as well? (Not sure about the word of the standard) I think it 
> > makes sense to change the `assert` to `return false`
> This is a good point but I'd like to implement this as a second patch if you 
> don't mind.
> I created this bug to track it:
> https://bugs.llvm.org/show_bug.cgi?id=39401
Implementing this in a follow-up is no problem, but could such a case trigger 
the assertion by any means?



Comment at: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:83
+  } else {
+llvm_unreachable("Invalid state");
   }

gchatelet wrote:
> hokein wrote:
> > We expect that there are only two possible cases here, how about rearrange 
> > the code like below, which I think it would simpler and improve code 
> > readability.
> > 
> > ```
> > if (const auto *Op = Nodes.getNodeAs("op")) {
> >   // handle case for "op".
> >   return;
> > }
> > const auto *Cast = Nodes.getNodeAs("cast");
> > assert(Cast && "must be cast cases");
> > // ...
> > ```
> I see.
> However I plan to add two improvement to this clang tidy so the number of 
> cases will increase.
> What do you think about the following code?
> ```
>   if (const auto *Op = Nodes.getNodeAs("op"))
> return checkBinaryOperator(*Op);
>   if (const auto *Cast = Nodes.getNodeAs("cast"))
> return checkCastOperator(*Cast);
>   llvm_unreachable("must be binary operator or cast expression");
> 
> ```
That works as well. The most important thing is that the intended code paths 
are explicitly visible and early return is preferred in llvm (and else after 
return forbidden :))


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53488



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

[PATCH] D53635: [CodeComplete] Expose InBaseClass signal in code completion results.

[Eric Liu via Phabricator via cfe-commits]

ioeric added a comment.

In https://reviews.llvm.org/D53635#1273904, @ilya-biryukov wrote:

> LG, but could we add a test for the new flag it by printing it in 
> `PrintingCodeCompleteConsumer::ProcessCodeCompleteResults()` and adding 
> corresponding tests to `clang/test/CodeCompletion`?
>
> Similar to how it's done for the `Hidden` flag in the results


Done. That was way easy (comparing to c-index-test). Thanks for the pointers!


Repository:
  rC Clang

https://reviews.llvm.org/D53635



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

[PATCH] D53635: [CodeComplete] Expose InBaseClass signal in code completion results.

[Eric Liu via Phabricator via cfe-commits]

ioeric updated this revision to Diff 170838.
ioeric added a comment.

- Add tests for the new flag.


Repository:
  rC Clang

https://reviews.llvm.org/D53635

Files:
  include/clang/Sema/CodeCompleteConsumer.h
  lib/Sema/CodeCompleteConsumer.cpp
  lib/Sema/SemaCodeComplete.cpp
  test/CodeCompletion/member-access.cpp
  test/CodeCompletion/objc-message.mm

Index: test/CodeCompletion/objc-message.mm
===
--- test/CodeCompletion/objc-message.mm
+++ test/CodeCompletion/objc-message.mm
@@ -41,6 +41,6 @@
 // RUN: %clang_cc1 -fsyntax-only -std=c++11 -code-completion-at=%s:33:8 %s -o - | FileCheck -check-prefix=CHECK-CC1 %s
 // CHECK-CC1: categoryInstanceMethod : [#id#]categoryInstanceMethod
 // CHECK-CC1: instanceMethod1 : [#id#]instanceMethod1
-// CHECK-CC1: protocolInstanceMethod : [#id#]protocolInstanceMethod
+// CHECK-CC1: protocolInstanceMethod (InBase) : [#id#]protocolInstanceMethod
 // RUN: %clang_cc1 -fsyntax-only -std=c++11 -code-completion-at=%s:38:8 %s -o - | FileCheck -check-prefix=CHECK-CC2 %s
 // CHECK-CC2: protocolInstanceMethod : [#id#]protocolInstanceMethod
Index: test/CodeCompletion/member-access.cpp
===
--- test/CodeCompletion/member-access.cpp
+++ test/CodeCompletion/member-access.cpp
@@ -51,16 +51,16 @@
 };
 
   // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:31:6 %s -o - | FileCheck -check-prefix=CHECK-CC1 --implicit-check-not="Derived : Derived(" %s
-  // CHECK-CC1: Base1 : Base1::
-  // CHECK-CC1: member1 : [#int#][#Base1::#]member1
-  // CHECK-CC1: member1 : [#int#][#Base2::#]member1
-  // CHECK-CC1: member2 : [#float#][#Base1::#]member2
-  // CHECK-CC1: member3
+  // CHECK-CC1: Base1 (InBase) : Base1::
+  // CHECK-CC1: member1 (InBase) : [#int#][#Base1::#]member1
+  // CHECK-CC1: member1 (InBase) : [#int#][#Base2::#]member1
+  // CHECK-CC1: member2 (InBase) : [#float#][#Base1::#]member2
+  // CHECK-CC1: member3 (InBase)
   // CHECK-CC1: member4
-  // CHECK-CC1: memfun1 : [#void#][#Base3::#]memfun1(<#float#>)
-  // CHECK-CC1: memfun1 : [#void#][#Base3::#]memfun1(<#double#>)[# const#]
-  // CHECK-CC1: memfun1 (Hidden) : [#void#]Base2::memfun1(<#int#>)
-  // CHECK-CC1: memfun2 : [#void#][#Base3::#]memfun2(<#int#>)
+  // CHECK-CC1: memfun1 (InBase) : [#void#][#Base3::#]memfun1(<#float#>)
+  // CHECK-CC1: memfun1 (InBase) : [#void#][#Base3::#]memfun1(<#double#>)[# const#]
+  // CHECK-CC1: memfun1 (Hidden,InBase) : [#void#]Base2::memfun1(<#int#>)
+  // CHECK-CC1: memfun2 (InBase) : [#void#][#Base3::#]memfun2(<#int#>)
   // CHECK-CC1: memfun3 : [#int#]memfun3(<#int#>)
 
 // Make sure this doesn't crash
@@ -93,12 +93,12 @@
   TemplateClass *object2) {
   object.field;
   object2->field;
-// CHECK-CC2: baseTemplateField : [#T#][#BaseTemplate::#]baseTemplateField
-// CHECK-CC2: baseTemplateFunction : [#T#][#BaseTemplate::#]baseTemplateFunction()
+// CHECK-CC2: baseTemplateField (InBase) : [#T#][#BaseTemplate::#]baseTemplateField
+// CHECK-CC2: baseTemplateFunction (InBase) : [#T#][#BaseTemplate::#]baseTemplateFunction()
 // CHECK-CC2: field : [#T#]field
 // CHECK-CC2: function : [#T#]function()
-// CHECK-CC2: member1 : [#int#][#Base1::#]member1
-// CHECK-CC2: member2 : [#float#][#Base1::#]member2
+// CHECK-CC2: member1 (InBase) : [#int#][#Base1::#]member1
+// CHECK-CC2: member2 (InBase) : [#float#][#Base1::#]member2
 // CHECK-CC2: overload1 : [#void#]overload1(<#const T &#>)
 // CHECK-CC2: overload1 : [#void#]overload1(<#const S &#>)
 
@@ -111,12 +111,12 @@
  TemplateClass *object2) {
   object.field;
   object2->field;
-// CHECK-CC3: baseTemplateField : [#int#][#BaseTemplate::#]baseTemplateField
-// CHECK-CC3: baseTemplateFunction : [#int#][#BaseTemplate::#]baseTemplateFunction()
+// CHECK-CC3: baseTemplateField (InBase) : [#int#][#BaseTemplate::#]baseTemplateField
+// CHECK-CC3: baseTemplateFunction (InBase) : [#int#][#BaseTemplate::#]baseTemplateFunction()
 // CHECK-CC3: field : [#int#]field
 // CHECK-CC3: function : [#int#]function()
-// CHECK-CC3: member1 : [#int#][#Base1::#]member1
-// CHECK-CC3: member2 : [#float#][#Base1::#]member2
+// CHECK-CC3: member1 (InBase) : [#int#][#Base1::#]member1
+// CHECK-CC3: member2 (InBase) : [#float#][#Base1::#]member2
 // CHECK-CC3: overload1 : [#void#]overload1(<#const int &#>)
 // CHECK-CC3: overload1 : [#void#]overload1(<#const double &#>)
 
@@ -182,31 +182,31 @@
 }
 
 // RUN: %clang_cc1 -fsyntax-only -code-completion-with-fixits -code-completion-at=%s:177:6 %s -o - | FileCheck -check-prefix=CHECK-CC8 --implicit-check-not="Derived : Derived(" %s
-// CHECK-CC8: Base1 : Base1::
-// CHECK-CC8: member1 : [#int#][#Base1::#]member1
-// CHECK-CC8: member1 : [#int#][#Base2::#]member1
-// CHECK-CC8: member2 : [#float#][#Base1::#]member2
-// CHECK-CC8: member3 : [#double#][#Base2::#]member3
+// CHECK-CC8: Base1 (InBase) : Base1::
+// CHECK-CC8: member1 (InBase) : [

[PATCH] D53636: Do not always request an implicit taskgroup region inside the kmpc_taskloop function

[Sergi Mateo via Phabricator via cfe-commits]

smateo created this revision.
smateo added a reviewer: ABataev.
Herald added a subscriber: cfe-commits.

For the following code:

  int i;
  #pragma omp taskloop
  for (i = 0; i < 100; ++i)
  {}
  
  #pragma omp taskloop nogroup
  for (i = 0; i < 100; ++i)
  {}

Clang emits the following LLVM IR:

  ...
   call void @__kmpc_taskgroup(%struct.ident_t* @0, i32 %0)
   %2 = call i8* @__kmpc_omp_task_alloc(%struct.ident_t* @0, i32 %0, i32 1, i64 
80, i64 8, i32 (i32, i8*)* bitcast (i32 (i32, 
%struct.kmp_task_t_with_privates*)* @.omp_task_entry. to i32 (i32, i8*)*))
   ...
   call void @__kmpc_taskloop(%struct.ident_t* @0, i32 %0, i8* %2, i32 1, i64* 
%8, i64* %9, i64 %13, i32 0, i32 0, i64 0, i8* null)
   call void @__kmpc_end_taskgroup(%struct.ident_t* @0, i32 %0)
  
  
   ...
   %15 = call i8* @__kmpc_omp_task_alloc(%struct.ident_t* @0, i32 %0, i32 1, 
i64 80, i64 8, i32 (i32, i8*)* bitcast (i32 (i32, 
%struct.kmp_task_t_with_privates.1*)* @.omp_task_entry..2 to i32 (i32, i8*)*))
   ...
   call void @__kmpc_taskloop(%struct.ident_t* @0, i32 %0, i8* %15, i32 1, i64* 
%21, i64* %22, i64 %26, i32 0, i32 0, i64 0, i8* null)

The first set of instructions corresponds to the first taskloop construct. It 
is important to note that the implicit taskgroup region associated with the 
taskloop construct has been materialized in our IR:  the `__kmpc_taskloop` 
occurs inside a taskgroup region. Note also that this taskgroup region does not 
exist in our second taskloop because we are using the `nogroup` clause.

The issue here is the 4th argument of the kmpc_taskloop call, starting from the 
end,  is always a zero. Checking the LLVM OpenMP RT implementation, we see that 
this argument corresponds to the nogroup parameter:

  void __kmpc_taskloop(ident_t *loc, int gtid, kmp_task_t *task, int if_val,
   kmp_uint64 *lb, kmp_uint64 *ub, kmp_int64 st, int 
nogroup,
   int sched, kmp_uint64 grainsize, void *task_dup);

So basically we always tell to the RT to do another taskgroup region. For the 
first taskloop, this means that we create two taskgroup regions. For the second 
example, it means that despite the fact we had a nogroup clause we are going to 
have a taskgroup region, so we unnecessary wait until all descendant tasks have 
been executed.


Repository:
  rC Clang

https://reviews.llvm.org/D53636

Files:
  lib/CodeGen/CGOpenMPRuntime.cpp
  test/OpenMP/taskloop_codegen.cpp
  test/OpenMP/taskloop_firstprivate_codegen.cpp
  test/OpenMP/taskloop_lastprivate_codegen.cpp
  test/OpenMP/taskloop_private_codegen.cpp
  test/OpenMP/taskloop_reduction_codegen.cpp
  test/OpenMP/taskloop_simd_codegen.cpp
  test/OpenMP/taskloop_simd_firstprivate_codegen.cpp
  test/OpenMP/taskloop_simd_lastprivate_codegen.cpp
  test/OpenMP/taskloop_simd_private_codegen.cpp
  test/OpenMP/taskloop_simd_reduction_codegen.cpp

Index: test/OpenMP/taskloop_simd_reduction_codegen.cpp
===
--- test/OpenMP/taskloop_simd_reduction_codegen.cpp
+++ test/OpenMP/taskloop_simd_reduction_codegen.cpp
@@ -148,7 +148,7 @@
 // CHECK:[[SUB12:%.*]] = sub nsw i32 [[DIV]], 1
 // CHECK:store i32 [[SUB12]], i32* [[DOTCAPTURE_EXPR_9]],
 // CHECK:[[TMP65:%.*]] = call i8* @__kmpc_omp_task_alloc(%struct.ident_t* %{{.+}}, i32 [[TMP0]], i32 1, i64 888, i64 72, i32 (i32, i8*)* bitcast (i32 (i32, %struct.kmp_task_t_with_privates*)* @{{.+}} to i32 (i32, i8*)*))
-// CHECK:call void @__kmpc_taskloop(%struct.ident_t* %{{.+}}, i32 [[TMP0]], i8* [[TMP65]], i32 1, i64* %{{.+}}, i64* %{{.+}}, i64 %{{.+}}, i32 0, i32 0, i64 0, i8* null)
+// CHECK:call void @__kmpc_taskloop(%struct.ident_t* %{{.+}}, i32 [[TMP0]], i8* [[TMP65]], i32 1, i64* %{{.+}}, i64* %{{.+}}, i64 %{{.+}}, i32 1, i32 0, i64 0, i8* null)
 // CHECK:call void @__kmpc_end_taskgroup(%struct.ident_t*
 
 // CHECK:ret i32
Index: test/OpenMP/taskloop_simd_private_codegen.cpp
===
--- test/OpenMP/taskloop_simd_private_codegen.cpp
+++ test/OpenMP/taskloop_simd_private_codegen.cpp
@@ -65,7 +65,7 @@
   // LAMBDA: define{{.*}} internal{{.*}} void [[OUTER_LAMBDA]](
   // LAMBDA: [[RES:%.+]] = call i8* @__kmpc_omp_task_alloc(%{{[^ ]+}} @{{[^,]+}}, i32 %{{[^,]+}}, i32 1, i64 96, i64 1, i32 (i32, i8*)* bitcast (i32 (i32, %{{[^*]+}}*)* [[TASK_ENTRY:@[^ ]+]] to i32 (i32, i8*)*))
 // LAMBDA: [[PRIVATES:%.+]] = getelementptr inbounds %{{.+}}, %{{.+}}* %{{.+}}, i{{.+}} 0, i{{.+}} 1
-// LAMBDA: call void @__kmpc_taskloop(%{{.+}}* @{{.+}}, i32 %{{.+}}, i8* [[RES]], i32 1, i64* %{{.+}}, i64* %{{.+}}, i64 %{{.+}}, i32 0, i32 0, i64 0, i8* null)
+// LAMBDA: call void @__kmpc_taskloop(%{{.+}}* @{{.+}}, i32 %{{.+}}, i8* [[RES]], i32 1, i64* %{{.+}}, i64* %{{.+}}, i64 %{{.+}}, i32 1, i32 0, i64 0, i8* null)
 // LAMBDA: ret
 #pragma omp taskloop simd private(g, sivar)
   for (int i = 0; i < 10; ++i) {
@@ -101,7 +101,7 @@
   // BLOCKS: define{{.*}} internal{{.*}} vo

[PATCH] D53638: [clangd] Downrank members from base class

[Eric Liu via Phabricator via cfe-commits]

ioeric created this revision.
ioeric added reviewers: sammccall, ilya-biryukov.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay.
ioeric added a dependency: D53635: [CodeComplete] Expose InBaseClass signal in 
code completion results..

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53638

Files:
  clangd/Quality.cpp
  clangd/Quality.h
  unittests/clangd/QualityTests.cpp


Index: unittests/clangd/QualityTests.cpp
===
--- unittests/clangd/QualityTests.cpp
+++ unittests/clangd/QualityTests.cpp
@@ -184,7 +184,13 @@
   // The injected class name is treated as the outer class name.
   Relevance = {};
   Relevance.merge(CodeCompletionResult(&findDecl(AST, "S::S"), 42));
-  EXPECT_EQ(Relevance.Scope, SymbolRelevanceSignals::GlobalScope);
+
+  Relevance = {};
+  EXPECT_FALSE(Relevance.InBaseClass);
+  auto BaseMember = CodeCompletionResult(&findAnyDecl(AST, "y"), 42);
+  BaseMember.InBaseClass = true;
+  Relevance.merge(Member);
+  EXPECT_TRUE(Relevance.InBaseClass);
 }
 
 // Do the signals move the scores in the direction we expect?
@@ -276,6 +282,10 @@
   EXPECT_LT(Instance.evaluate(), Default.evaluate());
   Instance.IsInstanceMember = true;
   EXPECT_EQ(Instance.evaluate(), Default.evaluate());
+
+  SymbolRelevanceSignals InBaseClass;
+  InBaseClass.InBaseClass = true;
+  EXPECT_LT(InBaseClass.evaluate(), Default.evaluate());
 }
 
 TEST(QualityTests, ScopeProximity) {
Index: clangd/Quality.h
===
--- clangd/Quality.h
+++ clangd/Quality.h
@@ -87,6 +87,7 @@
   bool Forbidden = false; // Unavailable (e.g const) or inaccessible (private).
   /// Whether fixits needs to be applied for that completion or not.
   bool NeedsFixIts = false;
+  bool InBaseClass = false; // A member from base class of the accessed class.
 
   URIDistance *FileProximityMatch = nullptr;
   /// These are used to calculate proximity between the index symbol and the
Index: clangd/Quality.cpp
===
--- clangd/Quality.cpp
+++ clangd/Quality.cpp
@@ -299,6 +299,7 @@
   : 0.6;
 SemaFileProximityScore = std::max(DeclProximity, SemaFileProximityScore);
 IsInstanceMember |= isInstanceMember(SemaCCResult.Declaration);
+InBaseClass |= SemaCCResult.InBaseClass;
   }
 
   // Declarations are scoped, others (like macros) are assumed global.
@@ -375,6 +376,9 @@
 Score *= 0.5;
   }
 
+  if (InBaseClass)
+Score *= 0.7;
+
   // Penalize for FixIts.
   if (NeedsFixIts)
 Score *= 0.5;


Index: unittests/clangd/QualityTests.cpp
===
--- unittests/clangd/QualityTests.cpp
+++ unittests/clangd/QualityTests.cpp
@@ -184,7 +184,13 @@
   // The injected class name is treated as the outer class name.
   Relevance = {};
   Relevance.merge(CodeCompletionResult(&findDecl(AST, "S::S"), 42));
-  EXPECT_EQ(Relevance.Scope, SymbolRelevanceSignals::GlobalScope);
+
+  Relevance = {};
+  EXPECT_FALSE(Relevance.InBaseClass);
+  auto BaseMember = CodeCompletionResult(&findAnyDecl(AST, "y"), 42);
+  BaseMember.InBaseClass = true;
+  Relevance.merge(Member);
+  EXPECT_TRUE(Relevance.InBaseClass);
 }
 
 // Do the signals move the scores in the direction we expect?
@@ -276,6 +282,10 @@
   EXPECT_LT(Instance.evaluate(), Default.evaluate());
   Instance.IsInstanceMember = true;
   EXPECT_EQ(Instance.evaluate(), Default.evaluate());
+
+  SymbolRelevanceSignals InBaseClass;
+  InBaseClass.InBaseClass = true;
+  EXPECT_LT(InBaseClass.evaluate(), Default.evaluate());
 }
 
 TEST(QualityTests, ScopeProximity) {
Index: clangd/Quality.h
===
--- clangd/Quality.h
+++ clangd/Quality.h
@@ -87,6 +87,7 @@
   bool Forbidden = false; // Unavailable (e.g const) or inaccessible (private).
   /// Whether fixits needs to be applied for that completion or not.
   bool NeedsFixIts = false;
+  bool InBaseClass = false; // A member from base class of the accessed class.
 
   URIDistance *FileProximityMatch = nullptr;
   /// These are used to calculate proximity between the index symbol and the
Index: clangd/Quality.cpp
===
--- clangd/Quality.cpp
+++ clangd/Quality.cpp
@@ -299,6 +299,7 @@
   : 0.6;
 SemaFileProximityScore = std::max(DeclProximity, SemaFileProximityScore);
 IsInstanceMember |= isInstanceMember(SemaCCResult.Declaration);
+InBaseClass |= SemaCCResult.InBaseClass;
   }
 
   // Declarations are scoped, others (like macros) are assumed global.
@@ -375,6 +376,9 @@
 Score *= 0.5;
   }
 
+  if (InBaseClass)
+Score *= 0.7;
+
   // Penalize for FixIts.
   if (NeedsFixIts)
 Score *= 0.5;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
htt

[PATCH] D53639: [autocompletion] Handle the space before pressing tab

[Yuka Takahashi via Phabricator via cfe-commits]

yamaguchi created this revision.
yamaguchi added reviewers: teemperor, ruiu.

Distinguish "--autocomplete=-someflag" and "--autocomplete=-someflag,"
because the latter indicates that the user put space before pushing tab
which should end up in a file completion.


https://reviews.llvm.org/D53639

Files:
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/autocomplete.c


Index: clang/test/Driver/autocomplete.c
===
--- clang/test/Driver/autocomplete.c
+++ clang/test/Driver/autocomplete.c
@@ -121,3 +121,8 @@
 // MODULE_FILE_EQUAL-NOT: -fmodule-file=
 // RUN: %clang --autocomplete=-fmodule-file | FileCheck %s 
-check-prefix=MODULE_FILE
 // MODULE_FILE: -fmodule-file=
+
+// RUN: %clang --autocomplete=-Qunused-arguments, | FileCheck %s 
-check-prefix=QUNUSED_COMMA
+// QUNUSED_COMMA-NOT: -Qunused-arguments
+// RUN: %clang --autocomplete=-Qunused-arguments | FileCheck %s 
-check-prefix=QUNUSED
+// QUNUSED: -Qunused-arguments
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -1508,6 +1508,13 @@
   unsigned short DisableFlags =
   options::NoDriverOption | options::Unsupported | options::Ignored;
 
+  // Distinguish "--autocomplete=-someflag" and "--autocomplete=-someflag,"
+  // because the latter indicates that the user put space before pushing tab
+  // which should end up in a file completion.
+  bool HasSpace = false;
+  if (PassedFlags.endswith(","))
+HasSpace = true;
+
   // Parse PassedFlags by "," as all the command-line flags are passed to this
   // function separated by ","
   StringRef TargetFlags = PassedFlags;
@@ -1525,6 +1532,14 @@
 
   StringRef Cur;
   Cur = Flags.at(Flags.size() - 1);
+
+  // If Flags were empty, it means the user typed `clang [tab]` where we should
+  // list all possible flags.
+  if (HasSpace && !Flags.empty()) {
+llvm::outs() << '\n';
+return;
+  }
+
   StringRef Prev;
   if (Flags.size() >= 2) {
 Prev = Flags.at(Flags.size() - 2);


Index: clang/test/Driver/autocomplete.c
===
--- clang/test/Driver/autocomplete.c
+++ clang/test/Driver/autocomplete.c
@@ -121,3 +121,8 @@
 // MODULE_FILE_EQUAL-NOT: -fmodule-file=
 // RUN: %clang --autocomplete=-fmodule-file | FileCheck %s -check-prefix=MODULE_FILE
 // MODULE_FILE: -fmodule-file=
+
+// RUN: %clang --autocomplete=-Qunused-arguments, | FileCheck %s -check-prefix=QUNUSED_COMMA
+// QUNUSED_COMMA-NOT: -Qunused-arguments
+// RUN: %clang --autocomplete=-Qunused-arguments | FileCheck %s -check-prefix=QUNUSED
+// QUNUSED: -Qunused-arguments
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -1508,6 +1508,13 @@
   unsigned short DisableFlags =
   options::NoDriverOption | options::Unsupported | options::Ignored;
 
+  // Distinguish "--autocomplete=-someflag" and "--autocomplete=-someflag,"
+  // because the latter indicates that the user put space before pushing tab
+  // which should end up in a file completion.
+  bool HasSpace = false;
+  if (PassedFlags.endswith(","))
+HasSpace = true;
+
   // Parse PassedFlags by "," as all the command-line flags are passed to this
   // function separated by ","
   StringRef TargetFlags = PassedFlags;
@@ -1525,6 +1532,14 @@
 
   StringRef Cur;
   Cur = Flags.at(Flags.size() - 1);
+
+  // If Flags were empty, it means the user typed `clang [tab]` where we should
+  // list all possible flags.
+  if (HasSpace && !Flags.empty()) {
+llvm::outs() << '\n';
+return;
+  }
+
   StringRef Prev;
   if (Flags.size() >= 2) {
 Prev = Flags.at(Flags.size() - 2);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53488: [clang-tidy] Catching narrowing from double to float.

[Guillaume Chatelet via Phabricator via cfe-commits]

gchatelet updated this revision to Diff 170846.
gchatelet added a comment.

- Update documentation, makes returning code path more explicit.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53488

Files:
  clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
  clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.h
  docs/clang-tidy/checks/cppcoreguidelines-narrowing-conversions.rst
  test/clang-tidy/cppcoreguidelines-narrowing-conversions.cpp

Index: test/clang-tidy/cppcoreguidelines-narrowing-conversions.cpp
===
--- test/clang-tidy/cppcoreguidelines-narrowing-conversions.cpp
+++ test/clang-tidy/cppcoreguidelines-narrowing-conversions.cpp
@@ -9,12 +9,12 @@
 namespace floats {
 
 struct ConvertsToFloat {
-  operator float() const { return 0.5; }
+  operator float() const { return 0.5f; }
 };
 
-float operator "" _Pa(unsigned long long);
+float operator"" _float(unsigned long long);
 
-void not_ok(double d) {
+void narrow_double_to_int_not_ok(double d) {
   int i = 0;
   i = d;
   // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: narrowing conversion from 'double' to 'int' [cppcoreguidelines-narrowing-conversions]
@@ -24,11 +24,11 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: narrowing conversion from 'float' to 'int' [cppcoreguidelines-narrowing-conversions]
   i = ConvertsToFloat();
   // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: narrowing conversion from 'float' to 'int' [cppcoreguidelines-narrowing-conversions]
-  i = 15_Pa;
+  i = 15_float;
   // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: narrowing conversion from 'float' to 'int' [cppcoreguidelines-narrowing-conversions]
 }
 
-void not_ok_binary_ops(double d) {
+void narrow_double_to_int_not_ok_binary_ops(double d) {
   int i = 0;
   i += 0.5;
   // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: narrowing conversion from 'double' to 'int' [cppcoreguidelines-narrowing-conversions]
@@ -52,6 +52,43 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: narrowing conversion from 'double' to 'int' [cppcoreguidelines-narrowing-conversions]
 }
 
+double operator"" _double(unsigned long long);
+
+float narrow_double_to_float_return() {
+  return 0.5;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: narrowing conversion from 'double' to 'float' [cppcoreguidelines-narrowing-conversions]
+}
+
+void narrow_double_to_float_not_ok(double d) {
+  float f = 0;
+  f = d;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: narrowing conversion from 'double' to 'float' [cppcoreguidelines-narrowing-conversions]
+  f = 0.5;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: narrowing conversion from 'double' to 'float' [cppcoreguidelines-narrowing-conversions]
+  f = 15_double;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: narrowing conversion from 'double' to 'float' [cppcoreguidelines-narrowing-conversions]
+}
+
+void narrow_double_to_float_not_ok_binary_ops(double d) {
+  float f = 0;
+  f += 0.5;
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: narrowing conversion from 'double' to 'float' [cppcoreguidelines-narrowing-conversions]
+  f += d;
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: narrowing conversion from 'double' to 'float' [cppcoreguidelines-narrowing-conversions]
+  // We warn on the following even though it's not dangerous because there is no
+  // reason to use a double literal here.
+  // TODO(courbet): Provide an automatic fix.
+  f += 2.0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: narrowing conversion from 'double' to 'float' [cppcoreguidelines-narrowing-conversions]
+
+  f *= 0.5;
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: narrowing conversion from 'double' to 'float' [cppcoreguidelines-narrowing-conversions]
+  f /= 0.5;
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: narrowing conversion from 'double' to 'float' [cppcoreguidelines-narrowing-conversions]
+  f += (double)0.5f;
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: narrowing conversion from 'double' to 'float' [cppcoreguidelines-narrowing-conversions]
+}
+
 void ok(double d) {
   int i = 0;
   i = 1;
@@ -89,15 +126,19 @@
 
 void template_context() {
   f(1, 2);
+  f(1, .5f);
   f(1, .5);
+  f(1, .5l);
 }
 
 #define DERP(i, j) (i += j)
 
 void macro_context() {
   int i = 0;
   DERP(i, 2);
+  DERP(i, .5f);
   DERP(i, .5);
+  DERP(i, .5l);
 }
 
-}  // namespace floats
+} // namespace floats
Index: docs/clang-tidy/checks/cppcoreguidelines-narrowing-conversions.rst
===
--- docs/clang-tidy/checks/cppcoreguidelines-narrowing-conversions.rst
+++ docs/clang-tidy/checks/cppcoreguidelines-narrowing-conversions.rst
@@ -10,13 +10,15 @@
 This rule is part of the "Expressions and statements" profile of the C++ Core
 Guidelines, corresponding to rule ES.46. See
 
-https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Res-narrowing.
+https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#es46-avoid-lossy-narrowing-truncating-arithmeti

[PATCH] D53488: [clang-tidy] Catching narrowing from double to float.

[Guillaume Chatelet via Phabricator via cfe-commits]

gchatelet added inline comments.



Comment at: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:58
+   const QualType Rhs) {
+  assert(Lhs->isRealType()); // Either integer or floating point.
+  assert(Rhs->isFloatingType()); // Floating point only.

JonasToth wrote:
> gchatelet wrote:
> > JonasToth wrote:
> > > Couldn't be the conversion from an `int` to an `enum` be considered 
> > > narrowing as well? (Not sure about the word of the standard) I think it 
> > > makes sense to change the `assert` to `return false`
> > This is a good point but I'd like to implement this as a second patch if 
> > you don't mind.
> > I created this bug to track it:
> > https://bugs.llvm.org/show_bug.cgi?id=39401
> Implementing this in a follow-up is no problem, but could such a case trigger 
> the assertion by any means?
Not right now because the matchers do not pick integers for the RHS so it never 
matches int to enum conversions.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53488



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

[PATCH] D53639: [autocompletion] Handle the space before pressing tab

[Raphael Isemann via Phabricator via cfe-commits]

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



Comment at: clang/lib/Driver/Driver.cpp:1514
+  // which should end up in a file completion.
+  bool HasSpace = false;
+  if (PassedFlags.endswith(","))

Why not `const bool HasSpace = PassedFlags.endswith(",")`?



Comment at: clang/lib/Driver/Driver.cpp:1539
+  if (HasSpace && !Flags.empty()) {
+llvm::outs() << '\n';
+return;

Can you describe in the comment why printing a newline and returning here is 
the correct behavior? It's not obvious to the reader (including me).


https://reviews.llvm.org/D53639



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

[PATCH] D53138: Scalable type size queries (clang)

[Graham Hunter via Phabricator via cfe-commits]

huntergr abandoned this revision.
huntergr added a comment.

Abandoning this. At the devmeeting it was agreed that 'getPrimitiveSizeInBits' 
should continue to work as-is for fixed-length vectors and only behave 
differently for scalable vectors.


https://reviews.llvm.org/D53138



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

[PATCH] D53339: [clang-tidy] Add the abseil-duration-factory-float check

[Hyrum Wright via Phabricator via cfe-commits]

hwright updated this revision to Diff 170847.
hwright marked 2 inline comments as done.
hwright added a comment.

Update diagnostic text


https://reviews.llvm.org/D53339

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/DurationFactoryFloatCheck.cpp
  clang-tidy/abseil/DurationFactoryFloatCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-duration-factory-float.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-duration-factory-float.cpp

Index: test/clang-tidy/abseil-duration-factory-float.cpp
===
--- /dev/null
+++ test/clang-tidy/abseil-duration-factory-float.cpp
@@ -0,0 +1,133 @@
+// RUN: %check_clang_tidy %s abseil-duration-factory-float %t
+
+// Mimic the implementation of absl::Duration
+namespace absl {
+
+class Duration {};
+
+Duration Nanoseconds(long long);
+Duration Microseconds(long long);
+Duration Milliseconds(long long);
+Duration Seconds(long long);
+Duration Minutes(long long);
+Duration Hours(long long);
+
+#define GENERATE_DURATION_FACTORY_OVERLOADS(NAME) \
+  Duration NAME(float n); \
+  Duration NAME(double n);\
+  template\
+  Duration NAME(T n);
+
+GENERATE_DURATION_FACTORY_OVERLOADS(Nanoseconds);
+GENERATE_DURATION_FACTORY_OVERLOADS(Microseconds);
+GENERATE_DURATION_FACTORY_OVERLOADS(Milliseconds);
+GENERATE_DURATION_FACTORY_OVERLOADS(Seconds);
+GENERATE_DURATION_FACTORY_OVERLOADS(Minutes);
+GENERATE_DURATION_FACTORY_OVERLOADS(Hours);
+#undef GENERATE_DURATION_FACTORY_OVERLOADS
+
+}  // namespace absl
+
+void ConvertFloatTest() {
+  absl::Duration d;
+
+  d = absl::Seconds(60.0);
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: use the integer version of absl::Seconds [abseil-duration-factory-float]
+  // CHECK-FIXES: absl::Seconds(60);
+  d = absl::Minutes(300.0);
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: use the integer version of absl::Minutes [abseil-duration-factory-float]
+  // CHECK-FIXES: absl::Minutes(300);
+
+  d = absl::Milliseconds(1e2);
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: use the integer version of absl::Milliseconds [abseil-duration-factory-float]
+  // CHECK-FIXES: absl::Milliseconds(100);
+  d = absl::Seconds(3.0f);
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: use the integer version of absl::Seconds [abseil-duration-factory-float]
+  // CHECK-FIXES: absl::Seconds(3);
+  d = absl::Seconds(3.);
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: use the integer version of absl::Seconds [abseil-duration-factory-float]
+  // CHECK-FIXES: absl::Seconds(3);
+  d = absl::Seconds(0x3.p0);
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: use the integer version of absl::Seconds [abseil-duration-factory-float]
+  // CHECK-FIXES: absl::Seconds(3);
+  d = absl::Seconds(0x3.p1);
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: use the integer version of absl::Seconds [abseil-duration-factory-float]
+  // CHECK-FIXES: absl::Seconds(6);
+
+
+  // Ignored expressions
+  d = absl::Seconds(.001);
+  d = absl::Seconds(.100);
+  d = ::absl::Seconds(1);
+  d = ::absl::Minutes(1);
+  d = ::absl::Hours(1);
+  d = absl::Seconds(0x3.4p1);
+
+  // Negative literals (we don't yet handle this case)
+  d = absl::Seconds(-3.0);
+
+  // This is bigger than we can safely fit in a positive int32, so we ignore it.
+  d = absl::Seconds(1e12);
+
+  int x;
+  d = absl::Seconds(x);
+  float y;
+  d = absl::Minutes(y);
+
+#define SECONDS(x) absl::Seconds(x)
+  SECONDS(60);
+#undef SECONDS
+#define THIRTY 30.0
+  d = absl::Seconds(THIRTY);
+#undef THIRTY
+}
+
+template 
+void InTemplate() {
+  absl::Duration d;
+
+  d = absl::Seconds(N);  // 1
+  // ^ No replacement here.
+
+  d = absl::Minutes(1.0);  // 2
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: use the integer version of absl::Minutes [abseil-duration-factory-float]
+  // CHECK-FIXES: absl::Minutes(1);  // 2
+}
+
+void Instantiate() {
+  InTemplate<60>();
+  InTemplate<1>();
+}
+
+void ConvertCastTest() {
+  absl::Duration d;
+
+  d = absl::Seconds(static_cast(5));
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: use the integer version of absl::Seconds [abseil-duration-factory-float]
+  // CHECK-FIXES: absl::Seconds(5);
+
+  d = absl::Minutes(static_cast(5));
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: use the integer version of absl::Minutes [abseil-duration-factory-float]
+  // CHECK-FIXES: absl::Minutes(5);
+
+  d = absl::Seconds((double) 5);
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: use the integer version of absl::Seconds [abseil-duration-factory-float]
+  // CHECK-FIXES: absl::Seconds(5);
+
+  d = absl::Minutes((float) 5);
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: use the integer version of absl::Minutes [abseil-duration-factory-float]
+  // CHECK-FIXES: absl::Minutes(5);
+
+  d = absl::Seconds(double(5));
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: use the integer version of absl::Seconds [abseil-duration-factory-fl

[PATCH] D53488: [clang-tidy] Catching narrowing from double to float.

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

Please add a short notice in the release notes for this change.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53488



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

[PATCH] D53639: [autocompletion] Handle the space before pressing tab

[Yuka Takahashi via Phabricator via cfe-commits]

yamaguchi updated this revision to Diff 170849.
yamaguchi added a comment.

Updated HasSpace, added comments, fixed typo in the commit message


https://reviews.llvm.org/D53639

Files:
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/autocomplete.c


Index: clang/test/Driver/autocomplete.c
===
--- clang/test/Driver/autocomplete.c
+++ clang/test/Driver/autocomplete.c
@@ -25,6 +25,7 @@
 // STDLIBALL-NEXT: platform
 // RUN: %clang --autocomplete=-meabi,d | FileCheck %s -check-prefix=MEABI
 // MEABI: default
+// RUN: %clang --autocomplete=-meabi, | FileCheck %s -check-prefix=MEABIALL
 // RUN: %clang --autocomplete=-meabi | FileCheck %s -check-prefix=MEABIALL
 // MEABIALL: 4
 // MEABIALL-NEXT: 5
@@ -121,3 +122,8 @@
 // MODULE_FILE_EQUAL-NOT: -fmodule-file=
 // RUN: %clang --autocomplete=-fmodule-file | FileCheck %s 
-check-prefix=MODULE_FILE
 // MODULE_FILE: -fmodule-file=
+
+// RUN: %clang --autocomplete=-Qunused-arguments, | FileCheck %s 
-check-prefix=QUNUSED_COMMA
+// QUNUSED_COMMA-NOT: -Qunused-arguments
+// RUN: %clang --autocomplete=-Qunused-arguments | FileCheck %s 
-check-prefix=QUNUSED
+// QUNUSED: -Qunused-arguments
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -1508,6 +1508,11 @@
   unsigned short DisableFlags =
   options::NoDriverOption | options::Unsupported | options::Ignored;
 
+  // Distinguish "--autocomplete=-someflag" and "--autocomplete=-someflag,"
+  // because the latter indicates that the user put space before pushing tab
+  // which should end up in a file completion.
+  const bool HasSpace = PassedFlags.endswith(",");
+
   // Parse PassedFlags by "," as all the command-line flags are passed to this
   // function separated by ","
   StringRef TargetFlags = PassedFlags;
@@ -1534,6 +1539,16 @@
   if (SuggestedCompletions.empty())
 SuggestedCompletions = Opts->suggestValueCompletions(Cur, "");
 
+  // If Flags were empty, it means the user typed `clang [tab]` where we should
+  // list all possible flags. If there was no value completion and the user
+  // pressed tab after a space, we should fall back to a file completion.
+  // Printing a newline is to be consistent with what we print at the end of
+  // this function.
+  if (SuggestedCompletions.empty() && HasSpace && !Flags.empty()) {
+llvm::outs() << '\n';
+return;
+  }
+
   // When flag ends with '=' and there was no value completion, return empty
   // string and fall back to the file autocompletion.
   if (SuggestedCompletions.empty() && !Cur.endswith("=")) {


Index: clang/test/Driver/autocomplete.c
===
--- clang/test/Driver/autocomplete.c
+++ clang/test/Driver/autocomplete.c
@@ -25,6 +25,7 @@
 // STDLIBALL-NEXT: platform
 // RUN: %clang --autocomplete=-meabi,d | FileCheck %s -check-prefix=MEABI
 // MEABI: default
+// RUN: %clang --autocomplete=-meabi, | FileCheck %s -check-prefix=MEABIALL
 // RUN: %clang --autocomplete=-meabi | FileCheck %s -check-prefix=MEABIALL
 // MEABIALL: 4
 // MEABIALL-NEXT: 5
@@ -121,3 +122,8 @@
 // MODULE_FILE_EQUAL-NOT: -fmodule-file=
 // RUN: %clang --autocomplete=-fmodule-file | FileCheck %s -check-prefix=MODULE_FILE
 // MODULE_FILE: -fmodule-file=
+
+// RUN: %clang --autocomplete=-Qunused-arguments, | FileCheck %s -check-prefix=QUNUSED_COMMA
+// QUNUSED_COMMA-NOT: -Qunused-arguments
+// RUN: %clang --autocomplete=-Qunused-arguments | FileCheck %s -check-prefix=QUNUSED
+// QUNUSED: -Qunused-arguments
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -1508,6 +1508,11 @@
   unsigned short DisableFlags =
   options::NoDriverOption | options::Unsupported | options::Ignored;
 
+  // Distinguish "--autocomplete=-someflag" and "--autocomplete=-someflag,"
+  // because the latter indicates that the user put space before pushing tab
+  // which should end up in a file completion.
+  const bool HasSpace = PassedFlags.endswith(",");
+
   // Parse PassedFlags by "," as all the command-line flags are passed to this
   // function separated by ","
   StringRef TargetFlags = PassedFlags;
@@ -1534,6 +1539,16 @@
   if (SuggestedCompletions.empty())
 SuggestedCompletions = Opts->suggestValueCompletions(Cur, "");
 
+  // If Flags were empty, it means the user typed `clang [tab]` where we should
+  // list all possible flags. If there was no value completion and the user
+  // pressed tab after a space, we should fall back to a file completion.
+  // Printing a newline is to be consistent with what we print at the end of
+  // this function.
+  if (SuggestedCompletions.empty() && HasSpace && !Flags.empty()) {
+llvm::outs() << '\n';
+return;
+  }
+
   // When flag ends with '=' and there was no value completion, return empt

[PATCH] D53607: [AST] Only store the needed data in IfStmt.

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added a comment.

To add on the issue of changing the order of the children. The more I think 
about
it the less it seems to be a good idea. Every test passes but I strongly suspect
that a lot of code subtly rely on this.


Repository:
  rC Clang

https://reviews.llvm.org/D53607



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

[PATCH] D53639: [autocompletion] Handle the space before pressing tab

[Raphael Isemann via Phabricator via cfe-commits]

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

LGTM!


https://reviews.llvm.org/D53639



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

[PATCH] D52730: [analyzer] ConversionChecker: handle floating point

[Donát Nagy via Phabricator via cfe-commits]

donat.nagy added inline comments.



Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:163
+
+switch (FloatingSize) {
+  case 64:

NoQ wrote:
> Continuing the float semantics discussion on the new revision - Did you 
> consider `llvm::APFloat`? (http://llvm.org/doxygen/classllvm_1_1APFloat.html) 
> This looks like something that you're trying to re-implement.
This switch statement essentially fulfills two functions: it maps QualTypes to 
standardized IEEE floating point types and it immediately maps the standardized 
IEEE types to  their precision values.

The difficulty is that I don't think that the first map is available as a 
public function in the clang libraries. This means that although a switch over 
the bit width of the floating point type is certainly inelegant, I cannot avoid 
it.

The second map is available in the APFloat library (via the llvm::fltSemantics 
constants, which describe the standard IEEE types). However, this map is 
extremely stable (e.g. I don't think that the binary structure of the IEEE 
double type will ever change), so I think that re-implementing it is justified 
by the fact that it yields shorter and cleaner code. However, as I had noted 
previously, I am open to using the llvm::fltSemantics constants to implement 
this mapping.

As an additional remark, my current code supports the _Float16 type, which is 
currently not supported by the APFloat/fltSemantics library. If we decide to 
use the llvm::fltSemantics constants, then we must either extend the APFloat 
library or apply some workarounds for this case.  



Repository:
  rC Clang

https://reviews.llvm.org/D52730



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

[PATCH] D53543: [analyzer] MallocChecker: pr39348: Realize that sized delete isn't a custom delete.

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

Szelethus added inline comments.



Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:727
 
-  // Skip all operator new/delete methods.
-  if (isa(FD))
-return false;
-
-  // Return true if tested operator is a standard placement nothrow operator.
-  if (FD->getNumParams() == 2) {
-QualType T = FD->getParamDecl(1)->getType();
-if (const IdentifierInfo *II = T.getBaseTypeIdentifier())
-  return II->getName().equals("nothrow_t");
-  }
-
-  // Skip placement operators.
-  if (FD->getNumParams() != 1 || FD->isVariadic())
-return false;
-
-  // One of the standard new/new[]/delete/delete[] non-placement operators.
-  return true;
+  // This is standard iff it's not defined in a user file.
+  SourceLocation L = FD->getLocation();

typo: if



Comment at: test/Analysis/NewDelete-custom.cpp:3-4
 // RUN: %clang_analyze_cc1 
-analyzer-checker=core,cplusplus.NewDelete,cplusplus.NewDeleteLeaks,unix.Malloc 
-std=c++11 -analyzer-config c++-allocator-inlining=false -DLEAKS=1 -fblocks 
-verify %s
 // RUN: %clang_analyze_cc1 
-analyzer-checker=core,cplusplus.NewDelete,unix.Malloc -std=c++11 
-DALLOCATOR_INLINING=1 -fblocks -verify %s
 // RUN: %clang_analyze_cc1 
-analyzer-checker=core,cplusplus.NewDelete,cplusplus.NewDeleteLeaks,unix.Malloc 
-std=c++11 -DLEAKS=1 -DALLOCATOR_INLINING=1 -fblocks -verify %s
 #include "Inputs/system-header-simulator-cxx.h"

Do we still need `-DLEAKS=1` and ` -DALLOCATOR_INLINING=1`?


https://reviews.llvm.org/D53543



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

[PATCH] D53635: [CodeComplete] Expose InBaseClass signal in code completion results.

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




Comment at: lib/Sema/CodeCompleteConsumer.cpp:557
+  if (!Tags.empty())
+OS << " (" << llvm::join(Tags, ",") << ")";
+  if (CodeCompletionString *CCS = Results[I].CreateCodeCompletionString(

NIT: Maybe add a space between the items, i.e. join with `", "` as a separator?


Repository:
  rC Clang

https://reviews.llvm.org/D53635



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

[PATCH] D53638: [clangd] Downrank members from base class

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: unittests/clangd/QualityTests.cpp:187
   Relevance.merge(CodeCompletionResult(&findDecl(AST, "S::S"), 42));
-  EXPECT_EQ(Relevance.Scope, SymbolRelevanceSignals::GlobalScope);
 }

The test case was (accidentally?) removed. Maybe restore?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53638



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

[PATCH] D53488: [clang-tidy] Catching narrowing from double to float.

[Guillaume Chatelet via Phabricator via cfe-commits]

gchatelet updated this revision to Diff 170851.
gchatelet added a comment.

- Add documentation to ReleaseNotes


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53488

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

Index: test/clang-tidy/cppcoreguidelines-narrowing-conversions.cpp
===
--- test/clang-tidy/cppcoreguidelines-narrowing-conversions.cpp
+++ test/clang-tidy/cppcoreguidelines-narrowing-conversions.cpp
@@ -9,12 +9,12 @@
 namespace floats {
 
 struct ConvertsToFloat {
-  operator float() const { return 0.5; }
+  operator float() const { return 0.5f; }
 };
 
-float operator "" _Pa(unsigned long long);
+float operator"" _float(unsigned long long);
 
-void not_ok(double d) {
+void narrow_double_to_int_not_ok(double d) {
   int i = 0;
   i = d;
   // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: narrowing conversion from 'double' to 'int' [cppcoreguidelines-narrowing-conversions]
@@ -24,11 +24,11 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: narrowing conversion from 'float' to 'int' [cppcoreguidelines-narrowing-conversions]
   i = ConvertsToFloat();
   // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: narrowing conversion from 'float' to 'int' [cppcoreguidelines-narrowing-conversions]
-  i = 15_Pa;
+  i = 15_float;
   // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: narrowing conversion from 'float' to 'int' [cppcoreguidelines-narrowing-conversions]
 }
 
-void not_ok_binary_ops(double d) {
+void narrow_double_to_int_not_ok_binary_ops(double d) {
   int i = 0;
   i += 0.5;
   // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: narrowing conversion from 'double' to 'int' [cppcoreguidelines-narrowing-conversions]
@@ -52,6 +52,43 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: narrowing conversion from 'double' to 'int' [cppcoreguidelines-narrowing-conversions]
 }
 
+double operator"" _double(unsigned long long);
+
+float narrow_double_to_float_return() {
+  return 0.5;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: narrowing conversion from 'double' to 'float' [cppcoreguidelines-narrowing-conversions]
+}
+
+void narrow_double_to_float_not_ok(double d) {
+  float f = 0;
+  f = d;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: narrowing conversion from 'double' to 'float' [cppcoreguidelines-narrowing-conversions]
+  f = 0.5;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: narrowing conversion from 'double' to 'float' [cppcoreguidelines-narrowing-conversions]
+  f = 15_double;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: narrowing conversion from 'double' to 'float' [cppcoreguidelines-narrowing-conversions]
+}
+
+void narrow_double_to_float_not_ok_binary_ops(double d) {
+  float f = 0;
+  f += 0.5;
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: narrowing conversion from 'double' to 'float' [cppcoreguidelines-narrowing-conversions]
+  f += d;
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: narrowing conversion from 'double' to 'float' [cppcoreguidelines-narrowing-conversions]
+  // We warn on the following even though it's not dangerous because there is no
+  // reason to use a double literal here.
+  // TODO(courbet): Provide an automatic fix.
+  f += 2.0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: narrowing conversion from 'double' to 'float' [cppcoreguidelines-narrowing-conversions]
+
+  f *= 0.5;
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: narrowing conversion from 'double' to 'float' [cppcoreguidelines-narrowing-conversions]
+  f /= 0.5;
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: narrowing conversion from 'double' to 'float' [cppcoreguidelines-narrowing-conversions]
+  f += (double)0.5f;
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: narrowing conversion from 'double' to 'float' [cppcoreguidelines-narrowing-conversions]
+}
+
 void ok(double d) {
   int i = 0;
   i = 1;
@@ -89,15 +126,19 @@
 
 void template_context() {
   f(1, 2);
+  f(1, .5f);
   f(1, .5);
+  f(1, .5l);
 }
 
 #define DERP(i, j) (i += j)
 
 void macro_context() {
   int i = 0;
   DERP(i, 2);
+  DERP(i, .5f);
   DERP(i, .5);
+  DERP(i, .5l);
 }
 
-}  // namespace floats
+} // namespace floats
Index: docs/clang-tidy/checks/cppcoreguidelines-narrowing-conversions.rst
===
--- docs/clang-tidy/checks/cppcoreguidelines-narrowing-conversions.rst
+++ docs/clang-tidy/checks/cppcoreguidelines-narrowing-conversions.rst
@@ -10,13 +10,15 @@
 This rule is part of the "Expressions and statements" profile of the C++ Core
 Guidelines, corresponding to rule ES.46. See
 
-https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Res-narrowing.
+https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#es46-avoid-lossy-narrowing-truncating-arithmetic-con

[PATCH] D53635: [CodeComplete] Expose InBaseClass signal in code completion results.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

And another NIT :-)




Comment at: lib/Sema/CodeCompleteConsumer.cpp:548
 OS << "COMPLETION: ";
+std::vector Tags;
 switch (Results[I].Kind) {

NIT: maybe move Tags into the corresponding case handler?

Would require putting a set of braces around it, but that shouldn't make the 
code less readable.


Repository:
  rC Clang

https://reviews.llvm.org/D53635



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

r345132 - Support accepting __gnu__ as a scoped attribute namespace that aliases to gnu.

[Aaron Ballman via cfe-commits]

Author: aaronballman
Date: Wed Oct 24 05:26:23 2018
New Revision: 345132

URL: http://llvm.org/viewvc/llvm-project?rev=345132&view=rev
Log:
Support accepting __gnu__ as a scoped attribute namespace that aliases to gnu.

This is useful in libstdc++ to avoid clashes with identifiers in the user's 
namespace.

Modified:
cfe/trunk/include/clang/Sema/ParsedAttr.h
cfe/trunk/lib/Basic/Attributes.cpp
cfe/trunk/lib/Parse/ParseDeclCXX.cpp
cfe/trunk/lib/Sema/ParsedAttr.cpp
cfe/trunk/lib/Sema/SemaDeclAttr.cpp
cfe/trunk/lib/Sema/SemaType.cpp
cfe/trunk/test/Preprocessor/has_attribute.cpp
cfe/trunk/test/SemaCXX/attr-gnu.cpp
cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp

Modified: cfe/trunk/include/clang/Sema/ParsedAttr.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/ParsedAttr.h?rev=345132&r1=345131&r2=345132&view=diff
==
--- cfe/trunk/include/clang/Sema/ParsedAttr.h (original)
+++ cfe/trunk/include/clang/Sema/ParsedAttr.h Wed Oct 24 05:26:23 2018
@@ -383,6 +383,11 @@ public:
   IdentifierInfo *getScopeName() const { return ScopeName; }
   SourceLocation getScopeLoc() const { return ScopeLoc; }
 
+  bool isGNUScope() const {
+return ScopeName &&
+   (ScopeName->isStr("gnu") || ScopeName->isStr("__gnu__"));
+  }
+
   bool hasParsedType() const { return HasParsedType; }
 
   /// Is this the Microsoft __declspec(property) attribute?

Modified: cfe/trunk/lib/Basic/Attributes.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Attributes.cpp?rev=345132&r1=345131&r2=345132&view=diff
==
--- cfe/trunk/lib/Basic/Attributes.cpp (original)
+++ cfe/trunk/lib/Basic/Attributes.cpp Wed Oct 24 05:26:23 2018
@@ -9,12 +9,17 @@ int clang::hasAttribute(AttrSyntax Synta
 const LangOptions &LangOpts) {
   StringRef Name = Attr->getName();
   // Normalize the attribute name, __foo__ becomes foo.
-  if (Name.size() >= 4 && Name.startswith("__") && Name.endswith("__"))
-Name = Name.substr(2, Name.size() - 4);
-
-#include "clang/Basic/AttrHasAttributeImpl.inc"
-
-  return 0;
+  if (Name.size() >= 4 && Name.startswith("__") && Name.endswith("__"))
+Name = Name.substr(2, Name.size() - 4);
+
+  // Normalize the scope name, but only for gnu attributes.
+  StringRef ScopeName = Scope ? Scope->getName() : "";
+  if (ScopeName == "__gnu__")
+ScopeName = ScopeName.slice(2, ScopeName.size() - 2);
+
+#include "clang/Basic/AttrHasAttributeImpl.inc"
+
+  return 0;
 }
 
 const char *attr::getSubjectMatchRuleSpelling(attr::SubjectMatchRule Rule) {

Modified: cfe/trunk/lib/Parse/ParseDeclCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDeclCXX.cpp?rev=345132&r1=345131&r2=345132&view=diff
==
--- cfe/trunk/lib/Parse/ParseDeclCXX.cpp (original)
+++ cfe/trunk/lib/Parse/ParseDeclCXX.cpp Wed Oct 24 05:26:23 2018
@@ -3865,13 +3865,14 @@ bool Parser::ParseCXX11AttributeArgs(Ide
 // Eat the left paren, then skip to the ending right paren.
 ConsumeParen();
 SkipUntil(tok::r_paren);
-return false;
-  }
-
-  if (ScopeName && ScopeName->getName() == "gnu") {
-// GNU-scoped attributes have some special cases to handle GNU-specific
-// behaviors.
-ParseGNUAttributeArgs(AttrName, AttrNameLoc, Attrs, EndLoc, ScopeName,
+return false;
+  }
+
+  if (ScopeName &&
+  (ScopeName->getName() == "gnu" || ScopeName->getName() == "__gnu__")) {
+// GNU-scoped attributes have some special cases to handle GNU-specific
+// behaviors.
+ParseGNUAttributeArgs(AttrName, AttrNameLoc, Attrs, EndLoc, ScopeName,
   ScopeLoc, Syntax, nullptr);
 return true;
   }

Modified: cfe/trunk/lib/Sema/ParsedAttr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/ParsedAttr.cpp?rev=345132&r1=345131&r2=345132&view=diff
==
--- cfe/trunk/lib/Sema/ParsedAttr.cpp (original)
+++ cfe/trunk/lib/Sema/ParsedAttr.cpp Wed Oct 24 05:26:23 2018
@@ -103,14 +103,25 @@ void AttributePool::takePool(AttributePo
 
 #include "clang/Sema/AttrParsedAttrKinds.inc"
 
-static StringRef normalizeAttrName(StringRef AttrName, StringRef ScopeName,
+static StringRef normalizeAttrScopeName(StringRef ScopeName,
+ParsedAttr::Syntax SyntaxUsed) {
+  // We currently only normalize the "__gnu__" scope name to be "gnu".
+  if ((SyntaxUsed == ParsedAttr::AS_CXX11 ||
+   SyntaxUsed == ParsedAttr::AS_C2x) &&
+  ScopeName == "__gnu__")
+ScopeName = ScopeName.slice(2, ScopeName.size() - 2);
+  return ScopeName;
+}
+
+static StringRef normalizeAttrName(StringRef AttrName,
+   StringRef NormalizedScopeName,
 

[PATCH] D53591: Support the __gnu__ scoped attribute token

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman closed this revision.
aaron.ballman added a comment.

In https://reviews.llvm.org/D53591#1273689, @rsmith wrote:

> Looks good.


Thanks! I've commit in r345132.

> We should provide some reserved-name way of naming clang attributes too, but 
> I'm not sure what would be best (`__clang__` is a macro name). Maybe 
> `_Clang`? (And maybe we should detect an integer literal `1` as a scope 
> specifier and check whether it was written as a macro `__clang__`, and 
> provide a custom error suggesting using whatever other scope specifier we 
> prefer.)

I agree. That's the approach I was thinking of taking as well. I wasn't quite 
certain if we needed `_Clang` though if we can just make `__clang__` work 
through macro hacks.

Should `#pragma clang attribute` also be updated to support `#pragma 
_Clang/__clang__ attribute`?


https://reviews.llvm.org/D53591



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

[PATCH] D53488: [clang-tidy] Catching narrowing from double to float.

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.



Comment at: docs/ReleaseNotes.rst:166
 
+- The :doc:`cppcoreguidelines-narrowing-conversions
+` check.

please concat the two parts, similar to the one above. 


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53488



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

[PATCH] D53488: [clang-tidy] Catching narrowing from double to float.

[Guillaume Chatelet via Phabricator via cfe-commits]

gchatelet added a comment.

In https://reviews.llvm.org/D53488#1273986, @JonasToth wrote:

> Please add a short notice in the release notes for this change.


Sorry I keep on missing to update doc/release notes.
Do you see anything else to add to the Patch?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53488



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

[PATCH] D53635: [CodeComplete] Expose InBaseClass signal in code completion results.

[Eric Liu via Phabricator via cfe-commits]

ioeric updated this revision to Diff 170854.
ioeric marked an inline comment as done.
ioeric added a comment.

- move tags into case.


Repository:
  rC Clang

https://reviews.llvm.org/D53635

Files:
  include/clang/Sema/CodeCompleteConsumer.h
  lib/Sema/CodeCompleteConsumer.cpp
  lib/Sema/SemaCodeComplete.cpp
  test/CodeCompletion/member-access.cpp
  test/CodeCompletion/objc-message.mm

Index: test/CodeCompletion/objc-message.mm
===
--- test/CodeCompletion/objc-message.mm
+++ test/CodeCompletion/objc-message.mm
@@ -41,6 +41,6 @@
 // RUN: %clang_cc1 -fsyntax-only -std=c++11 -code-completion-at=%s:33:8 %s -o - | FileCheck -check-prefix=CHECK-CC1 %s
 // CHECK-CC1: categoryInstanceMethod : [#id#]categoryInstanceMethod
 // CHECK-CC1: instanceMethod1 : [#id#]instanceMethod1
-// CHECK-CC1: protocolInstanceMethod : [#id#]protocolInstanceMethod
+// CHECK-CC1: protocolInstanceMethod (InBase) : [#id#]protocolInstanceMethod
 // RUN: %clang_cc1 -fsyntax-only -std=c++11 -code-completion-at=%s:38:8 %s -o - | FileCheck -check-prefix=CHECK-CC2 %s
 // CHECK-CC2: protocolInstanceMethod : [#id#]protocolInstanceMethod
Index: test/CodeCompletion/member-access.cpp
===
--- test/CodeCompletion/member-access.cpp
+++ test/CodeCompletion/member-access.cpp
@@ -51,16 +51,16 @@
 };
 
   // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:31:6 %s -o - | FileCheck -check-prefix=CHECK-CC1 --implicit-check-not="Derived : Derived(" %s
-  // CHECK-CC1: Base1 : Base1::
-  // CHECK-CC1: member1 : [#int#][#Base1::#]member1
-  // CHECK-CC1: member1 : [#int#][#Base2::#]member1
-  // CHECK-CC1: member2 : [#float#][#Base1::#]member2
-  // CHECK-CC1: member3
+  // CHECK-CC1: Base1 (InBase) : Base1::
+  // CHECK-CC1: member1 (InBase) : [#int#][#Base1::#]member1
+  // CHECK-CC1: member1 (InBase) : [#int#][#Base2::#]member1
+  // CHECK-CC1: member2 (InBase) : [#float#][#Base1::#]member2
+  // CHECK-CC1: member3 (InBase)
   // CHECK-CC1: member4
-  // CHECK-CC1: memfun1 : [#void#][#Base3::#]memfun1(<#float#>)
-  // CHECK-CC1: memfun1 : [#void#][#Base3::#]memfun1(<#double#>)[# const#]
-  // CHECK-CC1: memfun1 (Hidden) : [#void#]Base2::memfun1(<#int#>)
-  // CHECK-CC1: memfun2 : [#void#][#Base3::#]memfun2(<#int#>)
+  // CHECK-CC1: memfun1 (InBase) : [#void#][#Base3::#]memfun1(<#float#>)
+  // CHECK-CC1: memfun1 (InBase) : [#void#][#Base3::#]memfun1(<#double#>)[# const#]
+  // CHECK-CC1: memfun1 (Hidden,InBase) : [#void#]Base2::memfun1(<#int#>)
+  // CHECK-CC1: memfun2 (InBase) : [#void#][#Base3::#]memfun2(<#int#>)
   // CHECK-CC1: memfun3 : [#int#]memfun3(<#int#>)
 
 // Make sure this doesn't crash
@@ -93,12 +93,12 @@
   TemplateClass *object2) {
   object.field;
   object2->field;
-// CHECK-CC2: baseTemplateField : [#T#][#BaseTemplate::#]baseTemplateField
-// CHECK-CC2: baseTemplateFunction : [#T#][#BaseTemplate::#]baseTemplateFunction()
+// CHECK-CC2: baseTemplateField (InBase) : [#T#][#BaseTemplate::#]baseTemplateField
+// CHECK-CC2: baseTemplateFunction (InBase) : [#T#][#BaseTemplate::#]baseTemplateFunction()
 // CHECK-CC2: field : [#T#]field
 // CHECK-CC2: function : [#T#]function()
-// CHECK-CC2: member1 : [#int#][#Base1::#]member1
-// CHECK-CC2: member2 : [#float#][#Base1::#]member2
+// CHECK-CC2: member1 (InBase) : [#int#][#Base1::#]member1
+// CHECK-CC2: member2 (InBase) : [#float#][#Base1::#]member2
 // CHECK-CC2: overload1 : [#void#]overload1(<#const T &#>)
 // CHECK-CC2: overload1 : [#void#]overload1(<#const S &#>)
 
@@ -111,12 +111,12 @@
  TemplateClass *object2) {
   object.field;
   object2->field;
-// CHECK-CC3: baseTemplateField : [#int#][#BaseTemplate::#]baseTemplateField
-// CHECK-CC3: baseTemplateFunction : [#int#][#BaseTemplate::#]baseTemplateFunction()
+// CHECK-CC3: baseTemplateField (InBase) : [#int#][#BaseTemplate::#]baseTemplateField
+// CHECK-CC3: baseTemplateFunction (InBase) : [#int#][#BaseTemplate::#]baseTemplateFunction()
 // CHECK-CC3: field : [#int#]field
 // CHECK-CC3: function : [#int#]function()
-// CHECK-CC3: member1 : [#int#][#Base1::#]member1
-// CHECK-CC3: member2 : [#float#][#Base1::#]member2
+// CHECK-CC3: member1 (InBase) : [#int#][#Base1::#]member1
+// CHECK-CC3: member2 (InBase) : [#float#][#Base1::#]member2
 // CHECK-CC3: overload1 : [#void#]overload1(<#const int &#>)
 // CHECK-CC3: overload1 : [#void#]overload1(<#const double &#>)
 
@@ -182,31 +182,31 @@
 }
 
 // RUN: %clang_cc1 -fsyntax-only -code-completion-with-fixits -code-completion-at=%s:177:6 %s -o - | FileCheck -check-prefix=CHECK-CC8 --implicit-check-not="Derived : Derived(" %s
-// CHECK-CC8: Base1 : Base1::
-// CHECK-CC8: member1 : [#int#][#Base1::#]member1
-// CHECK-CC8: member1 : [#int#][#Base2::#]member1
-// CHECK-CC8: member2 : [#float#][#Base1::#]member2
-// CHECK-CC8: member3 : [#double#][#Base2::#]member3
+// CHECK-CC8: Base1 (InBase) : Base1::
+

r345133 - [autocompletion] Handle the space before pressing tab

[Yuka Takahashi via cfe-commits]

Author: yamaguchi
Date: Wed Oct 24 05:43:25 2018
New Revision: 345133

URL: http://llvm.org/viewvc/llvm-project?rev=345133&view=rev
Log:
[autocompletion] Handle the space before pressing tab

Summary:
Distinguish "--autocomplete=-someflag" and "--autocomplete=-someflag,"
because the latter indicates that the user put a space before pushing tab
which should end up in a file completion.

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

Modified:
cfe/trunk/lib/Driver/Driver.cpp
cfe/trunk/test/Driver/autocomplete.c

Modified: cfe/trunk/lib/Driver/Driver.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Driver.cpp?rev=345133&r1=345132&r2=345133&view=diff
==
--- cfe/trunk/lib/Driver/Driver.cpp (original)
+++ cfe/trunk/lib/Driver/Driver.cpp Wed Oct 24 05:43:25 2018
@@ -1508,6 +1508,11 @@ void Driver::HandleAutocompletions(Strin
   unsigned short DisableFlags =
   options::NoDriverOption | options::Unsupported | options::Ignored;
 
+  // Distinguish "--autocomplete=-someflag" and "--autocomplete=-someflag,"
+  // because the latter indicates that the user put space before pushing tab
+  // which should end up in a file completion.
+  const bool HasSpace = PassedFlags.endswith(",");
+
   // Parse PassedFlags by "," as all the command-line flags are passed to this
   // function separated by ","
   StringRef TargetFlags = PassedFlags;
@@ -1534,6 +1539,16 @@ void Driver::HandleAutocompletions(Strin
   if (SuggestedCompletions.empty())
 SuggestedCompletions = Opts->suggestValueCompletions(Cur, "");
 
+  // If Flags were empty, it means the user typed `clang [tab]` where we should
+  // list all possible flags. If there was no value completion and the user
+  // pressed tab after a space, we should fall back to a file completion.
+  // We're printing a newline to be consistent with what we print at the end of
+  // this function.
+  if (SuggestedCompletions.empty() && HasSpace && !Flags.empty()) {
+llvm::outs() << '\n';
+return;
+  }
+
   // When flag ends with '=' and there was no value completion, return empty
   // string and fall back to the file autocompletion.
   if (SuggestedCompletions.empty() && !Cur.endswith("=")) {

Modified: cfe/trunk/test/Driver/autocomplete.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/autocomplete.c?rev=345133&r1=345132&r2=345133&view=diff
==
--- cfe/trunk/test/Driver/autocomplete.c (original)
+++ cfe/trunk/test/Driver/autocomplete.c Wed Oct 24 05:43:25 2018
@@ -25,6 +25,7 @@
 // STDLIBALL-NEXT: platform
 // RUN: %clang --autocomplete=-meabi,d | FileCheck %s -check-prefix=MEABI
 // MEABI: default
+// RUN: %clang --autocomplete=-meabi, | FileCheck %s -check-prefix=MEABIALL
 // RUN: %clang --autocomplete=-meabi | FileCheck %s -check-prefix=MEABIALL
 // MEABIALL: 4
 // MEABIALL-NEXT: 5
@@ -121,3 +122,8 @@
 // MODULE_FILE_EQUAL-NOT: -fmodule-file=
 // RUN: %clang --autocomplete=-fmodule-file | FileCheck %s 
-check-prefix=MODULE_FILE
 // MODULE_FILE: -fmodule-file=
+
+// RUN: %clang --autocomplete=-Qunused-arguments, | FileCheck %s 
-check-prefix=QUNUSED_COMMA
+// QUNUSED_COMMA-NOT: -Qunused-arguments
+// RUN: %clang --autocomplete=-Qunused-arguments | FileCheck %s 
-check-prefix=QUNUSED
+// QUNUSED: -Qunused-arguments


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

[PATCH] D53639: [autocompletion] Handle the space before pressing tab

[Yuka Takahashi via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL345133: [autocompletion] Handle the space before pressing 
tab (authored by yamaguchi, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D53639?vs=170849&id=170857#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D53639

Files:
  cfe/trunk/lib/Driver/Driver.cpp
  cfe/trunk/test/Driver/autocomplete.c


Index: cfe/trunk/test/Driver/autocomplete.c
===
--- cfe/trunk/test/Driver/autocomplete.c
+++ cfe/trunk/test/Driver/autocomplete.c
@@ -25,6 +25,7 @@
 // STDLIBALL-NEXT: platform
 // RUN: %clang --autocomplete=-meabi,d | FileCheck %s -check-prefix=MEABI
 // MEABI: default
+// RUN: %clang --autocomplete=-meabi, | FileCheck %s -check-prefix=MEABIALL
 // RUN: %clang --autocomplete=-meabi | FileCheck %s -check-prefix=MEABIALL
 // MEABIALL: 4
 // MEABIALL-NEXT: 5
@@ -121,3 +122,8 @@
 // MODULE_FILE_EQUAL-NOT: -fmodule-file=
 // RUN: %clang --autocomplete=-fmodule-file | FileCheck %s 
-check-prefix=MODULE_FILE
 // MODULE_FILE: -fmodule-file=
+
+// RUN: %clang --autocomplete=-Qunused-arguments, | FileCheck %s 
-check-prefix=QUNUSED_COMMA
+// QUNUSED_COMMA-NOT: -Qunused-arguments
+// RUN: %clang --autocomplete=-Qunused-arguments | FileCheck %s 
-check-prefix=QUNUSED
+// QUNUSED: -Qunused-arguments
Index: cfe/trunk/lib/Driver/Driver.cpp
===
--- cfe/trunk/lib/Driver/Driver.cpp
+++ cfe/trunk/lib/Driver/Driver.cpp
@@ -1508,6 +1508,11 @@
   unsigned short DisableFlags =
   options::NoDriverOption | options::Unsupported | options::Ignored;
 
+  // Distinguish "--autocomplete=-someflag" and "--autocomplete=-someflag,"
+  // because the latter indicates that the user put space before pushing tab
+  // which should end up in a file completion.
+  const bool HasSpace = PassedFlags.endswith(",");
+
   // Parse PassedFlags by "," as all the command-line flags are passed to this
   // function separated by ","
   StringRef TargetFlags = PassedFlags;
@@ -1534,6 +1539,16 @@
   if (SuggestedCompletions.empty())
 SuggestedCompletions = Opts->suggestValueCompletions(Cur, "");
 
+  // If Flags were empty, it means the user typed `clang [tab]` where we should
+  // list all possible flags. If there was no value completion and the user
+  // pressed tab after a space, we should fall back to a file completion.
+  // We're printing a newline to be consistent with what we print at the end of
+  // this function.
+  if (SuggestedCompletions.empty() && HasSpace && !Flags.empty()) {
+llvm::outs() << '\n';
+return;
+  }
+
   // When flag ends with '=' and there was no value completion, return empty
   // string and fall back to the file autocompletion.
   if (SuggestedCompletions.empty() && !Cur.endswith("=")) {


Index: cfe/trunk/test/Driver/autocomplete.c
===
--- cfe/trunk/test/Driver/autocomplete.c
+++ cfe/trunk/test/Driver/autocomplete.c
@@ -25,6 +25,7 @@
 // STDLIBALL-NEXT: platform
 // RUN: %clang --autocomplete=-meabi,d | FileCheck %s -check-prefix=MEABI
 // MEABI: default
+// RUN: %clang --autocomplete=-meabi, | FileCheck %s -check-prefix=MEABIALL
 // RUN: %clang --autocomplete=-meabi | FileCheck %s -check-prefix=MEABIALL
 // MEABIALL: 4
 // MEABIALL-NEXT: 5
@@ -121,3 +122,8 @@
 // MODULE_FILE_EQUAL-NOT: -fmodule-file=
 // RUN: %clang --autocomplete=-fmodule-file | FileCheck %s -check-prefix=MODULE_FILE
 // MODULE_FILE: -fmodule-file=
+
+// RUN: %clang --autocomplete=-Qunused-arguments, | FileCheck %s -check-prefix=QUNUSED_COMMA
+// QUNUSED_COMMA-NOT: -Qunused-arguments
+// RUN: %clang --autocomplete=-Qunused-arguments | FileCheck %s -check-prefix=QUNUSED
+// QUNUSED: -Qunused-arguments
Index: cfe/trunk/lib/Driver/Driver.cpp
===
--- cfe/trunk/lib/Driver/Driver.cpp
+++ cfe/trunk/lib/Driver/Driver.cpp
@@ -1508,6 +1508,11 @@
   unsigned short DisableFlags =
   options::NoDriverOption | options::Unsupported | options::Ignored;
 
+  // Distinguish "--autocomplete=-someflag" and "--autocomplete=-someflag,"
+  // because the latter indicates that the user put space before pushing tab
+  // which should end up in a file completion.
+  const bool HasSpace = PassedFlags.endswith(",");
+
   // Parse PassedFlags by "," as all the command-line flags are passed to this
   // function separated by ","
   StringRef TargetFlags = PassedFlags;
@@ -1534,6 +1539,16 @@
   if (SuggestedCompletions.empty())
 SuggestedCompletions = Opts->suggestValueCompletions(Cur, "");
 
+  // If Flags were empty, it means the user typed `clang [tab]` where we should
+  // list all possible flags. If there was no value completion and the user
+  // pressed tab after a space, we should fall back to a file completion.
+  // We're printing a newline t

[PATCH] D53635: [CodeComplete] Expose InBaseClass signal in code completion results.

[Eric Liu via Phabricator via cfe-commits]

ioeric updated this revision to Diff 170855.
ioeric added a comment.

- Rebase


Repository:
  rC Clang

https://reviews.llvm.org/D53635

Files:
  include/clang/Sema/CodeCompleteConsumer.h
  lib/Sema/CodeCompleteConsumer.cpp
  lib/Sema/SemaCodeComplete.cpp
  test/CodeCompletion/member-access.cpp
  test/CodeCompletion/objc-message.mm

Index: test/CodeCompletion/objc-message.mm
===
--- test/CodeCompletion/objc-message.mm
+++ test/CodeCompletion/objc-message.mm
@@ -41,6 +41,6 @@
 // RUN: %clang_cc1 -fsyntax-only -std=c++11 -code-completion-at=%s:33:8 %s -o - | FileCheck -check-prefix=CHECK-CC1 %s
 // CHECK-CC1: categoryInstanceMethod : [#id#]categoryInstanceMethod
 // CHECK-CC1: instanceMethod1 : [#id#]instanceMethod1
-// CHECK-CC1: protocolInstanceMethod : [#id#]protocolInstanceMethod
+// CHECK-CC1: protocolInstanceMethod (InBase) : [#id#]protocolInstanceMethod
 // RUN: %clang_cc1 -fsyntax-only -std=c++11 -code-completion-at=%s:38:8 %s -o - | FileCheck -check-prefix=CHECK-CC2 %s
 // CHECK-CC2: protocolInstanceMethod : [#id#]protocolInstanceMethod
Index: test/CodeCompletion/member-access.cpp
===
--- test/CodeCompletion/member-access.cpp
+++ test/CodeCompletion/member-access.cpp
@@ -51,16 +51,16 @@
 };
 
   // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:31:6 %s -o - | FileCheck -check-prefix=CHECK-CC1 --implicit-check-not="Derived : Derived(" %s
-  // CHECK-CC1: Base1 : Base1::
-  // CHECK-CC1: member1 : [#int#][#Base1::#]member1
-  // CHECK-CC1: member1 : [#int#][#Base2::#]member1
-  // CHECK-CC1: member2 : [#float#][#Base1::#]member2
-  // CHECK-CC1: member3
+  // CHECK-CC1: Base1 (InBase) : Base1::
+  // CHECK-CC1: member1 (InBase) : [#int#][#Base1::#]member1
+  // CHECK-CC1: member1 (InBase) : [#int#][#Base2::#]member1
+  // CHECK-CC1: member2 (InBase) : [#float#][#Base1::#]member2
+  // CHECK-CC1: member3 (InBase)
   // CHECK-CC1: member4
-  // CHECK-CC1: memfun1 : [#void#][#Base3::#]memfun1(<#float#>)
-  // CHECK-CC1: memfun1 : [#void#][#Base3::#]memfun1(<#double#>)[# const#]
-  // CHECK-CC1: memfun1 (Hidden) : [#void#]Base2::memfun1(<#int#>)
-  // CHECK-CC1: memfun2 : [#void#][#Base3::#]memfun2(<#int#>)
+  // CHECK-CC1: memfun1 (InBase) : [#void#][#Base3::#]memfun1(<#float#>)
+  // CHECK-CC1: memfun1 (InBase) : [#void#][#Base3::#]memfun1(<#double#>)[# const#]
+  // CHECK-CC1: memfun1 (Hidden,InBase) : [#void#]Base2::memfun1(<#int#>)
+  // CHECK-CC1: memfun2 (InBase) : [#void#][#Base3::#]memfun2(<#int#>)
   // CHECK-CC1: memfun3 : [#int#]memfun3(<#int#>)
 
 // Make sure this doesn't crash
@@ -93,12 +93,12 @@
   TemplateClass *object2) {
   object.field;
   object2->field;
-// CHECK-CC2: baseTemplateField : [#T#][#BaseTemplate::#]baseTemplateField
-// CHECK-CC2: baseTemplateFunction : [#T#][#BaseTemplate::#]baseTemplateFunction()
+// CHECK-CC2: baseTemplateField (InBase) : [#T#][#BaseTemplate::#]baseTemplateField
+// CHECK-CC2: baseTemplateFunction (InBase) : [#T#][#BaseTemplate::#]baseTemplateFunction()
 // CHECK-CC2: field : [#T#]field
 // CHECK-CC2: function : [#T#]function()
-// CHECK-CC2: member1 : [#int#][#Base1::#]member1
-// CHECK-CC2: member2 : [#float#][#Base1::#]member2
+// CHECK-CC2: member1 (InBase) : [#int#][#Base1::#]member1
+// CHECK-CC2: member2 (InBase) : [#float#][#Base1::#]member2
 // CHECK-CC2: overload1 : [#void#]overload1(<#const T &#>)
 // CHECK-CC2: overload1 : [#void#]overload1(<#const S &#>)
 
@@ -111,12 +111,12 @@
  TemplateClass *object2) {
   object.field;
   object2->field;
-// CHECK-CC3: baseTemplateField : [#int#][#BaseTemplate::#]baseTemplateField
-// CHECK-CC3: baseTemplateFunction : [#int#][#BaseTemplate::#]baseTemplateFunction()
+// CHECK-CC3: baseTemplateField (InBase) : [#int#][#BaseTemplate::#]baseTemplateField
+// CHECK-CC3: baseTemplateFunction (InBase) : [#int#][#BaseTemplate::#]baseTemplateFunction()
 // CHECK-CC3: field : [#int#]field
 // CHECK-CC3: function : [#int#]function()
-// CHECK-CC3: member1 : [#int#][#Base1::#]member1
-// CHECK-CC3: member2 : [#float#][#Base1::#]member2
+// CHECK-CC3: member1 (InBase) : [#int#][#Base1::#]member1
+// CHECK-CC3: member2 (InBase) : [#float#][#Base1::#]member2
 // CHECK-CC3: overload1 : [#void#]overload1(<#const int &#>)
 // CHECK-CC3: overload1 : [#void#]overload1(<#const double &#>)
 
@@ -182,31 +182,31 @@
 }
 
 // RUN: %clang_cc1 -fsyntax-only -code-completion-with-fixits -code-completion-at=%s:177:6 %s -o - | FileCheck -check-prefix=CHECK-CC8 --implicit-check-not="Derived : Derived(" %s
-// CHECK-CC8: Base1 : Base1::
-// CHECK-CC8: member1 : [#int#][#Base1::#]member1
-// CHECK-CC8: member1 : [#int#][#Base2::#]member1
-// CHECK-CC8: member2 : [#float#][#Base1::#]member2
-// CHECK-CC8: member3 : [#double#][#Base2::#]member3
+// CHECK-CC8: Base1 (InBase) : Base1::
+// CHECK-CC8: member1 (InBase) : [#int#][#Base1::#]memb

[PATCH] D53635: [CodeComplete] Expose InBaseClass signal in code completion results.

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.



Comment at: lib/Sema/CodeCompleteConsumer.cpp:557
+  if (!Tags.empty())
+OS << " (" << llvm::join(Tags, ",") << ")";
+  if (CodeCompletionString *CCS = Results[I].CreateCodeCompletionString(

ilya-biryukov wrote:
> NIT: Maybe add a space between the items, i.e. join with `", "` as a 
> separator?
Considering we put everything in one line, I think it's reasonable to drop the 
spaces to keep the output compact. These tags are more expected to be read by 
tests than human after all.


Repository:
  rC Clang

https://reviews.llvm.org/D53635



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

[PATCH] D53639: [autocompletion] Handle the space before pressing tab

[Yuka Takahashi via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC345133: [autocompletion] Handle the space before pressing 
tab (authored by yamaguchi, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D53639?vs=170849&id=170856#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D53639

Files:
  lib/Driver/Driver.cpp
  test/Driver/autocomplete.c


Index: test/Driver/autocomplete.c
===
--- test/Driver/autocomplete.c
+++ test/Driver/autocomplete.c
@@ -25,6 +25,7 @@
 // STDLIBALL-NEXT: platform
 // RUN: %clang --autocomplete=-meabi,d | FileCheck %s -check-prefix=MEABI
 // MEABI: default
+// RUN: %clang --autocomplete=-meabi, | FileCheck %s -check-prefix=MEABIALL
 // RUN: %clang --autocomplete=-meabi | FileCheck %s -check-prefix=MEABIALL
 // MEABIALL: 4
 // MEABIALL-NEXT: 5
@@ -121,3 +122,8 @@
 // MODULE_FILE_EQUAL-NOT: -fmodule-file=
 // RUN: %clang --autocomplete=-fmodule-file | FileCheck %s 
-check-prefix=MODULE_FILE
 // MODULE_FILE: -fmodule-file=
+
+// RUN: %clang --autocomplete=-Qunused-arguments, | FileCheck %s 
-check-prefix=QUNUSED_COMMA
+// QUNUSED_COMMA-NOT: -Qunused-arguments
+// RUN: %clang --autocomplete=-Qunused-arguments | FileCheck %s 
-check-prefix=QUNUSED
+// QUNUSED: -Qunused-arguments
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -1508,6 +1508,11 @@
   unsigned short DisableFlags =
   options::NoDriverOption | options::Unsupported | options::Ignored;
 
+  // Distinguish "--autocomplete=-someflag" and "--autocomplete=-someflag,"
+  // because the latter indicates that the user put space before pushing tab
+  // which should end up in a file completion.
+  const bool HasSpace = PassedFlags.endswith(",");
+
   // Parse PassedFlags by "," as all the command-line flags are passed to this
   // function separated by ","
   StringRef TargetFlags = PassedFlags;
@@ -1534,6 +1539,16 @@
   if (SuggestedCompletions.empty())
 SuggestedCompletions = Opts->suggestValueCompletions(Cur, "");
 
+  // If Flags were empty, it means the user typed `clang [tab]` where we should
+  // list all possible flags. If there was no value completion and the user
+  // pressed tab after a space, we should fall back to a file completion.
+  // We're printing a newline to be consistent with what we print at the end of
+  // this function.
+  if (SuggestedCompletions.empty() && HasSpace && !Flags.empty()) {
+llvm::outs() << '\n';
+return;
+  }
+
   // When flag ends with '=' and there was no value completion, return empty
   // string and fall back to the file autocompletion.
   if (SuggestedCompletions.empty() && !Cur.endswith("=")) {


Index: test/Driver/autocomplete.c
===
--- test/Driver/autocomplete.c
+++ test/Driver/autocomplete.c
@@ -25,6 +25,7 @@
 // STDLIBALL-NEXT: platform
 // RUN: %clang --autocomplete=-meabi,d | FileCheck %s -check-prefix=MEABI
 // MEABI: default
+// RUN: %clang --autocomplete=-meabi, | FileCheck %s -check-prefix=MEABIALL
 // RUN: %clang --autocomplete=-meabi | FileCheck %s -check-prefix=MEABIALL
 // MEABIALL: 4
 // MEABIALL-NEXT: 5
@@ -121,3 +122,8 @@
 // MODULE_FILE_EQUAL-NOT: -fmodule-file=
 // RUN: %clang --autocomplete=-fmodule-file | FileCheck %s -check-prefix=MODULE_FILE
 // MODULE_FILE: -fmodule-file=
+
+// RUN: %clang --autocomplete=-Qunused-arguments, | FileCheck %s -check-prefix=QUNUSED_COMMA
+// QUNUSED_COMMA-NOT: -Qunused-arguments
+// RUN: %clang --autocomplete=-Qunused-arguments | FileCheck %s -check-prefix=QUNUSED
+// QUNUSED: -Qunused-arguments
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -1508,6 +1508,11 @@
   unsigned short DisableFlags =
   options::NoDriverOption | options::Unsupported | options::Ignored;
 
+  // Distinguish "--autocomplete=-someflag" and "--autocomplete=-someflag,"
+  // because the latter indicates that the user put space before pushing tab
+  // which should end up in a file completion.
+  const bool HasSpace = PassedFlags.endswith(",");
+
   // Parse PassedFlags by "," as all the command-line flags are passed to this
   // function separated by ","
   StringRef TargetFlags = PassedFlags;
@@ -1534,6 +1539,16 @@
   if (SuggestedCompletions.empty())
 SuggestedCompletions = Opts->suggestValueCompletions(Cur, "");
 
+  // If Flags were empty, it means the user typed `clang [tab]` where we should
+  // list all possible flags. If there was no value completion and the user
+  // pressed tab after a space, we should fall back to a file completion.
+  // We're printing a newline to be consistent with what we print at the end of
+  // this function.
+  if (SuggestedCompletions.empty() && HasSpace && !Flags.empty()) {
+llvm::outs() << '\n';
+return;
+ 

[PATCH] D53589: [bash-autocompletion] Fix bug when a flag ends with '='

[Yuka Takahashi via Phabricator via cfe-commits]

yamaguchi closed this revision.
yamaguchi added a comment.

Landed in https://reviews.llvm.org/rL345121


https://reviews.llvm.org/D53589



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

[PATCH] D53641: [clangd] Remove didOpen extraFlags extension.

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added a reviewer: ilya-biryukov.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay, 
ioeric.

This was added in https://reviews.llvm.org/D34947 to support YCM, but YCM 
actually provides *all* args,
and this was never actually used.
Meanwhile, we grew another extension that allows specifying all args.

I did find one user of this extension: https://github.com/thomasjo/atom-ide-cpp.
I'll reach out, there are multiple good alternatives:

- compile_commands.txt can serve the same purpose as .clang_complete there
- we can add an extension to support setting the fallback command


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53641

Files:
  clangd/ClangdLSPServer.cpp
  clangd/ClangdLSPServer.h
  clangd/GlobalCompilationDatabase.cpp
  clangd/GlobalCompilationDatabase.h
  clangd/Protocol.cpp
  clangd/Protocol.h
  test/clangd/extra-flags.test

Index: test/clangd/extra-flags.test
===
--- test/clangd/extra-flags.test
+++ /dev/null
@@ -1,52 +0,0 @@
-# RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
-{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}

-{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///foo.c","languageId":"c","version":1,"text":"int main() { int i; return i; }"},"metadata":{"extraFlags":["-Wall"]}}}
-#  CHECK:  "method": "textDocument/publishDiagnostics",
-# CHECK-NEXT:  "params": {
-# CHECK-NEXT:"diagnostics": [
-# CHECK-NEXT:  {
-# CHECK-NEXT:"message": "Variable 'i' is uninitialized when used here",
-# CHECK-NEXT:"range": {
-# CHECK-NEXT:  "end": {
-# CHECK-NEXT:"character": 28,
-# CHECK-NEXT:"line": 0
-# CHECK-NEXT:  },
-# CHECK-NEXT:  "start": {
-# CHECK-NEXT:"character": 27,
-# CHECK-NEXT:"line": 0
-# CHECK-NEXT:  }
-# CHECK-NEXT:},
-# CHECK-NEXT:"severity": 2
-# CHECK-NEXT:  }
-# CHECK-NEXT:],
-# CHECK-NEXT:"uri": "file://{{.*}}/foo.c"
-# CHECK-NEXT:  }

-{"jsonrpc":"2.0","method":"textDocument/didChange","params":{"textDocument":{"uri":"test:///foo.c","version":2},"contentChanges":[{"text":"int main() { int i; return i+1; }"}]}}
-#  CHECK:  "method": "textDocument/publishDiagnostics",
-# CHECK-NEXT:  "params": {
-# CHECK-NEXT:"diagnostics": [
-# CHECK-NEXT:  {
-# CHECK-NEXT:"message": "Variable 'i' is uninitialized when used here",
-# CHECK-NEXT:"range": {
-# CHECK-NEXT:  "end": {
-# CHECK-NEXT:"character": 28,
-# CHECK-NEXT:"line": 0
-# CHECK-NEXT:  },
-# CHECK-NEXT:  "start": {
-# CHECK-NEXT:"character": 27,
-# CHECK-NEXT:"line": 0
-# CHECK-NEXT:  }
-# CHECK-NEXT:},
-# CHECK-NEXT:"severity": 2
-# CHECK-NEXT:  }
-# CHECK-NEXT:],
-# CHECK-NEXT:"uri": "file://{{.*}}/foo.c"
-# CHECK-NEXT:  }

-{"jsonrpc":"2.0","id":5,"method":"shutdown"}

-{"jsonrpc":"2.0","method":"exit"}
-
-
Index: clangd/Protocol.h
===
--- clangd/Protocol.h
+++ clangd/Protocol.h
@@ -175,11 +175,6 @@
 llvm::json::Value toJSON(const Location &);
 llvm::raw_ostream &operator<<(llvm::raw_ostream &, const Location &);
 
-struct Metadata {
-  std::vector extraFlags;
-};
-bool fromJSON(const llvm::json::Value &, Metadata &);
-
 struct TextEdit {
   /// The range of the text document to be manipulated. To insert
   /// text into a document create a range where start === end.
@@ -411,9 +406,6 @@
 struct DidOpenTextDocumentParams {
   /// The document that was opened.
   TextDocumentItem textDocument;
-
-  /// Extension storing per-file metadata, such as compilation flags.
-  llvm::Optional metadata;
 };
 bool fromJSON(const llvm::json::Value &, DidOpenTextDocumentParams &);
 
Index: clangd/Protocol.cpp
===
--- clangd/Protocol.cpp
+++ clangd/Protocol.cpp
@@ -119,14 +119,6 @@
  O.map("version", R.version) && O.map("text", R.text);
 }
 
-bool fromJSON(const json::Value &Params, Metadata &R) {
-  json::ObjectMapper O(Params);
-  if (!O)
-return false;
-  O.map("extraFlags", R.extraFlags);
-  return true;
-}
-
 bool fromJSON(const json::Value &Params, TextEdit &R) {
   json::ObjectMapper O(Params);
   return O && O.map("range", R.range) && O.map("newText", R.newText);
@@ -262,8 +254,7 @@
 
 bool fromJSON(const json::Value &Params, DidOpenTextDocumentParams &R) {
   json::ObjectMapper O(Params);
-  return O && O.map("textDocument", R.textDocument) &&
- O.map("metadata", R.metadata);
+  return O && O.map("textDocument", R.textDocument);
 }
 
 bool fromJSON(const json::Value &Params, DidCloseTextDocumentParams &R) {
Index: clangd/GlobalCompi

[PATCH] D53339: [clang-tidy] Add the abseil-duration-factory-float check

[Hyrum Wright via Phabricator via cfe-commits]

hwright added inline comments.



Comment at: clang-tidy/abseil/AbseilTidyModule.cpp:32
+CheckFactories.registerCheck(
+"abseil-duration-factory-float");
 CheckFactories.registerCheck(

hokein wrote:
> Maybe drop the `factory`? we already have a duration-related check  
> `abseil-duration-division`, for consistency. 
> 
> `clang-tidy/rename_check.py` may help you.
The expectation is that there may be other checks relating to calls to duration 
factories, beyond just floating point literal and cast simplification.

Though I'm happy to be convinced otherwise.



Comment at: clang-tidy/abseil/DurationFactoryFloatCheck.cpp:94
+  diag(MatchedCall->getBeginLoc(),
+   (llvm::Twine("Use integer version of absl::") +
+MatchedCall->getDirectCallee()->getName())

hokein wrote:
> nit: clang-tidy message is not a complete sentence, use lower letter `use`.
This is a complete sentence, in the imperative style of other clang-tidy 
checks.  I've added a full-stop at the end.


https://reviews.llvm.org/D53339



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

[clang-tools-extra] r345134 - [clangd] Hide position line and column fields.

[Haojian Wu via cfe-commits]

Author: hokein
Date: Wed Oct 24 05:56:41 2018
New Revision: 345134

URL: http://llvm.org/viewvc/llvm-project?rev=345134&view=rev
Log:
[clangd] Hide position line and column fields.

Reviewers: sammccall

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

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

Modified:
clang-tools-extra/trunk/clangd/index/Index.h

Modified: clang-tools-extra/trunk/clangd/index/Index.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Index.h?rev=345134&r1=345133&r2=345134&view=diff
==
--- clang-tools-extra/trunk/clangd/index/Index.h (original)
+++ clang-tools-extra/trunk/clangd/index/Index.h Wed Oct 24 05:56:41 2018
@@ -50,8 +50,7 @@ struct SymbolLocation {
 static constexpr uint32_t MaxLine = (1 << 20) - 1;
 static constexpr uint32_t MaxColumn = (1 << 12) - 1;
 
-// Clients should use getters and setters to access these members.
-// FIXME: hide these members.
+  private:
 uint32_t Line : 20; // 0-based
 // Using UTF-16 code units.
 uint32_t Column : 12; // 0-based


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

r345135 - [CodeComplete] Expose InBaseClass signal in code completion results.

[Eric Liu via cfe-commits]

Author: ioeric
Date: Wed Oct 24 05:57:27 2018
New Revision: 345135

URL: http://llvm.org/viewvc/llvm-project?rev=345135&view=rev
Log:
[CodeComplete] Expose InBaseClass signal in code completion results.

Summary:
No new tests as the existing tests for result priority should give us
coverage. Also as the new flag is trivial enough, I'm reluctant to plumb the
flag to c-index-test output.

Reviewers: ilya-biryukov

Reviewed By: ilya-biryukov

Subscribers: cfe-commits

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

Modified:
cfe/trunk/include/clang/Sema/CodeCompleteConsumer.h
cfe/trunk/lib/Sema/CodeCompleteConsumer.cpp
cfe/trunk/lib/Sema/SemaCodeComplete.cpp
cfe/trunk/test/CodeCompletion/member-access.cpp
cfe/trunk/test/CodeCompletion/objc-message.mm

Modified: cfe/trunk/include/clang/Sema/CodeCompleteConsumer.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/CodeCompleteConsumer.h?rev=345135&r1=345134&r2=345135&view=diff
==
--- cfe/trunk/include/clang/Sema/CodeCompleteConsumer.h (original)
+++ cfe/trunk/include/clang/Sema/CodeCompleteConsumer.h Wed Oct 24 05:57:27 2018
@@ -821,6 +821,9 @@ public:
   /// Whether this result is hidden by another name.
   bool Hidden : 1;
 
+  /// Whether this is a class member from base class.
+  bool InBaseClass : 1;
+
   /// Whether this result was found via lookup into a base class.
   bool QualifierIsInformative : 1;
 
@@ -859,7 +862,7 @@ public:
bool Accessible = true,
std::vector FixIts = 
std::vector())
   : Declaration(Declaration), Priority(Priority), Kind(RK_Declaration),
-FixIts(std::move(FixIts)), Hidden(false),
+FixIts(std::move(FixIts)), Hidden(false), InBaseClass(false),
 QualifierIsInformative(QualifierIsInformative),
 StartsNestedNameSpecifier(false), AllParametersAreInformative(false),
 DeclaringEntity(false), Qualifier(Qualifier) {
@@ -870,7 +873,7 @@ public:
   /// Build a result that refers to a keyword or symbol.
   CodeCompletionResult(const char *Keyword, unsigned Priority = CCP_Keyword)
   : Keyword(Keyword), Priority(Priority), Kind(RK_Keyword),
-CursorKind(CXCursor_NotImplemented), Hidden(false),
+CursorKind(CXCursor_NotImplemented), Hidden(false), InBaseClass(false),
 QualifierIsInformative(false), StartsNestedNameSpecifier(false),
 AllParametersAreInformative(false), DeclaringEntity(false) {}
 
@@ -879,28 +882,29 @@ public:
const MacroInfo *MI = nullptr,
unsigned Priority = CCP_Macro)
   : Macro(Macro), Priority(Priority), Kind(RK_Macro),
-CursorKind(CXCursor_MacroDefinition), Hidden(false),
+CursorKind(CXCursor_MacroDefinition), Hidden(false), 
InBaseClass(false),
 QualifierIsInformative(false), StartsNestedNameSpecifier(false),
 AllParametersAreInformative(false), DeclaringEntity(false),
 MacroDefInfo(MI) {}
 
   /// Build a result that refers to a pattern.
-  CodeCompletionResult(CodeCompletionString *Pattern,
-   unsigned Priority = CCP_CodePattern,
-   CXCursorKind CursorKind = CXCursor_NotImplemented,
-   CXAvailabilityKind Availability = CXAvailability_Available,
-   const NamedDecl *D = nullptr)
+  CodeCompletionResult(
+  CodeCompletionString *Pattern, unsigned Priority = CCP_CodePattern,
+  CXCursorKind CursorKind = CXCursor_NotImplemented,
+  CXAvailabilityKind Availability = CXAvailability_Available,
+  const NamedDecl *D = nullptr)
   : Declaration(D), Pattern(Pattern), Priority(Priority), Kind(RK_Pattern),
 CursorKind(CursorKind), Availability(Availability), Hidden(false),
-QualifierIsInformative(false), StartsNestedNameSpecifier(false),
-AllParametersAreInformative(false), DeclaringEntity(false) {}
+InBaseClass(false), QualifierIsInformative(false),
+StartsNestedNameSpecifier(false), AllParametersAreInformative(false),
+DeclaringEntity(false) {}
 
   /// Build a result that refers to a pattern with an associated
   /// declaration.
   CodeCompletionResult(CodeCompletionString *Pattern, const NamedDecl *D,
unsigned Priority)
   : Declaration(D), Pattern(Pattern), Priority(Priority), Kind(RK_Pattern),
-Hidden(false), QualifierIsInformative(false),
+Hidden(false), InBaseClass(false), QualifierIsInformative(false),
 StartsNestedNameSpecifier(false), AllParametersAreInformative(false),
 DeclaringEntity(false) {
 computeCursorKindAndAvailability();

Modified: cfe/trunk/lib/Sema/CodeCompleteConsumer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/CodeCompleteConsumer.cpp?rev=345135&r1=345134&r2=345135&view=diff
=

[PATCH] D53577: [clangd] Hide position line and column fields.

[Haojian Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL345134: [clangd] Hide position line and column fields. 
(authored by hokein, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D53577

Files:
  clang-tools-extra/trunk/clangd/index/Index.h


Index: clang-tools-extra/trunk/clangd/index/Index.h
===
--- clang-tools-extra/trunk/clangd/index/Index.h
+++ clang-tools-extra/trunk/clangd/index/Index.h
@@ -50,8 +50,7 @@
 static constexpr uint32_t MaxLine = (1 << 20) - 1;
 static constexpr uint32_t MaxColumn = (1 << 12) - 1;
 
-// Clients should use getters and setters to access these members.
-// FIXME: hide these members.
+  private:
 uint32_t Line : 20; // 0-based
 // Using UTF-16 code units.
 uint32_t Column : 12; // 0-based


Index: clang-tools-extra/trunk/clangd/index/Index.h
===
--- clang-tools-extra/trunk/clangd/index/Index.h
+++ clang-tools-extra/trunk/clangd/index/Index.h
@@ -50,8 +50,7 @@
 static constexpr uint32_t MaxLine = (1 << 20) - 1;
 static constexpr uint32_t MaxColumn = (1 << 12) - 1;
 
-// Clients should use getters and setters to access these members.
-// FIXME: hide these members.
+  private:
 uint32_t Line : 20; // 0-based
 // Using UTF-16 code units.
 uint32_t Column : 12; // 0-based
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53635: [CodeComplete] Expose InBaseClass signal in code completion results.

[Eric Liu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL345135: [CodeComplete] Expose InBaseClass signal in code 
completion results. (authored by ioeric, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D53635

Files:
  cfe/trunk/include/clang/Sema/CodeCompleteConsumer.h
  cfe/trunk/lib/Sema/CodeCompleteConsumer.cpp
  cfe/trunk/lib/Sema/SemaCodeComplete.cpp
  cfe/trunk/test/CodeCompletion/member-access.cpp
  cfe/trunk/test/CodeCompletion/objc-message.mm

Index: cfe/trunk/include/clang/Sema/CodeCompleteConsumer.h
===
--- cfe/trunk/include/clang/Sema/CodeCompleteConsumer.h
+++ cfe/trunk/include/clang/Sema/CodeCompleteConsumer.h
@@ -821,6 +821,9 @@
   /// Whether this result is hidden by another name.
   bool Hidden : 1;
 
+  /// Whether this is a class member from base class.
+  bool InBaseClass : 1;
+
   /// Whether this result was found via lookup into a base class.
   bool QualifierIsInformative : 1;
 
@@ -859,7 +862,7 @@
bool Accessible = true,
std::vector FixIts = std::vector())
   : Declaration(Declaration), Priority(Priority), Kind(RK_Declaration),
-FixIts(std::move(FixIts)), Hidden(false),
+FixIts(std::move(FixIts)), Hidden(false), InBaseClass(false),
 QualifierIsInformative(QualifierIsInformative),
 StartsNestedNameSpecifier(false), AllParametersAreInformative(false),
 DeclaringEntity(false), Qualifier(Qualifier) {
@@ -870,37 +873,38 @@
   /// Build a result that refers to a keyword or symbol.
   CodeCompletionResult(const char *Keyword, unsigned Priority = CCP_Keyword)
   : Keyword(Keyword), Priority(Priority), Kind(RK_Keyword),
-CursorKind(CXCursor_NotImplemented), Hidden(false),
+CursorKind(CXCursor_NotImplemented), Hidden(false), InBaseClass(false),
 QualifierIsInformative(false), StartsNestedNameSpecifier(false),
 AllParametersAreInformative(false), DeclaringEntity(false) {}
 
   /// Build a result that refers to a macro.
   CodeCompletionResult(const IdentifierInfo *Macro,
const MacroInfo *MI = nullptr,
unsigned Priority = CCP_Macro)
   : Macro(Macro), Priority(Priority), Kind(RK_Macro),
-CursorKind(CXCursor_MacroDefinition), Hidden(false),
+CursorKind(CXCursor_MacroDefinition), Hidden(false), InBaseClass(false),
 QualifierIsInformative(false), StartsNestedNameSpecifier(false),
 AllParametersAreInformative(false), DeclaringEntity(false),
 MacroDefInfo(MI) {}
 
   /// Build a result that refers to a pattern.
-  CodeCompletionResult(CodeCompletionString *Pattern,
-   unsigned Priority = CCP_CodePattern,
-   CXCursorKind CursorKind = CXCursor_NotImplemented,
-   CXAvailabilityKind Availability = CXAvailability_Available,
-   const NamedDecl *D = nullptr)
+  CodeCompletionResult(
+  CodeCompletionString *Pattern, unsigned Priority = CCP_CodePattern,
+  CXCursorKind CursorKind = CXCursor_NotImplemented,
+  CXAvailabilityKind Availability = CXAvailability_Available,
+  const NamedDecl *D = nullptr)
   : Declaration(D), Pattern(Pattern), Priority(Priority), Kind(RK_Pattern),
 CursorKind(CursorKind), Availability(Availability), Hidden(false),
-QualifierIsInformative(false), StartsNestedNameSpecifier(false),
-AllParametersAreInformative(false), DeclaringEntity(false) {}
+InBaseClass(false), QualifierIsInformative(false),
+StartsNestedNameSpecifier(false), AllParametersAreInformative(false),
+DeclaringEntity(false) {}
 
   /// Build a result that refers to a pattern with an associated
   /// declaration.
   CodeCompletionResult(CodeCompletionString *Pattern, const NamedDecl *D,
unsigned Priority)
   : Declaration(D), Pattern(Pattern), Priority(Priority), Kind(RK_Pattern),
-Hidden(false), QualifierIsInformative(false),
+Hidden(false), InBaseClass(false), QualifierIsInformative(false),
 StartsNestedNameSpecifier(false), AllParametersAreInformative(false),
 DeclaringEntity(false) {
 computeCursorKindAndAvailability();
Index: cfe/trunk/test/CodeCompletion/member-access.cpp
===
--- cfe/trunk/test/CodeCompletion/member-access.cpp
+++ cfe/trunk/test/CodeCompletion/member-access.cpp
@@ -51,16 +51,16 @@
 };
 
   // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:31:6 %s -o - | FileCheck -check-prefix=CHECK-CC1 --implicit-check-not="Derived : Derived(" %s
-  // CHECK-CC1: Base1 : Base1::
-  // CHECK-CC1: member1 : [#int#][#Base1::#]member1
-  // CHECK-CC1: member1 : [#int#][#Base2::#]member1
-  // CHECK-CC1: member2 : [#float#][#Base1::#]member2
-  // CHECK-CC1: member3

[PATCH] D53339: [clang-tidy] Add the abseil-duration-factory-float check

[Haojian Wu via Phabricator via cfe-commits]

hokein accepted this revision.
hokein added a comment.

LGTM.




Comment at: clang-tidy/abseil/DurationFactoryFloatCheck.cpp:94
+  diag(MatchedCall->getBeginLoc(),
+   (llvm::Twine("Use integer version of absl::") +
+MatchedCall->getDirectCallee()->getName())

hwright wrote:
> hokein wrote:
> > nit: clang-tidy message is not a complete sentence, use lower letter `use`.
> This is a complete sentence, in the imperative style of other clang-tidy 
> checks.  I've added a full-stop at the end.
Ah, sorry for not clarifying it clearly. In clang-tidy, we don't use complete 
sentence for warnings, so just remove the "." at the end.


https://reviews.llvm.org/D53339



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

[PATCH] D53571: [clangd] Don't show base class versions of members as completions.

[Eric Liu via Phabricator via cfe-commits]

ioeric added a comment.

> Keep required quailifier machinery around though, for cross-ns completion.

Do we have cross-ns completion in sema?




Comment at: clangd/CodeComplete.cpp:732
+  // Class members that are shadowed by subclasses are usually noise.
+  if (Result.Hidden && Result.Declaration &&
+  Result.Declaration->isCXXClassMember())

Are there hidden results that are neither declarations nor class members and do 
not require qualifier? 


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53571



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

[PATCH] D52690: [clang-tidy] NFC use CHECK-NOTES in tests for misc-misplaced-const

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

ping :)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52690



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

[PATCH] D53642: [clangd] Don't invalidate LSP-set compile commands when closing a file.

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added a reviewer: ilya-biryukov.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay, 
ioeric.

It doesn't make much sense: setting them is not coupled to opening the file,
it's an asynchronous notification.

I don't think this is a breaking change - this behavior is hard to observe!


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53642

Files:
  clangd/ClangdLSPServer.cpp
  clangd/ClangdLSPServer.h
  clangd/GlobalCompilationDatabase.cpp
  clangd/GlobalCompilationDatabase.h


Index: clangd/GlobalCompilationDatabase.h
===
--- clangd/GlobalCompilationDatabase.h
+++ clangd/GlobalCompilationDatabase.h
@@ -100,10 +100,6 @@
   bool setCompilationCommandForFile(PathRef File,
 tooling::CompileCommand 
CompilationCommand);
 
-  /// Removes the compilation command for \p File if it's present in the
-  /// mapping.
-  void invalidate(PathRef File);
-
 private:
   mutable std::mutex Mutex;
   llvm::StringMap Commands; /* GUARDED_BY(Mut) */
Index: clangd/GlobalCompilationDatabase.cpp
===
--- clangd/GlobalCompilationDatabase.cpp
+++ clangd/GlobalCompilationDatabase.cpp
@@ -135,10 +135,5 @@
   return false;
 }
 
-void InMemoryCompilationDb::invalidate(PathRef File) {
-  std::unique_lock Lock(Mutex);
-  Commands.erase(File);
-}
-
 } // namespace clangd
 } // namespace clang
Index: clangd/ClangdLSPServer.h
===
--- clangd/ClangdLSPServer.h
+++ clangd/ClangdLSPServer.h
@@ -113,8 +113,6 @@
 static CompilationDB
 makeDirectoryBased(llvm::Optional CompileCommandsDir);
 
-void invalidate(PathRef File);
-
 /// Sets the compilation command for a particular file.
 /// Only valid for in-memory CDB, no-op and error log on DirectoryBasedCDB.
 ///
Index: clangd/ClangdLSPServer.cpp
===
--- clangd/ClangdLSPServer.cpp
+++ clangd/ClangdLSPServer.cpp
@@ -354,7 +354,6 @@
 // fail rather than giving wrong results.
 DraftMgr.removeDraft(File);
 Server->removeDocument(File);
-CDB->invalidate(File);
 elog("Failed to update {0}: {1}", File, Contents.takeError());
 return;
   }
@@ -450,7 +449,6 @@
   PathRef File = Params.textDocument.uri.file();
   DraftMgr.removeDraft(File);
   Server->removeDocument(File);
-  CDB->invalidate(File);
 }
 
 void ClangdLSPServer::onDocumentOnTypeFormatting(
@@ -765,11 +763,6 @@
/*IsDirectoryBased=*/true);
 }
 
-void ClangdLSPServer::CompilationDB::invalidate(PathRef File) {
-  if (!IsDirectoryBased)
-static_cast(CDB.get())->invalidate(File);
-}
-
 bool ClangdLSPServer::CompilationDB::setCompilationCommandForFile(
 PathRef File, tooling::CompileCommand CompilationCommand) {
   if (IsDirectoryBased) {


Index: clangd/GlobalCompilationDatabase.h
===
--- clangd/GlobalCompilationDatabase.h
+++ clangd/GlobalCompilationDatabase.h
@@ -100,10 +100,6 @@
   bool setCompilationCommandForFile(PathRef File,
 tooling::CompileCommand CompilationCommand);
 
-  /// Removes the compilation command for \p File if it's present in the
-  /// mapping.
-  void invalidate(PathRef File);
-
 private:
   mutable std::mutex Mutex;
   llvm::StringMap Commands; /* GUARDED_BY(Mut) */
Index: clangd/GlobalCompilationDatabase.cpp
===
--- clangd/GlobalCompilationDatabase.cpp
+++ clangd/GlobalCompilationDatabase.cpp
@@ -135,10 +135,5 @@
   return false;
 }
 
-void InMemoryCompilationDb::invalidate(PathRef File) {
-  std::unique_lock Lock(Mutex);
-  Commands.erase(File);
-}
-
 } // namespace clangd
 } // namespace clang
Index: clangd/ClangdLSPServer.h
===
--- clangd/ClangdLSPServer.h
+++ clangd/ClangdLSPServer.h
@@ -113,8 +113,6 @@
 static CompilationDB
 makeDirectoryBased(llvm::Optional CompileCommandsDir);
 
-void invalidate(PathRef File);
-
 /// Sets the compilation command for a particular file.
 /// Only valid for in-memory CDB, no-op and error log on DirectoryBasedCDB.
 ///
Index: clangd/ClangdLSPServer.cpp
===
--- clangd/ClangdLSPServer.cpp
+++ clangd/ClangdLSPServer.cpp
@@ -354,7 +354,6 @@
 // fail rather than giving wrong results.
 DraftMgr.removeDraft(File);
 Server->removeDocument(File);
-CDB->invalidate(File);
 elog("Failed to update {0}: {1}", File, Contents.takeError());
 return;
   }
@@ -450,7 +449,6 @@
   PathRef File = Params.textDocument.uri.file();
   DraftMgr.removeDraft(File);
   Server->removeDocument(File);
-  CDB->invalidate(File);
 }
 
 void Clan

[PATCH] D53399: [clangd] Ensure that we reply to each call exactly once. NFC (I think!)

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.



Comment at: clangd/ClangdLSPServer.cpp:112
+SPAN_ATTACH(Tracer, "Params", Params);
+ReplyOnce Reply(ID, Method, &Server, Tracer.Args);
 log("<-- {0}({1})", Method, ID);

Do we have guarantee that `Tracer.Args` outlives `Reply`?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53399



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

[PATCH] D53571: [clangd] Don't show base class versions of members as completions.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In https://reviews.llvm.org/D53571#1274082, @ioeric wrote:

> > Keep required quailifier machinery around though, for cross-ns completion.
>
> Do we have cross-ns completion in sema?


No, I don't think so. I meant RequiredQualifier etc.
Unfortunately I've got no idea whether there are !QualifierIsInformative cases 
in Sema we still care about, so I don't know whether that logic is safe to 
remove...




Comment at: clangd/CodeComplete.cpp:732
+  // Class members that are shadowed by subclasses are usually noise.
+  if (Result.Hidden && Result.Declaration &&
+  Result.Declaration->isCXXClassMember())

ioeric wrote:
> Are there hidden results that are neither declarations nor class members and 
> do not require qualifier? 
I don't know. I thought a namespace-scope thing shadowed by a member was such a 
case, but it doesn't seem to generate results at all.

But that (hypothetical) example illustrates a point: I do think we would want 
to show ns::foo() if it was shadowed by MyClass()::foo(). The difference is 
that they're probably fundamentally different things that share a name, while 
MySubClass::foo() is usually just a strictly better version of 
MyBaseClass::foo() as far as a caller is concerned.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53571



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

[PATCH] D53638: [clangd] Downrank members from base class

[Eric Liu via Phabricator via cfe-commits]

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



Comment at: unittests/clangd/QualityTests.cpp:187
   Relevance.merge(CodeCompletionResult(&findDecl(AST, "S::S"), 42));
-  EXPECT_EQ(Relevance.Scope, SymbolRelevanceSignals::GlobalScope);
 }

ilya-biryukov wrote:
> The test case was (accidentally?) removed. Maybe restore?
Thanks for the catch!


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53638



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

[PATCH] D53638: [clangd] Downrank members from base class

[Eric Liu via Phabricator via cfe-commits]

ioeric updated this revision to Diff 170862.
ioeric marked an inline comment as done.
ioeric added a comment.

- restore accidentally removed test.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53638

Files:
  clangd/Quality.cpp
  clangd/Quality.h
  unittests/clangd/QualityTests.cpp


Index: unittests/clangd/QualityTests.cpp
===
--- unittests/clangd/QualityTests.cpp
+++ unittests/clangd/QualityTests.cpp
@@ -185,6 +185,13 @@
   Relevance = {};
   Relevance.merge(CodeCompletionResult(&findDecl(AST, "S::S"), 42));
   EXPECT_EQ(Relevance.Scope, SymbolRelevanceSignals::GlobalScope);
+
+  Relevance = {};
+  EXPECT_FALSE(Relevance.InBaseClass);
+  auto BaseMember = CodeCompletionResult(&findAnyDecl(AST, "y"), 42);
+  BaseMember.InBaseClass = true;
+  Relevance.merge(BaseMember);
+  EXPECT_TRUE(Relevance.InBaseClass);
 }
 
 // Do the signals move the scores in the direction we expect?
@@ -276,6 +283,10 @@
   EXPECT_LT(Instance.evaluate(), Default.evaluate());
   Instance.IsInstanceMember = true;
   EXPECT_EQ(Instance.evaluate(), Default.evaluate());
+
+  SymbolRelevanceSignals InBaseClass;
+  InBaseClass.InBaseClass = true;
+  EXPECT_LT(InBaseClass.evaluate(), Default.evaluate());
 }
 
 TEST(QualityTests, ScopeProximity) {
Index: clangd/Quality.h
===
--- clangd/Quality.h
+++ clangd/Quality.h
@@ -87,6 +87,7 @@
   bool Forbidden = false; // Unavailable (e.g const) or inaccessible (private).
   /// Whether fixits needs to be applied for that completion or not.
   bool NeedsFixIts = false;
+  bool InBaseClass = false; // A member from base class of the accessed class.
 
   URIDistance *FileProximityMatch = nullptr;
   /// These are used to calculate proximity between the index symbol and the
Index: clangd/Quality.cpp
===
--- clangd/Quality.cpp
+++ clangd/Quality.cpp
@@ -299,6 +299,7 @@
   : 0.6;
 SemaFileProximityScore = std::max(DeclProximity, SemaFileProximityScore);
 IsInstanceMember |= isInstanceMember(SemaCCResult.Declaration);
+InBaseClass |= SemaCCResult.InBaseClass;
   }
 
   // Declarations are scoped, others (like macros) are assumed global.
@@ -375,6 +376,9 @@
 Score *= 0.5;
   }
 
+  if (InBaseClass)
+Score *= 0.7;
+
   // Penalize for FixIts.
   if (NeedsFixIts)
 Score *= 0.5;


Index: unittests/clangd/QualityTests.cpp
===
--- unittests/clangd/QualityTests.cpp
+++ unittests/clangd/QualityTests.cpp
@@ -185,6 +185,13 @@
   Relevance = {};
   Relevance.merge(CodeCompletionResult(&findDecl(AST, "S::S"), 42));
   EXPECT_EQ(Relevance.Scope, SymbolRelevanceSignals::GlobalScope);
+
+  Relevance = {};
+  EXPECT_FALSE(Relevance.InBaseClass);
+  auto BaseMember = CodeCompletionResult(&findAnyDecl(AST, "y"), 42);
+  BaseMember.InBaseClass = true;
+  Relevance.merge(BaseMember);
+  EXPECT_TRUE(Relevance.InBaseClass);
 }
 
 // Do the signals move the scores in the direction we expect?
@@ -276,6 +283,10 @@
   EXPECT_LT(Instance.evaluate(), Default.evaluate());
   Instance.IsInstanceMember = true;
   EXPECT_EQ(Instance.evaluate(), Default.evaluate());
+
+  SymbolRelevanceSignals InBaseClass;
+  InBaseClass.InBaseClass = true;
+  EXPECT_LT(InBaseClass.evaluate(), Default.evaluate());
 }
 
 TEST(QualityTests, ScopeProximity) {
Index: clangd/Quality.h
===
--- clangd/Quality.h
+++ clangd/Quality.h
@@ -87,6 +87,7 @@
   bool Forbidden = false; // Unavailable (e.g const) or inaccessible (private).
   /// Whether fixits needs to be applied for that completion or not.
   bool NeedsFixIts = false;
+  bool InBaseClass = false; // A member from base class of the accessed class.
 
   URIDistance *FileProximityMatch = nullptr;
   /// These are used to calculate proximity between the index symbol and the
Index: clangd/Quality.cpp
===
--- clangd/Quality.cpp
+++ clangd/Quality.cpp
@@ -299,6 +299,7 @@
   : 0.6;
 SemaFileProximityScore = std::max(DeclProximity, SemaFileProximityScore);
 IsInstanceMember |= isInstanceMember(SemaCCResult.Declaration);
+InBaseClass |= SemaCCResult.InBaseClass;
   }
 
   // Declarations are scoped, others (like macros) are assumed global.
@@ -375,6 +376,9 @@
 Score *= 0.5;
   }
 
+  if (InBaseClass)
+Score *= 0.7;
+
   // Penalize for FixIts.
   if (NeedsFixIts)
 Score *= 0.5;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53571: [clangd] Don't show base class versions of members as completions.

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 170863.
sammccall added a comment.

Remove misleading change to test.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53571

Files:
  clangd/ClangdLSPServer.cpp
  clangd/ClangdLSPServer.h
  clangd/CodeComplete.cpp
  clangd/GlobalCompilationDatabase.cpp
  clangd/GlobalCompilationDatabase.h
  unittests/clangd/CodeCompleteTests.cpp

Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -381,10 +381,13 @@
   void test() { Bar().^ }
   )cpp");
   EXPECT_THAT(Results.Completions,
-  HasSubsequence(AllOf(Qualifier(""), Named("bar")),
- AllOf(Qualifier("Foo::"), Named("foo";
+  Contains(AllOf(Qualifier(""), Named("bar";
+  // Hidden members are not shown.
   EXPECT_THAT(Results.Completions,
-  Not(Contains(AllOf(Qualifier(""), Named("foo"); // private
+  Not(Contains(AllOf(Qualifier("Foo::"), Named("foo");
+  // Private members are not shown.
+  EXPECT_THAT(Results.Completions,
+  Not(Contains(AllOf(Qualifier(""), Named("foo");
 }
 
 TEST(CompletionTest, InjectedTypename) {
Index: clangd/GlobalCompilationDatabase.h
===
--- clangd/GlobalCompilationDatabase.h
+++ clangd/GlobalCompilationDatabase.h
@@ -86,33 +86,6 @@
   llvm::Optional CompileCommandsDir;
 };
 
-/// A wrapper around GlobalCompilationDatabase that caches the compile commands.
-/// Note that only results of getCompileCommand are cached.
-class CachingCompilationDb : public GlobalCompilationDatabase {
-public:
-  explicit CachingCompilationDb(const GlobalCompilationDatabase &InnerCDB);
-
-  /// Gets compile command for \p File from cache or CDB if it's not in the
-  /// cache.
-  llvm::Optional
-  getCompileCommand(PathRef File) const override;
-
-  /// Forwards to the inner CDB. Results of this function are not cached.
-  tooling::CompileCommand getFallbackCommand(PathRef File) const override;
-
-  /// Removes an entry for \p File if it's present in the cache.
-  void invalidate(PathRef File);
-
-  /// Removes all cached compile commands.
-  void clear();
-
-private:
-  const GlobalCompilationDatabase &InnerCDB;
-  mutable std::mutex Mut;
-  mutable llvm::StringMap>
-  Cached; /* GUARDED_BY(Mut) */
-};
-
 /// Gets compile args from an in-memory mapping based on a filepath. Typically
 /// used by clients who provide the compile commands themselves.
 class InMemoryCompilationDb : public GlobalCompilationDatabase {
Index: clangd/GlobalCompilationDatabase.cpp
===
--- clangd/GlobalCompilationDatabase.cpp
+++ clangd/GlobalCompilationDatabase.cpp
@@ -116,38 +116,6 @@
   return nullptr;
 }
 
-CachingCompilationDb::CachingCompilationDb(
-const GlobalCompilationDatabase &InnerCDB)
-: InnerCDB(InnerCDB) {}
-
-Optional
-CachingCompilationDb::getCompileCommand(PathRef File) const {
-  std::unique_lock Lock(Mut);
-  auto It = Cached.find(File);
-  if (It != Cached.end())
-return It->second;
-
-  Lock.unlock();
-  Optional Command = InnerCDB.getCompileCommand(File);
-  Lock.lock();
-  return Cached.try_emplace(File, std::move(Command)).first->getValue();
-}
-
-tooling::CompileCommand
-CachingCompilationDb::getFallbackCommand(PathRef File) const {
-  return InnerCDB.getFallbackCommand(File);
-}
-
-void CachingCompilationDb::invalidate(PathRef File) {
-  std::unique_lock Lock(Mut);
-  Cached.erase(File);
-}
-
-void CachingCompilationDb::clear() {
-  std::unique_lock Lock(Mut);
-  Cached.clear();
-}
-
 Optional
 InMemoryCompilationDb::getCompileCommand(PathRef File) const {
   std::lock_guard Lock(Mutex);
Index: clangd/CodeComplete.cpp
===
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -728,9 +728,9 @@
 // Retain the results we might want.
 for (unsigned I = 0; I < NumResults; ++I) {
   auto &Result = InResults[I];
-  // Drop hidden items which cannot be found by lookup after completion.
-  // Exception: some items can be named by using a qualifier.
-  if (Result.Hidden && (!Result.Qualifier || Result.QualifierIsInformative))
+  // Class members that are shadowed by subclasses are usually noise.
+  if (Result.Hidden && Result.Declaration &&
+  Result.Declaration->isCXXClassMember())
 continue;
   if (!Opts.IncludeIneligibleResults &&
   (Result.Availability == CXAvailability_NotAvailable ||
Index: clangd/ClangdLSPServer.h
===
--- clangd/ClangdLSPServer.h
+++ clangd/ClangdLSPServer.h
@@ -135,21 +135,17 @@
 
 /// Returns a CDB that should be used to get compile commands for the
 /// current instance of ClangdLSPSer

[PATCH] D53638: [clangd] Downrank members from base class

[Sam McCall via Phabricator via cfe-commits]

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



Comment at: clangd/Quality.cpp:380
+  if (InBaseClass)
+Score *= 0.7;
+

This seems like a pretty light penalty to me, I'd consider 0.5...


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53638



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

[PATCH] D53586: Implement Function Multiversioning for Non-ELF Systems.

[Erich Keane via Phabricator via cfe-commits]

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

In https://reviews.llvm.org/D53586#1273546, @rnk wrote:

> Seems reasonable. Should the resolver still be called `?foo@.resolver`, or 
> should it get a new name, since it is quite functionally different now?


I'm not attached to the name.  I didn't have a good alternative (I'd thought of 
'dispatcher' at one point, but figured that was because my mind was attached to 
cpu-dispatch).  Implementation wise, '.resolver' is slightly easier, but not 
enough to motivate the discussion.

If you like '.dispatcher' instead of '.resolver', I can do that, otherwise a 
brainstorming session would be appreciated!




Comment at: lib/CodeGen/CodeGenFunction.cpp:2391-2393
+  llvm::SmallVector Args;
+  llvm::for_each(Resolver->args(),
+ [&](llvm::Argument &Arg) { Args.push_back(&Arg); });

rnk wrote:
> Surely this would be simpler as a range for or `Args{Resolver->arg_begin() 
> ...};`
Surely... Unfortunately the types don't match.  Resolver Args are an array of 
Arguments (so the iterator is a Arg*).

HOWEVER, "Args" here needs to be an array ref to Value*, so the iterator would 
have to be a Value** (not a Arg*/Value*).  The point of this small vector is to 
create an array of _pointers_ from the array of Arguments.



Comment at: lib/CodeGen/CodeGenFunction.cpp:2395
+
+  llvm::CallInst *Result = Builder.CreateCall(FuncToReturn, Args);
+

rnk wrote:
> This approach is... not going to work in the general case. Consider, for 
> example, varargs. Then consider 32-bit x86 inalloca, which is quite 
> widespread. Any non-trivially copyable object is not going to be passable 
> through such a thunk. You might consider looking at 
> CodeGenFunction::EmitMustTailThunk instead.
Oh dear... 
I'm unfamiliar with EmitMustTailThunk, is it self explanatory?  Any chance you 
can expound?  Should I call/use that function, or copy out of it?



Comment at: lib/CodeGen/CodeGenFunction.cpp:2432
 llvm::BasicBlock *RetBlock = createBasicBlock("resolver_return", Resolver);
 llvm::IRBuilder<> RetBuilder(RetBlock);
+  CreateMultiVersionResolverReturn(Resolver, RetBuilder, RO.Function,

rnk wrote:
> You can make this a CGBuilderTy. Just pass it a type cache.
I'd tried that a while and could never find a type cache to use. I'd not 
realized until just now (searching more for usages) that CodeGenFunction is one!



Comment at: test/CodeGenCXX/attr-target-mv-overloads.cpp:74
+// WINDOWS: define dso_local i32 @"?foo_overload@@YAHH@Z.resolver"(i32) comdat
+// WINDOWS: call i32 @"?foo_overload@@YAHH@Z.arch_sandybridge"
+// WINDOWS: call i32 @"?foo_overload@@YAHH@Z.arch_ivybridge"

rnk wrote:
> Does dumpbin demangle these names correctly? Just curious.
I don't know, I'd suspect not.  However, demangler.com (not sure what it uses?) 
seems to demangle them ignoring everything after the '.'.


Repository:
  rC Clang

https://reviews.llvm.org/D53586



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

[PATCH] D53399: [clangd] Ensure that we reply to each call exactly once. NFC (I think!)

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 170866.
sammccall added a comment.

Clarify Span::Args lifetime.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53399

Files:
  clangd/ClangdLSPServer.cpp
  clangd/Trace.h

Index: clangd/Trace.h
===
--- clangd/Trace.h
+++ clangd/Trace.h
@@ -87,6 +87,7 @@
 
   /// Mutable metadata, if this span is interested.
   /// Prefer to use SPAN_ATTACH rather than accessing this directly.
+  /// The lifetime of Args is the whole event, even if the Span dies.
   llvm::json::Object *const Args;
 
 private:
Index: clangd/ClangdLSPServer.cpp
===
--- clangd/ClangdLSPServer.cpp
+++ clangd/ClangdLSPServer.cpp
@@ -105,16 +105,21 @@
   }
 
   bool onCall(StringRef Method, json::Value Params, json::Value ID) override {
+// Calls can be canceled by the client. Add cancellation context.
+WithContext WithCancel(cancelableRequestContext(ID));
+trace::Span Tracer(Method);
+SPAN_ATTACH(Tracer, "Params", Params);
+ReplyOnce Reply(ID, Method, &Server, Tracer.Args);
 log("<-- {0}({1})", Method, ID);
 if (!Server.Server && Method != "initialize") {
   elog("Call {0} before initialization.", Method);
-  Server.reply(ID, make_error("server not initialized",
-ErrorCode::ServerNotInitialized));
+  Reply(make_error("server not initialized",
+ ErrorCode::ServerNotInitialized));
 } else if (auto Handler = Calls.lookup(Method))
-  Handler(std::move(Params), std::move(ID));
+  Handler(std::move(Params), std::move(Reply));
 else
-  Server.reply(ID, make_error("method not found",
-ErrorCode::MethodNotFound));
+  Reply(
+  make_error("method not found", ErrorCode::MethodNotFound));
 return true;
   }
 
@@ -128,36 +133,19 @@
   }
 
   // Bind an LSP method name to a call.
-  template 
+  template 
   void bind(const char *Method,
-void (ClangdLSPServer::*Handler)(const Param &, Callback)) {
+void (ClangdLSPServer::*Handler)(const Param &, Callback)) {
 Calls[Method] = [Method, Handler, this](json::Value RawParams,
-json::Value ID) {
+ReplyOnce Reply) {
   Param P;
-  if (!fromJSON(RawParams, P)) {
+  if (fromJSON(RawParams, P)) {
+(Server.*Handler)(P, std::move(Reply));
+  } else {
 elog("Failed to decode {0} request.", Method);
-Server.reply(ID, make_error("failed to decode request",
-  ErrorCode::InvalidRequest));
-return;
+Reply(make_error("failed to decode request",
+   ErrorCode::InvalidRequest));
   }
-  trace::Span Tracer(Method);
-  SPAN_ATTACH(Tracer, "Params", RawParams);
-  auto *Trace = Tracer.Args; // We attach reply from another thread.
-  // Calls can be canceled by the client. Add cancellation context.
-  WithContext WithCancel(cancelableRequestContext(ID));
-  // FIXME: this function should assert it's called exactly once.
-  (Server.*Handler)(P, [this, ID, Trace](Expected Result) {
-if (Result) {
-  if (Trace)
-(*Trace)["Reply"] = *Result;
-  Server.reply(ID, json::Value(std::move(*Result)));
-} else {
-  auto Err = Result.takeError();
-  if (Trace)
-(*Trace)["Error"] = to_string(Err);
-  Server.reply(ID, std::move(Err));
-}
-  });
 };
   }
 
@@ -178,8 +166,65 @@
   }
 
 private:
+  // Function object to reply to an LSP call.
+  // Each instance must be called exactly once, otherwise:
+  //  - the bug is logged, and (in debug mode) an assert will fire
+  //  - if there was no reply, an error reply is sent
+  //  - if there were multiple replies, only the first is sent
+  class ReplyOnce {
+std::atomic Replied = {false};
+json::Value ID;
+std::string Method;
+ClangdLSPServer *Server; // Null when moved-from.
+json::Object *TraceArgs;
+
+  public:
+ReplyOnce(const json::Value &ID, StringRef Method, ClangdLSPServer *Server,
+  json::Object *TraceArgs)
+: ID(ID), Method(Method), Server(Server), TraceArgs(TraceArgs) {
+  assert(Server);
+}
+ReplyOnce(ReplyOnce &&Other)
+: Replied(Other.Replied.load()), ID(std::move(Other.ID)),
+  Method(std::move(Other.Method)), Server(Other.Server),
+  TraceArgs(Other.TraceArgs) {
+  Other.Server = nullptr;
+}
+ReplyOnce& operator=(ReplyOnce&&) = delete;
+ReplyOnce(const ReplyOnce &) = delete;
+ReplyOnce& operator=(const ReplyOnce&) = delete;
+
+~ReplyOnce() {
+  if (Server && !Replied) {
+elog("No reply to message {0}({1})", Method, ID);
+ 

[PATCH] D53399: [clangd] Ensure that we reply to each call exactly once. NFC (I think!)

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clangd/ClangdLSPServer.cpp:112
+SPAN_ATTACH(Tracer, "Params", Params);
+ReplyOnce Reply(ID, Method, &Server, Tracer.Args);
 log("<-- {0}({1})", Method, ID);

ioeric wrote:
> Do we have guarantee that `Tracer.Args` outlives `Reply`?
Yes, assuming the Handler propagates contexts properly.
The args are owned by the context created by the `trace::Span`.
Added a comment to `Span` to guarantee this.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53399



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

[PATCH] D53141: [OpenMP][libomptarget] Add runtime function for pushing coalesced global records

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: libomptarget/deviceRTLs/nvptx/src/data_sharing.cu:389
   unsigned WID = getWarpId();
+  // void * volatile FramePointer = 0;
   void *&FrameP = DataSharingState.FramePtr[WID];

This must be removed



Comment at: libomptarget/deviceRTLs/nvptx/src/data_sharing.cu:438
+// point to the start of the new frame held in StackP.
+//atomicExch((unsigned long long *)&FrameP, (unsigned long 
long)StackP);
+FrameP = StackP;

Also, must be removed



Comment at: libomptarget/deviceRTLs/nvptx/src/data_sharing.cu:444
+}
+  } while (!FrameP);
 

It is a very bad idea to have something like this without atomic instructions.
Also, for writing, you need to use atomic instructions (+, possibly, `volatile` 
data type). Otherwise, it leads to undefined behavior and problems during 
optimization.


Repository:
  rOMP OpenMP

https://reviews.llvm.org/D53141



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

[PATCH] D53571: [clangd] Don't show base class versions of members as completions.

[Eric Liu via Phabricator via cfe-commits]

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

The diff seems to be wrong. Please rebase.




Comment at: clangd/CodeComplete.cpp:732
+  // Class members that are shadowed by subclasses are usually noise.
+  if (Result.Hidden && Result.Declaration &&
+  Result.Declaration->isCXXClassMember())

sammccall wrote:
> ioeric wrote:
> > Are there hidden results that are neither declarations nor class members 
> > and do not require qualifier? 
> I don't know. I thought a namespace-scope thing shadowed by a member was such 
> a case, but it doesn't seem to generate results at all.
> 
> But that (hypothetical) example illustrates a point: I do think we would want 
> to show ns::foo() if it was shadowed by MyClass()::foo(). The difference is 
> that they're probably fundamentally different things that share a name, while 
> MySubClass::foo() is usually just a strictly better version of 
> MyBaseClass::foo() as far as a caller is concerned.
Fair enough.  Thanks!


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53571



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

[PATCH] D51429: [AArch64] Return Address Signing B Key Support

[Luke Cheeseman via Phabricator via cfe-commits]

LukeCheeseman updated this revision to Diff 170869.

https://reviews.llvm.org/D51429

Files:
  include/clang/Basic/DiagnosticDriverKinds.td
  include/clang/Driver/CC1Options.td
  include/clang/Driver/Options.td
  include/clang/Frontend/CodeGenOptions.def
  include/clang/Frontend/CodeGenOptions.h
  lib/CodeGen/CGDeclCXX.cpp
  lib/CodeGen/TargetInfo.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/aarch64-sign-return-address.c
  test/CodeGenCXX/aarch64-sign-return-address-static-ctor.cpp
  test/Driver/aarch64-security-options.c

Index: test/Driver/aarch64-security-options.c
===
--- /dev/null
+++ test/Driver/aarch64-security-options.c
@@ -0,0 +1,54 @@
+// Check the -msign-return-address= option, which has a required argument to
+// select scope.
+// RUN: %clang -target aarch64--none-eabi -c %s -### -msign-return-address=none 2>&1 | \
+// RUN: FileCheck %s --check-prefix=RA-OFF  --check-prefix=KEY-A --check-prefix=BTE-OFF
+
+// RUN: %clang -target aarch64--none-eabi -c %s -### -msign-return-address=non-leaf 2>&1 | \
+// RUN: FileCheck %s --check-prefix=RA-NON-LEAF --check-prefix=KEY-A --check-prefix=BTE-OFF
+
+// RUN: %clang -target aarch64--none-eabi -c %s -### -msign-return-address=all  2>&1 | \
+// RUN: FileCheck %s --check-prefix=RA-ALL  --check-prefix=KEY-A --check-prefix=BTE-OFF
+
+// Check that the -msign-return-address= option can also accept the signing key
+// to use.
+
+// RUN: %clang -target aarch64--none-eabi -c %s -### -msign-return-address=non-leaf   2>&1 | \
+// RUN: FileCheck %s --check-prefix=RA-NON-LEAF --check-prefix=KEY-B --check-prefix=BTE-OFF
+
+// RUN: %clang -target aarch64--none-eabi -c %s -### -msign-return-address=all2>&1 | \
+// RUN: FileCheck %s --check-prefix=RA-ALL  --check-prefix=KEY-B --check-prefix=BTE-OFF
+
+// -mbranch-protection with standard
+// RUN: %clang -target aarch64--none-eabi -c %s -### -mbranch-protection=standard2>&1 | \
+// RUN: FileCheck %s --check-prefix=RA-NON-LEAF --check-prefix=KEY-A --check-prefix=BTE-ON
+
+// If the -msign-return-address and -mbranch-protection are both used, the
+// right-most one controls return address signing.
+// RUN: %clang -target aarch64--none-eabi -c %s -### -msign-return-address=non-leaf -mbranch-protection=none 2>&1 | \
+// RUN: FileCheck %s --check-prefix=CONFLICT
+
+// RUN: %clang -target aarch64--none-eabi -c %s -### -mbranch-protection=pac-ret -msign-return-address=none 2>&1 | \
+// RUN: FileCheck %s --check-prefix=CONFLICT
+
+// RUN: %clang -target aarch64--none-eabi -c %s -### -msign-return-address=foo 2>&1 | \
+// RUN: FileCheck %s --check-prefix=BAD-RA-PROTECTION
+
+// RUN: %clang -target aarch64--none-eabi -c %s -### -mbranch-protection=bar 2>&1 | \
+// RUN: FileCheck %s --check-prefix=BAD-BP-PROTECTION
+
+// RA-OFF: "-msign-return-address=none"
+// RA-NON-LEAF: "-msign-return-address=non-leaf"
+// RA-ALL: "-msign-return-address=all"
+
+// KEY-A: "-msign-return-address-key=a_key"
+
+// BTE-OFF-NOT: "-mbranch-target-enforce"
+// BTE-ON: "-mbranch-target-enforce"
+
+// CONFLICT: "-msign-return-address=none"
+
+// BAD-RA-PROTECTION: invalid branch protection option 'foo' in '-msign-return-address={{.*}}'
+// BAD-BP-PROTECTION: invalid branch protection option 'bar' in '-mbranch-protection={{.*}}'
+
+// BAD-B-KEY-COMBINATION: invalid branch protection option 'b-key' in '-mbranch-protection={{.*}}'
+// BAD-LEAF-COMBINATION: invalid branch protection option 'leaf' in '-mbranch-protection={{.*}}'
Index: test/CodeGenCXX/aarch64-sign-return-address-static-ctor.cpp
===
--- test/CodeGenCXX/aarch64-sign-return-address-static-ctor.cpp
+++ test/CodeGenCXX/aarch64-sign-return-address-static-ctor.cpp
@@ -1,9 +1,26 @@
 // RUN: %clang -target aarch64-arm-none-eabi -S -emit-llvm -o - -msign-return-address=none  %s | \
 // RUN: FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-NONE
 // RUN: %clang -target aarch64-arm-none-eabi -S -emit-llvm -o - -msign-return-address=non-leaf %s | \
-// RUN: FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-PARTIAL
+// RUN: FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-PARTIAL  --check-prefix=CHECK-A-KEY
 // RUN: %clang -target aarch64-arm-none-eabi -S -emit-llvm -o - -msign-return-address=all %s | \
-// RUN: FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-ALL
+// RUN: FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-ALL  --check-prefix=CHECK-A-KEY
+
+// RUN: %clang -target aarch64-arm-none-eabi -S -emit-llvm -o - -mbranch-protection=none %s | \
+// RUN: FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-NONE
+// RUN: %clang -target aarch64-arm-none-eabi -S -emit-llvm -o - -mbranch-protection=standard %s | \
+// RUN: FileChec

[PATCH] D53443: [OpenMP][NVPTX] Enable default scheduling for parallel for in non-SPMD cases.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

What about tests?


Repository:
  rC Clang

https://reviews.llvm.org/D53443



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

[PATCH] D53638: [clangd] Downrank members from base class

[Eric Liu via Phabricator via cfe-commits]

ioeric updated this revision to Diff 170870.
ioeric marked an inline comment as done.
ioeric added a comment.

- adjust parameter


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53638

Files:
  clangd/Quality.cpp
  clangd/Quality.h
  unittests/clangd/QualityTests.cpp


Index: unittests/clangd/QualityTests.cpp
===
--- unittests/clangd/QualityTests.cpp
+++ unittests/clangd/QualityTests.cpp
@@ -185,6 +185,13 @@
   Relevance = {};
   Relevance.merge(CodeCompletionResult(&findDecl(AST, "S::S"), 42));
   EXPECT_EQ(Relevance.Scope, SymbolRelevanceSignals::GlobalScope);
+
+  Relevance = {};
+  EXPECT_FALSE(Relevance.InBaseClass);
+  auto BaseMember = CodeCompletionResult(&findAnyDecl(AST, "y"), 42);
+  BaseMember.InBaseClass = true;
+  Relevance.merge(BaseMember);
+  EXPECT_TRUE(Relevance.InBaseClass);
 }
 
 // Do the signals move the scores in the direction we expect?
@@ -276,6 +283,10 @@
   EXPECT_LT(Instance.evaluate(), Default.evaluate());
   Instance.IsInstanceMember = true;
   EXPECT_EQ(Instance.evaluate(), Default.evaluate());
+
+  SymbolRelevanceSignals InBaseClass;
+  InBaseClass.InBaseClass = true;
+  EXPECT_LT(InBaseClass.evaluate(), Default.evaluate());
 }
 
 TEST(QualityTests, ScopeProximity) {
Index: clangd/Quality.h
===
--- clangd/Quality.h
+++ clangd/Quality.h
@@ -87,6 +87,7 @@
   bool Forbidden = false; // Unavailable (e.g const) or inaccessible (private).
   /// Whether fixits needs to be applied for that completion or not.
   bool NeedsFixIts = false;
+  bool InBaseClass = false; // A member from base class of the accessed class.
 
   URIDistance *FileProximityMatch = nullptr;
   /// These are used to calculate proximity between the index symbol and the
Index: clangd/Quality.cpp
===
--- clangd/Quality.cpp
+++ clangd/Quality.cpp
@@ -299,6 +299,7 @@
   : 0.6;
 SemaFileProximityScore = std::max(DeclProximity, SemaFileProximityScore);
 IsInstanceMember |= isInstanceMember(SemaCCResult.Declaration);
+InBaseClass |= SemaCCResult.InBaseClass;
   }
 
   // Declarations are scoped, others (like macros) are assumed global.
@@ -372,9 +373,12 @@
   if (!IsInstanceMember &&
   (Context == CodeCompletionContext::CCC_DotMemberAccess ||
Context == CodeCompletionContext::CCC_ArrowMemberAccess)) {
-Score *= 0.5;
+Score *= 0.2;
   }
 
+  if (InBaseClass)
+Score *= 0.5;
+
   // Penalize for FixIts.
   if (NeedsFixIts)
 Score *= 0.5;


Index: unittests/clangd/QualityTests.cpp
===
--- unittests/clangd/QualityTests.cpp
+++ unittests/clangd/QualityTests.cpp
@@ -185,6 +185,13 @@
   Relevance = {};
   Relevance.merge(CodeCompletionResult(&findDecl(AST, "S::S"), 42));
   EXPECT_EQ(Relevance.Scope, SymbolRelevanceSignals::GlobalScope);
+
+  Relevance = {};
+  EXPECT_FALSE(Relevance.InBaseClass);
+  auto BaseMember = CodeCompletionResult(&findAnyDecl(AST, "y"), 42);
+  BaseMember.InBaseClass = true;
+  Relevance.merge(BaseMember);
+  EXPECT_TRUE(Relevance.InBaseClass);
 }
 
 // Do the signals move the scores in the direction we expect?
@@ -276,6 +283,10 @@
   EXPECT_LT(Instance.evaluate(), Default.evaluate());
   Instance.IsInstanceMember = true;
   EXPECT_EQ(Instance.evaluate(), Default.evaluate());
+
+  SymbolRelevanceSignals InBaseClass;
+  InBaseClass.InBaseClass = true;
+  EXPECT_LT(InBaseClass.evaluate(), Default.evaluate());
 }
 
 TEST(QualityTests, ScopeProximity) {
Index: clangd/Quality.h
===
--- clangd/Quality.h
+++ clangd/Quality.h
@@ -87,6 +87,7 @@
   bool Forbidden = false; // Unavailable (e.g const) or inaccessible (private).
   /// Whether fixits needs to be applied for that completion or not.
   bool NeedsFixIts = false;
+  bool InBaseClass = false; // A member from base class of the accessed class.
 
   URIDistance *FileProximityMatch = nullptr;
   /// These are used to calculate proximity between the index symbol and the
Index: clangd/Quality.cpp
===
--- clangd/Quality.cpp
+++ clangd/Quality.cpp
@@ -299,6 +299,7 @@
   : 0.6;
 SemaFileProximityScore = std::max(DeclProximity, SemaFileProximityScore);
 IsInstanceMember |= isInstanceMember(SemaCCResult.Declaration);
+InBaseClass |= SemaCCResult.InBaseClass;
   }
 
   // Declarations are scoped, others (like macros) are assumed global.
@@ -372,9 +373,12 @@
   if (!IsInstanceMember &&
   (Context == CodeCompletionContext::CCC_DotMemberAccess ||
Context == CodeCompletionContext::CCC_ArrowMemberAccess)) {
-Score *= 0.5;
+Score *= 0.2;
   }
 
+  if (InBaseClass)
+Score *= 0.5;
+
   // Penalize for FixIts.
   if (NeedsFixIts)
 Score *= 0.

[PATCH] D53638: [clangd] Downrank members from base class

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.



Comment at: clangd/Quality.cpp:380
+  if (InBaseClass)
+Score *= 0.7;
+

sammccall wrote:
> This seems like a pretty light penalty to me, I'd consider 0.5...
0.5 sounds reasonable. I think we should penalize the non-instance member case 
more if this is 0.5 though. Made that 0.2. WDYT?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53638



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

[PATCH] D53638: [clangd] Downrank members from base class

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clangd/Quality.cpp:380
+  if (InBaseClass)
+Score *= 0.7;
+

ioeric wrote:
> sammccall wrote:
> > This seems like a pretty light penalty to me, I'd consider 0.5...
> 0.5 sounds reasonable. I think we should penalize the non-instance member 
> case more if this is 0.5 though. Made that 0.2. WDYT?
Agree, LG!


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53638



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

[PATCH] D53417: [Clang][PowerPC] Choose a better candidate in overload function call if there is a compatible vector conversion instead of ambiguous call error

[Zixuan Wu via Phabricator via cfe-commits]

wuzish marked 8 inline comments as done.
wuzish added inline comments.



Comment at: clang/lib/Sema/SemaOverload.cpp:3941
+  return ImplicitConversionSequence::Better;
+  }
+

hubert.reinterpretcast wrote:
> This seems to duplicate the bug described here in 
> https://bugs.llvm.org/show_bug.cgi?id=39361.
> ```
> typedef unsigned int GccType __attribute__((__vector_size__(16)));
> typedef __vector unsigned int AltiVecType;
> 
> typedef float GccOtherType __attribute__((__vector_size__(16)));
> 
> char *f(GccOtherType, int);
> template  int f(AltiVecType, T);
> template  int g(AltiVecType, T);
> char *g(GccOtherType, int);
> 
> bool zip(GccType v) { return f(v, 0) == g(v, 0); }
> ```
Thank you for the good case. I will fix it.

But the https://bugs.llvm.org/show_bug.cgi?id=39361 is not the same reason but 
similar. We can propose another patch to do this. I think the essential reason 
is that code as blow has not considered `ImplicitConversionSequence::Worse` so 
that outside caller goes into path to choose a non-template candidate first.


```
if (S.getLangOpts().MSVCCompat && SCS1.Second == ICK_Integral_Conversion &&
  SCS2.Second == ICK_Floating_Integral &&
  S.Context.getTypeSize(SCS1.getFromType()) ==
  S.Context.getTypeSize(SCS1.getToType(2)))
return ImplicitConversionSequence::Better;
```



Comment at: clang/test/Sema/altivec-generic-overload.c:3
+
+typedef signed char __v4sc __attribute__((__vector_size__(16)));
+typedef unsigned char __v4uc __attribute__((__vector_size__(16)));

hubert.reinterpretcast wrote:
> wuzish wrote:
> > hubert.reinterpretcast wrote:
> > > `__v4sc` is suspicious. The most consistent naming I have seen is 
> > > `v16i8`, `v16u8`, `v8i16`, ...
> > > Do we need the double underscores?
> > > 
> > Because it's generic gcc vector type instead of altivec type so I choose 
> > the naming style.
> The naming style from `clang/lib/Headers/avxintrin.h` would still say that 
> the `__v4sc` here should be `__v16qi`.
On, you mean the num. Yes, that should not be '4'. 


Repository:
  rC Clang

https://reviews.llvm.org/D53417



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

[clang-tools-extra] r345140 - [clangd] Downrank members from base class

[Eric Liu via cfe-commits]

Author: ioeric
Date: Wed Oct 24 06:45:17 2018
New Revision: 345140

URL: http://llvm.org/viewvc/llvm-project?rev=345140&view=rev
Log:
[clangd] Downrank members from base class

Reviewers: sammccall, ilya-biryukov

Reviewed By: sammccall

Subscribers: MaskRay, jkorous, arphaman, kadircet, cfe-commits

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

Modified:
clang-tools-extra/trunk/clangd/Quality.cpp
clang-tools-extra/trunk/clangd/Quality.h
clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp

Modified: clang-tools-extra/trunk/clangd/Quality.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Quality.cpp?rev=345140&r1=345139&r2=345140&view=diff
==
--- clang-tools-extra/trunk/clangd/Quality.cpp (original)
+++ clang-tools-extra/trunk/clangd/Quality.cpp Wed Oct 24 06:45:17 2018
@@ -299,6 +299,7 @@ void SymbolRelevanceSignals::merge(const
   : 0.6;
 SemaFileProximityScore = std::max(DeclProximity, SemaFileProximityScore);
 IsInstanceMember |= isInstanceMember(SemaCCResult.Declaration);
+InBaseClass |= SemaCCResult.InBaseClass;
   }
 
   // Declarations are scoped, others (like macros) are assumed global.
@@ -372,9 +373,12 @@ float SymbolRelevanceSignals::evaluate()
   if (!IsInstanceMember &&
   (Context == CodeCompletionContext::CCC_DotMemberAccess ||
Context == CodeCompletionContext::CCC_ArrowMemberAccess)) {
-Score *= 0.5;
+Score *= 0.2;
   }
 
+  if (InBaseClass)
+Score *= 0.5;
+
   // Penalize for FixIts.
   if (NeedsFixIts)
 Score *= 0.5;

Modified: clang-tools-extra/trunk/clangd/Quality.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Quality.h?rev=345140&r1=345139&r2=345140&view=diff
==
--- clang-tools-extra/trunk/clangd/Quality.h (original)
+++ clang-tools-extra/trunk/clangd/Quality.h Wed Oct 24 06:45:17 2018
@@ -87,6 +87,7 @@ struct SymbolRelevanceSignals {
   bool Forbidden = false; // Unavailable (e.g const) or inaccessible (private).
   /// Whether fixits needs to be applied for that completion or not.
   bool NeedsFixIts = false;
+  bool InBaseClass = false; // A member from base class of the accessed class.
 
   URIDistance *FileProximityMatch = nullptr;
   /// These are used to calculate proximity between the index symbol and the

Modified: clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp?rev=345140&r1=345139&r2=345140&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp Wed Oct 24 
06:45:17 2018
@@ -185,6 +185,13 @@ TEST(QualityTests, SymbolRelevanceSignal
   Relevance = {};
   Relevance.merge(CodeCompletionResult(&findDecl(AST, "S::S"), 42));
   EXPECT_EQ(Relevance.Scope, SymbolRelevanceSignals::GlobalScope);
+
+  Relevance = {};
+  EXPECT_FALSE(Relevance.InBaseClass);
+  auto BaseMember = CodeCompletionResult(&findAnyDecl(AST, "y"), 42);
+  BaseMember.InBaseClass = true;
+  Relevance.merge(BaseMember);
+  EXPECT_TRUE(Relevance.InBaseClass);
 }
 
 // Do the signals move the scores in the direction we expect?
@@ -276,6 +283,10 @@ TEST(QualityTests, SymbolRelevanceSignal
   EXPECT_LT(Instance.evaluate(), Default.evaluate());
   Instance.IsInstanceMember = true;
   EXPECT_EQ(Instance.evaluate(), Default.evaluate());
+
+  SymbolRelevanceSignals InBaseClass;
+  InBaseClass.InBaseClass = true;
+  EXPECT_LT(InBaseClass.evaluate(), Default.evaluate());
 }
 
 TEST(QualityTests, ScopeProximity) {


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

[PATCH] D53638: [clangd] Downrank members from base class

[Eric Liu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL345140: [clangd] Downrank members from base class (authored 
by ioeric, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D53638

Files:
  clang-tools-extra/trunk/clangd/Quality.cpp
  clang-tools-extra/trunk/clangd/Quality.h
  clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp


Index: clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp
@@ -185,6 +185,13 @@
   Relevance = {};
   Relevance.merge(CodeCompletionResult(&findDecl(AST, "S::S"), 42));
   EXPECT_EQ(Relevance.Scope, SymbolRelevanceSignals::GlobalScope);
+
+  Relevance = {};
+  EXPECT_FALSE(Relevance.InBaseClass);
+  auto BaseMember = CodeCompletionResult(&findAnyDecl(AST, "y"), 42);
+  BaseMember.InBaseClass = true;
+  Relevance.merge(BaseMember);
+  EXPECT_TRUE(Relevance.InBaseClass);
 }
 
 // Do the signals move the scores in the direction we expect?
@@ -276,6 +283,10 @@
   EXPECT_LT(Instance.evaluate(), Default.evaluate());
   Instance.IsInstanceMember = true;
   EXPECT_EQ(Instance.evaluate(), Default.evaluate());
+
+  SymbolRelevanceSignals InBaseClass;
+  InBaseClass.InBaseClass = true;
+  EXPECT_LT(InBaseClass.evaluate(), Default.evaluate());
 }
 
 TEST(QualityTests, ScopeProximity) {
Index: clang-tools-extra/trunk/clangd/Quality.cpp
===
--- clang-tools-extra/trunk/clangd/Quality.cpp
+++ clang-tools-extra/trunk/clangd/Quality.cpp
@@ -299,6 +299,7 @@
   : 0.6;
 SemaFileProximityScore = std::max(DeclProximity, SemaFileProximityScore);
 IsInstanceMember |= isInstanceMember(SemaCCResult.Declaration);
+InBaseClass |= SemaCCResult.InBaseClass;
   }
 
   // Declarations are scoped, others (like macros) are assumed global.
@@ -372,9 +373,12 @@
   if (!IsInstanceMember &&
   (Context == CodeCompletionContext::CCC_DotMemberAccess ||
Context == CodeCompletionContext::CCC_ArrowMemberAccess)) {
-Score *= 0.5;
+Score *= 0.2;
   }
 
+  if (InBaseClass)
+Score *= 0.5;
+
   // Penalize for FixIts.
   if (NeedsFixIts)
 Score *= 0.5;
Index: clang-tools-extra/trunk/clangd/Quality.h
===
--- clang-tools-extra/trunk/clangd/Quality.h
+++ clang-tools-extra/trunk/clangd/Quality.h
@@ -87,6 +87,7 @@
   bool Forbidden = false; // Unavailable (e.g const) or inaccessible (private).
   /// Whether fixits needs to be applied for that completion or not.
   bool NeedsFixIts = false;
+  bool InBaseClass = false; // A member from base class of the accessed class.
 
   URIDistance *FileProximityMatch = nullptr;
   /// These are used to calculate proximity between the index symbol and the


Index: clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp
@@ -185,6 +185,13 @@
   Relevance = {};
   Relevance.merge(CodeCompletionResult(&findDecl(AST, "S::S"), 42));
   EXPECT_EQ(Relevance.Scope, SymbolRelevanceSignals::GlobalScope);
+
+  Relevance = {};
+  EXPECT_FALSE(Relevance.InBaseClass);
+  auto BaseMember = CodeCompletionResult(&findAnyDecl(AST, "y"), 42);
+  BaseMember.InBaseClass = true;
+  Relevance.merge(BaseMember);
+  EXPECT_TRUE(Relevance.InBaseClass);
 }
 
 // Do the signals move the scores in the direction we expect?
@@ -276,6 +283,10 @@
   EXPECT_LT(Instance.evaluate(), Default.evaluate());
   Instance.IsInstanceMember = true;
   EXPECT_EQ(Instance.evaluate(), Default.evaluate());
+
+  SymbolRelevanceSignals InBaseClass;
+  InBaseClass.InBaseClass = true;
+  EXPECT_LT(InBaseClass.evaluate(), Default.evaluate());
 }
 
 TEST(QualityTests, ScopeProximity) {
Index: clang-tools-extra/trunk/clangd/Quality.cpp
===
--- clang-tools-extra/trunk/clangd/Quality.cpp
+++ clang-tools-extra/trunk/clangd/Quality.cpp
@@ -299,6 +299,7 @@
   : 0.6;
 SemaFileProximityScore = std::max(DeclProximity, SemaFileProximityScore);
 IsInstanceMember |= isInstanceMember(SemaCCResult.Declaration);
+InBaseClass |= SemaCCResult.InBaseClass;
   }
 
   // Declarations are scoped, others (like macros) are assumed global.
@@ -372,9 +373,12 @@
   if (!IsInstanceMember &&
   (Context == CodeCompletionContext::CCC_DotMemberAccess ||
Context == CodeCompletionContext::CCC_ArrowMemberAccess)) {
-Score *= 0.5;
+Score *= 0.2;
   }
 
+  if (InBaseClass)
+Score *= 0.5;
+
   // Penalize for FixIts.
   if (NeedsFixIts)
 Score *= 0.5;
Index: clang-tools-extra/tr

[PATCH] D53644: [clangd] workspace/symbol should be async, it reads from the index.

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added a reviewer: ilya-biryukov.
Herald added subscribers: cfe-commits, kadircet, jfb, arphaman, jkorous, 
MaskRay, ioeric, javed.absar.

To enable this, TUScheduler has to provide a way to run async tasks without
needing a preamble or AST!


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53644

Files:
  clangd/ClangdServer.cpp
  clangd/TUScheduler.cpp
  clangd/TUScheduler.h
  unittests/clangd/TUSchedulerTests.cpp


Index: unittests/clangd/TUSchedulerTests.cpp
===
--- unittests/clangd/TUSchedulerTests.cpp
+++ unittests/clangd/TUSchedulerTests.cpp
@@ -532,6 +532,18 @@
   ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10)));
 }
 
+TEST_F(TUSchedulerTests, Run) {
+  TUScheduler S(/*AsyncThreadsCount=*/getDefaultAsyncThreadsCount(),
+/*StorePreambleInMemory=*/true, /*ASTCallbacks=*/nullptr,
+/*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
+ASTRetentionPolicy());
+  std::atomic Counter(0);
+  S.run("add 1", [&] { Counter.fetch_add(1); });
+  S.run("add 2", [&] { Counter.fetch_add(2); });
+  ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10)));
+  EXPECT_EQ(Counter.load(), 3);
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clangd/TUScheduler.h
===
--- clangd/TUScheduler.h
+++ clangd/TUScheduler.h
@@ -108,6 +108,9 @@
   /// resources.
   void remove(PathRef File);
 
+  /// Schedule an async task with no dependencies.
+  void run(llvm::StringRef Name, llvm::unique_function Action);
+
   /// Schedule an async read of the AST. \p Action will be called when AST is
   /// ready. The AST passed to \p Action refers to the version of \p File
   /// tracked at the time of the call, even if new updates are received before
Index: clangd/TUScheduler.cpp
===
--- clangd/TUScheduler.cpp
+++ clangd/TUScheduler.cpp
@@ -720,6 +720,12 @@
  File);
 }
 
+void TUScheduler::run(StringRef Name, unique_function Action) {
+  if (!PreambleTasks)
+return Action();
+  PreambleTasks->runAsync(Name, std::move(Action));
+}
+
 void TUScheduler::runWithAST(
 StringRef Name, PathRef File,
 unique_function)> Action) {
Index: clangd/ClangdServer.cpp
===
--- clangd/ClangdServer.cpp
+++ clangd/ClangdServer.cpp
@@ -482,8 +482,15 @@
 
 void ClangdServer::workspaceSymbols(
 StringRef Query, int Limit, Callback> CB) {
-  CB(clangd::getWorkspaceSymbols(Query, Limit, Index,
- WorkspaceRoot.getValueOr("")));
+  std::string QueryCopy = Query;
+  WorkScheduler.run(
+  "getWorkspaceSymbols",
+  Bind(
+  [QueryCopy, Limit, this](decltype(CB) CB) {
+CB(clangd::getWorkspaceSymbols(QueryCopy, Limit, Index,
+   WorkspaceRoot.getValueOr("")));
+  },
+  std::move(CB)));
 }
 
 void ClangdServer::documentSymbols(


Index: unittests/clangd/TUSchedulerTests.cpp
===
--- unittests/clangd/TUSchedulerTests.cpp
+++ unittests/clangd/TUSchedulerTests.cpp
@@ -532,6 +532,18 @@
   ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10)));
 }
 
+TEST_F(TUSchedulerTests, Run) {
+  TUScheduler S(/*AsyncThreadsCount=*/getDefaultAsyncThreadsCount(),
+/*StorePreambleInMemory=*/true, /*ASTCallbacks=*/nullptr,
+/*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
+ASTRetentionPolicy());
+  std::atomic Counter(0);
+  S.run("add 1", [&] { Counter.fetch_add(1); });
+  S.run("add 2", [&] { Counter.fetch_add(2); });
+  ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10)));
+  EXPECT_EQ(Counter.load(), 3);
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clangd/TUScheduler.h
===
--- clangd/TUScheduler.h
+++ clangd/TUScheduler.h
@@ -108,6 +108,9 @@
   /// resources.
   void remove(PathRef File);
 
+  /// Schedule an async task with no dependencies.
+  void run(llvm::StringRef Name, llvm::unique_function Action);
+
   /// Schedule an async read of the AST. \p Action will be called when AST is
   /// ready. The AST passed to \p Action refers to the version of \p File
   /// tracked at the time of the call, even if new updates are received before
Index: clangd/TUScheduler.cpp
===
--- clangd/TUScheduler.cpp
+++ clangd/TUScheduler.cpp
@@ -720,6 +720,12 @@
  File);
 }
 
+void TUScheduler::run(StringRef Name, unique_function Action) {
+  if (!PreambleTasks)
+return Action();
+  PreambleTasks->runAsync(Name, std::move(Action));
+}
+
 void TUScheduler::runWithAST(
 StringRef Name, PathRef File,
 un

[PATCH] D53417: [Clang][PowerPC] Choose a better candidate in overload function call if there is a compatible vector conversion instead of ambiguous call error

[Zixuan Wu via Phabricator via cfe-commits]

wuzish updated this revision to Diff 170871.
wuzish marked 2 inline comments as done.
wuzish added a comment.

Updated the diff.

fix some points from comments.


https://reviews.llvm.org/D53417

Files:
  clang/lib/Sema/SemaOverload.cpp
  clang/test/Sema/altivec-generic-overload.c

Index: clang/test/Sema/altivec-generic-overload.c
===
--- /dev/null
+++ clang/test/Sema/altivec-generic-overload.c
@@ -0,0 +1,101 @@
+// RUN: %clang_cc1 %s -triple=powerpc64le-unknown-linux -target-feature +altivec -target-feature +vsx -verify -verify-ignore-unexpected=note -pedantic -fsyntax-only
+
+typedef signed char __v16sc __attribute__((__vector_size__(16)));
+typedef unsigned char __v16uc __attribute__((__vector_size__(16)));
+typedef signed short __v8ss __attribute__((__vector_size__(16)));
+typedef unsigned short __v8us __attribute__((__vector_size__(16)));
+typedef signed int __v4si __attribute__((__vector_size__(16)));
+typedef unsigned int __v4ui __attribute__((__vector_size__(16)));
+typedef signed long long __v2sll __attribute__((__vector_size__(16)));
+typedef unsigned long long __v2ull __attribute__((__vector_size__(16)));
+typedef signed __int128 __v1slll __attribute__((__vector_size__(16)));
+typedef unsigned __int128 __v1ulll __attribute__((__vector_size__(16)));
+typedef float __v4f __attribute__((__vector_size__(16)));
+typedef double __v2d __attribute__((__vector_size__(16)));
+
+void __attribute__((__overloadable__)) convert1(vector signed char);
+void __attribute__((__overloadable__)) convert1(vector unsigned char);
+void __attribute__((__overloadable__)) convert1(vector signed short);
+void __attribute__((__overloadable__)) convert1(vector unsigned short);
+void __attribute__((__overloadable__)) convert1(vector signed int);
+void __attribute__((__overloadable__)) convert1(vector unsigned int);
+void __attribute__((__overloadable__)) convert1(vector signed long long);
+void __attribute__((__overloadable__)) convert1(vector unsigned long long);
+void __attribute__((__overloadable__)) convert1(vector signed __int128);
+void __attribute__((__overloadable__)) convert1(vector unsigned __int128);
+void __attribute__((__overloadable__)) convert1(vector float);
+void __attribute__((__overloadable__)) convert1(vector double);
+void __attribute__((__overloadable__)) convert1(vector bool int);
+void __attribute__((__overloadable__)) convert1(vector pixel short);
+void __attribute__((__overloadable__)) convert2(__v16sc);
+void __attribute__((__overloadable__)) convert2(__v16uc);
+void __attribute__((__overloadable__)) convert2(__v8ss);
+void __attribute__((__overloadable__)) convert2(__v8us);
+void __attribute__((__overloadable__)) convert2(__v4si);
+void __attribute__((__overloadable__)) convert2(__v4ui);
+void __attribute__((__overloadable__)) convert2(__v2sll);
+void __attribute__((__overloadable__)) convert2(__v2ull);
+void __attribute__((__overloadable__)) convert2(__v1slll);
+void __attribute__((__overloadable__)) convert2(__v1ulll);
+void __attribute__((__overloadable__)) convert2(__v4f);
+void __attribute__((__overloadable__)) convert2(__v2d);
+
+void test()
+{
+  __v16sc gv1;
+  __v16uc gv2;
+  __v8ss gv3;
+  __v8us gv4;
+  __v4si gv5;
+  __v4ui gv6;
+  __v2sll gv7;
+  __v2ull gv8;
+  __v1slll gv9;
+  __v1ulll gv10;
+  __v4f gv11;
+  __v2d  gv12;
+
+  vector signed char av1;
+  vector unsigned char av2;
+  vector signed short av3;
+  vector unsigned short av4;
+  vector signed int av5;
+  vector unsigned int av6;
+  vector signed long long av7;
+  vector unsigned long long av8;
+  vector signed __int128 av9;
+  vector unsigned __int128 av10;
+  vector float av11;
+  vector double av12;
+  vector bool int av13;
+  vector pixel short av14;
+
+  convert1(gv1);
+  convert1(gv2);
+  convert1(gv3);
+  convert1(gv4);
+  convert1(gv5);
+  convert1(gv6);
+  convert1(gv7);
+  convert1(gv8);
+  convert1(gv9);
+  convert1(gv10);
+  convert1(gv11);
+  convert1(gv12);
+
+  convert2(av1);
+  convert2(av2);
+  convert2(av3);
+  convert2(av4);
+  convert2(av5);
+  convert2(av6);
+  convert2(av7);
+  convert2(av8);
+  convert2(av9);
+  convert2(av10);
+  convert2(av11);
+  convert2(av12);
+  convert2(av13); // expected-error {{call to 'convert2' is ambiguous}}
+  convert2(av14); // expected-error {{call to 'convert2' is ambiguous}}
+
+}
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -3900,6 +3900,56 @@
   S.Context.getTypeSize(SCS1.getToType(2)))
 return ImplicitConversionSequence::Better;
 
+  // For now only conversions with altivec-related kind and generic kind
+  // prefer to choose a compatible vector conversion
+  // FIXME: Can remove this constraint and extend to other vector kind?
+  auto IsPreferCompatibleConversionVectorKind =
+  [&S](const StandardConversionSequence &SCS1,
+   const StandardConve

[PATCH] D53571: [clangd] Don't show base class versions of members as completions.

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 170875.
sammccall added a comment.

Rebase


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53571

Files:
  clangd/CodeComplete.cpp
  unittests/clangd/CodeCompleteTests.cpp


Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -381,10 +381,13 @@
   void test() { Bar().^ }
   )cpp");
   EXPECT_THAT(Results.Completions,
-  HasSubsequence(AllOf(Qualifier(""), Named("bar")),
- AllOf(Qualifier("Foo::"), Named("foo";
+  Contains(AllOf(Qualifier(""), Named("bar";
+  // Hidden members are not shown.
   EXPECT_THAT(Results.Completions,
-  Not(Contains(AllOf(Qualifier(""), Named("foo"); // private
+  Not(Contains(AllOf(Qualifier("Foo::"), Named("foo");
+  // Private members are not shown.
+  EXPECT_THAT(Results.Completions,
+  Not(Contains(AllOf(Qualifier(""), Named("foo");
 }
 
 TEST(CompletionTest, InjectedTypename) {
Index: clangd/CodeComplete.cpp
===
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -728,9 +728,9 @@
 // Retain the results we might want.
 for (unsigned I = 0; I < NumResults; ++I) {
   auto &Result = InResults[I];
-  // Drop hidden items which cannot be found by lookup after completion.
-  // Exception: some items can be named by using a qualifier.
-  if (Result.Hidden && (!Result.Qualifier || 
Result.QualifierIsInformative))
+  // Class members that are shadowed by subclasses are usually noise.
+  if (Result.Hidden && Result.Declaration &&
+  Result.Declaration->isCXXClassMember())
 continue;
   if (!Opts.IncludeIneligibleResults &&
   (Result.Availability == CXAvailability_NotAvailable ||


Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -381,10 +381,13 @@
   void test() { Bar().^ }
   )cpp");
   EXPECT_THAT(Results.Completions,
-  HasSubsequence(AllOf(Qualifier(""), Named("bar")),
- AllOf(Qualifier("Foo::"), Named("foo";
+  Contains(AllOf(Qualifier(""), Named("bar";
+  // Hidden members are not shown.
   EXPECT_THAT(Results.Completions,
-  Not(Contains(AllOf(Qualifier(""), Named("foo"); // private
+  Not(Contains(AllOf(Qualifier("Foo::"), Named("foo");
+  // Private members are not shown.
+  EXPECT_THAT(Results.Completions,
+  Not(Contains(AllOf(Qualifier(""), Named("foo");
 }
 
 TEST(CompletionTest, InjectedTypename) {
Index: clangd/CodeComplete.cpp
===
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -728,9 +728,9 @@
 // Retain the results we might want.
 for (unsigned I = 0; I < NumResults; ++I) {
   auto &Result = InResults[I];
-  // Drop hidden items which cannot be found by lookup after completion.
-  // Exception: some items can be named by using a qualifier.
-  if (Result.Hidden && (!Result.Qualifier || Result.QualifierIsInformative))
+  // Class members that are shadowed by subclasses are usually noise.
+  if (Result.Hidden && Result.Declaration &&
+  Result.Declaration->isCXXClassMember())
 continue;
   if (!Opts.IncludeIneligibleResults &&
   (Result.Availability == CXAvailability_NotAvailable ||
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53571: [clangd] Don't show base class versions of members as completions.

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE345141: [clangd] Don't show base class versions of 
members as completions. (authored by sammccall, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D53571?vs=170875&id=170876#toc

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53571

Files:
  clangd/CodeComplete.cpp
  unittests/clangd/CodeCompleteTests.cpp


Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -381,10 +381,13 @@
   void test() { Bar().^ }
   )cpp");
   EXPECT_THAT(Results.Completions,
-  HasSubsequence(AllOf(Qualifier(""), Named("bar")),
- AllOf(Qualifier("Foo::"), Named("foo";
+  Contains(AllOf(Qualifier(""), Named("bar";
+  // Hidden members are not shown.
   EXPECT_THAT(Results.Completions,
-  Not(Contains(AllOf(Qualifier(""), Named("foo"); // private
+  Not(Contains(AllOf(Qualifier("Foo::"), Named("foo");
+  // Private members are not shown.
+  EXPECT_THAT(Results.Completions,
+  Not(Contains(AllOf(Qualifier(""), Named("foo");
 }
 
 TEST(CompletionTest, InjectedTypename) {
Index: clangd/CodeComplete.cpp
===
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -728,9 +728,9 @@
 // Retain the results we might want.
 for (unsigned I = 0; I < NumResults; ++I) {
   auto &Result = InResults[I];
-  // Drop hidden items which cannot be found by lookup after completion.
-  // Exception: some items can be named by using a qualifier.
-  if (Result.Hidden && (!Result.Qualifier || 
Result.QualifierIsInformative))
+  // Class members that are shadowed by subclasses are usually noise.
+  if (Result.Hidden && Result.Declaration &&
+  Result.Declaration->isCXXClassMember())
 continue;
   if (!Opts.IncludeIneligibleResults &&
   (Result.Availability == CXAvailability_NotAvailable ||


Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -381,10 +381,13 @@
   void test() { Bar().^ }
   )cpp");
   EXPECT_THAT(Results.Completions,
-  HasSubsequence(AllOf(Qualifier(""), Named("bar")),
- AllOf(Qualifier("Foo::"), Named("foo";
+  Contains(AllOf(Qualifier(""), Named("bar";
+  // Hidden members are not shown.
   EXPECT_THAT(Results.Completions,
-  Not(Contains(AllOf(Qualifier(""), Named("foo"); // private
+  Not(Contains(AllOf(Qualifier("Foo::"), Named("foo");
+  // Private members are not shown.
+  EXPECT_THAT(Results.Completions,
+  Not(Contains(AllOf(Qualifier(""), Named("foo");
 }
 
 TEST(CompletionTest, InjectedTypename) {
Index: clangd/CodeComplete.cpp
===
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -728,9 +728,9 @@
 // Retain the results we might want.
 for (unsigned I = 0; I < NumResults; ++I) {
   auto &Result = InResults[I];
-  // Drop hidden items which cannot be found by lookup after completion.
-  // Exception: some items can be named by using a qualifier.
-  if (Result.Hidden && (!Result.Qualifier || Result.QualifierIsInformative))
+  // Class members that are shadowed by subclasses are usually noise.
+  if (Result.Hidden && Result.Declaration &&
+  Result.Declaration->isCXXClassMember())
 continue;
   if (!Opts.IncludeIneligibleResults &&
   (Result.Availability == CXAvailability_NotAvailable ||
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r345141 - [clangd] Don't show base class versions of members as completions.

[Sam McCall via cfe-commits]

Author: sammccall
Date: Wed Oct 24 06:51:44 2018
New Revision: 345141

URL: http://llvm.org/viewvc/llvm-project?rev=345141&view=rev
Log:
[clangd] Don't show base class versions of members as completions.

Summary:
These are available via qualifiers, but signal to noise level is low.
Keep required quailifier machinery around though, for cross-ns completion.

Reviewers: ioeric

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

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

Modified:
clang-tools-extra/trunk/clangd/CodeComplete.cpp
clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp

Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.cpp?rev=345141&r1=345140&r2=345141&view=diff
==
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original)
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Wed Oct 24 06:51:44 2018
@@ -728,9 +728,9 @@ struct CompletionRecorder : public CodeC
 // Retain the results we might want.
 for (unsigned I = 0; I < NumResults; ++I) {
   auto &Result = InResults[I];
-  // Drop hidden items which cannot be found by lookup after completion.
-  // Exception: some items can be named by using a qualifier.
-  if (Result.Hidden && (!Result.Qualifier || 
Result.QualifierIsInformative))
+  // Class members that are shadowed by subclasses are usually noise.
+  if (Result.Hidden && Result.Declaration &&
+  Result.Declaration->isCXXClassMember())
 continue;
   if (!Opts.IncludeIneligibleResults &&
   (Result.Availability == CXAvailability_NotAvailable ||

Modified: clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp?rev=345141&r1=345140&r2=345141&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp Wed Oct 24 
06:51:44 2018
@@ -381,10 +381,13 @@ TEST(CompletionTest, Qualifiers) {
   void test() { Bar().^ }
   )cpp");
   EXPECT_THAT(Results.Completions,
-  HasSubsequence(AllOf(Qualifier(""), Named("bar")),
- AllOf(Qualifier("Foo::"), Named("foo";
+  Contains(AllOf(Qualifier(""), Named("bar";
+  // Hidden members are not shown.
   EXPECT_THAT(Results.Completions,
-  Not(Contains(AllOf(Qualifier(""), Named("foo"); // private
+  Not(Contains(AllOf(Qualifier("Foo::"), Named("foo");
+  // Private members are not shown.
+  EXPECT_THAT(Results.Completions,
+  Not(Contains(AllOf(Qualifier(""), Named("foo");
 }
 
 TEST(CompletionTest, InjectedTypename) {


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

[PATCH] D53488: [clang-tidy] Catching narrowing from double to float.

[Guillaume Chatelet via Phabricator via cfe-commits]

gchatelet updated this revision to Diff 170878.
gchatelet marked an inline comment as done.
gchatelet added a comment.

- Join the two parts of the ReleaseNotes update


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53488

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

Index: test/clang-tidy/cppcoreguidelines-narrowing-conversions.cpp
===
--- test/clang-tidy/cppcoreguidelines-narrowing-conversions.cpp
+++ test/clang-tidy/cppcoreguidelines-narrowing-conversions.cpp
@@ -9,12 +9,12 @@
 namespace floats {
 
 struct ConvertsToFloat {
-  operator float() const { return 0.5; }
+  operator float() const { return 0.5f; }
 };
 
-float operator "" _Pa(unsigned long long);
+float operator"" _float(unsigned long long);
 
-void not_ok(double d) {
+void narrow_double_to_int_not_ok(double d) {
   int i = 0;
   i = d;
   // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: narrowing conversion from 'double' to 'int' [cppcoreguidelines-narrowing-conversions]
@@ -24,11 +24,11 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: narrowing conversion from 'float' to 'int' [cppcoreguidelines-narrowing-conversions]
   i = ConvertsToFloat();
   // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: narrowing conversion from 'float' to 'int' [cppcoreguidelines-narrowing-conversions]
-  i = 15_Pa;
+  i = 15_float;
   // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: narrowing conversion from 'float' to 'int' [cppcoreguidelines-narrowing-conversions]
 }
 
-void not_ok_binary_ops(double d) {
+void narrow_double_to_int_not_ok_binary_ops(double d) {
   int i = 0;
   i += 0.5;
   // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: narrowing conversion from 'double' to 'int' [cppcoreguidelines-narrowing-conversions]
@@ -52,6 +52,43 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: narrowing conversion from 'double' to 'int' [cppcoreguidelines-narrowing-conversions]
 }
 
+double operator"" _double(unsigned long long);
+
+float narrow_double_to_float_return() {
+  return 0.5;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: narrowing conversion from 'double' to 'float' [cppcoreguidelines-narrowing-conversions]
+}
+
+void narrow_double_to_float_not_ok(double d) {
+  float f = 0;
+  f = d;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: narrowing conversion from 'double' to 'float' [cppcoreguidelines-narrowing-conversions]
+  f = 0.5;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: narrowing conversion from 'double' to 'float' [cppcoreguidelines-narrowing-conversions]
+  f = 15_double;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: narrowing conversion from 'double' to 'float' [cppcoreguidelines-narrowing-conversions]
+}
+
+void narrow_double_to_float_not_ok_binary_ops(double d) {
+  float f = 0;
+  f += 0.5;
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: narrowing conversion from 'double' to 'float' [cppcoreguidelines-narrowing-conversions]
+  f += d;
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: narrowing conversion from 'double' to 'float' [cppcoreguidelines-narrowing-conversions]
+  // We warn on the following even though it's not dangerous because there is no
+  // reason to use a double literal here.
+  // TODO(courbet): Provide an automatic fix.
+  f += 2.0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: narrowing conversion from 'double' to 'float' [cppcoreguidelines-narrowing-conversions]
+
+  f *= 0.5;
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: narrowing conversion from 'double' to 'float' [cppcoreguidelines-narrowing-conversions]
+  f /= 0.5;
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: narrowing conversion from 'double' to 'float' [cppcoreguidelines-narrowing-conversions]
+  f += (double)0.5f;
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: narrowing conversion from 'double' to 'float' [cppcoreguidelines-narrowing-conversions]
+}
+
 void ok(double d) {
   int i = 0;
   i = 1;
@@ -89,15 +126,19 @@
 
 void template_context() {
   f(1, 2);
+  f(1, .5f);
   f(1, .5);
+  f(1, .5l);
 }
 
 #define DERP(i, j) (i += j)
 
 void macro_context() {
   int i = 0;
   DERP(i, 2);
+  DERP(i, .5f);
   DERP(i, .5);
+  DERP(i, .5l);
 }
 
-}  // namespace floats
+} // namespace floats
Index: docs/clang-tidy/checks/cppcoreguidelines-narrowing-conversions.rst
===
--- docs/clang-tidy/checks/cppcoreguidelines-narrowing-conversions.rst
+++ docs/clang-tidy/checks/cppcoreguidelines-narrowing-conversions.rst
@@ -10,13 +10,15 @@
 This rule is part of the "Expressions and statements" profile of the C++ Core
 Guidelines, corresponding to rule ES.46. See
 
-https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Res-narrowing.
+https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines

[PATCH] D53488: [clang-tidy] Catching narrowing from double to float.

[Jonas Toth via Phabricator via cfe-commits]

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

Did you run this code over a real-world code-base and did you find new stuff 
and/or false positives or the like?

From my side LGTM unless other reviewers see outstanding issues.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53488



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

[PATCH] D53488: [clang-tidy] Catching narrowing from double to float.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:56-57
 
-void NarrowingConversionsCheck::check(const MatchFinder::MatchResult &Result) {
-  if (const auto *Op = Result.Nodes.getNodeAs("op")) {
-if (Op->getBeginLoc().isMacroID())
-  return;
-diag(Op->getOperatorLoc(), "narrowing conversion from %0 to %1")
-<< Op->getRHS()->getType() << Op->getLHS()->getType();
+static bool isNarrower(const ASTContext *const Context, const QualType Lhs,
+   const QualType Rhs) {
+  assert(Lhs->isRealType()); // Either integer or floating point.

Please drop the top-level `const` qualifiers (that's not something we do 
consistently elsewhere). Here and elsewhere.



Comment at: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:68
 return;
-  }
-  const auto *Cast = Result.Nodes.getNodeAs("cast");
-  if (Cast->getBeginLoc().isMacroID())
+  const auto LhsType = Op.getLHS()->getType();
+  const auto RhsType = Op.getRHS()->getType();

Do not use `auto` here as the type is not spelled out in the initialization 
(same elsewhere). You should also drop the top-level `const`.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53488



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

[clang-tools-extra] r345144 - [clangd] Ensure that we reply to each call exactly once. NFC (I think!)

[Sam McCall via cfe-commits]

Author: sammccall
Date: Wed Oct 24 07:26:26 2018
New Revision: 345144

URL: http://llvm.org/viewvc/llvm-project?rev=345144&view=rev
Log:
[clangd] Ensure that we reply to each call exactly once. NFC (I think!)

Summary:
In debug builds, getting this wrong will trigger asserts.
In production builds, it will send an error reply if none was sent,
and drop redundant replies. (And log).

No tests because this is always a programming error.
(We did have some cases of this, but I fixed them with the new dispatcher).

Reviewers: ioeric

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

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

Modified:
clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
clang-tools-extra/trunk/clangd/Trace.h

Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=345144&r1=345143&r2=345144&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Wed Oct 24 07:26:26 2018
@@ -105,16 +105,21 @@ public:
   }
 
   bool onCall(StringRef Method, json::Value Params, json::Value ID) override {
+// Calls can be canceled by the client. Add cancellation context.
+WithContext WithCancel(cancelableRequestContext(ID));
+trace::Span Tracer(Method);
+SPAN_ATTACH(Tracer, "Params", Params);
+ReplyOnce Reply(ID, Method, &Server, Tracer.Args);
 log("<-- {0}({1})", Method, ID);
 if (!Server.Server && Method != "initialize") {
   elog("Call {0} before initialization.", Method);
-  Server.reply(ID, make_error("server not initialized",
-ErrorCode::ServerNotInitialized));
+  Reply(make_error("server not initialized",
+ ErrorCode::ServerNotInitialized));
 } else if (auto Handler = Calls.lookup(Method))
-  Handler(std::move(Params), std::move(ID));
+  Handler(std::move(Params), std::move(Reply));
 else
-  Server.reply(ID, make_error("method not found",
-ErrorCode::MethodNotFound));
+  Reply(
+  make_error("method not found", ErrorCode::MethodNotFound));
 return true;
   }
 
@@ -128,36 +133,19 @@ public:
   }
 
   // Bind an LSP method name to a call.
-  template 
+  template 
   void bind(const char *Method,
-void (ClangdLSPServer::*Handler)(const Param &, Callback)) {
+void (ClangdLSPServer::*Handler)(const Param &, Callback)) 
{
 Calls[Method] = [Method, Handler, this](json::Value RawParams,
-json::Value ID) {
+ReplyOnce Reply) {
   Param P;
-  if (!fromJSON(RawParams, P)) {
+  if (fromJSON(RawParams, P)) {
+(Server.*Handler)(P, std::move(Reply));
+  } else {
 elog("Failed to decode {0} request.", Method);
-Server.reply(ID, make_error("failed to decode request",
-  ErrorCode::InvalidRequest));
-return;
+Reply(make_error("failed to decode request",
+   ErrorCode::InvalidRequest));
   }
-  trace::Span Tracer(Method);
-  SPAN_ATTACH(Tracer, "Params", RawParams);
-  auto *Trace = Tracer.Args; // We attach reply from another thread.
-  // Calls can be canceled by the client. Add cancellation context.
-  WithContext WithCancel(cancelableRequestContext(ID));
-  // FIXME: this function should assert it's called exactly once.
-  (Server.*Handler)(P, [this, ID, Trace](Expected Result) {
-if (Result) {
-  if (Trace)
-(*Trace)["Reply"] = *Result;
-  Server.reply(ID, json::Value(std::move(*Result)));
-} else {
-  auto Err = Result.takeError();
-  if (Trace)
-(*Trace)["Error"] = to_string(Err);
-  Server.reply(ID, std::move(Err));
-}
-  });
 };
   }
 
@@ -178,8 +166,65 @@ public:
   }
 
 private:
+  // Function object to reply to an LSP call.
+  // Each instance must be called exactly once, otherwise:
+  //  - the bug is logged, and (in debug mode) an assert will fire
+  //  - if there was no reply, an error reply is sent
+  //  - if there were multiple replies, only the first is sent
+  class ReplyOnce {
+std::atomic Replied = {false};
+json::Value ID;
+std::string Method;
+ClangdLSPServer *Server; // Null when moved-from.
+json::Object *TraceArgs;
+
+  public:
+ReplyOnce(const json::Value &ID, StringRef Method, ClangdLSPServer *Server,
+  json::Object *TraceArgs)
+: ID(ID), Method(Method), Server(Server), TraceArgs(TraceArgs) {
+  assert(Server);
+}
+ReplyOnce(ReplyOnce &&Other)
+: Replied(Other.Replied.l

[PATCH] D53448: [OpenMP][NVPTX] Use single loops when generating code for distribute parallel for

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: lib/CodeGen/CGOpenMPRuntime.h:904
+  ///
+  virtual bool isStaticChunked(OpenMPDistScheduleClauseKind ScheduleKind,
+   bool Chunked) const;

I'd rename this into `isDistStaticChunked`



Comment at: lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp:4249
 CodeGenFunction &CGF, const OMPLoopDirective &S,
-OpenMPScheduleClauseKind &ScheduleKind,
+OpenMPScheduleTy &ScheduleKind,
 llvm::Value *&Chunk) const {

If the `ChunkOne` field is not required, you need to restore original code here



Comment at: lib/CodeGen/CGStmtOpenMP.cpp:2360
 OMPCancelStack.emitExit(*this, S.getDirectiveKind(), CodeGen);
+  } else if (RT.isStaticChunked(ScheduleKind.Schedule,
+/* Chunked */ Chunk != nullptr) &&

This whole code is very similar to the unchunked case. Could you merge it?



Comment at: lib/CodeGen/CGStmtOpenMP.cpp:2362
+/* Chunked */ Chunk != nullptr) &&
+ ScheduleKind.HasChunkOne &&
+ isOpenMPLoopBoundSharingDirective(S.getDirectiveKind())) {

It allows you to check only the implicit case, what about if the user 
explicitly specifies that `chunk` is `1`?



Comment at: lib/CodeGen/CGStmtOpenMP.cpp:3421
 RT.emitForStaticFinish(*this, S.getBeginLoc(), S.getDirectiveKind());
+  } else if (RT.isStaticChunked(ScheduleKind,
+/* Chunked */ Chunk != nullptr) &&

Again, very similar to the unchunked code. Merge it.



Comment at: lib/Sema/SemaOpenMP.cpp:5207
+CombDistCond =
+SemaRef.BuildBinOp(CurScope, CondLoc, BO_LE, IV.get(), 
LastIteration.get());
+  }

Seems to me, you need to use `NumIterations` instead of `LastIteration`


Repository:
  rC Clang

https://reviews.llvm.org/D53448



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

[PATCH] D53399: [clangd] Ensure that we reply to each call exactly once. NFC (I think!)

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE345144: [clangd] Ensure that we reply to each call exactly 
once. NFC (I think!) (authored by sammccall, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D53399?vs=170866&id=170881#toc

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53399

Files:
  clangd/ClangdLSPServer.cpp
  clangd/Trace.h

Index: clangd/Trace.h
===
--- clangd/Trace.h
+++ clangd/Trace.h
@@ -87,6 +87,7 @@
 
   /// Mutable metadata, if this span is interested.
   /// Prefer to use SPAN_ATTACH rather than accessing this directly.
+  /// The lifetime of Args is the whole event, even if the Span dies.
   llvm::json::Object *const Args;
 
 private:
Index: clangd/ClangdLSPServer.cpp
===
--- clangd/ClangdLSPServer.cpp
+++ clangd/ClangdLSPServer.cpp
@@ -105,16 +105,21 @@
   }
 
   bool onCall(StringRef Method, json::Value Params, json::Value ID) override {
+// Calls can be canceled by the client. Add cancellation context.
+WithContext WithCancel(cancelableRequestContext(ID));
+trace::Span Tracer(Method);
+SPAN_ATTACH(Tracer, "Params", Params);
+ReplyOnce Reply(ID, Method, &Server, Tracer.Args);
 log("<-- {0}({1})", Method, ID);
 if (!Server.Server && Method != "initialize") {
   elog("Call {0} before initialization.", Method);
-  Server.reply(ID, make_error("server not initialized",
-ErrorCode::ServerNotInitialized));
+  Reply(make_error("server not initialized",
+ ErrorCode::ServerNotInitialized));
 } else if (auto Handler = Calls.lookup(Method))
-  Handler(std::move(Params), std::move(ID));
+  Handler(std::move(Params), std::move(Reply));
 else
-  Server.reply(ID, make_error("method not found",
-ErrorCode::MethodNotFound));
+  Reply(
+  make_error("method not found", ErrorCode::MethodNotFound));
 return true;
   }
 
@@ -128,36 +133,19 @@
   }
 
   // Bind an LSP method name to a call.
-  template 
+  template 
   void bind(const char *Method,
-void (ClangdLSPServer::*Handler)(const Param &, Callback)) {
+void (ClangdLSPServer::*Handler)(const Param &, Callback)) {
 Calls[Method] = [Method, Handler, this](json::Value RawParams,
-json::Value ID) {
+ReplyOnce Reply) {
   Param P;
-  if (!fromJSON(RawParams, P)) {
+  if (fromJSON(RawParams, P)) {
+(Server.*Handler)(P, std::move(Reply));
+  } else {
 elog("Failed to decode {0} request.", Method);
-Server.reply(ID, make_error("failed to decode request",
-  ErrorCode::InvalidRequest));
-return;
+Reply(make_error("failed to decode request",
+   ErrorCode::InvalidRequest));
   }
-  trace::Span Tracer(Method);
-  SPAN_ATTACH(Tracer, "Params", RawParams);
-  auto *Trace = Tracer.Args; // We attach reply from another thread.
-  // Calls can be canceled by the client. Add cancellation context.
-  WithContext WithCancel(cancelableRequestContext(ID));
-  // FIXME: this function should assert it's called exactly once.
-  (Server.*Handler)(P, [this, ID, Trace](Expected Result) {
-if (Result) {
-  if (Trace)
-(*Trace)["Reply"] = *Result;
-  Server.reply(ID, json::Value(std::move(*Result)));
-} else {
-  auto Err = Result.takeError();
-  if (Trace)
-(*Trace)["Error"] = to_string(Err);
-  Server.reply(ID, std::move(Err));
-}
-  });
 };
   }
 
@@ -178,8 +166,65 @@
   }
 
 private:
+  // Function object to reply to an LSP call.
+  // Each instance must be called exactly once, otherwise:
+  //  - the bug is logged, and (in debug mode) an assert will fire
+  //  - if there was no reply, an error reply is sent
+  //  - if there were multiple replies, only the first is sent
+  class ReplyOnce {
+std::atomic Replied = {false};
+json::Value ID;
+std::string Method;
+ClangdLSPServer *Server; // Null when moved-from.
+json::Object *TraceArgs;
+
+  public:
+ReplyOnce(const json::Value &ID, StringRef Method, ClangdLSPServer *Server,
+  json::Object *TraceArgs)
+: ID(ID), Method(Method), Server(Server), TraceArgs(TraceArgs) {
+  assert(Server);
+}
+ReplyOnce(ReplyOnce &&Other)
+: Replied(Other.Replied.load()), ID(std::move(Other.ID)),
+  Method(std::move(Other.Method)), Server(Other.Server),
+  TraceArgs(Other.TraceArgs) {
+  Other.Server = nullptr;
+}
+ReplyOnce& operator=(ReplyOnce&&) = delete;
+ReplyOnce(const Rep