[libcxx] r305373 - Mark `__is_inplace_*` tests as UNSUPPORTED in <= C++14.
Author: mpark Date: Wed Jun 14 02:12:55 2017 New Revision: 305373 URL: http://llvm.org/viewvc/llvm-project?rev=305373&view=rev Log: Mark `__is_inplace_*` tests as UNSUPPORTED in <= C++14. Modified: libcxx/trunk/test/libcxx/utilities/utility/__is_inplace_index.pass.cpp libcxx/trunk/test/libcxx/utilities/utility/__is_inplace_type.pass.cpp Modified: libcxx/trunk/test/libcxx/utilities/utility/__is_inplace_index.pass.cpp URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/libcxx/utilities/utility/__is_inplace_index.pass.cpp?rev=305373&r1=305372&r2=305373&view=diff == --- libcxx/trunk/test/libcxx/utilities/utility/__is_inplace_index.pass.cpp (original) +++ libcxx/trunk/test/libcxx/utilities/utility/__is_inplace_index.pass.cpp Wed Jun 14 02:12:55 2017 @@ -7,6 +7,8 @@ // //===--===// +// UNSUPPORTED: c++98, c++03, c++11, c++14 + // template using __is_inplace_index #include Modified: libcxx/trunk/test/libcxx/utilities/utility/__is_inplace_type.pass.cpp URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/libcxx/utilities/utility/__is_inplace_type.pass.cpp?rev=305373&r1=305372&r2=305373&view=diff == --- libcxx/trunk/test/libcxx/utilities/utility/__is_inplace_type.pass.cpp (original) +++ libcxx/trunk/test/libcxx/utilities/utility/__is_inplace_type.pass.cpp Wed Jun 14 02:12:55 2017 @@ -7,6 +7,8 @@ // //===--===// +// UNSUPPORTED: c++98, c++03, c++11, c++14 + // template using __is_inplace_type #include ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32480: clang-format: Add CompactNamespaces option
klimek added a comment. Generally LG from my side. Comment at: unittests/Format/FormatTest.cpp:1363-1381 + EXPECT_EQ("namespace {\n" + "namespace {\n" +"}} // namespace ::", +format("namespace {\n" + "namespace {\n" + "} // namespace \n" + "} // namespace ", These tests become more readable if you set up a style with a smaller column limit. https://reviews.llvm.org/D32480 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34194: [coroutines] Allow co_await and co_yield expressions that return an lvalue to compile
EricWF updated this revision to Diff 102494. https://reviews.llvm.org/D34194 Files: lib/AST/ExprClassification.cpp lib/CodeGen/CGCoroutine.cpp lib/CodeGen/CGExpr.cpp lib/CodeGen/CodeGenFunction.h test/CodeGenCoroutines/coro-await.cpp Index: test/CodeGenCoroutines/coro-await.cpp === --- test/CodeGenCoroutines/coro-await.cpp +++ test/CodeGenCoroutines/coro-await.cpp @@ -1,4 +1,5 @@ -// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fcoroutines-ts -std=c++14 -emit-llvm %s -o - -disable-llvm-passes | FileCheck %s +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fcoroutines-ts -std=c++14 \ +// RUN: -emit-llvm %s -o - -disable-llvm-passes -Wno-coroutine -Wno-unused | FileCheck %s namespace std { namespace experimental { @@ -278,3 +279,40 @@ suspend_always lval; co_await lval; } + +struct AwaitResumeReturnsLValue { + bool await_ready(); + void await_suspend(std::experimental::coroutine_handle<>); + int& await_resume(); +}; + + +template<> +struct std::experimental::coroutine_traits { + struct promise_type { +void get_return_object(); +init_susp initial_suspend(); +final_susp final_suspend(); +void return_void(); +AwaitResumeReturnsLValue yield_value(int); + }; +}; + +// Verifies that we don't chrash when returning an lvalue from a await_resume() +// expression. +// CHECK-LABEL: define void @_Z18AwaitReturnsLValued(double) +void AwaitReturnsLValue(double) { + AwaitResumeReturnsLValue a; + // CHECK: await.ready: + // CHECK-NEXT: %[[RES1:.+]] = call dereferenceable(4) i32* @_ZN24AwaitResumeReturnsLValue12await_resumeEv(%struct.AwaitResumeReturnsLValue* %a) + // CHECK-NEXT: store i32* %[[RES1]], i32** %x, align 8 + int& x = co_await a; + // CHECK: await2.ready: + // CHECK-NEXT: %[[RES2:.+]] = call dereferenceable(4) i32* @_ZN24AwaitResumeReturnsLValue12await_resumeEv(%struct.AwaitResumeReturnsLValue* %ref.tmp{{.+}}) + // CHECK-NEXT: store i32* %[[RES2]], i32** %y, align 8 + int& y = co_await AwaitResumeReturnsLValue{}; + // CHECK: yield.ready: + // CHECK-NEXT: %[[RES3:.+]] = call dereferenceable(4) i32* @_ZN24AwaitResumeReturnsLValue12await_resumeEv(%struct.AwaitResumeReturnsLValue* %ref.tmp{{.+}}) + // CHECK-NEXT: store i32* %[[RES3]], i32** %z, align 8 + int& z = co_yield 42; +} Index: lib/CodeGen/CodeGenFunction.h === --- lib/CodeGen/CodeGenFunction.h +++ lib/CodeGen/CodeGenFunction.h @@ -2550,9 +2550,11 @@ RValue EmitCoawaitExpr(const CoawaitExpr &E, AggValueSlot aggSlot = AggValueSlot::ignored(), bool ignoreResult = false); + LValue EmitCoawaitLValue(const CoawaitExpr *E); RValue EmitCoyieldExpr(const CoyieldExpr &E, AggValueSlot aggSlot = AggValueSlot::ignored(), bool ignoreResult = false); + LValue EmitCoyieldLValue(const CoyieldExpr *E); RValue EmitCoroutineIntrinsic(const CallExpr *E, unsigned int IID); void EnterCXXTryStmt(const CXXTryStmt &S, bool IsFnTryBlock = false); Index: lib/CodeGen/CGExpr.cpp === --- lib/CodeGen/CGExpr.cpp +++ lib/CodeGen/CGExpr.cpp @@ -1158,6 +1158,11 @@ case Expr::MaterializeTemporaryExprClass: return EmitMaterializeTemporaryExpr(cast(E)); + + case Expr::CoawaitExprClass: +return EmitCoawaitLValue(cast(E)); + case Expr::CoyieldExprClass: +return EmitCoyieldLValue(cast(E)); } } Index: lib/CodeGen/CGCoroutine.cpp === --- lib/CodeGen/CGCoroutine.cpp +++ lib/CodeGen/CGCoroutine.cpp @@ -148,10 +148,16 @@ // // See llvm's docs/Coroutines.rst for more details. // -static RValue emitSuspendExpression(CodeGenFunction &CGF, CGCoroData &Coro, +namespace { + struct LValueOrRValue { +LValue LV; +RValue RV; + }; +} +static LValueOrRValue emitSuspendExpression(CodeGenFunction &CGF, CGCoroData &Coro, CoroutineSuspendExpr const &S, AwaitKind Kind, AggValueSlot aggSlot, -bool ignoreResult) { +bool ignoreResult, bool forLValue) { auto *E = S.getCommonExpr(); // FIXME: rsmith 5/22/2017. Does it still make sense for us to have a @@ -217,29 +223,64 @@ // Emit await_resume expression. CGF.EmitBlock(ReadyBlock); - return CGF.EmitAnyExpr(S.getResumeExpr(), aggSlot, ignoreResult); + LValueOrRValue Res; + if (forLValue) +Res.LV = CGF.EmitLValue(S.getResumeExpr()); + else +Res.RV = CGF.EmitAnyExpr(S.getResumeExpr(), aggSlot, ignoreResult); + return Res; } RValue CodeGenFunction::EmitCoawaitExpr(const CoawaitExpr &E, AggValueSlot aggSlot, bool ignoreResult) { return emi
[PATCH] D34182: [analyzer] Performance optimizations for the CloneChecker
v.g.vassilev added inline comments. Comment at: include/clang/Analysis/CloneDetection.h:263 +public: + void constrain(std::vector &Sequences); +}; Could we typedef `std::vector` into `CloneDetector::CloneGroups`? https://reviews.llvm.org/D34182 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34182: [analyzer] Performance optimizations for the CloneChecker
teemperor added inline comments. Comment at: include/clang/Analysis/CloneDetection.h:263 +public: + void constrain(std::vector &Sequences); +}; v.g.vassilev wrote: > Could we typedef `std::vector` into > `CloneDetector::CloneGroups`? Yes, I'll do this in another patch for the whole CloneDetector code base. https://reviews.llvm.org/D34182 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34175: [driver][macOS] Pick the system version for the deployment target if the SDK is newer than the system
inouehrs added inline comments. Comment at: lib/Driver/ToolChains/Darwin.cpp:1133 + HadExtra) || + HadExtra || Major != 10 || Minor >= 100 || Micro >= 100) +return MacOSSDKVersion; What these magic numbers mean (especially minor and micro versions)? You may need some comments. Repository: rL LLVM https://reviews.llvm.org/D34175 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33644: Add default values for function parameter chunks
yvvan added a comment. Anyone? :) https://reviews.llvm.org/D33644 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32942: [clang-tidy] readability-function-size: add NestingThreshold param.
malcolm.parsons added a comment. In https://reviews.llvm.org/D32942#779143, @lebedev.ri wrote: > In https://reviews.llvm.org/D32942#778729, @malcolm.parsons wrote: > > > In https://reviews.llvm.org/D32942#777001, @lebedev.ri wrote: > > > > > Which makes sense, since in AST, they are nested: > > > > > > They're not nested in the formatting, so I don't think it makes sense. > > > As usual, all the meaningful review happens post-factum :) I didn't get an email about this change until it was pushed. > So, it should warn on: > ... > but should not on what you/I posted in the previous comment. Yes. > The difference seems to be some kind of implicit `CompoundStmt` added by > `IfStmt`? CompoundStmt represents the `{}`. > Assuming that is the case, maybe this is as simple as checking whether this > `CompoundStmt` is implicit or not? My prototype of this feature used `ifStmt(stmt().bind("if"), unless(hasParent(ifStmt(hasElse(equalsBoundNode("if"))`. > Best to move open a new bug about this. I'll see what can be done. Do you need me to report a bug? Repository: rL LLVM https://reviews.llvm.org/D32942 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32341: Fix a bug that warnings generated with -M or -MM flags
teemperor accepted this revision. teemperor added a comment. This revision is now accepted and ready to land. LGTM, good job! (Sorry for the delay, I think I got interrupted here by the GSoC start...) https://reviews.llvm.org/D32341 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33989: [OpenCL] Allow targets to select address space per type
bader added inline comments. Comment at: include/clang/Basic/TargetInfo.h:1041 +default: + return LangAS::Default; +} yaxunl wrote: > I think the default (including even_t, clk_event_t, queue_t, reserved_id_t) > should be global since these opaque OpenCL objects are pointers to some > shared resources. These pointers may be an automatic variable themselves but > the memory they point to should be global. Since these objects are > dynamically allocated, assuming them in private address space implies runtime > needs to maintain a private memory pool for each work item and allocate > objects from there. Considering the huge number of work items in typical > OpenCL applications, it would be very inefficient to implement these objects > in private memory pool. On the other hand, a global memory pool seems much > reasonable. > > Anastasia/Alexey, any comments on this? Thanks. I remember we discussed this a couple of time in the past. The address space for variables of these types is not clearly stated in the spec, so I think the right way to treat it - it's implementation defined. On the other hand your reasoning on using global address space as default AS makes sense in general - so can we put additional clarification to the spec to align it with the proposed implementation? Comment at: lib/Basic/Targets.cpp:2367 - LangAS::ID getOpenCLImageAddrSpace() const override { + virtual LangAS::ID + getOpenCLTypeAddrSpace(BuiltinType::Kind K) const override { I think the rule LLVM applies is: use virtual in base classes and override w/o virtual in derived classes. 'virtual' is not necessary here. https://reviews.llvm.org/D33989 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32646: Fix a bug that -Wmissing-braces fires on system headers
teemperor added a comment. Everything beside this last test case seems to be handled. And from what I remember there was a longer discussion how to properly handle this case and so this review got stuck. Can we add this last test case with a FIXME and then get this merged? https://reviews.llvm.org/D32646 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32942: [clang-tidy] readability-function-size: add NestingThreshold param.
lebedev.ri added a comment. In https://reviews.llvm.org/D32942#780053, @malcolm.parsons wrote: > In https://reviews.llvm.org/D32942#779143, @lebedev.ri wrote: > > > In https://reviews.llvm.org/D32942#778729, @malcolm.parsons wrote: > > > > > In https://reviews.llvm.org/D32942#777001, @lebedev.ri wrote: > > > > > > > Which makes sense, since in AST, they are nested: > > > > > > > > > They're not nested in the formatting, so I don't think it makes sense. > > > > > > As usual, all the meaningful review happens post-factum :) > > > I didn't get an email about this change until it was pushed. > > > So, it should warn on: > > ... > > but should not on what you/I posted in the previous comment. > > Yes. > > > The difference seems to be some kind of implicit `CompoundStmt` added by > > `IfStmt`? > > CompoundStmt represents the `{}`. > > > Assuming that is the case, maybe this is as simple as checking whether this > > `CompoundStmt` is implicit or not? > > My prototype of this feature used `ifStmt(stmt().bind("if"), > unless(hasParent(ifStmt(hasElse(equalsBoundNode("if"))`. The fix should ideally be fully contained in the `TraverseStmt()`, and should not be noticeably slower. >> Best to move open a new bug about this. I'll see what can be done. (that should have been "Best to open a new bug about this. I'll see what can be done.") > Do you need me to report a bug? Yes, and assign it to me, if possible. Repository: rL LLVM https://reviews.llvm.org/D32942 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r305376 - [clangd] Add a filename parameter to FileSystemProvider.
Author: ibiryukov Date: Wed Jun 14 04:46:44 2017 New Revision: 305376 URL: http://llvm.org/viewvc/llvm-project?rev=305376&view=rev Log: [clangd] Add a filename parameter to FileSystemProvider. Reviewers: krasimir Reviewed By: krasimir Subscribers: klimek, cfe-commits Tags: #clang-tools-extra Differential Revision: https://reviews.llvm.org/D34151 Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp clang-tools-extra/trunk/clangd/ClangdServer.h clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=305376&r1=305375&r2=305376&view=diff == --- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original) +++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Wed Jun 14 04:46:44 2017 @@ -59,7 +59,7 @@ Position clangd::offsetToPosition(String } Tagged> -RealFileSystemProvider::getTaggedFileSystem() { +RealFileSystemProvider::getTaggedFileSystem(PathRef File) { return make_tagged(vfs::getRealFileSystem(), VFSTag()); } @@ -156,7 +156,7 @@ void ClangdServer::addDocument(PathRef F assert(FileContents.Draft && "No contents inside a file that was scheduled for reparse"); -auto TaggedFS = FSProvider.getTaggedFileSystem(); +auto TaggedFS = FSProvider.getTaggedFileSystem(FileStr); Units.runOnUnit( FileStr, *FileContents.Draft, CDB, PCHs, TaggedFS.Value, [&](ClangdUnit const &Unit) { @@ -197,7 +197,7 @@ ClangdServer::codeComplete(PathRef File, } std::vector Result; - auto TaggedFS = FSProvider.getTaggedFileSystem(); + auto TaggedFS = FSProvider.getTaggedFileSystem(File); // It would be nice to use runOnUnitWithoutReparse here, but we can't // guarantee the correctness of code completion cache here if we don't do the // reparse. Modified: clang-tools-extra/trunk/clangd/ClangdServer.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.h?rev=305376&r1=305375&r2=305376&view=diff == --- clang-tools-extra/trunk/clangd/ClangdServer.h (original) +++ clang-tools-extra/trunk/clangd/ClangdServer.h Wed Jun 14 04:46:44 2017 @@ -82,16 +82,21 @@ public: class FileSystemProvider { public: virtual ~FileSystemProvider() = default; + /// Called by ClangdServer to obtain a vfs::FileSystem to be used for parsing. + /// Name of the file that will be parsed is passed in \p File. + /// /// \return A filesystem that will be used for all file accesses in clangd. /// A Tag returned by this method will be propagated to all results of clangd /// that will use this filesystem. - virtual Tagged> getTaggedFileSystem() = 0; + virtual Tagged> + getTaggedFileSystem(PathRef File) = 0; }; class RealFileSystemProvider : public FileSystemProvider { public: /// \return getRealFileSystem() tagged with default tag, i.e. VFSTag() - Tagged> getTaggedFileSystem() override; + Tagged> + getTaggedFileSystem(PathRef File) override; }; class ClangdServer; Modified: clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp?rev=305376&r1=305375&r2=305376&view=diff == --- clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp (original) +++ clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp Wed Jun 14 04:46:44 2017 @@ -172,9 +172,13 @@ public: class MockFSProvider : public FileSystemProvider { public: - Tagged> getTaggedFileSystem() override { + Tagged> + getTaggedFileSystem(PathRef File) override { IntrusiveRefCntPtr MemFS( new vfs::InMemoryFileSystem); +if (ExpectedFile) + EXPECT_EQ(*ExpectedFile, File); + for (auto &FileAndContents : Files) MemFS->addFile(FileAndContents.first(), time_t(), llvm::MemoryBuffer::getMemBuffer(FileAndContents.second, @@ -186,6 +190,7 @@ public: return make_tagged(OverlayFS, Tag); } + llvm::Optional> ExpectedFile; llvm::StringMap Files; VFSTag Tag = VFSTag(); }; @@ -248,7 +253,10 @@ protected: FileWithContents.second; auto SourceFilename = getVirtualTestFilePath(SourceFileRelPath); + +FS.ExpectedFile = SourceFilename; Server.addDocument(SourceFilename, SourceContents); + auto Result = dumpASTWithoutMemoryLocs(Server, SourceFilename); EXPECT_EQ(ExpectErrors, DiagConsumer.hadErrorInLastDiags()); return Result; @@ -307,6 +315,7 @@ int b = a; FS.Files[FooH] = "int a;"; FS.Files[FooCpp] = SourceContents; + FS.ExpectedFile = FooCpp; Server.addDocument(FooCpp, SourceContents); auto DumpParse1 = dumpASTWithoutMemoryLocs(Server, FooCpp); @@ -342,6 +351,7 @@ int b =
[PATCH] D34151: [clangd] Add a filename parameter to FileSystemProvider.
This revision was automatically updated to reflect the committed changes. Closed by commit rL305376: [clangd] Add a filename parameter to FileSystemProvider. (authored by ibiryukov). Changed prior to commit: https://reviews.llvm.org/D34151?vs=102347&id=102508#toc Repository: rL LLVM https://reviews.llvm.org/D34151 Files: clang-tools-extra/trunk/clangd/ClangdServer.cpp clang-tools-extra/trunk/clangd/ClangdServer.h clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp Index: clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp === --- clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp +++ clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp @@ -172,9 +172,13 @@ class MockFSProvider : public FileSystemProvider { public: - Tagged> getTaggedFileSystem() override { + Tagged> + getTaggedFileSystem(PathRef File) override { IntrusiveRefCntPtr MemFS( new vfs::InMemoryFileSystem); +if (ExpectedFile) + EXPECT_EQ(*ExpectedFile, File); + for (auto &FileAndContents : Files) MemFS->addFile(FileAndContents.first(), time_t(), llvm::MemoryBuffer::getMemBuffer(FileAndContents.second, @@ -186,6 +190,7 @@ return make_tagged(OverlayFS, Tag); } + llvm::Optional> ExpectedFile; llvm::StringMap Files; VFSTag Tag = VFSTag(); }; @@ -248,7 +253,10 @@ FileWithContents.second; auto SourceFilename = getVirtualTestFilePath(SourceFileRelPath); + +FS.ExpectedFile = SourceFilename; Server.addDocument(SourceFilename, SourceContents); + auto Result = dumpASTWithoutMemoryLocs(Server, SourceFilename); EXPECT_EQ(ExpectErrors, DiagConsumer.hadErrorInLastDiags()); return Result; @@ -307,6 +315,7 @@ FS.Files[FooH] = "int a;"; FS.Files[FooCpp] = SourceContents; + FS.ExpectedFile = FooCpp; Server.addDocument(FooCpp, SourceContents); auto DumpParse1 = dumpASTWithoutMemoryLocs(Server, FooCpp); @@ -342,6 +351,7 @@ FS.Files[FooH] = "int a;"; FS.Files[FooCpp] = SourceContents; + FS.ExpectedFile = FooCpp; Server.addDocument(FooCpp, SourceContents); auto DumpParse1 = dumpASTWithoutMemoryLocs(Server, FooCpp); @@ -371,8 +381,9 @@ auto FooCpp = getVirtualTestFilePath("foo.cpp"); const auto SourceContents = "int a;"; FS.Files[FooCpp] = SourceContents; - FS.Tag = "123"; + FS.ExpectedFile = FooCpp; + FS.Tag = "123"; Server.addDocument(FooCpp, SourceContents); EXPECT_EQ(DiagConsumer.lastVFSTag(), FS.Tag); EXPECT_EQ(Server.codeComplete(FooCpp, Position{0, 0}).Tag, FS.Tag); @@ -419,6 +430,7 @@ // miss). Position CompletePos = {2, 8}; FS.Files[FooCpp] = SourceContents; + FS.ExpectedFile = FooCpp; Server.addDocument(FooCpp, SourceContents); Index: clang-tools-extra/trunk/clangd/ClangdServer.h === --- clang-tools-extra/trunk/clangd/ClangdServer.h +++ clang-tools-extra/trunk/clangd/ClangdServer.h @@ -82,16 +82,21 @@ class FileSystemProvider { public: virtual ~FileSystemProvider() = default; + /// Called by ClangdServer to obtain a vfs::FileSystem to be used for parsing. + /// Name of the file that will be parsed is passed in \p File. + /// /// \return A filesystem that will be used for all file accesses in clangd. /// A Tag returned by this method will be propagated to all results of clangd /// that will use this filesystem. - virtual Tagged> getTaggedFileSystem() = 0; + virtual Tagged> + getTaggedFileSystem(PathRef File) = 0; }; class RealFileSystemProvider : public FileSystemProvider { public: /// \return getRealFileSystem() tagged with default tag, i.e. VFSTag() - Tagged> getTaggedFileSystem() override; + Tagged> + getTaggedFileSystem(PathRef File) override; }; class ClangdServer; Index: clang-tools-extra/trunk/clangd/ClangdServer.cpp === --- clang-tools-extra/trunk/clangd/ClangdServer.cpp +++ clang-tools-extra/trunk/clangd/ClangdServer.cpp @@ -59,7 +59,7 @@ } Tagged> -RealFileSystemProvider::getTaggedFileSystem() { +RealFileSystemProvider::getTaggedFileSystem(PathRef File) { return make_tagged(vfs::getRealFileSystem(), VFSTag()); } @@ -156,7 +156,7 @@ assert(FileContents.Draft && "No contents inside a file that was scheduled for reparse"); -auto TaggedFS = FSProvider.getTaggedFileSystem(); +auto TaggedFS = FSProvider.getTaggedFileSystem(FileStr); Units.runOnUnit( FileStr, *FileContents.Draft, CDB, PCHs, TaggedFS.Value, [&](ClangdUnit const &Unit) { @@ -197,7 +197,7 @@ } std::vector Result; - auto TaggedFS = FSProvider.getTaggedFileSystem(); + auto TaggedFS = FSProvider.getTaggedFileSystem(File); // It would be nice to use runOnUnitWithoutReparse here, but we can't // guarantee the correctness of code completion cache here if we don't do the
[PATCH] D34137: [clangd] Add priority to completion item sort text
ilya-biryukov added inline comments. Comment at: clangd/ClangdUnit.cpp:156 assert(CCS->getTypedText()); +CCS->getPriority(); Item.kind = getKind(Result.CursorKind); This is a no-op, commited incidentally? Comment at: clangd/ClangdUnit.cpp:163 +// is 99964a. +llvm::raw_string_ostream(Item.sortText) << llvm::format( +"%05d%s", 9 - CCS->getPriority(), CCS->getTypedText()); Maybe add an assert(CCS.getPriority() < 9)? https://reviews.llvm.org/D34137 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r305377 - Fix for Itanium mangler issue with templates
Author: dpolukhin Date: Wed Jun 14 04:47:47 2017 New Revision: 305377 URL: http://llvm.org/viewvc/llvm-project?rev=305377&view=rev Log: Fix for Itanium mangler issue with templates Patch by Serge Preis Differential Revision: https://reviews.llvm.org/D32428 Modified: cfe/trunk/lib/AST/ItaniumMangle.cpp Modified: cfe/trunk/lib/AST/ItaniumMangle.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ItaniumMangle.cpp?rev=305377&r1=305376&r2=305377&view=diff == --- cfe/trunk/lib/AST/ItaniumMangle.cpp (original) +++ cfe/trunk/lib/AST/ItaniumMangle.cpp Wed Jun 14 04:47:47 2017 @@ -4550,9 +4550,11 @@ CXXNameMangler::makeFunctionReturnTypeTa const FunctionProtoType *Proto = cast(FD->getType()->getAs()); + FunctionTypeDepthState saved = TrackReturnTypeTags.FunctionTypeDepth.push(); TrackReturnTypeTags.FunctionTypeDepth.enterResultType(); TrackReturnTypeTags.mangleType(Proto->getReturnType()); TrackReturnTypeTags.FunctionTypeDepth.leaveResultType(); + TrackReturnTypeTags.FunctionTypeDepth.pop(saved); return TrackReturnTypeTags.AbiTagsRoot.getSortedUniqueUsedAbiTags(); } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32428: Fix for Itanium mangler issue with templates (bug: 31405)
This revision was automatically updated to reflect the committed changes. Closed by commit rL305377: Fix for Itanium mangler issue with templates (authored by dpolukhin). Changed prior to commit: https://reviews.llvm.org/D32428?vs=96380&id=102509#toc Repository: rL LLVM https://reviews.llvm.org/D32428 Files: cfe/trunk/lib/AST/ItaniumMangle.cpp Index: cfe/trunk/lib/AST/ItaniumMangle.cpp === --- cfe/trunk/lib/AST/ItaniumMangle.cpp +++ cfe/trunk/lib/AST/ItaniumMangle.cpp @@ -4550,9 +4550,11 @@ const FunctionProtoType *Proto = cast(FD->getType()->getAs()); + FunctionTypeDepthState saved = TrackReturnTypeTags.FunctionTypeDepth.push(); TrackReturnTypeTags.FunctionTypeDepth.enterResultType(); TrackReturnTypeTags.mangleType(Proto->getReturnType()); TrackReturnTypeTags.FunctionTypeDepth.leaveResultType(); + TrackReturnTypeTags.FunctionTypeDepth.pop(saved); return TrackReturnTypeTags.AbiTagsRoot.getSortedUniqueUsedAbiTags(); } Index: cfe/trunk/lib/AST/ItaniumMangle.cpp === --- cfe/trunk/lib/AST/ItaniumMangle.cpp +++ cfe/trunk/lib/AST/ItaniumMangle.cpp @@ -4550,9 +4550,11 @@ const FunctionProtoType *Proto = cast(FD->getType()->getAs()); + FunctionTypeDepthState saved = TrackReturnTypeTags.FunctionTypeDepth.push(); TrackReturnTypeTags.FunctionTypeDepth.enterResultType(); TrackReturnTypeTags.mangleType(Proto->getReturnType()); TrackReturnTypeTags.FunctionTypeDepth.leaveResultType(); + TrackReturnTypeTags.FunctionTypeDepth.pop(saved); return TrackReturnTypeTags.AbiTagsRoot.getSortedUniqueUsedAbiTags(); } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32942: [clang-tidy] readability-function-size: add NestingThreshold param.
lebedev.ri added a comment. Hm, oh, maybe this is as simple as a partially-unintentional `switch` fallthrough Repository: rL LLVM https://reviews.llvm.org/D32942 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r305379 - Function with unparsed body is a definition
Author: sepavloff Date: Wed Jun 14 05:07:02 2017 New Revision: 305379 URL: http://llvm.org/viewvc/llvm-project?rev=305379&view=rev Log: Function with unparsed body is a definition While a function body is being parsed, the function declaration is not considered as a definition because it does not have a body yet. In some cases it leads to incorrect interpretation, the case is presented in https://bugs.llvm.org/show_bug.cgi?id=14785: ``` template struct Somewhat { void internal() const {} friend void operator+(int const &, Somewhat const &) {} }; void operator+(int const &, Somewhat const &x) { x.internal(); } ``` When statement `x.internal()` in the body of global `operator+` is parsed, the type of `x` must be completed, so the instantiation of `Somewhat` is started. It instantiates the declaration of `operator+` defined inline, and makes a check for redefinition. The check does not detect another definition because the declaration of `operator+` is still not defining as does not have a body yet. To solves this problem the function `isThisDeclarationADefinition` considers a function declaration as a definition if it has flag `WillHaveBody` set. This change fixes PR14785. Differential Revision: https://reviews.llvm.org/D30375 Modified: cfe/trunk/include/clang/AST/Decl.h cfe/trunk/lib/Sema/SemaCUDA.cpp cfe/trunk/lib/Sema/SemaDecl.cpp cfe/trunk/lib/Sema/SemaDeclCXX.cpp cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp cfe/trunk/test/SemaCXX/friend2.cpp Modified: cfe/trunk/include/clang/AST/Decl.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Decl.h?rev=305379&r1=305378&r2=305379&view=diff == --- cfe/trunk/include/clang/AST/Decl.h (original) +++ cfe/trunk/include/clang/AST/Decl.h Wed Jun 14 05:07:02 2017 @@ -1872,7 +1872,7 @@ public: /// bool isThisDeclarationADefinition() const { return IsDeleted || IsDefaulted || Body || IsLateTemplateParsed || - hasDefiningAttr(); + WillHaveBody || hasDefiningAttr(); } /// doesThisDeclarationHaveABody - Returns whether this specific Modified: cfe/trunk/lib/Sema/SemaCUDA.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCUDA.cpp?rev=305379&r1=305378&r2=305379&view=diff == --- cfe/trunk/lib/Sema/SemaCUDA.cpp (original) +++ cfe/trunk/lib/Sema/SemaCUDA.cpp Wed Jun 14 05:07:02 2017 @@ -629,12 +629,6 @@ static bool IsKnownEmitted(Sema &S, Func // emitted, because (say) the definition could include "inline". FunctionDecl *Def = FD->getDefinition(); - // We may currently be parsing the body of FD, in which case - // FD->getDefinition() will be null, but we still want to treat FD as though - // it's a definition. - if (!Def && FD->willHaveBody()) -Def = FD; - if (Def && !isDiscardableGVALinkage(S.getASTContext().GetGVALinkageForFunction(Def))) return true; Modified: cfe/trunk/lib/Sema/SemaDecl.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=305379&r1=305378&r2=305379&view=diff == --- cfe/trunk/lib/Sema/SemaDecl.cpp (original) +++ cfe/trunk/lib/Sema/SemaDecl.cpp Wed Jun 14 05:07:02 2017 @@ -12218,6 +12218,7 @@ Decl *Sema::ActOnFinishFunctionBody(Decl if (FD) { FD->setBody(Body); +FD->setWillHaveBody(false); if (getLangOpts().CPlusPlus14) { if (!FD->isInvalidDecl() && Body && !FD->isDependentContext() && Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=305379&r1=305378&r2=305379&view=diff == --- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original) +++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Wed Jun 14 05:07:02 2017 @@ -13878,6 +13878,9 @@ void Sema::SetDeclDeleted(Decl *Dcl, Sou return; } + // Deleted function does not have a body. + Fn->setWillHaveBody(false); + if (const FunctionDecl *Prev = Fn->getPreviousDecl()) { // Don't consider the implicit declaration we generate for explicit // specializations. FIXME: Do not generate these implicit declarations. Modified: cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp?rev=305379&r1=305378&r2=305379&view=diff == --- cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp (original) +++ cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp Wed Jun 14 05:07:02 2017 @@ -1782,6 +1782,12 @@ Decl *TemplateDeclInstantiator::VisitFun Previous.clear(); } + if (isFriend) { +Function->setObjectOfFriendDecl(); +if (FunctionTemplate) + FunctionTemplate->setOb
[PATCH] D30375: Function with unparsed body is a definition
This revision was automatically updated to reflect the committed changes. Closed by commit rL305379: Function with unparsed body is a definition (authored by sepavloff). Changed prior to commit: https://reviews.llvm.org/D30375?vs=101993&id=102512#toc Repository: rL LLVM https://reviews.llvm.org/D30375 Files: cfe/trunk/include/clang/AST/Decl.h cfe/trunk/lib/Sema/SemaCUDA.cpp cfe/trunk/lib/Sema/SemaDecl.cpp cfe/trunk/lib/Sema/SemaDeclCXX.cpp cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp cfe/trunk/test/SemaCXX/friend2.cpp Index: cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp === --- cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp +++ cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp @@ -1782,6 +1782,12 @@ Previous.clear(); } + if (isFriend) { +Function->setObjectOfFriendDecl(); +if (FunctionTemplate) + FunctionTemplate->setObjectOfFriendDecl(); + } + SemaRef.CheckFunctionDeclaration(/*Scope*/ nullptr, Function, Previous, isExplicitSpecialization); @@ -1792,7 +1798,6 @@ // If the original function was part of a friend declaration, // inherit its namespace state and add it to the owner. if (isFriend) { -PrincipalDecl->setObjectOfFriendDecl(); DC->makeDeclVisibleInContext(PrincipalDecl); bool QueuedInstantiation = false; Index: cfe/trunk/lib/Sema/SemaDecl.cpp === --- cfe/trunk/lib/Sema/SemaDecl.cpp +++ cfe/trunk/lib/Sema/SemaDecl.cpp @@ -12218,6 +12218,7 @@ if (FD) { FD->setBody(Body); +FD->setWillHaveBody(false); if (getLangOpts().CPlusPlus14) { if (!FD->isInvalidDecl() && Body && !FD->isDependentContext() && Index: cfe/trunk/lib/Sema/SemaDeclCXX.cpp === --- cfe/trunk/lib/Sema/SemaDeclCXX.cpp +++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp @@ -13878,6 +13878,9 @@ return; } + // Deleted function does not have a body. + Fn->setWillHaveBody(false); + if (const FunctionDecl *Prev = Fn->getPreviousDecl()) { // Don't consider the implicit declaration we generate for explicit // specializations. FIXME: Do not generate these implicit declarations. Index: cfe/trunk/lib/Sema/SemaCUDA.cpp === --- cfe/trunk/lib/Sema/SemaCUDA.cpp +++ cfe/trunk/lib/Sema/SemaCUDA.cpp @@ -629,12 +629,6 @@ // emitted, because (say) the definition could include "inline". FunctionDecl *Def = FD->getDefinition(); - // We may currently be parsing the body of FD, in which case - // FD->getDefinition() will be null, but we still want to treat FD as though - // it's a definition. - if (!Def && FD->willHaveBody()) -Def = FD; - if (Def && !isDiscardableGVALinkage(S.getASTContext().GetGVALinkageForFunction(Def))) return true; Index: cfe/trunk/include/clang/AST/Decl.h === --- cfe/trunk/include/clang/AST/Decl.h +++ cfe/trunk/include/clang/AST/Decl.h @@ -1872,7 +1872,7 @@ /// bool isThisDeclarationADefinition() const { return IsDeleted || IsDefaulted || Body || IsLateTemplateParsed || - hasDefiningAttr(); + WillHaveBody || hasDefiningAttr(); } /// doesThisDeclarationHaveABody - Returns whether this specific Index: cfe/trunk/test/SemaCXX/friend2.cpp === --- cfe/trunk/test/SemaCXX/friend2.cpp +++ cfe/trunk/test/SemaCXX/friend2.cpp @@ -170,3 +170,15 @@ template class Test; } + +namespace pr14785 { +template +struct Somewhat { + void internal() const { } + friend void operator+(int const &, Somewhat const &) {} // expected-error{{redefinition of 'operator+'}} +}; + +void operator+(int const &, Somewhat const &x) { // expected-note {{previous definition is here}} + x.internal(); // expected-note{{in instantiation of template class 'pr14785::Somewhat' requested here}} +} +} ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30375: Function with unparsed body is a definition
sepavloff added a comment. Thank you! Committed as is, improvements will be made as separate patches. Repository: rL LLVM https://reviews.llvm.org/D30375 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34198: Fix __has_trivial_destructor crash when the type is incomplete with unknown array bounds.
puneetha updated this revision to Diff 102516. puneetha added a comment. Incorrectly rejecting __is_destructible queries on arrays of unknown bound of incomplete types. https://reviews.llvm.org/D34198 Files: lib/Sema/SemaExprCXX.cpp Index: lib/Sema/SemaExprCXX.cpp === --- lib/Sema/SemaExprCXX.cpp +++ lib/Sema/SemaExprCXX.cpp @@ -4101,6 +4101,11 @@ case UTT_IsDestructible: case UTT_IsNothrowDestructible: case UTT_IsTriviallyDestructible: +if (ArgTy->isIncompleteArrayType() || ArgTy->isVoidType()) + return true; + +return !S.RequireCompleteType( +Loc, ArgTy, diag::err_incomplete_type_used_in_type_trait_expr); // Per the GCC type traits documentation, the same constraints apply to these. case UTT_HasNothrowAssign: case UTT_HasNothrowMoveAssign: @@ -4113,9 +4118,15 @@ case UTT_HasTrivialCopy: case UTT_HasTrivialDestructor: case UTT_HasVirtualDestructor: -if (ArgTy->isIncompleteArrayType() || ArgTy->isVoidType()) +if(ArgTy->isVoidType()) return true; +if (ArgTy->isIncompleteArrayType()) { + QualType ElTy = QualType(ArgTy->getBaseElementTypeUnsafe(), 0); + if (!ElTy->isIncompleteType()) +return true; +} + return !S.RequireCompleteType( Loc, ArgTy, diag::err_incomplete_type_used_in_type_trait_expr); } Index: lib/Sema/SemaExprCXX.cpp === --- lib/Sema/SemaExprCXX.cpp +++ lib/Sema/SemaExprCXX.cpp @@ -4101,6 +4101,11 @@ case UTT_IsDestructible: case UTT_IsNothrowDestructible: case UTT_IsTriviallyDestructible: +if (ArgTy->isIncompleteArrayType() || ArgTy->isVoidType()) + return true; + +return !S.RequireCompleteType( +Loc, ArgTy, diag::err_incomplete_type_used_in_type_trait_expr); // Per the GCC type traits documentation, the same constraints apply to these. case UTT_HasNothrowAssign: case UTT_HasNothrowMoveAssign: @@ -4113,9 +4118,15 @@ case UTT_HasTrivialCopy: case UTT_HasTrivialDestructor: case UTT_HasVirtualDestructor: -if (ArgTy->isIncompleteArrayType() || ArgTy->isVoidType()) +if(ArgTy->isVoidType()) return true; +if (ArgTy->isIncompleteArrayType()) { + QualType ElTy = QualType(ArgTy->getBaseElementTypeUnsafe(), 0); + if (!ElTy->isIncompleteType()) +return true; +} + return !S.RequireCompleteType( Loc, ArgTy, diag::err_incomplete_type_used_in_type_trait_expr); } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30946: [ScopePrinting] Added support to print full scopes of types and declarations.
schroedersi added a comment. Would it help the acceptance of the patch if I add a fourth option `LegacySuppressScope` (or similar) that emulates the current/pre-patch `SuppressScope==true`? This way the patched type printing would be a real superset of the current/non-patched type printing (except for some small changes to `SuppressScope==false`/`DefaultScope` as seen in the diff of tests `p6.cpp` and `comment-cplus-decls.cpp`). https://reviews.llvm.org/D30946 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r305381 - Reverted 305379 (Function with unparsed body is a definition)
Author: sepavloff Date: Wed Jun 14 05:57:56 2017 New Revision: 305381 URL: http://llvm.org/viewvc/llvm-project?rev=305381&view=rev Log: Reverted 305379 (Function with unparsed body is a definition) It broke clang-x86_64-linux-selfhost-modules-2 and some other buildbots. Modified: cfe/trunk/include/clang/AST/Decl.h cfe/trunk/lib/Sema/SemaCUDA.cpp cfe/trunk/lib/Sema/SemaDecl.cpp cfe/trunk/lib/Sema/SemaDeclCXX.cpp cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp cfe/trunk/test/SemaCXX/friend2.cpp Modified: cfe/trunk/include/clang/AST/Decl.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Decl.h?rev=305381&r1=305380&r2=305381&view=diff == --- cfe/trunk/include/clang/AST/Decl.h (original) +++ cfe/trunk/include/clang/AST/Decl.h Wed Jun 14 05:57:56 2017 @@ -1872,7 +1872,7 @@ public: /// bool isThisDeclarationADefinition() const { return IsDeleted || IsDefaulted || Body || IsLateTemplateParsed || - WillHaveBody || hasDefiningAttr(); + hasDefiningAttr(); } /// doesThisDeclarationHaveABody - Returns whether this specific Modified: cfe/trunk/lib/Sema/SemaCUDA.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCUDA.cpp?rev=305381&r1=305380&r2=305381&view=diff == --- cfe/trunk/lib/Sema/SemaCUDA.cpp (original) +++ cfe/trunk/lib/Sema/SemaCUDA.cpp Wed Jun 14 05:57:56 2017 @@ -629,6 +629,12 @@ static bool IsKnownEmitted(Sema &S, Func // emitted, because (say) the definition could include "inline". FunctionDecl *Def = FD->getDefinition(); + // We may currently be parsing the body of FD, in which case + // FD->getDefinition() will be null, but we still want to treat FD as though + // it's a definition. + if (!Def && FD->willHaveBody()) +Def = FD; + if (Def && !isDiscardableGVALinkage(S.getASTContext().GetGVALinkageForFunction(Def))) return true; Modified: cfe/trunk/lib/Sema/SemaDecl.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=305381&r1=305380&r2=305381&view=diff == --- cfe/trunk/lib/Sema/SemaDecl.cpp (original) +++ cfe/trunk/lib/Sema/SemaDecl.cpp Wed Jun 14 05:57:56 2017 @@ -12218,7 +12218,6 @@ Decl *Sema::ActOnFinishFunctionBody(Decl if (FD) { FD->setBody(Body); -FD->setWillHaveBody(false); if (getLangOpts().CPlusPlus14) { if (!FD->isInvalidDecl() && Body && !FD->isDependentContext() && Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=305381&r1=305380&r2=305381&view=diff == --- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original) +++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Wed Jun 14 05:57:56 2017 @@ -13878,9 +13878,6 @@ void Sema::SetDeclDeleted(Decl *Dcl, Sou return; } - // Deleted function does not have a body. - Fn->setWillHaveBody(false); - if (const FunctionDecl *Prev = Fn->getPreviousDecl()) { // Don't consider the implicit declaration we generate for explicit // specializations. FIXME: Do not generate these implicit declarations. Modified: cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp?rev=305381&r1=305380&r2=305381&view=diff == --- cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp (original) +++ cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp Wed Jun 14 05:57:56 2017 @@ -1782,12 +1782,6 @@ Decl *TemplateDeclInstantiator::VisitFun Previous.clear(); } - if (isFriend) { -Function->setObjectOfFriendDecl(); -if (FunctionTemplate) - FunctionTemplate->setObjectOfFriendDecl(); - } - SemaRef.CheckFunctionDeclaration(/*Scope*/ nullptr, Function, Previous, isExplicitSpecialization); @@ -1798,6 +1792,7 @@ Decl *TemplateDeclInstantiator::VisitFun // If the original function was part of a friend declaration, // inherit its namespace state and add it to the owner. if (isFriend) { +PrincipalDecl->setObjectOfFriendDecl(); DC->makeDeclVisibleInContext(PrincipalDecl); bool QueuedInstantiation = false; Modified: cfe/trunk/test/SemaCXX/friend2.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/friend2.cpp?rev=305381&r1=305380&r2=305381&view=diff == --- cfe/trunk/test/SemaCXX/friend2.cpp (original) +++ cfe/trunk/test/SemaCXX/friend2.cpp Wed Jun 14 05:57:56 2017 @@ -170,15 +170,3 @@ struct Test { template class Test; } - -namespace pr14785 { -template -struct Somewhat { - void internal() co
[PATCH] D34137: [clangd] Add priority to completion item sort text
krasimir updated this revision to Diff 102522. krasimir added a comment. - Address review comments https://reviews.llvm.org/D34137 Files: clangd/ClangdUnit.cpp test/clangd/authority-less-uri.test test/clangd/completion.test Index: test/clangd/completion.test === --- test/clangd/completion.test +++ test/clangd/completion.test @@ -16,25 +16,25 @@ # nondeterministic, so we check regardless of order. # # CHECK: {"jsonrpc":"2.0","id":1,"result":[ -# CHECK-DAG: {"label":"a","kind":5,"detail":"int","sortText":"a","filterText":"a","insertText":"a"} -# CHECK-DAG: {"label":"bb","kind":5,"detail":"int","sortText":"bb","filterText":"bb","insertText":"bb"} -# CHECK-DAG: {"label":"ccc","kind":5,"detail":"int","sortText":"ccc","filterText":"ccc","insertText":"ccc"} -# CHECK-DAG: {"label":"operator=(const fake &)","kind":2,"detail":"fake &","sortText":"operator=","filterText":"operator=","insertText":"operator="} -# CHECK-DAG: {"label":"~fake()","kind":4,"detail":"void","sortText":"~fake","filterText":"~fake","insertText":"~fake"} -# CHECK-DAG: {"label":"f(int i, const float f) const","kind":2,"detail":"int","sortText":"f","filterText":"f","insertText":"f"} +# CHECK-DAG: {"label":"a","kind":5,"detail":"int","sortText":"99964a","filterText":"a","insertText":"a"} +# CHECK-DAG: {"label":"bb","kind":5,"detail":"int","sortText":"99964bb","filterText":"bb","insertText":"bb"} +# CHECK-DAG: {"label":"ccc","kind":5,"detail":"int","sortText":"99964ccc","filterText":"ccc","insertText":"ccc"} +# CHECK-DAG: {"label":"operator=(const fake &)","kind":2,"detail":"fake &","sortText":"99965operator=","filterText":"operator=","insertText":"operator="} +# CHECK-DAG: {"label":"~fake()","kind":4,"detail":"void","sortText":"99965~fake","filterText":"~fake","insertText":"~fake"} +# CHECK-DAG: {"label":"f(int i, const float f) const","kind":2,"detail":"int","sortText":"99964f","filterText":"f","insertText":"f"} # CHECK: ]} Content-Length: 148 {"jsonrpc":"2.0","id":2,"method":"textDocument/completion","params":{"textDocument":{"uri":"file:///main.cpp"},"position":{"line":3,"character":5}}} # Repeat the completion request, expect the same results. # # CHECK: {"jsonrpc":"2.0","id":2,"result":[ -# CHECK-DAG: {"label":"a","kind":5,"detail":"int","sortText":"a","filterText":"a","insertText":"a"} -# CHECK-DAG: {"label":"bb","kind":5,"detail":"int","sortText":"bb","filterText":"bb","insertText":"bb"} -# CHECK-DAG: {"label":"ccc","kind":5,"detail":"int","sortText":"ccc","filterText":"ccc","insertText":"ccc"} -# CHECK-DAG: {"label":"operator=(const fake &)","kind":2,"detail":"fake &","sortText":"operator=","filterText":"operator=","insertText":"operator="} -# CHECK-DAG: {"label":"~fake()","kind":4,"detail":"void","sortText":"~fake","filterText":"~fake","insertText":"~fake"} -# CHECK-DAG: {"label":"f(int i, const float f) const","kind":2,"detail":"int","sortText":"f","filterText":"f","insertText":"f"} +# CHECK-DAG: {"label":"a","kind":5,"detail":"int","sortText":"99964a","filterText":"a","insertText":"a"} +# CHECK-DAG: {"label":"bb","kind":5,"detail":"int","sortText":"99964bb","filterText":"bb","insertText":"bb"} +# CHECK-DAG: {"label":"ccc","kind":5,"detail":"int","sortText":"99964ccc","filterText":"ccc","insertText":"ccc"} +# CHECK-DAG: {"label":"operator=(const fake &)","kind":2,"detail":"fake &","sortText":"99965operator=","filterText":"operator=","insertText":"operator="} +# CHECK-DAG: {"label":"~fake()","kind":4,"detail":"void","sortText":"99965~fake","filterText":"~fake","insertText":"~fake"} +# CHECK-DAG: {"label":"f(int i, const float f) const","kind":2,"detail":"int","sortText":"99964f","filterText":"f","insertText":"f"} # CHECK: ]} # Update the source file and check for completions again. Content-Length: 226 @@ -47,7 +47,7 @@ # Repeat the completion request, expect the same results. # # CHECK: {"jsonrpc":"2.0","id":3,"result":[ -# CHECK-DAG: {"label":"func()","kind":2,"detail":"int (*)(int, int)","sortText":"func","filterText":"func","insertText":"func"} +# CHECK-DAG: {"label":"func()","kind":2,"detail":"int (*)(int, int)","sortText":"99965func","filterText":"func","insertText":"func"} # CHECK: ]} Content-Length: 44 Index: test/clangd/authority-less-uri.test === --- test/clangd/authority-less-uri.test +++ test/clangd/authority-less-uri.test @@ -16,16 +16,16 @@ # Test authority-less URI # # CHECK: {"jsonrpc":"2.0","id":1,"result":[ -# CHECK-DAG: {"label":"a","kind":5,"detail":"int","sortText":"a","filterText":"a","insertText":"a"} +# CHECK-DAG: {"label":"a","kind":5,"detail":"int","sortText":"99964a","filterText":"a","insertText":"a"} # CHECK: ]} Content-Length: 172 {"jsonrpc":"2.0","id":2,"method":"textDocument/completion","params":{"textDocument":{"uri":"file:///main.cpp"},"uri":"file:///main.cpp","position":{"line":3,"character":5}}} # Test params parsing in the presence of a
[PATCH] D32480: clang-format: Add CompactNamespaces option
djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Yeah, looks good. Krasimir, any further concerns? https://reviews.llvm.org/D32480 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32480: clang-format: Add CompactNamespaces option
Typz updated this revision to Diff 102525. Typz added a comment. Make tests more readable https://reviews.llvm.org/D32480 Files: include/clang/Format/Format.h lib/Format/Format.cpp 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 @@ -1309,6 +1309,166 @@ Style)); } +TEST_F(FormatTest, FormatsCompactNamespaces) { + FormatStyle Style = getLLVMStyle(); + Style.CompactNamespaces = true; + + verifyFormat("namespace A { namespace B {\n" + "}} // namespace A::B", + Style); + + EXPECT_EQ("namespace out { namespace in {\n" +"}} // namespace out::in", +format("namespace out {\n" + "namespace in {\n" + "} // namespace in\n" + "} // namespace out", + Style)); + + // Only namespaces which have both consecutive opening and end get compacted + EXPECT_EQ("namespace out {\n" +"namespace in1 {\n" +"} // namespace in1\n" +"namespace in2 {\n" +"} // namespace in2\n" +"} // namespace out", +format("namespace out {\n" + "namespace in1 {\n" + "} // namespace in1\n" + "namespace in2 {\n" + "} // namespace in2\n" + "} // namespace out", + Style)); + + EXPECT_EQ("namespace out {\n" +"int i;\n" +"namespace in {\n" +"int j;\n" +"} // namespace in\n" +"int k;\n" +"} // namespace out", +format("namespace out { int i;\n" + "namespace in { int j; } // namespace in\n" + "int k; } // namespace out", + Style)); + + EXPECT_EQ("namespace A { namespace B { namespace C {\n" +"}}} // namespace A::B::C\n", +format("namespace A { namespace B {\n" + "namespace C {\n" + "}} // namespace B::C\n" + "} // namespace A\n", + Style)); + + Style.ColumnLimit = 40; + EXPECT_EQ("namespace aa {\n" +"namespace bb {\n" +"}} // namespace aa::bb", +format("namespace aa {\n" + "namespace bb {\n" + "} // namespace bb\n" + "} // namespace aa", + Style)); + + EXPECT_EQ("namespace aa { namespace bb {\n" +"namespace cc {\n" +"}}} // namespace aa::bb::cc", +format("namespace aa {\n" + "namespace bb {\n" + "namespace cc {\n" + "} // namespace cc\n" + "} // namespace bb\n" + "} // namespace aa", + Style)); + Style.ColumnLimit = 80; + + // Missing comments are added + EXPECT_EQ("namespace out { namespace in {\n" + "int i;\n" + "int j;\n" +"}} // namespace out::in", +format("namespace out {\n" + "namespace in {\n" + "int i;\n" + "int j;\n" + "}\n" + "}", + Style)); + + // Incorrect comments are fixed + EXPECT_EQ("namespace out { namespace in {\n" +"}} // namespace out::in", +format("namespace out { namespace in {\n" + "}} // namespace out", + Style)); + EXPECT_EQ("namespace out { namespace in {\n" +"}} // namespace out::in", +format("namespace out { namespace in {\n" + "}} // namespace in", + Style)); + + // Extra semicolon after 'inner' closing brace prevents merging + EXPECT_EQ("namespace out { namespace in {\n" +"}; } // namespace out::in", +format("namespace out {\n" + "namespace in {\n" + "}; // namespace in\n" + "} // namespace out", + Style)); + + // Extra semicolon after 'outer' closing brace is conserved + EXPECT_EQ("namespace out { namespace in {\n" +"}}; // namespace out::in", +format("namespace out {\n" + "namespace in {\n" + "} // namespace in\n" + "}; // namespace out", + Style)); + + Style.NamespaceIndentation = FormatStyle::NI_All; + EXPECT_EQ("namespace out { namespace in {\n" +" int i;\n" +"}} // namespace out::in", +format("namespace out {\n" + "namespace in {\n" +
[PATCH] D32480: clang-format: Add CompactNamespaces option
krasimir added a comment. Looks good, except for the tests that actually add or fix namespace end comments should be moved to NamespaceEndCommentsFixerTest.cpp. https://reviews.llvm.org/D32480 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32480: clang-format: Add CompactNamespaces option
krasimir added inline comments. Comment at: unittests/Format/FormatTest.cpp:1394 + "int j;\n" + "}\n" + "}", Re-indent this line. https://reviews.llvm.org/D32480 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33644: Add default values for function parameter chunks
klimek added a comment. Can you give a bit more background what this is trying to do? https://reviews.llvm.org/D33644 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34202: [clang-tidy] readability-function-size: fix nesting level calculation
lebedev.ri created this revision. lebedev.ri added a project: clang-tools-extra. A followup for https://reviews.llvm.org/D32942. Malcolm Parsons has provided a valid testcase that the initial version of the check complained about nested `if`'s. As it turns out, the culprit is the **partially** un-intentional `switch` fallthrough. So fix the unintentional part of the fallthrough, and add testcases with nested `if`' where there should be a warning and shouldn't be a warning. I guess, now it would be actually possible to pick some reasonable default for `NestingThreshold` setting. Repository: rL LLVM https://reviews.llvm.org/D34202 Files: clang-tidy/readability/FunctionSizeCheck.cpp test/clang-tidy/readability-function-size.cpp Index: test/clang-tidy/readability-function-size.cpp === --- test/clang-tidy/readability-function-size.cpp +++ test/clang-tidy/readability-function-size.cpp @@ -63,8 +63,9 @@ #define macro() {int x; {int y; {int z;}}} void baz0() { // 1 -// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'baz0' exceeds recommended size/complexity -// CHECK-MESSAGES: :[[@LINE-2]]:6: note: 9 statements (threshold 0) + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'baz0' exceeds recommended size/complexity + // CHECK-MESSAGES: :[[@LINE-2]]:6: note: 27 lines including whitespace and comments (threshold 0) + // CHECK-MESSAGES: :[[@LINE-3]]:6: note: 9 statements (threshold 0) int a; { // 2 int b; @@ -87,5 +88,51 @@ } macro() // CHECK-MESSAGES: :[[@LINE-1]]:3: note: nesting level 3 starts here (threshold 2) -// CHECK-MESSAGES: :[[@LINE-27]]:25: note: expanded from macro 'macro' +// CHECK-MESSAGES: :[[@LINE-28]]:25: note: expanded from macro 'macro' +} + +// check that nested if's are not reported. this was broken initially +void nesting_if() { // 1 + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'nesting_if' exceeds recommended size/complexity + // CHECK-MESSAGES: :[[@LINE-2]]:6: note: 18 lines including whitespace and comments (threshold 0) + // CHECK-MESSAGES: :[[@LINE-3]]:6: note: 15 statements (threshold 0) + // CHECK-MESSAGES: :[[@LINE-4]]:6: note: 5 branches (threshold 0) + if (true) { // 2 + int j; + } else if (true) { // 2 + int j; + if (true) { // 3 + // CHECK-MESSAGES: :[[@LINE-1]]:16: note: nesting level 3 starts here (threshold 2) + int j; + } + } else if (true) { // 2 + int j; + } else if (true) { // 2 + int j; + } +} + +// however this should warn +void bad_if_nesting() { // 1 +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'bad_if_nesting' exceeds recommended size/complexity +// CHECK-MESSAGES: :[[@LINE-2]]:6: note: 22 lines including whitespace and comments (threshold 0) +// CHECK-MESSAGES: :[[@LINE-3]]:6: note: 12 statements (threshold 0) +// CHECK-MESSAGES: :[[@LINE-4]]:6: note: 4 branches (threshold 0) + if (true) {// 2 +int j; + } else { // 2 +if (true) { // 3 + // CHECK-MESSAGES: :[[@LINE-1]]:15: note: nesting level 3 starts here (threshold 2) + int j; +} else { // 3 + // CHECK-MESSAGES: :[[@LINE-1]]:12: note: nesting level 3 starts here (threshold 2) + if (true) { // 4 +int j; + } else { // 4 +if (true) { // 5 + int j; +} + } +} + } } Index: clang-tidy/readability/FunctionSizeCheck.cpp === --- clang-tidy/readability/FunctionSizeCheck.cpp +++ clang-tidy/readability/FunctionSizeCheck.cpp @@ -36,15 +36,18 @@ case Stmt::ForStmtClass: case Stmt::SwitchStmtClass: ++Info.Branches; -// fallthrough + LLVM_FALLTHROUGH; case Stmt::CompoundStmtClass: - // If this new compound statement is located in a compound statement, - // which is already nested NestingThreshold levels deep, record the start - // location of this new compound statement - if (CurrentNestingLevel == Info.NestingThreshold) -Info.NestingThresholders.push_back(Node->getLocStart()); + if(Node->getStmtClass() == Stmt::CompoundStmtClass) { +// If this new compound statement is located in a compound statement, +// which is already nested NestingThreshold levels deep, record the start +// location of this new compound statement +if (CurrentNestingLevel == Info.NestingThreshold) + Info.NestingThresholders.push_back(Node->getLocStart()); + +++CurrentNestingLevel; + } - ++CurrentNestingLevel; TrackedParent.push_back(true); break; default: Index: test/clang-tidy/readability-function-size.cpp === --- test/clang-tidy/readability-function-size.cpp +++ test/clang-tidy/readability-function-size.cpp @@ -63,8 +63,9 @@ #define macro() {int x; {int y; {int z;}}} void baz0() { // 1 -// CHECK-MESSAGES: :[[@LINE-
[PATCH] D27753: [analyzer] alpha.security.DirtyScalar Checker
gerazo added a comment. Before abandoning this patch and rewriting it, I would like to get a thumbs up for my plans: I will reimplement all functionality included here but without creating a new checker. Some parts which relate to specific checkers will be put into the corresponding checkers (like ArrayBoundCheckerV2). General ideas on taintedness (cleansing rules and usage warnings on standard types) will be put into GenericTaintChecker. We will see how it goes, will we have a smaller patch or not. WDYT? https://reviews.llvm.org/D27753 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32480: clang-format: Add CompactNamespaces option
Typz updated this revision to Diff 102528. Typz marked an inline comment as done. Typz added a comment. Move tests that add or fix namespace end comments to NamespaceEndCommentsFixerTest.cpp https://reviews.llvm.org/D32480 Files: include/clang/Format/Format.h lib/Format/Format.cpp lib/Format/NamespaceEndCommentsFixer.cpp lib/Format/UnwrappedLineFormatter.cpp lib/Format/UnwrappedLineParser.cpp unittests/Format/FormatTest.cpp unittests/Format/NamespaceEndCommentsFixerTest.cpp Index: unittests/Format/NamespaceEndCommentsFixerTest.cpp === --- unittests/Format/NamespaceEndCommentsFixerTest.cpp +++ unittests/Format/NamespaceEndCommentsFixerTest.cpp @@ -185,6 +185,41 @@ "}\n" "}")); + // Add comment for namespaces which will be 'compacted' + FormatStyle CompactNamespacesStyle = getLLVMStyle(); + CompactNamespacesStyle.CompactNamespaces = true; + EXPECT_EQ("namespace out { namespace in {\n" +"int i;\n" +"int j;\n" +"}}// namespace out::in", +fixNamespaceEndComments("namespace out { namespace in {\n" +"int i;\n" +"int j;\n" +"}}", +CompactNamespacesStyle)); + EXPECT_EQ("namespace out {\n" +"namespace in {\n" +"int i;\n" +"int j;\n" +"}\n" +"}// namespace out::in", +fixNamespaceEndComments("namespace out {\n" +"namespace in {\n" +"int i;\n" +"int j;\n" +"}\n" +"}", +CompactNamespacesStyle)); + EXPECT_EQ("namespace out { namespace in {\n" +"int i;\n" +"int j;\n" +"};}// namespace out::in", +fixNamespaceEndComments("namespace out { namespace in {\n" +"int i;\n" +"int j;\n" +"};}", +CompactNamespacesStyle)); + // Adds an end comment after a semicolon. EXPECT_EQ("namespace {\n" " int i;\n" @@ -388,6 +423,27 @@ fixNamespaceEndComments("namespace A {} // namespace")); EXPECT_EQ("namespace A {}; // namespace A", fixNamespaceEndComments("namespace A {}; // namespace")); + + // Update invalid comments for compacted namespaces. + FormatStyle CompactNamespacesStyle = getLLVMStyle(); + CompactNamespacesStyle.CompactNamespaces = true; + EXPECT_EQ("namespace out { namespace in {\n" +"}} // namespace out::in", +fixNamespaceEndComments("namespace out { namespace in {\n" +"}} // namespace out", +CompactNamespacesStyle)); + EXPECT_EQ("namespace out { namespace in {\n" +"}} // namespace out::in", +fixNamespaceEndComments("namespace out { namespace in {\n" +"}} // namespace in", +CompactNamespacesStyle)); + EXPECT_EQ("namespace out { namespace in {\n" +"}\n" +"} // namespace out::in", +fixNamespaceEndComments("namespace out { namespace in {\n" +"}// banamespace in\n" +"} // namespace out", +CompactNamespacesStyle)); } TEST_F(NamespaceEndCommentsFixerTest, UpdatesInvalidEndBlockComment) { Index: unittests/Format/FormatTest.cpp === --- unittests/Format/FormatTest.cpp +++ unittests/Format/FormatTest.cpp @@ -1309,6 +1309,141 @@ Style)); } +TEST_F(FormatTest, FormatsCompactNamespaces) { + FormatStyle Style = getLLVMStyle(); + Style.CompactNamespaces = true; + + verifyFormat("namespace A { namespace B {\n" + "}} // namespace A::B", + Style); + + EXPECT_EQ("namespace out { namespace in {\n" +"}} // namespace out::in", +format("namespace out {\n" + "namespace in {\n" + "} // namespace in\n" + "} // namespace out", + Style)); + + // Only namespaces which have both consecutive opening and end get compacted + EXPECT_EQ("namespace out {\n" +"namespace in1 {\n" +"} // namespace in1\n" +"namespace in2 {\n" +"} // namespace in2\n" +"} // namespace out", +format("namespace out {\n" + "namespace in1 {\n" + "} // namespace in1\n" +
[PATCH] D34202: [clang-tidy] readability-function-size: fix nesting level calculation
lebedev.ri planned changes to this revision. lebedev.ri added a comment. Hmm, this is moving in the right direction, but now it invalidated (decreases the nesting level) too eagerly Repository: rL LLVM https://reviews.llvm.org/D34202 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r305384 - clang-format: Add CompactNamespaces option
Author: typz Date: Wed Jun 14 07:29:47 2017 New Revision: 305384 URL: http://llvm.org/viewvc/llvm-project?rev=305384&view=rev Log: clang-format: Add CompactNamespaces option Summary: Add CompactNamespaces option, to pack namespace declarations on the same line (somewhat similar to C++17 nested namespace definition). With this option, consecutive namespace declarations are kept on the same line: namespace foo { namespace bar { ... }} // namespace foo::bar Reviewers: krasimir, djasper, klimek Reviewed By: djasper Subscribers: kimgr, cfe-commits, klimek Tags: #clang-tools-extra Differential Revision: https://reviews.llvm.org/D32480 Modified: cfe/trunk/include/clang/Format/Format.h cfe/trunk/lib/Format/Format.cpp cfe/trunk/lib/Format/NamespaceEndCommentsFixer.cpp cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp cfe/trunk/lib/Format/UnwrappedLineParser.cpp cfe/trunk/unittests/Format/FormatTest.cpp cfe/trunk/unittests/Format/NamespaceEndCommentsFixerTest.cpp Modified: cfe/trunk/include/clang/Format/Format.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Format/Format.h?rev=305384&r1=305383&r2=305384&view=diff == --- cfe/trunk/include/clang/Format/Format.h (original) +++ cfe/trunk/include/clang/Format/Format.h Wed Jun 14 07:29:47 2017 @@ -791,6 +791,29 @@ struct FormatStyle { /// \endcode bool BreakBeforeInheritanceComma; + /// \brief If ``true``, consecutive namespace declarations will be on the same + /// line. If ``false``, each namespace is declared on a new line. + /// \code + /// true: + /// namespace Foo { namespace Bar { + /// }} + /// + /// false: + /// namespace Foo { + /// namespace Bar { + /// } + /// } + /// \endcode + /// + /// If it does not fit on a single line, the overflowing namespaces get + /// wrapped: + /// \code + /// namespace Foo { namespace Bar { + /// namespace Extra { + /// }}} + /// \endcode + bool CompactNamespaces; + /// \brief If the constructor initializers don't fit on a line, put each /// initializer on its own line. /// \code @@ -1422,6 +1445,7 @@ struct FormatStyle { BreakBeforeBraces == R.BreakBeforeBraces && BreakBeforeTernaryOperators == R.BreakBeforeTernaryOperators && BreakConstructorInitializers == R.BreakConstructorInitializers && + CompactNamespaces == R.CompactNamespaces && BreakAfterJavaFieldAnnotations == R.BreakAfterJavaFieldAnnotations && BreakStringLiterals == R.BreakStringLiterals && ColumnLimit == R.ColumnLimit && CommentPragmas == R.CommentPragmas && Modified: cfe/trunk/lib/Format/Format.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=305384&r1=305383&r2=305384&view=diff == --- cfe/trunk/lib/Format/Format.cpp (original) +++ cfe/trunk/lib/Format/Format.cpp Wed Jun 14 07:29:47 2017 @@ -310,6 +310,8 @@ template <> struct MappingTraits struct MappingTraitshttp://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/NamespaceEndCommentsFixer.cpp?rev=305384&r1=305383&r2=305384&view=diff == --- cfe/trunk/lib/Format/NamespaceEndCommentsFixer.cpp (original) +++ cfe/trunk/lib/Format/NamespaceEndCommentsFixer.cpp Wed Jun 14 07:29:47 2017 @@ -107,6 +107,24 @@ void updateEndComment(const FormatToken << llvm::toString(std::move(Err)) << "\n"; } } + +const FormatToken * +getNamespaceToken(const AnnotatedLine *line, + const SmallVectorImpl &AnnotatedLines) { + if (!line->Affected || line->InPPDirective || !line->startsWith(tok::r_brace)) +return nullptr; + size_t StartLineIndex = line->MatchingOpeningBlockLineIndex; + if (StartLineIndex == UnwrappedLine::kInvalidIndex) +return nullptr; + assert(StartLineIndex < AnnotatedLines.size()); + const FormatToken *NamespaceTok = AnnotatedLines[StartLineIndex]->First; + // Detect "(inline)? namespace" in the beginning of a line. + if (NamespaceTok->is(tok::kw_inline)) +NamespaceTok = NamespaceTok->getNextNonComment(); + if (!NamespaceTok || NamespaceTok->isNot(tok::kw_namespace)) +return nullptr; + return NamespaceTok; +} } // namespace NamespaceEndCommentsFixer::NamespaceEndCommentsFixer(const Environment &Env, @@ -120,20 +138,14 @@ tooling::Replacements NamespaceEndCommen AffectedRangeMgr.computeAffectedLines(AnnotatedLines.begin(), AnnotatedLines.end()); tooling::Replacements Fixes; + std::string AllNamespaceNames = ""; + size_t StartLineIndex = SIZE_MAX; + unsigned int CompactedNamespacesCount = 0; for (size_t I = 0, E = AnnotatedLines.size(); I != E; ++I) { -if (!AnnotatedLines[I]->Affected || AnnotatedLines[I]->InPPDirective || -!Annot
[PATCH] D32480: clang-format: Add CompactNamespaces option
This revision was automatically updated to reflect the committed changes. Closed by commit rL305384: clang-format: Add CompactNamespaces option (authored by Typz). Changed prior to commit: https://reviews.llvm.org/D32480?vs=102528&id=102531#toc Repository: rL LLVM https://reviews.llvm.org/D32480 Files: cfe/trunk/include/clang/Format/Format.h cfe/trunk/lib/Format/Format.cpp cfe/trunk/lib/Format/NamespaceEndCommentsFixer.cpp cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp cfe/trunk/lib/Format/UnwrappedLineParser.cpp cfe/trunk/unittests/Format/FormatTest.cpp cfe/trunk/unittests/Format/NamespaceEndCommentsFixerTest.cpp Index: cfe/trunk/unittests/Format/FormatTest.cpp === --- cfe/trunk/unittests/Format/FormatTest.cpp +++ cfe/trunk/unittests/Format/FormatTest.cpp @@ -1310,6 +1310,141 @@ Style)); } +TEST_F(FormatTest, FormatsCompactNamespaces) { + FormatStyle Style = getLLVMStyle(); + Style.CompactNamespaces = true; + + verifyFormat("namespace A { namespace B {\n" + "}} // namespace A::B", + Style); + + EXPECT_EQ("namespace out { namespace in {\n" +"}} // namespace out::in", +format("namespace out {\n" + "namespace in {\n" + "} // namespace in\n" + "} // namespace out", + Style)); + + // Only namespaces which have both consecutive opening and end get compacted + EXPECT_EQ("namespace out {\n" +"namespace in1 {\n" +"} // namespace in1\n" +"namespace in2 {\n" +"} // namespace in2\n" +"} // namespace out", +format("namespace out {\n" + "namespace in1 {\n" + "} // namespace in1\n" + "namespace in2 {\n" + "} // namespace in2\n" + "} // namespace out", + Style)); + + EXPECT_EQ("namespace out {\n" +"int i;\n" +"namespace in {\n" +"int j;\n" +"} // namespace in\n" +"int k;\n" +"} // namespace out", +format("namespace out { int i;\n" + "namespace in { int j; } // namespace in\n" + "int k; } // namespace out", + Style)); + + EXPECT_EQ("namespace A { namespace B { namespace C {\n" +"}}} // namespace A::B::C\n", +format("namespace A { namespace B {\n" + "namespace C {\n" + "}} // namespace B::C\n" + "} // namespace A\n", + Style)); + + Style.ColumnLimit = 40; + EXPECT_EQ("namespace aa {\n" +"namespace bb {\n" +"}} // namespace aa::bb", +format("namespace aa {\n" + "namespace bb {\n" + "} // namespace bb\n" + "} // namespace aa", + Style)); + + EXPECT_EQ("namespace aa { namespace bb {\n" +"namespace cc {\n" +"}}} // namespace aa::bb::cc", +format("namespace aa {\n" + "namespace bb {\n" + "namespace cc {\n" + "} // namespace cc\n" + "} // namespace bb\n" + "} // namespace aa", + Style)); + Style.ColumnLimit = 80; + + // Extra semicolon after 'inner' closing brace prevents merging + EXPECT_EQ("namespace out { namespace in {\n" +"}; } // namespace out::in", +format("namespace out {\n" + "namespace in {\n" + "}; // namespace in\n" + "} // namespace out", + Style)); + + // Extra semicolon after 'outer' closing brace is conserved + EXPECT_EQ("namespace out { namespace in {\n" +"}}; // namespace out::in", +format("namespace out {\n" + "namespace in {\n" + "} // namespace in\n" + "}; // namespace out", + Style)); + + Style.NamespaceIndentation = FormatStyle::NI_All; + EXPECT_EQ("namespace out { namespace in {\n" +" int i;\n" +"}} // namespace out::in", +format("namespace out {\n" + "namespace in {\n" + "int i;\n" + "} // namespace in\n" + "} // namespace out", + Style)); + EXPECT_EQ("namespace out { namespace mid {\n" +" namespace in {\n" +"int j;\n" +" } // namespace in\n" +" int k;\n" +"}} // namespace out::mid", +format("namespace out { namespace mid {\n" + "namespace in { int j; } // namespace in\n" + "int k; }
r305385 - Corrected some comment typos; NFC.
Author: aaronballman Date: Wed Jun 14 07:48:18 2017 New Revision: 305385 URL: http://llvm.org/viewvc/llvm-project?rev=305385&view=rev Log: Corrected some comment typos; NFC. Modified: cfe/trunk/include/clang/Frontend/FrontendOptions.h Modified: cfe/trunk/include/clang/Frontend/FrontendOptions.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/FrontendOptions.h?rev=305385&r1=305384&r2=305385&view=diff == --- cfe/trunk/include/clang/Frontend/FrontendOptions.h (original) +++ cfe/trunk/include/clang/Frontend/FrontendOptions.h Wed Jun 14 07:48:18 2017 @@ -317,8 +317,8 @@ public: /// \brief Auxiliary triple for CUDA compilation. std::string AuxTriple; - /// \brief If non-empty, search the pch input file as it was a header - // included by this file. + /// \brief If non-empty, search the pch input file as if it was a header + /// included by this file. std::string FindPchSource; /// Filename to write statistics to. ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34201: [clangd] Move dependencies of ClangdLSPServer out of its implementation
ilya-biryukov added inline comments. Comment at: clangd/ClangdLSPServer.h:26 /// dispatch and ClangdServer together. -class ClangdLSPServer { +class ClangdLSPServer : public DiagnosticsConsumer { public: I would not expect ClangdLSPServer to be passed as a DiagnosticsConsumer. It's not the purpose of this class at all, therefore I don't think it's a good idea to inherit publicly from DiagnosticsConsumer. Private inheritance would be fine (to reduce code repetition), but is generally considered to be hard-to-grasp. Comment at: clangd/ClangdLSPServer.h:29 + ClangdLSPServer(JSONOutput &Out, + GlobalCompilationDatabase &CDB, + FileSystemProvider &FSProvider, What are the use-cases for getting GlobalCompilationDatabase and FileSystemProvider as an outside parameter? It actually seems to me they are implementation details of ClangdLSPServer. I.e. the purpose of this class is to provide an easy interface to just run LSP server without worrying about its configuration. https://reviews.llvm.org/D34201 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33589: clang-format: consider not splitting tokens in optimization
Typz added a comment. @krasimir : ping https://reviews.llvm.org/D33589 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33644: Add default values for function parameter chunks
yvvan added a comment. In https://reviews.llvm.org/D33644#780188, @klimek wrote: > Can you give a bit more background what this is trying to do? Sure :) Currently default value string does not contain default value itself. This change fixes it and adds the default value to the string. https://reviews.llvm.org/D33644 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32146: PR32476: __nop_locale_mgmt.h not needed with newlib 2.5+
bcraig accepted this revision. bcraig added a comment. LGTM. @waltl : Do you need me to submit the changes, or will you handle that? https://reviews.llvm.org/D32146 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32942: [clang-tidy] readability-function-size: add NestingThreshold param.
malcolm.parsons added a comment. In https://reviews.llvm.org/D32942#780090, @lebedev.ri wrote: > In https://reviews.llvm.org/D32942#780053, @malcolm.parsons wrote: > > > My prototype of this feature used `ifStmt(stmt().bind("if"), > > unless(hasParent(ifStmt(hasElse(equalsBoundNode("if"))`. > > > The fix should ideally be fully contained in the `TraverseStmt()`, and should > not be noticeably slower. Yes, my prototype wasn't part of readability-function-size. Repository: rL LLVM https://reviews.llvm.org/D32942 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33644: Add default values for function parameter chunks
klimek added a comment. Ok, now I get it - can you please add tests? This is usually tested by adding a c-index-test based test. Comment at: lib/Sema/SemaCodeComplete.cpp:2411-2417 + const SourceLocation StartLoc = SrcRange.getBegin(); + const SourceLocation EndLoc = SrcRange.getEnd(); + if (StartLoc != SM.getExpansionLoc(StartLoc) || EndLoc != SM.getExpansionLoc(EndLoc)) + return ""; + const size_t PtrDiff = EndLoc.getRawEncoding() - StartLoc.getRawEncoding() + + Lexer::MeasureTokenLength(EndLoc, SM, LangOpts); + return std::string{SM.getCharacterData(StartLoc), PtrDiff}; Can you use Lexer::getSourceText instead? Comment at: lib/Sema/SemaCodeComplete.cpp:2453 std::string PlaceholderStr = FormatFunctionParameter(Policy, Param); +if (Param->hasDefaultArg() && PlaceholderStr.find("=") == std::string::npos) { +std::string DefaultValue = Why the check for = in the PlaceholderStr? https://reviews.llvm.org/D33644 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34202: [clang-tidy] readability-function-size: fix nesting level calculation
lebedev.ri updated this revision to Diff 102533. lebedev.ri edited the summary of this revision. lebedev.ri added a comment. Ok, i really messed up the initial https://reviews.llvm.org/D32942 :) Malcolm, thank you for bothering to report :) This should be so much better. Repository: rL LLVM https://reviews.llvm.org/D34202 Files: clang-tidy/readability/FunctionSizeCheck.cpp test/clang-tidy/readability-function-size.cpp Index: test/clang-tidy/readability-function-size.cpp === --- test/clang-tidy/readability-function-size.cpp +++ test/clang-tidy/readability-function-size.cpp @@ -63,8 +63,9 @@ #define macro() {int x; {int y; {int z;}}} void baz0() { // 1 -// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'baz0' exceeds recommended size/complexity -// CHECK-MESSAGES: :[[@LINE-2]]:6: note: 9 statements (threshold 0) + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'baz0' exceeds recommended size/complexity + // CHECK-MESSAGES: :[[@LINE-2]]:6: note: 27 lines including whitespace and comments (threshold 0) + // CHECK-MESSAGES: :[[@LINE-3]]:6: note: 9 statements (threshold 0) int a; { // 2 int b; @@ -87,5 +88,55 @@ } macro() // CHECK-MESSAGES: :[[@LINE-1]]:3: note: nesting level 3 starts here (threshold 2) -// CHECK-MESSAGES: :[[@LINE-27]]:25: note: expanded from macro 'macro' +// CHECK-MESSAGES: :[[@LINE-28]]:25: note: expanded from macro 'macro' +} + +// check that nested if's are not reported. this was broken initially +void nesting_if() { // 1 + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'nesting_if' exceeds recommended size/complexity + // CHECK-MESSAGES: :[[@LINE-2]]:6: note: 22 lines including whitespace and comments (threshold 0) + // CHECK-MESSAGES: :[[@LINE-3]]:6: note: 18 statements (threshold 0) + // CHECK-MESSAGES: :[[@LINE-4]]:6: note: 6 branches (threshold 0) + if (true) { // 2 + int j; + } else if (true) { // 2 + int j; + if (true) { // 3 + // CHECK-MESSAGES: :[[@LINE-1]]:16: note: nesting level 3 starts here (threshold 2) + int j; + } + } else if (true) { // 2 + int j; + if (true) { // 3 + // CHECK-MESSAGES: :[[@LINE-1]]:16: note: nesting level 3 starts here (threshold 2) + int j; + } + } else if (true) { // 2 + int j; + } +} + +// however this should warn +void bad_if_nesting() { // 1 +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'bad_if_nesting' exceeds recommended size/complexity +// CHECK-MESSAGES: :[[@LINE-2]]:6: note: 22 lines including whitespace and comments (threshold 0) +// CHECK-MESSAGES: :[[@LINE-3]]:6: note: 12 statements (threshold 0) +// CHECK-MESSAGES: :[[@LINE-4]]:6: note: 4 branches (threshold 0) + if (true) {// 2 +int j; + } else { // 2 +if (true) { // 3 + // CHECK-MESSAGES: :[[@LINE-1]]:15: note: nesting level 3 starts here (threshold 2) + int j; +} else { // 3 + // CHECK-MESSAGES: :[[@LINE-1]]:12: note: nesting level 3 starts here (threshold 2) + if (true) { // 4 +int j; + } else { // 4 +if (true) { // 5 + int j; +} + } +} + } } Index: clang-tidy/readability/FunctionSizeCheck.cpp === --- clang-tidy/readability/FunctionSizeCheck.cpp +++ clang-tidy/readability/FunctionSizeCheck.cpp @@ -25,8 +25,20 @@ if (!Node) return Base::TraverseStmt(Node); -if (TrackedParent.back() && !isa(Node)) - ++Info.Statements; +if (isa(Node)) { + // If this new compound statement is located in a compound statement, + // which is already nested NestingThreshold levels deep, record the + // start location of this new compound statement + if (CurrentNestingLevel == Info.NestingThreshold) +Info.NestingThresholders.push_back(Node->getLocStart()); + + // We have just entered CompoundStmt, increase current nesting level. + ++CurrentNestingLevel; +} else { + assert(!isa(Node)); + if (TrackedParent.back()) +++Info.Statements; +} switch (Node->getStmtClass()) { case Stmt::IfStmtClass: @@ -36,15 +48,8 @@ case Stmt::ForStmtClass: case Stmt::SwitchStmtClass: ++Info.Branches; -// fallthrough + LLVM_FALLTHROUGH; case Stmt::CompoundStmtClass: - // If this new compound statement is located in a compound statement, - // which is already nested NestingThreshold levels deep, record the start - // location of this new compound statement - if (CurrentNestingLevel == Info.NestingThreshold) -Info.NestingThresholders.push_back(Node->getLocStart()); - - ++CurrentNestingLevel; TrackedParent.push_back(true); break; default: @@ -54,8 +59,10 @@ Base::TraverseStmt(Node); -if (TrackedParent.back()) +// did we just process CompoundStmt? if yes, decrease current nesting level. +if (isa(Node))
[PATCH] D34201: [clangd] Move dependencies of ClangdLSPServer out of its implementation
krasimir abandoned this revision. krasimir added a comment. On a second thought, meh :D https://reviews.llvm.org/D34201 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34202: [clang-tidy] readability-function-size: fix nesting level calculation
malcolm.parsons added inline comments. Comment at: clang-tidy/readability/FunctionSizeCheck.cpp:38 +} else { + assert(!isa(Node)); + if (TrackedParent.back()) I don't think this assert adds anything. Comment at: clang-tidy/readability/FunctionSizeCheck.cpp:62 -if (TrackedParent.back()) +// did we just process CompoundStmt? if yes, decrease current nesting level. +if (isa(Node)) Comments are sentences, so start with a capital letter. Repository: rL LLVM https://reviews.llvm.org/D34202 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34202: [clang-tidy] readability-function-size: fix nesting level calculation
lebedev.ri added inline comments. Comment at: clang-tidy/readability/FunctionSizeCheck.cpp:38 +} else { + assert(!isa(Node)); + if (TrackedParent.back()) malcolm.parsons wrote: > I don't think this assert adds anything. Yes, however the original `if` did have that check. I **think**, having an assert here may help to not break this in the future. Repository: rL LLVM https://reviews.llvm.org/D34202 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34202: [clang-tidy] readability-function-size: fix nesting level calculation
lebedev.ri updated this revision to Diff 102542. lebedev.ri marked an inline comment as done. lebedev.ri added a comment. Fix spelling. Repository: rL LLVM https://reviews.llvm.org/D34202 Files: clang-tidy/readability/FunctionSizeCheck.cpp test/clang-tidy/readability-function-size.cpp Index: test/clang-tidy/readability-function-size.cpp === --- test/clang-tidy/readability-function-size.cpp +++ test/clang-tidy/readability-function-size.cpp @@ -63,8 +63,9 @@ #define macro() {int x; {int y; {int z;}}} void baz0() { // 1 -// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'baz0' exceeds recommended size/complexity -// CHECK-MESSAGES: :[[@LINE-2]]:6: note: 9 statements (threshold 0) + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'baz0' exceeds recommended size/complexity + // CHECK-MESSAGES: :[[@LINE-2]]:6: note: 27 lines including whitespace and comments (threshold 0) + // CHECK-MESSAGES: :[[@LINE-3]]:6: note: 9 statements (threshold 0) int a; { // 2 int b; @@ -87,5 +88,55 @@ } macro() // CHECK-MESSAGES: :[[@LINE-1]]:3: note: nesting level 3 starts here (threshold 2) -// CHECK-MESSAGES: :[[@LINE-27]]:25: note: expanded from macro 'macro' +// CHECK-MESSAGES: :[[@LINE-28]]:25: note: expanded from macro 'macro' +} + +// check that nested if's are not reported. this was broken initially +void nesting_if() { // 1 + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'nesting_if' exceeds recommended size/complexity + // CHECK-MESSAGES: :[[@LINE-2]]:6: note: 22 lines including whitespace and comments (threshold 0) + // CHECK-MESSAGES: :[[@LINE-3]]:6: note: 18 statements (threshold 0) + // CHECK-MESSAGES: :[[@LINE-4]]:6: note: 6 branches (threshold 0) + if (true) { // 2 + int j; + } else if (true) { // 2 + int j; + if (true) { // 3 + // CHECK-MESSAGES: :[[@LINE-1]]:16: note: nesting level 3 starts here (threshold 2) + int j; + } + } else if (true) { // 2 + int j; + if (true) { // 3 + // CHECK-MESSAGES: :[[@LINE-1]]:16: note: nesting level 3 starts here (threshold 2) + int j; + } + } else if (true) { // 2 + int j; + } +} + +// however this should warn +void bad_if_nesting() { // 1 +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'bad_if_nesting' exceeds recommended size/complexity +// CHECK-MESSAGES: :[[@LINE-2]]:6: note: 22 lines including whitespace and comments (threshold 0) +// CHECK-MESSAGES: :[[@LINE-3]]:6: note: 12 statements (threshold 0) +// CHECK-MESSAGES: :[[@LINE-4]]:6: note: 4 branches (threshold 0) + if (true) {// 2 +int j; + } else { // 2 +if (true) { // 3 + // CHECK-MESSAGES: :[[@LINE-1]]:15: note: nesting level 3 starts here (threshold 2) + int j; +} else { // 3 + // CHECK-MESSAGES: :[[@LINE-1]]:12: note: nesting level 3 starts here (threshold 2) + if (true) { // 4 +int j; + } else { // 4 +if (true) { // 5 + int j; +} + } +} + } } Index: clang-tidy/readability/FunctionSizeCheck.cpp === --- clang-tidy/readability/FunctionSizeCheck.cpp +++ clang-tidy/readability/FunctionSizeCheck.cpp @@ -25,8 +25,20 @@ if (!Node) return Base::TraverseStmt(Node); -if (TrackedParent.back() && !isa(Node)) - ++Info.Statements; +if (isa(Node)) { + // If this new compound statement is located in a compound statement, + // which is already nested NestingThreshold levels deep, record the + // start location of this new compound statement + if (CurrentNestingLevel == Info.NestingThreshold) +Info.NestingThresholders.push_back(Node->getLocStart()); + + // We have just entered CompoundStmt, increase current nesting level. + ++CurrentNestingLevel; +} else { + assert(!isa(Node)); + if (TrackedParent.back()) +++Info.Statements; +} switch (Node->getStmtClass()) { case Stmt::IfStmtClass: @@ -36,15 +48,8 @@ case Stmt::ForStmtClass: case Stmt::SwitchStmtClass: ++Info.Branches; -// fallthrough + LLVM_FALLTHROUGH; case Stmt::CompoundStmtClass: - // If this new compound statement is located in a compound statement, - // which is already nested NestingThreshold levels deep, record the start - // location of this new compound statement - if (CurrentNestingLevel == Info.NestingThreshold) -Info.NestingThresholders.push_back(Node->getLocStart()); - - ++CurrentNestingLevel; TrackedParent.push_back(true); break; default: @@ -54,8 +59,10 @@ Base::TraverseStmt(Node); -if (TrackedParent.back()) +// Did we just process CompoundStmt? if yes, decrease current nesting level. +if (isa(Node)) --CurrentNestingLevel; + TrackedParent.pop_back(); return true; ___ cfe-com
[PATCH] D33719: Add _Float16 as a C/C++ source language type
SjoerdMeijer updated this revision to Diff 102543. SjoerdMeijer added a comment. Float16 is added as a native type. Implementing it as some sort of alias to fp16 caused too many type issues in expression/literals/etc., and thus was not an easier first step, and also in the end we want it to be a native type anyway. The tests have been restructured, and now also x86 is tested. It turned out I needed a helper function isFloat16Ty, for which I created llvm patch https://reviews.llvm.org/D34205. https://reviews.llvm.org/D33719 Files: include/clang-c/Index.h include/clang/AST/ASTContext.h include/clang/AST/BuiltinTypes.def include/clang/AST/Type.h include/clang/Basic/Specifiers.h include/clang/Basic/TokenKinds.def include/clang/Lex/LiteralSupport.h include/clang/Sema/DeclSpec.h include/clang/Serialization/ASTBitCodes.h lib/AST/ASTContext.cpp lib/AST/ItaniumMangle.cpp lib/AST/MicrosoftMangle.cpp lib/AST/NSAPI.cpp lib/AST/StmtPrinter.cpp lib/AST/Type.cpp lib/AST/TypeLoc.cpp lib/Analysis/PrintfFormatString.cpp lib/CodeGen/CGDebugInfo.cpp lib/CodeGen/CGExprScalar.cpp lib/CodeGen/CodeGenTypes.cpp lib/CodeGen/ItaniumCXXABI.cpp lib/Format/FormatToken.cpp lib/Index/USRGeneration.cpp lib/Lex/LiteralSupport.cpp lib/Parse/ParseDecl.cpp lib/Parse/ParseExpr.cpp lib/Parse/ParseExprCXX.cpp lib/Parse/ParseTentative.cpp lib/Sema/DeclSpec.cpp lib/Sema/SemaDecl.cpp lib/Sema/SemaExpr.cpp lib/Sema/SemaTemplateVariadic.cpp lib/Sema/SemaType.cpp lib/Serialization/ASTCommon.cpp lib/Serialization/ASTReader.cpp test/CodeGenCXX/float16-declarations.cpp test/Lexer/half-literal.cpp tools/libclang/CXType.cpp Index: tools/libclang/CXType.cpp === --- tools/libclang/CXType.cpp +++ tools/libclang/CXType.cpp @@ -52,6 +52,7 @@ BTCASE(Float); BTCASE(Double); BTCASE(LongDouble); +BTCASE(Float16); BTCASE(Float128); BTCASE(NullPtr); BTCASE(Overload); @@ -490,7 +491,7 @@ TKIND(Char_U); TKIND(UChar); TKIND(Char16); -TKIND(Char32); +TKIND(Char32); TKIND(UShort); TKIND(UInt); TKIND(ULong); @@ -508,6 +509,7 @@ TKIND(Float); TKIND(Double); TKIND(LongDouble); +TKIND(Float16); TKIND(Float128); TKIND(NullPtr); TKIND(Overload); Index: test/Lexer/half-literal.cpp === --- test/Lexer/half-literal.cpp +++ test/Lexer/half-literal.cpp @@ -1,3 +1,6 @@ // RUN: %clang_cc1 -fsyntax-only -verify -pedantic %s float a = 1.0h; // expected-error{{invalid suffix 'h' on floating constant}} float b = 1.0H; // expected-error{{invalid suffix 'H' on floating constant}} + +_Float16 c = 1.f166; // expected-error{{invalid suffix 'f166' on floating constant}} +_Float16 d = 1.f1; // expected-error{{invalid suffix 'f1' on floating constant}} Index: test/CodeGenCXX/float16-declarations.cpp === --- /dev/null +++ test/CodeGenCXX/float16-declarations.cpp @@ -0,0 +1,132 @@ +// RUN: %clang -std=c++11 --target=aarch64-arm--eabi -S -emit-llvm %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-AARCH64 +// RUN: %clang -std=c++11 --target=x86_64 -S -emit-llvm %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-X86 + +/* Various contexts where type _Float16 can appear. */ + + +/* Namespace */ + +namespace { + _Float16 f1n; +// CHECK-DAG: @_ZN12_GLOBAL__N_13f1nE = internal global half 0xH, align 2 + + _Float16 f2n = 33.f16; +// CHECK-AARCH64-DAG: @_ZN12_GLOBAL__N_13f2nE = internal global half 0xH5020, align 2 +// CHECK-X86-DAG: @_ZN12_GLOBAL__N_13f2nE = internal global i16 20512, align 2 + + _Float16 arr1n[10]; +// CHECK-AARCH64-DAG: @_ZN12_GLOBAL__N_15arr1nE = internal global [10 x half] zeroinitializer, align 2 +// CHECK-X86-DAG: @_ZN12_GLOBAL__N_15arr1nE = internal global [10 x half] zeroinitializer, align 16 + + _Float16 arr2n[] = { 1.2, 3.0, 3.e4 }; +// CHECK-AARCH64-DAG: @_ZN12_GLOBAL__N_15arr2nE = internal global [3 x half] [half 0xH3CCD, half 0xH4200, half 0xH7753], align 2 +// CHECK-X86-DAG: @_ZN12_GLOBAL__N_15arr2nE = internal global [3 x i16] [i16 15565, i16 16896, i16 30547], align 2 + + const volatile _Float16 func1n(const _Float16 &arg) { +return arg + f2n + arr1n[4] - arr2n[1]; + } +} + + +/* File */ + +_Float16 f1f; +// CHECK-AARCH64-DAG: @f1f = global half 0xH, align 2 +// CHECK-X86-DAG: @f1f = global half 0xH, align 2 + +_Float16 f2f = 32.4; +// CHECK-AARCH64-DAG: @f2f = global half 0xH500D, align 2 +// CHECK-X86-DAG: @f2f = global i16 20493, align 2 + +_Float16 arr1f[10]; +// CHECK-AARCH64-DAG: @arr1f = global [10 x half] zeroinitializer, align 2 +// CHECK-X86-DAG: @arr1f = global [10 x half] zeroinitializer, align 16 + +_Float16 arr2f[] = { -1.2, -3.0, -3.e4 }; +// CHECK-AARCH64-DAG: @arr2f = global [3 x half] [half 0xHBCCD, half 0x
[PATCH] D33644: Add default values for function parameter chunks
yvvan added inline comments. Comment at: lib/Sema/SemaCodeComplete.cpp:2411-2417 + const SourceLocation StartLoc = SrcRange.getBegin(); + const SourceLocation EndLoc = SrcRange.getEnd(); + if (StartLoc != SM.getExpansionLoc(StartLoc) || EndLoc != SM.getExpansionLoc(EndLoc)) + return ""; + const size_t PtrDiff = EndLoc.getRawEncoding() - StartLoc.getRawEncoding() + + Lexer::MeasureTokenLength(EndLoc, SM, LangOpts); + return std::string{SM.getCharacterData(StartLoc), PtrDiff}; klimek wrote: > Can you use Lexer::getSourceText instead? If that does the same i will use it in the next diff update Comment at: lib/Sema/SemaCodeComplete.cpp:2453 std::string PlaceholderStr = FormatFunctionParameter(Policy, Param); +if (Param->hasDefaultArg() && PlaceholderStr.find("=") == std::string::npos) { +std::string DefaultValue = klimek wrote: > Why the check for = in the PlaceholderStr? Not to add default value twice. If there's already "=" in placeholder string that means we've already added it in FormatFunctionParameter call https://reviews.llvm.org/D33644 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34202: [clang-tidy] readability-function-size: fix nesting level calculation
lebedev.ri updated this revision to Diff 102548. lebedev.ri added a comment. Simplify it even further by moving the logic into it's proper place - `TraverseCompoundStmt()` Repository: rL LLVM https://reviews.llvm.org/D34202 Files: clang-tidy/readability/FunctionSizeCheck.cpp test/clang-tidy/readability-function-size.cpp Index: test/clang-tidy/readability-function-size.cpp === --- test/clang-tidy/readability-function-size.cpp +++ test/clang-tidy/readability-function-size.cpp @@ -63,8 +63,9 @@ #define macro() {int x; {int y; {int z;}}} void baz0() { // 1 -// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'baz0' exceeds recommended size/complexity -// CHECK-MESSAGES: :[[@LINE-2]]:6: note: 9 statements (threshold 0) + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'baz0' exceeds recommended size/complexity + // CHECK-MESSAGES: :[[@LINE-2]]:6: note: 27 lines including whitespace and comments (threshold 0) + // CHECK-MESSAGES: :[[@LINE-3]]:6: note: 9 statements (threshold 0) int a; { // 2 int b; @@ -87,5 +88,55 @@ } macro() // CHECK-MESSAGES: :[[@LINE-1]]:3: note: nesting level 3 starts here (threshold 2) -// CHECK-MESSAGES: :[[@LINE-27]]:25: note: expanded from macro 'macro' +// CHECK-MESSAGES: :[[@LINE-28]]:25: note: expanded from macro 'macro' +} + +// check that nested if's are not reported. this was broken initially +void nesting_if() { // 1 + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'nesting_if' exceeds recommended size/complexity + // CHECK-MESSAGES: :[[@LINE-2]]:6: note: 22 lines including whitespace and comments (threshold 0) + // CHECK-MESSAGES: :[[@LINE-3]]:6: note: 18 statements (threshold 0) + // CHECK-MESSAGES: :[[@LINE-4]]:6: note: 6 branches (threshold 0) + if (true) { // 2 + int j; + } else if (true) { // 2 + int j; + if (true) { // 3 + // CHECK-MESSAGES: :[[@LINE-1]]:16: note: nesting level 3 starts here (threshold 2) + int j; + } + } else if (true) { // 2 + int j; + if (true) { // 3 + // CHECK-MESSAGES: :[[@LINE-1]]:16: note: nesting level 3 starts here (threshold 2) + int j; + } + } else if (true) { // 2 + int j; + } +} + +// however this should warn +void bad_if_nesting() { // 1 +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'bad_if_nesting' exceeds recommended size/complexity +// CHECK-MESSAGES: :[[@LINE-2]]:6: note: 22 lines including whitespace and comments (threshold 0) +// CHECK-MESSAGES: :[[@LINE-3]]:6: note: 12 statements (threshold 0) +// CHECK-MESSAGES: :[[@LINE-4]]:6: note: 4 branches (threshold 0) + if (true) {// 2 +int j; + } else { // 2 +if (true) { // 3 + // CHECK-MESSAGES: :[[@LINE-1]]:15: note: nesting level 3 starts here (threshold 2) + int j; +} else { // 3 + // CHECK-MESSAGES: :[[@LINE-1]]:12: note: nesting level 3 starts here (threshold 2) + if (true) { // 4 +int j; + } else { // 4 +if (true) { // 5 + int j; +} + } +} + } } Index: clang-tidy/readability/FunctionSizeCheck.cpp === --- clang-tidy/readability/FunctionSizeCheck.cpp +++ clang-tidy/readability/FunctionSizeCheck.cpp @@ -36,15 +36,8 @@ case Stmt::ForStmtClass: case Stmt::SwitchStmtClass: ++Info.Branches; -// fallthrough + LLVM_FALLTHROUGH; case Stmt::CompoundStmtClass: - // If this new compound statement is located in a compound statement, - // which is already nested NestingThreshold levels deep, record the start - // location of this new compound statement - if (CurrentNestingLevel == Info.NestingThreshold) -Info.NestingThresholders.push_back(Node->getLocStart()); - - ++CurrentNestingLevel; TrackedParent.push_back(true); break; default: @@ -54,13 +47,25 @@ Base::TraverseStmt(Node); -if (TrackedParent.back()) - --CurrentNestingLevel; TrackedParent.pop_back(); return true; } + bool TraverseCompoundStmt(CompoundStmt *Node) { +// If this new compound statement is located in a compound statement, which +// is already nested NestingThreshold levels deep, record the start location +// of this new compound statement +if (CurrentNestingLevel == Info.NestingThreshold) + Info.NestingThresholders.push_back(Node->getLocStart()); + +++CurrentNestingLevel; +Base::TraverseCompoundStmt(Node); +--CurrentNestingLevel; + +return true; + } + bool TraverseDecl(Decl *Node) { TrackedParent.push_back(false); Base::TraverseDecl(Node); ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34055: Be more strict when checking the -flto option value
yamaguchi updated this revision to Diff 102551. yamaguchi added a comment. Herald added a subscriber: eraman. Update patch. Add testcase and check if argument is valid or not. https://reviews.llvm.org/D34055 Files: clang/lib/Frontend/CompilerInvocation.cpp clang/test/CodeGen/thinlto-backend-option.ll Index: clang/test/CodeGen/thinlto-backend-option.ll === --- clang/test/CodeGen/thinlto-backend-option.ll +++ clang/test/CodeGen/thinlto-backend-option.ll @@ -8,6 +8,8 @@ ; RUN: %clang -flto=thin -c -o %t.o %s ; RUN: llvm-lto -thinlto -o %t %t.o -; RUN: not %clang_cc1 -x ir %t.o -fthinlto-index=%t.thinlto.bc -backend-option -nonexistent -emit-obj -o /dev/null 2>&1 | FileCheck %s +; RUN: not %clang_cc1 -x ir %t.o -fthinlto-index=%t.thinlto.bc -backend-option -nonexistent -emit-obj -o /dev/null 2>&1 | FileCheck %s -check-prefix=UNKNOWN +; UNKNOWN: clang: Unknown command line argument '-nonexistent' -; CHECK: clang: Unknown command line argument '-nonexistent' +; RUN: not %clang_cc1 -flto=thinfoo 2>&1 | FileCheck %s -check-prefix=INVALID +; INVALID: error: invalid value 'thinfoo' in '-flto=thinfoo' Index: clang/lib/Frontend/CompilerInvocation.cpp === --- clang/lib/Frontend/CompilerInvocation.cpp +++ clang/lib/Frontend/CompilerInvocation.cpp @@ -649,8 +649,14 @@ Opts.NoUseJumpTables = Args.hasArg(OPT_fno_jump_tables); Opts.PrepareForLTO = Args.hasArg(OPT_flto, OPT_flto_EQ); - const Arg *A = Args.getLastArg(OPT_flto, OPT_flto_EQ); - Opts.EmitSummaryIndex = A && A->containsValue("thin"); + Opts.EmitSummaryIndex = false; + if (Arg *A = Args.getLastArg(OPT_flto_EQ)) { +StringRef S = A->getValue(); +if (S == "thin") + Opts.EmitSummaryIndex = true; +else if (S != "full") + Diags.Report(diag::err_drv_invalid_value) << A->getAsString(Args) << S; + } Opts.LTOUnit = Args.hasFlag(OPT_flto_unit, OPT_fno_lto_unit, false); if (Arg *A = Args.getLastArg(OPT_fthinlto_index_EQ)) { if (IK.getLanguage() != InputKind::LLVM_IR) Index: clang/test/CodeGen/thinlto-backend-option.ll === --- clang/test/CodeGen/thinlto-backend-option.ll +++ clang/test/CodeGen/thinlto-backend-option.ll @@ -8,6 +8,8 @@ ; RUN: %clang -flto=thin -c -o %t.o %s ; RUN: llvm-lto -thinlto -o %t %t.o -; RUN: not %clang_cc1 -x ir %t.o -fthinlto-index=%t.thinlto.bc -backend-option -nonexistent -emit-obj -o /dev/null 2>&1 | FileCheck %s +; RUN: not %clang_cc1 -x ir %t.o -fthinlto-index=%t.thinlto.bc -backend-option -nonexistent -emit-obj -o /dev/null 2>&1 | FileCheck %s -check-prefix=UNKNOWN +; UNKNOWN: clang: Unknown command line argument '-nonexistent' -; CHECK: clang: Unknown command line argument '-nonexistent' +; RUN: not %clang_cc1 -flto=thinfoo 2>&1 | FileCheck %s -check-prefix=INVALID +; INVALID: error: invalid value 'thinfoo' in '-flto=thinfoo' Index: clang/lib/Frontend/CompilerInvocation.cpp === --- clang/lib/Frontend/CompilerInvocation.cpp +++ clang/lib/Frontend/CompilerInvocation.cpp @@ -649,8 +649,14 @@ Opts.NoUseJumpTables = Args.hasArg(OPT_fno_jump_tables); Opts.PrepareForLTO = Args.hasArg(OPT_flto, OPT_flto_EQ); - const Arg *A = Args.getLastArg(OPT_flto, OPT_flto_EQ); - Opts.EmitSummaryIndex = A && A->containsValue("thin"); + Opts.EmitSummaryIndex = false; + if (Arg *A = Args.getLastArg(OPT_flto_EQ)) { +StringRef S = A->getValue(); +if (S == "thin") + Opts.EmitSummaryIndex = true; +else if (S != "full") + Diags.Report(diag::err_drv_invalid_value) << A->getAsString(Args) << S; + } Opts.LTOUnit = Args.hasFlag(OPT_flto_unit, OPT_fno_lto_unit, false); if (Arg *A = Args.getLastArg(OPT_fthinlto_index_EQ)) { if (IK.getLanguage() != InputKind::LLVM_IR) ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [clang-tools-extra] r303735 - Modify test so that it looks for patterns in stderr as well
2017-06-14 4:24 GMT+07:00 David Blaikie : > Ah, I find that the test passes if I remove the compile_commands.json file > from my build directory (I have Ninja configured to generate a > compile_commands.json file). > > Looks like what happens is it finds the compilation database and fails > hard when the database doesn't contain a compile command for the file in > question. If the database is not found, it falls back to some basic command > behavior, perhaps? > > You are right, constructor of `CommonOptionsParser` calls `autoDetectFromSource` or `autoDetectFromDirectory` prior to final construction of `FixedCompilationDatabase. Is there some way this test could be fixed to cope with this, otherwise it > seems to get in the way of people actually using clang tools in their > LLVM/Clang build environment? > > IIUC, presence of stale compilation database file in test directory could break many tests. I don't understand why only diagnostic.cpp fails, probably there is something wrong with the clang-tidy application cleanup in this case? > On Tue, Jun 13, 2017 at 7:41 AM Serge Pavlov wrote: > >> I cannot reproduce such fail, so I can only guess how changes made in >> https://reviews.llvm.org/rL303756 and https://reviews.llvm.org/rL303741 >> could cause such problem. Behavior of `Driver::BuildCompilation` is changed >> so that it returns null pointer if errors occur during driver argument >> parse. It is called in `CompilationDatabase.cpp` from >> `stripPositionalArgs`. The call stack at this point is: >> stripPositionalArgs >> clang::tooling::FixedCompilationDatabase::loadFromCommandLine >> clang::tooling::CommonOptionsParser::CommonOptionsParser >> clang::tidy::clangTidyMain >> main >> `FixedCompilationDatabase::loadFromCommandLine` returns null and >> CommonOptionsParser uses another method to create compilation database. The >> output "Compile command not found" means that no input file were found in >> `ClangTool::run`. Maybe some file names are nulls? >> >> >> Thanks, >> --Serge >> >> 2017-06-13 3:42 GMT+07:00 David Blaikie : >> >>> I've been seeing errors from this test recently: >>> >>> Command Output (stderr): >>> -- >>> 1 error generated. >>> Error while processing /usr/local/google/home/blaikie >>> /dev/llvm/src/tools/clang/tools/extra/test/clang-tidy/ >>> diagnostic.cpp.nonexistent.cpp. >>> /usr/local/google/home/blaikie/dev/llvm/src/tools/clang/ >>> tools/extra/test/clang-tidy/diagnostic.cpp:10:12: error: expected >>> string not found in input >>> // CHECK2: :[[@LINE+2]]:9: warning: implicit conversion from 'double' to >>> 'int' changes value from 1.5 to 1 [clang-diagnostic-literal-conversion] >>>^ >>> :2:1: note: scanning from here >>> Skipping /usr/local/google/home/blaikie/dev/llvm/src/tools/clang/ >>> tools/extra/test/clang-tidy/diagnostic.cpp. Compile command not found. >>> ^ >>> :2:1: note: with expression "@LINE+2" equal to "12" >>> Skipping /usr/local/google/home/blaikie/dev/llvm/src/tools/clang/ >>> tools/extra/test/clang-tidy/diagnostic.cpp. Compile command not found. >>> ^ >>> >>> >>> Specifically, the output is: >>> $ ./bin/clang-tidy >>> -checks='-*,clang-diagnostic-*,google-explicit-constructor' >>> /usr/local/google/home/blaikie/dev/llvm/src/tools/clang/ >>> tools/extra/test/clang-tidy/diagnostic.cpp -- -fan-unknown-option 2>&1 >>>error: unknown argument: '-fan-unknown-option' >>> >>>Skipping /usr/local/google/home/blaikie >>> /dev/llvm/src/tools/clang/tools/extra/test/clang-tidy/diagnostic.cpp. >>> Compile command not found. >>> >>> >>> Does this look like it might be related to any of your changes in this >>> area? Perhaps the error due to unknown argument is causing clang-tidy not >>> to continue on to run the check & report the warning? >>> >>> >>> On Wed, May 24, 2017 at 3:51 AM Serge Pavlov via cfe-commits < >>> cfe-commits@lists.llvm.org> wrote: >>> Author: sepavloff Date: Wed May 24 05:50:56 2017 New Revision: 303735 URL: http://llvm.org/viewvc/llvm-project?rev=303735&view=rev Log: Modify test so that it looks for patterns in stderr as well With the change https://reviews.llvm.org/D33013 driver will not build compilation object if command line is invalid, in particular, if unrecognized option is provided. In such cases it will prints diagnostics on stderr. The test 'clang-tidy/diagnostic.cpp' checks reaction on unrecognized option and will fail when D33013 is applied because it checks only stdout for test patterns and expects the name of diagnostic category prepared by clang-tidy. With this change the test makes more general check and must work in either case. Differential Revision: https://reviews.llvm.org/D33173 Modified: clang-tools-extra/trunk/test/clang-tidy/diagnostic.cpp Modified: clang-tools-extra/trunk/test/clang-tidy/diagnostic.cpp URL: http://llvm.org/viewvc/llvm-
[PATCH] D32146: PR32476: __nop_locale_mgmt.h not needed with newlib 2.5+
waltl added a comment. In https://reviews.llvm.org/D32146#780231, @bcraig wrote: > LGTM. > @waltl : Do you need me to submit the changes, or will you handle that? Thanks -- I think jyknight will handle it. https://reviews.llvm.org/D32146 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34055: Be more strict when checking the -flto option value
tejohnson accepted this revision. tejohnson added a comment. This revision is now accepted and ready to land. LGTM, thanks for the fix! https://reviews.llvm.org/D34055 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33304: [clang-tidy][Part1] Add a new module Android and three new checks.
hokein added inline comments. Comment at: clang-tidy/android/FileOpenFlagCheck.cpp:28 + hasParameter(1, hasType(isInteger())), + hasAnyName("open", "open64")) + .bind("funcDecl"))) I'd put the `hasAnyName` matcher in front of `hasParameter` which follows the order of function declaration. The same below. Comment at: clang-tidy/android/FileOpenFlagCheck.cpp:46 + const Expr *FlagArg; + if ((MatchedCall = Result.Nodes.getNodeAs("openFn"))) +FlagArg = MatchedCall->getArg(1); How about ``` const Expr *FlagArg = nullptr; if (const auto* OpenFnCall = Result.Nodes.getXXX()) { // ... } else if (const auto* OpenAtFnCall = Result.Nodes.getXX()) { // ... } assert(FlagArg); ``` ? Comment at: clang-tidy/android/FileOpenFlagCheck.cpp:61 + SourceLocation EndLoc = + Lexer::getLocForEndOfToken(FlagArg->getLocEnd(), 0, SM, getLangOpts()); + Instead of using getLangOpts(), you should use `Result.Context.getLangOpts()`. Comment at: clang-tidy/android/FileOpenFlagCheck.cpp:67 + +bool FileOpenFlagCheck::checkFlags(const Expr *Flags, const SourceManager &SM) { + bool IsFlagIn; No need to be a class member method, put it inside this translation unit. Comment at: clang-tidy/android/FileOpenFlagCheck.cpp:79 +std::pair ExpansionInfo = SM.getDecomposedLoc(Loc); +auto MacroName = +SM.getBufferData(ExpansionInfo.first) You could use `Lexer::getImmediateMacroName(Loc, SM, Opts);` to get the marco name, or doesn't it work here? Comment at: clang-tidy/android/FileOpenFlagCheck.cpp:84 + +IsFlagIn = (MacroName == "O_CLOEXEC"); + I think you can use early return here. With that you don't need to maintain the flag variable `IsFlagIn`, so the code structure like: ``` if (isa<..>) { return ...; } if (isa()) { // ... return ...; } retrun false; ``` Comment at: clang-tidy/android/FileOpenFlagCheck.cpp:88 + // If it's a binary OR operation. + else if ((isa(Flags)) && + (cast(Flags)->getOpcode() == You can use `if (const auto* BO = dyn_cast(Flags))`, so that you don't call `cast(Flags)` multiple times below. Comment at: test/clang-tidy/android-file-open-flag.cpp:76 +void e() { + open("filename", O_CLOEXEC); + // CHECK-MESSAGES-NOT: warning: I would add tests where `O_CLOEXEC` is not at the end of parameter 2 expression like `open("filename", O_RDWR | O_CLOEXEC | O_EXCL)`. https://reviews.llvm.org/D33304 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34206: [clang-tidy] Add "MakeSmartPtrFunction" option to modernize-make-shared/unique checks.
hokein created this revision. Herald added a subscriber: xazax.hun. https://reviews.llvm.org/D34206 Files: clang-tidy/modernize/MakeSmartPtrCheck.cpp clang-tidy/modernize/MakeSmartPtrCheck.h docs/clang-tidy/checks/modernize-make-shared.rst docs/clang-tidy/checks/modernize-make-unique.rst Index: docs/clang-tidy/checks/modernize-make-unique.rst === --- docs/clang-tidy/checks/modernize-make-unique.rst +++ docs/clang-tidy/checks/modernize-make-unique.rst @@ -25,3 +25,11 @@ // becomes my_ptr = std::make_unique(1, 2); + +Options +--- + +.. option:: MakeSmartPtrFunction + + A string specifying the name of make-unique-ptr function. Default is + `std::make_unique`. Index: docs/clang-tidy/checks/modernize-make-shared.rst === --- docs/clang-tidy/checks/modernize-make-shared.rst +++ docs/clang-tidy/checks/modernize-make-shared.rst @@ -25,3 +25,11 @@ // becomes my_ptr = std::make_shared(1, 2); + +Options +--- + +.. option:: MakeSmartPtrFunction + + A string specifying the name of make-shared-ptr function. Default is + `std::make_shared`. Index: clang-tidy/modernize/MakeSmartPtrCheck.h === --- clang-tidy/modernize/MakeSmartPtrCheck.h +++ clang-tidy/modernize/MakeSmartPtrCheck.h @@ -24,9 +24,10 @@ class MakeSmartPtrCheck : public ClangTidyCheck { public: MakeSmartPtrCheck(StringRef Name, ClangTidyContext *Context, -std::string makeSmartPtrFunctionName); +StringRef makeSmartPtrFunctionName); void registerMatchers(ast_matchers::MatchFinder *Finder) final; void check(const ast_matchers::MatchFinder::MatchResult &Result) final; + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; protected: using SmartPtrTypeMatcher = ast_matchers::internal::BindableMatcher; Index: clang-tidy/modernize/MakeSmartPtrCheck.cpp === --- clang-tidy/modernize/MakeSmartPtrCheck.cpp +++ clang-tidy/modernize/MakeSmartPtrCheck.cpp @@ -21,10 +21,17 @@ const char MakeSmartPtrCheck::ResetCall[] = "resetCall"; const char MakeSmartPtrCheck::NewExpression[] = "newExpression"; -MakeSmartPtrCheck::MakeSmartPtrCheck(StringRef Name, ClangTidyContext *Context, - std::string makeSmartPtrFunctionName) +MakeSmartPtrCheck::MakeSmartPtrCheck(StringRef Name, + ClangTidyContext* Context, + StringRef makeSmartPtrFunctionName) : ClangTidyCheck(Name, Context), - makeSmartPtrFunctionName(std::move(makeSmartPtrFunctionName)) {} + makeSmartPtrFunctionName( + Options.get("MakeSmartPtrFunction", makeSmartPtrFunctionName)) {} + +void MakeSmartPtrCheck::storeOptions( +ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "MakeSmartPtrFunction", makeSmartPtrFunctionName); +} void MakeSmartPtrCheck::registerMatchers(ast_matchers::MatchFinder *Finder) { if (!getLangOpts().CPlusPlus11) Index: docs/clang-tidy/checks/modernize-make-unique.rst === --- docs/clang-tidy/checks/modernize-make-unique.rst +++ docs/clang-tidy/checks/modernize-make-unique.rst @@ -25,3 +25,11 @@ // becomes my_ptr = std::make_unique(1, 2); + +Options +--- + +.. option:: MakeSmartPtrFunction + + A string specifying the name of make-unique-ptr function. Default is + `std::make_unique`. Index: docs/clang-tidy/checks/modernize-make-shared.rst === --- docs/clang-tidy/checks/modernize-make-shared.rst +++ docs/clang-tidy/checks/modernize-make-shared.rst @@ -25,3 +25,11 @@ // becomes my_ptr = std::make_shared(1, 2); + +Options +--- + +.. option:: MakeSmartPtrFunction + + A string specifying the name of make-shared-ptr function. Default is + `std::make_shared`. Index: clang-tidy/modernize/MakeSmartPtrCheck.h === --- clang-tidy/modernize/MakeSmartPtrCheck.h +++ clang-tidy/modernize/MakeSmartPtrCheck.h @@ -24,9 +24,10 @@ class MakeSmartPtrCheck : public ClangTidyCheck { public: MakeSmartPtrCheck(StringRef Name, ClangTidyContext *Context, -std::string makeSmartPtrFunctionName); +StringRef makeSmartPtrFunctionName); void registerMatchers(ast_matchers::MatchFinder *Finder) final; void check(const ast_matchers::MatchFinder::MatchResult &Result) final; + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; protected: using SmartPtrTypeMatcher = ast_matchers::internal::BindableMatcher; Index: clang-tidy/modernize/MakeSmartPtrCheck.cpp === --- clang-tidy/modernize/Ma
[PATCH] D34121: [ubsan] Teach the pointer overflow check that "p - <= p" (PR33430)
aprantl added inline comments. Comment at: lib/CodeGen/CGExpr.cpp:3010 +return CGF.EmitCheckedInBoundsGEP(ptr, indices, signedIndices, + /*IsSubtraction=*/false, loc, name); } else { Might want to define an `enum { NotSubtraction = false, IsSubtraction = true }` in the header. We do this in other places. https://reviews.llvm.org/D34121 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r305392 - Be more strict when checking the -flto option value
Author: yamaguchi Date: Wed Jun 14 10:37:11 2017 New Revision: 305392 URL: http://llvm.org/viewvc/llvm-project?rev=305392&view=rev Log: Be more strict when checking the -flto option value Summary: It seems -flto must be either "thin" or "full". I think the use of containValue is just a typo. Reviewers: ruiu, tejohnson Subscribers: mehdi_amini, inglorion Differential Revision: https://reviews.llvm.org/D34055 Modified: cfe/trunk/lib/Frontend/CompilerInvocation.cpp cfe/trunk/test/CodeGen/thinlto-backend-option.ll Modified: cfe/trunk/lib/Frontend/CompilerInvocation.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInvocation.cpp?rev=305392&r1=305391&r2=305392&view=diff == --- cfe/trunk/lib/Frontend/CompilerInvocation.cpp (original) +++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp Wed Jun 14 10:37:11 2017 @@ -649,8 +649,14 @@ static bool ParseCodeGenArgs(CodeGenOpti Opts.NoUseJumpTables = Args.hasArg(OPT_fno_jump_tables); Opts.PrepareForLTO = Args.hasArg(OPT_flto, OPT_flto_EQ); - const Arg *A = Args.getLastArg(OPT_flto, OPT_flto_EQ); - Opts.EmitSummaryIndex = A && A->containsValue("thin"); + Opts.EmitSummaryIndex = false; + if (Arg *A = Args.getLastArg(OPT_flto_EQ)) { +StringRef S = A->getValue(); +if (S == "thin") + Opts.EmitSummaryIndex = true; +else if (S != "full") + Diags.Report(diag::err_drv_invalid_value) << A->getAsString(Args) << S; + } Opts.LTOUnit = Args.hasFlag(OPT_flto_unit, OPT_fno_lto_unit, false); if (Arg *A = Args.getLastArg(OPT_fthinlto_index_EQ)) { if (IK.getLanguage() != InputKind::LLVM_IR) Modified: cfe/trunk/test/CodeGen/thinlto-backend-option.ll URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/thinlto-backend-option.ll?rev=305392&r1=305391&r2=305392&view=diff == --- cfe/trunk/test/CodeGen/thinlto-backend-option.ll (original) +++ cfe/trunk/test/CodeGen/thinlto-backend-option.ll Wed Jun 14 10:37:11 2017 @@ -8,6 +8,8 @@ ; RUN: %clang -flto=thin -c -o %t.o %s ; RUN: llvm-lto -thinlto -o %t %t.o -; RUN: not %clang_cc1 -x ir %t.o -fthinlto-index=%t.thinlto.bc -backend-option -nonexistent -emit-obj -o /dev/null 2>&1 | FileCheck %s +; RUN: not %clang_cc1 -x ir %t.o -fthinlto-index=%t.thinlto.bc -backend-option -nonexistent -emit-obj -o /dev/null 2>&1 | FileCheck %s -check-prefix=UNKNOWN +; UNKNOWN: clang: Unknown command line argument '-nonexistent' -; CHECK: clang: Unknown command line argument '-nonexistent' +; RUN: not %clang_cc1 -flto=thinfoo 2>&1 | FileCheck %s -check-prefix=INVALID +; INVALID: error: invalid value 'thinfoo' in '-flto=thinfoo' ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34055: Be more strict when checking the -flto option value
This revision was automatically updated to reflect the committed changes. Closed by commit rL305392: Be more strict when checking the -flto option value (authored by yamaguchi). Changed prior to commit: https://reviews.llvm.org/D34055?vs=102551&id=102555#toc Repository: rL LLVM https://reviews.llvm.org/D34055 Files: cfe/trunk/lib/Frontend/CompilerInvocation.cpp cfe/trunk/test/CodeGen/thinlto-backend-option.ll Index: cfe/trunk/test/CodeGen/thinlto-backend-option.ll === --- cfe/trunk/test/CodeGen/thinlto-backend-option.ll +++ cfe/trunk/test/CodeGen/thinlto-backend-option.ll @@ -8,6 +8,8 @@ ; RUN: %clang -flto=thin -c -o %t.o %s ; RUN: llvm-lto -thinlto -o %t %t.o -; RUN: not %clang_cc1 -x ir %t.o -fthinlto-index=%t.thinlto.bc -backend-option -nonexistent -emit-obj -o /dev/null 2>&1 | FileCheck %s +; RUN: not %clang_cc1 -x ir %t.o -fthinlto-index=%t.thinlto.bc -backend-option -nonexistent -emit-obj -o /dev/null 2>&1 | FileCheck %s -check-prefix=UNKNOWN +; UNKNOWN: clang: Unknown command line argument '-nonexistent' -; CHECK: clang: Unknown command line argument '-nonexistent' +; RUN: not %clang_cc1 -flto=thinfoo 2>&1 | FileCheck %s -check-prefix=INVALID +; INVALID: error: invalid value 'thinfoo' in '-flto=thinfoo' Index: cfe/trunk/lib/Frontend/CompilerInvocation.cpp === --- cfe/trunk/lib/Frontend/CompilerInvocation.cpp +++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp @@ -649,8 +649,14 @@ Opts.NoUseJumpTables = Args.hasArg(OPT_fno_jump_tables); Opts.PrepareForLTO = Args.hasArg(OPT_flto, OPT_flto_EQ); - const Arg *A = Args.getLastArg(OPT_flto, OPT_flto_EQ); - Opts.EmitSummaryIndex = A && A->containsValue("thin"); + Opts.EmitSummaryIndex = false; + if (Arg *A = Args.getLastArg(OPT_flto_EQ)) { +StringRef S = A->getValue(); +if (S == "thin") + Opts.EmitSummaryIndex = true; +else if (S != "full") + Diags.Report(diag::err_drv_invalid_value) << A->getAsString(Args) << S; + } Opts.LTOUnit = Args.hasFlag(OPT_flto_unit, OPT_fno_lto_unit, false); if (Arg *A = Args.getLastArg(OPT_fthinlto_index_EQ)) { if (IK.getLanguage() != InputKind::LLVM_IR) Index: cfe/trunk/test/CodeGen/thinlto-backend-option.ll === --- cfe/trunk/test/CodeGen/thinlto-backend-option.ll +++ cfe/trunk/test/CodeGen/thinlto-backend-option.ll @@ -8,6 +8,8 @@ ; RUN: %clang -flto=thin -c -o %t.o %s ; RUN: llvm-lto -thinlto -o %t %t.o -; RUN: not %clang_cc1 -x ir %t.o -fthinlto-index=%t.thinlto.bc -backend-option -nonexistent -emit-obj -o /dev/null 2>&1 | FileCheck %s +; RUN: not %clang_cc1 -x ir %t.o -fthinlto-index=%t.thinlto.bc -backend-option -nonexistent -emit-obj -o /dev/null 2>&1 | FileCheck %s -check-prefix=UNKNOWN +; UNKNOWN: clang: Unknown command line argument '-nonexistent' -; CHECK: clang: Unknown command line argument '-nonexistent' +; RUN: not %clang_cc1 -flto=thinfoo 2>&1 | FileCheck %s -check-prefix=INVALID +; INVALID: error: invalid value 'thinfoo' in '-flto=thinfoo' Index: cfe/trunk/lib/Frontend/CompilerInvocation.cpp === --- cfe/trunk/lib/Frontend/CompilerInvocation.cpp +++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp @@ -649,8 +649,14 @@ Opts.NoUseJumpTables = Args.hasArg(OPT_fno_jump_tables); Opts.PrepareForLTO = Args.hasArg(OPT_flto, OPT_flto_EQ); - const Arg *A = Args.getLastArg(OPT_flto, OPT_flto_EQ); - Opts.EmitSummaryIndex = A && A->containsValue("thin"); + Opts.EmitSummaryIndex = false; + if (Arg *A = Args.getLastArg(OPT_flto_EQ)) { +StringRef S = A->getValue(); +if (S == "thin") + Opts.EmitSummaryIndex = true; +else if (S != "full") + Diags.Report(diag::err_drv_invalid_value) << A->getAsString(Args) << S; + } Opts.LTOUnit = Args.hasFlag(OPT_flto_unit, OPT_fno_lto_unit, false); if (Arg *A = Args.getLastArg(OPT_fthinlto_index_EQ)) { if (IK.getLanguage() != InputKind::LLVM_IR) ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30691: [analyzer] Support for naive cross translational unit analysis
dkrupp added a comment. Thanks for the reviews so far. I think we have addressed all major concerns regarding this patch: -(Anna) Scan-build-py integration of the functionality is nearly finished (see https://github.com/rizsotto/scan-build/issues/83) (--ctu switch performs both analysis phases at once). This I think could go in a different patch, but until we could keep the ctu-build.py and ctu-analyze.py scripts. Do you agree? -(Devin,NoQ) In the externalFnMap.txt (which contains which function definitions is located where) Unified Symbol Resolution (USR) is used to identify functions instead of mangled names, which seems to work equally well for C and C++ -(Anna) Dumping the ASTs to the disk. We tried a version where, ASTs are not dumped in the 1st phase, but is recreated each time a function definition is needed from an external TU. It works fine, but the analysis time went up by 20-30% on open source C projects. Is it OK to add this functionality in a next patch? Or should we it as an optional feature right now? Do you see anything else that would prevent this patch to get in? https://reviews.llvm.org/D30691 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30691: [analyzer] Support for naive cross translational unit analysis
a.sidorin accepted this revision. a.sidorin added a comment. Hello Daniel & Gabor, Thank you very much for your work. This patch looks good to me but I think such a change should also be approved by maintainers. https://reviews.llvm.org/D30691 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33644: Add default values for function parameter chunks
klimek added inline comments. Comment at: lib/Sema/SemaCodeComplete.cpp:2453 std::string PlaceholderStr = FormatFunctionParameter(Policy, Param); +if (Param->hasDefaultArg() && PlaceholderStr.find("=") == std::string::npos) { +std::string DefaultValue = yvvan wrote: > klimek wrote: > > Why the check for = in the PlaceholderStr? > Not to add default value twice. If there's already "=" in placeholder string > that means we've already added it in FormatFunctionParameter call In which cases would the default value not be added in FormatFunctionParameter if there is one, and need to be added here? https://reviews.llvm.org/D33644 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34210: Add __has_feature(leak_sanitizer)
fjricci created this revision. Stand-alone leak sanitizer builds are supported with -fsanitize=leak, adds an attribute for consistency with the rest of the sanitizer attributes. https://reviews.llvm.org/D34210 Files: lib/Lex/PPMacroExpansion.cpp test/Lexer/has_feature_leak_sanitizer.cpp Index: test/Lexer/has_feature_leak_sanitizer.cpp === --- /dev/null +++ test/Lexer/has_feature_leak_sanitizer.cpp @@ -0,0 +1,11 @@ +// RUN: %clang_cc1 -E -fsanitize=leak %s -o - | FileCheck --check-prefix=CHECK-LSAN %s +// RUN: %clang_cc1 -E %s -o - | FileCheck --check-prefix=CHECK-NO-LSAN %s + +#if __has_feature(leak_sanitizer) +int LeakSanitizerEnabled(); +#else +int LeakSanitizerDisabled(); +#endif + +// CHECK-LSAN: LeakSanitizerEnabled +// CHECK-NO-LSAN: LeakSanitizerDisabled Index: lib/Lex/PPMacroExpansion.cpp === --- lib/Lex/PPMacroExpansion.cpp +++ lib/Lex/PPMacroExpansion.cpp @@ -1133,6 +1133,7 @@ .Case("enumerator_attributes", true) .Case("nullability", true) .Case("nullability_on_arrays", true) + .Case("leak_sanitizer", LangOpts.Sanitize.has(SanitizerKind::Leak)) .Case("memory_sanitizer", LangOpts.Sanitize.has(SanitizerKind::Memory)) .Case("thread_sanitizer", LangOpts.Sanitize.has(SanitizerKind::Thread)) .Case("dataflow_sanitizer", LangOpts.Sanitize.has(SanitizerKind::DataFlow)) Index: test/Lexer/has_feature_leak_sanitizer.cpp === --- /dev/null +++ test/Lexer/has_feature_leak_sanitizer.cpp @@ -0,0 +1,11 @@ +// RUN: %clang_cc1 -E -fsanitize=leak %s -o - | FileCheck --check-prefix=CHECK-LSAN %s +// RUN: %clang_cc1 -E %s -o - | FileCheck --check-prefix=CHECK-NO-LSAN %s + +#if __has_feature(leak_sanitizer) +int LeakSanitizerEnabled(); +#else +int LeakSanitizerDisabled(); +#endif + +// CHECK-LSAN: LeakSanitizerEnabled +// CHECK-NO-LSAN: LeakSanitizerDisabled Index: lib/Lex/PPMacroExpansion.cpp === --- lib/Lex/PPMacroExpansion.cpp +++ lib/Lex/PPMacroExpansion.cpp @@ -1133,6 +1133,7 @@ .Case("enumerator_attributes", true) .Case("nullability", true) .Case("nullability_on_arrays", true) + .Case("leak_sanitizer", LangOpts.Sanitize.has(SanitizerKind::Leak)) .Case("memory_sanitizer", LangOpts.Sanitize.has(SanitizerKind::Memory)) .Case("thread_sanitizer", LangOpts.Sanitize.has(SanitizerKind::Thread)) .Case("dataflow_sanitizer", LangOpts.Sanitize.has(SanitizerKind::DataFlow)) ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34202: [clang-tidy] readability-function-size: fix nesting level calculation
malcolm.parsons added a comment. What do you expect for this? if (true) if (true) if (true) { int j; } Repository: rL LLVM https://reviews.llvm.org/D34202 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34198: Fix __has_trivial_destructor crash when the type is incomplete with unknown array bounds.
rjmccall added a comment. Test cases? https://reviews.llvm.org/D34198 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34211: Implement the non-execution policy versions of `inclusive_scan` and `transform_ inclusive_scan` for C++17
mclow.lists created this revision. This is the last of three patches - https://reviews.llvm.org/D33997 and https://reviews.llvm.org/D34038 were the first two. When this lands, we'll have all of the new (non-parallel) algorithms that were added in C++17. https://reviews.llvm.org/D34211 Files: include/numeric test/std/numerics/numeric.ops/inclusive.scan/inclusive_scan.pass.cpp test/std/numerics/numeric.ops/inclusive.scan/inclusive_scan_op.pass.cpp test/std/numerics/numeric.ops/inclusive.scan/inclusive_scan_op_init.pass.cpp test/std/numerics/numeric.ops/transform.inclusive.scan/transform_inclusive_scan_bop_uop.pass.cpp test/std/numerics/numeric.ops/transform.inclusive.scan/transform_inclusive_scan_bop_uop_init.pass.cpp Index: test/std/numerics/numeric.ops/transform.inclusive.scan/transform_inclusive_scan_bop_uop_init.pass.cpp === --- test/std/numerics/numeric.ops/transform.inclusive.scan/transform_inclusive_scan_bop_uop_init.pass.cpp +++ test/std/numerics/numeric.ops/transform.inclusive.scan/transform_inclusive_scan_bop_uop_init.pass.cpp @@ -0,0 +1,160 @@ +//===--===// +// +// The LLVM Compiler Infrastructure +// +// This file is dual licensed under the MIT and the University of Illinois Open +// Source Licenses. See LICENSE.TXT for details. +// +//===--===// + +// +// UNSUPPORTED: c++98, c++03, c++11, c++14 + +// template +// OutputIterator transform_inclusive_scan(InputIterator first, InputIterator last, +// OutputIterator result, +// BinaryOperation binary_op, +// UnaryOperation unary_op, +// T init); + + +#include +#include +#include + +#include "test_iterators.h" + +template +struct identity : std::unary_function<_Tp, _Tp> +{ +constexpr const _Tp& operator()(const _Tp& __x) const { return __x;} +}; + +template <> +struct identity +{ +template +constexpr auto operator()(_Tp&& __x) const +_NOEXCEPT_(noexcept(_VSTD::forward<_Tp>(__x))) +-> decltype(_VSTD::forward<_Tp>(__x)) +{ return_VSTD::forward<_Tp>(__x); } +}; + +template +void +test(Iter1 first, Iter1 last, BOp bop, UOp uop, T init, Iter2 rFirst, Iter2 rLast) +{ +std::vector::value_type> v; +// Test not in-place +std::transform_inclusive_scan(first, last, std::back_inserter(v), bop, uop, init); +assert(std::equal(v.begin(), v.end(), rFirst, rLast)); + +// Test in-place +v.clear(); +v.assign(first, last); +std::transform_inclusive_scan(v.begin(), v.end(), v.begin(), bop, uop, init); +assert(std::equal(v.begin(), v.end(), rFirst, rLast)); +} + + +template +void +test() +{ + int ia[] = { 1, 3, 5,7, 9}; +const int pResI0[] = { 1, 4, 9, 16,25};// with identity +const int mResI0[] = { 0, 0, 0,0, 0}; +const int pResN0[] = { -1, -4, -9, -16, -25};// with negate +const int mResN0[] = { 0, 0, 0,0, 0}; +const int pResI2[] = { 3, 6, 11, 18,27};// with identity +const int mResI2[] = { 2, 6, 30, 210, 1890}; +const int pResN2[] = { 1, -2, -7, -14, -23};// with negate +const int mResN2[] = { -2, 6, -30, 210, -1890}; +const unsigned sa = sizeof(ia) / sizeof(ia[0]); +static_assert(sa == sizeof(pResI0) / sizeof(pResI0[0])); // just to be sure +static_assert(sa == sizeof(mResI0) / sizeof(mResI0[0])); // just to be sure +static_assert(sa == sizeof(pResN0) / sizeof(pResN0[0])); // just to be sure +static_assert(sa == sizeof(mResN0) / sizeof(mResN0[0])); // just to be sure +static_assert(sa == sizeof(pResI2) / sizeof(pResI2[0])); // just to be sure +static_assert(sa == sizeof(mResI2) / sizeof(mResI2[0])); // just to be sure +static_assert(sa == sizeof(pResN2) / sizeof(pResN2[0])); // just to be sure +static_assert(sa == sizeof(mResN2) / sizeof(mResN2[0])); // just to be sure + +for (unsigned int i = 0; i < sa; ++i ) { +test(Iter(ia), Iter(ia + i), std::plus<>(), identity<>(),0, pResI0, pResI0 + i); +test(Iter(ia), Iter(ia + i), std::multiplies<>(), identity<>(),0, mResI0, mResI0 + i); +test(Iter(ia), Iter(ia + i), std::plus<>(), std::negate<>(), 0, pResN0, pResN0 + i); +test(Iter(ia), Iter(ia + i), std::multiplies<>(), std::negate<>(), 0, mResN0, mResN0 + i); +test(Iter(ia), Iter(ia + i), std::plus<>(), identity<>(),2, pResI2, pResI2 + i); +test(Iter(ia), Iter(ia + i), std::multiplies<>(), identity<>(),2, mResI2, mResI2 + i); +test(Iter(ia), Iter(ia + i),
[PATCH] D34202: [clang-tidy] readability-function-size: fix nesting level calculation
lebedev.ri added a comment. In https://reviews.llvm.org/D34202#780394, @malcolm.parsons wrote: > What do you expect for this? > > if (true) > if (true) > if (true) { > int j; > } > that it is equivalent to if (true && true && true) { // 1 int j; } This was the intent of that option. There is only one compound statement in your example. All the docs say that it counts compound statements https://github.com/llvm-mirror/clang-tools-extra/blob/9fd3636de8d7034ca4640939fefebd9833ef9ea0/docs/clang-tidy/checks/readability-function-size.rst .. option:: NestingThreshold Flag compound statements ... Repository: rL LLVM https://reviews.llvm.org/D34202 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34202: [clang-tidy] readability-function-size: fix nesting level calculation
malcolm.parsons added a comment. My coding standards require the `{}`, so while I may not agree with your definition of nesting, it doesn't matter. Repository: rL LLVM https://reviews.llvm.org/D34202 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34202: [clang-tidy] readability-function-size: fix nesting level calculation
lebedev.ri added a comment. In https://reviews.llvm.org/D34202#780424, @malcolm.parsons wrote: > My coding standards require the `{}`, so while I may not agree with your > definition of nesting, it doesn't matter. Well, seeing `readability-deep-nesting.cpp` in the issue, i suppose the definition might be adjusted to fit that too, to avoid having more than one check doing just slightly different checking... Repository: rL LLVM https://reviews.llvm.org/D34202 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34202: [clang-tidy] readability-function-size: fix nesting level calculation
malcolm.parsons added a comment. LGTM! Comment at: clang-tidy/readability/FunctionSizeCheck.cpp:58 +// is already nested NestingThreshold levels deep, record the start location +// of this new compound statement +if (CurrentNestingLevel == Info.NestingThreshold) Missing full stop. Repository: rL LLVM https://reviews.llvm.org/D34202 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32146: PR32476: __nop_locale_mgmt.h not needed with newlib 2.5+
This revision was automatically updated to reflect the committed changes. Closed by commit rL305394: PR32476: __nop_locale_mgmt.h not needed with newlib 2.5+ (authored by jyknight). Changed prior to commit: https://reviews.llvm.org/D32146?vs=102265&id=102562#toc Repository: rL LLVM https://reviews.llvm.org/D32146 Files: libcxx/trunk/include/support/newlib/xlocale.h Index: libcxx/trunk/include/support/newlib/xlocale.h === --- libcxx/trunk/include/support/newlib/xlocale.h +++ libcxx/trunk/include/support/newlib/xlocale.h @@ -16,7 +16,10 @@ #include #include #include +#if !defined(__NEWLIB__) || __NEWLIB__ < 2 || \ +__NEWLIB__ == 2 && __NEWLIB_MINOR__ < 5 #include +#endif #include #include Index: libcxx/trunk/include/support/newlib/xlocale.h === --- libcxx/trunk/include/support/newlib/xlocale.h +++ libcxx/trunk/include/support/newlib/xlocale.h @@ -16,7 +16,10 @@ #include #include #include +#if !defined(__NEWLIB__) || __NEWLIB__ < 2 || \ +__NEWLIB__ == 2 && __NEWLIB_MINOR__ < 5 #include +#endif #include #include ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libcxx] r305394 - PR32476: __nop_locale_mgmt.h not needed with newlib 2.5+
Author: jyknight Date: Wed Jun 14 11:40:03 2017 New Revision: 305394 URL: http://llvm.org/viewvc/llvm-project?rev=305394&view=rev Log: PR32476: __nop_locale_mgmt.h not needed with newlib 2.5+ Newlib 2.5 added the locale management functions, so it should not include __nop_local_mgmt.h. This change adds proper guard around that include statement. For newlib 2.4, some releases contain these functions and some don't, and they all have the same version numbers. This patch will work properly with the initial "2.4.0" release which does not include these functions and require __nop_local_mgmt.h. This has been tested against newlib 2.2 and 2.5, and also sanity checks against other different version numbers. Patch by Martin J. O'Riordan and Walter Lee Differential Revision: https://reviews.llvm.org/D32146 Modified: libcxx/trunk/include/support/newlib/xlocale.h Modified: libcxx/trunk/include/support/newlib/xlocale.h URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/support/newlib/xlocale.h?rev=305394&r1=305393&r2=305394&view=diff == --- libcxx/trunk/include/support/newlib/xlocale.h (original) +++ libcxx/trunk/include/support/newlib/xlocale.h Wed Jun 14 11:40:03 2017 @@ -16,7 +16,10 @@ #include #include #include +#if !defined(__NEWLIB__) || __NEWLIB__ < 2 || \ +__NEWLIB__ == 2 && __NEWLIB_MINOR__ < 5 #include +#endif #include #include ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libcxx] r305397 - In several places in std::allocator (and one in shared_ptr, we were casting a 'const T*' to a 'void *' - implicitly casting away the const. Add const_cast to make that expl
Author: marshall Date: Wed Jun 14 11:54:43 2017 New Revision: 305397 URL: http://llvm.org/viewvc/llvm-project?rev=305397&view=rev Log: In several places in std::allocator (and one in shared_ptr, we were casting a 'const T*' to a 'void *' - implicitly casting away the const. Add const_cast to make that explicit. No functional change. Modified: libcxx/trunk/include/memory Modified: libcxx/trunk/include/memory URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/memory?rev=305397&r1=305396&r2=305397&view=diff == --- libcxx/trunk/include/memory (original) +++ libcxx/trunk/include/memory Wed Jun 14 11:54:43 2017 @@ -1884,7 +1884,7 @@ public: return static_cast(_VSTD::__allocate(__n * sizeof(_Tp))); } _LIBCPP_INLINE_VISIBILITY void deallocate(pointer __p, size_type) _NOEXCEPT -{_VSTD::__libcpp_deallocate((void*)__p);} +{_VSTD::__libcpp_deallocate((void*) const_cast<_Tp *>(__p));} _LIBCPP_INLINE_VISIBILITY size_type max_size() const _NOEXCEPT {return size_type(~0) / sizeof(_Tp);} #if !defined(_LIBCPP_HAS_NO_RVALUE_REFERENCES) && !defined(_LIBCPP_HAS_NO_VARIADICS) @@ -1900,7 +1900,7 @@ public: void construct(pointer __p) { -::new((void*)__p) _Tp(); +::new((void*) const_cast<_Tp *>(__p)) _Tp(); } # if defined(_LIBCPP_HAS_NO_RVALUE_REFERENCES) @@ -1909,14 +1909,14 @@ public: void construct(pointer __p, _A0& __a0) { -::new((void*)__p) _Tp(__a0); +::new((void*) const_cast<_Tp *>(__p)) _Tp(__a0); } template _LIBCPP_INLINE_VISIBILITY void construct(pointer __p, const _A0& __a0) { -::new((void*)__p) _Tp(__a0); +::new((void*) const_cast<_Tp *>(__p)) _Tp(__a0); } # endif // defined(_LIBCPP_HAS_NO_RVALUE_REFERENCES) template @@ -1924,28 +1924,28 @@ public: void construct(pointer __p, _A0& __a0, _A1& __a1) { -::new((void*)__p) _Tp(__a0, __a1); +::new((void*) const_cast<_Tp *>(__p)) _Tp(__a0, __a1); } template _LIBCPP_INLINE_VISIBILITY void construct(pointer __p, const _A0& __a0, _A1& __a1) { -::new((void*)__p) _Tp(__a0, __a1); +::new((void*) const_cast<_Tp *>(__p)) _Tp(__a0, __a1); } template _LIBCPP_INLINE_VISIBILITY void construct(pointer __p, _A0& __a0, const _A1& __a1) { -::new((void*)__p) _Tp(__a0, __a1); +::new((void*) const_cast<_Tp *>(__p)) _Tp(__a0, __a1); } template _LIBCPP_INLINE_VISIBILITY void construct(pointer __p, const _A0& __a0, const _A1& __a1) { -::new((void*)__p) _Tp(__a0, __a1); +::new((void*) const_cast<_Tp *>(__p)) _Tp(__a0, __a1); } #endif // !defined(_LIBCPP_HAS_NO_RVALUE_REFERENCES) && !defined(_LIBCPP_HAS_NO_VARIADICS) _LIBCPP_INLINE_VISIBILITY void destroy(pointer __p) {__p->~_Tp();} @@ -3890,7 +3890,9 @@ public: template _LIBCPP_INLINE_VISIBILITY _Dp* __get_deleter() const _NOEXCEPT -{return (_Dp*)(__cntrl_ ? __cntrl_->__get_deleter(typeid(_Dp)) : 0);} +{return static_cast<_Dp*>(__cntrl_ +? const_cast(__cntrl_->__get_deleter(typeid(_Dp))) + : nullptr);} #endif // _LIBCPP_NO_RTTI #ifndef _LIBCPP_HAS_NO_VARIADICS ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r305399 - Define _GNU_SOURCE for rtems c++
Author: jyknight Date: Wed Jun 14 12:01:18 2017 New Revision: 305399 URL: http://llvm.org/viewvc/llvm-project?rev=305399&view=rev Log: Define _GNU_SOURCE for rtems c++ This is required by the libc++ locale support. Patch by Walter Lee. Differential Revision: https://reviews.llvm.org/D34105 Modified: cfe/trunk/lib/Basic/Targets.cpp cfe/trunk/test/Preprocessor/init.c Modified: cfe/trunk/lib/Basic/Targets.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets.cpp?rev=305399&r1=305398&r2=305399&view=diff == --- cfe/trunk/lib/Basic/Targets.cpp (original) +++ cfe/trunk/lib/Basic/Targets.cpp Wed Jun 14 12:01:18 2017 @@ -4734,6 +4734,9 @@ protected: Builder.defineMacro("__rtems__"); Builder.defineMacro("__ELF__"); +// Required by the libc++ locale support. +if (Opts.CPlusPlus) + Builder.defineMacro("_GNU_SOURCE"); } public: Modified: cfe/trunk/test/Preprocessor/init.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/init.c?rev=305399&r1=305398&r2=305399&view=diff == --- cfe/trunk/test/Preprocessor/init.c (original) +++ cfe/trunk/test/Preprocessor/init.c Wed Jun 14 12:01:18 2017 @@ -8779,6 +8779,7 @@ // KFREEBSDI686-DEFINE:#define __GLIBC__ 1 // // RUN: %clang_cc1 -x c++ -triple i686-pc-linux-gnu -fobjc-runtime=gcc -E -dM < /dev/null | FileCheck -match-full-lines -check-prefix GNUSOURCE %s +// RUN: %clang_cc1 -x c++ -triple sparc-rtems-elf -E -dM < /dev/null | FileCheck -match-full-lines -check-prefix GNUSOURCE %s // GNUSOURCE:#define _GNU_SOURCE 1 // // RUN: %clang_cc1 -x c++ -std=c++98 -fno-rtti -E -dM < /dev/null | FileCheck -match-full-lines -check-prefix NORTTI %s ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34105: Define _GNU_SOURCE for rtems c++
This revision was automatically updated to reflect the committed changes. Closed by commit rL305399: Define _GNU_SOURCE for rtems c++ (authored by jyknight). Changed prior to commit: https://reviews.llvm.org/D34105?vs=102188&id=102565#toc Repository: rL LLVM https://reviews.llvm.org/D34105 Files: cfe/trunk/lib/Basic/Targets.cpp cfe/trunk/test/Preprocessor/init.c Index: cfe/trunk/test/Preprocessor/init.c === --- cfe/trunk/test/Preprocessor/init.c +++ cfe/trunk/test/Preprocessor/init.c @@ -8779,6 +8779,7 @@ // KFREEBSDI686-DEFINE:#define __GLIBC__ 1 // // RUN: %clang_cc1 -x c++ -triple i686-pc-linux-gnu -fobjc-runtime=gcc -E -dM < /dev/null | FileCheck -match-full-lines -check-prefix GNUSOURCE %s +// RUN: %clang_cc1 -x c++ -triple sparc-rtems-elf -E -dM < /dev/null | FileCheck -match-full-lines -check-prefix GNUSOURCE %s // GNUSOURCE:#define _GNU_SOURCE 1 // // RUN: %clang_cc1 -x c++ -std=c++98 -fno-rtti -E -dM < /dev/null | FileCheck -match-full-lines -check-prefix NORTTI %s Index: cfe/trunk/lib/Basic/Targets.cpp === --- cfe/trunk/lib/Basic/Targets.cpp +++ cfe/trunk/lib/Basic/Targets.cpp @@ -4734,6 +4734,9 @@ Builder.defineMacro("__rtems__"); Builder.defineMacro("__ELF__"); +// Required by the libc++ locale support. +if (Opts.CPlusPlus) + Builder.defineMacro("_GNU_SOURCE"); } public: Index: cfe/trunk/test/Preprocessor/init.c === --- cfe/trunk/test/Preprocessor/init.c +++ cfe/trunk/test/Preprocessor/init.c @@ -8779,6 +8779,7 @@ // KFREEBSDI686-DEFINE:#define __GLIBC__ 1 // // RUN: %clang_cc1 -x c++ -triple i686-pc-linux-gnu -fobjc-runtime=gcc -E -dM < /dev/null | FileCheck -match-full-lines -check-prefix GNUSOURCE %s +// RUN: %clang_cc1 -x c++ -triple sparc-rtems-elf -E -dM < /dev/null | FileCheck -match-full-lines -check-prefix GNUSOURCE %s // GNUSOURCE:#define _GNU_SOURCE 1 // // RUN: %clang_cc1 -x c++ -std=c++98 -fno-rtti -E -dM < /dev/null | FileCheck -match-full-lines -check-prefix NORTTI %s Index: cfe/trunk/lib/Basic/Targets.cpp === --- cfe/trunk/lib/Basic/Targets.cpp +++ cfe/trunk/lib/Basic/Targets.cpp @@ -4734,6 +4734,9 @@ Builder.defineMacro("__rtems__"); Builder.defineMacro("__ELF__"); +// Required by the libc++ locale support. +if (Opts.CPlusPlus) + Builder.defineMacro("_GNU_SOURCE"); } public: ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30268: Avoid copy of __atoms when char_type is char
hiraditya updated this revision to Diff 102566. hiraditya added a comment. Addressed comments from Eric. https://reviews.llvm.org/D30268 Files: libcxx/benchmarks/stringstream.bench.cpp libcxx/include/__config libcxx/include/locale Index: libcxx/include/locale === --- libcxx/include/locale +++ libcxx/include/locale @@ -380,19 +380,57 @@ struct __num_get : protected __num_get_base { -static string __stage2_int_prep(ios_base& __iob, _CharT* __atoms, _CharT& __thousands_sep); static string __stage2_float_prep(ios_base& __iob, _CharT* __atoms, _CharT& __decimal_point, _CharT& __thousands_sep); -static int __stage2_int_loop(_CharT __ct, int __base, char* __a, char*& __a_end, - unsigned& __dc, _CharT __thousands_sep, const string& __grouping, - unsigned* __g, unsigned*& __g_end, _CharT* __atoms); + static int __stage2_float_loop(_CharT __ct, bool& __in_units, char& __exp, char* __a, char*& __a_end, _CharT __decimal_point, _CharT __thousands_sep, const string& __grouping, unsigned* __g, unsigned*& __g_end, unsigned& __dc, _CharT* __atoms); +#ifndef _LIBCPP_ABI_OPTIMIZED_LOCALE_NUM_GET +static string __stage2_int_prep(ios_base& __iob, _CharT* __atoms, _CharT& __thousands_sep); +static int __stage2_int_loop(_CharT __ct, int __base, char* __a, char*& __a_end, + unsigned& __dc, _CharT __thousands_sep, const string& __grouping, + unsigned* __g, unsigned*& __g_end, _CharT* __atoms); + +#else +static string __stage2_int_prep(ios_base& __iob, _CharT& __thousands_sep) +{ +locale __loc = __iob.getloc(); +const numpunct<_CharT>& __np = use_facet >(__loc); +__thousands_sep = __np.thousands_sep(); +return __np.grouping(); +} + +const _CharT* __do_widen(ios_base& __iob, _CharT* __atoms) const +{ + return __do_widen_p(__iob, __atoms); +} + + +static int __stage2_int_loop(_CharT __ct, int __base, char* __a, char*& __a_end, + unsigned& __dc, _CharT __thousands_sep, const string& __grouping, + unsigned* __g, unsigned*& __g_end, const _CharT* __atoms); +private: +template +const T* __do_widen_p(ios_base& __iob, T* __atoms) const +{ + locale __loc = __iob.getloc(); + use_facet >(__loc).widen(__src, __src + 26, __atoms); + return __atoms; +} + +const char* __do_widen_p(ios_base& __iob, char* __atoms) const +{ + (void)__iob; + (void)__atoms; + return __src; +} +#endif }; +#ifndef _LIBCPP_ABI_OPTIMIZED_LOCALE_NUM_GET template string __num_get<_CharT>::__stage2_int_prep(ios_base& __iob, _CharT* __atoms, _CharT& __thousands_sep) @@ -403,6 +441,7 @@ __thousands_sep = __np.thousands_sep(); return __np.grouping(); } +#endif template string @@ -419,9 +458,16 @@ template int +#ifndef _LIBCPP_ABI_OPTIMIZED_LOCALE_NUM_GET __num_get<_CharT>::__stage2_int_loop(_CharT __ct, int __base, char* __a, char*& __a_end, unsigned& __dc, _CharT __thousands_sep, const string& __grouping, unsigned* __g, unsigned*& __g_end, _CharT* __atoms) +#else +__num_get<_CharT>::__stage2_int_loop(_CharT __ct, int __base, char* __a, char*& __a_end, + unsigned& __dc, _CharT __thousands_sep, const string& __grouping, + unsigned* __g, unsigned*& __g_end, const _CharT* __atoms) + +#endif { if (__a_end == __a && (__ct == __atoms[24] || __ct == __atoms[25])) { @@ -857,9 +903,16 @@ // Stage 1 int __base = this->__get_base(__iob); // Stage 2 -char_type __atoms[26]; char_type __thousands_sep; +const int __atoms_size = 26; +#ifdef _LIBCPP_ABI_OPTIMIZED_LOCALE_NUM_GET +char_type __atoms1[__atoms_size]; +const char_type *__atoms = this->__do_widen(__iob, __atoms1); +string __grouping = this->__stage2_int_prep(__iob, __thousands_sep); +#else +char_type __atoms[__atoms_size]; string __grouping = this->__stage2_int_prep(__iob, __atoms, __thousands_sep); +#endif string __buf; __buf.resize(__buf.capacity()); char* __a = &__buf[0]; @@ -907,9 +960,16 @@ // Stage 1 int __base = this->__get_base(__iob); // Stage 2 -char_type __atoms[26]; char_type __thousands_sep; +const int __atoms_size = 26; +#ifdef _LIBCPP_ABI_OPTIMIZED_LOCALE_NUM_GET +char_type __atoms1[__atoms_size]; +const char_type *__atoms = this->__do_widen(__iob, __atoms1); +string __grouping = this->__stage2_int_prep(__iob, __thousands_sep); +#else +char_type __atoms[__atoms_size]; string __grouping = this->__stage2_int_prep(__iob, __atoms, __thousands_sep); +#endif string __buf; __buf.resize(__bu
[PATCH] D33644: Add default values for function parameter chunks
yvvan added inline comments. Comment at: lib/Sema/SemaCodeComplete.cpp:2453 std::string PlaceholderStr = FormatFunctionParameter(Policy, Param); +if (Param->hasDefaultArg() && PlaceholderStr.find("=") == std::string::npos) { +std::string DefaultValue = klimek wrote: > yvvan wrote: > > klimek wrote: > > > Why the check for = in the PlaceholderStr? > > Not to add default value twice. If there's already "=" in placeholder > > string that means we've already added it in FormatFunctionParameter call > In which cases would the default value not be added in > FormatFunctionParameter if there is one, and need to be added here? If Param->evaluateValue() can evaluate it it will set the default value in FormatFunctionParameter (that means it's a primitive type like "void func(int i = 0)"). In case it's some kind of non-primitive type like "void func(Foo foo = Foo(0, 0))" it will not be evaluated and we can use here the source manager to get the default value string. In this example it will be Foo(0, 0). https://reviews.llvm.org/D33644 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34206: [clang-tidy] Add "MakeSmartPtrFunction" option to modernize-make-shared/unique checks.
Eugene.Zelenko added a comment. It'll be good idea to run modernize-make-unique on LLVM/Clang/etc for llvm::make_unique. https://reviews.llvm.org/D34206 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r305401 - [PPC] Enhance altivec conversion function macros implementation.
Author: jtony Date: Wed Jun 14 12:23:43 2017 New Revision: 305401 URL: http://llvm.org/viewvc/llvm-project?rev=305401&view=rev Log: [PPC] Enhance altivec conversion function macros implementation. Add checking for the second parameter of altivec conversion builtin to make sure it is compile-time constant int. This patch fixes PR33212: PPC vec_cst useless at -O0 Differential Revision: https://reviews.llvm.org/D34092 Modified: cfe/trunk/include/clang/Basic/BuiltinsPPC.def cfe/trunk/test/CodeGen/builtins-ppc-error.c Modified: cfe/trunk/include/clang/Basic/BuiltinsPPC.def URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsPPC.def?rev=305401&r1=305400&r2=305401&view=diff == --- cfe/trunk/include/clang/Basic/BuiltinsPPC.def (original) +++ cfe/trunk/include/clang/Basic/BuiltinsPPC.def Wed Jun 14 12:23:43 2017 @@ -51,10 +51,10 @@ BUILTIN(__builtin_altivec_vavguw, "V4UiV BUILTIN(__builtin_altivec_vrfip, "V4fV4f", "") -BUILTIN(__builtin_altivec_vcfsx, "V4fV4ii", "") -BUILTIN(__builtin_altivec_vcfux, "V4fV4ii", "") -BUILTIN(__builtin_altivec_vctsxs, "V4SiV4fi", "") -BUILTIN(__builtin_altivec_vctuxs, "V4UiV4fi", "") +BUILTIN(__builtin_altivec_vcfsx, "V4fV4iIi", "") +BUILTIN(__builtin_altivec_vcfux, "V4fV4iIi", "") +BUILTIN(__builtin_altivec_vctsxs, "V4SiV4fIi", "") +BUILTIN(__builtin_altivec_vctuxs, "V4UiV4fIi", "") BUILTIN(__builtin_altivec_dss, "vUi", "") BUILTIN(__builtin_altivec_dssall, "v", "") Modified: cfe/trunk/test/CodeGen/builtins-ppc-error.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/builtins-ppc-error.c?rev=305401&r1=305400&r2=305401&view=diff == --- cfe/trunk/test/CodeGen/builtins-ppc-error.c (original) +++ cfe/trunk/test/CodeGen/builtins-ppc-error.c Wed Jun 14 12:23:43 2017 @@ -11,6 +11,8 @@ #include extern vector signed int vsi; +extern vector signed int vui; +extern vector float vf; extern vector unsigned char vuc; void testInsertWord(void) { @@ -34,3 +36,34 @@ void testXXSLDWI(int index) { vec_xxsldwi(1, 2, 3); //expected-error {{first two arguments to '__builtin_vsx_xxsldwi' must be vectors}} vec_xxsldwi(vsi, vuc, 2); //expected-error {{first two arguments to '__builtin_vsx_xxsldwi' must have the same type}} } + +void testCTF(int index) { + vec_ctf(vsi, index); //expected-error {{argument to '__builtin_altivec_vcfsx' must be a constant integer}} + vec_ctf(vui, index); //expected-error {{argument to '__builtin_altivec_vcfsx' must be a constant integer}} +} + +void testVCFSX(int index) { + vec_vcfsx(vsi, index); //expected-error {{argument to '__builtin_altivec_vcfsx' must be a constant integer}} +} + +void testVCFUX(int index) { + vec_vcfux(vui, index); //expected-error {{argument to '__builtin_altivec_vcfux' must be a constant integer}} +} + +void testCTS(int index) { + vec_cts(vf, index); //expected-error {{argument to '__builtin_altivec_vctsxs' must be a constant integer}} + +} + +void testVCTSXS(int index) { + vec_vctsxs(vf, index); //expected-error {{argument to '__builtin_altivec_vctsxs' must be a constant integer}} +} + +void testCTU(int index) { + vec_ctu(vf, index); //expected-error {{argument to '__builtin_altivec_vctuxs' must be a constant integer}} + +} + +void testVCTUXS(int index) { + vec_vctuxs(vf, index); //expected-error {{argument to '__builtin_altivec_vctuxs' must be a constant integer}} +} ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34212: docs: Document binary compatibility issue due to bug in gcc
tstellar created this revision. Reported in PR33161. https://reviews.llvm.org/D34212 Files: docs/BinaryCompatibilityWithOtherCompilers.rst Index: docs/BinaryCompatibilityWithOtherCompilers.rst === --- /dev/null +++ docs/BinaryCompatibilityWithOtherCompilers.rst @@ -0,0 +1,39 @@ += +Binary compatibility with other compilers += + +Introduction + + +This document describes some of the known binary compatibility problems +when mixing object files built by clang with object files built by +other compilers. + +If you run into a compatibility issue, please file a bug at bugs.llvm.org. + +gcc C++ ABI bug with variadic templates +=== + +Older versions of gcc incorrectly mangle variadic class/function templates, +which can lead to undefined symbol errors when linking gcc built objects +with clang built objects. + +gcc does emit the correct symbol name as an alias for the incorrect one, +so libraries built by gcc are not affected by this bug. You can only +hit this bug if you have a library built by clang and you try to link +against it with a gcc built object that uses a variadic class/function +template from the library. + +workarounds: +^^^ + +* Use gcc 5.1.0 or newer. +* Pass the -fabi-version=6 option to gcc when using versions < 5.1.0. + +gcc bug report: +^^^ +https://gcc.gnu.org/bugzilla/show_bug.cgi?id=51322 + +llvm bug report: + +https://llvm.org/PR33161 Index: docs/BinaryCompatibilityWithOtherCompilers.rst === --- /dev/null +++ docs/BinaryCompatibilityWithOtherCompilers.rst @@ -0,0 +1,39 @@ += +Binary compatibility with other compilers += + +Introduction + + +This document describes some of the known binary compatibility problems +when mixing object files built by clang with object files built by +other compilers. + +If you run into a compatibility issue, please file a bug at bugs.llvm.org. + +gcc C++ ABI bug with variadic templates +=== + +Older versions of gcc incorrectly mangle variadic class/function templates, +which can lead to undefined symbol errors when linking gcc built objects +with clang built objects. + +gcc does emit the correct symbol name as an alias for the incorrect one, +so libraries built by gcc are not affected by this bug. You can only +hit this bug if you have a library built by clang and you try to link +against it with a gcc built object that uses a variadic class/function +template from the library. + +workarounds: +^^^ + +* Use gcc 5.1.0 or newer. +* Pass the -fabi-version=6 option to gcc when using versions < 5.1.0. + +gcc bug report: +^^^ +https://gcc.gnu.org/bugzilla/show_bug.cgi?id=51322 + +llvm bug report: + +https://llvm.org/PR33161 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34210: Add __has_feature(leak_sanitizer)
kcc added a comment. I'm not sure about this one. standalone lsan is a link-time feature only, it doesn't change the compilation, so the argument of consistency doesn't apply. https://reviews.llvm.org/D34210 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33304: [clang-tidy][Part1] Add a new module Android and three new checks.
yawanng added inline comments. Comment at: clang-tidy/android/FileOpenFlagCheck.cpp:61 + SourceLocation EndLoc = + Lexer::getLocForEndOfToken(FlagArg->getLocEnd(), 0, SM, getLangOpts()); + hokein wrote: > Instead of using getLangOpts(), you should use `Result.Context.getLangOpts()`. May I ask what's the difference between the `Result.Context.getLangOpts()` and `getLangOpts()`? Does `Result.Context.getLangOpts()` have a longer lifetime than `getLangOpts()`? Comment at: clang-tidy/android/FileOpenFlagCheck.cpp:79 +std::pair ExpansionInfo = SM.getDecomposedLoc(Loc); +auto MacroName = +SM.getBufferData(ExpansionInfo.first) hokein wrote: > You could use `Lexer::getImmediateMacroName(Loc, SM, Opts);` to get the marco > name, or doesn't it work here? `getImmediateMacroName` sometimes cannot return the `O_CLOEXEC `, like #define O_CLOEXEC _O_CLOEXEC #define _O_CLOEXEC 1 `getImmediateMacroName` will return `_O_CLOEXEC`, whereas `O_CLOEXEC` is what we need. I change this to directly get the source text if it's a macro, which seems to be simpler. https://reviews.llvm.org/D33304 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33304: [clang-tidy][Part1] Add a new module Android and three new checks.
yawanng updated this revision to Diff 102577. yawanng marked 7 inline comments as done. https://reviews.llvm.org/D33304 Files: clang-tidy/CMakeLists.txt clang-tidy/android/AndroidTidyModule.cpp clang-tidy/android/CMakeLists.txt clang-tidy/android/FileOpenFlagCheck.cpp clang-tidy/android/FileOpenFlagCheck.h clang-tidy/plugin/CMakeLists.txt clang-tidy/tool/CMakeLists.txt clang-tidy/tool/ClangTidyMain.cpp clang-tidy/tool/run-clang-tidy.py docs/ReleaseNotes.rst docs/clang-tidy/checks/android-file-open-flag.rst docs/clang-tidy/checks/list.rst docs/clang-tidy/index.rst test/clang-tidy/android-file-open-flag.cpp unittests/clang-tidy/CMakeLists.txt Index: unittests/clang-tidy/CMakeLists.txt === --- unittests/clang-tidy/CMakeLists.txt +++ unittests/clang-tidy/CMakeLists.txt @@ -25,6 +25,7 @@ clangFrontend clangLex clangTidy + clangTidyAndroidModule clangTidyGoogleModule clangTidyLLVMModule clangTidyMiscModule Index: test/clang-tidy/android-file-open-flag.cpp === --- /dev/null +++ test/clang-tidy/android-file-open-flag.cpp @@ -0,0 +1,110 @@ +// RUN: %check_clang_tidy %s android-file-open-flag %t + +#define O_RDWR 1 +#define O_EXCL 2 +#define __O_CLOEXEC 3 +#define O_CLOEXEC __O_CLOEXEC + +extern "C" int open(const char *fn, int flags, ...); +extern "C" int open64(const char *fn, int flags, ...); +extern "C" int openat(int dirfd, const char *pathname, int flags, ...); + +void a() { + open("filename", O_RDWR); + // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: 'open' should use O_CLOEXEC where possible [android-file-open-flag] + // CHECK-FIXES: O_RDWR | O_CLOEXEC + open("filename", O_RDWR | O_EXCL); + // CHECK-MESSAGES: :[[@LINE-1]]:35: warning: 'open' should use O_CLOEXEC where + // CHECK-FIXES: O_RDWR | O_EXCL | O_CLOEXEC +} + +void b() { + open64("filename", O_RDWR); + // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: 'open64' should use O_CLOEXEC where possible [android-file-open-flag] + // CHECK-FIXES: O_RDWR | O_CLOEXEC + open64("filename", O_RDWR | O_EXCL); + // CHECK-MESSAGES: :[[@LINE-1]]:37: warning: 'open64' should use O_CLOEXEC where + // CHECK-FIXES: O_RDWR | O_EXCL | O_CLOEXEC +} + +void c() { + openat(0, "filename", O_RDWR); + // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: 'openat' should use O_CLOEXEC where possible [android-file-open-flag] + // CHECK-FIXES: O_RDWR | O_CLOEXEC + openat(0, "filename", O_RDWR | O_EXCL); + // CHECK-MESSAGES: :[[@LINE-1]]:40: warning: 'openat' should use O_CLOEXEC where + // CHECK-FIXES: O_RDWR | O_EXCL | O_CLOEXEC +} + +void f() { + open("filename", 3); + // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 'open' should use O_CLOEXEC where possible [android-file-open-flag] + // CHECK-FIXES: 3 | O_CLOEXEC + open64("filename", 3); + // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: 'open64' should use O_CLOEXEC where possible [android-file-open-flag] + // CHECK-FIXES: 3 | O_CLOEXEC + openat(0, "filename", 3); + // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: 'openat' should use O_CLOEXEC where possible [android-file-open-flag] + // CHECK-FIXES: 3 | O_CLOEXEC + + int flag = 3; + open("filename", flag); + // CHECK-MESSAGES-NOT: warning: + open64("filename", flag); + // CHECK-MESSAGES-NOT: warning: + openat(0, "filename", flag); + // CHECK-MESSAGES-NOT: warning: +} + +namespace i { +int open(const char *pathname, int flags, ...); +int open64(const char *pathname, int flags, ...); +int openat(int dirfd, const char *pathname, int flags, ...); + +void d() { + open("filename", O_RDWR); + // CHECK-MESSAGES-NOT: warning: + open64("filename", O_RDWR); + // CHECK-MESSAGES-NOT: warning: + openat(0, "filename", O_RDWR); + // CHECK-MESSAGES-NOT: warning: +} + +} // namespace i + +void e() { + open("filename", O_CLOEXEC); + // CHECK-MESSAGES-NOT: warning: + open("filename", O_RDWR | O_CLOEXEC); + // CHECK-MESSAGES-NOT: warning: + open("filename", O_RDWR | O_CLOEXEC | O_EXCL); + // CHECK-MESSAGES-NOT: warning: + open64("filename", O_CLOEXEC); + // CHECK-MESSAGES-NOT: warning: + open64("filename", O_RDWR | O_CLOEXEC); + // CHECK-MESSAGES-NOT: warning: + open64("filename", O_RDWR | O_CLOEXEC | O_EXCL); + // CHECK-MESSAGES-NOT: warning: + openat(0, "filename", O_CLOEXEC); + // CHECK-MESSAGES-NOT: warning: + openat(0, "filename", O_RDWR | O_CLOEXEC); + // CHECK-MESSAGES-NOT: warning: + openat(0, "filename", O_RDWR | O_CLOEXEC | O_EXCL); + // CHECK-MESSAGES-NOT: warning: +} + +class G { +public: + int open(const char *pathname, int flags, ...); + int open64(const char *pathname, int flags, ...); + int openat(int dirfd, const char *pathname, int flags, ...); + + void h() { +open("filename", O_RDWR); +// CHECK-MESSAGES-NOT: warning: +open64("filename", O_RDWR); +// CHECK-MESSAGES-NOT: warning: +openat(0, "filename", O_RDWR); +// CHECK-MESSAGES-NOT: warning: +
[PATCH] D34185: [Parser][ObjC] Avoid crashing when skipping to EOF while parsing an ObjC interface/implementation
ahatanak added a comment. This patch fixes the crash and that is fine, but the users might find the last error ("expected '}'" at the end of the file) confusing. This seems to happen because Parser::ParseLexedObjCMethodDefs consumes all tokens in the file until it sees the EOF after consuming all the cached tokens of LexedMethod. I wonder whether we can insert a fake EOF token to prevent ParseLexedObjCMethodDefs from going all the way to the end, just like ParseCXXNonStaticMemberInitialize does. Do you think that would work or would it be too hackish? Repository: rL LLVM https://reviews.llvm.org/D34185 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34210: Add __has_feature(leak_sanitizer)
fjricci added a comment. Currently, the way that we tell users to gate on sanitizer-specific behavior is with `__has_feature(foo_sanitizer)`, as far as I know, it's the only way to do so. LSan provides several API functions for users, ie `__lsan_ignore_object`. If a user program wants to use these API functions in their program, they need a way to check that LSan is enabled at compilation time (even if LSan doesn't actually modify the compile step). I'm not sure of a better or more consistent way to allow that to happen. https://reviews.llvm.org/D34210 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34210: Add __has_feature(leak_sanitizer)
m.ostapenko added a comment. In https://reviews.llvm.org/D34210#780520, @fjricci wrote: > Currently, the way that we tell users to gate on sanitizer-specific behavior > is with `__has_feature(foo_sanitizer)`, as far as I know, it's the only way > to do so. LSan provides several API functions for users, ie > `__lsan_ignore_object`. If a user program wants to use these API functions in > their program, they need a way to check that LSan is enabled at compilation > time (even if LSan doesn't actually modify the compile step). I'm not sure of > a better or more consistent way to allow that to happen. Can't you use weak hooks in your code for this purpose? https://reviews.llvm.org/D34210 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34102: [analyzer] Add portability package for the checkers.
zaks.anna added inline comments. Comment at: include/clang/StaticAnalyzer/Checkers/Checkers.td:454 +def UnixAPIPortabilityChecker : Checker<"API">, + HelpText<"Finds implementation-defined behavior in UNIX/Posix functions">, + DescFile<"UnixAPIChecker.cpp">; NoQ wrote: > zaks.anna wrote: > > Does this need to be in unix? > Well, > 1. the description still says "UNIX/Posix" and > 2. even if we believe that `malloc` isn't a UNIX/Posix-specific thing, we'd > also need to move our `MallocChecker` out of the `unix` package. > > This is probably the right thing to do, but i didn't try to do it here. If we > want this, i'd move the portability checker out of `unix` now and deal with > `MallocChecker` in another patch. I can also move the portability checker's > code into `MallocChecker.cpp`, because that's what we always wanted to do, > according to a `UnixAPIChecker`'s FIXME. If someone is interested in checking if their code is portable, they'd just enable profitability package. I do not think "unix" adds much here. I suggest to just add the check to portability package directly, change the name and make no other changes. WDYT? https://reviews.llvm.org/D34102 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27753: [analyzer] alpha.security.DirtyScalar Checker
zaks.anna added a comment. This generally sounds good. Definitely do submit these changes in small pieces! See http://llvm.org/docs/DeveloperPolicy.html#incremental-development for rationale. https://reviews.llvm.org/D27753 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33383: [GSoC] Flag value completion for clang
ruiu added a comment. Overall looking good. Good work! A few minor comments. Comment at: clang/include/clang/Driver/Options.td:2198 def stdlib_EQ : Joined<["-", "--"], "stdlib=">, Flags<[CC1Option]>, - HelpText<"C++ standard library to use">; + HelpText<"C++ standard library to use">, Values<"libc++,libstdc++,platform">; def sub__library : JoinedOrSeparate<["-"], "sub_library">; v.g.vassilev wrote: > `Values` seems too generic, can we use `ArgValues` instead? I'd keep it as `Values`, as everything is essentially related to command line arguments, and `Args` seems redundant. For example, we didn't name `ArgFlags` but just `Flags`, and `HelpText` instead of `ArgHelpText`, etc. Comment at: clang/lib/Driver/Driver.cpp:1224 +SmallVector ListOfPassedFlags; +// PassedFlags are incomplete flags which format is "c,=,-std,-fsyntax-only" +// So spilit them with "," in order to make list of flags. nit: insert a blank line before a meaningful code block so that code doesn't look too dense. Comment at: clang/lib/Driver/Driver.cpp:1225 +// PassedFlags are incomplete flags which format is "c,=,-std,-fsyntax-only" +// So spilit them with "," in order to make list of flags. +PassedFlags.split(ListOfPassedFlags, ",", -1, false); make a list of flag values. Comment at: clang/lib/Driver/Driver.cpp:1226 +// So spilit them with "," in order to make list of flags. +PassedFlags.split(ListOfPassedFlags, ",", -1, false); +std::vector SuggestedCompletions = Opts->findByPrefix(ListOfPassedFlags[0]); You are using `PassedFlags` only once. Do you want to replace it with `StringRef(A->getValue()).split(...)`? Comment at: clang/test/Driver/autocomplete.c:7 // NONE: foo +// RUN: %clang --autocomplete=l,=,-stdlib | FileCheck %s -check-prefix=STDLIB +// STDLIB: libc++ libstdc++ Why do you want to pass the arguments in the reverse order? https://reviews.llvm.org/D33383 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34210: Add __has_feature(leak_sanitizer)
fjricci abandoned this revision. fjricci added a comment. Weak hooks do provide a good solution, abandoning for now (although it may need to be reconsidered if we get a windows LSan port up and running). https://reviews.llvm.org/D34210 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34156: [LTO] Add -femit-summary-index flag to emit summary for regular LTO
tobiasvk updated this revision to Diff 102585. tobiasvk added a comment. - Change patch to always emit a module summary for regular LTO I don't see any real downside of having a summary given the marginal time and space overhead it takes to construct and save it. https://reviews.llvm.org/D34156 Files: clang/include/clang/Frontend/CodeGenOptions.def clang/lib/CodeGen/BackendUtil.cpp clang/lib/Frontend/CompilerInvocation.cpp clang/test/CodeGen/emit-summary-index.c clang/test/Misc/thinlto.c Index: clang/test/Misc/thinlto.c === --- clang/test/Misc/thinlto.c +++ /dev/null @@ -1,4 +0,0 @@ -// RUN: %clang_cc1 -flto=thin -emit-llvm-bc < %s | llvm-bcanalyzer -dump | FileCheck %s -// ; Check that the -flto=thin option emits a summary -// CHECK: getValue(); if (S == "thin") - Opts.EmitSummaryIndex = true; + Opts.PrepareForThinLTO = true; else if (S != "full") Diags.Report(diag::err_drv_invalid_value) << A->getAsString(Args) << S; } Index: clang/lib/CodeGen/BackendUtil.cpp === --- clang/lib/CodeGen/BackendUtil.cpp +++ clang/lib/CodeGen/BackendUtil.cpp @@ -486,7 +486,7 @@ PMBuilder.Inliner = createFunctionInliningPass( CodeGenOpts.OptimizationLevel, CodeGenOpts.OptimizeSize, (!CodeGenOpts.SampleProfileFile.empty() && - CodeGenOpts.EmitSummaryIndex)); + CodeGenOpts.PrepareForThinLTO)); } PMBuilder.OptLevel = CodeGenOpts.OptimizationLevel; @@ -497,7 +497,7 @@ PMBuilder.DisableUnrollLoops = !CodeGenOpts.UnrollLoops; PMBuilder.MergeFunctions = CodeGenOpts.MergeFunctions; - PMBuilder.PrepareForThinLTO = CodeGenOpts.EmitSummaryIndex; + PMBuilder.PrepareForThinLTO = CodeGenOpts.PrepareForThinLTO; PMBuilder.PrepareForLTO = CodeGenOpts.PrepareForLTO; PMBuilder.RerollLoops = CodeGenOpts.RerollLoops; @@ -733,12 +733,20 @@ std::unique_ptr ThinLinkOS; + // Emit a module summary by default for Regular LTO except for ld64 targets + bool EmitLTOSummary = + (CodeGenOpts.PrepareForLTO && !CodeGenOpts.PrepareForThinLTO && + llvm::Triple(TheModule->getTargetTriple()).getVendor() != + llvm::Triple::Apple); + if (EmitLTOSummary) +TheModule->addModuleFlag(Module::Error, "ThinLTO", uint32_t(0)); + switch (Action) { case Backend_EmitNothing: break; case Backend_EmitBC: -if (CodeGenOpts.EmitSummaryIndex) { +if (CodeGenOpts.PrepareForThinLTO) { if (!CodeGenOpts.ThinLinkBitcodeFile.empty()) { std::error_code EC; ThinLinkOS.reset(new llvm::raw_fd_ostream( @@ -755,7 +763,8 @@ } else PerModulePasses.add( - createBitcodeWriterPass(*OS, CodeGenOpts.EmitLLVMUseLists)); + createBitcodeWriterPass(*OS, CodeGenOpts.EmitLLVMUseLists, + EmitLTOSummary)); break; case Backend_EmitLL: @@ -895,6 +904,14 @@ } } + // Emit a module summary by default for Regular LTO except for ld64 targets + bool EmitLTOSummary = + (CodeGenOpts.PrepareForLTO && !CodeGenOpts.PrepareForThinLTO && + llvm::Triple(TheModule->getTargetTriple()).getVendor() != + llvm::Triple::Apple); + if (EmitLTOSummary) +TheModule->addModuleFlag(Module::Error, "ThinLTO", uint32_t(0)); + // FIXME: We still use the legacy pass manager to do code generation. We // create that pass manager here and use it as needed below. legacy::PassManager CodeGenPasses; @@ -907,7 +924,7 @@ break; case Backend_EmitBC: -if (CodeGenOpts.EmitSummaryIndex) { +if (CodeGenOpts.PrepareForThinLTO) { if (!CodeGenOpts.ThinLinkBitcodeFile.empty()) { std::error_code EC; ThinLinkOS.emplace(CodeGenOpts.ThinLinkBitcodeFile, EC, @@ -922,8 +939,7 @@ ThinLTOBitcodeWriterPass(*OS, ThinLinkOS ? &*ThinLinkOS : nullptr)); } else { MPM.addPass(BitcodeWriterPass(*OS, CodeGenOpts.EmitLLVMUseLists, -CodeGenOpts.EmitSummaryIndex, -CodeGenOpts.EmitSummaryIndex)); +EmitLTOSummary)); } break; Index: clang/include/clang/Frontend/CodeGenOptions.def === --- clang/include/clang/Frontend/CodeGenOptions.def +++ clang/include/clang/Frontend/CodeGenOptions.def @@ -88,7 +88,7 @@ ///< be generated. CODEGENOPT(PrepareForLTO , 1, 0) ///< Set when -flto is enabled on the ///< compile step. -CODEGENOPT(EmitSummaryIndex, 1, 0) ///< Set when -flto=thin is enabled on the +CODEGENOPT(PrepareForThinLTO , 1, 0) ///< Set when -flto=thin is enabled on the ///< compile step. CODEGENOPT(LTOUnit, 1, 0) ///< Emit IR to support LTO unit features (CFI, whole
Re: [clang-tools-extra] r303735 - Modify test so that it looks for patterns in stderr as well
On Wed, Jun 14, 2017, 8:17 AM Serge Pavlov wrote: > 2017-06-14 4:24 GMT+07:00 David Blaikie : > >> Ah, I find that the test passes if I remove the compile_commands.json >> file from my build directory (I have Ninja configured to generate a >> compile_commands.json file). >> >> Looks like what happens is it finds the compilation database and fails >> hard when the database doesn't contain a compile command for the file in >> question. If the database is not found, it falls back to some basic command >> behavior, perhaps? >> >> > You are right, constructor of `CommonOptionsParser` calls > `autoDetectFromSource` or `autoDetectFromDirectory` prior to final > construction of `FixedCompilationDatabase. > > Is there some way this test could be fixed to cope with this, otherwise it >> seems to get in the way of people actually using clang tools in their >> LLVM/Clang build environment? >> >> > IIUC, presence of stale compilation database file in test directory could > break many tests. I don't understand why only diagnostic.cpp fails, > probably there is something wrong with the clang-tidy application cleanup > in this case? > Except it's neither stale nor in the test directory. It's the up to date/useful/used compile_commands.json generated by ninja in the root of the build tree. > >> On Tue, Jun 13, 2017 at 7:41 AM Serge Pavlov wrote: >> >>> I cannot reproduce such fail, so I can only guess how changes made in >>> https://reviews.llvm.org/rL303756 and https://reviews.llvm.org/rL303741 >>> could cause such problem. Behavior of `Driver::BuildCompilation` is changed >>> so that it returns null pointer if errors occur during driver argument >>> parse. It is called in `CompilationDatabase.cpp` from >>> `stripPositionalArgs`. The call stack at this point is: >>> stripPositionalArgs >>> clang::tooling::FixedCompilationDatabase::loadFromCommandLine >>> clang::tooling::CommonOptionsParser::CommonOptionsParser >>> clang::tidy::clangTidyMain >>> main >>> `FixedCompilationDatabase::loadFromCommandLine` returns null and >>> CommonOptionsParser uses another method to create compilation database. The >>> output "Compile command not found" means that no input file were found in >>> `ClangTool::run`. Maybe some file names are nulls? >>> >>> >>> Thanks, >>> --Serge >>> >>> 2017-06-13 3:42 GMT+07:00 David Blaikie : >>> I've been seeing errors from this test recently: Command Output (stderr): -- 1 error generated. Error while processing /usr/local/google/home/blaikie/dev/llvm/src/tools/clang/tools/extra/test/clang-tidy/diagnostic.cpp.nonexistent.cpp. /usr/local/google/home/blaikie/dev/llvm/src/tools/clang/tools/extra/test/clang-tidy/diagnostic.cpp:10:12: error: expected string not found in input // CHECK2: :[[@LINE+2]]:9: warning: implicit conversion from 'double' to 'int' changes value from 1.5 to 1 [clang-diagnostic-literal-conversion] ^ :2:1: note: scanning from here Skipping /usr/local/google/home/blaikie/dev/llvm/src/tools/clang/tools/extra/test/clang-tidy/diagnostic.cpp. Compile command not found. ^ :2:1: note: with expression "@LINE+2" equal to "12" Skipping /usr/local/google/home/blaikie/dev/llvm/src/tools/clang/tools/extra/test/clang-tidy/diagnostic.cpp. Compile command not found. ^ Specifically, the output is: $ ./bin/clang-tidy -checks='-*,clang-diagnostic-*,google-explicit-constructor' /usr/local/google/home/blaikie/dev/llvm/src/tools/clang/tools/extra/test/clang-tidy/diagnostic.cpp -- -fan-unknown-option 2>&1error: unknown argument: '-fan-unknown-option' Skipping /usr/local/google/home/blaikie/dev/llvm/src/tools/clang/tools/extra/test/clang-tidy/diagnostic.cpp. Compile command not found. Does this look like it might be related to any of your changes in this area? Perhaps the error due to unknown argument is causing clang-tidy not to continue on to run the check & report the warning? On Wed, May 24, 2017 at 3:51 AM Serge Pavlov via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: sepavloff > Date: Wed May 24 05:50:56 2017 > New Revision: 303735 > > URL: http://llvm.org/viewvc/llvm-project?rev=303735&view=rev > Log: > Modify test so that it looks for patterns in stderr as well > > With the change https://reviews.llvm.org/D33013 driver will not build > compilation object if command line is invalid, in particular, if > unrecognized option is provided. In such cases it will prints > diagnostics > on stderr. The test 'clang-tidy/diagnostic.cpp' checks reaction on > unrecognized option and will fail when D33013 is applied because it > checks > only stdout for test patterns and expects the name of diagnostic > category > prepared by clang-tid
[libcxx] r305410 - Add some const_casts in places where we were implicitly casting away constness. No functional change, but now they're explicit
Author: marshall Date: Wed Jun 14 15:00:36 2017 New Revision: 305410 URL: http://llvm.org/viewvc/llvm-project?rev=305410&view=rev Log: Add some const_casts in places where we were implicitly casting away constness. No functional change, but now they're explicit Modified: libcxx/trunk/include/__functional_03 libcxx/trunk/include/fstream libcxx/trunk/include/functional libcxx/trunk/include/locale libcxx/trunk/test/support/allocators.h Modified: libcxx/trunk/include/__functional_03 URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/__functional_03?rev=305410&r1=305409&r2=305410&view=diff == --- libcxx/trunk/include/__functional_03 (original) +++ libcxx/trunk/include/__functional_03 Wed Jun 14 15:00:36 2017 @@ -704,7 +704,7 @@ function<_Rp()>::target() { if (__f_ == 0) return (_Tp*)0; -return (_Tp*)__f_->target(typeid(_Tp)); +return (_Tp*) const_cast(__f_->target(typeid(_Tp))); } template @@ -980,7 +980,7 @@ function<_Rp(_A0)>::target() { if (__f_ == 0) return (_Tp*)0; -return (_Tp*)__f_->target(typeid(_Tp)); +return (_Tp*) const_cast(__f_->target(typeid(_Tp))); } template @@ -1256,7 +1256,7 @@ function<_Rp(_A0, _A1)>::target() { if (__f_ == 0) return (_Tp*)0; -return (_Tp*)__f_->target(typeid(_Tp)); +return (_Tp*) const_cast(__f_->target(typeid(_Tp))); } template @@ -1532,7 +1532,7 @@ function<_Rp(_A0, _A1, _A2)>::target() { if (__f_ == 0) return (_Tp*)0; -return (_Tp*)__f_->target(typeid(_Tp)); +return (_Tp*) const_cast(__f_->target(typeid(_Tp))); } template Modified: libcxx/trunk/include/fstream URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/fstream?rev=305410&r1=305409&r2=305410&view=diff == --- libcxx/trunk/include/fstream (original) +++ libcxx/trunk/include/fstream Wed Jun 14 15:00:36 2017 @@ -617,7 +617,7 @@ basic_filebuf<_CharT, _Traits>::underflo static_cast(__extbufend_ - __extbufnext_)); codecvt_base::result __r; __st_last_ = __st_; -size_t __nr = fread((void*)__extbufnext_, 1, __nmemb, __file_); +size_t __nr = fread((void*) const_cast(__extbufnext_), 1, __nmemb, __file_); if (__nr != 0) { if (!__cv_) @@ -630,7 +630,8 @@ basic_filebuf<_CharT, _Traits>::underflo this->eback() + __ibs_, __inext); if (__r == codecvt_base::noconv) { -this->setg((char_type*)__extbuf_, (char_type*)__extbuf_, (char_type*)__extbufend_); +this->setg((char_type*)__extbuf_, (char_type*)__extbuf_, + (char_type*)const_cast(__extbufend_)); __c = traits_type::to_int_type(*this->gptr()); } else if (__inext != this->eback() + __unget_sz) @@ -722,7 +723,7 @@ basic_filebuf<_CharT, _Traits>::overflow return traits_type::eof(); if (__r == codecvt_base::partial) { -this->setp((char_type*)__e, this->pptr()); +this->setp(const_cast(__e), this->pptr()); this->pbump(this->epptr() - this->pbase()); } } Modified: libcxx/trunk/include/functional URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/functional?rev=305410&r1=305409&r2=305410&view=diff == --- libcxx/trunk/include/functional (original) +++ libcxx/trunk/include/functional Wed Jun 14 15:00:36 2017 @@ -1941,8 +1941,8 @@ _Tp* function<_Rp(_ArgTypes...)>::target() _NOEXCEPT { if (__f_ == 0) -return (_Tp*)0; -return (_Tp*)__f_->target(typeid(_Tp)); +return nullptr; +return (_Tp*) const_cast(__f_->target(typeid(_Tp))); } template @@ -1951,7 +1951,7 @@ const _Tp* function<_Rp(_ArgTypes...)>::target() const _NOEXCEPT { if (__f_ == 0) -return (const _Tp*)0; +return nullptr; return (const _Tp*)__f_->target(typeid(_Tp)); } Modified: libcxx/trunk/include/locale URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/locale?rev=305410&r1=305409&r2=305410&view=diff == --- libcxx/trunk/include/locale (original) +++ libcxx/trunk/include/locale Wed Jun 14 15:00:36 2017 @@ -3960,7 +3960,8 @@ wbuffer_convert<_Codecvt, _Elem, _Tr>::u this->egptr(), __inext); if (__r == codecvt_base::noconv) { -this->setg((char_type*)__extbuf_, (char_type*)__extbuf_, (char_type*)
[PATCH] D33102: [clang] Implement -Wcast-qual for C++
mclow.lists added a comment. As of r305410, libc++ passes all the tests w/ -Wcast-qual enabled. Repository: rL LLVM https://reviews.llvm.org/D33102 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33308: [analyzer]: Improve test handling with multiple constraint managers
dcoughlin accepted this revision. dcoughlin added a comment. This revision is now accepted and ready to land. This looks good to me. Sorry of the delay here -- and thanks for your patience. I will review the other patches this weekend. https://reviews.llvm.org/D33308 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34185: [Parser][ObjC] Avoid crashing when skipping to EOF while parsing an ObjC interface/implementation
arphaman added a comment. In https://reviews.llvm.org/D34185#780494, @ahatanak wrote: > This patch fixes the crash and that is fine, but the users might find the > last error ("expected '}'" at the end of the file) confusing. This seems to > happen because Parser::ParseLexedObjCMethodDefs consumes all tokens in the > file until it sees the EOF after consuming all the cached tokens of > LexedMethod. > > I wonder whether we can insert a fake EOF token to prevent > ParseLexedObjCMethodDefs from going all the way to the end, just like > ParseCXXNonStaticMemberInitialize does. Do you think that would work or would > it be too hackish? I think that it would probably work quite well. I've thought about fixing it while working on a patch as well, but didn't realize that we could just inject EOF into the token stream. I will update this patch with this kind of handling then. Repository: rL LLVM https://reviews.llvm.org/D34185 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34175: [driver][macOS] Pick the system version for the deployment target if the SDK is newer than the system
arphaman added inline comments. Comment at: lib/Driver/ToolChains/Darwin.cpp:1133 + HadExtra) || + HadExtra || Major != 10 || Minor >= 100 || Micro >= 100) +return MacOSSDKVersion; inouehrs wrote: > What these magic numbers mean (especially minor and micro versions)? > You may need some comments. I actually just took it from the function below which checks if the macOS version is valid (so we want 10.XX.XX). But looking at it again, I think that we don't really them here as the SDK version should will probably be valid. I'll update the patch and remove them from the code. Repository: rL LLVM https://reviews.llvm.org/D34175 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34121: [ubsan] Teach the pointer overflow check that "p - <= p" (PR33430)
vsk updated this revision to Diff 102603. vsk marked an inline comment as done. vsk added a comment. Address Adrian's comment about using an enum to simplify some calls. https://reviews.llvm.org/D34121 Files: lib/CodeGen/CGExpr.cpp lib/CodeGen/CGExprScalar.cpp lib/CodeGen/CodeGenFunction.h test/CodeGen/ubsan-pointer-overflow.m Index: test/CodeGen/ubsan-pointer-overflow.m === --- test/CodeGen/ubsan-pointer-overflow.m +++ test/CodeGen/ubsan-pointer-overflow.m @@ -10,16 +10,20 @@ ++p; // CHECK: ptrtoint i8* {{.*}} to i64, !nosanitize - // CHECK-NEXT: add i64 {{.*}}, -1, !nosanitize - // CHECK: select i1 false{{.*}}, !nosanitize + // CHECK-NEXT: [[COMPGEP:%.*]] = add i64 {{.*}}, -1, !nosanitize + // CHECK: [[NEGVALID:%.*]] = icmp ule i64 [[COMPGEP]], {{.*}}, !nosanitize + // CHECK-NOT: select + // CHECK: br i1 [[NEGVALID]]{{.*}}, !nosanitize // CHECK: call void @__ubsan_handle_pointer_overflow{{.*}} --p; + // CHECK: icmp uge i64 // CHECK-NOT: select // CHECK: call void @__ubsan_handle_pointer_overflow{{.*}} p++; - // CHECK: select + // CHECK: icmp ule i64 + // CHECK-NOT: select // CHECK: call void @__ubsan_handle_pointer_overflow{{.*}} p--; } @@ -64,7 +68,8 @@ // CHECK: [[OFFSET:%.*]] = sub i64 0, {{.*}} // CHECK-NEXT: getelementptr inbounds {{.*}} [[OFFSET]] - // CHECK: select + // CHECK: icmp ule i64 + // CHECK-NOT: select // CHECK: call void @__ubsan_handle_pointer_overflow{{.*}} p - i; } Index: lib/CodeGen/CodeGenFunction.h === --- lib/CodeGen/CodeGenFunction.h +++ lib/CodeGen/CodeGenFunction.h @@ -3552,12 +3552,19 @@ /// nonnull, if \p LHS is marked _Nonnull. void EmitNullabilityCheck(LValue LHS, llvm::Value *RHS, SourceLocation Loc); + /// An enumeration which makes it easier to specify whether or not an + /// operation is a subtraction. + enum { NotSubtraction = false, IsSubtraction = true }; + /// Same as IRBuilder::CreateInBoundsGEP, but additionally emits a check to /// detect undefined behavior when the pointer overflow sanitizer is enabled. /// \p SignedIndices indicates whether any of the GEP indices are signed. + /// \p IsSubtraction indicates whether the expression used to form the GEP + /// is a subtraction. llvm::Value *EmitCheckedInBoundsGEP(llvm::Value *Ptr, ArrayRef IdxList, bool SignedIndices, + bool IsSubtraction, SourceLocation Loc, const Twine &Name = ""); Index: lib/CodeGen/CGExprScalar.cpp === --- lib/CodeGen/CGExprScalar.cpp +++ lib/CodeGen/CGExprScalar.cpp @@ -1851,7 +1851,7 @@ llvm::Value *input; int amount = (isInc ? 1 : -1); - bool signedIndex = !isInc; + bool isSubtraction = !isInc; if (const AtomicType *atomicTy = type->getAs()) { type = atomicTy->getValueType(); @@ -1941,8 +1941,9 @@ if (CGF.getLangOpts().isSignedOverflowDefined()) value = Builder.CreateGEP(value, numElts, "vla.inc"); else -value = CGF.EmitCheckedInBoundsGEP(value, numElts, signedIndex, - E->getExprLoc(), "vla.inc"); +value = CGF.EmitCheckedInBoundsGEP( +value, numElts, /*SignedIndices=*/false, isSubtraction, +E->getExprLoc(), "vla.inc"); // Arithmetic on function pointers (!) is just +-1. } else if (type->isFunctionType()) { @@ -1952,18 +1953,20 @@ if (CGF.getLangOpts().isSignedOverflowDefined()) value = Builder.CreateGEP(value, amt, "incdec.funcptr"); else -value = CGF.EmitCheckedInBoundsGEP(value, amt, signedIndex, - E->getExprLoc(), "incdec.funcptr"); +value = CGF.EmitCheckedInBoundsGEP(value, amt, /*SignedIndices=*/false, + isSubtraction, E->getExprLoc(), + "incdec.funcptr"); value = Builder.CreateBitCast(value, input->getType()); // For everything else, we can just do a simple increment. } else { llvm::Value *amt = Builder.getInt32(amount); if (CGF.getLangOpts().isSignedOverflowDefined()) value = Builder.CreateGEP(value, amt, "incdec.ptr"); else -value = CGF.EmitCheckedInBoundsGEP(value, amt, signedIndex, - E->getExprLoc(), "incdec.ptr"); +value = CGF.EmitCheckedInBoundsGEP(value, amt, /*SignedIndices=*/false, + isSubtraction, E->getExprLoc(), + "incdec.ptr"); } // Vector increment/decrement. @@ -2044,7 +2047,8 @@ if (CGF.getLan
[PATCH] D32046: [Preprocessor]Correct Macro-Arg allocation of StringifiedArguments, correct getNumArguments
arphaman accepted this revision. arphaman added a comment. This revision is now accepted and ready to land. LGTM! Please fix the code style issues that I pointed out before committing. Comment at: unittests/Lex/LexerTest.cpp:24 #include "clang/Lex/PreprocessorOptions.h" +#include "clang/Lex/MacroArgs.h" +#include "clang/Lex/MacroInfo.h" Please put these two new includes before `#include "clang/Lex/ModuleLoader.h"` Comment at: unittests/Lex/LexerTest.cpp:46 - std::vector Lex(StringRef Source) { + std::unique_ptr CreatePP(StringRef Source, TrivialModuleLoader& ModLoader) { std::unique_ptr Buf = I think this violates the 80 columns, please reformat this declaration. Comment at: unittests/Lex/LexerTest.cpp:62 +return PP; + } + std::vector Lex(StringRef Source) { NIT: Please add a new line after '}'. https://reviews.llvm.org/D32046 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits