r340251 - Removed unused variable [NFC]
Author: uabelho Date: Tue Aug 21 00:22:45 2018 New Revision: 340251 URL: http://llvm.org/viewvc/llvm-project?rev=340251&view=rev Log: Removed unused variable [NFC] The compiler warned: ../tools/clang/lib/Sema/SemaType.cpp:6788:31: error: unused variable 'AT' [-Werror,-Wunused-variable] if (const AttributedType *AT = S.getCallingConvAttributedType(type)) { ^ 1 error generated. 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=340251&r1=340250&r2=340251&view=diff == --- cfe/trunk/lib/Sema/SemaType.cpp (original) +++ cfe/trunk/lib/Sema/SemaType.cpp Tue Aug 21 00:22:45 2018 @@ -6785,7 +6785,7 @@ static bool handleFunctionTypeAttr(TypeP if (CCOld != CC) { // Error out on when there's already an attribute on the type // and the CCs don't match. -if (const AttributedType *AT = S.getCallingConvAttributedType(type)) { +if (S.getCallingConvAttributedType(type)) { S.Diag(attr.getLoc(), diag::err_attributes_are_not_compatible) << FunctionType::getNameForCallConv(CC) << FunctionType::getNameForCallConv(CCOld); ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50962: [clangd] Speculative code completion index request before Sema is run.
ioeric updated this revision to Diff 161651. ioeric marked an inline comment as done. ioeric added a comment. - Improve tests. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50962 Files: clangd/ClangdServer.cpp clangd/ClangdServer.h clangd/CodeComplete.cpp clangd/CodeComplete.h clangd/index/Index.cpp clangd/index/Index.h clangd/tool/ClangdMain.cpp unittests/clangd/CodeCompleteTests.cpp unittests/clangd/IndexTests.cpp Index: unittests/clangd/IndexTests.cpp === --- unittests/clangd/IndexTests.cpp +++ unittests/clangd/IndexTests.cpp @@ -321,6 +321,21 @@ EXPECT_EQ(M.Name, "right"); } +TEST(AsyncFuzzyFind, Simple) { + MemIndex Idx; + Idx.build(generateSymbols({"ns::abc", "ns::xyz"})); + + FuzzyFindRequest Req; + Req.Query = ""; + Req.Scopes = {"ns::"}; + + AsyncFuzzyFind Async(Idx, Req); + + EXPECT_EQ(Async.getRequest(), Req); + EXPECT_THAT(Async.getResult(), + UnorderedElementsAre(Named("abc"), Named("xyz"))); +} + } // namespace } // namespace clangd } // namespace clang Index: unittests/clangd/CodeCompleteTests.cpp === --- unittests/clangd/CodeCompleteTests.cpp +++ unittests/clangd/CodeCompleteTests.cpp @@ -923,7 +923,11 @@ llvm::function_ref Callback) const override {} - const std::vector allRequests() const { return Requests; } + const std::vector consumeRequests() const { +auto Reqs = std::move(Requests); +Requests.clear(); +return Reqs; + } private: mutable std::vector Requests; @@ -934,7 +938,7 @@ IndexRequestCollector Requests; Opts.Index = &Requests; completions(Code, {}, Opts); - return Requests.allRequests(); + return Requests.consumeRequests(); } TEST(CompletionTest, UnqualifiedIdQuery) { @@ -1700,6 +1704,75 @@ } } +TEST(SpeculateCompletionFilter, Filters) { + Annotations F(R"cpp($bof^ + $bol^ + ab$ab^ + x.ab$dot^ + x.$dotempty^ + x::ab$scoped^ + x::$scopedempty^ + + )cpp"); + auto speculate = [&](StringRef PointName) { +auto Filter = speculateCompletionFilter(F.code(), F.point(PointName)); +assert(Filter); +return *Filter; + }; + EXPECT_EQ(speculate("bof"), ""); + EXPECT_EQ(speculate("bol"), ""); + EXPECT_EQ(speculate("ab"), "ab"); + EXPECT_EQ(speculate("dot"), "ab"); + EXPECT_EQ(speculate("dotempty"), ""); + EXPECT_EQ(speculate("scoped"), "ab"); + EXPECT_EQ(speculate("scopedempty"), ""); +} + +TEST(CompletionTest, EnableSpeculativeIndexRequest) { + MockFSProvider FS; + MockCompilationDatabase CDB; + IgnoreDiagnostics DiagConsumer; + ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest()); + + auto File = testPath("foo.cpp"); + Annotations Test(R"cpp( + namespace ns1 { int abc; } + namespace ns2 { int abc; } + void f() { ns1::ab$1^; ns1::ab$2^; } + void f() { ns2::ab$3^; } + )cpp"); + runAddDocument(Server, File, Test.code()); + clangd::CodeCompleteOptions Opts = {}; + + IndexRequestCollector Requests; + Opts.Index = &Requests; + Opts.SpeculativeIndexRequest = true; + + auto CompleteAtPoint = [&](StringRef P) { +cantFail(runCodeComplete(Server, File, Test.point(P), Opts)); +// Sleep for a while to make sure asynchronous call (if applicable) is also +// triggered before callback is invoked. +std::this_thread::sleep_for(std::chrono::milliseconds(100)); + }; + + CompleteAtPoint("1"); + auto Reqs1 = Requests.consumeRequests(); + ASSERT_EQ(Reqs1.size(), 1u); + EXPECT_THAT(Reqs1[0].Scopes, UnorderedElementsAre("ns1::")); + + CompleteAtPoint("2"); + auto Reqs2 = Requests.consumeRequests(); + // Speculation succeeded. Used speculative index result. + ASSERT_EQ(Reqs2.size(), 1u); + EXPECT_EQ(Reqs2[0], Reqs1[0]); + + CompleteAtPoint("3"); + // Speculation failed. Sent speculative index request and the new index + // request after sema. + auto Reqs3 = Requests.consumeRequests(); + ASSERT_EQ(Reqs3.size(), 2u); +} + } // namespace } // namespace clangd } // namespace clang Index: clangd/tool/ClangdMain.cpp === --- clangd/tool/ClangdMain.cpp +++ clangd/tool/ClangdMain.cpp @@ -164,6 +164,20 @@ "an include line will be inserted or not."), llvm::cl::init(true)); +static llvm::cl::opt SpeculateCompletionIndexRequest( +"speculative-completion-index-request", +llvm::cl::desc( +R"(If set to true and static index is enabled, this will send an +asynchronous speculative index request, based on the index request for +the last code completion on the same file and the filter text typed +before the cursor, before sema code completion is invoked. This can +reduce the code completion latency (by roughly latency of sema code +completion) if the speculative reque
[PATCH] D50962: [clangd] Speculative code completion index request before Sema is run.
ioeric added inline comments. Comment at: unittests/clangd/CodeCompleteTests.cpp:1710 + $bol^ + ab$ab^ + x.ab$dot^ kadircet wrote: > Maybe an EXPECT for this one as well? Nice catch! Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50962 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51026: [CodeGen] Implicitly set stackrealign on the main function, if custom stack alignment is used
mstorsjo created this revision. mstorsjo added reviewers: craig.topper, erichkeane, rnk. If using a custom stack alignment, one is expected to make sure that all callers provide such alignment, or realign the stack in all entry points (and callbacks). Despite this, the compiler can assume that the main function will need realignment in these cases, since the startup routines calling the main function most probably won't provide the custom alignment. This matches what GCC does in similar cases; if compiling with -mincoming-stack-boundary=X -mpreferred-stack-boundary=X, GCC normally assumes such alignment on entry to a function, but specifically for the main function still does realignment. Repository: rC Clang https://reviews.llvm.org/D51026 Files: lib/CodeGen/CodeGenFunction.cpp test/CodeGen/stackrealign-main.c Index: test/CodeGen/stackrealign-main.c === --- /dev/null +++ test/CodeGen/stackrealign-main.c @@ -0,0 +1,19 @@ +// RUN: %clang_cc1 -triple i386-unknown-unknown -emit-llvm -o - -mstack-alignment=64 %s | FileCheck %s + +// CHECK-LABEL: define void @other() +// CHECK: [[OTHER:#[0-9]+]] +// CHECK: { +void other(void) {} + +// CHECK-LABEL: define i32 @main( +// CHECK: [[MAIN:#[0-9]+]] +// CHECK: { +int main(int argc, char **argv) { + other(); + return 0; +} + +// CHECK: attributes [[OTHER]] = { noinline nounwind optnone +// CHECK-NOT: "stackrealign" +// CHECK: } +// CHECK: attributes [[MAIN]] = { noinline nounwind optnone {{.*}}"stackrealign"{{.*}} } Index: lib/CodeGen/CodeGenFunction.cpp === --- lib/CodeGen/CodeGenFunction.cpp +++ lib/CodeGen/CodeGenFunction.cpp @@ -981,6 +981,13 @@ if (FD->isMain()) Fn->addFnAttr(llvm::Attribute::NoRecurse); + // If a custom alignment is used, force realigning to this alignment on + // any main function which certainly will need it. + if (const FunctionDecl *FD = dyn_cast_or_null(D)) +if ((FD->isMain() || FD->isMSVCRTEntryPoint()) && +CGM.getCodeGenOpts().StackAlignment) + Fn->addFnAttr("stackrealign"); + llvm::BasicBlock *EntryBB = createBasicBlock("entry", CurFn); // Create a marker to make it easy to insert allocas into the entryblock Index: test/CodeGen/stackrealign-main.c === --- /dev/null +++ test/CodeGen/stackrealign-main.c @@ -0,0 +1,19 @@ +// RUN: %clang_cc1 -triple i386-unknown-unknown -emit-llvm -o - -mstack-alignment=64 %s | FileCheck %s + +// CHECK-LABEL: define void @other() +// CHECK: [[OTHER:#[0-9]+]] +// CHECK: { +void other(void) {} + +// CHECK-LABEL: define i32 @main( +// CHECK: [[MAIN:#[0-9]+]] +// CHECK: { +int main(int argc, char **argv) { + other(); + return 0; +} + +// CHECK: attributes [[OTHER]] = { noinline nounwind optnone +// CHECK-NOT: "stackrealign" +// CHECK: } +// CHECK: attributes [[MAIN]] = { noinline nounwind optnone {{.*}}"stackrealign"{{.*}} } Index: lib/CodeGen/CodeGenFunction.cpp === --- lib/CodeGen/CodeGenFunction.cpp +++ lib/CodeGen/CodeGenFunction.cpp @@ -981,6 +981,13 @@ if (FD->isMain()) Fn->addFnAttr(llvm::Attribute::NoRecurse); + // If a custom alignment is used, force realigning to this alignment on + // any main function which certainly will need it. + if (const FunctionDecl *FD = dyn_cast_or_null(D)) +if ((FD->isMain() || FD->isMSVCRTEntryPoint()) && +CGM.getCodeGenOpts().StackAlignment) + Fn->addFnAttr("stackrealign"); + llvm::BasicBlock *EntryBB = createBasicBlock("entry", CurFn); // Create a marker to make it easy to insert allocas into the entryblock ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50926: [SourceManager] Extract 'findFileIDsForFile' from 'translateFile' to allow mapping from one file entry to multiple FileIDs
ioeric added inline comments. Comment at: lib/Basic/SourceManager.cpp:1705 // If we haven't found what we want yet, try again, but this time stat() // each of the files in case the files have changed since we originally Do we also need this in `findFileIDsForFile`? Comment at: unittests/Basic/SourceManagerTest.cpp:380 +TEST_F(SourceManagerTest, findFileIDsForFile) { + const char *header = "int x;"; Could you add a test case for getting file ID for main file, just to make sure we also covered cases handled by `if (MainFileID.isValid()) {...}` code path in `translateFile()`? Repository: rC Clang https://reviews.llvm.org/D50926 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50580: [clang-tidy] Abseil: no namespace check
hokein added inline comments. Comment at: clang-tidy/abseil/AbseilMatcher.h:16 + +/// Matches AST nodes that were expanded within abseil-header-files. +AST_POLYMORPHIC_MATCHER(isExpansionNotInAbseilHeader, nit: We need proper documentation for this matcher, since it is exposed to users. Comment at: clang-tidy/abseil/AbseilMatcher.h:17 +/// Matches AST nodes that were expanded within abseil-header-files. +AST_POLYMORPHIC_MATCHER(isExpansionNotInAbseilHeader, +AST_POLYMORPHIC_SUPPORTED_TYPES(Decl, Stmt, TypeLoc, - using negative for AST matcher seems uncommon in ASTMatchers, for negative we have `unless`, so I'd suggest implementing `isInAbseilHeader`. - it seems that this matcher is very similar to the matcher in https://reviews.llvm.org/D50542. I think `isInAbseilFile` should also cover your case here, and also ignore the warning if you run the check on abseil core files. Comment at: clang-tidy/abseil/AbseilMatcher.h:32 + auto Filename = FileEntry->getName(); + llvm::Regex RE("absl/(base|container|debugging|memory|meta|numeric|strings|" + "synchronization|types|utiliy)"); The regex seems incomplete, we are missing `algorithm`, `time` subdirectory. Comment at: clang-tidy/abseil/AbseilMatcher.h:33 + llvm::Regex RE("absl/(base|container|debugging|memory|meta|numeric|strings|" + "synchronization|types|utiliy)"); + return (!RE.match(Filename)); typo: `utility` Comment at: clang-tidy/abseil/AbseilMatcher.h:34 + "synchronization|types|utiliy)"); + return (!RE.match(Filename)); +} nit: remove the outer `()`. https://reviews.llvm.org/D50580 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50502: [clangd] Initial cancellation mechanism for LSP requests.
ilya-biryukov added inline comments. Comment at: clangd/Cancellation.h:96 +/// checks using it to avoid extra lookups in the Context. +class CancellationToken { +public: ioeric wrote: > As chatted offline, I have questions about the motivation of the > `CancellationToken` class. Intuitively, it seems to me that this can be > merged into `TaskHandle`, and we can simply stash the `TaskHandle` instead of > a token into the context. There would be fewer states to maintain, and the > design would be a bit more straightforward. I might be missing > context/concerns here, so I'd like to hear what you and Ilya think. > @ilya-biryukov I am for splitting cancellation checks from cancellation triggers. The processing code only needs to check if it was cancelled and exposing the `cancel()` does not add any useful functionality, merely ways to misuse it. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50502 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50502: [clangd] Initial cancellation mechanism for LSP requests.
ioeric added inline comments. Comment at: clangd/Cancellation.h:96 +/// checks using it to avoid extra lookups in the Context. +class CancellationToken { +public: ilya-biryukov wrote: > ioeric wrote: > > As chatted offline, I have questions about the motivation of the > > `CancellationToken` class. Intuitively, it seems to me that this can be > > merged into `TaskHandle`, and we can simply stash the `TaskHandle` instead > > of a token into the context. There would be fewer states to maintain, and > > the design would be a bit more straightforward. I might be missing > > context/concerns here, so I'd like to hear what you and Ilya think. > > @ilya-biryukov > I am for splitting cancellation checks from cancellation triggers. > The processing code only needs to check if it was cancelled and exposing the > `cancel()` does not add any useful functionality, merely ways to misuse it. Couldn't we prevent this by passing only const handle references to users who are not expect to call `cancel()`? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50502 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50580: [clang-tidy] Abseil: no namespace check
lebedev.ri added inline comments. Comment at: clang-tidy/abseil/AbseilMatcher.h:32 + auto Filename = FileEntry->getName(); + llvm::Regex RE("absl/(base|container|debugging|memory|meta|numeric|strings|" + "synchronization|types|utiliy)"); hokein wrote: > The regex seems incomplete, we are missing `algorithm`, `time` subdirectory. So what happens if open the namespace in a file that is located in my personal `absl/base` directory? It will be false-negatively not reported? https://reviews.llvm.org/D50580 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50862: [clang-tidy] Abseil: faster strsplit delimiter check
hokein accepted this revision. hokein added a comment. This revision is now accepted and ready to land. The patch looks good. I'll commit for you once @JonasToth has no further comments. https://reviews.llvm.org/D50862 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50580: [clang-tidy] Abseil: no namespace check
hokein added inline comments. Comment at: clang-tidy/abseil/AbseilMatcher.h:32 + auto Filename = FileEntry->getName(); + llvm::Regex RE("absl/(base|container|debugging|memory|meta|numeric|strings|" + "synchronization|types|utiliy)"); lebedev.ri wrote: > hokein wrote: > > The regex seems incomplete, we are missing `algorithm`, `time` > > subdirectory. > So what happens if open the namespace in a file that is located in my > personal `absl/base` directory? > It will be false-negatively not reported? Yes, I'd say this is a known limitation. https://reviews.llvm.org/D50580 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check
hokein added a subscriber: deannagarcia. hokein added a comment. Your patch seems have some comments unaddressed but you marked done. About the abseil matcher stuff, you and @deannagarcia have an overlap, maybe just wait for https://reviews.llvm.org/D50580 to submit, and reuse the matcher in this patch? Comment at: clang-tidy/abseil/NoInternalDepsCheck.cpp:24 + auto &SourceManager = Finder->getASTContext().getSourceManager(); + SourceLocation loc = Node.getBeginLoc(); + if (loc.isInvalid()) nit: loc => Loc https://reviews.llvm.org/D50542 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50580: [clang-tidy] Abseil: no namespace check
lebedev.ri added inline comments. Comment at: clang-tidy/abseil/AbseilMatcher.h:32 + auto Filename = FileEntry->getName(); + llvm::Regex RE("absl/(base|container|debugging|memory|meta|numeric|strings|" + "synchronization|types|utiliy)"); hokein wrote: > lebedev.ri wrote: > > hokein wrote: > > > The regex seems incomplete, we are missing `algorithm`, `time` > > > subdirectory. > > So what happens if open the namespace in a file that is located in my > > personal `absl/base` directory? > > It will be false-negatively not reported? > Yes, I'd say this is a known limitation. Similarly, the check will break (start producing false-positives) as soon as a new directory is added to abseil proper. I don't have any reliable idea on how to do this better, but the current solution seems rather suboptimal.. https://reviews.llvm.org/D50580 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50814: [clangd] transfer the fixits with the notes instead of adding them to the main diagnostic if request by the client
ilya-biryukov added a comment. Herald added a subscriber: kadircet. Just to make sure I fully understand the use-case: could you elaborate a little more? Do you need to get exactly the same set of notes that clang provides? Our current model seems to follow the clang's model, but it removes some notes and shows them as fix-its for the main diagnostic instead. (We think it should provide a better user experience, because those notes do look like a way to present a fix-it to the user when you have only text-based output and they seem redundant when one has an editor-based UI with fix-its). Not opposed to changing the model to meet other needs, but want to make sure what we're currently doing in clangd makes sense and understand your use-case better. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50814 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50703: [clangd] NFC: Mark Workspace Symbol feature complete in the documentation
ilya-biryukov added a comment. In https://reviews.llvm.org/D50703#1205167, @malaperle wrote: > Nah, I guess other things are partial too, like Go to Definition. Let's > assume they will be completed "soon" ;) https://reviews.llvm.org/D50889 should finally add the symbols from the main files. Sorry for the delays. Repository: rL LLVM https://reviews.llvm.org/D50703 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51029: [clangd] Implement LIMIT iterator
kbobyrev created this revision. kbobyrev added reviewers: ioeric, ilya-biryukov. kbobyrev added a project: clang-tools-extra. Herald added subscribers: kadircet, arphaman, jkorous, MaskRay. https://reviews.llvm.org/D51029 Files: clang-tools-extra/clangd/index/dex/DexIndex.cpp clang-tools-extra/clangd/index/dex/Iterator.cpp clang-tools-extra/clangd/index/dex/Iterator.h 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 @@ -62,7 +62,7 @@ auto AndWithEmpty = createAnd(create(L0), create(L1)); EXPECT_TRUE(AndWithEmpty->reachedEnd()); - EXPECT_THAT(consume(*AndWithEmpty), ElementsAre()); + EXPECT_THAT(consume(move(AndWithEmpty)), ElementsAre()); } TEST(DexIndexIterators, AndTwoLists) { @@ -72,7 +72,7 @@ auto And = createAnd(create(L1), create(L0)); EXPECT_FALSE(And->reachedEnd()); - EXPECT_THAT(consume(*And), ElementsAre(0U, 7U, 10U, 320U, 9000U)); + EXPECT_THAT(consume(move(And)), ElementsAre(0U, 7U, 10U, 320U, 9000U)); And = createAnd(create(L0), create(L1)); @@ -113,7 +113,7 @@ auto OrWithEmpty = createOr(create(L0), create(L1)); EXPECT_FALSE(OrWithEmpty->reachedEnd()); - EXPECT_THAT(consume(*OrWithEmpty), + EXPECT_THAT(consume(move(OrWithEmpty)), ElementsAre(0U, 5U, 7U, 10U, 42U, 320U, 9000U)); } @@ -146,7 +146,7 @@ Or = createOr(create(L0), create(L1)); - EXPECT_THAT(consume(*Or), + EXPECT_THAT(consume(move(Or)), ElementsAre(0U, 4U, 5U, 7U, 10U, 30U, 42U, 60U, 320U, 9000U)); } @@ -256,29 +256,29 @@ const PostingList L5; auto DocIterator = create(L0); - EXPECT_THAT(consume(*DocIterator, 42), ElementsAre(4, 7, 8, 20, 42, 100)); + EXPECT_THAT(consume(move(DocIterator), 42), ElementsAre(4, 7, 8, 20, 42, 100)); DocIterator = create(L0); - EXPECT_THAT(consume(*DocIterator), ElementsAre(4, 7, 8, 20, 42, 100)); + EXPECT_THAT(consume(move(DocIterator)), ElementsAre(4, 7, 8, 20, 42, 100)); DocIterator = create(L0); - EXPECT_THAT(consume(*DocIterator, 3), ElementsAre(4, 7, 8)); + EXPECT_THAT(consume(move(DocIterator), 3), ElementsAre(4, 7, 8)); DocIterator = create(L0); - EXPECT_THAT(consume(*DocIterator, 0), ElementsAre()); + EXPECT_THAT(consume(move(DocIterator), 0), ElementsAre()); } TEST(DexIndexIterators, True) { auto TrueIterator = createTrue(0U); EXPECT_TRUE(TrueIterator->reachedEnd()); - EXPECT_THAT(consume(*TrueIterator), ElementsAre()); + EXPECT_THAT(consume(move(TrueIterator)), ElementsAre()); PostingList L0 = {1, 2, 5, 7}; TrueIterator = createTrue(7U); EXPECT_THAT(TrueIterator->peek(), 0); auto AndIterator = createAnd(create(L0), move(TrueIterator)); EXPECT_FALSE(AndIterator->reachedEnd()); - EXPECT_THAT(consume(*AndIterator), ElementsAre(1, 2, 5)); + EXPECT_THAT(consume(move(AndIterator)), ElementsAre(1, 2, 5)); } testing::Matcher> Index: clang-tools-extra/clangd/index/dex/Iterator.h === --- clang-tools-extra/clangd/index/dex/Iterator.h +++ clang-tools-extra/clangd/index/dex/Iterator.h @@ -89,6 +89,13 @@ /// /// Note: reachedEnd() must be false. virtual DocID peek() const = 0; + /// Marks an element as "consumed" and triggers limit decrement for every + /// LIMIT iterator which yields given item. Root iterator should call + /// consume(peek) as the argument is used to propagate the tree, otherwise + /// the behavior will be incorrect. + virtual void consume(DocID ID) = 0; + /// Returns true if the iterator can retrieve more items upon request. + virtual bool canRetrieveMore() const { return !reachedEnd(); } virtual ~Iterator() {} @@ -113,7 +120,7 @@ /// Advances the iterator until it is either exhausted or the number of /// requested items is reached. The result contains sorted DocumentIDs. -std::vector consume(Iterator &It, +std::vector consume(std::unique_ptr It, size_t Limit = std::numeric_limits::max()); /// Returns a document iterator over given PostingList. @@ -133,6 +140,11 @@ /// all items in range [0, Size) in an efficient manner. std::unique_ptr createTrue(DocID Size); +/// Returns LIMIT iterator, which ... +// FIXME(kbobyrev): Add docs. +std::unique_ptr +createLimit(std::unique_ptr Child, DocID Size); + /// This allows createAnd(create(...), create(...)) syntax. template std::unique_ptr createAnd(Args... args) { std::vector> Children; 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 @@ -46,6 +46,8 @@ return *Index; } + void consume (DocID ID) override {} + private: llvm::raw
[PATCH] D51029: [clangd] Implement LIMIT iterator
kbobyrev planned changes to this revision. kbobyrev added a comment. Patch is in the preview mode, I will add documentation and more tests before opening a review. https://reviews.llvm.org/D51029 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51029: [clangd] Implement LIMIT iterator
kbobyrev updated this revision to Diff 161677. kbobyrev added a comment. Add comprehensive tests, improve documentation. https://reviews.llvm.org/D51029 Files: clang-tools-extra/clangd/index/dex/DexIndex.cpp clang-tools-extra/clangd/index/dex/Iterator.cpp clang-tools-extra/clangd/index/dex/Iterator.h 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 @@ -62,7 +62,7 @@ auto AndWithEmpty = createAnd(create(L0), create(L1)); EXPECT_TRUE(AndWithEmpty->reachedEnd()); - EXPECT_THAT(consume(*AndWithEmpty), ElementsAre()); + EXPECT_THAT(consume(move(AndWithEmpty)), ElementsAre()); } TEST(DexIndexIterators, AndTwoLists) { @@ -72,7 +72,7 @@ auto And = createAnd(create(L1), create(L0)); EXPECT_FALSE(And->reachedEnd()); - EXPECT_THAT(consume(*And), ElementsAre(0U, 7U, 10U, 320U, 9000U)); + EXPECT_THAT(consume(move(And)), ElementsAre(0U, 7U, 10U, 320U, 9000U)); And = createAnd(create(L0), create(L1)); @@ -113,7 +113,7 @@ auto OrWithEmpty = createOr(create(L0), create(L1)); EXPECT_FALSE(OrWithEmpty->reachedEnd()); - EXPECT_THAT(consume(*OrWithEmpty), + EXPECT_THAT(consume(move(OrWithEmpty)), ElementsAre(0U, 5U, 7U, 10U, 42U, 320U, 9000U)); } @@ -146,7 +146,7 @@ Or = createOr(create(L0), create(L1)); - EXPECT_THAT(consume(*Or), + EXPECT_THAT(consume(move(Or)), ElementsAre(0U, 4U, 5U, 7U, 10U, 30U, 42U, 60U, 320U, 9000U)); } @@ -248,37 +248,40 @@ } TEST(DexIndexIterators, Limit) { - const PostingList L0 = {4, 7, 8, 20, 42, 100}; - const PostingList L1 = {1, 3, 5, 8, 9}; - const PostingList L2 = {1, 5, 7, 9}; - const PostingList L3 = {0, 5}; - const PostingList L4 = {0, 1, 5}; - const PostingList L5; + const PostingList L0 = {3, 6, 7, 20, 42, 100}; + const PostingList L1 = {1, 3, 5, 6, 7, 30, 100}; + const PostingList L2 = {0, 3, 5, 7, 8, 100}; auto DocIterator = create(L0); - EXPECT_THAT(consume(*DocIterator, 42), ElementsAre(4, 7, 8, 20, 42, 100)); + EXPECT_THAT(consume(move(DocIterator), 42), + ElementsAre(3, 6, 7, 20, 42, 100)); DocIterator = create(L0); - EXPECT_THAT(consume(*DocIterator), ElementsAre(4, 7, 8, 20, 42, 100)); + EXPECT_THAT(consume(move(DocIterator)), ElementsAre(3, 6, 7, 20, 42, 100)); DocIterator = create(L0); - EXPECT_THAT(consume(*DocIterator, 3), ElementsAre(4, 7, 8)); + EXPECT_THAT(consume(move(DocIterator), 3), ElementsAre(3, 6, 7)); DocIterator = create(L0); - EXPECT_THAT(consume(*DocIterator, 0), ElementsAre()); + EXPECT_THAT(consume(move(DocIterator), 0), ElementsAre()); + + auto AndIterator = + createAnd(createLimit(createTrue(9000), 343), createLimit(create(L0), 2), +createLimit(create(L1), 3), createLimit(create(L2), 42)); + EXPECT_THAT(consume(move(AndIterator)), ElementsAre(3, 7)); } TEST(DexIndexIterators, True) { auto TrueIterator = createTrue(0U); EXPECT_TRUE(TrueIterator->reachedEnd()); - EXPECT_THAT(consume(*TrueIterator), ElementsAre()); + EXPECT_THAT(consume(move(TrueIterator)), ElementsAre()); PostingList L0 = {1, 2, 5, 7}; TrueIterator = createTrue(7U); EXPECT_THAT(TrueIterator->peek(), 0); auto AndIterator = createAnd(create(L0), move(TrueIterator)); EXPECT_FALSE(AndIterator->reachedEnd()); - EXPECT_THAT(consume(*AndIterator), ElementsAre(1, 2, 5)); + EXPECT_THAT(consume(move(AndIterator)), ElementsAre(1, 2, 5)); } testing::Matcher> Index: clang-tools-extra/clangd/index/dex/Iterator.h === --- clang-tools-extra/clangd/index/dex/Iterator.h +++ clang-tools-extra/clangd/index/dex/Iterator.h @@ -89,6 +89,13 @@ /// /// Note: reachedEnd() must be false. virtual DocID peek() const = 0; + /// Marks an element as "consumed" and triggers limit decrement for every + /// LIMIT iterator which yields given item. Root iterator should call + /// consume(peek) as the argument is used to propagate the tree, otherwise + /// the behavior will be incorrect. + virtual void consume(DocID ID) = 0; + /// Returns true if the iterator can retrieve more items upon request. + virtual bool canRetrieveMore() const { return !reachedEnd(); } virtual ~Iterator() {} @@ -113,7 +120,7 @@ /// Advances the iterator until it is either exhausted or the number of /// requested items is reached. The result contains sorted DocumentIDs. -std::vector consume(Iterator &It, +std::vector consume(std::unique_ptr It, size_t Limit = std::numeric_limits::max()); /// Returns a document iterator over given PostingList. @@ -133,6 +140,11 @@ /// all items in range [0, Size) in an efficient manner. std::unique_ptr createTrue(DocID Size); +/// Returns LIMIT iterator,
[PATCH] D51029: [clangd] Implement LIMIT iterator
kbobyrev planned changes to this revision. kbobyrev added a comment. Since it's bundled with the BOOST iterator and doesn't make too much sense without it, I should probable rebase on top of https://reviews.llvm.org/D50970 and add it as the parent revision. https://reviews.llvm.org/D51029 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50984: AMDGPU: Move target code into TargetParser
rampitec accepted this revision. rampitec added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D50984 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50897: [clangd] Allow using experimental Dex index
ioeric added inline comments. Comment at: clang-tools-extra/clangd/index/MemIndex.h:50 +// FIXME(kbobyrev): Document this one. +std::shared_ptr> Please add documentation. Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:96 +namespace { + Why the move? A chunk of code in the middle of flags seems strange. https://reviews.llvm.org/D50897 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50897: [clangd] Allow using experimental Dex index
kbobyrev updated this revision to Diff 161679. kbobyrev marked 2 inline comments as done. kbobyrev added a comment. Aww, the previous diff was the wrong one and didn't contain docs. The move of the code to the middle of Clangd driver was justified by the assumption that it might be better to move the code that uses `UseDex` flag closer to the flag itself, but I agree: it looks strange. I moved it back. https://reviews.llvm.org/D50897 Files: clang-tools-extra/clangd/index/MemIndex.cpp clang-tools-extra/clangd/index/MemIndex.h clang-tools-extra/clangd/index/dex/DexIndex.cpp clang-tools-extra/clangd/index/dex/DexIndex.h clang-tools-extra/clangd/tool/ClangdMain.cpp Index: clang-tools-extra/clangd/tool/ClangdMain.cpp === --- clang-tools-extra/clangd/tool/ClangdMain.cpp +++ clang-tools-extra/clangd/tool/ClangdMain.cpp @@ -12,6 +12,7 @@ #include "Path.h" #include "Trace.h" #include "index/SymbolYAML.h" +#include "index/dex/DexIndex.h" #include "clang/Basic/Version.h" #include "llvm/Support/CommandLine.h" #include "llvm/Support/FileSystem.h" @@ -29,6 +30,7 @@ using namespace clang::clangd; namespace { + enum class PCHStorageFlag { Disk, Memory }; // Build an in-memory static index for global symbols from a YAML-format file. @@ -45,8 +47,10 @@ for (auto Sym : Slab) SymsBuilder.insert(Sym); - return MemIndex::build(std::move(SymsBuilder).build()); + return UseDex ? DexIndex::build(std::move(SymsBuilder).build()) +: MemIndex::build(std::move(SymsBuilder).build()); } + } // namespace static llvm::cl::opt CompileCommandsDir( @@ -185,6 +189,11 @@ "'compile_commands.json' files")), llvm::cl::init(FilesystemCompileArgs), llvm::cl::Hidden); +static llvm::cl::opt +UseDex("use-dex-index", + llvm::cl::desc("Use experimental Dex static index."), + llvm::cl::init(false), llvm::cl::Hidden); + int main(int argc, char *argv[]) { llvm::sys::PrintStackTraceOnErrorSignal(argv[0]); llvm::cl::SetVersionPrinter([](llvm::raw_ostream &OS) { Index: clang-tools-extra/clangd/index/dex/DexIndex.h === --- clang-tools-extra/clangd/index/dex/DexIndex.h +++ clang-tools-extra/clangd/index/dex/DexIndex.h @@ -43,6 +43,9 @@ /// accessible as long as `Symbols` is kept alive. void build(std::shared_ptr> Symbols); + /// \brief Build index from a symbol slab. + static std::unique_ptr build(SymbolSlab Slab); + bool fuzzyFind(const FuzzyFindRequest &Req, llvm::function_ref Callback) const override; Index: clang-tools-extra/clangd/index/dex/DexIndex.cpp === --- clang-tools-extra/clangd/index/dex/DexIndex.cpp +++ clang-tools-extra/clangd/index/dex/DexIndex.cpp @@ -69,6 +69,12 @@ } } +std::unique_ptr DexIndex::build(SymbolSlab Slab) { + auto Idx = llvm::make_unique(); + Idx->build(getSymbolsFromSlab(std::move(Slab))); + return std::move(Idx); +} + /// Constructs iterators over tokens extracted from the query and exhausts it /// while applying Callback to each symbol in the order of decreasing quality /// of the matched symbols. Index: clang-tools-extra/clangd/index/MemIndex.h === --- clang-tools-extra/clangd/index/MemIndex.h +++ clang-tools-extra/clangd/index/MemIndex.h @@ -47,6 +47,11 @@ mutable std::mutex Mutex; }; +// Returns pointers to the symbols in given slab and bundles slab lifetime with +// returned symbol pointers so that the pointers are never invalid. +std::shared_ptr> +getSymbolsFromSlab(SymbolSlab Slab); + } // namespace clangd } // namespace clang Index: clang-tools-extra/clangd/index/MemIndex.cpp === --- clang-tools-extra/clangd/index/MemIndex.cpp +++ clang-tools-extra/clangd/index/MemIndex.cpp @@ -28,6 +28,12 @@ } } +std::unique_ptr MemIndex::build(SymbolSlab Slab) { + auto Idx = llvm::make_unique(); + Idx->build(getSymbolsFromSlab(std::move(Slab))); + return std::move(Idx); +} + bool MemIndex::fuzzyFind( const FuzzyFindRequest &Req, llvm::function_ref Callback) const { @@ -72,26 +78,24 @@ } } -std::unique_ptr MemIndex::build(SymbolSlab Slab) { +void MemIndex::findOccurrences( +const OccurrencesRequest &Req, +llvm::function_ref Callback) const { + log("findOccurrences is not implemented."); +} + +std::shared_ptr> +getSymbolsFromSlab(SymbolSlab Slab) { struct Snapshot { SymbolSlab Slab; std::vector Pointers; }; auto Snap = std::make_shared(); Snap->Slab = std::move(Slab); for (auto &Sym : Snap->Slab) Snap->Pointers.push_back(&Sym); - auto S = std::shared_ptr>(std::move(Snap), -&Snap->Pointers); - auto MemIdx = llvm::make_uni
[PATCH] D50897: [clangd] Allow using experimental Dex index
ioeric accepted this revision. ioeric added a comment. This revision is now accepted and ready to land. lg https://reviews.llvm.org/D50897 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50502: [clangd] Initial cancellation mechanism for LSP requests.
ilya-biryukov added inline comments. Comment at: clangd/Cancellation.h:96 +/// checks using it to avoid extra lookups in the Context. +class CancellationToken { +public: ioeric wrote: > ilya-biryukov wrote: > > ioeric wrote: > > > As chatted offline, I have questions about the motivation of the > > > `CancellationToken` class. Intuitively, it seems to me that this can be > > > merged into `TaskHandle`, and we can simply stash the `TaskHandle` > > > instead of a token into the context. There would be fewer states to > > > maintain, and the design would be a bit more straightforward. I might be > > > missing context/concerns here, so I'd like to hear what you and Ilya > > > think. @ilya-biryukov > > I am for splitting cancellation checks from cancellation triggers. > > The processing code only needs to check if it was cancelled and exposing > > the `cancel()` does not add any useful functionality, merely ways to misuse > > it. > Couldn't we prevent this by passing only const handle references to users who > are not expect to call `cancel()`? As long as `TaskHandle` does not have a copy constructor, that LG. I.e. the scheme that seems to work is: 1. return `shared_ptr` from async computations like code complete. 2. stash `shared_ptr` into the context. 3. return `const TaskHandle*` from the context. This should give a simpler implementation. The cons from my point of view are: 1. We return copyable task handles (i.e. the shared_ptr) from the API by default. This provokes usages that don't take ownership issues into account, i.e. it's easy to just copy the task everywhere without having clear ownership of it. As long as it's just cancellation there, we're probably fine, but if we add more stuff into it that might become a problem. 2. We expose implementation details (i.e. shared_ptr) by removing the layer of abstraction. This makes potential refactorings of the client code harder. In general, I think having our own wrappers there opens up more room for experiments with the API later (the API is very constrained, so it's easy to refactor the implementation). OTOH, if we believe the design is right or changing the clients is not too much work (which is probably true), either way is fine with me. But either solution has its pros and cons, I'm happy with trying out any of those on code completion, see how it goes and decide whether we want to change anything before finalizing our design and adding cancellation to all operations in clangd. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50502 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50897: [clangd] Allow using experimental Dex index
This revision was automatically updated to reflect the committed changes. Closed by commit rL340262: [clangd] Allow using experimental Dex index (authored by omtcyfz, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D50897?vs=161679&id=161682#toc Repository: rL LLVM https://reviews.llvm.org/D50897 Files: clang-tools-extra/trunk/clangd/index/MemIndex.cpp clang-tools-extra/trunk/clangd/index/MemIndex.h clang-tools-extra/trunk/clangd/index/dex/DexIndex.cpp clang-tools-extra/trunk/clangd/index/dex/DexIndex.h clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp Index: clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp === --- clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp +++ clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp @@ -12,6 +12,7 @@ #include "Path.h" #include "Trace.h" #include "index/SymbolYAML.h" +#include "index/dex/DexIndex.h" #include "clang/Basic/Version.h" #include "llvm/Support/CommandLine.h" #include "llvm/Support/FileSystem.h" @@ -29,6 +30,7 @@ using namespace clang::clangd; namespace { + enum class PCHStorageFlag { Disk, Memory }; // Build an in-memory static index for global symbols from a YAML-format file. @@ -45,8 +47,10 @@ for (auto Sym : Slab) SymsBuilder.insert(Sym); - return MemIndex::build(std::move(SymsBuilder).build()); + return UseDex ? DexIndex::build(std::move(SymsBuilder).build()) +: MemIndex::build(std::move(SymsBuilder).build()); } + } // namespace static llvm::cl::opt CompileCommandsDir( @@ -185,6 +189,11 @@ "'compile_commands.json' files")), llvm::cl::init(FilesystemCompileArgs), llvm::cl::Hidden); +static llvm::cl::opt +UseDex("use-dex-index", + llvm::cl::desc("Use experimental Dex static index."), + llvm::cl::init(false), llvm::cl::Hidden); + int main(int argc, char *argv[]) { llvm::sys::PrintStackTraceOnErrorSignal(argv[0]); llvm::cl::SetVersionPrinter([](llvm::raw_ostream &OS) { Index: clang-tools-extra/trunk/clangd/index/MemIndex.h === --- clang-tools-extra/trunk/clangd/index/MemIndex.h +++ clang-tools-extra/trunk/clangd/index/MemIndex.h @@ -47,6 +47,11 @@ mutable std::mutex Mutex; }; +// Returns pointers to the symbols in given slab and bundles slab lifetime with +// returned symbol pointers so that the pointers are never invalid. +std::shared_ptr> +getSymbolsFromSlab(SymbolSlab Slab); + } // namespace clangd } // namespace clang Index: clang-tools-extra/trunk/clangd/index/MemIndex.cpp === --- clang-tools-extra/trunk/clangd/index/MemIndex.cpp +++ clang-tools-extra/trunk/clangd/index/MemIndex.cpp @@ -28,6 +28,12 @@ } } +std::unique_ptr MemIndex::build(SymbolSlab Slab) { + auto Idx = llvm::make_unique(); + Idx->build(getSymbolsFromSlab(std::move(Slab))); + return std::move(Idx); +} + bool MemIndex::fuzzyFind( const FuzzyFindRequest &Req, llvm::function_ref Callback) const { @@ -72,26 +78,24 @@ } } -std::unique_ptr MemIndex::build(SymbolSlab Slab) { +void MemIndex::findOccurrences( +const OccurrencesRequest &Req, +llvm::function_ref Callback) const { + log("findOccurrences is not implemented."); +} + +std::shared_ptr> +getSymbolsFromSlab(SymbolSlab Slab) { struct Snapshot { SymbolSlab Slab; std::vector Pointers; }; auto Snap = std::make_shared(); Snap->Slab = std::move(Slab); for (auto &Sym : Snap->Slab) Snap->Pointers.push_back(&Sym); - auto S = std::shared_ptr>(std::move(Snap), -&Snap->Pointers); - auto MemIdx = llvm::make_unique(); - MemIdx->build(std::move(S)); - return std::move(MemIdx); -} - -void MemIndex::findOccurrences( -const OccurrencesRequest &Req, -llvm::function_ref Callback) const { - log("findOccurrences is not implemented."); + return std::shared_ptr>(std::move(Snap), + &Snap->Pointers); } } // namespace clangd Index: clang-tools-extra/trunk/clangd/index/dex/DexIndex.h === --- clang-tools-extra/trunk/clangd/index/dex/DexIndex.h +++ clang-tools-extra/trunk/clangd/index/dex/DexIndex.h @@ -43,6 +43,9 @@ /// accessible as long as `Symbols` is kept alive. void build(std::shared_ptr> Symbols); + /// \brief Build index from a symbol slab. + static std::unique_ptr build(SymbolSlab Slab); + bool fuzzyFind(const FuzzyFindRequest &Req, llvm::function_ref Callback) const override; Index: clang-tools-extra/trunk/clangd/index/dex/DexIndex.cpp === --- clang-tools-extra/trunk/clangd/index/dex/DexIndex.cpp +++ clang-tools-extra/trunk
[clang-tools-extra] r340262 - [clangd] Allow using experimental Dex index
Author: omtcyfz Date: Tue Aug 21 03:32:27 2018 New Revision: 340262 URL: http://llvm.org/viewvc/llvm-project?rev=340262&view=rev Log: [clangd] Allow using experimental Dex index This patch adds hidden Clangd flag ("use-dex-index") which replaces (currently) default `MemIndex` with `DexIndex` for the static index. Reviewed by: ioeric Differential Revision: https://reviews.llvm.org/D50897 Modified: clang-tools-extra/trunk/clangd/index/MemIndex.cpp clang-tools-extra/trunk/clangd/index/MemIndex.h clang-tools-extra/trunk/clangd/index/dex/DexIndex.cpp clang-tools-extra/trunk/clangd/index/dex/DexIndex.h clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp Modified: clang-tools-extra/trunk/clangd/index/MemIndex.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/MemIndex.cpp?rev=340262&r1=340261&r2=340262&view=diff == --- clang-tools-extra/trunk/clangd/index/MemIndex.cpp (original) +++ clang-tools-extra/trunk/clangd/index/MemIndex.cpp Tue Aug 21 03:32:27 2018 @@ -28,6 +28,12 @@ void MemIndex::build(std::shared_ptr MemIndex::build(SymbolSlab Slab) { + auto Idx = llvm::make_unique(); + Idx->build(getSymbolsFromSlab(std::move(Slab))); + return std::move(Idx); +} + bool MemIndex::fuzzyFind( const FuzzyFindRequest &Req, llvm::function_ref Callback) const { @@ -72,7 +78,14 @@ void MemIndex::lookup(const LookupReques } } -std::unique_ptr MemIndex::build(SymbolSlab Slab) { +void MemIndex::findOccurrences( +const OccurrencesRequest &Req, +llvm::function_ref Callback) const { + log("findOccurrences is not implemented."); +} + +std::shared_ptr> +getSymbolsFromSlab(SymbolSlab Slab) { struct Snapshot { SymbolSlab Slab; std::vector Pointers; @@ -81,17 +94,8 @@ std::unique_ptr MemIndex::b Snap->Slab = std::move(Slab); for (auto &Sym : Snap->Slab) Snap->Pointers.push_back(&Sym); - auto S = std::shared_ptr>(std::move(Snap), -&Snap->Pointers); - auto MemIdx = llvm::make_unique(); - MemIdx->build(std::move(S)); - return std::move(MemIdx); -} - -void MemIndex::findOccurrences( -const OccurrencesRequest &Req, -llvm::function_ref Callback) const { - log("findOccurrences is not implemented."); + return std::shared_ptr>(std::move(Snap), + &Snap->Pointers); } } // namespace clangd Modified: clang-tools-extra/trunk/clangd/index/MemIndex.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/MemIndex.h?rev=340262&r1=340261&r2=340262&view=diff == --- clang-tools-extra/trunk/clangd/index/MemIndex.h (original) +++ clang-tools-extra/trunk/clangd/index/MemIndex.h Tue Aug 21 03:32:27 2018 @@ -47,6 +47,11 @@ private: mutable std::mutex Mutex; }; +// Returns pointers to the symbols in given slab and bundles slab lifetime with +// returned symbol pointers so that the pointers are never invalid. +std::shared_ptr> +getSymbolsFromSlab(SymbolSlab Slab); + } // namespace clangd } // namespace clang Modified: clang-tools-extra/trunk/clangd/index/dex/DexIndex.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/dex/DexIndex.cpp?rev=340262&r1=340261&r2=340262&view=diff == --- clang-tools-extra/trunk/clangd/index/dex/DexIndex.cpp (original) +++ clang-tools-extra/trunk/clangd/index/dex/DexIndex.cpp Tue Aug 21 03:32:27 2018 @@ -69,6 +69,12 @@ void DexIndex::build(std::shared_ptr DexIndex::build(SymbolSlab Slab) { + auto Idx = llvm::make_unique(); + Idx->build(getSymbolsFromSlab(std::move(Slab))); + return std::move(Idx); +} + /// Constructs iterators over tokens extracted from the query and exhausts it /// while applying Callback to each symbol in the order of decreasing quality /// of the matched symbols. Modified: clang-tools-extra/trunk/clangd/index/dex/DexIndex.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/dex/DexIndex.h?rev=340262&r1=340261&r2=340262&view=diff == --- clang-tools-extra/trunk/clangd/index/dex/DexIndex.h (original) +++ clang-tools-extra/trunk/clangd/index/dex/DexIndex.h Tue Aug 21 03:32:27 2018 @@ -43,6 +43,9 @@ public: /// accessible as long as `Symbols` is kept alive. void build(std::shared_ptr> Symbols); + /// \brief Build index from a symbol slab. + static std::unique_ptr build(SymbolSlab Slab); + bool fuzzyFind(const FuzzyFindRequest &Req, llvm::function_ref Callback) const override; 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=340262&r1=340261&r2=340262&view
[PATCH] D50509: [analyzer][UninitializedObjectChecker] Refactoring p6.: Move dereferencing to a function
Szelethus added inline comments. Comment at: lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp:223 + // int*). + while (auto Tmp = V.getAs()) { +// We can't reason about symbolic regions, assume its initialized. Szelethus wrote: > NoQ wrote: > > Szelethus wrote: > > > NoQ wrote: > > > > Hmm, i still have concerns about things like `int *x = (int *)&x;`. Why > > > > not just check the type to terminate the loop? Type hierarchy is > > > > guaranteed to be finite. > > > There actually is a testcase for that -- it would create a > > > nonloc::LocAsInteger, not a loc::MemRegionVal. > > > > > > I'll add a TODO to revisit this loop condition (again :) ). > > Ok, let's try with one more asterisk: > > ``` > > 1 void test() { > > 2 int **x = (int **)&x; > > 3 int *y = *x; > > 4 int z = *y; > > 5 } > > ``` > > > > Here's what i get in the Store: > > ``` > > (x,0,direct) : &element{x,0 S64b,int *} > > (y,0,direct) : &element{x,0 S64b,int *} > > (z,0,direct) : &element{x,0 S64b,int *} > > ``` > Sounds fun, I'll see how the checker behaves to these when I'm in the office. Yup, you were correct, it ends up in an infinite loop. I'll add the testcase for it before commiting. https://reviews.llvm.org/D50509 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r340263 - [clangd] NFC: Fix broken build
Author: omtcyfz Date: Tue Aug 21 03:40:19 2018 New Revision: 340263 URL: http://llvm.org/viewvc/llvm-project?rev=340263&view=rev Log: [clangd] NFC: Fix broken build Modified: clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp 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=340263&r1=340262&r2=340263&view=diff == --- clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp (original) +++ clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp Tue Aug 21 03:40:19 2018 @@ -29,6 +29,11 @@ using namespace clang; using namespace clang::clangd; +static llvm::cl::opt +UseDex("use-dex-index", + llvm::cl::desc("Use experimental Dex static index."), + llvm::cl::init(false), llvm::cl::Hidden); + namespace { enum class PCHStorageFlag { Disk, Memory }; @@ -47,7 +52,7 @@ std::unique_ptr buildStatic for (auto Sym : Slab) SymsBuilder.insert(Sym); - return UseDex ? DexIndex::build(std::move(SymsBuilder).build()) + return UseDex ? dex::DexIndex::build(std::move(SymsBuilder).build()) : MemIndex::build(std::move(SymsBuilder).build()); } @@ -189,11 +194,6 @@ static llvm::cl::opt Co "'compile_commands.json' files")), llvm::cl::init(FilesystemCompileArgs), llvm::cl::Hidden); -static llvm::cl::opt -UseDex("use-dex-index", - llvm::cl::desc("Use experimental Dex static index."), - llvm::cl::init(false), llvm::cl::Hidden); - int main(int argc, char *argv[]) { llvm::sys::PrintStackTraceOnErrorSignal(argv[0]); llvm::cl::SetVersionPrinter([](llvm::raw_ostream &OS) { ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50697: [clang-format] fix PR38557 - comments between "default" and ':' causes the case label to be treated as an identifier
klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. lg Repository: rC Clang https://reviews.llvm.org/D50697 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50898: [clangd] Suggest code-completions for overriding base class virtual methods.
kadircet updated this revision to Diff 161683. kadircet marked 3 inline comments as done. kadircet added a comment. - Put overrides through scoring and resolve nits. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50898 Files: clangd/CodeComplete.cpp clangd/CodeComplete.h unittests/clangd/CodeCompleteTests.cpp Index: unittests/clangd/CodeCompleteTests.cpp === --- unittests/clangd/CodeCompleteTests.cpp +++ unittests/clangd/CodeCompleteTests.cpp @@ -61,6 +61,7 @@ MATCHER(InsertInclude, "") { return bool(arg.HeaderInsertion); } MATCHER_P(SnippetSuffix, Text, "") { return arg.SnippetSuffix == Text; } MATCHER_P(Origin, OriginSet, "") { return arg.Origin == OriginSet; } +MATCHER(IsOverride, "") { return bool(arg.IsOverride); } // Shorthand for Contains(Named(Name)). Matcher &> Has(std::string Name) { @@ -1648,6 +1649,58 @@ SigDoc("Doc from sema"; } +TEST(CompletionTest, SuggestOverrides) { + constexpr const char *const Text(R"cpp( + class A { + public: +virtual void vfunc(bool param); +virtual void vfunc(bool param, int p); +void func(bool param); + }; + class B : public A { + virtual void ttt(bool param); + void vfunc(bool param, int p) override; + }; + class C : public B { + public: +void vfunc(bool param) override; +^ + }; + )cpp"); + const auto Results = completions(Text); + EXPECT_THAT(Results.Completions, + AllOf(Contains(AllOf(Named("vfunc"), IsOverride(), + Labeled("vfunc(bool param, int p)"))), +Contains(AllOf(Named("ttt"), IsOverride(), + Labeled("ttt(bool param)"))), +Not(Contains(AllOf(Named("vfunc"), IsOverride(), + Labeled("vfunc(bool param)")); +} + +TEST(CompletionTest, RenderOverride) { + CodeCompletion C; + C.Name = "x"; + C.Signature = "(bool) const"; + C.SnippetSuffix = "(${0:bool})"; + C.ReturnType = "int"; + C.Documentation = "This is x()."; + C.Kind = CompletionItemKind::Method; + C.Score.Total = 1.0; + C.Origin = SymbolOrigin::AST; + C.IsOverride = true; + + CodeCompleteOptions Opts; + Opts.IncludeIndicator.NoInsert = ""; + auto R = C.render(Opts); + EXPECT_EQ(R.label, "int x(bool) const override"); + EXPECT_EQ(R.insertText, "int x(bool) const override"); + EXPECT_EQ(R.insertTextFormat, InsertTextFormat::PlainText); + EXPECT_EQ(R.filterText, "x"); + EXPECT_EQ(R.detail, "int"); + EXPECT_EQ(R.documentation, "This is x()."); + EXPECT_THAT(R.additionalTextEdits, IsEmpty()); +} + } // namespace } // namespace clangd } // namespace clang Index: clangd/CodeComplete.h === --- clangd/CodeComplete.h +++ clangd/CodeComplete.h @@ -127,6 +127,9 @@ /// Holds the range of the token we are going to replace with this completion. Range CompletionTokenRange; + /// Whether this completion is for overriding a virtual method. + bool IsOverride = false; + // Scores are used to rank completion items. struct Scores { // The score that items are ranked by. Index: clangd/CodeComplete.cpp === --- clangd/CodeComplete.cpp +++ clangd/CodeComplete.cpp @@ -188,14 +188,67 @@ return HeaderFile{std::move(*Resolved), /*Verbatim=*/false}; } +// First traverses all method definitions inside current class/struct/union +// definition. Than traverses base classes to find virtual methods that haven't +// been overriden within current context. +// FIXME(kadircet): Currently we cannot see declarations below completion point. +// It is because Sema gets run only upto completion point. Need to find a +// solution to run it for the whole class/struct/union definition. +static std::vector +getVirtualNonOverridenMethods(const DeclContext *DC, Sema *S) { + const auto *CR = llvm::dyn_cast(DC); + // If not inside a class/struct/union return empty. + if (!CR) +return {}; + // First store overrides within current class. + // These are stored by name to make querying fast in the later step. + llvm::StringMap> Overrides; + for (auto *Method : CR->methods()) { +if (!Method->isVirtual()) + continue; +const std::string Name = Method->getNameAsString(); +Overrides[Name].push_back(Method); + } + + std::vector Results; + for (const auto &Base : CR->bases()) { +const auto *BR = Base.getType().getTypePtr()->getAsCXXRecordDecl(); +if (!BR) + continue; +for (auto *Method : BR->methods()) { + if (!Method->isVirtual()) +continue; + const std::string Name = Method->getNameAsString(); + const auto it = Overrides.find(Name); + bool IsOverriden = false; + if (it != Overrides.end()) +for (auto *MD : it->second) { + // If the method in current body is not an overload o
[PATCH] D50898: [clangd] Suggest code-completions for overriding base class virtual methods.
kadircet marked an inline comment as done. kadircet added inline comments. Comment at: clangd/CodeComplete.cpp:1233 +// struct/class/union definition. +const auto Overrides = getVirtualNonOverridenMethods( +Recorder->CCSema->CurContext, Recorder->CCSema); hokein wrote: > It seems that we treat it as a special case, the code path here runs out of > the `ranking` path. Put override suggestions to ranking system. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50898 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50898: [clangd] Suggest code-completions for overriding base class virtual methods.
ioeric added a comment. In https://reviews.llvm.org/D50898#1205756, @hokein wrote: > I think it is reasonable to make Sema support suggesting override methods, > instead of implementing it in clangd side? drive-by: +1 to this. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50898 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r340265 - [analyzer][UninitializedObjectChecker] Refactoring p6.: Move dereferencing to a function
Author: szelethus Date: Tue Aug 21 03:45:21 2018 New Revision: 340265 URL: http://llvm.org/viewvc/llvm-project?rev=340265&view=rev Log: [analyzer][UninitializedObjectChecker] Refactoring p6.: Move dereferencing to a function Now that it has it's own file, it makes little sense for isPointerOrReferenceUninit to be this large, so I moved dereferencing to a separate function. Differential Revision: https://reviews.llvm.org/D50509 Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp cfe/trunk/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp?rev=340265&r1=340264&r2=340265&view=diff == --- cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp (original) +++ cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp Tue Aug 21 03:45:21 2018 @@ -265,7 +265,8 @@ bool FindUninitializedFields::isNonUnion continue; } -if (T->isAnyPointerType() || T->isReferenceType() || T->isBlockPointerType()) { +if (T->isAnyPointerType() || T->isReferenceType() || +T->isBlockPointerType()) { if (isPointerOrReferenceUninit(FR, LocalChain)) ContainsUninitField = true; continue; Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp?rev=340265&r1=340264&r2=340265&view=diff == --- cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp (original) +++ cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp Tue Aug 21 03:45:21 2018 @@ -95,6 +95,12 @@ public: /// known, and thus FD can not be analyzed. static bool isVoidPointer(QualType T); +/// Dereferences \p V and returns the value and dynamic type of the pointee, as +/// well as wether \p FR needs to be casted back to that type. If for whatever +/// reason dereferencing fails, returns with None. +static llvm::Optional> +dereference(ProgramStateRef State, const FieldRegion *FR); + //===--===// // Methods for FindUninitializedFields. //===--===// @@ -126,67 +132,22 @@ bool FindUninitializedFields::isPointerO return false; } - assert(V.getAs() && - "At this point V must be loc::MemRegionVal!"); - auto L = V.castAs(); - - // We can't reason about symbolic regions, assume its initialized. - // Note that this also avoids a potential infinite recursion, because - // constructors for list-like classes are checked without being called, and - // the Static Analyzer will construct a symbolic region for Node *next; or - // similar code snippets. - if (L.getRegion()->getSymbolicBase()) { -IsAnyFieldInitialized = true; -return false; - } - - DynamicTypeInfo DynTInfo = getDynamicTypeInfo(State, L.getRegion()); - if (!DynTInfo.isValid()) { + // At this point the pointer itself is initialized and points to a valid + // location, we'll now check the pointee. + llvm::Optional> DerefInfo = + dereference(State, FR); + if (!DerefInfo) { IsAnyFieldInitialized = true; return false; } - QualType DynT = DynTInfo.getType(); - - // If the static type of the field is a void pointer, we need to cast it back - // to the dynamic type before dereferencing. - bool NeedsCastBack = isVoidPointer(FR->getDecl()->getType()); - - if (isVoidPointer(DynT)) { -IsAnyFieldInitialized = true; -return false; - } - - // At this point the pointer itself is initialized and points to a valid - // location, we'll now check the pointee. - SVal DerefdV = State->getSVal(V.castAs(), DynT); - - // If DerefdV is still a pointer value, we'll dereference it again (e.g.: - // int** -> int*). - while (auto Tmp = DerefdV.getAs()) { -if (Tmp->getRegion()->getSymbolicBase()) { - IsAnyFieldInitialized = true; - return false; -} - -DynTInfo = getDynamicTypeInfo(State, Tmp->getRegion()); -if (!DynTInfo.isValid()) { - IsAnyFieldInitialized = true; - return false; -} - -DynT = DynTInfo.getType(); -if (isVoidPointer(DynT)) { - IsAnyFieldInitialized = true; - return false; -} - -DerefdV = State->getSVal(*Tmp, DynT); - } + V = std::get<0>(*DerefInfo); + QualType DynT = std::get<1>(*Deref
[PATCH] D50509: [analyzer][UninitializedObjectChecker] Refactoring p6.: Move dereferencing to a function
This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rL340265: [analyzer][UninitializedObjectChecker] Refactoring p6.: Move dereferencing to a… (authored by Szelethus, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D50509?vs=161209&id=161685#toc Repository: rL LLVM https://reviews.llvm.org/D50509 Files: cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp cfe/trunk/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp Index: cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp === --- cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp +++ cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp @@ -265,7 +265,8 @@ continue; } -if (T->isAnyPointerType() || T->isReferenceType() || T->isBlockPointerType()) { +if (T->isAnyPointerType() || T->isReferenceType() || +T->isBlockPointerType()) { if (isPointerOrReferenceUninit(FR, LocalChain)) ContainsUninitField = true; continue; Index: cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp === --- cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp +++ cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp @@ -95,6 +95,12 @@ /// known, and thus FD can not be analyzed. static bool isVoidPointer(QualType T); +/// Dereferences \p V and returns the value and dynamic type of the pointee, as +/// well as wether \p FR needs to be casted back to that type. If for whatever +/// reason dereferencing fails, returns with None. +static llvm::Optional> +dereference(ProgramStateRef State, const FieldRegion *FR); + //===--===// // Methods for FindUninitializedFields. //===--===// @@ -126,67 +132,22 @@ return false; } - assert(V.getAs() && - "At this point V must be loc::MemRegionVal!"); - auto L = V.castAs(); - - // We can't reason about symbolic regions, assume its initialized. - // Note that this also avoids a potential infinite recursion, because - // constructors for list-like classes are checked without being called, and - // the Static Analyzer will construct a symbolic region for Node *next; or - // similar code snippets. - if (L.getRegion()->getSymbolicBase()) { -IsAnyFieldInitialized = true; -return false; - } - - DynamicTypeInfo DynTInfo = getDynamicTypeInfo(State, L.getRegion()); - if (!DynTInfo.isValid()) { + // At this point the pointer itself is initialized and points to a valid + // location, we'll now check the pointee. + llvm::Optional> DerefInfo = + dereference(State, FR); + if (!DerefInfo) { IsAnyFieldInitialized = true; return false; } - QualType DynT = DynTInfo.getType(); - - // If the static type of the field is a void pointer, we need to cast it back - // to the dynamic type before dereferencing. - bool NeedsCastBack = isVoidPointer(FR->getDecl()->getType()); - - if (isVoidPointer(DynT)) { -IsAnyFieldInitialized = true; -return false; - } - - // At this point the pointer itself is initialized and points to a valid - // location, we'll now check the pointee. - SVal DerefdV = State->getSVal(V.castAs(), DynT); - - // If DerefdV is still a pointer value, we'll dereference it again (e.g.: - // int** -> int*). - while (auto Tmp = DerefdV.getAs()) { -if (Tmp->getRegion()->getSymbolicBase()) { - IsAnyFieldInitialized = true; - return false; -} - -DynTInfo = getDynamicTypeInfo(State, Tmp->getRegion()); -if (!DynTInfo.isValid()) { - IsAnyFieldInitialized = true; - return false; -} - -DynT = DynTInfo.getType(); -if (isVoidPointer(DynT)) { - IsAnyFieldInitialized = true; - return false; -} - -DerefdV = State->getSVal(*Tmp, DynT); - } + V = std::get<0>(*DerefInfo); + QualType DynT = std::get<1>(*DerefInfo); + bool NeedsCastBack = std::get<2>(*DerefInfo); // If FR is a pointer pointing to a non-primitive type. if (Optional RecordV = - DerefdV.getAs()) { + V.getAs()) { const TypedValueRegion *R = RecordV->getRegion(); @@ -220,7 +181,7 @@ "At this point FR must either have a primitive dynamic type, or it " "must be a null, undefined, unknown or concrete pointer!"); - if (isPrimitiveUninit(DerefdV)) { + if (isPrimitiveUninit(V)) {
[PATCH] D50904: [analyzer][UninitializedObjectChecker] Added documentation to the checker list
This revision was automatically updated to reflect the committed changes. Closed by commit rL340266: [analyzer][UninitializedObjectChecker] Added documentation to the checker list (authored by Szelethus, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D50904?vs=161262&id=161686#toc Repository: rL LLVM https://reviews.llvm.org/D50904 Files: cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp cfe/trunk/www/analyzer/alpha_checks.html Index: cfe/trunk/www/analyzer/alpha_checks.html === --- cfe/trunk/www/analyzer/alpha_checks.html +++ cfe/trunk/www/analyzer/alpha_checks.html @@ -323,6 +323,118 @@ }; + + +alpha.cplusplus.UninitializedObject +(C++) +This checker reports uninitialized fields in objects created +after a constructor call. It doesn't only find direct uninitialized +fields, but rather makes a deep inspection of the object, +analyzing all of it's fields subfields. +The checker regards inherited fields as direct fields, so one +will recieve warnings for uninitialized inherited data members +as well. + +It 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. + + + + +// With Pedantic and CheckPointeeInitialization set to true + +struct A { + struct B { +int x; // note: uninitialized field 'this->b.x' + // note: uninitialized field 'this->bptr->x' +int y; // note: uninitialized field 'this->b.y' + // note: uninitialized field 'this->bptr->y' + }; + int *iptr; // note: uninitialized pointer 'this->iptr' + B b; + B *bptr; + char *cptr; // note: uninitialized pointee 'this->cptr' + + A (B *bptr, char *cptr) : bptr(bptr), cptr(cptr) {} +}; + +void f() { + A::B b; + char c; + A a(&b, &c); // warning: 6 uninitialized fields + // after the constructor call +} + + +// With Pedantic set to false and +// CheckPointeeInitialization set to true +// (every field is uninitialized) + +struct A { + struct B { +int x; +int y; + }; + int *iptr; + B b; + B *bptr; + char *cptr; + + A (B *bptr, char *cptr) : bptr(bptr), cptr(cptr) {} +}; + +void f() { + A::B b; + char c; + A a(&b, &c); // no warning +} + + +// With Pedantic and CheckPointeeInitialization set to false +// (pointees are regarded as initialized) + +struct A { + struct B { +int x; // note: uninitialized field 'this->b.x' +int y; // note: uninitialized field 'this->b.y' + }; + int *iptr; // note: uninitialized pointer 'this->iptr' + B b; + B *bptr; + char *cptr; + + A (B *bptr, char *cptr) : bptr(bptr), cptr(cptr) {} +}; + +void f() { + A::B b; + char c; + A a(&b, &c); // warning: 3 uninitialized fields + // after the constructor call +} + + + + + Index: cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp === --- cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp +++ cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp @@ -286,18 +286,6 @@ } // Checking bases. - // FIXME: As of now, because of `willObjectBeAnalyzedLater`, objects whose - // type is a descendant of another type will emit warnings for uninitalized - // inherited members. - // This is not the only way to analyze bases of an object -- if we didn't - // filter them out, and didn't analyze the bases, this checker would run for - // each base of the object in order of base initailization and in theory would - // find every uninitalized field. This approach could also make handling - // diamond inheritances more easily. - // - // This rule (that a descendant type's cunstructor is responsible for - // initializing inherited data members) is not obvious, and should it should - // be. const auto *CXXRD = dyn_cast(RD); if (!CXXRD) return ContainsUninitField; ___ cf
r340266 - [analyzer][UninitializedObjectChecker] Added documentation to the checker list
Author: szelethus Date: Tue Aug 21 03:47:19 2018 New Revision: 340266 URL: http://llvm.org/viewvc/llvm-project?rev=340266&view=rev Log: [analyzer][UninitializedObjectChecker] Added documentation to the checker list Differential Revision: https://reviews.llvm.org/D50904 Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp cfe/trunk/www/analyzer/alpha_checks.html Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp?rev=340266&r1=340265&r2=340266&view=diff == --- cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp (original) +++ cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp Tue Aug 21 03:47:19 2018 @@ -286,18 +286,6 @@ bool FindUninitializedFields::isNonUnion } // Checking bases. - // FIXME: As of now, because of `willObjectBeAnalyzedLater`, objects whose - // type is a descendant of another type will emit warnings for uninitalized - // inherited members. - // This is not the only way to analyze bases of an object -- if we didn't - // filter them out, and didn't analyze the bases, this checker would run for - // each base of the object in order of base initailization and in theory would - // find every uninitalized field. This approach could also make handling - // diamond inheritances more easily. - // - // This rule (that a descendant type's cunstructor is responsible for - // initializing inherited data members) is not obvious, and should it should - // be. const auto *CXXRD = dyn_cast(RD); if (!CXXRD) return ContainsUninitField; Modified: cfe/trunk/www/analyzer/alpha_checks.html URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/www/analyzer/alpha_checks.html?rev=340266&r1=340265&r2=340266&view=diff == --- cfe/trunk/www/analyzer/alpha_checks.html (original) +++ cfe/trunk/www/analyzer/alpha_checks.html Tue Aug 21 03:47:19 2018 @@ -323,6 +323,118 @@ public: }; + + +alpha.cplusplus.UninitializedObject +(C++) +This checker reports uninitialized fields in objects created +after a constructor call. It doesn't only find direct uninitialized +fields, but rather makes a deep inspection of the object, +analyzing all of it's fields subfields. +The checker regards inherited fields as direct fields, so one +will recieve warnings for uninitialized inherited data members +as well. + +It 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. + + + + +// With Pedantic and CheckPointeeInitialization set to true + +struct A { + struct B { +int x; // note: uninitialized field 'this->b.x' + // note: uninitialized field 'this->bptr->x' +int y; // note: uninitialized field 'this->b.y' + // note: uninitialized field 'this->bptr->y' + }; + int *iptr; // note: uninitialized pointer 'this->iptr' + B b; + B *bptr; + char *cptr; // note: uninitialized pointee 'this->cptr' + + A (B *bptr, char *cptr) : bptr(bptr), cptr(cptr) {} +}; + +void f() { + A::B b; + char c; + A a(&b, &c); // warning: 6 uninitialized fields + // after the constructor call +} + + +// With Pedantic set to false and +// CheckPointeeInitialization set to true +// (every field is uninitialized) + +struct A { + struct B { +int x; +int y; + }; + int *iptr; + B b; + B *bptr; + char *cptr; + + A (B *bptr, char *cptr) : bptr(bptr), cptr(cptr) {} +}; + +void f() { + A::B b; + char c; + A a(&b, &c); // no warning +} + + +// With Pedantic and CheckPointeeInitialization set to false +// (pointees are regarded as initialized) + +struct A { + struct B { +int x; // note: uninitialized field 'this->b.x' +int y; // note: uninitialized field 'this->b.y' + }; + int *iptr; // note: uniniti
[PATCH] D50993: [clangd] Increase stack size of the new threads on macOS
ilya-biryukov added a comment. 1. Can we put the helper that executes the tasks asynchronously with a stack size into llvm's threading library (i.e. somewhere close to `llvm::execute_on_thread`?) All platform-specific code is already there, we can reuse most of the implementation too. 2. I wonder if we should expose the stack size as an option to clangd? Comment at: clangd/Threading.cpp:57 +template +static void executeOnThreadAndDetach(UserFnPtr UserFn, Data UserData) { +#if !defined(__APPLE__) Maybe add a parameter for the stack size to this function? (And document it's only taken into account when pthread-based implementation is used) Comment at: clangd/Threading.cpp:57 +template +static void executeOnThreadAndDetach(UserFnPtr UserFn, Data UserData) { +#if !defined(__APPLE__) ilya-biryukov wrote: > Maybe add a parameter for the stack size to this function? (And document it's > only taken into account when pthread-based implementation is used) Maybe accept a `llvm::unique_function` instead of `UserFn` and `Data`? This split is clearly implementation detail of pthreads-based solution. `std::thread` does not need it. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50993 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50557: [clang][mips] Set __mips_fpr correctly for -mfpxx
smaksimovic updated this revision to Diff 161690. smaksimovic added a comment. Expanded a piece of code using switch cases as suggested. https://reviews.llvm.org/D50557 Files: include/clang/Basic/DiagnosticCommonKinds.td lib/Basic/Targets/Mips.cpp lib/Basic/Targets/Mips.h test/Preprocessor/init.c Index: test/Preprocessor/init.c === --- test/Preprocessor/init.c +++ test/Preprocessor/init.c @@ -3442,7 +3442,7 @@ // MIPS32BE:#define __mips 32 // MIPS32BE:#define __mips__ 1 // MIPS32BE:#define __mips_abicalls 1 -// MIPS32BE:#define __mips_fpr 32 +// MIPS32BE:#define __mips_fpr 0 // MIPS32BE:#define __mips_hard_float 1 // MIPS32BE:#define __mips_o32 1 // MIPS32BE:#define _mips 1 @@ -3649,7 +3649,7 @@ // MIPS32EL:#define __mips 32 // MIPS32EL:#define __mips__ 1 // MIPS32EL:#define __mips_abicalls 1 -// MIPS32EL:#define __mips_fpr 32 +// MIPS32EL:#define __mips_fpr 0 // MIPS32EL:#define __mips_hard_float 1 // MIPS32EL:#define __mips_o32 1 // MIPS32EL:#define _mips 1 @@ -4900,6 +4900,41 @@ // RUN: | FileCheck -match-full-lines -check-prefix NOMIPS-ABS2008 %s // NOMIPS-ABS2008-NOT:#define __mips_abs2008 1 // +// RUN: %clang_cc1 \ +// RUN: -E -dM -triple=mips-none-none < /dev/null \ +// RUN: | FileCheck -match-full-lines -check-prefix MIPS32-NOFP %s +// MIPS32-NOFP:#define __mips_fpr 0 +// +// RUN: %clang_cc1 -target-feature +fpxx \ +// RUN: -E -dM -triple=mips-none-none < /dev/null \ +// RUN: | FileCheck -match-full-lines -check-prefix MIPS32-MFPXX %s +// MIPS32-MFPXX:#define __mips_fpr 0 +// +// RUN: %clang_cc1 -target-cpu mips32r6 -target-feature +fpxx \ +// RUN: -E -dM -triple=mips-none-none < /dev/null \ +// RUN: | FileCheck -match-full-lines -check-prefix MIPS32R6-MFPXX %s +// MIPS32R6-MFPXX:#define __mips_fpr 0 +// +// RUN: %clang_cc1 \ +// RUN: -E -dM -triple=mips64-none-none < /dev/null \ +// RUN: | FileCheck -match-full-lines -check-prefix MIPS64-NOFP %s +// MIPS64-NOFP:#define __mips_fpr 64 +// +// RUN: not %clang_cc1 -target-feature -fp64 \ +// RUN: -E -dM -triple=mips64-none-none < /dev/null 2>&1 \ +// RUN: | FileCheck -match-full-lines -check-prefix MIPS64-MFP32 %s +// MIPS64-MFP32:error: option '-mfpxx' cannot be specified with 'mips64r2' +// +// RUN: not %clang_cc1 -target-feature +fpxx \ +// RUN: -E -dM -triple=mips64-none-none < /dev/null 2>&1 \ +// RUN: | FileCheck -match-full-lines -check-prefix MIPS64-MFPXX %s +// MIPS64-MFPXX:error: '-mfpxx' can only be used with the 'o32' ABI +// +// RUN: not %clang_cc1 -target-cpu mips64r6 -target-feature +fpxx \ +// RUN: -E -dM -triple=mips64-none-none < /dev/null 2>&1 \ +// RUN: | FileCheck -match-full-lines -check-prefix MIPS64R6-MFPXX %s +// MIPS64R6-MFPXX:error: '-mfpxx' can only be used with the 'o32' ABI +// // RUN: %clang_cc1 -target-feature -fp64 \ // RUN: -E -dM -triple=mips-none-none < /dev/null \ // RUN: | FileCheck -match-full-lines -check-prefix MIPS32-MFP32 %s @@ -4916,7 +4951,7 @@ // RUN: -E -dM -triple=mips-none-none < /dev/null \ // RUN: | FileCheck -match-full-lines -check-prefix MIPS32-MFP32SF %s // MIPS32-MFP32SF:#define _MIPS_FPSET 32 -// MIPS32-MFP32SF:#define __mips_fpr 32 +// MIPS32-MFP32SF:#define __mips_fpr 0 // // RUN: %clang_cc1 -target-feature +fp64 \ // RUN: -E -dM -triple=mips64-none-none < /dev/null \ Index: lib/Basic/Targets/Mips.h === --- lib/Basic/Targets/Mips.h +++ lib/Basic/Targets/Mips.h @@ -57,16 +57,16 @@ bool UseIndirectJumpHazard; protected: - bool HasFP64; + enum FPModeEnum { FPXX, FP32, FP64 } FPMode; std::string ABI; public: MipsTargetInfo(const llvm::Triple &Triple, const TargetOptions &) : TargetInfo(Triple), IsMips16(false), IsMicromips(false), IsNan2008(false), IsAbs2008(false), IsSingleFloat(false), IsNoABICalls(false), CanUseBSDABICalls(false), FloatABI(HardFloat), DspRev(NoDSP), HasMSA(false), DisableMadd4(false), -UseIndirectJumpHazard(false), HasFP64(false) { +UseIndirectJumpHazard(false), FPMode(FPXX) { TheCXXABI.set(TargetCXXABI::GenericMIPS); setABI(getTriple().isMIPS32() ? "o32" : "n64"); @@ -181,6 +181,8 @@ return TargetInfo::initFeatureMap(Features, Diags, CPU, FeaturesVec); } + unsigned getISARev() const; + void getTargetDefines(const LangOptions &Opts, MacroBuilder &Builder) const override; @@ -305,7 +307,7 @@ IsSingleFloat = false; FloatABI = HardFloat; DspRev = NoDSP; -HasFP64 = isFP64Default(); +FPMode = isFP64Default() ? FP64 : FPXX; for (const auto &Feature : Features) { if (Feature == "+single-float") @@ -325,9 +327,11 @@ else if (Feature == "+nomadd4") DisableMadd4 = true; else if (Feature == "+fp64") -HasFP64 = true; +FPMode = FP64; else if (Feature == "-fp64") -HasFP64 = false;
[PATCH] D48027: [analyzer] Improve `CallDescription` to handle c++ method.
MTC added a comment. @xazax.hun What do you think? https://reviews.llvm.org/D48027 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44539: [Sema][Objective-C] Add check to warn when property of objc type has assign attribute
QF5690 updated this revision to Diff 161684. QF5690 added a comment. Using `isObjCARCImplicitlyUnretainedType` instead of `isObjCClassType` Tests in `property-in-class-extension-1.m` relaxed to `-Wproperty-attribute-mismatch` (was `-Weverything`) Repository: rC Clang https://reviews.llvm.org/D44539 Files: include/clang/Basic/DiagnosticGroups.td include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaObjCProperty.cpp test/SemaObjC/property-assign-on-object-type.m test/SemaObjC/property-in-class-extension-1.m Index: test/SemaObjC/property-in-class-extension-1.m === --- test/SemaObjC/property-in-class-extension-1.m +++ test/SemaObjC/property-in-class-extension-1.m @@ -1,5 +1,5 @@ -// RUN: %clang_cc1 -fsyntax-only -triple x86_64-apple-darwin11 -fobjc-runtime-has-weak -fobjc-weak -verify -Weverything %s -// RUN: %clang_cc1 -x objective-c++ -triple x86_64-apple-darwin11 -fobjc-runtime-has-weak -fobjc-weak -fsyntax-only -verify -Weverything %s +// RUN: %clang_cc1 -fsyntax-only -triple x86_64-apple-darwin11 -fobjc-runtime-has-weak -fobjc-weak -verify -Wproperty-attribute-mismatch %s +// RUN: %clang_cc1 -x objective-c++ -triple x86_64-apple-darwin11 -fobjc-runtime-has-weak -fobjc-weak -fsyntax-only -verify -Wproperty-attribute-mismatch %s // rdar://12103400 @class NSString; Index: test/SemaObjC/property-assign-on-object-type.m === --- /dev/null +++ test/SemaObjC/property-assign-on-object-type.m @@ -0,0 +1,19 @@ +// RUN: %clang_cc1 -fsyntax-only -verify -Wobjc-property-assign-on-object-type %s + +@interface Foo @end +@protocol Prot @end + +@interface Bar +@property(assign, readonly) Foo* o1; // expected-warning {{'assign' property of object type may become a dangling reference; consider using 'unsafe_unretained'}} +@property(unsafe_unretained, readonly) Foo* o2; + +@property(assign) Class classProperty; +@property(assign) Class classWithProtocolProperty; +@property(assign) int s1; +@property(assign) int* s2; +@end + +@interface Bar () +@property(readwrite) Foo* o1; +@property(readwrite) Foo* o2; +@end Index: lib/Sema/SemaObjCProperty.cpp === --- lib/Sema/SemaObjCProperty.cpp +++ lib/Sema/SemaObjCProperty.cpp @@ -2547,6 +2547,14 @@ PropertyDecl->setInvalidDecl(); } + // Check for assign on object types. + if ((Attributes & ObjCDeclSpec::DQ_PR_assign) && + !(Attributes & ObjCDeclSpec::DQ_PR_unsafe_unretained) && + PropertyTy->isObjCRetainableType() && + !PropertyTy->isObjCARCImplicitlyUnretainedType()) { +Diag(Loc, diag::warn_objc_property_assign_on_object); + } + // Check for more than one of { assign, copy, retain }. if (Attributes & ObjCDeclSpec::DQ_PR_assign) { if (Attributes & ObjCDeclSpec::DQ_PR_copy) { Index: include/clang/Basic/DiagnosticSemaKinds.td === --- include/clang/Basic/DiagnosticSemaKinds.td +++ include/clang/Basic/DiagnosticSemaKinds.td @@ -1014,6 +1014,9 @@ "property attributes '%0' and '%1' are mutually exclusive">; def err_objc_property_requires_object : Error< "property with '%0' attribute must be of object type">; +def warn_objc_property_assign_on_object : Warning< + "'assign' property of object type may become a dangling reference; consider using 'unsafe_unretained'">, + InGroup, DefaultIgnore; def warn_objc_property_no_assignment_attribute : Warning< "no 'assign', 'retain', or 'copy' attribute is specified - " "'assign' is assumed">, Index: include/clang/Basic/DiagnosticGroups.td === --- include/clang/Basic/DiagnosticGroups.td +++ include/clang/Basic/DiagnosticGroups.td @@ -367,6 +367,7 @@ def BadFunctionCast : DiagGroup<"bad-function-cast">; def ObjCPropertyImpl : DiagGroup<"objc-property-implementation">; def ObjCPropertyNoAttribute : DiagGroup<"objc-property-no-attribute">; +def ObjCPropertyAssignOnObjectType : DiagGroup<"objc-property-assign-on-object-type">; def ObjCProtocolQualifiers : DiagGroup<"objc-protocol-qualifiers">; def ObjCMissingSuperCalls : DiagGroup<"objc-missing-super-calls">; def ObjCDesignatedInit : DiagGroup<"objc-designated-initializers">; Index: test/SemaObjC/property-in-class-extension-1.m === --- test/SemaObjC/property-in-class-extension-1.m +++ test/SemaObjC/property-in-class-extension-1.m @@ -1,5 +1,5 @@ -// RUN: %clang_cc1 -fsyntax-only -triple x86_64-apple-darwin11 -fobjc-runtime-has-weak -fobjc-weak -verify -Weverything %s -// RUN: %clang_cc1 -x objective-c++ -triple x86_64-apple-darwin11 -fobjc-runtime-has-weak -fobjc-weak -fsyntax-only -verify -Weverything %s +// RUN: %clang_cc1 -fsyntax-only -triple x86_64-apple-darwin11 -fobjc-runtime-has-weak -fobjc-weak -verify -Wproperty-attri
[PATCH] D50889: [clangd] Make FileIndex aware of the main file
ilya-biryukov updated this revision to Diff 161694. ilya-biryukov marked 3 inline comments as done. ilya-biryukov added a comment. - Move DynamicIndex into ClangdServer.cpp - Rename FileIdx to DynamicIdx - Avoid modifying input args Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50889 Files: clangd/ClangdServer.cpp clangd/ClangdServer.h clangd/index/FileIndex.cpp clangd/index/FileIndex.h Index: clangd/index/FileIndex.h === --- clangd/index/FileIndex.h +++ clangd/index/FileIndex.h @@ -64,7 +64,11 @@ /// nullptr, this removes all symbols in the file. /// If \p AST is not null, \p PP cannot be null and it should be the /// preprocessor that was used to build \p AST. - void update(PathRef Path, ASTContext *AST, std::shared_ptr PP); + /// If \p TopLevelDecls is set, only these decls are indexed. Otherwise, all + /// top level decls obtained from \p AST are indexed. + void + update(PathRef Path, ASTContext *AST, std::shared_ptr PP, + llvm::Optional> TopLevelDecls = llvm::None); bool fuzzyFind(const FuzzyFindRequest &Req, @@ -86,8 +90,12 @@ /// Retrieves namespace and class level symbols in \p AST. /// Exposed to assist in unit tests. /// If URISchemes is empty, the default schemes in SymbolCollector will be used. -SymbolSlab indexAST(ASTContext &AST, std::shared_ptr PP, -llvm::ArrayRef URISchemes = {}); +/// If \p TopLevelDecls is set, only these decls are indexed. Otherwise, all top +/// level decls obtained from \p AST are indexed. +SymbolSlab +indexAST(ASTContext &AST, std::shared_ptr PP, + llvm::Optional> TopLevelDecls = llvm::None, + llvm::ArrayRef URISchemes = {}); } // namespace clangd } // namespace clang Index: clangd/index/FileIndex.cpp === --- clangd/index/FileIndex.cpp +++ clangd/index/FileIndex.cpp @@ -8,15 +8,16 @@ //===--===// #include "FileIndex.h" -#include "SymbolCollector.h" #include "../Logger.h" +#include "SymbolCollector.h" #include "clang/Index/IndexingAction.h" #include "clang/Lex/Preprocessor.h" namespace clang { namespace clangd { SymbolSlab indexAST(ASTContext &AST, std::shared_ptr PP, +llvm::Optional> TopLevelDecls, llvm::ArrayRef URISchemes) { SymbolCollector::Options CollectorOpts; // FIXME(ioeric): we might also want to collect include headers. We would need @@ -38,10 +39,14 @@ index::IndexingOptions::SystemSymbolFilterKind::DeclarationsOnly; IndexOpts.IndexFunctionLocals = false; - std::vector TopLevelDecls( - AST.getTranslationUnitDecl()->decls().begin(), - AST.getTranslationUnitDecl()->decls().end()); - index::indexTopLevelDecls(AST, TopLevelDecls, Collector, IndexOpts); + std::vector DeclsToIndex; + if (TopLevelDecls) +DeclsToIndex.assign(TopLevelDecls->begin(), TopLevelDecls->end()); + else +DeclsToIndex.assign(AST.getTranslationUnitDecl()->decls().begin(), +AST.getTranslationUnitDecl()->decls().end()); + + index::indexTopLevelDecls(AST, DeclsToIndex, Collector, IndexOpts); return Collector.takeSymbols(); } @@ -81,13 +86,14 @@ } void FileIndex::update(PathRef Path, ASTContext *AST, - std::shared_ptr PP) { + std::shared_ptr PP, + llvm::Optional> TopLevelDecls) { if (!AST) { FSymbols.update(Path, nullptr); } else { assert(PP); auto Slab = llvm::make_unique(); -*Slab = indexAST(*AST, PP, URISchemes); +*Slab = indexAST(*AST, PP, TopLevelDecls, URISchemes); FSymbols.update(Path, std::move(Slab)); } auto Symbols = FSymbols.allSymbols(); Index: clangd/ClangdServer.h === --- clangd/ClangdServer.h +++ clangd/ClangdServer.h @@ -99,6 +99,7 @@ /// synchronize access to shared state. ClangdServer(GlobalCompilationDatabase &CDB, FileSystemProvider &FSProvider, DiagnosticsConsumer &DiagConsumer, const Options &Opts); + ~ClangdServer(); /// Set the root path of the workspace. void setRootPath(PathRef RootPath); @@ -200,6 +201,7 @@ formatCode(llvm::StringRef Code, PathRef File, ArrayRef Ranges); + class DynamicIndex; typedef uint64_t DocVersion; void consumeDiagnostics(PathRef File, DocVersion Version, @@ -217,15 +219,14 @@ Path ResourceDir; // The index used to look up symbols. This could be: // - null (all index functionality is optional) - // - the dynamic index owned by ClangdServer (FileIdx) + // - the dynamic index owned by ClangdServer (DynamicIdx) // - the static index passed to the constructor // - a merged view of a static and dynamic index (MergedIndex) SymbolIndex *Index; - // If present, an up-to-date of
[PATCH] D50889: [clangd] Make FileIndex aware of the main file
ilya-biryukov added inline comments. Comment at: clangd/ClangdServer.cpp:70 +// FIXME(ibiryukov): this should be a generic helper instead. +class NoopCallbacks : public ParsingCallbacks { public: ioeric wrote: > Maybe provide noop implementations for `ParsingCallbacks::onPreambleAST()` > and `ParsingCallbacks::onMainAST` by default? I'll address this in a parent revision. Comment at: clangd/ClangdServer.h:246 + /// If present, an up-to-date of symbols in open files. Read via Index. + std::unique_ptr FileIdx; // If present, a merged view of FileIdx and an external index. Read via Index. ioeric wrote: > nit: `s/FileIdx/DymIdx`? (also need to update the comment above) Went with `DynamicIdx`. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50889 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r340271 - Add missing library dependency to fix build break after rC340247
Author: inouehrs Date: Tue Aug 21 04:41:41 2018 New Revision: 340271 URL: http://llvm.org/viewvc/llvm-project?rev=340271&view=rev Log: Add missing library dependency to fix build break after rC340247 Modified: cfe/trunk/lib/ARCMigrate/CMakeLists.txt Modified: cfe/trunk/lib/ARCMigrate/CMakeLists.txt URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/ARCMigrate/CMakeLists.txt?rev=340271&r1=340270&r2=340271&view=diff == --- cfe/trunk/lib/ARCMigrate/CMakeLists.txt (original) +++ cfe/trunk/lib/ARCMigrate/CMakeLists.txt Tue Aug 21 04:41:41 2018 @@ -35,4 +35,5 @@ add_clang_library(clangARCMigrate clangSema clangSerialization clangStaticAnalyzerCheckers + clangStaticAnalyzerCore ) ___ 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 keys
Szelethus added a comment. In https://reviews.llvm.org/D50488#1204655, @mgrang wrote: > In https://reviews.llvm.org/D50488#1203876, @Szelethus wrote: > > > > > > This is my first time with ASTMatchers and I am not sure how to get the > value_type from hasType (I don't see a matcher for value_types in > ASTMatchers.h). Would I end up using a visitor for that? If yes, then maybe > the entire check for pointer types needs to be done via visitors? Sorry for > the newbie questions :) I played around for a bit, my `clang-query` is a little out of date, but here's what I found: You can match `typedef`s with `typedefDecl()`, and `using` by `typeAliasDecl()`, and then add `hasName("value_type")` as a parameter. As to how to check whether a type has any of these, I'm a little unsure myself, but you could use `hasDescendant`, and narrow down the matches. I'm not sure whether this checks inherited type declarations, and it sure doesn't check template specializations of `std::iterator_traits`, but it is a good start :) I'll revisit this when I have a little more time on my hand. 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] D50451: [ASTImporter] Fix import of class templates partial specialization
martong updated this revision to Diff 161698. martong marked an inline comment as done. martong added a comment. - Change comments Repository: rC Clang https://reviews.llvm.org/D50451 Files: include/clang/ASTMatchers/ASTMatchers.h lib/AST/ASTImporter.cpp lib/ASTMatchers/ASTMatchersInternal.cpp unittests/AST/ASTImporterTest.cpp Index: unittests/AST/ASTImporterTest.cpp === --- unittests/AST/ASTImporterTest.cpp +++ unittests/AST/ASTImporterTest.cpp @@ -2683,6 +2683,263 @@ EXPECT_EQ(FromIndex, 3u); } +TEST_P(ASTImporterTestBase, MergeFieldDeclsOfClassTemplateSpecialization) { + std::string ClassTemplate = + R"( + template + struct X { + int a{0}; // FieldDecl with InitListExpr + X(char) : a(3) {} // (1) + X(int) {} // (2) + }; + )"; + Decl *ToTU = getToTuDecl(ClassTemplate + + R"( + void foo() { + // ClassTemplateSpec with ctor (1): FieldDecl without InitlistExpr + X xc('c'); + } + )", Lang_CXX11); + auto *ToSpec = FirstDeclMatcher().match( + ToTU, classTemplateSpecializationDecl(hasName("X"))); + // FieldDecl without InitlistExpr: + auto *ToField = *ToSpec->field_begin(); + ASSERT_TRUE(ToField); + ASSERT_FALSE(ToField->getInClassInitializer()); + Decl *FromTU = getTuDecl(ClassTemplate + + R"( + void bar() { + // ClassTemplateSpec with ctor (2): FieldDecl WITH InitlistExpr + X xc(1); + } + )", Lang_CXX11); + auto *FromSpec = FirstDeclMatcher().match( + FromTU, classTemplateSpecializationDecl(hasName("X"))); + // FieldDecl with InitlistExpr: + auto *FromField = *FromSpec->field_begin(); + ASSERT_TRUE(FromField); + ASSERT_TRUE(FromField->getInClassInitializer()); + + auto *ImportedSpec = Import(FromSpec, Lang_CXX11); + ASSERT_TRUE(ImportedSpec); + EXPECT_EQ(ImportedSpec, ToSpec); + // After the import, the FieldDecl has to be merged, thus it should have the + // InitListExpr. + EXPECT_TRUE(ToField->getInClassInitializer()); +} + +TEST_P(ASTImporterTestBase, MergeFunctionOfClassTemplateSpecialization) { + std::string ClassTemplate = + R"( + template + struct X { +void f() {} +void g() {} + }; + )"; + Decl *ToTU = getToTuDecl(ClassTemplate + + R"( + void foo() { + X x; + x.f(); + } + )", Lang_CXX11); + Decl *FromTU = getTuDecl(ClassTemplate + + R"( + void bar() { + X x; + x.g(); + } + )", Lang_CXX11); + auto *FromSpec = FirstDeclMatcher().match( + FromTU, classTemplateSpecializationDecl(hasName("X"))); + auto FunPattern = functionDecl(hasName("g"), + hasParent(classTemplateSpecializationDecl())); + auto *FromFun = + FirstDeclMatcher().match(FromTU, FunPattern); + auto *ToFun = + FirstDeclMatcher().match(ToTU, FunPattern); + ASSERT_TRUE(FromFun->hasBody()); + ASSERT_FALSE(ToFun->hasBody()); + auto *ImportedSpec = Import(FromSpec, Lang_CXX11); + ASSERT_TRUE(ImportedSpec); + auto *ToSpec = FirstDeclMatcher().match( + ToTU, classTemplateSpecializationDecl(hasName("X"))); + EXPECT_EQ(ImportedSpec, ToSpec); + EXPECT_TRUE(ToFun->hasBody()); +} + +TEST_P(ASTImporterTestBase, + ODRViolationOfClassTemplateSpecializationsShouldBeReported) { + std::string ClassTemplate = + R"( + template + struct X {}; + )"; + Decl *ToTU = getToTuDecl(ClassTemplate + + R"( + template <> + struct X { + int a; + }; + void foo() { + X x; + } + )", + Lang_CXX11); + Decl *FromTU = getTuDecl(ClassTemplate + + R"( + template <> + struct X { + int b; + }; + void foo() { + X x; + } + )", + Lang_CXX11); + auto *FromSpec = FirstDeclMatcher().match( + FromTU, classTemplateSpecializationDecl(hasName("X"))); + auto *ImportedSpec = Import(FromSpec, Lang_CXX11); + + // We expect one (ODR) warning during the import. + EXPECT_EQ(1u, ToTU->getASTContext().getDiagnostics().getNumWarnings()); + + // The second specialization is different from the first, thus it violates + // ODR, consequently we expect to keep the first specialization only, which is + // already in the "To" context. + EXPECT_TRUE(ImportedSpec); + auto *ToSpec = FirstDeclMatcher().match( + ToTU, classTemplateSpecializationDecl(hasName("X"))); + EXPECT_EQ(ImportedSpec, ToSpec); + EXPECT_EQ(1u, DeclCounter().match( +ToTU, classTemplateSpecializationDecl())); +} + +TEST_P(ASTImporterTestBase, MergeCtorOfClassTemplateSpecialization) { + std::string ClassTemplate = + R"( + template + struct X { + X(char) {} + X(int) {} + }; + )"; + Decl *ToTU = getToTuDecl
[PATCH] D50451: [ASTImporter] Fix import of class templates partial specialization
martong marked an inline comment as done. martong added inline comments. Comment at: lib/AST/ASTImporter.cpp:4550 + // in the "From" context, but not in the "To" context. + for (auto *FromField : D->fields()) +Importer.Import(FromField); a_sidorin wrote: > martong wrote: > > martong wrote: > > > a_sidorin wrote: > > > > Importing additional fields can change the layout of the > > > > specialization. For CSA, this usually results in strange assertions > > > > hard to debug. Could you please share the results of testing of this > > > > change? > > > > This change also doesn't seem to have related tests in this patch. > > > TLDR; We will not create additional fields. > > > > > > By the time when we import the field, we already know that the existing > > > specialization is structurally equivalent with the new one. > > > Since a ClassTemplateSpecializationDecl is the descendant of RecordDecl, > > > the structural equivalence check ensures that they have the exact same > > > fields. > > > When we import the field of the new spec and if there is an existing > > > FieldDecl in the "To" context, then no new FieldDecl will be created > > > (this is handled in `VisitFieldDecl` by first doing a lookup of existing > > > field with the same name and type). > > > This patch extends `VisitFieldDecl` in a way that we add new initializer > > > expressions to the existing FieldDecl, if it didn't have and in the > > > "From" context it has. > > > > > > For the record, I added a new test to make sure that a new FieldDecl > > > will not be created during the merge. > > This is the new test: > > `ODRViolationOfClassTemplateSpecializationsShouldBeReported`. It checks > > that it is not possible to add new fields to a specialization, rather an > > ODR violation is diagnosed. > Thank you for the explanation. However, I find the comment very misleading. > It tells: > ``` > // Check and merge those fields which have been instantiated > // in the "From" context, but not in the "To" context. > ``` > Would it be correct to change it to "Import field initializers that are still > not instantiated", or do I still misunderstand something? Yes, I agree that this comment is not precise and could be misleading, thank you for pointing this out. I changed this comment and others too. Repository: rC Clang https://reviews.llvm.org/D50451 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50451: [ASTImporter] Fix import of class templates partial specialization
martong updated this revision to Diff 161699. martong added a comment. - Add one more TODO to import instantiated exception specifications Repository: rC Clang https://reviews.llvm.org/D50451 Files: include/clang/ASTMatchers/ASTMatchers.h lib/AST/ASTImporter.cpp lib/ASTMatchers/ASTMatchersInternal.cpp unittests/AST/ASTImporterTest.cpp Index: unittests/AST/ASTImporterTest.cpp === --- unittests/AST/ASTImporterTest.cpp +++ unittests/AST/ASTImporterTest.cpp @@ -2683,6 +2683,263 @@ EXPECT_EQ(FromIndex, 3u); } +TEST_P(ASTImporterTestBase, MergeFieldDeclsOfClassTemplateSpecialization) { + std::string ClassTemplate = + R"( + template + struct X { + int a{0}; // FieldDecl with InitListExpr + X(char) : a(3) {} // (1) + X(int) {} // (2) + }; + )"; + Decl *ToTU = getToTuDecl(ClassTemplate + + R"( + void foo() { + // ClassTemplateSpec with ctor (1): FieldDecl without InitlistExpr + X xc('c'); + } + )", Lang_CXX11); + auto *ToSpec = FirstDeclMatcher().match( + ToTU, classTemplateSpecializationDecl(hasName("X"))); + // FieldDecl without InitlistExpr: + auto *ToField = *ToSpec->field_begin(); + ASSERT_TRUE(ToField); + ASSERT_FALSE(ToField->getInClassInitializer()); + Decl *FromTU = getTuDecl(ClassTemplate + + R"( + void bar() { + // ClassTemplateSpec with ctor (2): FieldDecl WITH InitlistExpr + X xc(1); + } + )", Lang_CXX11); + auto *FromSpec = FirstDeclMatcher().match( + FromTU, classTemplateSpecializationDecl(hasName("X"))); + // FieldDecl with InitlistExpr: + auto *FromField = *FromSpec->field_begin(); + ASSERT_TRUE(FromField); + ASSERT_TRUE(FromField->getInClassInitializer()); + + auto *ImportedSpec = Import(FromSpec, Lang_CXX11); + ASSERT_TRUE(ImportedSpec); + EXPECT_EQ(ImportedSpec, ToSpec); + // After the import, the FieldDecl has to be merged, thus it should have the + // InitListExpr. + EXPECT_TRUE(ToField->getInClassInitializer()); +} + +TEST_P(ASTImporterTestBase, MergeFunctionOfClassTemplateSpecialization) { + std::string ClassTemplate = + R"( + template + struct X { +void f() {} +void g() {} + }; + )"; + Decl *ToTU = getToTuDecl(ClassTemplate + + R"( + void foo() { + X x; + x.f(); + } + )", Lang_CXX11); + Decl *FromTU = getTuDecl(ClassTemplate + + R"( + void bar() { + X x; + x.g(); + } + )", Lang_CXX11); + auto *FromSpec = FirstDeclMatcher().match( + FromTU, classTemplateSpecializationDecl(hasName("X"))); + auto FunPattern = functionDecl(hasName("g"), + hasParent(classTemplateSpecializationDecl())); + auto *FromFun = + FirstDeclMatcher().match(FromTU, FunPattern); + auto *ToFun = + FirstDeclMatcher().match(ToTU, FunPattern); + ASSERT_TRUE(FromFun->hasBody()); + ASSERT_FALSE(ToFun->hasBody()); + auto *ImportedSpec = Import(FromSpec, Lang_CXX11); + ASSERT_TRUE(ImportedSpec); + auto *ToSpec = FirstDeclMatcher().match( + ToTU, classTemplateSpecializationDecl(hasName("X"))); + EXPECT_EQ(ImportedSpec, ToSpec); + EXPECT_TRUE(ToFun->hasBody()); +} + +TEST_P(ASTImporterTestBase, + ODRViolationOfClassTemplateSpecializationsShouldBeReported) { + std::string ClassTemplate = + R"( + template + struct X {}; + )"; + Decl *ToTU = getToTuDecl(ClassTemplate + + R"( + template <> + struct X { + int a; + }; + void foo() { + X x; + } + )", + Lang_CXX11); + Decl *FromTU = getTuDecl(ClassTemplate + + R"( + template <> + struct X { + int b; + }; + void foo() { + X x; + } + )", + Lang_CXX11); + auto *FromSpec = FirstDeclMatcher().match( + FromTU, classTemplateSpecializationDecl(hasName("X"))); + auto *ImportedSpec = Import(FromSpec, Lang_CXX11); + + // We expect one (ODR) warning during the import. + EXPECT_EQ(1u, ToTU->getASTContext().getDiagnostics().getNumWarnings()); + + // The second specialization is different from the first, thus it violates + // ODR, consequently we expect to keep the first specialization only, which is + // already in the "To" context. + EXPECT_TRUE(ImportedSpec); + auto *ToSpec = FirstDeclMatcher().match( + ToTU, classTemplateSpecializationDecl(hasName("X"))); + EXPECT_EQ(ImportedSpec, ToSpec); + EXPECT_EQ(1u, DeclCounter().match( +ToTU, classTemplateSpecializationDecl())); +} + +TEST_P(ASTImporterTestBase, MergeCtorOfClassTemplateSpecialization) { + std::string ClassTemplate = + R"( + template + struct X { + X(char) {} + X(int) {} + }; + )"; + Decl *ToTU = get
r340272 - [analyzer][UninitializedObjectChecker] Explicit namespace resolution for inherited data members
Author: szelethus Date: Tue Aug 21 05:16:59 2018 New Revision: 340272 URL: http://llvm.org/viewvc/llvm-project?rev=340272&view=rev Log: [analyzer][UninitializedObjectChecker] Explicit namespace resolution for inherited data members For the following example: struct Base { int x; }; // In a different translation unit struct Derived : public Base { Derived() {} }; For a call to Derived::Derived(), we'll receive a note that this->x is uninitialized. Since x is not a direct field of Derived, it could be a little confusing. This patch aims to fix this, as well as the case when the derived object has a field that has the name as an inherited uninitialized data member: struct Base { int x; // note: uninitialized field 'this->Base::x' }; struct Derived : public Base { int x = 5; Derived() {} }; Differential Revision: https://reviews.llvm.org/D50905 Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp cfe/trunk/test/Analysis/cxx-uninitialized-object-inheritance.cpp Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h?rev=340272&r1=340271&r2=340272&view=diff == --- cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h (original) +++ cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h Tue Aug 21 05:16:59 2018 @@ -32,10 +32,10 @@ class FieldNode { protected: const FieldRegion *FR; - ~FieldNode() = default; + /* non-virtual */ ~FieldNode() = default; public: - FieldNode(const FieldRegion *FR) : FR(FR) { assert(FR); } + FieldNode(const FieldRegion *FR) : FR(FR) {} FieldNode() = delete; FieldNode(const FieldNode &) = delete; @@ -47,11 +47,21 @@ public: /// FoldingSet. void Profile(llvm::FoldingSetNodeID &ID) const { ID.AddPointer(this); } - bool operator<(const FieldNode &Other) const { return FR < Other.FR; } - bool isSameRegion(const FieldRegion *OtherFR) const { return FR == OtherFR; } + // Helper method for uniqueing. + bool isSameRegion(const FieldRegion *OtherFR) const { +// Special FieldNode descendants may wrap nullpointers -- we wouldn't like +// to unique these objects. +if (FR == nullptr) + return false; + +return FR == OtherFR; + } const FieldRegion *getRegion() const { return FR; } - const FieldDecl *getDecl() const { return FR->getDecl(); } + const FieldDecl *getDecl() const { +assert(FR); +return FR->getDecl(); + } // When a fieldchain is printed (a list of FieldNode objects), it will have // the following format: @@ -71,6 +81,8 @@ public: /// Print the separator. For example, fields may be separated with '.' or /// "->". virtual void printSeparator(llvm::raw_ostream &Out) const = 0; + + virtual bool isBase() const { return false; } }; /// Returns with Field's name. This is a helper function to get the correct name @@ -94,15 +106,24 @@ private: FieldChain::Factory &ChainFactory; FieldChain Chain; + FieldChainInfo(FieldChain::Factory &F, FieldChain NewChain) + : FieldChainInfo(F) { +Chain = NewChain; + } + public: FieldChainInfo() = delete; FieldChainInfo(FieldChain::Factory &F) : ChainFactory(F) {} FieldChainInfo(const FieldChainInfo &Other) = default; template FieldChainInfo add(const FieldNodeT &FN); + template FieldChainInfo replaceHead(const FieldNodeT &FN); bool contains(const FieldRegion *FR) const; + bool isEmpty() const { return Chain.isEmpty(); } + const FieldRegion *getUninitRegion() const; + const FieldNode &getHead() { return Chain.getHead(); } void printNoteMsg(llvm::raw_ostream &Out) const; }; @@ -250,6 +271,12 @@ inline FieldChainInfo FieldChainInfo::ad return NewChain; } +template +inline FieldChainInfo FieldChainInfo::replaceHead(const FieldNodeT &FN) { + FieldChainInfo NewChain(ChainFactory, Chain.getTail()); + return NewChain.add(FN); +} + } // end of namespace ento } // end of namespace clang Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp?rev=340272&r1=340271&r2=340272&view=diff == --- cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp (original) +++ cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp Tue Aug 21 05:16:59 2018 @@ -93,6 +93,33 @@ public: } }; +/// Represents that the FieldNode that comes after this is
[PATCH] D50905: [analyzer][UninitializedObjectChecker] Explicit namespace resolution for inherited data members
This revision was automatically updated to reflect the committed changes. Closed by commit rC340272: [analyzer][UninitializedObjectChecker] Explicit namespace resolution for… (authored by Szelethus, committed by ). Changed prior to commit: https://reviews.llvm.org/D50905?vs=161271&id=161700#toc Repository: rC Clang https://reviews.llvm.org/D50905 Files: lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp test/Analysis/cxx-uninitialized-object-inheritance.cpp Index: test/Analysis/cxx-uninitialized-object-inheritance.cpp === --- test/Analysis/cxx-uninitialized-object-inheritance.cpp +++ test/Analysis/cxx-uninitialized-object-inheritance.cpp @@ -35,7 +35,7 @@ } class NonPolymorphicBaseClass2 { - int x; // expected-note{{uninitialized field 'this->x'}} + int x; // expected-note{{uninitialized field 'this->NonPolymorphicBaseClass2::x'}} protected: int y; @@ -62,7 +62,7 @@ int x; protected: - int y; // expected-note{{uninitialized field 'this->y'}} + int y; // expected-note{{uninitialized field 'this->NonPolymorphicBaseClass3::y'}} public: NonPolymorphicBaseClass3() = default; NonPolymorphicBaseClass3(int) : x(7) {} @@ -140,7 +140,7 @@ } class PolymorphicRight1 { - int x; // expected-note{{uninitialized field 'this->x'}} + int x; // expected-note{{uninitialized field 'this->PolymorphicRight1::x'}} protected: int y; @@ -168,7 +168,7 @@ int x; protected: - int y; // expected-note{{uninitialized field 'this->y'}} + int y; // expected-note{{uninitialized field 'this->PolymorphicBaseClass3::y'}} public: virtual ~PolymorphicBaseClass3() = default; PolymorphicBaseClass3() = default; @@ -248,7 +248,7 @@ } class VirtualPolymorphicRight1 { - int x; // expected-note{{uninitialized field 'this->x'}} + int x; // expected-note{{uninitialized field 'this->VirtualPolymorphicRight1::x'}} protected: int y; @@ -276,7 +276,7 @@ int x; protected: - int y; // expected-note{{uninitialized field 'this->y'}} + int y; // expected-note{{uninitialized field 'this->VirtualPolymorphicBaseClass3::y'}} public: virtual ~VirtualPolymorphicBaseClass3() = default; VirtualPolymorphicBaseClass3() = default; @@ -358,7 +358,7 @@ Left2(int) : x(36) {} }; struct Right2 { - int y; // expected-note{{uninitialized field 'this->y'}} + int y; // expected-note{{uninitialized field 'this->Right2::y'}} Right2() = default; Right2(int) : y(37) {} }; @@ -378,7 +378,7 @@ } struct Left3 { - int x; // expected-note{{uninitialized field 'this->x'}} + int x; // expected-note{{uninitialized field 'this->Left3::x'}} Left3() = default; Left3(int) : x(39) {} }; @@ -433,7 +433,7 @@ Left5(int) : x(44) {} }; struct Right5 { - int y; // expected-note{{uninitialized field 'this->y'}} + int y; // expected-note{{uninitialized field 'this->Right5::y'}} Right5() = default; Right5(int) : y(45) {} }; @@ -514,7 +514,7 @@ } struct NonVirtualBase2 { - int x; // expected-note{{uninitialized field 'this->x'}} + int x; // expected-note{{uninitialized field 'this->NonVirtualBase2::x'}} NonVirtualBase2() = default; NonVirtualBase2(int) : x(52) {} }; @@ -542,7 +542,7 @@ } struct NonVirtualBase3 { - int x; // expected-note{{uninitialized field 'this->x'}} + int x; // expected-note{{uninitialized field 'this->NonVirtualBase3::x'}} NonVirtualBase3() = default; NonVirtualBase3(int) : x(54) {} }; @@ -570,8 +570,8 @@ } struct NonVirtualBase4 { - int x; // expected-note{{uninitialized field 'this->x'}} - // expected-note@-1{{uninitialized field 'this->x'}} + int x; // expected-note{{uninitialized field 'this->NonVirtualBase4::x'}} + // expected-note@-1{{uninitialized field 'this->NonVirtualBase4::x'}} NonVirtualBase4() = default; NonVirtualBase4(int) : x(56) {} }; @@ -626,7 +626,7 @@ } struct NonVirtualBase6 { - int x; // expected-note{{uninitialized field 'this->x'}} + int x; // expected-note{{uninitialized field 'this->NonVirtualBase6::x'}} NonVirtualBase6() = default; NonVirtualBase6(int) : x(59) {} }; @@ -712,7 +712,7 @@ } struct VirtualBase2 { - int x; // expected-note{{uninitialized field 'this->x'}} + int x; // expected-note{{uninitialized field 'this->VirtualBase2::x'}} VirtualBase2() = default; VirtualBase2(int) : x(63) {} }; @@ -751,7 +751,7 @@ } struct VirtualBase3 { - int x; // expected-note{{uninitialized field 'this->x'}} + int x; // expected-note{{uninitialized field 'this->VirtualBase3::x'}} VirtualBase3() = default; VirtualBase3(int) : x(66) {} }; Index: lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp === --- lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp +++ lib/StaticAnalyzer/Checkers
[PATCH] D50905: [analyzer][UninitializedObjectChecker] Explicit namespace resolution for inherited data members
This revision was automatically updated to reflect the committed changes. Closed by commit rL340272: [analyzer][UninitializedObjectChecker] Explicit namespace resolution for… (authored by Szelethus, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D50905?vs=161271&id=161701#toc Repository: rL LLVM https://reviews.llvm.org/D50905 Files: cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp cfe/trunk/test/Analysis/cxx-uninitialized-object-inheritance.cpp Index: cfe/trunk/test/Analysis/cxx-uninitialized-object-inheritance.cpp === --- cfe/trunk/test/Analysis/cxx-uninitialized-object-inheritance.cpp +++ cfe/trunk/test/Analysis/cxx-uninitialized-object-inheritance.cpp @@ -35,7 +35,7 @@ } class NonPolymorphicBaseClass2 { - int x; // expected-note{{uninitialized field 'this->x'}} + int x; // expected-note{{uninitialized field 'this->NonPolymorphicBaseClass2::x'}} protected: int y; @@ -62,7 +62,7 @@ int x; protected: - int y; // expected-note{{uninitialized field 'this->y'}} + int y; // expected-note{{uninitialized field 'this->NonPolymorphicBaseClass3::y'}} public: NonPolymorphicBaseClass3() = default; NonPolymorphicBaseClass3(int) : x(7) {} @@ -140,7 +140,7 @@ } class PolymorphicRight1 { - int x; // expected-note{{uninitialized field 'this->x'}} + int x; // expected-note{{uninitialized field 'this->PolymorphicRight1::x'}} protected: int y; @@ -168,7 +168,7 @@ int x; protected: - int y; // expected-note{{uninitialized field 'this->y'}} + int y; // expected-note{{uninitialized field 'this->PolymorphicBaseClass3::y'}} public: virtual ~PolymorphicBaseClass3() = default; PolymorphicBaseClass3() = default; @@ -248,7 +248,7 @@ } class VirtualPolymorphicRight1 { - int x; // expected-note{{uninitialized field 'this->x'}} + int x; // expected-note{{uninitialized field 'this->VirtualPolymorphicRight1::x'}} protected: int y; @@ -276,7 +276,7 @@ int x; protected: - int y; // expected-note{{uninitialized field 'this->y'}} + int y; // expected-note{{uninitialized field 'this->VirtualPolymorphicBaseClass3::y'}} public: virtual ~VirtualPolymorphicBaseClass3() = default; VirtualPolymorphicBaseClass3() = default; @@ -358,7 +358,7 @@ Left2(int) : x(36) {} }; struct Right2 { - int y; // expected-note{{uninitialized field 'this->y'}} + int y; // expected-note{{uninitialized field 'this->Right2::y'}} Right2() = default; Right2(int) : y(37) {} }; @@ -378,7 +378,7 @@ } struct Left3 { - int x; // expected-note{{uninitialized field 'this->x'}} + int x; // expected-note{{uninitialized field 'this->Left3::x'}} Left3() = default; Left3(int) : x(39) {} }; @@ -433,7 +433,7 @@ Left5(int) : x(44) {} }; struct Right5 { - int y; // expected-note{{uninitialized field 'this->y'}} + int y; // expected-note{{uninitialized field 'this->Right5::y'}} Right5() = default; Right5(int) : y(45) {} }; @@ -514,7 +514,7 @@ } struct NonVirtualBase2 { - int x; // expected-note{{uninitialized field 'this->x'}} + int x; // expected-note{{uninitialized field 'this->NonVirtualBase2::x'}} NonVirtualBase2() = default; NonVirtualBase2(int) : x(52) {} }; @@ -542,7 +542,7 @@ } struct NonVirtualBase3 { - int x; // expected-note{{uninitialized field 'this->x'}} + int x; // expected-note{{uninitialized field 'this->NonVirtualBase3::x'}} NonVirtualBase3() = default; NonVirtualBase3(int) : x(54) {} }; @@ -570,8 +570,8 @@ } struct NonVirtualBase4 { - int x; // expected-note{{uninitialized field 'this->x'}} - // expected-note@-1{{uninitialized field 'this->x'}} + int x; // expected-note{{uninitialized field 'this->NonVirtualBase4::x'}} + // expected-note@-1{{uninitialized field 'this->NonVirtualBase4::x'}} NonVirtualBase4() = default; NonVirtualBase4(int) : x(56) {} }; @@ -626,7 +626,7 @@ } struct NonVirtualBase6 { - int x; // expected-note{{uninitialized field 'this->x'}} + int x; // expected-note{{uninitialized field 'this->NonVirtualBase6::x'}} NonVirtualBase6() = default; NonVirtualBase6(int) : x(59) {} }; @@ -712,7 +712,7 @@ } struct VirtualBase2 { - int x; // expected-note{{uninitialized field 'this->x'}} + int x; // expected-note{{uninitialized field 'this->VirtualBase2::x'}} VirtualBase2() = default; VirtualBase2(int) : x(63) {} }; @@ -751,7 +751,7 @@ } struct VirtualBase3 { - int x; // expected-note{{uninitialized field 'this->x'}} + int x; // expected-note{{uninitialized field 'this->VirtualBase3::x'}} VirtualBase3() = default; VirtualBase3(int) : x(66) {} }; Index: cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp === --
[PATCH] D51001: [ASTImporter] Add test for CXXForRangeStmt
martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. Herald added a subscriber: rnkovacs. LGTM. Repository: rC Clang https://reviews.llvm.org/D51001 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51026: [CodeGen] Implicitly set stackrealign on the main function, if custom stack alignment is used
erichkeane added inline comments. Comment at: lib/CodeGen/CodeGenFunction.cpp:989 +CGM.getCodeGenOpts().StackAlignment) + Fn->addFnAttr("stackrealign"); + Is there not an attribute name for this already in LLVM? I guess I'm otherwise fine with this, but would want @craig.topper or @rnk to confirm that we're Ok just sending this string as an attribute. Repository: rC Clang https://reviews.llvm.org/D51026 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51026: [CodeGen] Implicitly set stackrealign on the main function, if custom stack alignment is used
mstorsjo added inline comments. Comment at: lib/CodeGen/CodeGenFunction.cpp:989 +CGM.getCodeGenOpts().StackAlignment) + Fn->addFnAttr("stackrealign"); + erichkeane wrote: > Is there not an attribute name for this already in LLVM? I guess I'm > otherwise fine with this, but would want @craig.topper or @rnk to confirm > that we're Ok just sending this string as an attribute. I don't see one in llvm/include/llvm/IR/Attributes.td at least, and all other occurrances in clang just use the plain string. Repository: rC Clang https://reviews.llvm.org/D51026 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50993: [clangd] Increase stack size of the new threads on macOS
Dmitry.Kozhevnikov added a comment. In https://reviews.llvm.org/D50993#1207357, @ilya-biryukov wrote: > 1. Can we put the helper that executes the tasks asynchronously with a stack > size into llvm's threading library (i.e. somewhere close to > `llvm::execute_on_thread`?) All platform-specific code is already there, we > can reuse most of the implementation too. Do you think it would be OK to have pthreads-only version there, with `std::thread` fallback on Windows? I'd like to avoid writing a Windows-only implementation myself, as it's a bit alien for me. > 2. I wonder if we should expose the stack size as an option to clangd? Do you expect it would be ever used? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50993 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50557: [clang][mips] Set __mips_fpr correctly for -mfpxx
atanasyan accepted this revision. atanasyan added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D50557 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50564: Add support for SEH unwinding on Windows.
kristina added a comment. In https://reviews.llvm.org/D50564#1206393, @mstorsjo wrote: > In https://reviews.llvm.org/D50564#1206370, @cdavis5x wrote: > > > In https://reviews.llvm.org/D50564#1206302, @kristina wrote: > > > > > I'm all for this change except the core issue is that you're using > > > libunwind as a shim around the actual unwinding API provided by Windows. > > > It would be nice to have something that did not have to do that and was > > > capable of performing unwinding of SEH-style exceptions without needing > > > additional runtime support. > > > > > > It would be nice, but that would require extra work. We'd have to implement > > reading and interpreting unwind codes, and calling any handlers present at > > each frame (which all have a different calling convention from Itanium > > handlers), and handling chained unwind info... Or we could use the > > implementation that MS provided to us for free--and which gets loaded into > > every process anyway by virtue of being in NTDLL, and which is extremely > > well tested. Given all that, I'm wondering what implementing all that > > ourselves would gain us. I suppose we could eventually do all that, but for > > now, I think this is outside the scope of my change. > > > +1. I guess such a library would be very nice to have, but from the point of > view of implementing exception handling, using the underlying APIs probably > is the way to go. The other question, as posted before, is whether we want to > wrap the whole CONTEXT structs in the UnwindCursor class and expose it via > the unw_* set of APIs. It gives quite a significant amount of extra code here > compared to libgcc's unwind-seh.c which is <500 loc altogether, providing > only the _Unwind_* API, implementing it directly with the Windows Rtl* APIs > from ntdll. That would give only a partial libunwind, with only the higher > level API available, but that's the only part used for exception handling at > least. That still makes the underlying assumption that SEH is exclusive to Windows (and by that virtue PE/COFF) which doesn't necessarily hold true. There is also the issue of generic names like `_LIBUNWIND_SUPPORT_SEH_UNWIND` to guard includes of Windows specific headers which ties into my previous point. Would it be possible to somehow abstract away the Windows runtime function call and Windows specific header includes, even if it's via CMake options. I'm raising this issue as there are other implementations of SEH that share the same (or extremely similar, depending on SDK versions) structures, but would not have the Windows headers available when compiling libunwind, and while the runtime function call to the `Rtl*` API is easy to change later on, separating rest of it from Windows headers is slightly messier. Would it, at very least, be possible to decouple most of it from a dependency on Windows headers? Would be much appreciated, as I'm going through a backlog of patches I'd like to put up for review one of which includes SEH for x86_64 Linux (ELF) implemented using RT signals and additional compiler runtime glue code, currently residing within compiler-rt builtins (one of the issues I want to address before). It's quite a wonky ABI since I tried to keep as much compatible with 64-bit Windows SEH (`.xdata` variation) in terms of frame lowering etc, but run-time mechanisms differ vastly for obvious reasons. Repository: rUNW libunwind https://reviews.llvm.org/D50564 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50993: [clangd] Increase stack size of the new threads on macOS
ilya-biryukov added a comment. In https://reviews.llvm.org/D50993#1207464, @Dmitry.Kozhevnikov wrote: > Do you think it would be OK to have pthreads-only version there, with > `std::thread` fallback on Windows? I'd like to avoid writing a Windows-only > implementation myself, as it's a bit alien for me. Yeah, we should probably do both Unix and Windows there. My guess is that the Windows implementation shouldn't be too hard, since most of the code is already there. In any way, it's fine to start with a Unix-only version and a stub in Windows and figuring out the implementation during the review. Happy to look into Windows myself, it shouldn't be too hard :-) >> 2. I wonder if we should expose the stack size as an option to clangd? > > Do you expect it would be ever used? Not frequently, but I don't see how we can figure out good defaults there. It's one of those things where the guideline is "as small as possible, as long as it does not crash clangd". An option could provide a simple workaround for cases when clangd is actually crashing. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50993 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51020: MultiSource/Benchmarks/DOE-ProxyApps-C++/CLAMP: Fix build with newer libstdc++
homerdin accepted this revision. homerdin added a comment. This revision is now accepted and ready to land. LGTM. Thanks for fixing this Repository: rT test-suite https://reviews.llvm.org/D51020 ___ 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 created this revision. Herald added a subscriber: cfe-commits. 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. Repository: rC Clang https://reviews.llvm.org/D51036 Files: lib/Format/Format.cpp lib/Format/FormatToken.h lib/Format/NamespaceEndCommentsFixer.cpp lib/Format/UnwrappedLineFormatter.cpp lib/Format/UnwrappedLineParser.cpp unittests/Format/FormatTest.cpp Index: unittests/Format/FormatTest.cpp === --- unittests/Format/FormatTest.cpp +++ unittests/Format/FormatTest.cpp @@ -200,6 +200,24 @@ "inti;\n" "}", getGoogleStyle())); + EXPECT_EQ("inline namespace N {\n" +"\n" +"int i;\n" +"}", +format("inline namespace N {\n" + "\n" + "inti;\n" + "}", + getGoogleStyle())); + EXPECT_EQ("export namespace N {\n" +"\n" +"int i;\n" +"}", +format("export namespace N {\n" + "\n" + "inti;\n" + "}", + getGoogleStyle())); EXPECT_EQ("extern /**/ \"C\" /**/ {\n" "\n" "int i;\n" @@ -1573,6 +1591,11 @@ "void f() { f(); }\n" "}", LLVMWithNoNamespaceFix); + verifyFormat("export namespace X {\n" + "class A {};\n" + "void f() { f(); }\n" + "}", + LLVMWithNoNamespaceFix); verifyFormat("using namespace some_namespace;\n" "class A {};\n" "void f() { f(); }", @@ -7556,6 +7579,9 @@ verifyFormat("inline namespace Foo\n" "{};", Style); + verifyFormat("export namespace Foo\n" + "{};", + Style); verifyFormat("namespace Foo\n" "{\n" "void Bar();\n" Index: lib/Format/UnwrappedLineParser.cpp === --- lib/Format/UnwrappedLineParser.cpp +++ lib/Format/UnwrappedLineParser.cpp @@ -1063,6 +1063,13 @@ parseJavaScriptEs6ImportExport(); return; } +if (Style.isCpp()) { + nextToken(); + if (FormatTok->Tok.is(tok::kw_namespace)) { +parseNamespace(); +return; + } +} break; case tok::identifier: if (FormatTok->is(TT_ForEachMacro)) { Index: lib/Format/UnwrappedLineFormatter.cpp === --- lib/Format/UnwrappedLineFormatter.cpp +++ lib/Format/UnwrappedLineFormatter.cpp @@ -529,7 +529,10 @@ Tok->SpacesRequiredBefore = 0; Tok->CanBreakBefore = true; return 1; - } else if (Limit != 0 && !Line.startsWith(tok::kw_namespace) && + } else if (Limit != 0 && + !(Line.startsWith(tok::kw_namespace) || + Line.startsWith(tok::kw_inline, tok::kw_namespace) || + Line.startsWith(tok::kw_export, tok::kw_namespace)) && !startsExternCBlock(Line)) { // We don't merge short records. FormatToken *RecordTok = Line.First; @@ -1154,7 +1157,9 @@ // Remove empty lines after "{". if (!Style.KeepEmptyLinesAtTheStartOfBlocks && PreviousLine && PreviousLine->Last->is(tok::l_brace) && - PreviousLine->First->isNot(tok::kw_namespace) && + !(PreviousLine->startsWith(tok::kw_namespace) || +PreviousLine->startsWith(tok::kw_inline, tok::kw_namespace) || +PreviousLine->startsWith(tok::kw_export, tok::kw_namespace)) && !startsExternCBlock(*PreviousLine)) Newlines = 1; Index: lib/Format/NamespaceEndCommentsFixer.cpp === --- lib/Format/NamespaceEndCommentsFixer.cpp +++ lib/Format/NamespaceEndCommentsFixer.cpp @@ -125,8 +125,8 @@ if (StartLineIndex > 0) NamespaceTok = AnnotatedLines[StartLineIndex - 1]->First; } - // Detect "(inline)? namespace" in the beginning of a line. - if (NamespaceTok->is(tok::kw_inline)) + // Detect "(inline|export)? namespace" in the beginning of a line. + if (NamespaceTok->is(tok::kw_inline) || NamespaceTok->is(tok::kw_export)) NamespaceTok = NamespaceTok->getNextNonComment(); if (!NamespaceTok || NamespaceTok->isNot(tok::kw_namespace)) return nullptr; Index: lib/Format/FormatToken.h ==
[PATCH] D47814: Teach libc++ to use native NetBSD's max_align_t
ldionne added inline comments. Comment at: include/stddef.h:55 // Re-use the compiler's max_align_t where possible. #if !defined(__CLANG_MAX_ALIGN_T_DEFINED) && !defined(_GCC_MAX_ALIGN_T) && \ chandlerc wrote: > Unrelated to your patch, but this comment is now amazingly out of place. Nope, based on `git blame` I think it's still where it should be. Repository: rCXX libc++ https://reviews.llvm.org/D47814 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check
hugoeg updated this revision to Diff 161714. hugoeg marked an inline comment as done. hugoeg added a comment. All comments have been address and documentation has been added to matcher https://reviews.llvm.org/D50542 Files: clang-tidy/abseil/AbseilTidyModule.cpp clang-tidy/abseil/CMakeLists.txt clang-tidy/abseil/NoInternalDepsCheck.cpp clang-tidy/abseil/NoInternalDepsCheck.h docs/ReleaseNotes.rst docs/clang-tidy/checks/abseil-no-internal-deps.rst docs/clang-tidy/checks/list.rst test/clang-tidy/Inputs/absl/external-file.h test/clang-tidy/Inputs/absl/strings/internal-file.h test/clang-tidy/abseil-no-internal-deps.cpp Index: test/clang-tidy/abseil-no-internal-deps.cpp === --- test/clang-tidy/abseil-no-internal-deps.cpp +++ test/clang-tidy/abseil-no-internal-deps.cpp @@ -0,0 +1,39 @@ +// RUN: %check_clang_tidy %s abseil-no-internal-deps %t, -- -- -I %S/Inputs +// RUN: clang-tidy -checks='-*, abseil-no-internal-deps' -header-filter='.*' %s -- -I %S/Inputs 2>&1 | FileCheck %s + +#include "absl/strings/internal-file.h" +// CHECK-NOT: warning: + +#include "absl/external-file.h" +// CHECK: absl/external-file.h:1:23: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil [abseil-no-internal-deps] + +void DirectAcess() { + absl::strings_internal::InternalFunction(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil + + absl::strings_internal::InternalTemplateFunction("a"); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil +} + +class FriendUsage { + friend struct absl::container_internal::InternalStruct; + // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil +}; + +namespace absl { +void OpeningNamespace() { + strings_internal::InternalFunction(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil +} +} // namespace absl + +// should not trigger warnings +void CorrectUsage() { + std::string Str = absl::StringsFunction("a"); + absl::SomeContainer b; +} + +namespace absl { +SomeContainer b; +std::string Str = absl::StringsFunction("a"); +} // namespace absl Index: test/clang-tidy/Inputs/absl/strings/internal-file.h === --- test/clang-tidy/Inputs/absl/strings/internal-file.h +++ test/clang-tidy/Inputs/absl/strings/internal-file.h @@ -0,0 +1,33 @@ +namespace std { +struct string { + string(const char *); + ~string(); +}; +} // namespace std + +namespace absl { +std::string StringsFunction(std::string s1) { return s1; } +class SomeContainer {}; +namespace strings_internal { +void InternalFunction() {} +template P InternalTemplateFunction(P a) {} +} // namespace strings_internal + +namespace container_internal { +struct InternalStruct {}; +} // namespace container_internal +} // namespace absl + +// should not trigger warnings because inside Abseil files +void DirectAcessInternal() { + absl::strings_internal::InternalFunction(); + absl::strings_internal::InternalTemplateFunction("a"); +} + +class FriendUsageInternal { + friend struct absl::container_internal::InternalStruct; +}; + +namespace absl { +void OpeningNamespaceInternally() { strings_internal::InternalFunction(); } +} // namespace absl Index: test/clang-tidy/Inputs/absl/external-file.h === --- test/clang-tidy/Inputs/absl/external-file.h +++ test/clang-tidy/Inputs/absl/external-file.h @@ -0,0 +1 @@ +void DirectAcess2() { absl::strings_internal::InternalFunction(); } Index: docs/clang-tidy/checks/list.rst === --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -4,6 +4,7 @@ = .. toctree:: + abseil-no-internal-deps abseil-string-find-startswith android-cloexec-accept android-cloexec-accept4 Index: docs/clang-tidy/checks/abseil-no-internal-deps.rst === --- docs/clang-tidy/checks/abseil-no-internal-deps.rst +++ docs/clang-tidy/checks/abseil-no-internal-deps.rst @@ -0,0 +1,23 @@ +subl.. title:: clang-tidy - abseil-no-internal-deps + +abseil-no-internal-deps +=== + +Warns if code using Abseil depends on internal details. If something is in a +namespace that includes the word “internal”, code is not allowed to depend upon +it beaucse it’s an implementation detail. They cannot friend it, include it, +you mention it or refer to it in any way. Doing so violates Abseil's +compatibility guidelines and may result in breakage. See +https://abse
r340277 - [ASTImporter] Adding some friend function related unittests.
Author: balazske Date: Tue Aug 21 07:32:21 2018 New Revision: 340277 URL: http://llvm.org/viewvc/llvm-project?rev=340277&view=rev Log: [ASTImporter] Adding some friend function related unittests. Reviewers: a.sidorin, a_sidorin Reviewed By: a_sidorin Subscribers: a_sidorin, martong, cfe-commits Differential Revision: https://reviews.llvm.org/D49798 Modified: cfe/trunk/unittests/AST/ASTImporterTest.cpp Modified: cfe/trunk/unittests/AST/ASTImporterTest.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/AST/ASTImporterTest.cpp?rev=340277&r1=340276&r2=340277&view=diff == --- cfe/trunk/unittests/AST/ASTImporterTest.cpp (original) +++ cfe/trunk/unittests/AST/ASTImporterTest.cpp Tue Aug 21 07:32:21 2018 @@ -2274,6 +2274,211 @@ TEST_P(ImportFriendFunctions, ImportFrie EXPECT_EQ(ImportedD1->getPreviousDecl(), ImportedD); } +TEST_P(ImportFriendFunctions, Lookup) { + auto FunctionPattern = functionDecl(hasName("f")); + auto ClassPattern = cxxRecordDecl(hasName("X")); + + TranslationUnitDecl *FromTU = + getTuDecl("struct X { friend void f(); };", Lang_CXX, "input0.cc"); + auto *FromD = FirstDeclMatcher().match(FromTU, FunctionPattern); + ASSERT_TRUE(FromD->isInIdentifierNamespace(Decl::IDNS_OrdinaryFriend)); + ASSERT_FALSE(FromD->isInIdentifierNamespace(Decl::IDNS_Ordinary)); + { +auto FromName = FromD->getDeclName(); +auto *Class = FirstDeclMatcher().match(FromTU, ClassPattern); +auto LookupRes = Class->noload_lookup(FromName); +ASSERT_EQ(LookupRes.size(), 0u); +LookupRes = FromTU->noload_lookup(FromName); +ASSERT_EQ(LookupRes.size(), 1u); + } + + auto *ToD = cast(Import(FromD, Lang_CXX)); + auto ToName = ToD->getDeclName(); + + TranslationUnitDecl *ToTU = ToAST->getASTContext().getTranslationUnitDecl(); + auto *Class = FirstDeclMatcher().match(ToTU, ClassPattern); + auto LookupRes = Class->noload_lookup(ToName); + EXPECT_EQ(LookupRes.size(), 0u); + LookupRes = ToTU->noload_lookup(ToName); + EXPECT_EQ(LookupRes.size(), 1u); + + EXPECT_EQ(DeclCounter().match(ToTU, FunctionPattern), 1u); + auto *To0 = FirstDeclMatcher().match(ToTU, FunctionPattern); + EXPECT_TRUE(To0->isInIdentifierNamespace(Decl::IDNS_OrdinaryFriend)); + EXPECT_FALSE(To0->isInIdentifierNamespace(Decl::IDNS_Ordinary)); +} + +TEST_P(ImportFriendFunctions, DISABLED_LookupWithProtoAfter) { + auto FunctionPattern = functionDecl(hasName("f")); + auto ClassPattern = cxxRecordDecl(hasName("X")); + + TranslationUnitDecl *FromTU = getTuDecl( + "struct X { friend void f(); };" + // This proto decl makes f available to normal + // lookup, otherwise it is hidden. + // Normal C++ lookup (implemented in + // `clang::Sema::CppLookupName()` and in `LookupDirect()`) + // returns the found `NamedDecl` only if the set IDNS is matched + "void f();", + Lang_CXX, "input0.cc"); + auto *FromFriend = + FirstDeclMatcher().match(FromTU, FunctionPattern); + auto *FromNormal = + LastDeclMatcher().match(FromTU, FunctionPattern); + ASSERT_TRUE(FromFriend->isInIdentifierNamespace(Decl::IDNS_OrdinaryFriend)); + ASSERT_FALSE(FromFriend->isInIdentifierNamespace(Decl::IDNS_Ordinary)); + ASSERT_FALSE(FromNormal->isInIdentifierNamespace(Decl::IDNS_OrdinaryFriend)); + ASSERT_TRUE(FromNormal->isInIdentifierNamespace(Decl::IDNS_Ordinary)); + + auto FromName = FromFriend->getDeclName(); + auto *FromClass = + FirstDeclMatcher().match(FromTU, ClassPattern); + auto LookupRes = FromClass->noload_lookup(FromName); + ASSERT_EQ(LookupRes.size(), 0u); + LookupRes = FromTU->noload_lookup(FromName); + ASSERT_EQ(LookupRes.size(), 1u); + + auto *ToFriend = cast(Import(FromFriend, Lang_CXX)); + auto ToName = ToFriend->getDeclName(); + + TranslationUnitDecl *ToTU = ToAST->getASTContext().getTranslationUnitDecl(); + auto *ToClass = FirstDeclMatcher().match(ToTU, ClassPattern); + LookupRes = ToClass->noload_lookup(ToName); + EXPECT_EQ(LookupRes.size(), 0u); + LookupRes = ToTU->noload_lookup(ToName); + // Test is disabled because this result is 2. + EXPECT_EQ(LookupRes.size(), 1u); + + ASSERT_EQ(DeclCounter().match(ToTU, FunctionPattern), 2u); + ToFriend = FirstDeclMatcher().match(ToTU, FunctionPattern); + auto *ToNormal = LastDeclMatcher().match(ToTU, FunctionPattern); + EXPECT_TRUE(ToFriend->isInIdentifierNamespace(Decl::IDNS_OrdinaryFriend)); + EXPECT_FALSE(ToFriend->isInIdentifierNamespace(Decl::IDNS_Ordinary)); + EXPECT_FALSE(ToNormal->isInIdentifierNamespace(Decl::IDNS_OrdinaryFriend)); + EXPECT_TRUE(ToNormal->isInIdentifierNamespace(Decl::IDNS_Ordinary)); +} + +TEST_P(ImportFriendFunctions, LookupWithProtoBefore) { + auto FunctionPattern = functionDecl(hasName("f")); + auto ClassPattern = cxxRecordDecl(hasName("X")); + + TranslationUnitDecl *FromTU = getTuDecl( + "void f();" + "struct X { friend void f(); };", + Lang_CXX, "input0.cc"); + auto
[PATCH] D49798: [ASTImporter] Adding some friend function related unittests.
This revision was automatically updated to reflect the committed changes. Closed by commit rC340277: [ASTImporter] Adding some friend function related unittests. (authored by balazske, committed by ). Changed prior to commit: https://reviews.llvm.org/D49798?vs=160972&id=161715#toc Repository: rC Clang https://reviews.llvm.org/D49798 Files: unittests/AST/ASTImporterTest.cpp Index: unittests/AST/ASTImporterTest.cpp === --- unittests/AST/ASTImporterTest.cpp +++ unittests/AST/ASTImporterTest.cpp @@ -2274,6 +2274,211 @@ EXPECT_EQ(ImportedD1->getPreviousDecl(), ImportedD); } +TEST_P(ImportFriendFunctions, Lookup) { + auto FunctionPattern = functionDecl(hasName("f")); + auto ClassPattern = cxxRecordDecl(hasName("X")); + + TranslationUnitDecl *FromTU = + getTuDecl("struct X { friend void f(); };", Lang_CXX, "input0.cc"); + auto *FromD = FirstDeclMatcher().match(FromTU, FunctionPattern); + ASSERT_TRUE(FromD->isInIdentifierNamespace(Decl::IDNS_OrdinaryFriend)); + ASSERT_FALSE(FromD->isInIdentifierNamespace(Decl::IDNS_Ordinary)); + { +auto FromName = FromD->getDeclName(); +auto *Class = FirstDeclMatcher().match(FromTU, ClassPattern); +auto LookupRes = Class->noload_lookup(FromName); +ASSERT_EQ(LookupRes.size(), 0u); +LookupRes = FromTU->noload_lookup(FromName); +ASSERT_EQ(LookupRes.size(), 1u); + } + + auto *ToD = cast(Import(FromD, Lang_CXX)); + auto ToName = ToD->getDeclName(); + + TranslationUnitDecl *ToTU = ToAST->getASTContext().getTranslationUnitDecl(); + auto *Class = FirstDeclMatcher().match(ToTU, ClassPattern); + auto LookupRes = Class->noload_lookup(ToName); + EXPECT_EQ(LookupRes.size(), 0u); + LookupRes = ToTU->noload_lookup(ToName); + EXPECT_EQ(LookupRes.size(), 1u); + + EXPECT_EQ(DeclCounter().match(ToTU, FunctionPattern), 1u); + auto *To0 = FirstDeclMatcher().match(ToTU, FunctionPattern); + EXPECT_TRUE(To0->isInIdentifierNamespace(Decl::IDNS_OrdinaryFriend)); + EXPECT_FALSE(To0->isInIdentifierNamespace(Decl::IDNS_Ordinary)); +} + +TEST_P(ImportFriendFunctions, DISABLED_LookupWithProtoAfter) { + auto FunctionPattern = functionDecl(hasName("f")); + auto ClassPattern = cxxRecordDecl(hasName("X")); + + TranslationUnitDecl *FromTU = getTuDecl( + "struct X { friend void f(); };" + // This proto decl makes f available to normal + // lookup, otherwise it is hidden. + // Normal C++ lookup (implemented in + // `clang::Sema::CppLookupName()` and in `LookupDirect()`) + // returns the found `NamedDecl` only if the set IDNS is matched + "void f();", + Lang_CXX, "input0.cc"); + auto *FromFriend = + FirstDeclMatcher().match(FromTU, FunctionPattern); + auto *FromNormal = + LastDeclMatcher().match(FromTU, FunctionPattern); + ASSERT_TRUE(FromFriend->isInIdentifierNamespace(Decl::IDNS_OrdinaryFriend)); + ASSERT_FALSE(FromFriend->isInIdentifierNamespace(Decl::IDNS_Ordinary)); + ASSERT_FALSE(FromNormal->isInIdentifierNamespace(Decl::IDNS_OrdinaryFriend)); + ASSERT_TRUE(FromNormal->isInIdentifierNamespace(Decl::IDNS_Ordinary)); + + auto FromName = FromFriend->getDeclName(); + auto *FromClass = + FirstDeclMatcher().match(FromTU, ClassPattern); + auto LookupRes = FromClass->noload_lookup(FromName); + ASSERT_EQ(LookupRes.size(), 0u); + LookupRes = FromTU->noload_lookup(FromName); + ASSERT_EQ(LookupRes.size(), 1u); + + auto *ToFriend = cast(Import(FromFriend, Lang_CXX)); + auto ToName = ToFriend->getDeclName(); + + TranslationUnitDecl *ToTU = ToAST->getASTContext().getTranslationUnitDecl(); + auto *ToClass = FirstDeclMatcher().match(ToTU, ClassPattern); + LookupRes = ToClass->noload_lookup(ToName); + EXPECT_EQ(LookupRes.size(), 0u); + LookupRes = ToTU->noload_lookup(ToName); + // Test is disabled because this result is 2. + EXPECT_EQ(LookupRes.size(), 1u); + + ASSERT_EQ(DeclCounter().match(ToTU, FunctionPattern), 2u); + ToFriend = FirstDeclMatcher().match(ToTU, FunctionPattern); + auto *ToNormal = LastDeclMatcher().match(ToTU, FunctionPattern); + EXPECT_TRUE(ToFriend->isInIdentifierNamespace(Decl::IDNS_OrdinaryFriend)); + EXPECT_FALSE(ToFriend->isInIdentifierNamespace(Decl::IDNS_Ordinary)); + EXPECT_FALSE(ToNormal->isInIdentifierNamespace(Decl::IDNS_OrdinaryFriend)); + EXPECT_TRUE(ToNormal->isInIdentifierNamespace(Decl::IDNS_Ordinary)); +} + +TEST_P(ImportFriendFunctions, LookupWithProtoBefore) { + auto FunctionPattern = functionDecl(hasName("f")); + auto ClassPattern = cxxRecordDecl(hasName("X")); + + TranslationUnitDecl *FromTU = getTuDecl( + "void f();" + "struct X { friend void f(); };", + Lang_CXX, "input0.cc"); + auto *FromNormal = + FirstDeclMatcher().match(FromTU, FunctionPattern); + auto *FromFriend = + LastDeclMatcher().match(FromTU, FunctionPattern); + ASSERT_FALSE(FromNormal->isInIdentifierNamespace(Decl::IDNS_OrdinaryFriend)); + ASSERT_TRUE(FromNormal->isInIdentifierNamesp
[PATCH] D50847: [clangd] Add callbacks on parsed AST in addition to parsed preambles
ilya-biryukov updated this revision to Diff 161717. ilya-biryukov added a comment. - Provide noop callbacks implementation by default. - Update docs. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50847 Files: clangd/ClangdServer.cpp clangd/ClangdServer.h clangd/TUScheduler.cpp clangd/TUScheduler.h unittests/clangd/TUSchedulerTests.cpp Index: unittests/clangd/TUSchedulerTests.cpp === --- unittests/clangd/TUSchedulerTests.cpp +++ unittests/clangd/TUSchedulerTests.cpp @@ -17,10 +17,11 @@ namespace clang { namespace clangd { +namespace { using ::testing::_; -using ::testing::Each; using ::testing::AnyOf; +using ::testing::Each; using ::testing::Pair; using ::testing::Pointee; using ::testing::UnorderedElementsAre; @@ -44,8 +45,7 @@ TEST_F(TUSchedulerTests, MissingFiles) { TUScheduler S(getDefaultAsyncThreadsCount(), -/*StorePreamblesInMemory=*/true, -/*PreambleParsedCallback=*/nullptr, +/*StorePreamblesInMemory=*/true, noopParsingCallbacks(), /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(), ASTRetentionPolicy()); @@ -101,8 +101,7 @@ Notification Ready; TUScheduler S( getDefaultAsyncThreadsCount(), -/*StorePreamblesInMemory=*/true, -/*PreambleParsedCallback=*/nullptr, +/*StorePreamblesInMemory=*/true, noopParsingCallbacks(), /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(), ASTRetentionPolicy()); auto Path = testPath("foo.cpp"); @@ -130,8 +129,7 @@ std::atomic CallbackCount(0); { TUScheduler S(getDefaultAsyncThreadsCount(), - /*StorePreamblesInMemory=*/true, - /*PreambleParsedCallback=*/nullptr, + /*StorePreamblesInMemory=*/true, noopParsingCallbacks(), /*UpdateDebounce=*/std::chrono::seconds(1), ASTRetentionPolicy()); // FIXME: we could probably use timeouts lower than 1 second here. @@ -162,8 +160,7 @@ // Run TUScheduler and collect some stats. { TUScheduler S(getDefaultAsyncThreadsCount(), - /*StorePreamblesInMemory=*/true, - /*PreambleParsedCallback=*/nullptr, + /*StorePreamblesInMemory=*/true, noopParsingCallbacks(), /*UpdateDebounce=*/std::chrono::milliseconds(50), ASTRetentionPolicy()); @@ -261,7 +258,7 @@ Policy.MaxRetainedASTs = 2; TUScheduler S( /*AsyncThreadsCount=*/1, /*StorePreambleInMemory=*/true, - PreambleParsedCallback(), + noopParsingCallbacks(), /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(), Policy); llvm::StringLiteral SourceContents = R"cpp( @@ -313,7 +310,7 @@ // the same time. All reads should get the same non-null preamble. TUScheduler S( /*AsyncThreadsCount=*/4, /*StorePreambleInMemory=*/true, - PreambleParsedCallback(), + noopParsingCallbacks(), /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(), ASTRetentionPolicy()); auto Foo = testPath("foo.cpp"); @@ -346,7 +343,7 @@ TEST_F(TUSchedulerTests, NoopOnEmptyChanges) { TUScheduler S( /*AsyncThreadsCount=*/getDefaultAsyncThreadsCount(), - /*StorePreambleInMemory=*/true, PreambleParsedCallback(), + /*StorePreambleInMemory=*/true, noopParsingCallbacks(), /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(), ASTRetentionPolicy()); @@ -399,7 +396,7 @@ TEST_F(TUSchedulerTests, NoChangeDiags) { TUScheduler S( /*AsyncThreadsCount=*/getDefaultAsyncThreadsCount(), - /*StorePreambleInMemory=*/true, PreambleParsedCallback(), + /*StorePreambleInMemory=*/true, noopParsingCallbacks(), /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(), ASTRetentionPolicy()); @@ -430,5 +427,6 @@ ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(1))); } +} // namespace } // namespace clangd } // namespace clang Index: clangd/TUScheduler.h === --- clangd/TUScheduler.h +++ clangd/TUScheduler.h @@ -51,6 +51,30 @@ unsigned MaxRetainedASTs = 3; }; +class ParsingCallbacks { +public: + virtual ~ParsingCallbacks() = default; + + /// Called on the AST that was built for emitting the preamble. The built AST + /// contains only AST nodes from the #include directives at the start of the + /// file. AST node in the current file should be observed on onMainAST call. + virtual void onPreambleAST(PathRef Path, ASTContext &Ctx, + std::shared_ptr PP) {} + /// Called on the AST built for the file itself. Note that preamble AST nodes + /// are not deserialized and should be processed in the onPreambleAST call + /// instead. + /// The \p AST always contains all AST nodes for the main file itself, an
[PATCH] D50502: [clangd] Initial cancellation mechanism for LSP requests.
kadircet updated this revision to Diff 161716. kadircet marked 4 inline comments as done. kadircet added a comment. - Get rid of CancellationToken && Resolve discussions. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50502 Files: clangd/CMakeLists.txt clangd/Cancellation.cpp clangd/Cancellation.h clangd/ClangdLSPServer.cpp clangd/ClangdLSPServer.h clangd/ClangdServer.cpp clangd/ClangdServer.h clangd/JSONRPCDispatcher.cpp clangd/JSONRPCDispatcher.h clangd/Protocol.cpp clangd/Protocol.h clangd/ProtocolHandlers.cpp clangd/ProtocolHandlers.h unittests/clangd/CMakeLists.txt unittests/clangd/CancellationTests.cpp Index: unittests/clangd/CancellationTests.cpp === --- /dev/null +++ unittests/clangd/CancellationTests.cpp @@ -0,0 +1,74 @@ +#include "Cancellation.h" +#include "Context.h" +#include "Threading.h" +#include "llvm/Support/Error.h" +#include "gmock/gmock.h" +#include "gtest/gtest.h" +#include +#include +#include + +namespace clang { +namespace clangd { +namespace { + +TEST(CancellationTest, CancellationTest) { + TaskHandle TH = createTaskHandle(); + WithContext ContextWithCancellation(setCurrentTask(TH)); + EXPECT_FALSE(isCancelled()); + TH->cancel(); + EXPECT_TRUE(isCancelled()); +} + +TEST(CancellationTest, TaskTestHandleDiesContextLives) { + llvm::Optional ContextWithCancellation; + { +TaskHandle TH = createTaskHandle(); +ContextWithCancellation.emplace(setCurrentTask(TH)); +EXPECT_FALSE(isCancelled()); +TH->cancel(); +EXPECT_TRUE(isCancelled()); + } + EXPECT_TRUE(isCancelled()); +} + +TEST(CancellationTest, TaskContextDiesHandleLives) { + TaskHandle TH = createTaskHandle(); + { +WithContext ContextWithCancellation(setCurrentTask(TH)); +EXPECT_FALSE(isCancelled()); +TH->cancel(); +EXPECT_TRUE(isCancelled()); + } + // Still should be able to cancel without any problems. + TH->cancel(); +} + +TEST(CancellationTest, CancellationToken) { + TaskHandle TH = createTaskHandle(); + WithContext ContextWithCancellation(setCurrentTask(TH)); + const auto &CT = getCurrentTask(); + EXPECT_FALSE(CT.isCancelled()); + TH->cancel(); + EXPECT_TRUE(CT.isCancelled()); +} + +TEST(CancellationTest, AsynCancellationTest) { + std::atomic HasCancelled(false); + Notification Cancelled; + auto TaskToBeCancelled = [&](ConstTaskHandle CT) { +WithContext ContextGuard(setCurrentTask(std::move(CT))); +Cancelled.wait(); +HasCancelled = isCancelled(); + }; + TaskHandle TH = createTaskHandle(); + std::thread AsyncTask(TaskToBeCancelled, TH); + TH->cancel(); + Cancelled.notify(); + AsyncTask.join(); + + EXPECT_TRUE(HasCancelled); +} +} // namespace +} // namespace clangd +} // namespace clang Index: unittests/clangd/CMakeLists.txt === --- unittests/clangd/CMakeLists.txt +++ unittests/clangd/CMakeLists.txt @@ -10,6 +10,7 @@ add_extra_unittest(ClangdTests Annotations.cpp + CancellationTests.cpp ClangdTests.cpp ClangdUnitTests.cpp CodeCompleteTests.cpp Index: clangd/ProtocolHandlers.h === --- clangd/ProtocolHandlers.h +++ clangd/ProtocolHandlers.h @@ -55,6 +55,7 @@ virtual void onDocumentHighlight(TextDocumentPositionParams &Params) = 0; virtual void onHover(TextDocumentPositionParams &Params) = 0; virtual void onChangeConfiguration(DidChangeConfigurationParams &Params) = 0; + virtual void onCancelRequest(CancelParams &Params) = 0; }; void registerCallbackHandlers(JSONRPCDispatcher &Dispatcher, Index: clangd/ProtocolHandlers.cpp === --- clangd/ProtocolHandlers.cpp +++ clangd/ProtocolHandlers.cpp @@ -75,4 +75,5 @@ Register("workspace/didChangeConfiguration", &ProtocolCallbacks::onChangeConfiguration); Register("workspace/symbol", &ProtocolCallbacks::onWorkspaceSymbol); + Register("$/cancelRequest", &ProtocolCallbacks::onCancelRequest); } Index: clangd/Protocol.h === --- clangd/Protocol.h +++ clangd/Protocol.h @@ -867,6 +867,16 @@ llvm::json::Value toJSON(const DocumentHighlight &DH); llvm::raw_ostream &operator<<(llvm::raw_ostream &, const DocumentHighlight &); +struct CancelParams { + /// The request id to cancel. + /// This can be either a number or string, if it is a number simply print it + /// out and always use a string. + std::string ID; +}; +llvm::json::Value toJSON(const CancelParams &); +llvm::raw_ostream &operator<<(llvm::raw_ostream &, const CancelParams &); +bool fromJSON(const llvm::json::Value &, CancelParams &); + } // namespace clangd } // namespace clang Index: clangd/Protocol.cpp === --- clangd/Protocol.cpp +++ clangd/Protocol.cpp @@ -615,5 +615,30 @@
[PATCH] D50847: [clangd] Add callbacks on parsed AST in addition to parsed preambles
ilya-biryukov added inline comments. Comment at: clangd/ClangdServer.cpp:73 + + void onPreambleAST(PathRef Path, ASTContext &Ctx, + std::shared_ptr PP) override { hokein wrote: > I think `Ctx` should be a `pointer` which gives us a way (passing a nullptr) > to clean up the `FileIndex`, the same as `ParsedAST` below. > > And I discover that we don't cleanup dynamic index when a file is being > closed, is it expected or a bug? I suggest we add an extra method to `DynamicIndex` that we call when the file is closed. I don't think it's intentional that we don't clean up the index for the closed files. Not sure what's the best way to handle invalid ASTs, but we're never calling the current callback with `nullptr` anyway, so I suggest we keep it a reference to better reflect that callbacks don't actually handle any errors for us. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50847 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50889: [clangd] Make FileIndex aware of the main file
ilya-biryukov updated this revision to Diff 161718. ilya-biryukov added a comment. - Use noop callbacks from the parent revision Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50889 Files: clangd/ClangdServer.cpp clangd/ClangdServer.h clangd/index/FileIndex.cpp clangd/index/FileIndex.h Index: clangd/index/FileIndex.h === --- clangd/index/FileIndex.h +++ clangd/index/FileIndex.h @@ -64,7 +64,11 @@ /// nullptr, this removes all symbols in the file. /// If \p AST is not null, \p PP cannot be null and it should be the /// preprocessor that was used to build \p AST. - void update(PathRef Path, ASTContext *AST, std::shared_ptr PP); + /// If \p TopLevelDecls is set, only these decls are indexed. Otherwise, all + /// top level decls obtained from \p AST are indexed. + void + update(PathRef Path, ASTContext *AST, std::shared_ptr PP, + llvm::Optional> TopLevelDecls = llvm::None); bool fuzzyFind(const FuzzyFindRequest &Req, @@ -86,8 +90,12 @@ /// Retrieves namespace and class level symbols in \p AST. /// Exposed to assist in unit tests. /// If URISchemes is empty, the default schemes in SymbolCollector will be used. -SymbolSlab indexAST(ASTContext &AST, std::shared_ptr PP, -llvm::ArrayRef URISchemes = {}); +/// If \p TopLevelDecls is set, only these decls are indexed. Otherwise, all top +/// level decls obtained from \p AST are indexed. +SymbolSlab +indexAST(ASTContext &AST, std::shared_ptr PP, + llvm::Optional> TopLevelDecls = llvm::None, + llvm::ArrayRef URISchemes = {}); } // namespace clangd } // namespace clang Index: clangd/index/FileIndex.cpp === --- clangd/index/FileIndex.cpp +++ clangd/index/FileIndex.cpp @@ -8,15 +8,16 @@ //===--===// #include "FileIndex.h" -#include "SymbolCollector.h" #include "../Logger.h" +#include "SymbolCollector.h" #include "clang/Index/IndexingAction.h" #include "clang/Lex/Preprocessor.h" namespace clang { namespace clangd { SymbolSlab indexAST(ASTContext &AST, std::shared_ptr PP, +llvm::Optional> TopLevelDecls, llvm::ArrayRef URISchemes) { SymbolCollector::Options CollectorOpts; // FIXME(ioeric): we might also want to collect include headers. We would need @@ -38,10 +39,14 @@ index::IndexingOptions::SystemSymbolFilterKind::DeclarationsOnly; IndexOpts.IndexFunctionLocals = false; - std::vector TopLevelDecls( - AST.getTranslationUnitDecl()->decls().begin(), - AST.getTranslationUnitDecl()->decls().end()); - index::indexTopLevelDecls(AST, TopLevelDecls, Collector, IndexOpts); + std::vector DeclsToIndex; + if (TopLevelDecls) +DeclsToIndex.assign(TopLevelDecls->begin(), TopLevelDecls->end()); + else +DeclsToIndex.assign(AST.getTranslationUnitDecl()->decls().begin(), +AST.getTranslationUnitDecl()->decls().end()); + + index::indexTopLevelDecls(AST, DeclsToIndex, Collector, IndexOpts); return Collector.takeSymbols(); } @@ -81,13 +86,14 @@ } void FileIndex::update(PathRef Path, ASTContext *AST, - std::shared_ptr PP) { + std::shared_ptr PP, + llvm::Optional> TopLevelDecls) { if (!AST) { FSymbols.update(Path, nullptr); } else { assert(PP); auto Slab = llvm::make_unique(); -*Slab = indexAST(*AST, PP, URISchemes); +*Slab = indexAST(*AST, PP, TopLevelDecls, URISchemes); FSymbols.update(Path, std::move(Slab)); } auto Symbols = FSymbols.allSymbols(); Index: clangd/ClangdServer.h === --- clangd/ClangdServer.h +++ clangd/ClangdServer.h @@ -99,6 +99,7 @@ /// synchronize access to shared state. ClangdServer(GlobalCompilationDatabase &CDB, FileSystemProvider &FSProvider, DiagnosticsConsumer &DiagConsumer, const Options &Opts); + ~ClangdServer(); /// Set the root path of the workspace. void setRootPath(PathRef RootPath); @@ -200,6 +201,7 @@ formatCode(llvm::StringRef Code, PathRef File, ArrayRef Ranges); + class DynamicIndex; typedef uint64_t DocVersion; void consumeDiagnostics(PathRef File, DocVersion Version, @@ -217,15 +219,14 @@ Path ResourceDir; // The index used to look up symbols. This could be: // - null (all index functionality is optional) - // - the dynamic index owned by ClangdServer (FileIdx) + // - the dynamic index owned by ClangdServer (DynamicIdx) // - the static index passed to the constructor // - a merged view of a static and dynamic index (MergedIndex) SymbolIndex *Index; - // If present, an up-to-date of symbols in open files. Read via Index. - std::unique_ptr FileIdx; - /// Callbacks responsible for upd
Re: r314872 - We allow implicit function declarations as an extension in all C dialects. Remove OpenCL special case.
If there are no objections I would like to revert this old commit that coverts error about implicit function declaration into a warning. We have decided to generate an error for this https://reviews.llvm.org/D31745 because for OpenCL variadic prototypes are disallowed (section 6.9.e, https://www.khronos.org/registry/OpenCL/specs/opencl-2.0-openclc.pdf) and the implicit prototype requires variadic support. As most vendors that support OpenCL don't support variadic functions it was decided to restrict this explicitly in the spec (section s6.9.u). There is a little bit of more history in https://reviews.llvm.org/D17438. Currently the code that can't run correctly on most OpenCL targets compiles successfully. The problem can't be easily seen by the OpenCL developers since it's not very common to retrieve the compilation warning log during online compilation. Also generated IR doesn't seem to be correct if I compare with the similar code in C. Example: 1 typedef long long16 __attribute__((ext_vector_type(16))); 2 void test_somefunc( __global int *d, __global void *s ) 3 { 4 int i = get_global_id(0); 5 d[i] = somefunc((( __global long16 *)s)[i]); 6 } Is generated to: %call1 = call i32 (<16 x i64>*, ...) bitcast (i32 ()* @somefunc to i32 (<16 x i64>*, ...)*)(<16 x i64>* byval nonnull align 128 %indirect-arg-temp) #2 ... declare i32 @somefunc() local_unnamed_addr #1 Equivalent C code at least generates variadic function prototype correctly: %call1 = call i32 (<16 x i64>*, ...) bitcast (i32 (...)* @somefunc to i32 (<16 x i64>*, ...)*)(<16 x i64>* byval align 128 %indirect-arg-temp) ... declare i32 @somefunc(...) Anastasia From: cfe-commits on behalf of Richard Smith via cfe-commits Sent: 04 October 2017 02:58 To: cfe-commits@lists.llvm.org Subject: r314872 - We allow implicit function declarations as an extension in all C dialects. Remove OpenCL special case. Author: rsmith Date: Tue Oct 3 18:58:22 2017 New Revision: 314872 URL: http://llvm.org/viewvc/llvm-project?rev=314872&view=rev Log: We allow implicit function declarations as an extension in all C dialects. Remove OpenCL special case. Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td cfe/trunk/lib/Sema/SemaDecl.cpp cfe/trunk/test/SemaOpenCL/clang-builtin-version.cl cfe/trunk/test/SemaOpenCL/to_addr_builtin.cl Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=314872&r1=314871&r2=314872&view=diff == --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original) +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Oct 3 18:58:22 2017 @@ -355,7 +355,7 @@ def warn_implicit_function_decl : Warnin "implicit declaration of function %0">, InGroup, DefaultIgnore; def ext_implicit_function_decl : ExtWarn< - "implicit declaration of function %0 is invalid in C99">, + "implicit declaration of function %0 is invalid in %select{C99|OpenCL}1">, InGroup; def note_function_suggestion : Note<"did you mean %0?">; @@ -8449,8 +8449,6 @@ def err_opencl_scalar_type_rank_greater_ "element. (%0 and %1)">; def err_bad_kernel_param_type : Error< "%0 cannot be used as the type of a kernel parameter">; -def err_opencl_implicit_function_decl : Error< - "implicit declaration of function %0 is invalid in OpenCL">; def err_record_with_pointers_kernel_param : Error< "%select{struct|union}0 kernel parameters may not contain pointers">; def note_within_field_of_type : Note< Modified: cfe/trunk/lib/Sema/SemaDecl.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=314872&r1=314871&r2=314872&view=diff == --- cfe/trunk/lib/Sema/SemaDecl.cpp (original) +++ cfe/trunk/lib/Sema/SemaDecl.cpp Tue Oct 3 18:58:22 2017 @@ -12642,17 +12642,15 @@ NamedDecl *Sema::ImplicitlyDefineFunctio } // Extension in C99. Legal in C90, but warn about it. + // OpenCL v2.0 s6.9.u - Implicit function declaration is not supported. unsigned diag_id; if (II.getName().startswith("__builtin_")) diag_id = diag::warn_builtin_unknown; - // OpenCL v2.0 s6.9.u - Implicit function declaration is not supported. - else if (getLangOpts().OpenCL) -diag_id = diag::err_opencl_implicit_function_decl; - else if (getLangOpts().C99) + else if (getLangOpts().C99 || getLangOpts().OpenCL) diag_id = diag::ext_implicit_function_decl; else diag_id = diag::warn_implicit_function_decl; - Diag(Loc, diag_id) << &II; + Diag(Loc, diag_id) << &II << getLangOpts().OpenCL; // If we found a prior declaration of this function, don't bother building // another one. We've already pushed that one into scope, so there's nothing Modified: cfe/trunk/test/SemaOpenCL/clang-builtin-ver
[PATCH] D50889: [clangd] Make FileIndex aware of the main file
ilya-biryukov marked 2 inline comments as done. ilya-biryukov added a comment. All the issues should be addressed at this point Comment at: clangd/ClangdServer.cpp:70 +// FIXME(ibiryukov): this should be a generic helper instead. +class NoopCallbacks : public ParsingCallbacks { public: ilya-biryukov wrote: > ioeric wrote: > > Maybe provide noop implementations for `ParsingCallbacks::onPreambleAST()` > > and `ParsingCallbacks::onMainAST` by default? > I'll address this in a parent revision. Done Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50889 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50889: [clangd] Make FileIndex aware of the main file
ilya-biryukov marked an inline comment as done. ilya-biryukov added inline comments. Comment at: clangd/index/FileIndex.h:70 + void + update(PathRef Path, ASTContext *AST, std::shared_ptr PP, + llvm::Optional> TopLevelDecls = llvm::None); hokein wrote: > Any strong reason to unify the interface (and `indexAST`)? > > It seems to me that we do different things on `PreambleAST` and `MainAST`: > > - on `PreambleAST`, we only index `Symbols` for code completion. > - on `MainAST`, we index `Symbols` (to collect other information missing from > `PreambleAST`, e.g. definition location), and collect symbol references. > > Therefore, we'd need to set different options for `SymbolCollector` > accordingly. Having a single interface would make the implementation tricky > (checking `TopLevelDecls` to see whether this is a `PreambleASt`). The only reason is to keep the changes to `FileIndex` minimal in this change. We'll have to change more code to cover those differences, but I'm not sure I have enough context on what's going to be different for this change, a change that will plug xrefs into clangd is probably a better candidate for those. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50889 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check
hugoeg added a comment. Ok , so I've clean some stuff up, I'm currently working with @deannagarcia to get https://reviews.llvm.org/D50580 submitted with the match I implemented for us first, then will adjust this as necessary https://reviews.llvm.org/D50542 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50580: [clang-tidy] Abseil: no namespace check
deannagarcia updated this revision to Diff 161720. deannagarcia marked 12 inline comments as done. https://reviews.llvm.org/D50580 Files: clang-tidy/abseil/AbseilMatcher.h clang-tidy/abseil/AbseilTidyModule.cpp clang-tidy/abseil/CMakeLists.txt clang-tidy/abseil/NoNamespaceCheck.cpp clang-tidy/abseil/NoNamespaceCheck.h docs/ReleaseNotes.rst docs/clang-tidy/checks/abseil-no-namespace.rst docs/clang-tidy/checks/list.rst test/clang-tidy/Inputs/absl/external-file.h test/clang-tidy/Inputs/absl/strings/internal-file.h test/clang-tidy/abseil-no-namespace.cpp Index: test/clang-tidy/abseil-no-namespace.cpp === --- test/clang-tidy/abseil-no-namespace.cpp +++ test/clang-tidy/abseil-no-namespace.cpp @@ -0,0 +1,26 @@ +// RUN: %check_clang_tidy %s abseil-no-namespace %t -- -- -I %S/Inputs +// RUN: clang-tidy -checks='-*, abseil-no-namespace' -header-filter='.*' %s -- -I %S/Inputs 2>&1 | FileCheck %s + +// Warning will not be triggered on internal Abseil code that is included. +#include "absl/strings/internal-file.h" +// CHECK-NOT: warning: + +// Warning will be triggered on code that is not internal that is included. +#include "absl/external-file.h" +// CHECK: absl/external-file.h:1:11: warning: namespace 'absl' is reserved +// for implementation of the Abseil library and should not be opened in +// user code [abseil-no-namespace] + +namespace absl {} +// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: namespace 'absl' is reserved for +// implementation of the Abseil library and should not be opened in the user +// code [abseil-no-namespace] + +namespace absl { +int i = 5; +} +// CHECK-MESSAGES: :[[@LINE-3]]:11: warning: namespace 'absl' + +// Things that shouldn't trigger the check +int i = 5; +namespace std {} Index: test/clang-tidy/Inputs/absl/strings/internal-file.h === --- test/clang-tidy/Inputs/absl/strings/internal-file.h +++ test/clang-tidy/Inputs/absl/strings/internal-file.h @@ -0,0 +1 @@ +namespace absl {} Index: test/clang-tidy/Inputs/absl/external-file.h === --- test/clang-tidy/Inputs/absl/external-file.h +++ test/clang-tidy/Inputs/absl/external-file.h @@ -0,0 +1 @@ +namespace absl {} Index: docs/clang-tidy/checks/list.rst === --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -4,6 +4,7 @@ = .. toctree:: + abseil-no-namespace abseil-string-find-startswith android-cloexec-accept android-cloexec-accept4 Index: docs/clang-tidy/checks/abseil-no-namespace.rst === --- docs/clang-tidy/checks/abseil-no-namespace.rst +++ docs/clang-tidy/checks/abseil-no-namespace.rst @@ -0,0 +1,21 @@ +.. title:: clang-tidy - abseil-no-namespace + +abseil-no-namespace +=== + +This check gives users a warning if they attempt to open ``namespace absl``. +Users should not open ``namespace absl`` as that conflicts with abseil's +compatibility guidelines and may result in breakage. + +Any user that uses: + +.. code-block:: c++ + + namespace absl { + ... + } + +will be prompted with a warning. + +See `the full Abseil compatibility guidelines `_ for more information. Index: docs/ReleaseNotes.rst === --- docs/ReleaseNotes.rst +++ docs/ReleaseNotes.rst @@ -57,7 +57,11 @@ Improvements to clang-tidy -- -The improvements are... +- New :doc:`abseil-no-namespace + ` check. + + Checks to ensure code does not open `namespace absl` as that + violates Abseil's compatibility guidelines. Improvements to include-fixer - Index: clang-tidy/abseil/NoNamespaceCheck.h === --- clang-tidy/abseil/NoNamespaceCheck.h +++ clang-tidy/abseil/NoNamespaceCheck.h @@ -0,0 +1,36 @@ +//===--- NoNamespaceCheck.h - clang-tidy-*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_NONAMESPACECHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_NONAMESPACECHECK_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { +namespace abseil { + +/// This test ensures users don't open namespace absl, as that violates +/// our compatibility guidelines. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/abseil-no-namespace.html +class NoNamespaceCheck : public ClangTidyCheck { +public: + NoNamespaceCheck(StringRef Name, ClangTid
[PATCH] D50580: [clang-tidy] Abseil: no namespace check
deannagarcia added inline comments. Comment at: clang-tidy/abseil/AbseilMatcher.h:32 + auto Filename = FileEntry->getName(); + llvm::Regex RE("absl/(base|container|debugging|memory|meta|numeric|strings|" + "synchronization|types|utiliy)"); lebedev.ri wrote: > hokein wrote: > > lebedev.ri wrote: > > > hokein wrote: > > > > The regex seems incomplete, we are missing `algorithm`, `time` > > > > subdirectory. > > > So what happens if open the namespace in a file that is located in my > > > personal `absl/base` directory? > > > It will be false-negatively not reported? > > Yes, I'd say this is a known limitation. > Similarly, the check will break (start producing false-positives) as soon as > a new directory is added to abseil proper. > I don't have any reliable idea on how to do this better, but the current > solution seems rather suboptimal.. We are aware that there will be a false-negative if code opens namespace in the absl directories, however we think that is pretty rare and that users shouldn't be doing that anyway. No matter how we do this there will be a way for users to circumvent the check, and we will allow those users to do it because it will be their code that breaks in the end. Comment at: clang-tidy/abseil/AbseilMatcher.h:32 + auto Filename = FileEntry->getName(); + llvm::Regex RE("absl/(base|container|debugging|memory|meta|numeric|strings|" + "synchronization|types|utiliy)"); deannagarcia wrote: > lebedev.ri wrote: > > hokein wrote: > > > lebedev.ri wrote: > > > > hokein wrote: > > > > > The regex seems incomplete, we are missing `algorithm`, `time` > > > > > subdirectory. > > > > So what happens if open the namespace in a file that is located in my > > > > personal `absl/base` directory? > > > > It will be false-negatively not reported? > > > Yes, I'd say this is a known limitation. > > Similarly, the check will break (start producing false-positives) as soon > > as a new directory is added to abseil proper. > > I don't have any reliable idea on how to do this better, but the current > > solution seems rather suboptimal.. > We are aware that there will be a false-negative if code opens namespace in > the absl directories, however we think that is pretty rare and that users > shouldn't be doing that anyway. No matter how we do this there will be a way > for users to circumvent the check, and we will allow those users to do it > because it will be their code that breaks in the end. The false-positive with new directories is something we thought about, but new directories aren't added to absl very often so we thought it wouldn't be too hard to add them to this regex when they are. https://reviews.llvm.org/D50580 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50970: [clangd] Implement BOOST iterator
ilya-biryukov added a comment. Looking at this one. (to make sure we don't clash with @ioeric) Comment at: clang-tools-extra/clangd/index/dex/Iterator.h:111 + const static unsigned DEFAULT_BOOST_SCORE; + Maybe make it `constexpr` and put the value into the header? https://reviews.llvm.org/D50970 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51038: [clangd] Make sure codecompletion is called for calls even when inside a token.
kadircet created this revision. kadircet added reviewers: ilya-biryukov, ioeric, hokein. Herald added subscribers: cfe-commits, arphaman, jkorous, MaskRay. Currently CodeCompleteCall only gets called after a comma or parantheses. This patch makes sure it is called even at the cases like: foo(1^); Repository: rC Clang https://reviews.llvm.org/D51038 Files: include/clang/Parse/Parser.h lib/Parse/ParseExpr.cpp Index: lib/Parse/ParseExpr.cpp === --- lib/Parse/ParseExpr.cpp +++ lib/Parse/ParseExpr.cpp @@ -1648,6 +1648,10 @@ Actions.CodeCompleteCall(getCurScope(), LHS.get(), ArgExprs); })) { (void)Actions.CorrectDelayedTyposInExpr(LHS); +if (PP.isCodeCompletionReached() && !CalledOverloadCompletion) { + Actions.CodeCompleteCall(getCurScope(), LHS.get(), ArgExprs); + CalledOverloadCompletion = true; +} LHS = ExprError(); } else if (LHS.isInvalid()) { for (auto &E : ArgExprs) @@ -2813,6 +2817,7 @@ Completer(); else Actions.CodeCompleteOrdinaryName(getCurScope(), Sema::PCC_Expression); + CalledOverloadCompletion = true; cutOffParsing(); return true; } Index: include/clang/Parse/Parser.h === --- include/clang/Parse/Parser.h +++ include/clang/Parse/Parser.h @@ -2965,6 +2965,10 @@ void CodeCompleteMacroArgument(IdentifierInfo *Macro, MacroInfo *MacroInfo, unsigned ArgumentIndex) override; void CodeCompleteNaturalLanguage() override; + + /// Gets set to true after calling CodeCompleteCall, it is for a hack to make + /// signature help working even when it is triggered inside a token. + bool CalledOverloadCompletion = false; }; } // end namespace clang Index: lib/Parse/ParseExpr.cpp === --- lib/Parse/ParseExpr.cpp +++ lib/Parse/ParseExpr.cpp @@ -1648,6 +1648,10 @@ Actions.CodeCompleteCall(getCurScope(), LHS.get(), ArgExprs); })) { (void)Actions.CorrectDelayedTyposInExpr(LHS); +if (PP.isCodeCompletionReached() && !CalledOverloadCompletion) { + Actions.CodeCompleteCall(getCurScope(), LHS.get(), ArgExprs); + CalledOverloadCompletion = true; +} LHS = ExprError(); } else if (LHS.isInvalid()) { for (auto &E : ArgExprs) @@ -2813,6 +2817,7 @@ Completer(); else Actions.CodeCompleteOrdinaryName(getCurScope(), Sema::PCC_Expression); + CalledOverloadCompletion = true; cutOffParsing(); return true; } Index: include/clang/Parse/Parser.h === --- include/clang/Parse/Parser.h +++ include/clang/Parse/Parser.h @@ -2965,6 +2965,10 @@ void CodeCompleteMacroArgument(IdentifierInfo *Macro, MacroInfo *MacroInfo, unsigned ArgumentIndex) override; void CodeCompleteNaturalLanguage() override; + + /// Gets set to true after calling CodeCompleteCall, it is for a hack to make + /// signature help working even when it is triggered inside a token. + bool CalledOverloadCompletion = false; }; } // end namespace clang ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51039: [clangd] Add unittests for D51038
kadircet created this revision. kadircet added reviewers: ilya-biryukov, ioeric, hokein. Herald added subscribers: cfe-commits, arphaman, jkorous, MaskRay. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51039 Files: unittests/clangd/CodeCompleteTests.cpp Index: unittests/clangd/CodeCompleteTests.cpp === --- unittests/clangd/CodeCompleteTests.cpp +++ unittests/clangd/CodeCompleteTests.cpp @@ -1590,6 +1590,56 @@ ElementsAre(Sig("foo(T, U) -> void", {"T", "U"}))); } +TEST(SignatureHelpTest, InsideArgument) { + { +const auto Results = signatures(R"cpp( + void foo(int x); + void foo(int x, int y); + int main() { foo(1+^); } +)cpp"); +EXPECT_THAT( +Results.signatures, +ElementsAre(Sig("foo(int x) -> void", {"int x"}), +Sig("foo(int x, int y) -> void", {"int x", "int y"}))); +EXPECT_EQ(0, Results.activeParameter); + } + { +const auto Results = signatures(R"cpp( + void foo(int x); + void foo(int x, int y); + int main() { foo(1^); } +)cpp"); +EXPECT_THAT( +Results.signatures, +ElementsAre(Sig("foo(int x) -> void", {"int x"}), +Sig("foo(int x, int y) -> void", {"int x", "int y"}))); +EXPECT_EQ(0, Results.activeParameter); + } + { +const auto Results = signatures(R"cpp( + void foo(int x); + void foo(int x, int y); + int main() { foo(1^0); } +)cpp"); +EXPECT_THAT( +Results.signatures, +ElementsAre(Sig("foo(int x) -> void", {"int x"}), +Sig("foo(int x, int y) -> void", {"int x", "int y"}))); +EXPECT_EQ(0, Results.activeParameter); + } + { +const auto Results = signatures(R"cpp( + void foo(int x); + void foo(int x, int y); + int bar(int x, int y); + int main() { bar(foo(2, 3^)); } +)cpp"); +EXPECT_THAT(Results.signatures, ElementsAre(Sig("foo(int x, int y) -> void", +{"int x", "int y"}))); +EXPECT_EQ(1, Results.activeParameter); + } +} + } // namespace } // namespace clangd } // namespace clang Index: unittests/clangd/CodeCompleteTests.cpp === --- unittests/clangd/CodeCompleteTests.cpp +++ unittests/clangd/CodeCompleteTests.cpp @@ -1590,6 +1590,56 @@ ElementsAre(Sig("foo(T, U) -> void", {"T", "U"}))); } +TEST(SignatureHelpTest, InsideArgument) { + { +const auto Results = signatures(R"cpp( + void foo(int x); + void foo(int x, int y); + int main() { foo(1+^); } +)cpp"); +EXPECT_THAT( +Results.signatures, +ElementsAre(Sig("foo(int x) -> void", {"int x"}), +Sig("foo(int x, int y) -> void", {"int x", "int y"}))); +EXPECT_EQ(0, Results.activeParameter); + } + { +const auto Results = signatures(R"cpp( + void foo(int x); + void foo(int x, int y); + int main() { foo(1^); } +)cpp"); +EXPECT_THAT( +Results.signatures, +ElementsAre(Sig("foo(int x) -> void", {"int x"}), +Sig("foo(int x, int y) -> void", {"int x", "int y"}))); +EXPECT_EQ(0, Results.activeParameter); + } + { +const auto Results = signatures(R"cpp( + void foo(int x); + void foo(int x, int y); + int main() { foo(1^0); } +)cpp"); +EXPECT_THAT( +Results.signatures, +ElementsAre(Sig("foo(int x) -> void", {"int x"}), +Sig("foo(int x, int y) -> void", {"int x", "int y"}))); +EXPECT_EQ(0, Results.activeParameter); + } + { +const auto Results = signatures(R"cpp( + void foo(int x); + void foo(int x, int y); + int bar(int x, int y); + int main() { bar(foo(2, 3^)); } +)cpp"); +EXPECT_THAT(Results.signatures, ElementsAre(Sig("foo(int x, int y) -> void", +{"int x", "int y"}))); +EXPECT_EQ(1, Results.activeParameter); + } +} + } // namespace } // namespace clangd } // namespace clang ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50580: [clang-tidy] Abseil: no namespace check
hugoeg added inline comments. Comment at: clang-tidy/abseil/AbseilMatcher.h:32 + auto Filename = FileEntry->getName(); + llvm::Regex RE("absl/(base|container|debugging|memory|meta|numeric|strings|" + "synchronization|types|utiliy)"); deannagarcia wrote: > deannagarcia wrote: > > lebedev.ri wrote: > > > hokein wrote: > > > > lebedev.ri wrote: > > > > > hokein wrote: > > > > > > The regex seems incomplete, we are missing `algorithm`, `time` > > > > > > subdirectory. > > > > > So what happens if open the namespace in a file that is located in my > > > > > personal `absl/base` directory? > > > > > It will be false-negatively not reported? > > > > Yes, I'd say this is a known limitation. > > > Similarly, the check will break (start producing false-positives) as soon > > > as a new directory is added to abseil proper. > > > I don't have any reliable idea on how to do this better, but the current > > > solution seems rather suboptimal.. > > We are aware that there will be a false-negative if code opens namespace in > > the absl directories, however we think that is pretty rare and that users > > shouldn't be doing that anyway. No matter how we do this there will be a > > way for users to circumvent the check, and we will allow those users to do > > it because it will be their code that breaks in the end. > The false-positive with new directories is something we thought about, but > new directories aren't added to absl very often so we thought it wouldn't be > too hard to add them to this regex when they are. Members of the Abseil team can also update this check as new directories are release https://reviews.llvm.org/D50580 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r340280 - [analyzer] Correctly marked a virtual function 'override'
Author: szelethus Date: Tue Aug 21 08:09:22 2018 New Revision: 340280 URL: http://llvm.org/viewvc/llvm-project?rev=340280&view=rev Log: [analyzer] Correctly marked a virtual function 'override' Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp?rev=340280&r1=340279&r2=340280&view=diff == --- cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp (original) +++ cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp Tue Aug 21 08:09:22 2018 @@ -117,7 +117,7 @@ public: virtual void printSeparator(llvm::raw_ostream &Out) const override {} - virtual bool isBase() const { return true; } + virtual bool isBase() const override { return true; } }; } // end of anonymous namespace ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50898: [clangd] Suggest code-completions for overriding base class virtual methods.
kadircet added a comment. After today's offline discussion I suppose we are not going to implement it within Sema. And also I think getVirtualNonOverridenMethods is a pretty generic function that has no ties to clangd, so it can be easily moved into Sema. Should we still move it into a separate file or leave it there? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50898 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r314872 - We allow implicit function declarations as an extension in all C dialects. Remove OpenCL special case.
Hi Anastasia, Richard and I discussed the way to enforce the error w/o customizing the diagnostic for OpenCL off-list and Richard proposed to use "-Werror=implicit-function-declaration" command line option if error is required. Will it work for you? Thanks, Alexey On Tue, Aug 21, 2018 at 5:41 PM Anastasia Stulova via cfe-commits wrote: > > If there are no objections I would like to revert this old commit that > coverts error about implicit function declaration into a warning. > > > We have decided to generate an error for this https://reviews.llvm.org/D31745 > because for OpenCL variadic prototypes are disallowed (section 6.9.e, > https://www.khronos.org/registry/OpenCL/specs/opencl-2.0-openclc.pdf) and the > implicit prototype requires variadic support. As most vendors that support > OpenCL don't support variadic functions it was decided to restrict this > explicitly in the spec (section s6.9.u). There is a little bit of more > history in https://reviews.llvm.org/D17438. > > Currently the code that can't run correctly on most OpenCL targets compiles > successfully. The problem can't be easily seen by the OpenCL developers since > it's not very common to retrieve the compilation warning log during online > compilation. Also generated IR doesn't seem to be correct if I compare with > the similar code in C. > > Example: > 1 typedef long long16 __attribute__((ext_vector_type(16))); > 2 void test_somefunc( __global int *d, __global void *s ) > 3 { > 4 int i = get_global_id(0); > 5 d[i] = somefunc((( __global long16 *)s)[i]); > 6 } > > Is generated to: > > %call1 = call i32 (<16 x i64>*, ...) bitcast (i32 ()* @somefunc to i32 (<16 x > i64>*, ...)*)(<16 x i64>* byval nonnull align 128 %indirect-arg-temp) #2 > ... > > declare i32 @somefunc() local_unnamed_addr #1 > > Equivalent C code at least generates variadic function prototype correctly: > > %call1 = call i32 (<16 x i64>*, ...) bitcast (i32 (...)* @somefunc to i32 > (<16 x i64>*, ...)*)(<16 x i64>* byval align 128 %indirect-arg-temp) > ... > declare i32 @somefunc(...) > > Anastasia > > From: cfe-commits on behalf of Richard > Smith via cfe-commits > Sent: 04 October 2017 02:58 > To: cfe-commits@lists.llvm.org > Subject: r314872 - We allow implicit function declarations as an extension in > all C dialects. Remove OpenCL special case. > > Author: rsmith > Date: Tue Oct 3 18:58:22 2017 > New Revision: 314872 > > URL: http://llvm.org/viewvc/llvm-project?rev=314872&view=rev > Log: > We allow implicit function declarations as an extension in all C dialects. > Remove OpenCL special case. > > Modified: > cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td > cfe/trunk/lib/Sema/SemaDecl.cpp > cfe/trunk/test/SemaOpenCL/clang-builtin-version.cl > cfe/trunk/test/SemaOpenCL/to_addr_builtin.cl > > Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=314872&r1=314871&r2=314872&view=diff > == > --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original) > +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Oct 3 18:58:22 > 2017 > @@ -355,7 +355,7 @@ def warn_implicit_function_decl : Warnin >"implicit declaration of function %0">, >InGroup, DefaultIgnore; > def ext_implicit_function_decl : ExtWarn< > - "implicit declaration of function %0 is invalid in C99">, > + "implicit declaration of function %0 is invalid in %select{C99|OpenCL}1">, >InGroup; > def note_function_suggestion : Note<"did you mean %0?">; > > @@ -8449,8 +8449,6 @@ def err_opencl_scalar_type_rank_greater_ > "element. (%0 and %1)">; > def err_bad_kernel_param_type : Error< >"%0 cannot be used as the type of a kernel parameter">; > -def err_opencl_implicit_function_decl : Error< > - "implicit declaration of function %0 is invalid in OpenCL">; > def err_record_with_pointers_kernel_param : Error< >"%select{struct|union}0 kernel parameters may not contain pointers">; > def note_within_field_of_type : Note< > > Modified: cfe/trunk/lib/Sema/SemaDecl.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=314872&r1=314871&r2=314872&view=diff > == > --- cfe/trunk/lib/Sema/SemaDecl.cpp (original) > +++ cfe/trunk/lib/Sema/SemaDecl.cpp Tue Oct 3 18:58:22 2017 > @@ -12642,17 +12642,15 @@ NamedDecl *Sema::ImplicitlyDefineFunctio >} > >// Extension in C99. Legal in C90, but warn about it. > + // OpenCL v2.0 s6.9.u - Implicit function declaration is not supported. >unsigned diag_id; >if (II.getName().startswith("__builtin_")) > diag_id = diag::warn_builtin_unknown; > - // OpenCL v2.0 s6.9.u - Implicit function declaration is not supported. > - else if (getLangOpts().OpenCL) > -
[PATCH] D48027: [analyzer] Improve `CallDescription` to handle c++ method.
xazax.hun accepted this revision. xazax.hun added a comment. Sorry for the delay, I think this is OK to commit. As a possible improvement, I can imagine making it slightly stricter, e.g. you could only skip QualifiedNames for inline namespaces and the beginning. Maybe add support for staring with `""` or `::` to even disable skipping namespaces at the beginning? But these are just nice to have features, I am perfectly ok with not having them or doing it in a followup patch. https://reviews.llvm.org/D48027 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48027: [analyzer] Improve `CallDescription` to handle c++ method.
xazax.hun added a comment. Oh, and thanks for working on this, this improvement was long overdue, but everybody was busy with something else :) https://reviews.llvm.org/D48027 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51041: [clang-tidy] Don't run misc-unused-using-decls check in C++17.
hokein created this revision. hokein added a reviewer: ilya-biryukov. Herald added a subscriber: xazax.hun. It introduces some false positives which are non-trivial to fix. Ignore running the check in C++17. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51041 Files: clang-tidy/misc/UnusedUsingDeclsCheck.cpp test/clang-tidy/misc-unused-using-decls-cxx17.cpp Index: test/clang-tidy/misc-unused-using-decls-cxx17.cpp === --- /dev/null +++ test/clang-tidy/misc-unused-using-decls-cxx17.cpp @@ -0,0 +1,15 @@ +// RUN: %check_clang_tidy %s misc-unused-using-decls %t -- -- -fno-delayed-template-parsing -std=gnu++17 + +namespace ns { +template +class KV { +public: + KV(K, V); +}; +} + +using ns::KV; + +void f() { + KV(1, 2); +} Index: clang-tidy/misc/UnusedUsingDeclsCheck.cpp === --- clang-tidy/misc/UnusedUsingDeclsCheck.cpp +++ clang-tidy/misc/UnusedUsingDeclsCheck.cpp @@ -29,6 +29,12 @@ } void UnusedUsingDeclsCheck::registerMatchers(MatchFinder *Finder) { + // FIXME: make the check support C++17 or later. The check relies on the fact + // that the used declarations are visited after the "using" declaration, but + // it is not ture in C++17's template argument deduction. + if (!getLangOpts().CPlusPlus || getLangOpts().CPlusPlus17 || + getLangOpts().CPlusPlus2a) +return; Finder->addMatcher(usingDecl(isExpansionInMainFile()).bind("using"), this); auto DeclMatcher = hasDeclaration(namedDecl().bind("used")); Finder->addMatcher(loc(enumType(DeclMatcher)), this); Index: test/clang-tidy/misc-unused-using-decls-cxx17.cpp === --- /dev/null +++ test/clang-tidy/misc-unused-using-decls-cxx17.cpp @@ -0,0 +1,15 @@ +// RUN: %check_clang_tidy %s misc-unused-using-decls %t -- -- -fno-delayed-template-parsing -std=gnu++17 + +namespace ns { +template +class KV { +public: + KV(K, V); +}; +} + +using ns::KV; + +void f() { + KV(1, 2); +} Index: clang-tidy/misc/UnusedUsingDeclsCheck.cpp === --- clang-tidy/misc/UnusedUsingDeclsCheck.cpp +++ clang-tidy/misc/UnusedUsingDeclsCheck.cpp @@ -29,6 +29,12 @@ } void UnusedUsingDeclsCheck::registerMatchers(MatchFinder *Finder) { + // FIXME: make the check support C++17 or later. The check relies on the fact + // that the used declarations are visited after the "using" declaration, but + // it is not ture in C++17's template argument deduction. + if (!getLangOpts().CPlusPlus || getLangOpts().CPlusPlus17 || + getLangOpts().CPlusPlus2a) +return; Finder->addMatcher(usingDecl(isExpansionInMainFile()).bind("using"), this); auto DeclMatcher = hasDeclaration(namedDecl().bind("used")); Finder->addMatcher(loc(enumType(DeclMatcher)), this); ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51041: [clang-tidy] Don't run misc-unused-using-decls check in C++17.
xazax.hun added inline comments. Comment at: clang-tidy/misc/UnusedUsingDeclsCheck.cpp:35 + // it is not ture in C++17's template argument deduction. + if (!getLangOpts().CPlusPlus || getLangOpts().CPlusPlus17 || + getLangOpts().CPlusPlus2a) Isn't the check for `getLangOpts().CPlusPlus2a` redundant? Shouldn't it imply the C++17 flag? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51041 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50580: [clang-tidy] Abseil: no namespace check
hokein added a comment. Thanks for the updates. Looks mostly good, a few nits. Comment at: clang-tidy/abseil/AbseilMatcher.h:32 + +AST_POLYMORPHIC_MATCHER(isExpansionInAbseilHeader, +AST_POLYMORPHIC_SUPPORTED_TYPES( nit: this matcher name now should be `isInAbseilFile`. Comment at: clang-tidy/abseil/NoNamespaceCheck.cpp:26 + Finder->addMatcher( + namespaceDecl(hasName("absl"), unless(isExpansionInAbseilHeader())) + .bind("abslNamespace"), Does the `absl` namespace is always the top level namespace? If so, I think it would be safer to use qualified name "::absl" here. Comment at: clang-tidy/abseil/NoNamespaceCheck.h:19 + +/// This test ensures users don't open namespace absl, as that violates +/// our compatibility guidelines. nit: test => check. Comment at: clang-tidy/abseil/NoNamespaceCheck.h:20 +/// This test ensures users don't open namespace absl, as that violates +/// our compatibility guidelines. +/// nit: our = abseil? Comment at: test/clang-tidy/abseil-no-namespace.cpp:22 +} +// CHECK-MESSAGES: :[[@LINE-3]]:11: warning: namespace 'absl' + nit: put it immediately after the above `name absl {`. https://reviews.llvm.org/D50580 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r314872 - We allow implicit function declarations as an extension in all C dialects. Remove OpenCL special case.
Hi Alexey, I understand that avoiding adding extra diagnostics is desirable, but I am not sure it's a good idea to accept and generate code that is not going to work. Variadic functions are currently not expected to be supported by all OpenCL targets hence we disallow this in the spec explicitly. I would be inclined to explicitly reject the code by default, unless there is a valid way to handle the generated bitcast of variadic prototype to non-variadic one? Then we could look at updating the spec and explain that this is the difference in implicit function prototypes for OpenCL. One compromise could be to try to set "-Werror=implicit-function-declaration" by default in the OpenCL mode? Not sure if it's easier/better than an extra diagnostic though. Cheers, Anastasia From: aleksey.ba...@gmail.com Sent: 21 August 2018 16:11 To: Anastasia Stulova Cc: Richard Smith; cfe-commits; nd Subject: Re: r314872 - We allow implicit function declarations as an extension in all C dialects. Remove OpenCL special case. Hi Anastasia, Richard and I discussed the way to enforce the error w/o customizing the diagnostic for OpenCL off-list and Richard proposed to use "-Werror=implicit-function-declaration" command line option if error is required. Will it work for you? Thanks, Alexey On Tue, Aug 21, 2018 at 5:41 PM Anastasia Stulova via cfe-commits wrote: > > If there are no objections I would like to revert this old commit that > coverts error about implicit function declaration into a warning. > > > We have decided to generate an error for this https://reviews.llvm.org/D31745 > because for OpenCL variadic prototypes are disallowed (section 6.9.e, > https://www.khronos.org/registry/OpenCL/specs/opencl-2.0-openclc.pdf) and the > implicit prototype requires variadic support. As most vendors that support > OpenCL don't support variadic functions it was decided to restrict this > explicitly in the spec (section s6.9.u). There is a little bit of more > history in https://reviews.llvm.org/D17438. > > Currently the code that can't run correctly on most OpenCL targets compiles > successfully. The problem can't be easily seen by the OpenCL developers since > it's not very common to retrieve the compilation warning log during online > compilation. Also generated IR doesn't seem to be correct if I compare with > the similar code in C. > > Example: > 1 typedef long long16 __attribute__((ext_vector_type(16))); > 2 void test_somefunc( __global int *d, __global void *s ) > 3 { > 4 int i = get_global_id(0); > 5 d[i] = somefunc((( __global long16 *)s)[i]); > 6 } > > Is generated to: > > %call1 = call i32 (<16 x i64>*, ...) bitcast (i32 ()* @somefunc to i32 (<16 x > i64>*, ...)*)(<16 x i64>* byval nonnull align 128 %indirect-arg-temp) #2 > ... > > declare i32 @somefunc() local_unnamed_addr #1 > > Equivalent C code at least generates variadic function prototype correctly: > > %call1 = call i32 (<16 x i64>*, ...) bitcast (i32 (...)* @somefunc to i32 > (<16 x i64>*, ...)*)(<16 x i64>* byval align 128 %indirect-arg-temp) > ... > declare i32 @somefunc(...) > > Anastasia > > From: cfe-commits on behalf of Richard > Smith via cfe-commits > Sent: 04 October 2017 02:58 > To: cfe-commits@lists.llvm.org > Subject: r314872 - We allow implicit function declarations as an extension in > all C dialects. Remove OpenCL special case. > > Author: rsmith > Date: Tue Oct 3 18:58:22 2017 > New Revision: 314872 > > URL: http://llvm.org/viewvc/llvm-project?rev=314872&view=rev > Log: > We allow implicit function declarations as an extension in all C dialects. > Remove OpenCL special case. > > Modified: > cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td > cfe/trunk/lib/Sema/SemaDecl.cpp > cfe/trunk/test/SemaOpenCL/clang-builtin-version.cl > cfe/trunk/test/SemaOpenCL/to_addr_builtin.cl > > Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=314872&r1=314871&r2=314872&view=diff > == > --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original) > +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Oct 3 18:58:22 > 2017 > @@ -355,7 +355,7 @@ def warn_implicit_function_decl : Warnin >"implicit declaration of function %0">, >InGroup, DefaultIgnore; > def ext_implicit_function_decl : ExtWarn< > - "implicit declaration of function %0 is invalid in C99">, > + "implicit declaration of function %0 is invalid in %select{C99|OpenCL}1">, >InGroup; > def note_function_suggestion : Note<"did you mean %0?">; > > @@ -8449,8 +8449,6 @@ def err_opencl_scalar_type_rank_greater_ > "element. (%0 and %1)">; > def err_bad_kernel_param_type : Error< >"%0 cannot be used as the type of a kernel parameter">; > -def err_opencl_impl
[PATCH] D51043: [clang][NFC] Fix typo in the name of a note
ldionne created this revision. ldionne added a reviewer: ahatanak. Herald added subscribers: cfe-commits, dexonsmith. r306722 introduced a new note called note_silence_unligned_allocation_unavailable where I believe what was meant is note_silence_aligned_allocation_unavailable. Repository: rCXX libc++ https://reviews.llvm.org/D51043 Files: clang/include/clang/Basic/DiagnosticSemaKinds.td clang/lib/Sema/SemaExprCXX.cpp Index: clang/lib/Sema/SemaExprCXX.cpp === --- clang/lib/Sema/SemaExprCXX.cpp +++ clang/lib/Sema/SemaExprCXX.cpp @@ -1744,7 +1744,7 @@ S.Diag(Loc, diag::err_aligned_allocation_unavailable) << IsDelete << FD.getType().getAsString() << OSName << alignedAllocMinVersion(T.getOS()).getAsString(); -S.Diag(Loc, diag::note_silence_unligned_allocation_unavailable); +S.Diag(Loc, diag::note_silence_aligned_allocation_unavailable); } } Index: clang/include/clang/Basic/DiagnosticSemaKinds.td === --- clang/include/clang/Basic/DiagnosticSemaKinds.td +++ clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -6468,7 +6468,7 @@ def err_aligned_allocation_unavailable : Error< "aligned %select{allocation|deallocation}0 function of type '%1' is only " "available on %2 %3 or newer">; -def note_silence_unligned_allocation_unavailable : Note< +def note_silence_aligned_allocation_unavailable : Note< "if you supply your own aligned allocation functions, use " "-faligned-allocation to silence this diagnostic">; Index: clang/lib/Sema/SemaExprCXX.cpp === --- clang/lib/Sema/SemaExprCXX.cpp +++ clang/lib/Sema/SemaExprCXX.cpp @@ -1744,7 +1744,7 @@ S.Diag(Loc, diag::err_aligned_allocation_unavailable) << IsDelete << FD.getType().getAsString() << OSName << alignedAllocMinVersion(T.getOS()).getAsString(); -S.Diag(Loc, diag::note_silence_unligned_allocation_unavailable); +S.Diag(Loc, diag::note_silence_aligned_allocation_unavailable); } } Index: clang/include/clang/Basic/DiagnosticSemaKinds.td === --- clang/include/clang/Basic/DiagnosticSemaKinds.td +++ clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -6468,7 +6468,7 @@ def err_aligned_allocation_unavailable : Error< "aligned %select{allocation|deallocation}0 function of type '%1' is only " "available on %2 %3 or newer">; -def note_silence_unligned_allocation_unavailable : Note< +def note_silence_aligned_allocation_unavailable : Note< "if you supply your own aligned allocation functions, use " "-faligned-allocation to silence this diagnostic">; ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51043: [clang][NFC] Fix typo in the name of a note
ahatanak accepted this revision. ahatanak added a comment. This revision is now accepted and ready to land. Thanks. Yes, it's a typo. Repository: rCXX libc++ https://reviews.llvm.org/D51043 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r340288 - [clang][NFC] Fix typo in the name of a note
Author: ldionne Date: Tue Aug 21 08:54:24 2018 New Revision: 340288 URL: http://llvm.org/viewvc/llvm-project?rev=340288&view=rev Log: [clang][NFC] Fix typo in the name of a note Summary: r306722 introduced a new note called note_silence_unligned_allocation_unavailable where I believe what was meant is note_silence_aligned_allocation_unavailable. Reviewers: ahatanak Subscribers: dexonsmith, cfe-commits Differential Revision: https://reviews.llvm.org/D51043 Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td cfe/trunk/lib/Sema/SemaExprCXX.cpp Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=340288&r1=340287&r2=340288&view=diff == --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original) +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Aug 21 08:54:24 2018 @@ -6468,7 +6468,7 @@ def warn_overaligned_type : Warning< def err_aligned_allocation_unavailable : Error< "aligned %select{allocation|deallocation}0 function of type '%1' is only " "available on %2 %3 or newer">; -def note_silence_unligned_allocation_unavailable : Note< +def note_silence_aligned_allocation_unavailable : Note< "if you supply your own aligned allocation functions, use " "-faligned-allocation to silence this diagnostic">; Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=340288&r1=340287&r2=340288&view=diff == --- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original) +++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Tue Aug 21 08:54:24 2018 @@ -1744,7 +1744,7 @@ static void diagnoseUnavailableAlignedAl S.Diag(Loc, diag::err_aligned_allocation_unavailable) << IsDelete << FD.getType().getAsString() << OSName << alignedAllocMinVersion(T.getOS()).getAsString(); -S.Diag(Loc, diag::note_silence_unligned_allocation_unavailable); +S.Diag(Loc, diag::note_silence_aligned_allocation_unavailable); } } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51043: [clang][NFC] Fix typo in the name of a note
This revision was automatically updated to reflect the committed changes. Closed by commit rL340288: [clang][NFC] Fix typo in the name of a note (authored by ldionne, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D51043?vs=161737&id=161739#toc Repository: rL LLVM https://reviews.llvm.org/D51043 Files: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td cfe/trunk/lib/Sema/SemaExprCXX.cpp Index: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td === --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td @@ -6468,7 +6468,7 @@ def err_aligned_allocation_unavailable : Error< "aligned %select{allocation|deallocation}0 function of type '%1' is only " "available on %2 %3 or newer">; -def note_silence_unligned_allocation_unavailable : Note< +def note_silence_aligned_allocation_unavailable : Note< "if you supply your own aligned allocation functions, use " "-faligned-allocation to silence this diagnostic">; Index: cfe/trunk/lib/Sema/SemaExprCXX.cpp === --- cfe/trunk/lib/Sema/SemaExprCXX.cpp +++ cfe/trunk/lib/Sema/SemaExprCXX.cpp @@ -1744,7 +1744,7 @@ S.Diag(Loc, diag::err_aligned_allocation_unavailable) << IsDelete << FD.getType().getAsString() << OSName << alignedAllocMinVersion(T.getOS()).getAsString(); -S.Diag(Loc, diag::note_silence_unligned_allocation_unavailable); +S.Diag(Loc, diag::note_silence_aligned_allocation_unavailable); } } Index: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td === --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td @@ -6468,7 +6468,7 @@ def err_aligned_allocation_unavailable : Error< "aligned %select{allocation|deallocation}0 function of type '%1' is only " "available on %2 %3 or newer">; -def note_silence_unligned_allocation_unavailable : Note< +def note_silence_aligned_allocation_unavailable : Note< "if you supply your own aligned allocation functions, use " "-faligned-allocation to silence this diagnostic">; Index: cfe/trunk/lib/Sema/SemaExprCXX.cpp === --- cfe/trunk/lib/Sema/SemaExprCXX.cpp +++ cfe/trunk/lib/Sema/SemaExprCXX.cpp @@ -1744,7 +1744,7 @@ S.Diag(Loc, diag::err_aligned_allocation_unavailable) << IsDelete << FD.getType().getAsString() << OSName << alignedAllocMinVersion(T.getOS()).getAsString(); -S.Diag(Loc, diag::note_silence_unligned_allocation_unavailable); +S.Diag(Loc, diag::note_silence_aligned_allocation_unavailable); } } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r340048 - Make __shiftleft128 / __shiftright128 real compiler built-ins.
Merged to 7.0 in r340289. On Fri, Aug 17, 2018 at 10:19 AM, Nico Weber via cfe-commits wrote: > Author: nico > Date: Fri Aug 17 10:19:06 2018 > New Revision: 340048 > > URL: http://llvm.org/viewvc/llvm-project?rev=340048&view=rev > Log: > Make __shiftleft128 / __shiftright128 real compiler built-ins. > > r337619 added __shiftleft128 / __shiftright128 as functions in intrin.h. > Microsoft's STL plans on using these functions, and they're using intrin0.h > which just has declarations of built-ins to not pull in the huge intrin.h > header in the standard library headers. That requires that these functions are > real built-ins. > > https://reviews.llvm.org/D50907 > > Modified: > cfe/trunk/include/clang/Basic/BuiltinsX86_64.def > cfe/trunk/lib/CodeGen/CGBuiltin.cpp > cfe/trunk/lib/Headers/intrin.h > cfe/trunk/test/CodeGen/ms-x86-intrinsics.c > cfe/trunk/test/Headers/ms-intrin.cpp > > Modified: cfe/trunk/include/clang/Basic/BuiltinsX86_64.def > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsX86_64.def?rev=340048&r1=340047&r2=340048&view=diff > == > --- cfe/trunk/include/clang/Basic/BuiltinsX86_64.def (original) > +++ cfe/trunk/include/clang/Basic/BuiltinsX86_64.def Fri Aug 17 10:19:06 2018 > @@ -31,6 +31,8 @@ TARGET_HEADER_BUILTIN(_mul128, "LLiLLiLL > TARGET_HEADER_BUILTIN(_umul128, "ULLiULLiULLiULLi*", "nch", "intrin.h", > ALL_MS_LANGUAGES, "") > > TARGET_HEADER_BUILTIN(__faststorefence, "v", "nh", "intrin.h", > ALL_MS_LANGUAGES, "") > +TARGET_HEADER_BUILTIN(__shiftleft128, "ULLiULLiULLiUc", "nch", "intrin.h", > ALL_MS_LANGUAGES, "") > +TARGET_HEADER_BUILTIN(__shiftright128, "ULLiULLiULLiUc", "nch", "intrin.h", > ALL_MS_LANGUAGES, "") > > TARGET_HEADER_BUILTIN(_InterlockedAnd64, "LLiLLiD*LLi", "nh", > "intrin.h", ALL_MS_LANGUAGES, "") > TARGET_HEADER_BUILTIN(_InterlockedDecrement64, "LLiLLiD*","nh", > "intrin.h", ALL_MS_LANGUAGES, "") > > Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=340048&r1=340047&r2=340048&view=diff > == > --- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original) > +++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Fri Aug 17 10:19:06 2018 > @@ -10440,6 +10440,27 @@ Value *CodeGenFunction::EmitX86BuiltinEx > return Builder.CreateFence(llvm::AtomicOrdering::SequentiallyConsistent, > llvm::SyncScope::System); >} > + case X86::BI__shiftleft128: > + case X86::BI__shiftright128: { > +// FIXME: Once fshl/fshr no longer add an unneeded and and cmov, do this: > +// llvm::Function *F = CGM.getIntrinsic( > +// BuiltinID == X86::BI__shiftleft128 ? Intrinsic::fshl : > Intrinsic::fshr, > +// Int64Ty); > +// Ops[2] = Builder.CreateZExt(Ops[2], Int64Ty); > +// return Builder.CreateCall(F, Ops); > +llvm::Type *Int128Ty = Builder.getInt128Ty(); > +Value *Val = Builder.CreateOr( > +Builder.CreateShl(Builder.CreateZExt(Ops[1], Int128Ty), 64), > +Builder.CreateZExt(Ops[0], Int128Ty)); > +Value *Amt = Builder.CreateAnd(Builder.CreateZExt(Ops[2], Int128Ty), > + llvm::ConstantInt::get(Int128Ty, 0x3f)); > +Value *Res; > +if (BuiltinID == X86::BI__shiftleft128) > + Res = Builder.CreateLShr(Builder.CreateShl(Val, Amt), 64); > +else > + Res = Builder.CreateLShr(Val, Amt); > +return Builder.CreateTrunc(Res, Int64Ty); > + } >case X86::BI_ReadWriteBarrier: >case X86::BI_ReadBarrier: >case X86::BI_WriteBarrier: { > > Modified: cfe/trunk/lib/Headers/intrin.h > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/intrin.h?rev=340048&r1=340047&r2=340048&view=diff > == > --- cfe/trunk/lib/Headers/intrin.h (original) > +++ cfe/trunk/lib/Headers/intrin.h Fri Aug 17 10:19:06 2018 > @@ -863,20 +863,6 @@ __nop(void) { >__asm__ volatile ("nop"); > } > #endif > -#if defined(__x86_64__) > -static __inline__ unsigned __int64 __DEFAULT_FN_ATTRS > -__shiftleft128(unsigned __int64 __l, unsigned __int64 __h, unsigned char > __d) { > - unsigned __int128 __val = ((unsigned __int128)__h << 64) | __l; > - unsigned __int128 __res = __val << (__d & 63); > - return (unsigned __int64)(__res >> 64); > -} > -static __inline__ unsigned __int64 __DEFAULT_FN_ATTRS > -__shiftright128(unsigned __int64 __l, unsigned __int64 __h, unsigned char > __d) { > - unsigned __int128 __val = ((unsigned __int128)__h << 64) | __l; > - unsigned __int128 __res = __val >> (__d & 63); > - return (unsigned __int64)__res; > -} > -#endif > > > /**\ > |* Privileged intrinsics > > Modified: cfe/trunk/test/CodeGen/ms-x86-intrinsic
[PATCH] D48027: [analyzer] Improve `CallDescription` to handle c++ method.
rnkovacs accepted this revision. rnkovacs added a comment. In https://reviews.llvm.org/D48027#1203944, @MTC wrote: > However this approach has limit. Given the code below, we cannot distinguish > whether the `basic_string` is user-defined struct or namespace. That's means > when the user provide {"std", "basic_string", "append"}, we can only know the > qualified name of the call sequentially contains `std`, `basic_string`, > `append`. We don't know if these names come from `RecordDecl` or > `NamespaceDecl`. > > namespace std { > namespace basic_string { > struct A { > void append() {} > }; > } > } > > void foo() { > std::basic_string::A a; > a.append(); // Match > } > > > @rnkovacs What do you think? Can this approach meet `InnerPointerChecker`'s > needs? I guess it is highly unlikely for someone to write namespaces like that, and if they do, I think they deserve to have them treated as a `std::string`. Thanks, Henry, this is great! https://reviews.llvm.org/D48027 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r340292 - AMDGPU: Move target code into TargetParser
Author: arsenm Date: Tue Aug 21 09:13:29 2018 New Revision: 340292 URL: http://llvm.org/viewvc/llvm-project?rev=340292&view=rev Log: AMDGPU: Move target code into TargetParser Modified: cfe/trunk/lib/Basic/Targets/AMDGPU.cpp cfe/trunk/lib/Basic/Targets/AMDGPU.h Modified: cfe/trunk/lib/Basic/Targets/AMDGPU.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/AMDGPU.cpp?rev=340292&r1=340291&r2=340292&view=diff == --- cfe/trunk/lib/Basic/Targets/AMDGPU.cpp (original) +++ cfe/trunk/lib/Basic/Targets/AMDGPU.cpp Tue Aug 21 09:13:29 2018 @@ -127,12 +127,14 @@ bool AMDGPUTargetInfo::initFeatureMap( llvm::StringMap &Features, DiagnosticsEngine &Diags, StringRef CPU, const std::vector &FeatureVec) const { + using namespace llvm::AMDGPU; + // XXX - What does the member GPU mean if device name string passed here? if (isAMDGCN(getTriple())) { if (CPU.empty()) CPU = "gfx600"; -switch (parseAMDGCNName(CPU).Kind) { +switch (llvm::AMDGPU::parseArchAMDGCN(CPU)) { case GK_GFX906: Features["dl-insts"] = true; LLVM_FALLTHROUGH; @@ -169,7 +171,7 @@ bool AMDGPUTargetInfo::initFeatureMap( if (CPU.empty()) CPU = "r600"; -switch (parseR600Name(CPU).Kind) { +switch (llvm::AMDGPU::parseArchR600(CPU)) { case GK_CAYMAN: case GK_CYPRESS: case GK_RV770: @@ -201,7 +203,7 @@ void AMDGPUTargetInfo::adjustTargetOptio TargetOptions &TargetOpts) const { bool hasFP32Denormals = false; bool hasFP64Denormals = false; - GPUInfo CGOptsGPU = parseGPUName(TargetOpts.CPU); + for (auto &I : TargetOpts.FeaturesAsWritten) { if (I == "+fp32-denormals" || I == "-fp32-denormals") hasFP32Denormals = true; @@ -210,54 +212,20 @@ void AMDGPUTargetInfo::adjustTargetOptio } if (!hasFP32Denormals) TargetOpts.Features.push_back( -(Twine(CGOptsGPU.HasFastFMAF && CGOptsGPU.HasFullRateF32Denorms && - !CGOpts.FlushDenorm - ? '+' - : '-') + - Twine("fp32-denormals")) + (Twine(hasFastFMAF() && hasFullRateDenormalsF32() && !CGOpts.FlushDenorm + ? '+' : '-') + Twine("fp32-denormals")) .str()); // Always do not flush fp64 or fp16 denorms. - if (!hasFP64Denormals && CGOptsGPU.HasFP64) + if (!hasFP64Denormals && hasFP64()) TargetOpts.Features.push_back("+fp64-fp16-denormals"); } -constexpr AMDGPUTargetInfo::GPUInfo AMDGPUTargetInfo::InvalidGPU; -constexpr AMDGPUTargetInfo::GPUInfo AMDGPUTargetInfo::R600GPUs[]; -constexpr AMDGPUTargetInfo::GPUInfo AMDGPUTargetInfo::AMDGCNGPUs[]; - -AMDGPUTargetInfo::GPUInfo AMDGPUTargetInfo::parseR600Name(StringRef Name) { - const auto *Result = llvm::find_if( - R600GPUs, [Name](const GPUInfo &GPU) { return GPU.Name == Name; }); - - if (Result == std::end(R600GPUs)) -return InvalidGPU; - return *Result; -} - -AMDGPUTargetInfo::GPUInfo AMDGPUTargetInfo::parseAMDGCNName(StringRef Name) { - const auto *Result = llvm::find_if( - AMDGCNGPUs, [Name](const GPUInfo &GPU) { return GPU.Name == Name; }); - - if (Result == std::end(AMDGCNGPUs)) -return InvalidGPU; - return *Result; -} - -AMDGPUTargetInfo::GPUInfo AMDGPUTargetInfo::parseGPUName(StringRef Name) const { - if (isAMDGCN(getTriple())) -return parseAMDGCNName(Name); - else -return parseR600Name(Name); -} - void AMDGPUTargetInfo::fillValidCPUList( SmallVectorImpl &Values) const { if (isAMDGCN(getTriple())) -llvm::for_each(AMDGCNGPUs, [&Values](const GPUInfo &GPU) { - Values.emplace_back(GPU.Name);}); +llvm::AMDGPU::fillValidArchListAMDGCN(Values); else -llvm::for_each(R600GPUs, [&Values](const GPUInfo &GPU) { - Values.emplace_back(GPU.Name);}); +llvm::AMDGPU::fillValidArchListR600(Values); } void AMDGPUTargetInfo::setAddressSpaceMap(bool DefaultIsPrivate) { @@ -267,7 +235,12 @@ void AMDGPUTargetInfo::setAddressSpaceMa AMDGPUTargetInfo::AMDGPUTargetInfo(const llvm::Triple &Triple, const TargetOptions &Opts) : TargetInfo(Triple), - GPU(isAMDGCN(Triple) ? AMDGCNGPUs[0] : parseR600Name(Opts.CPU)) { + GPUKind(isAMDGCN(Triple) ? + llvm::AMDGPU::parseArchAMDGCN(Opts.CPU) : + llvm::AMDGPU::parseArchR600(Opts.CPU)), + GPUFeatures(isAMDGCN(Triple) ? + llvm::AMDGPU::getArchAttrAMDGCN(GPUKind) : + llvm::AMDGPU::getArchAttrR600(GPUKind)) { resetDataLayout(isAMDGCN(getTriple()) ? DataLayoutStringAMDGCN : DataLayoutStringR600); assert(DataLayout->getAllocaAddrSpace() == Private); @@ -312,19 +285,22 @@ void AMDGPUTargetInfo::getTargetDefines( else Builder.defineMacro("__R600__"); - if (GPU.Kind != GK_NONE) -Builder.defineMacro(Twine("__") + Twine(GPU.C
[PATCH] D50984: AMDGPU: Move target code into TargetParser
arsenm closed this revision. arsenm added a comment. r340292 https://reviews.llvm.org/D50984 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r340297 - [ASTImporter] Add test for CXXForRangeStmt
Author: teemperor Date: Tue Aug 21 09:36:49 2018 New Revision: 340297 URL: http://llvm.org/viewvc/llvm-project?rev=340297&view=rev Log: [ASTImporter] Add test for CXXForRangeStmt Reviewers: a.sidorin, martong Reviewed By: martong Subscribers: rnkovacs, martong, cfe-commits Differential Revision: https://reviews.llvm.org/D51001 Added: cfe/trunk/test/Import/cxx-for-range/ cfe/trunk/test/Import/cxx-for-range/Inputs/ cfe/trunk/test/Import/cxx-for-range/Inputs/F.cpp cfe/trunk/test/Import/cxx-for-range/test.cpp Added: cfe/trunk/test/Import/cxx-for-range/Inputs/F.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Import/cxx-for-range/Inputs/F.cpp?rev=340297&view=auto == --- cfe/trunk/test/Import/cxx-for-range/Inputs/F.cpp (added) +++ cfe/trunk/test/Import/cxx-for-range/Inputs/F.cpp Tue Aug 21 09:36:49 2018 @@ -0,0 +1,11 @@ +struct Container { + int *begin(); + int *end(); +}; + +void f() { + Container c; + for (int varname : c) { +return; + } +} Added: cfe/trunk/test/Import/cxx-for-range/test.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Import/cxx-for-range/test.cpp?rev=340297&view=auto == --- cfe/trunk/test/Import/cxx-for-range/test.cpp (added) +++ cfe/trunk/test/Import/cxx-for-range/test.cpp Tue Aug 21 09:36:49 2018 @@ -0,0 +1,53 @@ +// RUN: clang-import-test -dump-ast -import %S/Inputs/F.cpp -expression %s | FileCheck %s + +// CHECK: CXXForRangeStmt + +// CHECK-NEXT: DeclStmt +// CHECK-NEXT: VarDecl +// CHECK-NEXT: DeclRefExpr +// CHECK-SAME: 'c' +// CHECK-SAME: Container + +// CHECK-NEXT: DeclStmt +// CHECK-NEXT: VarDecl +// CHECK-NEXT: CXXMemberCallExpr +// CHECK-SAME: 'int *' +// CHECK-NEXT: MemberExpr +// CHECK-SAME: .begin +// CHECK-NEXT: DeclRefExpr +// CHECK-SAME: '__range1' +// CHECK-SAME: Container + +// CHECK-NEXT: DeclStmt +// CHECK-NEXT: VarDecl +// CHECK-NEXT: CXXMemberCallExpr +// CHECK-SAME: 'int *' +// CHECK-NEXT: MemberExpr +// CHECK-SAME: .end +// CHECK-NEXT: DeclRefExpr +// CHECK-SAME: '__range1' +// CHECK-SAME: Container + +// CHECK-NEXT: BinaryOperator +// CHECK-SAME: '!=' +// CHECK-NEXT: ImplicitCastExpr +// CHECK-NEXT: DeclRefExpr +// CHECK-SAME: '__begin1' +// CHECK-NEXT: ImplicitCastExpr +// CHECK-NEXT: DeclRefExpr +// CHECK-SAME: '__end1' + +// CHECK-NEXT: UnaryOperator +// CHECK-SAME: '++' +// CHECK-NEXT: DeclRefExpr +// CHECK-SAME: '__begin1' + +// CHECK-NEXT: DeclStmt +// CHECK-NEXT: VarDecl +// CHECK-SAME: varname + +// CHECK: ReturnStmt + +void expr() { + f(); +} ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51001: [ASTImporter] Add test for CXXForRangeStmt
This revision was automatically updated to reflect the committed changes. Closed by commit rC340297: [ASTImporter] Add test for CXXForRangeStmt (authored by teemperor, committed by ). Repository: rC Clang https://reviews.llvm.org/D51001 Files: test/Import/cxx-for-range/Inputs/F.cpp test/Import/cxx-for-range/test.cpp Index: test/Import/cxx-for-range/Inputs/F.cpp === --- test/Import/cxx-for-range/Inputs/F.cpp +++ test/Import/cxx-for-range/Inputs/F.cpp @@ -0,0 +1,11 @@ +struct Container { + int *begin(); + int *end(); +}; + +void f() { + Container c; + for (int varname : c) { +return; + } +} Index: test/Import/cxx-for-range/test.cpp === --- test/Import/cxx-for-range/test.cpp +++ test/Import/cxx-for-range/test.cpp @@ -0,0 +1,53 @@ +// RUN: clang-import-test -dump-ast -import %S/Inputs/F.cpp -expression %s | FileCheck %s + +// CHECK: CXXForRangeStmt + +// CHECK-NEXT: DeclStmt +// CHECK-NEXT: VarDecl +// CHECK-NEXT: DeclRefExpr +// CHECK-SAME: 'c' +// CHECK-SAME: Container + +// CHECK-NEXT: DeclStmt +// CHECK-NEXT: VarDecl +// CHECK-NEXT: CXXMemberCallExpr +// CHECK-SAME: 'int *' +// CHECK-NEXT: MemberExpr +// CHECK-SAME: .begin +// CHECK-NEXT: DeclRefExpr +// CHECK-SAME: '__range1' +// CHECK-SAME: Container + +// CHECK-NEXT: DeclStmt +// CHECK-NEXT: VarDecl +// CHECK-NEXT: CXXMemberCallExpr +// CHECK-SAME: 'int *' +// CHECK-NEXT: MemberExpr +// CHECK-SAME: .end +// CHECK-NEXT: DeclRefExpr +// CHECK-SAME: '__range1' +// CHECK-SAME: Container + +// CHECK-NEXT: BinaryOperator +// CHECK-SAME: '!=' +// CHECK-NEXT: ImplicitCastExpr +// CHECK-NEXT: DeclRefExpr +// CHECK-SAME: '__begin1' +// CHECK-NEXT: ImplicitCastExpr +// CHECK-NEXT: DeclRefExpr +// CHECK-SAME: '__end1' + +// CHECK-NEXT: UnaryOperator +// CHECK-SAME: '++' +// CHECK-NEXT: DeclRefExpr +// CHECK-SAME: '__begin1' + +// CHECK-NEXT: DeclStmt +// CHECK-NEXT: VarDecl +// CHECK-SAME: varname + +// CHECK: ReturnStmt + +void expr() { + f(); +} Index: test/Import/cxx-for-range/Inputs/F.cpp === --- test/Import/cxx-for-range/Inputs/F.cpp +++ test/Import/cxx-for-range/Inputs/F.cpp @@ -0,0 +1,11 @@ +struct Container { + int *begin(); + int *end(); +}; + +void f() { + Container c; + for (int varname : c) { +return; + } +} Index: test/Import/cxx-for-range/test.cpp === --- test/Import/cxx-for-range/test.cpp +++ test/Import/cxx-for-range/test.cpp @@ -0,0 +1,53 @@ +// RUN: clang-import-test -dump-ast -import %S/Inputs/F.cpp -expression %s | FileCheck %s + +// CHECK: CXXForRangeStmt + +// CHECK-NEXT: DeclStmt +// CHECK-NEXT: VarDecl +// CHECK-NEXT: DeclRefExpr +// CHECK-SAME: 'c' +// CHECK-SAME: Container + +// CHECK-NEXT: DeclStmt +// CHECK-NEXT: VarDecl +// CHECK-NEXT: CXXMemberCallExpr +// CHECK-SAME: 'int *' +// CHECK-NEXT: MemberExpr +// CHECK-SAME: .begin +// CHECK-NEXT: DeclRefExpr +// CHECK-SAME: '__range1' +// CHECK-SAME: Container + +// CHECK-NEXT: DeclStmt +// CHECK-NEXT: VarDecl +// CHECK-NEXT: CXXMemberCallExpr +// CHECK-SAME: 'int *' +// CHECK-NEXT: MemberExpr +// CHECK-SAME: .end +// CHECK-NEXT: DeclRefExpr +// CHECK-SAME: '__range1' +// CHECK-SAME: Container + +// CHECK-NEXT: BinaryOperator +// CHECK-SAME: '!=' +// CHECK-NEXT: ImplicitCastExpr +// CHECK-NEXT: DeclRefExpr +// CHECK-SAME: '__begin1' +// CHECK-NEXT: ImplicitCastExpr +// CHECK-NEXT: DeclRefExpr +// CHECK-SAME: '__end1' + +// CHECK-NEXT: UnaryOperator +// CHECK-SAME: '++' +// CHECK-NEXT: DeclRefExpr +// CHECK-SAME: '__begin1' + +// CHECK-NEXT: DeclStmt +// CHECK-NEXT: VarDecl +// CHECK-SAME: varname + +// CHECK: ReturnStmt + +void expr() { + f(); +} ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50993: [clangd] Increase stack size of the new threads on macOS
jfb added a comment. Isn't this duplicating code in lib/Support/Unix/Threading.inc with a different implementation? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50993 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r340301 - [Parser] Support alternative operator token keyword args in Objective-C++
Author: epilk Date: Tue Aug 21 09:47:04 2018 New Revision: 340301 URL: http://llvm.org/viewvc/llvm-project?rev=340301&view=rev Log: [Parser] Support alternative operator token keyword args in Objective-C++ rdar://30741878 Differential revision: https://reviews.llvm.org/D50527 Added: cfe/trunk/test/Parser/message-expr-alt-op.mm Modified: cfe/trunk/lib/Parse/ParseExpr.cpp Modified: cfe/trunk/lib/Parse/ParseExpr.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseExpr.cpp?rev=340301&r1=340300&r2=340301&view=diff == --- cfe/trunk/lib/Parse/ParseExpr.cpp (original) +++ cfe/trunk/lib/Parse/ParseExpr.cpp Tue Aug 21 09:47:04 2018 @@ -315,6 +315,19 @@ Parser::ParseRHSOfBinaryExpression(ExprR return LHS; } +// In Objective-C++, alternative operator tokens can be used as keyword args +// in message expressions. Unconsume the token so that it can reinterpreted +// as an identifier in ParseObjCMessageExpressionBody. i.e., we support: +// [foo meth:0 and:0]; +// [foo not_eq]; +if (getLangOpts().ObjC1 && getLangOpts().CPlusPlus && +Tok.isOneOf(tok::colon, tok::r_square) && +OpToken.getIdentifierInfo() != nullptr) { + PP.EnterToken(Tok); + Tok = OpToken; + return LHS; +} + // Special case handling for the ternary operator. ExprResult TernaryMiddle(true); if (NextTokPrec == prec::Conditional) { Added: cfe/trunk/test/Parser/message-expr-alt-op.mm URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/message-expr-alt-op.mm?rev=340301&view=auto == --- cfe/trunk/test/Parser/message-expr-alt-op.mm (added) +++ cfe/trunk/test/Parser/message-expr-alt-op.mm Tue Aug 21 09:47:04 2018 @@ -0,0 +1,41 @@ +// RUN: %clang_cc1 -fsyntax-only -verify %s + +@interface WeirdInterface +-(void)allOfThem:(int)a + and:(int)b + and_eq:(int)c + bitand:(int)d + bitor:(int)e + compl:(int)f + not:(int)g + not_eq:(int)h + or:(int)i + or_eq:(int)j + xor:(int)k + xor_eq:(int)l; + +-(void)justAnd:(int)x and:(int)y; +-(void)and; +-(void)and:(int)x; +@end + +void call_it(WeirdInterface *x) { + [x allOfThem:0 + and:0 +and_eq:0 +bitand:0 + bitor:0 + compl:0 + not:0 +not_eq:0 +or:0 + or_eq:0 + xor:0 +xor_eq:0]; + + [x and]; + [x and:0]; + [x &&:0]; // expected-error{{expected expression}}; + [x justAnd:0 and:1]; + [x and: 0 ? : 1]; +} ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50527: [Parser] Support alternative operator token keyword args in Objective-C++
This revision was automatically updated to reflect the committed changes. Closed by commit rC340301: [Parser] Support alternative operator token keyword args in Objective-C++ (authored by epilk, committed by ). Changed prior to commit: https://reviews.llvm.org/D50527?vs=159995&id=161748#toc Repository: rC Clang https://reviews.llvm.org/D50527 Files: lib/Parse/ParseExpr.cpp test/Parser/message-expr-alt-op.mm Index: test/Parser/message-expr-alt-op.mm === --- test/Parser/message-expr-alt-op.mm +++ test/Parser/message-expr-alt-op.mm @@ -0,0 +1,41 @@ +// RUN: %clang_cc1 -fsyntax-only -verify %s + +@interface WeirdInterface +-(void)allOfThem:(int)a + and:(int)b + and_eq:(int)c + bitand:(int)d + bitor:(int)e + compl:(int)f + not:(int)g + not_eq:(int)h + or:(int)i + or_eq:(int)j + xor:(int)k + xor_eq:(int)l; + +-(void)justAnd:(int)x and:(int)y; +-(void)and; +-(void)and:(int)x; +@end + +void call_it(WeirdInterface *x) { + [x allOfThem:0 + and:0 +and_eq:0 +bitand:0 + bitor:0 + compl:0 + not:0 +not_eq:0 +or:0 + or_eq:0 + xor:0 +xor_eq:0]; + + [x and]; + [x and:0]; + [x &&:0]; // expected-error{{expected expression}}; + [x justAnd:0 and:1]; + [x and: 0 ? : 1]; +} Index: lib/Parse/ParseExpr.cpp === --- lib/Parse/ParseExpr.cpp +++ lib/Parse/ParseExpr.cpp @@ -315,6 +315,19 @@ return LHS; } +// In Objective-C++, alternative operator tokens can be used as keyword args +// in message expressions. Unconsume the token so that it can reinterpreted +// as an identifier in ParseObjCMessageExpressionBody. i.e., we support: +// [foo meth:0 and:0]; +// [foo not_eq]; +if (getLangOpts().ObjC1 && getLangOpts().CPlusPlus && +Tok.isOneOf(tok::colon, tok::r_square) && +OpToken.getIdentifierInfo() != nullptr) { + PP.EnterToken(Tok); + Tok = OpToken; + return LHS; +} + // Special case handling for the ternary operator. ExprResult TernaryMiddle(true); if (NextTokPrec == prec::Conditional) { Index: test/Parser/message-expr-alt-op.mm === --- test/Parser/message-expr-alt-op.mm +++ test/Parser/message-expr-alt-op.mm @@ -0,0 +1,41 @@ +// RUN: %clang_cc1 -fsyntax-only -verify %s + +@interface WeirdInterface +-(void)allOfThem:(int)a + and:(int)b + and_eq:(int)c + bitand:(int)d + bitor:(int)e + compl:(int)f + not:(int)g + not_eq:(int)h + or:(int)i + or_eq:(int)j + xor:(int)k + xor_eq:(int)l; + +-(void)justAnd:(int)x and:(int)y; +-(void)and; +-(void)and:(int)x; +@end + +void call_it(WeirdInterface *x) { + [x allOfThem:0 + and:0 +and_eq:0 +bitand:0 + bitor:0 + compl:0 + not:0 +not_eq:0 +or:0 + or_eq:0 + xor:0 +xor_eq:0]; + + [x and]; + [x and:0]; + [x &&:0]; // expected-error{{expected expression}}; + [x justAnd:0 and:1]; + [x and: 0 ? : 1]; +} Index: lib/Parse/ParseExpr.cpp === --- lib/Parse/ParseExpr.cpp +++ lib/Parse/ParseExpr.cpp @@ -315,6 +315,19 @@ return LHS; } +// In Objective-C++, alternative operator tokens can be used as keyword args +// in message expressions. Unconsume the token so that it can reinterpreted +// as an identifier in ParseObjCMessageExpressionBody. i.e., we support: +// [foo meth:0 and:0]; +// [foo not_eq]; +if (getLangOpts().ObjC1 && getLangOpts().CPlusPlus && +Tok.isOneOf(tok::colon, tok::r_square) && +OpToken.getIdentifierInfo() != nullptr) { + PP.EnterToken(Tok); + Tok = OpToken; + return LHS; +} + // Special case handling for the ternary operator. ExprResult TernaryMiddle(true); if (NextTokPrec == prec::Conditional) { ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50970: [clangd] Implement BOOST iterator
ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/index/dex/Iterator.h:92 + /// function, the parameter is needed to propagate through the tree. + virtual unsigned boost(DocID ID) const = 0; Maybe add a note to the comment on why an `ID` parameter actually is here? IIUC, we need to because various iterators in the tree may point to different elements and we need to know which one we've actually matched. Comment at: clang-tools-extra/clangd/index/dex/Iterator.h:129 +std::vector> +consumeAndBoost(Iterator &It, +size_t Limit = std::numeric_limits::max()); Could we describe the rationale for keeping both `consume` and `consumeAndBoost` somewhere in the comments? From the offline conversation, it seems `consumeAndBoost` is more expensive, but our clients will all use it at some point in the future. The idea of paying for boosting without actually using it seems bad, so keeping this function separate makes sense. Comment at: clang-tools-extra/clangd/index/dex/Iterator.h:164 +std::unique_ptr createBoost(std::unique_ptr Child, + unsigned Factor); + Maybe use `float` scores to align with the scoring code we have for completion? https://reviews.llvm.org/D50970 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits