Thank you!
On Fri, Oct 2, 2015 at 5:54 PM, NAKAMURA Takumi
wrote:
> chapuni added a subscriber: chapuni.
>
>
> Comment at: test/CodeGenCXX/strict-vtable-pointers.cpp:145
> @@ +144,3 @@
> +// CHECK-CTORS: %[[THIS3:.*]] = bitcast i8* %[[THIS2]] to
> %[[DynamicDerived]]*
> +// CHEC
Prazek marked 9 inline comments as done.
Prazek added a comment.
http://reviews.llvm.org/D11859
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Prazek added inline comments.
Comment at: lib/CodeGen/CGClass.cpp:1842
@@ +1841,3 @@
+ CGM.getCXXABI().getVTableAddressPoint(vptr.Base, vptr.VTableClass);
+ if (!VTableGlobal) return;
+
djasper wrote:
> Prazek wrote:
> > majnemer wrote:
> > > The return shou
Prazek added inline comments.
Comment at: lib/CodeGen/CodeGenFunction.h:1328-1334
@@ -1320,8 +1327,9 @@
typedef llvm::SmallPtrSet VisitedVirtualBasesSetTy;
- void InitializeVTablePointers(BaseSubobject Base,
-const CXXRecordDecl *NearestVBase,
-
Prazek updated this revision to Diff 31766.
http://reviews.llvm.org/D11859
Files:
lib/CodeGen/CGCXXABI.h
lib/CodeGen/CGClass.cpp
lib/CodeGen/CodeGenFunction.h
lib/CodeGen/ItaniumCXXABI.cpp
lib/CodeGen/MicrosoftCXXABI.cpp
test/CodeGen/available-externally-hidden.cpp
test/CodeGenCXX/c
Prazek created this revision.
Prazek added reviewers: rsmith, majnemer.
Prazek added a subscriber: cfe-commits.
http://reviews.llvm.org/D11928
Files:
include/clang/AST/VTableBuilder.h
Index: include/clang/AST/VTableBuilder.h
===
-
Prazek updated this revision to Diff 31768.
http://reviews.llvm.org/D11859
Files:
lib/CodeGen/CGCXXABI.h
lib/CodeGen/CGClass.cpp
lib/CodeGen/CodeGenFunction.h
lib/CodeGen/ItaniumCXXABI.cpp
lib/CodeGen/MicrosoftCXXABI.cpp
test/CodeGen/available-externally-hidden.cpp
test/CodeGenCXX/c
Prazek marked 4 inline comments as done.
Comment at: lib/CodeGen/ItaniumCXXABI.cpp:193-194
@@ -192,1 +192,4 @@
+ bool isVirtualOffsetNeeded(CodeGenFunction &CGF,
+ const CXXRecordDecl *NearestVBase) override;
+
isVirtualOffsetNeededF
Prazek marked 3 inline comments as done.
Prazek added a comment.
http://reviews.llvm.org/D11859
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Prazek updated this revision to Diff 31846.
http://reviews.llvm.org/D11859
Files:
lib/CodeGen/CGCXXABI.h
lib/CodeGen/CGClass.cpp
lib/CodeGen/CodeGenFunction.h
lib/CodeGen/ItaniumCXXABI.cpp
lib/CodeGen/MicrosoftCXXABI.cpp
test/CodeGen/available-externally-hidden.cpp
test/CodeGenCXX/c
Prazek marked 4 inline comments as done.
Prazek added a comment.
http://reviews.llvm.org/D11859
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Prazek updated this revision to Diff 31863.
http://reviews.llvm.org/D11928
Files:
include/clang/AST/VTableBuilder.h
Index: include/clang/AST/VTableBuilder.h
===
--- include/clang/AST/VTableBuilder.h
+++ include/clang/AST/VTableBui
Prazek added inline comments.
Comment at: include/clang/AST/VTableBuilder.h:54
@@ -53,3 +53,3 @@
- VTableComponent() { }
+ VTableComponent() = default;
rnk wrote:
> Is this ctor used? It leaves Value uninitialized. Maybe we should delete it
> to ensure that
Prazek retitled this revision from "Small fixup" to "Deleted old fixme.".
Prazek updated this revision to Diff 31891.
http://reviews.llvm.org/D11928
Files:
include/clang/AST/VTableBuilder.h
Index: include/clang/AST/VTableBuilder.h
===
Prazek added inline comments.
Comment at: lib/CodeGen/CGClass.cpp:1832
@@ +1831,3 @@
+ // Generate vtable assumptions if we are calling dynamic class ctor
+ // and we are not in another ctor.
+ if (CGM.getCodeGenOpts().OptimizationLevel > 0 &&
hfinkel wrote:
>
Prazek marked 5 inline comments as done.
Prazek added a comment.
http://reviews.llvm.org/D11859
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Prazek marked an inline comment as done.
Comment at: lib/CodeGen/CGClass.cpp:1832
@@ +1831,3 @@
+ // Generate vtable assumptions if we are calling dynamic class ctor
+ // and we are not in another ctor.
+ if (CGM.getCodeGenOpts().OptimizationLevel > 0 &&
rsmith
Prazek marked 3 inline comments as done.
Comment at: lib/CodeGen/CGClass.cpp:1832
@@ +1831,3 @@
+ // Generate vtable assumptions if we are calling dynamic class ctor
+ // and we are not in another ctor.
+ if (CGM.getCodeGenOpts().OptimizationLevel > 0 &&
rsmith
Prazek updated this revision to Diff 32122.
Prazek marked an inline comment as done.
http://reviews.llvm.org/D11859
Files:
lib/CodeGen/CGCXXABI.h
lib/CodeGen/CGClass.cpp
lib/CodeGen/CodeGenFunction.h
lib/CodeGen/ItaniumCXXABI.cpp
lib/CodeGen/MicrosoftCXXABI.cpp
test/CodeGen/available-
Prazek created this revision.
Prazek added reviewers: rsmith, majnemer, pcc.
Prazek added a subscriber: cfe-commits.
Adding !invariant.group to vptr load/stores for devirtualization purposes.
For more goto:
http://lists.llvm.org/pipermail/cfe-dev/2015-July/044227.htm
http://reviews.llvm.org/D120
Prazek added a comment.
In http://reviews.llvm.org/D11859#225025, @rjmccall wrote:
> Mostly LGTM. Are you going to emit assumptions for vbptrs in a separate
> patch?
I wasn't planning to. I am focusing now on upgrading GVN for using new
invariant.barrier metadata.
http://reviews.llvm.org/D
Prazek marked an inline comment as done.
Comment at: lib/CodeGen/CGCXXABI.h:352
@@ +351,3 @@
+ isVirtualOffsetNeededForVTableField(CodeGenFunction &CGF,
+ const CXXRecordDecl *NearestVBase) = 0;
+
rjmccall wrote:
> This method
Prazek marked an inline comment as done.
Prazek added a comment.
In http://reviews.llvm.org/D11859#225404, @rjmccall wrote:
> In http://reviews.llvm.org/D11859#225384, @Prazek wrote:
>
> > In http://reviews.llvm.org/D11859#225025, @rjmccall wrote:
> >
> > > Mostly LGTM. Are you going to emit ass
Prazek updated this revision to Diff 32262.
http://reviews.llvm.org/D11859
Files:
lib/CodeGen/CGCXXABI.h
lib/CodeGen/CGClass.cpp
lib/CodeGen/CodeGenFunction.h
lib/CodeGen/ItaniumCXXABI.cpp
lib/CodeGen/MicrosoftCXXABI.cpp
test/CodeGen/available-externally-hidden.cpp
test/CodeGenCXX/c
Prazek updated this revision to Diff 32263.
http://reviews.llvm.org/D11859
Files:
lib/CodeGen/CGCXXABI.h
lib/CodeGen/CGClass.cpp
lib/CodeGen/CodeGenFunction.h
lib/CodeGen/ItaniumCXXABI.cpp
lib/CodeGen/MicrosoftCXXABI.cpp
test/CodeGen/available-externally-hidden.cpp
test/CodeGenCXX/c
Prazek marked 3 inline comments as done.
Comment at: lib/CodeGen/CGClass.cpp:1862
@@ +1861,3 @@
+ for (const VPtr &Vptr : getVTablePointers(ClassDecl))
+if (CGM.getCXXABI().requiresVPtrInitialization(Vptr))
+ EmitVTableAssumptionLoad(Vptr, This);
rjmccal
Prazek updated this revision to Diff 32343.
Prazek marked an inline comment as done.
http://reviews.llvm.org/D11859
Files:
lib/CodeGen/CGCXXABI.h
lib/CodeGen/CGClass.cpp
lib/CodeGen/CodeGenFunction.h
lib/CodeGen/ItaniumCXXABI.cpp
lib/CodeGen/MicrosoftCXXABI.cpp
test/CodeGen/available-
Prazek marked 3 inline comments as done.
Prazek added a comment.
http://reviews.llvm.org/D11859
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Prazek updated this revision to Diff 32355.
Prazek marked 2 inline comments as done.
http://reviews.llvm.org/D11859
Files:
lib/CodeGen/CGCXXABI.h
lib/CodeGen/CGClass.cpp
lib/CodeGen/CodeGenFunction.h
lib/CodeGen/ItaniumCXXABI.cpp
lib/CodeGen/MicrosoftCXXABI.cpp
test/CodeGen/available-
Author: prazek
Date: Mon Aug 17 18:33:49 2015
New Revision: 245257
URL: http://llvm.org/viewvc/llvm-project?rev=245257&view=rev
Log:
Generating assumption loads of vptr after ctor call
Generating call assume(icmp %vtable, %global_vtable) after constructor
call for devirtualization purposes.
For
Prazek updated this revision to Diff 32363.
Prazek added a comment.
Fix for the PR24479
http://reviews.llvm.org/D11859
Files:
lib/CodeGen/CGCXXABI.h
lib/CodeGen/CGCall.cpp
lib/CodeGen/CGClass.cpp
lib/CodeGen/CodeGenFunction.h
lib/CodeGen/ItaniumCXXABI.cpp
lib/CodeGen/MicrosoftCXXABI
Author: prazek
Date: Mon Aug 17 22:52:00 2015
New Revision: 245264
URL: http://llvm.org/viewvc/llvm-project?rev=245264&view=rev
Log:
Generating assumption loads of vptr after ctor call (fixed)
Generating call assume(icmp %vtable, %global_vtable) after constructor
call for devirtualization purpose
Prazek added inline comments.
Comment at: lib/CodeGen/CodeGenModule.cpp:3677
@@ +3676,3 @@
+llvm::Metadata *CodeGenModule::CreateMetadataIdentifierForType(QualType T) {
+ llvm::Metadata *&InternalId = MetadataIdMap[T.getCanonicalType()];
+ if (InternalId)
questi
Thanks. I have no idea what happen, but I will fix it tomorrow.
Piotr
On Mon, Aug 17, 2015 at 10:43 PM, Justin Bogner
wrote:
> Piotr Padlewski via cfe-commits writes:
> > Author: prazek
> > Date: Mon Aug 17 22:52:00 2015
> > New Revision: 245264
> >
> >
Confirm, the assertions are different from the last one.
On Mon, Aug 17, 2015 at 11:45 PM, Justin Bogner via cfe-commits <
cfe-commits@lists.llvm.org> wrote:
> Richard Smith writes:
> > Are you sure it was this change and not r245265?
>
> Pretty sure - I ran the crash repro script with tot and i
Prazek created this revision.
Prazek added reviewers: rsmith, majnemer, rjmccall.
Prazek added a subscriber: cfe-commits.
Bugfix revealed in r245264.
http://reviews.llvm.org/D12128
Files:
include/clang/AST/VTableBuilder.h
lib/CodeGen/ItaniumCXXABI.cpp
Index: lib/CodeGen/ItaniumCXXABI.cpp
==
Prazek updated this revision to Diff 32509.
Prazek added a comment.
Added tests, and changed the VTableComponent code which was wrong on so many
levels. Sorry
http://reviews.llvm.org/D12128
Files:
include/clang/AST/VTableBuilder.h
lib/CodeGen/ItaniumCXXABI.cpp
test/CodeGenCXX/vtable-avai
Prazek updated this revision to Diff 32587.
http://reviews.llvm.org/D12128
Files:
include/clang/AST/VTableBuilder.h
lib/CodeGen/ItaniumCXXABI.cpp
test/CodeGenCXX/vtable-available-externally.cpp
Index: test/CodeGenCXX/vtable-available-externally.cpp
=
Author: prazek
Date: Wed Aug 19 15:09:09 2015
New Revision: 245489
URL: http://llvm.org/viewvc/llvm-project?rev=245489&view=rev
Log:
Generating available_externally vtables bugfix
Bugfix revealed in r245264.
http://reviews.llvm.org/D12128
Modified:
cfe/trunk/include/clang/AST/VTableBuilder.
Author: prazek
Date: Fri Aug 21 13:28:00 2015
New Revision: 245721
URL: http://llvm.org/viewvc/llvm-project?rev=245721&view=rev
Log:
Generating assumption loads of vptr after ctor call (fixed)
Generating call assume(icmp %vtable, %global_vtable) after constructor
call for devirtualization purpose
Author: prazek
Date: Fri Aug 21 14:49:41 2015
New Revision: 245727
URL: http://llvm.org/viewvc/llvm-project?rev=245727&view=rev
Log:
Revert "Generating assumption loads of vptr after ctor call (fixed)"
Reverting because of 245721
This reverts commit 552658e2b60543c928030b09cc9b5dfcb40c3f28.
Remo
Prazek created this revision.
Prazek added reviewers: rsmith, rjmccall, majnemer, nlewycky.
Prazek added a subscriber: cfe-commits.
Keep in mind, that I will not push it untill I will update GVN for using
!invariant.group metadata
http://reviews.llvm.org/D12312
Files:
include/clang/Driver/Opt
Prazek added a comment.
@rsmith I wonder If we have to put invariant.group.barrier when we call regular
new expression
http://reviews.llvm.org/D12312
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/list
Prazek created this revision.
Prazek added reviewers: rsmith, majnemer, rjmccall.
Prazek added a subscriber: cfe-commits.
It wasn't always safe to generate assumption loads. Last time build failed on
linking of classes like FenceInst because
it didn't introduce any new virtual function, and it ha
Prazek marked 3 inline comments as done.
Prazek added a comment.
http://reviews.llvm.org/D12385
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Prazek updated this revision to Diff 33269.
http://reviews.llvm.org/D12385
Files:
lib/CodeGen/CGCXXABI.h
lib/CodeGen/CGClass.cpp
lib/CodeGen/CGVTables.cpp
lib/CodeGen/ItaniumCXXABI.cpp
lib/CodeGen/MicrosoftCXXABI.cpp
test/CodeGenCXX/template-instantiation.cpp
test/CodeGenCXX/thunks.
Prazek added a comment.
I will wait with pushing for tomorrow noon, in case someone want's to check the
code.
http://reviews.llvm.org/D12385
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-
Prazek marked 2 inline comments as done.
Comment at: lib/CodeGen/CGClass.cpp:1861
@@ +1860,3 @@
+ // We also have to make sure if we can refer to vtable:
+ // - If vtable is external then it's safe to use it (for availabe_externally
+ // CGVTables will make sure if it can emi
Prazek updated this revision to Diff 33273.
Prazek marked an inline comment as done.
http://reviews.llvm.org/D12385
Files:
lib/CodeGen/CGCXXABI.h
lib/CodeGen/CGClass.cpp
lib/CodeGen/CGVTables.cpp
lib/CodeGen/ItaniumCXXABI.cpp
lib/CodeGen/MicrosoftCXXABI.cpp
test/CodeGenCXX/template-in
Prazek marked an inline comment as done.
Prazek added a comment.
http://reviews.llvm.org/D12385
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Author: prazek
Date: Thu Aug 27 16:35:37 2015
New Revision: 246213
URL: http://llvm.org/viewvc/llvm-project?rev=246213&view=rev
Log:
Generating assumption loads of vptr after ctor call (fixed)
Generating call assume(icmp %vtable, %global_vtable) after constructor
call for devirtualization purpose
Author: prazek
Date: Thu Aug 27 16:35:41 2015
New Revision: 246214
URL: http://llvm.org/viewvc/llvm-project?rev=246214&view=rev
Log:
Assume loads fix #2
There was linker problem, and it turns out that it is not always safe
to refer to vtable. If the vtable is used, then we can refer to it
without
Prazek added inline comments.
Comment at: lib/CodeGen/CGClass.cpp:1279
@@ +1278,3 @@
+ if (CGM.getCodeGenOpts().StrictVPtrs && BaseVPtrsInitialized)
+CXXThisValue = Builder.CreateInvariantGroupBarrier(LoadCXXThis());
+
rjmccall wrote:
> Should this just be in
Prazek added inline comments.
Comment at: include/clang/Driver/Options.td:990
@@ -988,2 +989,3 @@
"value range">;
+def fstrict_vptrs: Flag<["-"], "fstrict-vptrs">, Group,
Flags<[CC1Option]>;
def fstrict_overflow : Flag<["-"], "fstrict-overflow">, Group;
-
Prazek marked 3 inline comments as done.
Comment at: lib/CodeGen/CGClass.cpp:1279
@@ +1278,3 @@
+ if (CGM.getCodeGenOpts().StrictVPtrs && BaseVPtrsInitialized)
+CXXThisValue = Builder.CreateInvariantGroupBarrier(LoadCXXThis());
+
rjmccall wrote:
> Prazek wrot
Prazek updated the summary for this revision.
Prazek updated this revision to Diff 33775.
http://reviews.llvm.org/D12312
Files:
include/clang/Driver/Options.td
include/clang/Frontend/CodeGenOptions.def
lib/CodeGen/CGClass.cpp
lib/CodeGen/CGExprCXX.cpp
lib/Driver/Tools.cpp
lib/Frontend
Prazek created this revision.
Prazek added reviewers: rsmith, rjmccall, majnemer, nlewycky.
Prazek added a subscriber: cfe-commits.
Generates code like this
!llvm.module.flags = !{!0, !1}
!0 = !{i32 1, !"StrictVTablePointers", i32 1}
!1 = !{i32 3, !"StrictVTablePointersRequirement", !2}
!2 = !{!
Prazek added a comment.
I see that CGObjCMac.cpp names it like this:
"Objective-C Garbage Collection"
http://reviews.llvm.org/D12580
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Prazek updated this revision to Diff 33969.
http://reviews.llvm.org/D12580
Files:
lib/CodeGen/CodeGenModule.cpp
test/CodeGenCXX/strict-vtable-pointers.cpp
Index: test/CodeGenCXX/strict-vtable-pointers.cpp
===
--- test/CodeGenCXX
Prazek created this revision.
Prazek added reviewers: rsmith, majnemer.
Prazek added a subscriber: cfe-commits.
Generating vptr assumption loads for better devirtualization.
More here: http://lists.llvm.org/pipermail/cfe-dev/2015-July/044227.html
http://reviews.llvm.org/D11859
Files:
include/
Prazek added inline comments.
Comment at: lib/CodeGen/CGClass.cpp:1842
@@ +1841,3 @@
+ CGM.getCXXABI().getVTableAddressPoint(vptr.Base, vptr.VTableClass);
+ if (!VTableGlobal) return;
+
majnemer wrote:
> The return should be on it's own line, please clang-fo
Prazek added a subscriber: Prazek.
Comment at: clang-tidy/cppcoreguidelines/RuleOfFiveAndZeroCheck.cpp:73-77
@@ +72,7 @@
+
+ checkRuleOfFiveViolation(Result, "dtor", "destructor");
+ checkRuleOfFiveViolation(Result, "copy-ctor", "copy constructor");
+ checkRuleOfFiveViolation(R
Prazek marked 5 inline comments as done.
Prazek added a comment.
Aaron, Alex thanks for the review. After running it on llvm I also have found
the private constructor bug so I also fixed it.
Comment at: clang-tidy/modernize/UseEmplaceCheck.cpp:21
@@ +20,3 @@
+llvm::Optional>
+g
Prazek updated this revision to Diff 64640.
Prazek marked an inline comment as done.
Repository:
rL LLVM
https://reviews.llvm.org/D22208
Files:
clang-tidy/modernize/UseEmplaceCheck.cpp
clang-tidy/modernize/UseEmplaceCheck.h
clang-tidy/utils/Matchers.h
docs/clang-tidy/checks/modernize-u
Prazek added a subscriber: Prazek.
Comment at: clang-tidy/cppcoreguidelines/SlicingCheck.cpp:30
@@ +29,3 @@
+ // or
+ // - B does not define any additional members (either variables or
+ // overrides) wrt A.
What is A and what is B? I guess you are missin
Prazek added inline comments.
Comment at: clang-tidy/modernize/UseEmplaceCheck.h:36-37
@@ -32,1 +35,4 @@
+private:
+ std::vector ContainersWithPushBack;
+ std::vector SmartPointers;
};
aaron.ballman wrote:
> Why not use a SmallVector for these instead of a std:
Prazek added inline comments.
Comment at: clang-tidy/modernize/UseEmplaceCheck.cpp:45
@@ -33,3 +44,3 @@
hasDeclaration(functionDecl(hasName("push_back"))),
- on(hasType(cxxRecordDecl(hasAnyName("std::vector", "llvm::SmallVector",
-
Prazek added inline comments.
Comment at: clang-tidy/modernize/UseEmplaceCheck.cpp:112
@@ -95,1 +111,3 @@
+ auto CtorCallSourceRange = CharSourceRange::getTokenRange(
+ InnerCtorCall->getExprLoc(), CallParensRange.getBegin());
alexfh wrote:
> > There is a
Prazek added a comment.
There is one bug left:
In the ClangIncludeFixer.cpp:169 there is push back that looks like this
Symbols.push_back(find_all_symbols::SymbolInfo(
Split.first.trim(),
find_all_symbols::SymbolInfo::SymbolKind::Unknown,
CommaSplits[I].trim(), 1, {}, /*NumOccurrences=*/E
Prazek added a subscriber: klimek.
Prazek added a comment.
Can you look at my previous comment? I nedd an expert for AST.
Repository:
rL LLVM
https://reviews.llvm.org/D22208
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llv
Prazek removed rL LLVM as the repository for this revision.
Prazek updated this revision to Diff 65023.
Prazek marked 4 inline comments as done.
https://reviews.llvm.org/D22208
Files:
clang-tidy/modernize/UseEmplaceCheck.cpp
clang-tidy/modernize/UseEmplaceCheck.h
clang-tidy/utils/Matchers.h
Prazek marked 6 inline comments as done.
Prazek added a comment.
Repository:
rL LLVM
https://reviews.llvm.org/D22208
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Prazek added a comment.
Is it in upstream yet?
Comment at: clang-tidy/cppcoreguidelines/SlicingCheck.h:19
@@ +18,3 @@
+
+/// Flags slicing of member variables or vtable. See:
+/// -
courbet wrote:
> Prazek wrote:
> > some short description what does this check
Prazek marked 3 inline comments as done.
Comment at: test/clang-tidy/boost-use-to-string.cpp:154
@@ +153,3 @@
+ float floating;
+ Fields* wierd;
+ const int &getConstInteger() const {return integer;}
alexfh wrote:
> alexfh wrote:
> > "wierd" is weird ;)
> I sho
Prazek added a comment.
I will look again soon, but it looks much better right now!
Comment at: clang-tidy/cppcoreguidelines/RuleOfFiveAndZeroCheck.cpp:43-57
@@ +42,17 @@
+
+llvm::StringRef RuleOfFiveAndZeroCheck::toString(
+RuleOfFiveAndZeroCheck::SpecialMemberFunctionKind
Prazek added a comment.
In https://reviews.llvm.org/D22513#493303, @Eugene.Zelenko wrote:
> Since rule name is different in C++98/03 and C++11 or newer it will make
> sense to create two checks which will work only for their respective versions
> (of course, implementation should be shared). Or
Prazek added a subscriber: Prazek.
Prazek added a comment.
Thanks for the contribution. Is it your first check?
Some main issues:
1. I think it would be much better to move this check to modernize module. I
think the name 'modernize-use-copy' would follow the convention, or just
'modernize-rep
Prazek added a comment.
In https://reviews.llvm.org/D22725#493942, @JDevlieghere wrote:
> In https://reviews.llvm.org/D22725#493940, @aaron.ballman wrote:
>
> > > Using std::copy opens up the possibility of type-aware optimizations
> > > which are not possible with memcpy.
> >
> >
> > To my know
Prazek removed rL LLVM as the repository for this revision.
Prazek updated this revision to Diff 65243.
Prazek added a comment.
Ok works right now. I don't know why but I could not reproduce the error in
test file, but I manged to fix it.
https://reviews.llvm.org/D22208
Files:
clang-tidy/mod
Prazek added inline comments.
Comment at: clang-tidy/modernize/UseCopyCheck.cpp:49
@@ +48,3 @@
+ if (getLangOpts().CPlusPlus) {
+Inserter.reset(new utils::IncludeInserter(
+Compiler.getSourceManager(), Compiler.getLangOpts(), IncludeStyle));
there is
Prazek added a comment.
In https://reviews.llvm.org/D22725#493970, @JDevlieghere wrote:
> In https://reviews.llvm.org/D22725#493947, @Prazek wrote:
>
> > Thanks for the contribution. Is it your first check?
>
>
> Yes, it is! :-)
Cool! hope to see some other cool checks. You could probably use p
Prazek added a comment.
hmm It seems that I mislead you, I suck at C api - memmove source and
destination can overlap, but std::move can't. So I guess you have to remove the
memmove support. Sorry.
https://reviews.llvm.org/D22725
___
cfe-commits m
Prazek added a comment.
In https://reviews.llvm.org/D22725#494168, @JDevlieghere wrote:
> In https://reviews.llvm.org/D22725#494167, @Prazek wrote:
>
> > hmm It seems that I mislead you, I suck at C api - memmove source and
> > destination can overlap, but std::move can't. So I guess you have to
Prazek added a subscriber: Prazek.
Comment at: clang-tidy/misc/EnumMisuseCheck.cpp:19
@@ +18,3 @@
+
+// Stores a min and a max value which describe an interval.
+struct ValueRange {
s/\/\//\/\/\/
In other words, change // to /// (doxygen comment)
===
Prazek added a comment.
In https://reviews.llvm.org/D22725#495329, @etienneb wrote:
> In https://reviews.llvm.org/D22725#494167, @Prazek wrote:
>
> > hmm It seems that I mislead you, I suck at C api - memmove source and
> > destination can overlap, but std::move can't. So I guess you have to rem
Prazek added inline comments.
Comment at: clang-tidy/modernize/UseEmplaceCheck.cpp:115
@@ -95,1 +114,3 @@
+ auto CtorCallSourceRange = CharSourceRange::getTokenRange(
+ InnerCtorCall->getExprLoc(), CallParensRange.getBegin());
alexfh wrote:
> This doesn't
We could also just add nothing() matcher, so debugging would be much
easier, just add anything() or nothing() matcher as extra argument.
The other pros of it is that new developers won't send the patches that
uses those variadic matchers with only one argument.
2016-07-26 16:02 GMT-07:00 Zac Hans
Is, but it is still a lot of typing and we are talking about debuging.
2016-07-27 3:40 GMT-07:00 Manuel Klimek :
> On Wed, Jul 27, 2016 at 1:06 AM Piotr Padlewski via llvm-dev <
> llvm-...@lists.llvm.org> wrote:
>
>> We could also just add nothing() matcher, so debugging would be much
>> easier,
Author: prazek
Date: Thu Jul 28 21:10:23 2016
New Revision: 277097
URL: http://llvm.org/viewvc/llvm-project?rev=277097&view=rev
Log:
[clang-tidy] Fixes to modernize-use-emplace
Not everything is valid, but it should works for 99.8% cases
https://reviews.llvm.org/D22208
Modified:
clang-tools
Prazek added a comment.
I see you solved the void and conmpatible types problems. Excellent!
Can you post a patch with changes after running LLVM? I would wait for Alex or
Aaron to accept it.
Comment at: clang-tidy/modernize/UseAlgorithmCheck.cpp:58-60
@@ +57,5 @@
+
+ for (co
Prazek added a comment.
In https://reviews.llvm.org/D22725#502374, @JDevlieghere wrote:
> Addressed comments from Piotr Padlewski
>
> LLVM compiles with the latest version of this check, however some tests are
> failing. I'll investigate the cause of this and update this check if it can
> be pr
Prazek added inline comments.
Comment at: clang-tidy/modernize/UseEmplaceCheck.h:36-37
@@ -32,1 +35,4 @@
+private:
+ std::vector ContainersWithPushBack;
+ std::vector SmartPointers;
};
aaron.ballman wrote:
> What about `llvm::make_range()`?
llvm::make_range do
Prazek added inline comments.
Comment at: clang-tidy/modernize/UseEmplaceCheck.cpp:115
@@ -95,1 +114,3 @@
+ auto CtorCallSourceRange = CharSourceRange::getTokenRange(
+ InnerCtorCall->getExprLoc(), CallParensRange.getBegin());
alexfh wrote:
> Prazek wrote:
Prazek added a comment.
In https://reviews.llvm.org/D22208#503194, @alexfh wrote:
> Please add revision number (this can be automated, if include differential
> revision URL in your commit message as described in
> http://llvm.org/docs/Phabricator.html#committing-a-change).
I normally use arc
Prazek marked 5 inline comments as done.
Comment at: clang-tidy/modernize/UseEmplaceCheck.cpp:115
@@ -95,1 +114,3 @@
+ auto CtorCallSourceRange = CharSourceRange::getTokenRange(
+ InnerCtorCall->getExprLoc(), CallParensRange.getBegin());
alexfh wrote:
> Pr
Prazek added a subscriber: Prazek.
Prazek accepted this revision.
Prazek added a reviewer: Prazek.
Prazek added a comment.
This revision is now accepted and ready to land.
LGTM
https://reviews.llvm.org/D23008
___
cfe-commits mailing list
cfe-commits
Prazek added a subscriber: Prazek.
Prazek added a comment.
In https://reviews.llvm.org/D20196#504394, @bittnerbarni wrote:
> Thank you for all the assistance. Could you please do that?
btw obtaining commit access and commiting is very simple, so if you are
planning to send us some more cool pa
Prazek added a comment.
In https://reviews.llvm.org/D20196#504397, @bittnerbarni wrote:
> I'm planning to submit more patches in the future, as I have time for them.
> So it wouldn't be in vain :)
http://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access
https://reviews.llvm.org/D201
Prazek added a comment.
In https://reviews.llvm.org/D22725#505020, @JDevlieghere wrote:
> Addresses comments from Aaron Ballman
>
> @aaron.ballman Thanks for the thorough review! Can you check whether the
> tests I added address your concerns? Could you also elaborate on the case
> with the C-f
Prazek added a subscriber: Prazek.
Prazek added a comment.
LG(TM)
https://reviews.llvm.org/D23135
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
101 - 200 of 373 matches
Mail list logo
