On Sat, Sep 15, 2018, 08:36 Eric Liu via Phabricator <
revi...@reviews.llvm.org> wrote:
> ioeric added inline comments.
>
>
>
> Comment at: clangd/index/FileIndex.h:93
> std::pair
> indexAST(ASTContext &AST, std::shared_ptr PP,
> + bool IndexMacros = false,
> ---
lebedev.ri added a comment.
In https://reviews.llvm.org/D52120#1235515, @lebedev.ri wrote:
> Though some of that is still false-positives (pointers, va_arg, callback
> lambdas passed as templated function param), but i'll file further bugs i
> guess.
Filed the ones that i can pinpoint, marked
jdoerfert accepted this revision.
jdoerfert added a comment.
This revision is now accepted and ready to land.
> save an iteration over the loop's basic blocks (which is what getLoopLatch
> does)
I'm not sure this is true. getLoopLatch() in LoopInfoImpl.h
only traverses the children of the heade
JonasToth added a comment.
Thank you very much for the good work from my side as well!
Comment at: unittests/Analysis/ExprMutationAnalyzerTest.cpp:387
match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
- EXPECT_THAT(mutatedBy(Results, AST.get()), Elemen
JonasToth added inline comments.
Comment at: unittests/Analysis/ExprMutationAnalyzerTest.cpp:387
match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
- EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("std::move(x)"));
+ EXPECT_FALSE(isMutated(Result
Author: kli
Date: Sat Sep 15 06:54:15 2018
New Revision: 342322
URL: http://llvm.org/viewvc/llvm-project?rev=342322&view=rev
Log:
[OPENMP] Move OMPClauseReader/Writer classes to ASTReader/Writer (NFC)
Move declarations for OMPClauseReader, OMPClauseWriter to ASTReader.h
and ASTWriter.h and move
This revision was automatically updated to reflect the committed changes.
Closed by commit rL342322: [OPENMP] Move OMPClauseReader/Writer classes to
ASTReader/Writer (NFC) (authored by kli, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.or
IdrissRio created this revision.
IdrissRio added reviewers: aaron.ballman, hokein, alexfh.
Herald added a subscriber: cfe-commits.
[Clang-Tidy: modernize] Fix for modernize-redundant-void-arg: complains about
variable cast to void
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D
lebedev.ri added a comment.
This looks weird.
It will now completely skip all the explicitly instantiated functions, no?
I'd think the problem that needs to be fixed is that it considers the local
variable `int a_template;` to be in the function argument list.
Repository:
rCTE Clang Tools Ext
wgml created this revision.
wgml added reviewers: alexfh, aaron.ballman, hokein.
Herald added subscribers: xazax.hun, mgorny.
Finds instances of namespaces concatenated using explicit syntax, such as
`namespace a { namespace b { [...] }}` and offers fix to glue it to `namespace
a::b { [...] }`.
lebedev.ri added inline comments.
Comment at: clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp:73-74
+void ConcatNestedNamespacesCheck::registerMatchers(MatchFinder *Finder) {
+ if (getLangOpts().CPlusPlus)
+Finder->addMatcher(namespaceDecl().bind("namespace"), this);
+}
IdrissRio added a comment.
In https://reviews.llvm.org/D52135#1235950, @lebedev.ri wrote:
>
> It will now completely skip all the explicitly instantiated functions, no?
In my opinion, for this kind of check, we don't have the necessity to take in
consideration the function instantiation. W
lebedev.ri added inline comments.
Comment at: clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp:42-45
+ if (Decl->getKind() != Decl::Kind::Namespace)
+return false;
+
+ auto const *NS = reinterpret_cast(Decl);
Use proper casting, `isa<>()`, `dyn_cast<>`,
Eugene.Zelenko added inline comments.
Comment at: clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp:37
+static bool singleNamedNamespaceChild(NamespaceDecl const &ND) {
+ auto const Decls = ND.decls();
+ if (childrenCount(Decls) != 1)
Type is not spelled in
xbolva00 created this revision.
xbolva00 added a reviewer: rsmith.
Herald added a subscriber: cfe-commits.
Inspired by MSVC, which found some occurrences of this expression on our code
base.
Fixes PR38950
Repository:
rC Clang
https://reviews.llvm.org/D52137
Files:
include/clang/Basic/Dia
wgml added inline comments.
Comment at: clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp:102-108
+ if (childrenCount(ND.decls()) == 0) {
+SourceRange Removal(Namespaces.front().Begin, Namespaces.front().RBrace);
+
+diag(Namespaces.front().Begin,
+ "Nested nam
xbolva00 added a subscriber: RKSimon.
xbolva00 added a comment.
/home/xbolva00/LLVM/build/lib/clang/8.0.0/include/bmiintrin.h:312:16: error:
unary minus operator applied to type 'unsigned long long', result value is
still unsigned
return __X & -__X;
@RKSimon what do you think?
Repository:
xbolva00 updated this revision to Diff 165651.
https://reviews.llvm.org/D52137
Files:
include/clang/Basic/DiagnosticSemaKinds.td
lib/Sema/SemaExpr.cpp
test/OpenMP/teams_thread_limit_messages.cpp
test/Sema/unary-minus-unsigned.c
Index: test/Sema/unary-minus-unsigned.c
===
lebedev.ri added a comment.
I don't think this makes much sense.
In https://reviews.llvm.org/D52137#1236011, @xbolva00 wrote:
> /home/xbolva00/LLVM/build/lib/clang/8.0.0/include/bmiintrin.h:312:16: error:
> unary minus operator applied to type 'unsigned long long', result value is
> still unsi
xbolva00 added inline comments.
Comment at: lib/Sema/SemaExpr.cpp:12651
+ if (Opc == UO_Minus && resultType->isUnsignedIntegerType())
+return ExprError(Diag(OpLoc, diag::err_unsignedtypecheck_unary_minus)
+ << resultType << Input.get()->getSou
lebedev.ri added a comment.
In https://reviews.llvm.org/D52137#1236011, @xbolva00 wrote:
> /home/xbolva00/LLVM/build/lib/clang/8.0.0/include/bmiintrin.h:312:16: error:
> unary minus operator applied to type 'unsigned long long', result value is
> still unsigned
>
> return __X & -__X;
>
>
>
xbolva00 added a comment.
The first is not broken, but it is now detected by this new check. I think the
second form is more explicit and better.
https://reviews.llvm.org/D52137
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.l
xbolva00 added a comment.
@RKSimon are fine to rewrite X & -X to X & ((~X)+1) in bmiintrin.h?
https://reviews.llvm.org/D52137
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
lebedev.ri added a comment.
> found some occurrences of this expression on our code base.
Standard question: what is the SNR of this warning? What value it brings?
How many times it fired? How many of these are actual real bugs?
https://reviews.llvm.org/D52137
___
kristina added a comment.
Yeah but what about the rest of them, most are completely on their own line,
some no newlines, mipsel64 was split across two lines while others weren't, and
switch is meant to have default as the topmost case IIRC. I mean I'm if you
don't think it's more readable that
xbolva00 added a comment.
15-20 times / 10 000 lines. I checked and first two are bugs, I don't track
other warnings yet.
Anyway, I think this pattern should be diagnosed, as confusing and potentially
buggy.
https://reviews.llvm.org/D52137
___
cf
xbolva00 updated this revision to Diff 165654.
xbolva00 added a comment.
1. Error -> Warning
https://reviews.llvm.org/D52137
Files:
include/clang/Basic/DiagnosticSemaKinds.td
lib/Sema/SemaExpr.cpp
test/CXX/drs/dr6xx.cpp
test/OpenMP/teams_distribute_thread_limit_messages.cpp
test/OpenM
wgml updated this revision to Diff 165658.
wgml marked 12 inline comments as done.
wgml added a comment.
Adjusted to review comments.
https://reviews.llvm.org/D52136
Files:
clang-tidy/modernize/CMakeLists.txt
clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp
clang-tidy/modernize/Concat
wgml added inline comments.
Comment at: clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp:102-108
+ if (childrenCount(ND.decls()) == 0) {
+SourceRange Removal(Namespaces.front().Begin, Namespaces.front().RBrace);
+
+diag(Namespaces.front().Begin,
+ "Nested nam
rsmith added a comment.
I think we can and should do better about false positives here. If you move
this check to SemaChecking, you can produce the warning in a context where you
know what the final type is -- I don't think there's any reason to warn if the
final type is signed and no wider tha
Author: jvesely
Date: Sat Sep 15 13:00:12 2018
New Revision: 342337
URL: http://llvm.org/viewvc/llvm-project?rev=342337&view=rev
Log:
.travis: Use source whitelist alias for llvm-6 repository
Fixes issue with unauthenticated packages.
Signed-off-by: Jan Vesely
Reviewer: Aaron Watry
Modified:
Author: jvesely
Date: Sat Sep 15 13:00:37 2018
New Revision: 342338
URL: http://llvm.org/viewvc/llvm-project?rev=342338&view=rev
Log:
.travis: Add llvm-7 build
Signed-off-by: Jan Vesely
Reviewer: Aaron Watry
Modified:
libclc/trunk/.travis.yml
Modified: libclc/trunk/.travis.yml
URL:
http:/
Quuxplusone added a comment.
In https://reviews.llvm.org/D52137#1236053, @rsmith wrote:
> I think we can and should do better about false positives here. If you move
> this check to SemaChecking, you can produce the warning in a context where
> you know what the final type is -- I don't think t
xbolva00 added a comment.
I think we can emit fixit hint always, not even for -u32 case :)
https://reviews.llvm.org/D52137
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
sammccall added a comment.
I don't have an opinion on the style per se, but please use clang-format. If it
doesn't produce the formatting you want, then change the code so it does. (e.g.
adding a trailing , at the end of an init-list will force one-per-line).
Repository:
rC Clang
https://re
Author: shuaiwang
Date: Sat Sep 15 14:38:18 2018
New Revision: 342340
URL: http://llvm.org/viewvc/llvm-project?rev=342340&view=rev
Log:
[NFC] cosmetic tweaks to ExprMutationAnalyzer to be more consistent
especially considering future changes.
Modified:
cfe/trunk/include/clang/Analysis/Analyse
aaronpuchert created this revision.
aaronpuchert added reviewers: aaron.ballman, delesley, grooverdan.
Herald added a subscriber: cfe-commits.
We run the tests for -Wthread-safety-{negative,verbose} with the new
attributes as well as the old ones. Also put the macros in a header so
that we don't h
xbolva00 added a comment.
/home/xbolva00/LLVM/llvm/tools/clang/test/Sema/unary-minus-unsigned.c:13:15:
warning: unary minus operator applied to type 'unsigned long', result value is
still unsigned [-Wsign-conversion]
long b3 = -b; // expected-warning {{unary minus operator applied to type
'u
Author: jvesely
Date: Sat Sep 15 15:02:01 2018
New Revision: 342341
URL: http://llvm.org/viewvc/llvm-project?rev=342341&view=rev
Log:
configure: Rework support for gfx9+ devices that were added post LLVM 3.9
v2: Fix reference to Vega12/20 enabling commit
Signed-off-by: Jan Vesely
Reviewer: Aaro
xbolva00 updated this revision to Diff 165663.
xbolva00 added a comment.
Added fixit hints
https://reviews.llvm.org/D52137
Files:
include/clang/Basic/DiagnosticSemaKinds.td
lib/Sema/SemaExpr.cpp
test/CXX/drs/dr6xx.cpp
test/OpenMP/teams_distribute_thread_limit_messages.cpp
test/OpenMP/
a_sidorin accepted this revision.
a_sidorin added inline comments.
This revision is now accepted and ready to land.
Comment at: lib/AST/ASTImporter.cpp:1441
+ To->setInit(ToInit);
+ if (From->isInitKnownICE()) {
+EvaluatedStmt *Eval = To->ensureEvaluatedStmt();
xbolva00 updated this revision to Diff 165664.
https://reviews.llvm.org/D52137
Files:
include/clang/Basic/DiagnosticSemaKinds.td
lib/Sema/SemaExpr.cpp
test/CXX/drs/dr6xx.cpp
test/OpenMP/teams_distribute_thread_limit_messages.cpp
test/OpenMP/teams_thread_limit_messages.cpp
test/Sema/un
xbolva00 added a comment.
Hm, isUnsignedIntegerType seems to ignore uintN_t types?
int f(void) {
uint8_t u8 = 1;
uint8_t b = -u8;
}
No warning now :/
https://reviews.llvm.org/D52137
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
htt
malaperle added a comment.
In https://reviews.llvm.org/D52089#1235851, @ioeric wrote:
> In https://reviews.llvm.org/D52089#1235777, @malaperle wrote:
>
> > why?
>
>
> I wanted to get some numbers and update the patch summary, but somehow
> forgot. Sorry about that and thanks for asking!
>
> The
44 matches
Mail list logo
