Hahnfeld updated this revision to Diff 93213.
Hahnfeld added a comment.
Herald added a subscriber: rengolin.
Rebase and ping.
https://reviews.llvm.org/D30087
Files:
lib/Driver/ToolChains/CommonArgs.cpp
lib/Driver/ToolChains/CommonArgs.h
lib/Driver/ToolChains/Darwin.cpp
lib/Driver/ToolCh
mehdi_amini added a comment.
In https://reviews.llvm.org/D31272#711877, @EricWF wrote:
> In https://reviews.llvm.org/D31272#711876, @mehdi_amini wrote:
>
> > Strange. So installing the command line tools is not enough. It has to be
> > that CMAKE_OSX_SYSROOT is only defined on Apple internal ins
djasper added a comment.
Thank you for working on this. Unfortunately, this is done at the wrong level:
- You are using a separate pass, which means that as soon as the preprocessor
directives exceed the column limit, they won't be wrapped correctly.
- You aren't using Clang's Lexer to separate
t-tye added inline comments.
Comment at: lib/AST/ASTContext.cpp:9547-9555
+unsigned ASTContext::getTargetAddressSpace(unsigned AS) const {
+ // For OpenCL, the address space qualifier is 0 in AST.
+ if (AS == 0 && LangOpts.OpenCL)
+return getTargetInfo().getDataLayout().get
djasper added inline comments.
Comment at: lib/Format/WhitespaceManager.cpp:413
+
+ while (Param && !Param->is(tok::l_paren)) {
+if (!Param->is(tok::identifier) && !Param->is(tok::comma))
enyquist wrote:
> djasper wrote:
> > I think you should be able to use
EricWF added a comment.
In https://reviews.llvm.org/D31272#711876, @mehdi_amini wrote:
> Strange. So installing the command line tools is not enough. It has to be
> that CMAKE_OSX_SYSROOT is only defined on Apple internal install maybe?
Are you sure you're starting with a clean CMake build dir
mehdi_amini added a comment.
Strange. So installing the command line tools is not enough. It has to be that
CMAKE_OSX_SYSROOT is only defined on Apple internal install maybe?
Anyway, if you're exporting through the ABI list and have
LIBCXX_ENABLE_NEW_DELETE_DEFINITIONS I expect the link to fail
EricWF added a comment.
Is this still needed?
https://reviews.llvm.org/D23796
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
EricWF resigned from this revision.
EricWF added a comment.
This revision is no longer relevant after. The `__threading_support` changes
have applied changes similar to this.
Repository:
rL LLVM
https://reviews.llvm.org/D11781
___
cfe-commits ma
EricWF added a comment.
@rsmith ping.
Repository:
rL LLVM
https://reviews.llvm.org/D24371
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
EricWF added a comment.
@AntonBikineev when will you be able to make he requested changes? I would like
to land this ASAP.
https://reviews.llvm.org/D26830
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman
EricWF accepted this revision.
EricWF added a comment.
This revision is now accepted and ready to land.
This LGTM, but I have a question: Should we add these dylib definitions right
away so that it's easier to adopt the ABI change in future? This will allow
legacy dylibs to support this ABI chan
EricWF added a comment.
In https://reviews.llvm.org/D31272#711860, @mehdi_amini wrote:
> In https://reviews.llvm.org/D31272#711804, @EricWF wrote:
>
> > I'm a bit confused by the description of this change. Libc++ has been
> > enabling the new/delete definitions in its dylib since forever and I'
mehdi_amini added a comment.
Not that I'm claiming to understand why we're changing strategy when we have
the command line tools installed or not in this case ;)
(CC @beanz if he know by any chance!)
https://reviews.llvm.org/D31272
___
cfe-commits
mehdi_amini added a comment.
In https://reviews.llvm.org/D31272#711804, @EricWF wrote:
> I'm a bit confused by the description of this change. Libc++ has been
> enabling the new/delete definitions in its dylib since forever and I've never
> experienced a link error. Did you mean to say the link
EricWF added a comment.
Woops. this patch also needed changes to `TreeTransform.h`. I'll add those
tonight.
Repository:
rL LLVM
https://reviews.llvm.org/D31399
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin
faisalv updated this revision to Diff 93207.
faisalv added a comment.
Don't forget to destroy the trailing objects.
https://reviews.llvm.org/D31414
Files:
include/clang/Basic/TemplateKinds.h
include/clang/Sema/ParsedTemplate.h
lib/Parse/ParseExprCXX.cpp
lib/Parse/ParseTemplate.cpp
Inde
faisalv created this revision.
faisalv added a project: clang-c.
Refactor TemplateIdAnnotation to use TrailingObjects and add assorted comments.
Repository:
rL LLVM
https://reviews.llvm.org/D31414
Files:
include/clang/Basic/TemplateKinds.h
include/clang/Sema/ParsedTemplate.h
lib/Parse/
EricWF added a comment.
In https://reviews.llvm.org/D31375#710897, @jroelofs wrote:
> In https://reviews.llvm.org/D31375#710891, @compnerd wrote:
>
> > What happens when you try building it in tree?
>
>
> The docs-libunwind-html target is missing, and the docs don't get built.
Shouldn't that be
EricWF accepted this revision.
EricWF added a comment.
This revision is now accepted and ready to land.
This LGTM. We should also get a bot setup to build it.
https://reviews.llvm.org/D31375
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
ht
EricWF accepted this revision.
EricWF added a comment.
This revision is now accepted and ready to land.
LGTM minus the suggested cleanup.
Comment at: include/__libunwind_config.h:15
!defined(__ARM_DWARF_EH__)
#define _LIBUNWIND_ARM_EHABI 1
#endif
This sh
EricWF added a comment.
@rsmith gentle ping.
https://reviews.llvm.org/D29930
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Author: gornishanov
Date: Mon Mar 27 21:51:45 2017
New Revision: 298893
URL: http://llvm.org/viewvc/llvm-project?rev=298893&view=rev
Log:
Use BuildReturnStmt in SemaCoroutine to unbreak sanitizer tests.
FIXME: ActOnReturnStmt expects a scope that is inside of the function, due
to CheckJumpOutOf
EricWF created this revision.
This patch fixes http://llvm.org/PR28954 using the `init_priority` attribute.
All supported compilers accept this attribute, including clang-cl.
I'm only putting this up for review because IDK how to write a test for it.
Can anybody suggest a way to test this?
htt
enyquist updated this revision to Diff 93204.
enyquist marked 3 inline comments as done.
enyquist added a comment.
Addressed all comments, except for the one about FormatToken.MatchingParen (see
reply comment)
https://reviews.llvm.org/D28462
Files:
docs/ClangFormatStyleOptions.rst
include/
EricWF added a comment.
OK, so ABI wise this seems good. I'll need a fresh head to review the locale
changes for correctness. I'll get around to that tomorrow.
Comment at: libcxx/include/locale:891
+#ifndef _LIBCPP_ABI_OPTIMIZED_LOCALE
// signed
Would it be
enyquist marked 8 inline comments as done.
enyquist added inline comments.
Comment at: lib/Format/WhitespaceManager.cpp:287
SmallVector
&Changes,
+bool ConsiderScope, bool ConsiderCommas,
unsig
EricWF added a comment.
I'm a bit confused by the description of this change. Libc++ has been enabling
the new/delete definitions in its dylib since forever and I've never
experienced a link error. Did you mean to say the link error occurs only when
libc++abi doesn't define them?
https://revi
EricWF added a comment.
For the most part this LGTM other than the nits mentioned.
Comment at: test/libcxxabi/test/config.py:34
+'libunwind_src',
+os.path.join(self.libcxxabi_src_root, '/../libunwind/src'))
I think the correct default
EricWF added a comment.
I really dislike that `__libcpp_clock_monotonic` and `__libcpp_clock_realtime`
are never declared, and are expected to be magically defined by the external
threading header.
This makes the configuration seem incorrect and unused.
Also the previous threading support chang
alekseyshl added a comment.
Seems like this bot is not happy about this change:
http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/3771
Repository:
rL LLVM
https://reviews.llvm.org/D31399
___
cfe-commits mailing list
cfe-commit
EricWF added a comment.
Thanks for fixing this. Sucks that we have to manually enumerate source files
but this is the correct solution according to CMake.
Would it be easy to still use `glob` to verify that none of the source files
have accidentally been forgotten?
Comment a
rjmccall added a comment.
In https://reviews.llvm.org/D31003#711359, @bkelley wrote:
> Thank you @rjmccall for the approval. I don't have commit access; would
> someone be willing to commit this path for me please? Thanks!
You have a lot of patches here. :) I would encourage you to just ask f
EricWF added inline comments.
Comment at: include/memory:3933
+typedef __shared_ptr_pointer<_Yp*, default_delete<_Yp>, _AllocT >
_CntrlBlk;
+__cntrl_ = new _CntrlBlk(__p, default_delete<_Yp>(), _AllocT());
__hold.release();
Quuxplusone wrote:
> Eric
This revision was automatically updated to reflect the committed changes.
GorNishanov marked an inline comment as done.
Closed by commit rL298891: [coroutines] Handle
get_return_object_on_allocation_failure (authored by GorNishanov).
Changed prior to commit:
https://reviews.llvm.org/D31399?vs=9
Author: gornishanov
Date: Mon Mar 27 18:36:59 2017
New Revision: 298891
URL: http://llvm.org/viewvc/llvm-project?rev=298891&view=rev
Log:
[coroutines] Handle get_return_object_on_allocation_failure
Summary:
If promise_type has get_return_object_on_allocation_failure defined,
check if an allocatio
sylvestre.ledru updated this revision to Diff 93195.
sylvestre.ledru added a comment.
Thanks, indeed! :)
https://reviews.llvm.org/D31408
Files:
docs/ClangFormatStyleOptions.rst
include/clang/Format/Format.h
Index: include/clang/Format/Format.h
==
Eugene.Zelenko added inline comments.
Comment at: docs/ClangFormatStyleOptions.rst:644
+ .. code-block:: c++
+
Shouldn't it be java?
Comment at: docs/ClangFormatStyleOptions.rst:1293
+ .. code-block:: c++
+
Shouldn't it b
malaperle-ericsson added a comment.
Ideas/Observations:
- One thing I has done in my version is to introduce "ASTUnitRunnable", a
lambda function type that has an ASTUnit as parameter and is executed by the
ASTManager. So the ASTManager takes care of locking the AST but the actual code
using t
malaperle-ericsson added inline comments.
Comment at: test/clangd/formatting.test:14
+# CHECK: "codeActionProvider": true,
+# CHECK: "completionProvider": {"resolveProvider": false,
"triggerCharacters": [".",">"]}
# CHECK: }}}
It would be good eventually to
jlebar added a comment.
With your latest set of updates, I am not sure which, if any, patches you need
me to take another look at. Unfortunately I don't have a ton of time for CUDA
stuff these days, so where possible I'd prefer to shunt reviews over to hfinkel
or chandlerc. But do let me know
gtbercea updated this revision to Diff 93192.
gtbercea added a comment.
Herald added a subscriber: rengolin.
Update patch to reflect latest source code changes.
Repository:
rL LLVM
https://reviews.llvm.org/D29654
Files:
lib/Driver/ToolChains/CommonArgs.cpp
lib/Driver/ToolChains/CommonArg
sylvestre.ledru created this revision.
Herald added a subscriber: klimek.
https://reviews.llvm.org/D31408
Files:
docs/ClangFormatStyleOptions.rst
include/clang/Format/Format.h
Index: include/clang/Format/Format.h
===
--- include
gtbercea updated this revision to Diff 93188.
gtbercea added a comment.
Herald added a subscriber: rengolin.
Update patch to reflect latest source code changes.
Repository:
rL LLVM
https://reviews.llvm.org/D29651
Files:
lib/Driver/ToolChains/Cuda.cpp
test/Driver/openmp-offload.c
Index:
teemperor marked 7 inline comments as done.
teemperor added a comment.
Hey Leslie,
regarding performance: Last time I checked we spend most of the time on the
verification of the hash values. We can do some tricks to make this faster
(like delaying the verification to the end of the constraints
gtbercea added inline comments.
Comment at: lib/Driver/Tools.cpp:12136
// Obtain architecture from the action.
- CudaArch gpu_arch = StringToCudaArch(JA.getOffloadingArch());
assert(gpu_arch != CudaArch::UNKNOWN &&
jlebar wrote:
> Why does JA.getOffloading
gtbercea added inline comments.
Comment at: lib/Driver/ToolChains.cpp:4902
+ DeviceOffloadingKind == Action::OFK_Cuda) &&
+ "The offloading kind is not OpenMP or CUDA.");
jlebar wrote:
> Not sure this assertion message helps us much beyond what
gtbercea updated this revision to Diff 93181.
gtbercea marked 5 inline comments as done.
gtbercea added a comment.
Herald added subscribers: sbc100, dschuff, jfb, rengolin.
Update patch to reflect latest source code changes.
Repository:
rL LLVM
https://reviews.llvm.org/D29647
Files:
includ
teemperor updated this revision to Diff 93180.
teemperor added a comment.
Herald added a subscriber: mgorny.
Thanks for the review Artem!
Changes:
- No longer including the old LLVM hashing header.
- Fixed the messed up comment formatting when i removed all the `\brief`s...
- `CloneConstraint` i
Quuxplusone added inline comments.
Comment at: include/memory:3933
+typedef __shared_ptr_pointer<_Yp*, default_delete<_Yp>, _AllocT >
_CntrlBlk;
+__cntrl_ = new _CntrlBlk(__p, default_delete<_Yp>(), _AllocT());
__hold.release();
EricWF wrote:
> Thi
On Mon, Mar 27, 2017 at 10:20 AM Aaron Ballman via Phabricator <
revi...@reviews.llvm.org> wrote:
> aaron.ballman added a comment.
>
> In https://reviews.llvm.org/D31153#711287, @dblaikie wrote:
>
> > As I mentioned to Craig Topper recently on another review, generally
> when implementing const an
GorNishanov updated this revision to Diff 93177.
GorNishanov added a comment.
Added diagnostic test to SemaCXX/coroutines.cpp
Preparing to Land
https://reviews.llvm.org/D31399
Files:
include/clang/AST/StmtCXX.h
include/clang/Basic/DiagnosticSemaKinds.td
lib/AST/StmtCXX.cpp
lib/CodeGen/C
gtbercea updated this revision to Diff 93176.
gtbercea added a comment.
Herald added a subscriber: rengolin.
Update patch to reflect latest source code changes.
Repository:
rL LLVM
https://reviews.llvm.org/D29644
Files:
lib/Driver/ToolChains/Cuda.cpp
test/Driver/openmp-offload.c
Index:
gtbercea updated this revision to Diff 93172.
gtbercea added a comment.
Herald added a subscriber: rengolin.
Update patch to reflect latest source code changes.
Repository:
rL LLVM
https://reviews.llvm.org/D29642
Files:
lib/Driver/ToolChains/Cuda.cpp
test/Driver/openmp-offload.c
Index:
gtbercea updated this revision to Diff 93171.
gtbercea added a comment.
Herald added a subscriber: rengolin.
Update patch to reflect latest source code changes.
Repository:
rL LLVM
https://reviews.llvm.org/D29339
Files:
lib/Driver/ToolChains/Clang.cpp
lib/Frontend/CompilerInstance.cpp
mgehre added inline comments.
Comment at:
clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp:83
+ unless(hasSourceExpression(stringLiteral())),
+ unless(sysSymbolDecayInSysHeader()))
.bind("cast"),
--
EricWF added a comment.
This patch seems to support constructing a `shared_ptr` without
providing a non-default deleter. I don't think this should work because the
default deleter will attempt to free a function pointer, which is never valid.
(Although I think this case will still cause a compi
This revision was automatically updated to reflect the committed changes.
Closed by commit rL298880: Add [[clang::suppress(rule, ...)]] attribute
(authored by mgehre).
Changed prior to commit:
https://reviews.llvm.org/D24886?vs=93055&id=93168#toc
Repository:
rL LLVM
https://reviews.llvm.org
Author: mgehre
Date: Mon Mar 27 14:45:24 2017
New Revision: 298880
URL: http://llvm.org/viewvc/llvm-project?rev=298880&view=rev
Log:
Add [[clang::suppress(rule, ...)]] attribute
Summary:
This patch implements parsing of [[clang::suppress(rule, ...)]]
and [[gsl::suppress(rule, ...)]] attributes.
EricWF accepted this revision.
EricWF added a comment.
This revision is now accepted and ready to land.
This LGTM minus inline comments.
Comment at: lib/Sema/SemaCoroutine.cpp:793
+ LookupResult Found(S, DN, Loc, Sema::LookupMemberName);
+ // Suppress diagnostics when a priva
This revision was automatically updated to reflect the committed changes.
Closed by commit rL298877: Encapsulate FPOptions and use it consistently
(authored by anemet).
Changed prior to commit:
https://reviews.llvm.org/D31166?vs=92898&id=93166#toc
Repository:
rL LLVM
https://reviews.llvm.or
Author: anemet
Date: Mon Mar 27 14:17:25 2017
New Revision: 298877
URL: http://llvm.org/viewvc/llvm-project?rev=298877&view=rev
Log:
Encapsulate FPOptions and use it consistently
Sema holds the current FPOptions which is adjusted by 'pragma STDC
FP_CONTRACT'. This then gets propagated into expre
yaxunl created this revision.
Herald added subscribers: nhaehnle, wdng.
There is an incoming change in LLVM allowing alloca to return a private pointer
which does not pointing to address space 0:
https://reviews.llvm.org/D31042#03b9d490
After this change is committed, alloca will return a point
kuang_he abandoned this revision.
kuang_he added a comment.
In https://reviews.llvm.org/D30415#703652, @uweigand wrote:
> In https://reviews.llvm.org/D30415#703442, @hfinkel wrote:
>
> > In https://reviews.llvm.org/D30415#703398, @echristo wrote:
> >
> > > Different suggestion:
> > >
> > > Remove
sepavloff added a comment.
Any feedback?
https://reviews.llvm.org/D31126
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
bruno added a comment.
Ping!
https://reviews.llvm.org/D31269
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
bruno added inline comments.
Comment at: test/Sema/vector-gcc-compat.c:61
+ //match.
+ v2i64_r = v2i64_a == 1; // expected-warning {{incompatible vector types
assigning to 'v2i64' (vector of 2 'long long' values) from 'long
__attribute__((ext_vector_type(2)))' (vector
sfertile added a comment.
Thanks Eric.
Kuang, commit r298449 contained this fix so you can close the review now.
https://reviews.llvm.org/D30415
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo
bruno accepted this revision.
bruno added a comment.
This revision is now accepted and ready to land.
LGTM!
https://reviews.llvm.org/D29923
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-co
erichkeane updated this revision to Diff 93158.
erichkeane marked 2 inline comments as done.
https://reviews.llvm.org/D29599
Files:
include/clang/Basic/Attr.td
include/clang/Basic/AttrDocs.td
include/clang/Sema/Sema.h
lib/CodeGen/CGCall.cpp
lib/CodeGen/CodeGenFunction.h
lib/Sema/SemaD
erichkeane marked 15 inline comments as done.
erichkeane added a comment.
Comments on 2 cases, otherwise a Patch incoming that fixes the rest of Aaron's
comments.
Comment at: include/clang/Basic/Attr.td:1224
+def AllocAlign : InheritableAttr {
+ let Spellings = [ GCC<"alloc_a
stanionascu created this revision.
rfc8089#appendix-E.2 specifies that paths can begin with a drive letter e.g. as
file:///c:/.
In this case just consuming front file:// is not enough and the 3rd slash must
be consumed to produce a valid path on windows.
The patch introduce a generic way of con
aaron.ballman added a comment.
In https://reviews.llvm.org/D31153#711287, @dblaikie wrote:
> As I mentioned to Craig Topper recently on another review, generally when
> implementing const and non-const overloads the non-const is implemented in
> terms of the const overload (& const_casts away c
ahatanak added inline comments.
Comment at: lib/Sema/SemaDecl.cpp:2157
if (getDiagnostics().getSuppressSystemWarnings() &&
- (Context.getSourceManager().isInSystemHeader(Old->getLocation()) ||
+ // Some standard types are defined implicitly in Clang (e.g. OpenCL).
+
ahatanak added a comment.
Is it possible to enable cantunwind for one function and enable for another? If
so, does this have to be a function attribute?
https://reviews.llvm.org/D31140
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://
aaron.ballman added inline comments.
Comment at: clang-tidy/readability/OperatorsRepresentationCheck.cpp:34
+
+ if (const auto *B = Result.Nodes.getNodeAs("binary")) {
+switch (B->getOpcode()) {
alexfh wrote:
> aaron.ballman wrote:
> > I think this would mak
alexfh added inline comments.
Comment at: clang-tidy/readability/OperatorsRepresentationCheck.cpp:34
+
+ if (const auto *B = Result.Nodes.getNodeAs("binary")) {
+switch (B->getOpcode()) {
aaron.ballman wrote:
> I think this would make more sense lifted into
GorNishanov created this revision.
Herald added a subscriber: mehdi_amini.
If promise_type has get_return_object_on_allocation_failure defined,
check if an allocation function returns nullptr, and if so,
return the result of get_return_object_on_allocation_failure().
https://reviews.llvm.org/D3
Author: djasper
Date: Mon Mar 27 11:29:41 2017
New Revision: 298853
URL: http://llvm.org/viewvc/llvm-project?rev=298853&view=rev
Log:
Look through CXXBindTemporaryExprs when checking CXXFunctionCastExprs
for unused values.
This fixes a regression caused by r298676, where constructor calls to
clas
bkelley added a comment.
Thank you @rjmccall for the approval. I don't have commit access; would someone
be willing to commit this path for me please? Thanks!
https://reviews.llvm.org/D31007
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
h
bkelley added a comment.
Thank you @rjmccall for the approval. I don't have commit access; would someone
be willing to commit this path for me please? Thanks!
https://reviews.llvm.org/D31006
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
h
bkelley added a comment.
Thank you @rjmccall for the approval. I don't have commit access; would someone
be willing to commit this path for me please? Thanks!
https://reviews.llvm.org/D31005
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
h
bkelley added a comment.
Thank you @rjmccall for the approval. I don't have commit access; would someone
be willing to commit this path for me please? Thanks!
https://reviews.llvm.org/D31004
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
h
bkelley added a comment.
Thank you @rjmccall for the approval. I don't have commit access; would someone
be willing to commit this path for me please? Thanks!
https://reviews.llvm.org/D31003
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
h
Author: sanwou01
Date: Mon Mar 27 10:34:52 2017
New Revision: 298850
URL: http://llvm.org/viewvc/llvm-project?rev=298850&view=rev
Log:
[ARM] Add a driver option for +no-neg-immediates
Reviewers: olista01, rengolin, javed.absar, samparker
Reviewed By: samparker
Subscribers: samparker, llvm-commi
aaron.ballman added inline comments.
Comment at: clang-tidy/readability/RedundantInlineCheck.cpp:24
+AST_MATCHER(FriendDecl, isInlineAndHasBody) {
+ NamedDecl *D = Node.getFriendDecl();
+ if (!D)
const NamedDecl (same for const auto * below).
dblaikie added a comment.
As I mentioned to Craig Topper recently on another review, generally when
implementing const and non-const overloads the non-const is implemented in
terms of the const overload (& const_casts away const on the result). This
ensures no UB if the const overload is called
aaron.ballman added inline comments.
Comment at: clang-tidy/readability/OperatorsRepresentationCheck.cpp:34
+
+ if (const auto *B = Result.Nodes.getNodeAs("binary")) {
+switch (B->getOpcode()) {
I think this would make more sense lifted into an AST matcher -
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.
LGTM!
Comment at: lib/CodeGen/CGExprScalar.cpp:1712
BinOp.Opcode = IsInc ? BO_Add : BO_Sub;
- BinOp.FPContractable = false;
+ // FIXME: once UnaryOperator
echuraev updated this revision to Diff 93132.
echuraev marked an inline comment as done.
https://reviews.llvm.org/D31183
Files:
include/clang/Parse/Parser.h
lib/Parse/ParseExpr.cpp
test/Parser/vector-cast-define.cl
Index: test/Parser/vector-cast-define.cl
==
Anastasia created this revision.
Herald added a subscriber: yaxunl.
Fixing the assertion due to absence of source location for implicitly defined
types (using addImplicitTypedef()). During Sema checks the source location is
being expected and therefore an assertion is triggered.
The change is n
-Wunused-value is always triggered if a constructor of an object with a
non-trivial destructor has an initializer list as first parameter. So in
the test, even "Used({});" triggers -Wunused-value. That seems inconsistent
(replacing the init list with something else silences the warning) and also
th
What is the effect on that testcase? (Sorry, heading to vacation and can't
easily check.)
On 27 Mar 2017 6:33 am, "Daniel Jasper" wrote:
> Hi Richard,
>
> this seems to have an unwanted side-effect on -Wunused-value (test case
> attached). Could you take a look?
>
> Cheers,
> Daniel
>
> On Fri,
Anastasia added a comment.
I don't think that diagnostics can always be very clear. This is not the case
neither for C nor C++.
As I said I don't see any issue to continue with this patch. I would just like
to see the test simplified a bit.
https://reviews.llvm.org/D31183
Hi Richard,
this seems to have an unwanted side-effect on -Wunused-value (test case
attached). Could you take a look?
Cheers,
Daniel
On Fri, Mar 24, 2017 at 2:14 AM, Richard Smith via cfe-commits <
cfe-commits@lists.llvm.org> wrote:
> Author: rsmith
> Date: Thu Mar 23 20:14:25 2017
> New Revisi
Author: vvassilev
Date: Mon Mar 27 08:11:32 2017
New Revision: 298842
URL: http://llvm.org/viewvc/llvm-project?rev=298842&view=rev
Log:
Publish one more parser RAII for external use.
Modified:
cfe/trunk/include/clang/Parse/RAIIObjectsForParser.h
cfe/trunk/lib/Parse/Parser.cpp
Modified: c
aaron.ballman added a comment.
Ping.
Because I expect this to be fairly uncontroversial, I'll give it a week in case
folks have comments, then commit (and will handle anything in post-commit
review).
https://reviews.llvm.org/D31153
___
cfe-commit
leanil marked 2 inline comments as done.
leanil added inline comments.
Comment at: clang-tidy/misc/ForwardingReferenceOverloadCheck.cpp:119
+ DisabledMove = false;
+ for (const auto *OtherCtor : Ctor->getParent()->ctors()) {
+if (OtherCtor->isCopyConstructor()) {
-
leanil updated this revision to Diff 93114.
leanil added a comment.
Correct earlier diff issue with outdated master.
Repository:
rL LLVM
https://reviews.llvm.org/D30547
Files:
clang-tidy/misc/CMakeLists.txt
clang-tidy/misc/ForwardingReferenceOverloadCheck.cpp
clang-tidy/misc/Forwarding
JonasToth added a comment.
Why does diff contain so many files?
Could you maybe merge the latest master into your branch?
Repository:
rL LLVM
https://reviews.llvm.org/D30547
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llv
1 - 100 of 108 matches
Mail list logo
