javed.absar added inline comments.
Comment at: test/SemaCXX/coroutines.cpp:169
+void check_auto_await_suspend() {
+ co_await auto_await_suspend{}; // OK
+}
maybe change the comment to something more meaningful, or remove it.
https://reviews.llvm.org/D37454
delena accepted this revision.
delena added a comment.
This revision is now accepted and ready to land.
LGTM
Repository:
rL LLVM
https://reviews.llvm.org/D37449
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin
Hey Hans,
Just a heads up that I fixed the issue and reapplies the commit in r312512.
Let me know if any new issues occur! (and feel free to revert of course).
Thanks,
--
Mehdi
2017-08-28 14:18 GMT-07:00 Mehdi AMINI :
> Sorry for the inconvenience!
>
> I will follow-up in the PR.
>
> --
> Meh
Author: mehdi_amini
Date: Mon Sep 4 20:58:35 2017
New Revision: 312512
URL: http://llvm.org/viewvc/llvm-project?rev=312512&view=rev
Log:
Emit static constexpr member as available_externally definition
By exposing the constant initializer, the optimizer can fold many
of these constructs.
This is
GorNishanov marked an inline comment as done.
GorNishanov added inline comments.
Comment at: lib/Sema/SemaCoroutine.cpp:451
if (RetType->isReferenceType() ||
(AdjRetType != S.Context.BoolTy && AdjRetType != S.Context.VoidTy)) {
S.Diag(AwaitSuspend->getC
GorNishanov updated this revision to Diff 113800.
GorNishanov retitled this revision from "[coroutines] desugar auto type of
await_resume before checking whether it is void or bool" to "[coroutines] Make
sure auto return type of await_resume is properly handled".
GorNishanov added a comment.
use
Author: rsmith
Date: Mon Sep 4 17:50:19 2017
New Revision: 312506
URL: http://llvm.org/viewvc/llvm-project?rev=312506&view=rev
Log:
Always allocate room for a ModuleDecl on the TranslationUnitDecl.
Sometimes we create the ASTContext and thus the TranslationUnitDecl before we
know the LangOption
rsmith added inline comments.
Comment at: lib/Sema/SemaCoroutine.cpp:451
if (RetType->isReferenceType() ||
(AdjRetType != S.Context.BoolTy && AdjRetType != S.Context.VoidTy)) {
S.Diag(AwaitSuspend->getCalleeDecl()->getLocation(),
Don't
GorNishanov created this revision.
https://reviews.llvm.org/D37454
Files:
lib/Sema/SemaCoroutine.cpp
test/SemaCXX/coroutines.cpp
Index: test/SemaCXX/coroutines.cpp
===
--- test/SemaCXX/coroutines.cpp
+++ test/SemaCXX/coroutines
yaxunl accepted this revision.
yaxunl added a comment.
LGTM. Thanks.
https://reviews.llvm.org/D37386
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
mstorsjo added a comment.
In https://reviews.llvm.org/D37133#860563, @compnerd wrote:
> `$` should give you the `.o` or `.obj`
> files used to construct the library.
I guess that would only work if they're built all as part of the same CMake
invocation? In the setup where I'm trying this (for
compnerd added a comment.
`$` should give you the `.o` or `.obj`
files used to construct the library.
https://reviews.llvm.org/D37133
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
mstorsjo added a comment.
Do you have any pointers on how to do it with CMake?
https://reviews.llvm.org/D37133
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Author: mstorsjo
Date: Mon Sep 4 12:46:53 2017
New Revision: 312498
URL: http://llvm.org/viewvc/llvm-project?rev=312498&view=rev
Log:
Add MINGW_LIBRARIES to the linker flags
This is essential when building with -nodefaultlibs.
This is similar to what already is done in libcxxabi in SVN r302760.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL312498: Add MINGW_LIBRARIES to the linker flags (authored by
mstorsjo).
Changed prior to commit:
https://reviews.llvm.org/D37207?vs=112873&id=113787#toc
Repository:
rL LLVM
https://reviews.llvm.org/
compnerd requested changes to this revision.
compnerd added a comment.
This revision now requires changes to proceed.
If we handle this in CMake, this will be unnecessary.
https://reviews.llvm.org/D37134
___
cfe-commits mailing list
cfe-commits@list
compnerd requested changes to this revision.
compnerd added a comment.
This revision now requires changes to proceed.
I think we should avoid this logic entirely and use CMake to do this.
https://reviews.llvm.org/D37133
___
cfe-commits mailing list
v.g.vassilev added a comment.
You should probably update the code creating the vfs before the call to
`createFileManager` in `lib/Frontend/FrontendAction.cpp`.
Repository:
rL LLVM
https://reviews.llvm.org/D37416
___
cfe-commits mailing list
cfe-
RKSimon created this revision.
Based off the Intel Intrinsics guide, we should expect a void const* argument.
Prevents 'passing 'const void *' to parameter of type 'void *' discards
qualifiers' warnings.
Repository:
rL LLVM
https://reviews.llvm.org/D37449
Files:
lib/Headers/avx512fintrin
RKSimon created this revision.
I can add the other test cases from PR34021 if required?
Repository:
rL LLVM
https://reviews.llvm.org/D37448
Files:
lib/CodeGen/CGStmt.cpp
test/CodeGen/pr34021.c
Index: test/CodeGen/pr34021.c
===
On Mon, Sep 4, 2017 at 1:57 AM, Chandler Carruth
wrote:
> On Sun, Sep 3, 2017 at 10:41 PM Hal Finkel via llvm-commits <
> llvm-comm...@lists.llvm.org> wrote:
>
>> Nevertheless, I think that you've convinced me that this is a least-bad
>> solution. I'll want a flag preserving the old behavior. Som
On Mon, Sep 4, 2017 at 9:17 AM, Hal Finkel wrote:
>
> On 09/04/2017 03:57 AM, Chandler Carruth wrote:
>
> On Sun, Sep 3, 2017 at 10:41 PM Hal Finkel via llvm-commits <
> llvm-comm...@lists.llvm.org> wrote:
>
>> Nevertheless, I think that you've convinced me that this is a least-bad
>> solution. I
kasaurov updated this revision to Diff 113780.
kasaurov added a comment.
Test added to check passing of -On/default
https://reviews.llvm.org/D37386
Files:
lib/Driver/ToolChains/AMDGPU.cpp
lib/Driver/ToolChains/AMDGPU.h
test/Driver/amdgpu-toolchain-opencl.cl
Index: test/Driver/amdgpu-tool
boris updated this revision to Diff 113779.
boris marked an inline comment as done.
boris added a comment.
New patch revision with David's comments addressed.
https://reviews.llvm.org/D37299
Files:
docs/Modules.rst
include/clang/Basic/DiagnosticDriverKinds.td
include/clang/Driver/Options.
boris marked 3 inline comments as done.
boris added a comment.
David, thanks for the review. Uploading the new revision (also rebased on HEAD).
Comment at: lib/Frontend/CompilerInvocation.cpp:1576
+ StringRef Str = Buf.get()->getBuffer();
+ for (size_t B(0), E(B); B < Str.siz
rampitec accepted this revision.
rampitec added a comment.
This revision is now accepted and ready to land.
LGTM
https://reviews.llvm.org/D37386
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/c
arphaman accepted this revision.
arphaman added a comment.
This revision is now accepted and ready to land.
LGTM.
Please precommit the `auto` loop modernization though. You might want to use
`llvm::make_range` for for-ranged loops as well.
https://reviews.llvm.org/D37390
___
kasaurov added inline comments.
Comment at: lib/Driver/ToolChains/AMDGPU.h:60
bool IsIntegratedAssemblerDefault() const override { return true; }
+ llvm::opt::DerivedArgList *
+ TranslateArgs(const llvm::opt::DerivedArgList &Args, StringRef BoundArch,
emanko
kasaurov updated this revision to Diff 113767.
kasaurov added a comment.
Several requested changes:
- move map initialization out of header file -- visually it looks better in .h
file, but logically it should be in .cpp
- add assert() in getOptionDefault() -- check for unknown to OptionsDefault
On 09/04/2017 03:57 AM, Chandler Carruth wrote:
On Sun, Sep 3, 2017 at 10:41 PM Hal Finkel via llvm-commits
mailto:llvm-comm...@lists.llvm.org>> wrote:
Nevertheless, I think that you've convinced me that this is a
least-bad solution. I'll want a flag preserving the old behavior.
So
Rakete created this revision.
This revision disallows lambdas in template parameters, as reported in PR33696.
https://reviews.llvm.org/D37442
Files:
include/clang/Basic/DiagnosticSemaKinds.td
include/clang/Sema/Sema.h
lib/Parse/ParseTemplate.cpp
lib/Sema/Sema.cpp
lib/Sema/SemaExpr
dblaikie added a comment.
Couple of drive by comments - no doubt Richard will have a more in depth
review, but figured this might save a round or two.
Comment at: lib/Frontend/CompilerInvocation.cpp:1561-1565
+ if (File.empty())
+ {
+Diags.Report(diag::err_drv_invalid_va
Author: jvesely
Date: Mon Sep 4 08:52:03 2017
New Revision: 312491
URL: http://llvm.org/viewvc/llvm-project?rev=312491&view=rev
Log:
Fixup clc.h comment
Signed-off-by: Jan Vesely
Modified:
libclc/trunk/generic/include/clc/clc.h
Modified: libclc/trunk/generic/include/clc/clc.h
URL:
http:/
Author: jvesely
Date: Mon Sep 4 08:52:05 2017
New Revision: 312492
URL: http://llvm.org/viewvc/llvm-project?rev=312492&view=rev
Log:
r600: Cleanup barrier implementation.
We don't have memory fences for r600 so just call group barrier directly
Make sure that barrier is called even with 0 flags
Author: jvesely
Date: Mon Sep 4 08:52:07 2017
New Revision: 312493
URL: http://llvm.org/viewvc/llvm-project?rev=312493&view=rev
Log:
amdgcn,waitcnt: Add datalayout info
This file is only compiled for GCN which all share the same layout
Signed-off-by: Jan Vesely
Reviewed-by: Aaron Watry
Modif
Seems like the test case should/could be simplified a bit, maybe? (could it
be something really simple like "void f(bool b) { #pragma ... while (b) ;
}"?)
Also, is it relying on LLVM optimizations to run (add
"-disable-llvm-passes" to the command line to be sure it isn't, perhaps?)?
On Sun, Sep 3
NoQ added a comment.
Cool. Thanks!
> In the future probably it would be better to alter the signature of the
> checkers' constructor to set the name in the constructor so it is possible to
> create the BugType eagerly.
Still, should we add an assertion so that we could be sure that every bug t
hintonda added a comment.
ping...
https://reviews.llvm.org/D36347
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
xazax.hun created this revision.
Herald added subscribers: baloghadamsoftware, whisperity.
Unfortunately, currently, the analyzer core sets the checker name after the
constructor was already run. So if we set the BugType in the constructor, the
output plist will not contain a checker name. Righ
aaron.ballman created this revision.
Herald added a subscriber: javed.absar.
WG14 has demonstrated sincere interest in adding C++-style attributes to C for
C2x, and have added the proposal to their SD-3 document tracking features which
are possibly desired for the next version of the standard. T
Author: djasper
Date: Mon Sep 4 06:33:52 2017
New Revision: 312484
URL: http://llvm.org/viewvc/llvm-project?rev=312484&view=rev
Log:
clang-format: Fix indentation of macros in include guards (after r312125).
Before:
#ifndef A_H
#define A_H
#define A() \
int i;\
int j;
#endif //
ilya-biryukov added a comment.
In https://reviews.llvm.org/D37282#860239, @puremourning wrote:
> No I don't have commit rights. Can you land for me? Thanks!
Done.
Repository:
rL LLVM
https://reviews.llvm.org/D37282
___
cfe-commits mailing list
Author: ibiryukov
Date: Mon Sep 4 05:28:15 2017
New Revision: 312483
URL: http://llvm.org/viewvc/llvm-project?rev=312483&view=rev
Log:
clangd: Tolerate additional headers
Summary:
The language server protocol specified 2 headers (Content-Length and
Content-Type), but does not specify their sequ
This revision was automatically updated to reflect the committed changes.
Closed by commit rL312483: clangd: Tolerate additional headers (authored by
ibiryukov).
Repository:
rL LLVM
https://reviews.llvm.org/D37282
Files:
clang-tools-extra/trunk/clangd/JSONRPCDispatcher.cpp
clang-tools-ext
klimek added inline comments.
Comment at: test/Refactor/LocalRename/Field.cpp:4
+class Baz {
+ int /*range=*/Foo; // CHECK: symbol [[@LINE]]:17 -> [[@LINE]]:20
+public:
arphaman wrote:
> klimek wrote:
> > arphaman wrote:
> > > klimek wrote:
> > > > Does this jus
xazax.hun added inline comments.
Comment at: lib/CrossTU/CrossTranslationUnit.cpp:82
+size_t Pos = Line.find(" ");
+StringRef LineRef{Line};
+if (Pos > 0 && Pos != std::string::npos) {
danielmarjamaki wrote:
> LineRef can be const
StringRef is an immu
xazax.hun updated this revision to Diff 113740.
xazax.hun marked 19 inline comments as done.
xazax.hun added a comment.
- Make the API capable of using custom lookup strategies for CTU
- Fix review comments
https://reviews.llvm.org/D34512
Files:
include/clang/Basic/AllDiagnostics.h
include/
puremourning added a comment.
In https://reviews.llvm.org/D37282#860200, @ilya-biryukov wrote:
> Looks good and ready to land. Thanks for this change.
> Do you have commit rights to llvm repo?
Thanks for the review!
No I don't have commit rights. Can you land for me? Thanks!
https://reviews
arphaman added a comment.
In https://reviews.llvm.org/D36574#860203, @klimek wrote:
> In https://reviews.llvm.org/D36574#860197, @arphaman wrote:
>
> > In https://reviews.llvm.org/D36574#858763, @klimek wrote:
> >
> > > One of my main concerns is still that I don't see the need for all the
> > >
ilya-biryukov added a comment.
Looks good overall, just a bunch of comments before accepting the revision.
Mostly minor code style issues, sorry for spamming you with those.
Comment at: clangd/ClangdLSPServer.h:30
ClangdLSPServer(JSONOutput &Out, unsigned AsyncThreadsCount,
Author: rksimon
Date: Mon Sep 4 03:54:39 2017
New Revision: 312479
URL: http://llvm.org/viewvc/llvm-project?rev=312479&view=rev
Log:
Fix MSVC narrowing conversion warning.
Modified:
cfe/trunk/lib/Serialization/ASTWriter.cpp
Modified: cfe/trunk/lib/Serialization/ASTWriter.cpp
URL:
http://ll
ioeric added inline comments.
Comment at: include/clang/Tooling/Refactoring/RefactoringAction.h:20
+
+class RefactoringAction {
+public:
Please add documentation for the class.
I appreciate your RFC that explains all these concepts; it would be nice if you
cou
ilya-biryukov added inline comments.
Comment at: clangd/JSONRPCDispatcher.cpp:167
+ // It's another header, ignore it.
continue;
+} else {
puremourning wrote:
> ilya-biryukov wrote:
> > Maybe log the skipped header?
> I think this would just be no
klimek added a comment.
In https://reviews.llvm.org/D36574#860197, @arphaman wrote:
> In https://reviews.llvm.org/D36574#858763, @klimek wrote:
>
> > One of my main concerns is still that I don't see the need for all the
> > template magic yet :) Why doesn't everybody use the RefactoringResult w
ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.
Looks good and ready to land. Thanks for this change.
Do you have commit rights to llvm repo?
https://reviews.llvm.org/D37282
___
c
JDevlieghere updated this revision to Diff 113737.
JDevlieghere added a comment.
Thanks Alex!
https://reviews.llvm.org/D37390
Files:
test/Misc/warning-flags-tree.c
tools/diagtool/TreeView.cpp
Index: tools/diagtool/TreeView.cpp
===
arphaman added a comment.
In https://reviews.llvm.org/D36574#858763, @klimek wrote:
> One of my main concerns is still that I don't see the need for all the
> template magic yet :) Why doesn't everybody use the RefactoringResult we
> define here?
@klimek, are you talking about the template us
arphaman updated this revision to Diff 113736.
arphaman marked an inline comment as done.
arphaman added a comment.
Get rid of inheritance of `RefactoringEngine` by getting rid of the class
itself.
Repository:
rL LLVM
https://reviews.llvm.org/D36574
Files:
include/clang/Tooling/Refactorin
arphaman added a comment.
The diagtool tests should go to `test/Misc`, and there's an existing `diagtool
tree` test there that has to be updated as well.
Repository:
rL LLVM
https://reviews.llvm.org/D37390
___
cfe-commits mailing list
cfe-commit
JDevlieghere updated this revision to Diff 113731.
JDevlieghere added a comment.
Herald added subscribers: aheejin, klimek.
Thanks for the review Adrian!
I somehow completely overlooked the existing `--flags-only` option in `diagtool
tree`. Obviously it makes much more sense to implement printin
yvvan added a comment.
ping
https://reviews.llvm.org/D36390
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
On Sun, Sep 3, 2017 at 10:41 PM Hal Finkel via llvm-commits <
llvm-comm...@lists.llvm.org> wrote:
> Nevertheless, I think that you've convinced me that this is a least-bad
> solution. I'll want a flag preserving the old behavior. Something like
> -fwiden-bitfield-accesses (modulo bikeshedding).
>
arphaman accepted this revision.
arphaman added a comment.
This revision is now accepted and ready to land.
LGTM
https://reviews.llvm.org/D36998
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/c
arphaman added a comment.
LGTM with one inline comment below:
Comment at: utils/TableGen/TableGen.cpp:151
clEnumValN(GenOptDocs, "gen-opt-docs", "Generate option
documentation"),
+clEnumValN(GenDataCollectors, "gen-clang-data-collectors", "Generate
data colle
arphaman accepted this revision.
arphaman added a comment.
`def`s could work, but I think that we should stick with TableGen. As you've
said, we'd be able to introduce some sort of structure instead of just code in
the future which will be better (Ideally we'd try to do it now, but given the
fa
jvesely requested review of this revision.
jvesely added a comment.
please let me know if your accept still stands for the modified version.
Repository:
rL LLVM
https://reviews.llvm.org/D37231
___
cfe-commits mailing list
cfe-commits@lists.llvm.o
66 matches
Mail list logo
