ps4-buildslave1a down 08.23.2017

[Victor Leschuk via cfe-commits]

Hello all, ps4-buildslave1a is currently down. We are detecting the
cause and will fix it ASAP. I will inform when it is accessible again.

Sorry for the inconvenience.

-- 
Best Regards,

Victor Leschuk | Software Engineer |Access Softek

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r309226 - Headers: improve ARM EHABI coverage of unwind.h

[Saleem Abdulrasool via cfe-commits]

I believe that it is merely a compile-time issue (warning), it doesn't
prevent the use of compiler-rt on ARM as a libgcc replacement.  I need to
understand why my attempt to remove the `struct` modifier failed on one of
the buildbots.

On Tue, Aug 22, 2017 at 2:45 PM, Evgenii Stepanov  wrote:

> No. I don't have a easy way of reproducing this.
>
> On Tue, Aug 22, 2017 at 11:10 AM, Hans Wennborg  wrote:
> > Is there a bug filed? Since this was merged to 5.0.0, I'd like to know
> > if we broke something and if that is something that needs to be fixed.
> >
> > On Tue, Aug 22, 2017 at 10:46 AM, Evgenii Stepanov
> >  wrote:
> >> As I understand, using compiler-rt as libgcc replacement on ARM is
> >> currently broken because of this change, but I have not looked since
> >> my last message.
> >>
> >> On Mon, Aug 21, 2017 at 4:56 PM, Hans Wennborg 
> wrote:
> >>> Is there something we need for 5.0.0 here?
> >>>
> >>> On Sat, Aug 12, 2017 at 9:58 PM, Saleem Abdulrasool
> >>>  wrote:
>  Yeah, we should adjust that.  Technically, it is
> `_Unwind_Control_Block on
>  ARM EHABI.  However, _Unwind_Exception is aliased to that, which is
> why we
>  can use that in the personality routine.  We should adjust the
> sources for
>  the personality routine.
> 
>  On Fri, Aug 11, 2017 at 1:12 PM, Evgenii Stepanov
>   wrote:
> >
> > Hi,
> >
> > I've noticed that the code in
> > compiler-rt/lib/builtins/gcc_personality_v0.c refers to
> > _Unwind_Exception as "struct _Unwind_Exception". With this change, it
> > is not a struct anymore on ARM. Should that code be fixed, or is it a
> > problem in this change?
> >
> > compiler-rt/lib/builtins/gcc_personality_v0.c:153:23: error:
> > declaration of 'struct _Unwind_Exception' will not be visible outside
> > of this function [-Werror,-Wvisibility]
> > continueUnwind(struct _Unwind_Exception *exceptionObject,
> >
> > On Thu, Jul 27, 2017 at 9:46 AM, Hans Wennborg via cfe-commits
> >  wrote:
> > > Merged to 5.0 in r309290.
> > >
> > > On Wed, Jul 26, 2017 at 3:55 PM, Saleem Abdulrasool via cfe-commits
> > >  wrote:
> > >> Author: compnerd
> > >> Date: Wed Jul 26 15:55:23 2017
> > >> New Revision: 309226
> > >>
> > >> URL: http://llvm.org/viewvc/llvm-project?rev=309226=rev
> > >> Log:
> > >> Headers: improve ARM EHABI coverage of unwind.h
> > >>
> > >> Ensure that we define the `_Unwind_Control_Block` structure used
> on ARM
> > >> EHABI targets.  This is needed for building libc++abi with the
> unwind.h
> > >> from the resource dir.  A minor fallout of this is that we needed
> to
> > >> create a typedef for _Unwind_Exception to work across ARM EHABI
> and
> > >> non-EHABI targets.  The structure definitions here are based
> originally
> > >> on the documentation from ARM under the "Exception Handling ABI
> for the
> > >> ARM® Architecture" Section 7.2.  They are then adjusted to more
> closely
> > >> reflect the definition in libunwind from LLVM.  Those changes are
> > >> compatible in layout but permit easier use in libc++abi and help
> > >> maintain compatibility between libunwind and the compiler provided
> > >> definition.
> > >>
> > >> Modified:
> > >> cfe/trunk/lib/Headers/unwind.h
> > >>
> > >> Modified: cfe/trunk/lib/Headers/unwind.h
> > >> URL:
> > >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/
> unwind.h?rev=309226=309225=309226=diff
> > >>
> > >> 
> ==
> > >> --- cfe/trunk/lib/Headers/unwind.h (original)
> > >> +++ cfe/trunk/lib/Headers/unwind.h Wed Jul 26 15:55:23 2017
> > >> @@ -76,7 +76,13 @@ typedef intptr_t _sleb128_t;
> > >>  typedef uintptr_t _uleb128_t;
> > >>
> > >>  struct _Unwind_Context;
> > >> +#if defined(__arm__) && !(defined(__USING_SJLJ_EXCEPTIONS__) ||
> > >> defined(__ARM_DWARF_EH___))
> > >> +struct _Unwind_Control_Block;
> > >> +typedef struct _Unwind_Control_Block _Unwind_Exception; /* Alias
> */
> > >> +#else
> > >>  struct _Unwind_Exception;
> > >> +typedef struct _Unwind_Exception _Unwind_Exception;
> > >> +#endif
> > >>  typedef enum {
> > >>_URC_NO_REASON = 0,
> > >>  #if defined(__arm__) && !defined(__USING_SJLJ_EXCEPTIONS__) && \
> > >> @@ -109,8 +115,42 @@ typedef enum {
> > >>  } _Unwind_Action;
> > >>
> > >>  typedef void (*_Unwind_Exception_Cleanup_
> Fn)(_Unwind_Reason_Code,
> > >> - struct
> _Unwind_Exception
> > >> *);
> > >> + _Unwind_Exception
> *);
> 

[PATCH] D36555: Move x86-specific sources to x86-specific source lists.

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd accepted this revision.
compnerd added a comment.

Would be nice to split up the PPC fixes into its own commit.


https://reviews.llvm.org/D36555



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36555: Move x86-specific sources to x86-specific source lists.

[Michał Górny via Phabricator via cfe-commits]

mgorny added inline comments.



Comment at: compiler-rt/lib/builtins/CMakeLists.txt:223
+  cpu_model.c
+  divxc3.c
+  fixxfdi.c

saugustine wrote:
> mgorny wrote:
> > This and the following files have only:
> > 
> > ```
> > #if !_ARCH_PPC
> > ```
> > 
> > so I suppose it's currently included on more targets than x86. If it's 
> > really not useful there, I think it'd be better to update the `#if`s first 
> > (and preferably get the author to review).
> As discussed in D36764, these all assume 80-bit floats, and therefore are 
> only useful on x86 (and ancient 68ks)--and, in fact, the tests for these 
> functions are disabled for everything but x86. I have left these #ifs in for 
> now, but will remove them after this patch gets submitted (and add comments 
> about them needing 80-bit floats). Otherwise, powerpc builds will be broken.
Please do not remove the guards. We're using them in other arch-specific files 
as well. Instead, update them to state the correct targets.


https://reviews.llvm.org/D36555



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36876: [IRGen] Evaluate constant static variables referenced through member expressions

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: lib/CodeGen/CGExprComplex.cpp:163
+else
+  CGF.EmitLValue(ME->getBase());
+return *Constant;

rjmccall wrote:
> There's an EmitIgnoredExpr you could use.
> 
> Also, I think it would be fine to add a generic tryEmitMemberExprAsConstant 
> that takes a MemberExpr and does this DRE stuff behind the scenes; it's not 
> at all different for the different emitters.
Well, actually, now I see why it's different for the complex emitter, but I 
think you could probably extract out a function that forms a complex pair from 
a constant output without too much trouble.

Also, as long as you're working with this, I think it's likely that the Agg 
emitter needs to handle this, too.  I'm not sure I accept the claim there that 
constant r-value emission never applies to aggregates, but at the very least 
you need to handle references just as the DRE case does.


Repository:
  rL LLVM

https://reviews.llvm.org/D36876



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D36562: [Bitfield] Make the bitfield a separate location if it has width of legal integer type and its bit offset is naturally aligned for the type

[Wei Mi via cfe-commits]

On Tue, Aug 22, 2017 at 7:03 PM, Xinliang David Li  wrote:
>
>
> On Tue, Aug 22, 2017 at 6:37 PM, Chandler Carruth via Phabricator
>  wrote:
>>
>> chandlerc added a comment.
>>
>> I'm really not a fan of the degree of complexity and subtlety that this
>> introduces into the frontend, all to allow particular backend optimizations.
>>
>> I feel like this is Clang working around a fundamental deficiency in LLVM
>> and we should instead find a way to fix this in LLVM itself.
>>
>> As has been pointed out before, user code can synthesize large integers
>> that small bit sequences are extracted from, and Clang and LLVM should
>> handle those just as well as actual bitfields.
>>
>> Can we see how far we can push the LLVM side before we add complexity to
>> Clang here? I understand that there remain challenges to LLVM's stuff, but I
>> don't think those challenges make *all* of the LLVM improvements off the
>> table, I don't think we've exhausted all ways of improving the LLVM changes
>> being proposed, and I think we should still land all of those and
>> re-evaluate how important these issues are when all of that is in place.
>
>
> The main challenge of doing  this in LLVM is that inter-procedural analysis
> (and possibly cross module) is needed (for store forwarding issues).
>
> Wei, perhaps you can provide concrete test case to illustrate the issue so
> that reviewers have a good understanding.
>
> David

Here is a runable testcase:
 1.cc 
class A {
public:
  unsigned long f1:2;
  unsigned long f2:6;
  unsigned long f3:8;
  unsigned long f4:4;
};
A a;
unsigned long b;
unsigned long N = 10;

__attribute__((noinline))
void foo() {
  a.f3 = 3;
}

__attribute__((noinline))
void goo() {
  b = a.f3;
}

int main() {
  unsigned long i;
  for (i = 0; i < N; i++) {
foo();
goo();
  }
}

Now trunk takes about twice running time compared with trunk + this
patch. That is because trunk shrinks the store of a.f3 in foo (Done by
DagCombiner) but not shrink the load of a.f3 in goo, so store
forwarding will be blocked.

The testcases shows the potential problem of store shrinking. Before
we decide to do store shrinking, we need to know all the related loads
will be shrunk,  and that requires IPA analysis. Otherwise, when load
shrinking was blocked for some difficult case (Like the instcombine
case described in
https://www.mail-archive.com/cfe-commits@lists.llvm.org/msg65085.html),
performance regression will happen.

Wei.


>>
>>
>>
>> Repository:
>>   rL LLVM
>>
>> https://reviews.llvm.org/D36562
>>
>>
>>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37042: Teach clang to tolerate the 'p = nullptr + n' idiom used by glibc

[David Majnemer via Phabricator via cfe-commits]

majnemer added a comment.

In https://reviews.llvm.org/D37042#849726, @majnemer wrote:

> I'd expect this to be more limited.
>
> Something like, "if we have a BinaryOperator of + between a CStyleCastExpr of 
> NullToPointer and 'X', emit inttoptr(X)"


Although maybe that is too strict... I guess stuff gets hairy quickly if you 
want to be able to also catch (char*)NULL + x


https://reviews.llvm.org/D37042



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36876: [IRGen] Evaluate constant static variables referenced through member expressions

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: lib/CodeGen/CGExprComplex.cpp:163
+else
+  CGF.EmitLValue(ME->getBase());
+return *Constant;

There's an EmitIgnoredExpr you could use.

Also, I think it would be fine to add a generic tryEmitMemberExprAsConstant 
that takes a MemberExpr and does this DRE stuff behind the scenes; it's not at 
all different for the different emitters.


Repository:
  rL LLVM

https://reviews.llvm.org/D36876



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37042: Teach clang to tolerate the 'p = nullptr + n' idiom used by glibc

[David Majnemer via Phabricator via cfe-commits]

majnemer added a comment.

I'd expect this to be more limited.

Something like, "if we have a BinaryOperator of + between a CStyleCastExpr of 
NullToPointer and 'X', emit inttoptr(X)"


https://reviews.llvm.org/D37042



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35082: [OpenCL] Add LangAS::opencl_private to represent private address space in AST

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: lib/Sema/SemaDecl.cpp:11846
   // an address space.
   if (T.getAddressSpace() != 0) {
 // OpenCL allows function arguments declared to be an array of a type

yaxunl wrote:
> Anastasia wrote:
> > yaxunl wrote:
> > > Anastasia wrote:
> > > > yaxunl wrote:
> > > > > Anastasia wrote:
> > > > > > Could we use `LangAS::Default` here too?
> > > > > done
> > > > Sorry, I wasn't clear. I think we could have:
> > > > 
> > > >   if (T.getAddressSpace() != LangAS::Default && T.getAddressSpace() != 
> > > > LangAS::opencl_private)
> > > > 
> > > > and then original condition. It is a bit clearer I think.
> > > No. For OpenCL, the condition is on line 11847 and 11848. An array type 
> > > in other address spaces is allowed.
> > I think the initial condition was `T.getAddressSpace() != 0` i.e. if not 
> > private address space.
> > 
> > So now since we added private this should be `T.getAddressSpace() 
> > !=LangAS::opencl_private` but we can have the default case as well hence 
> > 'T.getAddressSpace() !=LangAS::opencl_private || T.getAddressSpace() 
> > !=LangAS::Default'.
> > 
> > This entire case seems to be for OpenCL anyways. So you could also move out 
> > the OpenCL check if you prefer. I am just trying to see if we can make this 
> > easier to understand.
> This is not just for OpenCL.
> 
> The condition expresses the following logic:
> 
> For non-OpenCL languages, only default addr space is allowed on function 
> argument.
> 
> For OpenCL, for array type argument, any addr space is allowed; for non-array 
> type argument, only private addr space is allowed.
Parameters declared with type 'foo T[]' will be canonicalized to 'foo T*' when 
forming the function type, so that's not actually a different rule in the end.


https://reviews.llvm.org/D35082



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D36562: [Bitfield] Make the bitfield a separate location if it has width of legal integer type and its bit offset is naturally aligned for the type

[Xinliang David Li via cfe-commits]

On Tue, Aug 22, 2017 at 7:46 PM, Chandler Carruth 
wrote:

>
>
> On Tue, Aug 22, 2017 at 7:18 PM Xinliang David Li via llvm-commits <
> llvm-comm...@lists.llvm.org> wrote:
>
>> On Tue, Aug 22, 2017 at 7:10 PM, Chandler Carruth via llvm-commits <
>> llvm-comm...@lists.llvm.org> wrote:
>>
>>> On Tue, Aug 22, 2017 at 7:03 PM Xinliang David Li via cfe-commits <
>>> cfe-commits@lists.llvm.org> wrote:
>>>
 On Tue, Aug 22, 2017 at 6:37 PM, Chandler Carruth via Phabricator <
 revi...@reviews.llvm.org> wrote:

> chandlerc added a comment.
>
> I'm really not a fan of the degree of complexity and subtlety that
> this introduces into the frontend, all to allow particular backend
> optimizations.
>
> I feel like this is Clang working around a fundamental deficiency in
> LLVM and we should instead find a way to fix this in LLVM itself.
>
> As has been pointed out before, user code can synthesize large
> integers that small bit sequences are extracted from, and Clang and LLVM
> should handle those just as well as actual bitfields.
>
> Can we see how far we can push the LLVM side before we add complexity
> to Clang here? I understand that there remain challenges to LLVM's stuff,
> but I don't think those challenges make *all* of the LLVM improvements off
> the table, I don't think we've exhausted all ways of improving the LLVM
> changes being proposed, and I think we should still land all of those and
> re-evaluate how important these issues are when all of that is in place.
>

 The main challenge of doing  this in LLVM is that inter-procedural
 analysis (and possibly cross module) is needed (for store forwarding
 issues).

 Wei, perhaps you can provide concrete test case to illustrate the issue
 so that reviewers have a good understanding.

>>>
>>> It doesn't seem like all options for addressing that have been
>>> exhausted. And even then, I feel like trying to fix this with non-obvious
>>> (to the programmer) frontend heuristics isn't a good solution. I actually
>>> *prefer* the source work around of "don't use a bitfield if you *must* have
>>> narrow width access across modules where the optimizer cannot see enough to
>>> narrow them and you happen to know that there is a legal narrow access that
>>> works". Because that way the programmer has *control* over this rather than
>>> being at the whim of whichever side of the heuristic they end up on.
>>>
>>
>>
>> The source workaround solution *does not* scale. Most importantly, user
>> may not even be aware of the problem (and performance loss) unless
>>  compiling the code with another compiler and notice the performance
>> difference.
>>
>
> I don't really understand this argument...
>
> I don't think we can realistically chase all performance problems that
> users aren't aware of or can't measure. And our users *do* care and seem
> very good at benchmarking and noticing problems. =]
>
>
Here you assume that there is single hotspot that user can notice easily,
but this is usually not the case. In a very large scale, we have a long
tail of not so hot functions, but similar problems like this can add up.

Besides if the problem is obvious for user to detect, why wasn't this
problem detected in LLVM earlier?   I admit, in this particular case, we
actually relied on the competitive analysis -- that is why I said depending
on users for this is not reliable.

For yearsI have told our users (optimization) not to hack source code/or do
manual performance tuning (for instance, adding always inlining, adding
inline keyword, adding manually inserted prefetchings, manual loop
unrolling/peelings, etc ). Not only does this method does not scale (it
only helps user's own program), but more importantly it may get stale
overtime -- for instance, due to hardware change or shifting of hot spots.
It is not uncommon to see user's manual optimiztion which used to work
later becomes performance 'bug' without being noticed for years.   I am
totally fine with temporary source workaround, but they should not relied
upon longer term.



> I also think it is OK if in rare cases programmers have to adapt their
> source code to optimize well. We can and should make this as rare as
> reasonable from an engineering tradeoff perspective, but at a certain point
> we need to work with programmers to find a way to solve the problem. There
> exist coding patterns that Clang and LLVM will never be particularly
> effective at optimizing, and that seems OK. We just need to have reasonable
> engineering requirements for those cases:
>

As I said, for rare cases or as a temporary solution, it is fine to use
source workaround, but we should continue to push for compiler based
solution because compiler is for everybody/every app.  If compiler really
sucks at something, it should at least produce warnings about (with
something like performance sanitizer), but that should 

[PATCH] D36678: [OpenCL] Do not use vararg in emitted functions for enqueue_kernel

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl marked an inline comment as done.
yaxunl added inline comments.



Comment at: test/CodeGenOpenCL/cl20-device-side-enqueue.cl:116
+  // B32: store i32 4, i32* %[[TMP3]], align 4
+  // B32: call i32 @__enqueue_kernel_vaargs(%opencl.queue_t{{.*}}* [[DEF_Q]], 
i32 [[FLAGS]], %struct.ndrange_t* [[NDR]]{{(.[0-9]+)?}}, i8 addrspace(4)* 
addrspacecast (i8 addrspace(1)* bitcast ({ i8**, i32, i32, i8*, 
%struct.__block_descriptor addrspace(2)* } addrspace(1)* 
@__block_literal_global{{(.[0-9]+)?}} to i8 addrspace(1)*) to i8 
addrspace(4)*), i32 3, i32* %[[TMP1]])
+  // B64: %[[TMP:.*]] = alloca [3 x i64]

Anastasia wrote:
> You are not checking the arrays in the other calls too?
The logic is the same and the same lamba is called for emitting the IR. Is it 
necessary to do the same check for all the cases?


https://reviews.llvm.org/D36678



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35082: [OpenCL] Add LangAS::opencl_private to represent private address space in AST

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl marked 6 inline comments as done.
yaxunl added inline comments.



Comment at: lib/Sema/SemaDecl.cpp:11846
   // an address space.
   if (T.getAddressSpace() != 0) {
 // OpenCL allows function arguments declared to be an array of a type

Anastasia wrote:
> yaxunl wrote:
> > Anastasia wrote:
> > > yaxunl wrote:
> > > > Anastasia wrote:
> > > > > Could we use `LangAS::Default` here too?
> > > > done
> > > Sorry, I wasn't clear. I think we could have:
> > > 
> > >   if (T.getAddressSpace() != LangAS::Default && T.getAddressSpace() != 
> > > LangAS::opencl_private)
> > > 
> > > and then original condition. It is a bit clearer I think.
> > No. For OpenCL, the condition is on line 11847 and 11848. An array type in 
> > other address spaces is allowed.
> I think the initial condition was `T.getAddressSpace() != 0` i.e. if not 
> private address space.
> 
> So now since we added private this should be `T.getAddressSpace() 
> !=LangAS::opencl_private` but we can have the default case as well hence 
> 'T.getAddressSpace() !=LangAS::opencl_private || T.getAddressSpace() 
> !=LangAS::Default'.
> 
> This entire case seems to be for OpenCL anyways. So you could also move out 
> the OpenCL check if you prefer. I am just trying to see if we can make this 
> easier to understand.
This is not just for OpenCL.

The condition expresses the following logic:

For non-OpenCL languages, only default addr space is allowed on function 
argument.

For OpenCL, for array type argument, any addr space is allowed; for non-array 
type argument, only private addr space is allowed.


https://reviews.llvm.org/D35082



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35082: [OpenCL] Add LangAS::opencl_private to represent private address space in AST

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl updated this revision to Diff 112288.
yaxunl edited the summary of this revision.
yaxunl added a comment.
Herald added subscribers: nhaehnle, jholewinski.

Add a flag to indicate whether address space qualifier is implicit and only 
print explicit address space in diagnostics.


https://reviews.llvm.org/D35082

Files:
  include/clang/AST/ASTContext.h
  include/clang/AST/Type.h
  include/clang/Basic/AddressSpaces.h
  lib/AST/ASTContext.cpp
  lib/AST/Expr.cpp
  lib/AST/ItaniumMangle.cpp
  lib/AST/TypePrinter.cpp
  lib/Basic/Targets/AMDGPU.cpp
  lib/Basic/Targets/NVPTX.h
  lib/Basic/Targets/SPIR.h
  lib/Basic/Targets/TCE.h
  lib/CodeGen/CGDecl.cpp
  lib/Sema/SemaChecking.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaType.cpp
  test/CodeGenOpenCL/address-spaces-mangling.cl
  test/SemaOpenCL/address-spaces-conversions-cl2.0.cl
  test/SemaOpenCL/address-spaces.cl
  test/SemaOpenCL/atomic-ops.cl
  test/SemaOpenCL/cl20-device-side-enqueue.cl
  test/SemaOpenCL/invalid-block.cl
  test/SemaOpenCL/invalid-pipes-cl2.0.cl
  test/SemaOpenCL/null_literal.cl

Index: test/SemaOpenCL/null_literal.cl
===
--- test/SemaOpenCL/null_literal.cl
+++ test/SemaOpenCL/null_literal.cl
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 -verify %s
-// RUN: %clang_cc1 -cl-std=CL2.0 -DCL20 -verify %s
+// RUN: %clang_cc1 -cl-std=CL2.0 -verify %s
 
 #define NULL ((void*)0)
 
@@ -13,9 +13,17 @@
 
 constant int* ptr4 = (global void*)0; // expected-error{{initializing '__constant int *' with an expression of type '__global void *' changes address space of pointer}}
 
-#ifdef CL20
+#if __OPENCL_C_VERSION__ >= 200
+
 // Accept explicitly pointer to generic address space in OpenCL v2.0.
 global int* ptr5 = (generic void*)0;
+
+global int *ptr = (private void *)0; // expected-error{{initializing '__global int *' with an expression of type '__private void *' changes address space of pointer}}
+
+#else
+
+global int *ptr = (private void *)0;
+
 #endif
 
 global int* ptr6 = (local void*)0; // expected-error{{initializing '__global int *' with an expression of type '__local void *' changes address space of pointer}}
Index: test/SemaOpenCL/invalid-pipes-cl2.0.cl
===
--- test/SemaOpenCL/invalid-pipes-cl2.0.cl
+++ test/SemaOpenCL/invalid-pipes-cl2.0.cl
@@ -3,7 +3,7 @@
 global pipe int gp;// expected-error {{type '__global read_only pipe int' can only be used as a function parameter in OpenCL}}
 global reserve_id_t rid;  // expected-error {{the '__global reserve_id_t' type cannot be used to declare a program scope variable}}
 
-extern pipe write_only int get_pipe(); // expected-error {{type '__global write_only pipe int ()' can only be used as a function parameter in OpenCL}}
+extern pipe write_only int get_pipe(); // expected-error {{type 'write_only pipe int ()' can only be used as a function parameter in OpenCL}}
 
 kernel void test_invalid_reserved_id(reserve_id_t ID) { // expected-error {{'reserve_id_t' cannot be used as the type of a kernel parameter}}
 }
Index: test/SemaOpenCL/invalid-block.cl
===
--- test/SemaOpenCL/invalid-block.cl
+++ test/SemaOpenCL/invalid-block.cl
@@ -12,7 +12,7 @@
   };
   f0(bl1);
   f0(bl2);
-  bl1 = bl2;  // expected-error{{invalid operands to binary expression ('int (__generic ^const)(void)' and 'int (__generic ^const)(void)')}}
+  bl1 = bl2;  // expected-error{{invalid operands to binary expression ('int (^const)(void)' and 'int (^const)(void)')}}
   int (^const bl3)(); // expected-error{{invalid block variable declaration - must be initialized}}
 }
 
@@ -28,10 +28,10 @@
 
 // A block cannot be the return value of a function.
 typedef int (^bl_t)(void);
-bl_t f3(bl_t bl); // expected-error{{declaring function return value of type 'bl_t' (aka 'int (__generic ^const)(void)') is not allowed}}
+bl_t f3(bl_t bl); // expected-error{{declaring function return value of type 'bl_t' (aka 'int (^const)(void)') is not allowed}}
 
 struct bl_s {
-  int (^bl)(void); // expected-error {{the 'int (__generic ^const)(void)' type cannot be used to declare a structure or union field}}
+  int (^bl)(void); // expected-error {{the 'int (^const)(void)' type cannot be used to declare a structure or union field}}
 };
 
 void f4() {
@@ -53,18 +53,18 @@
   bl2_t bl2 = ^(int i) {
 return 2;
   };
-  bl2_t arr[] = {bl1, bl2}; // expected-error {{array of 'bl2_t' (aka 'int (__generic ^const)(int)') type is invalid in OpenCL}}
+  bl2_t arr[] = {bl1, bl2}; // expected-error {{array of 'bl2_t' (aka 'int (^const)(int)') type is invalid in OpenCL}}
   int tmp = i ? bl1(i)  // expected-error {{block type cannot be used as expression in ternary expression in OpenCL}}
   : bl2(i); // expected-error {{block type cannot be used as expression in ternary expression in OpenCL}}
 }
 // A block pointer type and all pointer 

[PATCH] D35533: [Basic] Update CMakeLists.txt to handle Repo

[MinSeong Kim via Phabricator via cfe-commits]

minseong.kim added a comment.

I will test your patch with repo. Thanks for your time and efforts, @hintonda.


https://reviews.llvm.org/D35533



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D36562: [Bitfield] Make the bitfield a separate location if it has width of legal integer type and its bit offset is naturally aligned for the type

[Chandler Carruth via cfe-commits]

On Tue, Aug 22, 2017 at 7:18 PM Xinliang David Li via llvm-commits <
llvm-comm...@lists.llvm.org> wrote:

> On Tue, Aug 22, 2017 at 7:10 PM, Chandler Carruth via llvm-commits <
> llvm-comm...@lists.llvm.org> wrote:
>
>> On Tue, Aug 22, 2017 at 7:03 PM Xinliang David Li via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>>> On Tue, Aug 22, 2017 at 6:37 PM, Chandler Carruth via Phabricator <
>>> revi...@reviews.llvm.org> wrote:
>>>
 chandlerc added a comment.

 I'm really not a fan of the degree of complexity and subtlety that this
 introduces into the frontend, all to allow particular backend 
 optimizations.

 I feel like this is Clang working around a fundamental deficiency in
 LLVM and we should instead find a way to fix this in LLVM itself.

 As has been pointed out before, user code can synthesize large integers
 that small bit sequences are extracted from, and Clang and LLVM should
 handle those just as well as actual bitfields.

 Can we see how far we can push the LLVM side before we add complexity
 to Clang here? I understand that there remain challenges to LLVM's stuff,
 but I don't think those challenges make *all* of the LLVM improvements off
 the table, I don't think we've exhausted all ways of improving the LLVM
 changes being proposed, and I think we should still land all of those and
 re-evaluate how important these issues are when all of that is in place.

>>>
>>> The main challenge of doing  this in LLVM is that inter-procedural
>>> analysis (and possibly cross module) is needed (for store forwarding
>>> issues).
>>>
>>> Wei, perhaps you can provide concrete test case to illustrate the issue
>>> so that reviewers have a good understanding.
>>>
>>
>> It doesn't seem like all options for addressing that have been exhausted.
>> And even then, I feel like trying to fix this with non-obvious (to the
>> programmer) frontend heuristics isn't a good solution. I actually *prefer*
>> the source work around of "don't use a bitfield if you *must* have narrow
>> width access across modules where the optimizer cannot see enough to narrow
>> them and you happen to know that there is a legal narrow access that
>> works". Because that way the programmer has *control* over this rather than
>> being at the whim of whichever side of the heuristic they end up on.
>>
>
>
> The source workaround solution *does not* scale. Most importantly, user
> may not even be aware of the problem (and performance loss) unless
>  compiling the code with another compiler and notice the performance
> difference.
>

I don't really understand this argument...

I don't think we can realistically chase all performance problems that
users aren't aware of or can't measure. And our users *do* care and seem
very good at benchmarking and noticing problems. =]

I also think it is OK if in rare cases programmers have to adapt their
source code to optimize well. We can and should make this as rare as
reasonable from an engineering tradeoff perspective, but at a certain point
we need to work with programmers to find a way to solve the problem. There
exist coding patterns that Clang and LLVM will never be particularly
effective at optimizing, and that seems OK. We just need to have reasonable
engineering requirements for those cases:

1) They need to be quite rare. So far, this issue has come up fairly
infrequently. Not never, but there are many issues that seem to come up
much more often such as inlining issues.

2) We need an effective way to explain how programmers should write code to
optimize well, and this explanation needs to be as simple as possible.

3) We likely need to give programmers *choices* about which direction their
code gets optimized for. And this is (IMO) the biggest practical issue with
the approach in this change (beyond race detection and other semantic
issues).


I'm suggesting that we teach programmers to choose between a bitfield to
get combining-friendly access to tightly packed data, and integer types of
an appropriate size to get inexpensive and predictable loads and stores.
For sub-byte-width and non-byte-aligned bitfields this is already a
necessary property and so it seems like a consistent and easily explained
and taught model.

While there are times where there is a single "right" answer and we only
have to provide that, I don't think this is one of them. I've seen rare but
serious complaints about *both* failing to combine adjacent bit fields and
failing to narrow to small legal integer types. These seem in fundamental
tension, and I would prefer to establish idiomatic patterns for programmers
to request the behavior then need in their application.


>
> David
>
>>
>>
>>>
>>> David
>>>


 Repository:
   rL LLVM

 https://reviews.llvm.org/D36562



 ___
>>> cfe-commits mailing list
>>> cfe-commits@lists.llvm.org
>>> 

r311519 - [ODRHash] Diagnose differing template parameters.

[Richard Trieu via cfe-commits]

Author: rtrieu
Date: Tue Aug 22 19:43:59 2017
New Revision: 311519

URL: http://llvm.org/viewvc/llvm-project?rev=311519=rev
Log:
[ODRHash] Diagnose differing template parameters.

Modified:
cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td
cfe/trunk/lib/AST/ODRHash.cpp
cfe/trunk/lib/Serialization/ASTReader.cpp
cfe/trunk/test/Modules/odr_hash.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td?rev=311519=311518=311519=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td Tue Aug 22 
19:43:59 2017
@@ -122,6 +122,24 @@ def note_second_module_difference : Note
 def err_module_odr_violation_different_instantiations : Error<
   "instantiation of %q0 is different in different modules">;
 
+def err_module_odr_violation_template_parameter : Error <
+  "%q0 has different definitions in different modules; first difference is "
+  "%select{definition in module '%2'|defined here}1 found "
+  "%select{"
+  "unnamed template parameter|"
+  "template parameter %4|"
+  "template parameter with %select{no |}4default argument|"
+  "template parameter with default argument}3">;
+
+
+def note_module_odr_violation_template_parameter : Note <
+  "but in '%0' found "
+  "%select{"
+  "unnamed template parameter %2|"
+  "template parameter %2|"
+  "template parameter with %select{no |}2default argument|"
+  "template parameter with different default argument}1">;
+
 def err_module_odr_violation_mismatch_decl : Error<
   "%q0 has different definitions in different modules; first difference is "
   "%select{definition in module '%2'|defined here}1 found "

Modified: cfe/trunk/lib/AST/ODRHash.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ODRHash.cpp?rev=311519=311518=311519=diff
==
--- cfe/trunk/lib/AST/ODRHash.cpp (original)
+++ cfe/trunk/lib/AST/ODRHash.cpp Tue Aug 22 19:43:59 2017
@@ -158,7 +158,14 @@ void ODRHash::AddTemplateArgument(Templa
   }
 }
 
-void ODRHash::AddTemplateParameterList(const TemplateParameterList *TPL) {}
+void ODRHash::AddTemplateParameterList(const TemplateParameterList *TPL) {
+  assert(TPL && "Expecting non-null pointer.");
+
+  ID.AddInteger(TPL->size());
+  for (auto *ND : TPL->asArray()) {
+AddSubDecl(ND);
+  }
+}
 
 void ODRHash::clear() {
   DeclMap.clear();
@@ -236,6 +243,10 @@ public:
 }
   }
 
+  void AddTemplateArgument(TemplateArgument TA) {
+Hash.AddTemplateArgument(TA);
+  }
+
   void Visit(const Decl *D) {
 ID.AddInteger(D->getKind());
 Inherited::Visit(D);
@@ -343,6 +354,42 @@ public:
   AddDecl(D->getFriendDecl());
 }
   }
+
+  void VisitTemplateTypeParmDecl(const TemplateTypeParmDecl *D) {
+// Only care about default arguments as part of the definition.
+const bool hasDefaultArgument =
+D->hasDefaultArgument() && !D->defaultArgumentWasInherited();
+Hash.AddBoolean(hasDefaultArgument);
+if (hasDefaultArgument) {
+  AddTemplateArgument(D->getDefaultArgument());
+}
+
+Inherited::VisitTemplateTypeParmDecl(D);
+  }
+
+  void VisitNonTypeTemplateParmDecl(const NonTypeTemplateParmDecl *D) {
+// Only care about default arguments as part of the definition.
+const bool hasDefaultArgument =
+D->hasDefaultArgument() && !D->defaultArgumentWasInherited();
+Hash.AddBoolean(hasDefaultArgument);
+if (hasDefaultArgument) {
+  AddStmt(D->getDefaultArgument());
+}
+
+Inherited::VisitNonTypeTemplateParmDecl(D);
+  }
+
+  void VisitTemplateTemplateParmDecl(const TemplateTemplateParmDecl *D) {
+// Only care about default arguments as part of the definition.
+const bool hasDefaultArgument =
+D->hasDefaultArgument() && !D->defaultArgumentWasInherited();
+Hash.AddBoolean(hasDefaultArgument);
+if (hasDefaultArgument) {
+  AddTemplateArgument(D->getDefaultArgument().getArgument());
+}
+
+Inherited::VisitTemplateTemplateParmDecl(D);
+  }
 };
 } // namespace
 
@@ -403,6 +450,12 @@ void ODRHash::AddCXXRecordDecl(const CXX
   for (auto SubDecl : Decls) {
 AddSubDecl(SubDecl);
   }
+
+  const ClassTemplateDecl *TD = Record->getDescribedClassTemplate();
+  AddBoolean(TD);
+  if (TD) {
+AddTemplateParameterList(TD->getTemplateParameters());
+  }
 }
 
 void ODRHash::AddDecl(const Decl *D) {

Modified: cfe/trunk/lib/Serialization/ASTReader.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=311519=311518=311519=diff
==
--- cfe/trunk/lib/Serialization/ASTReader.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReader.cpp Tue Aug 22 19:43:59 2017
@@ 

Re: [PATCH] D36562: [Bitfield] Make the bitfield a separate location if it has width of legal integer type and its bit offset is naturally aligned for the type

[Hal Finkel via cfe-commits]

On 08/22/2017 09:18 PM, Xinliang David Li via llvm-commits wrote:



On Tue, Aug 22, 2017 at 7:10 PM, Chandler Carruth via llvm-commits 
> wrote:


On Tue, Aug 22, 2017 at 7:03 PM Xinliang David Li via cfe-commits
>
wrote:

On Tue, Aug 22, 2017 at 6:37 PM, Chandler Carruth via
Phabricator > wrote:

chandlerc added a comment.

I'm really not a fan of the degree of complexity and
subtlety that this introduces into the frontend, all to
allow particular backend optimizations.

I feel like this is Clang working around a fundamental
deficiency in LLVM and we should instead find a way to fix
this in LLVM itself.

As has been pointed out before, user code can synthesize
large integers that small bit sequences are extracted
from, and Clang and LLVM should handle those just as well
as actual bitfields.

Can we see how far we can push the LLVM side before we add
complexity to Clang here? I understand that there remain
challenges to LLVM's stuff, but I don't think those
challenges make *all* of the LLVM improvements off the
table, I don't think we've exhausted all ways of improving
the LLVM changes being proposed, and I think we should
still land all of those and re-evaluate how important
these issues are when all of that is in place.


The main challenge of doing  this in LLVM is that
inter-procedural analysis (and possibly cross module) is
needed (for store forwarding issues).

Wei, perhaps you can provide concrete test case to illustrate
the issue so that reviewers have a good understanding.


It doesn't seem like all options for addressing that have been
exhausted. And even then, I feel like trying to fix this with
non-obvious (to the programmer) frontend heuristics isn't a good
solution. I actually *prefer* the source work around of "don't use
a bitfield if you *must* have narrow width access across modules
where the optimizer cannot see enough to narrow them and you
happen to know that there is a legal narrow access that works".
Because that way the programmer has *control* over this rather
than being at the whim of whichever side of the heuristic they end
up on.



The source workaround solution *does not* scale. Most importantly, 
user may not even be aware of the problem (and performance loss) 
unless  compiling the code with another compiler and notice the 
performance difference.


I agree with this, but it's not clear that this has to scale in that 
sense. I don't like basing this on the bitfield widths because it makes 
users pick between expressing semantic information and expressing target 
tuning information using the same construct. What if the optimal answer 
here is different on different platforms? I don't want to encourage 
users to ifdef their aggregates to sometimes be bitfields and sometimes 
not for tuning reasons. If need be, please add an attribute. Any 
heuristic that you pick here is going to help some cases and hurt 
others. If we're at the level of needing IPA to look at store-to-load 
forwarding effects, then we've really already lost. Either you need to 
actually do the IPA, or even in the backend, any heuristic that you 
choose will help some things and hurt others. Hopefully, we're not 
really there yet. I'm looking forward to seeing more examples of the 
kinds of problems you're trying to solve.


Thanks again,
Hal



David


David



Repository:
  rL LLVM

https://reviews.llvm.org/D36562




___
cfe-commits mailing list
cfe-commits@lists.llvm.org 
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits



___
llvm-commits mailing list
llvm-comm...@lists.llvm.org 
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits





___
llvm-commits mailing list
llvm-comm...@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits


--
Hal Finkel
Lead, Compiler Technology and Programming Languages
Leadership Computing Facility
Argonne National Laboratory

___
cfe-commits mailing list
cfe-commits@lists.llvm.org

Re: [PATCH] D36562: [Bitfield] Make the bitfield a separate location if it has width of legal integer type and its bit offset is naturally aligned for the type

[Xinliang David Li via cfe-commits]

On Tue, Aug 22, 2017 at 7:10 PM, Chandler Carruth via llvm-commits <
llvm-comm...@lists.llvm.org> wrote:

> On Tue, Aug 22, 2017 at 7:03 PM Xinliang David Li via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> On Tue, Aug 22, 2017 at 6:37 PM, Chandler Carruth via Phabricator <
>> revi...@reviews.llvm.org> wrote:
>>
>>> chandlerc added a comment.
>>>
>>> I'm really not a fan of the degree of complexity and subtlety that this
>>> introduces into the frontend, all to allow particular backend optimizations.
>>>
>>> I feel like this is Clang working around a fundamental deficiency in
>>> LLVM and we should instead find a way to fix this in LLVM itself.
>>>
>>> As has been pointed out before, user code can synthesize large integers
>>> that small bit sequences are extracted from, and Clang and LLVM should
>>> handle those just as well as actual bitfields.
>>>
>>> Can we see how far we can push the LLVM side before we add complexity to
>>> Clang here? I understand that there remain challenges to LLVM's stuff, but
>>> I don't think those challenges make *all* of the LLVM improvements off the
>>> table, I don't think we've exhausted all ways of improving the LLVM changes
>>> being proposed, and I think we should still land all of those and
>>> re-evaluate how important these issues are when all of that is in place.
>>>
>>
>> The main challenge of doing  this in LLVM is that inter-procedural
>> analysis (and possibly cross module) is needed (for store forwarding
>> issues).
>>
>> Wei, perhaps you can provide concrete test case to illustrate the issue
>> so that reviewers have a good understanding.
>>
>
> It doesn't seem like all options for addressing that have been exhausted.
> And even then, I feel like trying to fix this with non-obvious (to the
> programmer) frontend heuristics isn't a good solution. I actually *prefer*
> the source work around of "don't use a bitfield if you *must* have narrow
> width access across modules where the optimizer cannot see enough to narrow
> them and you happen to know that there is a legal narrow access that
> works". Because that way the programmer has *control* over this rather than
> being at the whim of whichever side of the heuristic they end up on.
>


The source workaround solution *does not* scale. Most importantly, user may
not even be aware of the problem (and performance loss) unless  compiling
the code with another compiler and notice the performance difference.

David

>
>
>>
>> David
>>
>>>
>>>
>>> Repository:
>>>   rL LLVM
>>>
>>> https://reviews.llvm.org/D36562
>>>
>>>
>>>
>>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>
> ___
> llvm-commits mailing list
> llvm-comm...@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D36562: [Bitfield] Make the bitfield a separate location if it has width of legal integer type and its bit offset is naturally aligned for the type

[Chandler Carruth via cfe-commits]

On Tue, Aug 22, 2017 at 7:03 PM Xinliang David Li via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> On Tue, Aug 22, 2017 at 6:37 PM, Chandler Carruth via Phabricator <
> revi...@reviews.llvm.org> wrote:
>
>> chandlerc added a comment.
>>
>> I'm really not a fan of the degree of complexity and subtlety that this
>> introduces into the frontend, all to allow particular backend optimizations.
>>
>> I feel like this is Clang working around a fundamental deficiency in LLVM
>> and we should instead find a way to fix this in LLVM itself.
>>
>> As has been pointed out before, user code can synthesize large integers
>> that small bit sequences are extracted from, and Clang and LLVM should
>> handle those just as well as actual bitfields.
>>
>> Can we see how far we can push the LLVM side before we add complexity to
>> Clang here? I understand that there remain challenges to LLVM's stuff, but
>> I don't think those challenges make *all* of the LLVM improvements off the
>> table, I don't think we've exhausted all ways of improving the LLVM changes
>> being proposed, and I think we should still land all of those and
>> re-evaluate how important these issues are when all of that is in place.
>>
>
> The main challenge of doing  this in LLVM is that inter-procedural
> analysis (and possibly cross module) is needed (for store forwarding
> issues).
>
> Wei, perhaps you can provide concrete test case to illustrate the issue so
> that reviewers have a good understanding.
>

It doesn't seem like all options for addressing that have been exhausted.
And even then, I feel like trying to fix this with non-obvious (to the
programmer) frontend heuristics isn't a good solution. I actually *prefer*
the source work around of "don't use a bitfield if you *must* have narrow
width access across modules where the optimizer cannot see enough to narrow
them and you happen to know that there is a legal narrow access that
works". Because that way the programmer has *control* over this rather than
being at the whim of whichever side of the heuristic they end up on.


>
> David
>
>>
>>
>> Repository:
>>   rL LLVM
>>
>> https://reviews.llvm.org/D36562
>>
>>
>>
>> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D36562: [Bitfield] Make the bitfield a separate location if it has width of legal integer type and its bit offset is naturally aligned for the type

[Xinliang David Li via cfe-commits]

On Tue, Aug 22, 2017 at 6:37 PM, Chandler Carruth via Phabricator <
revi...@reviews.llvm.org> wrote:

> chandlerc added a comment.
>
> I'm really not a fan of the degree of complexity and subtlety that this
> introduces into the frontend, all to allow particular backend optimizations.
>
> I feel like this is Clang working around a fundamental deficiency in LLVM
> and we should instead find a way to fix this in LLVM itself.
>
> As has been pointed out before, user code can synthesize large integers
> that small bit sequences are extracted from, and Clang and LLVM should
> handle those just as well as actual bitfields.
>
> Can we see how far we can push the LLVM side before we add complexity to
> Clang here? I understand that there remain challenges to LLVM's stuff, but
> I don't think those challenges make *all* of the LLVM improvements off the
> table, I don't think we've exhausted all ways of improving the LLVM changes
> being proposed, and I think we should still land all of those and
> re-evaluate how important these issues are when all of that is in place.
>

The main challenge of doing  this in LLVM is that inter-procedural analysis
(and possibly cross module) is needed (for store forwarding issues).

Wei, perhaps you can provide concrete test case to illustrate the issue so
that reviewers have a good understanding.

David

>
>
> Repository:
>   rL LLVM
>
> https://reviews.llvm.org/D36562
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [libcxx] r307966 - [libc++] Mark string operator+ _LIBCPP_FUNC_VIS

[Akira Hatanaka via cfe-commits]

Ah I see. I think r309474 should fix our problem.

Thanks for your quick response.

> On Aug 22, 2017, at 6:28 PM, Shoaib Meenai  wrote:
> 
> The point of the extern template was to prevent that operator from being 
> defined in any other object files. The extern template wasn't taking effect 
> because of its placement in the file, but I fixed that in r309474. Does your 
> issue get resolved with that second change in place?
>  
> From:  on behalf of Akira Hatanaka 
> Date: Tuesday, August 22, 2017 at 6:21 PM
> To: Shoaib Meenai 
> Cc: "cfe-commits@lists.llvm.org" 
> Subject: Re: [libcxx] r307966 - [libc++] Mark string operator+ 
> _LIBCPP_FUNC_VIS
>  
> This change makes it impossible to change the visibility of operator+ with 
> -fvisibility=hidden, which is not desirable on Darwin.
>  
> For example:
>  
> $ cat test.cpp
> #include 
>  
> using namespace std;
>  
> string foo1(string s) {
>   return "abc" + s;
> }
> 
> 
> $ clang++ test.cpp -fvisibility=hidden -c; nm -m -a test.o | grep 
> __ZNSt3__1plIcNS_11char_traitsIcEENS_9allocatorIcNS_12basic_stringIT_T0_T1_EEPKS6_RKS9_
> 0030 (__TEXT,__text) weak external 
> __ZNSt3__1plIcNS_11char_traitsIcEENS_9allocatorIcNS_12basic_stringIT_T0_T1_EEPKS6_RKS9_
>  
> The symbol used to be “weak private external”, but it is “weak external” now.
>  
> On Jul 13, 2017, at 2:35 PM, Shoaib Meenai via cfe-commits 
> > wrote:
>  
> Author: smeenai
> Date: Thu Jul 13 14:35:52 2017
> New Revision: 307966
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=307966=rev 
> 
> Log:
> [libc++] Mark string operator+ _LIBCPP_FUNC_VIS
> 
> It has an extern template instantiation declaration in the headers and a
> corresponding instantiation definition in the library, so we must mark
> it with _LIBCPP_FUNC_VIS to make it available outside the library.
> 
> This doesn't cause any ABI changes as-is since we don't build libc++
> with hidden visibility (so the function is exported anyway). It's needed
> for building libc++ with hidden visibility, however.
> 
> Clarify the Windows behavior for extern function templates while I'm
> here, since this exercises that behavior.
> 
> Modified:
>libcxx/trunk/docs/DesignDocs/VisibilityMacros.rst
>libcxx/trunk/include/string
> 
> Modified: libcxx/trunk/docs/DesignDocs/VisibilityMacros.rst
> URL: 
> http://llvm.org/viewvc/llvm-project/libcxx/trunk/docs/DesignDocs/VisibilityMacros.rst?rev=307966=307965=307966=diff
>  
> 
> ==
> --- libcxx/trunk/docs/DesignDocs/VisibilityMacros.rst (original)
> +++ libcxx/trunk/docs/DesignDocs/VisibilityMacros.rst Thu Jul 13 14:35:52 2017
> @@ -98,7 +98,8 @@ Visibility Macros
>   explicit instantiations themselves are marked as exported. Note that this
>   applies *only* to extern *class* templates. Extern *function* templates obey
>   regular import/export semantics, and applying `dllexport` directly to the
> -  extern template declaration is the correct thing to do for them.
> +  extern template declaration (i.e. using `_LIBCPP_FUNC_VIS`) is the correct
> +  thing to do for them.
> 
> **_LIBCPP_CLASS_TEMPLATE_INSTANTIATION_VIS**
>   Mark the member functions, typeinfo, and vtable of an explicit instantiation
> 
> Modified: libcxx/trunk/include/string
> URL: 
> http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/string?rev=307966=307965=307966=diff
>  
> 
> ==
> --- libcxx/trunk/include/string (original)
> +++ libcxx/trunk/include/string Thu Jul 13 14:35:52 2017
> @@ -4006,7 +4006,7 @@ basic_string<_CharT, _Traits, _Allocator
> 
> _LIBCPP_EXTERN_TEMPLATE(class _LIBCPP_EXTERN_TEMPLATE_TYPE_VIS 
> basic_string)
> _LIBCPP_EXTERN_TEMPLATE(class _LIBCPP_EXTERN_TEMPLATE_TYPE_VIS 
> basic_string)
> -_LIBCPP_EXTERN_TEMPLATE(string operator+ allocator >(char const*, string 

[PATCH] D36562: [Bitfield] Make the bitfield a separate location if it has width of legal integer type and its bit offset is naturally aligned for the type

[Chandler Carruth via Phabricator via cfe-commits]

chandlerc added a comment.

I'm really not a fan of the degree of complexity and subtlety that this 
introduces into the frontend, all to allow particular backend optimizations.

I feel like this is Clang working around a fundamental deficiency in LLVM and 
we should instead find a way to fix this in LLVM itself.

As has been pointed out before, user code can synthesize large integers that 
small bit sequences are extracted from, and Clang and LLVM should handle those 
just as well as actual bitfields.

Can we see how far we can push the LLVM side before we add complexity to Clang 
here? I understand that there remain challenges to LLVM's stuff, but I don't 
think those challenges make *all* of the LLVM improvements off the table, I 
don't think we've exhausted all ways of improving the LLVM changes being 
proposed, and I think we should still land all of those and re-evaluate how 
important these issues are when all of that is in place.


Repository:
  rL LLVM

https://reviews.llvm.org/D36562



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36562: [Bitfield] Make the bitfield a separate location if it has width of legal integer type and its bit offset is naturally aligned for the type

[Wei Mi via Phabricator via cfe-commits]

wmi updated this revision to Diff 112271.
wmi added a comment.

Try another idea suggested by David.

All the bitfields in a single run are still wrapped inside of a large integer 
according to CGBitFieldInfo. For the bitfields with legal integer types and 
aligned, change their access manner when we generate load/store in llvm IR for 
bitfield (In EmitLoadOfBitfieldLValue and EmitStoreThroughBitfieldLValue). All 
the other bitfields will still be accessed using widen load/store plus masking 
operations. Here is an example:

class A {

  unsigned long f1:2;
  unsigned long f2:6;
  unsigned long f3:8;
  unsigned long f4:4;

};
A a;

f1, f2, f3 and f4 will still be wrapped as a large integer. f1, f2, f4 will 
have the same access code as before. f3 will be accessed as if it is a separate 
unsigned char variable.

In this way, we can reduce the chance of blocking bitfield access combining. 
a.f1 access and a.f4 access can be combined if only no a.f3 access stands in 
between a.f1 and a.f4.  We will generate two less instructions for foo, and one 
more instruction for goo. So it is better, but not perfect.

void foo (unsigned long n1, unsigned long n2, unsigned long n3) {

  a.f1 = n1;
  a.f4 = n4;
  a.f3 = n3;

}

void goo (unsigned long n1, unsigned long n2, unsigned long n3) {

  a.f1 = n1;
  a.f3 = n3;// a.f3 will still block the combining of a.f1 and a.f2 because 
a.f3 is accessed independently.
  a.f4 = n4;

}


Repository:
  rL LLVM

https://reviews.llvm.org/D36562

Files:
  lib/CodeGen/CGExpr.cpp
  test/CodeGen/2009-12-07-BitFieldAlignment.c

Index: test/CodeGen/2009-12-07-BitFieldAlignment.c
===
--- test/CodeGen/2009-12-07-BitFieldAlignment.c
+++ test/CodeGen/2009-12-07-BitFieldAlignment.c
@@ -4,7 +4,7 @@
 struct S {
   int a, b;
   void *c;
-  unsigned d : 8;
+  unsigned d : 7;
   unsigned e : 8;
 };
 
Index: lib/CodeGen/CGExpr.cpp
===
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -1653,30 +1653,72 @@
   return EmitLoadOfBitfieldLValue(LV, Loc);
 }
 
+/// Check if current Field is better to be accessed separately. When current
+/// field has legal integer width, and its bitfield offset is naturally
+/// aligned, it is better to access the bitfield like a separate integer var.
+static bool IsSeparatableBitfield(const CGBitFieldInfo ,
+  const llvm::DataLayout ,
+  ASTContext ) {
+  if (!DL.isLegalInteger(Info.Size))
+return false;
+  // Make sure Field is natually aligned.
+  if (Info.Offset %
+  (DL.getABIIntegerTypeAlignment(Info.Size) * Context.getCharWidth()) !=
+  0)
+return false;
+  return true;
+}
+
 RValue CodeGenFunction::EmitLoadOfBitfieldLValue(LValue LV,
  SourceLocation Loc) {
   const CGBitFieldInfo  = LV.getBitFieldInfo();
 
   // Get the output type.
   llvm::Type *ResLTy = ConvertType(LV.getType());
 
   Address Ptr = LV.getBitFieldAddress();
-  llvm::Value *Val = Builder.CreateLoad(Ptr, LV.isVolatileQualified(), "bf.load");
-
-  if (Info.IsSigned) {
-assert(static_cast(Info.Offset + Info.Size) <= Info.StorageSize);
-unsigned HighBits = Info.StorageSize - Info.Offset - Info.Size;
-if (HighBits)
-  Val = Builder.CreateShl(Val, HighBits, "bf.shl");
-if (Info.Offset + HighBits)
-  Val = Builder.CreateAShr(Val, Info.Offset + HighBits, "bf.ashr");
+  llvm::Value *Val = nullptr;
+  if (IsSeparatableBitfield(Info, CGM.getDataLayout(), getContext())) {
+// Ptr is the pointer to the Bitfield Storage. Add Offset to the Ptr
+// if the Offset is not zero.
+if (Info.Offset != 0) {
+  Address BC = Builder.CreateBitCast(Ptr, Int8PtrTy);
+  llvm::Value *GEP = Builder.CreateGEP(
+  BC.getPointer(),
+  llvm::ConstantInt::get(Int32Ty,
+ Info.Offset / getContext().getCharWidth()));
+  Ptr = Address(GEP, Ptr.getAlignment());
+}
+// Adjust the element type of Ptr if it has different size with Info.Size.
+if (Ptr.getElementType()->getScalarSizeInBits() != Info.Size) {
+  llvm::Type *BFType = llvm::Type::getIntNTy(getLLVMContext(), Info.Size);
+  llvm::Type *BFTypePtr =
+  llvm::Type::getIntNPtrTy(getLLVMContext(), Info.Size);
+  unsigned Align = CGM.getDataLayout().getABITypeAlignment(BFType) *
+   getContext().getCharWidth();
+  Ptr = Builder.CreateBitCast(
+  Address(Ptr.getPointer(), getContext().toCharUnitsFromBits(Align)),
+  BFTypePtr);
+}
+Val = Builder.CreateLoad(Ptr, LV.isVolatileQualified(), "bf.load");
   } else {
-if (Info.Offset)
-  Val = Builder.CreateLShr(Val, Info.Offset, "bf.lshr");
-if (static_cast(Info.Offset) + Info.Size < Info.StorageSize)
-  Val = Builder.CreateAnd(Val, llvm::APInt::getLowBitsSet(Info.StorageSize,
-   

Re: [libcxx] r307966 - [libc++] Mark string operator+ _LIBCPP_FUNC_VIS

[Shoaib Meenai via cfe-commits]

The point of the extern template was to prevent that operator from being 
defined in any other object files. The extern template wasn't taking effect 
because of its placement in the file, but I fixed that in r309474. Does your 
issue get resolved with that second change in place?

From:  on behalf of Akira Hatanaka 
Date: Tuesday, August 22, 2017 at 6:21 PM
To: Shoaib Meenai 
Cc: "cfe-commits@lists.llvm.org" 
Subject: Re: [libcxx] r307966 - [libc++] Mark string operator+ _LIBCPP_FUNC_VIS

This change makes it impossible to change the visibility of operator+ with 
-fvisibility=hidden, which is not desirable on Darwin.

For example:

$ cat test.cpp
#include 

using namespace std;

string foo1(string s) {
  return "abc" + s;
}


$ clang++ test.cpp -fvisibility=hidden -c; nm -m -a test.o | grep 
__ZNSt3__1plIcNS_11char_traitsIcEENS_9allocatorIcNS_12basic_stringIT_T0_T1_EEPKS6_RKS9_
0030 (__TEXT,__text) weak external 
__ZNSt3__1plIcNS_11char_traitsIcEENS_9allocatorIcNS_12basic_stringIT_T0_T1_EEPKS6_RKS9_

The symbol used to be “weak private external”, but it is “weak external” now.

On Jul 13, 2017, at 2:35 PM, Shoaib Meenai via cfe-commits 
> wrote:

Author: smeenai
Date: Thu Jul 13 14:35:52 2017
New Revision: 307966

URL: 
http://llvm.org/viewvc/llvm-project?rev=307966=rev
Log:
[libc++] Mark string operator+ _LIBCPP_FUNC_VIS

It has an extern template instantiation declaration in the headers and a
corresponding instantiation definition in the library, so we must mark
it with _LIBCPP_FUNC_VIS to make it available outside the library.

This doesn't cause any ABI changes as-is since we don't build libc++
with hidden visibility (so the function is exported anyway). It's needed
for building libc++ with hidden visibility, however.

Clarify the Windows behavior for extern function templates while I'm
here, since this exercises that behavior.

Modified:
   libcxx/trunk/docs/DesignDocs/VisibilityMacros.rst
   libcxx/trunk/include/string

Modified: libcxx/trunk/docs/DesignDocs/VisibilityMacros.rst
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/docs/DesignDocs/VisibilityMacros.rst?rev=307966=307965=307966=diff
==
--- libcxx/trunk/docs/DesignDocs/VisibilityMacros.rst (original)
+++ libcxx/trunk/docs/DesignDocs/VisibilityMacros.rst Thu Jul 13 14:35:52 2017
@@ -98,7 +98,8 @@ Visibility Macros
  explicit instantiations themselves are marked as exported. Note that this
  applies *only* to extern *class* templates. Extern *function* templates obey
  regular import/export semantics, and applying `dllexport` directly to the
-  extern template declaration is the correct thing to do for them.
+  extern template declaration (i.e. using `_LIBCPP_FUNC_VIS`) is the correct
+  thing to do for them.

**_LIBCPP_CLASS_TEMPLATE_INSTANTIATION_VIS**
  Mark the member functions, typeinfo, and vtable of an explicit instantiation

Modified: libcxx/trunk/include/string
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/string?rev=307966=307965=307966=diff
==
--- libcxx/trunk/include/string (original)
+++ libcxx/trunk/include/string Thu Jul 13 14:35:52 2017
@@ -4006,7 +4006,7 @@ basic_string<_CharT, _Traits, _Allocator

_LIBCPP_EXTERN_TEMPLATE(class _LIBCPP_EXTERN_TEMPLATE_TYPE_VIS 
basic_string)
_LIBCPP_EXTERN_TEMPLATE(class _LIBCPP_EXTERN_TEMPLATE_TYPE_VIS 
basic_string)
-_LIBCPP_EXTERN_TEMPLATE(string operator+(char const*, string const&))
+_LIBCPP_EXTERN_TEMPLATE(_LIBCPP_FUNC_VIS string operator+(char const*, string const&))

#if _LIBCPP_STD_VER > 11
// Literal suffixes for basic_string [basic.string.literals]


___
cfe-commits mailing list
cfe-commits@lists.llvm.org

[PATCH] D36914: Implement CFG construction for __try / __except / __leave.

[Reid Kleckner via Phabricator via cfe-commits]

rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.

Looks good!




Comment at: test/Sema/warn-unreachable-ms.c:42
+  }
+}

rnk wrote:
> Can we add a test to exercise that this builds the right CFG?
> ```
> __try {
>   __try {
> f();
>   } __except(1) {
> __leave; // should exit outer try
>   }
>   __leave;
>   f(); // expected-warning{{never be executed}}
> } __except(1) {
> }
> ```
> Sure. Did you intentionally put two __leaves in there, or do you only want 
> the one in the inner __except?

I think both are required to trigger the warning in case f() doesn't throw, but 
I could be wrong.


https://reviews.llvm.org/D36914



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36915: [Sema] Diagnose local variables and parameters captured by lambda and block expressions in a default argument

[Faisal Vali via Phabricator via cfe-commits]

faisalv added a comment.

I don't think this approach is entirely correct for at least the following 
reasons:

1. in the lambda case the machinery that diagnoses capture failures should be 
the machinery erroring on the lambda (when the parameter is odr-used)
2. in the unevaluated case, once you disable the error, the instantiation 
machinery will fail to find the corresponding instantiated parmvardecl.

I think - in addition to allowing unevaluated uses of parmvardecls by tweaking 
the DefaultArgumentChecker, you would also need to add the instantiated 
mappings of the parameters, prior to instantiating the default argument (to 
avoid the assertion) and perhaps need to tweak DoMarkVarDeclReferenced and/or 
tryCaptureVariable to make sure such cases for lambdas produce errors (if they 
don't, w the above fix).

Thanks for working on this!


https://reviews.llvm.org/D36915



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [libcxx] r307966 - [libc++] Mark string operator+ _LIBCPP_FUNC_VIS

[Akira Hatanaka via cfe-commits]

This change makes it impossible to change the visibility of operator+ with 
-fvisibility=hidden, which is not desirable on Darwin.

For example:

$ cat test.cpp
#include 

using namespace std;

string foo1(string s) {
  return "abc" + s;
}

$ clang++ test.cpp -fvisibility=hidden -c; nm -m -a test.o | grep 
__ZNSt3__1plIcNS_11char_traitsIcEENS_9allocatorIcNS_12basic_stringIT_T0_T1_EEPKS6_RKS9_
0030 (__TEXT,__text) weak external 
__ZNSt3__1plIcNS_11char_traitsIcEENS_9allocatorIcNS_12basic_stringIT_T0_T1_EEPKS6_RKS9_

The symbol used to be “weak private external”, but it is “weak external” now.

> On Jul 13, 2017, at 2:35 PM, Shoaib Meenai via cfe-commits 
>  wrote:
> 
> Author: smeenai
> Date: Thu Jul 13 14:35:52 2017
> New Revision: 307966
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=307966=rev
> Log:
> [libc++] Mark string operator+ _LIBCPP_FUNC_VIS
> 
> It has an extern template instantiation declaration in the headers and a
> corresponding instantiation definition in the library, so we must mark
> it with _LIBCPP_FUNC_VIS to make it available outside the library.
> 
> This doesn't cause any ABI changes as-is since we don't build libc++
> with hidden visibility (so the function is exported anyway). It's needed
> for building libc++ with hidden visibility, however.
> 
> Clarify the Windows behavior for extern function templates while I'm
> here, since this exercises that behavior.
> 
> Modified:
>libcxx/trunk/docs/DesignDocs/VisibilityMacros.rst
>libcxx/trunk/include/string
> 
> Modified: libcxx/trunk/docs/DesignDocs/VisibilityMacros.rst
> URL: 
> http://llvm.org/viewvc/llvm-project/libcxx/trunk/docs/DesignDocs/VisibilityMacros.rst?rev=307966=307965=307966=diff
> ==
> --- libcxx/trunk/docs/DesignDocs/VisibilityMacros.rst (original)
> +++ libcxx/trunk/docs/DesignDocs/VisibilityMacros.rst Thu Jul 13 14:35:52 2017
> @@ -98,7 +98,8 @@ Visibility Macros
>   explicit instantiations themselves are marked as exported. Note that this
>   applies *only* to extern *class* templates. Extern *function* templates obey
>   regular import/export semantics, and applying `dllexport` directly to the
> -  extern template declaration is the correct thing to do for them.
> +  extern template declaration (i.e. using `_LIBCPP_FUNC_VIS`) is the correct
> +  thing to do for them.
> 
> **_LIBCPP_CLASS_TEMPLATE_INSTANTIATION_VIS**
>   Mark the member functions, typeinfo, and vtable of an explicit instantiation
> 
> Modified: libcxx/trunk/include/string
> URL: 
> http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/string?rev=307966=307965=307966=diff
> ==
> --- libcxx/trunk/include/string (original)
> +++ libcxx/trunk/include/string Thu Jul 13 14:35:52 2017
> @@ -4006,7 +4006,7 @@ basic_string<_CharT, _Traits, _Allocator
> 
> _LIBCPP_EXTERN_TEMPLATE(class _LIBCPP_EXTERN_TEMPLATE_TYPE_VIS 
> basic_string)
> _LIBCPP_EXTERN_TEMPLATE(class _LIBCPP_EXTERN_TEMPLATE_TYPE_VIS 
> basic_string)
> -_LIBCPP_EXTERN_TEMPLATE(string operator+ allocator >(char const*, string const&))
> +_LIBCPP_EXTERN_TEMPLATE(_LIBCPP_FUNC_VIS string operator+ char_traits, allocator >(char const*, string const&))
> 
> #if _LIBCPP_STD_VER > 11 
> // Literal suffixes for basic_string [basic.string.literals]
> 
> 
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.

[Eric Fiselier via Phabricator via cfe-commits]

EricWF marked an inline comment as done.
EricWF added a comment.

In https://reviews.llvm.org/D37035#849574, @rsmith wrote:

> I don't like the model of conditionally rebuilding the default initializer / 
> default argument if it contains one of these builtins; it seems more 
> heavy-handed than necessary. I'm also not convinced that we need to store the 
> computed value in the AST.
>
> As an alternative, `CodeGen` and the expression evaluator can track a current 
> call context (the innermost call expression or list initialization that is 
> not within a default argument / default member initializer) consisting of the 
> enclosing `FunctionDecl` and `SourceLocation`, and when the value of one of 
> these builtins is needed, pass that context to the AST node to ask what value 
> it has in that context. This will require a bit more work for the static 
> analyzer, but should be fairly simple elsewhere and will avoid the extra AST 
> nodes for the desugared form.


That sounds good to me. Could you suggest where said context should correctly 
live?




Comment at: docs/LanguageExtensions.rst:2118
+point is the location of the caller. When the builtins appear as part of a
+NSDMI the invocation point is the location of the constructor or
+aggregate initialization used to create the object. Otherwise the invocation

rsmith wrote:
> What is an NSDMI? I think you mean "default member initializer".
"Non-static default member initializer". I'll change it to your suggestion.



Comment at: include/clang/AST/Expr.h:3838
+  /// function.
+  const char *getBuiltinStr() const LLVM_READONLY;
+

majnemer wrote:
> StringRef?
Sure. Why not. I'm never sure how to handle string literals.


https://reviews.llvm.org/D37035



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r311516 - Update Clang fuzzers to use libFuzzer bundled with the toolchain.

[George Karpenkov via cfe-commits]

Author: george.karpenkov
Date: Tue Aug 22 17:42:22 2017
New Revision: 311516

URL: http://llvm.org/viewvc/llvm-project?rev=311516=rev
Log:
Update Clang fuzzers to use libFuzzer bundled with the toolchain.

Differential Revision: https://reviews.llvm.org/D37043

Modified:
cfe/trunk/tools/clang-format/fuzzer/CMakeLists.txt
cfe/trunk/tools/clang-fuzzer/CMakeLists.txt

Modified: cfe/trunk/tools/clang-format/fuzzer/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/clang-format/fuzzer/CMakeLists.txt?rev=311516=311515=311516=diff
==
--- cfe/trunk/tools/clang-format/fuzzer/CMakeLists.txt (original)
+++ cfe/trunk/tools/clang-format/fuzzer/CMakeLists.txt Tue Aug 22 17:42:22 2017
@@ -1,11 +1,11 @@
 set(LLVM_LINK_COMPONENTS support)
 
+set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=fuzzer")
+
 add_clang_executable(clang-format-fuzzer
   EXCLUDE_FROM_ALL
   ClangFormatFuzzer.cpp
   )
 
 target_link_libraries(clang-format-fuzzer
-  ${CLANG_FORMAT_LIB_DEPS}
-  LLVMFuzzer
-  )
+  ${CLANG_FORMAT_LIB_DEPS})

Modified: cfe/trunk/tools/clang-fuzzer/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/clang-fuzzer/CMakeLists.txt?rev=311516=311515=311516=diff
==
--- cfe/trunk/tools/clang-fuzzer/CMakeLists.txt (original)
+++ cfe/trunk/tools/clang-fuzzer/CMakeLists.txt Tue Aug 22 17:42:22 2017
@@ -1,5 +1,6 @@
 if( LLVM_USE_SANITIZE_COVERAGE )
   set(LLVM_LINK_COMPONENTS ${LLVM_TARGETS_TO_BUILD})
+  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=fuzzer")
 
   if(CLANG_ENABLE_PROTO_FUZZER)
 # Create protobuf .h and .cc files, and put them in a library for use by
@@ -38,7 +39,6 @@ if( LLVM_USE_SANITIZE_COVERAGE )
   clangCXXProto
   clangHandleCXX
   clangProtoToCXX
-  LLVMFuzzer
   )
   else()
 # Hack to bypass LLVM's cmake sources check and allow multiple libraries 
and
@@ -55,6 +55,5 @@ if( LLVM_USE_SANITIZE_COVERAGE )
 
   target_link_libraries(clang-fuzzer
 clangHandleCXX
-LLVMFuzzer
 )
 endif()


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37042: Teach clang to tolerate the 'p = nullptr + n' idiom used by glibc

[Andy Kaylor via Phabricator via cfe-commits]

andrew.w.kaylor added a comment.

I'm not sure why the svn attributes got attached to the file I added.  I'll 
remove them before committing.


https://reviews.llvm.org/D37042



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

I don't like the model of conditionally rebuilding the default initializer / 
default argument if it contains one of these builtins; it seems more 
heavy-handed than necessary. I'm also not convinced that we need to store the 
computed value in the AST.

As an alternative, `CodeGen` and the expression evaluator can track a current 
call context (the innermost call expression or list initialization that is not 
within a default argument / default member initializer) consisting of the 
enclosing `FunctionDecl` and `SourceLocation`, and when the value of one of 
these builtins is needed, pass that context to the AST node to ask what value 
it has in that context. This will require a bit more work for the static 
analyzer, but should be fairly simple elsewhere and will avoid the extra AST 
nodes for the desugared form.




Comment at: docs/LanguageExtensions.rst:2118
+point is the location of the caller. When the builtins appear as part of a
+NSDMI the invocation point is the location of the constructor or
+aggregate initialization used to create the object. Otherwise the invocation

What is an NSDMI? I think you mean "default member initializer".


https://reviews.llvm.org/D37035



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37042: Teach clang to tolerate the 'p = nullptr + n' idiom used by glibc

[Andy Kaylor via Phabricator via cfe-commits]

andrew.w.kaylor created this revision.

This patch adds a hack to clang's emitPointerArithmetic() function to get it to 
emit an inttoptr instruction rather than a GEP with a null base pointer when 
the 'p = nullptr + n' idiom is used to convert a pointer-sized integer to a 
pointer.  This idiom is used by some versions of glibc and gcc.

This was previously discussed here: 
http://lists.llvm.org/pipermail/llvm-dev/2017-July/115053.html


https://reviews.llvm.org/D37042

Files:
  lib/CodeGen/CGExprScalar.cpp
  test/CodeGen/nullptr-arithmetic.c


Index: lib/CodeGen/CGExprScalar.cpp
===
--- lib/CodeGen/CGExprScalar.cpp
+++ lib/CodeGen/CGExprScalar.cpp
@@ -2671,6 +2671,37 @@
   unsigned width = cast(index->getType())->getBitWidth();
   auto  = CGF.CGM.getDataLayout();
   auto PtrTy = cast(pointer->getType());
+
+  // Some versions of glibc and gcc use idioms (particularly in their malloc
+  // routines) that add a pointer-sized integer (known to be a pointer value)
+  // to a null pointer in order to cast the value back to an integer or as
+  // part of a pointer alignment algorithm.  This is undefined behavior, but
+  // we'd like to be able to compile programs that use it.
+  //
+  // Normally, we'd generate a GEP with a null-pointer base here in response
+  // to that code, but it's also UB to dereference a pointer created that
+  // way.  Instead (as an acknowledged hack to tolerate the idiom) we will
+  // generate a direct cast of the integer value to a pointer.
+  //
+  // The idiom (p = nullptr + N) is not met if any of the following are true:
+  //
+  //   The operation is subtraction.
+  //   The index is not pointer-sized.
+  //   The pointer type is not byte-sized.
+  //   The index operand is a constant.
+  //
+  if (isa(pointer) && !isSubtraction && 
+  (width == DL.getTypeSizeInBits(PtrTy)) && 
+  !isa(index)) {
+// The pointer type might come back as null, so it's deferred until here.
+const PointerType *pointerType 
+  = pointerOperand->getType()->getAs();
+if (pointerType && pointerType->getPointeeType()->isCharType()) { 
+  // (nullptr + N) -> inttoptr N to 
+  return CGF.Builder.CreateIntToPtr(index, pointer->getType());
+}
+  }
+
   if (width != DL.getTypeSizeInBits(PtrTy)) {
 // Zero-extend or sign-extend the pointer value according to
 // whether the index is signed or not.
Index: test/CodeGen/nullptr-arithmetic.c
===
--- test/CodeGen/nullptr-arithmetic.c
+++ test/CodeGen/nullptr-arithmetic.c
@@ -0,0 +1,51 @@
+// RUN: %clang_cc1 -S %s -emit-llvm -o - | FileCheck %s
+
+#include 
+
+// This test is meant to verify code that handles the 'p = nullptr + n' idiom
+// used by some versions of glibc and gcc.  This is undefined behavior but
+// it is intended there to act like a conversion from a pointer-sized integer
+// to a pointer, and we would like to tolerate that.
+
+#define NULLPTRI8 ((int8_t*)0)
+
+// This should get the inttoptr instruction.
+int8_t *test1(intptr_t n) {
+  return NULLPTRI8 + n;
+}
+// CHECK-LABEL: test1
+// CHECK: inttoptr
+// CHECK-NOT: getelementptr
+
+// This doesn't meet the idiom because the offset type isn't pointer-sized.
+int8_t *test2(int16_t n) {
+  return NULLPTRI8 + n;
+}
+// CHECK-LABEL: test2
+// CHECK: getelementptr
+// CHECK-NOT: inttoptr
+
+// This doesn't meet the idiom because the offset is constant.
+int8_t *test3() {
+  return NULLPTRI8 + 16;
+}
+// CHECK-LABEL: test3
+// CHECK: getelementptr
+// CHECK-NOT: inttoptr
+
+// This doesn't meet the idiom because the element type is larger than a byte.
+int16_t *test4(intptr_t n) {
+  return (int16_t*)0 + n;
+}
+// CHECK-LABEL: test4
+// CHECK: getelementptr
+// CHECK-NOT: inttoptr
+
+// This doesn't meet the idiom because the offset is subtracted.
+int8_t* test5(intptr_t n) {
+  return NULLPTRI8 - n;
+}
+// CHECK-LABEL: test5
+// CHECK: getelementptr
+// CHECK-NOT: inttoptr
+


Index: lib/CodeGen/CGExprScalar.cpp
===
--- lib/CodeGen/CGExprScalar.cpp
+++ lib/CodeGen/CGExprScalar.cpp
@@ -2671,6 +2671,37 @@
   unsigned width = cast(index->getType())->getBitWidth();
   auto  = CGF.CGM.getDataLayout();
   auto PtrTy = cast(pointer->getType());
+
+  // Some versions of glibc and gcc use idioms (particularly in their malloc
+  // routines) that add a pointer-sized integer (known to be a pointer value)
+  // to a null pointer in order to cast the value back to an integer or as
+  // part of a pointer alignment algorithm.  This is undefined behavior, but
+  // we'd like to be able to compile programs that use it.
+  //
+  // Normally, we'd generate a GEP with a null-pointer base here in response
+  // to that code, but it's also UB to dereference a pointer created that
+  // way.  Instead (as an acknowledged hack to tolerate the idiom) we will
+  // generate a direct cast of the 

[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.

[Eric Fiselier via Phabricator via cfe-commits]

EricWF updated this revision to Diff 112268.
EricWF marked 4 inline comments as done.
EricWF added a comment.

- Address inline comments.
- Fix calls to class call operators.


https://reviews.llvm.org/D37035

Files:
  docs/LanguageExtensions.rst
  include/clang/AST/Decl.h
  include/clang/AST/Expr.h
  include/clang/AST/RecursiveASTVisitor.h
  include/clang/AST/Stmt.h
  include/clang/Basic/StmtNodes.td
  include/clang/Basic/TokenKinds.def
  include/clang/Sema/Sema.h
  include/clang/Serialization/ASTBitCodes.h
  lib/AST/Decl.cpp
  lib/AST/Expr.cpp
  lib/AST/ExprClassification.cpp
  lib/AST/ExprConstant.cpp
  lib/AST/ItaniumMangle.cpp
  lib/AST/StmtPrinter.cpp
  lib/AST/StmtProfile.cpp
  lib/CodeGen/CGExprAgg.cpp
  lib/CodeGen/CGExprConstant.cpp
  lib/CodeGen/CGExprScalar.cpp
  lib/Parse/ParseExpr.cpp
  lib/Sema/Sema.cpp
  lib/Sema/SemaDeclCXX.cpp
  lib/Sema/SemaExceptionSpec.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaInit.cpp
  lib/Sema/SemaOverload.cpp
  lib/Sema/TreeTransform.h
  lib/Serialization/ASTReaderStmt.cpp
  lib/Serialization/ASTWriterStmt.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  test/CodeGenCXX/builtin_FUNCTION.cpp
  test/CodeGenCXX/builtin_LINE.cpp
  test/CodeGenCXX/debug-info-line.cpp
  test/Parser/builtin_source_location.c
  test/Sema/source_location.c
  test/SemaCXX/Inputs/source-location-file.h
  test/SemaCXX/source_location.cpp

Index: test/SemaCXX/source_location.cpp
===
--- /dev/null
+++ test/SemaCXX/source_location.cpp
@@ -0,0 +1,472 @@
+// RUN: %clang_cc1 -std=c++1z -fcxx-exceptions -fexceptions -verify %s
+// expected-no-diagnostics
+
+#define assert(...) ((__VA_ARGS__) ? ((void)0) : throw 42)
+#define CURRENT_FROM_MACRO() SL::current()
+#define FORWARD(...) __VA_ARGS__
+
+namespace std {
+namespace experimental {
+struct source_location {
+private:
+  unsigned int __m_line = 0;
+  unsigned int __m_col = 0;
+  const char *__m_file = nullptr;
+  const char *__m_func = nullptr;
+public:
+  static constexpr source_location current(
+  const char *__file = __builtin_FILE(),
+  const char *__func = __builtin_FUNCTION(),
+  unsigned int __line = __builtin_LINE(),
+  unsigned int __col = __builtin_COLUMN()) noexcept {
+source_location __loc;
+__loc.__m_line = __line;
+__loc.__m_col = __col;
+__loc.__m_file = __file;
+__loc.__m_func = __func;
+return __loc;
+  }
+  constexpr source_location() = default;
+  constexpr source_location(source_location const &) = default;
+  constexpr unsigned int line() const noexcept { return __m_line; }
+  constexpr unsigned int column() const noexcept { return __m_col; }
+  constexpr const char *file() const noexcept { return __m_file; }
+  constexpr const char *function() const noexcept { return __m_func; }
+};
+} // namespace experimental
+} // namespace std
+
+using SL = std::experimental::source_location;
+
+#include "Inputs/source-location-file.h"
+namespace SLF = source_location_file;
+
+constexpr bool is_equal(const char *LHS, const char *RHS) {
+  while (*LHS != 0 && *RHS != 0) {
+if (*LHS != *RHS)
+  return false;
+++LHS;
+++RHS;
+  }
+  return *LHS == 0 && *RHS == 0;
+}
+
+template 
+constexpr T identity(T t) {
+  return t;
+}
+
+template 
+struct Pair {
+  T first;
+  U second;
+};
+
+template 
+constexpr bool is_same = false;
+template 
+constexpr bool is_same = true;
+
+// test types
+static_assert(is_same);
+static_assert(is_same);
+static_assert(is_same);
+static_assert(is_same);
+
+// test noexcept
+static_assert(noexcept(__builtin_LINE()));
+static_assert(noexcept(__builtin_COLUMN()));
+static_assert(noexcept(__builtin_FILE()));
+static_assert(noexcept(__builtin_FUNCTION()));
+
+//===--===//
+//__builtin_LINE()
+//===--===//
+
+namespace test_line {
+
+static_assert(SL::current().line() == __LINE__);
+static_assert(SL::current().line() == CURRENT_FROM_MACRO().line());
+
+static constexpr SL GlobalS = SL::current();
+
+static_assert(GlobalS.line() == __LINE__ - 2);
+
+// clang-format off
+constexpr bool test_line_fn() {
+  constexpr SL S = SL::current();
+  static_assert(S.line() == (__LINE__ - 1), "");
+  // The start of the call expression to `current()` begins at the token `SL`
+  constexpr int ExpectLine = __LINE__ + 3;
+  constexpr SL S2
+  =
+  SL // Call expression starts here
+  ::
+  current
+  (
+
+  )
+  ;
+  static_assert(S2.line() == ExpectLine, "");
+
+  static_assert(
+  FORWARD(
+ __builtin_LINE
+(
+)
+  )
+== __LINE__ - 1, "");
+  static_assert(\
+\
+  __builtin_LINE()\
+\
+  == __LINE__ - 2, "");
+  static_assert(\
+   

[PATCH] D37038: Replace temp MD nodes with unique/distinct before cloning

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

Performance-wise the change is fine because it does the same amount of work, 
but I would prefer someone to audit the code to make sure that we aren't 
uniquing a node while we still want to make changes to it. Unfortunately 
testcases for these issues will involve impossibly nested C++ so it may take a 
while to find a counterexample in the wild. That's why I'd rather read the code 
instead :-)


https://reviews.llvm.org/D37038



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r311514 - Remove LLVMFuzzer from list of bundled libraries for Fuchsia

[George Karpenkov via cfe-commits]

Author: george.karpenkov
Date: Tue Aug 22 17:09:33 2017
New Revision: 311514

URL: http://llvm.org/viewvc/llvm-project?rev=311514=rev
Log:
Remove LLVMFuzzer from list of bundled libraries for Fuchsia

As of now, libFuzzer is located in compiler-rt and is bundled with
Clang's toolchain by default.

Differential Revision: https://reviews.llvm.org/D37037

Modified:
cfe/trunk/cmake/caches/Fuchsia-stage2.cmake

Modified: cfe/trunk/cmake/caches/Fuchsia-stage2.cmake
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/cmake/caches/Fuchsia-stage2.cmake?rev=311514=311513=311514=diff
==
--- cfe/trunk/cmake/caches/Fuchsia-stage2.cmake (original)
+++ cfe/trunk/cmake/caches/Fuchsia-stage2.cmake Tue Aug 22 17:09:33 2017
@@ -87,7 +87,6 @@ set(LLVM_DISTRIBUTION_COMPONENTS
   lld
   lldb
   liblldb
-  LLVMFuzzer
   LTO
   clang-format
   clang-headers


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.



Comment at: test/SemaCXX/source_location.cpp:10-35
+struct source_location {
+private:
+  unsigned int __m_line = 0;
+  unsigned int __m_col = 0;
+  const char *__m_file = nullptr;
+  const char *__m_func = nullptr;
+public:

This seems suboptimal. It would seem better for the compiler to generate a 
global containing the relevant data and to represent a `source_location` as a 
pointer to such a value. We should also try to minimize the number of 
relocations necessary to build a `source_location` object, since such 
constructions are likely to be extremely common in some codebases. We should 
also keep in mind that we're likely to want to add fields to `source_location` 
in future, so designing it in a way that avoids an ABI break for such cases 
would be preferable.

How long has GCC supported this?


https://reviews.llvm.org/D37035



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36914: Implement CFG construction for __try / __except / __leave.

[Nico Weber via Phabricator via cfe-commits]

thakis updated this revision to Diff 112261.
thakis added a comment.

add test from rnk


https://reviews.llvm.org/D36914

Files:
  lib/Analysis/CFG.cpp
  test/Sema/warn-unreachable-ms.c

Index: test/Sema/warn-unreachable-ms.c
===
--- test/Sema/warn-unreachable-ms.c
+++ test/Sema/warn-unreachable-ms.c
@@ -0,0 +1,55 @@
+// RUN: %clang_cc1 %s -triple=i686-pc-win32 -fsyntax-only -verify -fms-extensions -Wunreachable-code
+
+void f();
+
+void g1() {
+  __try {
+f();
+__leave;
+f();  // expected-warning{{will never be executed}}
+  } __except(1) {
+f();
+  }
+
+  // Completely empty.
+  __try {
+  } __except(1) {
+  }
+
+  __try {
+f();
+return;
+  } __except(1) {  // Filter expression should not be marked as unreachable.
+// Empty __except body.
+  }
+}
+
+void g2() {
+  __try {
+// Nested __try.
+__try {
+  f();
+  __leave;
+  f(); // expected-warning{{will never be executed}}
+} __except(2) {
+}
+f();
+__leave;
+f(); // expected-warning{{will never be executed}}
+  } __except(1) {
+f();
+  }
+}
+
+void g3() {
+  __try {
+__try {
+  f();
+} __except (1) {
+  __leave; // should exit outer try
+}
+__leave;
+f(); // expected-warning{{never be executed}}
+  } __except (1) {
+  }
+}
Index: lib/Analysis/CFG.cpp
===
--- lib/Analysis/CFG.cpp
+++ lib/Analysis/CFG.cpp
@@ -395,12 +395,16 @@
   ASTContext *Context;
   std::unique_ptr cfg;
 
-  CFGBlock *Block;
-  CFGBlock *Succ;
+  CFGBlock *Block;  // Current block.
+  CFGBlock *Succ;  // Block after the current block.
   JumpTarget ContinueJumpTarget;
   JumpTarget BreakJumpTarget;
+  JumpTarget SEHLeaveJumpTarget;
   CFGBlock *SwitchTerminatedBlock;
   CFGBlock *DefaultCaseBlock;
+
+  // This can point either to a try or a __try block. The frontend forbids
+  // mixing both kinds in one function, so having one for both is enough.
   CFGBlock *TryTerminatedBlock;
 
   // Current position in local scope.
@@ -436,13 +440,12 @@
 
 public:
   explicit CFGBuilder(ASTContext *astContext,
-  const CFG::BuildOptions ) 
-: Context(astContext), cfg(new CFG()), // crew a new CFG
-  Block(nullptr), Succ(nullptr),
-  SwitchTerminatedBlock(nullptr), DefaultCaseBlock(nullptr),
-  TryTerminatedBlock(nullptr), badCFG(false), BuildOpts(buildOpts),
-  switchExclusivelyCovered(false), switchCond(nullptr),
-  cachedEntry(nullptr), lastLookup(nullptr) {}
+  const CFG::BuildOptions )
+  : Context(astContext), cfg(new CFG()), // crew a new CFG
+Block(nullptr), Succ(nullptr), SwitchTerminatedBlock(nullptr),
+DefaultCaseBlock(nullptr), TryTerminatedBlock(nullptr), badCFG(false),
+BuildOpts(buildOpts), switchExclusivelyCovered(false),
+switchCond(nullptr), cachedEntry(nullptr), lastLookup(nullptr) {}
 
   // buildCFG - Used by external clients to construct the CFG.
   std::unique_ptr buildCFG(const Decl *D, Stmt *Statement);
@@ -501,6 +504,10 @@
   CFGBlock *VisitObjCForCollectionStmt(ObjCForCollectionStmt *S);
   CFGBlock *VisitPseudoObjectExpr(PseudoObjectExpr *E);
   CFGBlock *VisitReturnStmt(ReturnStmt *R);
+  CFGBlock *VisitSEHExceptStmt(SEHExceptStmt *S);
+  CFGBlock *VisitSEHFinallyStmt(SEHFinallyStmt *S);
+  CFGBlock *VisitSEHLeaveStmt(SEHLeaveStmt *S);
+  CFGBlock *VisitSEHTryStmt(SEHTryStmt *S);
   CFGBlock *VisitStmtExpr(StmtExpr *S, AddStmtChoice asc);
   CFGBlock *VisitSwitchStmt(SwitchStmt *S);
   CFGBlock *VisitUnaryExprOrTypeTraitExpr(UnaryExprOrTypeTraitExpr *E,
@@ -1731,6 +1738,18 @@
 case Stmt::ReturnStmtClass:
   return VisitReturnStmt(cast(S));
 
+case Stmt::SEHExceptStmtClass:
+  return VisitSEHExceptStmt(cast(S));
+
+case Stmt::SEHFinallyStmtClass:
+  return VisitSEHFinallyStmt(cast(S));
+
+case Stmt::SEHLeaveStmtClass:
+  return VisitSEHLeaveStmt(cast(S));
+
+case Stmt::SEHTryStmtClass:
+  return VisitSEHTryStmt(cast(S));
+
 case Stmt::UnaryExprOrTypeTraitExprClass:
   return VisitUnaryExprOrTypeTraitExpr(cast(S),
asc);
@@ -2462,6 +2481,117 @@
   return VisitStmt(R, AddStmtChoice::AlwaysAdd);
 }
 
+CFGBlock *CFGBuilder::VisitSEHExceptStmt(SEHExceptStmt *ES) {
+  // SEHExceptStmt are treated like labels, so they are the first statement in a
+  // block.
+
+  // Save local scope position because in case of exception variable ScopePos
+  // won't be restored when traversing AST.
+  SaveAndRestore save_scope_pos(ScopePos);
+
+  addStmt(ES->getBlock());
+  CFGBlock *SEHExceptBlock = Block;
+  if (!SEHExceptBlock)
+SEHExceptBlock = createBlock();
+
+  appendStmt(SEHExceptBlock, ES);
+
+  // Also add the SEHExceptBlock as a label, like with regular labels.
+  SEHExceptBlock->setLabel(ES);
+
+  // Bail out if the CFG is bad.
+  if (badCFG)
+return nullptr;
+

[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.

[David Majnemer via Phabricator via cfe-commits]

majnemer added inline comments.



Comment at: include/clang/AST/Expr.h:3816
+/// __builtin_LINE(), __builtin_COLUMN(), __builtin_FUNCTION(), or
+/// __BUILTIN_FILE()
+class SourceLocExpr final : public Expr {

__BUILTIN_FILE -> __builtin_FILE



Comment at: include/clang/AST/Expr.h:3838
+  /// function.
+  const char *getBuiltinStr() const LLVM_READONLY;
+

StringRef?


https://reviews.llvm.org/D37035



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D36914: Implement CFG construction for __try / __except / __leave.

[Nico Weber via cfe-commits]

On Tue, Aug 22, 2017 at 5:56 PM, Reid Kleckner via Phabricator via
cfe-commits  wrote:

> rnk added a comment.
>
> Looks functionally correct
>
>
>
> 
> Comment at: test/Sema/warn-unreachable-ms.c:42
> +  }
> +}
> 
> Can we add a test to exercise that this builds the right CFG?
> ```
> __try {
>   __try {
> f();
>   } __except(1) {
> __leave; // should exit outer try
>   }
>   __leave;
>   f(); // expected-warning{{never be executed}}
> } __except(1) {
> }
> ```
>

Sure. Did you intentionally put two __leaves in there, or do you only want
the one in the inner __except?


>
>
> https://reviews.llvm.org/D36914
>
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37038: Replace temp MD nodes with unique/distinct before cloning

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

I mentioned this in the PR but I should have restated it here, sorry...
Wolfgang authored this patch.  He is away for a month so I volunteered to post 
this, in case it was okay for resolving the PR as the crash is present in the 
5.0 branch.
I do know Wolfgang was not really happy with it, and your questions might well 
be part of why that is.
The patch has been in our local version of 5.0 for a couple of weeks without 
obvious problems, but that is not proof that it is always okay.


https://reviews.llvm.org/D37038



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37038: Replace temp MD nodes with unique/distinct before cloning

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added inline comments.



Comment at: lib/CodeGen/CGVTables.cpp:157
+  if (DebugInfo)
+DebugInfo->replaceTemporaryNodes();
+

Have you looked at what it would take to only finalize the nodes reachable via 
the function?
Otherwise — have you audited that this is always safe?


https://reviews.llvm.org/D37038



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added inline comments.



Comment at: include/clang/AST/Expr.h:3814
 
+/// BuiltinSourceLocExpr - Represents a function call to one of
+/// __builtin_LINE(), __builtin_COLUMN(), __builtin_FUNCTION(), or

BuiltinSourceLocExpr -> SourceLocExpr



Comment at: lib/Sema/SemaExpr.cpp:12955
+  SourceMgr.getPresumedLoc(SourceMgr.getExpansionRange(CallerLoc).second);
+  assert(PLoc.isValid()); // FIXME: Learn how to handle this.
+

Do you have an example where you don't get back a valid location? Are you going 
to give back ("", 0, 0) for (file, line, column)? Probably doesn't matter for 
the function name.



Comment at: lib/Sema/SemaInit.cpp:563
+  if (SemaRef.CheckCXXDefaultInitExpr(Loc, Field)) {
+/* nothing todo */
+  } else if (SourceLocExpr::containsSourceLocExpr(

  // Nothing to do.


https://reviews.llvm.org/D37035



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37038: Replace temp MD nodes with unique/distinct before cloning

[Paul Robinson via Phabricator via cfe-commits]

probinson created this revision.

Make sure all temporary MD nodes have been replaced with uniqued or distinct 
nodes before we clone a function.

Fixes PR33930.


https://reviews.llvm.org/D37038

Files:
  lib/CodeGen/CGDebugInfo.cpp
  lib/CodeGen/CGDebugInfo.h
  lib/CodeGen/CGVTables.cpp
  test/CodeGenCXX/temp-md-nodes1.cpp
  test/CodeGenCXX/temp-md-nodes2.cpp

Index: test/CodeGenCXX/temp-md-nodes2.cpp
===
--- test/CodeGenCXX/temp-md-nodes2.cpp
+++ test/CodeGenCXX/temp-md-nodes2.cpp
@@ -0,0 +1,33 @@
+// REQUIRES: asserts
+// RUN: %clang_cc1 -O0 -triple %itanium_abi_triple -debug-info-kind=limited -S -emit-llvm %s -o - | \
+// RUN: FileCheck %s
+
+// This test simply checks that the varargs thunk is created. The failing test
+// case asserts.
+
+typedef signed char __int8_t;
+typedef int BOOL;
+class CMsgAgent;
+
+class CFs {
+public:
+  typedef enum {} CACHE_HINT;
+  virtual BOOL ReqCacheHint( CMsgAgent* p_ma, CACHE_HINT hint, ... ) ;
+};
+
+typedef struct {} _Lldiv_t;
+
+class CBdVfs {
+public:
+  virtual ~CBdVfs( ) {}
+};
+
+class CBdVfsImpl : public CBdVfs, public CFs {
+  BOOL ReqCacheHint( CMsgAgent* p_ma, CACHE_HINT hint, ... );
+};
+
+BOOL CBdVfsImpl::ReqCacheHint( CMsgAgent* p_ma, CACHE_HINT hint, ... ) {
+  return true;
+}
+
+// CHECK: define {{.*}} @_ZThn8_N10CBdVfsImpl12ReqCacheHintEP9CMsgAgentN3CFs10CACHE_HINTEz(
Index: test/CodeGenCXX/temp-md-nodes1.cpp
===
--- test/CodeGenCXX/temp-md-nodes1.cpp
+++ test/CodeGenCXX/temp-md-nodes1.cpp
@@ -0,0 +1,18 @@
+// REQUIRES: asserts
+// RUN: %clang_cc1 -O0 -triple %itanium_abi_triple -debug-info-kind=limited -S -emit-llvm %s -o - | \
+// RUN: FileCheck %s
+
+// This test simply checks that the varargs thunk is created. The failing test
+// case asserts.
+
+struct Alpha {
+  virtual void bravo(...);
+};
+struct Charlie {
+  virtual ~Charlie() {}
+};
+struct CharlieImpl : Charlie, Alpha {
+  void bravo(...) {}
+} delta;
+
+// CHECK: define {{.*}} void @_ZThn8_N11CharlieImpl5bravoEz(
Index: lib/CodeGen/CGVTables.cpp
===
--- lib/CodeGen/CGVTables.cpp
+++ lib/CodeGen/CGVTables.cpp
@@ -152,6 +152,10 @@
   llvm::Value *Callee = CGM.GetAddrOfFunction(GD, Ty, /*ForVTable=*/true);
   llvm::Function *BaseFn = cast(Callee);
 
+  // Ensure we don't have any temporary MD nodes before we clone the function.
+  if (DebugInfo)
+DebugInfo->replaceTemporaryNodes();
+
   // Clone to thunk.
   llvm::ValueToValueMapTy VMap;
   llvm::Function *NewFn = llvm::CloneFunction(BaseFn, VMap);
Index: lib/CodeGen/CGDebugInfo.h
===
--- lib/CodeGen/CGDebugInfo.h
+++ lib/CodeGen/CGDebugInfo.h
@@ -440,6 +440,7 @@
 
   void completeTemplateDefinition(const ClassTemplateSpecializationDecl );
   void completeUnusedClass(const CXXRecordDecl );
+  void replaceTemporaryNodes();
 
   /// Create debug info for a macro defined by a #define directive or a macro
   /// undefined by a #undef directive.
Index: lib/CodeGen/CGDebugInfo.cpp
===
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -4091,18 +4091,7 @@
   TheCU->setDWOId(Signature);
 }
 
-
-void CGDebugInfo::finalize() {
-  // Creating types might create further types - invalidating the current
-  // element and the size(), so don't cache/reference them.
-  for (size_t i = 0; i != ObjCInterfaceCache.size(); ++i) {
-ObjCInterfaceCacheEntry E = ObjCInterfaceCache[i];
-llvm::DIType *Ty = E.Type->getDecl()->getDefinition()
-   ? CreateTypeDefinition(E.Type, E.Unit)
-   : E.Decl;
-DBuilder.replaceTemporary(llvm::TempDIType(E.Decl), Ty);
-  }
-
+void CGDebugInfo::replaceTemporaryNodes() {
   for (auto p : ReplaceMap) {
 assert(p.second);
 auto *Ty = cast(p.second);
@@ -4115,6 +4104,21 @@
 DBuilder.replaceTemporary(llvm::TempDIType(Ty),
   cast(it->second));
   }
+  ReplaceMap.clear();
+}
+
+void CGDebugInfo::finalize() {
+  // Creating types might create further types - invalidating the current
+  // element and the size(), so don't cache/reference them.
+  for (size_t i = 0; i != ObjCInterfaceCache.size(); ++i) {
+ObjCInterfaceCacheEntry E = ObjCInterfaceCache[i];
+llvm::DIType *Ty = E.Type->getDecl()->getDefinition()
+   ? CreateTypeDefinition(E.Type, E.Unit)
+   : E.Decl;
+DBuilder.replaceTemporary(llvm::TempDIType(E.Decl), Ty);
+  }
+
+  replaceTemporaryNodes();
 
   for (const auto  : FwdDeclReplaceMap) {
 assert(p.second);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.

[Eric Fiselier via Phabricator via cfe-commits]

EricWF created this revision.

This patch implements the source location builtins `__builtin_LINE(), 
`__builtin_FUNCTION()`, `__builtin_FILE()` and `__builtin_COLUMN()`. These 
builtins are needed to implement 
[`std::experimental::source_location`](https://rawgit.com/cplusplus/fundamentals-ts/v2/main.html#reflection.src_loc.creation).

With the exception of `__builtin_COLUMN`, GCC also implements these builtins, 
and Clangs behavior is intended to match as closely as possible.


https://reviews.llvm.org/D37035

Files:
  docs/LanguageExtensions.rst
  include/clang/AST/Expr.h
  include/clang/AST/RecursiveASTVisitor.h
  include/clang/AST/Stmt.h
  include/clang/Basic/StmtNodes.td
  include/clang/Basic/TokenKinds.def
  include/clang/Sema/Sema.h
  include/clang/Serialization/ASTBitCodes.h
  lib/AST/Expr.cpp
  lib/AST/ExprClassification.cpp
  lib/AST/ExprConstant.cpp
  lib/AST/ItaniumMangle.cpp
  lib/AST/StmtPrinter.cpp
  lib/AST/StmtProfile.cpp
  lib/CodeGen/CGExprAgg.cpp
  lib/CodeGen/CGExprConstant.cpp
  lib/CodeGen/CGExprScalar.cpp
  lib/Parse/ParseExpr.cpp
  lib/Sema/Sema.cpp
  lib/Sema/SemaDeclCXX.cpp
  lib/Sema/SemaExceptionSpec.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaInit.cpp
  lib/Sema/TreeTransform.h
  lib/Serialization/ASTReaderStmt.cpp
  lib/Serialization/ASTWriterStmt.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  test/CodeGenCXX/builtin_FUNCTION.cpp
  test/CodeGenCXX/builtin_LINE.cpp
  test/CodeGenCXX/debug-info-line.cpp
  test/Parser/builtin_source_location.c
  test/Sema/source_location.c
  test/SemaCXX/Inputs/source-location-file.h
  test/SemaCXX/source_location.cpp

Index: test/SemaCXX/source_location.cpp
===
--- /dev/null
+++ test/SemaCXX/source_location.cpp
@@ -0,0 +1,425 @@
+// RUN: %clang_cc1 -std=c++1z -fcxx-exceptions -fexceptions -verify %s
+// expected-no-diagnostics
+
+#define assert(...) ((__VA_ARGS__) ? ((void)0) : throw 42)
+#define CURRENT_FROM_MACRO() SL::current()
+#define FORWARD(...) __VA_ARGS__
+
+namespace std {
+namespace experimental {
+struct source_location {
+private:
+  unsigned int __m_line = 0;
+  unsigned int __m_col = 0;
+  const char *__m_file = nullptr;
+  const char *__m_func = nullptr;
+public:
+  static constexpr source_location current(
+  const char *__file = __builtin_FILE(),
+  const char *__func = __builtin_FUNCTION(),
+  unsigned int __line = __builtin_LINE(),
+  unsigned int __col = __builtin_COLUMN()) noexcept {
+source_location __loc;
+__loc.__m_line = __line;
+__loc.__m_col = __col;
+__loc.__m_file = __file;
+__loc.__m_func = __func;
+return __loc;
+  }
+  constexpr source_location() = default;
+  constexpr source_location(source_location const &) = default;
+  constexpr unsigned int line() const noexcept { return __m_line; }
+  constexpr unsigned int column() const noexcept { return __m_col; }
+  constexpr const char *file() const noexcept { return __m_file; }
+  constexpr const char *function() const noexcept { return __m_func; }
+};
+} // namespace experimental
+} // namespace std
+
+using SL = std::experimental::source_location;
+
+#include "Inputs/source-location-file.h"
+namespace SLF = source_location_file;
+
+constexpr bool is_equal(const char *LHS, const char *RHS) {
+  while (*LHS != 0 && *RHS != 0) {
+if (*LHS != *RHS)
+  return false;
+++LHS;
+++RHS;
+  }
+  return *LHS == 0 && *RHS == 0;
+}
+
+template 
+constexpr T identity(T t) {
+  return t;
+}
+
+template 
+struct Pair {
+  T first;
+  U second;
+};
+
+template 
+constexpr bool is_same = false;
+template 
+constexpr bool is_same = true;
+
+// test types
+static_assert(is_same);
+static_assert(is_same);
+static_assert(is_same);
+static_assert(is_same);
+
+// test noexcept
+static_assert(noexcept(__builtin_LINE()));
+static_assert(noexcept(__builtin_COLUMN()));
+static_assert(noexcept(__builtin_FILE()));
+static_assert(noexcept(__builtin_FUNCTION()));
+
+//===--===//
+//__builtin_LINE()
+//===--===//
+
+namespace test_line {
+
+static_assert(SL::current().line() == __LINE__);
+static_assert(SL::current().line() == CURRENT_FROM_MACRO().line());
+
+static constexpr SL GlobalS = SL::current();
+
+static_assert(GlobalS.line() == __LINE__ - 2);
+
+// clang-format off
+constexpr bool test_line_fn() {
+  constexpr SL S = SL::current();
+  static_assert(S.line() == (__LINE__ - 1), "");
+  // The start of the call expression to `current()` begins at the token `SL`
+  constexpr int ExpectLine = __LINE__ + 3;
+  constexpr SL S2
+  =
+  SL // Call expression starts here
+  ::
+  current
+  (
+
+  )
+  ;
+  static_assert(S2.line() == ExpectLine, 

[PATCH] D36555: Move x86-specific sources to x86-specific source lists.

[Weiming Zhao via Phabricator via cfe-commits]

weimingz added inline comments.



Comment at: compiler-rt/lib/builtins/CMakeLists.txt:483
+set(powerpc64le_SOURCES ${powerpc64_SOURCES})
+
 set(wasm32_SOURCES ${GENERIC_SOURCES})

why these files were not used before? should the change be in a separate patch? 
or update the patch title/summary as well?


https://reviews.llvm.org/D36555



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36555: Move x86-specific sources to x86-specific source lists.

[Weiming Zhao via Phabricator via cfe-commits]

weimingz added reviewers: rengolin, compnerd.
weimingz added a comment.

LGTM. Adding Renato and Saleem to approve.


https://reviews.llvm.org/D36555



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36555: Move x86-specific sources to x86-specific source lists.

[Sterling Augustine via Phabricator via cfe-commits]

saugustine updated this revision to Diff 112244.
saugustine added a comment.

Remove two files inadvertantly included in last patch.


https://reviews.llvm.org/D36555

Files:
  compiler-rt/lib/builtins/CMakeLists.txt

Index: compiler-rt/lib/builtins/CMakeLists.txt
===
--- compiler-rt/lib/builtins/CMakeLists.txt
+++ compiler-rt/lib/builtins/CMakeLists.txt
@@ -51,7 +51,6 @@
   cmpti2.c
   comparedf2.c
   comparesf2.c
-  cpu_model.c
   ctzdi2.c
   ctzsi2.c
   ctzti2.c
@@ -66,7 +65,6 @@
   divtc3.c
   divti3.c
   divtf3.c
-  divxc3.c
   extendsfdf2.c
   extendhfsf2.c
   ffsdi2.c
@@ -84,27 +82,18 @@
   fixunssfdi.c
   fixunssfsi.c
   fixunssfti.c
-  fixunsxfdi.c
-  fixunsxfsi.c
-  fixunsxfti.c
-  fixxfdi.c
-  fixxfti.c
   floatdidf.c
   floatdisf.c
-  floatdixf.c
   floatsidf.c
   floatsisf.c
   floattidf.c
   floattisf.c
-  floattixf.c
   floatundidf.c
   floatundisf.c
-  floatundixf.c
   floatunsidf.c
   floatunsisf.c
   floatuntidf.c
   floatuntisf.c
-  floatuntixf.c
   int_util.c
   lshrdi3.c
   lshrti3.c
@@ -124,7 +113,6 @@
   mulvdi3.c
   mulvsi3.c
   mulvti3.c
-  mulxc3.c
   negdf2.c
   negdi2.c
   negsf2.c
@@ -142,7 +130,6 @@
   powidf2.c
   powisf2.c
   powitf2.c
-  powixf2.c
   subdf3.c
   subsf3.c
   subvdi3.c
@@ -226,6 +213,23 @@
 clear_cache.c)
 endif()
 
+# These sources work on all x86 variants, but only x86 variants.
+set(x86_ARCH_SOURCES
+  cpu_model.c
+  divxc3.c
+  fixxfdi.c
+  fixxfti.c
+  fixunsxfdi.c
+  fixunsxfsi.c
+  fixunsxfti.c
+  floatdixf.c
+  floattixf.c
+  floatundixf.c
+  floatuntixf.c
+  mulxc3.c
+  powixf2.c
+)
+
 if (NOT MSVC)
   set(x86_64_SOURCES
   x86_64/chkstk.S
@@ -288,6 +292,11 @@
   set(i686_SOURCES ${i386_SOURCES})
 endif () # if (NOT MSVC)
 
+set(x86_64h_SOURCES ${x86_64h_SOURCES} ${x86_ARCH_SOURCES})
+set(x86_64_SOURCES ${x86_64_SOURCES} ${x86_ARCH_SOURCES})
+set(i386_SOURCES ${i386_SOURCES} ${x86_ARCH_SOURCES})
+set(i686_SOURCES ${i686_SOURCES} ${x86_ARCH_SOURCES})
+
 set(arm_SOURCES
   arm/bswapdi2.S
   arm/bswapsi2.S
@@ -458,6 +467,20 @@
 set(mips64el_SOURCES ${GENERIC_TF_SOURCES}
  ${mips_SOURCES})
 
+set(powerpc64_SOURCES
+  ppc/divtc3.c
+  ppc/fixtfdi.c
+  ppc/fixunstfdi.c
+  ppc/floatditf.c
+  ppc/floatunditf.c
+  ppc/gcc_qadd.c
+  ppc/gcc_qdiv.c
+  ppc/gcc_qmul.c
+  ppc/gcc_qsub.c
+  ppc/multc3.c
+  ${GENERIC_SOURCES})
+set(powerpc64le_SOURCES ${powerpc64_SOURCES})
+
 set(wasm32_SOURCES ${GENERIC_SOURCES})
 set(wasm64_SOURCES ${GENERIC_SOURCES})
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36336: [X86] Add support for __builtin_cpu_init

[Craig Topper via Phabricator via cfe-commits]

craig.topper added a comment.

Ping


https://reviews.llvm.org/D36336



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r311397 - [Driver][Darwin] Do not pass -munwind-table if -fno-excpetions is

[Hans Wennborg via cfe-commits]

Okay, r311505.

On Tue, Aug 22, 2017 at 12:02 PM, Akira Hatanaka  wrote:
> Hans, can this be merged to 5.0 too? This is a follow-up to r310006.
>
> Thanks.
>
>> On Aug 21, 2017, at 3:46 PM, Akira Hatanaka via cfe-commits 
>>  wrote:
>>
>> Author: ahatanak
>> Date: Mon Aug 21 15:46:46 2017
>> New Revision: 311397
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=311397=rev
>> Log:
>> [Driver][Darwin] Do not pass -munwind-table if -fno-excpetions is
>> supplied.
>>
>> With this change, -fno-exceptions disables unwind tables unless
>> -funwind-tables is supplied too or the target is x86-64 (x86-64 requires
>> emitting unwind tables).
>>
>> rdar://problem/33934446
>>
>> Modified:
>>cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
>>cfe/trunk/test/Driver/clang-translation.c
>>
>> Modified: cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
>> URL: 
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Darwin.cpp?rev=311397=311396=311397=diff
>> ==
>> --- cfe/trunk/lib/Driver/ToolChains/Darwin.cpp (original)
>> +++ cfe/trunk/lib/Driver/ToolChains/Darwin.cpp Mon Aug 21 15:46:46 2017
>> @@ -1845,7 +1845,12 @@ Darwin::TranslateArgs(const DerivedArgLi
>> }
>>
>> bool MachO::IsUnwindTablesDefault(const ArgList ) const {
>> -  return !UseSjLjExceptions(Args);
>> +  // Unwind tables are not emitted if -fno-exceptions is supplied (except 
>> when
>> +  // targeting x86_64).
>> +  return getArch() == llvm::Triple::x86_64 ||
>> + (!UseSjLjExceptions(Args) &&
>> +  Args.hasFlag(options::OPT_fexceptions, 
>> options::OPT_fno_exceptions,
>> +   true));
>> }
>>
>> bool MachO::UseDwarfDebugFlags() const {
>>
>> Modified: cfe/trunk/test/Driver/clang-translation.c
>> URL: 
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/clang-translation.c?rev=311397=311396=311397=diff
>> ==
>> --- cfe/trunk/test/Driver/clang-translation.c (original)
>> +++ cfe/trunk/test/Driver/clang-translation.c Mon Aug 21 15:46:46 2017
>> @@ -73,6 +73,10 @@
>> // RUN: FileCheck -check-prefix=ARM64-APPLE %s
>> // ARM64-APPLE: -munwind-table
>>
>> +// RUN: %clang -target arm64-apple-ios10 -fno-exceptions -### -S %s -arch 
>> arm64 2>&1 | \
>> +// RUN: FileCheck -check-prefix=ARM64-APPLE-EXCEP %s
>> +// ARM64-APPLE-EXCEP-NOT: -munwind-table
>> +
>> // RUN: %clang -target armv7k-apple-watchos4.0 -### -S %s -arch armv7k 2>&1 
>> | \
>> // RUN: FileCheck -check-prefix=ARMV7K-APPLE %s
>> // ARMV7K-APPLE: -munwind-table
>>
>>
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36555: Move x86-specific sources to x86-specific source lists.

[Sterling Augustine via Phabricator via cfe-commits]

saugustine added a reviewer: weimingz.
saugustine added inline comments.



Comment at: compiler-rt/lib/builtins/CMakeLists.txt:223
+  cpu_model.c
+  divxc3.c
+  fixxfdi.c

mgorny wrote:
> This and the following files have only:
> 
> ```
> #if !_ARCH_PPC
> ```
> 
> so I suppose it's currently included on more targets than x86. If it's really 
> not useful there, I think it'd be better to update the `#if`s first (and 
> preferably get the author to review).
As discussed in D36764, these all assume 80-bit floats, and therefore are only 
useful on x86 (and ancient 68ks)--and, in fact, the tests for these functions 
are disabled for everything but x86. I have left these #ifs in for now, but 
will remove them after this patch gets submitted (and add comments about them 
needing 80-bit floats). Otherwise, powerpc builds will be broken.


https://reviews.llvm.org/D36555



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36555: Move x86-specific sources to x86-specific source lists.

[Sterling Augustine via Phabricator via cfe-commits]

saugustine updated this revision to Diff 112242.
saugustine added a comment.

Thanks for the various comments. Please take another look.


https://reviews.llvm.org/D36555

Files:
  compiler-rt/cmake/Modules/CompilerRTUtils.cmake
  compiler-rt/cmake/builtin-config-ix.cmake
  compiler-rt/lib/builtins/CMakeLists.txt

Index: compiler-rt/lib/builtins/CMakeLists.txt
===
--- compiler-rt/lib/builtins/CMakeLists.txt
+++ compiler-rt/lib/builtins/CMakeLists.txt
@@ -51,7 +51,6 @@
   cmpti2.c
   comparedf2.c
   comparesf2.c
-  cpu_model.c
   ctzdi2.c
   ctzsi2.c
   ctzti2.c
@@ -66,7 +65,6 @@
   divtc3.c
   divti3.c
   divtf3.c
-  divxc3.c
   extendsfdf2.c
   extendhfsf2.c
   ffsdi2.c
@@ -84,27 +82,18 @@
   fixunssfdi.c
   fixunssfsi.c
   fixunssfti.c
-  fixunsxfdi.c
-  fixunsxfsi.c
-  fixunsxfti.c
-  fixxfdi.c
-  fixxfti.c
   floatdidf.c
   floatdisf.c
-  floatdixf.c
   floatsidf.c
   floatsisf.c
   floattidf.c
   floattisf.c
-  floattixf.c
   floatundidf.c
   floatundisf.c
-  floatundixf.c
   floatunsidf.c
   floatunsisf.c
   floatuntidf.c
   floatuntisf.c
-  floatuntixf.c
   int_util.c
   lshrdi3.c
   lshrti3.c
@@ -124,7 +113,6 @@
   mulvdi3.c
   mulvsi3.c
   mulvti3.c
-  mulxc3.c
   negdf2.c
   negdi2.c
   negsf2.c
@@ -142,7 +130,6 @@
   powidf2.c
   powisf2.c
   powitf2.c
-  powixf2.c
   subdf3.c
   subsf3.c
   subvdi3.c
@@ -226,6 +213,23 @@
 clear_cache.c)
 endif()
 
+# These sources work on all x86 variants, but only x86 variants.
+set(x86_ARCH_SOURCES
+  cpu_model.c
+  divxc3.c
+  fixxfdi.c
+  fixxfti.c
+  fixunsxfdi.c
+  fixunsxfsi.c
+  fixunsxfti.c
+  floatdixf.c
+  floattixf.c
+  floatundixf.c
+  floatuntixf.c
+  mulxc3.c
+  powixf2.c
+)
+
 if (NOT MSVC)
   set(x86_64_SOURCES
   x86_64/chkstk.S
@@ -288,6 +292,11 @@
   set(i686_SOURCES ${i386_SOURCES})
 endif () # if (NOT MSVC)
 
+set(x86_64h_SOURCES ${x86_64h_SOURCES} ${x86_ARCH_SOURCES})
+set(x86_64_SOURCES ${x86_64_SOURCES} ${x86_ARCH_SOURCES})
+set(i386_SOURCES ${i386_SOURCES} ${x86_ARCH_SOURCES})
+set(i686_SOURCES ${i686_SOURCES} ${x86_ARCH_SOURCES})
+
 set(arm_SOURCES
   arm/bswapdi2.S
   arm/bswapsi2.S
@@ -458,6 +467,20 @@
 set(mips64el_SOURCES ${GENERIC_TF_SOURCES}
  ${mips_SOURCES})
 
+set(powerpc64_SOURCES
+  ppc/divtc3.c
+  ppc/fixtfdi.c
+  ppc/fixunstfdi.c
+  ppc/floatditf.c
+  ppc/floatunditf.c
+  ppc/gcc_qadd.c
+  ppc/gcc_qdiv.c
+  ppc/gcc_qmul.c
+  ppc/gcc_qsub.c
+  ppc/multc3.c
+  ${GENERIC_SOURCES})
+set(powerpc64le_SOURCES ${powerpc64_SOURCES})
+
 set(wasm32_SOURCES ${GENERIC_SOURCES})
 set(wasm64_SOURCES ${GENERIC_SOURCES})
 
Index: compiler-rt/cmake/builtin-config-ix.cmake
===
--- compiler-rt/cmake/builtin-config-ix.cmake
+++ compiler-rt/cmake/builtin-config-ix.cmake
@@ -40,7 +40,7 @@
 endif()
 
 set(ALL_BUILTIN_SUPPORTED_ARCH ${X86} ${X86_64} ${ARM32} ${ARM64}
-${MIPS32} ${MIPS64} ${WASM32} ${WASM64})
+${MIPS32} ${MIPS64} ${PPC64} ${WASM32} ${WASM64})
 
 include(CompilerRTUtils)
 include(CompilerRTDarwinUtils)
Index: compiler-rt/cmake/Modules/CompilerRTUtils.cmake
===
--- compiler-rt/cmake/Modules/CompilerRTUtils.cmake
+++ compiler-rt/cmake/Modules/CompilerRTUtils.cmake
@@ -167,6 +167,8 @@
   check_symbol_exists(__i386__ "" __I386)
   check_symbol_exists(__mips__ "" __MIPS)
   check_symbol_exists(__mips64__ "" __MIPS64)
+  check_symbol_exists(__powerpc64__ "" __PPC64)
+  check_symbol_exists(__powerpc64le__ "" __PPC64LE)
   check_symbol_exists(__s390x__ "" __S390X)
   check_symbol_exists(__wasm32__ "" __WEBASSEMBLY32)
   check_symbol_exists(__wasm64__ "" __WEBASSEMBLY64)
@@ -184,6 +186,10 @@
 add_default_target_arch(mips64)
   elseif(__MIPS)
 add_default_target_arch(mips)
+  elseif(__PPC64)
+add_default_target_arch(powerpc64)
+  elseif(__PPC64LE)
+add_default_target_arch(powerpc64le)
   elseif(__S390X)
 add_default_target_arch(s390x)
   elseif(__WEBASSEMBLY32)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36764: The following functions and tests work fine for powerpc64, so enable them.

[Sterling Augustine via Phabricator via cfe-commits]

saugustine added a comment.

OK. I understand how these should work now.

As they are 80-bit specific, I am going to abandon this change and add them to 
my other patch which moves 80-bit specific floats and tests to x86-specific 
lists.


https://reviews.llvm.org/D36764



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r311487 - Revert "Revert "Revert "Revert "Fix LLVMgold plugin name/path for non-Linux.""""

[Adrian Prantl via cfe-commits]

Great. Thanks!

-- adrian
> On Aug 22, 2017, at 3:11 PM, Dan Albert  wrote:
> 
> r311488 was submitted moments later and should fix this (one of the other 
> buildbots caught this really quickly).
> 
> On Tue, Aug 22, 2017 at 3:09 PM, Adrian Prantl  > wrote:
> 
> > On Aug 22, 2017, at 2:05 PM, Dan Albert via cfe-commits 
> > > wrote:
> >
> > Author: danalbert
> > Date: Tue Aug 22 14:05:01 2017
> > New Revision: 311487
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=311487=rev 
> > 
> > Log:
> > Revert "Revert "Revert "Revert "Fix LLVMgold plugin name/path for 
> > non-Linux.
> >
> > With tests fixed for Windows style paths now that they are going
> > through path canonicalization.
> >
> > Added:
> >cfe/trunk/test/Driver/lto-plugin-darwin.c
> 
> This test fails on green dragon. Could you please take a look?
> 
> http://green.lab.llvm.org/green/job/clang-stage1-configure-RA_check/34792/consoleFull#11650695138254eaf0-7326-4999-85b0-388101f2d404
>  
> 
> 
> -- adrian
> 
> >cfe/trunk/test/Driver/lto-plugin-linux.c
> >cfe/trunk/test/Driver/lto-plugin-windows.c
> > Modified:
> >cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
> >cfe/trunk/test/Driver/freebsd.c
> >cfe/trunk/test/Driver/gold-lto.c
> >cfe/trunk/test/Driver/lto.c
> >cfe/trunk/test/Driver/thinlto.c
> >
> > Modified: cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
> > URL: 
> > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp?rev=311487=311486=311487=diff
> >  
> > 
> > ==
> > --- cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp (original)
> > +++ cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp Tue Aug 22 14:05:01 2017
> > @@ -376,8 +376,20 @@ void tools::AddGoldPlugin(const ToolChai
> >   // as gold requires -plugin to come before any -plugin-opt that -Wl might
> >   // forward.
> >   CmdArgs.push_back("-plugin");
> > -  std::string Plugin =
> > -  ToolChain.getDriver().Dir + "/../lib" CLANG_LIBDIR_SUFFIX 
> > "/LLVMgold.so";
> > +
> > +#if defined(LLVM_ON_WIN32)
> > +  const char *Suffix = ".dll";
> > +#elif defined(__APPLE__)
> > +  const char *Suffix = ".dylib";
> > +#else
> > +  const char *Suffix = ".so";
> > +#endif
> > +
> > +  SmallString<1024> Plugin;
> > +  llvm::sys::path::native(Twine(ToolChain.getDriver().Dir) +
> > +  "/../lib" CLANG_LIBDIR_SUFFIX "/LLVMgold" +
> > +  Suffix,
> > +  Plugin);
> >   CmdArgs.push_back(Args.MakeArgString(Plugin));
> >
> >   // Try to pass driver level flags relevant to LTO code generation down to
> >
> > Modified: cfe/trunk/test/Driver/freebsd.c
> > URL: 
> > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/freebsd.c?rev=311487=311486=311487=diff
> >  
> > 
> > ==
> > --- cfe/trunk/test/Driver/freebsd.c (original)
> > +++ cfe/trunk/test/Driver/freebsd.c Tue Aug 22 14:05:01 2017
> > @@ -127,7 +127,7 @@
> >
> > // RUN: %clang -target x86_64-pc-freebsd8 %s -### -flto 2>&1 \
> > // RUN:   | FileCheck --check-prefix=CHECK-LTO %s
> > -// CHECK-LTO: ld{{.*}}" "-plugin{{.*}}LLVMgold.so
> > +// CHECK-LTO: ld{{.*}}" "-plugin{{.*}}{{[/\\]}}LLVMgold.{{dll|dylib|so}}
> >
> > // RUN: %clang -target sparc-unknown-freebsd8 %s -### -fpic 
> > -no-integrated-as 2>&1 \
> > // RUN:   | FileCheck --check-prefix=CHECK-SPARC-PIE %s
> >
> > Modified: cfe/trunk/test/Driver/gold-lto.c
> > URL: 
> > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/gold-lto.c?rev=311487=311486=311487=diff
> >  
> > 
> > ==
> > --- cfe/trunk/test/Driver/gold-lto.c (original)
> > +++ cfe/trunk/test/Driver/gold-lto.c Tue Aug 22 14:05:01 2017
> > @@ -3,14 +3,14 @@
> > // RUN: %clang -target x86_64-unknown-linux -### %t.o -flto 2>&1 \
> > // RUN: -Wl,-plugin-opt=foo -O3 \
> > // RUN: | FileCheck %s --check-prefix=CHECK-X86-64-BASIC
> > -// CHECK-X86-64-BASIC: "-plugin" "{{.*}}/LLVMgold.so"
> > +// CHECK-X86-64-BASIC: "-plugin" "{{.*}}{{[/\\]}}LLVMgold.{{dll|dylib|so}}"
> > // CHECK-X86-64-BASIC: "-plugin-opt=O3"
> > // CHECK-X86-64-BASIC: "-plugin-opt=foo"
> > //
> > // RUN: %clang -target x86_64-unknown-linux -### %t.o -flto 2>&1 \
> > 

Re: r311487 - Revert "Revert "Revert "Revert "Fix LLVMgold plugin name/path for non-Linux.""""

[Dan Albert via cfe-commits]

r311488 was submitted moments later and should fix this (one of the other
buildbots caught this really quickly).

On Tue, Aug 22, 2017 at 3:09 PM, Adrian Prantl  wrote:

>
> > On Aug 22, 2017, at 2:05 PM, Dan Albert via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
> >
> > Author: danalbert
> > Date: Tue Aug 22 14:05:01 2017
> > New Revision: 311487
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=311487=rev
> > Log:
> > Revert "Revert "Revert "Revert "Fix LLVMgold plugin name/path for
> non-Linux.
> >
> > With tests fixed for Windows style paths now that they are going
> > through path canonicalization.
> >
> > Added:
> >cfe/trunk/test/Driver/lto-plugin-darwin.c
>
> This test fails on green dragon. Could you please take a look?
>
> http://green.lab.llvm.org/green/job/clang-stage1-configure-RA_check/34792/
> consoleFull#11650695138254eaf0-7326-4999-85b0-388101f2d404
>
> -- adrian
>
> >cfe/trunk/test/Driver/lto-plugin-linux.c
> >cfe/trunk/test/Driver/lto-plugin-windows.c
> > Modified:
> >cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
> >cfe/trunk/test/Driver/freebsd.c
> >cfe/trunk/test/Driver/gold-lto.c
> >cfe/trunk/test/Driver/lto.c
> >cfe/trunk/test/Driver/thinlto.c
> >
> > Modified: cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
> > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/
> ToolChains/CommonArgs.cpp?rev=311487=311486=311487=diff
> > 
> ==
> > --- cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp (original)
> > +++ cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp Tue Aug 22 14:05:01
> 2017
> > @@ -376,8 +376,20 @@ void tools::AddGoldPlugin(const ToolChai
> >   // as gold requires -plugin to come before any -plugin-opt that -Wl
> might
> >   // forward.
> >   CmdArgs.push_back("-plugin");
> > -  std::string Plugin =
> > -  ToolChain.getDriver().Dir + "/../lib" CLANG_LIBDIR_SUFFIX
> "/LLVMgold.so";
> > +
> > +#if defined(LLVM_ON_WIN32)
> > +  const char *Suffix = ".dll";
> > +#elif defined(__APPLE__)
> > +  const char *Suffix = ".dylib";
> > +#else
> > +  const char *Suffix = ".so";
> > +#endif
> > +
> > +  SmallString<1024> Plugin;
> > +  llvm::sys::path::native(Twine(ToolChain.getDriver().Dir) +
> > +  "/../lib" CLANG_LIBDIR_SUFFIX "/LLVMgold"
> +
> > +  Suffix,
> > +  Plugin);
> >   CmdArgs.push_back(Args.MakeArgString(Plugin));
> >
> >   // Try to pass driver level flags relevant to LTO code generation down
> to
> >
> > Modified: cfe/trunk/test/Driver/freebsd.c
> > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/
> freebsd.c?rev=311487=311486=311487=diff
> > 
> ==
> > --- cfe/trunk/test/Driver/freebsd.c (original)
> > +++ cfe/trunk/test/Driver/freebsd.c Tue Aug 22 14:05:01 2017
> > @@ -127,7 +127,7 @@
> >
> > // RUN: %clang -target x86_64-pc-freebsd8 %s -### -flto 2>&1 \
> > // RUN:   | FileCheck --check-prefix=CHECK-LTO %s
> > -// CHECK-LTO: ld{{.*}}" "-plugin{{.*}}LLVMgold.so
> > +// CHECK-LTO: ld{{.*}}" "-plugin{{.*}}{{[/\\]}}
> LLVMgold.{{dll|dylib|so}}
> >
> > // RUN: %clang -target sparc-unknown-freebsd8 %s -### -fpic
> -no-integrated-as 2>&1 \
> > // RUN:   | FileCheck --check-prefix=CHECK-SPARC-PIE %s
> >
> > Modified: cfe/trunk/test/Driver/gold-lto.c
> > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/
> gold-lto.c?rev=311487=311486=311487=diff
> > 
> ==
> > --- cfe/trunk/test/Driver/gold-lto.c (original)
> > +++ cfe/trunk/test/Driver/gold-lto.c Tue Aug 22 14:05:01 2017
> > @@ -3,14 +3,14 @@
> > // RUN: %clang -target x86_64-unknown-linux -### %t.o -flto 2>&1 \
> > // RUN: -Wl,-plugin-opt=foo -O3 \
> > // RUN: | FileCheck %s --check-prefix=CHECK-X86-64-BASIC
> > -// CHECK-X86-64-BASIC: "-plugin" "{{.*}}/LLVMgold.so"
> > +// CHECK-X86-64-BASIC: "-plugin" "{{.*}}{{[/\\]}}LLVMgold.{{
> dll|dylib|so}}"
> > // CHECK-X86-64-BASIC: "-plugin-opt=O3"
> > // CHECK-X86-64-BASIC: "-plugin-opt=foo"
> > //
> > // RUN: %clang -target x86_64-unknown-linux -### %t.o -flto 2>&1 \
> > // RUN: -march=corei7 -Wl,-plugin-opt=foo -Ofast \
> > // RUN: | FileCheck %s --check-prefix=CHECK-X86-64-COREI7
> > -// CHECK-X86-64-COREI7: "-plugin" "{{.*}}/LLVMgold.so"
> > +// CHECK-X86-64-COREI7: "-plugin" "{{.*}}{{[/\\]}}LLVMgold.{{
> dll|dylib|so}}"
> > // CHECK-X86-64-COREI7: "-plugin-opt=mcpu=corei7"
> > // CHECK-X86-64-COREI7: "-plugin-opt=O3"
> > // CHECK-X86-64-COREI7: "-plugin-opt=foo"
> > @@ -18,11 +18,11 @@
> > // RUN: %clang -target arm-unknown-linux -### %t.o -flto 2>&1 \
> > // RUN: -march=armv7a -Wl,-plugin-opt=foo -O0 \
> > // RUN: | FileCheck %s --check-prefix=CHECK-ARM-V7A
> > -// CHECK-ARM-V7A: "-plugin" "{{.*}}/LLVMgold.so"
> > +// CHECK-ARM-V7A: "-plugin" 

Re: r311487 - Revert "Revert "Revert "Revert "Fix LLVMgold plugin name/path for non-Linux.""""

[Adrian Prantl via cfe-commits]

> On Aug 22, 2017, at 2:05 PM, Dan Albert via cfe-commits 
>  wrote:
> 
> Author: danalbert
> Date: Tue Aug 22 14:05:01 2017
> New Revision: 311487
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=311487=rev
> Log:
> Revert "Revert "Revert "Revert "Fix LLVMgold plugin name/path for 
> non-Linux.
> 
> With tests fixed for Windows style paths now that they are going
> through path canonicalization.
> 
> Added:
>cfe/trunk/test/Driver/lto-plugin-darwin.c

This test fails on green dragon. Could you please take a look?

http://green.lab.llvm.org/green/job/clang-stage1-configure-RA_check/34792/consoleFull#11650695138254eaf0-7326-4999-85b0-388101f2d404

-- adrian

>cfe/trunk/test/Driver/lto-plugin-linux.c
>cfe/trunk/test/Driver/lto-plugin-windows.c
> Modified:
>cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
>cfe/trunk/test/Driver/freebsd.c
>cfe/trunk/test/Driver/gold-lto.c
>cfe/trunk/test/Driver/lto.c
>cfe/trunk/test/Driver/thinlto.c
> 
> Modified: cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp?rev=311487=311486=311487=diff
> ==
> --- cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp (original)
> +++ cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp Tue Aug 22 14:05:01 2017
> @@ -376,8 +376,20 @@ void tools::AddGoldPlugin(const ToolChai
>   // as gold requires -plugin to come before any -plugin-opt that -Wl might
>   // forward.
>   CmdArgs.push_back("-plugin");
> -  std::string Plugin =
> -  ToolChain.getDriver().Dir + "/../lib" CLANG_LIBDIR_SUFFIX 
> "/LLVMgold.so";
> +
> +#if defined(LLVM_ON_WIN32)
> +  const char *Suffix = ".dll";
> +#elif defined(__APPLE__)
> +  const char *Suffix = ".dylib";
> +#else
> +  const char *Suffix = ".so";
> +#endif
> +
> +  SmallString<1024> Plugin;
> +  llvm::sys::path::native(Twine(ToolChain.getDriver().Dir) +
> +  "/../lib" CLANG_LIBDIR_SUFFIX "/LLVMgold" +
> +  Suffix,
> +  Plugin);
>   CmdArgs.push_back(Args.MakeArgString(Plugin));
> 
>   // Try to pass driver level flags relevant to LTO code generation down to
> 
> Modified: cfe/trunk/test/Driver/freebsd.c
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/freebsd.c?rev=311487=311486=311487=diff
> ==
> --- cfe/trunk/test/Driver/freebsd.c (original)
> +++ cfe/trunk/test/Driver/freebsd.c Tue Aug 22 14:05:01 2017
> @@ -127,7 +127,7 @@
> 
> // RUN: %clang -target x86_64-pc-freebsd8 %s -### -flto 2>&1 \
> // RUN:   | FileCheck --check-prefix=CHECK-LTO %s
> -// CHECK-LTO: ld{{.*}}" "-plugin{{.*}}LLVMgold.so
> +// CHECK-LTO: ld{{.*}}" "-plugin{{.*}}{{[/\\]}}LLVMgold.{{dll|dylib|so}}
> 
> // RUN: %clang -target sparc-unknown-freebsd8 %s -### -fpic -no-integrated-as 
> 2>&1 \
> // RUN:   | FileCheck --check-prefix=CHECK-SPARC-PIE %s
> 
> Modified: cfe/trunk/test/Driver/gold-lto.c
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/gold-lto.c?rev=311487=311486=311487=diff
> ==
> --- cfe/trunk/test/Driver/gold-lto.c (original)
> +++ cfe/trunk/test/Driver/gold-lto.c Tue Aug 22 14:05:01 2017
> @@ -3,14 +3,14 @@
> // RUN: %clang -target x86_64-unknown-linux -### %t.o -flto 2>&1 \
> // RUN: -Wl,-plugin-opt=foo -O3 \
> // RUN: | FileCheck %s --check-prefix=CHECK-X86-64-BASIC
> -// CHECK-X86-64-BASIC: "-plugin" "{{.*}}/LLVMgold.so"
> +// CHECK-X86-64-BASIC: "-plugin" "{{.*}}{{[/\\]}}LLVMgold.{{dll|dylib|so}}"
> // CHECK-X86-64-BASIC: "-plugin-opt=O3"
> // CHECK-X86-64-BASIC: "-plugin-opt=foo"
> //
> // RUN: %clang -target x86_64-unknown-linux -### %t.o -flto 2>&1 \
> // RUN: -march=corei7 -Wl,-plugin-opt=foo -Ofast \
> // RUN: | FileCheck %s --check-prefix=CHECK-X86-64-COREI7
> -// CHECK-X86-64-COREI7: "-plugin" "{{.*}}/LLVMgold.so"
> +// CHECK-X86-64-COREI7: "-plugin" "{{.*}}{{[/\\]}}LLVMgold.{{dll|dylib|so}}"
> // CHECK-X86-64-COREI7: "-plugin-opt=mcpu=corei7"
> // CHECK-X86-64-COREI7: "-plugin-opt=O3"
> // CHECK-X86-64-COREI7: "-plugin-opt=foo"
> @@ -18,11 +18,11 @@
> // RUN: %clang -target arm-unknown-linux -### %t.o -flto 2>&1 \
> // RUN: -march=armv7a -Wl,-plugin-opt=foo -O0 \
> // RUN: | FileCheck %s --check-prefix=CHECK-ARM-V7A
> -// CHECK-ARM-V7A: "-plugin" "{{.*}}/LLVMgold.so"
> +// CHECK-ARM-V7A: "-plugin" "{{.*}}{{[/\\]}}LLVMgold.{{dll|dylib|so}}"
> // CHECK-ARM-V7A: "-plugin-opt=mcpu=generic"
> // CHECK-ARM-V7A: "-plugin-opt=O0"
> // CHECK-ARM-V7A: "-plugin-opt=foo"
> //
> // RUN: %clang -target i686-linux-android -### %t.o -flto 2>&1 \
> // RUN: | FileCheck %s --check-prefix=CHECK-X86-ANDROID
> -// CHECK-X86-ANDROID: "-plugin" "{{.*}}/LLVMgold.so"
> +// CHECK-X86-ANDROID: "-plugin" 

[PATCH] D35955: clang-format: Add preprocessor directive indentation

[Erik Uhlmann via Phabricator via cfe-commits]

euhlmann added inline comments.



Comment at: lib/Format/ContinuationIndenter.cpp:387
+// hash. This causes second-level indents onward to have an extra space
+// after the tabs. We set the state to column 0 to avoid this misalignment.
+if (Style.UseTab != FormatStyle::UT_Never)

krasimir wrote:
> I don't understand this comment. Could you please give an example?
When tabs are enabled, extra spaces would be added because of the column offset 
caused by the `#` token.
```
#if FOO
#if BAR
#define BAZ
#endif
#endif
```
Let me know if there's a better way to fix that.



Comment at: lib/Format/UnwrappedLineParser.cpp:732
+  // then we count it as a real include guard and subtract one from every
+  // preprocessor indent.
+  unsigned TokenPosition = Tokens->getPosition();

krasimir wrote:
> Why do we need to subtract one from every preprocessor indent?
This only knows for sure when something is an include guard at the end of the 
file. By that time, everything inside the include guard has already been 
indented one extra level since we didn't know about the guard yet, so it would 
look like
```
#ifndef FILE_H
#  define FILE_H
// ...
#  other directives
// ...
#endif
```
Subtracting one is the action taken when we decide that there's a valid include 
guard, so it looks like
```
#ifndef FILE_H
#define FILE_H
// ...
#other directives
// ...
#endif
```



Comment at: lib/Format/UnwrappedLineParser.cpp:760
+  }
+  PPMaybeIncludeGuard = nullptr;
   nextToken();

krasimir wrote:
> Why do we reset `PPMaybeIncludeGuard` here?
The idea is if there's any preprocessor lines between the #ifndef and the 
#define, it doesn't treat it as an include guard. This is the same reason for 
the other PPMaybeIncludeGuard reset.



Comment at: lib/Format/UnwrappedLineParser.cpp:770
   addUnwrappedLine();
-  Line->Level = 1;
+  ++Line->Level;
 

krasimir wrote:
> Why do we `++Line->Level` here?
This sets correct indent for multi-line macros, so they are indented one level 
more than the indent of the #define instead of always at level 1.


https://reviews.llvm.org/D35955



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r311391 - [Driver] Recognize DevDiv internal builds of MSVC, with a different directory structure.

[Hans Wennborg via cfe-commits]

Merged to 5.0 in r311500.

On Mon, Aug 21, 2017 at 3:19 PM, Stephan T. Lavavej via cfe-commits
 wrote:
> Author: stl_msft
> Date: Mon Aug 21 15:19:33 2017
> New Revision: 311391
>
> URL: http://llvm.org/viewvc/llvm-project?rev=311391=rev
> Log:
> [Driver] Recognize DevDiv internal builds of MSVC, with a different directory 
> structure.
>
> This is a reasonably non-intrusive change, which I've verified
> works for both x86 and x64 DevDiv-internal builds.
>
> The idea is to change `bool IsVS2017OrNewer` into a 3-state
> `ToolsetLayout VSLayout`. Either a build is DevDiv-internal,
> released VS 2017 or newer, or released VS 2015 or older. When looking at
> the directory structure, if instead of `"VC"` we see `"x86ret"`, `"x86chk"`,
> `"amd64ret"`, or `"amd64chk"`, we recognize this as a DevDiv-internal build.
>
> After we get past the directory structure validation, we use this knowledge
> to regenerate paths appropriately. `llvmArchToDevDivInternalArch()` knows how
> we use `"i386"` subdirectories, and `MSVCToolChain::getSubDirectoryPath()`
> uses that. It also knows that DevDiv-internal builds have an `"inc"`
> subdirectory instead of `"include"`.
>
> This may still not be the "right" fix in any sense, but I believe that it's
> non-intrusive in the sense that if the special directory names aren't found,
> no codepaths are affected. (`ToolsetLayout::OlderVS` and
> `ToolsetLayout::VS2017OrNewer` correspond to `IsVS2017OrNewer` being `false`
> or `true`, respectively.) I searched for all references to `IsVS2017OrNewer`,
> which are places where Clang cares about VS's directory structure, and the
> only one that isn't being patched is some logic to deal with
> cross-compilation. I'm fine with that not working for DevDiv-internal builds
> for the moment (we typically test the native compilers), so I added a comment.
>
> Fixes D36860.
>
> Modified:
> cfe/trunk/lib/Driver/ToolChains/MSVC.cpp
> cfe/trunk/lib/Driver/ToolChains/MSVC.h
>
> Modified: cfe/trunk/lib/Driver/ToolChains/MSVC.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/MSVC.cpp?rev=311391=311390=311391=diff
> ==
> --- cfe/trunk/lib/Driver/ToolChains/MSVC.cpp (original)
> +++ cfe/trunk/lib/Driver/ToolChains/MSVC.cpp Mon Aug 21 15:19:33 2017
> @@ -76,7 +76,7 @@ static bool getSystemRegistryString(cons
>
>  // Check various environment variables to try and find a toolchain.
>  static bool findVCToolChainViaEnvironment(std::string ,
> -  bool ) {
> +  MSVCToolChain::ToolsetLayout 
> ) {
>// These variables are typically set by vcvarsall.bat
>// when launching a developer command prompt.
>if (llvm::Optional VCToolsInstallDir =
> @@ -84,7 +84,7 @@ static bool findVCToolChainViaEnvironmen
>  // This is only set by newer Visual Studios, and it leads straight to
>  // the toolchain directory.
>  Path = std::move(*VCToolsInstallDir);
> -IsVS2017OrNewer = true;
> +VSLayout = MSVCToolChain::ToolsetLayout::VS2017OrNewer;
>  return true;
>}
>if (llvm::Optional VCInstallDir =
> @@ -94,7 +94,7 @@ static bool findVCToolChainViaEnvironmen
>  // so this check has to appear second.
>  // In older Visual Studios, the VC directory is the toolchain.
>  Path = std::move(*VCInstallDir);
> -IsVS2017OrNewer = false;
> +VSLayout = MSVCToolChain::ToolsetLayout::OlderVS;
>  return true;
>}
>
> @@ -134,9 +134,16 @@ static bool findVCToolChainViaEnvironmen
>}
>if (IsBin) {
>  llvm::StringRef ParentPath = llvm::sys::path::parent_path(TestPath);
> -if (llvm::sys::path::filename(ParentPath) == "VC") {
> +llvm::StringRef ParentFilename = 
> llvm::sys::path::filename(ParentPath);
> +if (ParentFilename == "VC") {
>Path = ParentPath;
> -  IsVS2017OrNewer = false;
> +  VSLayout = MSVCToolChain::ToolsetLayout::OlderVS;
> +  return true;
> +}
> +if (ParentFilename == "x86ret" || ParentFilename == "x86chk"
> +  || ParentFilename == "amd64ret" || ParentFilename == "amd64chk") {
> +  Path = ParentPath;
> +  VSLayout = MSVCToolChain::ToolsetLayout::DevDivInternal;
>return true;
>  }
>
> @@ -165,7 +172,7 @@ static bool findVCToolChainViaEnvironmen
>ToolChainPath = llvm::sys::path::parent_path(ToolChainPath);
>
>  Path = ToolChainPath;
> -IsVS2017OrNewer = true;
> +VSLayout = MSVCToolChain::ToolsetLayout::VS2017OrNewer;
>  return true;
>}
>
> @@ -181,7 +188,7 @@ static bool findVCToolChainViaEnvironmen
>  // This is the preferred way to discover new Visual Studios, as they're no
>  // longer listed in the registry.
>  static bool findVCToolChainViaSetupConfig(std::string ,
> -

[PATCH] D36914: Implement CFG construction for __try / __except / __leave.

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

Looks functionally correct




Comment at: test/Sema/warn-unreachable-ms.c:42
+  }
+}

Can we add a test to exercise that this builds the right CFG?
```
__try {
  __try {
f();
  } __except(1) {
__leave; // should exit outer try
  }
  __leave;
  f(); // expected-warning{{never be executed}}
} __except(1) {
}
```


https://reviews.llvm.org/D36914



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r309226 - Headers: improve ARM EHABI coverage of unwind.h

[Evgenii Stepanov via cfe-commits]

No. I don't have a easy way of reproducing this.

On Tue, Aug 22, 2017 at 11:10 AM, Hans Wennborg  wrote:
> Is there a bug filed? Since this was merged to 5.0.0, I'd like to know
> if we broke something and if that is something that needs to be fixed.
>
> On Tue, Aug 22, 2017 at 10:46 AM, Evgenii Stepanov
>  wrote:
>> As I understand, using compiler-rt as libgcc replacement on ARM is
>> currently broken because of this change, but I have not looked since
>> my last message.
>>
>> On Mon, Aug 21, 2017 at 4:56 PM, Hans Wennborg  wrote:
>>> Is there something we need for 5.0.0 here?
>>>
>>> On Sat, Aug 12, 2017 at 9:58 PM, Saleem Abdulrasool
>>>  wrote:
 Yeah, we should adjust that.  Technically, it is `_Unwind_Control_Block on
 ARM EHABI.  However, _Unwind_Exception is aliased to that, which is why we
 can use that in the personality routine.  We should adjust the sources for
 the personality routine.

 On Fri, Aug 11, 2017 at 1:12 PM, Evgenii Stepanov
  wrote:
>
> Hi,
>
> I've noticed that the code in
> compiler-rt/lib/builtins/gcc_personality_v0.c refers to
> _Unwind_Exception as "struct _Unwind_Exception". With this change, it
> is not a struct anymore on ARM. Should that code be fixed, or is it a
> problem in this change?
>
> compiler-rt/lib/builtins/gcc_personality_v0.c:153:23: error:
> declaration of 'struct _Unwind_Exception' will not be visible outside
> of this function [-Werror,-Wvisibility]
> continueUnwind(struct _Unwind_Exception *exceptionObject,
>
> On Thu, Jul 27, 2017 at 9:46 AM, Hans Wennborg via cfe-commits
>  wrote:
> > Merged to 5.0 in r309290.
> >
> > On Wed, Jul 26, 2017 at 3:55 PM, Saleem Abdulrasool via cfe-commits
> >  wrote:
> >> Author: compnerd
> >> Date: Wed Jul 26 15:55:23 2017
> >> New Revision: 309226
> >>
> >> URL: http://llvm.org/viewvc/llvm-project?rev=309226=rev
> >> Log:
> >> Headers: improve ARM EHABI coverage of unwind.h
> >>
> >> Ensure that we define the `_Unwind_Control_Block` structure used on ARM
> >> EHABI targets.  This is needed for building libc++abi with the unwind.h
> >> from the resource dir.  A minor fallout of this is that we needed to
> >> create a typedef for _Unwind_Exception to work across ARM EHABI and
> >> non-EHABI targets.  The structure definitions here are based originally
> >> on the documentation from ARM under the "Exception Handling ABI for the
> >> ARM® Architecture" Section 7.2.  They are then adjusted to more closely
> >> reflect the definition in libunwind from LLVM.  Those changes are
> >> compatible in layout but permit easier use in libc++abi and help
> >> maintain compatibility between libunwind and the compiler provided
> >> definition.
> >>
> >> Modified:
> >> cfe/trunk/lib/Headers/unwind.h
> >>
> >> Modified: cfe/trunk/lib/Headers/unwind.h
> >> URL:
> >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/unwind.h?rev=309226=309225=309226=diff
> >>
> >> ==
> >> --- cfe/trunk/lib/Headers/unwind.h (original)
> >> +++ cfe/trunk/lib/Headers/unwind.h Wed Jul 26 15:55:23 2017
> >> @@ -76,7 +76,13 @@ typedef intptr_t _sleb128_t;
> >>  typedef uintptr_t _uleb128_t;
> >>
> >>  struct _Unwind_Context;
> >> +#if defined(__arm__) && !(defined(__USING_SJLJ_EXCEPTIONS__) ||
> >> defined(__ARM_DWARF_EH___))
> >> +struct _Unwind_Control_Block;
> >> +typedef struct _Unwind_Control_Block _Unwind_Exception; /* Alias */
> >> +#else
> >>  struct _Unwind_Exception;
> >> +typedef struct _Unwind_Exception _Unwind_Exception;
> >> +#endif
> >>  typedef enum {
> >>_URC_NO_REASON = 0,
> >>  #if defined(__arm__) && !defined(__USING_SJLJ_EXCEPTIONS__) && \
> >> @@ -109,8 +115,42 @@ typedef enum {
> >>  } _Unwind_Action;
> >>
> >>  typedef void (*_Unwind_Exception_Cleanup_Fn)(_Unwind_Reason_Code,
> >> - struct _Unwind_Exception
> >> *);
> >> + _Unwind_Exception *);
> >>
> >> +#if defined(__arm__) && !(defined(__USING_SJLJ_EXCEPTIONS__) ||
> >> defined(__ARM_DWARF_EH___))
> >> +typedef struct _Unwind_Control_Block _Unwind_Control_Block;
> >> +typedef uint32_t _Unwind_EHT_Header;
> >> +
> >> +struct _Unwind_Control_Block {
> >> +  uint64_t exception_class;
> >> +  void (*exception_cleanup)(_Unwind_Reason_Code, _Unwind_Control_Block
> >> *);
> >> +  /* unwinder cache (private fields for the unwinder's use) */
> >> +  struct {
> >> +uint32_t reserved1; /* 

Re: [PATCH] D36914: Implement CFG construction for __try / __except / __leave.

[Nico Weber via cfe-commits]

rnk: ping :-)

On Mon, Aug 21, 2017 at 1:43 PM, Nico Weber via Phabricator via cfe-commits
 wrote:

> thakis added inline comments.
>
>
> 
> Comment at: lib/Analysis/CFG.cpp:448
> +BuildOpts(buildOpts), switchExclusivelyCovered(false),
> +switchCond(nullptr), cachedEntry(nullptr), lastLookup(nullptr) {}
>
> 
> (this is now a no-op and only a clang-formatting of the existing ctor
> code. I can omit this if you want.)
>
>
> https://reviews.llvm.org/D36914
>
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r311488 - Degeneralize more tests.

[Dan Albert via cfe-commits]

Author: danalbert
Date: Tue Aug 22 14:16:22 2017
New Revision: 311488

URL: http://llvm.org/viewvc/llvm-project?rev=311488=rev
Log:
Degeneralize more tests.

As before, not every platform supports LTO. Make sure the platform
we're targeting is one that supports it (regardless of the *host*
platform).

Modified:
cfe/trunk/test/Driver/lto-plugin-darwin.c
cfe/trunk/test/Driver/lto-plugin-linux.c
cfe/trunk/test/Driver/lto-plugin-windows.c

Modified: cfe/trunk/test/Driver/lto-plugin-darwin.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/lto-plugin-darwin.c?rev=311488=311487=311488=diff
==
--- cfe/trunk/test/Driver/lto-plugin-darwin.c (original)
+++ cfe/trunk/test/Driver/lto-plugin-darwin.c Tue Aug 22 14:16:22 2017
@@ -1,6 +1,6 @@
 // Check that Darwin uses LLVMgold.dylib.
 // REQUIRES: system-darwin
-// RUN: %clang -### %s -flto 2>&1 \
+// RUN: %clang -### %s -target x86_64-unknown-linux -flto 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHECK-LTO-PLUGIN %s
 //
 // CHECK-LTO-PLUGIN: "-plugin" "{{.*}}/LLVMgold.dylib"

Modified: cfe/trunk/test/Driver/lto-plugin-linux.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/lto-plugin-linux.c?rev=311488=311487=311488=diff
==
--- cfe/trunk/test/Driver/lto-plugin-linux.c (original)
+++ cfe/trunk/test/Driver/lto-plugin-linux.c Tue Aug 22 14:16:22 2017
@@ -1,6 +1,6 @@
 // Check that non-Windows, non-Darwin OSs use LLVMgold.so.
 // REQUIRES: !system-darwin && !system-windows
-// RUN: %clang -### %s -flto 2>&1 \
+// RUN: %clang -### %s -target x86_64-unknown-linux -flto 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHECK-LTO-PLUGIN %s
 //
 // CHECK-LTO-PLUGIN: "-plugin" "{{.*}}/LLVMgold.so"

Modified: cfe/trunk/test/Driver/lto-plugin-windows.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/lto-plugin-windows.c?rev=311488=311487=311488=diff
==
--- cfe/trunk/test/Driver/lto-plugin-windows.c (original)
+++ cfe/trunk/test/Driver/lto-plugin-windows.c Tue Aug 22 14:16:22 2017
@@ -1,6 +1,6 @@
 // Check that Windows uses LLVMgold.dll.
 // REQUIRES: system-windows
-// RUN: %clang -### %s -flto 2>&1 \
+// RUN: %clang -### %s -target x86_64-unknown-linux -flto 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHECK-LTO-PLUGIN %s
 //
 // CHECK-LTO-PLUGIN: "-plugin" "{{.*}}\\LLVMgold.dll"


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37024: [libcxx] [test] Cleanup nullopt_t tests

[Casey Carter via Phabricator via cfe-commits]

CaseyCarter updated this revision to Diff 112224.
CaseyCarter added a comment.

Don't unnecessarily fully qualify `std::nullopt`.


https://reviews.llvm.org/D37024

Files:
  test/std/utilities/optional/optional.nullopt/not_brace_initializable.fail.cpp
  test/std/utilities/optional/optional.nullopt/nullopt_t.fail.cpp
  test/std/utilities/optional/optional.nullopt/nullopt_t.pass.cpp

Index: test/std/utilities/optional/optional.nullopt/nullopt_t.pass.cpp
===
--- test/std/utilities/optional/optional.nullopt/nullopt_t.pass.cpp
+++ test/std/utilities/optional/optional.nullopt/nullopt_t.pass.cpp
@@ -11,33 +11,30 @@
 // 
 
 // struct nullopt_t{see below};
-// constexpr nullopt_t nullopt(unspecified);
+// inline constexpr nullopt_t nullopt(unspecified);
 
 // [optional.nullopt]/2:
-//   Type nullopt_t shall not have a default constructor or an initializer-list constructor.
-//   It shall not be an aggregate and shall be a literal type.
-//   Constant nullopt shall be initialized with an argument of literal type.
+//   Type nullopt_t shall not have a default constructor or an initializer-list
+//   constructor, and shall not be an aggregate.
 
 #include 
 #include 
 
-using std::optional;
 using std::nullopt_t;
 using std::nullopt;
 
-constexpr
-int
-test(const nullopt_t&)
+constexpr bool test()
 {
-return 3;
+nullopt_t foo{nullopt};
+(void)foo;
+return true;
 }
 
 int main()
 {
-static_assert(( std::is_class::value), "");
-static_assert(( std::is_empty::value), "");
-static_assert(( std::is_literal_type::value), "");
-static_assert((!std::is_default_constructible::value), "");
+static_assert(std::is_empty_v);
+static_assert(!std::is_default_constructible_v);
 
-static_assert(test(nullopt) == 3, "");
+static_assert(std::is_same_v);
+static_assert(test());
 }
Index: test/std/utilities/optional/optional.nullopt/nullopt_t.fail.cpp
===
--- test/std/utilities/optional/optional.nullopt/nullopt_t.fail.cpp
+++ test/std/utilities/optional/optional.nullopt/nullopt_t.fail.cpp
@@ -11,15 +11,13 @@
 // 
 
 // struct nullopt_t{see below};
-// constexpr nullopt_t nullopt(unspecified);
+// inline constexpr nullopt_t nullopt(unspecified);
 
 // [optional.nullopt]/2:
-//   Type nullopt_t shall not have a default constructor or an initializer-list constructor.
-//   It shall not be an aggregate and shall be a literal type.
-//   Constant nullopt shall be initialized with an argument of literal type.
+//   Type nullopt_t shall not have a default constructor or an initializer-list
+//   constructor, and shall not be an aggregate.
 
 #include 
-#include "test_macros.h"
 
 int main()
 {
Index: test/std/utilities/optional/optional.nullopt/not_brace_initializable.fail.cpp
===
--- test/std/utilities/optional/optional.nullopt/not_brace_initializable.fail.cpp
+++ /dev/null
@@ -1,25 +0,0 @@
-//===--===//
-//
-// The LLVM Compiler Infrastructure
-//
-// This file is dual licensed under the MIT and the University of Illinois Open
-// Source Licenses. See LICENSE.TXT for details.
-//
-//===--===//
-
-// UNSUPPORTED: c++98, c++03, c++11, c++14
-// 
-
-// struct nullopt_t{see below};
-
-#include 
-
-using std::optional;
-using std::nullopt_t;
-
-int main()
-{
-// I roughly interpret LWG2736 as "it shall not be possible to copy-list-initialize nullopt_t with an
-// empty braced-init-list."
-nullopt_t foo = {};
-}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37024: [libcxx] [test] Cleanup nullopt_t tests

[Stephan T. Lavavej via Phabricator via cfe-commits]

STL_MSFT added a comment.

Otherwise, looks cromulent to me.




Comment at: test/std/utilities/optional/optional.nullopt/nullopt_t.pass.cpp:38
 
-static_assert(test(nullopt) == 3, "");
+static_assert(std::is_same_v);
+static_assert(test());

You're saying `std::nullopt` but you already dragged it in.


https://reviews.llvm.org/D37024



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37025: [analyzer] Support more pointer arithmetic in bugreporter::getDerefExpr().

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: test/Analysis/nullptr.cpp:54
 void zoo1() {
-  char **p = 0;
+  char **p = 0; // expected-note{{'p' initialized to a null pointer value}}
   delete *(p + 0); // expected-warning{{Dereference of null pointer}}

This note has been added by the patch. Other newly displayed notes in the old 
tests here have always been there, but just now got displayed.


https://reviews.llvm.org/D37025



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37025: [analyzer] Support more pointer arithmetic in bugreporter::getDerefExpr().

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
Herald added a subscriber: eraman.

This patch adds support for a few more pointer arithmetic cases. For instance, 
when `p` is a null pointer, it would be possible to track that expressions like 
`*(++p) = 5` and `*(p + 2) = 5` are null pointer dereferences that are based on 
pointer `p`. The analyzer would later be able to display additional diagnostics 
to track where `p` has become null and why. I noticed these cases accidentally 
when i was working on https://reviews.llvm.org/D37023.


https://reviews.llvm.org/D37025

Files:
  lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  test/Analysis/inlining/inline-defensive-checks.c
  test/Analysis/nullptr.cpp

Index: test/Analysis/nullptr.cpp
===
--- test/Analysis/nullptr.cpp
+++ test/Analysis/nullptr.cpp
@@ -1,11 +1,12 @@
-// RUN: %clang_analyze_cc1 -std=c++11 -Wno-conversion-null -analyzer-checker=core,debug.ExprInspection -analyzer-store region -verify %s
+// RUN: %clang_analyze_cc1 -std=c++11 -Wno-conversion-null -analyzer-checker=core,debug.ExprInspection -analyzer-store region -analyzer-output=text -verify %s
 
 void clang_analyzer_eval(int);
 
 // test to see if nullptr is detected as a null pointer
 void foo1(void) {
-  char  *np = nullptr;
+  char  *np = nullptr; // expected-note{{'np' initialized to a null pointer value}}
   *np = 0;  // expected-warning{{Dereference of null pointer}}
+// expected-note@-1{{Dereference of null pointer}}
 }
 
 // check if comparing nullptr to nullptr is detected properly
@@ -23,10 +24,11 @@
   struct foo {
 int a, f;
   };
-  char *np = nullptr;
+  char *np = nullptr; // expected-note{{'np' initialized to a null pointer value}}
   // casting a nullptr to anything should be caught eventually
-  int *ip = &(((struct foo *)np)->f);
+  int *ip = &(((struct foo *)np)->f); // expected-note{{'ip' initialized to a null pointer value}}
   *ip = 0;  // expected-warning{{Dereference of null pointer}}
+// expected-note@-1{{Dereference of null pointer}}
   // should be error here too, but analysis gets stopped
 //  *np = 0;
 }
@@ -49,16 +51,31 @@
 }
 
 void zoo1() {
-  char **p = 0;
+  char **p = 0; // expected-note{{'p' initialized to a null pointer value}}
   delete *(p + 0); // expected-warning{{Dereference of null pointer}}
+   // expected-note@-1{{Dereference of null pointer}}
+}
+
+void zoo1backwards() {
+  char **p = 0; // expected-note{{'p' initialized to a null pointer value}}
+  delete *(0 + p); // expected-warning{{Dereference of null pointer}}
+   // expected-note@-1{{Dereference of null pointer}}
+}
+
+typedef __INTPTR_TYPE__ intptr_t;
+void zoo1multiply() {
+  char **p = 0; // FIXME-should-be-note:{{'p' initialized to a null pointer value}}
+  delete *((char **)((intptr_t)p * 2)); // expected-warning{{Dereference of null pointer}}
+   // expected-note@-1{{Dereference of null pointer}}
 }
 
 void zoo2() {
   int **a = 0;
-  int **b = 0;
+  int **b = 0; // expected-note{{'b' initialized to a null pointer value}}
   asm ("nop"
   :"=r"(*a)
   :"0"(*b) // expected-warning{{Dereference of null pointer}}
+   // expected-note@-1{{Dereference of null pointer}}
   );
 }
 
@@ -70,17 +87,19 @@
 int a;
   };
 
-  int *x = 0;
+  int *x = 0; // expected-note{{'x' initialized to a null pointer value}}
   return S(*x).a; // expected-warning{{Dereference of null pointer}}
+  // expected-note@-1{{Dereference of null pointer}}
 }
 
 int materializeTempExpr() {
-  int *n = 0;
+  int *n = 0; // expected-note{{'n' initialized to a null pointer value}}
   struct S {
 int a;
 S(int i): a(i) {}
   };
   const S  = S(*n); // expected-warning{{Dereference of null pointer}}
+  // expected-note@-1{{Dereference of null pointer}}
   return s.a;
 }
 
@@ -98,33 +117,49 @@
 
 void invokeF(X* x) {
   x->f(); // expected-warning{{Called C++ object pointer is null}}
+  // expected-note@-1{{Called C++ object pointer is null}}
 }
 
 struct Type {
   decltype(nullptr) x;
 };
 
 void shouldNotCrash() {
-  decltype(nullptr) p;
-  if (getSymbol())
-invokeF(p); // expected-warning{{1st function call argument is an uninit}}
-  if (getSymbol())
-invokeF(nullptr);
-  if (getSymbol()) {
-X *x = Type().x;
+  decltype(nullptr) p; // expected-note{{'p' declared without an initial value}}
+  if (getSymbol()) // expected-note   {{Assuming the condition is false}}
+   // expected-note@-1{{Taking false branch}}
+   // expected-note@-2{{Assuming the condition is false}}
+   // expected-note@-3{{Taking false branch}}
+   // expected-note@-4{{Assuming the condition is true}}
+   // expected-note@-5{{Taking true branch}}
+invokeF(p); // expected-warning{{1st function call argument is an uninitialized value}}
+// 

r311487 - Revert "Revert "Revert "Revert "Fix LLVMgold plugin name/path for non-Linux.""""

[Dan Albert via cfe-commits]

Author: danalbert
Date: Tue Aug 22 14:05:01 2017
New Revision: 311487

URL: http://llvm.org/viewvc/llvm-project?rev=311487=rev
Log:
Revert "Revert "Revert "Revert "Fix LLVMgold plugin name/path for non-Linux.

With tests fixed for Windows style paths now that they are going
through path canonicalization.

Added:
cfe/trunk/test/Driver/lto-plugin-darwin.c
cfe/trunk/test/Driver/lto-plugin-linux.c
cfe/trunk/test/Driver/lto-plugin-windows.c
Modified:
cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
cfe/trunk/test/Driver/freebsd.c
cfe/trunk/test/Driver/gold-lto.c
cfe/trunk/test/Driver/lto.c
cfe/trunk/test/Driver/thinlto.c

Modified: cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp?rev=311487=311486=311487=diff
==
--- cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp Tue Aug 22 14:05:01 2017
@@ -376,8 +376,20 @@ void tools::AddGoldPlugin(const ToolChai
   // as gold requires -plugin to come before any -plugin-opt that -Wl might
   // forward.
   CmdArgs.push_back("-plugin");
-  std::string Plugin =
-  ToolChain.getDriver().Dir + "/../lib" CLANG_LIBDIR_SUFFIX "/LLVMgold.so";
+
+#if defined(LLVM_ON_WIN32)
+  const char *Suffix = ".dll";
+#elif defined(__APPLE__)
+  const char *Suffix = ".dylib";
+#else
+  const char *Suffix = ".so";
+#endif
+
+  SmallString<1024> Plugin;
+  llvm::sys::path::native(Twine(ToolChain.getDriver().Dir) +
+  "/../lib" CLANG_LIBDIR_SUFFIX "/LLVMgold" +
+  Suffix,
+  Plugin);
   CmdArgs.push_back(Args.MakeArgString(Plugin));
 
   // Try to pass driver level flags relevant to LTO code generation down to

Modified: cfe/trunk/test/Driver/freebsd.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/freebsd.c?rev=311487=311486=311487=diff
==
--- cfe/trunk/test/Driver/freebsd.c (original)
+++ cfe/trunk/test/Driver/freebsd.c Tue Aug 22 14:05:01 2017
@@ -127,7 +127,7 @@
 
 // RUN: %clang -target x86_64-pc-freebsd8 %s -### -flto 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-LTO %s
-// CHECK-LTO: ld{{.*}}" "-plugin{{.*}}LLVMgold.so
+// CHECK-LTO: ld{{.*}}" "-plugin{{.*}}{{[/\\]}}LLVMgold.{{dll|dylib|so}}
 
 // RUN: %clang -target sparc-unknown-freebsd8 %s -### -fpic -no-integrated-as 
2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-SPARC-PIE %s

Modified: cfe/trunk/test/Driver/gold-lto.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/gold-lto.c?rev=311487=311486=311487=diff
==
--- cfe/trunk/test/Driver/gold-lto.c (original)
+++ cfe/trunk/test/Driver/gold-lto.c Tue Aug 22 14:05:01 2017
@@ -3,14 +3,14 @@
 // RUN: %clang -target x86_64-unknown-linux -### %t.o -flto 2>&1 \
 // RUN: -Wl,-plugin-opt=foo -O3 \
 // RUN: | FileCheck %s --check-prefix=CHECK-X86-64-BASIC
-// CHECK-X86-64-BASIC: "-plugin" "{{.*}}/LLVMgold.so"
+// CHECK-X86-64-BASIC: "-plugin" "{{.*}}{{[/\\]}}LLVMgold.{{dll|dylib|so}}"
 // CHECK-X86-64-BASIC: "-plugin-opt=O3"
 // CHECK-X86-64-BASIC: "-plugin-opt=foo"
 //
 // RUN: %clang -target x86_64-unknown-linux -### %t.o -flto 2>&1 \
 // RUN: -march=corei7 -Wl,-plugin-opt=foo -Ofast \
 // RUN: | FileCheck %s --check-prefix=CHECK-X86-64-COREI7
-// CHECK-X86-64-COREI7: "-plugin" "{{.*}}/LLVMgold.so"
+// CHECK-X86-64-COREI7: "-plugin" "{{.*}}{{[/\\]}}LLVMgold.{{dll|dylib|so}}"
 // CHECK-X86-64-COREI7: "-plugin-opt=mcpu=corei7"
 // CHECK-X86-64-COREI7: "-plugin-opt=O3"
 // CHECK-X86-64-COREI7: "-plugin-opt=foo"
@@ -18,11 +18,11 @@
 // RUN: %clang -target arm-unknown-linux -### %t.o -flto 2>&1 \
 // RUN: -march=armv7a -Wl,-plugin-opt=foo -O0 \
 // RUN: | FileCheck %s --check-prefix=CHECK-ARM-V7A
-// CHECK-ARM-V7A: "-plugin" "{{.*}}/LLVMgold.so"
+// CHECK-ARM-V7A: "-plugin" "{{.*}}{{[/\\]}}LLVMgold.{{dll|dylib|so}}"
 // CHECK-ARM-V7A: "-plugin-opt=mcpu=generic"
 // CHECK-ARM-V7A: "-plugin-opt=O0"
 // CHECK-ARM-V7A: "-plugin-opt=foo"
 //
 // RUN: %clang -target i686-linux-android -### %t.o -flto 2>&1 \
 // RUN: | FileCheck %s --check-prefix=CHECK-X86-ANDROID
-// CHECK-X86-ANDROID: "-plugin" "{{.*}}/LLVMgold.so"
+// CHECK-X86-ANDROID: "-plugin" "{{.*}}{{[/\\]}}LLVMgold.{{dll|dylib|so}}"

Added: cfe/trunk/test/Driver/lto-plugin-darwin.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/lto-plugin-darwin.c?rev=311487=auto
==
--- cfe/trunk/test/Driver/lto-plugin-darwin.c (added)
+++ cfe/trunk/test/Driver/lto-plugin-darwin.c Tue Aug 22 14:05:01 2017
@@ -0,0 +1,6 @@
+// Check that Darwin uses LLVMgold.dylib.
+// REQUIRES: system-darwin
+// RUN: %clang -### %s -flto 

[PATCH] D37024: [libcxx] [test] Cleanup nullopt_t tests

[Casey Carter via Phabricator via cfe-commits]

CaseyCarter created this revision.

- Update specification text from N4387

- Delete `not_brace_initializable.fail.cpp`: it's redundant with 
`nullopt_t.fail.cpp`

- `is_empty` implies `is_class`

- `is_literal` is deprecated; directly verify that we can create a `nullopt_t` 
in a `constexpr` context


https://reviews.llvm.org/D37024

Files:
  test/std/utilities/optional/optional.nullopt/not_brace_initializable.fail.cpp
  test/std/utilities/optional/optional.nullopt/nullopt_t.fail.cpp
  test/std/utilities/optional/optional.nullopt/nullopt_t.pass.cpp

Index: test/std/utilities/optional/optional.nullopt/nullopt_t.pass.cpp
===
--- test/std/utilities/optional/optional.nullopt/nullopt_t.pass.cpp
+++ test/std/utilities/optional/optional.nullopt/nullopt_t.pass.cpp
@@ -11,33 +11,30 @@
 // 
 
 // struct nullopt_t{see below};
-// constexpr nullopt_t nullopt(unspecified);
+// inline constexpr nullopt_t nullopt(unspecified);
 
 // [optional.nullopt]/2:
-//   Type nullopt_t shall not have a default constructor or an initializer-list constructor.
-//   It shall not be an aggregate and shall be a literal type.
-//   Constant nullopt shall be initialized with an argument of literal type.
+//   Type nullopt_t shall not have a default constructor or an initializer-list
+//   constructor, and shall not be an aggregate.
 
 #include 
 #include 
 
-using std::optional;
 using std::nullopt_t;
 using std::nullopt;
 
-constexpr
-int
-test(const nullopt_t&)
+constexpr bool test()
 {
-return 3;
+nullopt_t foo{nullopt};
+(void)foo;
+return true;
 }
 
 int main()
 {
-static_assert(( std::is_class::value), "");
-static_assert(( std::is_empty::value), "");
-static_assert(( std::is_literal_type::value), "");
-static_assert((!std::is_default_constructible::value), "");
+static_assert(std::is_empty_v);
+static_assert(!std::is_default_constructible_v);
 
-static_assert(test(nullopt) == 3, "");
+static_assert(std::is_same_v);
+static_assert(test());
 }
Index: test/std/utilities/optional/optional.nullopt/nullopt_t.fail.cpp
===
--- test/std/utilities/optional/optional.nullopt/nullopt_t.fail.cpp
+++ test/std/utilities/optional/optional.nullopt/nullopt_t.fail.cpp
@@ -11,15 +11,13 @@
 // 
 
 // struct nullopt_t{see below};
-// constexpr nullopt_t nullopt(unspecified);
+// inline constexpr nullopt_t nullopt(unspecified);
 
 // [optional.nullopt]/2:
-//   Type nullopt_t shall not have a default constructor or an initializer-list constructor.
-//   It shall not be an aggregate and shall be a literal type.
-//   Constant nullopt shall be initialized with an argument of literal type.
+//   Type nullopt_t shall not have a default constructor or an initializer-list
+//   constructor, and shall not be an aggregate.
 
 #include 
-#include "test_macros.h"
 
 int main()
 {
Index: test/std/utilities/optional/optional.nullopt/not_brace_initializable.fail.cpp
===
--- test/std/utilities/optional/optional.nullopt/not_brace_initializable.fail.cpp
+++ /dev/null
@@ -1,25 +0,0 @@
-//===--===//
-//
-// The LLVM Compiler Infrastructure
-//
-// This file is dual licensed under the MIT and the University of Illinois Open
-// Source Licenses. See LICENSE.TXT for details.
-//
-//===--===//
-
-// UNSUPPORTED: c++98, c++03, c++11, c++14
-// 
-
-// struct nullopt_t{see below};
-
-#include 
-
-using std::optional;
-using std::nullopt_t;
-
-int main()
-{
-// I roughly interpret LWG2736 as "it shall not be possible to copy-list-initialize nullopt_t with an
-// empty braced-init-list."
-nullopt_t foo = {};
-}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37014: [clang-tidy] Add a checker to remove useless intermediate variables before return statements with comparisons

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

Please add the following test: (and make sure that it does the right thing :))

  bool f_with_preproc_condition() {
auto test = 42;
assert(test == 42);
return test;
  }

I.e. if `-DNDEBUG` is present, variable is not needed, but if `-DNDEBUG` is 
*NOT* present...


Repository:
  rL LLVM

https://reviews.llvm.org/D37014



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37023: [analyzer] Fix bugreporter::getDerefExpr() again.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.

This patch continues work that was started in https://reviews.llvm.org/D32291.

Our `bugreporter::getDerefExpr()` API tries to find out what has been 
dereferenced. For example, if we have an lvalue expression `x->y.z` which 
causes a null dereference when dereferenced, the function returns lvalue `x->y` 
- the object from which the null pointer must have been loaded. Similarly, 
unwrapping lvalue `x->y` would result in `x`.

I believe i found a more correct way to implement it, namely to see where 
lvalue-to-rvalue casts are located in the expression. In our example, `x->y` is 
surrounded by an lvalue-to-rvalue cast, which indicates that we should not 
unwrap the expression further. And it is irrelevant whether the member 
expression is a dot or an arrow, or whether C++ `this->` or ObjC `self->` is 
written explicitly or assumed implicitly, or whether the expression or a 
sub-expression is a pointer or a reference (we used to look at these).

This patch refactors `getDerefExpr()` with this design in mind. Now the 
function must be much easier to understand, and also behave correctly.

Unwrapping of binary operators that caused the dereference (eg. `*x = 2` -> 
`*x`) was removed from `getDerefExpr()` because it contradicts its purpose and 
seems to have never actually been used (we should be receiving `*x` in this 
function instead in all cases).

Current implementation has the benefit of not crashing on the newly added test 
case. The crash was caused by the fact that the old `getDerefExpr()` was 
thinking that `self` was dereferenced, even though in fact it wasn't.

I should probably have a look at what else might have changed and add more test 
cases, because the old code was quite strange.


https://reviews.llvm.org/D37023

Files:
  lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  test/Analysis/null-deref-path-notes.m

Index: test/Analysis/null-deref-path-notes.m
===
--- test/Analysis/null-deref-path-notes.m
+++ test/Analysis/null-deref-path-notes.m
@@ -50,6 +50,23 @@
   *p = 1; // expected-warning{{Dereference of null pointer}} expected-note{{Dereference of null pointer}}
 }
 
+@interface WithArrayPtr
+- (void) useArray;
+@end
+
+@implementation WithArrayPtr {
+@public int *p;
+}
+- (void)useArray {
+  p[1] = 2; // expected-warning{{Array access (via ivar 'p') results in a null pointer dereference}}
+// expected-note@-1{{Array access (via ivar 'p') results in a null pointer dereference}}
+}
+@end
+
+void testWithArrayPtr(WithArrayPtr *w) {
+  w->p = 0; // expected-note{{Null pointer value stored to 'p'}}
+  [w useArray]; // expected-note{{Calling 'useArray'}}
+}
 
 // CHECK:  diagnostics
 // CHECK-NEXT:  
@@ -801,4 +818,227 @@
 // CHECK-NEXT:file0
 // CHECK-NEXT:   
 // CHECK-NEXT:   
+// CHECK-NEXT:   
+// CHECK-NEXT:path
+// CHECK-NEXT:
+// CHECK-NEXT: 
+// CHECK-NEXT:  kindevent
+// CHECK-NEXT:  location
+// CHECK-NEXT:  
+// CHECK-NEXT:   line67
+// CHECK-NEXT:   col3
+// CHECK-NEXT:   file0
+// CHECK-NEXT:  
+// CHECK-NEXT:  ranges
+// CHECK-NEXT:  
+// CHECK-NEXT:
+// CHECK-NEXT: 
+// CHECK-NEXT:  line67
+// CHECK-NEXT:  col3
+// CHECK-NEXT:  file0
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT:  line67
+// CHECK-NEXT:  col10
+// CHECK-NEXT:  file0
+// CHECK-NEXT: 
+// CHECK-NEXT:
+// CHECK-NEXT:  
+// CHECK-NEXT:  depth0
+// CHECK-NEXT:  extended_message
+// CHECK-NEXT:  Null pointer value stored to p
+// CHECK-NEXT:  message
+// CHECK-NEXT:  Null pointer value stored to p
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT:  kindcontrol
+// CHECK-NEXT:  edges
+// CHECK-NEXT:   
+// CHECK-NEXT:
+// CHECK-NEXT: start
+// CHECK-NEXT:  
+// CHECK-NEXT:   
+// CHECK-NEXT:line67
+// CHECK-NEXT:col3
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:   
+// CHECK-NEXT:line67
+// CHECK-NEXT:col3
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:  
+// CHECK-NEXT: end
+// CHECK-NEXT:  
+// CHECK-NEXT:   
+// CHECK-NEXT:line68
+// CHECK-NEXT:col3
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:   
+// CHECK-NEXT:line68
+// CHECK-NEXT:col3
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:  
+// CHECK-NEXT:
+// CHECK-NEXT:   
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT:  kindevent
+// CHECK-NEXT:  location
+// CHECK-NEXT:  
+// CHECK-NEXT:   line68
+// CHECK-NEXT:   col3
+// CHECK-NEXT:   file0
+// CHECK-NEXT:  
+// CHECK-NEXT:  ranges
+// CHECK-NEXT:  
+// CHECK-NEXT:  

[PATCH] D37014: [clang-tidy] Add a checker to remove useless intermediate variables before return statements with comparisons

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: docs/ReleaseNotes.rst:60
 
+- New `readability-useless-intermediate-var
+  
`_
 check

Please place new checks in alphabetical order.



Comment at: docs/ReleaseNotes.rst:63
+
+  This new checker detects useless intermediate variables before return
+  statements that return the result of a simple comparison. This checker also

JonasToth wrote:
> maybe "useless" should be replaced with "unnecessary". but thats only my 
> opinion.
Checker -> check.
Please enclose return (in statement context) in ``.
Same for other places.


Repository:
  rL LLVM

https://reviews.llvm.org/D37014



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36492: [time-report] Add preprocessor timer

[Matthias Braun via Phabricator via cfe-commits]

MatzeB added inline comments.



Comment at: lib/Lex/Preprocessor.cpp:746
 void Preprocessor::Lex(Token ) {
+  llvm::TimeRegion(PPOpts->getTimer());
+

modocache wrote:
> erik.pilkington wrote:
> > Doesn't this just start a timer and immediately end the timer? Shouldn't we 
> > do: `llvm::TimeRegion LexTime(PPOpts->getTimer())` so that the dtor gets 
> > called when this function returns and we track the time spent in this 
> > function?
> > 
> > Also: this is a pretty hot function, and it looks like TimeRegion does some 
> > non-trivial work if time is being tracked. Have you tried testing this on a 
> > big c++ file with and without this patch and seeing what the difference in 
> > compile time looks like?
> Ah, yes you're right! Sorry about that. Actually keeping the timer alive for 
> the duration of the method also reveals that the method is called 
> recursively, which causes an assert, because timers can't be started twice.
> 
> Another spot in Clang works around this with a "reference counted" timer: 
> https://github.com/llvm-mirror/clang/blob/6ac9c51ede0a50cca13dd4ac03562c036f7a3f48/lib/CodeGen/CodeGenAction.cpp#L130-L134.
>  I have a more generic version of this "reference counting timer" that I've 
> been using for some of the other timers I've been adding; maybe I'll use it 
> here as well.
FWIF: I share Eriks concerns about compiletime. Timers are enabled in optimized 
builds, and querying them is not free. So putting one into a function that is 
called a lot and is time critical seems like a bad idea (do benchmarking to 
prove or disprove this!).


https://reviews.llvm.org/D36492



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36951: [OpenCL][5.0.0 Release] Release notes for OpenCL in Clang

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia closed this revision.
Anastasia added a comment.

Committed to release_50@311485


https://reviews.llvm.org/D36951



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36503: [libcxx] [test] Update for C++17 feature removals.

[Stephan T. Lavavej via Phabricator via cfe-commits]

STL_MSFT added a comment.

It's been two weeks - if there are no objections, I'd like to commit this soon. 
Thanks!


https://reviews.llvm.org/D36503



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r311397 - [Driver][Darwin] Do not pass -munwind-table if -fno-excpetions is

[Akira Hatanaka via cfe-commits]

Hans, can this be merged to 5.0 too? This is a follow-up to r310006.

Thanks.

> On Aug 21, 2017, at 3:46 PM, Akira Hatanaka via cfe-commits 
>  wrote:
> 
> Author: ahatanak
> Date: Mon Aug 21 15:46:46 2017
> New Revision: 311397
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=311397=rev
> Log:
> [Driver][Darwin] Do not pass -munwind-table if -fno-excpetions is
> supplied.
> 
> With this change, -fno-exceptions disables unwind tables unless
> -funwind-tables is supplied too or the target is x86-64 (x86-64 requires
> emitting unwind tables).
> 
> rdar://problem/33934446
> 
> Modified:
>cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
>cfe/trunk/test/Driver/clang-translation.c
> 
> Modified: cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Darwin.cpp?rev=311397=311396=311397=diff
> ==
> --- cfe/trunk/lib/Driver/ToolChains/Darwin.cpp (original)
> +++ cfe/trunk/lib/Driver/ToolChains/Darwin.cpp Mon Aug 21 15:46:46 2017
> @@ -1845,7 +1845,12 @@ Darwin::TranslateArgs(const DerivedArgLi
> }
> 
> bool MachO::IsUnwindTablesDefault(const ArgList ) const {
> -  return !UseSjLjExceptions(Args);
> +  // Unwind tables are not emitted if -fno-exceptions is supplied (except 
> when
> +  // targeting x86_64).
> +  return getArch() == llvm::Triple::x86_64 ||
> + (!UseSjLjExceptions(Args) &&
> +  Args.hasFlag(options::OPT_fexceptions, options::OPT_fno_exceptions,
> +   true));
> }
> 
> bool MachO::UseDwarfDebugFlags() const {
> 
> Modified: cfe/trunk/test/Driver/clang-translation.c
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/clang-translation.c?rev=311397=311396=311397=diff
> ==
> --- cfe/trunk/test/Driver/clang-translation.c (original)
> +++ cfe/trunk/test/Driver/clang-translation.c Mon Aug 21 15:46:46 2017
> @@ -73,6 +73,10 @@
> // RUN: FileCheck -check-prefix=ARM64-APPLE %s
> // ARM64-APPLE: -munwind-table
> 
> +// RUN: %clang -target arm64-apple-ios10 -fno-exceptions -### -S %s -arch 
> arm64 2>&1 | \
> +// RUN: FileCheck -check-prefix=ARM64-APPLE-EXCEP %s
> +// ARM64-APPLE-EXCEP-NOT: -munwind-table
> +
> // RUN: %clang -target armv7k-apple-watchos4.0 -### -S %s -arch armv7k 2>&1 | 
> \
> // RUN: FileCheck -check-prefix=ARMV7K-APPLE %s
> // ARMV7K-APPLE: -munwind-table
> 
> 
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35385: [Driver] Darwin: Link in the profile runtime archive first

[Juergen Ributzka via Phabricator via cfe-commits]

ributzka accepted this revision.
ributzka added a comment.
This revision is now accepted and ready to land.

Nice cleanup with the RuntimeLinkOptions enum. LGTM


https://reviews.llvm.org/D35385



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35955: clang-format: Add preprocessor directive indentation

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir added inline comments.



Comment at: lib/Format/ContinuationIndenter.cpp:383
+   State.Line->Type == LT_ImportStatement) &&
+  Current.Previous->is(tok::hash) && State.FirstIndent > 0) {
+Spaces += State.FirstIndent;

You can replace `Current.Previous` with `Previous`. Also, I'd swap the checks a 
bit, like:
```
  if (Previous.is(tok::hash) && State.FirstIndent > 0 &&
  Style.IndentPPDirectives == FormatStyle::PPDIS_AfterHash &&
  (State.Line->Type == LT_PreprocessorDirective ||
   State.Line->Type == LT_ImportStatement)) {
```
That way, the common case `Previous.is(tok::hash) == false` is handled quickly.



Comment at: lib/Format/ContinuationIndenter.cpp:387
+// hash. This causes second-level indents onward to have an extra space
+// after the tabs. We set the state to column 0 to avoid this misalignment.
+if (Style.UseTab != FormatStyle::UT_Never)

I don't understand this comment. Could you please give an example?



Comment at: lib/Format/UnwrappedLineParser.cpp:701
+  bool MaybeIncludeGuard = IfNDef;
+  for (auto& Line : Lines) {
+if (!Line.Tokens.front().Tok->is(tok::comment)) {

This can easily lead to a pretty bad runtime: consider a file starting with a 
few hundred lines of comments and having a few hundred `#ifdef`-s.

I'd say that after we do this MaybeIncludeGuard thing once, we don't repeat it 
again.

Also, lines could start with a block comment and continue with code:
```
/* small */ int ten = 10;
```



Comment at: lib/Format/UnwrappedLineParser.cpp:732
+  // then we count it as a real include guard and subtract one from every
+  // preprocessor indent.
+  unsigned TokenPosition = Tokens->getPosition();

Why do we need to subtract one from every preprocessor indent?



Comment at: lib/Format/UnwrappedLineParser.cpp:737
+for (auto& Line : Lines) {
+  if (Line.InPPDirective && Line.Level > 0)
+--Line.Level;

Wouldn't this also indent lines continuing macro definitions, as in:
```
#define A(x) int f(int x) { \
  return x; \
}
```



Comment at: lib/Format/UnwrappedLineParser.cpp:760
+  }
+  PPMaybeIncludeGuard = nullptr;
   nextToken();

Why do we reset `PPMaybeIncludeGuard` here?



Comment at: lib/Format/UnwrappedLineParser.cpp:770
   addUnwrappedLine();
-  Line->Level = 1;
+  ++Line->Level;
 

Why do we `++Line->Level` here?



Comment at: lib/Format/UnwrappedLineParser.cpp:787
   addUnwrappedLine();
+  PPMaybeIncludeGuard = nullptr;
 }

Why do we reset `PPMaybeIncludeGuard` here?



Comment at: lib/Format/UnwrappedLineParser.h:249
 
+  FormatToken *PPMaybeIncludeGuard;
+  bool FoundIncludeGuardStart;

Please add a comment for `PPMaybeIncludeGuard`: is it expected to point to the 
hash token of the `#ifdef`, or?


https://reviews.llvm.org/D35955



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35271: Fix printing policy for AST context loaded from file

[Vedant Kumar via Phabricator via cfe-commits]

vsk added a comment.

Thanks for adding the test! This is looking good.




Comment at: unittests/Frontend/ASTUnitTest.cpp:32
+llvm::SmallString<256> Dir;
+ASSERT_FALSE(llvm::sys::fs::createUniqueDirectory("astunit-test", Dir));
+TestDir = Dir.str();

If a directory isn't necessary to exercise the desired functionality, it'd be 
better to use createTemporaryFile() to open a temporary file descriptor, and 
then to use tool_output_file() to manage the descriptor's lifetime. 
Tool_output_file has logic to clean up files when a signal is received, etc.



Comment at: unittests/Frontend/ASTUnitTest.cpp:48
+TemporaryFiles.insert(Filename);
+llvm::sys::fs::create_directories(llvm::sys::path::parent_path(Filename));
+  }

It doesn't look like all of the directories which could be created here would 
be deleted? With any luck it'll be possible to use tool_output_file() to side 
step the issue.


https://reviews.llvm.org/D35271



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36678: [OpenCL] Do not use vararg in emitted functions for enqueue_kernel

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: test/CodeGenOpenCL/cl20-device-side-enqueue.cl:116
+  // B32: store i32 4, i32* %[[TMP3]], align 4
+  // B32: call i32 @__enqueue_kernel_vaargs(%opencl.queue_t{{.*}}* [[DEF_Q]], 
i32 [[FLAGS]], %struct.ndrange_t* [[NDR]]{{(.[0-9]+)?}}, i8 addrspace(4)* 
addrspacecast (i8 addrspace(1)* bitcast ({ i8**, i32, i32, i8*, 
%struct.__block_descriptor addrspace(2)* } addrspace(1)* 
@__block_literal_global{{(.[0-9]+)?}} to i8 addrspace(1)*) to i8 
addrspace(4)*), i32 3, i32* %[[TMP1]])
+  // B64: %[[TMP:.*]] = alloca [3 x i64]

You are not checking the arrays in the other calls too?


https://reviews.llvm.org/D36678



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r309226 - Headers: improve ARM EHABI coverage of unwind.h

[Hans Wennborg via cfe-commits]

Is there a bug filed? Since this was merged to 5.0.0, I'd like to know
if we broke something and if that is something that needs to be fixed.

On Tue, Aug 22, 2017 at 10:46 AM, Evgenii Stepanov
 wrote:
> As I understand, using compiler-rt as libgcc replacement on ARM is
> currently broken because of this change, but I have not looked since
> my last message.
>
> On Mon, Aug 21, 2017 at 4:56 PM, Hans Wennborg  wrote:
>> Is there something we need for 5.0.0 here?
>>
>> On Sat, Aug 12, 2017 at 9:58 PM, Saleem Abdulrasool
>>  wrote:
>>> Yeah, we should adjust that.  Technically, it is `_Unwind_Control_Block on
>>> ARM EHABI.  However, _Unwind_Exception is aliased to that, which is why we
>>> can use that in the personality routine.  We should adjust the sources for
>>> the personality routine.
>>>
>>> On Fri, Aug 11, 2017 at 1:12 PM, Evgenii Stepanov
>>>  wrote:

 Hi,

 I've noticed that the code in
 compiler-rt/lib/builtins/gcc_personality_v0.c refers to
 _Unwind_Exception as "struct _Unwind_Exception". With this change, it
 is not a struct anymore on ARM. Should that code be fixed, or is it a
 problem in this change?

 compiler-rt/lib/builtins/gcc_personality_v0.c:153:23: error:
 declaration of 'struct _Unwind_Exception' will not be visible outside
 of this function [-Werror,-Wvisibility]
 continueUnwind(struct _Unwind_Exception *exceptionObject,

 On Thu, Jul 27, 2017 at 9:46 AM, Hans Wennborg via cfe-commits
  wrote:
 > Merged to 5.0 in r309290.
 >
 > On Wed, Jul 26, 2017 at 3:55 PM, Saleem Abdulrasool via cfe-commits
 >  wrote:
 >> Author: compnerd
 >> Date: Wed Jul 26 15:55:23 2017
 >> New Revision: 309226
 >>
 >> URL: http://llvm.org/viewvc/llvm-project?rev=309226=rev
 >> Log:
 >> Headers: improve ARM EHABI coverage of unwind.h
 >>
 >> Ensure that we define the `_Unwind_Control_Block` structure used on ARM
 >> EHABI targets.  This is needed for building libc++abi with the unwind.h
 >> from the resource dir.  A minor fallout of this is that we needed to
 >> create a typedef for _Unwind_Exception to work across ARM EHABI and
 >> non-EHABI targets.  The structure definitions here are based originally
 >> on the documentation from ARM under the "Exception Handling ABI for the
 >> ARM® Architecture" Section 7.2.  They are then adjusted to more closely
 >> reflect the definition in libunwind from LLVM.  Those changes are
 >> compatible in layout but permit easier use in libc++abi and help
 >> maintain compatibility between libunwind and the compiler provided
 >> definition.
 >>
 >> Modified:
 >> cfe/trunk/lib/Headers/unwind.h
 >>
 >> Modified: cfe/trunk/lib/Headers/unwind.h
 >> URL:
 >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/unwind.h?rev=309226=309225=309226=diff
 >>
 >> ==
 >> --- cfe/trunk/lib/Headers/unwind.h (original)
 >> +++ cfe/trunk/lib/Headers/unwind.h Wed Jul 26 15:55:23 2017
 >> @@ -76,7 +76,13 @@ typedef intptr_t _sleb128_t;
 >>  typedef uintptr_t _uleb128_t;
 >>
 >>  struct _Unwind_Context;
 >> +#if defined(__arm__) && !(defined(__USING_SJLJ_EXCEPTIONS__) ||
 >> defined(__ARM_DWARF_EH___))
 >> +struct _Unwind_Control_Block;
 >> +typedef struct _Unwind_Control_Block _Unwind_Exception; /* Alias */
 >> +#else
 >>  struct _Unwind_Exception;
 >> +typedef struct _Unwind_Exception _Unwind_Exception;
 >> +#endif
 >>  typedef enum {
 >>_URC_NO_REASON = 0,
 >>  #if defined(__arm__) && !defined(__USING_SJLJ_EXCEPTIONS__) && \
 >> @@ -109,8 +115,42 @@ typedef enum {
 >>  } _Unwind_Action;
 >>
 >>  typedef void (*_Unwind_Exception_Cleanup_Fn)(_Unwind_Reason_Code,
 >> - struct _Unwind_Exception
 >> *);
 >> + _Unwind_Exception *);
 >>
 >> +#if defined(__arm__) && !(defined(__USING_SJLJ_EXCEPTIONS__) ||
 >> defined(__ARM_DWARF_EH___))
 >> +typedef struct _Unwind_Control_Block _Unwind_Control_Block;
 >> +typedef uint32_t _Unwind_EHT_Header;
 >> +
 >> +struct _Unwind_Control_Block {
 >> +  uint64_t exception_class;
 >> +  void (*exception_cleanup)(_Unwind_Reason_Code, _Unwind_Control_Block
 >> *);
 >> +  /* unwinder cache (private fields for the unwinder's use) */
 >> +  struct {
 >> +uint32_t reserved1; /* forced unwind stop function, 0 if not
 >> forced */
 >> +uint32_t reserved2; /* personality routine */
 >> +uint32_t reserved3; /* callsite */
 >> +uint32_t reserved4; /* forced unwind stop argument */
 

[PATCH] D36853: [Parser] Correct initalizer typos before lambda capture type is deduced.

[Volodymyr Sapsai via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL311480: [Parser] Correct initalizer typos before lambda 
capture type is deduced. (authored by vsapsai).

Changed prior to commit:
  https://reviews.llvm.org/D36853?vs=111716=112201#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D36853

Files:
  cfe/trunk/lib/Parse/ParseExprCXX.cpp
  cfe/trunk/test/SemaCXX/cxx1y-init-captures.cpp


Index: cfe/trunk/test/SemaCXX/cxx1y-init-captures.cpp
===
--- cfe/trunk/test/SemaCXX/cxx1y-init-captures.cpp
+++ cfe/trunk/test/SemaCXX/cxx1y-init-captures.cpp
@@ -206,3 +206,11 @@
   find(weight); // expected-note {{in instantiation of function template 
specialization}}
 }
 }
+
+namespace init_capture_undeclared_identifier {
+  auto a = [x = y]{}; // expected-error{{use of undeclared identifier 'y'}}
+
+  int typo_foo; // expected-note 2 {{'typo_foo' declared here}}
+  auto b = [x = typo_boo]{}; // expected-error{{use of undeclared identifier 
'typo_boo'; did you mean 'typo_foo'}}
+  auto c = [x(typo_boo)]{}; // expected-error{{use of undeclared identifier 
'typo_boo'; did you mean 'typo_foo'}}
+}
Index: cfe/trunk/lib/Parse/ParseExprCXX.cpp
===
--- cfe/trunk/lib/Parse/ParseExprCXX.cpp
+++ cfe/trunk/lib/Parse/ParseExprCXX.cpp
@@ -966,6 +966,8 @@
 // that would be an error.
 
 ParsedType InitCaptureType;
+if (!Init.isInvalid())
+  Init = Actions.CorrectDelayedTyposInExpr(Init.get());
 if (Init.isUsable()) {
   // Get the pointer and store it in an lvalue, so we can use it as an
   // out argument.


Index: cfe/trunk/test/SemaCXX/cxx1y-init-captures.cpp
===
--- cfe/trunk/test/SemaCXX/cxx1y-init-captures.cpp
+++ cfe/trunk/test/SemaCXX/cxx1y-init-captures.cpp
@@ -206,3 +206,11 @@
   find(weight); // expected-note {{in instantiation of function template specialization}}
 }
 }
+
+namespace init_capture_undeclared_identifier {
+  auto a = [x = y]{}; // expected-error{{use of undeclared identifier 'y'}}
+
+  int typo_foo; // expected-note 2 {{'typo_foo' declared here}}
+  auto b = [x = typo_boo]{}; // expected-error{{use of undeclared identifier 'typo_boo'; did you mean 'typo_foo'}}
+  auto c = [x(typo_boo)]{}; // expected-error{{use of undeclared identifier 'typo_boo'; did you mean 'typo_foo'}}
+}
Index: cfe/trunk/lib/Parse/ParseExprCXX.cpp
===
--- cfe/trunk/lib/Parse/ParseExprCXX.cpp
+++ cfe/trunk/lib/Parse/ParseExprCXX.cpp
@@ -966,6 +966,8 @@
 // that would be an error.
 
 ParsedType InitCaptureType;
+if (!Init.isInvalid())
+  Init = Actions.CorrectDelayedTyposInExpr(Init.get());
 if (Init.isUsable()) {
   // Get the pointer and store it in an lvalue, so we can use it as an
   // out argument.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r311480 - [Parser] Correct initalizer typos before lambda capture type is deduced.

[Volodymyr Sapsai via cfe-commits]

Author: vsapsai
Date: Tue Aug 22 10:55:19 2017
New Revision: 311480

URL: http://llvm.org/viewvc/llvm-project?rev=311480=rev
Log:
[Parser] Correct initalizer typos before lambda capture type is deduced.

This is the same assertion as in https://reviews.llvm.org/D25206 that is
triggered when RecordLayoutBuilder tries to compute the size of a field
(for capture "typo_boo" in the test case) whose type hasn't been
deduced.

The fix is to add CorrectDelayedTyposInExpr call to the cases when we
aren't disambiguating between an Obj-C message send and a lambda
expression.

rdar://problem/31760839

Reviewers: rsmith, ahatanak

Reviewed By: arphaman

Subscribers: cfe-commits

Differential Revision: https://reviews.llvm.org/D36853

Modified:
cfe/trunk/lib/Parse/ParseExprCXX.cpp
cfe/trunk/test/SemaCXX/cxx1y-init-captures.cpp

Modified: cfe/trunk/lib/Parse/ParseExprCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseExprCXX.cpp?rev=311480=311479=311480=diff
==
--- cfe/trunk/lib/Parse/ParseExprCXX.cpp (original)
+++ cfe/trunk/lib/Parse/ParseExprCXX.cpp Tue Aug 22 10:55:19 2017
@@ -966,6 +966,8 @@ Optional Parser::ParseLambdaIn
 // that would be an error.
 
 ParsedType InitCaptureType;
+if (!Init.isInvalid())
+  Init = Actions.CorrectDelayedTyposInExpr(Init.get());
 if (Init.isUsable()) {
   // Get the pointer and store it in an lvalue, so we can use it as an
   // out argument.

Modified: cfe/trunk/test/SemaCXX/cxx1y-init-captures.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/cxx1y-init-captures.cpp?rev=311480=311479=311480=diff
==
--- cfe/trunk/test/SemaCXX/cxx1y-init-captures.cpp (original)
+++ cfe/trunk/test/SemaCXX/cxx1y-init-captures.cpp Tue Aug 22 10:55:19 2017
@@ -206,3 +206,11 @@ void test(double weight) {
   find(weight); // expected-note {{in instantiation of function template 
specialization}}
 }
 }
+
+namespace init_capture_undeclared_identifier {
+  auto a = [x = y]{}; // expected-error{{use of undeclared identifier 'y'}}
+
+  int typo_foo; // expected-note 2 {{'typo_foo' declared here}}
+  auto b = [x = typo_boo]{}; // expected-error{{use of undeclared identifier 
'typo_boo'; did you mean 'typo_foo'}}
+  auto c = [x(typo_boo)]{}; // expected-error{{use of undeclared identifier 
'typo_boo'; did you mean 'typo_foo'}}
+}


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r311479 - [OPENMP] Fix for PR34014: OpenMP 4.5: Target construct in static method

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Tue Aug 22 10:54:52 2017
New Revision: 311479

URL: http://llvm.org/viewvc/llvm-project?rev=311479=rev
Log:
[OPENMP] Fix for PR34014: OpenMP 4.5: Target construct in static method
of class fails to map class static variable.

If the global variable is captured and it has several redeclarations,
sometimes it may lead to a compiler crash. Patch fixes this by working
only with canonical declarations.

Modified:
cfe/trunk/lib/CodeGen/CGExpr.cpp
cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp
cfe/trunk/lib/CodeGen/CodeGenFunction.h
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/test/OpenMP/target_map_codegen.cpp

Modified: cfe/trunk/lib/CodeGen/CGExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExpr.cpp?rev=311479=311478=311479=diff
==
--- cfe/trunk/lib/CodeGen/CGExpr.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExpr.cpp Tue Aug 22 10:54:52 2017
@@ -2300,6 +2300,7 @@ LValue CodeGenFunction::EmitDeclRefLValu
 
 // Check for captured variables.
 if (E->refersToEnclosingVariableOrCapture()) {
+  VD = VD->getCanonicalDecl();
   if (auto *FD = LambdaCaptureFields.lookup(VD))
 return EmitCapturedFieldLValue(*this, FD, CXXABIThisValue);
   else if (CapturedStmtInfo) {

Modified: cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp?rev=311479=311478=311479=diff
==
--- cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp Tue Aug 22 10:54:52 2017
@@ -65,6 +65,8 @@ public:
 for (auto  : CS->captures()) {
   if (C.capturesVariable() || C.capturesVariableByCopy()) {
 auto *VD = C.getCapturedVar();
+assert(VD == VD->getCanonicalDecl() &&
+"Canonical decl must be captured.");
 DeclRefExpr DRE(const_cast(VD),
 isCapturedVar(CGF, VD) ||
 (CGF.CapturedStmtInfo &&

Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.h?rev=311479=311478=311479=diff
==
--- cfe/trunk/lib/CodeGen/CodeGenFunction.h (original)
+++ cfe/trunk/lib/CodeGen/CodeGenFunction.h Tue Aug 22 10:54:52 2017
@@ -709,6 +709,7 @@ public:
llvm::function_ref PrivateGen) {
   assert(PerformCleanup && "adding private to dead scope");
 
+  LocalVD = LocalVD->getCanonicalDecl();
   // Only save it once.
   if (SavedLocals.count(LocalVD)) return false;
 
@@ -761,6 +762,7 @@ public:
 
 /// Checks if the global variable is captured in current function. 
 bool isGlobalVarCaptured(const VarDecl *VD) const {
+  VD = VD->getCanonicalDecl();
   return !VD->isLocalVarDeclOrParm() && CGF.LocalDeclMap.count(VD) > 0;
 }
 

Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=311479=311478=311479=diff
==
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Tue Aug 22 10:54:52 2017
@@ -14200,6 +14200,7 @@ bool Sema::tryCaptureVariable(
   bool IsGlobal = !Var->hasLocalStorage();
   if (IsGlobal && !(LangOpts.OpenMP && IsOpenMPCapturedDecl(Var)))
 return true;
+  Var = Var->getCanonicalDecl();
 
   // Walk up the stack to determine whether we can capture the variable,
   // performing the "simple" checks that don't depend on type. We stop when

Modified: cfe/trunk/test/OpenMP/target_map_codegen.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/target_map_codegen.cpp?rev=311479=311478=311479=diff
==
--- cfe/trunk/test/OpenMP/target_map_codegen.cpp (original)
+++ cfe/trunk/test/OpenMP/target_map_codegen.cpp Tue Aug 22 10:54:52 2017
@@ -15,12 +15,30 @@
 // RUN: %clang_cc1 -fopenmp -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple 
i386-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | 
FileCheck %s  --check-prefix CK1 --check-prefix CK1-32
 #ifdef CK1
 
+class B {
+public:
+  static double VAR;
+  B() {
+  }
+
+  static void modify(int ) {
+#pragma omp target map(tofrom \
+   : res)
+{
+  res = B::VAR;
+}
+  }
+};
+double B::VAR = 1.0;
+
 // CK1-DAG: [[SIZES:@.+]] = {{.+}}constant [1 x i[[sz:64|32]]] [i{{64|32}} 4]
 // Map types: OMP_MAP_PRIVATE_VAL | OMP_MAP_IS_FIRST = 288
 // CK1-DAG: [[TYPES:@.+]] = {{.+}}constant [1 x i32] [i32 288]
 
 // CK1-LABEL: implicit_maps_integer
 void implicit_maps_integer (int a){
+  // CK1: call void{{.*}}modify
+  B::modify(a);
   int i = a;
 
   // CK1-DAG: 

Re: r309226 - Headers: improve ARM EHABI coverage of unwind.h

[Evgenii Stepanov via cfe-commits]

As I understand, using compiler-rt as libgcc replacement on ARM is
currently broken because of this change, but I have not looked since
my last message.

On Mon, Aug 21, 2017 at 4:56 PM, Hans Wennborg  wrote:
> Is there something we need for 5.0.0 here?
>
> On Sat, Aug 12, 2017 at 9:58 PM, Saleem Abdulrasool
>  wrote:
>> Yeah, we should adjust that.  Technically, it is `_Unwind_Control_Block on
>> ARM EHABI.  However, _Unwind_Exception is aliased to that, which is why we
>> can use that in the personality routine.  We should adjust the sources for
>> the personality routine.
>>
>> On Fri, Aug 11, 2017 at 1:12 PM, Evgenii Stepanov
>>  wrote:
>>>
>>> Hi,
>>>
>>> I've noticed that the code in
>>> compiler-rt/lib/builtins/gcc_personality_v0.c refers to
>>> _Unwind_Exception as "struct _Unwind_Exception". With this change, it
>>> is not a struct anymore on ARM. Should that code be fixed, or is it a
>>> problem in this change?
>>>
>>> compiler-rt/lib/builtins/gcc_personality_v0.c:153:23: error:
>>> declaration of 'struct _Unwind_Exception' will not be visible outside
>>> of this function [-Werror,-Wvisibility]
>>> continueUnwind(struct _Unwind_Exception *exceptionObject,
>>>
>>> On Thu, Jul 27, 2017 at 9:46 AM, Hans Wennborg via cfe-commits
>>>  wrote:
>>> > Merged to 5.0 in r309290.
>>> >
>>> > On Wed, Jul 26, 2017 at 3:55 PM, Saleem Abdulrasool via cfe-commits
>>> >  wrote:
>>> >> Author: compnerd
>>> >> Date: Wed Jul 26 15:55:23 2017
>>> >> New Revision: 309226
>>> >>
>>> >> URL: http://llvm.org/viewvc/llvm-project?rev=309226=rev
>>> >> Log:
>>> >> Headers: improve ARM EHABI coverage of unwind.h
>>> >>
>>> >> Ensure that we define the `_Unwind_Control_Block` structure used on ARM
>>> >> EHABI targets.  This is needed for building libc++abi with the unwind.h
>>> >> from the resource dir.  A minor fallout of this is that we needed to
>>> >> create a typedef for _Unwind_Exception to work across ARM EHABI and
>>> >> non-EHABI targets.  The structure definitions here are based originally
>>> >> on the documentation from ARM under the "Exception Handling ABI for the
>>> >> ARM® Architecture" Section 7.2.  They are then adjusted to more closely
>>> >> reflect the definition in libunwind from LLVM.  Those changes are
>>> >> compatible in layout but permit easier use in libc++abi and help
>>> >> maintain compatibility between libunwind and the compiler provided
>>> >> definition.
>>> >>
>>> >> Modified:
>>> >> cfe/trunk/lib/Headers/unwind.h
>>> >>
>>> >> Modified: cfe/trunk/lib/Headers/unwind.h
>>> >> URL:
>>> >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/unwind.h?rev=309226=309225=309226=diff
>>> >>
>>> >> ==
>>> >> --- cfe/trunk/lib/Headers/unwind.h (original)
>>> >> +++ cfe/trunk/lib/Headers/unwind.h Wed Jul 26 15:55:23 2017
>>> >> @@ -76,7 +76,13 @@ typedef intptr_t _sleb128_t;
>>> >>  typedef uintptr_t _uleb128_t;
>>> >>
>>> >>  struct _Unwind_Context;
>>> >> +#if defined(__arm__) && !(defined(__USING_SJLJ_EXCEPTIONS__) ||
>>> >> defined(__ARM_DWARF_EH___))
>>> >> +struct _Unwind_Control_Block;
>>> >> +typedef struct _Unwind_Control_Block _Unwind_Exception; /* Alias */
>>> >> +#else
>>> >>  struct _Unwind_Exception;
>>> >> +typedef struct _Unwind_Exception _Unwind_Exception;
>>> >> +#endif
>>> >>  typedef enum {
>>> >>_URC_NO_REASON = 0,
>>> >>  #if defined(__arm__) && !defined(__USING_SJLJ_EXCEPTIONS__) && \
>>> >> @@ -109,8 +115,42 @@ typedef enum {
>>> >>  } _Unwind_Action;
>>> >>
>>> >>  typedef void (*_Unwind_Exception_Cleanup_Fn)(_Unwind_Reason_Code,
>>> >> - struct _Unwind_Exception
>>> >> *);
>>> >> + _Unwind_Exception *);
>>> >>
>>> >> +#if defined(__arm__) && !(defined(__USING_SJLJ_EXCEPTIONS__) ||
>>> >> defined(__ARM_DWARF_EH___))
>>> >> +typedef struct _Unwind_Control_Block _Unwind_Control_Block;
>>> >> +typedef uint32_t _Unwind_EHT_Header;
>>> >> +
>>> >> +struct _Unwind_Control_Block {
>>> >> +  uint64_t exception_class;
>>> >> +  void (*exception_cleanup)(_Unwind_Reason_Code, _Unwind_Control_Block
>>> >> *);
>>> >> +  /* unwinder cache (private fields for the unwinder's use) */
>>> >> +  struct {
>>> >> +uint32_t reserved1; /* forced unwind stop function, 0 if not
>>> >> forced */
>>> >> +uint32_t reserved2; /* personality routine */
>>> >> +uint32_t reserved3; /* callsite */
>>> >> +uint32_t reserved4; /* forced unwind stop argument */
>>> >> +uint32_t reserved5;
>>> >> +  } unwinder_cache;
>>> >> +  /* propagation barrier cache (valid after phase 1) */
>>> >> +  struct {
>>> >> +uint32_t sp;
>>> >> +uint32_t bitpattern[5];
>>> >> +  } barrier_cache;
>>> >> +  /* cleanup cache (preserved over cleanup) */
>>> >> +  struct {
>>> >> +uint32_t bitpattern[4];
>>> 

[PATCH] D36989: [clang-diff] Refactor stop-after command-line flag

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL311476: [clang-diff] Refactor stop-after command-line flag 
(authored by jgravelle).

Repository:
  rL LLVM

https://reviews.llvm.org/D36989

Files:
  cfe/trunk/test/Tooling/clang-diff-topdown.cpp
  cfe/trunk/tools/clang-diff/ClangDiff.cpp


Index: cfe/trunk/test/Tooling/clang-diff-topdown.cpp
===
--- cfe/trunk/test/Tooling/clang-diff-topdown.cpp
+++ cfe/trunk/test/Tooling/clang-diff-topdown.cpp
@@ -1,6 +1,6 @@
 // RUN: %clang_cc1 -E %s > %t.src.cpp
 // RUN: %clang_cc1 -E %s > %t.dst.cpp -DDEST
-// RUN: clang-diff -dump-matches -stop-after=topdown %t.src.cpp %t.dst.cpp -- 
-std=c++11 | FileCheck %s
+// RUN: clang-diff -dump-matches -stop-diff-after=topdown %t.src.cpp 
%t.dst.cpp -- -std=c++11 | FileCheck %s
 //
 // Test the top-down matching of identical subtrees only.
 
Index: cfe/trunk/tools/clang-diff/ClangDiff.cpp
===
--- cfe/trunk/tools/clang-diff/ClangDiff.cpp
+++ cfe/trunk/tools/clang-diff/ClangDiff.cpp
@@ -50,7 +50,7 @@
 cl::Optional,
 cl::cat(ClangDiffCategory));
 
-static cl::opt StopAfter("stop-after",
+static cl::opt StopAfter("stop-diff-after",
   cl::desc(""),
   cl::Optional, cl::init(""),
   cl::cat(ClangDiffCategory));


Index: cfe/trunk/test/Tooling/clang-diff-topdown.cpp
===
--- cfe/trunk/test/Tooling/clang-diff-topdown.cpp
+++ cfe/trunk/test/Tooling/clang-diff-topdown.cpp
@@ -1,6 +1,6 @@
 // RUN: %clang_cc1 -E %s > %t.src.cpp
 // RUN: %clang_cc1 -E %s > %t.dst.cpp -DDEST
-// RUN: clang-diff -dump-matches -stop-after=topdown %t.src.cpp %t.dst.cpp -- -std=c++11 | FileCheck %s
+// RUN: clang-diff -dump-matches -stop-diff-after=topdown %t.src.cpp %t.dst.cpp -- -std=c++11 | FileCheck %s
 //
 // Test the top-down matching of identical subtrees only.
 
Index: cfe/trunk/tools/clang-diff/ClangDiff.cpp
===
--- cfe/trunk/tools/clang-diff/ClangDiff.cpp
+++ cfe/trunk/tools/clang-diff/ClangDiff.cpp
@@ -50,7 +50,7 @@
 cl::Optional,
 cl::cat(ClangDiffCategory));
 
-static cl::opt StopAfter("stop-after",
+static cl::opt StopAfter("stop-diff-after",
   cl::desc(""),
   cl::Optional, cl::init(""),
   cl::cat(ClangDiffCategory));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r311476 - [clang-diff] Refactor stop-after command-line flag

[Jacob Gravelle via cfe-commits]

Author: jgravelle
Date: Tue Aug 22 10:42:44 2017
New Revision: 311476

URL: http://llvm.org/viewvc/llvm-project?rev=311476=rev
Log:
[clang-diff] Refactor stop-after command-line flag

Summary:
Rename stop-after to stop-diff-after. When building LLVM with
-DLLVM_BUILD_LLVM_DYLIB=ON, stop-after collides with the stop-after
already present in LLVM.

Reviewers: johannes, arphaman

Subscribers: klimek, aheejin, cfe-commits

Differential Revision: https://reviews.llvm.org/D36989

Modified:
cfe/trunk/test/Tooling/clang-diff-topdown.cpp
cfe/trunk/tools/clang-diff/ClangDiff.cpp

Modified: cfe/trunk/test/Tooling/clang-diff-topdown.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Tooling/clang-diff-topdown.cpp?rev=311476=311475=311476=diff
==
--- cfe/trunk/test/Tooling/clang-diff-topdown.cpp (original)
+++ cfe/trunk/test/Tooling/clang-diff-topdown.cpp Tue Aug 22 10:42:44 2017
@@ -1,6 +1,6 @@
 // RUN: %clang_cc1 -E %s > %t.src.cpp
 // RUN: %clang_cc1 -E %s > %t.dst.cpp -DDEST
-// RUN: clang-diff -dump-matches -stop-after=topdown %t.src.cpp %t.dst.cpp -- 
-std=c++11 | FileCheck %s
+// RUN: clang-diff -dump-matches -stop-diff-after=topdown %t.src.cpp 
%t.dst.cpp -- -std=c++11 | FileCheck %s
 //
 // Test the top-down matching of identical subtrees only.
 

Modified: cfe/trunk/tools/clang-diff/ClangDiff.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/clang-diff/ClangDiff.cpp?rev=311476=311475=311476=diff
==
--- cfe/trunk/tools/clang-diff/ClangDiff.cpp (original)
+++ cfe/trunk/tools/clang-diff/ClangDiff.cpp Tue Aug 22 10:42:44 2017
@@ -50,7 +50,7 @@ static cl::opt DestinationP
 cl::Optional,
 cl::cat(ClangDiffCategory));
 
-static cl::opt StopAfter("stop-after",
+static cl::opt StopAfter("stop-diff-after",
   cl::desc(""),
   cl::Optional, cl::init(""),
   cl::cat(ClangDiffCategory));


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36969: [Basic] Add a DiagnosticError llvm::ErrorInfo subclass

[Vedant Kumar via Phabricator via cfe-commits]

vsk added a comment.

Nice! I'd like to get your thoughts on two candidate ergonomic changes:




Comment at: unittests/Basic/DiagnosticTest.cpp:81
+  llvm::Expected> Value =
+  llvm::make_error(PartialDiagnosticAt(
+  SourceLocation(), PartialDiagnostic(diag::err_cannot_open_file, 
Alloc)

It might be useful to add an Error-returning wrapper around PDA, e.g:
`static Error PartialDiagnosticAt::get(...)`



Comment at: unittests/Basic/DiagnosticTest.cpp:90
+ErrDiag = std::move(Err.getDiagnostic());
+  });
+  EXPECT_EQ(ErrDiag.first, SourceLocation());

Creating a null diagnostic before taking an error may become cumbersome. An 
alternative is to add a static helper in DiagnosticError:

```
static ParitalDiagnosticAt ::take(Error E) {
  PartialDiagnosticAt *PDA = nullptr;
  handleAllErrors(std::move(E), [&](DE) { PDA = (); });
  assert(PDA && "Expected a DiagnosticError");
  return *PDA;
}
```

This is more ergonomic, but the downside is that you lose some flexibility 
(what happens if there are two kinds of errors?, etc).

Another alternative is to switch to a callback-driven style (though that might 
require a lot of refactoring).


Repository:
  rL LLVM

https://reviews.llvm.org/D36969



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r311474 - Test commit access.

[Volodymyr Sapsai via cfe-commits]

Author: vsapsai
Date: Tue Aug 22 10:39:25 2017
New Revision: 311474

URL: http://llvm.org/viewvc/llvm-project?rev=311474=rev
Log:
Test commit access.

Modified:
cfe/trunk/include/clang/Analysis/CloneDetection.h

Modified: cfe/trunk/include/clang/Analysis/CloneDetection.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/CloneDetection.h?rev=311474=311473=311474=diff
==
--- cfe/trunk/include/clang/Analysis/CloneDetection.h (original)
+++ cfe/trunk/include/clang/Analysis/CloneDetection.h Tue Aug 22 10:39:25 2017
@@ -7,7 +7,7 @@
 //
 
//===--===//
 ///
-/// /file
+/// \file
 /// This file defines classes for searching and anlyzing source code clones.
 ///
 
//===--===//


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35955: clang-format: Add preprocessor directive indentation

[Daniel Jasper via Phabricator via cfe-commits]

djasper added a comment.

Krasimir: Can you actually give this a round of review? I will also try to do 
so tomorrow.


https://reviews.llvm.org/D35955



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34275: [analyzer] Re-implemente current virtual calls checker in a path-sensitive way

[wangxin via Phabricator via cfe-commits]

wangxindsb updated this revision to Diff 112196.
wangxindsb added a comment.

- Highlight pure virtual even in non-pure-only mode;
- Add change to the header.


https://reviews.llvm.org/D34275

Files:
  lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
  test/Analysis/virtualcall.cpp
  test/Analysis/virtualcall.h

Index: test/Analysis/virtualcall.h
===
--- test/Analysis/virtualcall.h
+++ test/Analysis/virtualcall.h
@@ -1,36 +1,14 @@
-#ifdef AS_SYSTEM
-#pragma clang system_header
-
-namespace system {
-  class A {
-  public:
-A() {
-  foo(); // no-warning
-}
-
-virtual int foo();
-  };
-}
-
-#else
-
 namespace header {
-  class A {
+  class Z {
   public:
-A() {
+Z() {
   foo();
 #if !PUREONLY
-#if INTERPROCEDURAL
-  // expected-warning-re@-3 ^}}Call Path : fooCall to virtual function during construction will not dispatch to derived class}}
-#else
-  // expected-warning-re@-5 ^}}Call to virtual function during construction will not dispatch to derived class}}
-#endif
+	// expected-warning-re@-2 ^}}Call to virtual function during construction}}
+	// expected-note-re@-3 ^}}This constructor of an object of type 'Z' has not returned when the virtual method was called}}
+	// expected-note-re@-4 ^}}Call to virtual function during construction}}	
 #endif
-
 }
-
 virtual int foo();
   };
 }
-
-#endif
Index: test/Analysis/virtualcall.cpp
===
--- test/Analysis/virtualcall.cpp
+++ test/Analysis/virtualcall.cpp
@@ -1,98 +1,83 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=optin.cplusplus.VirtualCall -analyzer-store region -verify -std=c++11 %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=optin.cplusplus.VirtualCall -analyzer-store region -analyzer-config optin.cplusplus.VirtualCall:Interprocedural=true -DINTERPROCEDURAL=1 -verify -std=c++11 %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=optin.cplusplus.VirtualCall -analyzer-store region -analyzer-config optin.cplusplus.VirtualCall:PureOnly=true -DPUREONLY=1 -verify -std=c++11 %s
-
-/* When INTERPROCEDURAL is set, we expect diagnostics in all functions reachable
-   from a constructor or destructor. If it is not set, we expect diagnostics
-   only in the constructor or destructor.
-
-   When PUREONLY is set, we expect diagnostics only for calls to pure virtual
-   functions not to non-pure virtual functions.
-*/
+// RUN: %clang_analyze_cc1 -analyzer-checker=optin.cplusplus.VirtualCall -analyzer-store region -analyzer-output=text -verify -std=c++11 %s
+
+// RUN: %clang_analyze_cc1 -analyzer-checker=optin.cplusplus.VirtualCall -analyzer-store region -analyzer-config optin.cplusplus.VirtualCall:PureOnly=true -DPUREONLY=1 -analyzer-output=text -verify -std=c++11 %s
+
+#include "virtualcall.h"
 
 class A {
 public:
   A();
-  A(int i);
 
-  ~A() {};
-  
-  virtual int foo() = 0; // from Sema: expected-note {{'foo' declared here}}
+  ~A(){};
+
+  virtual int foo() = 0;
   virtual void bar() = 0;
   void f() {
 foo();
-#if INTERPROCEDURAL
-// expected-warning-re@-2 ^}}Call Path : foo <-- fCall to pure virtual function during construction has undefined behavior}}
-#endif
+	// expected-warning-re@-1 ^}}Call to pure virtual function during construction}}
+	// expected-note-re@-2 ^}}Call to pure virtual function during construction}}
   }
 };
 
 class B : public A {
 public:
-  B() {
-foo();
+  B() { // expected-note {{Calling default constructor for 'A'}}
+foo(); 
 #if !PUREONLY
-#if INTERPROCEDURAL
-// expected-warning-re@-3 ^}}Call Path : fooCall to virtual function during construction will not dispatch to derived class}}
-#else
-// expected-warning-re@-5 ^}}Call to virtual function during construction will not dispatch to derived class}}
+  	// expected-warning-re@-2 ^}}Call to virtual function during construction}}
+	// expected-note-re@-3 ^}}This constructor of an object of type 'B' has not returned when the virtual method was called}}
+  	// expected-note-re@-4 ^}}Call to virtual function during construction}}
 #endif
-#endif
-
   }
   ~B();
-  
+
   virtual int foo();
-  virtual void bar() { foo(); }
-#if INTERPROCEDURAL
-  // expected-warning-re@-2 ^}}Call Path : foo <-- barCall to virtual function during destruction will not dispatch to derived class}}
+  virtual void bar() {
+foo(); 
+#if !PUREONLY
+  	// expected-warning-re@-2 ^}}Call to virtual function during destruction}}
+  	// expected-note-re@-3 ^}}Call to virtual function during destruction}}
 #endif
+  } 
 };
 
-A::A() {
-  f();
-}
-
-A::A(int i) {
-  foo(); // From Sema: expected-warning {{call to pure virtual member function 'foo' has undefined behavior}}
-#if INTERPROCEDURAL
-  // expected-warning-re@-2 ^}}Call Path : fooCall to pure virtual function during construction has undefined behavior}}
-#else
-  // 

[PATCH] D36075: [refactor] Initial support for refactoring action rules

[Alex Lorenz via Phabricator via cfe-commits]

arphaman updated this revision to Diff 112195.
arphaman marked 7 inline comments as done.
arphaman added a comment.

- Split the header
- Remove DiagnosticOr in favour of Expected that will use a DiagnosticError 
that was proposed in the other patch.
- Address the other review comments


Repository:
  rL LLVM

https://reviews.llvm.org/D36075

Files:
  include/clang/Basic/LLVM.h
  include/clang/Tooling/Refactoring/AtomicChange.h
  include/clang/Tooling/Refactoring/EvaluateSourceSelectionConstraints.h
  include/clang/Tooling/Refactoring/RefactoringActionRule.h
  include/clang/Tooling/Refactoring/RefactoringActionRuleRequirements.h
  include/clang/Tooling/Refactoring/RefactoringActionRuleRequirementsInternal.h
  include/clang/Tooling/Refactoring/RefactoringActionRules.h
  include/clang/Tooling/Refactoring/RefactoringActionRulesInternal.h
  include/clang/Tooling/Refactoring/RefactoringOperation.h
  include/clang/Tooling/Refactoring/RefactoringResult.h
  include/clang/Tooling/Refactoring/SourceSelectionConstraints.h
  unittests/Tooling/CMakeLists.txt
  unittests/Tooling/RefactoringActionRulesTest.cpp

Index: unittests/Tooling/RefactoringActionRulesTest.cpp
===
--- /dev/null
+++ unittests/Tooling/RefactoringActionRulesTest.cpp
@@ -0,0 +1,157 @@
+//===- unittest/Tooling/RefactoringTestActionRulesTest.cpp ===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "ReplacementTest.h"
+#include "RewriterTestContext.h"
+#include "clang/Tooling/Refactoring.h"
+#include "clang/Tooling/Refactoring/RefactoringActionRules.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/Support/Errc.h"
+#include "gtest/gtest.h"
+
+using namespace clang;
+using namespace tooling;
+using namespace refactoring_action_rules;
+
+namespace {
+
+class RefactoringActionRulesTest : public ::testing::Test {
+protected:
+  void SetUp() override {
+Context.Sources.setMainFileID(
+Context.createInMemoryFile("input.cpp", DefaultCode));
+  }
+
+  RewriterTestContext Context;
+  std::string DefaultCode = std::string(100, 'a');
+};
+
+TEST_F(RefactoringActionRulesTest, MyFirstRefactoringRule) {
+  auto ReplaceAWithB =
+  [](std::pair Selection)
+  -> Expected {
+const SourceManager  = Selection.first.getSources();
+SourceLocation Loc = Selection.first.getRange().getBegin().getLocWithOffset(
+Selection.second);
+AtomicChange Change(SM, Loc);
+llvm::Error E = Change.replace(SM, Loc, 1, "b");
+if (E)
+  return std::move(E);
+return Change;
+  };
+  class SelectionRequirement : public selection::Requirement {
+  public:
+std::pair
+evaluateSelection(selection::SourceSelectionRange Selection) const {
+  return std::make_pair(Selection, 20);
+}
+  };
+  auto Rule = createRefactoringRule(ReplaceAWithB,
+requiredSelection(SelectionRequirement()));
+
+  // When the requirements are satisifed, the rule's function must be invoked.
+  {
+RefactoringOperation Operation(Context.Sources);
+SourceLocation Cursor =
+Context.Sources.getLocForStartOfFile(Context.Sources.getMainFileID())
+.getLocWithOffset(10);
+Operation.setSelectionRange({Cursor, Cursor});
+
+Expected ErrorOrResult =
+Rule->perform(Operation);
+ASSERT_FALSE(!ErrorOrResult);
+ASSERT_FALSE(!*ErrorOrResult);
+RefactoringResult Result = std::move(**ErrorOrResult);
+ASSERT_EQ(Result.getKind(), RefactoringResult::AtomicChanges);
+ASSERT_EQ(Result.getChanges().size(), 1u);
+std::string YAMLString =
+const_cast(Result.getChanges()[0]).toYAMLString();
+
+ASSERT_STREQ("---\n"
+ "Key: 'input.cpp:30'\n"
+ "FilePath:input.cpp\n"
+ "Error:   ''\n"
+ "InsertedHeaders: \n"
+ "RemovedHeaders:  \n"
+ "Replacements:\n" // Extra whitespace here!
+ "  - FilePath:input.cpp\n"
+ "Offset:  30\n"
+ "Length:  1\n"
+ "ReplacementText: b\n"
+ "...\n",
+ YAMLString.c_str());
+  }
+
+  // When one of the requirements is not satisfied, perform should return either
+  // None or a valid diagnostic.
+  {
+RefactoringOperation Operation(Context.Sources);
+Expected ErrorOrResult =
+Rule->perform(Operation);
+
+ASSERT_FALSE(!ErrorOrResult);
+Optional Value = std::move(*ErrorOrResult);
+EXPECT_TRUE(!Value);
+  }
+}
+
+TEST_F(RefactoringActionRulesTest, ReturnError) {
+  

[PATCH] D36075: [refactor] Initial support for refactoring action rules

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added inline comments.



Comment at: 
include/clang/Tooling/Refactoring/RefactoringOperationController.h:19
+
+/// Encapsulates all of the possible state that an individual refactoring
+/// operation might have. Controls the process of initiation of refactoring

ioeric wrote:
> What are all the possible states, for example?
Bad comment. I think `inputs` is more descriptive.



Comment at: include/clang/Tooling/Refactoring/RefactoringResult.h:27
+
+  RefactoringResult(AtomicChange Change) : Kind(AtomicChanges) {
+Changes.push_back(std::move(Change));

alexshap wrote:
> explicit ?
Nah, it's more convenient to be able to return a single `AtomicChanges` without 
an explicit initializer I think.



Comment at: include/clang/Tooling/Refactoring/RefactoringResult.h:35
+
+  llvm::MutableArrayRef getChanges() {
+assert(getKind() == AtomicChanges &&

ioeric wrote:
> Do we expect the result changes to be modified? Why?
No, that was a workaround for a non-const member use. I'll use a const cast 
instead.



Comment at: include/clang/Tooling/Refactoring/SourceSelectionConstraints.h:35
+/// A custom selection requirement.
+class Requirement {
+  /// Subclasses must implement 'T evaluateSelection(SelectionConstraint) 
const'

ioeric wrote:
> It might worth explaining the relationship between this and the 
> `RequirementBase`. 
This class is not related to `RequirementBase` though. Were you talking about 
another class?


Repository:
  rL LLVM

https://reviews.llvm.org/D36075



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D16808: [MCU] PR26438: Fix assertion failure on function returning an empty struct or union

[Alexander Kyte via Phabricator via cfe-commits]

alexanderkyte added a comment.

This change is causing mono's native interop with C code compiled from clang to 
break. After some investigation, we're tracked bitcode differences between 
tests that fail versus not. When we upgrade the compiler to Xcode 8.3, we see 
that code generated to call functions returning empty structs will now generate 
code that crashes.

This seems to be us trying to pass arguments assuming that clang exhibits the 
same behavior it used to. It is easy enough for us to generate code for what 
clang does now, but that code would break if forced to interact with C compiled 
by gcc and older clangs.

Is there a way for callers of clang-emitted code to detect which ABI clang is 
going to use when returning an empty struct? This ABI change does not seem to 
admit backwards compatibility.

xcode73's clang -emit-llvm generates:

- define void @ret(%struct.AStruct* noalias sret %agg.result, i32 %a) #0 {

while xcode83's clang generates:

- define void @ret(i32) #0 {


Repository:
  rL LLVM

https://reviews.llvm.org/D16808



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36989: [clang-diff] Refactor stop-after command-line flag

[Johannes Altmanninger via Phabricator via cfe-commits]

johannes added a comment.

In https://reviews.llvm.org/D36989#848835, @jgravelle-google wrote:

> If you have more stop-after options it'd probably be simpler to leave it as 
> one field. I'll just rename it to "stop-diff-after" to avoid the name 
> collision.


Ok, you can go ahead. Or, if I should do it let me know.


https://reviews.llvm.org/D36989



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36989: [clang-diff] Refactor stop-after command-line flag

[Jacob Gravelle via Phabricator via cfe-commits]

jgravelle-google updated this revision to Diff 112193.
jgravelle-google added a comment.

- Undo refactor, just change name


https://reviews.llvm.org/D36989

Files:
  test/Tooling/clang-diff-topdown.cpp
  tools/clang-diff/ClangDiff.cpp


Index: tools/clang-diff/ClangDiff.cpp
===
--- tools/clang-diff/ClangDiff.cpp
+++ tools/clang-diff/ClangDiff.cpp
@@ -50,7 +50,7 @@
 cl::Optional,
 cl::cat(ClangDiffCategory));
 
-static cl::opt StopAfter("stop-after",
+static cl::opt StopAfter("stop-diff-after",
   cl::desc(""),
   cl::Optional, cl::init(""),
   cl::cat(ClangDiffCategory));
Index: test/Tooling/clang-diff-topdown.cpp
===
--- test/Tooling/clang-diff-topdown.cpp
+++ test/Tooling/clang-diff-topdown.cpp
@@ -1,6 +1,6 @@
 // RUN: %clang_cc1 -E %s > %t.src.cpp
 // RUN: %clang_cc1 -E %s > %t.dst.cpp -DDEST
-// RUN: clang-diff -dump-matches -stop-after=topdown %t.src.cpp %t.dst.cpp -- 
-std=c++11 | FileCheck %s
+// RUN: clang-diff -dump-matches -stop-diff-after=topdown %t.src.cpp 
%t.dst.cpp -- -std=c++11 | FileCheck %s
 //
 // Test the top-down matching of identical subtrees only.
 


Index: tools/clang-diff/ClangDiff.cpp
===
--- tools/clang-diff/ClangDiff.cpp
+++ tools/clang-diff/ClangDiff.cpp
@@ -50,7 +50,7 @@
 cl::Optional,
 cl::cat(ClangDiffCategory));
 
-static cl::opt StopAfter("stop-after",
+static cl::opt StopAfter("stop-diff-after",
   cl::desc(""),
   cl::Optional, cl::init(""),
   cl::cat(ClangDiffCategory));
Index: test/Tooling/clang-diff-topdown.cpp
===
--- test/Tooling/clang-diff-topdown.cpp
+++ test/Tooling/clang-diff-topdown.cpp
@@ -1,6 +1,6 @@
 // RUN: %clang_cc1 -E %s > %t.src.cpp
 // RUN: %clang_cc1 -E %s > %t.dst.cpp -DDEST
-// RUN: clang-diff -dump-matches -stop-after=topdown %t.src.cpp %t.dst.cpp -- -std=c++11 | FileCheck %s
+// RUN: clang-diff -dump-matches -stop-diff-after=topdown %t.src.cpp %t.dst.cpp -- -std=c++11 | FileCheck %s
 //
 // Test the top-down matching of identical subtrees only.
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36989: [clang-diff] Refactor stop-after command-line flag

[Jacob Gravelle via Phabricator via cfe-commits]

jgravelle-google added a comment.

If you have more stop-after options it'd probably be simpler to leave it as one 
field. I'll just rename it to "stop-diff-after" to avoid the name collision.


https://reviews.llvm.org/D36989



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36848: [CodeGen] Use reentrant methods to time IR gen

[Brian Gesiak via Phabricator via cfe-commits]

modocache updated this revision to Diff 112187.
modocache retitled this revision from "[CodeGen] Use RefCntTimer to time IR 
generation" to "[CodeGen] Use reentrant methods to time IR gen".
modocache edited the summary of this revision.
modocache added a comment.

Use `startReentrantTimer` and `stopReentrantTimer`


https://reviews.llvm.org/D36848

Files:
  lib/CodeGen/CodeGenAction.cpp


Index: lib/CodeGen/CodeGenAction.cpp
===
--- lib/CodeGen/CodeGenAction.cpp
+++ lib/CodeGen/CodeGenAction.cpp
@@ -60,7 +60,6 @@
 ASTContext *Context;
 
 Timer LLVMIRGeneration;
-unsigned LLVMIRGenerationRefCount;
 
 /// True if we've finished generating IR. This prevents us from generating
 /// additional LLVM IR after emitting output in HandleTranslationUnit. This
@@ -90,7 +89,6 @@
   CodeGenOpts(CodeGenOpts), TargetOpts(TargetOpts), LangOpts(LangOpts),
   AsmOutStream(std::move(OS)), Context(nullptr),
   LLVMIRGeneration("irgen", "LLVM IR Generation Time"),
-  LLVMIRGenerationRefCount(0),
   Gen(CreateLLVMCodeGen(Diags, InFile, HeaderSearchOpts, PPOpts,
 CodeGenOpts, C, CoverageInfo)),
   LinkModules(std::move(LinkModules)) {
@@ -113,33 +111,27 @@
   Context = 
 
   if (llvm::TimePassesIsEnabled)
-LLVMIRGeneration.startTimer();
+LLVMIRGeneration.startReentrantTimer();
 
   Gen->Initialize(Ctx);
 
   if (llvm::TimePassesIsEnabled)
-LLVMIRGeneration.stopTimer();
+LLVMIRGeneration.stopReentrantTimer();
 }
 
 bool HandleTopLevelDecl(DeclGroupRef D) override {
   PrettyStackTraceDecl CrashInfo(*D.begin(), SourceLocation(),
  Context->getSourceManager(),
  "LLVM IR generation of declaration");
 
   // Recurse.
-  if (llvm::TimePassesIsEnabled) {
-LLVMIRGenerationRefCount += 1;
-if (LLVMIRGenerationRefCount == 1)
-  LLVMIRGeneration.startTimer();
-  }
+  if (llvm::TimePassesIsEnabled)
+LLVMIRGeneration.startReentrantTimer();
 
   Gen->HandleTopLevelDecl(D);
 
-  if (llvm::TimePassesIsEnabled) {
-LLVMIRGenerationRefCount -= 1;
-if (LLVMIRGenerationRefCount == 0)
-  LLVMIRGeneration.stopTimer();
-  }
+  if (llvm::TimePassesIsEnabled)
+LLVMIRGeneration.stopReentrantTimer();
 
   return true;
 }
@@ -149,12 +141,12 @@
  Context->getSourceManager(),
  "LLVM IR generation of inline function");
   if (llvm::TimePassesIsEnabled)
-LLVMIRGeneration.startTimer();
+LLVMIRGeneration.startReentrantTimer();
 
   Gen->HandleInlineFunctionDefinition(D);
 
   if (llvm::TimePassesIsEnabled)
-LLVMIRGeneration.stopTimer();
+LLVMIRGeneration.stopReentrantTimer();
 }
 
 void HandleInterestingDecl(DeclGroupRef D) override {
@@ -195,19 +187,13 @@
 void HandleTranslationUnit(ASTContext ) override {
   {
 PrettyStackTraceString CrashInfo("Per-file LLVM IR generation");
-if (llvm::TimePassesIsEnabled) {
-  LLVMIRGenerationRefCount += 1;
-  if (LLVMIRGenerationRefCount == 1)
-LLVMIRGeneration.startTimer();
-}
+if (llvm::TimePassesIsEnabled)
+  LLVMIRGeneration.startReentrantTimer();
 
 Gen->HandleTranslationUnit(C);
 
-if (llvm::TimePassesIsEnabled) {
-  LLVMIRGenerationRefCount -= 1;
-  if (LLVMIRGenerationRefCount == 0)
-LLVMIRGeneration.stopTimer();
-}
+if (llvm::TimePassesIsEnabled)
+  LLVMIRGeneration.stopReentrantTimer();
 
IRGenFinished = true;
   }


Index: lib/CodeGen/CodeGenAction.cpp
===
--- lib/CodeGen/CodeGenAction.cpp
+++ lib/CodeGen/CodeGenAction.cpp
@@ -60,7 +60,6 @@
 ASTContext *Context;
 
 Timer LLVMIRGeneration;
-unsigned LLVMIRGenerationRefCount;
 
 /// True if we've finished generating IR. This prevents us from generating
 /// additional LLVM IR after emitting output in HandleTranslationUnit. This
@@ -90,7 +89,6 @@
   CodeGenOpts(CodeGenOpts), TargetOpts(TargetOpts), LangOpts(LangOpts),
   AsmOutStream(std::move(OS)), Context(nullptr),
   LLVMIRGeneration("irgen", "LLVM IR Generation Time"),
-  LLVMIRGenerationRefCount(0),
   Gen(CreateLLVMCodeGen(Diags, InFile, HeaderSearchOpts, PPOpts,
 CodeGenOpts, C, CoverageInfo)),
   LinkModules(std::move(LinkModules)) {
@@ -113,33 +111,27 @@
   Context = 
 
   if (llvm::TimePassesIsEnabled)
-LLVMIRGeneration.startTimer();
+LLVMIRGeneration.startReentrantTimer();
 
   Gen->Initialize(Ctx);
 
   if (llvm::TimePassesIsEnabled)
-

[PATCH] D37014: [clang-tidy] Add a checker to remove useless intermediate variables before return statements with comparisons

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

Hi :)

I added my thoughts for the check. Many variables in your code could be 
`const`, i didn't mention all cases.




Comment at: clang-tidy/readability/UselessIntermediateVarCheck.cpp:24
+
+  auto directDeclRefExprLHS1 =
+// We match a direct declaration reference expression pointing

The matching expression can be `const auto`.
I would write the comment above the declaration, that the splitting does not 
occur (here and the next simpler matchers).

The local variables for matcher should have a capital letter as start -> 
s/directDeclRefExprLHS1/DirectDeclRefExpr/ here and elsewhere.



Comment at: clang-tidy/readability/UselessIntermediateVarCheck.cpp:62
+
+  auto hasVarDecl1 =
+// We match a single declaration which is a variable declaration,

here and elsewhere, is the indendation result of clang-format? Personally i 
like it, since it shows the structure, but there might be concerns since it 
probably does not match with the coding guideline.



Comment at: clang-tidy/readability/UselessIntermediateVarCheck.cpp:186
+const VarDecl *VarDecl1, const VarDecl *VarDecl2) {
+  diag(VarDecl1->getLocation(), "intermediate variable %0 is useless",
+   DiagnosticIDs::Warning)

The warning message sounds a little harsh/judging, and does not explain why the 
intermediate is useless. (the notes do which is ok i think).

Maybe go with `unnecessary intermediate variable %0`



Comment at: clang-tidy/readability/UselessIntermediateVarCheck.cpp:236
+  if (BinOpToReverse) {
+auto ReversedBinOpText =
+  BinaryOperator::getOpcodeStr(

`const auto` ?



Comment at: clang-tidy/readability/UselessIntermediateVarCheck.cpp:290
+// declaration.
+auto Init1TextOpt =
+  utils::lexer::getStmtText(Init1, Result.Context->getSourceManager());

some `const` is possible for this an the following variables.



Comment at: clang-tidy/readability/UselessIntermediateVarCheck.cpp:335
+  // operands of the binary operator to keep the same execution order.
+  auto LHSTextOpt =
+utils::lexer::getStmtText(BinOp->getLHS(), 
Result.Context->getSourceManager());

`const auto`?



Comment at: clang-tidy/readability/UselessIntermediateVarCheck.cpp:350
+// statement.
+bool HasVarRef1 = VarRefLHS1 || VarRefRHS1;
+bool HasVarRef2 = VarRefLHS2 || VarRefRHS2;

`const`



Comment at: clang-tidy/readability/UselessIntermediateVarCheck.h:29
+  : ClangTidyCheck(Name, Context),
+MaximumLineLength(Options.get("MaximumLineLength", 100)) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;

It would be nice, if there is a way to get this information from a clang-format 
file. 



Comment at: clang-tidy/readability/UselessIntermediateVarCheck.h:47
+  unsigned MaximumLineLength;
+  std::unordered_set CheckedDeclStmt;
+};

Maybe a SmallSet? the optimized datastructures from llvm would save dynamic 
memory allocations.



Comment at: clang-tidy/utils/Matchers.h:67
+  // We build a Control Flow Graph (CFG) from the parent statement.
+  std::unique_ptr StatementCFG
+  = CFG::buildCFG(nullptr, const_cast(Parent), >getASTContext(),

formatted? usually the `=` ends up on the same line



Comment at: docs/ReleaseNotes.rst:63
+
+  This new checker detects useless intermediate variables before return
+  statements that return the result of a simple comparison. This checker also

maybe "useless" should be replaced with "unnecessary". but thats only my 
opinion.



Comment at: docs/clang-tidy/checks/readability-useless-intermediate-var.rst:14
+
+  auto test = 1;
+  return (test == 2);

especially to avoid magic numbers, this kind of code could be more readable.



Comment at: test/clang-tidy/readability-useless-intermediate-var.cpp:1
+// RUN: %check_clang_tidy %s readability-useless-intermediate-var %t
+

the tests seem to be to less compared to the code volume of the check.

situations i think should be tested:

- initialization from a function, method call
- initialization with a lambda
```
const auto SomeVar = []() { /* super complicated stuff */ return Result; } ();
return SomeVar;
```
- template stuff -> proof that they work two, even thought it doesnt seem to be 
relevant, at least what i can see.
- what happens if a "temporary" is used as an argument for a function call?
```
const auto Var = std::sqrt(10);
return std::pow(Var, 10);
```
- proof that really long function calls (like STL Algorithm tends to be) are 
not inlined as well


https://reviews.llvm.org/D37014



___
cfe-commits 

Re: r311443 - [ObjC] Check written attributes only when synthesizing ambiguous property

[Hans Wennborg via cfe-commits]

Merged in r311464. Thanks!

On Tue, Aug 22, 2017 at 3:42 AM, Alex L  wrote:
> Hi Hans,
>
> Can you please merge this into LLVM 5? It fixes a rather serious Objective-C
> bug that I introduced just a couple of weeks before the branch.
>
> Cheers,
> Alex
>
>
> On 22 August 2017 at 11:38, Alex Lorenz via cfe-commits
>  wrote:
>>
>> Author: arphaman
>> Date: Tue Aug 22 03:38:07 2017
>> New Revision: 311443
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=311443=rev
>> Log:
>> [ObjC] Check written attributes only when synthesizing ambiguous property
>>
>> This commit fixes a bug introduced in r307903. The attribute ambiguity
>> checker
>> that was introduced in r307903 checked all property attributes, which
>> caused
>> errors for source-compatible properties, like:
>>
>> @property (nonatomic, readonly) NSObject *prop;
>> @property (nonatomic, readwrite) NSObject *prop;
>>
>> because the readwrite property would get implicit 'strong' attribute. The
>> ambiguity checker should be concerned about explicitly specified
>> attributes
>> only.
>>
>> rdar://33748089
>>
>> Modified:
>> cfe/trunk/lib/Sema/SemaObjCProperty.cpp
>> cfe/trunk/test/SemaObjC/arc-property-decl-attrs.m
>>
>> Modified: cfe/trunk/lib/Sema/SemaObjCProperty.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaObjCProperty.cpp?rev=311443=311442=311443=diff
>>
>> ==
>> --- cfe/trunk/lib/Sema/SemaObjCProperty.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaObjCProperty.cpp Tue Aug 22 03:38:07 2017
>> @@ -872,7 +872,7 @@ SelectPropertyForSynthesisFromProtocols(
>>}
>>
>>QualType RHSType = S.Context.getCanonicalType(Property->getType());
>> -  unsigned OriginalAttributes = Property->getPropertyAttributes();
>> +  unsigned OriginalAttributes =
>> Property->getPropertyAttributesAsWritten();
>>enum MismatchKind {
>>  IncompatibleType = 0,
>>  HasNoExpectedAttribute,
>> @@ -890,7 +890,7 @@ SelectPropertyForSynthesisFromProtocols(
>>SmallVector Mismatches;
>>for (ObjCPropertyDecl *Prop : Properties) {
>>  // Verify the property attributes.
>> -unsigned Attr = Prop->getPropertyAttributes();
>> +unsigned Attr = Prop->getPropertyAttributesAsWritten();
>>  if (Attr != OriginalAttributes) {
>>auto Diag = [&](bool OriginalHasAttribute, StringRef AttributeName)
>> {
>>  MismatchKind Kind = OriginalHasAttribute ? HasNoExpectedAttribute
>>
>> Modified: cfe/trunk/test/SemaObjC/arc-property-decl-attrs.m
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/arc-property-decl-attrs.m?rev=311443=311442=311443=diff
>>
>> ==
>> --- cfe/trunk/test/SemaObjC/arc-property-decl-attrs.m (original)
>> +++ cfe/trunk/test/SemaObjC/arc-property-decl-attrs.m Tue Aug 22 03:38:07
>> 2017
>> @@ -225,3 +225,30 @@ __attribute__((objc_root_class))
>>  @implementation TypeVsSetter
>>  @synthesize prop; // expected-note {{property synthesized here}}
>>  @end
>> +
>> +@protocol AutoStrongProp
>> +
>> +@property (nonatomic, readonly) NSObject *prop;
>> +
>> +@end
>> +
>> +@protocol AutoStrongProp_Internal 
>> +
>> +// This property gets the 'strong' attribute automatically.
>> +@property (nonatomic, readwrite) NSObject *prop;
>> +
>> +@end
>> +
>> +@interface SynthesizeWithImplicitStrongNoError : NSObject
>> 
>> +@end
>> +
>> +@interface SynthesizeWithImplicitStrongNoError ()
>> 
>> +
>> +@end
>> +
>> +@implementation SynthesizeWithImplicitStrongNoError
>> +
>> +// no error, 'strong' is implicit in the 'readwrite' property.
>> +@synthesize prop = _prop;
>> +
>> +@end
>>
>>
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37014: [clang-tidy] Add a checker to remove useless intermediate variables before return statements with comparisons

[Tristan Bourvon via Phabricator via cfe-commits]

tbourvon created this revision.
Herald added subscribers: xazax.hun, JDevlieghere, mgorny.

This patch adds a checker to detect patterns of the following form:

  auto IntermediateVar = foo();
  return (IntermediateVar == 1);

and suggests to turn them into:

  return (foo() == 1);

The reasoning behind this checker is that this kind of pattern is useless and 
lowers readability as long as the return statement remains rather short.
The idea of this checker was suggested to me by Sylvestre Ledru.


https://reviews.llvm.org/D37014

Files:
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tidy/readability/UselessIntermediateVarCheck.cpp
  clang-tidy/readability/UselessIntermediateVarCheck.h
  clang-tidy/utils/LexerUtils.cpp
  clang-tidy/utils/LexerUtils.h
  clang-tidy/utils/Matchers.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-useless-intermediate-var.rst
  test/clang-tidy/readability-useless-intermediate-var.cpp

Index: test/clang-tidy/readability-useless-intermediate-var.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-useless-intermediate-var.cpp
@@ -0,0 +1,138 @@
+// RUN: %check_clang_tidy %s readability-useless-intermediate-var %t
+
+bool f() {
+  auto test = 1; // Test
+  // CHECK-FIXES: {{^}}  // Test{{$}}
+  return (test == 1);
+  // CHECK-FIXES: {{^}}  return (1 == 1);{{$}}
+  // CHECK-MESSAGES: :[[@LINE-4]]:8: warning: intermediate variable 'test' is useless [readability-useless-intermediate-var]
+  // CHECK-MESSAGES: :[[@LINE-3]]:11: note: because it is only used when returning the result of this comparison
+  // CHECK-MESSAGES: :[[@LINE-6]]:8: note: consider removing the variable declaration
+  // CHECK-MESSAGES: :[[@LINE-5]]:11: note: and directly using the variable initialization expression here
+}
+
+bool f2() {
+  auto test1 = 1; // Test1
+  // CHECK-FIXES: {{^}}  // Test1{{$}}
+  auto test2 = 2; // Test2
+  // CHECK-FIXES: {{^}}  // Test2{{$}}
+  return (test1 == test2);
+  // CHECK-FIXES: {{^}}  return (1 == 2);{{$}}
+  // CHECK-MESSAGES: :[[@LINE-6]]:8: warning: intermediate variable 'test1' is useless [readability-useless-intermediate-var]
+  // CHECK-MESSAGES: :[[@LINE-5]]:8: warning: and so is 'test2' [readability-useless-intermediate-var]
+  // CHECK-MESSAGES: :[[@LINE-4]]:11: note: because they are only used when returning the result of this comparison
+  // CHECK-MESSAGES: :[[@LINE-9]]:8: note: consider removing both this variable declaration
+  // CHECK-MESSAGES: :[[@LINE-8]]:8: note: and this one
+  // CHECK-MESSAGES: :[[@LINE-7]]:11: note: and directly using the variable initialization expressions here
+}
+
+bool f3() {
+  auto test1 = 1; // Test1
+  // CHECK-FIXES: {{^}}  // Test1{{$}}
+  auto test2 = 2; // Test2
+  return (test1 == 2);
+  // CHECK-FIXES: {{^}}  return (1 == 2);{{$}}
+  // CHECK-MESSAGES: :[[@LINE-5]]:8: warning: intermediate variable 'test1' is useless [readability-useless-intermediate-var]
+  // CHECK-MESSAGES: :[[@LINE-3]]:11: note: because it is only used when returning the result of this comparison
+  // CHECK-MESSAGES: :[[@LINE-7]]:8: note: consider removing the variable declaration
+  // CHECK-MESSAGES: :[[@LINE-5]]:11: note: and directly using the variable initialization expression here
+}
+
+bool f4() {
+  auto test1 = 1; // Test1
+  auto test2 = 2; // Test2
+  // CHECK-FIXES: {{^}}  // Test2{{$}}
+  return (test2 == 3);
+  // CHECK-FIXES: {{^}}  return (2 == 3);{{$}}
+  // CHECK-MESSAGES: :[[@LINE-4]]:8: warning: intermediate variable 'test2' is useless [readability-useless-intermediate-var]
+  // CHECK-MESSAGES: :[[@LINE-3]]:11: note: because it is only used when returning the result of this comparison
+  // CHECK-MESSAGES: :[[@LINE-6]]:8: note: consider removing the variable declaration
+  // CHECK-MESSAGES: :[[@LINE-5]]:11: note: and directly using the variable initialization expression here
+}
+
+bool f5() {
+  auto test1 = 1; // Test1
+  // CHECK-FIXES: {{^}}  // Test1{{$}}
+  auto test2 = 2; // Test2
+  return (2 == test1);
+  // CHECK-FIXES: {{^}}  return (1 == 2);{{$}}
+  // CHECK-MESSAGES: :[[@LINE-5]]:8: warning: intermediate variable 'test1' is useless [readability-useless-intermediate-var]
+  // CHECK-MESSAGES: :[[@LINE-3]]:16: note: because it is only used when returning the result of this comparison
+  // CHECK-MESSAGES: :[[@LINE-7]]:8: note: consider removing the variable declaration
+  // CHECK-MESSAGES: :[[@LINE-5]]:11: note: and directly using the variable initialization expression here
+}
+
+bool f6() {
+  auto test1 = 1; // Test1
+  auto test2 = 2; // Test2
+  // CHECK-FIXES: {{^}}  // Test2{{$}}
+  return (3 == test2);
+  // CHECK-FIXES: {{^}}  return (2 == 3);{{$}}
+  // CHECK-MESSAGES: :[[@LINE-4]]:8: warning: intermediate variable 'test2' is useless [readability-useless-intermediate-var]
+  // CHECK-MESSAGES: :[[@LINE-3]]:16: note: because it is only used when