[PATCH] D44609: [Clang-Format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style)
christophe-calmejane added a comment.
Hi,
consider the following code
myFunc([](int x) {
if (x)
call();
});
When activating your BeforeLambdaBody option, it changes the code to this
(breaking the start of the lambda to a new line)
myFunc(
[](int x)
{
if (x)
call();
});
Another issue if I declare the lambda as noexcept, clang-format changes the
code to this (looks like it's not detecting and applying your option):
myFunc([](int x) noexcept {
if (x)
call();
});
Repository:
rC Clang
https://reviews.llvm.org/D44609
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50179: [AArch64][ARM] Context sensitive meaning of option "crypto"
SjoerdMeijer added a comment. Thanks! https://reviews.llvm.org/D50179 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r343758 - [AArch64][ARM] Context sensitive meaning of crypto
Author: sjoerdmeijer
Date: Thu Oct 4 00:38:53 2018
New Revision: 343758
URL: http://llvm.org/viewvc/llvm-project?rev=343758&view=rev
Log:
[AArch64][ARM] Context sensitive meaning of crypto
For AArch64, crypto means:
- sm4 + sha3 + sha2 + aes for Armv8.4-A and up, and
- sha2 + aes for Armv8.3-A and earlier.
For AArch32:
Crypto means sha2 + aes, because the Armv8.2-A crypto instructions
were added to AArch64 only.
Differential Revision: https://reviews.llvm.org/D50179
Modified:
cfe/trunk/lib/Driver/ToolChains/Arch/AArch64.cpp
cfe/trunk/lib/Driver/ToolChains/Arch/ARM.cpp
cfe/trunk/test/Driver/arm-features.c
cfe/trunk/test/Preprocessor/aarch64-target-features.c
Modified: cfe/trunk/lib/Driver/ToolChains/Arch/AArch64.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Arch/AArch64.cpp?rev=343758&r1=343757&r2=343758&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Arch/AArch64.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Arch/AArch64.cpp Thu Oct 4 00:38:53 2018
@@ -219,6 +219,87 @@ fp16_fml_fallthrough:
Features.push_back("+fullfp16");
}
+ // FIXME: this needs reimplementation too after the TargetParser rewrite
+ //
+ // Context sensitive meaning of Crypto:
+ // 1) For Arch >= ARMv8.4a: crypto = sm4 + sha3 + sha2 + aes
+ // 2) For Arch <= ARMv8.3a: crypto = sha2 + aes
+ const auto ItBegin = Features.begin();
+ const auto ItEnd = Features.end();
+ const auto ItRBegin = Features.rbegin();
+ const auto ItREnd = Features.rend();
+ const auto ItRCrypto = std::find(ItRBegin, ItREnd, "+crypto");
+ const auto ItRNoCrypto = std::find(ItRBegin, ItREnd, "-crypto");
+ const auto HasCrypto = ItRCrypto != ItREnd;
+ const auto HasNoCrypto = ItRNoCrypto != ItREnd;
+ const ptrdiff_t PosCrypto = ItRCrypto - ItRBegin;
+ const ptrdiff_t PosNoCrypto = ItRNoCrypto - ItRBegin;
+
+ bool NoCrypto = false;
+ if (HasCrypto && HasNoCrypto) {
+if (PosNoCrypto < PosCrypto)
+ NoCrypto = true;
+ }
+
+ if (std::find(ItBegin, ItEnd, "+v8.4a") != ItEnd) {
+if (HasCrypto && !NoCrypto) {
+ // Check if we have NOT disabled an algorithm with something like:
+ // +crypto, -algorithm
+ // And if "-algorithm" does not occur, we enable that crypto algorithm.
+ const bool HasSM4 = (std::find(ItBegin, ItEnd, "-sm4") == ItEnd);
+ const bool HasSHA3 = (std::find(ItBegin, ItEnd, "-sha3") == ItEnd);
+ const bool HasSHA2 = (std::find(ItBegin, ItEnd, "-sha2") == ItEnd);
+ const bool HasAES = (std::find(ItBegin, ItEnd, "-aes") == ItEnd);
+ if (HasSM4)
+Features.push_back("+sm4");
+ if (HasSHA3)
+Features.push_back("+sha3");
+ if (HasSHA2)
+Features.push_back("+sha2");
+ if (HasAES)
+Features.push_back("+aes");
+} else if (HasNoCrypto) {
+ // Check if we have NOT enabled a crypto algorithm with something like:
+ // -crypto, +algorithm
+ // And if "+algorithm" does not occur, we disable that crypto algorithm.
+ const bool HasSM4 = (std::find(ItBegin, ItEnd, "+sm4") != ItEnd);
+ const bool HasSHA3 = (std::find(ItBegin, ItEnd, "+sha3") != ItEnd);
+ const bool HasSHA2 = (std::find(ItBegin, ItEnd, "+sha2") != ItEnd);
+ const bool HasAES = (std::find(ItBegin, ItEnd, "+aes") != ItEnd);
+ if (!HasSM4)
+Features.push_back("-sm4");
+ if (!HasSHA3)
+Features.push_back("-sha3");
+ if (!HasSHA2)
+Features.push_back("-sha2");
+ if (!HasAES)
+Features.push_back("-aes");
+}
+ } else {
+if (HasCrypto && !NoCrypto) {
+ const bool HasSHA2 = (std::find(ItBegin, ItEnd, "-sha2") == ItEnd);
+ const bool HasAES = (std::find(ItBegin, ItEnd, "-aes") == ItEnd);
+ if (HasSHA2)
+Features.push_back("+sha2");
+ if (HasAES)
+Features.push_back("+aes");
+} else if (HasNoCrypto) {
+ const bool HasSHA2 = (std::find(ItBegin, ItEnd, "+sha2") != ItEnd);
+ const bool HasAES = (std::find(ItBegin, ItEnd, "+aes") != ItEnd);
+ const bool HasV82a = (std::find(ItBegin, ItEnd, "+v8.2a") != ItEnd);
+ const bool HasV83a = (std::find(ItBegin, ItEnd, "+v8.3a") != ItEnd);
+ const bool HasV84a = (std::find(ItBegin, ItEnd, "+v8.4a") != ItEnd);
+ if (!HasSHA2)
+Features.push_back("-sha2");
+ if (!HasAES)
+Features.push_back("-aes");
+ if (HasV82a || HasV83a || HasV84a) {
+Features.push_back("-sm4");
+Features.push_back("-sha3");
+ }
+}
+ }
+
if (Arg *A = Args.getLastArg(options::OPT_mno_unaligned_access,
options::OPT_munaligned_access))
if (A->getOption().matches(options::OPT_mno_unaligned_access))
Modified: cfe/trunk/lib/Driver/ToolChains/Arch/ARM.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Arch/ARM.cpp?rev=343758&r1=343757&r2=343758&view=diff
[PATCH] D50179: [AArch64][ARM] Context sensitive meaning of option "crypto"
This revision was automatically updated to reflect the committed changes.
Closed by commit rL343758: [AArch64][ARM] Context sensitive meaning of crypto
(authored by SjoerdMeijer, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D50179?vs=166643&id=168233#toc
Repository:
rL LLVM
https://reviews.llvm.org/D50179
Files:
cfe/trunk/lib/Driver/ToolChains/Arch/AArch64.cpp
cfe/trunk/lib/Driver/ToolChains/Arch/ARM.cpp
cfe/trunk/test/Driver/arm-features.c
cfe/trunk/test/Preprocessor/aarch64-target-features.c
Index: cfe/trunk/lib/Driver/ToolChains/Arch/AArch64.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Arch/AArch64.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Arch/AArch64.cpp
@@ -219,6 +219,87 @@
Features.push_back("+fullfp16");
}
+ // FIXME: this needs reimplementation too after the TargetParser rewrite
+ //
+ // Context sensitive meaning of Crypto:
+ // 1) For Arch >= ARMv8.4a: crypto = sm4 + sha3 + sha2 + aes
+ // 2) For Arch <= ARMv8.3a: crypto = sha2 + aes
+ const auto ItBegin = Features.begin();
+ const auto ItEnd = Features.end();
+ const auto ItRBegin = Features.rbegin();
+ const auto ItREnd = Features.rend();
+ const auto ItRCrypto = std::find(ItRBegin, ItREnd, "+crypto");
+ const auto ItRNoCrypto = std::find(ItRBegin, ItREnd, "-crypto");
+ const auto HasCrypto = ItRCrypto != ItREnd;
+ const auto HasNoCrypto = ItRNoCrypto != ItREnd;
+ const ptrdiff_t PosCrypto = ItRCrypto - ItRBegin;
+ const ptrdiff_t PosNoCrypto = ItRNoCrypto - ItRBegin;
+
+ bool NoCrypto = false;
+ if (HasCrypto && HasNoCrypto) {
+if (PosNoCrypto < PosCrypto)
+ NoCrypto = true;
+ }
+
+ if (std::find(ItBegin, ItEnd, "+v8.4a") != ItEnd) {
+if (HasCrypto && !NoCrypto) {
+ // Check if we have NOT disabled an algorithm with something like:
+ // +crypto, -algorithm
+ // And if "-algorithm" does not occur, we enable that crypto algorithm.
+ const bool HasSM4 = (std::find(ItBegin, ItEnd, "-sm4") == ItEnd);
+ const bool HasSHA3 = (std::find(ItBegin, ItEnd, "-sha3") == ItEnd);
+ const bool HasSHA2 = (std::find(ItBegin, ItEnd, "-sha2") == ItEnd);
+ const bool HasAES = (std::find(ItBegin, ItEnd, "-aes") == ItEnd);
+ if (HasSM4)
+Features.push_back("+sm4");
+ if (HasSHA3)
+Features.push_back("+sha3");
+ if (HasSHA2)
+Features.push_back("+sha2");
+ if (HasAES)
+Features.push_back("+aes");
+} else if (HasNoCrypto) {
+ // Check if we have NOT enabled a crypto algorithm with something like:
+ // -crypto, +algorithm
+ // And if "+algorithm" does not occur, we disable that crypto algorithm.
+ const bool HasSM4 = (std::find(ItBegin, ItEnd, "+sm4") != ItEnd);
+ const bool HasSHA3 = (std::find(ItBegin, ItEnd, "+sha3") != ItEnd);
+ const bool HasSHA2 = (std::find(ItBegin, ItEnd, "+sha2") != ItEnd);
+ const bool HasAES = (std::find(ItBegin, ItEnd, "+aes") != ItEnd);
+ if (!HasSM4)
+Features.push_back("-sm4");
+ if (!HasSHA3)
+Features.push_back("-sha3");
+ if (!HasSHA2)
+Features.push_back("-sha2");
+ if (!HasAES)
+Features.push_back("-aes");
+}
+ } else {
+if (HasCrypto && !NoCrypto) {
+ const bool HasSHA2 = (std::find(ItBegin, ItEnd, "-sha2") == ItEnd);
+ const bool HasAES = (std::find(ItBegin, ItEnd, "-aes") == ItEnd);
+ if (HasSHA2)
+Features.push_back("+sha2");
+ if (HasAES)
+Features.push_back("+aes");
+} else if (HasNoCrypto) {
+ const bool HasSHA2 = (std::find(ItBegin, ItEnd, "+sha2") != ItEnd);
+ const bool HasAES = (std::find(ItBegin, ItEnd, "+aes") != ItEnd);
+ const bool HasV82a = (std::find(ItBegin, ItEnd, "+v8.2a") != ItEnd);
+ const bool HasV83a = (std::find(ItBegin, ItEnd, "+v8.3a") != ItEnd);
+ const bool HasV84a = (std::find(ItBegin, ItEnd, "+v8.4a") != ItEnd);
+ if (!HasSHA2)
+Features.push_back("-sha2");
+ if (!HasAES)
+Features.push_back("-aes");
+ if (HasV82a || HasV83a || HasV84a) {
+Features.push_back("-sm4");
+Features.push_back("-sha3");
+ }
+}
+ }
+
if (Arg *A = Args.getLastArg(options::OPT_mno_unaligned_access,
options::OPT_munaligned_access))
if (A->getOption().matches(options::OPT_mno_unaligned_access))
Index: cfe/trunk/lib/Driver/ToolChains/Arch/ARM.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Arch/ARM.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Arch/ARM.cpp
@@ -444,6 +444,26 @@
Features.push_back("-crc");
}
+ // For Arch >= ARMv8.0: crypto = sha2 + aes
+ // FIXME: this needs reimplementation after the TargetParser rewrite
+ if (ArchName.find_lower("armv8a") != StringRef::npos ||
+ ArchName.find_lower("armv8.1a") != StringRe
Re: r343285 - [cxx2a] P0641R2: (Some) type mismatches on defaulted functions only
On 04/10/2018 01:14, Richard Smith wrote: On Tue, 2 Oct 2018 at 01:18, Stephan Bergmann via cfe-commits mailto:cfe-commits@lists.llvm.org>> wrote: On 28/09/2018 03:16, Richard Smith via cfe-commits wrote: > Author: rsmith > Date: Thu Sep 27 18:16:43 2018 > New Revision: 343285 > > URL: http://llvm.org/viewvc/llvm-project?rev=343285&view=rev > Log: > [cxx2a] P0641R2: (Some) type mismatches on defaulted functions only > render the function deleted instead of rendering the program ill-formed. > > This change also adds an enabled-by-default warning for the case where > an explicitly-defaulted special member function of a non-template class > is implicitly deleted by the type checking rules. (This fires either due > to this language change or due to pre-C++20 reasons for the member being > implicitly deleted). I've tested this on a large codebase and found only > bugs (where the program means something that's clearly different from > what the programmer intended), so this is enabled by default, but we > should revisit this if there are problems with this being enabled by > default. Two questions on the new -Wdefaulted-function-deleted: 1 Do you have examples of actual bugs found by that? What do you mean by "actual bugs"? Every instance I've seen it flag is a case where the code means something other than what the programmer intended. (Eg, every case fixed by https://reviews.llvm.org/rL343369 is a bug in that sense.) You started to talk about "bugs" here :) What I had a hard time figuring out is what such a case of =default would look like where the programmer's intend obviously had been that the function wouldn't be implicitly deleted. However, it's rare that it'll flag something that leads to the runtime behavior of the program being different from the intent (usually you instead get a later compile error in cases where that would happen). It is possible, though -- in particular, it will flag cases where a move operation that you really wanted to result in a move actually results in a copy, and the program is silently accepted and is less efficient than you had expected. Thanks, that clarifies what you meant with "bugs". I'm running it on the LibreOffice code base now and it feels like lots of just noise. For example, I recently added explicitly defaulted decls of the four copy/move functions to lots of classes with a virtual dtor (where the dtor needs to be user-declared e.g. because it doesn't override any virtual base class dtor or because it shall not be defined inline), to silence GCC's new -Wdeprecated-copy. If such a class then for example has a const non-static data member, Clang's -Wdefaulted-function-deleted now kicks in for the assignment ops. I'm not sure whether it's better to have those functions explicitly deleted (which then needs revisiting when some details of the class change), or just explicitly defaulted and the compiler figure out whether its deleted or not (but which now causes Clang to bark), or simply not user-declared at all (but which now causes GCC to bark). When revising (say) a non-copyable class results in it becoming copyable, do you want that change to happen with no notice being given to the programmer? I at least don't think that it's obvious that you do. Having gone through all the warnings in the LibreOffice code base now, it wasn't that many individual warnings in the end (just blown up by being reported over and over again in the same include files), and the vast majority was involving classes that already exhibit pathological behavior anyway, like deriving from a base that has non-deleted copy ctor but deleted copy assignment op, due to somewhat broken overall design. (Plus some cases where members had recently been made const, implicitly deleting the assignment ops.) So I'm rather neutral now on whether that warning is on by default. I'm fine with turning this warning off by default (maybe moving it to -Wextra or similar) if there's a feeling that it's more noisy than it is useful. ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52726: [clangd] Support refs() in dex. Largely cloned from MemIndex.
hokein added inline comments. Comment at: clangd/index/dex/Dex.h:61 } // Symbols are owned by BackingData, Index takes ownership. + template nit: this comment is stale too. Comment at: unittests/clangd/DexTests.cpp:618 +Files.push_back(R.Location.FileURI); + }); +} The test seems doesn't verify any thing? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52726 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r343759 - [clangd] clangd-indexer: Drop support for MR-via-YAML
Author: sammccall
Date: Thu Oct 4 01:30:03 2018
New Revision: 343759
URL: http://llvm.org/viewvc/llvm-project?rev=343759&view=rev
Log:
[clangd] clangd-indexer: Drop support for MR-via-YAML
Summary:
It's slow, and the open-source reduce implementation doesn't scale properly.
While here, tidy up some dead headers and comments.
Reviewers: kadircet
Subscribers: ilya-biryukov, ioeric, MaskRay, jkorous, arphaman, cfe-commits
Differential Revision: https://reviews.llvm.org/D52517
Modified:
clang-tools-extra/trunk/clangd/index/Serialization.h
clang-tools-extra/trunk/clangd/index/YAMLSerialization.cpp
clang-tools-extra/trunk/clangd/indexer/IndexerMain.cpp
Modified: clang-tools-extra/trunk/clangd/index/Serialization.h
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Serialization.h?rev=343759&r1=343758&r2=343759&view=diff
==
--- clang-tools-extra/trunk/clangd/index/Serialization.h (original)
+++ clang-tools-extra/trunk/clangd/index/Serialization.h Thu Oct 4 01:30:03
2018
@@ -27,11 +27,6 @@
#include "Index.h"
#include "llvm/Support/Error.h"
-namespace llvm {
-namespace yaml {
-class Input;
-}
-} // namespace llvm
namespace clang {
namespace clangd {
@@ -61,10 +56,8 @@ struct IndexFileOut {
// Serializes an index file.
llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const IndexFileOut &O);
+// Convert a single symbol to YAML, a nice debug representation.
std::string toYAML(const Symbol &);
-// Returned symbol is backed by the YAML input.
-// FIXME: this is only needed for IndexerMain, find a better solution.
-llvm::Expected symbolFromYAML(llvm::yaml::Input &);
// Build an in-memory static index from an index file.
// The size should be relatively small, so data can be managed in memory.
Modified: clang-tools-extra/trunk/clangd/index/YAMLSerialization.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/YAMLSerialization.cpp?rev=343759&r1=343758&r2=343759&view=diff
==
--- clang-tools-extra/trunk/clangd/index/YAMLSerialization.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/YAMLSerialization.cpp Thu Oct 4
01:30:03 2018
@@ -218,13 +218,5 @@ std::string toYAML(const Symbol &S) {
return Buf;
}
-Expected symbolFromYAML(llvm::yaml::Input &Yin) {
- Symbol S;
- Yin >> S;
- if (Yin.error())
-return llvm::errorCodeToError(Yin.error());
- return S;
-}
-
} // namespace clangd
} // namespace clang
Modified: clang-tools-extra/trunk/clangd/indexer/IndexerMain.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/indexer/IndexerMain.cpp?rev=343759&r1=343758&r2=343759&view=diff
==
--- clang-tools-extra/trunk/clangd/indexer/IndexerMain.cpp (original)
+++ clang-tools-extra/trunk/clangd/indexer/IndexerMain.cpp Thu Oct 4 01:30:03
2018
@@ -12,26 +12,17 @@
//
//===--===//
-#include "RIFF.h"
-#include "index/CanonicalIncludes.h"
#include "index/Index.h"
#include "index/IndexAction.h"
#include "index/Merge.h"
#include "index/Serialization.h"
#include "index/SymbolCollector.h"
-#include "clang/Frontend/CompilerInstance.h"
-#include "clang/Frontend/FrontendActions.h"
-#include "clang/Index/IndexDataConsumer.h"
-#include "clang/Index/IndexingAction.h"
#include "clang/Tooling/CommonOptionsParser.h"
#include "clang/Tooling/Execution.h"
#include "clang/Tooling/Tooling.h"
#include "llvm/Support/CommandLine.h"
-#include "llvm/Support/MemoryBuffer.h"
#include "llvm/Support/Path.h"
#include "llvm/Support/Signals.h"
-#include "llvm/Support/ThreadPool.h"
-#include "llvm/Support/YAMLTraits.h"
using namespace llvm;
using namespace clang::tooling;
@@ -50,16 +41,6 @@ static llvm::cl::opt Assume
"not given, such headers will have relative paths."),
llvm::cl::init(""));
-static llvm::cl::opt MergeOnTheFly(
-"merge-on-the-fly",
-llvm::cl::desc(
-"Merges symbols for each processed translation unit as soon "
-"they become available. This results in a smaller memory "
-"usage and an almost instant reduce stage. Optimal for running as a "
-"standalone tool, but cannot be used with multi-process executors like
"
-"MapReduce."),
-llvm::cl::init(true), llvm::cl::Hidden);
-
static llvm::cl::opt
Format("format", llvm::cl::desc("Format of the index to be written"),
llvm::cl::values(clEnumValN(IndexFileFormat::YAML, "yaml",
@@ -68,89 +49,36 @@ static llvm::cl::opt
"binary RIFF format")),
llvm::cl::init(IndexFileFormat::YAML));
-/// Responsible for aggregating symbols from each processed file and producing
-/// the final results. All methods in this class must
[PATCH] D52517: [clangd] clangd-indexer: Drop support for MR-via-YAML
This revision was automatically updated to reflect the committed changes.
Closed by commit rL343759: [clangd] clangd-indexer: Drop support for
MR-via-YAML (authored by sammccall, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D52517
Files:
clang-tools-extra/trunk/clangd/index/Serialization.h
clang-tools-extra/trunk/clangd/index/YAMLSerialization.cpp
clang-tools-extra/trunk/clangd/indexer/IndexerMain.cpp
Index: clang-tools-extra/trunk/clangd/index/YAMLSerialization.cpp
===
--- clang-tools-extra/trunk/clangd/index/YAMLSerialization.cpp
+++ clang-tools-extra/trunk/clangd/index/YAMLSerialization.cpp
@@ -218,13 +218,5 @@
return Buf;
}
-Expected symbolFromYAML(llvm::yaml::Input &Yin) {
- Symbol S;
- Yin >> S;
- if (Yin.error())
-return llvm::errorCodeToError(Yin.error());
- return S;
-}
-
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/trunk/clangd/index/Serialization.h
===
--- clang-tools-extra/trunk/clangd/index/Serialization.h
+++ clang-tools-extra/trunk/clangd/index/Serialization.h
@@ -27,11 +27,6 @@
#include "Index.h"
#include "llvm/Support/Error.h"
-namespace llvm {
-namespace yaml {
-class Input;
-}
-} // namespace llvm
namespace clang {
namespace clangd {
@@ -61,10 +56,8 @@
// Serializes an index file.
llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const IndexFileOut &O);
+// Convert a single symbol to YAML, a nice debug representation.
std::string toYAML(const Symbol &);
-// Returned symbol is backed by the YAML input.
-// FIXME: this is only needed for IndexerMain, find a better solution.
-llvm::Expected symbolFromYAML(llvm::yaml::Input &);
// Build an in-memory static index from an index file.
// The size should be relatively small, so data can be managed in memory.
Index: clang-tools-extra/trunk/clangd/indexer/IndexerMain.cpp
===
--- clang-tools-extra/trunk/clangd/indexer/IndexerMain.cpp
+++ clang-tools-extra/trunk/clangd/indexer/IndexerMain.cpp
@@ -12,26 +12,17 @@
//
//===--===//
-#include "RIFF.h"
-#include "index/CanonicalIncludes.h"
#include "index/Index.h"
#include "index/IndexAction.h"
#include "index/Merge.h"
#include "index/Serialization.h"
#include "index/SymbolCollector.h"
-#include "clang/Frontend/CompilerInstance.h"
-#include "clang/Frontend/FrontendActions.h"
-#include "clang/Index/IndexDataConsumer.h"
-#include "clang/Index/IndexingAction.h"
#include "clang/Tooling/CommonOptionsParser.h"
#include "clang/Tooling/Execution.h"
#include "clang/Tooling/Tooling.h"
#include "llvm/Support/CommandLine.h"
-#include "llvm/Support/MemoryBuffer.h"
#include "llvm/Support/Path.h"
#include "llvm/Support/Signals.h"
-#include "llvm/Support/ThreadPool.h"
-#include "llvm/Support/YAMLTraits.h"
using namespace llvm;
using namespace clang::tooling;
@@ -50,107 +41,44 @@
"not given, such headers will have relative paths."),
llvm::cl::init(""));
-static llvm::cl::opt MergeOnTheFly(
-"merge-on-the-fly",
-llvm::cl::desc(
-"Merges symbols for each processed translation unit as soon "
-"they become available. This results in a smaller memory "
-"usage and an almost instant reduce stage. Optimal for running as a "
-"standalone tool, but cannot be used with multi-process executors like "
-"MapReduce."),
-llvm::cl::init(true), llvm::cl::Hidden);
-
static llvm::cl::opt
Format("format", llvm::cl::desc("Format of the index to be written"),
llvm::cl::values(clEnumValN(IndexFileFormat::YAML, "yaml",
"human-readable YAML format"),
clEnumValN(IndexFileFormat::RIFF, "binary",
"binary RIFF format")),
llvm::cl::init(IndexFileFormat::YAML));
-/// Responsible for aggregating symbols from each processed file and producing
-/// the final results. All methods in this class must be thread-safe,
-/// 'consumeSymbols' may be called from multiple threads.
-class SymbolsConsumer {
-public:
- virtual ~SymbolsConsumer() = default;
-
- /// Consume a SymbolSlab build for a file.
- virtual void consumeSymbols(SymbolSlab Symbols) = 0;
- /// Produce a resulting symbol slab, by combining occurrences of the same
- /// symbols across translation units.
- virtual SymbolSlab mergeResults() = 0;
-};
-
-class SymbolIndexActionFactory : public tooling::FrontendActionFactory {
+class IndexActionFactory : public tooling::FrontendActionFactory {
public:
- SymbolIndexActionFactory(SymbolsConsumer &Consumer) : Consumer(Consumer) {}
+ IndexActionFactory(IndexFileIn &Result) : Result(Result) {}
clang::FrontendAction *create() overr
[PATCH] D52782: [clang-tidy] Sequence statements with multiple parents correctly (PR39149)
mboehme updated this revision to Diff 168239.
mboehme added a comment.
- Responses to reviewer comments.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52782
Files:
clang-tidy/bugprone/UseAfterMoveCheck.cpp
clang-tidy/utils/ExprSequence.cpp
clang-tidy/utils/ExprSequence.h
test/clang-tidy/bugprone-use-after-move.cpp
Index: test/clang-tidy/bugprone-use-after-move.cpp
===
--- test/clang-tidy/bugprone-use-after-move.cpp
+++ test/clang-tidy/bugprone-use-after-move.cpp
@@ -1195,6 +1195,18 @@
}
}
+// Some statements in templates (e.g. null, break and continue statements) may
+// be shared between the uninstantiated and instantiated versions of the
+// template and therefore have multiple parents. Make sure the sequencing code
+// handles this correctly.
+template void nullStatementSequencesInTemplate() {
+ int c = 0;
+ (void)c;
+ ;
+ std::move(c);
+}
+template void nullStatementSequencesInTemplate();
+
namespace PR33020 {
class D {
~D();
Index: clang-tidy/utils/ExprSequence.h
===
--- clang-tidy/utils/ExprSequence.h
+++ clang-tidy/utils/ExprSequence.h
@@ -69,8 +69,8 @@
class ExprSequence {
public:
/// Initializes this `ExprSequence` with sequence information for the given
- /// `CFG`.
- ExprSequence(const CFG *TheCFG, ASTContext *TheContext);
+ /// `CFG`. `Root` is the root statement the CFG was built from.
+ ExprSequence(const CFG *TheCFG, const Stmt *Root, ASTContext *TheContext);
/// Returns whether \p Before is sequenced before \p After.
bool inSequence(const Stmt *Before, const Stmt *After) const;
@@ -94,6 +94,7 @@
const Stmt *resolveSyntheticStmt(const Stmt *S) const;
ASTContext *Context;
+ const Stmt *Root;
llvm::DenseMap SyntheticStmtSourceMap;
};
Index: clang-tidy/utils/ExprSequence.cpp
===
--- clang-tidy/utils/ExprSequence.cpp
+++ clang-tidy/utils/ExprSequence.cpp
@@ -63,8 +63,9 @@
}
}
-ExprSequence::ExprSequence(const CFG *TheCFG, ASTContext *TheContext)
-: Context(TheContext) {
+ExprSequence::ExprSequence(const CFG *TheCFG, const Stmt *Root,
+ ASTContext *TheContext)
+: Context(TheContext), Root(Root) {
for (const auto &SyntheticStmt : TheCFG->synthetic_stmts()) {
SyntheticStmtSourceMap[SyntheticStmt.first] = SyntheticStmt.second;
}
@@ -99,6 +100,11 @@
const Stmt *ExprSequence::getSequenceSuccessor(const Stmt *S) const {
for (const Stmt *Parent : getParentStmts(S, Context)) {
+// If a statement has multiple parents, make sure we're using the parent
+// that lies within the sub-tree under Root.
+if (!isDescendantOrEqual(Parent, Root, Context))
+ continue;
+
if (const auto *BO = dyn_cast(Parent)) {
// Comma operator: Right-hand side is sequenced after the left-hand side.
if (BO->getLHS() == S && BO->getOpcode() == BO_Comma)
Index: clang-tidy/bugprone/UseAfterMoveCheck.cpp
===
--- clang-tidy/bugprone/UseAfterMoveCheck.cpp
+++ clang-tidy/bugprone/UseAfterMoveCheck.cpp
@@ -102,8 +102,9 @@
if (!TheCFG)
return false;
- Sequence.reset(new ExprSequence(TheCFG.get(), Context));
- BlockMap.reset(new StmtToBlockMap(TheCFG.get(), Context));
+ Sequence =
+ llvm::make_unique(TheCFG.get(), FunctionBody, Context);
+ BlockMap = llvm::make_unique(TheCFG.get(), Context);
Visited.clear();
const CFGBlock *Block = BlockMap->blockContainingStmt(MovingCall);
Index: test/clang-tidy/bugprone-use-after-move.cpp
===
--- test/clang-tidy/bugprone-use-after-move.cpp
+++ test/clang-tidy/bugprone-use-after-move.cpp
@@ -1195,6 +1195,18 @@
}
}
+// Some statements in templates (e.g. null, break and continue statements) may
+// be shared between the uninstantiated and instantiated versions of the
+// template and therefore have multiple parents. Make sure the sequencing code
+// handles this correctly.
+template void nullStatementSequencesInTemplate() {
+ int c = 0;
+ (void)c;
+ ;
+ std::move(c);
+}
+template void nullStatementSequencesInTemplate();
+
namespace PR33020 {
class D {
~D();
Index: clang-tidy/utils/ExprSequence.h
===
--- clang-tidy/utils/ExprSequence.h
+++ clang-tidy/utils/ExprSequence.h
@@ -69,8 +69,8 @@
class ExprSequence {
public:
/// Initializes this `ExprSequence` with sequence information for the given
- /// `CFG`.
- ExprSequence(const CFG *TheCFG, ASTContext *TheContext);
+ /// `CFG`. `Root` is the root statement the CFG was built from.
+ ExprSequence(const CFG *TheCFG, const Stmt *Root, ASTContext *TheContext);
/// Returns whether \p Before is sequenced before \p After.
bool inSequence(const Stmt *Before, const Stmt
[PATCH] D52782: [clang-tidy] Sequence statements with multiple parents correctly (PR39149)
mboehme marked 3 inline comments as done.
mboehme added inline comments.
Comment at: clang-tidy/utils/ExprSequence.cpp:103
for (const Stmt *Parent : getParentStmts(S, Context)) {
+// For statements that have multiple parents, make sure we're using the
+// parent that lies within the sub-tree under Root.
aaron.ballman wrote:
> JonasToth wrote:
> > I find the first part of the comment unclear. Does this loop handle `for`
> > only?
> I think this means English "for" and not C `for`. Could rewrite to `If a
> statement has multiple parents, ` instead.
Yes, this is what I meant. Rephrased as Aaron suggested to remove the ambiguity.
Comment at: test/clang-tidy/bugprone-use-after-move.cpp:1198
+// Null statements (and some other statements) in templates may be shared
+// between the uninstantiated and instantiated versions of the template and
JonasToth wrote:
> Which other stmts? do they produce the same false positive?
I've added the two other examples I'm aware of (continue and break statements)
to the description. However, I haven't been able to use these to construct an
example that triggers the false positive.
In general, any statement that TemplateInstantiator leaves unchanged will have
multiple parents; I'm not sure which other statements this applies to. In my
experience, any statement that contains an expression will be rebuilt, but I
may be missing something here.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52782
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44609: [Clang-Format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style)
christophe-calmejane added a comment. Actually, without your change to HasMultipleNestedBlocks, I'm almost getting the expected result: Lambda body is correctly indented (and not by function name's length). The only thing not working (and it's not either way, with or without your change to HasMultipleNestedBlocks), is nested lambdas. The body is not properly indented (but it's not that bad). About the "noexcept" issue, it might not be related to your patch, I'm not sure. Although the "mutable" tag is working as expected. Repository: rC Clang https://reviews.llvm.org/D44609 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52726: [clangd] Support refs() in dex. Largely cloned from MemIndex.
sammccall marked 2 inline comments as done. sammccall added inline comments. Comment at: unittests/clangd/DexTests.cpp:618 +Files.push_back(R.Location.FileURI); + }); +} hokein wrote: > The test seems doesn't verify any thing? Oops, thanks! Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52726 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52695: [clang][Parse] Diagnose useless null statements (PR39111)
lebedev.ri updated this revision to Diff 168240.
lebedev.ri marked an inline comment as done.
lebedev.ri added a comment.
Thank you for taking a look!
- Reworded the diag as suggested a little
- Do consume consequtive semis as one
- Do still record each one of them in AST. I'm not sure if we want to do
that, but else existing tests break.
Repository:
rC Clang
https://reviews.llvm.org/D52695
Files:
docs/ReleaseNotes.rst
include/clang/Basic/DiagnosticGroups.td
include/clang/Basic/DiagnosticParseKinds.td
include/clang/Parse/Parser.h
lib/Parse/ParseExprCXX.cpp
lib/Parse/ParseStmt.cpp
test/Parser/extra-semi-resulting-in-nullstmt-in-init-statement.cpp
test/Parser/extra-semi-resulting-in-nullstmt.cpp
Index: test/Parser/extra-semi-resulting-in-nullstmt.cpp
===
--- /dev/null
+++ test/Parser/extra-semi-resulting-in-nullstmt.cpp
@@ -0,0 +1,81 @@
+// RUN: %clang_cc1 -fsyntax-only -Wextra-semi-stmt -verify %s
+// RUN: cp %s %t
+// RUN: %clang_cc1 -x c++ -Wextra-semi-stmt -fixit %t
+// RUN: %clang_cc1 -x c++ -Wextra-semi-stmt -Werror %t
+
+#define GOODMACRO(varname) int varname
+#define BETTERMACRO(varname) GOODMACRO(varname);
+#define NULLMACRO(varname)
+
+enum MyEnum {
+ E1,
+ E2
+};
+
+void test() {
+ ; // expected-warning {{statement has no effect, ';' with no preceding expression is a null statement}}
+ ;
+
+ // This removal of extra semi also consumes all the comments.
+ // clang-format: off
+ ;;;
+ // clang-format: on
+
+ // clang-format: off
+ ;NULLMACRO(ZZ);
+ // clang-format: on
+
+ {}; // expected-warning {{statement has no effect, ';' with no preceding expression is a null statement}}
+
+ {
+; // expected-warning {{statement has no effect, ';' with no preceding expression is a null statement}}
+ }
+
+ if (true) {
+; // expected-warning {{statement has no effect, ';' with no preceding expression is a null statement}}
+ }
+
+ GOODMACRO(v0); // OK
+
+ GOODMACRO(v1;) // OK
+
+ BETTERMACRO(v2) // OK
+
+ BETTERMACRO(v3;) // Extra ';', but within macro expansion, so ignored.
+
+ BETTERMACRO(v4); // expected-warning {{statement has no effect, ';' with no preceding expression is a null statement}}
+
+ BETTERMACRO(v5;); // expected-warning {{statement has no effect, ';' with no preceding expression is a null statement}}
+
+ NULLMACRO(v6) // OK
+
+ NULLMACRO(v7); // OK. This could be either GOODMACRO() or BETTERMACRO() situation, so we can't know we can drop it.
+
+ if (true)
+; // OK
+
+ while (true)
+; // OK
+
+ do
+; // OK
+ while (true);
+
+ for (;;) // OK
+; // OK
+
+ MyEnum my_enum;
+ switch (my_enum) {
+ case E1:
+// stuff
+break;
+ case E2:; // OK
+ }
+
+ for (;;) {
+for (;;) {
+ goto contin_outer;
+}
+ contin_outer:; // OK
+ }
+}
Index: test/Parser/extra-semi-resulting-in-nullstmt-in-init-statement.cpp
===
--- /dev/null
+++ test/Parser/extra-semi-resulting-in-nullstmt-in-init-statement.cpp
@@ -0,0 +1,64 @@
+// RUN: %clang_cc1 -fsyntax-only -Wextra -std=c++2a -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wextra-semi-stmt -std=c++2a -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wempty-init-stmt -std=c++2a -verify %s
+// RUN: cp %s %t
+// RUN: %clang_cc1 -x c++ -Wempty-init-stmt -std=c++2a -fixit %t
+// RUN: %clang_cc1 -x c++ -Wempty-init-stmt -std=c++2a -Werror %t
+
+struct S {
+ int *begin();
+ int *end();
+};
+
+void naive(int x) {
+ if (; true) // expected-warning {{init-statement of 'if' is a null statement}}
+;
+
+ switch (; x) { // expected-warning {{init-statement of 'switch' is a null statement}}
+ }
+
+ for (; int y : S()) // expected-warning {{init-statement of 'range-based for' is a null statement}}
+;
+
+ for (;;) // OK
+;
+}
+
+#define NULLMACRO
+
+void with_null_macro(int x) {
+ if (NULLMACRO; true)
+;
+
+ switch (NULLMACRO; x) {
+ }
+
+ for (NULLMACRO; int y : S())
+;
+}
+
+#define SEMIMACRO ;
+
+void with_semi_macro(int x) {
+ if (SEMIMACRO true)
+;
+
+ switch (SEMIMACRO x) {
+ }
+
+ for (SEMIMACRO int y : S())
+;
+}
+
+#define PASSTHROUGHMACRO(x) x
+
+void with_passthrough_macro(int x) {
+ if (PASSTHROUGHMACRO(;) true)
+;
+
+ switch (PASSTHROUGHMACRO(;) x) {
+ }
+
+ for (PASSTHROUGHMACRO(;) int y : S())
+;
+}
Index: lib/Parse/ParseStmt.cpp
===
--- lib/Parse/ParseStmt.cpp
+++ lib/Parse/ParseStmt.cpp
@@ -930,6 +930,34 @@
}
+/// Consume any extra semi-colons resulting in null statements,
+/// returning true if any tok::semi were consumed.
+bool Parser::ConsumeNullStmt(StmtVector &Stmts) {
+ if (!Tok.is(tok::semi))
+return false;
+
+ SourceLocation StartLoc = Tok.getLocation();
+ SourceLocation EndLoc;
+
+ while (Tok.is(tok::semi) && !Tok.hasLeadingEmptyMacro() &&
+ Tok.getLocation().isValid() && !Tok.getLocation().isMacroI
[PATCH] D52870: [NestedNameSpecifier] Add missing stream-specific dump methods
steveire created this revision.
steveire added a reviewer: aaron.ballman.
Herald added a subscriber: cfe-commits.
Repository:
rC Clang
https://reviews.llvm.org/D52870
Files:
include/clang/AST/NestedNameSpecifier.h
lib/AST/NestedNameSpecifier.cpp
Index: lib/AST/NestedNameSpecifier.cpp
===
--- lib/AST/NestedNameSpecifier.cpp
+++ lib/AST/NestedNameSpecifier.cpp
@@ -340,12 +340,22 @@
}
void NestedNameSpecifier::dump(const LangOptions &LO) const {
- print(llvm::errs(), PrintingPolicy(LO));
+ dump(llvm::errs(), LO);
}
-LLVM_DUMP_METHOD void NestedNameSpecifier::dump() const {
+void NestedNameSpecifier::dump() const {
+ dump(llvm::errs());
+}
+
+void NestedNameSpecifier::dump(llvm::raw_ostream& OS) const
+{
LangOptions LO;
- print(llvm::errs(), PrintingPolicy(LO));
+ dump(OS, LO);
+}
+
+LLVM_DUMP_METHOD void NestedNameSpecifier::dump(llvm::raw_ostream& OS, const
LangOptions &LO) const
+{
+ print(OS, PrintingPolicy(LO));
}
unsigned
Index: include/clang/AST/NestedNameSpecifier.h
===
--- include/clang/AST/NestedNameSpecifier.h
+++ include/clang/AST/NestedNameSpecifier.h
@@ -225,6 +225,8 @@
/// in debugging.
void dump(const LangOptions &LO) const;
void dump() const;
+ void dump(llvm::raw_ostream& OS) const;
+ void dump(llvm::raw_ostream& OS, const LangOptions &LO) const;
};
/// A C++ nested-name-specifier augmented with source location
Index: lib/AST/NestedNameSpecifier.cpp
===
--- lib/AST/NestedNameSpecifier.cpp
+++ lib/AST/NestedNameSpecifier.cpp
@@ -340,12 +340,22 @@
}
void NestedNameSpecifier::dump(const LangOptions &LO) const {
- print(llvm::errs(), PrintingPolicy(LO));
+ dump(llvm::errs(), LO);
}
-LLVM_DUMP_METHOD void NestedNameSpecifier::dump() const {
+void NestedNameSpecifier::dump() const {
+ dump(llvm::errs());
+}
+
+void NestedNameSpecifier::dump(llvm::raw_ostream& OS) const
+{
LangOptions LO;
- print(llvm::errs(), PrintingPolicy(LO));
+ dump(OS, LO);
+}
+
+LLVM_DUMP_METHOD void NestedNameSpecifier::dump(llvm::raw_ostream& OS, const LangOptions &LO) const
+{
+ print(OS, PrintingPolicy(LO));
}
unsigned
Index: include/clang/AST/NestedNameSpecifier.h
===
--- include/clang/AST/NestedNameSpecifier.h
+++ include/clang/AST/NestedNameSpecifier.h
@@ -225,6 +225,8 @@
/// in debugging.
void dump(const LangOptions &LO) const;
void dump() const;
+ void dump(llvm::raw_ostream& OS) const;
+ void dump(llvm::raw_ostream& OS, const LangOptions &LO) const;
};
/// A C++ nested-name-specifier augmented with source location
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52726: [clangd] Support refs() in dex. Largely cloned from MemIndex.
sammccall updated this revision to Diff 168242.
sammccall marked an inline comment as done.
sammccall added a comment.
Fix test and comment, and sync
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52726
Files:
clangd/index/Serialization.cpp
clangd/index/dex/Dex.cpp
clangd/index/dex/Dex.h
clangd/indexer/IndexerMain.cpp
unittests/clangd/DexTests.cpp
Index: unittests/clangd/DexTests.cpp
===
--- unittests/clangd/DexTests.cpp
+++ unittests/clangd/DexTests.cpp
@@ -423,7 +423,8 @@
//===--===//
TEST(Dex, Lookup) {
- auto I = Dex::build(generateSymbols({"ns::abc", "ns::xyz"}), URISchemes);
+ auto I = Dex::build(generateSymbols({"ns::abc", "ns::xyz"}), RefSlab(),
+ URISchemes);
EXPECT_THAT(lookup(*I, SymbolID("ns::abc")), UnorderedElementsAre("ns::abc"));
EXPECT_THAT(lookup(*I, {SymbolID("ns::abc"), SymbolID("ns::xyz")}),
UnorderedElementsAre("ns::abc", "ns::xyz"));
@@ -436,7 +437,7 @@
auto Index =
Dex::build(generateSymbols({"ns::ABC", "ns::BCD", "::ABC",
"ns::nested::ABC", "other::ABC", "other::A"}),
- URISchemes);
+ RefSlab(), URISchemes);
FuzzyFindRequest Req;
Req.Query = "ABC";
Req.Scopes = {"ns::"};
@@ -466,13 +467,13 @@
symbol("2") /* duplicate */};
FuzzyFindRequest Req;
Req.Query = "2";
- Dex I(Symbols, URISchemes);
+ Dex I(Symbols, RefSlab(), URISchemes);
EXPECT_FALSE(Req.Limit);
EXPECT_THAT(match(I, Req), ElementsAre("2", "2"));
}
TEST(DexTest, DexLimitedNumMatches) {
- auto I = Dex::build(generateNumSymbols(0, 100), URISchemes);
+ auto I = Dex::build(generateNumSymbols(0, 100), RefSlab(), URISchemes);
FuzzyFindRequest Req;
Req.Query = "5";
Req.Limit = 3;
@@ -486,58 +487,61 @@
TEST(DexTest, FuzzyMatch) {
auto I = Dex::build(
generateSymbols({"LaughingOutLoud", "LionPopulation", "LittleOldLady"}),
- URISchemes);
+ RefSlab(), URISchemes);
FuzzyFindRequest Req;
Req.Query = "lol";
Req.Limit = 2;
EXPECT_THAT(match(*I, Req),
UnorderedElementsAre("LaughingOutLoud", "LittleOldLady"));
}
TEST(DexTest, MatchQualifiedNamesWithoutSpecificScope) {
- auto I = Dex::build(generateSymbols({"a::y1", "b::y2", "y3"}), URISchemes);
+ auto I = Dex::build(generateSymbols({"a::y1", "b::y2", "y3"}), RefSlab(),
+ URISchemes);
FuzzyFindRequest Req;
Req.Query = "y";
EXPECT_THAT(match(*I, Req), UnorderedElementsAre("a::y1", "b::y2", "y3"));
}
TEST(DexTest, MatchQualifiedNamesWithGlobalScope) {
- auto I = Dex::build(generateSymbols({"a::y1", "b::y2", "y3"}), URISchemes);
+ auto I = Dex::build(generateSymbols({"a::y1", "b::y2", "y3"}), RefSlab(),
+ URISchemes);
FuzzyFindRequest Req;
Req.Query = "y";
Req.Scopes = {""};
EXPECT_THAT(match(*I, Req), UnorderedElementsAre("y3"));
}
TEST(DexTest, MatchQualifiedNamesWithOneScope) {
- auto I = Dex::build(
- generateSymbols({"a::y1", "a::y2", "a::x", "b::y2", "y3"}), URISchemes);
+ auto I =
+ Dex::build(generateSymbols({"a::y1", "a::y2", "a::x", "b::y2", "y3"}),
+ RefSlab(), URISchemes);
FuzzyFindRequest Req;
Req.Query = "y";
Req.Scopes = {"a::"};
EXPECT_THAT(match(*I, Req), UnorderedElementsAre("a::y1", "a::y2"));
}
TEST(DexTest, MatchQualifiedNamesWithMultipleScopes) {
auto I = Dex::build(
- generateSymbols({"a::y1", "a::y2", "a::x", "b::y3", "y3"}), URISchemes);
+ generateSymbols({"a::y1", "a::y2", "a::x", "b::y3", "y3"}), RefSlab(), URISchemes);
FuzzyFindRequest Req;
Req.Query = "y";
Req.Scopes = {"a::", "b::"};
EXPECT_THAT(match(*I, Req), UnorderedElementsAre("a::y1", "a::y2", "b::y3"));
}
TEST(DexTest, NoMatchNestedScopes) {
- auto I = Dex::build(generateSymbols({"a::y1", "a::b::y2"}), URISchemes);
+ auto I = Dex::build(generateSymbols({"a::y1", "a::b::y2"}), RefSlab(), URISchemes);
FuzzyFindRequest Req;
Req.Query = "y";
Req.Scopes = {"a::"};
EXPECT_THAT(match(*I, Req), UnorderedElementsAre("a::y1"));
}
TEST(DexTest, WildcardScope) {
auto I =
- Dex::build(generateSymbols({"a::y1", "a::b::y2", "c::y3"}), URISchemes);
+ Dex::build(generateSymbols({"a::y1", "a::b::y2", "c::y3"}), RefSlab(), URISchemes);
FuzzyFindRequest Req;
Req.Query = "y";
Req.Scopes = {"a::"};
@@ -547,15 +551,15 @@
}
TEST(DexTest, IgnoreCases) {
- auto I = Dex::build(generateSymbols({"ns::ABC", "ns::abc"}), URISchemes);
+ auto I = Dex::build(generateSymbols({"ns::ABC", "ns::abc"}), RefSlab(), URISchemes);
FuzzyFindRequest Req;
Req.Query = "AB";
Req.Scopes = {"ns::"};
EXPECT_THAT(match(*I, Req), UnorderedElementsAre("ns::ABC", "ns::abc"));
}
TEST(DexTest, Lookup) {
- auto I = Dex::build(generateSymbols({"ns::abc"
[PATCH] D52531: [clangd] clangd-indexer gathers refs and stores them in index files.
hokein added a comment. Looks great! I played around this patch a bit on LLVM. The output size is 50MB (binary)/1.2G (YAML). The ref output is not complete (missing all refs in headers, since we only collect refs from main file), I think we will add an option to `SymbolCollector` to allow collecting refs in header (but this option is only used for building static index)? Comment at: clangd/index/Serialization.cpp:301 +// A refs section has data grouped by Symbol. Each symbol has: +// - SymbolID: 20 bytes +// - NumRefs: varint Looks like we can improve the size further, we are storing SymbolID twice (one in `symb` section), I think we could optimize it by introducing a `symi` section or using `stri` section? Comment at: clangd/index/Serialization.cpp:333 // - stri: string table // - symb: symbols We are adding a new section in the RIFF, the comment needs update. Comment at: clangd/indexer/IndexerMain.cpp:81 + virtual void consumeRefs(RefSlab Refs) = 0; /// Produce a resulting symbol slab, by combining occurrences of the same /// symbols across translation units. nit: this comment needs update. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52531 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52726: [clangd] Support refs() in dex. Largely cloned from MemIndex.
hokein accepted this revision. hokein added a comment. This revision is now accepted and ready to land. LGTM Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52726 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r343760 - [clangd] Support refs() in dex. Largely cloned from MemIndex.
Author: sammccall
Date: Thu Oct 4 02:16:12 2018
New Revision: 343760
URL: http://llvm.org/viewvc/llvm-project?rev=343760&view=rev
Log:
[clangd] Support refs() in dex. Largely cloned from MemIndex.
Reviewers: hokein
Subscribers: ilya-biryukov, ioeric, MaskRay, jkorous, arphaman, kadircet,
cfe-commits
Differential Revision: https://reviews.llvm.org/D52726
Modified:
clang-tools-extra/trunk/clangd/index/Serialization.cpp
clang-tools-extra/trunk/clangd/index/dex/Dex.cpp
clang-tools-extra/trunk/clangd/index/dex/Dex.h
clang-tools-extra/trunk/clangd/indexer/IndexerMain.cpp
clang-tools-extra/trunk/unittests/clangd/DexTests.cpp
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=343760&r1=343759&r2=343760&view=diff
==
--- clang-tools-extra/trunk/clangd/index/Serialization.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/Serialization.cpp Thu Oct 4 02:16:12
2018
@@ -435,8 +435,9 @@ std::unique_ptr loadIndex(l
}
trace::Span Tracer("BuildIndex");
- auto Index = UseDex ? dex::Dex::build(std::move(Symbols), URISchemes)
- : MemIndex::build(std::move(Symbols), std::move(Refs));
+ auto Index =
+ UseDex ? dex::Dex::build(std::move(Symbols), std::move(Refs), URISchemes)
+ : MemIndex::build(std::move(Symbols), std::move(Refs));
vlog("Loaded {0} from {1} with estimated memory usage {2}",
UseDex ? "Dex" : "MemIndex", SymbolFilename,
Index->estimateMemoryUsage());
Modified: clang-tools-extra/trunk/clangd/index/dex/Dex.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/dex/Dex.cpp?rev=343760&r1=343759&r2=343760&view=diff
==
--- clang-tools-extra/trunk/clangd/index/dex/Dex.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/dex/Dex.cpp Thu Oct 4 02:16:12 2018
@@ -24,6 +24,15 @@ namespace clang {
namespace clangd {
namespace dex {
+std::unique_ptr
+Dex::build(SymbolSlab Symbols, RefSlab Refs,
+ llvm::ArrayRef URISchemes) {
+ auto Size = Symbols.bytes() + Refs.bytes();
+ auto Data = std::make_pair(std::move(Symbols), std::move(Refs));
+ return llvm::make_unique(Data.first, Data.second, std::move(Data), Size,
+std::move(URISchemes));
+}
+
namespace {
// Mark symbols which are can be used for code completion.
@@ -254,7 +263,10 @@ void Dex::lookup(const LookupRequest &Re
void Dex::refs(const RefsRequest &Req,
llvm::function_ref Callback) const {
trace::Span Tracer("Dex refs");
- log("refs is not implemented.");
+ for (const auto &ID : Req.IDs)
+for (const auto &Ref : Refs.lookup(ID))
+ if (static_cast(Req.Filter & Ref.Kind))
+Callback(Ref);
}
size_t Dex::estimateMemoryUsage() const {
@@ -264,6 +276,7 @@ size_t Dex::estimateMemoryUsage() const
Bytes += InvertedIndex.getMemorySize();
for (const auto &TokenToPostingList : InvertedIndex)
Bytes += TokenToPostingList.second.bytes();
+ Bytes += Refs.getMemorySize();
return Bytes + BackingDataSize;
}
Modified: clang-tools-extra/trunk/clangd/index/dex/Dex.h
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/dex/Dex.h?rev=343760&r1=343759&r2=343760&view=diff
==
--- clang-tools-extra/trunk/clangd/index/dex/Dex.h (original)
+++ clang-tools-extra/trunk/clangd/index/dex/Dex.h Thu Oct 4 02:16:12 2018
@@ -41,9 +41,10 @@ namespace dex {
// index on disk and then load it if available.
class Dex : public SymbolIndex {
public:
- // All symbols must outlive this index.
- template
- Dex(Range &&Symbols, llvm::ArrayRef Schemes)
+ // All data must outlive this index.
+ template
+ Dex(SymbolRange &&Symbols, RefsRange &&Refs,
+ llvm::ArrayRef Schemes)
: Corpus(0), URISchemes(Schemes) {
// If Schemes don't contain any items, fall back to SymbolCollector's
// default URI schemes.
@@ -53,26 +54,24 @@ public:
}
for (auto &&Sym : Symbols)
this->Symbols.push_back(&Sym);
+for (auto &&Ref : Refs)
+ this->Refs.try_emplace(Ref.first, Ref.second);
buildIndex();
}
- // Symbols are owned by BackingData, Index takes ownership.
- template
- Dex(Range &&Symbols, Payload &&BackingData, size_t BackingDataSize,
- llvm::ArrayRef URISchemes)
- : Dex(std::forward(Symbols), URISchemes) {
+ // Symbols and Refs are owned by BackingData, Index takes ownership.
+ template
+ Dex(SymbolRange &&Symbols, RefsRange &&Refs, Payload &&BackingData,
+ size_t BackingDataSize, llvm::ArrayRef URISchemes)
+ : Dex(std::forward(Symbols), std::forward(Refs),
+URISchemes) {
KeepAlive = std::shared_ptr(
std::make_shared(s
[PATCH] D52726: [clangd] Support refs() in dex. Largely cloned from MemIndex.
This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE343760: [clangd] Support refs() in dex. Largely cloned
from MemIndex. (authored by sammccall, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D52726?vs=168242&id=168243#toc
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52726
Files:
clangd/index/Serialization.cpp
clangd/index/dex/Dex.cpp
clangd/index/dex/Dex.h
clangd/indexer/IndexerMain.cpp
unittests/clangd/DexTests.cpp
Index: clangd/index/Serialization.cpp
===
--- clangd/index/Serialization.cpp
+++ clangd/index/Serialization.cpp
@@ -435,8 +435,9 @@
}
trace::Span Tracer("BuildIndex");
- auto Index = UseDex ? dex::Dex::build(std::move(Symbols), URISchemes)
- : MemIndex::build(std::move(Symbols), std::move(Refs));
+ auto Index =
+ UseDex ? dex::Dex::build(std::move(Symbols), std::move(Refs), URISchemes)
+ : MemIndex::build(std::move(Symbols), std::move(Refs));
vlog("Loaded {0} from {1} with estimated memory usage {2}",
UseDex ? "Dex" : "MemIndex", SymbolFilename,
Index->estimateMemoryUsage());
Index: clangd/index/dex/Dex.cpp
===
--- clangd/index/dex/Dex.cpp
+++ clangd/index/dex/Dex.cpp
@@ -24,6 +24,15 @@
namespace clangd {
namespace dex {
+std::unique_ptr
+Dex::build(SymbolSlab Symbols, RefSlab Refs,
+ llvm::ArrayRef URISchemes) {
+ auto Size = Symbols.bytes() + Refs.bytes();
+ auto Data = std::make_pair(std::move(Symbols), std::move(Refs));
+ return llvm::make_unique(Data.first, Data.second, std::move(Data), Size,
+std::move(URISchemes));
+}
+
namespace {
// Mark symbols which are can be used for code completion.
@@ -254,7 +263,10 @@
void Dex::refs(const RefsRequest &Req,
llvm::function_ref Callback) const {
trace::Span Tracer("Dex refs");
- log("refs is not implemented.");
+ for (const auto &ID : Req.IDs)
+for (const auto &Ref : Refs.lookup(ID))
+ if (static_cast(Req.Filter & Ref.Kind))
+Callback(Ref);
}
size_t Dex::estimateMemoryUsage() const {
@@ -264,6 +276,7 @@
Bytes += InvertedIndex.getMemorySize();
for (const auto &TokenToPostingList : InvertedIndex)
Bytes += TokenToPostingList.second.bytes();
+ Bytes += Refs.getMemorySize();
return Bytes + BackingDataSize;
}
Index: clangd/index/dex/Dex.h
===
--- clangd/index/dex/Dex.h
+++ clangd/index/dex/Dex.h
@@ -41,9 +41,10 @@
// index on disk and then load it if available.
class Dex : public SymbolIndex {
public:
- // All symbols must outlive this index.
- template
- Dex(Range &&Symbols, llvm::ArrayRef Schemes)
+ // All data must outlive this index.
+ template
+ Dex(SymbolRange &&Symbols, RefsRange &&Refs,
+ llvm::ArrayRef Schemes)
: Corpus(0), URISchemes(Schemes) {
// If Schemes don't contain any items, fall back to SymbolCollector's
// default URI schemes.
@@ -53,26 +54,24 @@
}
for (auto &&Sym : Symbols)
this->Symbols.push_back(&Sym);
+for (auto &&Ref : Refs)
+ this->Refs.try_emplace(Ref.first, Ref.second);
buildIndex();
}
- // Symbols are owned by BackingData, Index takes ownership.
- template
- Dex(Range &&Symbols, Payload &&BackingData, size_t BackingDataSize,
- llvm::ArrayRef URISchemes)
- : Dex(std::forward(Symbols), URISchemes) {
+ // Symbols and Refs are owned by BackingData, Index takes ownership.
+ template
+ Dex(SymbolRange &&Symbols, RefsRange &&Refs, Payload &&BackingData,
+ size_t BackingDataSize, llvm::ArrayRef URISchemes)
+ : Dex(std::forward(Symbols), std::forward(Refs),
+URISchemes) {
KeepAlive = std::shared_ptr(
std::make_shared(std::move(BackingData)), nullptr);
this->BackingDataSize = BackingDataSize;
}
- /// Builds an index from a slab. The index takes ownership of the slab.
+ /// Builds an index from slabs. The index takes ownership of the slab.
static std::unique_ptr
- build(SymbolSlab Slab, llvm::ArrayRef URISchemes) {
-// Store Slab size before it is moved.
-const auto BackingDataSize = Slab.bytes();
-return llvm::make_unique(Slab, std::move(Slab), BackingDataSize,
- URISchemes);
- }
+ build(SymbolSlab, RefSlab, llvm::ArrayRef URISchemes);
bool
fuzzyFind(const FuzzyFindRequest &Req,
@@ -102,6 +101,7 @@
/// during the fuzzyFind process.
llvm::DenseMap InvertedIndex;
dex::Corpus Corpus;
+ llvm::DenseMap> Refs;
std::shared_ptr KeepAlive; // poor man's move-only std::any
// Size of memory retained by KeepAlive.
size_t BackingDataSize = 0;
Index: clangd/indexer/IndexerMain.cpp
===
--- clangd/indexer/IndexerMain
[PATCH] D51855: [constexpr] Fix ICE when memcpy() is given a pointer to an incomplete array
petpav01 added a comment. No worries, thanks. https://reviews.llvm.org/D51855 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51855: [constexpr] Fix ICE when memcpy() is given a pointer to an incomplete array
This revision was automatically updated to reflect the committed changes.
Closed by commit rL343761: [constexpr] Fix ICE when memcpy() is given a pointer
to an incomplete array (authored by petr.pavlu, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D51855?vs=166073&id=168244#toc
Repository:
rL LLVM
https://reviews.llvm.org/D51855
Files:
cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td
cfe/trunk/lib/AST/ExprConstant.cpp
cfe/trunk/test/CodeGen/builtin-memfns.c
cfe/trunk/test/SemaCXX/constexpr-string.cpp
Index: cfe/trunk/lib/AST/ExprConstant.cpp
===
--- cfe/trunk/lib/AST/ExprConstant.cpp
+++ cfe/trunk/lib/AST/ExprConstant.cpp
@@ -6239,6 +6239,10 @@
Info.FFDiag(E, diag::note_constexpr_memcpy_type_pun) << Move << SrcT <<
T;
return false;
}
+if (T->isIncompleteType()) {
+ Info.FFDiag(E, diag::note_constexpr_memcpy_incomplete_type) << Move << T;
+ return false;
+}
if (!T.isTriviallyCopyableType(Info.Ctx)) {
Info.FFDiag(E, diag::note_constexpr_memcpy_nontrivial) << Move << T;
return false;
Index: cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td
===
--- cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td
+++ cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td
@@ -173,6 +173,9 @@
def note_constexpr_memcpy_nontrivial : Note<
"cannot constant evaluate '%select{memcpy|memmove}0' between objects of "
"non-trivially-copyable type %1">;
+def note_constexpr_memcpy_incomplete_type : Note<
+ "cannot constant evaluate '%select{memcpy|memmove}0' between objects of "
+ "incomplete type %1">;
def note_constexpr_memcpy_overlap : Note<
"'%select{memcpy|wmemcpy}0' between overlapping memory regions">;
def note_constexpr_memcpy_unsupported : Note<
Index: cfe/trunk/test/SemaCXX/constexpr-string.cpp
===
--- cfe/trunk/test/SemaCXX/constexpr-string.cpp
+++ cfe/trunk/test/SemaCXX/constexpr-string.cpp
@@ -387,4 +387,41 @@
// designators until we have a long enough matching size, if both designators
// point to the start of their respective final elements.
static_assert(test_derived_to_base(2) == 3434); // expected-error
{{constant}} expected-note {{in call}}
+
+ // Check that when address-of an array is passed to a tested function the
+ // array can be fully copied.
+ constexpr int test_address_of_const_array_type() {
+int arr[4] = {1, 2, 3, 4};
+__builtin_memmove(&arr, &arr, sizeof(arr));
+return arr[0] * 1000 + arr[1] * 100 + arr[2] * 10 + arr[3];
+ }
+ static_assert(test_address_of_const_array_type() == 1234);
+
+ // Check that an incomplete array is rejected.
+ constexpr int test_incomplete_array_type() { // expected-error {{never
produces a constant}}
+extern int arr[];
+__builtin_memmove(arr, arr, 4 * sizeof(arr[0]));
+// expected-note@-1 2{{'memmove' not supported: source is not a contiguous
array of at least 4 elements of type 'int'}}
+return arr[0] * 1000 + arr[1] * 100 + arr[2] * 10 + arr[3];
+ }
+ static_assert(test_incomplete_array_type() == 1234); // expected-error
{{constant}} expected-note {{in call}}
+
+ // Check that a pointer to an incomplete array is rejected.
+ constexpr int test_address_of_incomplete_array_type() { // expected-error
{{never produces a constant}}
+extern int arr[];
+__builtin_memmove(&arr, &arr, 4 * sizeof(arr[0]));
+// expected-note@-1 2{{cannot constant evaluate 'memmove' between objects
of incomplete type 'int []'}}
+return arr[0] * 1000 + arr[1] * 100 + arr[2] * 10 + arr[3];
+ }
+ static_assert(test_address_of_incomplete_array_type() == 1234); //
expected-error {{constant}} expected-note {{in call}}
+
+ // Check that a pointer to an incomplete struct is rejected.
+ constexpr bool test_address_of_incomplete_struct_type() { // expected-error
{{never produces a constant}}
+struct Incomplete;
+extern Incomplete x, y;
+__builtin_memcpy(&x, &x, 4);
+// expected-note@-1 2{{cannot constant evaluate 'memcpy' between objects
of incomplete type 'Incomplete'}}
+return true;
+ }
+ static_assert(test_address_of_incomplete_struct_type()); // expected-error
{{constant}} expected-note {{in call}}
}
Index: cfe/trunk/test/CodeGen/builtin-memfns.c
===
--- cfe/trunk/test/CodeGen/builtin-memfns.c
+++ cfe/trunk/test/CodeGen/builtin-memfns.c
@@ -111,3 +111,10 @@
memcpy(&d, (char *)&e.a, sizeof(e));
}
+// CHECK-LABEL: @test12
+extern char dest_array[];
+extern char src_array[];
+void test12() {
+ // CHECK: call void @llvm.memcpy{{.*}}(
+ memcpy(&dest_array, &dest_array, 2);
+}
Index: cfe/trunk/lib/AST/ExprConstant.cpp
===
--- cfe/trunk/lib/AST/Exp
r343761 - [constexpr] Fix ICE when memcpy() is given a pointer to an incomplete array
Author: petr.pavlu
Date: Thu Oct 4 02:25:44 2018
New Revision: 343761
URL: http://llvm.org/viewvc/llvm-project?rev=343761&view=rev
Log:
[constexpr] Fix ICE when memcpy() is given a pointer to an incomplete array
Fix code for constant evaluation of __builtin_memcpy() and
__builtin_memmove() that would attempt to divide by zero when given two
pointers to an incomplete array.
Differential Revision: https://reviews.llvm.org/D51855
Modified:
cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td
cfe/trunk/lib/AST/ExprConstant.cpp
cfe/trunk/test/CodeGen/builtin-memfns.c
cfe/trunk/test/SemaCXX/constexpr-string.cpp
Modified: cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td?rev=343761&r1=343760&r2=343761&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td Thu Oct 4 02:25:44 2018
@@ -173,6 +173,9 @@ def note_constexpr_memcpy_type_pun : Not
def note_constexpr_memcpy_nontrivial : Note<
"cannot constant evaluate '%select{memcpy|memmove}0' between objects of "
"non-trivially-copyable type %1">;
+def note_constexpr_memcpy_incomplete_type : Note<
+ "cannot constant evaluate '%select{memcpy|memmove}0' between objects of "
+ "incomplete type %1">;
def note_constexpr_memcpy_overlap : Note<
"'%select{memcpy|wmemcpy}0' between overlapping memory regions">;
def note_constexpr_memcpy_unsupported : Note<
Modified: cfe/trunk/lib/AST/ExprConstant.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=343761&r1=343760&r2=343761&view=diff
==
--- cfe/trunk/lib/AST/ExprConstant.cpp (original)
+++ cfe/trunk/lib/AST/ExprConstant.cpp Thu Oct 4 02:25:44 2018
@@ -6239,6 +6239,10 @@ bool PointerExprEvaluator::VisitBuiltinC
Info.FFDiag(E, diag::note_constexpr_memcpy_type_pun) << Move << SrcT <<
T;
return false;
}
+if (T->isIncompleteType()) {
+ Info.FFDiag(E, diag::note_constexpr_memcpy_incomplete_type) << Move << T;
+ return false;
+}
if (!T.isTriviallyCopyableType(Info.Ctx)) {
Info.FFDiag(E, diag::note_constexpr_memcpy_nontrivial) << Move << T;
return false;
Modified: cfe/trunk/test/CodeGen/builtin-memfns.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/builtin-memfns.c?rev=343761&r1=343760&r2=343761&view=diff
==
--- cfe/trunk/test/CodeGen/builtin-memfns.c (original)
+++ cfe/trunk/test/CodeGen/builtin-memfns.c Thu Oct 4 02:25:44 2018
@@ -111,3 +111,10 @@ void test11() {
memcpy(&d, (char *)&e.a, sizeof(e));
}
+// CHECK-LABEL: @test12
+extern char dest_array[];
+extern char src_array[];
+void test12() {
+ // CHECK: call void @llvm.memcpy{{.*}}(
+ memcpy(&dest_array, &dest_array, 2);
+}
Modified: cfe/trunk/test/SemaCXX/constexpr-string.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/constexpr-string.cpp?rev=343761&r1=343760&r2=343761&view=diff
==
--- cfe/trunk/test/SemaCXX/constexpr-string.cpp (original)
+++ cfe/trunk/test/SemaCXX/constexpr-string.cpp Thu Oct 4 02:25:44 2018
@@ -387,4 +387,41 @@ namespace MemcpyEtc {
// designators until we have a long enough matching size, if both designators
// point to the start of their respective final elements.
static_assert(test_derived_to_base(2) == 3434); // expected-error
{{constant}} expected-note {{in call}}
+
+ // Check that when address-of an array is passed to a tested function the
+ // array can be fully copied.
+ constexpr int test_address_of_const_array_type() {
+int arr[4] = {1, 2, 3, 4};
+__builtin_memmove(&arr, &arr, sizeof(arr));
+return arr[0] * 1000 + arr[1] * 100 + arr[2] * 10 + arr[3];
+ }
+ static_assert(test_address_of_const_array_type() == 1234);
+
+ // Check that an incomplete array is rejected.
+ constexpr int test_incomplete_array_type() { // expected-error {{never
produces a constant}}
+extern int arr[];
+__builtin_memmove(arr, arr, 4 * sizeof(arr[0]));
+// expected-note@-1 2{{'memmove' not supported: source is not a contiguous
array of at least 4 elements of type 'int'}}
+return arr[0] * 1000 + arr[1] * 100 + arr[2] * 10 + arr[3];
+ }
+ static_assert(test_incomplete_array_type() == 1234); // expected-error
{{constant}} expected-note {{in call}}
+
+ // Check that a pointer to an incomplete array is rejected.
+ constexpr int test_address_of_incomplete_array_type() { // expected-error
{{never produces a constant}}
+extern int arr[];
+__builtin_memmove(&arr, &arr, 4 * sizeof(arr[0]));
+// expected-note@-1 2{{cannot constant evaluate 'memmove' between objects
of incomple
[PATCH] D52531: [clangd] clangd-indexer gathers refs and stores them in index files.
sammccall marked 2 inline comments as done. sammccall added a comment. In https://reviews.llvm.org/D52531#1254828, @hokein wrote: > The ref output is not complete (missing all refs in headers, since we only > collect refs from main file), I think we will add an option to > `SymbolCollector` to allow collecting refs in header (but this option is only > used for building static index)? We indeed need to include them somehow. Added a fixme to IndexAction, because I'm not totally sure at what level we should address this. Comment at: clangd/index/Serialization.cpp:301 +// A refs section has data grouped by Symbol. Each symbol has: +// - SymbolID: 20 bytes +// - NumRefs: varint hokein wrote: > Looks like we can improve the size further, we are storing SymbolID twice > (one in `symb` section), I think we could optimize it by introducing a > `symi` section or using `stri` section? Yes, there's a TODO on line 250. I opted not to do so in this patch, as it's a more invasive change and I wasn't sure about the exact representation. It should be straightforward to add later and bump the version. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52531 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52531: [clangd] clangd-indexer gathers refs and stores them in index files.
sammccall updated this revision to Diff 168245.
sammccall added a comment.
Address review comments.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52531
Files:
clangd/index/IndexAction.cpp
clangd/index/IndexAction.h
clangd/index/Serialization.cpp
clangd/index/Serialization.h
clangd/index/YAMLSerialization.cpp
clangd/indexer/IndexerMain.cpp
unittests/clangd/SerializationTests.cpp
Index: unittests/clangd/SerializationTests.cpp
===
--- unittests/clangd/SerializationTests.cpp
+++ unittests/clangd/SerializationTests.cpp
@@ -13,14 +13,18 @@
#include "gmock/gmock.h"
#include "gtest/gtest.h"
+using testing::_;
+using testing::AllOf;
+using testing::Pair;
using testing::UnorderedElementsAre;
using testing::UnorderedElementsAreArray;
namespace clang {
namespace clangd {
namespace {
const char *YAML = R"(
---
+!Symbol
ID: 057557CEBF6E6B2DD437FBF60CC58F352D1DF856
Name: 'Foo1'
Scope: 'clang::'
@@ -46,6 +50,7 @@
References:3
...
---
+!Symbol
ID: 057557CEBF6E6B2DD437FBF60CC58F352D1DF858
Name: 'Foo2'
Scope: 'clang::'
@@ -64,6 +69,18 @@
Signature:'-sig'
CompletionSnippetSuffix:'-snippet'
...
+!Refs
+ID: 057557CEBF6E6B2DD437FBF60CC58F352D1DF856
+References:
+ - Kind: 4
+Location:
+ FileURI:file:///path/foo.cc
+ Start:
+Line: 5
+Column: 3
+ End:
+Line: 5
+Column: 8
)";
MATCHER_P(ID, I, "") { return arg.ID == cantFail(SymbolID::fromStr(I)); }
@@ -107,32 +124,55 @@
EXPECT_EQ(Sym2.CanonicalDeclaration.FileURI, "file:///path/bar.h");
EXPECT_FALSE(Sym2.Flags & Symbol::IndexedForCodeCompletion);
EXPECT_TRUE(Sym2.Flags & Symbol::Deprecated);
+
+ ASSERT_TRUE(bool(ParsedYAML->Refs));
+ EXPECT_THAT(*ParsedYAML->Refs,
+ UnorderedElementsAre(
+ Pair(cantFail(SymbolID::fromStr(
+ "057557CEBF6E6B2DD437FBF60CC58F352D1DF856")),
+ 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");
}
std::vector YAMLFromSymbols(const SymbolSlab &Slab) {
std::vector Result;
for (const auto &Sym : Slab)
Result.push_back(toYAML(Sym));
return Result;
}
+std::vector YAMLFromRefs(const RefSlab &Slab) {
+ std::vector Result;
+ for (const auto &Sym : Slab)
+Result.push_back(toYAML(Sym));
+ return Result;
+}
TEST(SerializationTest, BinaryConversions) {
auto In = readIndexFile(YAML);
EXPECT_TRUE(bool(In)) << In.takeError();
// Write to binary format, and parse again.
- IndexFileOut Out;
- Out.Symbols = In->Symbols.getPointer();
+ IndexFileOut Out(*In);
Out.Format = IndexFileFormat::RIFF;
std::string Serialized = llvm::to_string(Out);
+ {
+std::error_code EC;
+llvm::raw_fd_ostream F("/tmp/foo", EC);
+F << Serialized;
+ }
auto In2 = readIndexFile(Serialized);
ASSERT_TRUE(bool(In2)) << In.takeError();
- ASSERT_TRUE(In->Symbols);
+ ASSERT_TRUE(In2->Symbols);
+ ASSERT_TRUE(In2->Refs);
// Assert the YAML serializations match, for nice comparisons and diffs.
EXPECT_THAT(YAMLFromSymbols(*In2->Symbols),
UnorderedElementsAreArray(YAMLFromSymbols(*In->Symbols)));
+ EXPECT_THAT(YAMLFromRefs(*In2->Refs),
+ UnorderedElementsAreArray(YAMLFromRefs(*In->Refs)));
}
} // namespace
Index: clangd/indexer/IndexerMain.cpp
===
--- clangd/indexer/IndexerMain.cpp
+++ clangd/indexer/IndexerMain.cpp
@@ -67,18 +67,30 @@
else
Symbols.insert(Sym);
}
+ },
+ [&](RefSlab S) {
+ std::lock_guard Lock(SymbolsMu);
+ for (const auto &Sym : S) {
+ // No need to merge as currently all Refs are from main file.
+ for (const auto &Ref : Sym.second)
+ Refs.insert(Sym.first, Ref);
+ }
})
.release();
}
// Awkward: we write the result in the destructor, because the executor
// takes ownership so it's the easiest way to get our data back out.
- ~IndexActionFactory() { Result.Symbols = std::move(Symbols).build(); }
+ ~IndexActionFactory() {
+Result.Symbols = std::move(Symbols).build();
+Result.Refs = std::move(Refs).build();
+ }
private:
IndexFileIn &Result;
std::mutex SymbolsMu;
SymbolSlab::Builder Symbols;
+ RefSlab::Builder Refs;
};
} // namespace
Index: clangd/index/YAMLSerialization.cpp
===
--- clangd/index/YAMLSerialization.cpp
+++ clangd/index/YAMLSerialization.cpp
@@ -6,6 +6,12 @@
// License. See LICENSE.TXT for details.
//
//===-
[PATCH] D37813: clang-format: better handle namespace macros
klimek added a comment.
In https://reviews.llvm.org/D37813#1253813, @Typz wrote:
> In https://reviews.llvm.org/D37813#1184051, @klimek wrote:
>
> > The problem here is that we have different opinions on how the formatting
> > on namespace macros should behave in the first place- I think they should
> > behave like namespaces, you want them to be formatted differently.
> > Given that you want full control over the formatting of those macros, and
> > them not actually be formatted exactly like namespaces or classes, I think
> > we need a more generic mechanism for you to express that.
>
>
> Not sure what you mean here. I want them to behave like namespaces as well,
> this is actually the use case I have... As implemented, they indeed behave
> exactly like namespaces :
>
> TESTSUITE(a) { namespace a {
> } // TESTSUITE(a)} // namespace a
> VS
> TESTSUITE(a) { TESTSUITE(b) {namespace a { namespace b {
> } // TESTSUITE(a::b) }} // namespace a::b
I thought we had the discussion upthread that they indent differently from
namespaces. I'm working on a patch that allows you to configure macros.
Repository:
rC Clang
https://reviews.llvm.org/D37813
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52871: [clangd] Use canonical declarations in ReferenceFinder.
hokein created this revision.
hokein added a reviewer: sammccall.
Herald added subscribers: kadircet, arphaman, jkorous, MaskRay, ioeric,
ilya-biryukov.
handleDeclOccurrencce reports a canonical declartion, so stick to use
canonical declarations to determine whether a declaration is in the
target set.
Also fix a previous ref test which misses a matched label (it fails without this
patch).
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52871
Files:
clangd/XRefs.cpp
unittests/clangd/XRefsTests.cpp
Index: unittests/clangd/XRefsTests.cpp
===
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -1128,6 +1128,14 @@
}
)cpp",
+ R"cpp(// Forward declaration
+class $foo[[Foo]];
+class $foo[[Foo]] {}
+int main() {
+ $foo[[Fo^o]] foo;
+}
+ )cpp",
+
R"cpp(// Function
int $foo[[foo]](int) {}
int main() {
@@ -1148,11 +1156,11 @@
)cpp",
R"cpp(// Method call
-struct Foo { int [[foo]](); };
-int Foo::[[foo]]() {}
+struct Foo { int $foo[[foo]](); };
+int Foo::$foo[[foo]]() {}
int main() {
Foo f;
- f.^foo();
+ f.$foo[[^foo]]();
}
)cpp",
Index: clangd/XRefs.cpp
===
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -361,48 +361,50 @@
class ReferenceFinder : public index::IndexDataConsumer {
public:
struct Reference {
-const Decl *Target;
+const Decl *CanonicalTarget;
SourceLocation Loc;
index::SymbolRoleSet Role;
};
ReferenceFinder(ASTContext &AST, Preprocessor &PP,
const std::vector &TargetDecls)
: AST(AST) {
for (const Decl *D : TargetDecls)
- Targets.insert(D);
+ CanonicalTargets.insert(D->getCanonicalDecl());
}
std::vector take() && {
std::sort(References.begin(), References.end(),
[](const Reference &L, const Reference &R) {
-return std::tie(L.Loc, L.Target, L.Role) <
- std::tie(R.Loc, R.Target, R.Role);
+return std::tie(L.Loc, L.CanonicalTarget, L.Role) <
+ std::tie(R.Loc, R.CanonicalTarget, R.Role);
});
// We sometimes see duplicates when parts of the AST get traversed twice.
-References.erase(std::unique(References.begin(), References.end(),
- [](const Reference &L, const Reference &R) {
- return std::tie(L.Target, L.Loc, L.Role) ==
- std::tie(R.Target, R.Loc, R.Role);
- }),
- References.end());
+References.erase(
+std::unique(References.begin(), References.end(),
+[](const Reference &L, const Reference &R) {
+ return std::tie(L.CanonicalTarget, L.Loc, L.Role) ==
+ std::tie(R.CanonicalTarget, R.Loc, R.Role);
+}),
+References.end());
return std::move(References);
}
bool
handleDeclOccurence(const Decl *D, index::SymbolRoleSet Roles,
ArrayRef Relations,
SourceLocation Loc,
index::IndexDataConsumer::ASTNodeInfo ASTNode) override {
+assert(D->isCanonicalDecl() && "expect D to be a canonical declaration");
const SourceManager &SM = AST.getSourceManager();
Loc = SM.getFileLoc(Loc);
-if (SM.isWrittenInMainFile(Loc) && Targets.count(D))
+if (SM.isWrittenInMainFile(Loc) && CanonicalTargets.count(D))
References.push_back({D, Loc, Roles});
return true;
}
private:
- llvm::SmallSet Targets;
+ llvm::SmallSet CanonicalTargets;
std::vector References;
const ASTContext &AST;
};
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44609: [Clang-Format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style)
christophe-calmejane added a comment. Ok I found the issue with noexcept, it's not related to your patch. There is a missing case tok::kw_noexcept: in UnwrappedLineParser::tryToParseLambda Repository: rC Clang https://reviews.llvm.org/D44609 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52617: [clangd] Make stable_partition on include candidates less slow. NFC
sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. The implementation looks fine, but I believe we concluded that seeing huge lists here was basically a bug. So you may not want to land it, if you prefer to keep things simple. Up to you. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52617 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52531: [clangd] clangd-indexer gathers refs and stores them in index files.
hokein accepted this revision. hokein added a comment. This revision is now accepted and ready to land. looks good. Comment at: clangd/index/Serialization.cpp:301 +// A refs section has data grouped by Symbol. Each symbol has: +// - SymbolID: 20 bytes +// - NumRefs: varint sammccall wrote: > hokein wrote: > > Looks like we can improve the size further, we are storing SymbolID twice > > (one in `symb` section), I think we could optimize it by introducing a > > `symi` section or using `stri` section? > Yes, there's a TODO on line 250. I opted not to do so in this patch, as it's > a more invasive change and I wasn't sure about the exact representation. > It should be straightforward to add later and bump the version. SG. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52531 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52871: [clangd] Use canonical declarations in ReferenceFinder.
sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.
Nice catch!
Comment at: unittests/clangd/XRefsTests.cpp:1185
std::vector> ExpectedLocations;
for (const auto &R : T.ranges("foo"))
ExpectedLocations.push_back(RangeIs(R));
hmm, it looks like the "foo" labels on the ranges don't need to be there, and
they hurt readability.
Want to remove them from all the tests, rather than add them to the method call
test where they're missing?
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52871
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r343763 - [clangd] Use canonical declarations in ReferenceFinder.
Author: hokein
Date: Thu Oct 4 02:56:08 2018
New Revision: 343763
URL: http://llvm.org/viewvc/llvm-project?rev=343763&view=rev
Log:
[clangd] Use canonical declarations in ReferenceFinder.
Summary:
handleDeclOccurrencce reports a canonical declartion, so stick to use
canonical declarations to determine whether a declaration is in the
target set.
Also fix a previous ref test which misses a matched label (it fails without this
patch).
Reviewers: sammccall
Subscribers: ilya-biryukov, ioeric, MaskRay, jkorous, arphaman, kadircet,
cfe-commits
Differential Revision: https://reviews.llvm.org/D52871
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=343763&r1=343762&r2=343763&view=diff
==
--- clang-tools-extra/trunk/clangd/XRefs.cpp (original)
+++ clang-tools-extra/trunk/clangd/XRefs.cpp Thu Oct 4 02:56:08 2018
@@ -361,7 +361,7 @@ namespace {
class ReferenceFinder : public index::IndexDataConsumer {
public:
struct Reference {
-const Decl *Target;
+const Decl *CanonicalTarget;
SourceLocation Loc;
index::SymbolRoleSet Role;
};
@@ -370,22 +370,23 @@ public:
const std::vector &TargetDecls)
: AST(AST) {
for (const Decl *D : TargetDecls)
- Targets.insert(D);
+ CanonicalTargets.insert(D->getCanonicalDecl());
}
std::vector take() && {
std::sort(References.begin(), References.end(),
[](const Reference &L, const Reference &R) {
-return std::tie(L.Loc, L.Target, L.Role) <
- std::tie(R.Loc, R.Target, R.Role);
+return std::tie(L.Loc, L.CanonicalTarget, L.Role) <
+ std::tie(R.Loc, R.CanonicalTarget, R.Role);
});
// We sometimes see duplicates when parts of the AST get traversed twice.
-References.erase(std::unique(References.begin(), References.end(),
- [](const Reference &L, const Reference &R) {
- return std::tie(L.Target, L.Loc, L.Role) ==
- std::tie(R.Target, R.Loc, R.Role);
- }),
- References.end());
+References.erase(
+std::unique(References.begin(), References.end(),
+[](const Reference &L, const Reference &R) {
+ return std::tie(L.CanonicalTarget, L.Loc, L.Role) ==
+ std::tie(R.CanonicalTarget, R.Loc, R.Role);
+}),
+References.end());
return std::move(References);
}
@@ -394,15 +395,16 @@ public:
ArrayRef Relations,
SourceLocation Loc,
index::IndexDataConsumer::ASTNodeInfo ASTNode) override {
+assert(D->isCanonicalDecl() && "expect D to be a canonical declaration");
const SourceManager &SM = AST.getSourceManager();
Loc = SM.getFileLoc(Loc);
-if (SM.isWrittenInMainFile(Loc) && Targets.count(D))
+if (SM.isWrittenInMainFile(Loc) && CanonicalTargets.count(D))
References.push_back({D, Loc, Roles});
return true;
}
private:
- llvm::SmallSet Targets;
+ llvm::SmallSet CanonicalTargets;
std::vector References;
const ASTContext &AST;
};
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=343763&r1=343762&r2=343763&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp Thu Oct 4 02:56:08
2018
@@ -1113,37 +1113,45 @@ TEST(FindReferences, WithinAST) {
const char *Tests[] = {
R"cpp(// Local variable
int main() {
- int $foo[[foo]];
- $foo[[^foo]] = 2;
- int test1 = $foo[[foo]];
+ int [[foo]];
+ [[^foo]] = 2;
+ int test1 = [[foo]];
}
)cpp",
R"cpp(// Struct
namespace ns1 {
-struct $foo[[Foo]] {};
+struct [[Foo]] {};
} // namespace ns1
int main() {
- ns1::$foo[[Fo^o]]* Params;
+ ns1::[[Fo^o]]* Params;
+}
+ )cpp",
+
+ R"cpp(// Forward declaration
+class [[Foo]];
+class [[Foo]] {}
+int main() {
+ [[Fo^o]] foo;
}
)cpp",
R"cpp(// Function
-int $foo[[foo]](int) {}
+int [[foo]](int) {}
int main() {
- auto *X = &$foo[[^foo]];
- $foo[[foo]](42)
+ auto *X = &[[^foo]];
+ [[foo]](42)
}
)cpp",
R"
[PATCH] D37813: clang-format: better handle namespace macros
Typz added a comment.
In https://reviews.llvm.org/D37813#1254865, @klimek wrote:
> In https://reviews.llvm.org/D37813#1253813, @Typz wrote:
>
> > In https://reviews.llvm.org/D37813#1184051, @klimek wrote:
> >
> > > The problem here is that we have different opinions on how the formatting
> > > on namespace macros should behave in the first place- I think they should
> > > behave like namespaces, you want them to be formatted differently.
> > > Given that you want full control over the formatting of those macros,
> > > and them not actually be formatted exactly like namespaces or classes, I
> > > think we need a more generic mechanism for you to express that.
> >
> >
> > Not sure what you mean here. I want them to behave like namespaces as well,
> > this is actually the use case I have... As implemented, they indeed behave
> > exactly like namespaces :
> >
> > TESTSUITE(a) { namespace a {
> > } // TESTSUITE(a)} // namespace a
> > VS
> > TESTSUITE(a) { TESTSUITE(b) {namespace a { namespace b {
> > } // TESTSUITE(a::b) }} // namespace a::b
>
>
> I thought we had the discussion upthread that they indent differently from
> namespaces. I'm working on a patch that allows you to configure macros.
The current behavior is that they indent differently from namespace, because
clang does not know these macros are conceptually equivalent to namespaces. And
this patch actually fixes that, letting clang know they are actually macros.
Repository:
rC Clang
https://reviews.llvm.org/D37813
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52871: [clangd] Use canonical declarations in ReferenceFinder.
hokein updated this revision to Diff 168248.
hokein marked an inline comment as done.
hokein added a comment.
Remove unnecessary labels.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52871
Files:
clangd/XRefs.cpp
unittests/clangd/XRefsTests.cpp
Index: unittests/clangd/XRefsTests.cpp
===
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -1113,68 +1113,76 @@
const char *Tests[] = {
R"cpp(// Local variable
int main() {
- int $foo[[foo]];
- $foo[[^foo]] = 2;
- int test1 = $foo[[foo]];
+ int [[foo]];
+ [[^foo]] = 2;
+ int test1 = [[foo]];
}
)cpp",
R"cpp(// Struct
namespace ns1 {
-struct $foo[[Foo]] {};
+struct [[Foo]] {};
} // namespace ns1
int main() {
- ns1::$foo[[Fo^o]]* Params;
+ ns1::[[Fo^o]]* Params;
+}
+ )cpp",
+
+ R"cpp(// Forward declaration
+class [[Foo]];
+class [[Foo]] {}
+int main() {
+ [[Fo^o]] foo;
}
)cpp",
R"cpp(// Function
-int $foo[[foo]](int) {}
+int [[foo]](int) {}
int main() {
- auto *X = &$foo[[^foo]];
- $foo[[foo]](42)
+ auto *X = &[[^foo]];
+ [[foo]](42)
}
)cpp",
R"cpp(// Field
struct Foo {
- int $foo[[foo]];
- Foo() : $foo[[foo]](0) {}
+ int [[foo]];
+ Foo() : [[foo]](0) {}
};
int main() {
Foo f;
- f.$foo[[f^oo]] = 1;
+ f.[[f^oo]] = 1;
}
)cpp",
R"cpp(// Method call
struct Foo { int [[foo]](); };
int Foo::[[foo]]() {}
int main() {
Foo f;
- f.^foo();
+ f.[[^foo]]();
}
)cpp",
R"cpp(// Typedef
-typedef int $foo[[Foo]];
+typedef int [[Foo]];
int main() {
- $foo[[^Foo]] bar;
+ [[^Foo]] bar;
}
)cpp",
R"cpp(// Namespace
-namespace $foo[[ns]] {
+namespace [[ns]] {
struct Foo {};
} // namespace ns
-int main() { $foo[[^ns]]::Foo foo; }
+int main() { [[^ns]]::Foo foo; }
)cpp",
};
for (const char *Test : Tests) {
Annotations T(Test);
auto AST = TestTU::withCode(T.code()).build();
std::vector> ExpectedLocations;
-for (const auto &R : T.ranges("foo"))
+for (const auto &R : T.ranges())
ExpectedLocations.push_back(RangeIs(R));
EXPECT_THAT(findReferences(AST, T.point()),
ElementsAreArray(ExpectedLocations))
Index: clangd/XRefs.cpp
===
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -361,48 +361,50 @@
class ReferenceFinder : public index::IndexDataConsumer {
public:
struct Reference {
-const Decl *Target;
+const Decl *CanonicalTarget;
SourceLocation Loc;
index::SymbolRoleSet Role;
};
ReferenceFinder(ASTContext &AST, Preprocessor &PP,
const std::vector &TargetDecls)
: AST(AST) {
for (const Decl *D : TargetDecls)
- Targets.insert(D);
+ CanonicalTargets.insert(D->getCanonicalDecl());
}
std::vector take() && {
std::sort(References.begin(), References.end(),
[](const Reference &L, const Reference &R) {
-return std::tie(L.Loc, L.Target, L.Role) <
- std::tie(R.Loc, R.Target, R.Role);
+return std::tie(L.Loc, L.CanonicalTarget, L.Role) <
+ std::tie(R.Loc, R.CanonicalTarget, R.Role);
});
// We sometimes see duplicates when parts of the AST get traversed twice.
-References.erase(std::unique(References.begin(), References.end(),
- [](const Reference &L, const Reference &R) {
- return std::tie(L.Target, L.Loc, L.Role) ==
- std::tie(R.Target, R.Loc, R.Role);
- }),
- References.end());
+References.erase(
+std::unique(References.begin(), References.end(),
+[](const Reference &L, const Reference &R) {
+ return std::tie(L.CanonicalTarget, L.Loc, L.Role) ==
+ std::tie(R.CanonicalTarget, R.Loc, R.Role);
+}),
+References.end());
return std::move(References);
}
bool
handleDeclOccurence(const Decl *D, index::SymbolRoleSet Roles,
ArrayRef Relations,
SourceLocation Loc,
index::IndexDataConsumer::ASTNodeInfo ASTNode) override {
+assert(D->isCanonicalDecl() && "expect D to be a canonical declaration");
const SourceMana
[PATCH] D50766: Fix false positive unsequenced access and modification warning in array subscript expression.
stryku added a comment. @Rakete Any more comments? https://reviews.llvm.org/D50766 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52871: [clangd] Use canonical declarations in ReferenceFinder.
This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE343763: [clangd] Use canonical declarations in
ReferenceFinder. (authored by hokein, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D52871?vs=168248&id=168250#toc
Repository:
rL LLVM
https://reviews.llvm.org/D52871
Files:
clangd/XRefs.cpp
unittests/clangd/XRefsTests.cpp
Index: unittests/clangd/XRefsTests.cpp
===
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -1113,68 +1113,76 @@
const char *Tests[] = {
R"cpp(// Local variable
int main() {
- int $foo[[foo]];
- $foo[[^foo]] = 2;
- int test1 = $foo[[foo]];
+ int [[foo]];
+ [[^foo]] = 2;
+ int test1 = [[foo]];
}
)cpp",
R"cpp(// Struct
namespace ns1 {
-struct $foo[[Foo]] {};
+struct [[Foo]] {};
} // namespace ns1
int main() {
- ns1::$foo[[Fo^o]]* Params;
+ ns1::[[Fo^o]]* Params;
+}
+ )cpp",
+
+ R"cpp(// Forward declaration
+class [[Foo]];
+class [[Foo]] {}
+int main() {
+ [[Fo^o]] foo;
}
)cpp",
R"cpp(// Function
-int $foo[[foo]](int) {}
+int [[foo]](int) {}
int main() {
- auto *X = &$foo[[^foo]];
- $foo[[foo]](42)
+ auto *X = &[[^foo]];
+ [[foo]](42)
}
)cpp",
R"cpp(// Field
struct Foo {
- int $foo[[foo]];
- Foo() : $foo[[foo]](0) {}
+ int [[foo]];
+ Foo() : [[foo]](0) {}
};
int main() {
Foo f;
- f.$foo[[f^oo]] = 1;
+ f.[[f^oo]] = 1;
}
)cpp",
R"cpp(// Method call
struct Foo { int [[foo]](); };
int Foo::[[foo]]() {}
int main() {
Foo f;
- f.^foo();
+ f.[[^foo]]();
}
)cpp",
R"cpp(// Typedef
-typedef int $foo[[Foo]];
+typedef int [[Foo]];
int main() {
- $foo[[^Foo]] bar;
+ [[^Foo]] bar;
}
)cpp",
R"cpp(// Namespace
-namespace $foo[[ns]] {
+namespace [[ns]] {
struct Foo {};
} // namespace ns
-int main() { $foo[[^ns]]::Foo foo; }
+int main() { [[^ns]]::Foo foo; }
)cpp",
};
for (const char *Test : Tests) {
Annotations T(Test);
auto AST = TestTU::withCode(T.code()).build();
std::vector> ExpectedLocations;
-for (const auto &R : T.ranges("foo"))
+for (const auto &R : T.ranges())
ExpectedLocations.push_back(RangeIs(R));
EXPECT_THAT(findReferences(AST, T.point()),
ElementsAreArray(ExpectedLocations))
Index: clangd/XRefs.cpp
===
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -361,48 +361,50 @@
class ReferenceFinder : public index::IndexDataConsumer {
public:
struct Reference {
-const Decl *Target;
+const Decl *CanonicalTarget;
SourceLocation Loc;
index::SymbolRoleSet Role;
};
ReferenceFinder(ASTContext &AST, Preprocessor &PP,
const std::vector &TargetDecls)
: AST(AST) {
for (const Decl *D : TargetDecls)
- Targets.insert(D);
+ CanonicalTargets.insert(D->getCanonicalDecl());
}
std::vector take() && {
std::sort(References.begin(), References.end(),
[](const Reference &L, const Reference &R) {
-return std::tie(L.Loc, L.Target, L.Role) <
- std::tie(R.Loc, R.Target, R.Role);
+return std::tie(L.Loc, L.CanonicalTarget, L.Role) <
+ std::tie(R.Loc, R.CanonicalTarget, R.Role);
});
// We sometimes see duplicates when parts of the AST get traversed twice.
-References.erase(std::unique(References.begin(), References.end(),
- [](const Reference &L, const Reference &R) {
- return std::tie(L.Target, L.Loc, L.Role) ==
- std::tie(R.Target, R.Loc, R.Role);
- }),
- References.end());
+References.erase(
+std::unique(References.begin(), References.end(),
+[](const Reference &L, const Reference &R) {
+ return std::tie(L.CanonicalTarget, L.Loc, L.Role) ==
+ std::tie(R.CanonicalTarget, R.Loc, R.Role);
+}),
+References.end());
return std::move(References);
}
bool
handleDeclOccurence(const Decl *D, index::SymbolRoleSet Roles,
ArrayRef Relations,
SourceLocation Loc,
index::IndexDataConsumer::ASTNo
[PATCH] D52871: [clangd] Use canonical declarations in ReferenceFinder.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL343763: [clangd] Use canonical declarations in
ReferenceFinder. (authored by hokein, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D52871
Files:
clang-tools-extra/trunk/clangd/XRefs.cpp
clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp
Index: clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp
@@ -1113,68 +1113,76 @@
const char *Tests[] = {
R"cpp(// Local variable
int main() {
- int $foo[[foo]];
- $foo[[^foo]] = 2;
- int test1 = $foo[[foo]];
+ int [[foo]];
+ [[^foo]] = 2;
+ int test1 = [[foo]];
}
)cpp",
R"cpp(// Struct
namespace ns1 {
-struct $foo[[Foo]] {};
+struct [[Foo]] {};
} // namespace ns1
int main() {
- ns1::$foo[[Fo^o]]* Params;
+ ns1::[[Fo^o]]* Params;
+}
+ )cpp",
+
+ R"cpp(// Forward declaration
+class [[Foo]];
+class [[Foo]] {}
+int main() {
+ [[Fo^o]] foo;
}
)cpp",
R"cpp(// Function
-int $foo[[foo]](int) {}
+int [[foo]](int) {}
int main() {
- auto *X = &$foo[[^foo]];
- $foo[[foo]](42)
+ auto *X = &[[^foo]];
+ [[foo]](42)
}
)cpp",
R"cpp(// Field
struct Foo {
- int $foo[[foo]];
- Foo() : $foo[[foo]](0) {}
+ int [[foo]];
+ Foo() : [[foo]](0) {}
};
int main() {
Foo f;
- f.$foo[[f^oo]] = 1;
+ f.[[f^oo]] = 1;
}
)cpp",
R"cpp(// Method call
struct Foo { int [[foo]](); };
int Foo::[[foo]]() {}
int main() {
Foo f;
- f.^foo();
+ f.[[^foo]]();
}
)cpp",
R"cpp(// Typedef
-typedef int $foo[[Foo]];
+typedef int [[Foo]];
int main() {
- $foo[[^Foo]] bar;
+ [[^Foo]] bar;
}
)cpp",
R"cpp(// Namespace
-namespace $foo[[ns]] {
+namespace [[ns]] {
struct Foo {};
} // namespace ns
-int main() { $foo[[^ns]]::Foo foo; }
+int main() { [[^ns]]::Foo foo; }
)cpp",
};
for (const char *Test : Tests) {
Annotations T(Test);
auto AST = TestTU::withCode(T.code()).build();
std::vector> ExpectedLocations;
-for (const auto &R : T.ranges("foo"))
+for (const auto &R : T.ranges())
ExpectedLocations.push_back(RangeIs(R));
EXPECT_THAT(findReferences(AST, T.point()),
ElementsAreArray(ExpectedLocations))
Index: clang-tools-extra/trunk/clangd/XRefs.cpp
===
--- clang-tools-extra/trunk/clangd/XRefs.cpp
+++ clang-tools-extra/trunk/clangd/XRefs.cpp
@@ -361,48 +361,50 @@
class ReferenceFinder : public index::IndexDataConsumer {
public:
struct Reference {
-const Decl *Target;
+const Decl *CanonicalTarget;
SourceLocation Loc;
index::SymbolRoleSet Role;
};
ReferenceFinder(ASTContext &AST, Preprocessor &PP,
const std::vector &TargetDecls)
: AST(AST) {
for (const Decl *D : TargetDecls)
- Targets.insert(D);
+ CanonicalTargets.insert(D->getCanonicalDecl());
}
std::vector take() && {
std::sort(References.begin(), References.end(),
[](const Reference &L, const Reference &R) {
-return std::tie(L.Loc, L.Target, L.Role) <
- std::tie(R.Loc, R.Target, R.Role);
+return std::tie(L.Loc, L.CanonicalTarget, L.Role) <
+ std::tie(R.Loc, R.CanonicalTarget, R.Role);
});
// We sometimes see duplicates when parts of the AST get traversed twice.
-References.erase(std::unique(References.begin(), References.end(),
- [](const Reference &L, const Reference &R) {
- return std::tie(L.Target, L.Loc, L.Role) ==
- std::tie(R.Target, R.Loc, R.Role);
- }),
- References.end());
+References.erase(
+std::unique(References.begin(), References.end(),
+[](const Reference &L, const Reference &R) {
+ return std::tie(L.CanonicalTarget, L.Loc, L.Role) ==
+ std::tie(R.CanonicalTarget, R.Loc, R.Role);
+}),
+References.end());
return std::move(References);
}
bool
handleDeclOccurence(const Decl *D, index::SymbolRoleSet
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta added a comment. Ping? https://reviews.llvm.org/D51340 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52808: [cland] Dex: fix/simplify trigram generation
ioeric added a comment.
> Generate more short-query trigrams, e.g. "AbcDefGhi" now yields "d" and "ag".
I am concerned about the impact on the size of posting lists (we can measure)
and retrieval quality by adding more incomplete trigrams.
> This is effectively required by LSP, having "ag" not match but "agh" match
> will lead to glitches due to client-side filtering.
It seems hard to make index behave the same as LSP clients, considering all the
ranking signals we have; there can be other reasons that cause `having "ag" not
match but "agh" match`. And it seems reasonable to assume that you would get
better symbols as you type more characters (i.e. stronger relevance signal).
> Drop leading-punctuation short-query trigrams. Nice idea, but current
> implementation is broken (competes with non-punctuation short query trigrams).
Could you elaborate how this is broken? We should probably fix it instead of
removing it. `__` not matching `__some_macro` sounds like a regression.
Comment at: unittests/clangd/DexTests.cpp:366
+ auto X = generateIdentifierTrigrams("abc_defGhij__klm");
+ llvm::sort(X,
nit: remove?
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52808
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52789: [clangd] Dex: FALSE iterator, peephole optimizations, fix AND bug
ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.
LGTM and a few NITs.
Comment at: clangd/index/dex/Iterator.cpp:376
+}
+default:
+ RealChildren.push_back(std::move(Child));
Maybe replace with `case Kind::Other` to make sure compiler gives a warning to
update the switch when new kinds are added?
Same for other switches on kind
Comment at: clangd/index/dex/Iterator.cpp:378
+ RealChildren.push_back(std::move(Child));
+}
+ switch (RealChildren.size()) {
Add braces for `for` statement to make sure there's a closing brace at the
right indentation?
Comment at: clangd/index/dex/Iterator.cpp:385
+ default:
+return llvm::make_unique(move(RealChildren));
+ }
Maybe use the qualified `std::move` for consistency with the previous line?
Comment at: clangd/index/dex/Iterator.cpp:407
+ RealChildren.push_back(std::move(Child));
+}
+ switch (RealChildren.size()) {
Again, `for () switch` pattern ends up putting the closing brace at the wrong
indent level. Consider adding the braces around for the loop body.
Comment at: clangd/index/dex/Iterator.cpp:414
+ default:
+return llvm::make_unique(move(RealChildren));
+ }
Maybe use the qualified `std::move` for consistency with the previous line?
Comment at: clangd/index/dex/Iterator.h:109
private:
+ Kind MyKind;
virtual llvm::raw_ostream &dump(llvm::raw_ostream &OS) const = 0;
Maybe put data members after functions to follow the pattern we have in most
(all?) other places in clangd?
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52789
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52814: [PassManager/Sanitizer] Enable usage of ported AddressSanitizer passes with -fsanitize=address
philip.pfaffe added a comment. Is this the right place in the pipeline to put the passes? The legacy PM inserts the asan passes at EP_OptimizerLast, why not do the same thing? Repository: rC Clang https://reviews.llvm.org/D52814 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52753: [tooling] Create executor by name
ioeric added a comment. > This is useful in the case where the calling tool always wants to use a > particular non-default implementation (e.g. the tool should always be run > with the all-TUs executor) Any reason not to use the constructor of `AllTUsToolExecutor` directly? There is one that takes `CommonOptionsParser`. https://reviews.llvm.org/D52753 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52117: Generate llvm.loop.parallel_accesses instead of llvm.mem.parallel_loop_access metadata.
Meinersbur updated this revision to Diff 168251.
Meinersbur marked 2 inline comments as done.
Meinersbur added a comment.
- Address @pekka.jaaskelainen's review.
Repository:
rC Clang
https://reviews.llvm.org/D52117
Files:
lib/CodeGen/CGLoopInfo.cpp
lib/CodeGen/CGLoopInfo.h
test/CodeGenCXX/pragma-loop-safety-imperfectly_nested.cpp
test/CodeGenCXX/pragma-loop-safety-nested.cpp
test/CodeGenCXX/pragma-loop-safety-outer.cpp
test/CodeGenCXX/pragma-loop-safety.cpp
test/OpenMP/for_codegen.cpp
test/OpenMP/for_simd_codegen.cpp
test/OpenMP/loops_explicit_clauses_codegen.cpp
test/OpenMP/ordered_codegen.cpp
test/OpenMP/parallel_for_simd_codegen.cpp
test/OpenMP/schedule_codegen.cpp
test/OpenMP/simd_codegen.cpp
test/OpenMP/simd_metadata.c
test/OpenMP/target_parallel_for_simd_codegen.cpp
test/OpenMP/target_simd_codegen.cpp
test/OpenMP/taskloop_simd_codegen.cpp
Index: test/OpenMP/taskloop_simd_codegen.cpp
===
--- test/OpenMP/taskloop_simd_codegen.cpp
+++ test/OpenMP/taskloop_simd_codegen.cpp
@@ -83,17 +83,17 @@
// CHECK: [[LB_I32:%.+]] = trunc i64 [[LB_VAL]] to i32
// CHECK: store i32 [[LB_I32]], i32* [[CNT:%.+]],
// CHECK: br label
-// CHECK: [[VAL:%.+]] = load i32, i32* [[CNT]],{{.*}}!llvm.mem.parallel_loop_access [[LOOP1:!.+]]
+// CHECK: [[VAL:%.+]] = load i32, i32* [[CNT]],{{.*}}!llvm.access.group
// CHECK: [[VAL_I64:%.+]] = sext i32 [[VAL]] to i64
-// CHECK: [[UB_VAL:%.+]] = load i64, i64* [[UB]],{{.*}}!llvm.mem.parallel_loop_access [[LOOP1]]
+// CHECK: [[UB_VAL:%.+]] = load i64, i64* [[UB]],{{.*}}!llvm.access.group
// CHECK: [[CMP:%.+]] = icmp ule i64 [[VAL_I64]], [[UB_VAL]]
// CHECK: br i1 [[CMP]], label %{{.+}}, label %{{.+}}
-// CHECK: load i32, i32* %{{.*}}!llvm.mem.parallel_loop_access [[LOOP1]]
-// CHECK: store i32 %{{.*}}!llvm.mem.parallel_loop_access [[LOOP1]]
-// CHECK: load i32, i32* %{{.*}}!llvm.mem.parallel_loop_access [[LOOP1]]
+// CHECK: load i32, i32* %{{.*}}!llvm.access.group
+// CHECK: store i32 %{{.*}}!llvm.access.group
+// CHECK: load i32, i32* %{{.*}}!llvm.access.group
// CHECK: add nsw i32 %{{.+}}, 1
-// CHECK: store i32 %{{.+}}, i32* %{{.*}}!llvm.mem.parallel_loop_access [[LOOP1]]
-// CHECK: br label %{{.*}}!llvm.loop [[LOOP1]]
+// CHECK: store i32 %{{.+}}, i32* %{{.*}}!llvm.access.group
+// CHECK: br label %{{.*}}!llvm.loop
// CHECK: ret i32 0
// CHECK: define internal i32 [[TASK2]](
@@ -113,17 +113,17 @@
// CHECK: [[LB_I32:%.+]] = trunc i64 [[LB_VAL]] to i32
// CHECK: store i32 [[LB_I32]], i32* [[CNT:%.+]],
// CHECK: br label
-// CHECK: [[VAL:%.+]] = load i32, i32* [[CNT]],{{.*}}!llvm.mem.parallel_loop_access [[LOOP2:!.+]]
+// CHECK: [[VAL:%.+]] = load i32, i32* [[CNT]],{{.*}}!llvm.access.group
// CHECK: [[VAL_I64:%.+]] = sext i32 [[VAL]] to i64
-// CHECK: [[UB_VAL:%.+]] = load i64, i64* [[UB]],{{.*}}!llvm.mem.parallel_loop_access [[LOOP2]]
+// CHECK: [[UB_VAL:%.+]] = load i64, i64* [[UB]],{{.*}}!llvm.access.group
// CHECK: [[CMP:%.+]] = icmp ule i64 [[VAL_I64]], [[UB_VAL]]
// CHECK: br i1 [[CMP]], label %{{.+}}, label %{{.+}}
-// CHECK: load i32, i32* %{{.*}}!llvm.mem.parallel_loop_access [[LOOP2]]
-// CHECK: store i32 %{{.*}}!llvm.mem.parallel_loop_access [[LOOP2]]
-// CHECK: load i32, i32* %{{.*}}!llvm.mem.parallel_loop_access [[LOOP2]]
+// CHECK: load i32, i32* %{{.*}}!llvm.access.group
+// CHECK: store i32 %{{.*}}!llvm.access.group
+// CHECK: load i32, i32* %{{.*}}!llvm.access.group
// CHECK: add nsw i32 %{{.+}}, 1
-// CHECK: store i32 %{{.+}}, i32* %{{.*}}!llvm.mem.parallel_loop_access [[LOOP2]]
-// CHECK: br label %{{.*}}!llvm.loop [[LOOP2]]
+// CHECK: store i32 %{{.+}}, i32* %{{.*}}!llvm.access.group
+// CHECK: br label %{{.*}}!llvm.loop
// CHECK: ret i32 0
// CHECK: define internal i32 [[TASK3]](
@@ -142,7 +142,7 @@
// CHECK: [[LB_VAL:%.+]] = load i64, i64* [[LB]],
// CHECK: store i64 [[LB_VAL]], i64* [[CNT:%.+]],
// CHECK: br label
-// CHECK-NOT: !llvm.mem.parallel_loop_access
+// CHECK-NOT: !llvm.access.group
// CHECK: br label %{{.*}}!llvm.loop
// CHECK: ret i32 0
@@ -192,14 +192,14 @@
// CHECK: [[CMP:%.+]] = icmp ule i64 [[VAL_I64]], [[UB_VAL]]
// CHECK: br i1 [[CMP]], label %{{.+}}, label %{{.+}}
// CHECK: load i32, i32* %
-// CHECK-NOT: !llvm.mem.parallel_loop_access
+// CHECK-NOT: !llvm.access.group
// CHECK: store i32 %
-// CHECK-NOT: !llvm.mem.parallel_loop_access
+// CHECK-NOT: !llvm.access.group
// CHECK: load i32, i32* %
-// CHECK-NOT: !llvm.mem.parallel_loop_access
+// CHECK-NOT: !llvm.access.group
// CHECK: add nsw i32 %{{.+}}, 1
// CHECK: store i32 %{{.+}}, i32* %
-// CHECK-NOT: !llvm.mem.parallel_loop_access
+// CHECK-NOT: !llvm.access.group
// CHECK: br label %{{.*}}!llvm.loop
// CHECK: ret i32 0
Index: test/OpenMP/target_simd_codegen.cpp
===
--- test/OpenMP/target_simd_codegen.cpp
+++ test/OpenMP/target_simd_codegen.cpp
@@ -342,7 +342,7 @@
// CHECK-64:[[AA
[clang-tools-extra] r343764 - [clangd] Revert accidental flag change
Author: sammccall
Date: Thu Oct 4 03:10:35 2018
New Revision: 343764
URL: http://llvm.org/viewvc/llvm-project?rev=343764&view=rev
Log:
[clangd] Revert accidental flag change
Modified:
clang-tools-extra/trunk/clangd/indexer/IndexerMain.cpp
Modified: clang-tools-extra/trunk/clangd/indexer/IndexerMain.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/indexer/IndexerMain.cpp?rev=343764&r1=343763&r2=343764&view=diff
==
--- clang-tools-extra/trunk/clangd/indexer/IndexerMain.cpp (original)
+++ clang-tools-extra/trunk/clangd/indexer/IndexerMain.cpp Thu Oct 4 03:10:35
2018
@@ -47,7 +47,7 @@ static llvm::cl::opt
"human-readable YAML format"),
clEnumValN(IndexFileFormat::RIFF, "binary",
"binary RIFF format")),
- llvm::cl::init(IndexFileFormat::RIFF));
+ llvm::cl::init(IndexFileFormat::YAML));
class IndexActionFactory : public tooling::FrontendActionFactory {
public:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52814: [PassManager/Sanitizer] Enable usage of ported AddressSanitizer passes with -fsanitize=address
fedor.sergeev added inline comments. Comment at: lib/CodeGen/BackendUtil.cpp:1031-1038 + MPM.addPass(AddressSanitizerModulePass()); + + // Add Function Pass + CGSCCPassManager MainCGPipeline(CodeGenOpts.DebugPassManager); + MainCGPipeline.addPass(createCGSCCToFunctionPassAdaptor( + buildAddressSanitizerPipeline(CodeGenOpts.DebugPassManager))); + MPM.addPass( I dont believe CGSCC is appropriate here. I would expect to see a simple ModuleToFunction adapter, something like this: MPM.addPass(AddressSanitizerModulePass()); MPM.addPass(createModuleToFunctionPassAdaptor(AddressSanitizerFunctionPass()) Also, it seems that legacy runs Function sanitizer first and then Module sanitizer after it. This sequence does it backwards. Is it intentional? Repository: rC Clang https://reviews.llvm.org/D52814 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52872: [clangd] Make binary index format the default, remove dead flag.
sammccall created this revision.
sammccall added a reviewer: hokein.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay,
ioeric, ilya-biryukov.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52872
Files:
clangd/indexer/IndexerMain.cpp
Index: clangd/indexer/IndexerMain.cpp
===
--- clangd/indexer/IndexerMain.cpp
+++ clangd/indexer/IndexerMain.cpp
@@ -7,8 +7,7 @@
//
//===--===//
//
-// GlobalSymbolBuilder is a tool to extract symbols from a whole project.
-// This tool is **experimental** only. Don't use it in production code.
+// clangd-indexer is a tool to gather index data (symbols, xrefs) from source.
//
//===--===//
@@ -21,7 +20,6 @@
#include "clang/Tooling/Execution.h"
#include "clang/Tooling/Tooling.h"
#include "llvm/Support/CommandLine.h"
-#include "llvm/Support/Path.h"
#include "llvm/Support/Signals.h"
using namespace llvm;
@@ -32,30 +30,20 @@
namespace clangd {
namespace {
-static llvm::cl::opt AssumedHeaderDir(
-"assume-header-dir",
-llvm::cl::desc("The index includes header that a symbol is defined in. "
- "If the absolute path cannot be determined (e.g. an "
- "in-memory VFS) then the relative path is resolved against "
- "this directory, which must be absolute. If this flag is "
- "not given, such headers will have relative paths."),
-llvm::cl::init(""));
-
static llvm::cl::opt
Format("format", llvm::cl::desc("Format of the index to be written"),
llvm::cl::values(clEnumValN(IndexFileFormat::YAML, "yaml",
"human-readable YAML format"),
clEnumValN(IndexFileFormat::RIFF, "binary",
"binary RIFF format")),
- llvm::cl::init(IndexFileFormat::YAML));
+ llvm::cl::init(IndexFileFormat::RIFF));
class IndexActionFactory : public tooling::FrontendActionFactory {
public:
IndexActionFactory(IndexFileIn &Result) : Result(Result) {}
clang::FrontendAction *create() override {
SymbolCollector::Options Opts;
-Opts.FallbackDir = AssumedHeaderDir;
return createStaticIndexingAction(
Opts,
[&](SymbolSlab S) {
@@ -90,15 +78,14 @@
const char *Overview = R"(
Creates an index of symbol information etc in a whole project.
- This is **experimental** and not production-ready!
Example usage for a project using CMake compile commands:
- $ clangd-indexer --executor=all-TUs compile_commands.json > index.yaml
+ $ clangd-indexer --executor=all-TUs compile_commands.json > clangd-index
Example usage for file sequence index without flags:
- $ clangd-indexer File1.cpp File2.cpp ... FileN.cpp > index.yaml
+ $ clangd-indexer File1.cpp File2.cpp ... FileN.cpp > clangd-index
Note: only symbols from header files will be indexed.
)";
@@ -111,12 +98,6 @@
return 1;
}
- if (!clang::clangd::AssumedHeaderDir.empty() &&
- !llvm::sys::path::is_absolute(clang::clangd::AssumedHeaderDir)) {
-llvm::errs() << "--assume-header-dir must be an absolute path.\n";
-return 1;
- }
-
// Collect symbols found in each translation unit, merging as we go.
clang::clangd::IndexFileIn Data;
auto Err = Executor->get()->execute(
Index: clangd/indexer/IndexerMain.cpp
===
--- clangd/indexer/IndexerMain.cpp
+++ clangd/indexer/IndexerMain.cpp
@@ -7,8 +7,7 @@
//
//===--===//
//
-// GlobalSymbolBuilder is a tool to extract symbols from a whole project.
-// This tool is **experimental** only. Don't use it in production code.
+// clangd-indexer is a tool to gather index data (symbols, xrefs) from source.
//
//===--===//
@@ -21,7 +20,6 @@
#include "clang/Tooling/Execution.h"
#include "clang/Tooling/Tooling.h"
#include "llvm/Support/CommandLine.h"
-#include "llvm/Support/Path.h"
#include "llvm/Support/Signals.h"
using namespace llvm;
@@ -32,30 +30,20 @@
namespace clangd {
namespace {
-static llvm::cl::opt AssumedHeaderDir(
-"assume-header-dir",
-llvm::cl::desc("The index includes header that a symbol is defined in. "
- "If the absolute path cannot be determined (e.g. an "
- "in-memory VFS) then the relative path is resolved against "
- "this directory, which must be absolute. If this flag is "
- "not given, such headers will have relative paths."),
-llvm::cl::init(""));
-
static llvm::cl::opt
Format("format", llvm::cl::desc("Format of the index to be written"),
[PATCH] D52782: [clang-tidy] Sequence statements with multiple parents correctly (PR39149)
JonasToth added a comment. Thanks for clarification :) Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52782 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52873: Remove unwanted signedness conversion from tests
mantognini created this revision.
mantognini added a reviewer: Anastasia.
Herald added a subscriber: cfe-commits.
This is just a minor update of the tests.
Repository:
rC Clang
https://reviews.llvm.org/D52873
Files:
test/SemaOpenCL/cl20-device-side-enqueue.cl
Index: test/SemaOpenCL/cl20-device-side-enqueue.cl
===
--- test/SemaOpenCL/cl20-device-side-enqueue.cl
+++ test/SemaOpenCL/cl20-device-side-enqueue.cl
@@ -212,15 +212,15 @@
#pragma OPENCL EXTENSION cl_khr_subgroups : enable
-kernel void foo(global int *buf)
+kernel void foo(global unsigned int *buf)
{
ndrange_t n;
buf[0] = get_kernel_max_sub_group_size_for_ndrange(n, ^(){});
buf[0] = get_kernel_max_sub_group_size_for_ndrange(0, ^(){}); //
expected-error{{illegal call to 'get_kernel_max_sub_group_size_for_ndrange',
expected 'ndrange_t' argument type}}
buf[0] = get_kernel_max_sub_group_size_for_ndrange(n, 1); //
expected-error{{illegal call to 'get_kernel_max_sub_group_size_for_ndrange',
expected block argument type}}
}
-kernel void bar(global int *buf)
+kernel void bar(global unsigned int *buf)
{
__private ndrange_t n;
buf[0] = get_kernel_sub_group_count_for_ndrange(n, ^(){});
@@ -230,13 +230,13 @@
#pragma OPENCL EXTENSION cl_khr_subgroups : disable
-kernel void foo1(global int *buf)
+kernel void foo1(global unsigned int *buf)
{
ndrange_t n;
buf[0] = get_kernel_max_sub_group_size_for_ndrange(n, ^(){}); //
expected-error {{use of declaration 'get_kernel_max_sub_group_size_for_ndrange'
requires cl_khr_subgroups extension to be enabled}}
}
-kernel void bar1(global int *buf)
+kernel void bar1(global unsigned int *buf)
{
ndrange_t n;
buf[0] = get_kernel_sub_group_count_for_ndrange(n, ^(){}); // expected-error
{{use of declaration 'get_kernel_sub_group_count_for_ndrange' requires
cl_khr_subgroups extension to be enabled}}
Index: test/SemaOpenCL/cl20-device-side-enqueue.cl
===
--- test/SemaOpenCL/cl20-device-side-enqueue.cl
+++ test/SemaOpenCL/cl20-device-side-enqueue.cl
@@ -212,15 +212,15 @@
#pragma OPENCL EXTENSION cl_khr_subgroups : enable
-kernel void foo(global int *buf)
+kernel void foo(global unsigned int *buf)
{
ndrange_t n;
buf[0] = get_kernel_max_sub_group_size_for_ndrange(n, ^(){});
buf[0] = get_kernel_max_sub_group_size_for_ndrange(0, ^(){}); // expected-error{{illegal call to 'get_kernel_max_sub_group_size_for_ndrange', expected 'ndrange_t' argument type}}
buf[0] = get_kernel_max_sub_group_size_for_ndrange(n, 1); // expected-error{{illegal call to 'get_kernel_max_sub_group_size_for_ndrange', expected block argument type}}
}
-kernel void bar(global int *buf)
+kernel void bar(global unsigned int *buf)
{
__private ndrange_t n;
buf[0] = get_kernel_sub_group_count_for_ndrange(n, ^(){});
@@ -230,13 +230,13 @@
#pragma OPENCL EXTENSION cl_khr_subgroups : disable
-kernel void foo1(global int *buf)
+kernel void foo1(global unsigned int *buf)
{
ndrange_t n;
buf[0] = get_kernel_max_sub_group_size_for_ndrange(n, ^(){}); // expected-error {{use of declaration 'get_kernel_max_sub_group_size_for_ndrange' requires cl_khr_subgroups extension to be enabled}}
}
-kernel void bar1(global int *buf)
+kernel void bar1(global unsigned int *buf)
{
ndrange_t n;
buf[0] = get_kernel_sub_group_count_for_ndrange(n, ^(){}); // expected-error {{use of declaration 'get_kernel_sub_group_count_for_ndrange' requires cl_khr_subgroups extension to be enabled}}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52782: [clang-tidy] Sequence statements with multiple parents correctly (PR39149)
mboehme marked 2 inline comments as done. mboehme added a comment. In https://reviews.llvm.org/D52782#1254933, @JonasToth wrote: > Thanks for clarification :) Thanks! Do you agree this is ready to land now? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52782 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52872: [clangd] Make binary index format the default, remove dead flag.
ilya-biryukov added inline comments. Comment at: clangd/indexer/IndexerMain.cpp:84 - $ clangd-indexer --executor=all-TUs compile_commands.json > index.yaml + $ clangd-indexer --executor=all-TUs compile_commands.json > clangd-index Maybe we should suggest a default extensions here? E.g. `index.riff`? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52872 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52875: Fix definitions of __builtin_(add|sub|mul)_overflow
mantognini created this revision. mantognini added reviewers: Anastasia, erichkeane. Herald added subscribers: cfe-commits, kristina. Ensure __builtin_(add|sub|mul)_overflow return bool instead of void as per specification (LanguageExtensions). Repository: rC Clang https://reviews.llvm.org/D52875 Files: include/clang/Basic/Builtins.def Index: include/clang/Basic/Builtins.def === --- include/clang/Basic/Builtins.def +++ include/clang/Basic/Builtins.def @@ -1397,9 +1397,9 @@ BUILTIN(__builtin_subcll, "ULLiULLiCULLiCULLiCULLi*", "n") // Checked Arithmetic Builtins for Security. -BUILTIN(__builtin_add_overflow, "v.", "nt") -BUILTIN(__builtin_sub_overflow, "v.", "nt") -BUILTIN(__builtin_mul_overflow, "v.", "nt") +BUILTIN(__builtin_add_overflow, "b.", "nt") +BUILTIN(__builtin_sub_overflow, "b.", "nt") +BUILTIN(__builtin_mul_overflow, "b.", "nt") BUILTIN(__builtin_uadd_overflow, "bUiCUiCUi*", "n") BUILTIN(__builtin_uaddl_overflow, "bULiCULiCULi*", "n") BUILTIN(__builtin_uaddll_overflow, "bULLiCULLiCULLi*", "n") Index: include/clang/Basic/Builtins.def === --- include/clang/Basic/Builtins.def +++ include/clang/Basic/Builtins.def @@ -1397,9 +1397,9 @@ BUILTIN(__builtin_subcll, "ULLiULLiCULLiCULLiCULLi*", "n") // Checked Arithmetic Builtins for Security. -BUILTIN(__builtin_add_overflow, "v.", "nt") -BUILTIN(__builtin_sub_overflow, "v.", "nt") -BUILTIN(__builtin_mul_overflow, "v.", "nt") +BUILTIN(__builtin_add_overflow, "b.", "nt") +BUILTIN(__builtin_sub_overflow, "b.", "nt") +BUILTIN(__builtin_mul_overflow, "b.", "nt") BUILTIN(__builtin_uadd_overflow, "bUiCUiCUi*", "n") BUILTIN(__builtin_uaddl_overflow, "bULiCULiCULi*", "n") BUILTIN(__builtin_uaddll_overflow, "bULLiCULLiCULLi*", "n") ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52877: [Index] Respect "IndexFunctionLocals" option during index.
hokein created this revision.
hokein added a reviewer: sammccall.
Herald added subscribers: kadircet, arphaman, ioeric, ilya-biryukov.
Previously, clangd index ignored symbols defined in the function body even
IndexFunctionLocals is true.
Repository:
rC Clang
https://reviews.llvm.org/D52877
Files:
lib/Index/IndexTypeSourceInfo.cpp
test/Index/index-local-symbol.cpp
Index: test/Index/index-local-symbol.cpp
===
--- /dev/null
+++ test/Index/index-local-symbol.cpp
@@ -0,0 +1,6 @@
+void ff() {
+ struct Foo {};
+}
+
+// RUN: env CINDEXTEST_INDEXLOCALSYMBOLS=1 c-index-test -index-file %s |
FileCheck %s
+// CHECK: [indexDeclaration]: kind: struct | name: Foo | {{.*}} | loc: 2:10
\ No newline at end of file
Index: lib/Index/IndexTypeSourceInfo.cpp
===
--- lib/Index/IndexTypeSourceInfo.cpp
+++ lib/Index/IndexTypeSourceInfo.cpp
@@ -100,7 +100,8 @@
bool VisitTagTypeLoc(TagTypeLoc TL) {
TagDecl *D = TL.getDecl();
-if (D->getParentFunctionOrMethod())
+if (!IndexCtx.shouldIndexFunctionLocalSymbols() &&
+D->getParentFunctionOrMethod())
return true;
if (TL.isDefinition()) {
Index: test/Index/index-local-symbol.cpp
===
--- /dev/null
+++ test/Index/index-local-symbol.cpp
@@ -0,0 +1,6 @@
+void ff() {
+ struct Foo {};
+}
+
+// RUN: env CINDEXTEST_INDEXLOCALSYMBOLS=1 c-index-test -index-file %s | FileCheck %s
+// CHECK: [indexDeclaration]: kind: struct | name: Foo | {{.*}} | loc: 2:10
\ No newline at end of file
Index: lib/Index/IndexTypeSourceInfo.cpp
===
--- lib/Index/IndexTypeSourceInfo.cpp
+++ lib/Index/IndexTypeSourceInfo.cpp
@@ -100,7 +100,8 @@
bool VisitTagTypeLoc(TagTypeLoc TL) {
TagDecl *D = TL.getDecl();
-if (D->getParentFunctionOrMethod())
+if (!IndexCtx.shouldIndexFunctionLocalSymbols() &&
+D->getParentFunctionOrMethod())
return true;
if (TL.isDefinition()) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52877: [Index] Respect "IndexFunctionLocals" option during index.
sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. (can you update the title/summary to reflect that only local *types* are affected?) Repository: rC Clang https://reviews.llvm.org/D52877 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52872: [clangd] Make binary index format the default, remove dead flag.
sammccall added inline comments. Comment at: clangd/indexer/IndexerMain.cpp:84 - $ clangd-indexer --executor=all-TUs compile_commands.json > index.yaml + $ clangd-indexer --executor=all-TUs compile_commands.json > clangd-index hokein wrote: > ilya-biryukov wrote: > > Maybe we should suggest a default extensions here? E.g. `index.riff`? > +1 I considered this but am not sure it's useful: - means an unneccesary change to workflows if we change the format (happened once!) - riff is associated with audio, and I'd expect OSes that care about extensions to draw the wrong conclusion :-) Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52872 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52880: [clang-tidy] fix PR39167, bugprone-exception-escape hangs-up
JonasToth created this revision.
JonasToth added reviewers: alexfh, aaron.ballman, baloghadamsoftware.
Herald added subscribers: cfe-commits, rnkovacs, xazax.hun.
The check bugprone-exception-escape should not register
if -fno-exceptions is set for the compile options. Bailing out on non-cplusplus
and non-exceptions language options resolves the issue.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52880
Files:
clang-tidy/bugprone/ExceptionEscapeCheck.cpp
Index: clang-tidy/bugprone/ExceptionEscapeCheck.cpp
===
--- clang-tidy/bugprone/ExceptionEscapeCheck.cpp
+++ clang-tidy/bugprone/ExceptionEscapeCheck.cpp
@@ -186,6 +186,9 @@
}
void ExceptionEscapeCheck::registerMatchers(MatchFinder *Finder) {
+ if (!getLangOpts().CPlusPlus || !getLangOpts().CXXExceptions)
+return;
+
Finder->addMatcher(
functionDecl(allOf(throws(unless(isIgnored(IgnoredExceptions))),
anyOf(isNoThrow(), cxxDestructorDecl(),
Index: clang-tidy/bugprone/ExceptionEscapeCheck.cpp
===
--- clang-tidy/bugprone/ExceptionEscapeCheck.cpp
+++ clang-tidy/bugprone/ExceptionEscapeCheck.cpp
@@ -186,6 +186,9 @@
}
void ExceptionEscapeCheck::registerMatchers(MatchFinder *Finder) {
+ if (!getLangOpts().CPlusPlus || !getLangOpts().CXXExceptions)
+return;
+
Finder->addMatcher(
functionDecl(allOf(throws(unless(isIgnored(IgnoredExceptions))),
anyOf(isNoThrow(), cxxDestructorDecl(),
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52872: [clangd] Make binary index format the default, remove dead flag.
hokein added inline comments. Comment at: clangd/indexer/IndexerMain.cpp:58 SymbolCollector::Options Opts; -Opts.FallbackDir = AssumedHeaderDir; return createStaticIndexingAction( If we remove the `assume-header-dir`, we probably remove `FallbackDir` too? I can't remember why we added this option, there is no usage internally either. Comment at: clangd/indexer/IndexerMain.cpp:84 - $ clangd-indexer --executor=all-TUs compile_commands.json > index.yaml + $ clangd-indexer --executor=all-TUs compile_commands.json > clangd-index ilya-biryukov wrote: > Maybe we should suggest a default extensions here? E.g. `index.riff`? +1 Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52872 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52879: Derive builtin return type from its definition
mantognini created this revision.
mantognini added reviewers: Anastasia, spatel.
Herald added subscribers: cfe-commits, kristina.
mantognini added dependencies: D52873: Remove unwanted signedness conversion
from tests, D52875: Fix definitions of __builtin_(add|sub|mul)_overflow.
Prior to this patch, OpenCL code such as the following would attempt to create
a BranchInst with a non-bool argument:
if (enqueue_kernel(get_default_queue(), 0, nd, ^(void){})) /* ... */
This patch is a follow up on a similar issue with pipe builtin
operations. See commit r280800 and https://bugs.llvm.org/show_bug.cgi?id=30219.
This change, while being conservative on non-builtin functions,
should set the type of expressions invoking builtins to the
proper type, instead of defaulting to `bool` and requiring
manual overrides in Sema::CheckBuiltinFunctionCall.
In addition to tests for enqueue_kernel, the tests are extended to
check other OpenCL builtins.
Repository:
rC Clang
https://reviews.llvm.org/D52879
Files:
lib/Sema/SemaChecking.cpp
lib/Sema/SemaExpr.cpp
test/CodeGenOpenCL/builtins.cl
test/CodeGenOpenCL/pipe_builtin.cl
Index: test/CodeGenOpenCL/pipe_builtin.cl
===
--- test/CodeGenOpenCL/pipe_builtin.cl
+++ test/CodeGenOpenCL/pipe_builtin.cl
@@ -69,25 +69,3 @@
// CHECK: call i32 @__get_pipe_max_packets_wo(%opencl.pipe_wo_t* %{{.*}}, i32 4, i32 4)
*ptr = get_pipe_max_packets(p);
}
-
-void test9(read_only pipe int r, write_only pipe int w, global int *ptr) {
- // verify that return type is correctly casted to i1 value
- // CHECK: %[[R:[0-9]+]] = call i32 @__read_pipe_2
- // CHECK: icmp ne i32 %[[R]], 0
- if (read_pipe(r, ptr)) *ptr = -1;
- // CHECK: %[[W:[0-9]+]] = call i32 @__write_pipe_2
- // CHECK: icmp ne i32 %[[W]], 0
- if (write_pipe(w, ptr)) *ptr = -1;
- // CHECK: %[[NR:[0-9]+]] = call i32 @__get_pipe_num_packets_ro
- // CHECK: icmp ne i32 %[[NR]], 0
- if (get_pipe_num_packets(r)) *ptr = -1;
- // CHECK: %[[NW:[0-9]+]] = call i32 @__get_pipe_num_packets_wo
- // CHECK: icmp ne i32 %[[NW]], 0
- if (get_pipe_num_packets(w)) *ptr = -1;
- // CHECK: %[[MR:[0-9]+]] = call i32 @__get_pipe_max_packets_ro
- // CHECK: icmp ne i32 %[[MR]], 0
- if (get_pipe_max_packets(r)) *ptr = -1;
- // CHECK: %[[MW:[0-9]+]] = call i32 @__get_pipe_max_packets_wo
- // CHECK: icmp ne i32 %[[MW]], 0
- if (get_pipe_max_packets(w)) *ptr = -1;
-}
Index: test/CodeGenOpenCL/builtins.cl
===
--- /dev/null
+++ test/CodeGenOpenCL/builtins.cl
@@ -0,0 +1,83 @@
+// RUN: %clang_cc1 %s -finclude-default-header -cl-std=CL2.0 -O0 -emit-llvm -o - -triple "spir-unknown-unknown" | FileCheck %s
+
+void testBranchingOnEnqueueKernel(queue_t default_queue, unsigned flags, ndrange_t ndrange) {
+// Ensure `enqueue_kernel` can be branched upon.
+
+if (enqueue_kernel(default_queue, flags, ndrange, ^(void) {}))
+(void)0;
+// CHECK: [[P:%[0-9]+]] = call i32 @__enqueue_kernel
+// CHECK-NEXT: [[Q:%[a-z0-9]+]] = icmp ne i32 [[P]], 0
+// CHECK-NEXT: br i1 [[Q]]
+
+if (get_kernel_work_group_size(^(void) {}))
+(void)0;
+// CHECK: [[P:%[0-9]+]] = call i32 @__get_kernel_work_group_size
+// CHECK-NEXT: [[Q:%[a-z0-9]+]] = icmp ne i32 [[P]], 0
+// CHECK-NEXT: br i1 [[Q]]
+
+if (get_kernel_preferred_work_group_size_multiple(^(void) {}))
+(void)0;
+// CHECK: [[P:%[0-9]+]] = call i32 @__get_kernel_preferred_work_group_size_multiple_impl
+// CHECK-NEXT: [[Q:%[a-z0-9]+]] = icmp ne i32 [[P]], 0
+// CHECK-NEXT: br i1 [[Q]]
+}
+
+void testBranchinOnPipeOperations(read_only pipe int r, write_only pipe int w, global int* ptr) {
+// Verify that return type is correctly casted to i1 value.
+
+if (read_pipe(r, ptr))
+(void)0;
+// CHECK: [[R:%[0-9]+]] = call i32 @__read_pipe_2
+// CHECK-NEXT: icmp ne i32 [[R]], 0
+
+if (write_pipe(w, ptr))
+(void)0;
+// CHECK: [[R:%[0-9]+]] = call i32 @__write_pipe_2
+// CHECK-NEXT: icmp ne i32 [[R]], 0
+
+if (get_pipe_num_packets(r))
+(void)0;
+// CHECK: [[R:%[0-9]+]] = call i32 @__get_pipe_num_packets_ro
+// CHECK-NEXT: icmp ne i32 [[R]], 0
+
+if (get_pipe_num_packets(w))
+(void)0;
+// CHECK: [[R:%[0-9]+]] = call i32 @__get_pipe_num_packets_wo
+// CHECK-NEXT: icmp ne i32 [[R]], 0
+
+if (get_pipe_max_packets(r))
+(void)0;
+// CHECK: [[R:%[0-9]+]] = call i32 @__get_pipe_max_packets_ro
+// CHECK-NEXT: icmp ne i32 [[R]], 0
+
+if (get_pipe_max_packets(w))
+(void)0;
+// CHECK: [[R:%[0-9]+]] = call i32 @__get_pipe_max_packets_wo
+// CHECK-NEXT: icmp ne i32 [[R]], 0
+}
+
+void testBranchingOnAddressSpaceCast(generic long* ptr) {
+// Verify that pointer types are properly casted, respecting address spaces.
+
+if (to_global(ptr))
+(void)0;
+// CHECK: [[P:%[0-9]+]] = call [[GLOBAL_VOID:i8
[PATCH] D52782: [clang-tidy] Sequence statements with multiple parents correctly (PR39149)
JonasToth added a comment. In https://reviews.llvm.org/D52782#1254945, @mboehme wrote: > In https://reviews.llvm.org/D52782#1254933, @JonasToth wrote: > > > Thanks for clarification :) > > > Thanks! Do you agree this is ready to land now? Sure! Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52782 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52880: [clang-tidy] fix PR39167, bugprone-exception-escape hangs-up
JonasToth added a comment. This should definitly be backported, as exception-escape is in 7.0 and it hangs up on no-except builds Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52880 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52880: [clang-tidy] fix PR39167, bugprone-exception-escape hangs-up
lebedev.ri added a comment. Needs a test. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52880 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r343767 - [Index] Respect "IndexFunctionLocals" option for type loc.
Author: hokein
Date: Thu Oct 4 04:03:55 2018
New Revision: 343767
URL: http://llvm.org/viewvc/llvm-project?rev=343767&view=rev
Log:
[Index] Respect "IndexFunctionLocals" option for type loc.
Summary:
Previously, clang index ignored local symbols defined in the function body even
IndexFunctionLocals is true.
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: ilya-biryukov, ioeric, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D52877
Added:
cfe/trunk/test/Index/index-local-symbol.cpp
Modified:
cfe/trunk/lib/Index/IndexTypeSourceInfo.cpp
Modified: cfe/trunk/lib/Index/IndexTypeSourceInfo.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Index/IndexTypeSourceInfo.cpp?rev=343767&r1=343766&r2=343767&view=diff
==
--- cfe/trunk/lib/Index/IndexTypeSourceInfo.cpp (original)
+++ cfe/trunk/lib/Index/IndexTypeSourceInfo.cpp Thu Oct 4 04:03:55 2018
@@ -100,7 +100,8 @@ public:
bool VisitTagTypeLoc(TagTypeLoc TL) {
TagDecl *D = TL.getDecl();
-if (D->getParentFunctionOrMethod())
+if (!IndexCtx.shouldIndexFunctionLocalSymbols() &&
+D->getParentFunctionOrMethod())
return true;
if (TL.isDefinition()) {
Added: cfe/trunk/test/Index/index-local-symbol.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/index-local-symbol.cpp?rev=343767&view=auto
==
--- cfe/trunk/test/Index/index-local-symbol.cpp (added)
+++ cfe/trunk/test/Index/index-local-symbol.cpp Thu Oct 4 04:03:55 2018
@@ -0,0 +1,6 @@
+void ff() {
+ struct Foo {};
+}
+
+// RUN: env CINDEXTEST_INDEXLOCALSYMBOLS=1 c-index-test -index-file %s |
FileCheck %s
+// CHECK: [indexDeclaration]: kind: struct | name: Foo | {{.*}} | loc: 2:10
\ No newline at end of file
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52877: [Index] Respect "IndexFunctionLocals" option for type loc.
This revision was automatically updated to reflect the committed changes.
Closed by commit rC343767: [Index] Respect "IndexFunctionLocals"
option for type loc. (authored by hokein, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D52877?vs=168261&id=168263#toc
Repository:
rC Clang
https://reviews.llvm.org/D52877
Files:
lib/Index/IndexTypeSourceInfo.cpp
test/Index/index-local-symbol.cpp
Index: test/Index/index-local-symbol.cpp
===
--- test/Index/index-local-symbol.cpp
+++ test/Index/index-local-symbol.cpp
@@ -0,0 +1,6 @@
+void ff() {
+ struct Foo {};
+}
+
+// RUN: env CINDEXTEST_INDEXLOCALSYMBOLS=1 c-index-test -index-file %s |
FileCheck %s
+// CHECK: [indexDeclaration]: kind: struct | name: Foo | {{.*}} | loc: 2:10
\ No newline at end of file
Index: lib/Index/IndexTypeSourceInfo.cpp
===
--- lib/Index/IndexTypeSourceInfo.cpp
+++ lib/Index/IndexTypeSourceInfo.cpp
@@ -100,7 +100,8 @@
bool VisitTagTypeLoc(TagTypeLoc TL) {
TagDecl *D = TL.getDecl();
-if (D->getParentFunctionOrMethod())
+if (!IndexCtx.shouldIndexFunctionLocalSymbols() &&
+D->getParentFunctionOrMethod())
return true;
if (TL.isDefinition()) {
Index: test/Index/index-local-symbol.cpp
===
--- test/Index/index-local-symbol.cpp
+++ test/Index/index-local-symbol.cpp
@@ -0,0 +1,6 @@
+void ff() {
+ struct Foo {};
+}
+
+// RUN: env CINDEXTEST_INDEXLOCALSYMBOLS=1 c-index-test -index-file %s | FileCheck %s
+// CHECK: [indexDeclaration]: kind: struct | name: Foo | {{.*}} | loc: 2:10
\ No newline at end of file
Index: lib/Index/IndexTypeSourceInfo.cpp
===
--- lib/Index/IndexTypeSourceInfo.cpp
+++ lib/Index/IndexTypeSourceInfo.cpp
@@ -100,7 +100,8 @@
bool VisitTagTypeLoc(TagTypeLoc TL) {
TagDecl *D = TL.getDecl();
-if (D->getParentFunctionOrMethod())
+if (!IndexCtx.shouldIndexFunctionLocalSymbols() &&
+D->getParentFunctionOrMethod())
return true;
if (TL.isDefinition()) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52880: [clang-tidy] fix PR39167, bugprone-exception-escape hangs-up
JonasToth added a comment. In https://reviews.llvm.org/D52880#1255031, @lebedev.ri wrote: > Needs a test. How shall i test it? It feels that this condition should have been there from the beginning on, as the check only makes sense in c++ and with exceptions. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52880 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52880: [clang-tidy] fix PR39167, bugprone-exception-escape hangs-up
lebedev.ri added a comment. In https://reviews.llvm.org/D52880#1255035, @JonasToth wrote: > In https://reviews.llvm.org/D52880#1255031, @lebedev.ri wrote: > > > Needs a test. > > > How shall i test it? It feels that this condition should have been there from > the beginning on, as the check only makes sense in c++ and with exceptions. I'm guessing you need a run line with `-x c`, and with `-x c++ -fno-exceptions`, not sure about the contents of the file, but the clang-tidy crashes are easy to creduce. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52880 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52808: [cland] Dex: fix/simplify trigram generation
sammccall marked an inline comment as done. sammccall added a comment. **TL;DR**: i'm no longer convinced of my conclusions for short-query, look at the proposal below. In https://reviews.llvm.org/D52808#1254899, @ioeric wrote: > > Generate more short-query trigrams, e.g. "AbcDefGhi" now yields "d" and > > "ag". > > I am concerned about the impact on the size of posting lists (we can measure) > and retrieval quality by adding more incomplete trigrams. There is indeed an impact here. Before: 22309480, After: 22531144. About 1/3 of this is posting lists, so 1% to overall, 3% on posting lists. I haven't measured quality (don't have a good way to do that for dex currently). It's true that the new results we're admitting are probably worse as they don't start with the right letter. >> This is effectively required by LSP, having "ag" not match but "agh" match >> will lead to glitches due to client-side filtering. > > It seems hard to make index behave the same as LSP clients, considering all > the ranking signals we have; there can be other reasons that cause `having > "ag" not match but "agh" match`. No, there's only one such reason: we truncated the result list (the symbol matched, but it wasn't one of the best N). This reason is accounted for by LSP: client-side filtering only occurs when there was no truncation. > And it seems reasonable to assume that you would get better symbols as you > type more characters (i.e. stronger relevance signal). The problem is LSP clients are free to assume that the result list is complete (unless marked as incomplete) and therefore will never retrieve the better symbols. >> Drop leading-punctuation short-query trigrams. Nice idea, but current >> implementation is broken (competes with non-punctuation short query >> trigrams). > > Could you elaborate how this is broken? We should probably fix it instead of > removing it. `__` not matching `__some_macro` sounds like a regression. `fuzzyFind` considers `_ptr` to match `unique_ptr`, so it needs to consider `_` to match it too. We will still match `__some_macro`, but the posting lists will match ~everything and then postfilter. It may make sense to do optimizations or tweak overall trigram generation for this case, but the way it's currently done seems unprincipled and a little broken. --- OK, so after some thought on the tradeoffs here, what about this alternative design: - for query length <3, we support really restrictive matches: exact prefix, or first char + next head char. We - we get around the LSP restrictions by always marking such result sets as incomplete Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52808 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r343768 - [clang-tidy] Sequence statements with multiple parents correctly (PR39149)
Author: mboehme
Date: Thu Oct 4 04:36:39 2018
New Revision: 343768
URL: http://llvm.org/viewvc/llvm-project?rev=343768&view=rev
Log:
[clang-tidy] Sequence statements with multiple parents correctly (PR39149)
Summary:
Before this fix, the bugprone-use-after-move check could incorrectly
conclude that a use and move in a function template were not sequenced.
For details, see
https://bugs.llvm.org/show_bug.cgi?id=39149
Reviewers: alexfh, hokein, aaron.ballman, JonasToth
Reviewed By: aaron.ballman
Subscribers: xazax.hun, cfe-commits
Tags: #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D52782
Modified:
clang-tools-extra/trunk/clang-tidy/bugprone/UseAfterMoveCheck.cpp
clang-tools-extra/trunk/clang-tidy/utils/ExprSequence.cpp
clang-tools-extra/trunk/clang-tidy/utils/ExprSequence.h
clang-tools-extra/trunk/test/clang-tidy/bugprone-use-after-move.cpp
Modified: clang-tools-extra/trunk/clang-tidy/bugprone/UseAfterMoveCheck.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/bugprone/UseAfterMoveCheck.cpp?rev=343768&r1=343767&r2=343768&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/bugprone/UseAfterMoveCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/bugprone/UseAfterMoveCheck.cpp Thu Oct
4 04:36:39 2018
@@ -102,8 +102,9 @@ bool UseAfterMoveFinder::find(Stmt *Func
if (!TheCFG)
return false;
- Sequence.reset(new ExprSequence(TheCFG.get(), Context));
- BlockMap.reset(new StmtToBlockMap(TheCFG.get(), Context));
+ Sequence =
+ llvm::make_unique(TheCFG.get(), FunctionBody, Context);
+ BlockMap = llvm::make_unique(TheCFG.get(), Context);
Visited.clear();
const CFGBlock *Block = BlockMap->blockContainingStmt(MovingCall);
Modified: clang-tools-extra/trunk/clang-tidy/utils/ExprSequence.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/utils/ExprSequence.cpp?rev=343768&r1=343767&r2=343768&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/utils/ExprSequence.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/utils/ExprSequence.cpp Thu Oct 4
04:36:39 2018
@@ -63,8 +63,9 @@ bool isDescendantOrEqual(const Stmt *Des
}
}
-ExprSequence::ExprSequence(const CFG *TheCFG, ASTContext *TheContext)
-: Context(TheContext) {
+ExprSequence::ExprSequence(const CFG *TheCFG, const Stmt *Root,
+ ASTContext *TheContext)
+: Context(TheContext), Root(Root) {
for (const auto &SyntheticStmt : TheCFG->synthetic_stmts()) {
SyntheticStmtSourceMap[SyntheticStmt.first] = SyntheticStmt.second;
}
@@ -99,6 +100,11 @@ bool ExprSequence::potentiallyAfter(cons
const Stmt *ExprSequence::getSequenceSuccessor(const Stmt *S) const {
for (const Stmt *Parent : getParentStmts(S, Context)) {
+// If a statement has multiple parents, make sure we're using the parent
+// that lies within the sub-tree under Root.
+if (!isDescendantOrEqual(Parent, Root, Context))
+ continue;
+
if (const auto *BO = dyn_cast(Parent)) {
// Comma operator: Right-hand side is sequenced after the left-hand side.
if (BO->getLHS() == S && BO->getOpcode() == BO_Comma)
Modified: clang-tools-extra/trunk/clang-tidy/utils/ExprSequence.h
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/utils/ExprSequence.h?rev=343768&r1=343767&r2=343768&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/utils/ExprSequence.h (original)
+++ clang-tools-extra/trunk/clang-tidy/utils/ExprSequence.h Thu Oct 4 04:36:39
2018
@@ -69,8 +69,8 @@ namespace utils {
class ExprSequence {
public:
/// Initializes this `ExprSequence` with sequence information for the given
- /// `CFG`.
- ExprSequence(const CFG *TheCFG, ASTContext *TheContext);
+ /// `CFG`. `Root` is the root statement the CFG was built from.
+ ExprSequence(const CFG *TheCFG, const Stmt *Root, ASTContext *TheContext);
/// Returns whether \p Before is sequenced before \p After.
bool inSequence(const Stmt *Before, const Stmt *After) const;
@@ -94,6 +94,7 @@ private:
const Stmt *resolveSyntheticStmt(const Stmt *S) const;
ASTContext *Context;
+ const Stmt *Root;
llvm::DenseMap SyntheticStmtSourceMap;
};
Modified: clang-tools-extra/trunk/test/clang-tidy/bugprone-use-after-move.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/bugprone-use-after-move.cpp?rev=343768&r1=343767&r2=343768&view=diff
==
--- clang-tools-extra/trunk/test/clang-tidy/bugprone-use-after-move.cpp
(original)
+++ clang-tools-extra/trunk/test/clang-tidy/bugprone-use-after-move.cpp Thu Oct
4 04:36:39 2018
@@ -1195,6 +1195,18 @@ void ifWhileAndSwitchSequenceInitDeclAnd
[PATCH] D52782: [clang-tidy] Sequence statements with multiple parents correctly (PR39149)
This revision was automatically updated to reflect the committed changes.
Closed by commit rL343768: [clang-tidy] Sequence statements with multiple
parents correctly (PR39149) (authored by mboehme, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D52782
Files:
clang-tools-extra/trunk/clang-tidy/bugprone/UseAfterMoveCheck.cpp
clang-tools-extra/trunk/clang-tidy/utils/ExprSequence.cpp
clang-tools-extra/trunk/clang-tidy/utils/ExprSequence.h
clang-tools-extra/trunk/test/clang-tidy/bugprone-use-after-move.cpp
Index: clang-tools-extra/trunk/clang-tidy/bugprone/UseAfterMoveCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/bugprone/UseAfterMoveCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/bugprone/UseAfterMoveCheck.cpp
@@ -102,8 +102,9 @@
if (!TheCFG)
return false;
- Sequence.reset(new ExprSequence(TheCFG.get(), Context));
- BlockMap.reset(new StmtToBlockMap(TheCFG.get(), Context));
+ Sequence =
+ llvm::make_unique(TheCFG.get(), FunctionBody, Context);
+ BlockMap = llvm::make_unique(TheCFG.get(), Context);
Visited.clear();
const CFGBlock *Block = BlockMap->blockContainingStmt(MovingCall);
Index: clang-tools-extra/trunk/clang-tidy/utils/ExprSequence.h
===
--- clang-tools-extra/trunk/clang-tidy/utils/ExprSequence.h
+++ clang-tools-extra/trunk/clang-tidy/utils/ExprSequence.h
@@ -69,8 +69,8 @@
class ExprSequence {
public:
/// Initializes this `ExprSequence` with sequence information for the given
- /// `CFG`.
- ExprSequence(const CFG *TheCFG, ASTContext *TheContext);
+ /// `CFG`. `Root` is the root statement the CFG was built from.
+ ExprSequence(const CFG *TheCFG, const Stmt *Root, ASTContext *TheContext);
/// Returns whether \p Before is sequenced before \p After.
bool inSequence(const Stmt *Before, const Stmt *After) const;
@@ -94,6 +94,7 @@
const Stmt *resolveSyntheticStmt(const Stmt *S) const;
ASTContext *Context;
+ const Stmt *Root;
llvm::DenseMap SyntheticStmtSourceMap;
};
Index: clang-tools-extra/trunk/clang-tidy/utils/ExprSequence.cpp
===
--- clang-tools-extra/trunk/clang-tidy/utils/ExprSequence.cpp
+++ clang-tools-extra/trunk/clang-tidy/utils/ExprSequence.cpp
@@ -63,8 +63,9 @@
}
}
-ExprSequence::ExprSequence(const CFG *TheCFG, ASTContext *TheContext)
-: Context(TheContext) {
+ExprSequence::ExprSequence(const CFG *TheCFG, const Stmt *Root,
+ ASTContext *TheContext)
+: Context(TheContext), Root(Root) {
for (const auto &SyntheticStmt : TheCFG->synthetic_stmts()) {
SyntheticStmtSourceMap[SyntheticStmt.first] = SyntheticStmt.second;
}
@@ -99,6 +100,11 @@
const Stmt *ExprSequence::getSequenceSuccessor(const Stmt *S) const {
for (const Stmt *Parent : getParentStmts(S, Context)) {
+// If a statement has multiple parents, make sure we're using the parent
+// that lies within the sub-tree under Root.
+if (!isDescendantOrEqual(Parent, Root, Context))
+ continue;
+
if (const auto *BO = dyn_cast(Parent)) {
// Comma operator: Right-hand side is sequenced after the left-hand side.
if (BO->getLHS() == S && BO->getOpcode() == BO_Comma)
Index: clang-tools-extra/trunk/test/clang-tidy/bugprone-use-after-move.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/bugprone-use-after-move.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/bugprone-use-after-move.cpp
@@ -1195,6 +1195,18 @@
}
}
+// Some statements in templates (e.g. null, break and continue statements) may
+// be shared between the uninstantiated and instantiated versions of the
+// template and therefore have multiple parents. Make sure the sequencing code
+// handles this correctly.
+template void nullStatementSequencesInTemplate() {
+ int c = 0;
+ (void)c;
+ ;
+ std::move(c);
+}
+template void nullStatementSequencesInTemplate();
+
namespace PR33020 {
class D {
~D();
Index: clang-tools-extra/trunk/clang-tidy/bugprone/UseAfterMoveCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/bugprone/UseAfterMoveCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/bugprone/UseAfterMoveCheck.cpp
@@ -102,8 +102,9 @@
if (!TheCFG)
return false;
- Sequence.reset(new ExprSequence(TheCFG.get(), Context));
- BlockMap.reset(new StmtToBlockMap(TheCFG.get(), Context));
+ Sequence =
+ llvm::make_unique(TheCFG.get(), FunctionBody, Context);
+ BlockMap = llvm::make_unique(TheCFG.get(), Context);
Visited.clear();
const CFGBlock *Block = BlockMap->blockContainingStmt(MovingCall);
Index: clang-tools-extra/trunk/clang-tidy/utils/ExprSequence.h
===
--- clang-tool
[PATCH] D52773: clang-cl: Add /showFilenames option (PR31957)
hans added a comment.
In https://reviews.llvm.org/D52773#1252584, @zturner wrote:
> Canyou add a test that demonstrates that multiple input files on the command
> line will print all of them?
Will do.
> Also does this really need to be a cc1 arg? Why not just print it in the
> driver?
Yeah, I'll give printing from the driver a try.
Comment at: include/clang/Driver/CLCompatOptions.td:163
+ HelpText<"Print the name of each compiled file">,
+ Alias;
def _SLASH_source_charset : CLCompileJoined<"source-charset:">,
thakis wrote:
> Can you add /showFilenames- (a no-op) too, for symmetry?
Will do. Actually not just a no-op, but the last option should win.
Comment at: lib/Driver/ToolChains/Clang.cpp:3533-3534
CmdArgs.push_back(getBaseInputName(Args, Input));
+ if (Arg *A = Args.getLastArg(options::OPT_echo_main_file_name))
+A->render(Args, CmdArgs);
rnk wrote:
> Is a -cc1 flag actually necessary? I was imagining we'd print it in the
> driver before we launch the compile job.
Yeah, that's probably better, but will take a bit more work to keep track of if
and what to print in the compile job. I'll give it a try.
Comment at: lib/Frontend/CompilerInvocation.cpp:792
Opts.MainFileName = Args.getLastArgValue(OPT_main_file_name);
+ if (Args.hasArg(OPT_echo_main_file_name))
+llvm::outs() << Opts.MainFileName << "\n";
zturner wrote:
> steveire wrote:
> > zturner wrote:
> > > zturner wrote:
> > > > steveire wrote:
> > > > > hans wrote:
> > > > > > zturner wrote:
> > > > > > > steveire wrote:
> > > > > > > > I'll have to keep a patch around anyway for myself locally to
> > > > > > > > remove this condition. But maybe someone else will benefit from
> > > > > > > > this.
> > > > > > > >
> > > > > > > > What do you think about changing CMake to so that this is
> > > > > > > > passed by default when using Clang-CL?
> > > > > > > >
> > > > > > > Do you mean llvms cmake? Or cmake itself? Which generator are
> > > > > > > you using?
> > > > > > Can't you just configure your build to pass the flag to clang-cl?
> > > > > >
> > > > > > I suppose CMake could be made to do that by default when using
> > > > > > clang-cl versions that support the flag and using the MSBuild
> > > > > > generator, but that's for the CMake community to decide I think. Or
> > > > > > perhaps our VS integration could do that, but it gets tricky
> > > > > > because it needs to know whether the flag is supported or not.
> > > > > I mean upstream cmake. My suggestion is independent of the generator,
> > > > > but it would depend on what Brad decides anyway. I don't intend to
> > > > > implement it, just report it.
> > > > >
> > > > > I only have one clang but I have multiple buildsystems, and I don't
> > > > > want to have to add a new flag too all of them. In each toplevel
> > > > > CMakeLists everyone will need this to make use of the flag:
> > > > >
> > > > > ```
> > > > >
> > > > > # Have to remember to use "${CMAKE_CXX_SIMULATE_ID}" instead of
> > > > > # Undecorated "CMAKE_CXX_SIMULATE_ID" because there is a
> > > > > # variable named "MSVC" and
> > > > > # the way CMake variables and the "if()" command work requires this.
> > > > > if (CMAKE_CXX_COMPILER_ID STREQUAL Clang
> > > > > AND ${CMAKE_CXX_SIMULATE_ID} STREQUAL MSVC)
> > > > > # CMake does not have explicit Clang-CL support yet.
> > > > > # It should have a COMPILER_ID for it.
> > > > > set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /showFilename")
> > > > > # etc
> > > > > endif()
> > > > > ```
> > > > >
> > > > > Upstream CMake can have the logic to add the flag instead.
> > > > But then what if you don’t want it? There would be no way to turn it
> > > > off. I don’t think it’s a good fit for cmake
> > > Yes, and actually for this reason i was wondering if we can do it without
> > > a flag at all. What if we just set an magic environment variable? It
> > > handles Stephen’s use case (he can just set it globally), and MSBuild (we
> > > can set it in the extension).
> > > But then what if you don’t want it? There would be no way to turn it off.
> >
> > Oh, I thought that the last flag would dominate. ie, if cmake generated
> > `/showFilename` and the user adds `/showFilename-`, then you would end up
> > with
> >
> > `clang-cl.exe /showFilename /showFilename-`
> >
> > and it would not be shown. I guess that's not how it works.
> >
> > Maybe users want to not show it, but not seeing the filename is a
> > surprising first-time experience when porting from MSVC to clang-cl using
> > Visual Studio.
> >
> > However, I'll just drop out of the discussion and not make any bug to
> > CMake. If anyone else feels strongly, they can do so.
> >
> > Thanks!
> Right, but if we update the VS Integration Extension so that MSBuild
> specifies the flag (or sets the environment variable, etc), then any time you
> u
[PATCH] D52882: [clang-tidy] Added pointer types to clang-tidy readability-identifier-naming check.
ffigueras created this revision.
ffigueras added reviewers: alexfh, kbobyrev.
Herald added subscribers: cfe-commits, xazax.hun.
Option to check for different naming conventions on the following types:
- GlobalConstantPointer
- GlobalPointer
- LocalConstantPointer
- LocalPointer
- PointerParameter
- ConstantPointerParameter
When not specified, the conventions for the non pointer types will be applied
(GlobalConstant, GlobalVariable, LocalConstant, ...).
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52882
Files:
clang-tidy/readability/IdentifierNamingCheck.cpp
test/clang-tidy/readability-identifier-naming.cpp
Index: test/clang-tidy/readability-identifier-naming.cpp
===
--- test/clang-tidy/readability-identifier-naming.cpp
+++ test/clang-tidy/readability-identifier-naming.cpp
@@ -67,7 +67,19 @@
// RUN: {key: readability-identifier-naming.MacroDefinitionCase, value: UPPER_CASE}, \
// RUN: {key: readability-identifier-naming.TypeAliasCase, value: camel_Snake_Back}, \
// RUN: {key: readability-identifier-naming.TypeAliasSuffix, value: '_t'}, \
-// RUN: {key: readability-identifier-naming.IgnoreFailedSplit, value: 0} \
+// RUN: {key: readability-identifier-naming.IgnoreFailedSplit, value: 0}, \
+// RUN: {key: readability-identifier-naming.GlobalPointerCase, value: CamelCase}, \
+// RUN: {key: readability-identifier-naming.GlobalPointerSuffix, value: '_Ptr'}, \
+// RUN: {key: readability-identifier-naming.GlobalConstantPointerCase, value: UPPER_CASE}, \
+// RUN: {key: readability-identifier-naming.GlobalConstantPointerSuffix, value: '_Ptr'}, \
+// RUN: {key: readability-identifier-naming.PointerParameterCase, value: lower_case}, \
+// RUN: {key: readability-identifier-naming.PointerParameterPrefix, value: 'p_'}, \
+// RUN: {key: readability-identifier-naming.ConstantPointerParameterCase, value: CamelCase}, \
+// RUN: {key: readability-identifier-naming.ConstantPointerParameterPrefix, value: 'cp_'}, \
+// RUN: {key: readability-identifier-naming.LocalPointerCase, value: CamelCase}, \
+// RUN: {key: readability-identifier-naming.LocalPointerPrefix, value: 'l_'}, \
+// RUN: {key: readability-identifier-naming.LocalConstantPointerCase, value: CamelCase}, \
+// RUN: {key: readability-identifier-naming.LocalConstantPointerPrefix, value: 'lc_'}, \
// RUN: ]}' -- -std=c++11 -fno-delayed-template-parsing \
// RUN: -I%S/Inputs/readability-identifier-naming \
// RUN: -isystem %S/Inputs/readability-identifier-naming/system
@@ -235,8 +247,8 @@
CMyWellNamedClass2() = default;
CMyWellNamedClass2(CMyWellNamedClass2 const&) = default;
CMyWellNamedClass2(CMyWellNamedClass2 &&) = default;
- CMyWellNamedClass2(t_t a_v, void *a_p) : my_class(a_p), my_Bad_Member(a_v) {}
- // CHECK-FIXES: {{^}} CMyWellNamedClass2(t_t a_v, void *a_p) : CMyClass(a_p), __my_Bad_Member(a_v) {}{{$}}
+ CMyWellNamedClass2(t_t a_v, void *p_p) : my_class(p_p), my_Bad_Member(a_v) {}
+ // CHECK-FIXES: {{^}} CMyWellNamedClass2(t_t a_v, void *p_p) : CMyClass(p_p), __my_Bad_Member(a_v) {}{{$}}
CMyWellNamedClass2(t_t a_v) : my_class(), my_Bad_Member(a_v), my_Other_Bad_Member(11) {}
// CHECK-FIXES: {{^}} CMyWellNamedClass2(t_t a_v) : CMyClass(), __my_Bad_Member(a_v), __my_Other_Bad_Member(11) {}{{$}}
@@ -474,3 +486,18 @@
unsigned const MyConstGlobal_array[] = {1,2,3};
// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: invalid case style for global constant 'MyConstGlobal_array'
// CHECK-FIXES: {{^}}unsigned const MY_CONST_GLOBAL_ARRAY[] = {1,2,3};{{$}}
+
+int * MyGlobal_Ptr;// -> ok
+int * const MyConstantGlobalPointer = nullptr;
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: invalid case style for global constant pointer 'MyConstantGlobalPointer'
+// CHECK-FIXES: {{^}}int * const MY_CONSTANT_GLOBAL_POINTER_Ptr = nullptr;{{$}}
+
+void MyPoiterFunction(int * p_normal_pointer, int * const constant_ptr){
+// CHECK-MESSAGES: :[[@LINE-1]]:59: warning: invalid case style for constant pointer parameter 'constant_ptr'
+// CHECK-FIXES: {{^}}void MyPoiterFunction(int * p_normal_pointer, int * const cp_ConstantPtr){{{$}}
+int * l_PointerA;
+int * const pointer_b = nullptr;
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: invalid case style for local constant pointer 'pointer_b'
+// CHECK-FIXES: {{^}}int * const lc_PointerB = nullptr;{{$}}
+}
+
Index: clang-tidy/readability/IdentifierNamingCheck.cpp
===
--- clang-tidy/readability/IdentifierNamingCheck.cpp
+++ clang-tidy/readability/IdentifierNamingCheck.cpp
@@ -77,16 +77,22 @@
m(ClassConstant) \
m(ClassMember) \
m(GlobalConstant) \
+m(GlobalConstantPointer) \
+m(GlobalPointer) \
m(GlobalVariable) \
m(LocalConstant) \
+m(LocalConstantPointer) \
+m(LocalPointer) \
m(LocalVariable) \
m(StaticConstant) \
m(StaticVaria
[PATCH] D52727: [clang-tidy] White List Option for performance-unnecessary-value-param, performance-unnecessary-copy-initialization and performance-for-range-copy
baloghadamsoftware updated this revision to Diff 168274.
baloghadamsoftware added a comment.
Herald added a subscriber: mgorny.
Updated according to the comments.
https://reviews.llvm.org/D52727
Files:
clang-tidy/performance/ForRangeCopyCheck.cpp
clang-tidy/performance/ForRangeCopyCheck.h
clang-tidy/performance/UnnecessaryCopyInitialization.cpp
clang-tidy/performance/UnnecessaryCopyInitialization.h
clang-tidy/performance/UnnecessaryValueParamCheck.cpp
clang-tidy/performance/UnnecessaryValueParamCheck.h
clang-tidy/utils/CMakeLists.txt
clang-tidy/utils/Matchers.cpp
clang-tidy/utils/Matchers.h
docs/clang-tidy/checks/performance-for-range-copy.rst
docs/clang-tidy/checks/performance-unnecessary-copy-initialization.rst
docs/clang-tidy/checks/performance-unnecessary-value-param.rst
test/clang-tidy/performance-for-range-copy-allowed-types.cpp
test/clang-tidy/performance-unnecessary-copy-initialization-allowed-types.cpp
test/clang-tidy/performance-unnecessary-value-param-allowed-types.cpp
Index: test/clang-tidy/performance-unnecessary-value-param-allowed-types.cpp
===
--- /dev/null
+++ test/clang-tidy/performance-unnecessary-value-param-allowed-types.cpp
@@ -0,0 +1,66 @@
+// RUN: %check_clang_tidy %s performance-unnecessary-value-param %t -- -config="{CheckOptions: [{key: performance-unnecessary-value-param.AllowedTypes, value: '[Pp]ointer$;[Pp]tr$;[Rr]ef(erence)?$'}]}" --
+
+struct SmartPointer {
+ ~SmartPointer();
+};
+
+struct smart_pointer {
+ ~smart_pointer();
+};
+
+struct SmartPtr {
+ ~SmartPtr();
+};
+
+struct smart_ptr {
+ ~smart_ptr();
+};
+
+struct SmartReference {
+ ~SmartReference();
+};
+
+struct smart_reference {
+ ~smart_reference();
+};
+
+struct SmartRef {
+ ~SmartRef();
+};
+
+struct smart_ref {
+ ~smart_ref();
+};
+
+struct OtherType {
+ ~OtherType();
+};
+
+void negativeSmartPointer(SmartPointer P) {
+}
+
+void negative_smart_pointer(smart_pointer p) {
+}
+
+void negativeSmartPtr(SmartPtr P) {
+}
+
+void negative_smart_ptr(smart_ptr p) {
+}
+
+void negativeSmartReference(SmartReference R) {
+}
+
+void negative_smart_reference(smart_reference r) {
+}
+
+void negativeSmartRef(SmartRef R) {
+}
+
+void negative_smart_ref(smart_ref r) {
+}
+
+void positiveOtherType(OtherType O) {
+ // CHECK-MESSAGES: [[@LINE-1]]:34: warning: the parameter 'O' is copied for each invocation but only used as a const reference; consider making it a const reference [performance-unnecessary-value-param]
+ // CHECK-FIXES: void positiveOtherType(const OtherType& O) {
+}
Index: test/clang-tidy/performance-unnecessary-copy-initialization-allowed-types.cpp
===
--- /dev/null
+++ test/clang-tidy/performance-unnecessary-copy-initialization-allowed-types.cpp
@@ -0,0 +1,85 @@
+// RUN: %check_clang_tidy %s performance-unnecessary-copy-initialization %t -- -config="{CheckOptions: [{key: performance-unnecessary-copy-initialization.AllowedTypes, value: '[Pp]ointer$;[Pp]tr$;[Rr]ef(erence)?$'}]}" --
+
+struct SmartPointer {
+ ~SmartPointer();
+};
+
+struct smart_pointer {
+ ~smart_pointer();
+};
+
+struct SmartPtr {
+ ~SmartPtr();
+};
+
+struct smart_ptr {
+ ~smart_ptr();
+};
+
+struct SmartReference {
+ ~SmartReference();
+};
+
+struct smart_reference {
+ ~smart_reference();
+};
+
+struct SmartRef {
+ ~SmartRef();
+};
+
+struct smart_ref {
+ ~smart_ref();
+};
+
+struct OtherType {
+ ~OtherType();
+};
+
+const SmartPointer &getSmartPointer();
+const smart_pointer &get_smart_pointer();
+const SmartPtr &getSmartPtr();
+const smart_ptr &get_smart_ptr();
+const SmartReference &getSmartReference();
+const smart_reference &get_smart_reference();
+const SmartRef &getSmartRef();
+const smart_ref &get_smart_ref();
+const OtherType &getOtherType();
+
+void negativeSmartPointer() {
+ const auto P = getSmartPointer();
+}
+
+void negative_smart_pointer() {
+ const auto p = get_smart_pointer();
+}
+
+void negativeSmartPtr() {
+ const auto P = getSmartPtr();
+}
+
+void negative_smart_ptr() {
+ const auto p = get_smart_ptr();
+}
+
+void negativeSmartReference() {
+ const auto R = getSmartReference();
+}
+
+void negative_smart_reference() {
+ const auto r = get_smart_reference();
+}
+
+void negativeSmartRef() {
+ const auto R = getSmartRef();
+}
+
+void negative_smart_ref() {
+ const auto r = get_smart_ref();
+}
+
+void positiveOtherType() {
+ const auto O = getOtherType();
+ // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'O' is copy-constructed from a const reference; consider making it a const reference [performance-unnecessary-copy-initialization]
+ // CHECK-FIXES: const auto& O = getOtherType();
+}
Index: test/clang-tidy/performance-for-range-copy-allowed-types.cpp
===
--- /dev/null
+++ test/clang-tidy/performance-for-range-copy-allowed-types.cpp
@@ -0,0 +1,112 @@
+// RUN
Re: [PATCH] D52773: clang-cl: Add /showFilenames option (PR31957)
I agree magic environment variables are bad, but without it we don’t
address the only current actual use we have for this, which is making the
vs integration print filenames. Detecting compiler version from inside the
integration is hard, but with an environment variable it’s very easy to
solve.
I’m not against a flag as the supported way, but I think we should also
have some backdoor into this functionality, because if we’re not going to
satisfy the only known use case, then maybe it’s better to not even do it
at all.
On Thu, Oct 4, 2018 at 5:13 AM Hans Wennborg via Phabricator <
revi...@reviews.llvm.org> wrote:
> hans added a comment.
>
> In https://reviews.llvm.org/D52773#1252584, @zturner wrote:
>
> > Canyou add a test that demonstrates that multiple input files on the
> command line will print all of them?
>
>
> Will do.
>
> > Also does this really need to be a cc1 arg? Why not just print it in
> the driver?
>
> Yeah, I'll give printing from the driver a try.
>
>
>
>
> Comment at: include/clang/Driver/CLCompatOptions.td:163
> + HelpText<"Print the name of each compiled file">,
> + Alias;
> def _SLASH_source_charset : CLCompileJoined<"source-charset:">,
>
> thakis wrote:
> > Can you add /showFilenames- (a no-op) too, for symmetry?
> Will do. Actually not just a no-op, but the last option should win.
>
>
>
> Comment at: lib/Driver/ToolChains/Clang.cpp:3533-3534
>CmdArgs.push_back(getBaseInputName(Args, Input));
> + if (Arg *A = Args.getLastArg(options::OPT_echo_main_file_name))
> +A->render(Args, CmdArgs);
>
>
> rnk wrote:
> > Is a -cc1 flag actually necessary? I was imagining we'd print it in the
> driver before we launch the compile job.
> Yeah, that's probably better, but will take a bit more work to keep track
> of if and what to print in the compile job. I'll give it a try.
>
>
>
> Comment at: lib/Frontend/CompilerInvocation.cpp:792
>Opts.MainFileName = Args.getLastArgValue(OPT_main_file_name);
> + if (Args.hasArg(OPT_echo_main_file_name))
> +llvm::outs() << Opts.MainFileName << "\n";
>
> zturner wrote:
> > steveire wrote:
> > > zturner wrote:
> > > > zturner wrote:
> > > > > steveire wrote:
> > > > > > hans wrote:
> > > > > > > zturner wrote:
> > > > > > > > steveire wrote:
> > > > > > > > > I'll have to keep a patch around anyway for myself locally
> to remove this condition. But maybe someone else will benefit from this.
> > > > > > > > >
> > > > > > > > > What do you think about changing CMake to so that this is
> passed by default when using Clang-CL?
> > > > > > > > >
> > > > > > > > Do you mean llvms cmake? Or cmake itself? Which generator
> are you using?
> > > > > > > Can't you just configure your build to pass the flag to
> clang-cl?
> > > > > > >
> > > > > > > I suppose CMake could be made to do that by default when using
> clang-cl versions that support the flag and using the MSBuild generator,
> but that's for the CMake community to decide I think. Or perhaps our VS
> integration could do that, but it gets tricky because it needs to know
> whether the flag is supported or not.
> > > > > > I mean upstream cmake. My suggestion is independent of the
> generator, but it would depend on what Brad decides anyway. I don't intend
> to implement it, just report it.
> > > > > >
> > > > > > I only have one clang but I have multiple buildsystems, and I
> don't want to have to add a new flag too all of them. In each toplevel
> CMakeLists everyone will need this to make use of the flag:
> > > > > >
> > > > > > ```
> > > > > >
> > > > > > # Have to remember to use "${CMAKE_CXX_SIMULATE_ID}" instead of
> > > > > > # Undecorated "CMAKE_CXX_SIMULATE_ID" because there is a
> > > > > > # variable named "MSVC" and
> > > > > > # the way CMake variables and the "if()" command work requires
> this.
> > > > > > if (CMAKE_CXX_COMPILER_ID STREQUAL Clang
> > > > > > AND ${CMAKE_CXX_SIMULATE_ID} STREQUAL MSVC)
> > > > > > # CMake does not have explicit Clang-CL support yet.
> > > > > > # It should have a COMPILER_ID for it.
> > > > > > set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /showFilename")
> > > > > > # etc
> > > > > > endif()
> > > > > > ```
> > > > > >
> > > > > > Upstream CMake can have the logic to add the flag instead.
> > > > > But then what if you don’t want it? There would be no way to turn
> it off. I don’t think it’s a good fit for cmake
> > > > Yes, and actually for this reason i was wondering if we can do it
> without a flag at all. What if we just set an magic environment variable?
> It handles Stephen’s use case (he can just set it globally), and MSBuild
> (we can set it in the extension).
> > > > But then what if you don’t want it? There would be no way to turn it
> off.
> > >
> > > Oh, I thought that the last flag would dominate. ie, if cmake
> generated `/showFilename` and the user adds `/showFilename-`, then you
> would end up with
> > >
> > >
[PATCH] D52773: clang-cl: Add /showFilenames option (PR31957)
zturner added a subscriber: rnk. zturner added a comment. I agree magic environment variables are bad, but without it we don’t address the only current actual use we have for this, which is making the vs integration print filenames. Detecting compiler version from inside the integration is hard, but with an environment variable it’s very easy to solve. I’m not against a flag as the supported way, but I think we should also have some backdoor into this functionality, because if we’re not going to satisfy the only known use case, then maybe it’s better to not even do it at all. https://reviews.llvm.org/D52773 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52727: [clang-tidy] White List Option for performance-unnecessary-value-param, performance-unnecessary-copy-initialization and performance-for-range-copy
lebedev.ri added a comment.
Thanks looks even better, getting there.
There are some spurious whiteline changes.
Comment at: clang-tidy/performance/ForRangeCopyCheck.cpp:53
const auto *Var = Result.Nodes.getNodeAs("loopVar");
+
// Ignore code in macros since we can't place the fixes correctly.
Spurious newline
Comment at: clang-tidy/utils/Matchers.cpp:20
+matchesAnyListedName(const std::vector &NameList) {
+ SmallString<256> NameRegEx;
+ llvm::raw_svector_ostream NameOut(NameRegEx);
```
if(NameList.empty())
return false;
```
Comment at: clang-tidy/utils/Matchers.cpp:29
+NameOut << "^(?!.*)";
+ return matchesName(NameRegEx.str());
+}
Why do you need to 'concatenate' all the regexes?
Why not simply match in a loop?
https://reviews.llvm.org/D52727
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52727: [clang-tidy] White List Option for performance-unnecessary-value-param, performance-unnecessary-copy-initialization and performance-for-range-copy
baloghadamsoftware added inline comments.
Comment at: clang-tidy/utils/Matchers.cpp:20
+matchesAnyListedName(const std::vector &NameList) {
+ SmallString<256> NameRegEx;
+ llvm::raw_svector_ostream NameOut(NameRegEx);
lebedev.ri wrote:
> ```
> if(NameList.empty())
> return false;
> ```
`false`? But this functions returns `Matcher`.
Comment at: clang-tidy/utils/Matchers.cpp:29
+NameOut << "^(?!.*)";
+ return matchesName(NameRegEx.str());
+}
lebedev.ri wrote:
> Why do you need to 'concatenate' all the regexes?
> Why not simply match in a loop?
This is what @JonasToth suggested. How to match them in a loop if the function
returns a `Matcher`?
https://reviews.llvm.org/D52727
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52727: [clang-tidy] White List Option for performance-unnecessary-value-param, performance-unnecessary-copy-initialization and performance-for-range-copy
lebedev.ri added inline comments.
Comment at: clang-tidy/utils/Matchers.cpp:20
+matchesAnyListedName(const std::vector &NameList) {
+ SmallString<256> NameRegEx;
+ llvm::raw_svector_ostream NameOut(NameRegEx);
baloghadamsoftware wrote:
> lebedev.ri wrote:
> > ```
> > if(NameList.empty())
> > return false;
> > ```
> `false`? But this functions returns `Matcher`.
Hm, then `unless(anything())`, or `unless(anything()).matches(Node, Finder,
Builder)`.
https://reviews.llvm.org/D52727
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52727: [clang-tidy] White List Option for performance-unnecessary-value-param, performance-unnecessary-copy-initialization and performance-for-range-copy
lebedev.ri added inline comments.
Comment at: clang-tidy/utils/Matchers.cpp:18-19
+
+Matcher
+matchesAnyListedName(const std::vector &NameList) {
+ SmallString<256> NameRegEx;
Actually wait, what is this?
It should be something like
```
AST_MATCHER_P(NamedDecl, matchesAnyListedName, std::vector &,
NameList) {
...
```
Comment at: clang-tidy/utils/Matchers.cpp:20
+matchesAnyListedName(const std::vector &NameList) {
+ SmallString<256> NameRegEx;
+ llvm::raw_svector_ostream NameOut(NameRegEx);
lebedev.ri wrote:
> baloghadamsoftware wrote:
> > lebedev.ri wrote:
> > > ```
> > > if(NameList.empty())
> > > return false;
> > > ```
> > `false`? But this functions returns `Matcher`.
> Hm, then `unless(anything())`, or `unless(anything()).matches(Node, Finder,
> Builder)`.
Are you *sure* it doesn't work? Have you tried?
Comment at: clang-tidy/utils/Matchers.cpp:29
+NameOut << "^(?!.*)";
+ return matchesName(NameRegEx.str());
+}
baloghadamsoftware wrote:
> lebedev.ri wrote:
> > Why do you need to 'concatenate' all the regexes?
> > Why not simply match in a loop?
> This is what @JonasToth suggested. How to match them in a loop if the
> function returns a `Matcher`?
Are you *sure* it doesn't work? Have you tried?
https://reviews.llvm.org/D52727
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52808: [cland] Dex: fix/simplify trigram generation
sammccall updated this revision to Diff 168280.
sammccall added a comment.
Use similar but better-defined rules for short trigram matches.
Modify Dex to account for the matches not being exhaustive.
Unfortunately the test needs https://reviews.llvm.org/D52796, which depends on
this patch.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52808
Files:
clangd/index/dex/Dex.cpp
clangd/index/dex/Trigram.cpp
clangd/index/dex/Trigram.h
unittests/clangd/DexTests.cpp
Index: unittests/clangd/DexTests.cpp
===
--- unittests/clangd/DexTests.cpp
+++ unittests/clangd/DexTests.cpp
@@ -333,53 +333,54 @@
TEST(DexTrigrams, IdentifierTrigrams) {
EXPECT_THAT(generateIdentifierTrigrams("X86"),
- trigramsAre({"x86", "x$$", "x8$"}));
+ trigramsAre({"x86", "x", "x8"}));
- EXPECT_THAT(generateIdentifierTrigrams("nl"), trigramsAre({"nl$", "n$$"}));
+ EXPECT_THAT(generateIdentifierTrigrams("nl"), trigramsAre({"nl", "n"}));
- EXPECT_THAT(generateIdentifierTrigrams("n"), trigramsAre({"n$$"}));
+ EXPECT_THAT(generateIdentifierTrigrams("n"), trigramsAre({"n"}));
EXPECT_THAT(generateIdentifierTrigrams("clangd"),
- trigramsAre({"c$$", "cl$", "cla", "lan", "ang", "ngd"}));
+ trigramsAre({"c", "cl", "cla", "lan", "ang", "ngd"}));
EXPECT_THAT(generateIdentifierTrigrams("abc_def"),
- trigramsAre({"a$$", "abc", "abd", "ade", "bcd", "bde", "cde",
- "def", "ab$", "ad$"}));
+ trigramsAre({"a", "ab", "ad", "abc", "abd", "ade", "bcd", "bde",
+ "cde", "def"}));
EXPECT_THAT(generateIdentifierTrigrams("a_b_c_d_e_"),
- trigramsAre({"a$$", "a_$", "a_b", "abc", "abd", "acd", "ace",
- "bcd", "bce", "bde", "cde", "ab$"}));
+ trigramsAre({"a", "a_", "ab", "abc", "abd", "acd", "ace", "bcd",
+ "bce", "bde", "cde"}));
EXPECT_THAT(generateIdentifierTrigrams("unique_ptr"),
- trigramsAre({"u$$", "uni", "unp", "upt", "niq", "nip", "npt",
- "iqu", "iqp", "ipt", "que", "qup", "qpt", "uep",
- "ept", "ptr", "un$", "up$"}));
+ trigramsAre({"u", "un", "up", "uni", "unp", "upt", "niq", "nip",
+ "npt", "iqu", "iqp", "ipt", "que", "qup", "qpt",
+ "uep", "ept", "ptr"}));
EXPECT_THAT(
generateIdentifierTrigrams("TUDecl"),
- trigramsAre({"t$$", "tud", "tde", "ude", "dec", "ecl", "tu$", "td$"}));
+ trigramsAre({"t", "tu", "td", "tud", "tde", "ude", "dec", "ecl"}));
EXPECT_THAT(generateIdentifierTrigrams("IsOK"),
- trigramsAre({"i$$", "iso", "iok", "sok", "is$", "io$"}));
+ trigramsAre({"i", "is", "io", "iso", "iok", "sok"}));
+ auto X = generateIdentifierTrigrams("abc_defGhij__klm");
EXPECT_THAT(
generateIdentifierTrigrams("abc_defGhij__klm"),
- trigramsAre({"a$$", "abc", "abd", "abg", "ade", "adg", "adk", "agh",
- "agk", "bcd", "bcg", "bde", "bdg", "bdk", "bgh", "bgk",
- "cde", "cdg", "cdk", "cgh", "cgk", "def", "deg", "dek",
- "dgh", "dgk", "dkl", "efg", "efk", "egh", "egk", "ekl",
- "fgh", "fgk", "fkl", "ghi", "ghk", "gkl", "hij", "hik",
- "hkl", "ijk", "ikl", "jkl", "klm", "ab$", "ad$"}));
+ trigramsAre({"a", "ab", "ad", "abc", "abd", "abg", "ade", "adg",
+ "adk", "agh", "agk", "bcd", "bcg", "bde", "bdg", "bdk",
+ "bgh", "bgk", "cde", "cdg", "cdk", "cgh", "cgk", "def",
+ "deg", "dek", "dgh", "dgk", "dkl", "efg", "efk", "egh",
+ "egk", "ekl", "fgh", "fgk", "fkl", "ghi", "ghk", "gkl",
+ "hij", "hik", "hkl", "ijk", "ikl", "jkl", "klm"}));
}
TEST(DexTrigrams, QueryTrigrams) {
- EXPECT_THAT(generateQueryTrigrams("c"), trigramsAre({"c$$"}));
- EXPECT_THAT(generateQueryTrigrams("cl"), trigramsAre({"cl$"}));
+ EXPECT_THAT(generateQueryTrigrams("c"), trigramsAre({"c"}));
+ EXPECT_THAT(generateQueryTrigrams("cl"), trigramsAre({"cl"}));
EXPECT_THAT(generateQueryTrigrams("cla"), trigramsAre({"cla"}));
- EXPECT_THAT(generateQueryTrigrams("_"), trigramsAre({"_$$"}));
- EXPECT_THAT(generateQueryTrigrams("__"), trigramsAre({"__$"}));
- EXPECT_THAT(generateQueryTrigrams("___"), trigramsAre({"___"}));
+ EXPECT_THAT(generateQueryTrigrams("_"), trigramsAre({"_"}));
+ EXPECT_THAT(generateQueryTrigrams("__"), trigramsAre({"__"}));
+ EXPECT_THAT(generateQueryTrigrams("___"), trigramsAre({}));
EXPECT_THAT(generateQueryTrigrams("X86"), trigramsAre({"x86"}));
@@ -495,6 +496,31 @@
UnorderedElementsAre("LaughingOutLoud", "LittleOldLady"));
}
+// TODO(sammccall): enable after D52796 bugfix.
+#if 0
+TEST(DexTest, ShortQuery) {
+ auto I =
+ De
[PATCH] D52727: [clang-tidy] White List Option for performance-unnecessary-value-param, performance-unnecessary-copy-initialization and performance-for-range-copy
lebedev.ri added inline comments. Comment at: clang-tidy/utils/Matchers.cpp:1 +//===--- Matchers.cpp - clang-tidy-===// +// Also, why is this not in a header? I suspect that heavily pessimizes inlining. https://reviews.llvm.org/D52727 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52789: [clangd] Dex: FALSE iterator, peephole optimizations, fix AND bug
sammccall marked 5 inline comments as done. sammccall added inline comments. Comment at: clangd/index/dex/Iterator.cpp:376 +} +default: + RealChildren.push_back(std::move(Child)); ilya-biryukov wrote: > Maybe replace with `case Kind::Other` to make sure compiler gives a warning > to update the switch when new kinds are added? > Same for other switches on kind Actually this is deliberate, there's no particular need to specialize on every kind. (and in fact we have Or specialized on one fewer than And, but I wrote the case out in order to leave a comment) Comment at: clangd/index/dex/Iterator.cpp:385 + default: +return llvm::make_unique(move(RealChildren)); + } ilya-biryukov wrote: > Maybe use the qualified `std::move` for consistency with the previous line? Updated throughout. (This file was inconsistent and I was trying to preserve... something, but I'm not sure what) Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52789 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52727: [clang-tidy] White List Option for performance-unnecessary-value-param, performance-unnecessary-copy-initialization and performance-for-range-copy
baloghadamsoftware added inline comments.
Comment at: clang-tidy/utils/Matchers.cpp:18-19
+
+Matcher
+matchesAnyListedName(const std::vector &NameList) {
+ SmallString<256> NameRegEx;
lebedev.ri wrote:
> Actually wait, what is this?
> It should be something like
> ```
> AST_MATCHER_P(NamedDecl, matchesAnyListedName, std::vector &,
> NameList) {
> ...
> ```
Taken from `cppcoreguidelines-no-malloc` as @JonasToth suggested.
https://reviews.llvm.org/D52727
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r343774 - [clangd] Dex: FALSE iterator, peephole optimizations, fix AND bug
Author: sammccall
Date: Thu Oct 4 06:12:23 2018
New Revision: 343774
URL: http://llvm.org/viewvc/llvm-project?rev=343774&view=rev
Log:
[clangd] Dex: FALSE iterator, peephole optimizations, fix AND bug
Summary:
The FALSE iterator will be used in a followup patch to fix a logic bug in Dex
(currently, tokens that don't have posting lists in the index are simply dropped
from the query, changing semantics).
It can usually be optimized away, so added the following opmitizations:
- simplify booleans inside AND/OR
- replace effectively-empty AND/OR with booleans
- flatten nested AND/ORs
While working on this, found a bug in the AND iterator: its constructor sync()
assumes that ReachedEnd is set if applicable, but the constructor never sets it.
This crashes if a non-first iterator is nonempty.
Reviewers: ilya-biryukov
Subscribers: ioeric, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D52789
Modified:
clang-tools-extra/trunk/clangd/index/dex/Iterator.cpp
clang-tools-extra/trunk/clangd/index/dex/Iterator.h
clang-tools-extra/trunk/unittests/clangd/DexTests.cpp
Modified: clang-tools-extra/trunk/clangd/index/dex/Iterator.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/dex/Iterator.cpp?rev=343774&r1=343773&r2=343774&view=diff
==
--- clang-tools-extra/trunk/clangd/index/dex/Iterator.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/dex/Iterator.cpp Thu Oct 4 06:12:23
2018
@@ -8,6 +8,7 @@
//===--===//
#include "Iterator.h"
+#include "llvm/Support/Casting.h"
#include
#include
#include
@@ -26,9 +27,11 @@ namespace {
class AndIterator : public Iterator {
public:
explicit AndIterator(std::vector> AllChildren)
- : Children(std::move(AllChildren)) {
+ : Iterator(Kind::And), Children(std::move(AllChildren)) {
assert(!Children.empty() && "AND iterator should have at least one
child.");
// Establish invariants.
+for (const auto &Child : Children)
+ ReachedEnd |= Child->reachedEnd();
sync();
// When children are sorted by the estimateSize(), sync() calls are more
// effective. Each sync() starts with the first child and makes sure all
@@ -122,6 +125,7 @@ private:
/// update the field, rather than traversing the whole subtree in each
/// reachedEnd() call.
bool ReachedEnd = false;
+ friend Corpus; // For optimizations.
};
/// Implements Iterator over the union of other iterators.
@@ -133,7 +137,7 @@ private:
class OrIterator : public Iterator {
public:
explicit OrIterator(std::vector> AllChildren)
- : Children(std::move(AllChildren)) {
+ : Iterator(Kind::Or), Children(std::move(AllChildren)) {
assert(!Children.empty() && "OR iterator should have at least one child.");
}
@@ -208,6 +212,7 @@ private:
// FIXME(kbobyrev): Would storing Children in min-heap be faster?
std::vector> Children;
+ friend Corpus; // For optimizations.
};
/// TrueIterator handles PostingLists which contain all items of the index. It
@@ -215,7 +220,7 @@ private:
/// in O(1).
class TrueIterator : public Iterator {
public:
- explicit TrueIterator(DocID Size) : Size(Size) {}
+ explicit TrueIterator(DocID Size) : Iterator(Kind::True), Size(Size) {}
bool reachedEnd() const override { return Index >= Size; }
@@ -251,12 +256,35 @@ private:
DocID Size;
};
+/// FalseIterator yields no results.
+class FalseIterator : public Iterator {
+public:
+ FalseIterator() : Iterator(Kind::False) {}
+ bool reachedEnd() const override { return true; }
+ void advance() override { assert(false); }
+ void advanceTo(DocID ID) override { assert(false); }
+ DocID peek() const override {
+assert(false);
+return 0;
+ }
+ float consume() override {
+assert(false);
+return 1;
+ }
+ size_t estimateSize() const override { return 0; }
+
+private:
+ llvm::raw_ostream &dump(llvm::raw_ostream &OS) const override {
+return OS << "false";
+ }
+};
+
/// Boost iterator is a wrapper around its child which multiplies scores of
/// each retrieved item by a given factor.
class BoostIterator : public Iterator {
public:
BoostIterator(std::unique_ptr Child, float Factor)
- : Child(move(Child)), Factor(Factor) {}
+ : Child(std::move(Child)), Factor(Factor) {}
bool reachedEnd() const override { return Child->reachedEnd(); }
@@ -286,7 +314,7 @@ private:
class LimitIterator : public Iterator {
public:
LimitIterator(std::unique_ptr Child, size_t Limit)
- : Child(move(Child)), Limit(Limit), ItemsLeft(Limit) {}
+ : Child(std::move(Child)), Limit(Limit), ItemsLeft(Limit) {}
bool reachedEnd() const override {
return ItemsLeft == 0 || Child->reachedEnd();
@@ -331,30 +359,86 @@ std::vector> con
std::unique_ptr
Corpus::intersect(std::vector> Children) co
[PATCH] D52789: [clangd] Dex: FALSE iterator, peephole optimizations, fix AND bug
This revision was automatically updated to reflect the committed changes.
sammccall marked an inline comment as done.
Closed by commit rCTE343774: [clangd] Dex: FALSE iterator, peephole
optimizations, fix AND bug (authored by sammccall, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D52789?vs=167972&id=168281#toc
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52789
Files:
clangd/index/dex/Iterator.cpp
clangd/index/dex/Iterator.h
unittests/clangd/DexTests.cpp
Index: clangd/index/dex/Iterator.h
===
--- clangd/index/dex/Iterator.h
+++ clangd/index/dex/Iterator.h
@@ -98,8 +98,16 @@
return Iterator.dump(OS);
}
+ /// Inspect iterator type, used internally for optimizing query trees.
+ enum class Kind { And, Or, True, False, Other };
+ Kind kind() const { return MyKind; }
+
+protected:
+ Iterator(Kind MyKind = Kind::Other) : MyKind(MyKind) {}
+
private:
virtual llvm::raw_ostream &dump(llvm::raw_ostream &OS) const = 0;
+ Kind MyKind;
};
/// Advances the iterator until it is exhausted. Returns pairs of document IDs
@@ -150,6 +158,9 @@
/// containing all items in range [0, Size) in an efficient manner.
std::unique_ptr all() const;
+ /// Returns FALSE Iterator which iterates over no documents.
+ std::unique_ptr none() const;
+
/// Returns BOOST iterator which multiplies the score of each item by given
/// factor. Boosting can be used as a computationally inexpensive filtering.
/// Users can return significantly more items using consumeAndBoost() and
Index: clangd/index/dex/Iterator.cpp
===
--- clangd/index/dex/Iterator.cpp
+++ clangd/index/dex/Iterator.cpp
@@ -8,6 +8,7 @@
//===--===//
#include "Iterator.h"
+#include "llvm/Support/Casting.h"
#include
#include
#include
@@ -26,9 +27,11 @@
class AndIterator : public Iterator {
public:
explicit AndIterator(std::vector> AllChildren)
- : Children(std::move(AllChildren)) {
+ : Iterator(Kind::And), Children(std::move(AllChildren)) {
assert(!Children.empty() && "AND iterator should have at least one child.");
// Establish invariants.
+for (const auto &Child : Children)
+ ReachedEnd |= Child->reachedEnd();
sync();
// When children are sorted by the estimateSize(), sync() calls are more
// effective. Each sync() starts with the first child and makes sure all
@@ -122,6 +125,7 @@
/// update the field, rather than traversing the whole subtree in each
/// reachedEnd() call.
bool ReachedEnd = false;
+ friend Corpus; // For optimizations.
};
/// Implements Iterator over the union of other iterators.
@@ -133,7 +137,7 @@
class OrIterator : public Iterator {
public:
explicit OrIterator(std::vector> AllChildren)
- : Children(std::move(AllChildren)) {
+ : Iterator(Kind::Or), Children(std::move(AllChildren)) {
assert(!Children.empty() && "OR iterator should have at least one child.");
}
@@ -208,14 +212,15 @@
// FIXME(kbobyrev): Would storing Children in min-heap be faster?
std::vector> Children;
+ friend Corpus; // For optimizations.
};
/// TrueIterator handles PostingLists which contain all items of the index. It
/// stores size of the virtual posting list, and all operations are performed
/// in O(1).
class TrueIterator : public Iterator {
public:
- explicit TrueIterator(DocID Size) : Size(Size) {}
+ explicit TrueIterator(DocID Size) : Iterator(Kind::True), Size(Size) {}
bool reachedEnd() const override { return Index >= Size; }
@@ -251,12 +256,35 @@
DocID Size;
};
+/// FalseIterator yields no results.
+class FalseIterator : public Iterator {
+public:
+ FalseIterator() : Iterator(Kind::False) {}
+ bool reachedEnd() const override { return true; }
+ void advance() override { assert(false); }
+ void advanceTo(DocID ID) override { assert(false); }
+ DocID peek() const override {
+assert(false);
+return 0;
+ }
+ float consume() override {
+assert(false);
+return 1;
+ }
+ size_t estimateSize() const override { return 0; }
+
+private:
+ llvm::raw_ostream &dump(llvm::raw_ostream &OS) const override {
+return OS << "false";
+ }
+};
+
/// Boost iterator is a wrapper around its child which multiplies scores of
/// each retrieved item by a given factor.
class BoostIterator : public Iterator {
public:
BoostIterator(std::unique_ptr Child, float Factor)
- : Child(move(Child)), Factor(Factor) {}
+ : Child(std::move(Child)), Factor(Factor) {}
bool reachedEnd() const override { return Child->reachedEnd(); }
@@ -286,7 +314,7 @@
class LimitIterator : public Iterator {
public:
LimitIterator(std::unique_ptr Child, size_t Limit)
- : Child(move(Child)), Limit(Limit), ItemsLeft(Limit) {}
+ : Child(std::move(Child)), Limit(Limit), Item
[PATCH] D52875: Fix definitions of __builtin_(add|sub|mul)_overflow
erichkeane added a comment. Can you write tests for this please? Particularly validate the results in a constexpr context. Additionally, these all have the 't' flag, which means that these signatures are meaningless, right? What are you seeing where this works incorrectly? Repository: rC Clang https://reviews.llvm.org/D52875 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52727: [clang-tidy] White List Option for performance-unnecessary-value-param, performance-unnecessary-copy-initialization and performance-for-range-copy
lebedev.ri added inline comments.
Comment at: clang-tidy/utils/Matchers.cpp:18-19
+
+Matcher
+matchesAnyListedName(const std::vector &NameList) {
+ SmallString<256> NameRegEx;
baloghadamsoftware wrote:
> lebedev.ri wrote:
> > Actually wait, what is this?
> > It should be something like
> > ```
> > AST_MATCHER_P(NamedDecl, matchesAnyListedName, std::vector &,
> > NameList) {
> > ...
> > ```
> Taken from `cppcoreguidelines-no-malloc` as @JonasToth suggested.
Ok, but as you can see in `clang-tidy/utils/Matchers.h` (and `ASTMatchers.h`),
it's not really correct prototype.
https://reviews.llvm.org/D52727
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52398: Thread safety analysis: Unwrap __builtin_expect in getTrylockCallExpr
hokein added a comment.
Hi, @aaronpuchert, the patch has caused many new warnings in our internal
codebase, below is a reduced one. Do you mind reverting the patch?
// if the condition is not true, CHECK will terminate the program.
#define CHECK(condition) (__builtin_expect((condition), true)) ? 1 : 0
void foo() {
CHECK(mu.TryLock());
mu.Unlock();
}
Repository:
rL LLVM
https://reviews.llvm.org/D52398
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52727: [clang-tidy] White List Option for performance-unnecessary-value-param, performance-unnecessary-copy-initialization and performance-for-range-copy
JonasToth added inline comments.
Comment at: clang-tidy/utils/Matchers.cpp:18-19
+
+Matcher
+matchesAnyListedName(const std::vector &NameList) {
+ SmallString<256> NameRegEx;
lebedev.ri wrote:
> baloghadamsoftware wrote:
> > lebedev.ri wrote:
> > > Actually wait, what is this?
> > > It should be something like
> > > ```
> > > AST_MATCHER_P(NamedDecl, matchesAnyListedName, std::vector
> > > &, NameList) {
> > > ...
> > > ```
> > Taken from `cppcoreguidelines-no-malloc` as @JonasToth suggested.
> Ok, but as you can see in `clang-tidy/utils/Matchers.h` (and
> `ASTMatchers.h`), it's not really correct prototype.
>
>
This is not a full matcher, but a utility function creating one. You can make a
full matcher-class with the AST_MATCHER macros as well.
Given this now is a public (within clang-tidy) matcher, it should probably be a
AST_MATCHER based matcher. The cppcoreguidelines-check can then be migrated.
https://reviews.llvm.org/D52727
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52885: [clangd] Remove one-segment-skipping from Dex trigrams.
sammccall created this revision.
sammccall added reviewers: ilya-biryukov, ioeric.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay.
Currently queries like "ab" can match identifiers like a_yellow_bee.
The value of allowing this for exactly one segment but no more seems dubious.
It costs ~3% of overall ram (~9% of posting list ram) and some quality.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52885
Files:
clangd/index/dex/Trigram.cpp
clangd/index/dex/Trigram.h
unittests/clangd/DexTests.cpp
Index: unittests/clangd/DexTests.cpp
===
--- unittests/clangd/DexTests.cpp
+++ unittests/clangd/DexTests.cpp
@@ -381,8 +381,7 @@
"def", "ab$", "ad$"}));
EXPECT_THAT(generateIdentifierTrigrams("a_b_c_d_e_"),
- trigramsAre({"a$$", "a_$", "a_b", "abc", "abd", "acd", "ace",
- "bcd", "bce", "bde", "cde", "ab$"}));
+ trigramsAre({"a$$", "a_$", "a_b", "abc", "bcd", "cde", "ab$"}));
EXPECT_THAT(generateIdentifierTrigrams("unique_ptr"),
trigramsAre({"u$$", "uni", "unp", "upt", "niq", "nip", "npt",
@@ -398,11 +397,9 @@
EXPECT_THAT(
generateIdentifierTrigrams("abc_defGhij__klm"),
- trigramsAre({"a$$", "abc", "abd", "abg", "ade", "adg", "adk", "agh",
- "agk", "bcd", "bcg", "bde", "bdg", "bdk", "bgh", "bgk",
- "cde", "cdg", "cdk", "cgh", "cgk", "def", "deg", "dek",
- "dgh", "dgk", "dkl", "efg", "efk", "egh", "egk", "ekl",
- "fgh", "fgk", "fkl", "ghi", "ghk", "gkl", "hij", "hik",
+ trigramsAre({"a$$", "abc", "abd", "ade", "adg", "bcd", "bde", "bdg",
+ "cde", "cdg", "def", "deg", "dgh", "dgk", "efg", "egh",
+ "egk", "fgh", "fgk", "ghi", "ghk", "gkl", "hij", "hik",
"hkl", "ijk", "ikl", "jkl", "klm", "ab$", "ad$"}));
}
Index: clangd/index/dex/Trigram.h
===
--- clangd/index/dex/Trigram.h
+++ clangd/index/dex/Trigram.h
@@ -42,8 +42,7 @@
/// characters is inserted into the result.
///
/// Trigrams can start at any character in the input. Then we can choose to
move
-/// to the next character, move to the start of the next segment, or skip over
a
-/// segment.
+/// to the next character, move to the start of the next segment.
///
/// This also generates incomplete trigrams for short query scenarios:
/// * Empty trigram: "$$$".
Index: clangd/index/dex/Trigram.cpp
===
--- clangd/index/dex/Trigram.cpp
+++ clangd/index/dex/Trigram.cpp
@@ -46,14 +46,13 @@
// Next stores tuples of three indices in the presented order, if a variant
is
// not available then 0 is stored.
std::vector> Next(LowercaseIdentifier.size());
- unsigned NextTail = 0, NextHead = 0, NextNextHead = 0;
+ unsigned NextTail = 0, NextHead = 0;
// Store two first HEAD characters in the identifier (if present).
std::deque TwoHeads;
for (int I = LowercaseIdentifier.size() - 1; I >= 0; --I) {
-Next[I] = {{NextTail, NextHead, NextNextHead}};
+Next[I] = {{NextTail, NextHead}};
NextTail = Roles[I] == Tail ? I : 0;
if (Roles[I] == Head) {
- NextNextHead = NextHead;
NextHead = I;
TwoHeads.push_front(LowercaseIdentifier[I]);
if (TwoHeads.size() > 2)
Index: unittests/clangd/DexTests.cpp
===
--- unittests/clangd/DexTests.cpp
+++ unittests/clangd/DexTests.cpp
@@ -381,8 +381,7 @@
"def", "ab$", "ad$"}));
EXPECT_THAT(generateIdentifierTrigrams("a_b_c_d_e_"),
- trigramsAre({"a$$", "a_$", "a_b", "abc", "abd", "acd", "ace",
- "bcd", "bce", "bde", "cde", "ab$"}));
+ trigramsAre({"a$$", "a_$", "a_b", "abc", "bcd", "cde", "ab$"}));
EXPECT_THAT(generateIdentifierTrigrams("unique_ptr"),
trigramsAre({"u$$", "uni", "unp", "upt", "niq", "nip", "npt",
@@ -398,11 +397,9 @@
EXPECT_THAT(
generateIdentifierTrigrams("abc_defGhij__klm"),
- trigramsAre({"a$$", "abc", "abd", "abg", "ade", "adg", "adk", "agh",
- "agk", "bcd", "bcg", "bde", "bdg", "bdk", "bgh", "bgk",
- "cde", "cdg", "cdk", "cgh", "cgk", "def", "deg", "dek",
- "dgh", "dgk", "dkl", "efg", "efk", "egh", "egk", "ekl",
- "fgh", "fgk", "fkl", "ghi", "ghk", "gkl", "hij", "hik",
+ trigramsAre({"a$$", "abc", "abd", "ade", "adg", "bcd", "bde", "bdg",
+ "cde", "cdg", "def", "deg", "dgh", "dgk", "efg", "egh",
+ "egk", "fgh", "fgk", "ghi", "ghk", "gkl", "hij", "hik",
"hkl", "ijk", "ikl", "jkl", "klm", "ab$", "ad$"}));
}
Index: clangd/index/dex/Trigram.h
===
[PATCH] D52875: Fix definitions of __builtin_(add|sub|mul)_overflow
mantognini added a comment. In https://reviews.llvm.org/D52875#1255146, @erichkeane wrote: > Can you write tests for this please? Particularly validate the results in a > constexpr context. There are already some tests for those builtins (not sure about constexpr context). They already tests that the builtins can be used as branching condition. However, the current implementation of `Sema::BuildResolvedCallExpr` assumes that by default builtins return `bool`. In https://reviews.llvm.org/D52879, I improve that and not having the above fix makes the existing tests fail, so I believe we don't need to add more tests. > Additionally, these all have the 't' flag, which means that these signatures > are meaningless, right? What are you seeing where this works incorrectly? I reckon the signature does't include the return type, hence it isn't meaningless even with the `t` flag. Repository: rC Clang https://reviews.llvm.org/D52875 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52873: Remove unwanted signedness conversion from tests
mantognini added a comment. Note: the `get_kernel_*` functions used here all return unsigned integers, hence the warning/present patch. Repository: rC Clang https://reviews.llvm.org/D52873 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52808: [cland] Dex: fix/simplify trigram generation
ioeric accepted this revision.
ioeric added a comment.
This revision is now accepted and ready to land.
> The problem is LSP clients are free to assume that the result list is
> complete (unless marked as incomplete) and therefore will never retrieve the
> better symbols.
Good point. Thanks for the explanation!
Comment at: clangd/index/dex/Dex.cpp:222
SPAN_ATTACH(Tracer, "query", llvm::to_string(*Root));
- vlog("Dex query tree: {0}", *Root);
+ log("Dex query tree: {0}", *Root);
should this still be a `vlog`?
Comment at: clangd/index/dex/Trigram.h:47
+/// For "FooBar" we get the following trigrams:
+/// {f, b, fo, fb, foo, fob, fba, oob, oba, bar}.
+///
is `b` still generated?
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52808
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52885: [clangd] Remove one-segment-skipping from Dex trigrams.
ioeric accepted this revision. ioeric added a comment. This revision is now accepted and ready to land. lgtm Comment at: clangd/index/dex/Trigram.cpp:44 // * Next Head - front character of the next segment // * Skip-1-Next Head - front character of the skip-1-next segment // This can be removed now? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52885 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r343775 - [cland] Dex: fix/simplify short-trigram generation
Author: sammccall
Date: Thu Oct 4 07:01:55 2018
New Revision: 343775
URL: http://llvm.org/viewvc/llvm-project?rev=343775&view=rev
Log:
[cland] Dex: fix/simplify short-trigram generation
Summary:
1) Instead of x$$ for a short-query trigram, just use x
2) Make rules more coherent: prefixes of length 1-2, and first char + next head
3) Fix Dex::fuzzyFind to mark results as incomplete, because
short-trigram rules only yield a subset of results.
Reviewers: ioeric
Subscribers: ilya-biryukov, jkorous, mgrang, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D52808
Modified:
clang-tools-extra/trunk/clangd/index/dex/Dex.cpp
clang-tools-extra/trunk/clangd/index/dex/Trigram.cpp
clang-tools-extra/trunk/clangd/index/dex/Trigram.h
clang-tools-extra/trunk/unittests/clangd/DexTests.cpp
Modified: clang-tools-extra/trunk/clangd/index/dex/Dex.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/dex/Dex.cpp?rev=343775&r1=343774&r2=343775&view=diff
==
--- clang-tools-extra/trunk/clangd/index/dex/Dex.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/dex/Dex.cpp Thu Oct 4 07:01:55 2018
@@ -156,7 +156,9 @@ bool Dex::fuzzyFind(const FuzzyFindReque
"There must be no :: in query.");
trace::Span Tracer("Dex fuzzyFind");
FuzzyMatcher Filter(Req.Query);
- bool More = false;
+ // For short queries we use specialized trigrams that don't yield all
results.
+ // Prevent clients from postfiltering them for longer queries.
+ bool More = !Req.Query.empty() && Req.Query.size() < 3;
std::vector> TopLevelChildren;
const auto TrigramTokens = generateQueryTrigrams(Req.Query);
Modified: clang-tools-extra/trunk/clangd/index/dex/Trigram.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/dex/Trigram.cpp?rev=343775&r1=343774&r2=343775&view=diff
==
--- clang-tools-extra/trunk/clangd/index/dex/Trigram.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/dex/Trigram.cpp Thu Oct 4 07:01:55
2018
@@ -23,11 +23,6 @@ namespace clang {
namespace clangd {
namespace dex {
-/// This is used to mark unigrams and bigrams and distinct them from complete
-/// trigrams. Since '$' is not present in valid identifier names, it is safe to
-/// use it as the special symbol.
-static const char END_MARKER = '$';
-
std::vector generateIdentifierTrigrams(llvm::StringRef Identifier) {
// Apply fuzzy matching text segmentation.
std::vector Roles(Identifier.size());
@@ -47,40 +42,22 @@ std::vector generateIdentifierTri
// not available then 0 is stored.
std::vector> Next(LowercaseIdentifier.size());
unsigned NextTail = 0, NextHead = 0, NextNextHead = 0;
- // Store two first HEAD characters in the identifier (if present).
- std::deque TwoHeads;
for (int I = LowercaseIdentifier.size() - 1; I >= 0; --I) {
Next[I] = {{NextTail, NextHead, NextNextHead}};
NextTail = Roles[I] == Tail ? I : 0;
if (Roles[I] == Head) {
NextNextHead = NextHead;
NextHead = I;
- TwoHeads.push_front(LowercaseIdentifier[I]);
- if (TwoHeads.size() > 2)
-TwoHeads.pop_back();
}
}
DenseSet UniqueTrigrams;
- auto add = [&](std::string Chars) {
+ auto Add = [&](std::string Chars) {
UniqueTrigrams.insert(Token(Token::Kind::Trigram, Chars));
};
- if (TwoHeads.size() == 2)
-add({{TwoHeads.front(), TwoHeads.back(), END_MARKER}});
-
- if (!LowercaseIdentifier.empty())
-add({{LowercaseIdentifier.front(), END_MARKER, END_MARKER}});
-
- if (LowercaseIdentifier.size() >= 2)
-add({{LowercaseIdentifier[0], LowercaseIdentifier[1], END_MARKER}});
-
- if (LowercaseIdentifier.size() >= 3)
-add({{LowercaseIdentifier[0], LowercaseIdentifier[1],
- LowercaseIdentifier[2]}});
-
- // Iterate through valid seqneces of three characters Fuzzy Matcher can
+ // Iterate through valid sequneces of three characters Fuzzy Matcher can
// process.
for (size_t I = 0; I < LowercaseIdentifier.size(); ++I) {
// Skip delimiters.
@@ -92,66 +69,47 @@ std::vector generateIdentifierTri
for (const unsigned K : Next[J]) {
if (K == 0)
continue;
-add({{LowercaseIdentifier[I], LowercaseIdentifier[J],
+Add({{LowercaseIdentifier[I], LowercaseIdentifier[J],
LowercaseIdentifier[K]}});
}
}
}
+ // Emit short-query trigrams: FooBar -> f, fo, fb.
+ if (!LowercaseIdentifier.empty())
+Add({LowercaseIdentifier[0]});
+ if (LowercaseIdentifier.size() >= 2)
+Add({LowercaseIdentifier[0], LowercaseIdentifier[1]});
+ for (size_t I = 1; I < LowercaseIdentifier.size(); ++I)
+if (Roles[I] == Head) {
+ Add({LowercaseIdentifier[0], LowercaseIdentifier[I]});
+ break;
+}
- std::vector Result;
- for (const auto &Trigram : UniqueTrigrams)
[PATCH] D52808: [cland] Dex: fix/simplify trigram generation
This revision was automatically updated to reflect the committed changes.
sammccall marked 2 inline comments as done.
Closed by commit rCTE343775: [cland] Dex: fix/simplify short-trigram generation
(authored by sammccall, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D52808?vs=168280&id=168284#toc
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52808
Files:
clangd/index/dex/Dex.cpp
clangd/index/dex/Trigram.cpp
clangd/index/dex/Trigram.h
unittests/clangd/DexTests.cpp
Index: clangd/index/dex/Dex.cpp
===
--- clangd/index/dex/Dex.cpp
+++ clangd/index/dex/Dex.cpp
@@ -156,7 +156,9 @@
"There must be no :: in query.");
trace::Span Tracer("Dex fuzzyFind");
FuzzyMatcher Filter(Req.Query);
- bool More = false;
+ // For short queries we use specialized trigrams that don't yield all results.
+ // Prevent clients from postfiltering them for longer queries.
+ bool More = !Req.Query.empty() && Req.Query.size() < 3;
std::vector> TopLevelChildren;
const auto TrigramTokens = generateQueryTrigrams(Req.Query);
Index: clangd/index/dex/Trigram.h
===
--- clangd/index/dex/Trigram.h
+++ clangd/index/dex/Trigram.h
@@ -33,26 +33,21 @@
namespace dex {
/// Returns list of unique fuzzy-search trigrams from unqualified symbol.
+/// The trigrams give the 3-character query substrings this symbol can match.
///
-/// First, given Identifier (unqualified symbol name) is segmented using
-/// FuzzyMatch API and lowercased. After segmentation, the following technique
-/// is applied for generating trigrams: for each letter or digit in the input
-/// string the algorithms looks for the possible next and skip-1-next characters
-/// which can be jumped to during fuzzy matching. Each combination of such three
-/// characters is inserted into the result.
-///
+/// The symbol's name is broken into segments, e.g. "FooBar" has two segments.
/// Trigrams can start at any character in the input. Then we can choose to move
-/// to the next character, move to the start of the next segment, or skip over a
-/// segment.
+/// to the next character, move to the start of the next segment, or stop.
///
-/// This also generates incomplete trigrams for short query scenarios:
-/// * Empty trigram: "$$$".
-/// * Unigram: the first character of the identifier.
-/// * Bigrams: a 2-char prefix of the identifier and a bigram of the first two
-///HEAD characters (if they exist).
-//
-/// Note: the returned list of trigrams does not have duplicates, if any trigram
-/// belongs to more than one class it is only inserted once.
+/// Short trigrams (length 1-2) are used for short queries. These are:
+/// - prefixes of the identifier, of length 1 and 2
+/// - the first character + next head character
+///
+/// For "FooBar" we get the following trigrams:
+/// {f, fo, fb, foo, fob, fba, oob, oba, bar}.
+///
+/// Trigrams are lowercase, as trigram matching is case-insensitive.
+/// Trigrams in the returned list are deduplicated.
std::vector generateIdentifierTrigrams(llvm::StringRef Identifier);
/// Returns list of unique fuzzy-search trigrams given a query.
Index: clangd/index/dex/Trigram.cpp
===
--- clangd/index/dex/Trigram.cpp
+++ clangd/index/dex/Trigram.cpp
@@ -23,11 +23,6 @@
namespace clangd {
namespace dex {
-/// This is used to mark unigrams and bigrams and distinct them from complete
-/// trigrams. Since '$' is not present in valid identifier names, it is safe to
-/// use it as the special symbol.
-static const char END_MARKER = '$';
-
std::vector generateIdentifierTrigrams(llvm::StringRef Identifier) {
// Apply fuzzy matching text segmentation.
std::vector Roles(Identifier.size());
@@ -47,40 +42,22 @@
// not available then 0 is stored.
std::vector> Next(LowercaseIdentifier.size());
unsigned NextTail = 0, NextHead = 0, NextNextHead = 0;
- // Store two first HEAD characters in the identifier (if present).
- std::deque TwoHeads;
for (int I = LowercaseIdentifier.size() - 1; I >= 0; --I) {
Next[I] = {{NextTail, NextHead, NextNextHead}};
NextTail = Roles[I] == Tail ? I : 0;
if (Roles[I] == Head) {
NextNextHead = NextHead;
NextHead = I;
- TwoHeads.push_front(LowercaseIdentifier[I]);
- if (TwoHeads.size() > 2)
-TwoHeads.pop_back();
}
}
DenseSet UniqueTrigrams;
- auto add = [&](std::string Chars) {
+ auto Add = [&](std::string Chars) {
UniqueTrigrams.insert(Token(Token::Kind::Trigram, Chars));
};
- if (TwoHeads.size() == 2)
-add({{TwoHeads.front(), TwoHeads.back(), END_MARKER}});
-
- if (!LowercaseIdentifier.empty())
-add({{LowercaseIdentifier.front(), END_MARKER, END_MARKER}});
-
- if (LowercaseIdentifier.size() >= 2)
-add({{LowercaseIdentifier[0], LowercaseIdentifier[1], END_MARK
[PATCH] D51633: [ASTImporter] Added error handling for AST import.
balazske added inline comments.
Comment at: lib/AST/ASTImporter.cpp:194
+ // FIXME: This should be the final code.
+ //auto ToOrErr = Importer.Import(From);
+ //if (ToOrErr) {
a_sidorin wrote:
> Do I understand correctly that we have to use the upper variant because
> ASTImporter API is unchanged?
The "upper variant" (`importInto(ImportT &To, const ImportT &From)` form) is
the final one. It is only to make the code more simple, you can call
`importInto` in ASTImporter instead of `Importer.importInto`. This second
variant is needed because the ASTImporter API is not changed (yet), the
`Import` functions still return pointer (later they will return `Expected`).
But this special version for pointer is still needed to have type cast for
pointers. The commented-out code should be used later.
Comment at: lib/AST/ASTImporter.cpp:1663
+ // Ignore the error, continue with next Decl.
+ consumeError(ImportedOrErr.takeError());
+ }
a_sidorin wrote:
> Silent fail doesn't look correct in case of structures. Should we add a FIXME?
Maybe yes. But aborting the whole import of the DeclContext could cause lots of
not imported things. A warning can be printed but maybe it is already printed
at the called failed import. There was no FIXME in the old code either.
Repository:
rC Clang
https://reviews.llvm.org/D51633
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r343777 - [clangd] Remove one-segment-skipping from Dex trigrams.
Author: sammccall
Date: Thu Oct 4 07:08:11 2018
New Revision: 343777
URL: http://llvm.org/viewvc/llvm-project?rev=343777&view=rev
Log:
[clangd] Remove one-segment-skipping from Dex trigrams.
Summary:
Currently queries like "ab" can match identifiers like a_yellow_bee.
The value of allowing this for exactly one segment but no more seems dubious.
It costs ~3% of overall ram (~9% of posting list ram) and some quality.
Reviewers: ilya-biryukov, ioeric
Subscribers: MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D52885
Modified:
clang-tools-extra/trunk/clangd/index/dex/Trigram.cpp
clang-tools-extra/trunk/clangd/index/dex/Trigram.h
clang-tools-extra/trunk/unittests/clangd/DexTests.cpp
Modified: clang-tools-extra/trunk/clangd/index/dex/Trigram.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/dex/Trigram.cpp?rev=343777&r1=343776&r2=343777&view=diff
==
--- clang-tools-extra/trunk/clangd/index/dex/Trigram.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/dex/Trigram.cpp Thu Oct 4 07:08:11
2018
@@ -36,17 +36,15 @@ std::vector generateIdentifierTri
//
// * Next Tail - next character from the same segment
// * Next Head - front character of the next segment
- // * Skip-1-Next Head - front character of the skip-1-next segment
//
// Next stores tuples of three indices in the presented order, if a variant
is
// not available then 0 is stored.
std::vector> Next(LowercaseIdentifier.size());
- unsigned NextTail = 0, NextHead = 0, NextNextHead = 0;
+ unsigned NextTail = 0, NextHead = 0;
for (int I = LowercaseIdentifier.size() - 1; I >= 0; --I) {
-Next[I] = {{NextTail, NextHead, NextNextHead}};
+Next[I] = {{NextTail, NextHead}};
NextTail = Roles[I] == Tail ? I : 0;
if (Roles[I] == Head) {
- NextNextHead = NextHead;
NextHead = I;
}
}
Modified: clang-tools-extra/trunk/clangd/index/dex/Trigram.h
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/dex/Trigram.h?rev=343777&r1=343776&r2=343777&view=diff
==
--- clang-tools-extra/trunk/clangd/index/dex/Trigram.h (original)
+++ clang-tools-extra/trunk/clangd/index/dex/Trigram.h Thu Oct 4 07:08:11 2018
@@ -37,7 +37,7 @@ namespace dex {
///
/// The symbol's name is broken into segments, e.g. "FooBar" has two segments.
/// Trigrams can start at any character in the input. Then we can choose to
move
-/// to the next character, move to the start of the next segment, or stop.
+/// to the next character or to the start of the next segment.
///
/// Short trigrams (length 1-2) are used for short queries. These are:
/// - prefixes of the identifier, of length 1 and 2
Modified: clang-tools-extra/trunk/unittests/clangd/DexTests.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/DexTests.cpp?rev=343777&r1=343776&r2=343777&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/DexTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/DexTests.cpp Thu Oct 4 07:08:11
2018
@@ -381,8 +381,7 @@ TEST(DexTrigrams, IdentifierTrigrams) {
"cde", "def"}));
EXPECT_THAT(generateIdentifierTrigrams("a_b_c_d_e_"),
- trigramsAre({"a", "a_", "ab", "abc", "abd", "acd", "ace", "bcd",
- "bce", "bde", "cde"}));
+ trigramsAre({"a", "a_", "ab", "abc", "bcd", "cde"}));
EXPECT_THAT(generateIdentifierTrigrams("unique_ptr"),
trigramsAre({"u", "un", "up", "uni", "unp", "upt", "niq", "nip",
@@ -396,14 +395,11 @@ TEST(DexTrigrams, IdentifierTrigrams) {
EXPECT_THAT(generateIdentifierTrigrams("IsOK"),
trigramsAre({"i", "is", "io", "iso", "iok", "sok"}));
- auto X = generateIdentifierTrigrams("abc_defGhij__klm");
EXPECT_THAT(
generateIdentifierTrigrams("abc_defGhij__klm"),
- trigramsAre({"a", "ab", "ad", "abc", "abd", "abg", "ade", "adg",
- "adk", "agh", "agk", "bcd", "bcg", "bde", "bdg", "bdk",
- "bgh", "bgk", "cde", "cdg", "cdk", "cgh", "cgk", "def",
- "deg", "dek", "dgh", "dgk", "dkl", "efg", "efk", "egh",
- "egk", "ekl", "fgh", "fgk", "fkl", "ghi", "ghk", "gkl",
+ trigramsAre({"a", "ab", "ad", "abc", "abd", "ade", "adg", "bcd",
+ "bde", "bdg", "cde", "cdg", "def", "deg", "dgh", "dgk",
+ "efg", "egh", "egk", "fgh", "fgk", "ghi", "ghk", "gkl",
"hij", "hik", "hkl", "ijk", "ikl", "jkl", "klm"}));
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52885: [clangd] Remove one-segment-skipping from Dex trigrams.
This revision was automatically updated to reflect the committed changes.
sammccall marked an inline comment as done.
Closed by commit rCTE343777: [clangd] Remove one-segment-skipping from Dex
trigrams. (authored by sammccall, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D52885?vs=168282&id=168285#toc
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52885
Files:
clangd/index/dex/Trigram.cpp
clangd/index/dex/Trigram.h
unittests/clangd/DexTests.cpp
Index: unittests/clangd/DexTests.cpp
===
--- unittests/clangd/DexTests.cpp
+++ unittests/clangd/DexTests.cpp
@@ -381,8 +381,7 @@
"cde", "def"}));
EXPECT_THAT(generateIdentifierTrigrams("a_b_c_d_e_"),
- trigramsAre({"a", "a_", "ab", "abc", "abd", "acd", "ace", "bcd",
- "bce", "bde", "cde"}));
+ trigramsAre({"a", "a_", "ab", "abc", "bcd", "cde"}));
EXPECT_THAT(generateIdentifierTrigrams("unique_ptr"),
trigramsAre({"u", "un", "up", "uni", "unp", "upt", "niq", "nip",
@@ -396,14 +395,11 @@
EXPECT_THAT(generateIdentifierTrigrams("IsOK"),
trigramsAre({"i", "is", "io", "iso", "iok", "sok"}));
- auto X = generateIdentifierTrigrams("abc_defGhij__klm");
EXPECT_THAT(
generateIdentifierTrigrams("abc_defGhij__klm"),
- trigramsAre({"a", "ab", "ad", "abc", "abd", "abg", "ade", "adg",
- "adk", "agh", "agk", "bcd", "bcg", "bde", "bdg", "bdk",
- "bgh", "bgk", "cde", "cdg", "cdk", "cgh", "cgk", "def",
- "deg", "dek", "dgh", "dgk", "dkl", "efg", "efk", "egh",
- "egk", "ekl", "fgh", "fgk", "fkl", "ghi", "ghk", "gkl",
+ trigramsAre({"a", "ab", "ad", "abc", "abd", "ade", "adg", "bcd",
+ "bde", "bdg", "cde", "cdg", "def", "deg", "dgh", "dgk",
+ "efg", "egh", "egk", "fgh", "fgk", "ghi", "ghk", "gkl",
"hij", "hik", "hkl", "ijk", "ikl", "jkl", "klm"}));
}
Index: clangd/index/dex/Trigram.cpp
===
--- clangd/index/dex/Trigram.cpp
+++ clangd/index/dex/Trigram.cpp
@@ -36,17 +36,15 @@
//
// * Next Tail - next character from the same segment
// * Next Head - front character of the next segment
- // * Skip-1-Next Head - front character of the skip-1-next segment
//
// Next stores tuples of three indices in the presented order, if a variant
is
// not available then 0 is stored.
std::vector> Next(LowercaseIdentifier.size());
- unsigned NextTail = 0, NextHead = 0, NextNextHead = 0;
+ unsigned NextTail = 0, NextHead = 0;
for (int I = LowercaseIdentifier.size() - 1; I >= 0; --I) {
-Next[I] = {{NextTail, NextHead, NextNextHead}};
+Next[I] = {{NextTail, NextHead}};
NextTail = Roles[I] == Tail ? I : 0;
if (Roles[I] == Head) {
- NextNextHead = NextHead;
NextHead = I;
}
}
Index: clangd/index/dex/Trigram.h
===
--- clangd/index/dex/Trigram.h
+++ clangd/index/dex/Trigram.h
@@ -37,7 +37,7 @@
///
/// The symbol's name is broken into segments, e.g. "FooBar" has two segments.
/// Trigrams can start at any character in the input. Then we can choose to
move
-/// to the next character, move to the start of the next segment, or stop.
+/// to the next character or to the start of the next segment.
///
/// Short trigrams (length 1-2) are used for short queries. These are:
/// - prefixes of the identifier, of length 1 and 2
Index: unittests/clangd/DexTests.cpp
===
--- unittests/clangd/DexTests.cpp
+++ unittests/clangd/DexTests.cpp
@@ -381,8 +381,7 @@
"cde", "def"}));
EXPECT_THAT(generateIdentifierTrigrams("a_b_c_d_e_"),
- trigramsAre({"a", "a_", "ab", "abc", "abd", "acd", "ace", "bcd",
- "bce", "bde", "cde"}));
+ trigramsAre({"a", "a_", "ab", "abc", "bcd", "cde"}));
EXPECT_THAT(generateIdentifierTrigrams("unique_ptr"),
trigramsAre({"u", "un", "up", "uni", "unp", "upt", "niq", "nip",
@@ -396,14 +395,11 @@
EXPECT_THAT(generateIdentifierTrigrams("IsOK"),
trigramsAre({"i", "is", "io", "iso", "iok", "sok"}));
- auto X = generateIdentifierTrigrams("abc_defGhij__klm");
EXPECT_THAT(
generateIdentifierTrigrams("abc_defGhij__klm"),
- trigramsAre({"a", "ab", "ad", "abc", "abd", "abg", "ade", "adg",
- "adk", "agh", "agk", "bcd", "bcg", "bde", "bdg", "bdk",
- "bgh", "bgk", "cde", "cdg", "cdk", "cgh", "cgk", "def",
- "deg", "dek", "dgh", "dgk", "dkl", "efg", "efk", "egh",
- "egk", "ekl", "fgh", "fgk", "fkl", "ghi", "ghk", "gkl",
+ trigramsAre({"a", "ab", "ad",
Re: [libcxx] r342073 - Implement the infrastructure for feature-test macros. Very few actual feature test macros, though. Reviewed as: https://reviews.llvm.org/D51955
On Wed, Oct 3, 2018 at 3:38 AM Christof Douma wrote: > Hi. > > > > Yes, including would try to include the “version” file inside > the users project. The problem is not the existence of the header file, but > the #include directive that is not guarded. To give examples on when this > goes wrong: > > > > A project uses VERSION in their source directory to hold some version > string used in the build system. On platforms like Windows and OS X this > file is indistinguishable from the system include file that many headers > include. > > > > I don’t think this is a strange setup, and while I expect that for C++20 > code bases, people need to pick a different name, I think that existing > projects should not be bothered by this. It would be nice if everybody was > using -iquote, or better in my opinion, that -I was behaving like -iquote. > But a fix that we can apply now is to use: > > > > #if _LIBCPP_STD_VER > 17 > > #include > > #endif > > > > Would that be acceptable? > Christof - There are people in the community who really want these feature macros. In particular, they *really* want them for C++14 and C++17 (not just c++20) In general, I am against back-porting new features to old language versions (string_view being the one large exception), but this seems like a low-risk thing to do. However, it would make your suggestion unfeasible. -- Marshall > > > Thanks, > > Christof > > > > *From: *Eric Fiselier > *Date: *Tuesday, 2 October 2018 at 19:52 > *To: *Christof Douma > *Cc: *Marshall Clow , cfe-commits < > cfe-commits@lists.llvm.org>, nd > *Subject: *Re: [libcxx] r342073 - Implement the infrastructure for > feature-test macros. Very few actual feature test macros, though. Reviewed > as: https://reviews.llvm.org/D51955 > > > > > > On Tue, Oct 2, 2018 at 1:33 PM Christof Douma via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > > Hi Marshall. > > I think that this patch breaks backwards compatibility. Assumes that the > header file "version" is used by C++ projects that use a C++ standard that > did not specify a 'version' header. Many toolchains will put search paths > specified with -I in front of the system search path. The result is that > the application header file is included whenever a standard header file is > included. That is unexpected and can break builds. > > Do you agree this is an issue or do you consider this an issue with the > way toolchains handle include search paths? > > > > If I understand correctly, you have user code which provides a header > called "version", and now when files like include they > pick up the user header instead of the STL one? > > Are you specifying custom libc++ include paths when this occurs? Or just > passing "-stdlib=libc++" and letting the compiler do the rest? > > > > In general, I don't consider this a bug. There is no way for libc++ to > make the file disappear in older dialects, and libc++ headers are > free to include w/e additional headers > > they need. > > > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r343778 - [clangd] clangd-indexer gathers refs and stores them in index files.
Author: sammccall
Date: Thu Oct 4 07:09:55 2018
New Revision: 343778
URL: http://llvm.org/viewvc/llvm-project?rev=343778&view=rev
Log:
[clangd] clangd-indexer gathers refs and stores them in index files.
Reviewers: ioeric
Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D52531
Modified:
clang-tools-extra/trunk/clangd/index/IndexAction.cpp
clang-tools-extra/trunk/clangd/index/IndexAction.h
clang-tools-extra/trunk/clangd/index/Serialization.cpp
clang-tools-extra/trunk/clangd/index/Serialization.h
clang-tools-extra/trunk/clangd/index/YAMLSerialization.cpp
clang-tools-extra/trunk/clangd/indexer/IndexerMain.cpp
clang-tools-extra/trunk/unittests/clangd/SerializationTests.cpp
Modified: clang-tools-extra/trunk/clangd/index/IndexAction.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/IndexAction.cpp?rev=343778&r1=343777&r2=343778&view=diff
==
--- clang-tools-extra/trunk/clangd/index/IndexAction.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/IndexAction.cpp Thu Oct 4 07:09:55
2018
@@ -13,10 +13,11 @@ public:
IndexAction(std::shared_ptr C,
std::unique_ptr Includes,
const index::IndexingOptions &Opts,
- std::function &SymbolsCallback)
+ std::function SymbolsCallback,
+ std::function RefsCallback)
: WrapperFrontendAction(index::createIndexingAction(C, Opts, nullptr)),
-SymbolsCallback(SymbolsCallback), Collector(C),
-Includes(std::move(Includes)),
+SymbolsCallback(SymbolsCallback), RefsCallback(RefsCallback),
+Collector(C), Includes(std::move(Includes)),
PragmaHandler(collectIWYUHeaderMaps(this->Includes.get())) {}
std::unique_ptr CreateASTConsumer(CompilerInstance &CI,
@@ -41,10 +42,13 @@ public:
return;
}
SymbolsCallback(Collector->takeSymbols());
+if (RefsCallback != nullptr)
+ RefsCallback(Collector->takeRefs());
}
private:
std::function SymbolsCallback;
+ std::function RefsCallback;
std::shared_ptr Collector;
std::unique_ptr Includes;
std::unique_ptr PragmaHandler;
@@ -54,20 +58,23 @@ private:
std::unique_ptr
createStaticIndexingAction(SymbolCollector::Options Opts,
- std::function SymbolsCallback) {
+ std::function SymbolsCallback,
+ std::function RefsCallback) {
index::IndexingOptions IndexOpts;
IndexOpts.SystemSymbolFilter =
index::IndexingOptions::SystemSymbolFilterKind::All;
Opts.CollectIncludePath = true;
Opts.CountReferences = true;
Opts.Origin = SymbolOrigin::Static;
+ if (RefsCallback != nullptr)
+Opts.RefFilter = RefKind::All;
auto Includes = llvm::make_unique();
addSystemHeadersMapping(Includes.get());
Opts.Includes = Includes.get();
return llvm::make_unique(
std::make_shared(std::move(Opts)), std::move(Includes),
- IndexOpts, SymbolsCallback);
-}
+ IndexOpts, SymbolsCallback, RefsCallback);
+};
} // namespace clangd
} // namespace clang
Modified: clang-tools-extra/trunk/clangd/index/IndexAction.h
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/IndexAction.h?rev=343778&r1=343777&r2=343778&view=diff
==
--- clang-tools-extra/trunk/clangd/index/IndexAction.h (original)
+++ clang-tools-extra/trunk/clangd/index/IndexAction.h Thu Oct 4 07:09:55 2018
@@ -21,10 +21,13 @@ namespace clangd {
// Only a subset of SymbolCollector::Options are respected:
// - include paths are always collected, and canonicalized appropriately
// - references are always counted
+// - main-file refs are collected (if RefsCallback is non-null)
// - the symbol origin is always Static
+// FIXME: refs from headers should also be collected.
std::unique_ptr
createStaticIndexingAction(SymbolCollector::Options Opts,
- std::function SymbolsCallback);
+ std::function SymbolsCallback,
+ std::function RefsCallback);
} // namespace clangd
} // namespace clang
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=343778&r1=343777&r2=343778&view=diff
==
--- clang-tools-extra/trunk/clangd/index/Serialization.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/Serialization.cpp Thu Oct 4 07:09:55
2018
@@ -298,17 +298,47 @@ Symbol readSymbol(Reader &Data, ArrayRef
return Sym;
}
+// REFS ENCODING
+// A refs section has data grouped by Symbol. Each symbol has:
+// - SymbolID: 20 bytes
+// - NumRefs: varint
+// - Ref[Num
[PATCH] D52531: [clangd] clangd-indexer gathers refs and stores them in index files.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL343778: [clangd] clangd-indexer gathers refs and stores them
in index files. (authored by sammccall, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D52531
Files:
clang-tools-extra/trunk/clangd/index/IndexAction.cpp
clang-tools-extra/trunk/clangd/index/IndexAction.h
clang-tools-extra/trunk/clangd/index/Serialization.cpp
clang-tools-extra/trunk/clangd/index/Serialization.h
clang-tools-extra/trunk/clangd/index/YAMLSerialization.cpp
clang-tools-extra/trunk/clangd/indexer/IndexerMain.cpp
clang-tools-extra/trunk/unittests/clangd/SerializationTests.cpp
Index: clang-tools-extra/trunk/clangd/indexer/IndexerMain.cpp
===
--- clang-tools-extra/trunk/clangd/indexer/IndexerMain.cpp
+++ clang-tools-extra/trunk/clangd/indexer/IndexerMain.cpp
@@ -67,18 +67,30 @@
else
Symbols.insert(Sym);
}
+ },
+ [&](RefSlab S) {
+ std::lock_guard Lock(SymbolsMu);
+ for (const auto &Sym : S) {
+ // No need to merge as currently all Refs are from main file.
+ for (const auto &Ref : Sym.second)
+ Refs.insert(Sym.first, Ref);
+ }
})
.release();
}
// Awkward: we write the result in the destructor, because the executor
// takes ownership so it's the easiest way to get our data back out.
- ~IndexActionFactory() { Result.Symbols = std::move(Symbols).build(); }
+ ~IndexActionFactory() {
+Result.Symbols = std::move(Symbols).build();
+Result.Refs = std::move(Refs).build();
+ }
private:
IndexFileIn &Result;
std::mutex SymbolsMu;
SymbolSlab::Builder Symbols;
+ RefSlab::Builder Refs;
};
} // namespace
Index: clang-tools-extra/trunk/clangd/index/Serialization.h
===
--- clang-tools-extra/trunk/clangd/index/Serialization.h
+++ clang-tools-extra/trunk/clangd/index/Serialization.h
@@ -38,26 +38,29 @@
// Holds the contents of an index file that was read.
struct IndexFileIn {
llvm::Optional Symbols;
+ llvm::Optional Refs;
};
-// Parse an index file. The input must be a RIFF container chunk.
+// Parse an index file. The input must be a RIFF or YAML file.
llvm::Expected readIndexFile(llvm::StringRef);
// Specifies the contents of an index file to be written.
struct IndexFileOut {
- const SymbolSlab *Symbols;
- // TODO: Support serializing symbol occurrences.
+ const SymbolSlab *Symbols = nullptr;
+ const RefSlab *Refs = nullptr;
// TODO: Support serializing Dex posting lists.
IndexFileFormat Format = IndexFileFormat::RIFF;
IndexFileOut() = default;
IndexFileOut(const IndexFileIn &I)
- : Symbols(I.Symbols ? I.Symbols.getPointer() : nullptr) {}
+ : Symbols(I.Symbols ? I.Symbols.getPointer() : nullptr),
+Refs(I.Refs ? I.Refs.getPointer() : nullptr) {}
};
// Serializes an index file.
llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const IndexFileOut &O);
// Convert a single symbol to YAML, a nice debug representation.
std::string toYAML(const Symbol &);
+std::string toYAML(const std::pair> &);
// Build an in-memory static index from an index file.
// The size should be relatively small, so data can be managed in memory.
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
@@ -298,17 +298,47 @@
return Sym;
}
+// REFS ENCODING
+// A refs section has data grouped by Symbol. Each symbol has:
+// - SymbolID: 20 bytes
+// - NumRefs: varint
+// - Ref[NumRefs]
+// Fields of Ref are encoded in turn, see implementation.
+
+void writeRefs(const SymbolID &ID, ArrayRef Refs,
+ const StringTableOut &Strings, raw_ostream &OS) {
+ OS << ID.raw();
+ writeVar(Refs.size(), OS);
+ for (const auto &Ref : Refs) {
+OS.write(static_cast(Ref.Kind));
+writeLocation(Ref.Location, Strings, OS);
+ }
+}
+
+std::pair> readRefs(Reader &Data,
+ ArrayRef Strings) {
+ std::pair> Result;
+ Result.first = Data.consumeID();
+ Result.second.resize(Data.consumeVar());
+ for (auto &Ref : Result.second) {
+Ref.Kind = static_cast(Data.consume8());
+Ref.Location = readLocation(Data, Strings);
+ }
+ return Result;
+}
+
// FILE ENCODING
// A file is a RIFF chunk with type 'CdIx'.
// It contains the sections:
// - meta: version number
// - stri: string table
// - symb: symbols
+// - refs: references to symbols
// The current versioning scheme is simple - non-current versions are rejected.
// If you make
[PATCH] D52872: [clangd] Make binary index format the default, remove dead flag.
sammccall updated this revision to Diff 168287.
sammccall added a comment.
example filename "clangd.dex"
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52872
Files:
clangd/indexer/IndexerMain.cpp
Index: clangd/indexer/IndexerMain.cpp
===
--- clangd/indexer/IndexerMain.cpp
+++ clangd/indexer/IndexerMain.cpp
@@ -7,8 +7,7 @@
//
//===--===//
//
-// GlobalSymbolBuilder is a tool to extract symbols from a whole project.
-// This tool is **experimental** only. Don't use it in production code.
+// clangd-indexer is a tool to gather index data (symbols, xrefs) from source.
//
//===--===//
@@ -21,7 +20,6 @@
#include "clang/Tooling/Execution.h"
#include "clang/Tooling/Tooling.h"
#include "llvm/Support/CommandLine.h"
-#include "llvm/Support/Path.h"
#include "llvm/Support/Signals.h"
using namespace llvm;
@@ -32,15 +30,6 @@
namespace clangd {
namespace {
-static llvm::cl::opt AssumedHeaderDir(
-"assume-header-dir",
-llvm::cl::desc("The index includes header that a symbol is defined in. "
- "If the absolute path cannot be determined (e.g. an "
- "in-memory VFS) then the relative path is resolved against "
- "this directory, which must be absolute. If this flag is "
- "not given, such headers will have relative paths."),
-llvm::cl::init(""));
-
static llvm::cl::opt
Format("format", llvm::cl::desc("Format of the index to be written"),
llvm::cl::values(clEnumValN(IndexFileFormat::YAML, "yaml",
@@ -55,7 +44,6 @@
clang::FrontendAction *create() override {
SymbolCollector::Options Opts;
-Opts.FallbackDir = AssumedHeaderDir;
return createStaticIndexingAction(
Opts,
[&](SymbolSlab S) {
@@ -90,15 +78,14 @@
const char *Overview = R"(
Creates an index of symbol information etc in a whole project.
- This is **experimental** and not production-ready!
Example usage for a project using CMake compile commands:
- $ clangd-indexer --executor=all-TUs compile_commands.json > index.yaml
+ $ clangd-indexer --executor=all-TUs compile_commands.json > clangd.dex
Example usage for file sequence index without flags:
- $ clangd-indexer File1.cpp File2.cpp ... FileN.cpp > index.yaml
+ $ clangd-indexer File1.cpp File2.cpp ... FileN.cpp > clangd.dex
Note: only symbols from header files will be indexed.
)";
@@ -111,12 +98,6 @@
return 1;
}
- if (!clang::clangd::AssumedHeaderDir.empty() &&
- !llvm::sys::path::is_absolute(clang::clangd::AssumedHeaderDir)) {
-llvm::errs() << "--assume-header-dir must be an absolute path.\n";
-return 1;
- }
-
// Collect symbols found in each translation unit, merging as we go.
clang::clangd::IndexFileIn Data;
auto Err = Executor->get()->execute(
Index: clangd/indexer/IndexerMain.cpp
===
--- clangd/indexer/IndexerMain.cpp
+++ clangd/indexer/IndexerMain.cpp
@@ -7,8 +7,7 @@
//
//===--===//
//
-// GlobalSymbolBuilder is a tool to extract symbols from a whole project.
-// This tool is **experimental** only. Don't use it in production code.
+// clangd-indexer is a tool to gather index data (symbols, xrefs) from source.
//
//===--===//
@@ -21,7 +20,6 @@
#include "clang/Tooling/Execution.h"
#include "clang/Tooling/Tooling.h"
#include "llvm/Support/CommandLine.h"
-#include "llvm/Support/Path.h"
#include "llvm/Support/Signals.h"
using namespace llvm;
@@ -32,15 +30,6 @@
namespace clangd {
namespace {
-static llvm::cl::opt AssumedHeaderDir(
-"assume-header-dir",
-llvm::cl::desc("The index includes header that a symbol is defined in. "
- "If the absolute path cannot be determined (e.g. an "
- "in-memory VFS) then the relative path is resolved against "
- "this directory, which must be absolute. If this flag is "
- "not given, such headers will have relative paths."),
-llvm::cl::init(""));
-
static llvm::cl::opt
Format("format", llvm::cl::desc("Format of the index to be written"),
llvm::cl::values(clEnumValN(IndexFileFormat::YAML, "yaml",
@@ -55,7 +44,6 @@
clang::FrontendAction *create() override {
SymbolCollector::Options Opts;
-Opts.FallbackDir = AssumedHeaderDir;
return createStaticIndexingAction(
Opts,
[&](SymbolSlab S) {
@@ -90,15 +78,14 @@
const char *Overview = R"(
Creates an index of symbol information etc in a whole project.
- This is **experimental** and not production-ready!
Example usage
[PATCH] D52872: [clangd] Make binary index format the default, remove dead flag.
sammccall marked an inline comment as done. sammccall added inline comments. Comment at: clangd/indexer/IndexerMain.cpp:58 SymbolCollector::Options Opts; -Opts.FallbackDir = AssumedHeaderDir; return createStaticIndexingAction( hokein wrote: > If we remove the `assume-header-dir`, we probably remove `FallbackDir` too? I > can't remember why we added this option, there is no usage internally either. It's used in our internal `mr_indexer` Comment at: clangd/indexer/IndexerMain.cpp:84 - $ clangd-indexer --executor=all-TUs compile_commands.json > index.yaml + $ clangd-indexer --executor=all-TUs compile_commands.json > clangd-index sammccall wrote: > hokein wrote: > > ilya-biryukov wrote: > > > Maybe we should suggest a default extensions here? E.g. `index.riff`? > > +1 > I considered this but am not sure it's useful: > - means an unneccesary change to workflows if we change the format (happened > once!) > - riff is associated with audio, and I'd expect OSes that care about > extensions to draw the wrong conclusion :-) Settled on `clangd.dex` which is at least easy to understand and only kind of misleading. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52872 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
