thakis added a comment.
The test fails on my system like so:
--
FAIL: Clang :: Driver/print-multi-directory.c (4696 of 13174)
TEST 'Clang :: Driver/print-multi-directory.c' FAILED
Script:
--
: 'RUN: at line 1';
/u
ahatanak added inline comments.
Comment at: lib/Sema/SemaInit.cpp:1827-1829
+ if (SemaRef.DiagnoseUseOfDecl(Destructor, Loc))
+return false;
+ return true;
rsmith wrote:
> Usual Clang convention is to return `true` on error.
I also renamed the function to h
ahatanak updated this revision to Diff 163871.
ahatanak marked 2 inline comments as done.
ahatanak added a comment.
Point the diagnostic at either the relevant init list element or at the close
brace.
Repository:
rC Clang
https://reviews.llvm.org/D45898
Files:
lib/Sema/SemaInit.cpp
test
mgrang updated this revision to Diff 163876.
mgrang added a comment.
Addressed comments.
https://reviews.llvm.org/D50488
Files:
include/clang/StaticAnalyzer/Checkers/Checkers.td
lib/StaticAnalyzer/Checkers/CMakeLists.txt
lib/StaticAnalyzer/Checkers/PointerSortingChecker.cpp
test/Analysi
mgrang added a comment.
In https://reviews.llvm.org/D50488#1222837, @whisperity wrote:
>
> I'm not sure if we discussed, has this checker been tried on some in-the-wild
> examples? To see if there are some real findings or crashes?
> There is a good selection of projects here in this tool:
mgrang added a comment.
In https://reviews.llvm.org/D50488#1207398, @Szelethus wrote:
> In https://reviews.llvm.org/D50488#1204655, @mgrang wrote:
>
> > This is my first time with ASTMatchers and I am not sure how to get the
> > value_type from hasType (I don't see a matcher for value_types in
labath added a comment.
In https://reviews.llvm.org/D51576#1223234, @aprantl wrote:
> This is DWARF5+ only, right? (We shouldn't change the preference of Apple
> accelerator tables for DWARF 4 and earlier).
The interactions here are a bit weird, but the short answer is no, this will
not affec
beanz added a subscriber: dexonsmith.
beanz added a comment.
Unfortunately I can't make this kind of policy decision about whether or not
this would be acceptable for Darwin. That would probably need to be @dexonsmith.
Repository:
rC Clang
https://reviews.llvm.org/D51440
_
erik.pilkington created this revision.
erik.pilkington added a reviewer: arphaman.
Herald added a subscriber: dexonsmith.
rdar://42717026
Thanks for taking a look!
Repository:
rC Clang
https://reviews.llvm.org/D51649
Files:
clang/lib/Sema/SemaStmt.cpp
clang/test/Sema/switch-availability
efriedma added a comment.
getClassPointerAlignment is not really relevant here; that's the alignment of a
pointer to an object with the type of the class.
For the LLVM IR structs which don't have a corresponding clang type, you can
probably just use DataLayout::getABITypeAlignment(). I think t
clayborg added a comment.
We want to ensure that both Apple and DWARF5 tables never get generated though.
That would waste a lot of space. I would say if DWARF5 tables are enabled, then
we need ensure we disable Apple tables.
Repository:
rC Clang
https://reviews.llvm.org/D51576
_
clayborg added a comment.
Actually, we might need to still emit some Apple tables as the objective C and
namespace tables might still be needed even with the DWARF5 tables.
Repository:
rC Clang
https://reviews.llvm.org/D51576
___
cfe-commits mai
labath added a comment.
In https://reviews.llvm.org/D51576#1223596, @clayborg wrote:
> We want to ensure that both Apple and DWARF5 tables never get generated
> though. That would waste a lot of space. I would say if DWARF5 tables are
> enabled, then we need ensure we disable Apple tables.
Th
erichkeane created this revision.
erichkeane added reviewers: thiagomacieira, aaron.ballman.
As discussed here: https://lwn.net/Articles/691932/
GCC6.0 adds target_clones multiversioning. This functionality is
an odd cross between the cpu_dispatch and 'target' MV, but is compatible
with neither.
tstellar added a comment.
In https://reviews.llvm.org/D51567#1222704, @chandlerc wrote:
> I mean, sure.
>
> I really don't know that supporting this ever expanding diversity of build
> strategies is worth its cost, but I don't see a specific reason to not take
> this patch
I actually agre
hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.
Thanks, looks good from my side.
Comment at: clangd/XRefs.cpp:328
+ : AST(AST) {
+for (const Decl *D : TargetDecls)
+ Targets.insert(D);
nit: w
mstorsjo added a subscriber: beanz.
mstorsjo added a comment.
Sounds sensible to me, although it might be good with a second opinion I think.
@beanz?
Repository:
rUNW libunwind
https://reviews.llvm.org/D51645
___
cfe-commits mailing list
cfe-com
hokein added inline comments.
Comment at: clang-tidy/google/FunctionNamingCheck.cpp:57
+ functionDecl(
+ isDefinition(),
+ unless(anyOf(isMain(), matchesName(validFunctionNameRegex(true)),
stephanemoore wrote:
> hokein wrote:
> > any reason
NoQ added inline comments.
Comment at: llvm/include/llvm/Support/Allocator.h:304
+// Use negative index to denote custom sized slabs.
+int64_t CustomSlabOffset = 0;
+for (size_t Idx = 0; Idx < CustomSizedSlabs.size(); Idx++) {
We should start with -1
xbolva00 added inline comments.
Comment at: llvm/include/llvm/Support/Allocator.h:295
+int64_t TotalSlabOffset = 0;
+for (size_t Idx = 0; Idx < Slabs.size(); Idx++) {
+ const char *S = reinterpret_cast(Slabs[Idx]);
for (size_t Idx = 0, e = Slabs.size
Wizard added inline comments.
Comment at: clang-tidy/google/FunctionNamingCheck.cpp:50
+
+void FunctionNamingCheck::registerMatchers(MatchFinder *Finder) {
+ // This check should only be applied to Objective-C sources.
Can we do some simple check to see if some
rsmith added a comment.
> Support for constructor calls (`CXXTemporaryExpr`) should also be possible,
> but is not the part of this patch.
(You should handle the base class (`CXXConstructExpr`) that describes the
semantics, rather than the derived class (`CXXTemporaryObjectExpr`) that
describe
rsmith added a comment.
+rjmccall for CodeGen changes
https://reviews.llvm.org/D51200
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
george.karpenkov created this revision.
george.karpenkov added reviewers: dcoughlin, NoQ.
Herald added subscribers: Szelethus, mikhail.ramalho, a.sidorin, szepet,
baloghadamsoftware, xazax.hun, whisperity.
Ubigraph project has been dead since about 2008, and to the best of my
knowledge, no one w
kuhar added a comment.
In https://reviews.llvm.org/D51200#1223752, @rsmith wrote:
> +rjmccall for CodeGen changes
@rsmith @rjmccall
I have one high-level question about the CodeGen part that I wasn't able to
figure out: is it possible to bail out of custom CodeGen in CGBuiltin and
somehow sa
aprantl added a comment.
In https://reviews.llvm.org/D51576#1223562, @labath wrote:
> The interactions here are a bit weird, but the short answer is no, this will
> not affect apple tables in any way.
Then I have no problem with this patch :-)
Repository:
rC Clang
https://reviews.llvm.org
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.
Thank you for pointing out that the new form of type attributes aren't
automatically opted in from this change. With that (and the two problematic
attributes opted out), this LGT
kuhar added a comment.
Thank you for the comments, Richard!
In https://reviews.llvm.org/D51200#1223745, @rsmith wrote:
> Have you considered whether the builtin should apply to `new` expressions?
> (There are potentially three different top-level calls implied by a `new`
> expression -- an `op
dblaikie accepted this revision.
dblaikie added a comment.
This revision is now accepted and ready to land.
Looks good to me & the answers to others various questions seem well addressed.
Repository:
rC Clang
https://reviews.llvm.org/D51576
___
c
arphaman accepted this revision.
arphaman added a comment.
LGTM, Thank you!
Repository:
rC Clang
https://reviews.llvm.org/D51507
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
beanz accepted this revision.
beanz added a comment.
This revision is now accepted and ready to land.
LGTM.
Repository:
rUNW libunwind
https://reviews.llvm.org/D51645
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/c
Author: cdavis
Date: Tue Sep 4 13:57:50 2018
New Revision: 341404
URL: http://llvm.org/viewvc/llvm-project?rev=341404&view=rev
Log:
[CMake] Don't use -rtlib=compiler-rt with -nodefaultlibs.
Summary:
This switch only has an effect at link time. It changes the default
compiler support library to `
This revision was automatically updated to reflect the committed changes.
Closed by commit rUNW341404: [CMake] Don't use -rtlib=compiler-rt with
-nodefaultlibs. (authored by cdavis, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D51645?vs=163864&id=163907#toc
Repository:
r
cdavis5x created this revision.
cdavis5x added a reviewer: beanz.
Herald added subscribers: cfe-commits, chrib, christof, mgorny, dberris.
If `-nodefaultlibs` is given, we weren't actually linking to it. This
was true irrespective of passing `-rtlib=compiler-rt` (see previous
patch). Now we explic
Wawha marked an inline comment as done.
Wawha added inline comments.
Comment at: lib/Format/ContinuationIndenter.cpp:1307
+ (Style.BraceWrapping.BeforeLambdaBody && Current.Next != nullptr &&
+Current.Next->is(TT_LambdaLSquare)));
State.Stack.back().IsInsideObjCA
This is breaking buildbots:
http://lab.llvm.org:8011/builders/clang-x86_64-linux-selfhost-modules/builds/19509
Can you take a look? Thanks!
On Tue, 4 Sep 2018 at 08:36, Christian Bruel via cfe-commits <
cfe-commits@lists.llvm.org> wrote:
> Author: chrib
> Date: Tue Sep 4 08:22:13 2018
> New Re
timshen added a comment.
> The test fails on my system like so:
I also observed the same failure.
Bots also fail:
http://lab.llvm.org:8011/builders/clang-x86_64-linux-selfhost-modules/builds/19509/steps/check-all/logs/FAIL%3A%20Clang%3A%3Aprint-multi-directory.c
I'm going to revert this patch.
Author: timshen
Date: Tue Sep 4 15:20:11 2018
New Revision: 341418
URL: http://llvm.org/viewvc/llvm-project?rev=341418&view=rev
Log:
Revert r341373, since it fails on some targets.
Differential Revision: https://reviews.llvm.org/D51354
Removed:
cfe/trunk/test/Driver/print-multi-directory.c
Author: rtrieu
Date: Tue Sep 4 15:53:19 2018
New Revision: 341421
URL: http://llvm.org/viewvc/llvm-project?rev=341421&view=rev
Log:
[ODRHash] Extend hash to support all Type's.
Added:
cfe/trunk/test/Modules/odr_hash-gnu.cpp
cfe/trunk/test/Modules/odr_hash-vector.cpp
cfe/trunk/test/Mo
echristo added a reviewer: dblaikie.
echristo added a comment.
The change in name here from "line tables" to "directives only" feels a bit
confusing. "Limited" seems to be a bit more clear, or even remaining line
tables only. Can you explain where you were going with this particular set of
cha
dblaikie added a comment.
In https://reviews.llvm.org/D51554#1224049, @echristo wrote:
> The change in name here from "line tables" to "directives only" feels a bit
> confusing. "Limited" seems to be a bit more clear, or even remaining line
> tables only. Can you explain where you were going w
mikerice updated this revision to Diff 163946.
mikerice marked 7 inline comments as done.
mikerice added a comment.
Thanks for the review. Updated based on comments from Hans.
https://reviews.llvm.org/D51391
Files:
include/clang/Basic/DiagnosticDriverKinds.td
include/clang/Basic/Diagnostic
mikerice added inline comments.
Comment at: lib/Frontend/CompilerInvocation.cpp:2862
+ Opts.PCHWithHdrStopCreate =
+ Args.getLastArgValue(OPT_pch_through_hdrstop_EQ) == "create";
Opts.PCHThroughHeader = Args.getLastArgValue(OPT_pch_through_header_EQ);
ha
rsmith added a comment.
I think this patch is nearly ready to commit. However... if you want to handle
macros that specify a sequence of attributes, we should switch to treating the
macro name as a separate type sugar node rather than modeling it as a name for
the `AttributedType` node. Up to y
mcgrathr added inline comments.
Comment at: lib/Parse/ParseDecl.cpp:244-252
+// If this was declared in a macro, attatch the macro IdentifierInfo to the
+// parsed attribute.
+for (const auto &MacroPair : PP.getAttributeMacros()) {
+ if (SourceLocInSourceRange(At
arphaman accepted this revision.
arphaman added a comment.
This revision is now accepted and ready to land.
LGTM.
Repository:
rC Clang
https://reviews.llvm.org/D51649
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/c
Author: rsmith
Date: Tue Sep 4 17:28:57 2018
New Revision: 341437
URL: http://llvm.org/viewvc/llvm-project?rev=341437&view=rev
Log:
Allow all supportable non-type attributes to be used with #pragma clang
attribute.
Summary:
We previously disallowed use of undocumented attributes with #pragma cl
This revision was automatically updated to reflect the committed changes.
Closed by commit rL341437: Allow all supportable non-type attributes to be used
with #pragma clang… (authored by rsmith, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.o
rsmith added a comment.
I think this is addressing a symptom rather than the cause. The bug appears to
be that when parsing a.h, we end up with inconsistent exception specifications
along the redeclaration chain. It looks like
`Sema::AdjustDestructorExceptionSpec` doesn't do the right thing whe
rjmccall added a comment.
In https://reviews.llvm.org/D51200#1223768, @kuhar wrote:
> In https://reviews.llvm.org/D51200#1223752, @rsmith wrote:
>
> > +rjmccall for CodeGen changes
>
>
> @rsmith @rjmccall
> I have one high-level question about the CodeGen part that I wasn't able to
> figure ou
rsmith added inline comments.
Comment at: lib/Serialization/ASTReaderDecl.cpp:1766
+
+// FIXME: handle serialized lambdas
assert(!DD.IsLambda && !MergeDD.IsLambda && "faked up lambda definition?");
Did you mean to add this FIXME?
Comme
kadircet updated this revision to Diff 163968.
kadircet added a comment.
Rebase
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D51039
Files:
unittests/clangd/CodeCompleteTests.cpp
Index: unittests/clangd/CodeCompleteTests.cpp
=
kadircet updated this revision to Diff 163970.
kadircet added a comment.
- Update tests.
- Update comment.
- Rebase.
Repository:
rC Clang
https://reviews.llvm.org/D51038
Files:
include/clang/Parse/Parser.h
lib/Parse/ParseExpr.cpp
test/CodeCompletion/function-overloads-inside-param.cpp
kadircet updated this revision to Diff 163971.
kadircet added a comment.
- Update comments.
Repository:
rC Clang
https://reviews.llvm.org/D51038
Files:
include/clang/Parse/Parser.h
lib/Parse/ParseExpr.cpp
test/CodeCompletion/function-overloads-inside-param.cpp
test/CodeCompletion/fun
rsmith added a comment.
Please also teach constant expression evaluation (lib/AST/ExprConstant.cpp) to
look through these builtins when evaluating.
https://reviews.llvm.org/D51200
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists
Oh, yes, sure
Best Regards
Christian
On 09/05/2018 12:05 AM, Richard Smith wrote:
This is breaking buildbots:
http://lab.llvm.org:8011/builders/clang-x86_64-linux-selfhost-modules/builds/19509
Can you take a look? Thanks!
On Tue, 4 Sep 2018 at 08:36, Christian Bruel via cfe-commits
mailto:
owenpan accepted this revision.
owenpan added a comment.
This revision is now accepted and ready to land.
In https://reviews.llvm.org/D51036#1223254, @sammccall wrote:
> In https://reviews.llvm.org/D51036#1223230, @melver wrote:
>
> > Awaiting remaining reviewer acceptance.
> >
> > FYI: I do not
ilya-biryukov added a comment.
@hokein, would it be fine if I submit this change for you?
It would be nice to get this fix in before you get back from vacation.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50438
___
cfe-commits ma
hokein added a comment.
In https://reviews.llvm.org/D50438#1224287, @ilya-biryukov wrote:
> @hokein, would it be fine if I submit this change for you?
> It would be nice to get this fix in before you get back from vacation.
Thanks, and sorry for the delay here, I was planning to submit it afte
Hi Sam,
This doesn't compile for me. Both clang 3.6.0 and gcc 5.4.0 complain:
[1/6] Building CXX object
tools/clang/tools/extra/clangd/CMakeFiles/clangDaemon.dir/index/Serialization.cpp.o
FAILED:
tools/clang/tools/extra/clangd/CMakeFiles/clangDaemon.dir/index/Serialization.cpp.o
/usr/bin/cla
ilya-biryukov added a comment.
Sorry for the delayed response. It seems we absolutely need this if mirroring
libclang is an absolute requirement.
We seem to have multiple implementation options:
Which classes to use for representing diagnostics? We can:
1. Reuse existing hierarchy for diagnost
101 - 161 of 161 matches
Mail list logo
