[PATCH] D51638: [clangd] Load static index asynchronously, add tracing.
ilya-biryukov added inline comments. Comment at: clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp:287 +StaticIdx.reset(Placeholder = new SwapIndex(llvm::make_unique())); +runAsync([Placeholder] { + if (auto Idx = loadIndex(YamlSymbolFile)) Wouldn't the future returned by `runAsync` wait in destructor? Comment at: clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp:288 +runAsync([Placeholder] { + if (auto Idx = loadIndex(YamlSymbolFile)) +Placeholder->reset(std::move(Idx)); What happens if clangd tries to exit before the index is loaded? Could lead to crashes. Repository: rL LLVM https://reviews.llvm.org/D51638 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50438: [clangd] Sort GoToDefinition results.
ilya-biryukov added a comment. In https://reviews.llvm.org/D50438#1224289, @hokein wrote:> > Thanks, and sorry for the delay here, I was planning to submit it after > https://reviews.llvm.org/D50958 is submitted (probably today?)-- because it > requires some rebasing changes. Thanks for the update and sorry for disturbing on your leave time. If you want, happy to rebase and land this after https://reviews.llvm.org/D50958 is submitted (just so that you can enjoy your leave without further disturbance ;-)) Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50438 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r341449 - [clangd] Tune macro quality scoring for code completion.
Author: ioeric Date: Wed Sep 5 00:40:38 2018 New Revision: 341449 URL: http://llvm.org/viewvc/llvm-project?rev=341449&view=rev Log: [clangd] Tune macro quality scoring for code completion. x0.2 seems to be too much penalty, macros might be wanted in some cases; changing to 0.5x instead. The tuning didn't affect ranking for non-macro completions. Modified: clang-tools-extra/trunk/clangd/Quality.cpp Modified: clang-tools-extra/trunk/clangd/Quality.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Quality.cpp?rev=341449&r1=341448&r2=341449&view=diff == --- clang-tools-extra/trunk/clangd/Quality.cpp (original) +++ clang-tools-extra/trunk/clangd/Quality.cpp Wed Sep 5 00:40:38 2018 @@ -221,7 +221,7 @@ float SymbolQualitySignals::evaluate() c Score *= 0.8f; break; case Macro: -Score *= 0.2f; +Score *= 0.5f; break; case Unknown: case Constructor: // No boost constructors so they are after class types. ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51638: [clangd] Load static index asynchronously, add tracing.
sammccall added a comment. I think you're right - this isn't actually asynchronous (I didn't manage to test that!), and if it were it'd interfere with clean shutdown. I think both issues can be addressed by assigning the future to a variable scoped to main. Will send a patch. Repository: rL LLVM https://reviews.llvm.org/D51638 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51036: clang-format: Fix formatting C++ namespaces with preceding 'inline' or 'export' specifier
This revision was automatically updated to reflect the committed changes. Closed by commit rL341450: clang-format: Fix formatting C++ namespaces with preceding 'inline' or 'export'… (authored by sammccall, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D51036 Files: cfe/trunk/lib/Format/Format.cpp cfe/trunk/lib/Format/FormatToken.h cfe/trunk/lib/Format/NamespaceEndCommentsFixer.cpp cfe/trunk/lib/Format/TokenAnnotator.h cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp cfe/trunk/lib/Format/UnwrappedLineParser.cpp cfe/trunk/unittests/Format/FormatTest.cpp Index: cfe/trunk/lib/Format/UnwrappedLineParser.cpp === --- cfe/trunk/lib/Format/UnwrappedLineParser.cpp +++ cfe/trunk/lib/Format/UnwrappedLineParser.cpp @@ -992,13 +992,6 @@ case tok::kw_namespace: parseNamespace(); return; - case tok::kw_inline: -nextToken(); -if (FormatTok->Tok.is(tok::kw_namespace)) { - parseNamespace(); - return; -} -break; case tok::kw_public: case tok::kw_protected: case tok::kw_private: @@ -1066,6 +1059,16 @@ parseJavaScriptEs6ImportExport(); return; } +if (!Style.isCpp()) + break; +// Handle C++ "(inline|export) namespace". +LLVM_FALLTHROUGH; + case tok::kw_inline: +nextToken(); +if (FormatTok->Tok.is(tok::kw_namespace)) { + parseNamespace(); + return; +} break; case tok::identifier: if (FormatTok->is(TT_ForEachMacro)) { Index: cfe/trunk/lib/Format/Format.cpp === --- cfe/trunk/lib/Format/Format.cpp +++ cfe/trunk/lib/Format/Format.cpp @@ -1309,8 +1309,7 @@ std::set DeletedLines; for (unsigned i = 0, e = AnnotatedLines.size(); i != e; ++i) { auto &Line = *AnnotatedLines[i]; - if (Line.startsWith(tok::kw_namespace) || - Line.startsWith(tok::kw_inline, tok::kw_namespace)) { + if (Line.startsWithNamespace()) { checkEmptyNamespace(AnnotatedLines, i, i, DeletedLines); } } @@ -1347,9 +1346,7 @@ if (AnnotatedLines[CurrentLine]->startsWith(tok::r_brace)) break; - if (AnnotatedLines[CurrentLine]->startsWith(tok::kw_namespace) || - AnnotatedLines[CurrentLine]->startsWith(tok::kw_inline, - tok::kw_namespace)) { + if (AnnotatedLines[CurrentLine]->startsWithNamespace()) { if (!checkEmptyNamespace(AnnotatedLines, CurrentLine, NewLine, DeletedLines)) return false; Index: cfe/trunk/lib/Format/NamespaceEndCommentsFixer.cpp === --- cfe/trunk/lib/Format/NamespaceEndCommentsFixer.cpp +++ cfe/trunk/lib/Format/NamespaceEndCommentsFixer.cpp @@ -125,12 +125,7 @@ if (StartLineIndex > 0) NamespaceTok = AnnotatedLines[StartLineIndex - 1]->First; } - // Detect "(inline)? namespace" in the beginning of a line. - if (NamespaceTok->is(tok::kw_inline)) -NamespaceTok = NamespaceTok->getNextNonComment(); - if (!NamespaceTok || NamespaceTok->isNot(tok::kw_namespace)) -return nullptr; - return NamespaceTok; + return NamespaceTok->getNamespaceToken(); } NamespaceEndCommentsFixer::NamespaceEndCommentsFixer(const Environment &Env, Index: cfe/trunk/lib/Format/TokenAnnotator.h === --- cfe/trunk/lib/Format/TokenAnnotator.h +++ cfe/trunk/lib/Format/TokenAnnotator.h @@ -105,6 +105,13 @@ return !Last->isOneOf(tok::semi, tok::comment); } + /// \c true if this line starts a namespace definition. + bool startsWithNamespace() const { +return startsWith(tok::kw_namespace) || + startsWith(tok::kw_inline, tok::kw_namespace) || + startsWith(tok::kw_export, tok::kw_namespace); + } + FormatToken *First; FormatToken *Last; Index: cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp === --- cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp +++ cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp @@ -535,7 +535,7 @@ Tok->SpacesRequiredBefore = 0; Tok->CanBreakBefore = true; return 1; - } else if (Limit != 0 && !Line.startsWith(tok::kw_namespace) && + } else if (Limit != 0 && !Line.startsWithNamespace() && !startsExternCBlock(Line)) { // We don't merge short records. FormatToken *RecordTok = Line.First; @@ -1160,7 +1160,7 @@ // Remove empty lines after "{". if (!Style.KeepEmptyLinesAtTheStartOfBlocks && PreviousLine && PreviousLine->Last->is(tok::l_brace) && - PreviousLine->First->isNot(tok::kw_namespace) && + !PreviousLine->startsWithNamespace() && !startsExternCBlock(*PreviousLine)) Newlines = 1; Index: cfe/trunk/lib/F
[PATCH] D50488: [Analyzer] Checker for non-determinism caused by sorting of pointer-like elements
whisperity added a subscriber: martong. whisperity added a comment. Make sure you use only the C++ projects like BitCoin, LLVM/Clang, ProtoBuf and such from the Xazax CSA Test suite because this checker is not really applicable to C projects. Generally I like the idea (personally I've ran into a similar issue to ordering of sets in Java missed and on another OS we failed the tests where they tried to stringify elements and compare the output...) very much, this is one of those subleties you can really miss when writing and when reviewing code, and it will only bite you in the ass months down the line. I think I'll summon @martong here, //ASTMatchers// are not my fortée, but he has been tinkering with them much recently. Comment at: lib/StaticAnalyzer/Checkers/PointerSortingChecker.cpp:108-110 +void ento::registerPointerSortingChecker(CheckerManager &Mgr) { + Mgr.registerChecker(); +} Speaking of not being applicable to C code, I think this snippet here should allow you to skip running the checker if you are not on a C++ file: ``` if (!Mgr.getLangOpts().CPlusPlus) return; ``` Repository: rC Clang https://reviews.llvm.org/D50488 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r341450 - clang-format: Fix formatting C++ namespaces with preceding 'inline' or 'export' specifier
Author: sammccall Date: Wed Sep 5 00:44:02 2018 New Revision: 341450 URL: http://llvm.org/viewvc/llvm-project?rev=341450&view=rev Log: clang-format: Fix formatting C++ namespaces with preceding 'inline' or 'export' specifier This fixes formatting namespaces with preceding 'inline' and 'export' (Modules TS) specifiers. This change fixes namespaces not being identified as such with preceding 'inline' or 'export' specifiers. Motivation: I was experimenting with the Modules TS (-fmodules-ts) and found it would be useful if clang-format would correctly format 'export namespace'. While making the changes, I noticed that similar issues still exist with 'inline namespace', and addressed them as well. Patch by Marco Elver! Reviewers: klimek, djasper, owenpan, sammccall Reviewed By: owenpan, sammccall Subscribers: owenpan, cfe-commits Differential Revision: https://reviews.llvm.org/D51036 Modified: cfe/trunk/lib/Format/Format.cpp cfe/trunk/lib/Format/FormatToken.h cfe/trunk/lib/Format/NamespaceEndCommentsFixer.cpp cfe/trunk/lib/Format/TokenAnnotator.h cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp cfe/trunk/lib/Format/UnwrappedLineParser.cpp cfe/trunk/unittests/Format/FormatTest.cpp Modified: cfe/trunk/lib/Format/Format.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=341450&r1=341449&r2=341450&view=diff == --- cfe/trunk/lib/Format/Format.cpp (original) +++ cfe/trunk/lib/Format/Format.cpp Wed Sep 5 00:44:02 2018 @@ -1309,8 +1309,7 @@ private: std::set DeletedLines; for (unsigned i = 0, e = AnnotatedLines.size(); i != e; ++i) { auto &Line = *AnnotatedLines[i]; - if (Line.startsWith(tok::kw_namespace) || - Line.startsWith(tok::kw_inline, tok::kw_namespace)) { + if (Line.startsWithNamespace()) { checkEmptyNamespace(AnnotatedLines, i, i, DeletedLines); } } @@ -1347,9 +1346,7 @@ private: if (AnnotatedLines[CurrentLine]->startsWith(tok::r_brace)) break; - if (AnnotatedLines[CurrentLine]->startsWith(tok::kw_namespace) || - AnnotatedLines[CurrentLine]->startsWith(tok::kw_inline, - tok::kw_namespace)) { + if (AnnotatedLines[CurrentLine]->startsWithNamespace()) { if (!checkEmptyNamespace(AnnotatedLines, CurrentLine, NewLine, DeletedLines)) return false; Modified: cfe/trunk/lib/Format/FormatToken.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/FormatToken.h?rev=341450&r1=341449&r2=341450&view=diff == --- cfe/trunk/lib/Format/FormatToken.h (original) +++ cfe/trunk/lib/Format/FormatToken.h Wed Sep 5 00:44:02 2018 @@ -520,8 +520,8 @@ struct FormatToken { const FormatToken *NamespaceTok = this; if (is(tok::comment)) NamespaceTok = NamespaceTok->getNextNonComment(); -// Detect "(inline)? namespace" in the beginning of a line. -if (NamespaceTok && NamespaceTok->is(tok::kw_inline)) +// Detect "(inline|export)? namespace" in the beginning of a line. +if (NamespaceTok && NamespaceTok->isOneOf(tok::kw_inline, tok::kw_export)) NamespaceTok = NamespaceTok->getNextNonComment(); return NamespaceTok && NamespaceTok->is(tok::kw_namespace) ? NamespaceTok : nullptr; Modified: cfe/trunk/lib/Format/NamespaceEndCommentsFixer.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/NamespaceEndCommentsFixer.cpp?rev=341450&r1=341449&r2=341450&view=diff == --- cfe/trunk/lib/Format/NamespaceEndCommentsFixer.cpp (original) +++ cfe/trunk/lib/Format/NamespaceEndCommentsFixer.cpp Wed Sep 5 00:44:02 2018 @@ -125,12 +125,7 @@ getNamespaceToken(const AnnotatedLine *L if (StartLineIndex > 0) NamespaceTok = AnnotatedLines[StartLineIndex - 1]->First; } - // Detect "(inline)? namespace" in the beginning of a line. - if (NamespaceTok->is(tok::kw_inline)) -NamespaceTok = NamespaceTok->getNextNonComment(); - if (!NamespaceTok || NamespaceTok->isNot(tok::kw_namespace)) -return nullptr; - return NamespaceTok; + return NamespaceTok->getNamespaceToken(); } NamespaceEndCommentsFixer::NamespaceEndCommentsFixer(const Environment &Env, Modified: cfe/trunk/lib/Format/TokenAnnotator.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.h?rev=341450&r1=341449&r2=341450&view=diff == --- cfe/trunk/lib/Format/TokenAnnotator.h (original) +++ cfe/trunk/lib/Format/TokenAnnotator.h Wed Sep 5 00:44:02 2018 @@ -105,6 +105,13 @@ public: return !Last->isOneOf(tok::semi, tok::comment); } + /// \c true if this
[clang-tools-extra] r341451 - [clangd] Fix buildbot failures on older compilers from r341375
Author: sammccall Date: Wed Sep 5 00:52:49 2018 New Revision: 341451 URL: http://llvm.org/viewvc/llvm-project?rev=341451&view=rev Log: [clangd] Fix buildbot failures on older compilers from r341375 Modified: clang-tools-extra/trunk/clangd/RIFF.cpp clang-tools-extra/trunk/clangd/index/Serialization.cpp Modified: clang-tools-extra/trunk/clangd/RIFF.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/RIFF.cpp?rev=341451&r1=341450&r2=341451&view=diff == --- clang-tools-extra/trunk/clangd/RIFF.cpp (original) +++ clang-tools-extra/trunk/clangd/RIFF.cpp Wed Sep 5 00:52:49 2018 @@ -36,11 +36,11 @@ Expected readChunk(StringRef &Str return makeError("nonzero padding byte"); Stream = Stream.drop_front(); } - return C; + return std::move(C); }; raw_ostream &operator<<(raw_ostream &OS, const Chunk &C) { - OS.write(C.ID.begin(), C.ID.size()); + OS.write(C.ID.data(), C.ID.size()); char Size[4]; llvm::support::endian::write32le(Size, C.Data.size()); OS.write(Size, sizeof(Size)); @@ -65,7 +65,7 @@ llvm::Expected readFile(llvm::Stri F.Chunks.push_back(*Chunk); } else return Chunk.takeError(); - return F; + return std::move(F); } raw_ostream &operator<<(raw_ostream &OS, const File &F) { @@ -77,7 +77,7 @@ raw_ostream &operator<<(raw_ostream &OS, char Size[4]; llvm::support::endian::write32le(Size, DataLen); OS.write(Size, sizeof(Size)); - OS.write(F.Type.begin(), F.Type.size()); + OS.write(F.Type.data(), F.Type.size()); for (const auto &C : F.Chunks) OS << C; return OS; 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=341451&r1=341450&r2=341451&view=diff == --- clang-tools-extra/trunk/clangd/index/Serialization.cpp (original) +++ clang-tools-extra/trunk/clangd/index/Serialization.cpp Wed Sep 5 00:52:49 2018 @@ -151,7 +151,7 @@ Expected readStringTable( Table.Strings.push_back(Saver.save(consume(Rest, Len))); Rest = Rest.drop_front(); } - return Table; + return std::move(Table); } // SYMBOL ENCODING @@ -272,7 +272,7 @@ Expected readSymbol(StringRef &D } #undef READ_STRING - return Sym; + return std::move(Sym); } } // namespace @@ -322,7 +322,7 @@ Expected readIndexFile(Stri return Sym.takeError(); Result.Symbols = std::move(Symbols).build(); } - return Result; + return std::move(Result); } raw_ostream &operator<<(raw_ostream &OS, const IndexFileOut &Data) { ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [clang-tools-extra] r341375 - [clangd] Define a compact binary serialization fomat for symbol slab/index.
Sorry! r341451 should fix this, will keep an eye on the buildbots. On Wed, Sep 5, 2018 at 8:46 AM Mikael Holmén wrote: > Hi Sam, > > This doesn't compile for me. Both clang 3.6.0 and gcc 5.4.0 complain: > > [1/6] Building CXX object > > tools/clang/tools/extra/clangd/CMakeFiles/clangDaemon.dir/index/Serialization.cpp.o > FAILED: > tools/clang/tools/extra/clangd/CMakeFiles/clangDaemon.dir/index/Serialization.cpp.o > > > /usr/bin/clang++ -march=corei7 -DGTEST_HAS_RTTI=0 -D_DEBUG > -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS > -D__STDC_LIMIT_MACROS -Itools/clang/tools/extra/clangd > -I../tools/clang/tools/extra/clangd -I../tools/clang/include > -Itools/clang/include -I/usr/include/libxml2 -Iinclude -I../include > -I/proj/flexasic/app/valgrind/3.11.0/include -fPIC > -fvisibility-inlines-hidden -Werror -Werror=date-time -std=c++11 -Wall > -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual > -Wmissing-field-initializers -pedantic -Wno-long-long > -Wcovered-switch-default -Wnon-virtual-dtor -Wdelete-non-virtual-dtor > -Wstring-conversion -fdiagnostics-color -ffunction-sections > -fdata-sections -fno-common -Woverloaded-virtual -Wno-nested-anon-types > -O3-UNDEBUG -fno-exceptions -fno-rtti -MMD -MT > tools/clang/tools/extra/clangd/CMakeFiles/clangDaemon.dir/index/Serialization.cpp.o > > -MF > tools/clang/tools/extra/clangd/CMakeFiles/clangDaemon.dir/index/Serialization.cpp.o.d > > -o > tools/clang/tools/extra/clangd/CMakeFiles/clangDaemon.dir/index/Serialization.cpp.o > > -c ../tools/clang/tools/extra/clangd/index/Serialization.cpp > ../tools/clang/tools/extra/clangd/index/Serialization.cpp:154:10: error: > no viable conversion from 'clang::clangd::(anonymous > namespace)::StringTableIn' to 'Expected namespace)::StringTableIn>' >return Table; > ^ > ../include/llvm/Support/Error.h:434:41: note: candidate constructor (the > implicit copy constructor) not viable: no known conversion from > 'clang::clangd::(anonymous namespace)::StringTableIn' to 'const > llvm::Expected &' > for 1st argument > template class LLVM_NODISCARD Expected { > ^ > ../include/llvm/Support/Error.h:456:3: note: candidate constructor not > viable: no known conversion from 'clang::clangd::(anonymous > namespace)::StringTableIn' to 'llvm::Error' for 1st argument >Expected(Error Err) >^ > ../include/llvm/Support/Error.h:470:3: note: candidate constructor not > viable: no known conversion from 'clang::clangd::(anonymous > namespace)::StringTableIn' to 'llvm::ErrorSuccess' for 1st argument >Expected(ErrorSuccess) = delete; >^ > ../include/llvm/Support/Error.h:488:3: note: candidate constructor not > viable: no known conversion from 'clang::clangd::(anonymous > namespace)::StringTableIn' to 'llvm::Expected namespace)::StringTableIn> &&' for 1st argument >Expected(Expected &&Other) { moveConstruct(std::move(Other)); } >^ > ../include/llvm/Support/Error.h:476:36: note: candidate template > ignored: disabled by 'enable_if' [with OtherT = > clang::clangd::(anonymous namespace)::StringTableIn &] > typename std::enable_if T>::value>::type > ^ > ../include/llvm/Support/Error.h:493:3: note: candidate template ignored: > could not match 'Expected' against > 'clang::clangd::(anonymous namespace)::StringTableIn' >Expected(Expected &&Other, >^ > In file included from > ../tools/clang/tools/extra/clangd/index/Serialization.cpp:9: > In file included from > ../tools/clang/tools/extra/clangd/index/Serialization.h:23: > In file included from ../tools/clang/tools/extra/clangd/index/Index.h:13: > In file included from ../tools/clang/include/clang/Index/IndexSymbol.h:14: > In file included from ../tools/clang/include/clang/Lex/MacroInfo.h:18: > In file included from ../tools/clang/include/clang/Lex/Token.h:17: > In file included from > ../tools/clang/include/clang/Basic/SourceLocation.h:19: > In file included from ../include/llvm/ADT/StringRef.h:13: > In file included from ../include/llvm/ADT/STLExtras.h:20: > ../include/llvm/ADT/Optional.h:41:28: error: call to implicitly-deleted > copy constructor of 'clang::clangd::SymbolSlab' >new (storage.buffer) T(*O.getPointer()); > ^ ~~~ > ../include/llvm/ADT/Optional.h:141:3: note: in instantiation of member > function > 'llvm::optional_detail::OptionalStorage false>::OptionalStorage' requested here >Optional(const Optional &O) = default; >^ > ../tools/clang/tools/extra/clangd/index/Serialization.cpp:325:10: note: > in instantiation of function template specialization > 'llvm::Expected::Expected > &>' requested here >return Result; > ^ > ../tools/clang/tools/extra/clangd/index/Index.h:324:26: note: copy > constructor of 'SymbolSlab' is implicitly deleted because field 'Arena' > has a deleted copy constructor >llvm::BumpPtrAllocator Arena; // Owns Symbol data that the Symbols do > not.
[clang-tools-extra] r341452 - [clangd] Fix typo. NFC
Author: maskray Date: Wed Sep 5 01:01:37 2018 New Revision: 341452 URL: http://llvm.org/viewvc/llvm-project?rev=341452&view=rev Log: [clangd] Fix typo. NFC Modified: clang-tools-extra/trunk/clangd/Cancellation.h clang-tools-extra/trunk/clangd/CodeComplete.cpp clang-tools-extra/trunk/clangd/FindSymbols.cpp clang-tools-extra/trunk/clangd/JSONRPCDispatcher.cpp Modified: clang-tools-extra/trunk/clangd/Cancellation.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Cancellation.h?rev=341452&r1=341451&r2=341452&view=diff == --- clang-tools-extra/trunk/clangd/Cancellation.h (original) +++ clang-tools-extra/trunk/clangd/Cancellation.h Wed Sep 5 01:01:37 2018 @@ -93,7 +93,7 @@ public: /// extra lookups in the Context. bool isCancelled() const { return CT; } - /// Creates a task handle that can be used by an asyn task to check for + /// Creates a task handle that can be used by an async task to check for /// information that can change during it's runtime, like Cancellation. static std::shared_ptr createHandle() { return std::shared_ptr(new Task()); Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.cpp?rev=341452&r1=341451&r2=341452&view=diff == --- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original) +++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Wed Sep 5 01:01:37 2018 @@ -1629,7 +1629,7 @@ CompletionItem CodeCompletion::render(co LSP.sortText = sortText(Score.Total, Name); LSP.filterText = Name; LSP.textEdit = {CompletionTokenRange, RequiredQualifier + Name}; - // Merge continious additionalTextEdits into main edit. The main motivation + // Merge continuous additionalTextEdits into main edit. The main motivation // behind this is to help LSP clients, it seems most of them are confused when // they are provided with additionalTextEdits that are consecutive to main // edit. Modified: clang-tools-extra/trunk/clangd/FindSymbols.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/FindSymbols.cpp?rev=341452&r1=341451&r2=341452&view=diff == --- clang-tools-extra/trunk/clangd/FindSymbols.cpp (original) +++ clang-tools-extra/trunk/clangd/FindSymbols.cpp Wed Sep 5 01:01:37 2018 @@ -226,7 +226,7 @@ public: // We should be only be looking at "local" decls in the main file. if (!SourceMgr.isWrittenInMainFile(NameLoc)) { // Even thought we are visiting only local (non-preamble) decls, - // we can get here when in the presense of "extern" decls. + // we can get here when in the presence of "extern" decls. return true; } const NamedDecl *ND = llvm::dyn_cast(ASTNode.OrigD); Modified: clang-tools-extra/trunk/clangd/JSONRPCDispatcher.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/JSONRPCDispatcher.cpp?rev=341452&r1=341451&r2=341452&view=diff == --- clang-tools-extra/trunk/clangd/JSONRPCDispatcher.cpp (original) +++ clang-tools-extra/trunk/clangd/JSONRPCDispatcher.cpp Wed Sep 5 01:01:37 2018 @@ -28,7 +28,7 @@ namespace { static Key RequestID; static Key RequestOut; -// When tracing, we trace a request and attach the repsonse in reply(). +// When tracing, we trace a request and attach the response in reply(). // Because the Span isn't available, we find the current request using Context. class RequestSpan { RequestSpan(llvm::json::Object *Args) : Args(Args) {} ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51674: [clangd] Fix async index loading (from r341376).
sammccall created this revision. sammccall added a reviewer: ilya-biryukov. Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay, ioeric. This wasn't actually async (due to std::future destructor blocking). If it were, we would have clean shutdown issues if main returned and destroyed Placeholder before the thread is done with it. We could attempt to avoid any blocking by using shared_ptr or weak_ptr tricks so the thread can detect Placeholder's destruction, but there are other potential issues (e.g. loadIndex does tracing, and we'll destroy the tracer...) Instead, once LSPServer::run returns, we wait for the index to finish loading before exiting. Performance is not critical in this situation. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51674 Files: clangd/tool/ClangdMain.cpp Index: clangd/tool/ClangdMain.cpp === --- clangd/tool/ClangdMain.cpp +++ clangd/tool/ClangdMain.cpp @@ -280,11 +280,12 @@ Opts.ResourceDir = ResourceDir; Opts.BuildDynamicSymbolIndex = EnableIndex; std::unique_ptr StaticIdx; + std::future AsyncIndexLoad; // Block exit while loading the index. if (EnableIndex && !YamlSymbolFile.empty()) { // Load the index asynchronously. Meanwhile SwapIndex returns no results. SwapIndex *Placeholder; StaticIdx.reset(Placeholder = new SwapIndex(llvm::make_unique())); -runAsync([Placeholder] { +AsyncIndexLoad = runAsync([Placeholder] { if (auto Idx = loadIndex(YamlSymbolFile)) Placeholder->reset(std::move(Idx)); }); Index: clangd/tool/ClangdMain.cpp === --- clangd/tool/ClangdMain.cpp +++ clangd/tool/ClangdMain.cpp @@ -280,11 +280,12 @@ Opts.ResourceDir = ResourceDir; Opts.BuildDynamicSymbolIndex = EnableIndex; std::unique_ptr StaticIdx; + std::future AsyncIndexLoad; // Block exit while loading the index. if (EnableIndex && !YamlSymbolFile.empty()) { // Load the index asynchronously. Meanwhile SwapIndex returns no results. SwapIndex *Placeholder; StaticIdx.reset(Placeholder = new SwapIndex(llvm::make_unique())); -runAsync([Placeholder] { +AsyncIndexLoad = runAsync([Placeholder] { if (auto Idx = loadIndex(YamlSymbolFile)) Placeholder->reset(std::move(Idx)); }); ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51641: [VFS] Cache the current working directory for the real FS.
sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land. Comment at: lib/Basic/VirtualFileSystem.cpp:248 +private: + // Make sure `CWDCache` update is thread safe in `getCurrentWorkingDirectory`. + mutable std::mutex Mutex; nit: it's slightly more than than, you're also making sure that concurrent getCurrentWorkingDirectory calls are coalesced. I think that's fine and maybe not worth spelling out, but maybe the comment adds as much confusion as it resolves. Comment at: lib/Basic/VirtualFileSystem.cpp:249 + // Make sure `CWDCache` update is thread safe in `getCurrentWorkingDirectory`. + mutable std::mutex Mutex; + mutable std::string CWDCache; CWDMutex? If this was other state added to this class, you would *not* want it using the same mutex (because we're doing actual FS operations with the lock held) Comment at: lib/Basic/VirtualFileSystem.cpp:291 // file system abstraction that allows openat() style interactions. - return llvm::sys::fs::set_current_path(Path); + if (auto EC = llvm::sys::fs::set_current_path(Path)) +return EC; don't you also want to do this under the lock? Otherwise you can write this code: ``` Notification N; RealFileSystem FS; async([&] { FS.setCWD("a"); FS.setCWD("b"); N.notify(); }); async([&] { N.wait(); FS.getCWD(); } ``` and the getCWD call can see "a". Repository: rC Clang https://reviews.llvm.org/D51641 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51036: clang-format: Fix formatting C++ namespaces with preceding 'inline' or 'export' specifier
melver added a comment. In https://reviews.llvm.org/D51036#1223254, @sammccall wrote: > In https://reviews.llvm.org/D51036#1223230, @melver wrote: > > > Awaiting remaining reviewer acceptance. > > > > FYI: I do not have commit commit access -- what is the procedure to commit > > once diff is accepted? > > > > Many thanks! > > > Anyone with commit access can land it for you - I'm happy to do this. > @owenpan any concerns? Great, many thanks for committing. Repository: rL LLVM https://reviews.llvm.org/D51036 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [clang-tools-extra] r341375 - [clangd] Define a compact binary serialization fomat for symbol slab/index.
On 09/05/2018 09:56 AM, Sam McCall wrote: Sorry! r341451 should fix this, will keep an eye on the buildbots. Now it compiles with clang 3.6.0 but with gcc 5.4.0 it fails with /proj/bbi_twh/wh_bbi/x86_64-Linux2/bbigcc/1.5.4.0/crosscompiler/bin/g++ -I/proj/bbi_twh/wh_bbi/x86_64-Linux2/bbilibxml2/1/include -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Itools/clang/tools/extra/clangd/global-symbol-builder -I../tools/clang/tools/extra/clangd/global-symbol-builder -I../tools/clang/include -Itools/clang/include -I/usr/include/libxml2 -Iinclude -I../include -I../tools/clang/tools/extra/clangd/global-symbol-builder/.. -I/repo/app/valgrind/3.11.0/include -fPIC -fvisibility-inlines-hidden -Werror=date-time -std=c++11 -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wno-missing-field-initializers -pedantic -Wno-long-long -Wno-maybe-uninitialized -Wdelete-non-virtual-dtor -Wno-comment -fdiagnostics-color -ffunction-sections -fdata-sections -fno-common -Woverloaded-virtual -fno-strict-aliasing -O3-UNDEBUG -fno-exceptions -fno-rtti -MMD -MT tools/clang/tools/extra/clangd/global-symbol-builder/CMakeFiles/global-symbol-builder.dir/GlobalSymbolBuilderMain.cpp.o -MF tools/clang/tools/extra/clangd/global-symbol-builder/CMakeFiles/global-symbol-builder.dir/GlobalSymbolBuilderMain.cpp.o.d -o tools/clang/tools/extra/clangd/global-symbol-builder/CMakeFiles/global-symbol-builder.dir/GlobalSymbolBuilderMain.cpp.o -c ../tools/clang/tools/extra/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp In file included from ../tools/clang/include/clang/Frontend/CommandLineSourceLoc.h:19:0, from ../tools/clang/include/clang/Frontend/FrontendOptions.h:13, from ../tools/clang/include/clang/Frontend/CompilerInvocation.h:19, from ../tools/clang/include/clang/Frontend/CompilerInstance.h:16, from ../tools/clang/tools/extra/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp:22: ../include/llvm/Support/CommandLine.h:606:52: error: invalid cast from type 'llvm::cl::opt' to type 'int' llvm::cl::OptionEnumValue { FLAGNAME, int(ENUMVAL), DESC } ^ ../tools/clang/tools/extra/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp:67:16: note: in expansion of macro 'clEnumValN' clEnumValN(Format::YAML, "yaml", "human-readable YAML format"), ^ ../include/llvm/Support/CommandLine.h:606:29: error: expected primary-expression before '{' token llvm::cl::OptionEnumValue { FLAGNAME, int(ENUMVAL), DESC } ^ ../tools/clang/tools/extra/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp:67:16: note: in expansion of macro 'clEnumValN' clEnumValN(Format::YAML, "yaml", "human-readable YAML format"), ^ ../include/llvm/Support/CommandLine.h:606:52: error: invalid cast from type 'llvm::cl::opt' to type 'int' llvm::cl::OptionEnumValue { FLAGNAME, int(ENUMVAL), DESC } ^ ../tools/clang/tools/extra/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp:68:16: note: in expansion of macro 'clEnumValN' clEnumValN(Format::Binary, "binary", "binary RIFF format")), ^ ../include/llvm/Support/CommandLine.h:606:29: error: expected primary-expression before '{' token llvm::cl::OptionEnumValue { FLAGNAME, int(ENUMVAL), DESC } ^ ../tools/clang/tools/extra/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp:68:16: note: in expansion of macro 'clEnumValN' clEnumValN(Format::Binary, "binary", "binary RIFF format")), ^ ../tools/clang/tools/extra/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp:69:27: error: 'Format' is not a class, namespace, or enumeration llvm::cl::init(Format::YAML)); ^ ../tools/clang/tools/extra/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp: In function 'int main(int, const char**)': ../tools/clang/tools/extra/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp:276:23: error: 'clang::clangd::Format' is not a class, namespace, or enumeration case clang::clangd::Format::YAML: ^ ../tools/clang/tools/extra/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp:279:23: error: 'clang::clangd::Format' is not a class, namespace, or enumeration case clang::clangd::Format::Binary: { ^ ../tools/clang/tools/extra/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp:275:10: warning: enumeration value 'YAML' not handled in switch [-Wswitch] switch (clang::clangd::Format) { ^ ../tools/clang/tools/extra/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp:275:10: warning: enumeration value 'Binary' not handled in sw
[PATCH] D32902: [Analyzer] Iterator Checker - Part 7: Support for push and pop operations
baloghadamsoftware updated this revision to Diff 163984. baloghadamsoftware added a comment. Updated according to the comments and rebased. https://reviews.llvm.org/D32902 Files: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp test/Analysis/Inputs/system-header-simulator-cxx.h test/Analysis/diagnostics/explicit-suppression.cpp test/Analysis/invalidated-iterator.cpp test/Analysis/iterator-range.cpp Index: test/Analysis/iterator-range.cpp === --- test/Analysis/iterator-range.cpp +++ test/Analysis/iterator-range.cpp @@ -115,8 +115,66 @@ } } +void good_push_back(std::list &L, int n) { + auto i0 = --L.cend(); + L.push_back(n); + *++i0; // no-warning +} + +void bad_push_back(std::list &L, int n) { + auto i0 = --L.cend(); + L.push_back(n); + ++i0; + *++i0; // expected-warning{{Iterator accessed outside of its range}} +} + +void good_pop_back(std::list &L, int n) { + auto i0 = --L.cend(); --i0; + L.pop_back(); + *i0; // no-warning +} + +void bad_pop_back(std::list &L, int n) { + auto i0 = --L.cend(); --i0; + L.pop_back(); + *++i0; // expected-warning{{Iterator accessed outside of its range}} +} + +void good_push_front(std::list &L, int n) { + auto i0 = L.cbegin(); + L.push_front(n); + *--i0; // no-warning +} + +void bad_push_front(std::list &L, int n) { + auto i0 = L.cbegin(); + L.push_front(n); + --i0; + *--i0; // expected-warning{{Iterator accessed outside of its range}} +} + +void good_pop_front(std::list &L, int n) { + auto i0 = ++L.cbegin(); + L.pop_front(); + *i0; // no-warning +} + +void bad_pop_front(std::list &L, int n) { + auto i0 = ++L.cbegin(); + L.pop_front(); + *--i0; // expected-warning{{Iterator accessed outside of its range}} +} + void bad_move(std::list &L1, std::list &L2) { auto i0 = --L2.cend(); L1 = std::move(L2); *++i0; // expected-warning{{Iterator accessed outside of its range}} } + +void bad_move_push_back(std::list &L1, std::list &L2, int n) { + auto i0 = --L2.cend(); + L2.push_back(n); + L1 = std::move(L2); + ++i0; + *++i0; // expected-warning{{Iterator accessed outside of its range}} +} Index: test/Analysis/invalidated-iterator.cpp === --- test/Analysis/invalidated-iterator.cpp +++ test/Analysis/invalidated-iterator.cpp @@ -30,3 +30,170 @@ FL1 = FL2; *i0; // expected-warning{{Invalidated iterator accessed}} } + +void good_push_back_list1(std::list &L, int n) { + auto i0 = L.cbegin(), i1 = L.cend(); + L.push_back(n); + *i0; // no-warning + --i1; // no-warning +} + +void good_push_back_vector1(std::vector &V, int n) { + auto i0 = V.cbegin(), i1 = V.cend(); + V.push_back(n); + *i0; // no-warning +} + +void bad_push_back_vector1(std::vector &V, int n) { + auto i0 = V.cbegin(), i1 = V.cend(); + V.push_back(n); + --i1; // expected-warning{{Invalidated iterator accessed}} +} + +void bad_push_back_deque1(std::deque &D, int n) { + auto i0 = D.cbegin(), i1 = D.cend(); + D.push_back(n); + *i0; // expected-warning{{Invalidated iterator accessed}} + --i1; // expected-warning{{Invalidated iterator accessed}} +} + +void good_emplace_back_list1(std::list &L, int n) { + auto i0 = L.cbegin(), i1 = L.cend(); + L.emplace_back(n); + *i0; // no-warning + --i1; // no-warning +} + +void good_emplace_back_vector1(std::vector &V, int n) { + auto i0 = V.cbegin(), i1 = V.cend(); + V.emplace_back(n); + *i0; // no-warning +} + +void bad_emplace_back_vector1(std::vector &V, int n) { + auto i0 = V.cbegin(), i1 = V.cend(); + V.emplace_back(n); + --i1; // expected-warning{{Invalidated iterator accessed}} +} + +void bad_emplace_back_deque1(std::deque &D, int n) { + auto i0 = D.cbegin(), i1 = D.cend(); + D.emplace_back(n); + *i0; // expected-warning{{Invalidated iterator accessed}} + --i1; // expected-warning{{Invalidated iterator accessed}} +} + +void good_pop_back_list1(std::list &L, int n) { + auto i0 = L.cbegin(), i1 = L.cend(), i2 = i1--; + L.pop_back(); + *i0; // no-warning + *i2; // no-warning +} + +void bad_pop_back_list1(std::list &L, int n) { + auto i0 = L.cbegin(), i1 = L.cend(), i2 = i1--; + L.pop_back(); + *i1; // expected-warning{{Invalidated iterator accessed}} +} + +void good_pop_back_vector1(std::vector &V, int n) { + auto i0 = V.cbegin(), i1 = V.cend(), i2 = i1--; + V.pop_back(); + *i0; // no-warning +} + +void bad_pop_back_vector1(std::vector &V, int n) { + auto i0 = V.cbegin(), i1 = V.cend(), i2 = i1--; + V.pop_back(); + *i1; // expected-warning{{Invalidated iterator accessed}} + --i2; // expected-warning{{Invalidated iterator accessed}} +} + +void good_pop_back_deque1(std::deque &D, int n) { + auto i0 = D.cbegin(), i1 = D.cend(), i2 = i1--; + D.pop_back(); + *i0; // no-warning +} + +void bad_pop_back_deque1(std::deque &D, int n) { + auto i0 = D.cbegin(), i1 = D.cend(), i2 = i1--; + D.pop_back(); + *i1; // expected-warning{{Invalidated iterator accessed}} + --
[PATCH] D32902: [Analyzer] Iterator Checker - Part 7: Support for push and pop operations
baloghadamsoftware marked an inline comment as done. baloghadamsoftware added inline comments. Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:1449-1464 +const CXXRecordDecl *getCXXRecordDecl(const MemRegion *Reg) { + QualType Type; + if (const auto *TVReg = Reg->getAs()) { +Type = TVReg->getValueType(); + } else if (const auto *SymReg = Reg->getAs()) { +Type = SymReg->getSymbol()->getType(); + } else { NoQ wrote: > Would `getDynamicTypeInfo()` be of any help? I changed the function to use `getDynamicTypeInfo()`, but now I had to include a `ProgramStateRef` parameter to the function and to all its callers so it did not become much more simple. Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:1477-1530 +bool isPushBackCall(const FunctionDecl *Func) { + const auto *IdInfo = Func->getIdentifier(); + if (!IdInfo) +return false; + if (Func->getNumParams() != 1) +return false; + return IdInfo->getName() == "push_back"; NoQ wrote: > I guess we should think if we want to use `CallDescription` for these when > D48027 lands. Here we do not need the qualified name. https://reviews.llvm.org/D32902 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51641: [VFS] Cache the current working directory for the real FS.
ioeric updated this revision to Diff 163986. ioeric marked an inline comment as done. ioeric added a comment. - s/Mutex/CWDMutex/ Repository: rC Clang https://reviews.llvm.org/D51641 Files: lib/Basic/VirtualFileSystem.cpp Index: lib/Basic/VirtualFileSystem.cpp === --- lib/Basic/VirtualFileSystem.cpp +++ lib/Basic/VirtualFileSystem.cpp @@ -25,9 +25,9 @@ #include "llvm/ADT/Twine.h" #include "llvm/ADT/iterator_range.h" #include "llvm/Config/llvm-config.h" -#include "llvm/Support/Compiler.h" #include "llvm/Support/Casting.h" #include "llvm/Support/Chrono.h" +#include "llvm/Support/Compiler.h" #include "llvm/Support/Debug.h" #include "llvm/Support/Errc.h" #include "llvm/Support/ErrorHandling.h" @@ -48,6 +48,7 @@ #include #include #include +#include #include #include #include @@ -243,6 +244,9 @@ std::error_code setCurrentWorkingDirectory(const Twine &Path) override; std::error_code getRealPath(const Twine &Path, SmallVectorImpl &Output) const override; +private: + mutable std::mutex CWDMutex; + mutable std::string CWDCache; }; } // namespace @@ -265,10 +269,14 @@ } llvm::ErrorOr RealFileSystem::getCurrentWorkingDirectory() const { + std::lock_guard Lock(CWDMutex); + if (!CWDCache.empty()) +return CWDCache; SmallString<256> Dir; if (std::error_code EC = llvm::sys::fs::current_path(Dir)) return EC; - return Dir.str().str(); + CWDCache = Dir.str(); + return CWDCache; } std::error_code RealFileSystem::setCurrentWorkingDirectory(const Twine &Path) { @@ -279,7 +287,13 @@ // difference for example on network filesystems, where symlinks might be // switched during runtime of the tool. Fixing this depends on having a // file system abstraction that allows openat() style interactions. - return llvm::sys::fs::set_current_path(Path); + if (auto EC = llvm::sys::fs::set_current_path(Path)) +return EC; + + // Invalidate cache. + std::lock_guard Lock(CWDMutex); + CWDCache.clear(); + return std::error_code(); } std::error_code Index: lib/Basic/VirtualFileSystem.cpp === --- lib/Basic/VirtualFileSystem.cpp +++ lib/Basic/VirtualFileSystem.cpp @@ -25,9 +25,9 @@ #include "llvm/ADT/Twine.h" #include "llvm/ADT/iterator_range.h" #include "llvm/Config/llvm-config.h" -#include "llvm/Support/Compiler.h" #include "llvm/Support/Casting.h" #include "llvm/Support/Chrono.h" +#include "llvm/Support/Compiler.h" #include "llvm/Support/Debug.h" #include "llvm/Support/Errc.h" #include "llvm/Support/ErrorHandling.h" @@ -48,6 +48,7 @@ #include #include #include +#include #include #include #include @@ -243,6 +244,9 @@ std::error_code setCurrentWorkingDirectory(const Twine &Path) override; std::error_code getRealPath(const Twine &Path, SmallVectorImpl &Output) const override; +private: + mutable std::mutex CWDMutex; + mutable std::string CWDCache; }; } // namespace @@ -265,10 +269,14 @@ } llvm::ErrorOr RealFileSystem::getCurrentWorkingDirectory() const { + std::lock_guard Lock(CWDMutex); + if (!CWDCache.empty()) +return CWDCache; SmallString<256> Dir; if (std::error_code EC = llvm::sys::fs::current_path(Dir)) return EC; - return Dir.str().str(); + CWDCache = Dir.str(); + return CWDCache; } std::error_code RealFileSystem::setCurrentWorkingDirectory(const Twine &Path) { @@ -279,7 +287,13 @@ // difference for example on network filesystems, where symlinks might be // switched during runtime of the tool. Fixing this depends on having a // file system abstraction that allows openat() style interactions. - return llvm::sys::fs::set_current_path(Path); + if (auto EC = llvm::sys::fs::set_current_path(Path)) +return EC; + + // Invalidate cache. + std::lock_guard Lock(CWDMutex); + CWDCache.clear(); + return std::error_code(); } std::error_code ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51481: [clangd] Implement proximity path boosting for Dex
kbobyrev updated this revision to Diff 163987. kbobyrev marked 14 inline comments as done. kbobyrev added a comment. - Rebase on top of master - Address suggestions Eric wrote here - Address suggestions from the offline discussion by reusing `Quality.cpp` infrastructure for path proximity boosting calculation https://reviews.llvm.org/D51481 Files: clang-tools-extra/clangd/CMakeLists.txt clang-tools-extra/clangd/FileDistance.h clang-tools-extra/clangd/Quality.cpp clang-tools-extra/clangd/index/SymbolYAML.cpp clang-tools-extra/clangd/index/SymbolYAML.h clang-tools-extra/clangd/index/dex/DexIndex.cpp clang-tools-extra/clangd/index/dex/DexIndex.h clang-tools-extra/clangd/index/dex/Iterator.cpp clang-tools-extra/clangd/index/dex/Token.cpp clang-tools-extra/clangd/index/dex/Token.h clang-tools-extra/clangd/tool/ClangdMain.cpp clang-tools-extra/unittests/clangd/DexIndexTests.cpp clang-tools-extra/unittests/clangd/TestIndex.cpp clang-tools-extra/unittests/clangd/TestIndex.h Index: clang-tools-extra/unittests/clangd/TestIndex.h === --- clang-tools-extra/unittests/clangd/TestIndex.h +++ clang-tools-extra/unittests/clangd/TestIndex.h @@ -39,6 +39,13 @@ const FuzzyFindRequest &Req, bool *Incomplete = nullptr); +// Performs fuzzy matching-based symbol lookup given a query and an index. +// Incomplete is set true if more items than requested can be retrieved, false +// otherwise. Returns filename URIs of matched symbols canonical declarations. +std::vector matchDeclURIs(const SymbolIndex &I, + const FuzzyFindRequest &Req, + bool *Incomplete = nullptr); + // Returns qualified names of symbols with any of IDs in the index. std::vector lookup(const SymbolIndex &I, llvm::ArrayRef IDs); Index: clang-tools-extra/unittests/clangd/TestIndex.cpp === --- clang-tools-extra/unittests/clangd/TestIndex.cpp +++ clang-tools-extra/unittests/clangd/TestIndex.cpp @@ -55,6 +55,18 @@ return Matches; } +std::vector matchDeclURIs(const SymbolIndex &I, + const FuzzyFindRequest &Req, + bool *Incomplete) { + std::vector Matches; + bool IsIncomplete = I.fuzzyFind(Req, [&](const Symbol &Sym) { +Matches.push_back(Sym.CanonicalDeclaration.FileURI); + }); + if (Incomplete) +*Incomplete = IsIncomplete; + return Matches; +} + // Returns qualified names of symbols with any of IDs in the index. std::vector lookup(const SymbolIndex &I, llvm::ArrayRef IDs) { Index: clang-tools-extra/unittests/clangd/DexIndexTests.cpp === --- clang-tools-extra/unittests/clangd/DexIndexTests.cpp +++ clang-tools-extra/unittests/clangd/DexIndexTests.cpp @@ -7,6 +7,8 @@ // //===--===// +#include "FuzzyMatch.h" +#include "TestFS.h" #include "TestIndex.h" #include "index/Index.h" #include "index/Merge.h" @@ -30,6 +32,12 @@ namespace dex { namespace { +std::vector URISchemes = {"unittest"}; + +//===--===// +// Query iterator tests. +//===--===// + std::vector consumeIDs(Iterator &It) { auto IDAndScore = consume(It); std::vector IDs(IDAndScore.size()); @@ -325,15 +333,38 @@ EXPECT_THAT(ElementBoost, 3); } +//===--===// +// Search token tests. +//===--===// + testing::Matcher> -trigramsAre(std::initializer_list Trigrams) { +tokensAre(std::initializer_list Strings, Token::Kind Kind) { std::vector Tokens; - for (const auto &Symbols : Trigrams) { -Tokens.push_back(Token(Token::Kind::Trigram, Symbols)); + for (const auto &TokenData : Strings) { +Tokens.push_back(Token(Kind, TokenData)); } return testing::UnorderedElementsAreArray(Tokens); } +testing::Matcher> +trigramsAre(std::initializer_list Trigrams) { + return tokensAre(Trigrams, Token::Kind::Trigram); +} + +testing::Matcher> +pathsAre(std::initializer_list Paths) { + return tokensAre(Paths, Token::Kind::ProximityURI); +} + +testing::Matcher> +proximityPathsAre(std::initializer_list ProximityPaths) { + std::vector Result; + for (const auto &Path : ProximityPaths) { +Result.emplace_back(Token::Kind::ProximityURI, Path); + } + return testing::UnorderedElementsAreArray(Result); +} + TEST(DexIndexTrigrams, IdentifierTrigrams) { EXPECT_THAT(generateIdentifierTrigrams("X86"), trigramsAre({"x86", "x$$", "x8$"
[PATCH] D51297: [docs] Create a guide for Vim users on how to setup Clangd
kbobyrev added a comment. Ping; this one just redirects to the appropriate Wiki entry on GitHub. https://reviews.llvm.org/D51297 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r341455 - [VFS] Cache the current working directory for the real FS.
Author: ioeric Date: Wed Sep 5 02:45:27 2018 New Revision: 341455 URL: http://llvm.org/viewvc/llvm-project?rev=341455&view=rev Log: [VFS] Cache the current working directory for the real FS. Reviewers: sammccall Subscribers: cfe-commits Differential Revision: https://reviews.llvm.org/D51641 Modified: cfe/trunk/lib/Basic/VirtualFileSystem.cpp Modified: cfe/trunk/lib/Basic/VirtualFileSystem.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/VirtualFileSystem.cpp?rev=341455&r1=341454&r2=341455&view=diff == --- cfe/trunk/lib/Basic/VirtualFileSystem.cpp (original) +++ cfe/trunk/lib/Basic/VirtualFileSystem.cpp Wed Sep 5 02:45:27 2018 @@ -49,6 +49,7 @@ #include #include #include +#include #include #include #include @@ -244,6 +245,9 @@ public: std::error_code setCurrentWorkingDirectory(const Twine &Path) override; std::error_code getRealPath(const Twine &Path, SmallVectorImpl &Output) const override; +private: + mutable std::mutex CWDMutex; + mutable std::string CWDCache; }; } // namespace @@ -266,10 +270,14 @@ RealFileSystem::openFileForRead(const Tw } llvm::ErrorOr RealFileSystem::getCurrentWorkingDirectory() const { + std::lock_guard Lock(CWDMutex); + if (!CWDCache.empty()) +return CWDCache; SmallString<256> Dir; if (std::error_code EC = llvm::sys::fs::current_path(Dir)) return EC; - return Dir.str().str(); + CWDCache = Dir.str(); + return CWDCache; } std::error_code RealFileSystem::setCurrentWorkingDirectory(const Twine &Path) { @@ -280,7 +288,13 @@ std::error_code RealFileSystem::setCurre // difference for example on network filesystems, where symlinks might be // switched during runtime of the tool. Fixing this depends on having a // file system abstraction that allows openat() style interactions. - return llvm::sys::fs::set_current_path(Path); + if (auto EC = llvm::sys::fs::set_current_path(Path)) +return EC; + + // Invalidate cache. + std::lock_guard Lock(CWDMutex); + CWDCache.clear(); + return std::error_code(); } std::error_code ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51057: [analyzer][UninitializedObjectChecker] Fixed dereferencing
Szelethus updated this revision to Diff 163992. Szelethus added a comment. Fixed an assertation failure where a lambda field's this capture was undefined. https://reviews.llvm.org/D51057 Files: lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp test/Analysis/cxx-uninitialized-object-ptr-ref.cpp test/Analysis/cxx-uninitialized-object.cpp test/Analysis/objcpp-uninitialized-object.mm Index: test/Analysis/objcpp-uninitialized-object.mm === --- test/Analysis/objcpp-uninitialized-object.mm +++ test/Analysis/objcpp-uninitialized-object.mm @@ -4,7 +4,7 @@ struct StructWithBlock { int a; - myBlock z; // expected-note{{uninitialized pointer 'this->z'}} + myBlock z; // expected-note{{uninitialized field 'this->z'}} StructWithBlock() : a(0), z(^{}) {} Index: test/Analysis/cxx-uninitialized-object.cpp === --- test/Analysis/cxx-uninitialized-object.cpp +++ test/Analysis/cxx-uninitialized-object.cpp @@ -1,11 +1,11 @@ // RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.cplusplus.UninitializedObject \ // RUN: -analyzer-config alpha.cplusplus.UninitializedObject:Pedantic=true -DPEDANTIC \ // RUN: -analyzer-config alpha.cplusplus.UninitializedObject:CheckPointeeInitialization=true \ -// RUN: -std=c++11 -verify %s +// RUN: -std=c++14 -verify %s // RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.cplusplus.UninitializedObject \ // RUN: -analyzer-config alpha.cplusplus.UninitializedObject:CheckPointeeInitialization=true \ -// RUN: -std=c++11 -verify %s +// RUN: -std=c++14 -verify %s //===--===// // Default constructor test. @@ -781,7 +781,7 @@ void fLambdaTest2() { int b; - auto equals = [&b](int a) { return a == b; }; // expected-note{{uninitialized pointee 'this->functor.b'}} + auto equals = [&b](int a) { return a == b; }; // expected-note{{uninitialized pointee 'this->functor./*captured variable*/b'}} LambdaTest2(equals, int()); } #else @@ -803,8 +803,8 @@ namespace LT3Detail { struct RecordType { - int x; // expected-note{{uninitialized field 'this->functor.rec1.x'}} - int y; // expected-note{{uninitialized field 'this->functor.rec1.y'}} + int x; // expected-note{{uninitialized field 'this->functor./*captured variable*/rec1.x'}} + int y; // expected-note{{uninitialized field 'this->functor./*captured variable*/rec1.y'}} }; } // namespace LT3Detail @@ -857,8 +857,8 @@ void fMultipleLambdaCapturesTest1() { int b1, b2 = 3, b3; - auto equals = [&b1, &b2, &b3](int a) { return a == b1 == b2 == b3; }; // expected-note{{uninitialized pointee 'this->functor.b1'}} - // expected-note@-1{{uninitialized pointee 'this->functor.b3'}} + auto equals = [&b1, &b2, &b3](int a) { return a == b1 == b2 == b3; }; // expected-note{{uninitialized pointee 'this->functor./*captured variable*/b1'}} + // expected-note@-1{{uninitialized pointee 'this->functor./*captured variable*/b3'}} MultipleLambdaCapturesTest1(equals, int()); } @@ -872,10 +872,35 @@ void fMultipleLambdaCapturesTest2() { int b1, b2 = 3, b3; - auto equals = [b1, &b2, &b3](int a) { return a == b1 == b2 == b3; }; // expected-note{{uninitialized pointee 'this->functor.b3'}} + auto equals = [b1, &b2, &b3](int a) { return a == b1 == b2 == b3; }; // expected-note{{uninitialized pointee 'this->functor./*captured variable*/b3'}} MultipleLambdaCapturesTest2(equals, int()); } +struct LambdaWrapper { + void *func; // no-crash + int dontGetFilteredByNonPedanticMode = 0; + + LambdaWrapper(void *ptr) : func(ptr) {} // expected-warning{{1 uninitialized field}} +}; + +struct ThisCapturingLambdaFactory { + int a; // expected-note{{uninitialized field 'static_cast(this->func)->/*'this' capture*/->a'}} + + auto ret() { +return [this] { (void)this; }; + } +}; + +void fLambdaFieldWithInvalidThisCapture() { + void *ptr; + { +ThisCapturingLambdaFactory a; +decltype(a.ret()) lambda = a.ret(); +ptr = λ + } + LambdaWrapper t(ptr); +} + //===--===// // System header tests. //===--===// Index: test/Analysis/cxx-uninitialized-object-ptr-ref.cpp === --- test/Analysis/cxx-uninitialized-object-ptr-ref.cpp +++ test/Analysis/cxx-uninitialized-object-ptr-ref.cpp @@ -46,6 +46,50 @@ } //===--===// +// Alloca tests. +//===--===// + +struct UntypedAllocaTest { + void *allocaPtr; +
[PATCH] D51641: [VFS] Cache the current working directory for the real FS.
This revision was automatically updated to reflect the committed changes. Closed by commit rC341455: [VFS] Cache the current working directory for the real FS. (authored by ioeric, committed by ). Changed prior to commit: https://reviews.llvm.org/D51641?vs=163986&id=163994#toc Repository: rC Clang https://reviews.llvm.org/D51641 Files: lib/Basic/VirtualFileSystem.cpp Index: lib/Basic/VirtualFileSystem.cpp === --- lib/Basic/VirtualFileSystem.cpp +++ lib/Basic/VirtualFileSystem.cpp @@ -49,6 +49,7 @@ #include #include #include +#include #include #include #include @@ -244,6 +245,9 @@ std::error_code setCurrentWorkingDirectory(const Twine &Path) override; std::error_code getRealPath(const Twine &Path, SmallVectorImpl &Output) const override; +private: + mutable std::mutex CWDMutex; + mutable std::string CWDCache; }; } // namespace @@ -266,10 +270,14 @@ } llvm::ErrorOr RealFileSystem::getCurrentWorkingDirectory() const { + std::lock_guard Lock(CWDMutex); + if (!CWDCache.empty()) +return CWDCache; SmallString<256> Dir; if (std::error_code EC = llvm::sys::fs::current_path(Dir)) return EC; - return Dir.str().str(); + CWDCache = Dir.str(); + return CWDCache; } std::error_code RealFileSystem::setCurrentWorkingDirectory(const Twine &Path) { @@ -280,7 +288,13 @@ // difference for example on network filesystems, where symlinks might be // switched during runtime of the tool. Fixing this depends on having a // file system abstraction that allows openat() style interactions. - return llvm::sys::fs::set_current_path(Path); + if (auto EC = llvm::sys::fs::set_current_path(Path)) +return EC; + + // Invalidate cache. + std::lock_guard Lock(CWDMutex); + CWDCache.clear(); + return std::error_code(); } std::error_code Index: lib/Basic/VirtualFileSystem.cpp === --- lib/Basic/VirtualFileSystem.cpp +++ lib/Basic/VirtualFileSystem.cpp @@ -49,6 +49,7 @@ #include #include #include +#include #include #include #include @@ -244,6 +245,9 @@ std::error_code setCurrentWorkingDirectory(const Twine &Path) override; std::error_code getRealPath(const Twine &Path, SmallVectorImpl &Output) const override; +private: + mutable std::mutex CWDMutex; + mutable std::string CWDCache; }; } // namespace @@ -266,10 +270,14 @@ } llvm::ErrorOr RealFileSystem::getCurrentWorkingDirectory() const { + std::lock_guard Lock(CWDMutex); + if (!CWDCache.empty()) +return CWDCache; SmallString<256> Dir; if (std::error_code EC = llvm::sys::fs::current_path(Dir)) return EC; - return Dir.str().str(); + CWDCache = Dir.str(); + return CWDCache; } std::error_code RealFileSystem::setCurrentWorkingDirectory(const Twine &Path) { @@ -280,7 +288,13 @@ // difference for example on network filesystems, where symlinks might be // switched during runtime of the tool. Fixing this depends on having a // file system abstraction that allows openat() style interactions. - return llvm::sys::fs::set_current_path(Path); + if (auto EC = llvm::sys::fs::set_current_path(Path)) +return EC; + + // Invalidate cache. + std::lock_guard Lock(CWDMutex); + CWDCache.clear(); + return std::error_code(); } std::error_code ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51675: [Sema] Store MacroInfo in CodeCompletionResult for macro results.
ioeric created this revision. ioeric added a reviewer: sammccall. Herald added a subscriber: cfe-commits. This provides information about the macro definition. For example, it can be used to compute macro USRs. Repository: rC Clang https://reviews.llvm.org/D51675 Files: include/clang/Sema/CodeCompleteConsumer.h lib/Sema/SemaCodeComplete.cpp tools/libclang/CXCursor.cpp Index: tools/libclang/CXCursor.cpp === --- tools/libclang/CXCursor.cpp +++ tools/libclang/CXCursor.cpp @@ -1412,16 +1412,16 @@ } } else if (kind == CXCursor_MacroDefinition) { const MacroDefinitionRecord *definition = getCursorMacroDefinition(cursor); -const IdentifierInfo *MacroInfo = definition->getName(); +const IdentifierInfo *Macro = definition->getName(); ASTUnit *unit = getCursorASTUnit(cursor); -CodeCompletionResult Result(MacroInfo); -CodeCompletionString *String - = Result.CreateCodeCompletionString(unit->getASTContext(), - unit->getPreprocessor(), - CodeCompletionContext::CCC_Other, - unit->getCodeCompletionTUInfo().getAllocator(), - unit->getCodeCompletionTUInfo(), - false); +CodeCompletionResult Result( +Macro, +unit->getPreprocessor().getMacroDefinition(Macro).getMacroInfo()); +CodeCompletionString *String = Result.CreateCodeCompletionString( +unit->getASTContext(), unit->getPreprocessor(), +CodeCompletionContext::CCC_Other, +unit->getCodeCompletionTUInfo().getAllocator(), +unit->getCodeCompletionTUInfo(), false); return String; } return nullptr; Index: lib/Sema/SemaCodeComplete.cpp === --- lib/Sema/SemaCodeComplete.cpp +++ lib/Sema/SemaCodeComplete.cpp @@ -3313,14 +3313,14 @@ M != MEnd; ++M) { auto MD = PP.getMacroDefinition(M->first); if (IncludeUndefined || MD) { - if (MacroInfo *MI = MD.getMacroInfo()) -if (MI->isUsedForHeaderGuard()) - continue; + MacroInfo *MI = MD.getMacroInfo(); + if (MI && MI->isUsedForHeaderGuard()) +continue; - Results.AddResult(Result(M->first, - getMacroUsagePriority(M->first->getName(), - PP.getLangOpts(), - TargetTypeIsPointer))); + Results.AddResult( + Result(M->first, MI, + getMacroUsagePriority(M->first->getName(), PP.getLangOpts(), + TargetTypeIsPointer))); } } Index: include/clang/Sema/CodeCompleteConsumer.h === --- include/clang/Sema/CodeCompleteConsumer.h +++ include/clang/Sema/CodeCompleteConsumer.h @@ -17,6 +17,7 @@ #include "clang-c/Index.h" #include "clang/AST/Type.h" #include "clang/Basic/LLVM.h" +#include "clang/Lex/MacroInfo.h" #include "clang/Sema/CodeCompleteOptions.h" #include "clang/Sema/DeclSpec.h" #include "llvm/ADT/ArrayRef.h" @@ -843,6 +844,10 @@ /// corresponding `using decl::qualified::name;` nearby. const UsingShadowDecl *ShadowDecl = nullptr; + /// If the result is RK_Macro, this can store the information about the macro + /// definition. + const MacroInfo *MacroDefInfo = nullptr; + /// Build a result that refers to a declaration. CodeCompletionResult(const NamedDecl *Declaration, unsigned Priority, NestedNameSpecifier *Qualifier = nullptr, @@ -867,11 +872,13 @@ /// Build a result that refers to a macro. CodeCompletionResult(const IdentifierInfo *Macro, + const MacroInfo *MI = nullptr, unsigned Priority = CCP_Macro) : Macro(Macro), Priority(Priority), Kind(RK_Macro), CursorKind(CXCursor_MacroDefinition), Hidden(false), QualifierIsInformative(false), StartsNestedNameSpecifier(false), -AllParametersAreInformative(false), DeclaringEntity(false) {} +AllParametersAreInformative(false), DeclaringEntity(false), +MacroDefInfo(MI) {} /// Build a result that refers to a pattern. CodeCompletionResult(CodeCompletionString *Pattern, Index: tools/libclang/CXCursor.cpp === --- tools/libclang/CXCursor.cpp +++ tools/libclang/CXCursor.cpp @@ -1412,16 +1412,16 @@ } } else if (kind == CXCursor_MacroDefinition) { const MacroDefinitionRecord *definition = getCursorMacroDefinition(cursor); -const IdentifierInfo *MacroInfo = definition->getName(); +const IdentifierInfo *Macro = definition->getName(); ASTUnit *unit = getCursorASTUnit(cursor); -CodeCompletionResult Result(MacroInfo); -CodeComplet
[PATCH] D51057: [analyzer][UninitializedObjectChecker] Fixed dereferencing
Szelethus added inline comments. Comment at: lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp:126-127 if (V.isUndef()) { +assert(!FR->getDecl()->getType()->isReferenceType() && + "References must be initialized!"); return addFieldToUninits( NoQ wrote: > Good catch. > > It might still be possible to initialize a reference with an > already-undefined pointer if core checkers are turned off, but we don't > support turning them off, so i guess it's fine. I removed it, because it did crash couple times on LLVM. Note that the assert checked whether the reference for undefined, not uninitialized :/. It's no longer in the code, but this was it: ``` assert(!FR->getDecl()->getType()->isReferenceType() && "References must be initialized!"); ``` Comment at: test/Analysis/cxx-uninitialized-object.cpp:879-902 +struct LambdaWrapper { + void *func; // no-crash + int dontGetFilteredByNonPedanticMode = 0; + + LambdaWrapper(void *ptr) : func(ptr) {} // expected-warning{{1 uninitialized field}} +}; + I'm 99% sure this is a FP, but it doesn't originate from the checker. Shouldn't `*ptr` be undef after the end of the code block as `lambda`'s lifetime ends? Nevertheless, it did cause a crash, so here's a quick fix for it. https://reviews.llvm.org/D51057 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51676: [clangd] Use TopN instead of std::priority_queue
kbobyrev created this revision. kbobyrev added reviewers: ioeric, ilya-biryukov, sammccall. kbobyrev added a project: clang-tools-extra. Herald added subscribers: kadircet, arphaman, jkorous, MaskRay. Quality.cpp defines a structure for convenient storage of Top N items, it should be used instead of the `std::priority_queue` with slightly obscure semantics. This patch does not affect functionality. https://reviews.llvm.org/D51676 Files: clang-tools-extra/clangd/index/MemIndex.cpp Index: clang-tools-extra/clangd/index/MemIndex.cpp === --- clang-tools-extra/clangd/index/MemIndex.cpp +++ clang-tools-extra/clangd/index/MemIndex.cpp @@ -10,7 +10,7 @@ #include "MemIndex.h" #include "../FuzzyMatch.h" #include "../Logger.h" -#include +#include "../Quality.h" namespace clang { namespace clangd { @@ -26,7 +26,7 @@ assert(!StringRef(Req.Query).contains("::") && "There must be no :: in query."); - std::priority_queue> Top; + TopN> Top(Req.MaxCandidateCount); FuzzyMatcher Filter(Req.Query); bool More = false; for (const auto Pair : Index) { @@ -39,15 +39,14 @@ continue; if (auto Score = Filter.match(Sym->Name)) { - Top.emplace(-*Score * quality(*Sym), Sym); - if (Top.size() > Req.MaxCandidateCount) { + // Top.push(...) returns true if the capacity is reached and the heap had + // to pop() and item before inserting a new one. + if (Top.push({*Score * quality(*Sym), Sym})) More = true; -Top.pop(); - } } } - for (; !Top.empty(); Top.pop()) -Callback(*Top.top().second); + for (const auto &Item : std::move(Top).items()) +Callback(*Item.second); return More; } Index: clang-tools-extra/clangd/index/MemIndex.cpp === --- clang-tools-extra/clangd/index/MemIndex.cpp +++ clang-tools-extra/clangd/index/MemIndex.cpp @@ -10,7 +10,7 @@ #include "MemIndex.h" #include "../FuzzyMatch.h" #include "../Logger.h" -#include +#include "../Quality.h" namespace clang { namespace clangd { @@ -26,7 +26,7 @@ assert(!StringRef(Req.Query).contains("::") && "There must be no :: in query."); - std::priority_queue> Top; + TopN> Top(Req.MaxCandidateCount); FuzzyMatcher Filter(Req.Query); bool More = false; for (const auto Pair : Index) { @@ -39,15 +39,14 @@ continue; if (auto Score = Filter.match(Sym->Name)) { - Top.emplace(-*Score * quality(*Sym), Sym); - if (Top.size() > Req.MaxCandidateCount) { + // Top.push(...) returns true if the capacity is reached and the heap had + // to pop() and item before inserting a new one. + if (Top.push({*Score * quality(*Sym), Sym})) More = true; -Top.pop(); - } } } - for (; !Top.empty(); Top.pop()) -Callback(*Top.top().second); + for (const auto &Item : std::move(Top).items()) +Callback(*Item.second); return More; } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50958: [clangd] Implement findReferences function
sammccall updated this revision to Diff 164004. sammccall added a comment. Finish tests, fix TestTU::index() to include occurrences. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50958 Files: clangd/XRefs.cpp clangd/XRefs.h unittests/clangd/TestTU.cpp unittests/clangd/XRefsTests.cpp Index: unittests/clangd/XRefsTests.cpp === --- unittests/clangd/XRefsTests.cpp +++ unittests/clangd/XRefsTests.cpp @@ -1068,6 +1068,143 @@ ElementsAre(Location{FooCppUri, FooWithoutHeader.range()})); } +TEST(FindReferences, WithinAST) { + const char *Tests[] = { + R"cpp(// Local variable +int main() { + int $foo[[foo]]; + $foo[[^foo]] = 2; + int test1 = $foo[[foo]]; +} + )cpp", + + R"cpp(// Struct +namespace ns1 { +struct $foo[[Foo]] {}; +} // namespace ns1 +int main() { + ns1::$foo[[Fo^o]]* Params; +} + )cpp", + + R"cpp(// Function +int $foo[[foo]](int) {} +int main() { + auto *X = &$foo[[^foo]]; + $foo[[foo]](42) +} + )cpp", + + R"cpp(// Field +struct Foo { + int $foo[[foo]]; + Foo() : $foo[[foo]](0) {} +}; +int main() { + Foo f; + f.$foo[[f^oo]] = 1; +} + )cpp", + + R"cpp(// Method call +struct Foo { int [[foo]](); }; +int Foo::[[foo]]() {} +int main() { + Foo f; + f.^foo(); +} + )cpp", + + R"cpp(// Typedef +typedef int $foo[[Foo]]; +int main() { + $foo[[^Foo]] bar; +} + )cpp", + + R"cpp(// Namespace +namespace $foo[[ns]] { +struct Foo {}; +} // namespace ns +int main() { $foo[[^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")) + ExpectedLocations.push_back(RangeIs(R)); +EXPECT_THAT(findReferences(AST, T.point()), +ElementsAreArray(ExpectedLocations)) +<< Test; + } +} + +TEST(FindReferences, NeedsIndex) { + const char *Header = "int foo();"; + Annotations Main("int main() { [[f^oo]](); }"); + TestTU TU; + TU.Code = Main.code(); + TU.HeaderCode = Header; + auto AST = TU.build(); + + // References in main file are returned without index. + EXPECT_THAT(findReferences(AST, Main.point(), /*Index=*/nullptr), + ElementsAre(RangeIs(Main.range(; + Annotations IndexedMain(R"cpp( +int main() { [[f^oo]](); } + )cpp"); + + // References from indexed files are included. + TestTU IndexedTU; + IndexedTU.Code = IndexedMain.code(); + IndexedTU.Filename = "Indexed.cpp"; + IndexedTU.HeaderCode = Header; + EXPECT_THAT(findReferences(AST, Main.point(), IndexedTU.index().get()), + ElementsAre(RangeIs(Main.range()), RangeIs(IndexedMain.range(; + + // If the main file is in the index, we don't return duplicates. + // (even if the references are in a different location) + TU.Code = ("\n\n" + Main.code()).str(); + EXPECT_THAT(findReferences(AST, Main.point(), TU.index().get()), + ElementsAre(RangeIs(Main.range(; +}; + +TEST(FindReferences, NoQueryForLocalSymbols) { + struct RecordingIndex : public MemIndex { +mutable Optional> RefIDs; +void refs(const RefsRequest &Req, + llvm::function_ref) const override { + RefIDs = Req.IDs; +} + }; + + struct Test { +StringRef AnnotatedCode; +bool WantQuery; + } Tests[] = { + {"int ^x;", true}, + // For now we don't assume header structure which would allow skipping. + {"namespace { int ^x; }", true}, + {"static int ^x;", true}, + // Anything in a function certainly can't be referenced though. + {"void foo() { int ^x; }", false}, + {"void foo() { struct ^x{}; }", false}, + {"auto lambda = []{ int ^x; };", false}, + }; + for (Test T : Tests) { +Annotations File(T.AnnotatedCode); +RecordingIndex Rec; +auto AST = TestTU::withCode(File.code()).build(); +findReferences(AST, File.point(), &Rec); +if (T.WantQuery) + EXPECT_NE(Rec.RefIDs, llvm::None) << T.AnnotatedCode; +else + EXPECT_EQ(Rec.RefIDs, llvm::None) << T.AnnotatedCode; + } +}; + } // namespace } // namespace clangd } // namespace clang Index: unittests/clangd/TestTU.cpp === --- unittests/clangd/TestTU.cpp +++ unittests/clangd/TestTU.cpp @@ -49,8 +49,10 @@ } std::unique_ptr TestTU::index() const { - // FIXME: we should generate proper refs for TestTU. - return MemIndex::build(headerSymbols(), RefSlab()); + auto AST = build(); + auto Content = indexAST(AST.getASTContext(), AST.getPreprocessorPtr(), +
[PATCH] D51481: [clangd] Implement proximity path boosting for Dex
ioeric added inline comments. Comment at: clang-tools-extra/clangd/index/dex/DexIndex.cpp:59 LookupTable[Sym->ID] = Sym; -SymbolQuality[Sym] = quality(*Sym); +SymbolAndScores[I] = {Sym, static_cast(quality(*Sym))}; } ioeric wrote: > ioeric wrote: > > We should fix `quality` to return `float` for consistency. > nit: Instead of reconstruct the structure, I'd probably set the fields > manually. We shouldn't need the `static_cast` anymore? Comment at: clang-tools-extra/clangd/index/dex/DexIndex.cpp:55 // are stored in the descending order of symbol quality. - std::sort(begin(Symbols), end(Symbols), -[&](const Symbol *LHS, const Symbol *RHS) { - return SymbolQuality[LHS] > SymbolQuality[RHS]; -}); + std::sort(begin(ScoredSymbols), end(ScoredSymbols)); + This sorts in ascending order by default? Comment at: clang-tools-extra/clangd/index/dex/DexIndex.cpp:118 + SymbolRelevanceSignals PathProximitySignals; + for (const auto &ProximityPath : Req.ProximityPaths) { +const auto PathProximityTokens = As chatted offline, we only need a single URIDistacne structure for all proximity paths. And we could also deduplicate parent directories. `generateQueryPathProximityTokens` should return URIs instead of tokens as the token data doesn't need to be raw URIs (e.g. hash). And the function should probably live in DexIndex.cpp instead of Token.cpp. I'd also suggest pulling this code into a helper. Comment at: clang-tools-extra/clangd/index/dex/DexIndex.cpp:159 + + std::vector> IDAndScores = consume(*Root); + I think `using IDAndScore = decltype(IDAndScores.fron())` might help here. Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:287 StaticIdx.reset(Placeholder = new SwapIndex(llvm::make_unique())); -runAsync([Placeholder] { - if (auto Idx = loadIndex(YamlSymbolFile)) +runAsync([Placeholder, Opts] { + if (auto Idx = loadIndex(YamlSymbolFile, Opts.URISchemes, UseDex)) This could take `Opts` by reference. https://reviews.llvm.org/D51481 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r341458 - [clangd] Implement findReferences function
Author: sammccall Date: Wed Sep 5 03:33:36 2018 New Revision: 341458 URL: http://llvm.org/viewvc/llvm-project?rev=341458&view=rev Log: [clangd] Implement findReferences function clangd will use findReferences to provide LSP's reference feature. Modified: clang-tools-extra/trunk/clangd/XRefs.cpp clang-tools-extra/trunk/clangd/XRefs.h clang-tools-extra/trunk/unittests/clangd/TestTU.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=341458&r1=341457&r2=341458&view=diff == --- clang-tools-extra/trunk/clangd/XRefs.cpp (original) +++ clang-tools-extra/trunk/clangd/XRefs.cpp Wed Sep 5 03:33:36 2018 @@ -174,30 +174,27 @@ IdentifiedSymbol getSymbolAtPosition(Par return {DeclMacrosFinder.takeDecls(), DeclMacrosFinder.takeMacroInfos()}; } -llvm::Optional -makeLocation(ParsedAST &AST, const SourceRange &ValSourceRange) { +Range getTokenRange(ParsedAST &AST, SourceLocation TokLoc) { const SourceManager &SourceMgr = AST.getASTContext().getSourceManager(); - const LangOptions &LangOpts = AST.getASTContext().getLangOpts(); - SourceLocation LocStart = ValSourceRange.getBegin(); + SourceLocation LocEnd = Lexer::getLocForEndOfToken( + TokLoc, 0, SourceMgr, AST.getASTContext().getLangOpts()); + return {sourceLocToPosition(SourceMgr, TokLoc), + sourceLocToPosition(SourceMgr, LocEnd)}; +} - const FileEntry *F = - SourceMgr.getFileEntryForID(SourceMgr.getFileID(LocStart)); +llvm::Optional makeLocation(ParsedAST &AST, SourceLocation TokLoc) { + const SourceManager &SourceMgr = AST.getASTContext().getSourceManager(); + const FileEntry *F = SourceMgr.getFileEntryForID(SourceMgr.getFileID(TokLoc)); if (!F) return llvm::None; - SourceLocation LocEnd = Lexer::getLocForEndOfToken(ValSourceRange.getEnd(), 0, - SourceMgr, LangOpts); - Position Begin = sourceLocToPosition(SourceMgr, LocStart); - Position End = sourceLocToPosition(SourceMgr, LocEnd); - Range R = {Begin, End}; - Location L; - auto FilePath = getRealPath(F, SourceMgr); if (!FilePath) { log("failed to get path!"); return llvm::None; } + Location L; L.uri = URIForFile(*FilePath); - L.range = R; + L.range = getTokenRange(AST, TokLoc); return L; } @@ -223,7 +220,7 @@ std::vector findDefinitions(Pa for (auto Item : Symbols.Macros) { auto Loc = Item.Info->getDefinitionLoc(); -auto L = makeLocation(AST, SourceRange(Loc, Loc)); +auto L = makeLocation(AST, Loc); if (L) Result.push_back(*L); } @@ -266,7 +263,7 @@ std::vector findDefinitions(Pa auto &Candidate = ResultCandidates[Key]; auto Loc = findNameLoc(D); -auto L = makeLocation(AST, SourceRange(Loc, Loc)); +auto L = makeLocation(AST, Loc); // The declaration in the identified symbols is a definition if possible // otherwise it is declaration. bool IsDef = getDefinition(D) == D; @@ -316,24 +313,36 @@ std::vector findDefinitions(Pa namespace { -/// Finds document highlights that a given list of declarations refers to. -class DocumentHighlightsFinder : public index::IndexDataConsumer { - std::vector &Decls; - std::vector DocumentHighlights; - const ASTContext &AST; - +/// Collects references to symbols within the main file. +class ReferenceFinder : public index::IndexDataConsumer { public: - DocumentHighlightsFinder(ASTContext &AST, Preprocessor &PP, - std::vector &Decls) - : Decls(Decls), AST(AST) {} - std::vector takeHighlights() { -// Don't keep the same highlight multiple times. -// This can happen when nodes in the AST are visited twice. -std::sort(DocumentHighlights.begin(), DocumentHighlights.end()); -auto Last = -std::unique(DocumentHighlights.begin(), DocumentHighlights.end()); -DocumentHighlights.erase(Last, DocumentHighlights.end()); -return std::move(DocumentHighlights); + struct Reference { +const Decl *Target; +SourceLocation Loc; +index::SymbolRoleSet Role; + }; + + ReferenceFinder(ASTContext &AST, Preprocessor &PP, + const std::vector &TargetDecls) + : AST(AST) { +for (const Decl *D : TargetDecls) + Targets.insert(D); + } + + 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); + }); +// 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) { + r
[clang-tools-extra] r341459 - [clangd] Avoid enum class+enumValN to avoid GCC bug(?), and use consistent style.
Author: sammccall Date: Wed Sep 5 03:39:58 2018 New Revision: 341459 URL: http://llvm.org/viewvc/llvm-project?rev=341459&view=rev Log: [clangd] Avoid enum class+enumValN to avoid GCC bug(?), and use consistent style. Modified: clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp Modified: clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp?rev=341459&r1=341458&r2=341459&view=diff == --- clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp (original) +++ clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp Wed Sep 5 03:39:58 2018 @@ -60,7 +60,7 @@ static llvm::cl::opt MergeOnTheFly "MapReduce."), llvm::cl::init(true), llvm::cl::Hidden); -enum class Format { YAML, Binary }; +enum Format { YAML, Binary }; static llvm::cl::opt Format("format", llvm::cl::desc("Format of the index to be written"), llvm::cl::values( Modified: clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp?rev=341459&r1=341458&r2=341459&view=diff == --- clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp (original) +++ clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp Wed Sep 5 03:39:58 2018 @@ -36,12 +36,6 @@ static llvm::cl::opt llvm::cl::desc("Use experimental Dex static index."), llvm::cl::init(true), llvm::cl::Hidden); -namespace { - -enum class PCHStorageFlag { Disk, Memory }; - -} // namespace - static llvm::cl::opt CompileCommandsDir( "compile-commands-dir", llvm::cl::desc("Specify a path to look for compile_commands.json. If path " @@ -54,10 +48,7 @@ static llvm::cl::opt llvm::cl::init(getDefaultAsyncThreadsCount())); // FIXME: also support "plain" style where signatures are always omitted. -enum CompletionStyleFlag { - Detailed, - Bundled, -}; +enum CompletionStyleFlag { Detailed, Bundled }; static llvm::cl::opt CompletionStyle( "completion-style", llvm::cl::desc("Granularity of code completion suggestions"), @@ -106,6 +97,7 @@ static llvm::cl::opt Test( "Intended to simplify lit tests."), llvm::cl::init(false), llvm::cl::Hidden); +enum PCHStorageFlag { Disk, Memory }; static llvm::cl::opt PCHStorage( "pch-storage", llvm::cl::desc("Storing PCHs in memory increases memory usages, but may " @@ -167,7 +159,6 @@ static llvm::cl::opt YamlSymbolFil llvm::cl::init(""), llvm::cl::Hidden); enum CompileArgsFrom { LSPCompileArgs, FilesystemCompileArgs }; - static llvm::cl::opt CompileArgsFrom( "compile_args_from", llvm::cl::desc("The source of compile commands"), llvm::cl::values(clEnumValN(LSPCompileArgs, "lsp", ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r341460 - Fix -Wdocumentation warning. NFCI.
Author: rksimon Date: Wed Sep 5 03:44:03 2018 New Revision: 341460 URL: http://llvm.org/viewvc/llvm-project?rev=341460&view=rev Log: Fix -Wdocumentation warning. NFCI. Modified: cfe/trunk/lib/Sema/SemaType.cpp Modified: cfe/trunk/lib/Sema/SemaType.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaType.cpp?rev=341460&r1=341459&r2=341460&view=diff == --- cfe/trunk/lib/Sema/SemaType.cpp (original) +++ cfe/trunk/lib/Sema/SemaType.cpp Wed Sep 5 03:44:03 2018 @@ -6385,7 +6385,7 @@ static NullabilityKind mapNullabilityAtt /// /// \param attr The attribute as written on the type. /// -/// \param allowArrayTypes Whether to accept nullability specifiers on an +/// \param allowOnArrayType Whether to accept nullability specifiers on an /// array type (e.g., because it will decay to a pointer). /// /// \returns true if a problem has been diagnosed, false on success. ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50488: [Analyzer] Checker for non-determinism caused by sorting of pointer-like elements
martong added a comment. From this little information I have hear are my thoughts: > match callExpr(allOf (callee(functionDecl(hasName("std::sort"))), > hasArgument(0, > hasDescendant(declRefExpr(to(fieldDecl(hasName("value_type")) I think this is a good direction, but keep in mind that `value_type` is a typedef, thus you should use the `typedefNameDecl` matcher instead of the `fieldDecl`. (Also if I understand correctly then this is good that this matcher does not match in case of the `intPointerArray` example, because the array does not have any member at all ...) Repository: rC Clang https://reviews.llvm.org/D50488 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32904: [Analyzer] Iterator Checker - Part 8: Support for assign, clear, insert, emplace and erase operations
baloghadamsoftware updated this revision to Diff 164006. baloghadamsoftware added a comment. Herald added subscribers: Szelethus, mikhail.ramalho. Rebased. https://reviews.llvm.org/D32904 Files: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp test/Analysis/Inputs/system-header-simulator-cxx.h test/Analysis/diagnostics/explicit-suppression.cpp test/Analysis/invalidated-iterator.cpp test/Analysis/mismatched-iterator.cpp Index: test/Analysis/mismatched-iterator.cpp === --- test/Analysis/mismatched-iterator.cpp +++ test/Analysis/mismatched-iterator.cpp @@ -3,6 +3,40 @@ #include "Inputs/system-header-simulator-cxx.h" +void good_insert1(std::vector &v, int n) { + v.insert(v.cbegin(), n); // no-warning +} + + +void good_insert2(std::vector &v, int len, int n) { + v.insert(v.cbegin(), len, n); // no-warning +} + +void good_insert3(std::vector &v1, std::vector &v2) { + v1.insert(v1.cbegin(), v2.cbegin(), v2.cend()); // no-warning +} + +void good_insert4(std::vector &v, int len, int n) { + v.insert(v.cbegin(), {n-1, n, n+1}); // no-warning +} + +void good_insert_find(std::vector &v, int n, int m) { + auto i = std::find(v.cbegin(), v.cend(), n); + v.insert(i, m); // no-warning +} + +void good_erase1(std::vector &v) { + v.erase(v.cbegin()); // no-warning +} + +void good_erase2(std::vector &v) { + v.erase(v.cbegin(), v.cend()); // no-warning +} + +void good_emplace(std::vector &v, int n) { + v.emplace(v.cbegin(), n); // no-warning +} + void good_ctor(std::vector &v) { std::vector new_v(v.cbegin(), v.cend()); // no-warning } @@ -25,6 +59,38 @@ std::find(i0, v1.cend(), n); // no-warning } +void bad_insert1(std::vector &v1, std::vector &v2, int n) { + v2.insert(v1.cbegin(), n); // expected-warning{{Container accessed using foreign iterator argument}} +} + +void bad_insert2(std::vector &v1, std::vector &v2, int len, int n) { + v2.insert(v1.cbegin(), len, n); // expected-warning{{Container accessed using foreign iterator argument}} +} + +void bad_insert3(std::vector &v1, std::vector &v2) { + v2.insert(v1.cbegin(), v2.cbegin(), v2.cend()); // expected-warning{{Container accessed using foreign iterator argument}} + v1.insert(v1.cbegin(), v1.cbegin(), v2.cend()); // expected-warning{{Iterators of different containers used where the same container is expected}} + v1.insert(v1.cbegin(), v2.cbegin(), v1.cend()); // expected-warning{{Iterators of different containers used where the same container is expected}} +} + +void bad_insert4(std::vector &v1, std::vector &v2, int len, int n) { + v2.insert(v1.cbegin(), {n-1, n, n+1}); // expected-warning{{Container accessed using foreign iterator argument}} +} + +void bad_erase1(std::vector &v1, std::vector &v2) { + v2.erase(v1.cbegin()); // expected-warning{{Container accessed using foreign iterator argument}} +} + +void bad_erase2(std::vector &v1, std::vector &v2) { + v2.erase(v2.cbegin(), v1.cend()); // expected-warning{{Container accessed using foreign iterator argument}} + v2.erase(v1.cbegin(), v2.cend()); // expected-warning{{Container accessed using foreign iterator argument}} + v2.erase(v1.cbegin(), v1.cend()); // expected-warning{{Container accessed using foreign iterator argument}} +} + +void bad_emplace(std::vector &v1, std::vector &v2, int n) { + v2.emplace(v1.cbegin(), n); // expected-warning{{Container accessed using foreign iterator argument}} +} + void good_move_find2(std::vector &v1, std::vector &v2, int n) { auto i0 = --v2.cend(); v1 = std::move(v2); @@ -61,6 +127,35 @@ std::find(i0, v2.cend(), n); // expected-warning{{Iterators of different containers used where the same container is expected}} } +void bad_insert_find(std::vector &v1, std::vector &v2, int n, int m) { + auto i = std::find(v1.cbegin(), v1.cend(), n); + v2.insert(i, m); // expected-warning{{Container accessed using foreign iterator argument}} +} + +void good_overwrite(std::vector &v1, std::vector &v2, int n) { + auto i = v1.cbegin(); + i = v2.cbegin(); + v2.insert(i, n); // no-warning +} + +void bad_overwrite(std::vector &v1, std::vector &v2, int n) { + auto i = v1.cbegin(); + i = v2.cbegin(); + v1.insert(i, n); // expected-warning{{Container accessed using foreign iterator argument}} +} + +void good_move(std::vector &v1, std::vector &v2) { + const auto i0 = ++v2.cbegin(); + v1 = std::move(v2); + v1.erase(i0); // no-warning +} + +void bad_move(std::vector &v1, std::vector &v2) { + const auto i0 = ++v2.cbegin(); + v1 = std::move(v2); + v2.erase(i0); // expected-warning{{Container accessed using foreign iterator argument}} +} + void bad_move_find2(std::vector &v1, std::vector &v2, int n) { auto i0 = --v2.cend(); v1 = std::move(v2); Index: test/Analysis/invalidated-iterator.cpp === --- test/Analysis/invalidated-iterator.cpp +++ test/Analysis/invalidated-iterator.cpp @@ -31,6 +31,56 @@ *i0; // expecte
[PATCH] D50958: [clangd] Implement findReferences function
sammccall added inline comments. Comment at: clangd/XRefs.cpp:328 + : AST(AST) { +for (const Decl *D : TargetDecls) + Targets.insert(D); hokein wrote: > nit: we can initialize TargetDecls like `Targets(TargetDecls.begin(), > TargetDecls.end())`. In fact `SmallSet` is missing this constructor :-( Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50958 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50958: [clangd] Implement findReferences function
sammccall closed this revision. sammccall added a comment. Oops, I forgot to associate my patch with this review. It landed as https://reviews.llvm.org/rL341458. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50958 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51417: [analyzer][UninitializedObjectChecker] Updated comments
Szelethus added a comment. Polite ping ^-^ Repository: rC Clang https://reviews.llvm.org/D51417 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51679: [analyzer][UninitializedObjectChecker] Refactored checker options
Szelethus created this revision. Szelethus added reviewers: george.karpenkov, NoQ, xazax.hun, rnkovacs. Herald added subscribers: cfe-commits, mikhail.ramalho, a.sidorin, szepet, whisperity. Since I plan to add a number of new flags, it made sense to encapsulate them in a new struct, in order not to pollute `FindUninitializedFields`s constructor with new boolean options with super long names. This revision practically reverts https://reviews.llvm.org/D50508, since `FindUninitializedFields` now accesses the pedantic flag anyways. I guess that revision was a poor design choice :^). Repository: rC Clang https://reviews.llvm.org/D51679 Files: lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp Index: lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp === --- lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp +++ lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp @@ -10,11 +10,8 @@ // This file defines functions and methods for handling pointers and references // to reduce the size and complexity of UninitializedObjectChecker.cpp. // -// To read about command line options and a description what this checker does, -// refer to UninitializedObjectChecker.cpp. -// -// To read about how the checker works, refer to the comments in -// UninitializedObject.h. +// To read about command line options and documentation about how the checker +// works, refer to UninitializedObjectChecker.h. // //===--===// @@ -127,7 +124,7 @@ LocalChain.add(LocField(FR, /*IsDereferenced*/ false))); } - if (!CheckPointeeInitialization) { + if (!Opts.CheckPointeeInitialization) { IsAnyFieldInitialized = true; return false; } Index: lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp === --- lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp +++ lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp @@ -10,36 +10,8 @@ // This file defines a checker that reports uninitialized fields in objects // created after a constructor call. // -// This checker has several options: -// - "Pedantic" (boolean). If its not set or is set to false, the checker -// won't emit warnings for objects that don't have at least one initialized -// field. This may be set with -// -// `-analyzer-config alpha.cplusplus.UninitializedObject:Pedantic=true`. -// -// - "NotesAsWarnings" (boolean). If set to true, the checker will emit a -// warning for each uninitalized field, as opposed to emitting one warning -// per constructor call, and listing the uninitialized fields that belongs -// to it in notes. Defaults to false. -// -// `-analyzer-config \ -// alpha.cplusplus.UninitializedObject:NotesAsWarnings=true`. -// -// - "CheckPointeeInitialization" (boolean). If set to false, the checker will -// not analyze the pointee of pointer/reference fields, and will only check -// whether the object itself is initialized. Defaults to false. -// -// `-analyzer-config \ -// alpha.cplusplus.UninitializedObject:CheckPointeeInitialization=true`. -// -// TODO: With some clever heuristics, some pointers should be dereferenced -// by default. For example, if the pointee is constructed within the -// constructor call, it's reasonable to say that no external object -// references it, and we wouldn't generate multiple report on the same -// pointee. -// -// To read about how the checker works, refer to the comments in -// UninitializedObject.h. +// To read about command line options and how the checker works, refer to the +// top of the file and inline comments in UninitializedObject.h. // // Some of the logic is implemented in UninitializedPointee.cpp, to reduce the // complexity of this file. @@ -62,10 +34,8 @@ std::unique_ptr BT_uninitField; public: - // These fields will be initialized when registering the checker. - bool IsPedantic; - bool ShouldConvertNotesToWarnings; - bool CheckPointeeInitialization; + // The fields of this struct will be initialized when registering the checker. + UninitObjCheckerOptions Opts; UninitializedObjectChecker() : BT_uninitField(new BuiltinBug(this, "Uninitialized fields")) {} @@ -163,20 +133,13 @@ if (!Object) return; - FindUninitializedFields F(Context.getState(), Object->getRegion(), -CheckPointeeInitialization); + FindUninitializedFields F(Context.getState(), Object->getRegion(), Opts); const UninitFieldMap &UninitFields = F.getUninitFields();
[PATCH] D50896: [clangd] Add xrefs LSP boilerplate implementation.
sammccall commandeered this revision. sammccall added a reviewer: hokein. sammccall added a comment. Herald added a subscriber: kadircet. (Taking this as @hokein is on leave and the dependency has landed in https://reviews.llvm.org/D50958) Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50896 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50896: [clangd] Add xrefs LSP boilerplate implementation.
sammccall updated this revision to Diff 164010. sammccall added a comment. Updated based on findReferences implementation which has now landed. Removed ReferenceContext support for now, implementation has no options. references->findReferences in ClangdServer (more consistent with other methods) Added lit test for this feature. Set this feature to true in LSP capabilities. Made initialize-params-invalid less brittle instead of updating it. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50896 Files: clangd/ClangdLSPServer.cpp clangd/ClangdLSPServer.h clangd/ClangdServer.cpp clangd/ClangdServer.h clangd/Protocol.cpp clangd/Protocol.h clangd/ProtocolHandlers.cpp clangd/ProtocolHandlers.h clangd/XRefs.cpp test/clangd/initialize-params-invalid.test test/clangd/initialize-params.test test/clangd/references.test Index: test/clangd/references.test === --- /dev/null +++ test/clangd/references.test @@ -0,0 +1,40 @@ +# RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s +{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}} +--- +{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,"text":"int x; int y = x;"}}} +--- +{"jsonrpc":"2.0","id":1,"method":"textDocument/references","params":{"textDocument":{"uri":"test:///main.cpp"},"position":{"line":0,"character":4}}} +# CHECK: "id": 1 +# CHECK-NEXT: "jsonrpc": "2.0", +# CHECK-NEXT: "result": [ +# CHECK-NEXT:{ +# CHECK-NEXT: "range": { +# CHECK-NEXT:"end": { +# CHECK-NEXT: "character": 5, +# CHECK-NEXT: "line": 0 +# CHECK-NEXT:}, +# CHECK-NEXT:"start": { +# CHECK-NEXT: "character": 4, +# CHECK-NEXT: "line": 0 +# CHECK-NEXT:} +# CHECK-NEXT: }, +# CHECK-NEXT: "uri": "test:///main.cpp" +# CHECK-NEXT:}, +# CHECK-NEXT:{ +# CHECK-NEXT: "range": { +# CHECK-NEXT:"end": { +# CHECK-NEXT: "character": 16, +# CHECK-NEXT: "line": 0 +# CHECK-NEXT:}, +# CHECK-NEXT:"start": { +# CHECK-NEXT: "character": 15, +# CHECK-NEXT: "line": 0 +# CHECK-NEXT:} +# CHECK-NEXT: }, +# CHECK-NEXT: "uri": "test:///main.cpp" +# CHECK-NEXT:}, +# CHECK-NEXT: ] +--- +{"jsonrpc":"2.0","id":3,"method":"shutdown"} +--- +{"jsonrpc":"2.0","method":"exit"} Index: test/clangd/initialize-params.test === --- test/clangd/initialize-params.test +++ test/clangd/initialize-params.test @@ -29,6 +29,7 @@ # CHECK-NEXT:] # CHECK-NEXT: }, # CHECK-NEXT: "hoverProvider": true, +# CHECK-NEXT: "referencesProvider": true, # CHECK-NEXT: "renameProvider": true, # CHECK-NEXT: "signatureHelpProvider": { # CHECK-NEXT:"triggerCharacters": [ Index: test/clangd/initialize-params-invalid.test === --- test/clangd/initialize-params-invalid.test +++ test/clangd/initialize-params-invalid.test @@ -5,41 +5,7 @@ # CHECK-NEXT: "jsonrpc": "2.0", # CHECK-NEXT: "result": { # CHECK-NEXT:"capabilities": { -# CHECK-NEXT: "codeActionProvider": true, -# CHECK-NEXT: "completionProvider": { -# CHECK-NEXT:"resolveProvider": false, -# CHECK-NEXT:"triggerCharacters": [ -# CHECK-NEXT: ".", -# CHECK-NEXT: ">", -# CHECK-NEXT: ":" -# CHECK-NEXT:] -# CHECK-NEXT: }, -# CHECK-NEXT: "definitionProvider": true, -# CHECK-NEXT: "documentFormattingProvider": true, -# CHECK-NEXT: "documentHighlightProvider": true, -# CHECK-NEXT: "documentOnTypeFormattingProvider": { -# CHECK-NEXT:"firstTriggerCharacter": "}", -# CHECK-NEXT:"moreTriggerCharacter": [] -# CHECK-NEXT: }, -# CHECK-NEXT: "documentRangeFormattingProvider": true, -# CHECK-NEXT: "documentSymbolProvider": true, -# CHECK-NEXT: "executeCommandProvider": { -# CHECK-NEXT:"commands": [ -# CHECK-NEXT: "clangd.applyFix" -# CHECK-NEXT:] -# CHECK-NEXT: }, -# CHECK-NEXT: "hoverProvider": true, -# CHECK-NEXT: "renameProvider": true, -# CHECK-NEXT: "signatureHelpProvider": { -# CHECK-NEXT:"triggerCharacters": [ -# CHECK-NEXT: "(", -# CHECK-NEXT: "," -# CHECK-NEXT:] -# CHECK-NEXT: }, -# CHECK-NEXT: "textDocumentSync": 2, -# CHECK-NEXT: "workspaceSymbolProvider": true -# CHECK-NEXT:} -# CHECK-NEXT: } +# ... --- {"jsonrpc":"2.0","id":3,"method":"shutdown"} --- Index: clangd/XRefs.cpp === --- clangd/XRefs.cpp +++ clangd/XRefs.cpp @@ -17,6 +17,7 @@ #include "clang/Index/IndexingAction.h" #include "clang/Index/USRGenera
Re: [PATCH] D51354: Fix the -print-multi-directory flag to print the selected multilib.
Hello all, Is there a way to see how the patch looked like after my commit and before the revert from the git mirror ? I cannot reproduce the test failure from the bots in my local build, (which is based on the the git mirror). but I noticed that a similar behavior however occurs when the test patch is applied twice. (I have exactly the same error if I duplicate the lines in print-multi-directory.c) I'm curious to see if there was a difference between the review D51354 patch and the actual svn commit with arcanist that I might not have used correctly. That was actually my first commit. What I did was from a clean subversion update arc patch D51354 arc commit --revision D51354 I'm not sure what could be wrong, any idea ? thanks Christian On 09/05/2018 12:11 AM, Tim Shen via Phabricator wrote: > timshen added a comment. > >> The test fails on my system like so: > I also observed the same failure. > > Bots also fail: > http://lab.llvm.org:8011/builders/clang-x86_64-linux-selfhost-modules/builds/19509/steps/check-all/logs/FAIL%3A%20Clang%3A%3Aprint-multi-directory.c > > I'm going to revert this patch. > > > Repository: >rC Clang > > https://reviews.llvm.org/D51354 > > > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50896: [clangd] Add xrefs LSP boilerplate implementation.
sammccall marked 4 inline comments as done. sammccall added a comment. PTAL Comment at: clangd/ClangdServer.h:158 + /// Retrieve locations for symbol references. + void references(PathRef File, Position Pos, bool includeDeclaration, + Callback> CB); ioeric wrote: > ioeric wrote: > > I think the C++ API can return `SymbolOccurrence` in the callback, which > > would allow C++ API users to get richer information about the occurrences > > e.g. kind, relationship, code snippet. > nit: s/includeDeclaration/IncludeDeclaration/ I hadn't seen this comment, I agree, but the implementation that landed just returns Locations. We can extend the C++ API but it doesn't belong in this patch. Comment at: clangd/XRefs.cpp:665 +std::vector references(ParsedAST &AST, Position Pos, + bool IncludeDeclaration, + const SymbolIndex *Index) { hokein wrote: > ilya-biryukov wrote: > > Are we going to support the `IncludeDeclaration` option? > > What should its semantics be? > > > > I'm trying to figure out a better name for it, can't get what should it do > > by looking at the code at the moment :-) > I think so. It is a term defined in LSP (although vscode always sets it > `true`). I think it aligns with the `clang::index::SymbolRole::Declaration`. It's not supported in the underlying implementation today, partly because we weren't sure about semantics (and usefulness). FWIW I think it should control whether the roles `Declaration` *and* `Definition` are included, because I guess it's meant to distinguish between usages that introduce a name vs usages that reference an existing name. For now, we don't support any options and don't parse them out of the LSP. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50896 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51674: [clangd] Fix async index loading (from r341376).
sammccall updated this revision to Diff 164012. sammccall added a comment. Don't load index asynchronously if -run-synchronously is passed. Nothing needs this today, but it's less surprising behavior. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51674 Files: clangd/tool/ClangdMain.cpp Index: clangd/tool/ClangdMain.cpp === --- clangd/tool/ClangdMain.cpp +++ clangd/tool/ClangdMain.cpp @@ -271,14 +271,17 @@ Opts.ResourceDir = ResourceDir; Opts.BuildDynamicSymbolIndex = EnableIndex; std::unique_ptr StaticIdx; + std::future AsyncIndexLoad; // Block exit while loading the index. if (EnableIndex && !YamlSymbolFile.empty()) { // Load the index asynchronously. Meanwhile SwapIndex returns no results. SwapIndex *Placeholder; StaticIdx.reset(Placeholder = new SwapIndex(llvm::make_unique())); -runAsync([Placeholder] { +AsyncIndexLoad = runAsync([Placeholder] { if (auto Idx = loadIndex(YamlSymbolFile)) Placeholder->reset(std::move(Idx)); }); +if (RunSynchronously) + AsyncIndexLoad.wait(); } Opts.StaticIndex = StaticIdx.get(); Opts.AsyncThreadsCount = WorkerThreadsCount; Index: clangd/tool/ClangdMain.cpp === --- clangd/tool/ClangdMain.cpp +++ clangd/tool/ClangdMain.cpp @@ -271,14 +271,17 @@ Opts.ResourceDir = ResourceDir; Opts.BuildDynamicSymbolIndex = EnableIndex; std::unique_ptr StaticIdx; + std::future AsyncIndexLoad; // Block exit while loading the index. if (EnableIndex && !YamlSymbolFile.empty()) { // Load the index asynchronously. Meanwhile SwapIndex returns no results. SwapIndex *Placeholder; StaticIdx.reset(Placeholder = new SwapIndex(llvm::make_unique())); -runAsync([Placeholder] { +AsyncIndexLoad = runAsync([Placeholder] { if (auto Idx = loadIndex(YamlSymbolFile)) Placeholder->reset(std::move(Idx)); }); +if (RunSynchronously) + AsyncIndexLoad.wait(); } Opts.StaticIndex = StaticIdx.get(); Opts.AsyncThreadsCount = WorkerThreadsCount; ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50438: [clangd] Sort GoToDefinition results.
hokein updated this revision to Diff 164013. hokein added a comment. Rebase to master. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50438 Files: clangd/XRefs.cpp unittests/clangd/XRefsTests.cpp Index: unittests/clangd/XRefsTests.cpp === --- unittests/clangd/XRefsTests.cpp +++ unittests/clangd/XRefsTests.cpp @@ -311,6 +311,50 @@ } } +TEST(GoToDefinition, Rank) { + auto T = Annotations(R"cpp( +struct $foo1[[Foo]] { + $foo2[[Foo]](); + $foo3[[Foo]](Foo&&); + $foo4[[Foo]](const char*); +}; + +Foo $f[[f]](); + +void $g[[g]](Foo foo); + +void call() { + const char* $str[[str]] = "123"; + Foo a = $1^str; + Foo b = Foo($2^str); + Foo c = $3^f(); + $4^g($5^f()); + g($6^str); +} + )cpp"); + auto AST = TestTU::withCode(T.code()).build(); + EXPECT_THAT(findDefinitions(AST, T.point("1")), + ElementsAre(RangeIs(T.range("str")), RangeIs(T.range("foo4"; + EXPECT_THAT(findDefinitions(AST, T.point("2")), + ElementsAre(RangeIs(T.range("str"; + EXPECT_THAT(findDefinitions(AST, T.point("3")), + ElementsAre(RangeIs(T.range("f")), RangeIs(T.range("foo3"; + EXPECT_THAT(findDefinitions(AST, T.point("4")), + ElementsAre(RangeIs(T.range("g"; + EXPECT_THAT(findDefinitions(AST, T.point("5")), + ElementsAre(RangeIs(T.range("f")), + RangeIs(T.range("foo3"; + + auto DefinitionAtPoint6 = findDefinitions(AST, T.point("6")); + EXPECT_EQ(3ul, DefinitionAtPoint6.size()); + EXPECT_THAT( + DefinitionAtPoint6, + HasSubsequence(RangeIs(T.range("str")), RangeIs(T.range("foo4"; + EXPECT_THAT( + DefinitionAtPoint6, + HasSubsequence(RangeIs(T.range("str")), RangeIs(T.range("foo3"; +} + TEST(GoToDefinition, RelPathsInCompileCommand) { // The source is in "/clangd-test/src". // We build in "/clangd-test/build". Index: clangd/XRefs.cpp === --- clangd/XRefs.cpp +++ clangd/XRefs.cpp @@ -68,10 +68,20 @@ const MacroInfo *Info; }; +struct DeclInfo { + const Decl *D; + // Indicates the declaration is referenced by an explicit AST node. + bool IsReferencedExplicitly = false; +}; + /// Finds declarations locations that a given source location refers to. class DeclarationAndMacrosFinder : public index::IndexDataConsumer { - std::vector Decls; std::vector MacroInfos; + // The value of the map indicates whether the declaration has been referenced + // explicitly in the code. + // True means the declaration is explicitly referenced at least once; false + // otherwise. + llvm::DenseMap Decls; const SourceLocation &SearchedLocation; const ASTContext &AST; Preprocessor &PP; @@ -81,13 +91,25 @@ ASTContext &AST, Preprocessor &PP) : SearchedLocation(SearchedLocation), AST(AST), PP(PP) {} - std::vector takeDecls() { -// Don't keep the same declaration multiple times. -// This can happen when nodes in the AST are visited twice. -std::sort(Decls.begin(), Decls.end()); -auto Last = std::unique(Decls.begin(), Decls.end()); -Decls.erase(Last, Decls.end()); -return std::move(Decls); + // Get all DeclInfo of the found declarations. + // The results are sorted by "IsReferencedExplicitly" and declaration + // location. + std::vector getFoundDecls() const { +std::vector Result; +for (auto It : Decls) { + Result.emplace_back(); + Result.back().D = It.first; + Result.back().IsReferencedExplicitly = It.second; +} + +// Sort results. Declarations being referenced explicitly come first. +std::sort(Result.begin(), Result.end(), + [](const DeclInfo &L, const DeclInfo &R) { +if (L.IsReferencedExplicitly != R.IsReferencedExplicitly) + return L.IsReferencedExplicitly > R.IsReferencedExplicitly; +return L.D->getBeginLoc() < R.D->getBeginLoc(); + }); +return Result; } std::vector takeMacroInfos() { @@ -111,15 +133,30 @@ SourceLocation Loc, index::IndexDataConsumer::ASTNodeInfo ASTNode) override { if (Loc == SearchedLocation) { + // Check whether the E has an implicit AST node (e.g. ImplicitCastExpr). + auto hasImplicitExpr = [](const Expr *E) { +if (!E || E->child_begin() == E->child_end()) + return false; +// Use the first child is good enough for most cases -- normally the +// expression returned by handleDeclOccurence contains exactly one +// child expression. +const auto *FirstChild = *E->child_begin(); +return llvm::isa(FirstChild) || + llvm::isa(FirstChild) || + llvm::isa(FirstChild) || + llvm::isa(FirstChild); + }; + + bool
[PATCH] D50896: [clangd] Add xrefs LSP boilerplate implementation.
hokein accepted this revision. hokein added a comment. This revision is now accepted and ready to land. Thanks, LGTM, ship it! Comment at: clangd/Protocol.h:881 +struct ReferenceParams : public TextDocumentPositionParams {}; +bool fromJSON(const llvm::json::Value &, ReferenceParams &); Maybe add a comment saying we don't support `includeDeclaration`? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50896 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51680: [analyzer][UninitializedObjectChecker] New flag to ignore union-like constructs
Szelethus created this revision. Szelethus added reviewers: george.karpenkov, NoQ, rnkovacs, xazax.hun. Herald added subscribers: cfe-commits, mikhail.ramalho, a.sidorin, szepet, whisperity. Based on a suggestion from @george.karpenkov. In some cases, structs are used as unions with a help of a tag/kind field. This patch adds a new flag, which will ignore these constructs when enabled. I decided to set this to false by default, out of fear that fields like this might refer to the dynamic type of the object. For more info refer to http://lists.llvm.org/pipermail/cfe-dev/2018-August/058906.html and to the responses to that, especially http://lists.llvm.org/pipermail/cfe-dev/2018-August/059215.html. Repository: rC Clang https://reviews.llvm.org/D51680 Files: lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp test/Analysis/cxx-uninitialized-object-unionlike-constructs.cpp Index: test/Analysis/cxx-uninitialized-object-unionlike-constructs.cpp === --- /dev/null +++ test/Analysis/cxx-uninitialized-object-unionlike-constructs.cpp @@ -0,0 +1,110 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.cplusplus.UninitializedObject \ +// RUN: -analyzer-config alpha.cplusplus.UninitializedObject:Pedantic=true -DPEDANTIC \ +// RUN: -analyzer-config alpha.cplusplus.UninitializedObject:IgnoreUnionlikeConstructs=true \ +// RUN: -std=c++11 -verify %s + +// expected-no-diagnostics + +// Both type and name contains "kind". +struct UnionLikeStruct1 { + enum Kind { +volume, +area + } kind; + + int Volume; + int Area; + + UnionLikeStruct1(Kind kind, int Val) : kind(kind) { +switch (kind) { +case volume: + Volume = Val; + break; +case area: + Area = Val; + break; +} + } +}; + +void fUnionLikeStruct1() { + UnionLikeStruct1 t(UnionLikeStruct1::volume, 10); +} + +// Only name contains "kind". +struct UnionLikeStruct2 { + enum Type { +volume, +area + } kind; + + int Volume; + int Area; + + UnionLikeStruct2(Type kind, int Val) : kind(kind) { +switch (kind) { +case volume: + Volume = Val; + break; +case area: + Area = Val; + break; +} + } +}; + +void fUnionLikeStruct2() { + UnionLikeStruct2 t(UnionLikeStruct2::volume, 10); +} + +// Only type contains "kind". +struct UnionLikeStruct3 { + enum Kind { +volume, +area + } type; + + int Volume; + int Area; + + UnionLikeStruct3(Kind type, int Val) : type(type) { +switch (type) { +case volume: + Volume = Val; + break; +case area: + Area = Val; + break; +} + } +}; + +void fUnionLikeStruct3() { + UnionLikeStruct3 t(UnionLikeStruct3::volume, 10); +} + +// Only type contains "tag". +struct UnionLikeStruct4 { + enum Tag { +volume, +area + } type; + + int Volume; + int Area; + + UnionLikeStruct4(Tag type, int Val) : type(type) { +switch (type) { +case volume: + Volume = Val; + break; +case area: + Area = Val; + break; +} + } +}; + +void fUnionLikeStruct4() { + UnionLikeStruct4 t(UnionLikeStruct4::volume, 10); +} Index: lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp === --- lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp +++ lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp @@ -107,6 +107,11 @@ static bool willObjectBeAnalyzedLater(const CXXConstructorDecl *Ctor, CheckerContext &Context); +/// Checks whether RD is a union-like construct. We'll consider a RecordDecl +/// union-like, if it has a field that has [Kk]ind or [Tt]ag in it's identifier +/// or type name. +static bool isUnionLike(const RecordDecl *RD); + //===--===// // Methods for UninitializedObjectChecker. //===--===// @@ -223,6 +228,11 @@ R->getValueType()->getAs()->getDecl()->getDefinition(); assert(RD && "Referred record has no definition"); + if (Opts.IgnoreUnionlikeConstructs && isUnionLike(RD)) { +IsAnyFieldInitialized = true; +return false; + } + bool ContainsUninitField = false; // Are all of this non-union's fields initialized? @@ -454,6 +464,19 @@ return false; } +static bool isUnionLike(const RecordDecl *RD) { + llvm::Regex ContainsKindOrTag("[kK]ind|[tT]ag"); + + for (const FieldDecl *FD : RD->fields()) { +if (ContainsKindOrTag.match(FD->getType().getAsString())) + return true; +if (ContainsKindOrTag.match(FD->getName())) + return true; + } + + return false; +} + StringRef clang::ento::getVariableName(const FieldDecl *Field) {
[PATCH] D50438: [clangd] Sort GoToDefinition results.
hokein updated this revision to Diff 164014. hokein added a comment. Fix code style. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50438 Files: clangd/XRefs.cpp unittests/clangd/XRefsTests.cpp Index: unittests/clangd/XRefsTests.cpp === --- unittests/clangd/XRefsTests.cpp +++ unittests/clangd/XRefsTests.cpp @@ -311,6 +311,47 @@ } } +TEST(GoToDefinition, Rank) { + auto T = Annotations(R"cpp( +struct $foo1[[Foo]] { + $foo2[[Foo]](); + $foo3[[Foo]](Foo&&); + $foo4[[Foo]](const char*); +}; + +Foo $f[[f]](); + +void $g[[g]](Foo foo); + +void call() { + const char* $str[[str]] = "123"; + Foo a = $1^str; + Foo b = Foo($2^str); + Foo c = $3^f(); + $4^g($5^f()); + g($6^str); +} + )cpp"); + auto AST = TestTU::withCode(T.code()).build(); + EXPECT_THAT(findDefinitions(AST, T.point("1")), + ElementsAre(RangeIs(T.range("str")), RangeIs(T.range("foo4"; + EXPECT_THAT(findDefinitions(AST, T.point("2")), + ElementsAre(RangeIs(T.range("str"; + EXPECT_THAT(findDefinitions(AST, T.point("3")), + ElementsAre(RangeIs(T.range("f")), RangeIs(T.range("foo3"; + EXPECT_THAT(findDefinitions(AST, T.point("4")), + ElementsAre(RangeIs(T.range("g"; + EXPECT_THAT(findDefinitions(AST, T.point("5")), + ElementsAre(RangeIs(T.range("f")), RangeIs(T.range("foo3"; + + auto DefinitionAtPoint6 = findDefinitions(AST, T.point("6")); + EXPECT_EQ(3ul, DefinitionAtPoint6.size()); + EXPECT_THAT(DefinitionAtPoint6, HasSubsequence(RangeIs(T.range("str")), + RangeIs(T.range("foo4"; + EXPECT_THAT(DefinitionAtPoint6, HasSubsequence(RangeIs(T.range("str")), + RangeIs(T.range("foo3"; +} + TEST(GoToDefinition, RelPathsInCompileCommand) { // The source is in "/clangd-test/src". // We build in "/clangd-test/build". Index: clangd/XRefs.cpp === --- clangd/XRefs.cpp +++ clangd/XRefs.cpp @@ -68,10 +68,20 @@ const MacroInfo *Info; }; +struct DeclInfo { + const Decl *D; + // Indicates the declaration is referenced by an explicit AST node. + bool IsReferencedExplicitly = false; +}; + /// Finds declarations locations that a given source location refers to. class DeclarationAndMacrosFinder : public index::IndexDataConsumer { - std::vector Decls; std::vector MacroInfos; + // The value of the map indicates whether the declaration has been referenced + // explicitly in the code. + // True means the declaration is explicitly referenced at least once; false + // otherwise. + llvm::DenseMap Decls; const SourceLocation &SearchedLocation; const ASTContext &AST; Preprocessor &PP; @@ -81,13 +91,25 @@ ASTContext &AST, Preprocessor &PP) : SearchedLocation(SearchedLocation), AST(AST), PP(PP) {} - std::vector takeDecls() { -// Don't keep the same declaration multiple times. -// This can happen when nodes in the AST are visited twice. -std::sort(Decls.begin(), Decls.end()); -auto Last = std::unique(Decls.begin(), Decls.end()); -Decls.erase(Last, Decls.end()); -return std::move(Decls); + // Get all DeclInfo of the found declarations. + // The results are sorted by "IsReferencedExplicitly" and declaration + // location. + std::vector getFoundDecls() const { +std::vector Result; +for (auto It : Decls) { + Result.emplace_back(); + Result.back().D = It.first; + Result.back().IsReferencedExplicitly = It.second; +} + +// Sort results. Declarations being referenced explicitly come first. +std::sort(Result.begin(), Result.end(), + [](const DeclInfo &L, const DeclInfo &R) { +if (L.IsReferencedExplicitly != R.IsReferencedExplicitly) + return L.IsReferencedExplicitly > R.IsReferencedExplicitly; +return L.D->getBeginLoc() < R.D->getBeginLoc(); + }); +return Result; } std::vector takeMacroInfos() { @@ -111,15 +133,30 @@ SourceLocation Loc, index::IndexDataConsumer::ASTNodeInfo ASTNode) override { if (Loc == SearchedLocation) { + // Check whether the E has an implicit AST node (e.g. ImplicitCastExpr). + auto hasImplicitExpr = [](const Expr *E) { +if (!E || E->child_begin() == E->child_end()) + return false; +// Use the first child is good enough for most cases -- normally the +// expression returned by handleDeclOccurence contains exactly one +// child expression. +const auto *FirstChild = *E->child_begin(); +return llvm::isa(FirstChild) || + llvm::isa(FirstChild) || + llvm::isa(FirstChild) || + llvm::
[clang-tools-extra] r341462 - [clangd] Add xrefs LSP boilerplate implementation.
Author: sammccall Date: Wed Sep 5 04:53:07 2018 New Revision: 341462 URL: http://llvm.org/viewvc/llvm-project?rev=341462&view=rev Log: [clangd] Add xrefs LSP boilerplate implementation. Reviewers: ilya-biryukov, ioeric Subscribers: MaskRay, jkorous, arphaman, cfe-commits Differential Revision: https://reviews.llvm.org/D50896 Added: clang-tools-extra/trunk/test/clangd/references.test Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp clang-tools-extra/trunk/clangd/ClangdLSPServer.h clang-tools-extra/trunk/clangd/ClangdServer.cpp clang-tools-extra/trunk/clangd/ClangdServer.h clang-tools-extra/trunk/clangd/Protocol.cpp clang-tools-extra/trunk/clangd/Protocol.h clang-tools-extra/trunk/clangd/ProtocolHandlers.cpp clang-tools-extra/trunk/clangd/ProtocolHandlers.h clang-tools-extra/trunk/clangd/XRefs.cpp clang-tools-extra/trunk/test/clangd/initialize-params-invalid.test clang-tools-extra/trunk/test/clangd/initialize-params.test Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=341462&r1=341461&r2=341462&view=diff == --- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original) +++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Wed Sep 5 04:53:07 2018 @@ -129,6 +129,7 @@ void ClangdLSPServer::onInitialize(Initi {"renameProvider", true}, {"documentSymbolProvider", true}, {"workspaceSymbolProvider", true}, +{"referencesProvider", true}, {"executeCommandProvider", json::Object{ {"commands", {ExecuteCommandParams::CLANGD_APPLY_FIX_COMMAND}}, @@ -449,6 +450,17 @@ void ClangdLSPServer::onChangeConfigurat applyConfiguration(Params.settings); } +void ClangdLSPServer::onReference(ReferenceParams &Params) { + Server.findReferences(Params.textDocument.uri.file(), Params.position, +[](llvm::Expected> Locations) { + if (!Locations) +return replyError( +ErrorCode::InternalError, +llvm::toString(Locations.takeError())); + reply(llvm::json::Array(*Locations)); +}); +} + ClangdLSPServer::ClangdLSPServer(JSONOutput &Out, const clangd::CodeCompleteOptions &CCOpts, llvm::Optional CompileCommandsDir, Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.h?rev=341462&r1=341461&r2=341462&view=diff == --- clang-tools-extra/trunk/clangd/ClangdLSPServer.h (original) +++ clang-tools-extra/trunk/clangd/ClangdLSPServer.h Wed Sep 5 04:53:07 2018 @@ -67,6 +67,7 @@ private: void onCompletion(TextDocumentPositionParams &Params) override; void onSignatureHelp(TextDocumentPositionParams &Params) override; void onGoToDefinition(TextDocumentPositionParams &Params) override; + void onReference(ReferenceParams &Params) override; void onSwitchSourceHeader(TextDocumentIdentifier &Params) override; void onDocumentHighlight(TextDocumentPositionParams &Params) override; void onFileEvent(DidChangeWatchedFilesParams &Params) override; Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=341462&r1=341461&r2=341462&view=diff == --- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original) +++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Wed Sep 5 04:53:07 2018 @@ -560,6 +560,18 @@ void ClangdServer::documentSymbols( Bind(Action, std::move(CB))); } +void ClangdServer::findReferences(PathRef File, Position Pos, + Callback> CB) { + auto Action = [Pos, this](Callback> CB, +llvm::Expected InpAST) { +if (!InpAST) + return CB(InpAST.takeError()); +CB(clangd::findReferences(InpAST->AST, Pos, Index)); + }; + + WorkScheduler.runWithAST("References", File, Bind(Action, std::move(CB))); +} + std::vector> ClangdServer::getUsedBytesPerFile() const { return WorkScheduler.getUsedBytesPerFile(); Modified: clang-tools-extra/trunk/clangd/ClangdServer.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.h?rev=341462&r1=341461&r2=341462&view=diff == --- clang-tools-extra/trunk/clangd/ClangdServer.h (original) +++ clang-tools-extra/trunk/clangd/ClangdServer.h Wed Sep 5 04:53:07 2018 @@ -
[PATCH] D50896: [clangd] Add xrefs LSP boilerplate implementation.
This revision was automatically updated to reflect the committed changes. sammccall marked 3 inline comments as done. Closed by commit rCTE341462: [clangd] Add xrefs LSP boilerplate implementation. (authored by sammccall, committed by ). Changed prior to commit: https://reviews.llvm.org/D50896?vs=164010&id=164015#toc Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50896 Files: clangd/ClangdLSPServer.cpp clangd/ClangdLSPServer.h clangd/ClangdServer.cpp clangd/ClangdServer.h clangd/Protocol.cpp clangd/Protocol.h clangd/ProtocolHandlers.cpp clangd/ProtocolHandlers.h clangd/XRefs.cpp test/clangd/initialize-params-invalid.test test/clangd/initialize-params.test test/clangd/references.test Index: clangd/ProtocolHandlers.cpp === --- clangd/ProtocolHandlers.cpp +++ clangd/ProtocolHandlers.cpp @@ -63,6 +63,7 @@ Register("textDocument/completion", &ProtocolCallbacks::onCompletion); Register("textDocument/signatureHelp", &ProtocolCallbacks::onSignatureHelp); Register("textDocument/definition", &ProtocolCallbacks::onGoToDefinition); + Register("textDocument/references", &ProtocolCallbacks::onReference); Register("textDocument/switchSourceHeader", &ProtocolCallbacks::onSwitchSourceHeader); Register("textDocument/rename", &ProtocolCallbacks::onRename); Index: clangd/Protocol.h === --- clangd/Protocol.h +++ clangd/Protocol.h @@ -878,6 +878,11 @@ llvm::json::Value toJSON(const DocumentHighlight &DH); llvm::raw_ostream &operator<<(llvm::raw_ostream &, const DocumentHighlight &); +struct ReferenceParams : public TextDocumentPositionParams { + // For now, no options like context.includeDeclaration are supported. +}; +bool fromJSON(const llvm::json::Value &, ReferenceParams &); + struct CancelParams { /// The request id to cancel. /// This can be either a number or string, if it is a number simply print it Index: clangd/ProtocolHandlers.h === --- clangd/ProtocolHandlers.h +++ clangd/ProtocolHandlers.h @@ -47,6 +47,7 @@ virtual void onCompletion(TextDocumentPositionParams &Params) = 0; virtual void onSignatureHelp(TextDocumentPositionParams &Params) = 0; virtual void onGoToDefinition(TextDocumentPositionParams &Params) = 0; + virtual void onReference(ReferenceParams &Params) = 0; virtual void onSwitchSourceHeader(TextDocumentIdentifier &Params) = 0; virtual void onFileEvent(DidChangeWatchedFilesParams &Params) = 0; virtual void onCommand(ExecuteCommandParams &Params) = 0; Index: clangd/XRefs.cpp === --- clangd/XRefs.cpp +++ clangd/XRefs.cpp @@ -17,6 +17,7 @@ #include "clang/Index/IndexingAction.h" #include "clang/Index/USRGeneration.h" #include "llvm/Support/Path.h" + namespace clang { namespace clangd { using namespace llvm; Index: clangd/ClangdServer.h === --- clangd/ClangdServer.h +++ clangd/ClangdServer.h @@ -157,6 +157,10 @@ void documentSymbols(StringRef File, Callback> CB); + /// Retrieve locations for symbol references. + void findReferences(PathRef File, Position Pos, + Callback> CB); + /// Run formatting for \p Rng inside \p File with content \p Code. llvm::Expected formatRange(StringRef Code, PathRef File, Range Rng); Index: clangd/Protocol.cpp === --- clangd/Protocol.cpp +++ clangd/Protocol.cpp @@ -616,6 +616,11 @@ O.map("compilationDatabaseChanges", CCPC.compilationDatabaseChanges); } +bool fromJSON(const json::Value &Params, ReferenceParams &R) { + TextDocumentPositionParams &Base = R; + return fromJSON(Params, Base); +} + json::Value toJSON(const CancelParams &CP) { return json::Object{{"id", CP.ID}}; } Index: clangd/ClangdLSPServer.cpp === --- clangd/ClangdLSPServer.cpp +++ clangd/ClangdLSPServer.cpp @@ -129,6 +129,7 @@ {"renameProvider", true}, {"documentSymbolProvider", true}, {"workspaceSymbolProvider", true}, +{"referencesProvider", true}, {"executeCommandProvider", json::Object{ {"commands", {ExecuteCommandParams::CLANGD_APPLY_FIX_COMMAND}}, @@ -449,6 +450,17 @@ applyConfiguration(Params.settings); } +void ClangdLSPServer::onReference(ReferenceParams &Params) { + Server.findReferences(Params.textDocument.uri.file(), Params.position, +[](llvm::Expected> Locations) { + if (!Locations) +return replyError( +ErrorCode::Intern
[clang-tools-extra] r341463 - [clangd] Sort GoToDefinition results.
Author: hokein Date: Wed Sep 5 05:00:15 2018 New Revision: 341463 URL: http://llvm.org/viewvc/llvm-project?rev=341463&view=rev Log: [clangd] Sort GoToDefinition results. Summary: GoToDefinition returns all declaration results (implicit/explicit) that are in the same location, and the results are returned in arbitrary order. Some LSP clients defaultly take the first result as the final result, which might present a bad result (implicit decl) to users. This patch ranks the result based on whether the declarations are referenced explicitly/implicitly. We put explicit declarations first. This also improves the "hover" (which just take the first result) feature in some cases. Reviewers: ilya-biryukov Subscribers: kadircet, ioeric, MaskRay, jkorous, mgrang, arphaman, cfe-commits Differential Revision: https://reviews.llvm.org/D50438 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=341463&r1=341462&r2=341463&view=diff == --- clang-tools-extra/trunk/clangd/XRefs.cpp (original) +++ clang-tools-extra/trunk/clangd/XRefs.cpp Wed Sep 5 05:00:15 2018 @@ -69,10 +69,20 @@ struct MacroDecl { const MacroInfo *Info; }; +struct DeclInfo { + const Decl *D; + // Indicates the declaration is referenced by an explicit AST node. + bool IsReferencedExplicitly = false; +}; + /// Finds declarations locations that a given source location refers to. class DeclarationAndMacrosFinder : public index::IndexDataConsumer { - std::vector Decls; std::vector MacroInfos; + // The value of the map indicates whether the declaration has been referenced + // explicitly in the code. + // True means the declaration is explicitly referenced at least once; false + // otherwise. + llvm::DenseMap Decls; const SourceLocation &SearchedLocation; const ASTContext &AST; Preprocessor &PP; @@ -82,13 +92,25 @@ public: ASTContext &AST, Preprocessor &PP) : SearchedLocation(SearchedLocation), AST(AST), PP(PP) {} - std::vector takeDecls() { -// Don't keep the same declaration multiple times. -// This can happen when nodes in the AST are visited twice. -std::sort(Decls.begin(), Decls.end()); -auto Last = std::unique(Decls.begin(), Decls.end()); -Decls.erase(Last, Decls.end()); -return std::move(Decls); + // Get all DeclInfo of the found declarations. + // The results are sorted by "IsReferencedExplicitly" and declaration + // location. + std::vector getFoundDecls() const { +std::vector Result; +for (auto It : Decls) { + Result.emplace_back(); + Result.back().D = It.first; + Result.back().IsReferencedExplicitly = It.second; +} + +// Sort results. Declarations being referenced explicitly come first. +std::sort(Result.begin(), Result.end(), + [](const DeclInfo &L, const DeclInfo &R) { +if (L.IsReferencedExplicitly != R.IsReferencedExplicitly) + return L.IsReferencedExplicitly > R.IsReferencedExplicitly; +return L.D->getBeginLoc() < R.D->getBeginLoc(); + }); +return Result; } std::vector takeMacroInfos() { @@ -112,15 +134,30 @@ public: SourceLocation Loc, index::IndexDataConsumer::ASTNodeInfo ASTNode) override { if (Loc == SearchedLocation) { + // Check whether the E has an implicit AST node (e.g. ImplicitCastExpr). + auto hasImplicitExpr = [](const Expr *E) { +if (!E || E->child_begin() == E->child_end()) + return false; +// Use the first child is good enough for most cases -- normally the +// expression returned by handleDeclOccurence contains exactly one +// child expression. +const auto *FirstChild = *E->child_begin(); +return llvm::isa(FirstChild) || + llvm::isa(FirstChild) || + llvm::isa(FirstChild) || + llvm::isa(FirstChild); + }; + + bool IsExplicit = !hasImplicitExpr(ASTNode.OrigE); // Find and add definition declarations (for GoToDefinition). // We don't use parameter `D`, as Parameter `D` is the canonical // declaration, which is the first declaration of a redeclarable // declaration, and it could be a forward declaration. if (const auto *Def = getDefinition(D)) { -Decls.push_back(Def); +Decls[Def] |= IsExplicit; } else { // Couldn't find a definition, fall back to use `D`. -Decls.push_back(D); +Decls[D] |= IsExplicit; } } return true; @@ -158,7 +195,7 @@ private: }; struct IdentifiedSymbol { - std::vector Decls; + std::vector Decls; std::vector Macros; }; @@ -172,7 +209,7 @@ Identif
[PATCH] D50438: [clangd] Sort GoToDefinition results.
This revision was automatically updated to reflect the committed changes. Closed by commit rL341463: [clangd] Sort GoToDefinition results. (authored by hokein, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D50438 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 @@ -311,6 +311,47 @@ } } +TEST(GoToDefinition, Rank) { + auto T = Annotations(R"cpp( +struct $foo1[[Foo]] { + $foo2[[Foo]](); + $foo3[[Foo]](Foo&&); + $foo4[[Foo]](const char*); +}; + +Foo $f[[f]](); + +void $g[[g]](Foo foo); + +void call() { + const char* $str[[str]] = "123"; + Foo a = $1^str; + Foo b = Foo($2^str); + Foo c = $3^f(); + $4^g($5^f()); + g($6^str); +} + )cpp"); + auto AST = TestTU::withCode(T.code()).build(); + EXPECT_THAT(findDefinitions(AST, T.point("1")), + ElementsAre(RangeIs(T.range("str")), RangeIs(T.range("foo4"; + EXPECT_THAT(findDefinitions(AST, T.point("2")), + ElementsAre(RangeIs(T.range("str"; + EXPECT_THAT(findDefinitions(AST, T.point("3")), + ElementsAre(RangeIs(T.range("f")), RangeIs(T.range("foo3"; + EXPECT_THAT(findDefinitions(AST, T.point("4")), + ElementsAre(RangeIs(T.range("g"; + EXPECT_THAT(findDefinitions(AST, T.point("5")), + ElementsAre(RangeIs(T.range("f")), RangeIs(T.range("foo3"; + + auto DefinitionAtPoint6 = findDefinitions(AST, T.point("6")); + EXPECT_EQ(3ul, DefinitionAtPoint6.size()); + EXPECT_THAT(DefinitionAtPoint6, HasSubsequence(RangeIs(T.range("str")), + RangeIs(T.range("foo4"; + EXPECT_THAT(DefinitionAtPoint6, HasSubsequence(RangeIs(T.range("str")), + RangeIs(T.range("foo3"; +} + TEST(GoToDefinition, RelPathsInCompileCommand) { // The source is in "/clangd-test/src". // We build in "/clangd-test/build". Index: clang-tools-extra/trunk/clangd/XRefs.cpp === --- clang-tools-extra/trunk/clangd/XRefs.cpp +++ clang-tools-extra/trunk/clangd/XRefs.cpp @@ -69,10 +69,20 @@ const MacroInfo *Info; }; +struct DeclInfo { + const Decl *D; + // Indicates the declaration is referenced by an explicit AST node. + bool IsReferencedExplicitly = false; +}; + /// Finds declarations locations that a given source location refers to. class DeclarationAndMacrosFinder : public index::IndexDataConsumer { - std::vector Decls; std::vector MacroInfos; + // The value of the map indicates whether the declaration has been referenced + // explicitly in the code. + // True means the declaration is explicitly referenced at least once; false + // otherwise. + llvm::DenseMap Decls; const SourceLocation &SearchedLocation; const ASTContext &AST; Preprocessor &PP; @@ -82,13 +92,25 @@ ASTContext &AST, Preprocessor &PP) : SearchedLocation(SearchedLocation), AST(AST), PP(PP) {} - std::vector takeDecls() { -// Don't keep the same declaration multiple times. -// This can happen when nodes in the AST are visited twice. -std::sort(Decls.begin(), Decls.end()); -auto Last = std::unique(Decls.begin(), Decls.end()); -Decls.erase(Last, Decls.end()); -return std::move(Decls); + // Get all DeclInfo of the found declarations. + // The results are sorted by "IsReferencedExplicitly" and declaration + // location. + std::vector getFoundDecls() const { +std::vector Result; +for (auto It : Decls) { + Result.emplace_back(); + Result.back().D = It.first; + Result.back().IsReferencedExplicitly = It.second; +} + +// Sort results. Declarations being referenced explicitly come first. +std::sort(Result.begin(), Result.end(), + [](const DeclInfo &L, const DeclInfo &R) { +if (L.IsReferencedExplicitly != R.IsReferencedExplicitly) + return L.IsReferencedExplicitly > R.IsReferencedExplicitly; +return L.D->getBeginLoc() < R.D->getBeginLoc(); + }); +return Result; } std::vector takeMacroInfos() { @@ -112,15 +134,30 @@ SourceLocation Loc, index::IndexDataConsumer::ASTNodeInfo ASTNode) override { if (Loc == SearchedLocation) { + // Check whether the E has an implicit AST node (e.g. ImplicitCastExpr). + auto hasImplicitExpr = [](const Expr *E) { +if (!E || E->child_begin() == E->child_end()) + return false; +// Use the first child is good enough for most cases -- normally
[PATCH] D50438: [clangd] Sort GoToDefinition results.
This revision was automatically updated to reflect the committed changes. Closed by commit rCTE341463: [clangd] Sort GoToDefinition results. (authored by hokein, committed by ). Changed prior to commit: https://reviews.llvm.org/D50438?vs=164014&id=164017#toc Repository: rL LLVM https://reviews.llvm.org/D50438 Files: clangd/XRefs.cpp unittests/clangd/XRefsTests.cpp Index: unittests/clangd/XRefsTests.cpp === --- unittests/clangd/XRefsTests.cpp +++ unittests/clangd/XRefsTests.cpp @@ -311,6 +311,47 @@ } } +TEST(GoToDefinition, Rank) { + auto T = Annotations(R"cpp( +struct $foo1[[Foo]] { + $foo2[[Foo]](); + $foo3[[Foo]](Foo&&); + $foo4[[Foo]](const char*); +}; + +Foo $f[[f]](); + +void $g[[g]](Foo foo); + +void call() { + const char* $str[[str]] = "123"; + Foo a = $1^str; + Foo b = Foo($2^str); + Foo c = $3^f(); + $4^g($5^f()); + g($6^str); +} + )cpp"); + auto AST = TestTU::withCode(T.code()).build(); + EXPECT_THAT(findDefinitions(AST, T.point("1")), + ElementsAre(RangeIs(T.range("str")), RangeIs(T.range("foo4"; + EXPECT_THAT(findDefinitions(AST, T.point("2")), + ElementsAre(RangeIs(T.range("str"; + EXPECT_THAT(findDefinitions(AST, T.point("3")), + ElementsAre(RangeIs(T.range("f")), RangeIs(T.range("foo3"; + EXPECT_THAT(findDefinitions(AST, T.point("4")), + ElementsAre(RangeIs(T.range("g"; + EXPECT_THAT(findDefinitions(AST, T.point("5")), + ElementsAre(RangeIs(T.range("f")), RangeIs(T.range("foo3"; + + auto DefinitionAtPoint6 = findDefinitions(AST, T.point("6")); + EXPECT_EQ(3ul, DefinitionAtPoint6.size()); + EXPECT_THAT(DefinitionAtPoint6, HasSubsequence(RangeIs(T.range("str")), + RangeIs(T.range("foo4"; + EXPECT_THAT(DefinitionAtPoint6, HasSubsequence(RangeIs(T.range("str")), + RangeIs(T.range("foo3"; +} + TEST(GoToDefinition, RelPathsInCompileCommand) { // The source is in "/clangd-test/src". // We build in "/clangd-test/build". Index: clangd/XRefs.cpp === --- clangd/XRefs.cpp +++ clangd/XRefs.cpp @@ -69,10 +69,20 @@ const MacroInfo *Info; }; +struct DeclInfo { + const Decl *D; + // Indicates the declaration is referenced by an explicit AST node. + bool IsReferencedExplicitly = false; +}; + /// Finds declarations locations that a given source location refers to. class DeclarationAndMacrosFinder : public index::IndexDataConsumer { - std::vector Decls; std::vector MacroInfos; + // The value of the map indicates whether the declaration has been referenced + // explicitly in the code. + // True means the declaration is explicitly referenced at least once; false + // otherwise. + llvm::DenseMap Decls; const SourceLocation &SearchedLocation; const ASTContext &AST; Preprocessor &PP; @@ -82,13 +92,25 @@ ASTContext &AST, Preprocessor &PP) : SearchedLocation(SearchedLocation), AST(AST), PP(PP) {} - std::vector takeDecls() { -// Don't keep the same declaration multiple times. -// This can happen when nodes in the AST are visited twice. -std::sort(Decls.begin(), Decls.end()); -auto Last = std::unique(Decls.begin(), Decls.end()); -Decls.erase(Last, Decls.end()); -return std::move(Decls); + // Get all DeclInfo of the found declarations. + // The results are sorted by "IsReferencedExplicitly" and declaration + // location. + std::vector getFoundDecls() const { +std::vector Result; +for (auto It : Decls) { + Result.emplace_back(); + Result.back().D = It.first; + Result.back().IsReferencedExplicitly = It.second; +} + +// Sort results. Declarations being referenced explicitly come first. +std::sort(Result.begin(), Result.end(), + [](const DeclInfo &L, const DeclInfo &R) { +if (L.IsReferencedExplicitly != R.IsReferencedExplicitly) + return L.IsReferencedExplicitly > R.IsReferencedExplicitly; +return L.D->getBeginLoc() < R.D->getBeginLoc(); + }); +return Result; } std::vector takeMacroInfos() { @@ -112,15 +134,30 @@ SourceLocation Loc, index::IndexDataConsumer::ASTNodeInfo ASTNode) override { if (Loc == SearchedLocation) { + // Check whether the E has an implicit AST node (e.g. ImplicitCastExpr). + auto hasImplicitExpr = [](const Expr *E) { +if (!E || E->child_begin() == E->child_end()) + return false; +// Use the first child is good enough for most cases -- normally the +// expression returned by handleDeclOccurence contains exactly one +// child expression. +const auto *FirstChild = *E-
Re: [clang-tools-extra] r341375 - [clangd] Define a compact binary serialization fomat for symbol slab/index.
On 09/05/2018 12:41 PM, Sam McCall wrote: Thanks. Unclear to me whether it's the enum class or the anonymous namespace that's triggering this (I believe) compiler bug, but r341459 may help... Still doesn't work. In file included from ../tools/clang/include/clang/Frontend/CommandLineSourceLoc.h:19:0, from ../tools/clang/include/clang/Frontend/FrontendOptions.h:13, from ../tools/clang/include/clang/Frontend/CompilerInvocation.h:19, from ../tools/clang/include/clang/Frontend/CompilerInstance.h:16, from ../tools/clang/tools/extra/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp:22: ../include/llvm/Support/CommandLine.h:606:29: error: expected primary-expression before '{' token llvm::cl::OptionEnumValue { FLAGNAME, int(ENUMVAL), DESC } ^ ../tools/clang/tools/extra/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp:67:16: note: in expansion of macro 'clEnumValN' clEnumValN(Format::YAML, "yaml", "human-readable YAML format"), ^ ../include/llvm/Support/CommandLine.h:606:29: error: expected primary-expression before '{' token llvm::cl::OptionEnumValue { FLAGNAME, int(ENUMVAL), DESC } ^ ../tools/clang/tools/extra/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp:68:16: note: in expansion of macro 'clEnumValN' clEnumValN(Format::Binary, "binary", "binary RIFF format")), ^ ../tools/clang/tools/extra/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp:69:27: error: 'Format' is not a class, namespace, or enumeration llvm::cl::init(Format::YAML)); ^ ../tools/clang/tools/extra/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp: In function 'int main(int, const char**)': ../tools/clang/tools/extra/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp:276:23: error: 'clang::clangd::Format' is not a class, namespace, or enumeration case clang::clangd::Format::YAML: ^ ../tools/clang/tools/extra/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp:279:23: error: 'clang::clangd::Format' is not a class, namespace, or enumeration case clang::clangd::Format::Binary: { ^ ../tools/clang/tools/extra/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp:275:10: warning: enumeration value 'YAML' not handled in switch [-Wswitch] switch (clang::clangd::Format) { ^ ../tools/clang/tools/extra/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp:275:10: warning: enumeration value 'Binary' not handled in switch [-Wswitch] Changing the name of the enum from Format to Formats (so the name isn't the same as the variable) makes it compile. I.e: @@ -58,17 +58,17 @@ static llvm::cl::opt MergeOnTheFly( "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); -enum Format { YAML, Binary }; -static llvm::cl::opt +enum Formats { YAML, Binary }; +static llvm::cl::opt Format("format", llvm::cl::desc("Format of the index to be written"), llvm::cl::values( - clEnumValN(Format::YAML, "yaml", "human-readable YAML format"), - clEnumValN(Format::Binary, "binary", "binary RIFF format")), - llvm::cl::init(Format::YAML)); + clEnumValN(Formats::YAML, "yaml", "human-readable YAML format"), + clEnumValN(Formats::Binary, "binary", "binary RIFF format")), + llvm::cl::init(Formats::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 { @@ -271,14 +271,14 @@ int main(int argc, const char **argv) { } // Reduce phase: combine symbols with the same IDs. auto UniqueSymbols = Consumer->mergeResults(); // Output phase: emit result symbols. switch (clang::clangd::Format) { - case clang::clangd::Format::YAML: + case clang::clangd::Formats::YAML: SymbolsToYAML(UniqueSymbols, llvm::outs()); break; - case clang::clangd::Format::Binary: { + case clang::clangd::Formats::Binary: { clang::clangd::IndexFileOut Out; Out.Symbols = &UniqueSymbols; llvm::outs() << Out; } } seems to compile with gcc 5.4.0. I've no idea if this is a gcc bug or if it's a bug in clang to not also complain about it. /Mikael On Wed, Sep 5, 2018 at 11:05 AM Mikael Holmén mailto:mikael.hol...@ericsson.com>> wrote: On 09/05/2018 09:56 AM, Sam McCall wrote: > Sorry! r341451 should fix this, will keep an eye on the buildbots. > Now it compiles with clang 3.6.0 but with gcc 5.4.0 it fails with /
[PATCH] D51481: [clangd] Implement proximity path boosting for Dex
kbobyrev updated this revision to Diff 164023. kbobyrev marked 5 inline comments as done. kbobyrev added a comment. Address another round of comments. https://reviews.llvm.org/D51481 Files: clang-tools-extra/clangd/CMakeLists.txt clang-tools-extra/clangd/FileDistance.h clang-tools-extra/clangd/Quality.cpp clang-tools-extra/clangd/index/SymbolYAML.cpp clang-tools-extra/clangd/index/SymbolYAML.h clang-tools-extra/clangd/index/dex/DexIndex.cpp clang-tools-extra/clangd/index/dex/DexIndex.h clang-tools-extra/clangd/index/dex/Iterator.cpp clang-tools-extra/clangd/index/dex/Token.cpp clang-tools-extra/clangd/index/dex/Token.h clang-tools-extra/clangd/tool/ClangdMain.cpp clang-tools-extra/unittests/clangd/DexIndexTests.cpp clang-tools-extra/unittests/clangd/TestIndex.cpp clang-tools-extra/unittests/clangd/TestIndex.h Index: clang-tools-extra/unittests/clangd/TestIndex.h === --- clang-tools-extra/unittests/clangd/TestIndex.h +++ clang-tools-extra/unittests/clangd/TestIndex.h @@ -39,6 +39,13 @@ const FuzzyFindRequest &Req, bool *Incomplete = nullptr); +// Performs fuzzy matching-based symbol lookup given a query and an index. +// Incomplete is set true if more items than requested can be retrieved, false +// otherwise. Returns filename URIs of matched symbols canonical declarations. +std::vector matchDeclURIs(const SymbolIndex &I, + const FuzzyFindRequest &Req, + bool *Incomplete = nullptr); + // Returns qualified names of symbols with any of IDs in the index. std::vector lookup(const SymbolIndex &I, llvm::ArrayRef IDs); Index: clang-tools-extra/unittests/clangd/TestIndex.cpp === --- clang-tools-extra/unittests/clangd/TestIndex.cpp +++ clang-tools-extra/unittests/clangd/TestIndex.cpp @@ -55,6 +55,18 @@ return Matches; } +std::vector matchDeclURIs(const SymbolIndex &I, + const FuzzyFindRequest &Req, + bool *Incomplete) { + std::vector Matches; + bool IsIncomplete = I.fuzzyFind(Req, [&](const Symbol &Sym) { +Matches.push_back(Sym.CanonicalDeclaration.FileURI); + }); + if (Incomplete) +*Incomplete = IsIncomplete; + return Matches; +} + // Returns qualified names of symbols with any of IDs in the index. std::vector lookup(const SymbolIndex &I, llvm::ArrayRef IDs) { Index: clang-tools-extra/unittests/clangd/DexIndexTests.cpp === --- clang-tools-extra/unittests/clangd/DexIndexTests.cpp +++ clang-tools-extra/unittests/clangd/DexIndexTests.cpp @@ -7,6 +7,8 @@ // //===--===// +#include "FuzzyMatch.h" +#include "TestFS.h" #include "TestIndex.h" #include "index/Index.h" #include "index/Merge.h" @@ -30,6 +32,12 @@ namespace dex { namespace { +std::vector URISchemes = {"unittest"}; + +//===--===// +// Query iterator tests. +//===--===// + std::vector consumeIDs(Iterator &It) { auto IDAndScore = consume(It); std::vector IDs(IDAndScore.size()); @@ -325,15 +333,24 @@ EXPECT_THAT(ElementBoost, 3); } +//===--===// +// Search token tests. +//===--===// + testing::Matcher> -trigramsAre(std::initializer_list Trigrams) { +tokensAre(std::initializer_list Strings, Token::Kind Kind) { std::vector Tokens; - for (const auto &Symbols : Trigrams) { -Tokens.push_back(Token(Token::Kind::Trigram, Symbols)); + for (const auto &TokenData : Strings) { +Tokens.push_back(Token(Kind, TokenData)); } return testing::UnorderedElementsAreArray(Tokens); } +testing::Matcher> +trigramsAre(std::initializer_list Trigrams) { + return tokensAre(Trigrams, Token::Kind::Trigram); +} + TEST(DexIndexTrigrams, IdentifierTrigrams) { EXPECT_THAT(generateIdentifierTrigrams("X86"), trigramsAre({"x86", "x$$", "x8$"})); @@ -408,8 +425,30 @@ "hij", "ijk", "jkl", "klm"})); } +TEST(DexSearchTokens, SymbolPath) { + EXPECT_THAT(generateProximityURIs( + "unittest:///clang-tools-extra/clangd/index/Token.h"), + ElementsAre("unittest:///clang-tools-extra/clangd/index/Token.h", + "unittest:///clang-tools-extra/clangd/index", + "unittest:///clang-tools-extra/clangd", + "unittest:///clang-tools-extra", "unittest:///")); +} + +TEST(D
[PATCH] D51481: [clangd] Implement proximity path boosting for Dex
kbobyrev updated this revision to Diff 164024. kbobyrev added a comment. Keep 2 minor refactorings out of the scope of this patch. https://reviews.llvm.org/D51481 Files: clang-tools-extra/clangd/CMakeLists.txt clang-tools-extra/clangd/FileDistance.h clang-tools-extra/clangd/index/SymbolYAML.cpp clang-tools-extra/clangd/index/SymbolYAML.h clang-tools-extra/clangd/index/dex/DexIndex.cpp clang-tools-extra/clangd/index/dex/DexIndex.h clang-tools-extra/clangd/index/dex/Iterator.cpp clang-tools-extra/clangd/index/dex/Token.cpp clang-tools-extra/clangd/index/dex/Token.h clang-tools-extra/clangd/tool/ClangdMain.cpp clang-tools-extra/unittests/clangd/DexIndexTests.cpp clang-tools-extra/unittests/clangd/TestIndex.cpp clang-tools-extra/unittests/clangd/TestIndex.h Index: clang-tools-extra/unittests/clangd/TestIndex.h === --- clang-tools-extra/unittests/clangd/TestIndex.h +++ clang-tools-extra/unittests/clangd/TestIndex.h @@ -39,6 +39,13 @@ const FuzzyFindRequest &Req, bool *Incomplete = nullptr); +// Performs fuzzy matching-based symbol lookup given a query and an index. +// Incomplete is set true if more items than requested can be retrieved, false +// otherwise. Returns filename URIs of matched symbols canonical declarations. +std::vector matchDeclURIs(const SymbolIndex &I, + const FuzzyFindRequest &Req, + bool *Incomplete = nullptr); + // Returns qualified names of symbols with any of IDs in the index. std::vector lookup(const SymbolIndex &I, llvm::ArrayRef IDs); Index: clang-tools-extra/unittests/clangd/TestIndex.cpp === --- clang-tools-extra/unittests/clangd/TestIndex.cpp +++ clang-tools-extra/unittests/clangd/TestIndex.cpp @@ -55,6 +55,18 @@ return Matches; } +std::vector matchDeclURIs(const SymbolIndex &I, + const FuzzyFindRequest &Req, + bool *Incomplete) { + std::vector Matches; + bool IsIncomplete = I.fuzzyFind(Req, [&](const Symbol &Sym) { +Matches.push_back(Sym.CanonicalDeclaration.FileURI); + }); + if (Incomplete) +*Incomplete = IsIncomplete; + return Matches; +} + // Returns qualified names of symbols with any of IDs in the index. std::vector lookup(const SymbolIndex &I, llvm::ArrayRef IDs) { Index: clang-tools-extra/unittests/clangd/DexIndexTests.cpp === --- clang-tools-extra/unittests/clangd/DexIndexTests.cpp +++ clang-tools-extra/unittests/clangd/DexIndexTests.cpp @@ -7,6 +7,8 @@ // //===--===// +#include "FuzzyMatch.h" +#include "TestFS.h" #include "TestIndex.h" #include "index/Index.h" #include "index/Merge.h" @@ -30,6 +32,12 @@ namespace dex { namespace { +std::vector URISchemes = {"unittest"}; + +//===--===// +// Query iterator tests. +//===--===// + std::vector consumeIDs(Iterator &It) { auto IDAndScore = consume(It); std::vector IDs(IDAndScore.size()); @@ -325,15 +333,24 @@ EXPECT_THAT(ElementBoost, 3); } +//===--===// +// Search token tests. +//===--===// + testing::Matcher> -trigramsAre(std::initializer_list Trigrams) { +tokensAre(std::initializer_list Strings, Token::Kind Kind) { std::vector Tokens; - for (const auto &Symbols : Trigrams) { -Tokens.push_back(Token(Token::Kind::Trigram, Symbols)); + for (const auto &TokenData : Strings) { +Tokens.push_back(Token(Kind, TokenData)); } return testing::UnorderedElementsAreArray(Tokens); } +testing::Matcher> +trigramsAre(std::initializer_list Trigrams) { + return tokensAre(Trigrams, Token::Kind::Trigram); +} + TEST(DexIndexTrigrams, IdentifierTrigrams) { EXPECT_THAT(generateIdentifierTrigrams("X86"), trigramsAre({"x86", "x$$", "x8$"})); @@ -408,8 +425,30 @@ "hij", "ijk", "jkl", "klm"})); } +TEST(DexSearchTokens, SymbolPath) { + EXPECT_THAT(generateProximityURIs( + "unittest:///clang-tools-extra/clangd/index/Token.h"), + ElementsAre("unittest:///clang-tools-extra/clangd/index/Token.h", + "unittest:///clang-tools-extra/clangd/index", + "unittest:///clang-tools-extra/clangd", + "unittest:///clang-tools-extra", "unittest:///")); +} + +TEST(DexSearchTokens, QueryProximityDistances) { + llvm::SmallSt
[clang-tools-extra] r341466 - [clangd] Fix references.test assertions
Author: sammccall Date: Wed Sep 5 06:17:51 2018 New Revision: 341466 URL: http://llvm.org/viewvc/llvm-project?rev=341466&view=rev Log: [clangd] Fix references.test assertions Modified: clang-tools-extra/trunk/test/clangd/references.test Modified: clang-tools-extra/trunk/test/clangd/references.test URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/references.test?rev=341466&r1=341465&r2=341466&view=diff == --- clang-tools-extra/trunk/test/clangd/references.test (original) +++ clang-tools-extra/trunk/test/clangd/references.test Wed Sep 5 06:17:51 2018 @@ -18,7 +18,7 @@ # CHECK-NEXT: "line": 0 # CHECK-NEXT:} # CHECK-NEXT: }, -# CHECK-NEXT: "uri": "test:///main.cpp" +# CHECK-NEXT: "uri": "{{.*}}/main.cpp" # CHECK-NEXT:}, # CHECK-NEXT:{ # CHECK-NEXT: "range": { @@ -31,8 +31,8 @@ # CHECK-NEXT: "line": 0 # CHECK-NEXT:} # CHECK-NEXT: }, -# CHECK-NEXT: "uri": "test:///main.cpp" -# CHECK-NEXT:}, +# CHECK-NEXT: "uri": "{{.*}}/main.cpp" +# CHECK-NEXT:} # CHECK-NEXT: ] --- {"jsonrpc":"2.0","id":3,"method":"shutdown"} ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r341465 - [clangd] make zlib compression optional for binary format
Author: sammccall Date: Wed Sep 5 06:17:47 2018 New Revision: 341465 URL: http://llvm.org/viewvc/llvm-project?rev=341465&view=rev Log: [clangd] make zlib compression optional for binary format Modified: clang-tools-extra/trunk/clangd/index/Serialization.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=341465&r1=341464&r2=341465&view=diff == --- clang-tools-extra/trunk/clangd/index/Serialization.cpp (original) +++ clang-tools-extra/trunk/clangd/index/Serialization.cpp Wed Sep 5 06:17:47 2018 @@ -86,7 +86,7 @@ uint32_t consumeVar(StringRef &Data) { // We store each string once, and refer to them by index. // // The string table's format is: -// - UncompressedSize : uint32 +// - UncompressedSize : uint32 (or 0 for no compression) // - CompressedData : byte[CompressedSize] // // CompressedData is a zlib-compressed byte[UncompressedSize]. @@ -102,6 +102,11 @@ class StringTableOut { DenseMap, unsigned> Index; public: + StringTableOut() { +// Ensure there's at least one string in the table. +// Table size zero is reserved to indicate no compression. +Unique.insert(""); + } // Add a string to the table. Overwrites S if an identical string exists. void intern(StringRef &S) { S = *Unique.insert(S).first; }; // Finalize the table and write it to OS. No more strings may be added. @@ -116,10 +121,15 @@ public: RawTable.append(S); RawTable.push_back(0); } -SmallString<1> Compressed; -cantFail(zlib::compress(RawTable, Compressed)); -write32(RawTable.size(), OS); -OS << Compressed; +if (zlib::isAvailable()) { + SmallString<1> Compressed; + cantFail(zlib::compress(RawTable, Compressed)); + write32(RawTable.size(), OS); + OS << Compressed; +} else { + write32(0, OS); // No compression. + OS << RawTable; +} } // Get the ID of an string, which must be interned. Table must be finalized. unsigned index(StringRef S) const { @@ -138,9 +148,17 @@ Expected readStringTable( if (Data.size() < 4) return makeError("Bad string table: not enough metadata"); size_t UncompressedSize = consume32(Data); - SmallString<1> Uncompressed; - if (Error E = llvm::zlib::uncompress(Data, Uncompressed, UncompressedSize)) -return std::move(E); + + StringRef Uncompressed; + SmallString<1> UncompressedStorage; + if (UncompressedSize == 0) // No compression +Uncompressed = Data; + else { +if (Error E = +llvm::zlib::uncompress(Data, UncompressedStorage, UncompressedSize)) + return std::move(E); +Uncompressed = UncompressedStorage; + } StringTableIn Table; StringSaver Saver(Table.Arena); @@ -285,9 +303,9 @@ Expected readSymbol(StringRef &D // - symb: symbols // The current versioning scheme is simple - non-current versions are rejected. -// This allows arbitrary format changes, which invalidate stored data. -// Later we may want to support some backward compatibility. -constexpr static uint32_t Version = 1; +// If you make a breaking change, bump this version number to invalidate stored +// data. Later we may want to support some backward compatibility. +constexpr static uint32_t Version = 2; Expected readIndexFile(StringRef Data) { auto RIFF = riff::readFile(Data); ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r341467 - [clangd] Fix type/variable name conflict on some compilers
Author: sammccall Date: Wed Sep 5 06:22:11 2018 New Revision: 341467 URL: http://llvm.org/viewvc/llvm-project?rev=341467&view=rev Log: [clangd] Fix type/variable name conflict on some compilers Modified: clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp Modified: clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp?rev=341467&r1=341466&r2=341467&view=diff == --- clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp (original) +++ clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp Wed Sep 5 06:22:11 2018 @@ -60,13 +60,12 @@ static llvm::cl::opt MergeOnTheFly "MapReduce."), llvm::cl::init(true), llvm::cl::Hidden); -enum Format { YAML, Binary }; -static llvm::cl::opt -Format("format", llvm::cl::desc("Format of the index to be written"), - llvm::cl::values( - clEnumValN(Format::YAML, "yaml", "human-readable YAML format"), - clEnumValN(Format::Binary, "binary", "binary RIFF format")), - llvm::cl::init(Format::YAML)); +enum IndexFormat { YAML, Binary }; +static llvm::cl::opt Format( +"format", llvm::cl::desc("Format of the index to be written"), +llvm::cl::values(clEnumValN(YAML, "yaml", "human-readable YAML format"), + clEnumValN(Binary, "binary", "binary RIFF format")), +llvm::cl::init(YAML)); /// Responsible for aggregating symbols from each processed file and producing /// the final results. All methods in this class must be thread-safe, @@ -273,10 +272,10 @@ int main(int argc, const char **argv) { auto UniqueSymbols = Consumer->mergeResults(); // Output phase: emit result symbols. switch (clang::clangd::Format) { - case clang::clangd::Format::YAML: + case clang::clangd::IndexFormat::YAML: SymbolsToYAML(UniqueSymbols, llvm::outs()); break; - case clang::clangd::Format::Binary: { + case clang::clangd::IndexFormat::Binary: { clang::clangd::IndexFileOut Out; Out.Symbols = &UniqueSymbols; llvm::outs() << Out; ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51608: [modules] when deserializing method, ensure it has correct exception spec
elsteveogrande added a comment. Hi @rsmith ! Thanks for taking a look at this. I'd much prefer to fix the underlying problem and not swat at these symptoms, so thanks for the analysis of what's actually going on here. :) Let me take a stab at fixing the real problem in the manner you describe. Repository: rC Clang https://reviews.llvm.org/D51608 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Zurich LLVM Compiler Social, Tech-Talks: Compilers meet Isabelle & The Lean Theorem Prover (Thursday 13th)
Dear LLVM and compiler interested people, next Thursday, 19:00, we will have two exciting tech-talks at the LLVM compiler social! # The Lean Theorem Prover Lean is a new theorem prover using dependent type theory (DTT). Lean 3 gained support for meta programming, employing dependent types not only as logic but also as a programming language. Lean as a programming language is used to implement tactics in Lean itself. In this talk I want to give an overview of Lean, its meta programming framework, its (mathematical) library and current projects. Bio: Johannes Hoelzl is since 2018 a postdoc at the VU Amsterdam working on the Matryoshka project and comaintaining Lean's mathematical library. In 2017, he did a postdoc with Jeremy Avigad (CMU). From 2009 to 2016 he was a PhD student and postdoc in Tobias Nipkow's Isabelle group at TU München. His main topic was Isabelle's analysis and probability theory. He, working with theorem provers since 2005. # Compilers meet Isabelle Abstract: Isabelle is an interactive proof assistant. Isabelle assists its users in writing functional programs and proofs about these and meticulously checks the proofs' correctness. This talk will consist of two parts. The first part will be a hands-on introduction to Isabelle, in which we develop an interpreter for a simple, Turing-complete programming language and verify a simple source-code transformation. The second part will outline what it takes to prove a compiler for a more realistic programming language correct. Speakers: Dmitriy Traytel (1st part) and Andreas Lochbihler (2nd part) Bio: Dmitriy Traytel is a senior researcher in David Basin's information security group at ETH Zürich working on runtime verification and monitoring. He completed his PhD in 2015 in Tobias Nipkow's logic and verification group at TU München—the home of the Isabelle proof assistant. Dmitriy is both a developer and a user of Isabelle. Andreas Lochbihler is a formal methods engineer at Digital Asset. From 2013 to 2018, Andreas was a senior researcher in the Information Security Group at ETH Zürich. His research revolved around the theory and tools to formalise protocols and their proofs such that definitions and proofs can be check mechanically. His framework CryptHOL brings to cryptography the expressiveness and rigour of higher-order logic and coalgebraic methods as implemented in the proof assistant Isabelle/HOL. Before joining ETH, he was a member of Programming Paradigms group at the Karlsruhe Institute of Technology and of the Sofware Systems group at the University of Passau. In his thesis, he built a formal model of Java concurrency which formalises source code, bytecode, a virtual machine, the compiler and the Java memory model in Isabelle/HOL. # Registration https://www.meetup.com/llvm-compiler-and-code-generation-socials-zurich/events/cbffknyxlbmb/ # What A social meetup to discuss compilation and code generation questions with a focus on LLVM, clang, Polly and related projects. Our primary focus is to provide a venue (and drinks & snacks) that enables free discussions between interested people without imposing an agenda/program. This is a great opportunity to informally discuss your own projects, get project ideas or just learn about what people at ETH and around Zurich are doing with LLVM. Related technical presentations held by participants are welcome (please contact us). # Who: - Anybody interested - - ETH students and staff - LLVM developers and enthusiasts external to ETH # When: 13.09.2018, 19:00 # Where: CAB E 72, ETH Zurich # What is LLVM ? LLVM (http://www.llvm.org) is an open source project that provides a collection of modular compiler and toolchain technologies. It is centered around a modern SSA-based compiler around which an entire ecosystem of compiler technology was developed. Most well know is the clang C++ compiler, which is e.g. used to deploy iOS. Beyond this a diverse set of projects is developed under the umbrella of LLVM. These include code generators and assemblers for various interesting architectures, a jit compiler, a debugger, run-time libraries (C++ Standard Library, OpenMP, Opencl library), program sanity checkers, and many more. LLVM has itself grown out of a research project more than 10 years ago and is the base of many exciting research projects today: https://scholar.google.ch/scholar?cites=7792455789532680075&as_sdt=2005&sciodt=0,5&hl=de Best, Tobias ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51411: [OpenCL] Improve diagnostic of argument in address space conversion builtins
Anastasia added a comment. In https://reviews.llvm.org/D51411#1217477, @yaxunl wrote: > Is this a feature requested by users? > > I can understand that this may be useful to pinpoint some conversions which > are potentially harmful to performance. However such conversions can often be > eliminated by optimization, which makes this warning less useful. > > On the other hand, too many warnings is a nuance to users, therefore I am > wondering whether this warning should be off by default. > > Do you have any chance to have any feedback from users about how useful or > intrusive this warning is. Hi Sam, no feedback from the user. If you think this is not that useful we won't commit or make is off by default as you suggested. Let us know what you think makes more sense. https://reviews.llvm.org/D51411 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51675: [Sema] Store MacroInfo in CodeCompletionResult for macro results.
sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land. Comment at: include/clang/Sema/CodeCompleteConsumer.h:847 + /// If the result is RK_Macro, this can store the information about the macro + /// definition. Let's not make this maybe/optional: `If the result is RK_Macro, information about the macro definition.` There's only one relevant constructor, with only 3 callsites (the two you have, and one in clangd), so we can fix them all. One of them statically appears like it could be null sometimes, but dynamically should never be null for macros triggering code complete I think (we may want to add an assert). Comment at: include/clang/Sema/CodeCompleteConsumer.h:875 CodeCompletionResult(const IdentifierInfo *Macro, + const MacroInfo *MI = nullptr, unsigned Priority = CCP_Macro) (so I'd drop the default argument here) Repository: rC Clang https://reviews.llvm.org/D51675 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32905: [Analyzer] Iterator Checker - Part 9: Evaluation of std::find-like calls
baloghadamsoftware added inline comments. Herald added subscribers: Szelethus, mikhail.ramalho. Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:1584-1588 + auto stateFound = state->BindExpr(CE, LCtx, RetVal); + auto stateNotFound = state->BindExpr(CE, LCtx, SecondParam); + + C.addTransition(stateFound); + C.addTransition(stateNotFound); NoQ wrote: > We discussed this in D25660 but i couldn't find what we decided upon. It > seems scary to me that in every situation, we declare that it is possible > that the element is not in the container. It should be allowed, even if not > necessarily efficient, to add the element to the container, and then use > `std::find` to obtain an iterator to it. So i think that this state split, > even if generally approved by your users, may need to stay under an analyzer > checker option. Maybe instead of an option, I should put it into a separate checker that does not emit warnings just simulates these functions, like you did it in rC284960. The file name could be `StdCppLibraryFunctions.cpp`. Common functions and macros should then be moved into a header that both `StdLibraryFunctions.cpp` and `StdCppLibraryFunctions.cpp` includes. https://reviews.llvm.org/D32905 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51411: [OpenCL] Improve diagnostic of argument in address space conversion builtins
yaxunl added a comment. In https://reviews.llvm.org/D51411#1224615, @Anastasia wrote: > In https://reviews.llvm.org/D51411#1217477, @yaxunl wrote: > > > Is this a feature requested by users? > > > > I can understand that this may be useful to pinpoint some conversions which > > are potentially harmful to performance. However such conversions can often > > be eliminated by optimization, which makes this warning less useful. > > > > On the other hand, too many warnings is a nuance to users, therefore I am > > wondering whether this warning should be off by default. > > > > Do you have any chance to have any feedback from users about how useful or > > intrusive this warning is. > > > Hi Sam, no feedback from the user. If you think this is not that useful we > won't commit or make is off by default as you suggested. Let us know what you > think makes more sense. I suggest to make it off by default. Since implicit casting to generic pointer is allowed by spec, we only want to see this warning when we want to debug performance issues. https://reviews.llvm.org/D51411 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51683: Fix arm_neon.h and arm_fp16.h generation for compiling with std=c89
dnsampaio created this revision. dnsampaio added reviewers: pbarrio, SjoerdMeijer. Herald added a reviewer: javed.absar. Herald added subscribers: cfe-commits, chrib, kristof.beyls. The **inline** attribute is not valid for C standard 89. Replace the argument in the generation of header files with **__inline**, as well adding tests for both header files. Repository: rC Clang https://reviews.llvm.org/D51683 Files: test/Headers/arm-fp16-header.c test/Headers/arm-neon-header.c utils/TableGen/NeonEmitter.cpp Index: utils/TableGen/NeonEmitter.cpp === --- utils/TableGen/NeonEmitter.cpp +++ utils/TableGen/NeonEmitter.cpp @@ -2409,7 +2409,7 @@ OS << "#endif\n"; OS << "\n"; - OS << "#define __ai static inline __attribute__((__always_inline__, " + OS << "#define __ai static __inline __attribute__((__always_inline__, " "__nodebug__))\n\n"; SmallVector Defs; @@ -2518,7 +2518,7 @@ OS << "typedef __fp16 float16_t;\n"; - OS << "#define __ai static inline __attribute__((__always_inline__, " + OS << "#define __ai static __inline __attribute__((__always_inline__, " "__nodebug__))\n\n"; SmallVector Defs; Index: test/Headers/arm-neon-header.c === --- test/Headers/arm-neon-header.c +++ test/Headers/arm-neon-header.c @@ -2,4 +2,23 @@ // RUN: %clang_cc1 -triple thumbv7-apple-darwin10 -target-cpu cortex-a8 -fsyntax-only -fno-lax-vector-conversions -ffreestanding %s // RUN: %clang_cc1 -x c++ -triple thumbv7-apple-darwin10 -target-cpu cortex-a8 -fsyntax-only -Wvector-conversions -ffreestanding %s +// RUN: %clang -fsyntax-only -ffreestanding --target=aarch64-arm-none-eabi -march=armv8.2-a+fp16 -std=c89 -xc %s +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-arm-none-eabi -march=armv8.2-a+fp16 -std=c99 -xc %s +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-arm-none-eabi -march=armv8.2-a+fp16 -std=c11 -xc %s + +// RUN: %clang -fsyntax-only -ffreestanding --target=aarch64-armeb-none-eabi -march=armv8.2-a+fp16 -std=c89 -xc %s +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-armeb-none-eabi -march=armv8.2-a+fp16 -std=c99 -xc %s +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-armeb-none-eabi -march=armv8.2-a+fp16 -std=c11 -xc %s + +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-arm-none-eabi -march=armv8.2-a+fp16 -std=c++98 -xc++ %s +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-arm-none-eabi -march=armv8.2-a+fp16 -std=c++11 -xc++ %s +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-arm-none-eabi -march=armv8.2-a+fp16 -std=c++14 -xc++ %s +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-arm-none-eabi -march=armv8.2-a+fp16 -std=c++17 -xc++ %s + +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-armeb-none-eabi -march=armv8.2-a+fp16 -std=c++98 -xc++ %s +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-armeb-none-eabi -march=armv8.2-a+fp16 -std=c++11 -xc++ %s +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-armeb-none-eabi -march=armv8.2-a+fp16 -std=c++14 -xc++ %s +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-armeb-none-eabi -march=armv8.2-a+fp16 -std=c++17 -xc++ %s + #include + Index: test/Headers/arm-fp16-header.c === --- /dev/null +++ test/Headers/arm-fp16-header.c @@ -0,0 +1,19 @@ +// RUN: %clang -fsyntax-only -ffreestanding --target=aarch64-arm-none-eabi -march=armv8.2-a+fp16 -std=c89 -xc %s +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-arm-none-eabi -march=armv8.2-a+fp16 -std=c99 -xc %s +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-arm-none-eabi -march=armv8.2-a+fp16 -std=c11 -xc %s + +// RUN: %clang -fsyntax-only -ffreestanding --target=aarch64-armeb-none-eabi -march=armv8.2-a+fp16 -std=c89 -xc %s +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-armeb-none-eabi -march=armv8.2-a+fp16 -std=c99 -xc %s +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-armeb-none-eabi -march=armv8.2-a+fp16 -std=c11 -xc %s + +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-arm-none-eabi -march=armv8.2-a+fp16 -std=c++98 -xc++ %s +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-arm-none-eabi -march=armv8.2-a+fp16 -std=c++11 -xc++ %s +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-arm-none-eabi -march=armv8.2-a+fp16 -std=c++14 -xc++ %s +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-arm-none-eabi -march=armv8.2-a+fp16 -std=c++17 -xc++
RE: D51354: Fix the -print-multi-directory flag to print the selected multilib.
Understood, The test worked for me because I have both 64 bit and 32 bit libs installed on my default sysroot. Otherwise might fail indeed. Will amend the test with an explicit sysroot, > -Original Message- > From: Tim Shen via Phabricator [mailto:revi...@reviews.llvm.org] > Sent: Wednesday, September 05, 2018 12:12 AM > To: Christian BRUEL ; jroel...@jroelofs.com > Cc: tims...@google.com; srhi...@google.com; cfe-commits@lists.llvm.org > Subject: [PATCH] D51354: Fix the -print-multi-directory flag to print the > selected multilib. > > timshen added a comment. > > > The test fails on my system like so: > > I also observed the same failure. > > Bots also fail: http://lab.llvm.org:8011/builders/clang-x86_64-linux-selfhost- > modules/builds/19509/steps/check-all/logs/FAIL%3A%20Clang%3A%3Aprint- > multi-directory.c > > I'm going to revert this patch. > > > Repository: > rC Clang > > https://reviews.llvm.org/D51354 > > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51481: [clangd] Implement proximity path boosting for Dex
ioeric requested changes to this revision. ioeric added a comment. This revision now requires changes to proceed. This should be the last round! ;) Comment at: clang-tools-extra/clangd/index/dex/DexIndex.cpp:45 +std::vector> +createPathIterators(llvm::ArrayRef ProximityPaths, +llvm::ArrayRef URISchemes, To be clearer, maybe `createFileProximityIterators`? Comment at: clang-tools-extra/clangd/index/dex/DexIndex.cpp:50 + // Deduplicate parent URIs extracted from the FuzzyFindRequest. + llvm::StringSet UniqueURIs; + // Store all root URIs converted from the original FuzzyFindRequest absolute nit: just `llvm::StringSet<>`? `s/UniqueURIs/ParentURIs/`? As this is a set, we don't need to be explicit about uniqueness here. `s/FuzzyFindRequest/ProximityPaths` (in comment). FuzzyFindRequest shouldn't be relevant in this function, and we should avoid mentioning it here. same below. Comment at: clang-tools-extra/clangd/index/dex/DexIndex.cpp:51 + llvm::StringSet UniqueURIs; + // Store all root URIs converted from the original FuzzyFindRequest absolute + // paths within ProximityPath vector. nit: maybe `// Use ProximityPaths as proximity sources`? Comment at: clang-tools-extra/clangd/index/dex/DexIndex.cpp:55 + for (const auto &Path : ProximityPaths) { +const auto PathProximityURIs = generateQueryProximityURIs(Path, URISchemes); +// Use default distance parameters for the first URI in ProximityURIs, which I'd double check `PathProximityURIs` is not empty. Comment at: clang-tools-extra/clangd/index/dex/DexIndex.cpp:62 + } + // Use SymbolRelevanceSignals for symbol quality evaluation: use defaults for + // all parameters except for Proximity Path distance signal. nit: this is `symbol *relevance* evaluation` Comment at: clang-tools-extra/clangd/index/dex/DexIndex.cpp:77 + PathProximitySignals.SymbolURI = URI; + float BoostFactor = 1 + PathProximitySignals.evaluate(); + BoostingIterators.push_back(createBoost(create(It->second), BoostFactor)); `evaluate()` should return an appropriate boosting score (<1 for down-boosting and >1 for up-boosting), so there is no need to plus 1 here. Comment at: clang-tools-extra/clangd/index/dex/DexIndex.cpp:182 + + auto Compare = [](IDAndScore LHS, IDAndScore RHS) { +return LHS.second > RHS.second; `const IDAndScore&`? Comment at: clang-tools-extra/clangd/index/dex/DexIndex.cpp:244 + "URI."); + const auto Scheme = ParsedURI->scheme(); + const auto Authority = ParsedURI->authority(); nit: `Scheme` and `Authority` are both used once. I'd just inline them. Comment at: clang-tools-extra/clangd/index/dex/DexIndex.cpp:246 + const auto Authority = ParsedURI->authority(); + StringRef Path = ParsedURI->body(); + // FIXME(kbobyrev): Currently, this is a heuristic which defines the maximum nit: I'd call this `Body` for clarity. Comment at: clang-tools-extra/clangd/index/dex/DexIndex.h:98 +/// Should be used within the index build process. +std::vector generateProximityURIs(llvm::StringRef Path); + nit: s/Path/URIPath/ Comment at: clang-tools-extra/clangd/index/dex/DexIndex.h:98 +/// Should be used within the index build process. +std::vector generateProximityURIs(llvm::StringRef Path); + ioeric wrote: > nit: s/Path/URIPath/ nit: maybe mention that these functions are exposed for testing only? Comment at: clang-tools-extra/clangd/index/dex/DexIndex.h:104 +std::vector +generateQueryProximityURIs(llvm::StringRef AbsolutePath, + llvm::ArrayRef URISchemes); How about changing this one to just convert `AbsolutePath` to a `URI` of the preferred scheme and having the users call `generateProximityURIs`? This would simply the contract. And it can probably be a library function (e.g. `makePreferredURI(AbsPath, URISchemes)`) in URI.h. Comment at: clang-tools-extra/unittests/clangd/DexIndexTests.cpp:430 + EXPECT_THAT(generateProximityURIs( + "unittest:///clang-tools-extra/clangd/index/Token.h"), + ElementsAre("unittest:///clang-tools-extra/clangd/index/Token.h", Could you add a case where `generateProximityURIs` generates fewer than 5 proximity URIs? Comment at: clang-tools-extra/unittests/clangd/DexIndexTests.cpp:601 + + Symbol RootSymbol; + RootSymbol.Name = Name; Could you add a helper (e.g. lambda) for creating the symbol? The code looks almost the same. Alternatively, this can be: ``` auto Sym1 = symbol("na::X"); Sym1.CanonicalDeclaration.FileURI = "unittest:///..."; au
[PATCH] D51378: [OPENMP] Add support for nested 'declare target' directives
patricklyster updated this revision to Diff 164037. patricklyster added a comment. Previous implementation only supported immediately nested declare targets such as below: #pragma omp declare target #pragma omp declare target int foo(); #pragma omp end declare target int bar(); #pragma omp end declare target Update allows for nesting in this format: #pragma omp declare target int foo(); #pragma omp declare target int bar(); #pragma omp end declare target #pragma omp end declare target https://reviews.llvm.org/D51378 Files: clang/include/clang/Basic/DiagnosticSemaKinds.td clang/include/clang/Sema/Sema.h clang/lib/Parse/ParseOpenMP.cpp clang/lib/Sema/SemaOpenMP.cpp clang/test/OpenMP/Inputs/declare_target_include.h clang/test/OpenMP/declare_target_ast_print.cpp clang/test/OpenMP/declare_target_messages.cpp Index: clang/test/OpenMP/declare_target_messages.cpp === --- clang/test/OpenMP/declare_target_messages.cpp +++ clang/test/OpenMP/declare_target_messages.cpp @@ -59,8 +59,19 @@ } } +#pragma omp declare target + void abc(); +#pragma omp end declare target +void cba(); +#pragma omp end declare target // expected-error {{unexpected OpenMP directive '#pragma omp end declare target'}} -#pragma omp declare target // expected-note {{to match this '#pragma omp declare target'}} +#pragma omp declare target + #pragma omp declare target +void def(); + #pragma omp end declare target + void fed(); + +#pragma omp declare target #pragma omp threadprivate(a) // expected-note {{defined as threadprivate or thread local}} extern int b; int g; @@ -100,9 +111,12 @@ f(); c(); } -#pragma omp declare target // expected-error {{expected '#pragma omp end declare target'}} +#pragma omp declare target void foo1() {} #pragma omp end declare target + +#pragma omp end declare target +#pragma omp end declare target #pragma omp end declare target // expected-error {{unexpected OpenMP directive '#pragma omp end declare target'}} int C::method() { Index: clang/test/OpenMP/declare_target_ast_print.cpp === --- clang/test/OpenMP/declare_target_ast_print.cpp +++ clang/test/OpenMP/declare_target_ast_print.cpp @@ -1,10 +1,10 @@ -// RUN: %clang_cc1 -verify -fopenmp -ast-print %s | FileCheck %s -// RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -emit-pch -o %t %s -// RUN: %clang_cc1 -fopenmp -std=c++11 -include-pch %t -fsyntax-only -verify %s -ast-print | FileCheck %s +// RUN: %clang_cc1 -verify -fopenmp -I %S/Inputs -ast-print %s | FileCheck %s +// RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -I %S/Inputs -emit-pch -o %t %s +// RUN: %clang_cc1 -fopenmp -std=c++11 -include-pch %t -fsyntax-only -I %S/Inputs -verify %s -ast-print | FileCheck %s -// RUN: %clang_cc1 -verify -fopenmp-simd -ast-print %s | FileCheck %s -// RUN: %clang_cc1 -fopenmp-simd -x c++ -std=c++11 -emit-pch -o %t %s -// RUN: %clang_cc1 -fopenmp-simd -std=c++11 -include-pch %t -fsyntax-only -verify %s -ast-print | FileCheck %s +// RUN: %clang_cc1 -verify -fopenmp-simd -I %S/Inputs -ast-print %s | FileCheck %s +// RUN: %clang_cc1 -fopenmp-simd -x c++ -std=c++11 -I %S/Inputs -emit-pch -o %t %s +// RUN: %clang_cc1 -fopenmp-simd -std=c++11 -include-pch %t -fsyntax-only -I %S/Inputs -verify %s -ast-print | FileCheck %s // expected-no-diagnostics #ifndef HEADER @@ -169,6 +169,32 @@ // CHECK: #pragma omp end declare target // CHECK: int b; +#pragma omp declare target + #include "declare_target_include.h" + void xyz(); +#pragma omp end declare target + +// CHECK: #pragma omp declare target +// CHECK: void zyx(); +// CHECK: #pragma omp end declare target +// CHECK: #pragma omp declare target +// CHECK: void xyz(); +// CHECK: #pragma omp end declare target + +#pragma omp declare target + #pragma omp declare target +void abc(); + #pragma omp end declare target + void cba(); +#pragma omp end declare target + +// CHECK: #pragma omp declare target +// CHECK: void abc(); +// CHECK: #pragma omp end declare target +// CHECK: #pragma omp declare target +// CHECK: void cba(); +// CHECK: #pragma omp end declare target + int main (int argc, char **argv) { foo(); foo_c(); Index: clang/test/OpenMP/Inputs/declare_target_include.h === --- /dev/null +++ clang/test/OpenMP/Inputs/declare_target_include.h @@ -0,0 +1,3 @@ +#pragma omp declare target + void zyx(); +#pragma omp end declare target Index: clang/lib/Sema/SemaOpenMP.cpp === --- clang/lib/Sema/SemaOpenMP.cpp +++ clang/lib/Sema/SemaOpenMP.cpp @@ -12870,19 +12870,14 @@ Diag(Loc, diag::err_omp_region_not_file_context); return false; } - if (IsInOpenMPDeclareTargetContext) { -Diag(Loc, diag::err_omp_enclosed_declare_target); -return false; - } - - IsInOpenMPDeclare
[PATCH] D51683: Fix arm_neon.h and arm_fp16.h generation for compiling with std=c89
t.p.northover accepted this revision. t.p.northover added a comment. This revision is now accepted and ready to land. Looks good to me. Repository: rC Clang https://reviews.llvm.org/D51683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r341472 - Enable DWARF accelerator tables by default when tuning for lldb (-glldb => -gpubnames)
Author: labath Date: Wed Sep 5 07:38:44 2018 New Revision: 341472 URL: http://llvm.org/viewvc/llvm-project?rev=341472&view=rev Log: Enable DWARF accelerator tables by default when tuning for lldb (-glldb => -gpubnames) Summary: DWARF v5 accelerator tables provide a considerable performance improvement for lldb and will make the default -glldb behavior same on all targets (right now we emit apple tables on apple targets, but these are not controlled by -gpubnames, only by -glldb). Reviewers: dblaikie Subscribers: probinson, clayborg, JDevlieghere, aprantl, cfe-commits Differential Revision: https://reviews.llvm.org/D51576 Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp cfe/trunk/test/Driver/debug-options.c Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=341472&r1=341471&r2=341472&view=diff == --- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original) +++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Wed Sep 5 07:38:44 2018 @@ -3072,7 +3072,7 @@ static void RenderDebugOptions(const Too const auto *PubnamesArg = Args.getLastArg(options::OPT_ggnu_pubnames, options::OPT_gno_gnu_pubnames, options::OPT_gpubnames, options::OPT_gno_pubnames); - if (SplitDWARFArg || + if (SplitDWARFArg || DebuggerTuning == llvm::DebuggerKind::LLDB || (PubnamesArg && checkDebugInfoOption(PubnamesArg, Args, D, TC))) if (!PubnamesArg || (!PubnamesArg->getOption().matches(options::OPT_gno_gnu_pubnames) && Modified: cfe/trunk/test/Driver/debug-options.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/debug-options.c?rev=341472&r1=341471&r2=341472&view=diff == --- cfe/trunk/test/Driver/debug-options.c (original) +++ cfe/trunk/test/Driver/debug-options.c Wed Sep 5 07:38:44 2018 @@ -165,6 +165,9 @@ // RUN: %clang -### -c -gsplit-dwarf %s 2>&1 | FileCheck -check-prefix=GPUB %s // RUN: %clang -### -c -gsplit-dwarf -gno-pubnames %s 2>&1 | FileCheck -check-prefix=NOPUB %s // +// RUN: %clang -### -c -glldb %s 2>&1 | FileCheck -check-prefix=GPUB %s +// RUN: %clang -### -c -glldb -gno-pubnames %s 2>&1 | FileCheck -check-prefix=NOPUB %s +// // RUN: %clang -### -c -gdwarf-aranges %s 2>&1 | FileCheck -check-prefix=GARANGE %s // // RUN: %clang -### -fdebug-types-section -target x86_64-unknown-linux %s 2>&1 \ ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51576: Enable DWARF accelerator tables by default when tuning for lldb (-glldb => -gpubnames)
This revision was automatically updated to reflect the committed changes. Closed by commit rL341472: Enable DWARF accelerator tables by default when tuning for lldb (-glldb =>… (authored by labath, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D51576 Files: cfe/trunk/lib/Driver/ToolChains/Clang.cpp cfe/trunk/test/Driver/debug-options.c Index: cfe/trunk/test/Driver/debug-options.c === --- cfe/trunk/test/Driver/debug-options.c +++ cfe/trunk/test/Driver/debug-options.c @@ -165,6 +165,9 @@ // RUN: %clang -### -c -gsplit-dwarf %s 2>&1 | FileCheck -check-prefix=GPUB %s // RUN: %clang -### -c -gsplit-dwarf -gno-pubnames %s 2>&1 | FileCheck -check-prefix=NOPUB %s // +// RUN: %clang -### -c -glldb %s 2>&1 | FileCheck -check-prefix=GPUB %s +// RUN: %clang -### -c -glldb -gno-pubnames %s 2>&1 | FileCheck -check-prefix=NOPUB %s +// // RUN: %clang -### -c -gdwarf-aranges %s 2>&1 | FileCheck -check-prefix=GARANGE %s // // RUN: %clang -### -fdebug-types-section -target x86_64-unknown-linux %s 2>&1 \ Index: cfe/trunk/lib/Driver/ToolChains/Clang.cpp === --- cfe/trunk/lib/Driver/ToolChains/Clang.cpp +++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp @@ -3072,7 +3072,7 @@ const auto *PubnamesArg = Args.getLastArg(options::OPT_ggnu_pubnames, options::OPT_gno_gnu_pubnames, options::OPT_gpubnames, options::OPT_gno_pubnames); - if (SplitDWARFArg || + if (SplitDWARFArg || DebuggerTuning == llvm::DebuggerKind::LLDB || (PubnamesArg && checkDebugInfoOption(PubnamesArg, Args, D, TC))) if (!PubnamesArg || (!PubnamesArg->getOption().matches(options::OPT_gno_gnu_pubnames) && Index: cfe/trunk/test/Driver/debug-options.c === --- cfe/trunk/test/Driver/debug-options.c +++ cfe/trunk/test/Driver/debug-options.c @@ -165,6 +165,9 @@ // RUN: %clang -### -c -gsplit-dwarf %s 2>&1 | FileCheck -check-prefix=GPUB %s // RUN: %clang -### -c -gsplit-dwarf -gno-pubnames %s 2>&1 | FileCheck -check-prefix=NOPUB %s // +// RUN: %clang -### -c -glldb %s 2>&1 | FileCheck -check-prefix=GPUB %s +// RUN: %clang -### -c -glldb -gno-pubnames %s 2>&1 | FileCheck -check-prefix=NOPUB %s +// // RUN: %clang -### -c -gdwarf-aranges %s 2>&1 | FileCheck -check-prefix=GARANGE %s // // RUN: %clang -### -fdebug-types-section -target x86_64-unknown-linux %s 2>&1 \ Index: cfe/trunk/lib/Driver/ToolChains/Clang.cpp === --- cfe/trunk/lib/Driver/ToolChains/Clang.cpp +++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp @@ -3072,7 +3072,7 @@ const auto *PubnamesArg = Args.getLastArg(options::OPT_ggnu_pubnames, options::OPT_gno_gnu_pubnames, options::OPT_gpubnames, options::OPT_gno_pubnames); - if (SplitDWARFArg || + if (SplitDWARFArg || DebuggerTuning == llvm::DebuggerKind::LLDB || (PubnamesArg && checkDebugInfoOption(PubnamesArg, Args, D, TC))) if (!PubnamesArg || (!PubnamesArg->getOption().matches(options::OPT_gno_gnu_pubnames) && ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51484: [OpenCL] Add support of cl_intel_device_side_avc_motion_estimation extension
Anastasia added inline comments. Comment at: include/clang/Basic/OpenCLExtensionTypes.def:1 +//===-- OpenCLExtensionTypes.def - Metadata about BuiltinTypes --*- C++ -*-===// +// I am confused about the purpose of this file. Is it supposed to contain intel extension specific types or generally OpenCL types? Comment at: include/clang/Basic/OpenCLExtensionTypes.def:27 + +INTEL_SGAVC_TYPE(mce_payload_t, McePayload) +INTEL_SGAVC_TYPE(ime_payload_t, ImePayload) From the specification of this extension I can't quite see if these types have to be in Clang instead of the header. Can you please elaborate on any example where it wouldn't be possible for this type to be declared in the header using the technique explained in: https://clang.llvm.org/docs/UsersManual.html#opencl-extensions Comment at: lib/Headers/opencl-c.h:16197 +#ifdef cl_intel_device_side_avc_motion_estimation +#pragma OPENCL EXTENSION cl_intel_device_side_avc_motion_estimation : enable + Should we be using: #pragma OPENCL EXTENSION my_ext : begin then the user can get correct diagnostics that the functions can only be used once the extension is enabled. Repository: rC Clang https://reviews.llvm.org/D51484 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51683: Fix arm_neon.h and arm_fp16.h generation for compiling with std=c89
This revision was automatically updated to reflect the committed changes. Closed by commit rL341475: Fix arm_neon.h and arm_fp16.h generation for compiling with std=c89 (authored by dnsampaio, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D51683?vs=164033&id=164043#toc Repository: rL LLVM https://reviews.llvm.org/D51683 Files: cfe/trunk/test/Headers/arm-fp16-header.c cfe/trunk/test/Headers/arm-neon-header.c cfe/trunk/utils/TableGen/NeonEmitter.cpp Index: cfe/trunk/test/Headers/arm-neon-header.c === --- cfe/trunk/test/Headers/arm-neon-header.c +++ cfe/trunk/test/Headers/arm-neon-header.c @@ -2,4 +2,23 @@ // RUN: %clang_cc1 -triple thumbv7-apple-darwin10 -target-cpu cortex-a8 -fsyntax-only -fno-lax-vector-conversions -ffreestanding %s // RUN: %clang_cc1 -x c++ -triple thumbv7-apple-darwin10 -target-cpu cortex-a8 -fsyntax-only -Wvector-conversions -ffreestanding %s +// RUN: %clang -fsyntax-only -ffreestanding --target=aarch64-arm-none-eabi -march=armv8.2-a+fp16 -std=c89 -xc %s +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-arm-none-eabi -march=armv8.2-a+fp16 -std=c99 -xc %s +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-arm-none-eabi -march=armv8.2-a+fp16 -std=c11 -xc %s + +// RUN: %clang -fsyntax-only -ffreestanding --target=aarch64-armeb-none-eabi -march=armv8.2-a+fp16 -std=c89 -xc %s +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-armeb-none-eabi -march=armv8.2-a+fp16 -std=c99 -xc %s +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-armeb-none-eabi -march=armv8.2-a+fp16 -std=c11 -xc %s + +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-arm-none-eabi -march=armv8.2-a+fp16 -std=c++98 -xc++ %s +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-arm-none-eabi -march=armv8.2-a+fp16 -std=c++11 -xc++ %s +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-arm-none-eabi -march=armv8.2-a+fp16 -std=c++14 -xc++ %s +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-arm-none-eabi -march=armv8.2-a+fp16 -std=c++17 -xc++ %s + +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-armeb-none-eabi -march=armv8.2-a+fp16 -std=c++98 -xc++ %s +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-armeb-none-eabi -march=armv8.2-a+fp16 -std=c++11 -xc++ %s +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-armeb-none-eabi -march=armv8.2-a+fp16 -std=c++14 -xc++ %s +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-armeb-none-eabi -march=armv8.2-a+fp16 -std=c++17 -xc++ %s + #include + Index: cfe/trunk/test/Headers/arm-fp16-header.c === --- cfe/trunk/test/Headers/arm-fp16-header.c +++ cfe/trunk/test/Headers/arm-fp16-header.c @@ -0,0 +1,19 @@ +// RUN: %clang -fsyntax-only -ffreestanding --target=aarch64-arm-none-eabi -march=armv8.2-a+fp16 -std=c89 -xc %s +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-arm-none-eabi -march=armv8.2-a+fp16 -std=c99 -xc %s +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-arm-none-eabi -march=armv8.2-a+fp16 -std=c11 -xc %s + +// RUN: %clang -fsyntax-only -ffreestanding --target=aarch64-armeb-none-eabi -march=armv8.2-a+fp16 -std=c89 -xc %s +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-armeb-none-eabi -march=armv8.2-a+fp16 -std=c99 -xc %s +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-armeb-none-eabi -march=armv8.2-a+fp16 -std=c11 -xc %s + +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-arm-none-eabi -march=armv8.2-a+fp16 -std=c++98 -xc++ %s +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-arm-none-eabi -march=armv8.2-a+fp16 -std=c++11 -xc++ %s +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-arm-none-eabi -march=armv8.2-a+fp16 -std=c++14 -xc++ %s +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-arm-none-eabi -march=armv8.2-a+fp16 -std=c++17 -xc++ %s + +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-armeb-none-eabi -march=armv8.2-a+fp16 -std=c++98 -xc++ %s +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-armeb-none-eabi -march=armv8.2-a+fp16 -std=c++11 -xc++ %s +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-armeb-none-eabi -march=armv8.2-a+fp16 -std=c++14 -xc++ %s +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-armeb-none-eabi -march=armv8.2-a+fp16 -std=c++17 -xc++ %s + +#include Index: cfe/trunk/utils/TableGen/NeonEmitter.cpp
[PATCH] D51675: [Sema] Store MacroInfo in CodeCompletionResult for macro results.
ioeric updated this revision to Diff 164044. ioeric added a comment. - update comment about missing MacroInfo. Repository: rC Clang https://reviews.llvm.org/D51675 Files: include/clang/Sema/CodeCompleteConsumer.h lib/Sema/SemaCodeComplete.cpp tools/libclang/CXCursor.cpp Index: tools/libclang/CXCursor.cpp === --- tools/libclang/CXCursor.cpp +++ tools/libclang/CXCursor.cpp @@ -1412,16 +1412,16 @@ } } else if (kind == CXCursor_MacroDefinition) { const MacroDefinitionRecord *definition = getCursorMacroDefinition(cursor); -const IdentifierInfo *MacroInfo = definition->getName(); +const IdentifierInfo *Macro = definition->getName(); ASTUnit *unit = getCursorASTUnit(cursor); -CodeCompletionResult Result(MacroInfo); -CodeCompletionString *String - = Result.CreateCodeCompletionString(unit->getASTContext(), - unit->getPreprocessor(), - CodeCompletionContext::CCC_Other, - unit->getCodeCompletionTUInfo().getAllocator(), - unit->getCodeCompletionTUInfo(), - false); +CodeCompletionResult Result( +Macro, +unit->getPreprocessor().getMacroDefinition(Macro).getMacroInfo()); +CodeCompletionString *String = Result.CreateCodeCompletionString( +unit->getASTContext(), unit->getPreprocessor(), +CodeCompletionContext::CCC_Other, +unit->getCodeCompletionTUInfo().getAllocator(), +unit->getCodeCompletionTUInfo(), false); return String; } return nullptr; Index: lib/Sema/SemaCodeComplete.cpp === --- lib/Sema/SemaCodeComplete.cpp +++ lib/Sema/SemaCodeComplete.cpp @@ -3313,14 +3313,14 @@ M != MEnd; ++M) { auto MD = PP.getMacroDefinition(M->first); if (IncludeUndefined || MD) { - if (MacroInfo *MI = MD.getMacroInfo()) -if (MI->isUsedForHeaderGuard()) - continue; + MacroInfo *MI = MD.getMacroInfo(); + if (MI && MI->isUsedForHeaderGuard()) +continue; - Results.AddResult(Result(M->first, - getMacroUsagePriority(M->first->getName(), - PP.getLangOpts(), - TargetTypeIsPointer))); + Results.AddResult( + Result(M->first, MI, + getMacroUsagePriority(M->first->getName(), PP.getLangOpts(), + TargetTypeIsPointer))); } } Index: include/clang/Sema/CodeCompleteConsumer.h === --- include/clang/Sema/CodeCompleteConsumer.h +++ include/clang/Sema/CodeCompleteConsumer.h @@ -17,6 +17,7 @@ #include "clang-c/Index.h" #include "clang/AST/Type.h" #include "clang/Basic/LLVM.h" +#include "clang/Lex/MacroInfo.h" #include "clang/Sema/CodeCompleteOptions.h" #include "clang/Sema/DeclSpec.h" #include "llvm/ADT/ArrayRef.h" @@ -843,6 +844,11 @@ /// corresponding `using decl::qualified::name;` nearby. const UsingShadowDecl *ShadowDecl = nullptr; + /// If the result is RK_Macro, this can store the information about the macro + /// definition. This should be set in most cases but can be missing when + /// the macro has been undefined. + const MacroInfo *MacroDefInfo = nullptr; + /// Build a result that refers to a declaration. CodeCompletionResult(const NamedDecl *Declaration, unsigned Priority, NestedNameSpecifier *Qualifier = nullptr, @@ -867,11 +873,13 @@ /// Build a result that refers to a macro. CodeCompletionResult(const IdentifierInfo *Macro, + const MacroInfo *MI = nullptr, unsigned Priority = CCP_Macro) : Macro(Macro), Priority(Priority), Kind(RK_Macro), CursorKind(CXCursor_MacroDefinition), Hidden(false), QualifierIsInformative(false), StartsNestedNameSpecifier(false), -AllParametersAreInformative(false), DeclaringEntity(false) {} +AllParametersAreInformative(false), DeclaringEntity(false), +MacroDefInfo(MI) {} /// Build a result that refers to a pattern. CodeCompletionResult(CodeCompletionString *Pattern, Index: tools/libclang/CXCursor.cpp === --- tools/libclang/CXCursor.cpp +++ tools/libclang/CXCursor.cpp @@ -1412,16 +1412,16 @@ } } else if (kind == CXCursor_MacroDefinition) { const MacroDefinitionRecord *definition = getCursorMacroDefinition(cursor); -const IdentifierInfo *MacroInfo = definition->getName(); +const IdentifierInfo *Macro = definition->getName(); ASTUnit *unit = getCursorASTUnit(cursor); -CodeCompletionResult Result(MacroInfo); -CodeCompletionStr
[PATCH] D51675: [Sema] Store MacroInfo in CodeCompletionResult for macro results.
ioeric added inline comments. Comment at: include/clang/Sema/CodeCompleteConsumer.h:847 + /// If the result is RK_Macro, this can store the information about the macro + /// definition. sammccall wrote: > Let's not make this maybe/optional: `If the result is RK_Macro, information > about the macro definition.` > There's only one relevant constructor, with only 3 callsites (the two you > have, and one in clangd), so we can fix them all. > > One of them statically appears like it could be null sometimes, but > dynamically should never be null for macros triggering code complete I think > (we may want to add an assert). As discussed offline, undefined macros do not have MacroInfo, and they can appear in sema results. Added this to comment. Repository: rC Clang https://reviews.llvm.org/D51675 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r341475 - Fix arm_neon.h and arm_fp16.h generation for compiling with std=c89
Author: dnsampaio Date: Wed Sep 5 07:56:21 2018 New Revision: 341475 URL: http://llvm.org/viewvc/llvm-project?rev=341475&view=rev Log: Fix arm_neon.h and arm_fp16.h generation for compiling with std=c89 Summary: The inline attribute is not valid for C standard 89. Replace the argument in the generation of header files with __inline, as well adding tests for both header files. Reviewers: pbarrio, SjoerdMeijer, javed.absar, t.p.northover Subscribers: t.p.northover, kristof.beyls, chrib, cfe-commits Differential Revision: https://reviews.llvm.org/D51683 test/Headers/arm-fp16-header.c test/Headers/arm-neon-header.c utils/TableGen/NeonEmitter.cpp Added: cfe/trunk/test/Headers/arm-fp16-header.c Modified: cfe/trunk/test/Headers/arm-neon-header.c cfe/trunk/utils/TableGen/NeonEmitter.cpp Added: cfe/trunk/test/Headers/arm-fp16-header.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Headers/arm-fp16-header.c?rev=341475&view=auto == --- cfe/trunk/test/Headers/arm-fp16-header.c (added) +++ cfe/trunk/test/Headers/arm-fp16-header.c Wed Sep 5 07:56:21 2018 @@ -0,0 +1,19 @@ +// RUN: %clang -fsyntax-only -ffreestanding --target=aarch64-arm-none-eabi -march=armv8.2-a+fp16 -std=c89 -xc %s +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-arm-none-eabi -march=armv8.2-a+fp16 -std=c99 -xc %s +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-arm-none-eabi -march=armv8.2-a+fp16 -std=c11 -xc %s + +// RUN: %clang -fsyntax-only -ffreestanding --target=aarch64-armeb-none-eabi -march=armv8.2-a+fp16 -std=c89 -xc %s +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-armeb-none-eabi -march=armv8.2-a+fp16 -std=c99 -xc %s +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-armeb-none-eabi -march=armv8.2-a+fp16 -std=c11 -xc %s + +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-arm-none-eabi -march=armv8.2-a+fp16 -std=c++98 -xc++ %s +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-arm-none-eabi -march=armv8.2-a+fp16 -std=c++11 -xc++ %s +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-arm-none-eabi -march=armv8.2-a+fp16 -std=c++14 -xc++ %s +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-arm-none-eabi -march=armv8.2-a+fp16 -std=c++17 -xc++ %s + +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-armeb-none-eabi -march=armv8.2-a+fp16 -std=c++98 -xc++ %s +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-armeb-none-eabi -march=armv8.2-a+fp16 -std=c++11 -xc++ %s +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-armeb-none-eabi -march=armv8.2-a+fp16 -std=c++14 -xc++ %s +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-armeb-none-eabi -march=armv8.2-a+fp16 -std=c++17 -xc++ %s + +#include Modified: cfe/trunk/test/Headers/arm-neon-header.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Headers/arm-neon-header.c?rev=341475&r1=341474&r2=341475&view=diff == --- cfe/trunk/test/Headers/arm-neon-header.c (original) +++ cfe/trunk/test/Headers/arm-neon-header.c Wed Sep 5 07:56:21 2018 @@ -2,4 +2,23 @@ // RUN: %clang_cc1 -triple thumbv7-apple-darwin10 -target-cpu cortex-a8 -fsyntax-only -fno-lax-vector-conversions -ffreestanding %s // RUN: %clang_cc1 -x c++ -triple thumbv7-apple-darwin10 -target-cpu cortex-a8 -fsyntax-only -Wvector-conversions -ffreestanding %s +// RUN: %clang -fsyntax-only -ffreestanding --target=aarch64-arm-none-eabi -march=armv8.2-a+fp16 -std=c89 -xc %s +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-arm-none-eabi -march=armv8.2-a+fp16 -std=c99 -xc %s +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-arm-none-eabi -march=armv8.2-a+fp16 -std=c11 -xc %s + +// RUN: %clang -fsyntax-only -ffreestanding --target=aarch64-armeb-none-eabi -march=armv8.2-a+fp16 -std=c89 -xc %s +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-armeb-none-eabi -march=armv8.2-a+fp16 -std=c99 -xc %s +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-armeb-none-eabi -march=armv8.2-a+fp16 -std=c11 -xc %s + +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-arm-none-eabi -march=armv8.2-a+fp16 -std=c++98 -xc++ %s +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-arm-none-eabi -march=armv8.2-a+fp16 -std=c++11 -xc++ %s +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-arm-none-eabi -march=armv8.2-a+fp16 -std=c++14 -xc++ %s +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-arm-none-eabi -march=armv8.2-a+fp16 -std=c++17 -xc++ %s + +// RUN: %cl
r341476 - [Sema] Store MacroInfo in CodeCompletionResult for macro results.
Author: ioeric Date: Wed Sep 5 07:59:17 2018 New Revision: 341476 URL: http://llvm.org/viewvc/llvm-project?rev=341476&view=rev Log: [Sema] Store MacroInfo in CodeCompletionResult for macro results. Summary: This provides information about the macro definition. For example, it can be used to compute macro USRs. Reviewers: sammccall Subscribers: cfe-commits Differential Revision: https://reviews.llvm.org/D51675 Modified: cfe/trunk/include/clang/Sema/CodeCompleteConsumer.h cfe/trunk/lib/Sema/SemaCodeComplete.cpp cfe/trunk/tools/libclang/CXCursor.cpp Modified: cfe/trunk/include/clang/Sema/CodeCompleteConsumer.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/CodeCompleteConsumer.h?rev=341476&r1=341475&r2=341476&view=diff == --- cfe/trunk/include/clang/Sema/CodeCompleteConsumer.h (original) +++ cfe/trunk/include/clang/Sema/CodeCompleteConsumer.h Wed Sep 5 07:59:17 2018 @@ -17,6 +17,7 @@ #include "clang-c/Index.h" #include "clang/AST/Type.h" #include "clang/Basic/LLVM.h" +#include "clang/Lex/MacroInfo.h" #include "clang/Sema/CodeCompleteOptions.h" #include "clang/Sema/DeclSpec.h" #include "llvm/ADT/ArrayRef.h" @@ -843,6 +844,11 @@ public: /// corresponding `using decl::qualified::name;` nearby. const UsingShadowDecl *ShadowDecl = nullptr; + /// If the result is RK_Macro, this can store the information about the macro + /// definition. This should be set in most cases but can be missing when + /// the macro has been undefined. + const MacroInfo *MacroDefInfo = nullptr; + /// Build a result that refers to a declaration. CodeCompletionResult(const NamedDecl *Declaration, unsigned Priority, NestedNameSpecifier *Qualifier = nullptr, @@ -867,11 +873,13 @@ public: /// Build a result that refers to a macro. CodeCompletionResult(const IdentifierInfo *Macro, + const MacroInfo *MI = nullptr, unsigned Priority = CCP_Macro) : Macro(Macro), Priority(Priority), Kind(RK_Macro), CursorKind(CXCursor_MacroDefinition), Hidden(false), QualifierIsInformative(false), StartsNestedNameSpecifier(false), -AllParametersAreInformative(false), DeclaringEntity(false) {} +AllParametersAreInformative(false), DeclaringEntity(false), +MacroDefInfo(MI) {} /// Build a result that refers to a pattern. CodeCompletionResult(CodeCompletionString *Pattern, Modified: cfe/trunk/lib/Sema/SemaCodeComplete.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCodeComplete.cpp?rev=341476&r1=341475&r2=341476&view=diff == --- cfe/trunk/lib/Sema/SemaCodeComplete.cpp (original) +++ cfe/trunk/lib/Sema/SemaCodeComplete.cpp Wed Sep 5 07:59:17 2018 @@ -3313,14 +3313,14 @@ static void AddMacroResults(Preprocessor M != MEnd; ++M) { auto MD = PP.getMacroDefinition(M->first); if (IncludeUndefined || MD) { - if (MacroInfo *MI = MD.getMacroInfo()) -if (MI->isUsedForHeaderGuard()) - continue; + MacroInfo *MI = MD.getMacroInfo(); + if (MI && MI->isUsedForHeaderGuard()) +continue; - Results.AddResult(Result(M->first, - getMacroUsagePriority(M->first->getName(), - PP.getLangOpts(), - TargetTypeIsPointer))); + Results.AddResult( + Result(M->first, MI, + getMacroUsagePriority(M->first->getName(), PP.getLangOpts(), + TargetTypeIsPointer))); } } Modified: cfe/trunk/tools/libclang/CXCursor.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/libclang/CXCursor.cpp?rev=341476&r1=341475&r2=341476&view=diff == --- cfe/trunk/tools/libclang/CXCursor.cpp (original) +++ cfe/trunk/tools/libclang/CXCursor.cpp Wed Sep 5 07:59:17 2018 @@ -1412,16 +1412,16 @@ CXCompletionString clang_getCursorComple } } else if (kind == CXCursor_MacroDefinition) { const MacroDefinitionRecord *definition = getCursorMacroDefinition(cursor); -const IdentifierInfo *MacroInfo = definition->getName(); +const IdentifierInfo *Macro = definition->getName(); ASTUnit *unit = getCursorASTUnit(cursor); -CodeCompletionResult Result(MacroInfo); -CodeCompletionString *String - = Result.CreateCodeCompletionString(unit->getASTContext(), - unit->getPreprocessor(), - CodeCompletionContext::CCC_Other, - unit->getCodeCompletionTUInfo().getAllocator(), - unit->getCodeCompletionTUInfo(), - false); +Co
[PATCH] D51675: [Sema] Store MacroInfo in CodeCompletionResult for macro results.
This revision was automatically updated to reflect the committed changes. Closed by commit rL341476: [Sema] Store MacroInfo in CodeCompletionResult for macro results. (authored by ioeric, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D51675 Files: cfe/trunk/include/clang/Sema/CodeCompleteConsumer.h cfe/trunk/lib/Sema/SemaCodeComplete.cpp cfe/trunk/tools/libclang/CXCursor.cpp Index: cfe/trunk/include/clang/Sema/CodeCompleteConsumer.h === --- cfe/trunk/include/clang/Sema/CodeCompleteConsumer.h +++ cfe/trunk/include/clang/Sema/CodeCompleteConsumer.h @@ -17,6 +17,7 @@ #include "clang-c/Index.h" #include "clang/AST/Type.h" #include "clang/Basic/LLVM.h" +#include "clang/Lex/MacroInfo.h" #include "clang/Sema/CodeCompleteOptions.h" #include "clang/Sema/DeclSpec.h" #include "llvm/ADT/ArrayRef.h" @@ -843,6 +844,11 @@ /// corresponding `using decl::qualified::name;` nearby. const UsingShadowDecl *ShadowDecl = nullptr; + /// If the result is RK_Macro, this can store the information about the macro + /// definition. This should be set in most cases but can be missing when + /// the macro has been undefined. + const MacroInfo *MacroDefInfo = nullptr; + /// Build a result that refers to a declaration. CodeCompletionResult(const NamedDecl *Declaration, unsigned Priority, NestedNameSpecifier *Qualifier = nullptr, @@ -867,11 +873,13 @@ /// Build a result that refers to a macro. CodeCompletionResult(const IdentifierInfo *Macro, + const MacroInfo *MI = nullptr, unsigned Priority = CCP_Macro) : Macro(Macro), Priority(Priority), Kind(RK_Macro), CursorKind(CXCursor_MacroDefinition), Hidden(false), QualifierIsInformative(false), StartsNestedNameSpecifier(false), -AllParametersAreInformative(false), DeclaringEntity(false) {} +AllParametersAreInformative(false), DeclaringEntity(false), +MacroDefInfo(MI) {} /// Build a result that refers to a pattern. CodeCompletionResult(CodeCompletionString *Pattern, Index: cfe/trunk/lib/Sema/SemaCodeComplete.cpp === --- cfe/trunk/lib/Sema/SemaCodeComplete.cpp +++ cfe/trunk/lib/Sema/SemaCodeComplete.cpp @@ -3313,14 +3313,14 @@ M != MEnd; ++M) { auto MD = PP.getMacroDefinition(M->first); if (IncludeUndefined || MD) { - if (MacroInfo *MI = MD.getMacroInfo()) -if (MI->isUsedForHeaderGuard()) - continue; + MacroInfo *MI = MD.getMacroInfo(); + if (MI && MI->isUsedForHeaderGuard()) +continue; - Results.AddResult(Result(M->first, - getMacroUsagePriority(M->first->getName(), - PP.getLangOpts(), - TargetTypeIsPointer))); + Results.AddResult( + Result(M->first, MI, + getMacroUsagePriority(M->first->getName(), PP.getLangOpts(), + TargetTypeIsPointer))); } } Index: cfe/trunk/tools/libclang/CXCursor.cpp === --- cfe/trunk/tools/libclang/CXCursor.cpp +++ cfe/trunk/tools/libclang/CXCursor.cpp @@ -1412,16 +1412,16 @@ } } else if (kind == CXCursor_MacroDefinition) { const MacroDefinitionRecord *definition = getCursorMacroDefinition(cursor); -const IdentifierInfo *MacroInfo = definition->getName(); +const IdentifierInfo *Macro = definition->getName(); ASTUnit *unit = getCursorASTUnit(cursor); -CodeCompletionResult Result(MacroInfo); -CodeCompletionString *String - = Result.CreateCodeCompletionString(unit->getASTContext(), - unit->getPreprocessor(), - CodeCompletionContext::CCC_Other, - unit->getCodeCompletionTUInfo().getAllocator(), - unit->getCodeCompletionTUInfo(), - false); +CodeCompletionResult Result( +Macro, +unit->getPreprocessor().getMacroDefinition(Macro).getMacroInfo()); +CodeCompletionString *String = Result.CreateCodeCompletionString( +unit->getASTContext(), unit->getPreprocessor(), +CodeCompletionContext::CCC_Other, +unit->getCodeCompletionTUInfo().getAllocator(), +unit->getCodeCompletionTUInfo(), false); return String; } return nullptr; Index: cfe/trunk/include/clang/Sema/CodeCompleteConsumer.h === --- cfe/trunk/include/clang/Sema/CodeCompleteConsumer.h +++ cfe/trunk/include/clang/Sema/CodeCompleteConsumer.h @@ -17,6 +17,7 @@ #include "clang-c/Index.h" #include "clang/AST/Type.
[PATCH] D51544: [OpenCL] Split opencl-c.h header
Anastasia added a comment. It seems generally good to partition this big header but I am trying to understand what problem is it trying to solve now? The unsupported declarations are guarded out by `#if defined(ext_name)` and therefore won't be parsed and put into PCH is extensions are not supported by some target. So I guess it can save the space when installing the headers because we don't need to copy all of them into the installation? Although this is not implemented currently. Repository: rC Clang https://reviews.llvm.org/D51544 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51686: [OpenMP] Improve search for libomptarget-nvptx
Hahnfeld created this revision. Hahnfeld added reviewers: gtbercea, ABataev. Herald added subscribers: cfe-commits, guansong. When looking for the bclib Clang considered the default library path first while it preferred directories in LIBRARY_PATH when constructing the invocation of nvlink. The latter actually makes more sense because during development it allows using a non-default runtime library. So change the search for the bclib to start looking in directories given by LIBRARY_PATH. Additionally add a new option --libomptarget-nvptx-path= which will be searched first. This will be handy for testing purposes. Repository: rC Clang https://reviews.llvm.org/D51686 Files: include/clang/Driver/Options.td lib/Driver/ToolChains/Cuda.cpp test/Driver/openmp-offload-gpu.c Index: test/Driver/openmp-offload-gpu.c === --- test/Driver/openmp-offload-gpu.c +++ test/Driver/openmp-offload-gpu.c @@ -30,6 +30,22 @@ /// ### +/// Check that -lomptarget-nvptx is passed to nvlink. +// RUN: %clang -### -no-canonical-prefixes -fopenmp=libomp \ +// RUN: -fopenmp-targets=nvptx64-nvidia-cuda %s 2>&1 \ +// RUN: | FileCheck -check-prefix=CHK-NVLINK %s +/// Check that the value of --libomptarget-nvptx-path is forwarded to nvlink. +// RUN: %clang -### -no-canonical-prefixes -fopenmp=libomp \ +// RUN: --libomptarget-nvptx-path=/path/to/libomptarget/ \ +// RUN: -fopenmp-targets=nvptx64-nvidia-cuda %s 2>&1 \ +// RUN: | FileCheck -check-prefixes=CHK-NVLINK,CHK-LIBOMPTARGET-NVPTX-PATH %s + +// CHK-NVLINK: nvlink +// CHK-LIBOMPTARGET-NVPTX-PATH-SAME: "-L/path/to/libomptarget/" +// CHK-NVLINK-SAME: "-lomptarget-nvptx" + +/// ### + /// Check cubin file generation and usage by nvlink // RUN: %clang -### -no-canonical-prefixes -target powerpc64le-unknown-linux-gnu -fopenmp=libomp \ // RUN: -fopenmp-targets=nvptx64-nvidia-cuda -save-temps %s 2>&1 \ @@ -151,6 +167,11 @@ // RUN: -Xopenmp-target -march=sm_20 --cuda-path=%S/Inputs/CUDA_80/usr/local/cuda \ // RUN: -fopenmp-relocatable-target -save-temps -no-canonical-prefixes %s 2>&1 \ // RUN: | FileCheck -check-prefix=CHK-BCLIB %s +/// The user can override default detection using --libomptarget-nvptx-path=. +// RUN: %clang -### -fopenmp=libomp -fopenmp-targets=nvptx64-nvidia-cuda --libomptarget-nvptx-path=%S/Inputs/libomptarget \ +// RUN: -Xopenmp-target -march=sm_20 --cuda-path=%S/Inputs/CUDA_80/usr/local/cuda \ +// RUN: -fopenmp-relocatable-target -save-temps -no-canonical-prefixes %s 2>&1 \ +// RUN: | FileCheck -check-prefix=CHK-BCLIB %s // CHK-BCLIB: clang{{.*}}-triple{{.*}}nvptx64-nvidia-cuda{{.*}}-mlink-builtin-bitcode{{.*}}libomptarget-nvptx-sm_20.bc // CHK-BCLIB-NOT: {{error:|warning:}} Index: lib/Driver/ToolChains/Cuda.cpp === --- lib/Driver/ToolChains/Cuda.cpp +++ lib/Driver/ToolChains/Cuda.cpp @@ -509,6 +509,11 @@ CmdArgs.push_back("-arch"); CmdArgs.push_back(Args.MakeArgString(GPUArch)); + // Assume that the directory specified with --libomptarget_nvptx_path + // contains the static library libomptarget-nvptx.a. + if (Arg *A = Args.getLastArg(options::OPT_libomptarget_nvptx_path_EQ)) +CmdArgs.push_back(Args.MakeArgString(Twine("-L") + A->getValue())); + // Add paths specified in LIBRARY_PATH environment variable as -L options. addDirectoryList(Args, CmdArgs, "-L", "LIBRARY_PATH"); @@ -642,12 +647,9 @@ if (DeviceOffloadingKind == Action::OFK_OpenMP) { SmallVector LibraryPaths; -// Add path to lib and/or lib64 folders. -SmallString<256> DefaultLibPath = - llvm::sys::path::parent_path(getDriver().Dir); -llvm::sys::path::append(DefaultLibPath, -Twine("lib") + CLANG_LIBDIR_SUFFIX); -LibraryPaths.emplace_back(DefaultLibPath.c_str()); + +if (Arg *A = DriverArgs.getLastArg(options::OPT_libomptarget_nvptx_path_EQ)) + LibraryPaths.push_back(A->getValue()); // Add user defined library paths from LIBRARY_PATH. llvm::Optional LibPath = @@ -660,6 +662,12 @@ LibraryPaths.emplace_back(Path.trim()); } +// Add path to lib / lib64 folder. +SmallString<256> DefaultLibPath = +llvm::sys::path::parent_path(getDriver().Dir); +llvm::sys::path::append(DefaultLibPath, Twine("lib") + CLANG_LIBDIR_SUFFIX); +LibraryPaths.emplace_back(DefaultLibPath.c_str()); + std::string LibOmpTargetName = "libomptarget-nvptx-" + GpuArch.str() + ".bc"; bool FoundBCLibrary = false; Index: include/clang/Driver/Options.td === --- include/clang/Driver/Options.td +++ include/clang/Driver/Options.td @@ -596,6 +596,8 @@ HelpText<"HIP device library">; def fhip_dump_offl
[PATCH] D51683: Fix arm_neon.h and arm_fp16.h generation for compiling with std=c89
joerg added inline comments. Comment at: cfe/trunk/utils/TableGen/NeonEmitter.cpp:2412 - OS << "#define __ai static inline __attribute__((__always_inline__, " + OS << "#define __ai static __inline __attribute__((__always_inline__, " "__nodebug__))\n\n"; If you want to change it, at least change it properly to use __inline__. Comment at: cfe/trunk/utils/TableGen/NeonEmitter.cpp:2521 - OS << "#define __ai static inline __attribute__((__always_inline__, " + OS << "#define __ai static __inline __attribute__((__always_inline__, " "__nodebug__))\n\n"; Same. Repository: rL LLVM https://reviews.llvm.org/D51683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51680: [analyzer][UninitializedObjectChecker] New flag to ignore union-like constructs
Szelethus updated this revision to Diff 164050. Szelethus added a comment. Added doc to `www/analyzer/alpha_checks.html`. https://reviews.llvm.org/D51680 Files: lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp test/Analysis/cxx-uninitialized-object-unionlike-constructs.cpp www/analyzer/alpha_checks.html Index: www/analyzer/alpha_checks.html === --- www/analyzer/alpha_checks.html +++ www/analyzer/alpha_checks.html @@ -356,6 +356,11 @@ whether the object itself is initialized. Defaults to false. -analyzer-config alpha.cplusplus.UninitializedObject:CheckPointeeInitialization=true. + "IgnoreUnionlikeConstructs" (boolean). If set to false, the checker will + not analyze structures that have a field a name or type name that + contains [Tt]ag or [Kk]ind. Defaults to false. + -analyzer-config alpha.cplusplus.UninitializedObject:IgnoreUnionlikeConstructs=true. + Index: test/Analysis/cxx-uninitialized-object-unionlike-constructs.cpp === --- /dev/null +++ test/Analysis/cxx-uninitialized-object-unionlike-constructs.cpp @@ -0,0 +1,110 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.cplusplus.UninitializedObject \ +// RUN: -analyzer-config alpha.cplusplus.UninitializedObject:Pedantic=true -DPEDANTIC \ +// RUN: -analyzer-config alpha.cplusplus.UninitializedObject:IgnoreUnionlikeConstructs=true \ +// RUN: -std=c++11 -verify %s + +// expected-no-diagnostics + +// Both type and name contains "kind". +struct UnionLikeStruct1 { + enum Kind { +volume, +area + } kind; + + int Volume; + int Area; + + UnionLikeStruct1(Kind kind, int Val) : kind(kind) { +switch (kind) { +case volume: + Volume = Val; + break; +case area: + Area = Val; + break; +} + } +}; + +void fUnionLikeStruct1() { + UnionLikeStruct1 t(UnionLikeStruct1::volume, 10); +} + +// Only name contains "kind". +struct UnionLikeStruct2 { + enum Type { +volume, +area + } kind; + + int Volume; + int Area; + + UnionLikeStruct2(Type kind, int Val) : kind(kind) { +switch (kind) { +case volume: + Volume = Val; + break; +case area: + Area = Val; + break; +} + } +}; + +void fUnionLikeStruct2() { + UnionLikeStruct2 t(UnionLikeStruct2::volume, 10); +} + +// Only type contains "kind". +struct UnionLikeStruct3 { + enum Kind { +volume, +area + } type; + + int Volume; + int Area; + + UnionLikeStruct3(Kind type, int Val) : type(type) { +switch (type) { +case volume: + Volume = Val; + break; +case area: + Area = Val; + break; +} + } +}; + +void fUnionLikeStruct3() { + UnionLikeStruct3 t(UnionLikeStruct3::volume, 10); +} + +// Only type contains "tag". +struct UnionLikeStruct4 { + enum Tag { +volume, +area + } type; + + int Volume; + int Area; + + UnionLikeStruct4(Tag type, int Val) : type(type) { +switch (type) { +case volume: + Volume = Val; + break; +case area: + Area = Val; + break; +} + } +}; + +void fUnionLikeStruct4() { + UnionLikeStruct4 t(UnionLikeStruct4::volume, 10); +} Index: lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp === --- lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp +++ lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp @@ -107,6 +107,11 @@ static bool willObjectBeAnalyzedLater(const CXXConstructorDecl *Ctor, CheckerContext &Context); +/// Checks whether RD is a union-like construct. We'll consider a RecordDecl +/// union-like, if it has a field that has [Kk]ind or [Tt]ag in it's identifier +/// or type name. +static bool isUnionLike(const RecordDecl *RD); + //===--===// // Methods for UninitializedObjectChecker. //===--===// @@ -223,6 +228,11 @@ R->getValueType()->getAs()->getDecl()->getDefinition(); assert(RD && "Referred record has no definition"); + if (Opts.IgnoreUnionlikeConstructs && isUnionLike(RD)) { +IsAnyFieldInitialized = true; +return false; + } + bool ContainsUninitField = false; // Are all of this non-union's fields initialized? @@ -454,6 +464,19 @@ return false; } +static bool isUnionLike(const RecordDecl *RD) { + llvm::Regex ContainsKindOrTag("[kK]ind|[tT]ag"); + + for (const FieldDecl *FD : RD->fields()) { +if (ContainsKindOrTag.match(FD->getType().getAsString())) + return true; +if (ContainsKindOrTag.match(FD->getName())) + return true; + } + + return fa
[PATCH] D51544: [OpenCL] Split opencl-c.h header
asavonic added a comment. In https://reviews.llvm.org/D51544#1224730, @Anastasia wrote: > It seems generally good to partition this big header but I am trying to > understand what problem is it trying to solve now? Main motivation is to reduce memory footprint by factoring out everything that is 'common' between PCHs compiled for different targets (CL1.2/2.0 and spir/spir64). > So if we want to add these 2 devices, we now need 4*2 different PCHs, > since every combination of -cl-std and -triple must be compiled twice > with different OpenCL extensions defines. > > Size of each PCH is 2.5M, so we need ~20M of memory to store our > PCHs. If we want to add more devices or support another OpenCL C > version, the size will double. This also allows to do 'partial' pre-compilation, where 'common' part is pre-compiled into a single target independent PCH, and plain header is used for all target-specific things. In this case, you no longer need to maintain different target-specific PCHs and this greatly simplifies the whole process. > opencl-c-platform.h (5K LOC) must still be pre-compiled for each > supported combination, but since it is a lot smaller (~0.5M vs > original 2.5M), it is not that bad. Or we can sacrifice some > performance and leave this header without pre-compilation: large > portion of opencl-c-platform.h contains vendor extensions, so it will > be removed by preprocessor anyway. Repository: rC Clang https://reviews.llvm.org/D51544 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50250: [clang][ubsan] Implicit Conversion Sanitizer - integer sign change - clang part
lebedev.ri added a comment. Ping. Repository: rC Clang https://reviews.llvm.org/D50250 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50901: [clang][ubsan] Split Implicit Integer Truncation Sanitizer into unsigned and signed checks
lebedev.ri added a comment. Ping. Repository: rC Clang https://reviews.llvm.org/D50901 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51575: [clang-tidy] Implement a clang-tidy check to verify Google Objective-C function naming conventions 📜
benhamilton requested changes to this revision. benhamilton added a comment. This revision now requires changes to proceed. Thanks for this! Let's consolidate this with the property name checker (either simplify the logic there and allow `arBiTRAryCapSAnYWHere` or apply the same registered acronym logic here). Comment at: clang-tidy/google/FunctionNamingCheck.cpp:35 + // non-standard capitalized character sequences including acronyms, + // initialisms, and prefixes of symbols (e.g., UIColorFromNSString). For this + // reason, the regex only verifies that the function name after the prefix Any reason why this is different from the implementation in the property name checker? Either we should allow both of: ``` void ARBiTraRilyNameDFuncTioN(); // ... @property (...) id arBItrArIlyNameD; ``` or we should require that acronyms in the middle of the name be registered/known acronyms for both properties and functions. I believe this diff allows arbitrary capitalization for functions, but we disallowed that for property names, so I think we should be consistent. Comment at: clang-tidy/google/FunctionNamingCheck.cpp:50 + +void FunctionNamingCheck::registerMatchers(MatchFinder *Finder) { + // This check should only be applied to Objective-C sources. Wizard wrote: > Can we do some simple check to see if some easy fix can be provided just like > `objc-property-declaration` check? > Something like `static bool isPositive` to `static bool IsPositive` and > `static bool is_upper_camel` to `IsUpperCamel`. Such check can help provide > code fix for a lot of very common mistake at a low cost (i.e. if the naming > pattern cannot be simply recognized, just provide no fix). +1, I think the two checks should be substantially similar. Comment at: clang-tidy/google/FunctionNamingCheck.h:21 + +/// Finds function names that do not conform to the recommendations of the +/// Google Objective-C Style Guide. Function names should be in upper camel case Worth mentioning this does not apply to Objective-C method names, nor Objective-C properties. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51575 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37310: [Atomic] Merge alignment information from Decl and from Type when emit atomic expression.
twoh added a comment. Herald added subscribers: llvm-commits, jfb. Hello, I observed a case where atomic builtin generates libcall when the corresponding sync builtin generates an atomic instruction (https://bugs.llvm.org/show_bug.cgi?id=38846). It seems that the alignment checking for __atomic builtins (line 759 of this patch) results the difference, and wonder if the check is actually necessary. Could anyone please shed some light on understanding this? Thanks! Repository: rL LLVM https://reviews.llvm.org/D37310 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51688: [clangd] Set SymbolID for sema macros so that they can be merged with index macros.
ioeric created this revision. ioeric added a reviewer: sammccall. Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay, ilya-biryukov. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51688 Files: clangd/AST.cpp clangd/AST.h clangd/CodeComplete.cpp clangd/index/SymbolCollector.cpp unittests/clangd/CodeCompleteTests.cpp Index: unittests/clangd/CodeCompleteTests.cpp === --- unittests/clangd/CodeCompleteTests.cpp +++ unittests/clangd/CodeCompleteTests.cpp @@ -1882,6 +1882,16 @@ AllOf(Named("Func"), HasInclude("\"foo.h\""), Not(InsertInclude(); } +TEST(CompletionTest, MergeMacrosFromIndexAndSema) { + Symbol Sym; + Sym.ID = SymbolID("c:foo.cpp@8@macro@Clangd_Macro_Test"); + Sym.SymInfo.Kind = index::SymbolKind::Macro; + Sym.IsIndexedForCodeCompletion = true; + EXPECT_THAT(completions("#define Clangd_Macro_Test\nClangd_Macro_T^", {Sym}) + .Completions, + UnorderedElementsAre(Named("Clangd_Macro_Test"))); +} + } // namespace } // namespace clangd } // namespace clang Index: clangd/index/SymbolCollector.cpp === --- clangd/index/SymbolCollector.cpp +++ clangd/index/SymbolCollector.cpp @@ -389,14 +389,16 @@ if (index::generateUSRForMacro(Name->getName(), MI->getDefinitionLoc(), SM, USR)) return true; - SymbolID ID(USR); + auto ID = getSymbolID(*Name, MI, SM); + if (!ID) +return true; // Only collect one instance in case there are multiple. - if (Symbols.find(ID) != nullptr) + if (Symbols.find(*ID) != nullptr) return true; Symbol S; - S.ID = std::move(ID); + S.ID = std::move(*ID); S.Name = Name->getName(); S.IsIndexedForCodeCompletion = true; S.SymInfo = index::getSymbolInfoForMacro(*MI); Index: clangd/CodeComplete.cpp === --- clangd/CodeComplete.cpp +++ clangd/CodeComplete.cpp @@ -505,14 +505,15 @@ }; // Determine the symbol ID for a Sema code completion result, if possible. -llvm::Optional getSymbolID(const CodeCompletionResult &R) { +llvm::Optional getSymbolID(const CodeCompletionResult &R, + const SourceManager &SM) { switch (R.Kind) { case CodeCompletionResult::RK_Declaration: case CodeCompletionResult::RK_Pattern: { return clang::clangd::getSymbolID(R.Declaration); } case CodeCompletionResult::RK_Macro: -// FIXME: Macros do have USRs, but the CCR doesn't contain enough info. +return clang::clangd::getSymbolID(*R.Macro, R.MacroDefInfo, SM); case CodeCompletionResult::RK_Keyword: return None; } @@ -1435,7 +1436,8 @@ llvm::DenseSet UsedIndexResults; auto CorrespondingIndexResult = [&](const CodeCompletionResult &SemaResult) -> const Symbol * { - if (auto SymID = getSymbolID(SemaResult)) { + if (auto SymID = + getSymbolID(SemaResult, Recorder->CCSema->getSourceManager())) { auto I = IndexResults.find(*SymID); if (I != IndexResults.end()) { UsedIndexResults.insert(&*I); Index: clangd/AST.h === --- clangd/AST.h +++ clangd/AST.h @@ -37,6 +37,11 @@ /// Gets the symbol ID for a declaration, if possible. llvm::Optional getSymbolID(const Decl *D); +/// Gets the symbol ID for a macro, if possible. +llvm::Optional getSymbolID(const IdentifierInfo &II, + const MacroInfo *MI, + const SourceManager &SM); + } // namespace clangd } // namespace clang Index: clangd/AST.cpp === --- clangd/AST.cpp +++ clangd/AST.cpp @@ -61,5 +61,16 @@ return SymbolID(USR); } +llvm::Optional getSymbolID(const IdentifierInfo &II, + const MacroInfo *MI, + const SourceManager &SM) { + if (MI == nullptr) +return None; + llvm::SmallString<128> USR; + if (index::generateUSRForMacro(II.getName(), MI->getDefinitionLoc(), SM, USR)) +return None; + return SymbolID(USR); +} + } // namespace clangd } // namespace clang ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50947: lambdas in modules: make PendingFakeDefinitionData a set
elsteveogrande added inline comments. Comment at: lib/Serialization/ASTReaderDecl.cpp:1766 + +// FIXME: handle serialized lambdas assert(!DD.IsLambda && !MergeDD.IsLambda && "faked up lambda definition?"); rsmith wrote: > Did you mean to add this FIXME? Yup: to indicate this is assert about `faked up lambda definition` will be addressed in an upcoming diff, i.e., fixing the lambda issues :) See D50948, D50949 Comment at: lib/Serialization/ASTReaderDecl.cpp:4246 OldDD && (OldDD->Definition != RD || !Reader.PendingFakeDefinitionData.count(OldDD)); RD->setParamDestroyedInCallee(Record.readInt()); rsmith wrote: > This does not appear to be NFC: the `FakeLoaded` vs absent-from-map > distinction matters here. I think the idea here is that we still need to load > the lexical contents of the declaration of the class that we pick to be the > definition even if we've already found and merged another definition into it. > > That said, if this //is// necessary, we should have some test coverage for > it, and based on this change not breaking any of our tests, we evidently > don't. :( I'll try this change out against our codebase and see if I can find > a testcase. Thanks for checking this! Yup I missed this; count would be different and therefore this changes logic, which surprisingly didn't break things as far as I could see. I'll change this back to `Fake` vs. `FakeLoaded`. Repository: rC Clang https://reviews.llvm.org/D50947 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51688: [clangd] Set SymbolID for sema macros so that they can be merged with index macros.
ioeric updated this revision to Diff 164055. ioeric added a comment. - Set name for the test symbol. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51688 Files: clangd/AST.cpp clangd/AST.h clangd/CodeComplete.cpp clangd/index/SymbolCollector.cpp unittests/clangd/CodeCompleteTests.cpp Index: unittests/clangd/CodeCompleteTests.cpp === --- unittests/clangd/CodeCompleteTests.cpp +++ unittests/clangd/CodeCompleteTests.cpp @@ -1882,6 +1882,17 @@ AllOf(Named("Func"), HasInclude("\"foo.h\""), Not(InsertInclude(); } +TEST(CompletionTest, MergeMacrosFromIndexAndSema) { + Symbol Sym; + Sym.Name = "Clangd_Macro_Test"; + Sym.ID = SymbolID("c:foo.cpp@8@macro@Clangd_Macro_Test"); + Sym.SymInfo.Kind = index::SymbolKind::Macro; + Sym.IsIndexedForCodeCompletion = true; + EXPECT_THAT(completions("#define Clangd_Macro_Test\nClangd_Macro_T^", {Sym}) + .Completions, + UnorderedElementsAre(Named("Clangd_Macro_Test"))); +} + } // namespace } // namespace clangd } // namespace clang Index: clangd/index/SymbolCollector.cpp === --- clangd/index/SymbolCollector.cpp +++ clangd/index/SymbolCollector.cpp @@ -389,14 +389,16 @@ if (index::generateUSRForMacro(Name->getName(), MI->getDefinitionLoc(), SM, USR)) return true; - SymbolID ID(USR); + auto ID = getSymbolID(*Name, MI, SM); + if (!ID) +return true; // Only collect one instance in case there are multiple. - if (Symbols.find(ID) != nullptr) + if (Symbols.find(*ID) != nullptr) return true; Symbol S; - S.ID = std::move(ID); + S.ID = std::move(*ID); S.Name = Name->getName(); S.IsIndexedForCodeCompletion = true; S.SymInfo = index::getSymbolInfoForMacro(*MI); Index: clangd/CodeComplete.cpp === --- clangd/CodeComplete.cpp +++ clangd/CodeComplete.cpp @@ -505,14 +505,15 @@ }; // Determine the symbol ID for a Sema code completion result, if possible. -llvm::Optional getSymbolID(const CodeCompletionResult &R) { +llvm::Optional getSymbolID(const CodeCompletionResult &R, + const SourceManager &SM) { switch (R.Kind) { case CodeCompletionResult::RK_Declaration: case CodeCompletionResult::RK_Pattern: { return clang::clangd::getSymbolID(R.Declaration); } case CodeCompletionResult::RK_Macro: -// FIXME: Macros do have USRs, but the CCR doesn't contain enough info. +return clang::clangd::getSymbolID(*R.Macro, R.MacroDefInfo, SM); case CodeCompletionResult::RK_Keyword: return None; } @@ -1435,7 +1436,8 @@ llvm::DenseSet UsedIndexResults; auto CorrespondingIndexResult = [&](const CodeCompletionResult &SemaResult) -> const Symbol * { - if (auto SymID = getSymbolID(SemaResult)) { + if (auto SymID = + getSymbolID(SemaResult, Recorder->CCSema->getSourceManager())) { auto I = IndexResults.find(*SymID); if (I != IndexResults.end()) { UsedIndexResults.insert(&*I); Index: clangd/AST.h === --- clangd/AST.h +++ clangd/AST.h @@ -37,6 +37,11 @@ /// Gets the symbol ID for a declaration, if possible. llvm::Optional getSymbolID(const Decl *D); +/// Gets the symbol ID for a macro, if possible. +llvm::Optional getSymbolID(const IdentifierInfo &II, + const MacroInfo *MI, + const SourceManager &SM); + } // namespace clangd } // namespace clang Index: clangd/AST.cpp === --- clangd/AST.cpp +++ clangd/AST.cpp @@ -61,5 +61,16 @@ return SymbolID(USR); } +llvm::Optional getSymbolID(const IdentifierInfo &II, + const MacroInfo *MI, + const SourceManager &SM) { + if (MI == nullptr) +return None; + llvm::SmallString<128> USR; + if (index::generateUSRForMacro(II.getName(), MI->getDefinitionLoc(), SM, USR)) +return None; + return SymbolID(USR); +} + } // namespace clangd } // namespace clang ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51689: [clangd] Dense posting lists proof-of-concept
sammccall created this revision. sammccall added a reviewer: kbobyrev. Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay, ioeric, ilya-biryukov. This uses a bitmap representation instead of a list if the density of the list is high enough (at least 1 in 32, which is the breakeven point sizewise). Experimenting with the LLVM index, this saves about 3% of total posting list size, which isn't worth the complexity. However it should also improve iterator performance somewhat: - advance is within a constant factor (find next set bit, average step is bounded) - advanceTo is constant time instead of log(n) with random accesses If the posting lists that are dense are also commonly used in queries (seems likely for common trigrams) then this may be worth doing for latency reasons. I'm uploading this so Kirill can experiment with benchmarks. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51689 Files: clangd/index/dex/DexIndex.cpp clangd/index/dex/Iterator.cpp clangd/index/dex/Iterator.h Index: clangd/index/dex/Iterator.h === --- clangd/index/dex/Iterator.h +++ clangd/index/dex/Iterator.h @@ -32,6 +32,7 @@ #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_DEX_ITERATOR_H #include "llvm/ADT/ArrayRef.h" +#include "llvm/ADT/BitVector.h" #include "llvm/Support/raw_ostream.h" #include #include @@ -44,20 +45,6 @@ /// Symbol position in the list of all index symbols sorted by a pre-computed /// symbol quality. using DocID = uint32_t; -/// Contains sorted sequence of DocIDs all of which belong to symbols matching -/// certain criteria, i.e. containing a Search Token. PostingLists are values -/// for the inverted index. -// FIXME(kbobyrev): Posting lists for incomplete trigrams (one/two symbols) are -// likely to be very dense and hence require special attention so that the index -// doesn't use too much memory. Possible solution would be to construct -// compressed posting lists which consist of ranges of DocIDs instead of -// distinct DocIDs. A special case would be the empty query: for that case -// TrueIterator should be implemented - an iterator which doesn't actually store -// any PostingList within itself, but "contains" all DocIDs in range -// [0, IndexSize). -using PostingList = std::vector; -/// Immutable reference to PostingList object. -using PostingListRef = llvm::ArrayRef; /// Iterator is the interface for Query Tree node. The simplest type of Iterator /// is DocumentIterator which is simply a wrapper around PostingList iterator @@ -131,11 +118,6 @@ /// to acquire preliminary scores of requested items. std::vector> consume(Iterator &It); -/// Returns a document iterator over given PostingList. -/// -/// DocumentIterator returns DEFAULT_BOOST_SCORE for each processed item. -std::unique_ptr create(PostingListRef Documents); - /// Returns AND Iterator which performs the intersection of the PostingLists of /// its children. /// @@ -200,6 +182,33 @@ Children.push_back(move(Head)); } +/// Contains sorted sequence of DocIDs all of which belong to symbols matching +/// certain criteria, i.e. containing a Search Token. PostingLists are values +/// for the inverted index. +class PostingList { +public: + PostingList() : Representation(Null) {} + PostingList(std::vector Docs); + /// Returns a document iterator over given PostingList. + std::unique_ptr iterator() const; + + PostingList(PostingList&&); + PostingList &operator=(PostingList&&); + ~PostingList(); + + size_t bytes() const; + +private: + enum Rep { Null, Dense, Sparse } Representation; + union { +struct { + llvm::BitVector Bitmap; + size_t Count; +} DenseRep; +std::vector SparseRep; + }; +}; + } // namespace dex } // namespace clangd } // namespace clang Index: clangd/index/dex/Iterator.cpp === --- clangd/index/dex/Iterator.cpp +++ clangd/index/dex/Iterator.cpp @@ -18,12 +18,11 @@ namespace { -/// Implements Iterator over a PostingList. DocumentIterator is the most basic -/// iterator: it doesn't have any children (hence it is the leaf of iterator -/// tree) and is simply a wrapper around PostingList::const_iterator. -class DocumentIterator : public Iterator { +/// Implements Iterator over a sparse PostingList. +/// This is a leaf iterator which simply wraps a list of DocIDs. +class SparseIterator : public Iterator { public: - DocumentIterator(PostingListRef Documents) + SparseIterator(llvm::ArrayRef Documents) : Documents(Documents), Index(std::begin(Documents)) {} bool reachedEnd() const override { return Index == std::end(Documents); } @@ -74,8 +73,52 @@ return OS; } - PostingListRef Documents; - PostingListRef::const_iterator Index; + llvm::ArrayRef Documents; + llvm::ArrayRef::iterator Index; +}; + +/// Implements Iterator over a dense PostingList. +/// This is a leaf iterator over a
[PATCH] D51688: [clangd] Set SymbolID for sema macros so that they can be merged with index macros.
sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land. Comment at: clangd/AST.h:41 +/// Gets the symbol ID for a macro, if possible. +llvm::Optional getSymbolID(const IdentifierInfo &II, + const MacroInfo *MI, maybe add a comment about the semantics of macro symbol IDs - particularly that they depend on the exact file offset. We could change these semantics in future by reimplementing this function, there's no reason we have to use USRs for everything, and macros are really simple. Comment at: clangd/index/SymbolCollector.cpp:389 llvm::SmallString<128> USR; if (index::generateUSRForMacro(Name->getName(), MI->getDefinitionLoc(), SM, USR)) this is now dead Comment at: clangd/index/SymbolCollector.cpp:452 if (const auto *MI = PP->getMacroDefinition(II).getMacroInfo()) if (!index::generateUSRForMacro(II->getName(), MI->getDefinitionLoc(), PP->getSourceManager(), USR)) update this to use getSymbolID Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51688 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r341483 - [OPENMP][NVPTX] Disable runtime-type info for CUDA devices.
Author: abataev Date: Wed Sep 5 10:10:30 2018 New Revision: 341483 URL: http://llvm.org/viewvc/llvm-project?rev=341483&view=rev Log: [OPENMP][NVPTX] Disable runtime-type info for CUDA devices. RTTI is not supported by the NVPTX target. Added: cfe/trunk/test/OpenMP/nvptx_target_rtti_messages.cpp Modified: cfe/trunk/lib/Frontend/CompilerInvocation.cpp Modified: cfe/trunk/lib/Frontend/CompilerInvocation.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInvocation.cpp?rev=341483&r1=341482&r2=341483&view=diff == --- cfe/trunk/lib/Frontend/CompilerInvocation.cpp (original) +++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp Wed Sep 5 10:10:30 2018 @@ -2650,6 +2650,11 @@ static void ParseLangArgs(LangOptions &O Opts.Exceptions = 0; Opts.CXXExceptions = 0; } + // NVPTX does not support RTTI. + if (Opts.OpenMPIsDevice && T.isNVPTX()) { +Opts.RTTI = 0; +Opts.RTTIData = 0; + } // Get the OpenMP target triples if any. if (Arg *A = Args.getLastArg(options::OPT_fopenmp_targets_EQ)) { Added: cfe/trunk/test/OpenMP/nvptx_target_rtti_messages.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/nvptx_target_rtti_messages.cpp?rev=341483&view=auto == --- cfe/trunk/test/OpenMP/nvptx_target_rtti_messages.cpp (added) +++ cfe/trunk/test/OpenMP/nvptx_target_rtti_messages.cpp Wed Sep 5 10:10:30 2018 @@ -0,0 +1,68 @@ +// RUN: %clang_cc1 -fopenmp -x c++ -triple powerpc64le-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm-bc %s -o %t-ppc-host.bc -fexceptions -fcxx-exceptions +// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple nvptx64-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o - -ferror-limit 100 + +#ifndef HEADER +#define HEADER + +namespace std { + class type_info; +} + +template +class TemplateClass { + T a; +public: + TemplateClass() { (void)typeid(int); } // expected-error {{use of typeid requires -frtti}} + T f_method() const { return a; } +}; + +int foo(); + +int baz1(); + +int baz2(); + +int baz4() { return 5; } + +template +T FA() { + TemplateClass s; + return s.f_method(); +} + +#pragma omp declare target +struct S { + int a; + S(int a) : a(a) { (void)typeid(int); } // expected-error {{use of typeid requires -frtti}} +}; + +int foo() { return 0; } +int b = 15; +int d; +#pragma omp end declare target +int c; + +int bar() { return 1 + foo() + bar() + baz1() + baz2(); } + +int maini1() { + int a; + static long aa = 32; +#pragma omp target map(tofrom \ + : a, b) + { +S s(a); +static long aaa = 23; +a = foo() + bar() + b + c + d + aa + aaa + FA(); +(void)typeid(int); // expected-error {{use of typeid requires -frtti}} + } + return baz4(); +} + +int baz3() { return 2 + baz2(); } +int baz2() { +#pragma omp target + (void)typeid(int); // expected-error {{use of typeid requires -frtti}} + return 2 + baz3(); +} + +#endif // HEADER ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51378: [OPENMP] Add support for nested 'declare target' directives
ABataev accepted this revision. ABataev added a comment. LG https://reviews.llvm.org/D51378 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51481: [clangd] Implement proximity path boosting for Dex
kbobyrev updated this revision to Diff 164062. kbobyrev marked 18 inline comments as done. kbobyrev added a comment. Address a round of comments https://reviews.llvm.org/D51481 Files: clang-tools-extra/clangd/FileDistance.h clang-tools-extra/clangd/URI.cpp clang-tools-extra/clangd/URI.h clang-tools-extra/clangd/index/SymbolYAML.cpp clang-tools-extra/clangd/index/SymbolYAML.h clang-tools-extra/clangd/index/dex/DexIndex.cpp clang-tools-extra/clangd/index/dex/DexIndex.h clang-tools-extra/clangd/index/dex/Iterator.cpp clang-tools-extra/clangd/index/dex/Token.h clang-tools-extra/clangd/tool/ClangdMain.cpp clang-tools-extra/unittests/clangd/DexIndexTests.cpp Index: clang-tools-extra/unittests/clangd/DexIndexTests.cpp === --- clang-tools-extra/unittests/clangd/DexIndexTests.cpp +++ clang-tools-extra/unittests/clangd/DexIndexTests.cpp @@ -7,6 +7,8 @@ // //===--===// +#include "FuzzyMatch.h" +#include "TestFS.h" #include "TestIndex.h" #include "index/Index.h" #include "index/Merge.h" @@ -30,6 +32,12 @@ namespace dex { namespace { +std::vector URISchemes = {"unittest"}; + +//===--===// +// Query iterator tests. +//===--===// + std::vector consumeIDs(Iterator &It) { auto IDAndScore = consume(It); std::vector IDs(IDAndScore.size()); @@ -325,15 +333,24 @@ EXPECT_THAT(ElementBoost, 3); } +//===--===// +// Search token tests. +//===--===// + testing::Matcher> -trigramsAre(std::initializer_list Trigrams) { +tokensAre(std::initializer_list Strings, Token::Kind Kind) { std::vector Tokens; - for (const auto &Symbols : Trigrams) { -Tokens.push_back(Token(Token::Kind::Trigram, Symbols)); + for (const auto &TokenData : Strings) { +Tokens.push_back(Token(Kind, TokenData)); } return testing::UnorderedElementsAreArray(Tokens); } +testing::Matcher> +trigramsAre(std::initializer_list Trigrams) { + return tokensAre(Trigrams, Token::Kind::Trigram); +} + TEST(DexIndexTrigrams, IdentifierTrigrams) { EXPECT_THAT(generateIdentifierTrigrams("X86"), trigramsAre({"x86", "x$$", "x8$"})); @@ -408,8 +425,25 @@ "hij", "ijk", "jkl", "klm"})); } +TEST(DexSearchTokens, SymbolPath) { + EXPECT_THAT(generateProximityURIs( + "unittest:///clang-tools-extra/clangd/index/Token.h"), + ElementsAre("unittest:///clang-tools-extra/clangd/index/Token.h", + "unittest:///clang-tools-extra/clangd/index", + "unittest:///clang-tools-extra/clangd", + "unittest:///clang-tools-extra", "unittest:///")); + + EXPECT_THAT(generateProximityURIs("unittest:///a/b/c.h"), + ElementsAre("unittest:///a/b/c.h", "unittest:///a/b", + "unittest:///a", "unittest:///")); +} + +//===--===// +// Index tests. +//===--===// + TEST(DexIndex, Lookup) { - auto I = DexIndex::build(generateSymbols({"ns::abc", "ns::xyz"})); + auto I = DexIndex::build(generateSymbols({"ns::abc", "ns::xyz"}), 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")); @@ -421,7 +455,8 @@ TEST(DexIndex, FuzzyFind) { auto Index = DexIndex::build( generateSymbols({"ns::ABC", "ns::BCD", "::ABC", "ns::nested::ABC", - "other::ABC", "other::A"})); + "other::ABC", "other::A"}), + URISchemes); FuzzyFindRequest Req; Req.Query = "ABC"; Req.Scopes = {"ns::"}; @@ -443,7 +478,8 @@ TEST(DexIndexTest, FuzzyMatchQ) { auto I = DexIndex::build( - generateSymbols({"LaughingOutLoud", "LionPopulation", "LittleOldLady"})); + generateSymbols({"LaughingOutLoud", "LionPopulation", "LittleOldLady"}), + URISchemes); FuzzyFindRequest Req; Req.Query = "lol"; Req.MaxCandidateCount = 2; @@ -461,12 +497,12 @@ symbol("2") /* duplicate */}; FuzzyFindRequest Req; Req.Query = "2"; - DexIndex I(Symbols); + DexIndex I(Symbols, URISchemes); EXPECT_THAT(match(I, Req), ElementsAre("2", "2")); } TEST(DexIndexTest, DexIndexLimitedNumMatches) { - auto I = DexIndex::build(generateNumSymbols(0, 100)); + auto I = DexIndex::build(generateNumSymbols(0, 100), URISchemes); FuzzyFindRequest Req; Req.Query = "5"; Req.MaxCandidateCount = 3; @@ -478,73
r341484 - Test Commit for git-svn-cleanup comment.
Author: erichkeane Date: Wed Sep 5 10:14:21 2018 New Revision: 341484 URL: http://llvm.org/viewvc/llvm-project?rev=341484&view=rev Log: Test Commit for git-svn-cleanup comment. Removes the class name for the Expr class, which isn't necessary. Modified: cfe/trunk/include/clang/AST/Expr.h Modified: cfe/trunk/include/clang/AST/Expr.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Expr.h?rev=341484&r1=341483&r2=341484&view=diff == --- cfe/trunk/include/clang/AST/Expr.h (original) +++ cfe/trunk/include/clang/AST/Expr.h Wed Sep 5 10:14:21 2018 @@ -99,10 +99,9 @@ struct SubobjectAdjustment { } }; -/// Expr - This represents one expression. Note that Expr's are subclasses of -/// Stmt. This allows an expression to be transparently used any place a Stmt -/// is required. -/// +/// This represents one expression. Note that Expr's are subclasses of Stmt. +/// This allows an expression to be transparently used any place a Stmt is +/// required. class Expr : public Stmt { QualType TR; ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48896: [libcxx][c++17] P0083R5: Splicing Maps and Sets Part 2: merge
erik.pilkington updated this revision to Diff 164064. erik.pilkington added a comment. Ping! In the new patch: - Uncomment __cpp_lib_node_extract, reverting r340544 - Rebase/Clean up the diff a bit https://reviews.llvm.org/D48896 Files: libcxx/include/__hash_table libcxx/include/__node_handle libcxx/include/__tree libcxx/include/map libcxx/include/set libcxx/include/unordered_map libcxx/include/unordered_set libcxx/test/std/containers/associative/map/map.modifiers/merge.pass.cpp libcxx/test/std/containers/associative/multimap/multimap.modifiers/merge.pass.cpp libcxx/test/std/containers/associative/multiset/merge.pass.cpp libcxx/test/std/containers/associative/set/merge.pass.cpp libcxx/test/std/containers/unord/unord.map/unord.map.modifiers/merge.pass.cpp libcxx/test/std/containers/unord/unord.multimap/unord.multimap.modifiers/merge.pass.cpp libcxx/test/std/containers/unord/unord.multiset/merge.pass.cpp libcxx/test/std/containers/unord/unord.set/merge.pass.cpp Index: libcxx/test/std/containers/unord/unord.set/merge.pass.cpp === --- /dev/null +++ libcxx/test/std/containers/unord/unord.set/merge.pass.cpp @@ -0,0 +1,135 @@ +//===--===// +// +// The LLVM Compiler Infrastructure +// +// This file is dual licensed under the MIT and the University of Illinois Open +// Source Licenses. See LICENSE.TXT for details. +// +//===--===// + +// UNSUPPORTED: c++98, c++03, c++11, c++14 + +// + +// class unordered_set + +// template +// void merge(unordered_set& source); + +#include +#include "test_macros.h" +#include "Counter.h" + +template +bool set_equal(const Set& set, Set other) +{ +return set == other; +} + +#ifndef TEST_HAS_NO_EXCEPTIONS +template +struct throw_hasher +{ +bool& should_throw_; + +throw_hasher(bool& should_throw) : should_throw_(should_throw) {} + +typedef size_t result_type; +typedef T argument_type; + +size_t operator()(const T& p) const +{ +if (should_throw_) +throw 0; +return std::hash()(p); +} +}; +#endif + +int main() +{ +{ +std::unordered_set src{1, 3, 5}; +std::unordered_set dst{2, 4, 5}; +dst.merge(src); +assert(set_equal(src, {5})); +assert(set_equal(dst, {1, 2, 3, 4, 5})); +} + +#ifndef TEST_HAS_NO_EXCEPTIONS +{ +bool do_throw = false; +typedef std::unordered_set, throw_hasher>> set_type; +set_type src({1, 3, 5}, 0, throw_hasher>(do_throw)); +set_type dst({2, 4, 5}, 0, throw_hasher>(do_throw)); + +assert(Counter_base::gConstructed == 6); + +do_throw = true; +try +{ +dst.merge(src); +} +catch (int) +{ +do_throw = false; +} +assert(!do_throw); +assert(set_equal(src, set_type({1, 3, 5}, 0, throw_hasher>(do_throw; +assert(set_equal(dst, set_type({2, 4, 5}, 0, throw_hasher>(do_throw; +} +#endif +assert(Counter_base::gConstructed == 0); +struct equal +{ +equal() = default; + +bool operator()(const Counter& lhs, const Counter& rhs) const +{ +return lhs == rhs; +} +}; +struct hasher +{ +hasher() = default; +typedef Counter argument_type; +typedef size_t result_type; +size_t operator()(const Counter& p) const { return std::hash>()(p); } +}; +{ +typedef std::unordered_set, std::hash>, std::equal_to>> first_set_type; +typedef std::unordered_set, hasher, equal> second_set_type; +typedef std::unordered_multiset, hasher, equal> third_set_type; + +first_set_type first{1, 2, 3}; +second_set_type second{2, 3, 4}; +third_set_type third{1, 3}; + +assert(Counter_base::gConstructed == 8); + +first.merge(second); +first.merge(std::move(second)); +first.merge(third); +first.merge(std::move(third)); + +assert(set_equal(first, {1, 2, 3, 4})); +assert(set_equal(second, {2, 3})); +assert(set_equal(third, {1, 3})); + +assert(Counter_base::gConstructed == 8); +} +assert(Counter_base::gConstructed == 0); +{ +std::unordered_set first; +{ +std::unordered_set second; +first.merge(second); +first.merge(std::move(second)); +} +{ +std::unordered_multiset second; +first.merge(second); +first.merge(std::move(second)); +} +} +} Index: libcxx/test/std/containers/unord/unord.multiset/merge.pass.cpp === --- /dev/null +++ libcxx/test/std/containers/unord/unord.multiset/merge.pass.cpp @@ -0,0 +1,135 @
[PATCH] D51554: [CUDA][OPENMP][NVPTX]Improve logic of the debug info support.
ABataev added a comment. In https://reviews.llvm.org/D51554#1224049, @echristo wrote: > The change in name here from "line tables" to "directives only" feels a bit > confusing. "Limited" seems to be a bit more clear, or even remaining line > tables only. Can you explain where you were going with this particular set of > changes in a bit more detail please? > > Thanks! > > -eric CUDA/NVPTX supports only 3 types of the debug info: limited/full, debug directives and no debug info at all. It does not support debug tables, so I just convert this into debug directives only. The main idea is to mimic what nvcc does. It behaves absolutely the same way. If the opt level is O0, we can use full debug info. if opt level is >O0, we can use only lineinfo(debug directives) or no debug info. If we enabling debug info for the device code using `--cuda-noopt-device-debug`, the opt level for the device code is lowered to O0 and we enable full debug info. The host code will be optimized still. Repository: rC Clang https://reviews.llvm.org/D51554 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51690: [clangd] NFC: mark single-parameter constructors explicit
kbobyrev created this revision. kbobyrev added reviewers: ioeric, ilya-biryukov, sammccall. kbobyrev added a project: clang-tools-extra. Herald added subscribers: kadircet, arphaman, jkorous, MaskRay. Code health: prevent implicit conversions to user-defined types. https://reviews.llvm.org/D51690 Files: clang-tools-extra/clangd/index/dex/Iterator.cpp Index: clang-tools-extra/clangd/index/dex/Iterator.cpp === --- clang-tools-extra/clangd/index/dex/Iterator.cpp +++ clang-tools-extra/clangd/index/dex/Iterator.cpp @@ -23,7 +23,7 @@ /// tree) and is simply a wrapper around PostingList::const_iterator. class DocumentIterator : public Iterator { public: - DocumentIterator(PostingListRef Documents) + explicit DocumentIterator(PostingListRef Documents) : Documents(Documents), Index(std::begin(Documents)) {} bool reachedEnd() const override { return Index == std::end(Documents); } @@ -85,7 +85,7 @@ /// iterator restores the invariant: all children must point to the same item. class AndIterator : public Iterator { public: - AndIterator(std::vector> AllChildren) + explicit AndIterator(std::vector> AllChildren) : Children(std::move(AllChildren)) { assert(!Children.empty() && "AND iterator should have at least one child."); // Establish invariants. @@ -193,7 +193,7 @@ /// soon as all of its children are exhausted. class OrIterator : public Iterator { public: - OrIterator(std::vector> AllChildren) + explicit OrIterator(std::vector> AllChildren) : Children(std::move(AllChildren)) { assert(Children.size() > 0 && "OR iterator must have at least one child."); } @@ -279,7 +279,7 @@ /// in O(1). class TrueIterator : public Iterator { public: - TrueIterator(DocID Size) : Size(Size) {} + explicit TrueIterator(DocID Size) : Size(Size) {} bool reachedEnd() const override { return Index >= Size; } Index: clang-tools-extra/clangd/index/dex/Iterator.cpp === --- clang-tools-extra/clangd/index/dex/Iterator.cpp +++ clang-tools-extra/clangd/index/dex/Iterator.cpp @@ -23,7 +23,7 @@ /// tree) and is simply a wrapper around PostingList::const_iterator. class DocumentIterator : public Iterator { public: - DocumentIterator(PostingListRef Documents) + explicit DocumentIterator(PostingListRef Documents) : Documents(Documents), Index(std::begin(Documents)) {} bool reachedEnd() const override { return Index == std::end(Documents); } @@ -85,7 +85,7 @@ /// iterator restores the invariant: all children must point to the same item. class AndIterator : public Iterator { public: - AndIterator(std::vector> AllChildren) + explicit AndIterator(std::vector> AllChildren) : Children(std::move(AllChildren)) { assert(!Children.empty() && "AND iterator should have at least one child."); // Establish invariants. @@ -193,7 +193,7 @@ /// soon as all of its children are exhausted. class OrIterator : public Iterator { public: - OrIterator(std::vector> AllChildren) + explicit OrIterator(std::vector> AllChildren) : Children(std::move(AllChildren)) { assert(Children.size() > 0 && "OR iterator must have at least one child."); } @@ -279,7 +279,7 @@ /// in O(1). class TrueIterator : public Iterator { public: - TrueIterator(DocID Size) : Size(Size) {} + explicit TrueIterator(DocID Size) : Size(Size) {} bool reachedEnd() const override { return Index >= Size; } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51691: [clangd] NFC: Document URIDistance
kbobyrev created this revision. kbobyrev added reviewers: sammccall, ioeric, ilya-biryukov. kbobyrev added a project: clang-tools-extra. Herald added subscribers: kadircet, arphaman, jkorous, MaskRay. `URIDistance` constructor should mention that `Sources` must contain *absolute paths*, not URIs. This is not very clear when looking at the interface, especially given that `distance(...)` accepts `URI`, not an absolute path which can give the wrong impression. https://reviews.llvm.org/D51691 Files: clang-tools-extra/clangd/FileDistance.h Index: clang-tools-extra/clangd/FileDistance.h === --- clang-tools-extra/clangd/FileDistance.h +++ clang-tools-extra/clangd/FileDistance.h @@ -63,7 +63,7 @@ }; // Supports lookups to find the minimum distance to a file from any source. -// This object should be reused, it memoizes intermediate computations. +// This object should be reused, it memorizes intermediate computations. class FileDistance { public: static constexpr unsigned Unreachable = std::numeric_limits::max(); @@ -86,6 +86,8 @@ // comparison on the bodies. class URIDistance { public: + // Memorizes paths from \p Sources and builds efficient structure for URI + // distance computations. \p Sources must contain absolute paths, not URIs. URIDistance(llvm::StringMap Sources, const FileDistanceOptions &Opts = {}) : Sources(Sources), Opts(Opts) {} Index: clang-tools-extra/clangd/FileDistance.h === --- clang-tools-extra/clangd/FileDistance.h +++ clang-tools-extra/clangd/FileDistance.h @@ -63,7 +63,7 @@ }; // Supports lookups to find the minimum distance to a file from any source. -// This object should be reused, it memoizes intermediate computations. +// This object should be reused, it memorizes intermediate computations. class FileDistance { public: static constexpr unsigned Unreachable = std::numeric_limits::max(); @@ -86,6 +86,8 @@ // comparison on the bodies. class URIDistance { public: + // Memorizes paths from \p Sources and builds efficient structure for URI + // distance computations. \p Sources must contain absolute paths, not URIs. URIDistance(llvm::StringMap Sources, const FileDistanceOptions &Opts = {}) : Sources(Sources), Opts(Opts) {} ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51564: Distinguish `__block` variables that are captured by escaping blocks from those that aren't
ahatanak added inline comments. Comment at: include/clang/AST/Decl.h:977 + +unsigned EscapingByref : 1; }; rjmccall wrote: > This doesn't actually seem to be initialized anywhere. VarDecl's constructor in Decl.cpp initializes it to false. ``` AllBits = 0; VarDeclBits.SClass = SC; // Everything else is implicitly initialized to false. ``` Comment at: lib/CodeGen/CGBlocks.cpp:497 + return VD->isNonEscapingByref() ? + CGF.getContext().getLValueReferenceType(VD->getType()) : VD->getType(); } rjmccall wrote: > Do you need to worry about the variable already having a reference type? Yes, I found out that the previous patch didn't handle `__block` variables with reference types correctly and caused assertion failures. To fix this, I made changes in computeBlockInfo so that the `__block` qualifier is ignored if the variable already has a reference type (I'm treating `__block T&` as `T&` ). There are no changes to this function. Comment at: lib/CodeGen/CGBlocks.cpp:1310 - if (capture.fieldType()->isReferenceType()) + if (capture.fieldType()->isReferenceType() || variable->isNonEscapingByref()) addr = EmitLoadOfReference(MakeAddrLValue(addr, capture.fieldType())); rjmccall wrote: > Isn't the field type already a reference type in this case? > > You should leave a comment that you're relying on that, of course. I added an assertion that checks non-escaping variable fields have reference types. Comment at: lib/Sema/Sema.cpp:1451 + // Call this function before popping the current function scope. + markEscapingByrefs(*FunctionScopes.back(), *this); + rjmccall wrote: > Why before? I remember I had to move the call to markEscapingByrefs to this place because it was making clang crash. It crashed when markEscapingByrefs triggered a call to PushFunctionScope, which caused clearing out PreallocatedFunctionScope, but I can't reproduce the crash anymore. I don't think markEscapingByrefs can cause PushFunctionScope to be called, so I'm moving the call back to the original location. Repository: rC Clang https://reviews.llvm.org/D51564 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51564: Distinguish `__block` variables that are captured by escaping blocks from those that aren't
ahatanak updated this revision to Diff 164069. ahatanak marked 11 inline comments as done. ahatanak added a comment. Address review comments. Fix bugs in the handling of `__block` variables that have reference types. Repository: rC Clang https://reviews.llvm.org/D51564 Files: include/clang/AST/Decl.h include/clang/Sema/ScopeInfo.h lib/AST/Decl.cpp lib/CodeGen/CGBlocks.cpp lib/CodeGen/CGClass.cpp lib/CodeGen/CGDecl.cpp lib/CodeGen/CGExpr.cpp lib/CodeGen/CodeGenFunction.h lib/Sema/ScopeInfo.cpp lib/Sema/Sema.cpp lib/Sema/SemaDecl.cpp lib/Sema/SemaExpr.cpp lib/Serialization/ASTReaderDecl.cpp lib/Serialization/ASTWriterDecl.cpp test/CodeGen/block-byref-aggr.c test/CodeGen/blocks-seq.c test/CodeGen/exceptions.c test/CodeGen/personality.c test/CodeGenCXX/block-capture.cpp test/CodeGenCXX/blocks.cpp test/CodeGenCXX/debug-info-blocks.cpp test/CodeGenCXX/noescape.cpp test/CodeGenObjC/arc-no-arc-exceptions.m test/CodeGenObjC/arc-unoptimized-byref-var.m test/CodeGenObjC/blocks-1.m test/CodeGenObjC/noescape.m test/CodeGenObjCXX/arc-blocks.mm test/PCH/block-helpers.cpp test/SemaObjCXX/blocks.mm test/SemaObjCXX/noescape.mm Index: test/SemaObjCXX/noescape.mm === --- test/SemaObjCXX/noescape.mm +++ test/SemaObjCXX/noescape.mm @@ -8,6 +8,7 @@ void m(); }; +void escapingFunc0(BlockTy); void noescapeFunc0(id, __attribute__((noescape)) BlockTy); void noescapeFunc1(id, [[clang::noescape]] BlockTy); void noescapeFunc2(__attribute__((noescape)) int *); // expected-note {{previous declaration is here}} @@ -127,3 +128,27 @@ -(void) m1:(int*) p { } @end + +struct S6 { + S6(); + S6(const S6 &) = delete; // expected-note 3 {{'S6' has been explicitly marked deleted here}} + int f; +}; + +void test1() { + id a; + // __block variables that are not captured by escaping blocks don't + // necessitate having accessible copy constructors. + __block S6 b0; + __block S6 b1; // expected-error {{call to deleted constructor of 'S6'}} + __block S6 b2; // expected-error {{call to deleted constructor of 'S6'}} + __block S6 b3; // expected-error {{call to deleted constructor of 'S6'}} + + noescapeFunc0(a, ^{ (void)b0; }); + escapingFunc0(^{ (void)b1; }); + { +noescapeFunc0(a, ^{ (void)b0; (void)b1; }); + } + noescapeFunc0(a, ^{ escapingFunc0(^{ (void)b2; }); }); + escapingFunc0(^{ noescapeFunc0(a, ^{ (void)b3; }); }); +} Index: test/SemaObjCXX/blocks.mm === --- test/SemaObjCXX/blocks.mm +++ test/SemaObjCXX/blocks.mm @@ -76,21 +76,27 @@ } // Make sure we successfully instantiate the copy constructor of a -// __block variable's type. +// __block variable's type when the variable is captured by an escaping block. namespace N2 { template struct A { A() {} A(const A &other) { int invalid[-n]; // expected-error 2 {{array with a negative size}} } +void m() {} }; + typedef void (^BlockFnTy)(); + void func(BlockFnTy); + void test1() { __block A<1> x; // expected-note {{requested here}} +func(^{ x.m(); }); } template void test2() { __block A x; // expected-note {{requested here}} +func(^{ x.m(); }); } template void test2<2>(); } Index: test/PCH/block-helpers.cpp === --- test/PCH/block-helpers.cpp +++ test/PCH/block-helpers.cpp @@ -1,6 +1,26 @@ // RUN: %clang_cc1 -x c++-header -triple x86_64-apple-darwin11 -emit-pch -fblocks -fexceptions -o %t %S/block-helpers.h // RUN: %clang_cc1 -triple x86_64-apple-darwin11 -include-pch %t -emit-llvm -fblocks -fexceptions -o - %s | FileCheck %s +// CHECK: %[[STRUCT_BLOCK_BYREF_X:.*]] = type { i8*, %[[STRUCT_BLOCK_BYREF_X]]*, i32, i32, i8*, i8*, %[[STRUCT_S0:.*]] } +// CHECK: %[[STRUCT_S0]] = type { i32 } +// CHECK: %[[STRUCT_BLOCK_BYREF_Y:.*]] = type { i8*, %[[STRUCT_BLOCK_BYREF_Y]]*, i32, i32, i8*, i8*, %[[STRUCT_S0]] } +// CHECK: %[[STRUCT_BLOCK_DESCRIPTOR:.*]] = type { i64, i64 } + +// Check that byref structs are allocated for x and y. + +// CHECK-LABEL: define linkonce_odr void @_ZN1S1mEv( +// CHECK: %[[X:.*]] = alloca %[[STRUCT_BLOCK_BYREF_X]], align 8 +// CHECK: %[[Y:.*]] = alloca %[[STRUCT_BLOCK_BYREF_Y]], align 8 +// CHECK: %[[BLOCK:.*]] = alloca <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8*, i8* }>, align 8 +// CHECK: %[[BLOCK_CAPTURED:.*]] = getelementptr inbounds <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8*, i8* }>, <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8*, i8* }>* %[[BLOCK]], i32 0, i32 5 +// CHECK: %[[V0:.*]] = bitcast %[[STRUCT_BLOCK_BYREF_X]]* %[[X]] to i8* +// CHECK: store i8* %[[V0]], i8** %[[BLOCK_CAPTURED]], align 8 +// CHECK: %[[BLOCK_CAPTURED10:.*]] = getelementptr inbounds <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8*, i8* }>, <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*,