Hahnfeld added a comment.
In https://reviews.llvm.org/D45449#1062642, @tra wrote:
> We could use specific instructions for what exactly one needs to do in order
> to use relocatable device code in practice. Could be a separate patch.
Maybe I should add an example to https://llvm.org/docs/Compi
courbet added a comment.
In https://reviews.llvm.org/D38455#1061681, @JonasToth wrote:
> Could you please add some tests that include user defined literals and there
> interaction with other literals. We should catch narrowing conversions from
> them, too.
User defined literals do not have th
courbet added a comment.
In https://reviews.llvm.org/D38455#1061926, @pfultz2 wrote:
> This seems like it would be nice to have in `bugprone-*` as well.
Sure. Is it OK to add a dependency from
`clang-tidy/bugprone/BugproneTidyModule.cpp` to stuff in
`clang-tidy/cpp-coreguidelines` ? Is there
courbet updated this revision to Diff 141800.
courbet added a comment.
- Simplify matcher expression
- Add other binary operators
- Add more tests.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D38455
Files:
clang-tidy/cppcoreguidelines/CMakeLists.txt
clang-tidy/cppcoreguid
JonasToth added a comment.
> Sure. Is it OK to add a dependency from
> clang-tidy/bugprone/BugproneTidyModule.cpp to stuff in
> clang-tidy/cpp-coreguidelines ? Is there an existing example of this ?
Take a look in the hicpp module, almost everything there is aliased :)
Repository:
rCTE Clan
courbet updated this revision to Diff 141801.
courbet added a comment.
- Add NarrowingConversionsCheck to bugprone module.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D38455
Files:
clang-tidy/bugprone/BugproneTidyModule.cpp
clang-tidy/bugprone/CMakeLists.txt
clang-tidy/
courbet added a comment.
In https://reviews.llvm.org/D38455#1062718, @JonasToth wrote:
> > Sure. Is it OK to add a dependency from
> > clang-tidy/bugprone/BugproneTidyModule.cpp to stuff in
> > clang-tidy/cpp-coreguidelines ? Is there an existing example of this ?
>
> Take a look in the hicpp m
Athosvk added a comment.
I'm a bit late on this, but I'd say that YAML is usually not a 'final' format.
What would be the use-cases for this? And if is meant as an alternative
intermediate format, why not instead of having one big file, have one file per
input file? Just wondering what the part
Athosvk added a comment.
I'm a bit late on this, but I'd say that YAML is usually not a 'final' format.
What would be the use-cases for this? And if is meant as an alternative
intermediate format, why not instead of having one big file, have one file per
input file? Just wondering what the part
jdemeule added a comment.
Ok, I understand the problem.
Previously, during the deduplication step, replacements per file where sorted
and it is not the case anymore.
That means now, clang-apply-replacement is highly dependant of reading file
order during error reporting.
I mainly see 2 options (
ilya-biryukov created this revision.
ilya-biryukov added reviewers: hokein, sammccall, klimek.
Herald added subscribers: MaskRay, ioeric, jkorous-apple.
This avoids storing intermediate symbols in memory, most of which are
duplicates.
The resulting .yaml file is ~120MB, while intermediate symbols
hokein added a comment.
Thanks for digging it out!
In upstream, we use `InMemoryToolResults` which saves all duplicated
"std::string"s into the memory, I think we could optimize `InMemoryToolResults`
by using Arena to keep the memory low, I will try it to see whether it can
reduce the memory.
hokein created this revision.
Herald added subscribers: cfe-commits, klimek.
Repository:
rC Clang
https://reviews.llvm.org/D45479
Files:
include/clang/Tooling/Execution.h
lib/Tooling/Execution.cpp
Index: lib/Tooling/Execution.cpp
==
dberris added a comment.
In the interest of better documentation, can you please update the description
of this change to reflect what we're trying to achieve here?
Something more descriptive than "libFuzzer OpenBSD Support" that substantiates
the change, along with related changes to other pro
Author: avt77
Date: Tue Apr 10 03:34:13 2018
New Revision: 329684
URL: http://llvm.org/viewvc/llvm-project?rev=329684&view=rev
Log:
-ftime-report switch support in Clang.
The current support of the feature produces only 2 lines in report:
-Some general Code Generation Time;
-Total time of Backen
Author: sammccall
Date: Tue Apr 10 03:36:46 2018
New Revision: 329685
URL: http://llvm.org/viewvc/llvm-project?rev=329685&view=rev
Log:
[Tooling] fix UB when interpolating compile commands with an empty index
Modified:
cfe/trunk/lib/Tooling/InterpolatingCompilationDatabase.cpp
Modified: cfe/
Apologies, missed this. r329685 should fix.
On Tue, Apr 10, 2018 at 12:53 AM Galina Kistanova
wrote:
> Hello Sam,
>
> It looks like this commit added broken tests to one of our builders:
>
> http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win/builds/8957/steps/test-check-all
ilya-biryukov created this revision.
ilya-biryukov added reviewers: sammccall, hokein.
Herald added subscribers: MaskRay, ioeric, jkorous-apple, klimek.
Previsouly, class completions items from the index were missing
template parameters in both the snippet and the label.
Repository:
rCTE Clang
hokein updated this revision to Diff 141822.
hokein added a comment.
Update.
Repository:
rC Clang
https://reviews.llvm.org/D45479
Files:
include/clang/Tooling/Execution.h
lib/Tooling/AllTUsExecution.cpp
lib/Tooling/Execution.cpp
Index: lib/Tooling/Execution.cpp
==
GBuella added a comment.
See: https://reviews.llvm.org/rL329689
Repository:
rC Clang
https://reviews.llvm.org/D43815
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
GBuella closed this revision.
GBuella added a comment.
Commited r329688
https://reviews.llvm.org/rL329689
Repository:
rC Clang
https://reviews.llvm.org/D43815
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/m
malcolm.parsons added a comment.
In https://reviews.llvm.org/D43764#1062748, @jdemeule wrote:
> Ok, I understand the problem.
> Previously, during the deduplication step, replacements per file where
> sorted and it is not the case anymore.
> That means now, clang-apply-replacement is highly de
ilya-biryukov added a comment.
ObjC++ definitely seems like a nicer default. Unfortunately, we'll start
getting ObjC++ completion results, which may be confusing. But that seems like
a smaller evil.
Is there an easy way to add a regression test that checks we don't get any
errors for headers w
kosarev created this revision.
kosarev added reviewers: az, sbaranga, t.p.northover.
kosarev added a project: clang.
Herald added subscribers: javed.absar, rengolin.
https://reviews.llvm.org/D45483
Files:
include/clang/Basic/arm_neon.td
test/CodeGen/aarch64-neon-2velem.c
Index: test/CodeGen
a.sidorin added a comment.
> The ultimate solution would probably be to add a fake cloned asm statement to
> the CFG (instead of the real asm statement) that would point to the correct
> output child-expression(s) that are untouched themselves but simply have
> their noop casts removed.
I have
Author: avt77
Date: Tue Apr 10 05:17:01 2018
New Revision: 329693
URL: http://llvm.org/viewvc/llvm-project?rev=329693&view=rev
Log:
The test was fixed.
Modified:
cfe/trunk/test/Frontend/ftime-report-template-decl.cpp
Modified: cfe/trunk/test/Frontend/ftime-report-template-decl.cpp
URL:
http
markoshorro added a comment.
This feature should have been merged time ago, any news?
Repository:
rL LLVM
https://reviews.llvm.org/D28462
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-c
davezarzycki created this revision.
davezarzycki added reviewers: mspertus, beanz, gottesmm.
Herald added a subscriber: mgorny.
Due to recent timer changes, we need to add LLVM's `Core` library to avoid
undefined symbol errors at link time (`llvm::TimePassesIsEnabled`) when using
LLVM's BUILD_SH
thakis added a comment.
Can you link to the recent timer changes?
Repository:
rC Clang
https://reviews.llvm.org/D45485
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Author: nico
Date: Tue Apr 10 06:14:03 2018
New Revision: 329695
URL: http://llvm.org/viewvc/llvm-project?rev=329695&view=rev
Log:
s/LLVM_ON_WIN32/_WIN32/, clang-tools-extra
LLVM_ON_WIN32 is set exactly with MSVC and MinGW (but not Cygwin) in
HandleLLVMOptions.cmake, which is where _WIN32 defined
davezarzycki added a comment.
I haven't proven that r329684 is the source of the regression, but it is the
only timer related change in the last 24 hours as far as I can tell.
Repository:
rC Clang
https://reviews.llvm.org/D45485
___
cfe-commits
hans accepted this revision.
hans added a comment.
This revision is now accepted and ready to land.
lgtm
https://reviews.llvm.org/D45165
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commi
sammccall added a comment.
Will try to add a test.
Comment at: clangd/GlobalCompilationDatabase.cpp:24
+ // Parsing as Objective C++ is friendly to more cases.
+ if (llvm::sys::path::extension(File) == ".h")
+Argv.push_back("-xobjective-c++-header");
ilya
ilya-biryukov added a comment.
This definitely helps with memory consumption of clangd's
global-symbol-indexer, I can now run it over LLVM on MacBook without it
swapping out of RAM. The physical memory usage goes up to 3GB at some point,
before it was > 20GB.
Strictly speaking, interning the s
thakis added a comment.
Thanks for the pointer! I commented on https://reviews.llvm.org/D43578; I'm
hoping there's a fix that doesn't require making so many libs depend on clang's
IR library.
Repository:
rC Clang
https://reviews.llvm.org/D45485
___
thakis added a comment.
r329698, thanks!
https://reviews.llvm.org/D45165
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Author: nico
Date: Tue Apr 10 06:36:38 2018
New Revision: 329698
URL: http://llvm.org/viewvc/llvm-project?rev=329698&view=rev
Log:
Use llvm::sys::fs::real_path() in clang.
No expected behavior change.
https://reviews.llvm.org/D45165
Modified:
cfe/trunk/lib/Basic/FileManager.cpp
cfe/trunk
aaron.ballman added inline comments.
Comment at: clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp:21
+
+static std::string exprToStr(const Expr *Expr,
+ const MatchFinder::MatchResult &Result) {
Do not name the parameter with the s
thakis added a comment.
Also, please add cfe-commits to clang changes. Since this wasn't here and the
patch wasn't seen by clang folks, since ftime-report-template-decl.cppwarnings
is still failing on the bots (e.g.
http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-f
sammccall added inline comments.
Comment at: include/clang/Tooling/Execution.h:55
class InMemoryToolResults : public ToolResults {
public:
please add a high-level comment explaining what this is caching and when we
expect it to work well.
Repository:
rC
aaron.ballman added a reviewer: rsmith.
aaron.ballman added inline comments.
Comment at: lib/Lex/Lexer.cpp:1667-1668
+ assert(CurPtr < BufferEnd && "eof at completion point");
+ while (isIdentifierBody(*CurPtr))
+++CurPtr;
+ BufferPtr = CurPtr;
aaron.ballman added inline comments.
Comment at: lib/Basic/Targets/PPC.h:349
LongLongAlign = 32;
+UseSignedCharForObjCBool = true;
resetDataLayout("E-m:o-p:32:32-f64:32:64-n32");
Do you need to specify this? Isn't it handled by the `TargetInfo` cons
Author: gbuella
Date: Tue Apr 10 04:20:05 2018
New Revision: 329689
URL: http://llvm.org/viewvc/llvm-project?rev=329689&view=rev
Log:
CodeGen tests - typo fixes NFC
Modified:
cfe/trunk/test/CodeGen/avx512-reduceMinMaxIntrin.c
cfe/trunk/test/CodeGen/avx512f-builtins.c
Modified: cfe/trunk/
t.p.northover accepted this revision.
t.p.northover added a comment.
This revision is now accepted and ready to land.
Looks fine to me.
https://reviews.llvm.org/D45483
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi
davezarzycki added a comment.
Great! I'll revisit this patch in a week and see if it is still needed.
Hopefully the author of https://reviews.llvm.org/D43578 will make this patch
unnecessary by then.
Repository:
rC Clang
https://reviews.llvm.org/D45485
___
alexfh added a comment.
In https://reviews.llvm.org/D38455#1062722, @courbet wrote:
> In https://reviews.llvm.org/D38455#1062718, @JonasToth wrote:
>
> > > Sure. Is it OK to add a dependency from
> > > clang-tidy/bugprone/BugproneTidyModule.cpp to stuff in
> > > clang-tidy/cpp-coreguidelines ?
thakis added a comment.
This landing made our clang trunk bots do an evaluation of this warning :-P It
fired 8 times, all false positives, and all from unit tests testing that
operator= works for self-assignment.
(https://chromium-review.googlesource.com/c/chromium/src/+/1000856 has the
exact
This revision was automatically updated to reflect the committed changes.
Closed by commit rC329701: [X86] Disable SGX for Skylake Server (authored by
GBuella, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D45058?vs=140302&id=141839#toc
Repository:
rL LLVM
https://review
This revision was automatically updated to reflect the committed changes.
Closed by commit rL329701: [X86] Disable SGX for Skylake Server (authored by
GBuella, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D45058
Files:
cfe/trunk/lib/B
Author: nico
Date: Tue Apr 10 07:08:25 2018
New Revision: 329702
URL: http://llvm.org/viewvc/llvm-project?rev=329702&view=rev
Log:
Attempt to fix Windows build after r329698.
Modified:
cfe/trunk/lib/Basic/FileManager.cpp
Modified: cfe/trunk/lib/Basic/FileManager.cpp
URL:
http://llvm.org/vie
obfuscated marked an inline comment as done.
obfuscated added a comment.
Ping?
https://reviews.llvm.org/D44765
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
lebedev.ri added a comment.
In https://reviews.llvm.org/D44883#1063003, @thakis wrote:
> This landing made our clang trunk bots do an evaluation of this warning :-P
> It fired 8 times, all false positives, and all from unit tests testing that
> operator= works for self-assignment.
> (https://c
hokein updated this revision to Diff 141842.
hokein edited the summary of this revision.
hokein added a comment.
Add a comment.
Repository:
rC Clang
https://reviews.llvm.org/D45479
Files:
include/clang/Tooling/Execution.h
lib/Tooling/AllTUsExecution.cpp
lib/Tooling/Execution.cpp
Inde
sammccall accepted this revision.
sammccall added a comment.
Sorry I lost track of this. LGTM!
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D44764
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bi
davezarzycki added a comment.
I think this change is incomplete. The following clang test fails on my SKX
machine: `Preprocessor/predefined-arch-macros.c`
Line 895: `// CHECK_SKX_M32: #define __SGX__ 1`
Repository:
rL LLVM
https://reviews.llvm.org/D45058
_
juliehockett added a comment.
In https://reviews.llvm.org/D43667#1062746, @Athosvk wrote:
> I'm a bit late on this, but I'd say that YAML is usually not a 'final'
> format. What would be the use-cases for this? And if is meant as an
> alternative intermediate format, why not instead of having o
GBuella created this revision.
GBuella added reviewers: craig.topper, davezarzycki.
Herald added a subscriber: cfe-commits.
Fix test case - corresponding to r329701
Repository:
rC Clang
https://reviews.llvm.org/D45488
Files:
test/Preprocessor/predefined-arch-macros.c
Index: test/Preproce
On Tue, Apr 10, 2018 at 7:21 AM Roman Lebedev via Phabricator <
revi...@reviews.llvm.org> wrote:
> lebedev.ri added a comment.
>
> In https://reviews.llvm.org/D44883#1063003, @thakis wrote:
>
> > This landing made our clang trunk bots do an evaluation of this warning
> :-P It fired 8 times, all fa
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.
LGTM aside from a minor docs nit.
Comment at: docs/clang-tidy/checks/fuchsia-header-anon-namespaces.rst:6
+fuchsia-header-anon-namespaces
+=
davezarzycki added a comment.
I don't think this change is correct. To the best of my knowledge, SKL does
support SGX but SKX does not.
Repository:
rC Clang
https://reviews.llvm.org/D45488
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
yaxunl created this revision.
yaxunl added reviewers: tra, Hahnfeld.
This patch is split from https://reviews.llvm.org/D45212
https://reviews.llvm.org/D45489
Files:
include/clang/Driver/Types.def
lib/Driver/Driver.cpp
lib/Driver/Types.cpp
Index: lib/Driver/Types.cpp
GBuella updated this revision to Diff 141853.
https://reviews.llvm.org/D45488
Files:
test/Preprocessor/predefined-arch-macros.c
Index: test/Preprocessor/predefined-arch-macros.c
===
--- test/Preprocessor/predefined-arch-macros.c
Athosvk added a comment.
In https://reviews.llvm.org/D43667#1063049, @juliehockett wrote:
> In https://reviews.llvm.org/D43667#1062746, @Athosvk wrote:
>
> > I'm a bit late on this, but I'd say that YAML is usually not a 'final'
> > format. What would be the use-cases for this? And if is meant a
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.
LGTM with some minor commenting nits.
Comment at: lib/CodeGen/CGBuiltin.cpp:992
+
+// We check whether we are in a recursive type
+if (CanonicalType->is
GBuella added a comment.
In https://reviews.llvm.org/D45488#1063067, @davezarzycki wrote:
> I don't think this change is correct. To the best of my knowledge, SKL does
> support SGX but SKX does not.
llvm-lit test/Preprocessor/predefined-arch-macros.c passes now.
I didn't know tests are not ru
This revision was automatically updated to reflect the committed changes.
Closed by commit rL329710: [X86] Disable SGX for Skylake Server - CPP test
(authored by GBuella, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D45488?vs=141853&
Author: gbuella
Date: Tue Apr 10 07:04:21 2018
New Revision: 329701
URL: http://llvm.org/viewvc/llvm-project?rev=329701&view=rev
Log:
[X86] Disable SGX for Skylake Server
Reviewers: craig.topper, zvi, echristo
Reviewed By: craig.topper
Differential Revision: https://reviews.llvm.org/D45058
Mo
Author: gbuella
Date: Tue Apr 10 08:03:03 2018
New Revision: 329710
URL: http://llvm.org/viewvc/llvm-project?rev=329710&view=rev
Log:
[X86] Disable SGX for Skylake Server - CPP test
Summary: Fix test case - corresponding to r329701
Reviewers: craig.topper, davezarzycki
Reviewed By: davezarzycki
alexfh added inline comments.
Comment at: lib/Format/FormatTokenLexer.cpp:371
FormatToken *Macro = Tokens[Tokens.size() - 4];
- if (Macro->TokenText != "_T")
+ if (std::find(Style.TMacros.begin(), Style.TMacros.end(), Macro->TokenText)
==
+ Style.TMacros.end())
---
"False positive" means "warning fires but didn't find anything
interesting", not "warning fires while being technically correct". So all
these instances do count as false positives.
clang tries super hard to make sure that every time a warning fires, it is
useful for a dev to fix it. If you build
alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.
Looks good. Thank you!
https://reviews.llvm.org/D45405
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mail
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.
LG aside from a diagnostic wording nit.
Comment at: clang-tidy/hicpp/SignedBitwiseCheck.cpp:92
+ if (N.getNodeAs("std_type"))
+diag(Location, "shifting a v
rsmith added a comment.
In https://reviews.llvm.org/D43578#1062984, @thakis wrote:
> Also, please add cfe-commits to clang changes. Since this wasn't here and the
> patch wasn't seen by clang folks, since
> ftime-report-template-decl.cppwarnings is still failing on the bots (e.g.
> http://lab.
arichardson added a comment.
I'm also often restricted to using printf for debugging so this looks really
useful!
However, before committing this I feel like the test should also verify that
the format strings that are generated are sensible.
Also what should happens when you have enum members
On Mon, Apr 9, 2018 at 8:09 PM Zinovy Nis wrote:
> I had compilation errors here on MVSV@PSP and for ARM:
>
>
>
> FAILED: /usr/bin/c++ -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GNU_SOURCE
> -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS
> -Itools/clang/too
Looks you are right, but I was in hurry trying to fix build bot failures
ASAP.
вт, 10 апр. 2018 г. в 18:28, Alexander Kornienko :
> On Mon, Apr 9, 2018 at 8:09 PM Zinovy Nis wrote:
>
>> I had compilation errors here on MVSV@PSP and for ARM:
>>
>>
>>
>> FAILED: /usr/bin/c
MTC created this revision.
MTC added reviewers: NoQ, george.karpenkov, a.sidorin, seaneveson, szepet.
Herald added subscribers: cfe-commits, rnkovacs, xazax.hun.
MTC edited the summary of this revision.
`this` pointer is not an l-value, although we have modeled `CXXThisRegion` for
`this` pointer,
alexfh added a comment.
In https://reviews.llvm.org/D45392#1061960, @Wizard wrote:
> In https://reviews.llvm.org/D45392#1061433, @alexfh wrote:
>
> > I wonder whether the readability-identifier-naming check could be extended
> > to support this use case instead of adding a new check specifically
alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.
LG. Thanks!
https://reviews.llvm.org/D45059
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinf
I totally get it, no worries. But if you have free cycles, it would be nice
to brush this code up again.
On Tue, Apr 10, 2018 at 5:32 PM Zinovy Nis wrote:
> Looks you are right, but I was in hurry trying to fix build bot failures
> ASAP.
>
> вт, 10 апр. 2018 г. в 18:28, Alexander Kornienko :
>
>
yaxunl updated this revision to Diff 141863.
yaxunl marked 2 inline comments as done.
yaxunl edited the summary of this revision.
yaxunl added a comment.
Separate file type changes to another patch.
https://reviews.llvm.org/D45212
Files:
include/clang/Driver/Options.td
include/clang/Driver/
Author: avt77
Date: Tue Apr 10 08:45:43 2018
New Revision: 329714
URL: http://llvm.org/viewvc/llvm-project?rev=329714&view=rev
Log:
I removed the failed test.
Removed:
cfe/trunk/test/Frontend/ftime-report-template-decl.cpp
Removed: cfe/trunk/test/Frontend/ftime-report-template-decl.cpp
URL:
davidxl added a comment.
With this change, gcov_flush will resolve to the copy in the main executable
for each shared library -- this is not the desired the behavior.
https://reviews.llvm.org/D45454
___
cfe-commits mailing list
cfe-commits@lists.l
yaxunl added a comment.
In https://reviews.llvm.org/D45212#1060628, @Hahnfeld wrote:
> Can this revision be split further? The summary mentions many things that
> might make up multiple independent changes...
I separated file type changes to https://reviews.llvm.org/D45489 and deferred
some o
OK)
вт, 10 апр. 2018 г. в 18:44, Alexander Kornienko :
> I totally get it, no worries. But if you have free cycles, it would be
> nice to brush this code up again.
>
>
> On Tue, Apr 10, 2018 at 5:32 PM Zinovy Nis wrote:
>
>> Looks you are right, but I was in hurry trying to fix build bot failure
If you have a concrete suggestion of how to suppress this warning for
user-defined operators just in unit tests, great. I don’t think 8
easily-suppressed warnings is an unacceptably large false-positive problem,
though. Most warnings have similar problems, and the standard cannot
possibly be “must
aaron.ballman added inline comments.
Comment at: clang-tidy/readability/FunctionSizeCheck.cpp:31
+!(isa(VD) || isa(VD)) &&
+!VD->getLocation().isMacroID())
+ Info.Variables++;
This isn't the restriction I was envisioning. I was thinking more
This is not an appropriate response to a test failing. Please revert the
entire patch, then put it up for review again and this time request review
from cfe-commits.
On Tue, 10 Apr 2018, 16:48 Andrew V. Tischenko via cfe-commits, <
cfe-commits@lists.llvm.org> wrote:
> Author: avt77
> Date: Tue Ap
jkorous-apple updated this revision to Diff 141865.
jkorous-apple added a comment.
Added test for decrement being disabled for bool.
Fixed comment in test (will put into separate NFC commit).
https://reviews.llvm.org/D44988
Files:
Sema/SemaOverload.cpp
SemaCXX/overloaded-builtin-operators.c
jkorous-apple added a comment.
Spot on with the negative test idea! Should've done that myself. Thanks.
https://reviews.llvm.org/D44988
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commit
It's more the fact that that's /all/ the warning improvement has found so
far. If it was 8 false positives & also found 80 actionable bugs/bad code,
that'd be one thing.
Now, admittedly, maybe it would help find bugs that people usually catch
with a unit test, etc but never make it to checked in c
Because *so far* it has been running on the existing code, which i guess
was already pretty warning free, and was run through the static analyzer,
which obviously should catch such issues.
Do you always use [clang] static analyzer, each time you build?
Great! It is indeed very good to do so. But d
Hahnfeld added a comment.
In https://reviews.llvm.org/D45212#1063186, @yaxunl wrote:
> In https://reviews.llvm.org/D45212#1060628, @Hahnfeld wrote:
>
> > Can this revision be split further? The summary mentions many things that
> > might make up multiple independent changes...
>
>
> I separated
On Tue, Apr 10, 2018 at 9:25 AM Roman Lebedev wrote:
> Because *so far* it has been running on the existing code, which i guess
> was already pretty warning free, and was run through the static analyzer,
which obviously should catch such issues.
>
Existing code this has been run over isn't nece
On Tue, Apr 10, 2018 at 12:13 PM, David Blaikie wrote:
> It's more the fact that that's /all/ the warning improvement has found so
> far. If it was 8 false positives & also found 80 actionable bugs/bad code,
> that'd be one thing.
>
Right. We used to hold ourselves to very high standards for war
Yeah, -Wself-assign in general is about catching a class of live-coding
errors which is pretty unlikely to ever make it into committed code, since
the code would need to be dead/redundant for the mistake to not be noticed.
So it’s specifically a rather poor match for the static analyzer, and I
woul
JonasToth added inline comments.
Comment at: clang-tidy/hicpp/SignedBitwiseCheck.cpp:92
+ if (N.getNodeAs("std_type"))
+diag(Location, "shifting a value of the standardized bitmask types");
+ else
aaron.ballman wrote:
> How about: "shifting a value of bitma
Any good ideas on how to evaluate it, then? (in addition to/other than
something like Google where we can track the diagnostic for - a few months?)
On Tue, Apr 10, 2018 at 9:34 AM John McCall wrote:
> Yeah, -Wself-assign in general is about catching a class of live-coding
> errors which is prett
a.sidorin updated this revision to Diff 141869.
a.sidorin edited the summary of this revision.
a.sidorin added a comment.
After thinking a bit more, I have removed the FIXME at all: dumping SVal is an
extremely rare event so this shouldn't affect the performance.
Repository:
rC Clang
https:/
a.sidorin updated this revision to Diff 141870.
a.sidorin added a comment.
Renamed the test file.
Repository:
rC Clang
https://reviews.llvm.org/D45417
Files:
lib/StaticAnalyzer/Core/SVals.cpp
test/Analysis/sval-dump-int128.c
Index: test/Analysis/sval-dump-int128.c
=
1 - 100 of 246 matches
Mail list logo
