courbet added inline comments.
Comment at: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:155
+ if (!LhsIntegerRange.Contains(IntegerConstant))
+diag(SourceLoc, "narrowing conversion from %0 to %1") << RhsType <<
LhsType;
+ return true;
I think
lebedev.ri added inline comments.
Comment at: test/clang-tidy/cppcoreguidelines-narrowing-conversions.cpp:164
+ while (i) {
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: narrowing conversion from
'int' to 'bool' [cppcoreguidelines-narrowing-conversions]
+ }
---
hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.
The change looks reasonable to me.
In https://reviews.llvm.org/D54061#1286505, @steveire wrote:
> After this change, it seems that `ClangTidyCheck::check` is not needed and
> all callers shou
JonasToth added a comment.
> Theoretically, we could replace `ClangTidyCheck::check` with
> `ClangTidyCheck::run`, but I'm not sure it is worth, `ClangTidyCheck::check`
> is a public API, and is widely-used (for all clang-tidy checks), replacing it
> requires large changes (although it is one-l
xbolva00 added a comment.
Ping :)
https://reviews.llvm.org/D52835
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
lewis-revill created this revision.
lewis-revill added a reviewer: asb.
Herald added subscribers: cfe-commits, jocewei, PkmX, rkruppe, the_o,
brucehoult, MartinMosbeck, rogfer01, mgrang, edward-jones, zzheng, jrtc27,
shiva0217, kito-cheng, niosHD, sabuasal, apazos, simoncook, johnrusso, rbar,
er
hokein created this revision.
hokein added a reviewer: ioeric.
We can run the tools on a subset files of compilation database.
Repository:
rC Clang
https://reviews.llvm.org/D54092
Files:
lib/Tooling/AllTUsExecution.cpp
Index: lib/Tooling/AllTUsExecution.cpp
==
JonasToth added inline comments.
Comment at: test/clang-tidy/cppcoreguidelines-narrowing-conversions.cpp:164
+ while (i) {
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: narrowing conversion from
'int' to 'bool' [cppcoreguidelines-narrowing-conversions]
+ }
baloghadamsoftware added a comment.
Your suggestion sounds completely reasonable. We cannot rely entirely on
inlining of comparison operators. However, there is an important side-effect of
inlining: on iterator-adapters inlining ensures that we handle the comparison
of the underlying iterator c
ioeric added inline comments.
Comment at: lib/Tooling/AllTUsExecution.cpp:58
+"filter",
+llvm::cl::desc("Only process files that match this filter"),
+llvm::cl::init(".*"));
Please also mention that this only applies to all-TUs.
Com
Author: kadircet
Date: Mon Nov 5 02:01:34 2018
New Revision: 346123
URL: http://llvm.org/viewvc/llvm-project?rev=346123&view=rev
Log:
Fix breakage on FrontendTest by initializing new field on constructor
Modified:
cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
Modified: cfe/t
Should be fixed in r346123.
+Kadir Çetinkaya who authored the fix.
On Fri, Nov 2, 2018 at 7:38 PM Galina Kistanova via cfe-commits <
cfe-commits@lists.llvm.org> wrote:
> Also one of your resent commits added another broken test to the builder:
>
> http://lab.llvm.org:8011/builders/llvm-clang-x86
This revision was automatically updated to reflect the committed changes.
Closed by commit rL346124: [mips][msa] Fix broken test (authored by
abeserminji, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D53984?vs=172152&id=172554#toc
R
ilya-biryukov added subscribers: simark, klimek.
ilya-biryukov added a comment.
Thanks for the patch! I believe many people I talked to want this behavior
(myself included).
Some people like what we do now more. It feels like it depends on the workflow:
for people who auto-save *all* files befo
gchatelet marked 8 inline comments as done.
gchatelet added inline comments.
Comment at: test/clang-tidy/cppcoreguidelines-narrowing-conversions.cpp:164
+ while (i) {
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: narrowing conversion from
'int' to 'bool' [cppcoreguidelines-
hokein updated this revision to Diff 172560.
hokein marked an inline comment as done.
hokein added a comment.
Update comment.
Repository:
rC Clang
https://reviews.llvm.org/D54092
Files:
lib/Tooling/AllTUsExecution.cpp
Index: lib/Tooling/AllTUsExecution.cpp
===
hokein added inline comments.
Comment at: lib/Tooling/AllTUsExecution.cpp:120
for (std::string File : Files) {
+ if (Filter.getNumOccurrences() != 0 && !RegexFilter.match(File))
+continue;
ioeric wrote:
> > `Filter.getNumOccurrences() != 0 `
> W
Szelethus marked 6 inline comments as done.
Szelethus added inline comments.
Comment at: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:677
+/// need to expanded further when it is nested inside another macro.
+class MacroArgMap : public std::map {
+public:
NoQ wro
ioeric added inline comments.
Comment at: lib/Tooling/AllTUsExecution.cpp:120
for (std::string File : Files) {
+ if (Filter.getNumOccurrences() != 0 && !RegexFilter.match(File))
+continue;
hokein wrote:
> ioeric wrote:
> > > `Filter.getNumOccurr
yamaguchi created this revision.
yamaguchi added a reviewer: rsmith.
ReadASTBlock was allocating extra memory by resizing vector for all
record decls, and half of them stayed nullptr without being loaded. For
us this part was crucial and this patch had large amount of memory
improvement.
About cl
ebevhan added a comment.
In https://reviews.llvm.org/D53738#1284213, @rjmccall wrote:
> >> 2. The question is easily answered by pointing at the language spec. The
> >> language does not say that the operands are promoted to a common type; it
> >> says the result is determined numerically from
yamaguchi added a comment.
FYI, raw benchmarking data:
https://gist.github.com/yamaguchi1024/4f28406f51b413129cc762365fdd76c8
https://gist.github.com/yamaguchi1024/318b30db03adb10b7b230b706012a14c
https://reviews.llvm.org/D54097
___
cfe-commits mail
JonasToth added inline comments.
Comment at: test/clang-tidy/cppcoreguidelines-narrowing-conversions.cpp:164
+ while (i) {
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: narrowing conversion from
'int' to 'bool' [cppcoreguidelines-narrowing-conversions]
+ }
klimek added a comment.
In https://reviews.llvm.org/D54077#1287040, @ilya-biryukov wrote:
> Thanks for the patch! I believe many people I talked to want this behavior
> (myself included).
> Some people like what we do now more. It feels like it depends on the
> workflow: for people who auto-sa
Disclaimer: I'm on a train with family today, and haven't actually read the
patch...
So I have concerns :-)
1. There's the usual concern that the current behavior is reasonable and
people like it, so adding a second reasonable behavior provides a small
amount of value to the userbase as a whole (
sammccall added subscribers: ilya-biryukov, LutsenkoDanil.
sammccall added a comment.
Disclaimer: I'm on a train with family today, and haven't actually read the
patch...
So I have concerns :-)
1. There's the usual concern that the current behavior is reasonable and
people like it, so adding a
joerg added a comment.
Nothing changed. I don't see how catering to the broken libstdc++
self-configuration helps. So no, I still object to this change.
Repository:
rL LLVM
https://reviews.llvm.org/D34018
___
cfe-commits mailing list
cfe-commits
Author: d0k
Date: Mon Nov 5 04:46:02 2018
New Revision: 346130
URL: http://llvm.org/viewvc/llvm-project?rev=346130&view=rev
Log:
Reapply "Fix regression in behavior of clang -x c++-header -fmodule-name=XXX"
This reverts commit r345963. We have a path forward now.
Original commit message:
The dr
hokein added inline comments.
Comment at: lib/Tooling/AllTUsExecution.cpp:120
for (std::string File : Files) {
+ if (Filter.getNumOccurrences() != 0 && !RegexFilter.match(File))
+continue;
ioeric wrote:
> hokein wrote:
> > ioeric wrote:
> > > >
hokein updated this revision to Diff 172571.
hokein added a comment.
Remove the default value check.
Repository:
rC Clang
https://reviews.llvm.org/D54092
Files:
lib/Tooling/AllTUsExecution.cpp
Index: lib/Tooling/AllTUsExecution.cpp
ilya-biryukov added a comment.
> There's the usual concern that the current behavior is reasonable and people
> like it.
I think it would be reasonable to say that a large portion of C++ users are
used to the behavior that this patch introduces.
Specifically, all IDEs (Clion, Eclipse, Visual St
ioeric accepted this revision.
ioeric added inline comments.
This revision is now accepted and ready to land.
Comment at: lib/Tooling/AllTUsExecution.cpp:120
for (std::string File : Files) {
+ if (Filter.getNumOccurrences() != 0 && !RegexFilter.match(File))
+con
ioeric added a comment.
maybe add a test?
Repository:
rC Clang
https://reviews.llvm.org/D54092
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
hokein updated this revision to Diff 172574.
hokein added a comment.
Add test.
Repository:
rC Clang
https://reviews.llvm.org/D54092
Files:
include/clang/Tooling/AllTUsExecution.h
lib/Tooling/AllTUsExecution.cpp
unittests/Tooling/ExecutionTest.cpp
Index: unittests/Tooling/ExecutionTes
gamesh411 added a comment.
Hi!
Thanks for your reviews, although I haven't been active for some time now.
I personally do not have commit rights, so could someone else take care of it?
https://reviews.llvm.org/D33672
___
cfe-commits mailing list
cfe
This revision was automatically updated to reflect the committed changes.
Closed by commit rC346131: [Tooling] Add "-filter" option to
AllTUsExecution (authored by hokein, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D54092?vs=172574&id=172575#toc
Repository:
rC Clang
h
Author: hokein
Date: Mon Nov 5 05:42:05 2018
New Revision: 346131
URL: http://llvm.org/viewvc/llvm-project?rev=346131&view=rev
Log:
[Tooling] Add "-filter" option to AllTUsExecution
Summary: We can run the tools on a subset files of compilation database.
Reviewers: ioeric
Subscribers: cfe-comm
On Mon, Nov 5, 2018, 13:58 Ilya Biryukov via Phabricator <
revi...@reviews.llvm.org wrote:
> ilya-biryukov added a comment.
>
> > There's the usual concern that the current behavior is reasonable and
> people like it.
>
> I think it would be reasonable to say that a large portion of C++ users
> ar
krytarowski planned changes to this revision.
krytarowski added a comment.
OK.
Repository:
rL LLVM
https://reviews.llvm.org/D34018
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Szelethus added a comment.
I read through your patch multiple times, have read your mail on cfe-dev, and I
still couldn't find eve minor nits, well done!
However, both in the mail, and in this discussion, there are a lot of
invaluable information about the inner workings of the analyzer, that I
hokein created this revision.
hokein added a reviewer: ioeric.
Repository:
rC Clang
https://reviews.llvm.org/D54104
Files:
lib/Tooling/AllTUsExecution.cpp
Index: lib/Tooling/AllTUsExecution.cpp
===
--- lib/Tooling/AllTUsExecut
ioeric accepted this revision.
ioeric added inline comments.
This revision is now accepted and ready to land.
Comment at: lib/Tooling/AllTUsExecution.cpp:103
+if (!RegexFilter.match(File))
+ continue;
+Files.push_back(File);
nit: `if (match) then pus
LutsenkoDanil planned changes to this revision.
LutsenkoDanil added a comment.
@ilya-biryukov, For example, VSCode saves all changed files by default when you
press Ctrl+Shift+B (if build task configured for project).
@klimek If behavior will be configurable, is it ok for you?
@sammccall Curren
klimek added a comment.
In https://reviews.llvm.org/D54077#1287282, @LutsenkoDanil wrote:
> @klimek If behavior will be configurable, is it ok for you?
I have the same concerns as Sam for making this an option.
> @sammccall Current behavior may confuse new users, since, other IDEs mostly
> (a
hokein updated this revision to Diff 172585.
hokein marked 2 inline comments as done.
hokein added a comment.
Address comments.
Repository:
rC Clang
https://reviews.llvm.org/D54104
Files:
lib/Tooling/AllTUsExecution.cpp
Index: lib/Tooling/AllTUsExecution.cpp
=
Author: hokein
Date: Mon Nov 5 07:08:00 2018
New Revision: 346135
URL: http://llvm.org/viewvc/llvm-project?rev=346135&view=rev
Log:
[Tooling] Correct the total number of files being processed when `filter` is
provided.
Reviewers: ioeric
Subscribers: cfe-commits
Differential Revision: https://
This revision was automatically updated to reflect the committed changes.
Closed by commit rL346135: [Tooling] Correct the total number of files being
processed when `filter` is… (authored by hokein, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.l
Author: abeserminji
Date: Mon Nov 5 02:22:51 2018
New Revision: 346124
URL: http://llvm.org/viewvc/llvm-project?rev=346124&view=rev
Log:
[mips][msa] Fix broken test
Test builtins-mips-msa-error.c wasn't reporting errors.
This patch fixes the test, so further test cases can be added.
Differentia
simark added a comment.
In https://reviews.llvm.org/D54077#1287153, @klimek wrote:
> I'm in yet another camp: I carefully save when I have something that is
> correct enough syntax, so I only want errors from with changes from the exact
> file I'm editing and the rest of the files in saved stat
aaron.ballman added inline comments.
Comment at: lib/StaticAnalyzer/Core/SarifDiagnostics.cpp:237-242
+#define GET_CHECKERS
+#define CHECKER(FULLNAME, CLASS, CXXFILE, HELPTEXT, GROUPINDEX, HIDDEN)
\
+ .Case(FULLNAME, HELPTEXT)
+#include "clang/StaticAnalyzer/Checkers/Che
aaron.ballman added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp:96
+ // Get the value of the expression to cast.
+ const auto ValueToCastOptional =
+ C.getSVal(CE->getSubExpr()).getAs();
NoQ wrote:
> aaron.ballman
ioeric created this revision.
ioeric added a reviewer: ilya-biryukov.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay.
For example, when anonymous namespace is present, duplicated namespaces might be
generated for the enclosing namespace.
Repository:
rCTE Clang Tool
ioeric abandoned this revision.
ioeric marked an inline comment as done.
ioeric added inline comments.
Comment at: clangd/CodeComplete.cpp:563
for (auto *Context : CCContext.getVisitedContexts()) {
- if (isa(Context))
+ if (isa(Context)) {
Info.AccessibleS
Szelethus added inline comments.
Comment at: lib/StaticAnalyzer/Core/SarifDiagnostics.cpp:237-242
+#define GET_CHECKERS
+#define CHECKER(FULLNAME, CLASS, CXXFILE, HELPTEXT, GROUPINDEX, HIDDEN)
\
+ .Case(FULLNAME, HELPTEXT)
+#include "clang/StaticAnalyzer/Checkers/Checker
ioeric updated this revision to Diff 172591.
ioeric marked an inline comment as done.
ioeric added a comment.
- Remove addressed FIXME.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53933
Files:
clangd/FindSymbols.cpp
clangd/index/Index.h
clangd/index/MemIndex.cpp
clan
aaron.ballman added inline comments.
Comment at: lib/StaticAnalyzer/Core/SarifDiagnostics.cpp:237-242
+#define GET_CHECKERS
+#define CHECKER(FULLNAME, CLASS, CXXFILE, HELPTEXT, GROUPINDEX, HIDDEN)
\
+ .Case(FULLNAME, HELPTEXT)
+#include "clang/StaticAnalyzer/Checkers/Che
hokein created this revision.
hokein added a reviewer: ioeric.
Herald added subscribers: kadircet, arphaman, jkorous, MaskRay, ilya-biryukov.
When the limit of FuzzyFindRequest is not specified, MemIndex and
DexIndex will set it MAX_UINT, which doesn't make sense in the active tool dexp
(we might
ioeric added inline comments.
Comment at: clangd/index/dex/dexp/Dexp.cpp:128
};
cl::opt Limit{
"limit",
I think we should make it configurable like what we do here.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D54106
__
russellmcc added a comment.
Bump! Still looking for feedback on the latest round of changes.
https://reviews.llvm.org/D40988
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Lekensteyn created this revision.
Lekensteyn added reviewers: cfe-commits, pcc.
Like clang-check, continue even if some source files were not found in
the compilation database. This makes it possible to do search for
matching sources and query files that exist in the compilation db:
clang-query
Lekensteyn added inline comments.
Comment at: test/clang-query/database-missing-entry.c:9
+// CHECK: deliberate parsing error
+// CHECK: Skipping {{.*[/\\]}}missing.c. Compile command not found.
+// CHECK-NOT-EXIST: Error while processing {{.*[/\\]}}missing.c.
On
ioeric added inline comments.
Comment at: clangd/index/Background.cpp:235
+IndexedFileDigests[Path] = FilesToUpdate.lookup(Path);
+IndexedSymbols.update(Path,
+ make_unique(std::move(Syms).build()),
kadircet wrote:
> This call is
kzhuravl added a comment.
ping
https://reviews.llvm.org/D53223
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
hokein updated this revision to Diff 172598.
hokein added a comment.
Address review comment.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D54106
Files:
clangd/index/dex/dexp/Dexp.cpp
Index: clangd/index/dex/dexp/Dexp.cpp
hokein added inline comments.
Comment at: clangd/index/dex/dexp/Dexp.cpp:128
};
cl::opt Limit{
"limit",
ioeric wrote:
> I think we should make it configurable like what we do here.
Made it configurable for `Lookup`. `Limit` option is used to restrict
benhamilton created this revision.
benhamilton added a reviewer: krasimir.
Herald added a subscriber: cfe-commits.
To handle diagnosing bugs where ObjCHeaderStyleGuesser guesses
wrong, this diff adds a bit more debug logging to the Objective-C
language guesser.
Repository:
rC Clang
https://re
krasimir accepted this revision.
krasimir added a comment.
This revision is now accepted and ready to land.
Helpful! Thanks!
Repository:
rC Clang
https://reviews.llvm.org/D54110
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists
Lekensteyn added a comment.
Before this patch, missing compilation database entries resulted in "Skipping
Compile command not found." which is assumed by the tests in this
clang-query patch: https://reviews.llvm.org/D54109
After this patch, a command is inferred, but this is not always des
Author: benhamilton
Date: Mon Nov 5 08:59:33 2018
New Revision: 346144
URL: http://llvm.org/viewvc/llvm-project?rev=346144&view=rev
Log:
[Format] Add debugging to ObjC language guesser
Summary:
To handle diagnosing bugs where ObjCHeaderStyleGuesser guesses
wrong, this diff adds a bit more debug
This revision was automatically updated to reflect the committed changes.
Closed by commit rC346144: [Format] Add debugging to ObjC language guesser
(authored by benhamilton, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D54110?vs=172599&id=172600#toc
Repository:
rC Clang
Szelethus added a comment.
Thanks for summarizing the problem so well in the summary! I should start doing
that too.
Comment at: lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp:343-346
+ // There should have originally been a symbol. If it was a concrete value
+ // to beg
krasimir created this revision.
Herald added a subscriber: cfe-commits.
The opening square of an inline asm clobber was being annotated as an ObjCExpr.
This caused, amongst other things, the ObjCGuesser to guess header files
containing that pattern as ObjC files.
Repository:
rC Clang
https:/
krasimir updated this revision to Diff 172602.
krasimir added a comment.
- Clean-up
Repository:
rC Clang
https://reviews.llvm.org/D54111
Files:
lib/Format/TokenAnnotator.cpp
unittests/Format/FormatTest.cpp
Index: unittests/Format/FormatTest.cpp
=
MaskRay created this revision.
MaskRay added reviewers: phosek, EricWF, mcgrathr.
Herald added a subscriber: cfe-commits.
The surrounding --push-state saves the "-Bdynamic" state across ld.bfd, gold
and lld.
lld saves the least states, but the intersection of these linkers is
--as-needed -Bdynam
t-tye accepted this revision.
t-tye added a comment.
This revision is now accepted and ready to land.
LGTM
https://reviews.llvm.org/D53223
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-com
Szelethus added a comment.
Please reupload with full context.
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2369
ProgramStateRef state = C.getState();
- RegionStateTy RS = state->get();
+ RegionStateTy OldRS = state->get();
RegionStateTy::Factory &F = state-
MaskRay added a comment.
I'm unclear if you also want as-needed `-lm` or if you accept static `-lm`
Repository:
rC Clang
https://reviews.llvm.org/D54112
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman
benhamilton accepted this revision.
benhamilton added a comment.
This revision is now accepted and ready to land.
In the diff description, please fix the typo: `Do not threat the asm clobber`
-> `Do not treat the asm clobber`
Comment at: unittests/Format/FormatTest.cpp:12756-1
Author: mattd
Date: Mon Nov 5 09:25:26 2018
New Revision: 346146
URL: http://llvm.org/viewvc/llvm-project?rev=346146&view=rev
Log:
[AST] Get aliased type info from an aliased TemplateSpecialization.
Summary:
Previously the TemplateSpecialization instance for 'template_alias', in the
example bel
This revision was automatically updated to reflect the committed changes.
Closed by commit rC346146: [AST] Get aliased type info from an aliased
TemplateSpecialization. (authored by mattd, committed by ).
Repository:
rC Clang
https://reviews.llvm.org/D54048
Files:
include/clang/AST/Type.h
modocache accepted this revision.
modocache added a comment.
This revision is now accepted and ready to land.
Sorry to have let this languish! LGTM.
Repository:
rC Clang
https://reviews.llvm.org/D53212
___
cfe-commits mailing list
cfe-commits@lis
echristo added a comment.
In https://reviews.llvm.org/D53919#1282994, @hjl.tools wrote:
> In https://reviews.llvm.org/D53919#1282952, @efriedma wrote:
>
> > With both 3.3 and trunk (I don't have a 7.0 handy; I can build it if it
> > would be helpful):
>
>
> Please try clang 2.6 on both testcases
hjl.tools added a comment.
In https://reviews.llvm.org/D53919#1287510, @echristo wrote:
> In https://reviews.llvm.org/D53919#1282994, @hjl.tools wrote:
>
> > In https://reviews.llvm.org/D53919#1282952, @efriedma wrote:
> >
> > > With both 3.3 and trunk (I don't have a 7.0 handy; I can build it if
echristo added a comment.
In https://reviews.llvm.org/D52296#1285328, @grimar wrote:
> In https://reviews.llvm.org/D52296#1284130, @probinson wrote:
>
> > In https://reviews.llvm.org/D52296#1283691, @grimar wrote:
> >
> > > Nice :)
> > > So seems the last unresolved question left is the naming
On Mon, Nov 5, 2018 at 9:45 AM H.J Lu via Phabricator <
revi...@reviews.llvm.org> wrote:
> hjl.tools added a comment.
>
> In https://reviews.llvm.org/D53919#1287510, @echristo wrote:
>
> > In https://reviews.llvm.org/D53919#1282994, @hjl.tools wrote:
> >
> > > In https://reviews.llvm.org/D53919#12
neerajksingh updated this revision to Diff 172615.
neerajksingh added a comment.
Make it clear in the documentation that the /clang flags are added to the end.
https://reviews.llvm.org/D53457
Files:
docs/UsersManual.rst
include/clang/Driver/CLCompatOptions.td
include/clang/Driver/Driver.h
george.karpenkov added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2369
ProgramStateRef state = C.getState();
- RegionStateTy RS = state->get();
+ RegionStateTy OldRS = state->get();
RegionStateTy::Factory &F = state->get_context();
Szelethus added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2369
ProgramStateRef state = C.getState();
- RegionStateTy RS = state->get();
+ RegionStateTy OldRS = state->get();
RegionStateTy::Factory &F = state->get_context();
---
Could you link to/quote the warnings - might be helpful to understanding
what's being addressed here
On Tue, Oct 30, 2018 at 10:00 PM Bill Wendling via cfe-commits <
cfe-commits@lists.llvm.org> wrote:
> Author: void
> Date: Tue Oct 30 21:58:34 2018
> New Revision: 345695
>
> URL: http://llvm.org/
If ThinLTO doesn't pass the machine verifier - should it maybe be turned
off at the thinlto level in general, rather than for this specific test?
On Tue, Oct 30, 2018 at 5:20 AM Francis Visoiu Mistrih via cfe-commits <
cfe-commits@lists.llvm.org> wrote:
> Author: thegameg
> Date: Tue Oct 30 05:18
Szelethus added a comment.
In https://reviews.llvm.org/D52790#1285039, @NoQ wrote:
> Looks great, let's land?
I'll probably land it after part 5, in order to ease on rebasing.
> Not sure if i already asked - am i understanding correctly that this is a
> "poor-man's" support for macro expansio
rnk added inline comments.
Comment at: include/clang/Basic/BuiltinsARM.def:270
+TARGET_HEADER_BUILTIN(_InterlockedCompareExchange64_nf, "LLiLLiD*LLiLLi",
"nh", "intrin.h", ALL_MS_LANGUAGES, "")
+TARGET_HEADER_BUILTIN(_InterlockedCompareExchange64_rel, "LLiLLiD*LLiLLi",
"nh", "
NoQ updated this revision to Diff 172629.
NoQ added a comment.
Re-upload with context. Whoops.
https://reviews.llvm.org/D54013
Files:
lib/StaticAnalyzer/Checkers/MallocChecker.cpp
Index: lib/StaticAnalyzer/Checkers/MallocChecker.cpp
==
NoQ added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2369
ProgramStateRef state = C.getState();
- RegionStateTy RS = state->get();
+ RegionStateTy OldRS = state->get();
RegionStateTy::Factory &F = state->get_context();
Szel
ztamas added inline comments.
Comment at: clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp:142
+ if (LoopVar->getType() != LoopIncrement->getType())
+return; // We matched the loop variable incorrectly
+
JonasToth wrote:
> Does this try to ensure a precondi
ztamas added inline comments.
Comment at: clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp:45
+
+ // We need to catch only those comparisons which contain any integer cast
+ StatementMatcher LoopVarConversionMatcher =
JonasToth wrote:
> missing full stop.
Sorr
mgrang added inline comments.
Comment at: include/clang/Basic/BuiltinsARM.def:270
+TARGET_HEADER_BUILTIN(_InterlockedCompareExchange64_nf, "LLiLLiD*LLiLLi",
"nh", "intrin.h", ALL_MS_LANGUAGES, "")
+TARGET_HEADER_BUILTIN(_InterlockedCompareExchange64_rel, "LLiLLiD*LLiLLi",
"nh"
ztamas updated this revision to Diff 172635.
ztamas added a comment.
- Add a range-based loop test case
- Restructure test cases a bit
- Fix-up comments, position, punctuation
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53974
Files:
clang-tidy/bugprone/BugproneTidyModule.c
george.karpenkov added a comment.
@lebedev.ri @erichkeane The test fails for me on macOS whenever asan and ubsan
are both enabled.
The failure is stack overflow at stack frame 943 (? maybe asan usage enforces
lower stack size?)
Repository:
rC Clang
https://reviews.llvm.org/D50050
___
steleman updated this revision to Diff 172638.
steleman added a comment.
- changed the -fveclib= argument value to 'sleefgnuabi'.
- added atan2 and pow.
- spreadsheet with comparison between libm and sleef is here:
https://docs.google.com/spreadsheets/d/1lcpESCnuzEoTl_XHBqE9FLL0tXJB_tZGR8yciCx1yj
1 - 100 of 173 matches
Mail list logo
