[PATCH] D25897: Add __no_instrument_function__ to _LIBCPP_INLINE_VISIBILITY and _LIBCPP_ALWAYS_INLINE
EricWF added a comment. I created a fix as described above. Could you please tell me if it works for you? It can be found here: https://reviews.llvm.org/D26324 https://reviews.llvm.org/D25897 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26306: [AVX-512] Make VBMI instruction set enabling imply that the BWI instruction set is also enabled.
zvi accepted this revision. zvi added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D26306 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26306: [AVX-512] Make VBMI instruction set enabling imply that the BWI instruction set is also enabled.
zvi added inline comments. Comment at: lib/Basic/Targets.cpp:3353 setSSELevel(Features, AVX512F, Enabled); +// Enable BWI instruction is VBMI is being enabled. +if (Name == "avx512vbmi" && Enabled) is -> if Comment at: test/Preprocessor/x86_target_features.c:212 +// RUN: %clang -target i386-unknown-unknown -march=atom -mavx512vbmi -mno-avx512vbmi -x c -E -dM -o - %s | FileCheck -match-full-lines --check-prefix=AVX512VBMINOAVX512BW %s + Test is turning on and then off the same feature. Did you mean -mavx512bw -mno-avx512vbmi ? If the latter, I would prefer the compiler shout at me for providing contradicting flags than let the last flag win. Consider enforcing this in handleTargetFeatures(). https://reviews.llvm.org/D26306 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26282: [PowerPC] Implement plain VSX load/store builtins.
jtony updated this revision to Diff 76945. jtony added a comment. Reorder the overloads according to their size. https://reviews.llvm.org/D26282 Files: lib/Headers/altivec.h test/CodeGen/builtins-ppc-altivec.c test/CodeGen/builtins-ppc-quadword.c test/CodeGen/builtins-ppc-vsx.c Index: test/CodeGen/builtins-ppc-vsx.c === --- test/CodeGen/builtins-ppc-vsx.c +++ test/CodeGen/builtins-ppc-vsx.c @@ -21,6 +21,7 @@ vector signed long long vsll = { 255LL, -937LL }; vector unsigned long long vull = { 1447LL, 2894LL }; double d = 23.4; +signed long long sll = 618LL; float af[4] = {23.4f, 56.7f, 89.0f, 12.3f}; double ad[2] = {23.4, 56.7}; signed char asc[16] = { -8, 9, -10, 11, -12, 13, -14, 15, @@ -31,8 +32,8 @@ unsigned short aus[8] = { 0, 1, 2, 3, 4, 5, 6, 7 }; signed int asi[4] = { -1, 2, -3, 4 }; unsigned int aui[4] = { 0, 1, 2, 3 }; -signed long asl[2] = { -1L, 2L }; -unsigned long aul[2] = { 1L, 2L }; +signed long long asll[2] = { -1L, 2L }; +unsigned long long aull[2] = { 1L, 2L }; vector float res_vf; vector double res_vd; @@ -1248,4 +1249,28 @@ res_vull = vec_sro(vull, vuc); // CHECK: @llvm.ppc.altivec.vsro // CHECK-LE: @llvm.ppc.altivec.vsro + +res_vsll = vec_xl(sll, asll); +// CHECK: load <2 x i64>, <2 x i64>* %{{[0-9]+}}, align 16 +// CHECK-LE: load <2 x i64>, <2 x i64>* %{{[0-9]+}}, align 16 + +res_vull = vec_xl(sll, aull); +// CHECK: load <2 x i64>, <2 x i64>* %{{[0-9]+}}, align 16 +// CHECK-LE: load <2 x i64>, <2 x i64>* %{{[0-9]+}}, align 16 + +res_vd = vec_xl(sll, ad); +// CHECK: load <2 x double>, <2 x double>* %{{[0-9]+}}, align 16 +// CHECK-LE: load <2 x double>, <2 x double>* %{{[0-9]+}}, align 16 + +vec_xst(vsll, sll, asll); +// CHECK: store <2 x i64> %{{[0-9]+}}, <2 x i64>* %{{[0-9]+}}, align 16 +// CHECK-LE: store <2 x i64> %{{[0-9]+}}, <2 x i64>* %{{[0-9]+}}, align 16 + +vec_xst(vull, sll, aull); +// CHECK: store <2 x i64> %{{[0-9]+}}, <2 x i64>* %{{[0-9]+}}, align 16 +// CHECK-LE: store <2 x i64> %{{[0-9]+}}, <2 x i64>* %{{[0-9]+}}, align 16 + +vec_xst(vd, sll, ad); +// CHECK: store <2 x double> %{{[0-9]+}}, <2 x double>* %{{[0-9]+}}, align 16 +// CHECK-LE: store <2 x double> %{{[0-9]+}}, <2 x double>* %{{[0-9]+}}, align 16 } Index: test/CodeGen/builtins-ppc-quadword.c === --- test/CodeGen/builtins-ppc-quadword.c +++ test/CodeGen/builtins-ppc-quadword.c @@ -15,6 +15,12 @@ // CHECK-PPC: error: __int128 is not supported on this target vector unsigned __int128 vulll = { 1 }; +signed long long param_sll; +// CHECK-PPC: error: __int128 is not supported on this target +signed __int128 param_lll; +// CHECK-PPC: error: __int128 is not supported on this target +unsigned __int128 param_ulll; + // CHECK-PPC: error: __int128 is not supported on this target vector signed __int128 res_vlll; // CHECK-PPC: error: __int128 is not supported on this target @@ -165,4 +171,26 @@ // CHECK-LE: xor <16 x i8> // CHECK-LE: call <4 x i32> @llvm.ppc.altivec.vperm(<4 x i32> {{%.+}}, <4 x i32> {{%.+}}, <16 x i8> {{%.+}}) // CHECK_PPC: error: call to 'vec_revb' is ambiguous + + /* vec_xl */ + res_vlll = vec_xl(param_sll, ¶m_lll); + // CHECK: load <1 x i128>, <1 x i128>* %{{[0-9]+}}, align 16 + // CHECK-LE: load <1 x i128>, <1 x i128>* %{{[0-9]+}}, align 16 + // CHECK-PPC: error: call to 'vec_xl' is ambiguous + + res_vulll = vec_xl(param_sll, ¶m_ulll); + // CHECK: load <1 x i128>, <1 x i128>* %{{[0-9]+}}, align 16 + // CHECK-LE: load <1 x i128>, <1 x i128>* %{{[0-9]+}}, align 16 + // CHECK-PPC: error: call to 'vec_xl' is ambiguous + + /* vec_xst */ + vec_xst(vlll, param_sll, ¶m_lll); + // CHECK: store <1 x i128> %{{[0-9]+}}, <1 x i128>* %{{[0-9]+}}, align 16 + // CHECK-LE: store <1 x i128> %{{[0-9]+}}, <1 x i128>* %{{[0-9]+}}, align 16 + // CHECK-PPC: error: call to 'vec_xst' is ambiguous + + vec_xst(vulll, param_sll, ¶m_ulll); + // CHECK: store <1 x i128> %{{[0-9]+}}, <1 x i128>* %{{[0-9]+}}, align 16 + // CHECK-LE: store <1 x i128> %{{[0-9]+}}, <1 x i128>* %{{[0-9]+}}, align 16 + // CHECK-PPC: error: call to 'vec_xst' is ambiguous } Index: test/CodeGen/builtins-ppc-altivec.c === --- test/CodeGen/builtins-ppc-altivec.c +++ test/CodeGen/builtins-ppc-altivec.c @@ -45,6 +45,7 @@ int param_i; unsigned int param_ui; float param_f; +signed long long param_sll; int res_sc; int res_uc; @@ -9200,3 +9201,69 @@ // CHECK-LE: xor <16 x i8> // CHECK-LE: call <4 x i32> @llvm.ppc.altivec.vperm(<4 x i32> {{%.+}}, <4 x i32> {{%.+}}, <16 x i8> {{%.+}}) } + +/* -- vec_xl */ +void test9() { + // CHECK-LABEL: define void @test9 + // CHECK-LE-LABEL: define void @test9 + res_vsc = vec_xl(param_sll, ¶m_sc); + // CHECK: load <16 x i8>, <16 x i8>* %{{[0-9]+}}, align 16 + // CHECK-LE: load <16 x i8>, <16 x i8>* %{{[0-9
[PATCH] D26138: [clang-tidy] Add modernize-use-equals-delete check
malcolm.parsons planned changes to this revision. malcolm.parsons added inline comments. Comment at: clang-tidy/modernize/UseEqualsDeleteCheck.cpp:58 + << FixItHint::CreateInsertion(StartLoc, "public: ") + << FixItHint::CreateInsertion(AfterLoc, " private:"); +} aaron.ballman wrote: > malcolm.parsons wrote: > > aaron.ballman wrote: > > > I am on the fence about this fixit. On the one hand, the fix is a > > > technical improvement because it means that implementations will > > > consistently find the declaration and bark about it being explicitly > > > deleted. On the other hand, the fixit suggests code that should never > > > pass a reasonable code review. > > > > > > I'm wondering if it would make more sense to leave the access specifiers > > > alone and just put a FIXME in the code to point the situation out. I am > > > guessing that at some point we will have a refactoring tool that can help > > > without resorting to making declarations like `public: C() = delete; > > > private:`. > > > > > > What do you think? > > I'm wondering whether no fixit would be better than a not-good-enough fixit. > Generally, no. Incremental improvements are almost always fine. However, the > user is asking to have their code modernized, and the fixit results in code > that looks more foreign than modern (at least, to me). > > I won't block the patch moving forward regardless of whether the fixit is in > or out, but I am curious if @alexfh has an opinion, or if you have a strong > preference one way or the other. I'll change the check to warn about deleted methods that aren't public; the user can fix those manually until a better fixit is possible. https://reviews.llvm.org/D26138 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26302: [OpenCL] Remove redundant test for OpenCL header file
Anastasia added a comment. LGTM! Thanks! Do you know the runtime of this test now? https://reviews.llvm.org/D26302 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26310: Add a method to get the list of registered static analyzer checkers.
zaks.anna added a comment. Thanks for fixing this! Comment at: lib/StaticAnalyzer/Core/AnalyzerOptions.cpp:39 +if (IncludeExperimental || +(!CheckName.startswith("debug.") && !CheckName.startswith("alpha."))) + Result.push_back(CheckName); Debug checkers are not user-facing; they are used to debug the analyzer. So we should either have another flag for them or always skip them. https://reviews.llvm.org/D26310 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26311: Use AnalyzerOptions::getRegisteredCheckers() instead of clang/StaticAnalyzer/Checkers/Checkers.inc
zaks.anna accepted this revision. zaks.anna added a comment. This revision is now accepted and ready to land. LGTM! https://reviews.llvm.org/D26311 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] Warning for main returning a bool.
+richard On Fri, Oct 14, 2016 at 10:18 AM Joshua Hurwitz via cfe-commits < cfe-commits@lists.llvm.org> wrote: > See attached. > > Returning a bool from main is a special case of return type mismatch. The > common convention when returning a bool is that 'true' (== 1) indicates > success and 'false' (== 0) failure. But since main expects a return value > of 0 on success, returning a bool is usually unintended. > ___ > cfe-commits mailing list > cfe-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r286041 - clang-format: Better support for CUDA's triple brackets.
Author: djasper Date: Sat Nov 5 12:43:16 2016 New Revision: 286041 URL: http://llvm.org/viewvc/llvm-project?rev=286041&view=rev Log: clang-format: Better support for CUDA's triple brackets. Before: aaa< a, aa, aa><<>>(); After: aaa <<>>(); Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp cfe/trunk/unittests/Format/FormatTest.cpp Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=286041&r1=286040&r2=286041&view=diff == --- cfe/trunk/lib/Format/TokenAnnotator.cpp (original) +++ cfe/trunk/lib/Format/TokenAnnotator.cpp Sat Nov 5 12:43:16 2016 @@ -2488,6 +2488,8 @@ bool TokenAnnotator::canBreakBefore(cons return true; if (Right.is(TT_RangeBasedForLoopColon)) return false; + if (Left.is(TT_TemplateCloser) && Right.is(TT_TemplateOpener)) +return true; if (Left.isOneOf(TT_TemplateCloser, TT_UnaryOperator) || Left.is(tok::kw_operator)) return false; Modified: cfe/trunk/unittests/Format/FormatTest.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=286041&r1=286040&r2=286041&view=diff == --- cfe/trunk/unittests/Format/FormatTest.cpp (original) +++ cfe/trunk/unittests/Format/FormatTest.cpp Sat Nov 5 12:43:16 2016 @@ -11391,6 +11391,8 @@ TEST_F(FormatTest, TripleAngleBrackets) EXPECT_EQ("f<<<1, 1>>>();", format("f< param > <<< 1, 1 >>> ();")); verifyFormat("aaa" "aaa<<<\n1, 1>>>();"); + verifyFormat("aaa\n" + "<<>>();"); } TEST_F(FormatTest, MergeLessLessAtEnd) { ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26288: Deduplicate replacements by FileEntry instead of file names.
klimek added a comment. In https://reviews.llvm.org/D26288#586932, @ioeric wrote: > - Addressed comments: handle non-existing files. We're not really handling them now though? We're just printing an error? My point is that we might run the replacement generation on a distributed system, and then group/deduplicate/apply them somewhere where the files might not actually exist (think the reduce stage of a mapreduce). If possible, I'd like to not rely on the existence of the file when we deal with Replacements. I'd find it especially problematic if the existence or non-existence of files changes the semantics of those operations. https://reviews.llvm.org/D26288 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26317: Fix use-of-temporary with StringRef in code coverage
vsk accepted this revision. vsk added a comment. This revision is now accepted and ready to land. Thanks, lgtm. I've made a note to clean this up. Repository: rL LLVM https://reviews.llvm.org/D26317 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26288: Deduplicate replacements by FileEntry instead of file names.
ioeric added a comment. In https://reviews.llvm.org/D26288#587513, @klimek wrote: > In https://reviews.llvm.org/D26288#586932, @ioeric wrote: > > > - Addressed comments: handle non-existing files. > > > We're not really handling them now though? We're just printing an error? > > My point is that we might run the replacement generation on a distributed > system, and then group/deduplicate/apply them somewhere where the files might > not actually exist (think the reduce stage of a mapreduce). If possible, I'd > like to not rely on the existence of the file when we deal with Replacements. > I'd find it especially problematic if the existence or non-existence of files > changes the semantics of those operations. This function is only used in replacements application phase, and I think this is the most easiest if not the best way to ensure that two sets of replacements for the same file are applied without needing to handle dots and relative/absolute paths. For distributed replacements generation, I think it makes more sense to duplications on the replacements application phase since this needs to be done when combining all replacements anyway. It might make more sense to make this function local in tooling/Refactoring.cpp. But in that case, I can't unit test it :( https://reviews.llvm.org/D26288 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26138: [clang-tidy] Add modernize-use-equals-delete check
malcolm.parsons updated this revision to Diff 76971. malcolm.parsons added a comment. Check for non-public deleted methods. Remove imperfect FixItHint. Add FIXME comments. https://reviews.llvm.org/D26138 Files: clang-tidy/modernize/CMakeLists.txt clang-tidy/modernize/ModernizeTidyModule.cpp clang-tidy/modernize/UseEqualsDeleteCheck.cpp clang-tidy/modernize/UseEqualsDeleteCheck.h docs/ReleaseNotes.rst docs/clang-tidy/checks/list.rst docs/clang-tidy/checks/modernize-use-default.rst docs/clang-tidy/checks/modernize-use-equals-delete.rst test/clang-tidy/modernize-use-equals-delete.cpp Index: test/clang-tidy/modernize-use-equals-delete.cpp === --- /dev/null +++ test/clang-tidy/modernize-use-equals-delete.cpp @@ -0,0 +1,134 @@ +// RUN: %check_clang_tidy %s modernize-use-equals-delete %t + +struct PositivePrivate { +private: + PositivePrivate(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= delete' to prohibit calling of a special member function [modernize-use-equals-delete] + // CHECK-FIXES: PositivePrivate() = delete; + PositivePrivate(const PositivePrivate &); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= delete' to prohibit calling of a special member function [modernize-use-equals-delete] + // CHECK-FIXES: PositivePrivate(const PositivePrivate &) = delete; + PositivePrivate &operator=(const PositivePrivate &); + // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: use '= delete' to prohibit calling of a special member function [modernize-use-equals-delete] + // CHECK-FIXES: PositivePrivate &operator=(const PositivePrivate &) = delete; + PositivePrivate(PositivePrivate &&); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= delete' to prohibit calling of a special member function [modernize-use-equals-delete] + // CHECK-FIXES: PositivePrivate(PositivePrivate &&) = delete; + PositivePrivate &operator=(PositivePrivate &&); + // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: use '= delete' to prohibit calling of a special member function [modernize-use-equals-delete] + // CHECK-FIXES: PositivePrivate &operator=(PositivePrivate &&) = delete; + ~PositivePrivate(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= delete' to prohibit calling of a special member function [modernize-use-equals-delete] + // CHECK-FIXES: ~PositivePrivate() = delete; +}; + +struct NegativePublic { + NegativePublic(const NegativePublic &); +}; + +struct NegativeProtected { +protected: + NegativeProtected(const NegativeProtected &); +}; + +struct PositiveInlineMember { + int foo() { return 0; } + +private: + PositiveInlineMember(const PositiveInlineMember &); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= delete' to prohibit calling of a special member function [modernize-use-equals-delete] + // CHECK-FIXES: PositiveInlineMember(const PositiveInlineMember &) = delete; +}; + +struct PositiveOutOfLineMember { + int foo(); + +private: + PositiveOutOfLineMember(const PositiveOutOfLineMember &); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= delete' to prohibit calling of a special member function [modernize-use-equals-delete] + // CHECK-FIXES: PositiveOutOfLineMember(const PositiveOutOfLineMember &) = delete; +}; + +int PositiveOutOfLineMember::foo() { return 0; } + +struct PositiveAbstractMember { + virtual int foo() = 0; + +private: + PositiveAbstractMember(const PositiveAbstractMember &); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= delete' to prohibit calling of a special member function [modernize-use-equals-delete] + // CHECK-FIXES: PositiveAbstractMember(const PositiveAbstractMember &) = delete; +}; + +struct NegativeMemberNotImpl { + int foo(); + +private: + NegativeMemberNotImpl(const NegativeMemberNotImpl &); +}; + +struct NegativeStaticMemberNotImpl { + static int foo(); + +private: + NegativeStaticMemberNotImpl(const NegativeStaticMemberNotImpl &); +}; + +struct NegativeInline { +private: + NegativeInline(const NegativeInline &) {} +}; + +struct NegativeOutOfLine { +private: + NegativeOutOfLine(const NegativeOutOfLine &); +}; + +NegativeOutOfLine::NegativeOutOfLine(const NegativeOutOfLine &) {} + +struct NegativeConstructNotImpl { + NegativeConstructNotImpl(); + +private: + NegativeConstructNotImpl(const NegativeConstructNotImpl &); +}; + +struct PositiveDefaultedConstruct { + PositiveDefaultedConstruct() = default; + +private: + PositiveDefaultedConstruct(const PositiveDefaultedConstruct &); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= delete' to prohibit calling of a special member function [modernize-use-equals-delete] + // CHECK-FIXES: PositiveDefaultedConstruct(const PositiveDefaultedConstruct &) = delete; +}; + +struct PositiveDeletedConstruct { + PositiveDeletedConstruct() = delete; + +private: + PositiveDeletedConstruct(const PositiveDeletedConstruct &); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= delete' to prohibit calling of a special member fun
[PATCH] D26288: Deduplicate replacements by FileEntry instead of file names.
klimek added a comment. Ok, makes sense. Can you add a comment to the function that explains what happens with replacements for files that do not exist (we ignore them, unless I'm mistaken). Otherwise LG. https://reviews.llvm.org/D26288 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26288: Deduplicate replacements by FileEntry instead of file names.
ioeric updated this revision to Diff 76975. ioeric added a comment. - addressed comments. https://reviews.llvm.org/D26288 Files: include/clang/Tooling/Core/Replacement.h lib/Tooling/Core/Replacement.cpp lib/Tooling/Refactoring.cpp unittests/Tooling/RefactoringTest.cpp Index: unittests/Tooling/RefactoringTest.cpp === --- unittests/Tooling/RefactoringTest.cpp +++ unittests/Tooling/RefactoringTest.cpp @@ -19,6 +19,7 @@ #include "clang/Basic/FileManager.h" #include "clang/Basic/LangOptions.h" #include "clang/Basic/SourceManager.h" +#include "clang/Basic/VirtualFileSystem.h" #include "clang/Format/Format.h" #include "clang/Frontend/CompilerInstance.h" #include "clang/Frontend/FrontendAction.h" @@ -972,40 +973,64 @@ toReplacements({{"", 0, 3, "cc"}, {"", 3, 3, "dd"}})); } -TEST(DeduplicateByFileTest, LeaveLeadingDotDot) { +TEST(DeduplicateByFileTest, PathsWithDots) { std::map FileToReplaces; + llvm::IntrusiveRefCntPtr VFS( + new vfs::InMemoryFileSystem()); + FileManager *FileMgr = new FileManager(FileSystemOptions(), VFS); #if !defined(LLVM_ON_WIN32) - FileToReplaces["../../a/b/.././c.h"] = Replacements(); - FileToReplaces["../../a/c.h"] = Replacements(); + StringRef Path1 = "a/b/.././c.h"; + StringRef Path2 = "a/c.h"; #else - FileToReplaces["..\\..\\a\\b\\..\\.\\c.h"] = Replacements(); - FileToReplaces["..\\..\\a\\c.h"] = Replacements(); + StringRef Path1 = "a\\b\\..\\.\\c.h"; + StringRef Path2 = "a\\c.h"; #endif - FileToReplaces = groupReplacementsByFile(FileToReplaces); + EXPECT_TRUE(VFS->addFile(Path1, 0, llvm::MemoryBuffer::getMemBuffer(""))); + EXPECT_TRUE(VFS->addFile(Path2, 0, llvm::MemoryBuffer::getMemBuffer(""))); + FileToReplaces[Path1] = Replacements(); + FileToReplaces[Path2] = Replacements(); + FileToReplaces = groupReplacementsByFile(*FileMgr, FileToReplaces); EXPECT_EQ(1u, FileToReplaces.size()); -#if !defined(LLVM_ON_WIN32) - EXPECT_EQ("../../a/c.h", FileToReplaces.begin()->first); -#else - EXPECT_EQ("..\\..\\a\\c.h", FileToReplaces.begin()->first); -#endif + EXPECT_EQ(Path1, FileToReplaces.begin()->first); } -TEST(DeduplicateByFileTest, RemoveDotSlash) { +TEST(DeduplicateByFileTest, PathWithDotSlash) { std::map FileToReplaces; + llvm::IntrusiveRefCntPtr VFS( + new vfs::InMemoryFileSystem()); + FileManager *FileMgr = new FileManager(FileSystemOptions(), VFS); #if !defined(LLVM_ON_WIN32) - FileToReplaces["./a/b/.././c.h"] = Replacements(); - FileToReplaces["a/c.h"] = Replacements(); + StringRef Path1 = "./a/b/c.h"; + StringRef Path2 = "a/b/c.h"; #else - FileToReplaces[".\\a\\b\\..\\.\\c.h"] = Replacements(); - FileToReplaces["a\\c.h"] = Replacements(); + StringRef Path1 = ".\\a\\b\\c.h"; + StringRef Path2 = "a\\b\\c.h"; #endif - FileToReplaces = groupReplacementsByFile(FileToReplaces); + EXPECT_TRUE(VFS->addFile(Path1, 0, llvm::MemoryBuffer::getMemBuffer(""))); + EXPECT_TRUE(VFS->addFile(Path2, 0, llvm::MemoryBuffer::getMemBuffer(""))); + FileToReplaces[Path1] = Replacements(); + FileToReplaces[Path2] = Replacements(); + FileToReplaces = groupReplacementsByFile(*FileMgr, FileToReplaces); EXPECT_EQ(1u, FileToReplaces.size()); + EXPECT_EQ(Path1, FileToReplaces.begin()->first); +} + +TEST(DeduplicateByFileTest, NonExistingFilePath) { + std::map FileToReplaces; + llvm::IntrusiveRefCntPtr VFS( + new vfs::InMemoryFileSystem()); + FileManager *FileMgr = new FileManager(FileSystemOptions(), VFS); #if !defined(LLVM_ON_WIN32) - EXPECT_EQ("a/c.h", FileToReplaces.begin()->first); + StringRef Path1 = "./a/b/c.h"; + StringRef Path2 = "a/b/c.h"; #else - EXPECT_EQ("a\\c.h", FileToReplaces.begin()->first); + StringRef Path1 = ".\\a\\b\\c.h"; + StringRef Path2 = "a\\b\\c.h"; #endif + FileToReplaces[Path1] = Replacements(); + FileToReplaces[Path2] = Replacements(); + FileToReplaces = groupReplacementsByFile(*FileMgr, FileToReplaces); + EXPECT_TRUE(FileToReplaces.empty()); } } // end namespace tooling Index: lib/Tooling/Refactoring.cpp === --- lib/Tooling/Refactoring.cpp +++ lib/Tooling/Refactoring.cpp @@ -57,7 +57,8 @@ bool RefactoringTool::applyAllReplacements(Rewriter &Rewrite) { bool Result = true; - for (const auto &Entry : groupReplacementsByFile(FileToReplaces)) + for (const auto &Entry : groupReplacementsByFile( + Rewrite.getSourceMgr().getFileManager(), FileToReplaces)) Result = tooling::applyAllReplacements(Entry.second, Rewrite) && Result; return Result; } @@ -73,7 +74,8 @@ FileManager &Files = SM.getFileManager(); bool Result = true; - for (const auto &FileAndReplaces : groupReplacementsByFile(FileToReplaces)) { + for (const auto &FileAndReplaces : groupReplacementsByFile( + Rewrite.getSourceMgr().getFileManager(), FileToReplaces)) { const std::string &FilePath = FileAndReplaces.first; auto &CurReplaces
[PATCH] D6833: Clang-format: Braces Indent Style Whitesmith
djasper added a comment. In case you still want to move forward with this, some more comments. Why have you opted for doing this in getNewLineColumn instead of the UnwrappedLineFormatter::formatFirstToken() function I suggested? I think the latter would be way easier, but there might be cases I am not thinking of right now. Comment at: docs/ClangFormatStyleOptions.rst:260 + * ``BS_Whitesmiths`` (in configuration: ``Whitesmiths``) +Like ``Allman``, with the braces intended too. * ``BS_GNU`` (in configuration: ``GNU``) indented? Comment at: lib/Format/ContinuationIndenter.h:270 + bool BlockNoExtraIndent; + bool operator<(const ParenState &Other) const { No. I don't think it is necessary to add an extra value to the indent state for this and doing so would be costly in runtime. https://reviews.llvm.org/D6833 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26139: Tests for strings conversions under libcpp-no-exceptions
mclow.lists added a comment. > I think it might be better to add TEST_TRY and TEST_CATCH(...) macros > defined like @rogfer01 said at the top that he didn't want to add "a magical TEST_TRY macro" - and I agree. Someone tried that in another review, and I nixed it there. https://reviews.llvm.org/D26139 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits