Prazek added a comment.
Good job.
I think it is resonable to warn in cases like:
if (str.compare(str2) == 1)
or even
if(str.compare(str2) == -1)
Sometimes people check for 1 or -1 instead of > 0 and < 0. If I remember
corectly PVS studio found some bugs like this.
Comm
Prazek added inline comments.
Comment at: docs/clang-tidy/checks/list.rst:68
misc-inefficient-algorithm
+ misc-invalid-range
misc-macro-parentheses
malcolm.parsons wrote:
> Prazek wrote:
> > malcolm.parsons wrote:
> > > misc.
> > > add_new_check.py can
Prazek added a comment.
In https://reviews.llvm.org/D27815#624294, @Eugene.Zelenko wrote:
> I'm not sure that this is good name for module.
>
> Singe reason for this is check for STL algorithms, may be //stl// module is
> more correct destination?
The reason for this module is that I want to m
Prazek added a comment.
In https://reviews.llvm.org/D27815#624322, @Eugene.Zelenko wrote:
> There are many other checks or compiler warnings which could be classified as
> obvious bugs.
Sure, but unfortunatelly they won't be able to become warning because of false
positive rate.
In the examp
Prazek added a comment.
In https://reviews.llvm.org/D27806#624675, @Eugene.Zelenko wrote:
> It may be good idea to create LLVM check which will suggest to use
> STLExtras.h wrappers instead of STL algorithms.
Sure, I actually didn't know about STLExtras, but anyway it would be something
LLVM
This revision was automatically updated to reflect the committed changes.
Closed by commit rL289930: [clang-tidy] fix missing anchor for MPI Module
(authored by Prazek).
Changed prior to commit:
https://reviews.llvm.org/D27813?vs=81593&id=81723#toc
Repository:
rL LLVM
https://reviews.llvm.o
Prazek marked 2 inline comments as done.
Prazek added inline comments.
Comment at: clang-tidy/obvious/InvalidRangeCheck.cpp:20-36
+"std::for_each; std::find; std::find_if; std::find_end; "
+"std::find_first_of; std::adjacent_find; std::count; std::count_if;"
+"std::mi
Prazek added a comment.
In https://reviews.llvm.org/D27815#625102, @aaron.ballman wrote:
> I am really not keen on the name "obvious" for this module. What is obvious
> to one person is not always obvious to another. Also, if the checks are
> finding *obvious bugs*, then that suggests they shou
Prazek marked an inline comment as done.
Prazek added inline comments.
Comment at: clang-tidy/obvious/InvalidRangeCheck.cpp:20-36
+"std::for_each; std::find; std::find_if; std::find_end; "
+"std::find_first_of; std::adjacent_find; std::count; std::count_if;"
+"std::mi
Prazek added inline comments.
Comment at: clang-tidy/obvious/InvalidRangeCheck.cpp:20-36
+"std::for_each; std::find; std::find_if; std::find_end; "
+"std::find_first_of; std::adjacent_find; std::count; std::count_if;"
+"std::mismatch; std::equal; std::search; std::cop
Prazek added a comment.
Do you need some help with implementing the other fixit, or you just need some
extra time? It seems to be almost the same as your second fixit
Comment at: clang-tidy/misc/StringCompareCheck.cpp:69-70
+diag(Matched->getLocStart(),
+ "do not u
Prazek added a comment.
The example of this kind of check is here:
https://reviews.llvm.org/D27806
I am not sure if it make sense to put it as clang warning.
After a little bit of thinking I guess name "typos" would be better, because I
want to look for checks that are mostly typos (which are o
Prazek added inline comments.
Comment at: docs/LibASTMatchersReference.html:2442
};
-fieldDecl(isBitField())
+fieldDecl(hasBitWidth(2))
matches 'int a;' and 'int c;' but not 'int b;'.
Fix not connected to patch?
Comment at: docs/LibASTMat
This revision was automatically updated to reflect the committed changes.
Closed by commit rL290424: Use after move bug fixes (authored by Prazek).
Changed prior to commit:
https://reviews.llvm.org/D27752?vs=81369&id=82408#toc
Repository:
rL LLVM
https://reviews.llvm.org/D27752
Files:
cfe
Prazek added inline comments.
Comment at: clang-tidy/obvious/InvalidRangeCheck.cpp:62
+ auto CallsAlgorithm = hasDeclaration(
+ functionDecl(Names.size() > 0 ? hasAnyName(Names) : anything()));
+
alexfh wrote:
> Does this check make sense without the names
Prazek created this revision.
Prazek added a reviewer: rjmccall.
Prazek added a subscriber: cfe-commits.
Small refactor
https://reviews.llvm.org/D28310
Files:
include/clang/AST/VTableBuilder.h
lib/CodeGen/ItaniumCXXABI.cpp
Index: lib/CodeGen/ItaniumCXXABI.cpp
=
Prazek created this revision.
We need to emit barrier if the union field
is CXXRecordDecl because it might have vptrs. The testcode
was wrongly devirtualized. It also proves that having different
groups for different dynamic types is not sufficient.
https://reviews.llvm.org/D31830
Files:
lib
Prazek added a comment.
Yep, I am also a fan of "and","or" and "not". The other tokens doesn't make it
more readable imho
https://reviews.llvm.org/D31308
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/
Prazek added a comment.
ping
https://reviews.llvm.org/D31830
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Prazek created this revision.
As discussed here
http://lists.llvm.org/pipermail/llvm-dev/2017-January/109332.html
having different groups doesn't solve the problem entirly.
https://reviews.llvm.org/D32110
Files:
lib/CodeGen/CodeGenModule.cpp
test/CodeGenCXX/invariant.group-for-vptrs.cpp
I
Prazek added a comment.
ping
https://reviews.llvm.org/D31830
___
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/CGExpr.cpp:3517
+CGM.getCodeGenOpts().StrictVTablePointers &&
+CGM.getCodeGenOpts().OptimizationLevel > 0)
+ addr = Address(Builder.CreateInvariantGroupBarrier(addr.getPointer()),
rjmccall
Prazek added inline comments.
Comment at: lib/CodeGen/CGExpr.cpp:3517
+CGM.getCodeGenOpts().StrictVTablePointers &&
+CGM.getCodeGenOpts().OptimizationLevel > 0)
+ addr = Address(Builder.CreateInvariantGroupBarrier(addr.getPointer()),
rjmccall
Prazek added inline comments.
Comment at: lib/CodeGen/CGExpr.cpp:3517
+CGM.getCodeGenOpts().StrictVTablePointers &&
+CGM.getCodeGenOpts().OptimizationLevel > 0)
+ addr = Address(Builder.CreateInvariantGroupBarrier(addr.getPointer()),
rjmccall
Prazek created this revision.
Prazek added a reviewer: hans.
Prazek added a subscriber: cfe-commits.
https://reviews.llvm.org/D28606
Files:
docs/ReleaseNotes.rst
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs
Prazek created this revision.
Prazek added a reviewer: hans.
Prazek added a subscriber: cfe-commits.
It would be good to merge it to 4.0 branch.
https://reviews.llvm.org/D28727
Files:
docs/UsersManual.rst
Index: docs/UsersManual.rst
==
Prazek created this revision.
Prazek added a reviewer: alexfh.
Prazek added a subscriber: cfe-commits.
Herald added a subscriber: JDevlieghere.
https://reviews.llvm.org/D28729
Files:
clang-tidy/ClangTidy.cpp
clang-tidy/ClangTidyOptions.cpp
clang-tidy/ClangTidyOptions.h
clang-tidy/tool/Cla
Prazek updated this revision to Diff 84448.
Prazek added a comment.
reformat
https://reviews.llvm.org/D28729
Files:
clang-tidy/ClangTidy.cpp
clang-tidy/ClangTidyOptions.cpp
clang-tidy/ClangTidyOptions.h
clang-tidy/tool/ClangTidyMain.cpp
test/clang-tidy/enable-alpha-checks.cpp
Index:
Prazek added a comment.
So the problem I got is that every time I want to check if there is already a
feature in clang-tidy/static-analyzer that solves my issue, I either have to
deal with static-analyzer command line, which is horrible, or I have to modify
and recompile the source code.
The c
Prazek updated this revision to Diff 84489.
Prazek added a comment.
suggested fix
https://reviews.llvm.org/D28727
Files:
docs/UsersManual.rst
Index: docs/UsersManual.rst
===
--- docs/UsersManual.rst
+++ docs/UsersManual.rst
@@
Prazek updated this revision to Diff 84491.
Prazek added a comment.
Added link to documentation
https://reviews.llvm.org/D28606
Files:
docs/ReleaseNotes.rst
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/Re
Prazek updated this revision to Diff 84492.
Prazek added a comment.
Removed [no-] so hyperlink would work
https://reviews.llvm.org/D28727
Files:
docs/UsersManual.rst
Index: docs/UsersManual.rst
===
--- docs/UsersManual.rst
+++
Prazek created this revision.
Prazek added reviewers: tejohnson, mehdi_amini, hans.
Prazek added a subscriber: cfe-commits.
It would be good to also mention
other improvements in ThinLTO for 4.0 release
https://reviews.llvm.org/D28746
Files:
docs/ReleaseNotes.rst
Index: docs/ReleaseNotes.rs
Prazek added a comment.
Maybe I should change ThinLTO for '-flto=thin' and also mention this
improvement in LLVM release notes?
BTW '-flto=thin' is not documented in UsersManual
https://reviews.llvm.org/D28746
___
cfe-commits mailing list
cfe-comm
Prazek updated this revision to Diff 84503.
Prazek added a comment.
update
https://reviews.llvm.org/D28746
Files:
docs/ReleaseNotes.rst
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -63
Prazek marked 2 inline comments as done.
Prazek added a comment.
Cool, This still might require some small fixes - I haven't generate docs for
it, so I will check it before release.
https://reviews.llvm.org/D28746
___
cfe-commits mailing list
cfe-c
Prazek updated this revision to Diff 84504.
Prazek marked an inline comment as done.
Prazek added a comment.
(PGO)
https://reviews.llvm.org/D28746
Files:
docs/ReleaseNotes.rst
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNo
Prazek added a comment.
Shoud I add the same note to LLVM Release notes?
https://reviews.llvm.org/D28746
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Prazek added a comment.
In https://reviews.llvm.org/D28729#646758, @aaron.ballman wrote:
> In https://reviews.llvm.org/D28729#646560, @alexfh wrote:
>
> > In https://reviews.llvm.org/D28729#646555, @aaron.ballman wrote:
> >
> > > In https://reviews.llvm.org/D28729#646548, @alexfh wrote:
> > >
> >
Prazek added a comment.
In https://reviews.llvm.org/D28746#646875, @mehdi_amini wrote:
> Oh I didn't notice that we were in the clang Release Notes, actually I've
> only updated the LLVM ones in the past.
>
> So yes, I think this should be in LLVM as well, I don't know if it should
> differ in
Prazek updated this revision to Diff 84550.
Prazek marked an inline comment as done.
Prazek added a comment.
-enable-alpha-checks is now not visible for users, but it make
my life as clang-tidy developer much easier.
https://reviews.llvm.org/D28729
Files:
clang-tidy/ClangTidy.cpp
clang-tid
Prazek added a comment.
Does solution like this works for you? We don't officially support alpha
checkers, but it is much easier to check if something is already implemented in
static analyzer easily
https://reviews.llvm.org/D28729
___
cfe-commits
Prazek closed this revision.
Prazek added a comment.
Pushed on branch
https://reviews.llvm.org/D28606
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Prazek closed this revision.
Prazek added a comment.
Pushed on branch.
Please check if everything looks good when RC will be released.
https://reviews.llvm.org/D28746
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi
This revision was automatically updated to reflect the committed changes.
Prazek marked an inline comment as done.
Closed by commit rL292112: Add -fstrict-vtable-pointers to UsersManual
(authored by Prazek).
Changed prior to commit:
https://reviews.llvm.org/D28727?vs=84492&id=84551#toc
Reposit
Prazek added a comment.
I also have sent it on the branch
Repository:
rL LLVM
https://reviews.llvm.org/D28727
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Prazek added a comment.
Thanks for the check. Have you run it on llvm?
Comment at: clang-tidy/modernize/ReturnBracedInitListCheck.cpp:27-32
+ auto soughtConstructExpr =
+ cxxConstructExpr(unless(isListInitialization())).bind("ctor");
+
+ auto hasConstructExpr = has(ignor
Prazek added inline comments.
Comment at: clang-tidy/misc/ThrowWithNoexceptCheck.cpp:52-53
+ for (const auto &NoExceptRange : NoExceptRanges) {
+// FIXME use DiagnosticIDs::Level::Note
+diag(NoExceptRange.getBegin(), "In function declared no-throw here:")
+<< Fix
Prazek added inline comments.
Comment at: test/clang-tidy/modernize-return-braced-init-list.cpp:95
+}
+
+vector f6() {
please also add test that contains
for function
Type foo():
return call(Type());
return OtherType(Type()); // implicit conversion
It would
Prazek accepted this revision.
Prazek added a comment.
This revision is now accepted and ready to land.
Do you have some results from running it on LLVM? If nothing crashes and all
fixit are correct then LGTM.
Comment at: clang-tidy/modernize/ReturnBracedInitListCheck.cpp:27-3
Prazek added a comment.
Ok, I didn't know that it is this easy to run alpha checks from clang.
https://reviews.llvm.org/D28729
___
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: clang-tidy/misc/InvalidatedIteratorsCheck.cpp:61
+ cxxMemberCallExpr(has(memberExpr(hasDeclaration(cxxMethodDecl(hasAnyName(
+ "push_back", "emplace_back",
"clear",
+
Prazek added a comment.
In https://reviews.llvm.org/D29151#656887, @Eugene.Zelenko wrote:
> General question: isn't Static Analyzer is better place for such workflow
> checks?
Probably yes, but it is much harder to implement this there. Other problem is
that it would be probably a part
of on
Prazek added a comment.
I have read the patch, but I don't have enough knowledge about C++ rules and
fronted to accept it. Richard?
Comment at: lib/Sema/Sema.cpp:684
+ for (auto PII : Pending)
+if (FunctionDecl *Func = dyn_cast(PII.first))
+ Func->setMar
Prazek added a comment.
In https://reviews.llvm.org/D29151#658484, @zaks.anna wrote:
> The static analyzer is definitely the place to go for bug detection that
> requires path sensitivity. It's also reasonably good for anything that needs
> flow-sensitivity. Although, most flow-sensitive analys
Prazek added a comment.
In https://reviews.llvm.org/D29151#665948, @alexfh wrote:
> In https://reviews.llvm.org/D29151#662504, @zaks.anna wrote:
>
> > Before clang-tidy came into existence the guidelines were very clear. One
> > should write a clang warning if the diagnostic:
> >
> > - can be im
Prazek added a comment.
In https://reviews.llvm.org/D29151#668197, @zaks.anna wrote:
> > I tried to come up with some advice on where checks should go in
> > http://clang.llvm.org/extra/clang-tidy/#choosing-the-right-place-for-your-check
>
> The guidelines stated on the clang-tidy page seem reas
Prazek added inline comments.
Comment at: clang-tidy/misc/ThrowWithNoexceptCheck.cpp:54
+// FIXME use DiagnosticIDs::Level::Note
+diag(NoExceptRange.getBegin(), "in a function declared no-throw here:",
DiagnosticIDs::Note)
+<< FixItHint::CreateRemoval(NoExceptRan
Prazek added inline comments.
Comment at: clang-tidy/misc/ThrowWithNoexceptCheck.cpp:54
+// FIXME use DiagnosticIDs::Level::Note
+diag(NoExceptRange.getBegin(), "in a function declared no-throw here:",
DiagnosticIDs::Note)
+<< FixItHint::CreateRemoval(NoExceptRan
Prazek added a comment.
In https://reviews.llvm.org/D33470#790484, @aaron.ballman wrote:
> In https://reviews.llvm.org/D33470#789791, @Prazek wrote:
>
> > In https://reviews.llvm.org/D33470#764846, @aaron.ballman wrote:
> >
> > > Once you fix the typo in the check, can you run it over some large
Prazek updated this revision to Diff 104281.
Prazek marked 2 inline comments as done.
Prazek added a comment.
Herald added a subscriber: JDevlieghere.
- fixed docs
- fixes
- Last fixes?
https://reviews.llvm.org/D33470
Files:
clang-tidy/misc/CMakeLists.txt
clang-tidy/misc/DefaultNumericsChec
Prazek updated this revision to Diff 104288.
Prazek added a comment.
Small fix
https://reviews.llvm.org/D33470
Files:
clang-tidy/misc/CMakeLists.txt
clang-tidy/misc/DefaultNumericsCheck.cpp
clang-tidy/misc/DefaultNumericsCheck.h
clang-tidy/misc/MiscTidyModule.cpp
docs/ReleaseNotes.rst
Prazek updated this revision to Diff 104297.
Prazek added a comment.
fixed broken test
https://reviews.llvm.org/D33470
Files:
clang-tidy/misc/CMakeLists.txt
clang-tidy/misc/DefaultNumericsCheck.cpp
clang-tidy/misc/DefaultNumericsCheck.h
clang-tidy/misc/MiscTidyModule.cpp
docs/ReleaseN
Prazek added inline comments.
Comment at: include/clang/Basic/Attr.td:2421
-def SelectAny : InheritableAttr, TargetSpecificAttr {
+def SelectAny : InheritableAttr, TargetSpecificAttr {
let Spellings = [Declspec<"selectany">, GCC<"selectany">];
davide wrote:
Prazek added a comment.
Nice check! :)
Comment at: clang-tidy/misc/IstreamOverflowCheck.cpp:59-61
+ if (ConstType) {
+ArraySize = ConstType->getSize();
+ }
same here
Comment at: clang-tidy/misc/IstreamOverflowCheck.cpp:78-80
+if (!A
Prazek added a comment.
In https://reviews.llvm.org/D29839#674301, @xazax.hun wrote:
> Shouldn't this be a path sensitive check within the clang static analyzer
> instead? So branches are properly handled and interprocedural analysis is
> done.
Do you have some examples? I would argue, that e
Prazek added a comment.
In https://reviews.llvm.org/D29839#674315, @xazax.hun wrote:
> In https://reviews.llvm.org/D29839#674307, @Prazek wrote:
>
> > In https://reviews.llvm.org/D29839#674301, @xazax.hun wrote:
> >
> > > Shouldn't this be a path sensitive check within the clang static analyzer
Prazek abandoned this revision.
Prazek added inline comments.
Comment at: lib/CodeGen/SplitKit.h:298
- typedef PointerIntPair ValueForcePair;
+ typedef PointerIntPair ValueForcePair;
typedef DenseMap, ValueForcePair> ValueMap;
alexfh wrote:
> Is it an auto
Prazek added inline comments.
Comment at: clang-tidy/misc/ThrowWithNoexceptCheck.cpp:54
+// FIXME use DiagnosticIDs::Level::Note
+diag(NoExceptRange.getBegin(), "in a function declared no-throw here:",
DiagnosticIDs::Note)
+<< FixItHint::CreateRemoval(NoExceptRan
Prazek added a reviewer: staronj.
Prazek added a comment.
gosh, I found it too late. Jakub started to work on the same problem and he
have made initial check for it.
One of the problems he got was multideclarations.
Does it work for cases like?
long a = lexical_cast(z), b = lexical_cast(y);
O
Prazek added inline comments.
Comment at: clang-tidy/misc/UseAfterMoveCheck.cpp:18
using namespace clang::ast_matchers;
+using namespace clang::tidy::utils;
+
I don't think it is required
Comment at: clang-tidy/utils/ExprSequence.cpp:180-182
+
Prazek added inline comments.
Comment at: clang-tidy/misc/UseAfterMoveCheck.cpp:18
using namespace clang::ast_matchers;
+using namespace clang::tidy::utils;
+
Prazek wrote:
> I don't think it is required
ok I guess I am wrong
https://reviews.llvm.org/D27700
Prazek added inline comments.
Comment at: clang-tidy/utils/ExprSequence.cpp:154
+return SyntheticStmtSourceMap.lookup(S);
+ else
+return S;
alexfh wrote:
> nit: No `else` after return, please.
Not sure if he should change it in this patch - it is just mo
Prazek added inline comments.
Comment at: clang-tidy/utils/ExprSequence.cpp:52
+
+bool isDescendantOrEqual(const Stmt *Descendant, const Stmt *Ancestor,
+ ASTContext *Context) {
staronj wrote:
> Shouldn't isDescendantOrEqual be static or i
Prazek added a comment.
In https://reviews.llvm.org/D27166#617696, @malcolm.parsons wrote:
> In https://reviews.llvm.org/D27166#617621, @Prazek wrote:.
>
> > Does it work for cases like?
>
>
> Yes; `replaceExpr()` checks that the variable has the same unqualified type
> as the initializer and th
Prazek created this revision.
Prazek added reviewers: rsmith, mboehme.
Prazek added a subscriber: cfe-commits.
Herald added a subscriber: klimek.
Bunch of fixed bugs in Clang after running misc-use-after-move in clang-tidy.
https://reviews.llvm.org/D27752
Files:
lib/Format/Format.cpp
lib/Fo
Prazek added a comment.
Cool!
Have you run this check on LLVM & Clang to check if it works?
I just runned it and I got this weird behavior
lib/Analysis/AssumptionCache.cpp:120
-for (const BasicBlock &B : cast(*I.first))
+for (const BasicBlock &B : auto(*I.first))
https://reviews.l
Prazek added a comment.
There is still one more problem:
/home/prazek/llvm/lib/Analysis/ScalarEvolution.cpp:2442:11: warning: use auto
when initializing with a template cast to avoid duplicating the type name
[modernize-use-auto]
const auto **O = SCEVAllocator.Allocate(Ops.size());
Prazek added a comment.
In https://reviews.llvm.org/D27166#622108, @malcolm.parsons wrote:
> In https://reviews.llvm.org/D27166#622103, @Prazek wrote:
>
> > There is still one more problem:
> >
> > /home/prazek/llvm/lib/Analysis/ScalarEvolution.cpp:2442:11: warning: use
> > auto when initializ
Prazek accepted this revision.
Prazek added a comment.
LGTM, but please leave the FIXME comment about the running of apply replacement
https://reviews.llvm.org/D32294
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-
Prazek added a comment.
In https://reviews.llvm.org/D32294#732861, @kuhar wrote:
> After thinking about Piotr's comment, I think that a better way to perform
> the check would be te invoking clang-apply-replacements with `--version` and
> seeing if it fails even before running clang-tidy.
> Th
Prazek created this revision.
This code was wrongly devirtualized before:
A* a = new A;
a->foo();
A* b = new(a) B;
if (a == b)
b->foo();
Now we insert barrier before comparing dynamic pointers.
https://reviews.llvm.org/D32378
Files:
lib/CodeGen/CGExprScalar.cpp
test/CodeGen
Prazek updated this revision to Diff 96254.
Prazek added a comment.
- Checking for vptrs
https://reviews.llvm.org/D31830
Files:
lib/CodeGen/CGExpr.cpp
test/CodeGenCXX/strict-vtable-pointers.cpp
Index: test/CodeGenCXX/strict-vtable-pointers.cpp
==
Prazek marked 6 inline comments as done.
Prazek added a comment.
For now I will check if it has any vptrs. It will be probably soon stored in
the CXXRecordDecl because it will be used much more frequently when generating
barriers for pointer casts.
https://reviews.llvm.org/D31830
__
Prazek updated this revision to Diff 96255.
Prazek added a comment.
- format
https://reviews.llvm.org/D31830
Files:
lib/CodeGen/CGExpr.cpp
test/CodeGenCXX/strict-vtable-pointers.cpp
Index: test/CodeGenCXX/strict-vtable-pointers.cpp
==
Prazek added inline comments.
Comment at: clang-tidy/modernize/UseEmplaceCheck.cpp:93
+ to(functionDecl(hasName("::std::make_pair"
+
+ .bind("make_pair"));
JonasToth wrote:
> is the new line here necessary? i think it l
Prazek added inline comments.
Comment at: test/clang-tidy/modernize-use-emplace.cpp:284
+ // CHECK-FIXES: v.emplace_back(42LL, 13);
+
+ v.push_back(std::make_pair(0, 3));
I would add here test like:
class X {
X(std:;pair a) {}
};
std::vector v;
Prazek marked an inline comment as done.
Prazek added inline comments.
Comment at: lib/CodeGen/CGExprScalar.cpp:3066-3067
+} else { // Unsigned integers and pointers.
+ if (CGF.CGM.getCodeGenOpts().StrictVTablePointers &&
+ CGF.CGM.getCodeGenOpts().OptimizationL
Prazek updated this revision to Diff 96311.
Prazek marked an inline comment as done.
Prazek added a comment.
Addressing Richard's comment
https://reviews.llvm.org/D32378
Files:
lib/CodeGen/CGExprScalar.cpp
test/CodeGenCXX/strict-vtable-pointers.cpp
Index: test/CodeGenCXX/strict-vtable-poin
Prazek updated this revision to Diff 96312.
Prazek added a comment.
- Inserting barrier with -O0
https://reviews.llvm.org/D31830
Files:
lib/CodeGen/CGExpr.cpp
test/CodeGenCXX/strict-vtable-pointers.cpp
Index: test/CodeGenCXX/strict-vtable-pointers.cpp
==
Prazek added a comment.
This is actually good catch, we also need to do it when inserting barrier in
placement new.
I think that for the ctors and dtors we can do it only with optimizations
enabled, because if optimizations are not enabled then ctors and dtors won't
have the invariant.group in
Prazek created this revision.
Herald added a subscriber: mehdi_amini.
To not break LTO with different optimizations levels, we should insert
the barrier regardles of optimization level.
https://reviews.llvm.org/D32401
Files:
lib/CodeGen/CGExprCXX.cpp
Index: lib/CodeGen/CGExprCXX.cpp
===
Prazek added a comment.
In https://reviews.llvm.org/D32401#734870, @rjmccall wrote:
> Won't you have the exact same problem when LTO'ing with code that wasn't
> compiled with strict v-table pointers?
No, because we fire error when combining module compiled with and without
StrictVtablePoint
Prazek added a comment.
In https://reviews.llvm.org/D32401#734921, @rjmccall wrote:
> I continue to be really uncomfortable with the entire design of this
> optimization, which appears to miscompile code by default, but as long as
> nobody's suggesting that we actually turn it on by default, I
Prazek updated this revision to Diff 96371.
Prazek added a comment.
Don't add barrier if compared with nullptr. With this it reduces added
barriers to compares by half. Note that we will transform barrier(null) ->
barrier
in llvm
https://reviews.llvm.org/D32378
Files:
lib/CodeGen/CGExprScal
This revision was automatically updated to reflect the committed changes.
Closed by commit rL301178: [Devirtualization] Emit invariant.group loads with
empty group md (authored by Prazek).
Changed prior to commit:
https://reviews.llvm.org/D32110?vs=95378&id=96387#toc
Repository:
rL LLVM
htt
Prazek accepted this revision.
Prazek added a comment.
This revision is now accepted and ready to land.
LGTM after the fixes
https://reviews.llvm.org/D32294
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailma
Prazek added inline comments.
Comment at: clang-tidy/modernize/UseEmplaceCheck.cpp:115
+ const auto *MakePairCall = Result.Nodes.getNodeAs("make_pair");
+ assert(InnerCtorCall || MakePairCall);
JDevlieghere wrote:
> It's highly recommended to put some kind of
Prazek added inline comments.
Comment at: clang-tidy/modernize/UseEmplaceCheck.cpp:115
+ const auto *MakePairCall = Result.Nodes.getNodeAs("make_pair");
+ assert(InnerCtorCall || MakePairCall);
kuhar wrote:
> Prazek wrote:
> > JDevlieghere wrote:
> > > It's h
Prazek added a comment.
In https://reviews.llvm.org/D32401#738513, @rjmccall wrote:
> In https://reviews.llvm.org/D32401#735127, @Prazek wrote:
>
> > In https://reviews.llvm.org/D32401#734921, @rjmccall wrote:
> >
> > > I continue to be really uncomfortable with the entire design of this
> > > o
101 - 200 of 219 matches
Mail list logo
