Re: [PATCH] D12406: [Analyzer] Add -analyzer-config option for function size the inliner considers as large

seaneveson added a comment. Ping http://reviews.llvm.org/D12406 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH][Solaris] Default to -fno-cxa-finalize

On Sat, Sep 05, 2015 at 12:25:28PM -0700, Saleem Abdulrasool wrote: > > Ping? > > > > Does this break with older Solaris releases? How far back did this change > in Solaris? The change itself should really be accompanied with a test. Hi, turns out I'm not really sure cxa_finalize.o was ever av

Re: [PATCH] D12400: Fix store detection for return value in CGCall

aadg accepted this revision. aadg added a reviewer: aadg. aadg added a comment. This revision is now accepted and ready to land. This looks ok to me. Repository: rL LLVM http://reviews.llvm.org/D12400 ___ cfe-commits mailing list cfe-commits@list

Re: [PATCH] D12666: [LibClang] Fix clang_getCursorAvailability

klimek added a subscriber: klimek. klimek accepted this revision. klimek added a reviewer: klimek. klimek added a comment. This revision is now accepted and ready to land. Wow, that goes a long while back into: http://reviews.llvm.org/rL111858 where the current case was handled somewhere else, and

Re: [PATCH] D11240: Add basic #include sorting functionality to clang-format

djasper updated the summary for this revision. djasper updated this revision to Diff 34137. djasper added a comment. Now properly implemented merging of two replacement sets that are meant to be applied in sequence. I think this functionality will also be highly useful, e.g. when formatting the

Re: [PATCH][www] Mention that libcxx is necessary on OS X

I’m also curious what exact test failed for you with the "can't find iostream” error. AFAIK, you should currently receive the explaining message instead. But anyway, I’d also like to see libcxx[abi] mentioned in the Getting started page. Kuba > On 04 Sep 2015, at 23:07, Yuri Gribov

Re: [PATCH] D10802: [mips] Interrupt attribute support.

dsanders accepted this revision. dsanders added a comment. This revision is now accepted and ready to land. LGTM with a few nits Comment at: include/clang/Basic/Attr.td:841-842 @@ +840,4 @@ +def MipsInterrupt : InheritableAttr, TargetSpecificAttr { + // NOTE: if you add any add

Re: [PATCH] D10802: [mips] Interrupt attribute support.

aaron.ballman added a subscriber: aaron.ballman. aaron.ballman requested changes to this revision. aaron.ballman added a reviewer: aaron.ballman. aaron.ballman added a comment. This revision now requires changes to proceed. Missing tests for the semantic warnings. For instance, I don't see any tes

Re: [PATCH] D12462: [PATCH] [clang-tidy] Add inconsistent declaration parameter name check

alexfh added a comment. > Now that I fixed all review issues, I think this version would be acceptable > for commit. @alexfh: do you agree? Almost. I've found a few more minor issues and also found out that the warnings could structured in a more useful way. See the inline comments. Thanks fo

[PATCH] D12675: Avoid using rvalue references with trivially copyable types.

angelgarcia created this revision. angelgarcia added a reviewer: klimek. angelgarcia added subscribers: cfe-commits, alexfh. When the dereference operator returns a value that is trivially copyable (like a pointer), copy it. After this change, modernize-loop-convert check can be applied to the who

Re: [PATCH] D12673: [analyzer] Remove whitespace in source code

honggyu.kim added a comment. If anyone thinks this patch is too big, I can submit smaller separated patches. http://reviews.llvm.org/D12673 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-co

Re: [PATCH] D12673: [analyzer] Remove whitespace in source code

sylvestre.ledru added a comment. Note: I am not the owner of this code but to me, this should be committed without a review. http://reviews.llvm.org/D12673 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailma

Re: [PATCH] D12673: [analyzer] Remove whitespace in source code

honggyu.kim added a comment. In http://reviews.llvm.org/D12673#240933, @sylvestre.ledru wrote: > Note: I am not the owner of this code but to me, this should be committed > without a review. I don't have commit permission, so please anyone commit it instead of me. http://reviews.llvm.org/D12

Re: [PATCH] D12675: Avoid using rvalue references with trivially copyable types.

alexfh accepted this revision. alexfh added a reviewer: alexfh. alexfh added a comment. This revision is now accepted and ready to land. Awesome! Looks good. http://reviews.llvm.org/D12675 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http

Re: [PATCH] D10305: [Clang Static Analyzer] Bug identification

honggyu.kim added a comment. Sorry for the late answer. I would like to write the comment for HTML report comparison first. > > As I mentioned, I think the simplist way of comparing HTML reports is to > > have the BugID in each HTML report so that we can just compare HTML reports > > >whether t

Re: [PATCH] D12675: Avoid using rvalue references with trivially copyable types.

klimek added inline comments. Comment at: clang-tidy/modernize/LoopConvertCheck.cpp:556-557 @@ -555,1 +555,4 @@ DerefByValue = true; +// We can assume that we have at least one Usage, because +// usagesAreConst() returned false. +QualType Type = Us

Re: [PATCH] D10305: [Clang Static Analyzer] Bug identification

honggyu.kim added a comment. I would like to also write about bug identification methods. As I observed the current CmpRuns.py script, the IssueIdentifier is defined as follows: def getIssueIdentifier(self) : id = self.getFileName() + "+" if 'issue_context' in self._data :

Re: [PATCH][Solaris] Add -lc also when linking shared libraries

On Sat, Sep 05, 2015 at 12:28:43PM -0700, Saleem Abdulrasool wrote: > > Ping? > > > > Testcase? Looks fine otherwise. Basically check that -lc is present when clang is called in a certain way I guess? Or something more sophisticated? Xan > > > > > From 8e81d6b095542c0ff1e28cf1f09d675f8afe1a2

Re: [PATCH] D11797: [LIbClang] Report the named type for ElaboratedType

klimek added a comment. Ok, so looking over the new patch: It seems like attributed types and decayed types are somewhat different from elaborated types: - decayed types don't lose any information if we look through to the original type - attributed types lose only information that can be consi

Re: [PATCH] D12675: Avoid using rvalue references with trivially copyable types.

angelgarcia updated this revision to Diff 34158. angelgarcia added a comment. Add struct with the parameters of the replacement. Also fix a potential source of crashes from a previous patch. http://reviews.llvm.org/D12675 Files: clang-tidy/modernize/LoopConvertCheck.cpp clang-tidy/moderniz

Re: [PATCH] D12675: Avoid using rvalue references with trivially copyable types.

angelgarcia marked 3 inline comments as done. Comment at: clang-tidy/modernize/LoopConvertCheck.cpp:658-663 @@ -646,7 +657,8 @@ + IsTriviallyCopyable = BeginPointeeType.isTriviallyCopyableType(*Context); } else { // Check for qualified types to avoid conversions f

Re: [PATCH][www] Mention that libcxx is necessary on OS X

libcxx is needed to run compiler-rt tests, but it shouldn't be needed to run clang's tests. And to _use_ your self-built compiler, you need to add `-isysroot $(xcrun -show-sdk-path)` to the compile flags to let the compiler find C headers anyway, which also makes it find the libc++ headers. For co

Re: [PATCH] D12675: Avoid using rvalue references with trivially copyable types.

klimek added inline comments. Comment at: clang-tidy/modernize/LoopConvertCheck.cpp:658-663 @@ -646,7 +657,8 @@ + IsTriviallyCopyable = BeginPointeeType.isTriviallyCopyableType(*Context); } else { // Check for qualified types to avoid conversions from non-const to

Re: [PATCH] D12675: Avoid using rvalue references with trivially copyable types.

angelgarcia updated this revision to Diff 34160. angelgarcia added a comment. Avoid relying in a single usage for determining if the type is trivially copyable. http://reviews.llvm.org/D12675 Files: clang-tidy/modernize/LoopConvertCheck.cpp clang-tidy/modernize/LoopConvertCheck.h clang-t

Re: [PATCH] D12675: Avoid using rvalue references with trivially copyable types.

klimek added inline comments. Comment at: clang-tidy/modernize/LoopConvertCheck.cpp:555-564 @@ +554,12 @@ +Descriptor.DerefByValue = true; +// Try to find the type of the elements on the container from the +// usages. We can assume that we have at least one

Re: [PATCH] D12675: Avoid using rvalue references with trivially copyable types.

angelgarcia added inline comments. Comment at: clang-tidy/modernize/LoopConvertCheck.cpp:659-664 @@ -646,7 +658,8 @@ + BeginPointeeType.isTriviallyCopyableType(*Context); } else { // Check for qualified types to avoid conversions from non-const to const

Re: [PATCH] D12675: Avoid using rvalue references with trivially copyable types.

klimek added inline comments. Comment at: clang-tidy/modernize/LoopConvertCheck.cpp:659-664 @@ -646,7 +658,8 @@ + BeginPointeeType.isTriviallyCopyableType(*Context); } else { // Check for qualified types to avoid conversions from non-const to const // i

Re: [PATCH] D12675: Avoid using rvalue references with trivially copyable types.

angelgarcia updated this revision to Diff 34162. angelgarcia added a comment. This is the test case I was worried about. http://reviews.llvm.org/D12675 Files: clang-tidy/modernize/LoopConvertCheck.cpp clang-tidy/modernize/LoopConvertCheck.h clang-tidy/modernize/LoopConvertUtils.cpp test

Re: [PATCH][Solaris] Add -lc also when linking shared libraries

On Mon, Sep 7, 2015 at 7:18 AM, Xan López wrote: > On Sat, Sep 05, 2015 at 12:28:43PM -0700, Saleem Abdulrasool wrote: > > > Ping? > > > > > > > Testcase? Looks fine otherwise. > > Basically check that -lc is present when clang is called in a certain > way I guess? Or something more sophisticate

Re: [PATCH][Solaris] Default to -fno-cxa-finalize

On Mon, Sep 7, 2015 at 2:28 AM, Xan López wrote: > On Sat, Sep 05, 2015 at 12:25:28PM -0700, Saleem Abdulrasool wrote: > > > Ping? > > > > > > > Does this break with older Solaris releases? How far back did this > change > > in Solaris? The change itself should really be accompanied with a test

Re: [PATCH][www] Mention that libcxx is necessary on OS X

> On 07 Sep 2015, at 17:21, Nico Weber wrote: > > libcxx is needed to run compiler-rt tests, but it shouldn't be needed to run > clang's tests. And to _use_ your self-built compiler, you need to add > `-isysroot $(xcrun -show-sdk-path)` to the compile flags to let the compiler > find C header

Re: [PATCH][www] Mention that libcxx is necessary on OS X

On Mon, Sep 7, 2015 at 9:17 AM, Kuba Brecka wrote: > > > On 07 Sep 2015, at 17:21, Nico Weber wrote: > > > > libcxx is needed to run compiler-rt tests, but it shouldn't be needed to > run clang's tests. And to _use_ your self-built compiler, you need to add > `-isysroot $(xcrun -show-sdk-path)`

Re: [PATCH] D12669: [libcxxabi] Fix alignment of pointers returned by fallback_malloc

compnerd added a comment. While I think that ensuring that the fallback malloc path works properly is needed, AIUI, this is still insufficient, as there is a first attempt at using malloc, which doesn't have alignment guarantees (except on Darwin). Also, a clang-format over the patch would be

Re: [PATCH] D9898: MismatchingNewDeleteDetector uses incorrect field, and finds no initializer

ismailp added a comment. Ping! It seems like this patch was forgotten. This fixes PR24730, which is reported recently. http://reviews.llvm.org/D9898 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listi

Re: [PATCH][Solaris] Add -lc also when linking shared libraries

On Mon, Sep 07, 2015 at 09:07:41AM -0700, Saleem Abdulrasool wrote: > > Basically check that -lc is present when clang is called in a certain > > way I guess? Or something more sophisticated? > > > Yeah, that it is present when a DSO or executable is linked. Right, maybe something like this. Pat

Re: [PATCH][Solaris] Default to -fno-cxa-finalize

OK! So here's the patch with a test. Xan On Mon, Sep 07, 2015 at 09:14:05AM -0700, Saleem Abdulrasool wrote: > On Mon, Sep 7, 2015 at 2:28 AM, Xan López wrote: > > > On Sat, Sep 05, 2015 at 12:25:28PM -0700, Saleem Abdulrasool wrote: > > > > Ping? > > > > > > > > > > Does this break with older

Re: [PATCH][Solaris] Default to -fno-cxa-finalize

On Mon, Sep 7, 2015 at 10:34 AM, Xan López wrote: > OK! So here's the patch with a test. The patch LGTM with a minor request for tweaking the commit message to have the context about the fact that cxa_finalize.o never shipped, and so this doesn't really cause any problems for older releases.

Re: [PATCH][Solaris] Default to -fno-cxa-finalize

On Mon, Sep 07, 2015 at 10:38:08AM -0700, Saleem Abdulrasool wrote: > The patch LGTM with a minor request for tweaking the commit message to have > the context about the fact that cxa_finalize.o never shipped, and so this > doesn't really cause any problems for older releases. Right. Tried to expl

r246974 - [X86][SSE4A] Added SSE4A IR + assembly codegen builtin tests

Author: rksimon Date: Mon Sep 7 15:10:11 2015 New Revision: 246974 URL: http://llvm.org/viewvc/llvm-project?rev=246974&view=rev Log: [X86][SSE4A] Added SSE4A IR + assembly codegen builtin tests Modified: cfe/trunk/test/CodeGen/sse4a-builtins.c Modified: cfe/trunk/test/CodeGen/sse4a-builtins

Re: [PATCH] D12406: [Analyzer] Add -analyzer-config option for function size the inliner considers as large

krememek added a subscriber: krememek. krememek added a comment. This looks fine, but I'm not a fan of the actual name chosen here. It's not very clear what it means by just looking at the name, and as we grow the number of configuration options that will become more important. A few suggestio

Re: [PATCH][Solaris] Default to -fno-cxa-finalize

On Mon, Sep 7, 2015 at 10:42 AM, Xan López wrote: > On Mon, Sep 07, 2015 at 10:38:08AM -0700, Saleem Abdulrasool wrote: > > The patch LGTM with a minor request for tweaking the commit message to > have > > the context about the fact that cxa_finalize.o never shipped, and so this > > doesn't reall

Re: [PATCH] D12669: [libcxxabi] Fix alignment of pointers returned by fallback_malloc

EricWF added a comment. In http://reviews.llvm.org/D12669#241020, @compnerd wrote: > While I think that ensuring that the fallback malloc path works properly is > needed, AIUI, this is still insufficient, as there is a first attempt at > using malloc, which doesn't have alignment guarantees (ex

Re: [PATCH] D9040: [analyzer] Make realloc(ptr, 0) handling equivalent to malloc(0).

ayartsev added a comment. Ping. http://reviews.llvm.org/D9040 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D12673: [analyzer] Remove whitespace in source code

dblaikie added a subscriber: dblaikie. dblaikie added a comment. We generally don't do large whitespace changes like this because of issue with version control (not all told are whitespace ignorant which makes history work difficult). I don't mind these changes personally, but it should get sign

Re: [PATCH] D12673: [analyzer] Remove whitespace in source code

We generally don't do large whitespace changes like this because of issue with version control (not all told are whitespace ignorant which makes history work difficult). I don't mind these changes personally, but it should get sign of from Ted or similar first. On Sep 7, 2015 5:47 AM, "Honggyu Kim

Re: [PATCH] D12247: [libc++] remove possible trailing padding from aligned_storage

EricWF accepted this revision. EricWF added a comment. This revision is now accepted and ready to land. LGTM. I don't want to hold this up any longer. @mclow.lists can comment on this after it's committed if he sees any issues. http://reviews.llvm.org/D12247 _

Re: [PATCH] D6964: [libcxx] SFINAE-friendly common_type

EricWF accepted this revision. EricWF added a comment. This revision is now accepted and ready to land. LGTM. I don't know why I've been putting this off for so long. There is still some contention over weither common_type should apply "decay" to every result or just the last one. This patch app

Re: [PATCH] D12644: Using -isysroot on Apple platform

EricWF added a comment. Why would we not use "-isysroot" on other platforms as well? http://reviews.llvm.org/D12644 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D12669: [libcxxabi] Fix alignment of pointers returned by fallback_malloc

EricWF added inline comments. Comment at: src/fallback_malloc.ipp:69 @@ -59,1 +68,3 @@ +// Size: 4 +// Alignment: 2 struct heap_node { compnerd wrote: > Make this a set of static_asserts? I was going to but I wasn't 100% sure this would be true on all platforms.

Re: [PATCH] D6964: [libcxx] SFINAE-friendly common_type

K-ballo added a comment. In http://reviews.llvm.org/D6964#241105, @EricWF wrote: > @K-ballo May I commit this with a couple of small changes? I want to fully > qualify all of the calls to "declval(...)". Please do. I have submitted bug reports in the past for pesky unqualified calls to `declv

[libcxx] r246977 - make common_type SFINAE-friendly and support void. Patch from Agustin Berge.

Author: ericwf Date: Mon Sep 7 19:13:57 2015 New Revision: 246977 URL: http://llvm.org/viewvc/llvm-project?rev=246977&view=rev Log: make common_type SFINAE-friendly and support void. Patch from Agustin Berge. This patch also fixes PR22135. (https://llvm.org/bugs/show_bug.cgi?id=22135) See the re

Re: [PATCH] D6964: [libcxx] SFINAE-friendly common_type

EricWF closed this revision. EricWF added a comment. Committed in r246977. Thanks again. http://reviews.llvm.org/D6964 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D12646: Add libc++ header path for DarwinClang builds

hintonda updated this revision to Diff 34175. hintonda removed reviewers: thakis, EricWF, kubabrecka. hintonda added a comment. Removed redundant code from InitHeaderSearch.cpp, it's handled more cleanly in AddClangCXXStdlibIncludeArgs. Also, newer versions of OSX add libc++ as the default. Per

Re: [PATCH] D12646: Add libc++ header path for DarwinClang builds

Not sure why arc removed some reviewers when I added the latest diff, but I've added them back. Sorry for the noise... On Mon, Sep 7, 2015 at 8:36 PM, don hinton wrote: > hintonda updated this revision to Diff 34175. > hintonda removed reviewers: thakis, EricWF, kubabrecka. > hintonda added a c

Re: [PATCH] D12673: [analyzer] Remove whitespace in source code

honggyu.kim added a comment. In http://reviews.llvm.org/D12673#241100, @dblaikie wrote: > We generally don't do large whitespace changes like this because of issue > with version control (not all told are whitespace ignorant which makes > history work difficult). I agree that it would be diff

Re: [PATCH] D12644: Using -isysroot on Apple platform

steven_wu added a comment. That works as well. I am not sure if there is any specific reason for using "--sysroot" so I proposed a safer fix. http://reviews.llvm.org/D12644 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.o

[PATCH] D12684: [INSTALL.txt] Fix formatting - 80 character line

beltex created this revision. beltex added a subscriber: cfe-commits. http://reviews.llvm.org/D12684 Files: INSTALL.txt Index: INSTALL.txt === --- INSTALL.txt +++ INSTALL.txt @@ -44,6 +44,6 @@ compiler and header files into the p

Re: [PATCH] D12673: [analyzer] Remove whitespace in source code

krememek added a comment. I am OK with taking these changes. For those using git, git blame -w should suffice to show the real blame history. http://reviews.llvm.org/D12673 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.

Re: [PATCH] D12673: [analyzer] Remove whitespace in source code

krememek accepted this revision. krememek added a comment. This revision is now accepted and ready to land. Committed in r246978. http://reviews.llvm.org/D12673 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/ma